compare: move FreeBSD loose keyword comparisons to gomtree command

FreeBSD has quite unfortunate behaviour when dealing with keywords that
are missing in one of the manifests being compared -- namely, they
ignore these instances.

Commit 21723a3974 ("*: fix comparison of missing keywords") re-added
this behaviour after the introduction of the Compare API, but
unfortunately it was implemented in the Compare API itself -- meaning
that library users (which didn't want this behaviour) were silently
opted into it.

This patch moves the behaviour to the command-line, where it belongs
(a future patch in this series will allow users to opt-out of this
unfortunate behaviour, as well as some other unfortunate FreeBSD
compatibility behaviours).

Fixes: 21723a3974 ("*: fix comparison of missing keywords")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>

cmd/gomtree/cmd/validate.go

@@ -368,6 +368,7 @@ func validateAction(c *cli.Context) error {
 		if isTarSpec(specDh) || c.String("tar") != "" {
 			filters = append(filters, tarKeywordFilter)
 		}
+		filters = append(filters, freebsdCompatKeywordFilter)
 		res = filterDeltas(res, filters...)
 
 		if len(res) > 0 {
@@ -458,6 +459,25 @@ func tarKeywordFilter(delta *mtree.InodeDelta) bool {
 	return true
 }
 
+// freebsdCompatKeywordFilter removes any deltas where a key is not present in
+// both manifests being compared. This is necessary for compatibility with
+// FreeBSD's mtree(8) but is generally undesireable for most users.
+func freebsdCompatKeywordFilter(delta *mtree.InodeDelta) bool {
+	if delta.Type() != mtree.Modified {
+		return true
+	}
+	keys := delta.DiffPtr()
+	*keys = slices.DeleteFunc(*keys, func(kd mtree.KeyDelta) bool {
+		if kd.Name().Prefix() == "xattr" {
+			// Even in FreeBSD compatibility mode, any xattr changes should
+			// still be treated as a proper change and not filtered out.
+			return false
+		}
+		return kd.Type() != mtree.Modified
+	})
+	return true
+}
+
 type deltaFilterFn func(*mtree.InodeDelta) bool
 
 // filterDeltas takes the set of deltas generated by mtree and applies the

compare.go

@@ -4,7 +4,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"iter"
-	"maps"
 	"slices"
 
 	"github.com/sirupsen/logrus"
@@ -247,31 +246,6 @@ func compareEntry(oldEntry, newEntry Entry) ([]KeyDelta, error) {
 		newKeys = newEntry.allKeysMap()
 	)
 
-	// Delete any keys which are not present in both maps.
-	keyFilterFn := func(otherMap map[Keyword]KeyVal) func(Keyword, KeyVal) bool {
-		return func(k Keyword, _ KeyVal) bool {
-			switch {
-			case k.Prefix() == "xattr":
-				// xattrs are presented as one keyword to users but are actually
-				// implemented as separate keywords and so we should always include
-				// them (even if the same xattr is not present in both sides).
-				// TODO: I actually think this is not inline with the original
-				//       purpose of this code, but I'm leaving it as-is to not not
-				//       introduce bugs.
-				return false
-			case k == "time" || k == "tar_time":
-				// These have special handling later.
-				return false
-			default:
-				// Drop keys which do not exist in the other entry.
-				_, ok := otherMap[k]
-				return !ok
-			}
-		}
-	}
-	maps.DeleteFunc(oldKeys, keyFilterFn(newKeys))
-	maps.DeleteFunc(newKeys, keyFilterFn(oldKeys))
-
 	// If both tar_time and time were specified in the set of keys, we have to
 	// convert the "time" entries to "tar_time" to allow for tar archive
 	// manifests to be compared with proper filesystem manifests.

compare_test.go

@@ -7,6 +7,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"slices"
 	"testing"
 	"time"
 
@@ -230,6 +231,77 @@ func TestCompareKeySubset(t *testing.T) {
 	}
 }
 
+func TestCompareKeyDelta(t *testing.T) {
+	dir := t.TempDir()
+
+	// Create a bunch of objects.
+	tmpfile := filepath.Join(dir, "tmpfile")
+	require.NoError(t, os.WriteFile(tmpfile, []byte("some content here"), 0666))
+
+	tmpdir := filepath.Join(dir, "testdir")
+	require.NoError(t, os.Mkdir(tmpdir, 0755))
+
+	tmpsubfile := filepath.Join(tmpdir, "anotherfile")
+	require.NoError(t, os.WriteFile(tmpsubfile, []byte("aaa"), 0666))
+
+	// Walk the current state.
+	manifestKeywords := append(DefaultKeywords[:], "sha1digest")
+	old, err := Walk(dir, nil, manifestKeywords, nil)
+	require.NoErrorf(t, err, "walk %s", dir)
+
+	t.Run("Extra-Key", func(t *testing.T) {
+		extraKeyword := Keyword("sha256digest")
+		newManifestKeywords := append(manifestKeywords[:], extraKeyword)
+
+		new, err := Walk(dir, nil, newManifestKeywords, nil)
+		require.NoErrorf(t, err, "walk %s", dir)
+
+		diffs, err := Compare(old, new, nil)
+		require.NoError(t, err, "compare")
+
+		assert.NotEmpty(t, diffs, "extra keys in manifest should result in deltas")
+		for _, diff := range diffs {
+			if assert.Equal(t, Modified, diff.Type(), "extra keyword diff element should be 'modified'") {
+				kds := diff.Diff()
+				if assert.Len(t, kds, 1, "should only get a single key delta") {
+					kd := kds[0]
+					assert.Equalf(t, Extra, kd.Type(), "key %q", kd.Name())
+					assert.Equal(t, extraKeyword, kd.Name())
+					assert.Nil(t, kd.Old(), "Old for extra keyword delta")
+					assert.NotNil(t, kd.New(), "New for extra keyword delta")
+				}
+			}
+		}
+	})
+
+	t.Run("Missing-Key", func(t *testing.T) {
+		missingKeyword := Keyword("sha1digest")
+		newManifestKeywords := slices.DeleteFunc(manifestKeywords[:], func(kw Keyword) bool {
+			return kw == missingKeyword
+		})
+
+		new, err := Walk(dir, nil, newManifestKeywords, nil)
+		require.NoErrorf(t, err, "walk %s", dir)
+
+		diffs, err := Compare(old, new, nil)
+		require.NoError(t, err, "compare")
+
+		assert.NotEmpty(t, diffs, "missing keys in manifest should result in deltas")
+		for _, diff := range diffs {
+			if assert.Equal(t, Modified, diff.Type(), "missing keyword diff element should be 'modified'") {
+				kds := diff.Diff()
+				if assert.Len(t, kds, 1, "should only get a single key delta") {
+					kd := kds[0]
+					assert.Equalf(t, Missing, kd.Type(), "key %q", kd.Name())
+					assert.Equal(t, missingKeyword, kd.Name())
+					assert.NotNil(t, kd.Old(), "Old for missing keyword delta")
+					assert.Nil(t, kd.New(), "New for missing keyword delta")
+				}
+			}
+		}
+	})
+}
+
 //gocyclo:ignore
 func TestTarCompare(t *testing.T) {
 	dir := t.TempDir()