feat: add FieldIgnore functionality

cmd/main.go

@@ -30,6 +30,7 @@ import (
 	"github.com/istio-ecosystem/sail-operator/controllers/ztunnel"
 	"github.com/istio-ecosystem/sail-operator/pkg/config"
 	"github.com/istio-ecosystem/sail-operator/pkg/enqueuelogger"
+	"github.com/istio-ecosystem/sail-operator/pkg/fieldignore"
 	"github.com/istio-ecosystem/sail-operator/pkg/helm"
 	"github.com/istio-ecosystem/sail-operator/pkg/scheme"
 	"github.com/istio-ecosystem/sail-operator/pkg/version"
@@ -202,7 +203,8 @@ func main() {
 		os.Exit(1)
 	}
 
-	chartManager := helm.NewChartManager(mgr.GetConfig(), os.Getenv("HELM_DRIVER"))
+	chartManager := helm.NewChartManager(mgr.GetConfig(), os.Getenv("HELM_DRIVER"),
+		helm.WithFieldIgnoreRules(fieldignore.DefaultIgnoreRules))
 
 	err = istio.NewReconciler(reconcilerCfg, mgr.GetClient(), mgr.GetScheme(), tlsConfig).
 		SetupWithManager(mgr)

controllers/istiocni/istiocni_controller.go

@@ -25,6 +25,7 @@ import (
 	"github.com/istio-ecosystem/sail-operator/pkg/constants"
 	"github.com/istio-ecosystem/sail-operator/pkg/enqueuelogger"
 	"github.com/istio-ecosystem/sail-operator/pkg/errlist"
+	"github.com/istio-ecosystem/sail-operator/pkg/fieldignore"
 	"github.com/istio-ecosystem/sail-operator/pkg/helm"
 	"github.com/istio-ecosystem/sail-operator/pkg/predicate"
 	sharedreconcile "github.com/istio-ecosystem/sail-operator/pkg/reconcile"
@@ -167,11 +168,10 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 
 		// +lint-watches:ignore: NetworkPolicy (FIXME: NetworkPolicy has not yet been added upstream, but is WIP)
 		Watches(&networkingv1.NetworkPolicy{}, ownedResourceHandler, builder.WithPredicates(predicate.IgnoreUpdate())).
-
-		// We use predicate.IgnoreUpdate() so that we skip the reconciliation when a pull secret is added to the ServiceAccount.
-		// This is necessary so that we don't remove the newly-added secret.
-		// TODO: this is a temporary hack until we implement the correct solution on the Helm-render side
-		Watches(&corev1.ServiceAccount{}, ownedResourceHandler, builder.WithPredicates(predicate.IgnoreUpdate())).
+		Watches(&corev1.ServiceAccount{}, ownedResourceHandler,
+			builder.WithPredicates(
+				fieldignore.ServiceAccountIgnoreRules.NewPredicate(),
+				predicate.IgnoreUpdateWhenAnnotation())).
 
 		// TODO: only register NetAttachDef if the CRD is installed (may also need to watch for CRD creation)
 		// Owns(&multusv1.NetworkAttachmentDefinition{}).

controllers/istiorevision/istiorevision_controller.go

@@ -26,6 +26,7 @@ import (
 	"github.com/istio-ecosystem/sail-operator/pkg/constants"
 	"github.com/istio-ecosystem/sail-operator/pkg/enqueuelogger"
 	"github.com/istio-ecosystem/sail-operator/pkg/errlist"
+	"github.com/istio-ecosystem/sail-operator/pkg/fieldignore"
 	"github.com/istio-ecosystem/sail-operator/pkg/helm"
 	predicate2 "github.com/istio-ecosystem/sail-operator/pkg/predicate"
 	sharedreconcile "github.com/istio-ecosystem/sail-operator/pkg/reconcile"
@@ -286,11 +287,8 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 		// +lint-watches:ignore: NetworkPolicy (FIXME: NetworkPolicy has not yet been added upstream, but is WIP)
 		Watches(&networkingv1.NetworkPolicy{}, ownedResourceHandler,
 			builder.WithPredicates(ignoreStatusChange(), predicate2.IgnoreUpdateWhenAnnotation())).
-
-		// We use predicate.IgnoreUpdate() so that we skip the reconciliation when a pull secret is added to the ServiceAccount.
-		// This is necessary so that we don't remove the newly-added secret.
-		// TODO: this is a temporary hack until we implement the correct solution on the Helm-render side
-		Watches(&corev1.ServiceAccount{}, ownedResourceHandler, builder.WithPredicates(predicate2.IgnoreUpdate())).
+		Watches(&corev1.ServiceAccount{}, ownedResourceHandler,
+			builder.WithPredicates(fieldignore.ServiceAccountIgnoreRules.NewPredicate(), predicate2.IgnoreUpdateWhenAnnotation())).
 		Watches(&rbacv1.Role{}, ownedResourceHandler, builder.WithPredicates(predicate2.IgnoreUpdateWhenAnnotation())).
 		Watches(&rbacv1.RoleBinding{}, ownedResourceHandler, builder.WithPredicates(predicate2.IgnoreUpdateWhenAnnotation())).
 		Watches(&policyv1.PodDisruptionBudget{}, ownedResourceHandler,
@@ -311,9 +309,13 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Watches(&rbacv1.ClusterRole{}, ownedResourceHandler, builder.WithPredicates(predicate2.IgnoreUpdateWhenAnnotation())).
 		Watches(&rbacv1.ClusterRoleBinding{}, ownedResourceHandler, builder.WithPredicates(predicate2.IgnoreUpdateWhenAnnotation())).
 		Watches(&admissionv1.MutatingWebhookConfiguration{}, ownedResourceHandler,
-			builder.WithPredicates(webhookConfigPredicate(), predicate2.IgnoreUpdateWhenAnnotation())).
+			builder.WithPredicates(
+				fieldignore.MutatingWebhookIgnoreRules.NewPredicate(),
+				predicate2.IgnoreUpdateWhenAnnotation())).
 		Watches(&admissionv1.ValidatingWebhookConfiguration{}, ownedResourceHandler,
-			builder.WithPredicates(webhookConfigPredicate(), predicate2.IgnoreUpdateWhenAnnotation())).
+			builder.WithPredicates(
+				fieldignore.ValidatingWebhookIgnoreRules.NewPredicate(),
+				predicate2.IgnoreUpdateWhenAnnotation())).
 
 		// +lint-watches:ignore: IstioCNI (not found in charts, but this controller needs to watch it to update the IstioRevision status)
 		Watches(&v1.IstioCNI{}, istioCniHandler).
@@ -725,42 +727,6 @@ func specWasUpdated(oldObject client.Object, newObject client.Object) bool {
 	return oldObject.GetGeneration() != newObject.GetGeneration()
 }
 
-func webhookConfigPredicate() predicate.Funcs {
-	return predicate.Funcs{
-		UpdateFunc: func(e event.TypedUpdateEvent[client.Object]) bool {
-			if e.ObjectOld == nil || e.ObjectNew == nil {
-				return false
-			}
-
-			// Istiod updates the caBundle and failurePolicy fields in its webhook configs.
-			// We must ignore changes to these fields to prevent an endless update loop.
-			// We must use deep copies to avoid mutating the shared informer cache.
-			oldCopy := e.ObjectOld.DeepCopyObject().(client.Object)
-			newCopy := e.ObjectNew.DeepCopyObject().(client.Object)
-			clearIgnoredFields(oldCopy)
-			clearIgnoredFields(newCopy)
-			return !reflect.DeepEqual(newCopy, oldCopy)
-		},
-	}
-}
-
-func clearIgnoredFields(obj client.Object) {
-	obj.SetResourceVersion("")
-	obj.SetGeneration(0)
-	obj.SetManagedFields(nil)
-	switch webhookConfig := obj.(type) {
-	case *admissionv1.ValidatingWebhookConfiguration:
-		for i := range len(webhookConfig.Webhooks) {
-			webhookConfig.Webhooks[i].FailurePolicy = nil
-			webhookConfig.Webhooks[i].ClientConfig.CABundle = nil
-		}
-	case *admissionv1.MutatingWebhookConfiguration:
-		for i := range len(webhookConfig.Webhooks) {
-			webhookConfig.Webhooks[i].ClientConfig.CABundle = nil
-		}
-	}
-}
-
 func wrapEventHandler(logger logr.Logger, handler handler.EventHandler) handler.EventHandler {
 	return enqueuelogger.WrapIfNecessary(v1.IstioRevisionKind, logger, handler)
 }

controllers/istiorevisiontag/istiorevisiontag_controller.go

@@ -27,6 +27,7 @@ import (
 	"github.com/istio-ecosystem/sail-operator/pkg/constants"
 	"github.com/istio-ecosystem/sail-operator/pkg/enqueuelogger"
 	"github.com/istio-ecosystem/sail-operator/pkg/errlist"
+	"github.com/istio-ecosystem/sail-operator/pkg/fieldignore"
 	"github.com/istio-ecosystem/sail-operator/pkg/helm"
 	"github.com/istio-ecosystem/sail-operator/pkg/reconciler"
 	"github.com/istio-ecosystem/sail-operator/pkg/revision"
@@ -251,8 +252,9 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 		// cluster-scoped resources
 		Watches(&v1.Istio{}, operatorResourcesHandler).
 		Watches(&v1.IstioRevision{}, operatorResourcesHandler).
-		Watches(&admissionv1.MutatingWebhookConfiguration{}, ownedResourceHandler).
-		Complete(reconciler.NewStandardReconcilerWithFinalizer[*v1.IstioRevisionTag](r.Client, r.Reconcile, r.Finalize, constants.FinalizerName))
+		Watches(&admissionv1.MutatingWebhookConfiguration{}, ownedResourceHandler,
+			builder.WithPredicates(fieldignore.ValidatingWebhookIgnoreRules.NewPredicate())).
+		Complete(reconciler.NewStandardReconcilerWithFinalizer(r.Client, r.Reconcile, r.Finalize, constants.FinalizerName))
 }
 
 func (r *Reconciler) determineStatus(ctx context.Context, tag *v1.IstioRevisionTag,

controllers/webhook/webhook_controller.go

@@ -30,6 +30,7 @@ import (
 	"github.com/istio-ecosystem/sail-operator/pkg/config"
 	"github.com/istio-ecosystem/sail-operator/pkg/constants"
 	"github.com/istio-ecosystem/sail-operator/pkg/enqueuelogger"
+	"github.com/istio-ecosystem/sail-operator/pkg/fieldignore"
 	"github.com/istio-ecosystem/sail-operator/pkg/reconciler"
 	"github.com/istio-ecosystem/sail-operator/pkg/revision"
 	admissionv1 "k8s.io/api/admissionregistration/v1"
@@ -189,7 +190,10 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 
 		// we use the Watches function instead of For(), so that we can wrap the handler so that events that cause the object to be enqueued are logged
 		// +lint-watches:ignore: IstioRevision (not found in charts, but this is the main resource watched by this controller)
-		Watches(&admissionv1.MutatingWebhookConfiguration{}, objectHandler, builder.WithPredicates(ownedByRemoteIstioRevisionPredicate(mgr.GetClient()))).
+		Watches(&admissionv1.MutatingWebhookConfiguration{}, objectHandler,
+			builder.WithPredicates(
+				fieldignore.MutatingWebhookIgnoreRules.NewPredicate(),
+				ownedByRemoteIstioRevisionPredicate(mgr.GetClient()))).
 		Named("mutatingwebhookconfiguration").
 		Complete(reconciler.NewStandardReconciler[*admissionv1.MutatingWebhookConfiguration](r.Client, r.Reconcile))
 }

controllers/ztunnel/ztunnel_controller.go

@@ -26,6 +26,7 @@ import (
 	"github.com/istio-ecosystem/sail-operator/pkg/constants"
 	"github.com/istio-ecosystem/sail-operator/pkg/enqueuelogger"
 	"github.com/istio-ecosystem/sail-operator/pkg/errlist"
+	"github.com/istio-ecosystem/sail-operator/pkg/fieldignore"
 	"github.com/istio-ecosystem/sail-operator/pkg/helm"
 	"github.com/istio-ecosystem/sail-operator/pkg/predicate"
 	sharedreconcile "github.com/istio-ecosystem/sail-operator/pkg/reconcile"
@@ -187,11 +188,8 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Watches(&corev1.ConfigMap{}, ownedResourceHandler).
 		Watches(&appsv1.DaemonSet{}, ownedResourceHandler).
 		Watches(&corev1.ResourceQuota{}, ownedResourceHandler).
-
-		// We use predicate.IgnoreUpdate() so that we skip the reconciliation when a pull secret is added to the ServiceAccount.
-		// This is necessary so that we don't remove the newly-added secret.
-		// TODO: this is a temporary hack until we implement the correct solution on the Helm-render side
-		Watches(&corev1.ServiceAccount{}, ownedResourceHandler, builder.WithPredicates(predicate.IgnoreUpdate())).
+		Watches(&corev1.ServiceAccount{}, ownedResourceHandler,
+			builder.WithPredicates(fieldignore.ServiceAccountIgnoreRules.NewPredicate(), predicate.IgnoreUpdateWhenAnnotation())).
 
 		// cluster-scoped resources
 		// +lint-watches:ignore: Namespace (not present in charts, but must be watched to reconcile ZTunnel when its namespace is created)

pkg/fieldignore/defaults.go

@@ -0,0 +1,67 @@
+// Copyright Istio Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package fieldignore
+
+import (
+	"slices"
+
+	admissionv1 "k8s.io/api/admissionregistration/v1"
+	corev1 "k8s.io/api/core/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+// DefaultRules defines the default set of fields to ignore on resources managed
+// by the operator. These prevent reconciliation loops caused by other
+// controllers (e.g. istiod, pull-secret injectors, Azure admission enforcer)
+// updating fields that the operator also manages via Helm.
+var DefaultIgnoreRules = slices.Concat(
+	convertToGenericRules(ValidatingWebhookIgnoreRules),
+	convertToGenericRules(MutatingWebhookIgnoreRules),
+	convertToGenericRules(ServiceAccountIgnoreRules),
+)
+
+// convertToGenericRules converts a slice of a specific impl and returns a generic slice.
+func convertToGenericRules[T client.Object](rules RuleSet[T]) RuleSet[client.Object] {
+	var rs RuleSet[client.Object]
+	for _, rule := range rules {
+		if rule.Name != "" {
+			rs = append(rs, NewFieldIgnoreRuleWithName[client.Object](rule.obj, rule.Name, rule.Fields, rule.Scope))
+		} else {
+			rs = append(rs, NewFieldIgnoreRule[client.Object](rule.obj, rule.Fields, rule.Scope))
+		}
+	}
+	return rs
+}
+
+var ValidatingWebhookIgnoreRules = RuleSet[*admissionv1.ValidatingWebhookConfiguration]{
+	NewFieldIgnoreRule(&admissionv1.ValidatingWebhookConfiguration{}, []string{"webhooks[*].failurePolicy"}, IgnoreScopeReconcileAndUpgrade),
+	NewFieldIgnoreRule(&admissionv1.ValidatingWebhookConfiguration{}, []string{"webhooks[*].clientConfig.caBundle"}, IgnoreScopeAlways),
+}
+
+var MutatingWebhookIgnoreRules = RuleSet[*admissionv1.MutatingWebhookConfiguration]{
+	NewFieldIgnoreRule(&admissionv1.MutatingWebhookConfiguration{}, []string{"webhooks[*].clientConfig.caBundle"}, IgnoreScopeAlways),
+	// AKS manipulates MutatingWebhookConfigurations. See https://github.com/istio-ecosystem/sail-operator/issues/1148
+	NewFieldIgnoreRule(
+		&admissionv1.MutatingWebhookConfiguration{},
+		[]string{"webhooks[*].namespaceSelector.matchExpressions[key=kubernetes.azure.com/managedby]"},
+		IgnoreScopeReconcile,
+	),
+}
+
+var ServiceAccountIgnoreRules = RuleSet[*corev1.ServiceAccount]{
+	NewFieldIgnoreRule(&corev1.ServiceAccount{}, []string{"imagePullSecrets"}, IgnoreScopeAlways),
+	NewFieldIgnoreRule(&corev1.ServiceAccount{}, []string{"automountServiceAccountToken"}, IgnoreScopeAlways),
+	NewFieldIgnoreRule(&corev1.ServiceAccount{}, []string{"secrets"}, IgnoreScopeAlways),
+}