WIP: native ignore-mutation support in applier

e2e/testcases/ignore_mutation_test.go

@@ -24,15 +24,19 @@ import (
 	"github.com/GoogleContainerTools/config-sync/e2e/nomostest/taskgroup"
 	nomostesting "github.com/GoogleContainerTools/config-sync/e2e/nomostest/testing"
 	"github.com/GoogleContainerTools/config-sync/e2e/nomostest/testpredicates"
+	"github.com/GoogleContainerTools/config-sync/e2e/nomostest/testresourcegroup"
 	"github.com/GoogleContainerTools/config-sync/e2e/nomostest/testwatcher"
 	"github.com/GoogleContainerTools/config-sync/pkg/api/configsync"
+	"github.com/GoogleContainerTools/config-sync/pkg/api/kpt.dev/v1alpha1"
 	"github.com/GoogleContainerTools/config-sync/pkg/core"
 	"github.com/GoogleContainerTools/config-sync/pkg/core/k8sobjects"
 	"github.com/GoogleContainerTools/config-sync/pkg/kinds"
 	"github.com/GoogleContainerTools/config-sync/pkg/metadata"
 	"github.com/GoogleContainerTools/config-sync/pkg/reconcilermanager"
+	"github.com/GoogleContainerTools/config-sync/pkg/resourcegroup"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/types"
 	kstatus "sigs.k8s.io/cli-utils/pkg/kstatus/status"
 )
 
@@ -250,6 +254,10 @@ func TestMutationIgnoredObjectPruned(t *testing.T) {
 func TestAnnotationDrift(t *testing.T) {
 	nt := nomostest.New(t, nomostesting.DriftControl, ntopts.SyncWithGitSource(nomostest.DefaultRootSyncID, ntopts.Unstructured))
 	rootSyncGitRepo := nt.SyncSourceGitReadWriteRepository(nomostest.DefaultRootSyncID)
+	rgNN := types.NamespacedName{
+		Name:      nomostest.DefaultRootSyncID.Name,
+		Namespace: nomostest.DefaultRootSyncID.Namespace,
+	}
 
 	nt.T.Log("Adding namespace to Git")
 	namespace := k8sobjects.NamespaceObject("bookstore",
@@ -287,9 +295,77 @@ func TestAnnotationDrift(t *testing.T) {
 	nsObj2 := k8sobjects.NamespaceObject("new-ns")
 	nt.Must(rootSyncGitRepo.Add("acme/ns2.yaml", nsObj2))
 	nt.Must(rootSyncGitRepo.CommitAndPush("add another namespace"))
-	nt.Must(nt.WatchForAllSyncs())
-	secondCommitHash := rootSyncGitRepo.MustHash(nt.T)
+	nt.Must(nt.WatchForAllSyncs(nomostest.SkipAllResourceGroupChecks()))
+	// Explicitly verify ResourceGroup status. The mutation-ignored object should
+	// have "Skipped" actuation status
+	shortCommit := resourcegroup.TruncateSourceHash(rootSyncGitRepo.MustHash(nt.T))
+	expectedStatus := testresourcegroup.EmptyStatus()
+	expectedStatus.ObservedGeneration = 6
+	expectedStatus.ResourceStatuses = []v1alpha1.ResourceStatus{
+		{
+			ObjMetadata: v1alpha1.ObjMetadata{
+				Name: "safety-config-management-system-root-sync",
+				GroupKind: v1alpha1.GroupKind{
+					Kind: "Namespace",
+				},
+			},
+			Status:     v1alpha1.Current,
+			Strategy:   v1alpha1.Apply,
+			Actuation:  v1alpha1.ActuationSucceeded,
+			Reconcile:  v1alpha1.ReconcileSucceeded,
+			SourceHash: shortCommit,
+			Conditions: nil,
+		},
+		{
+			ObjMetadata: v1alpha1.ObjMetadata{
+				Name: "new-ns",
+				GroupKind: v1alpha1.GroupKind{
+					Kind: "Namespace",
+				},
+			},
+			Status:     v1alpha1.Current,
+			Strategy:   v1alpha1.Apply,
+			Actuation:  v1alpha1.ActuationSucceeded,
+			Reconcile:  v1alpha1.ReconcileSucceeded,
+			SourceHash: shortCommit,
+			Conditions: nil,
+		},
+		{
+			ObjMetadata: v1alpha1.ObjMetadata{
+				Name: "safety-config-management-system-root-sync",
+				GroupKind: v1alpha1.GroupKind{
+					Kind:  "ClusterRole",
+					Group: "rbac.authorization.k8s.io",
+				},
+			},
+			Status:     v1alpha1.Current,
+			Strategy:   v1alpha1.Apply,
+			Actuation:  v1alpha1.ActuationSucceeded,
+			Reconcile:  v1alpha1.ReconcileSucceeded,
+			SourceHash: shortCommit,
+			Conditions: nil,
+		},
+		{
+			ObjMetadata: v1alpha1.ObjMetadata{
+				Name: "bookstore",
+				GroupKind: v1alpha1.GroupKind{
+					Kind: "Namespace",
+				},
+			},
+			Status:     v1alpha1.Current,
+			Strategy:   v1alpha1.Apply,
+			Actuation:  v1alpha1.ActuationSkipped, // ActuationSkipped for ignore-mutation object
+			Reconcile:  v1alpha1.ReconcileSucceeded,
+			SourceHash: shortCommit,
+			Conditions: nil,
+		},
+	}
+	nt.Must(nt.Watcher.WatchObject(kinds.ResourceGroup(), rgNN.Name, rgNN.Namespace,
+		testwatcher.WatchPredicates(
+			testpredicates.ResourceGroupStatusEquals(expectedStatus),
+		)))
 
+	secondCommitHash := rootSyncGitRepo.MustHash(nt.T)
 	nt.Must(nt.Watcher.WatchObject(kinds.Namespace(), nsObj.Name, "",
 		testwatcher.WatchPredicates(
 			testpredicates.HasAnnotation(metadata.SyncTokenAnnotationKey, secondCommitHash),

go.mod

@@ -52,7 +52,7 @@ require (
 	k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b
 	k8s.io/kubectl v0.34.1
 	k8s.io/kubernetes v1.34.1
-	k8s.io/utils v0.0.0-20250604170112-4c0f3b243397
+	k8s.io/utils v0.0.0-20250820121507-0af2bda4dd1d
 	sigs.k8s.io/cli-utils v0.37.3-0.20250410211241-63a8e151c476
 	sigs.k8s.io/controller-runtime v0.22.3
 	sigs.k8s.io/controller-runtime/tools/setup-envtest v0.0.0-20231023142458-b9f29826ee83
@@ -64,6 +64,8 @@ require (
 	sigs.k8s.io/yaml v1.6.0
 )
 
+replace sigs.k8s.io/cli-utils => github.com/sdowell/cli-utils v0.26.1-0.20251013220248-e8062eb527c2
+
 require (
 	al.essio.dev/pkg/shellescape v1.5.1 // indirect
 	cloud.google.com/go v0.120.0 // indirect
@@ -127,7 +129,7 @@ require (
 	github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
 	github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f // indirect
-	github.com/onsi/gomega v1.38.0 // indirect
+	github.com/onsi/gomega v1.38.2 // indirect
 	github.com/opencontainers/go-digest v1.0.0 // indirect
 	github.com/opencontainers/image-spec v1.1.1 // indirect
 	github.com/otiai10/copy v1.14.0 // indirect

go.sum

@@ -321,10 +321,10 @@ github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE=
 github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU=
 github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE=
 github.com/onsi/ginkgo v1.16.5/go.mod h1:+E8gABHa3K6zRBolWtd+ROzc/U5bkGt0FwiG042wbpU=
-github.com/onsi/ginkgo/v2 v2.23.4 h1:ktYTpKJAVZnDT4VjxSbiBenUjmlL/5QkBEocaWXiQus=
-github.com/onsi/ginkgo/v2 v2.23.4/go.mod h1:Bt66ApGPBFzHyR+JO10Zbt0Gsp4uWxu5mIOTusL46e8=
-github.com/onsi/gomega v1.38.0 h1:c/WX+w8SLAinvuKKQFh77WEucCnPk4j2OTUr7lt7BeY=
-github.com/onsi/gomega v1.38.0/go.mod h1:OcXcwId0b9QsE7Y49u+BTrL4IdKOBOKnD6VQNTJEB6o=
+github.com/onsi/ginkgo/v2 v2.25.2 h1:hepmgwx1D+llZleKQDMEvy8vIlCxMGt7W5ZxDjIEhsw=
+github.com/onsi/ginkgo/v2 v2.25.2/go.mod h1:43uiyQC4Ed2tkOzLsEYm7hnrb7UJTWHYNsuy3bG/snE=
+github.com/onsi/gomega v1.38.2 h1:eZCjf2xjZAqe+LeWvKb5weQ+NcPwX84kqJ0cZNxok2A=
+github.com/onsi/gomega v1.38.2/go.mod h1:W2MJcYxRGV63b418Ai34Ud0hEdTVXq9NW9+Sx6uXf3k=
 github.com/open-policy-agent/cert-controller v0.14.0 h1:TPc19BOHOs4tARruTT5o4bzir7Ed6FF+j3EXP/nmZBs=
 github.com/open-policy-agent/cert-controller v0.14.0/go.mod h1:UhE/FU54DnKo+Rt0Yf3r+oKjgy6kqSH8Vsjo+5bGrSo=
 github.com/open-policy-agent/frameworks/constraint v0.0.0-20241101234656-e78c8abd754a h1:gQtOJ50XFyL2Xh3lDD9zP4KQ2PY4mZKQ9hDcWc81Sp8=
@@ -363,6 +363,8 @@ github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR
 github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o=
 github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk=
 github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
+github.com/sdowell/cli-utils v0.26.1-0.20251013220248-e8062eb527c2 h1:H8lhDTwAR8Bp2gsnTMaMz/fSjoH3p2I1yTrC5qMojvY=
+github.com/sdowell/cli-utils v0.26.1-0.20251013220248-e8062eb527c2/go.mod h1:u5LTcoijf7f18rMNL7PVNyJzoGEriT+tS57ZSVG3nc4=
 github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM=
 github.com/sergi/go-diff v1.2.0 h1:XU+rvMAioB0UC3q1MFrIQy4Vo5/4VsRDQQXHsEya6xQ=
 github.com/sergi/go-diff v1.2.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM=
@@ -741,13 +743,11 @@ k8s.io/kubectl v0.34.1 h1:1qP1oqT5Xc93K+H8J7ecpBjaz511gan89KO9Vbsh/OI=
 k8s.io/kubectl v0.34.1/go.mod h1:JRYlhJpGPyk3dEmJ+BuBiOB9/dAvnrALJEiY/C5qa6A=
 k8s.io/kubernetes v1.34.1 h1:F3p8dtpv+i8zQoebZeK5zBqM1g9x1aIdnA5vthvcuUk=
 k8s.io/kubernetes v1.34.1/go.mod h1:iu+FhII+Oc/1gGWLJcer6wpyih441aNFHl7Pvm8yPto=
-k8s.io/utils v0.0.0-20250604170112-4c0f3b243397 h1:hwvWFiBzdWw1FhfY1FooPn3kzWuJ8tmbZBHi4zVsl1Y=
-k8s.io/utils v0.0.0-20250604170112-4c0f3b243397/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
+k8s.io/utils v0.0.0-20250820121507-0af2bda4dd1d h1:wAhiDyZ4Tdtt7e46e9M5ZSAJ/MnPGPs+Ki1gHw4w1R0=
+k8s.io/utils v0.0.0-20250820121507-0af2bda4dd1d/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
 rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=
 rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0=
 rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=
-sigs.k8s.io/cli-utils v0.37.3-0.20250410211241-63a8e151c476 h1:HjKF4Xfsh702Qx9J0Uu8QDywHA6aIKVoYFXIDTAEXZI=
-sigs.k8s.io/cli-utils v0.37.3-0.20250410211241-63a8e151c476/go.mod h1:bM9dkBKOU5vmCHVg5yr3Jf6ooy4S0giTbeaskiHkZfQ=
 sigs.k8s.io/controller-runtime v0.22.3 h1:I7mfqz/a/WdmDCEnXmSPm8/b/yRTy6JsKKENTijTq8Y=
 sigs.k8s.io/controller-runtime v0.22.3/go.mod h1:+QX1XUpTXN4mLoblf4tqr5CQcyHPAki2HLXqQMY6vh8=
 sigs.k8s.io/controller-runtime/tools/setup-envtest v0.0.0-20231023142458-b9f29826ee83 h1:37G7lMdeXe0kogUnwCa1K+EIVYR5f4BqM0bqITyaDBI=

pkg/applier/applier.go

@@ -207,7 +207,7 @@ func (s *supervisor) UpdateStatusMode(ctx context.Context) error {
 	})
 }
 
-func (s *supervisor) processApplyEvent(ctx context.Context, e event.ApplyEvent, syncStats *stats.ApplyEventStats, objectStatusMap ObjectStatusMap, unknownTypeResources map[core.ID]struct{}, resourceMap map[core.ID]client.Object, declaredResources *declared.Resources) status.Error {
+func (s *supervisor) processApplyEvent(ctx context.Context, e event.ApplyEvent, syncStats *stats.ApplyEventStats, objectStatusMap ObjectStatusMap, unknownTypeResources map[core.ID]struct{}, resourceMap map[core.ID]client.Object) status.Error {
 	id := idFrom(e.Identifier)
 	syncStats.Add(e.Status)
 
@@ -231,15 +231,6 @@ func (s *supervisor) processApplyEvent(ctx context.Context, e event.ApplyEvent,
 	case event.ApplyFailed:
 		objectStatus.Actuation = actuation.ActuationFailed
 		handleMetrics(ctx, "update", e.Error)
-		// If apply failed for an ignore-mutation object, delete it from the ignore cache.
-		// Normally the cached object should be updated by the remediator when it
-		// receives a watch event - This is a fallback to force a live lookup the
-		// next time the applier runs.
-		iObj, found := declaredResources.GetIgnored(id)
-		if found {
-			klog.Infof("Deleting object '%v' from the ignore cache (apply failed)", core.GKNN(iObj))
-			declaredResources.DeleteIgnored(id)
-		}
 		switch e.Error.(type) {
 		case *applyerror.UnknownTypeError:
 			unknownTypeResources[id] = struct{}{}
@@ -317,11 +308,52 @@ func (s *supervisor) handleApplySkippedEvent(obj *unstructured.Unstructured, id
 		return KptManagementConflictError(obj)
 	}
 
+	var annotationErr *filter.AnnotationPreventedUpdateError
+	if errors.As(err, &annotationErr) {
+		// For applies this is desired behavior, not unexpected. The following logic
+		// re-applies just the CS metadata to ensure metadata does not drift.
+		klog.Info("Got AnnotationPreventedUpdateError")
+		if err := s.updateObjectMetadata(context.TODO(), obj); err != nil {
+			return SkipErrorForResource(
+				fmt.Errorf("updating Config Sync metadata for ignore mutation object: %w", err),
+				id, actuation.ActuationStrategyApply)
+		}
+		return nil
+	}
+
 	return SkipErrorForResource(err, id, actuation.ActuationStrategyApply)
 }
 
+func (s *supervisor) updateObjectMetadata(ctx context.Context, obj *unstructured.Unstructured) error {
+	// Using PartialObjectMetadata optimizes the client calls (uses metadata client under the hood)
+	metaObj := &metav1.PartialObjectMetadata{}
+	metaObj.Name = obj.GetName()
+	metaObj.Namespace = obj.GetNamespace()
+	metaObj.SetGroupVersionKind(obj.GroupVersionKind())
+	key := client.ObjectKey{
+		Name:      obj.GetName(),
+		Namespace: obj.GetNamespace(),
+	}
+	if err := s.clientSet.Client.Get(ctx, key, metaObj); err != nil {
+		return err
+	}
+
+	existing := metaObj.DeepCopy()
+	if metadata.UpdateConfigSyncMetadata(obj, metaObj) {
+		if err := s.clientSet.Client.Patch(ctx, metaObj, client.MergeFrom(existing),
+			client.FieldOwner(configsync.FieldManager)); err != nil {
+			return err
+		}
+		klog.Infof("Patched drifted CS metadata on %s", key.String())
+		return nil
+	}
+	klog.Infof("Skip patching CS metadata on %s", key.String())
+	return nil
+
+}
+
 // processPruneEvent handles PruneEvents from the Applier
-func (s *supervisor) processPruneEvent(ctx context.Context, e event.PruneEvent, syncStats *stats.PruneEventStats, objectStatusMap ObjectStatusMap, declaredResources *declared.Resources) status.Error {
+func (s *supervisor) processPruneEvent(ctx context.Context, e event.PruneEvent, syncStats *stats.PruneEventStats, objectStatusMap ObjectStatusMap) status.Error {
 	id := idFrom(e.Identifier)
 	syncStats.Add(e.Status)
 
@@ -340,13 +372,6 @@ func (s *supervisor) processPruneEvent(ctx context.Context, e event.PruneEvent,
 	case event.PruneSuccessful:
 		objectStatus.Actuation = actuation.ActuationSucceeded
 		handleMetrics(ctx, "delete", e.Error)
-
-		iObj, found := declaredResources.GetIgnored(id)
-		if found {
-			klog.V(3).Infof("Deleting object '%v' from the ignore cache", core.GKNN(iObj))
-			declaredResources.DeleteIgnored(id)
-		}
-
 		return nil
 
 	case event.PruneFailed:
@@ -510,15 +535,6 @@ func (s *supervisor) applyInner(ctx context.Context, eventHandler func(Event), d
 	objStatusMap := make(ObjectStatusMap)
 	objs := declaredResources.DeclaredObjects()
 
-	if err := s.cacheIgnoreMutationObjects(ctx, declaredResources); err != nil {
-		sendErrorEvent(err, eventHandler)
-		return objStatusMap, syncStats
-	}
-
-	if len(declaredResources.IgnoredObjects()) > 0 {
-		klog.Infof("%v mutation-ignored objects: %v", len(declaredResources.IgnoredObjects()), core.GKNNs(declaredResources.IgnoredObjects()))
-	}
-
 	// disabledObjs are objects for which the management are disabled
 	// through annotation.
 	enabledObjs, disabledObjs := partitionObjs(objs)
@@ -535,10 +551,8 @@ func (s *supervisor) applyInner(ctx context.Context, eventHandler func(Event), d
 		}
 	}
 
-	objsToApply := handleIgnoredObjects(enabledObjs, declaredResources)
-
-	klog.Infof("%v objects to be applied: %v", len(objsToApply), core.GKNNs(objsToApply))
-	resources, err := toUnstructured(objsToApply)
+	klog.Infof("%v objects to be applied: %v", len(enabledObjs), core.GKNNs(enabledObjs))
+	resources, err := toUnstructured(enabledObjs)
 	if err != nil {
 		sendErrorEvent(err, eventHandler)
 		return objStatusMap, syncStats
@@ -613,7 +627,7 @@ func (s *supervisor) applyInner(ctx context.Context, eventHandler func(Event), d
 			} else {
 				klog.V(1).Info(e.ApplyEvent)
 			}
-			if err := s.processApplyEvent(ctx, e.ApplyEvent, syncStats.ApplyEvent, objStatusMap, unknownTypeResources, resourceMap, declaredResources); err != nil {
+			if err := s.processApplyEvent(ctx, e.ApplyEvent, syncStats.ApplyEvent, objStatusMap, unknownTypeResources, resourceMap); err != nil {
 				sendErrorEvent(err, eventHandler)
 			}
 		case event.PruneType:
@@ -622,7 +636,7 @@ func (s *supervisor) applyInner(ctx context.Context, eventHandler func(Event), d
 			} else {
 				klog.V(1).Info(e.PruneEvent)
 			}
-			if err := s.processPruneEvent(ctx, e.PruneEvent, syncStats.PruneEvent, objStatusMap, declaredResources); err != nil {
+			if err := s.processPruneEvent(ctx, e.PruneEvent, syncStats.PruneEvent, objStatusMap); err != nil {
 				sendErrorEvent(err, eventHandler)
 			}
 		default:
@@ -836,37 +850,3 @@ func (s *supervisor) abandonObject(ctx context.Context, obj client.Object) error
 	}
 	return nil
 }
-
-// cacheIgnoreMutationObjects gets the current cluster state of any declared objects with the ignore mutation annotation and puts it in the Resources ignore objects cache
-// Returns any errors that occur
-func (s *supervisor) cacheIgnoreMutationObjects(ctx context.Context, declaredResources *declared.Resources) error {
-	var objsToUpdate []client.Object
-	declaredObjs := declaredResources.DeclaredObjects()
-
-	for _, obj := range declaredObjs {
-		if obj.GetAnnotations()[metadata.LifecycleMutationAnnotation] == metadata.IgnoreMutation {
-
-			if _, found := declaredResources.GetIgnored(core.IDOf(obj)); !found {
-				// Fetch the cluster state of the object if not already in the cache
-				uObj := &unstructured.Unstructured{}
-				uObj.SetGroupVersionKind(obj.GetObjectKind().GroupVersionKind())
-				err := s.clientSet.Client.Get(ctx, client.ObjectKeyFromObject(obj), uObj)
-
-				// Object doesn't exist on the cluster
-				if apierrors.IsNotFound(err) {
-					continue
-				}
-
-				if err != nil {
-					return err
-				}
-
-				objsToUpdate = append(objsToUpdate, uObj)
-			}
-		}
-	}
-
-	declaredResources.UpdateIgnored(objsToUpdate...)
-
-	return nil
-}