Skip to content

Commit e24ecb7

Browse files
committed
feat: add FieldIgnore functionality
this is a cleaner API around ignoring certain fields in watches and when installing charts, through a PostRender func. The goal is to eventually expose this API to the user, but for now it'll just help to keep track of our default ignores that we have implemented to fix specific bugs. Signed-off-by: Daniel Grimm <dgrimm@redhat.com>
1 parent 4d758e0 commit e24ecb7

7 files changed

Lines changed: 978 additions & 115 deletions

File tree

cmd/main.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,8 @@ func main() {
160160
os.Exit(1)
161161
}
162162

163-
chartManager := helm.NewChartManager(mgr.GetConfig(), os.Getenv("HELM_DRIVER"))
163+
chartManager := helm.NewChartManager(mgr.GetConfig(), os.Getenv("HELM_DRIVER"),
164+
helm.WithFieldIgnoreRules(istiorevision.DefaultFieldIgnoreRules))
164165

165166
reconcilerCfg.Platform, err = config.DetectPlatform(mgr.GetConfig())
166167
if err != nil {

controllers/istiorevision/istiorevision_controller.go

Lines changed: 30 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/istio-ecosystem/sail-operator/pkg/constants"
2727
"github.com/istio-ecosystem/sail-operator/pkg/enqueuelogger"
2828
"github.com/istio-ecosystem/sail-operator/pkg/errlist"
29+
"github.com/istio-ecosystem/sail-operator/pkg/fieldignore"
2930
"github.com/istio-ecosystem/sail-operator/pkg/helm"
3031
"github.com/istio-ecosystem/sail-operator/pkg/kube"
3132
predicate2 "github.com/istio-ecosystem/sail-operator/pkg/predicate"
@@ -63,6 +64,33 @@ const (
6364
ztunnelName = "default"
6465
)
6566

67+
// DefaultFieldIgnoreRules defines the default set of fields to ignore on
68+
// resources managed by the operator. These prevent reconciliation loops caused
69+
// by other controllers (e.g. istiod) updating fields that the operator also
70+
// manages via Helm.
71+
var DefaultFieldIgnoreRules = fieldignore.MustCompileRules([]fieldignore.FieldIgnoreRule{
72+
{
73+
Group: "admissionregistration.k8s.io",
74+
Version: "v1",
75+
Kind: "ValidatingWebhookConfiguration",
76+
NameRegex: "istiod-.*-validator|istio-validator.*",
77+
Fields: []string{"webhooks[*].failurePolicy"},
78+
OnlyOnUpdate: true,
79+
},
80+
{
81+
Group: "admissionregistration.k8s.io",
82+
Version: "v1",
83+
Kind: "ValidatingWebhookConfiguration",
84+
Fields: []string{"webhooks[*].clientConfig.caBundle"},
85+
},
86+
{
87+
Group: "admissionregistration.k8s.io",
88+
Version: "v1",
89+
Kind: "MutatingWebhookConfiguration",
90+
Fields: []string{"webhooks[*].clientConfig.caBundle"},
91+
},
92+
})
93+
6694
// Reconciler reconciles an IstioRevision object
6795
type Reconciler struct {
6896
client.Client
@@ -312,9 +340,9 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
312340
Watches(&rbacv1.ClusterRole{}, ownedResourceHandler, builder.WithPredicates(predicate2.IgnoreUpdateWhenAnnotation())).
313341
Watches(&rbacv1.ClusterRoleBinding{}, ownedResourceHandler, builder.WithPredicates(predicate2.IgnoreUpdateWhenAnnotation())).
314342
Watches(&admissionv1.MutatingWebhookConfiguration{}, ownedResourceHandler,
315-
builder.WithPredicates(webhookConfigPredicate(), predicate2.IgnoreUpdateWhenAnnotation())).
343+
builder.WithPredicates(fieldignore.NewPredicate(r.Scheme, DefaultFieldIgnoreRules), predicate2.IgnoreUpdateWhenAnnotation())).
316344
Watches(&admissionv1.ValidatingWebhookConfiguration{}, ownedResourceHandler,
317-
builder.WithPredicates(webhookConfigPredicate(), predicate2.IgnoreUpdateWhenAnnotation())).
345+
builder.WithPredicates(fieldignore.NewPredicate(r.Scheme, DefaultFieldIgnoreRules), predicate2.IgnoreUpdateWhenAnnotation())).
318346

319347
// +lint-watches:ignore: IstioCNI (not found in charts, but this controller needs to watch it to update the IstioRevision status)
320348
Watches(&v1.IstioCNI{}, istioCniHandler).
@@ -741,42 +769,6 @@ func specWasUpdated(oldObject client.Object, newObject client.Object) bool {
741769
return oldObject.GetGeneration() != newObject.GetGeneration()
742770
}
743771

744-
func webhookConfigPredicate() predicate.Funcs {
745-
return predicate.Funcs{
746-
UpdateFunc: func(e event.TypedUpdateEvent[client.Object]) bool {
747-
if e.ObjectOld == nil || e.ObjectNew == nil {
748-
return false
749-
}
750-
751-
// Istiod updates the caBundle and failurePolicy fields in its webhook configs.
752-
// We must ignore changes to these fields to prevent an endless update loop.
753-
// We must use deep copies to avoid mutating the shared informer cache.
754-
oldCopy := e.ObjectOld.DeepCopyObject().(client.Object)
755-
newCopy := e.ObjectNew.DeepCopyObject().(client.Object)
756-
clearIgnoredFields(oldCopy)
757-
clearIgnoredFields(newCopy)
758-
return !reflect.DeepEqual(newCopy, oldCopy)
759-
},
760-
}
761-
}
762-
763-
func clearIgnoredFields(obj client.Object) {
764-
obj.SetResourceVersion("")
765-
obj.SetGeneration(0)
766-
obj.SetManagedFields(nil)
767-
switch webhookConfig := obj.(type) {
768-
case *admissionv1.ValidatingWebhookConfiguration:
769-
for i := range len(webhookConfig.Webhooks) {
770-
webhookConfig.Webhooks[i].FailurePolicy = nil
771-
webhookConfig.Webhooks[i].ClientConfig.CABundle = nil
772-
}
773-
case *admissionv1.MutatingWebhookConfiguration:
774-
for i := range len(webhookConfig.Webhooks) {
775-
webhookConfig.Webhooks[i].ClientConfig.CABundle = nil
776-
}
777-
}
778-
}
779-
780772
func wrapEventHandler(logger logr.Logger, handler handler.EventHandler) handler.EventHandler {
781773
return enqueuelogger.WrapIfNecessary(v1.IstioRevisionKind, logger, handler)
782774
}

pkg/fieldignore/fieldignore.go

Lines changed: 267 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,267 @@
1+
// Copyright Istio Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package fieldignore
16+
17+
import (
18+
"fmt"
19+
"reflect"
20+
"regexp"
21+
"strings"
22+
23+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
24+
"k8s.io/apimachinery/pkg/runtime"
25+
"k8s.io/apimachinery/pkg/runtime/schema"
26+
"sigs.k8s.io/controller-runtime/pkg/client"
27+
"sigs.k8s.io/controller-runtime/pkg/event"
28+
"sigs.k8s.io/controller-runtime/pkg/predicate"
29+
)
30+
31+
// FieldIgnoreRule defines a set of fields to ignore for resources matching
32+
// a specific Group/Version/Kind and optional name pattern.
33+
//
34+
// Field paths use dot notation with [*] for array wildcards:
35+
// - "webhooks[*].failurePolicy" → deletes failurePolicy from each webhook
36+
// - "webhooks[*].clientConfig.caBundle" → deletes caBundle nested inside each webhook
37+
// - "spec.template.metadata.annotations" → deletes a deeply nested field
38+
//
39+
// This type is designed to be easily exposed through a CRD in the future.
40+
type FieldIgnoreRule struct {
41+
// Group is the API group (e.g. "admissionregistration.k8s.io"). Empty matches the core group.
42+
Group string `json:"group"`
43+
44+
// Version is the API version (e.g. "v1").
45+
Version string `json:"version"`
46+
47+
// Kind is the resource kind (e.g. "ValidatingWebhookConfiguration").
48+
Kind string `json:"kind"`
49+
50+
// NameRegex is an optional regex to match resource names. Empty matches all names.
51+
NameRegex string `json:"nameRegex,omitempty"`
52+
53+
// Fields is the list of field paths to ignore.
54+
Fields []string `json:"fields"`
55+
56+
// OnlyOnUpdate when true means these fields are only stripped during Helm
57+
// upgrades (post-render), not on initial installs. This is useful when you
58+
// want Helm to set the initial value but let another controller manage it
59+
// afterward.
60+
OnlyOnUpdate bool `json:"onlyOnUpdate,omitempty"`
61+
62+
// compiledNameRegex is the pre-compiled form of NameRegex, populated by
63+
// CompileRules/MustCompileRules. If nil and NameRegex is non-empty,
64+
// Matches will compile on every call (slower).
65+
compiledNameRegex *regexp.Regexp
66+
}
67+
68+
// CompileRules validates and pre-compiles the NameRegex in each rule.
69+
// Returns an error if any NameRegex is invalid. The returned slice should be
70+
// used in place of the input; the originals are not modified.
71+
func CompileRules(rules []FieldIgnoreRule) ([]FieldIgnoreRule, error) {
72+
compiled := make([]FieldIgnoreRule, len(rules))
73+
for i, r := range rules {
74+
if r.NameRegex != "" {
75+
re, err := regexp.Compile("^(?:" + r.NameRegex + ")$")
76+
if err != nil {
77+
return nil, fmt.Errorf("invalid NameRegex %q in rule %d: %w", r.NameRegex, i, err)
78+
}
79+
r.compiledNameRegex = re
80+
}
81+
compiled[i] = r
82+
}
83+
return compiled, nil
84+
}
85+
86+
// MustCompileRules is like CompileRules but panics on error.
87+
// Use this for hardcoded rules that are known to be valid.
88+
func MustCompileRules(rules []FieldIgnoreRule) []FieldIgnoreRule {
89+
compiled, err := CompileRules(rules)
90+
if err != nil {
91+
panic(err)
92+
}
93+
return compiled
94+
}
95+
96+
// Matches returns true if the rule applies to a resource with the given GVK and name.
97+
func (r FieldIgnoreRule) Matches(gvk schema.GroupVersionKind, name string) bool {
98+
if r.Group != gvk.Group || r.Version != gvk.Version || r.Kind != gvk.Kind {
99+
return false
100+
}
101+
if r.compiledNameRegex != nil {
102+
return r.compiledNameRegex.MatchString(name)
103+
}
104+
if r.NameRegex != "" {
105+
matched, err := regexp.MatchString("^(?:"+r.NameRegex+")$", name)
106+
if err != nil || !matched {
107+
return false
108+
}
109+
}
110+
return true
111+
}
112+
113+
// MatchesManifest returns true if the rule applies to an unstructured manifest map.
114+
func (r FieldIgnoreRule) MatchesManifest(manifest map[string]any) bool {
115+
apiVersion, _, _ := unstructured.NestedString(manifest, "apiVersion")
116+
kind, _, _ := unstructured.NestedString(manifest, "kind")
117+
name, _, _ := unstructured.NestedString(manifest, "metadata", "name")
118+
119+
gv, err := schema.ParseGroupVersion(apiVersion)
120+
if err != nil {
121+
return false
122+
}
123+
return r.Matches(schema.GroupVersionKind{Group: gv.Group, Version: gv.Version, Kind: kind}, name)
124+
}
125+
126+
// RemoveFieldsFromManifest removes ignored fields from an unstructured manifest map.
127+
// Rules with OnlyOnUpdate=true are skipped when isUpdate is false.
128+
func RemoveFieldsFromManifest(manifest map[string]any, rules []FieldIgnoreRule, isUpdate bool) {
129+
for _, rule := range rules {
130+
if rule.OnlyOnUpdate && !isUpdate {
131+
continue
132+
}
133+
if !rule.MatchesManifest(manifest) {
134+
continue
135+
}
136+
for _, field := range rule.Fields {
137+
removeFieldPath(manifest, field)
138+
}
139+
}
140+
}
141+
142+
// NewPredicate returns a predicate that ignores changes to fields specified by
143+
// the given rules. On update events it converts both old and new objects to
144+
// unstructured maps, removes the ignored fields (plus standard metadata noise
145+
// like resourceVersion, generation, managedFields), and only triggers
146+
// reconciliation when the remaining content differs.
147+
func NewPredicate(scheme *runtime.Scheme, rules []FieldIgnoreRule) predicate.Funcs {
148+
return predicate.Funcs{
149+
UpdateFunc: func(e event.UpdateEvent) bool {
150+
if e.ObjectOld == nil || e.ObjectNew == nil {
151+
return false
152+
}
153+
return objectsChangedIgnoringFields(scheme, e.ObjectOld, e.ObjectNew, rules)
154+
},
155+
}
156+
}
157+
158+
func objectsChangedIgnoringFields(scheme *runtime.Scheme, oldObj, newObj client.Object, rules []FieldIgnoreRule) bool {
159+
gvk, err := gvkForObject(scheme, newObj)
160+
if err != nil {
161+
return true
162+
}
163+
164+
name := newObj.GetName()
165+
166+
// Collect fields from all matching rules regardless of OnlyOnUpdate.
167+
// OnlyOnUpdate only controls post-renderer behavior (whether Helm strips
168+
// the field on install vs upgrade). In the predicate we always want to
169+
// ignore these fields to avoid unnecessary reconciliation.
170+
var matchingFields []string
171+
for _, rule := range rules {
172+
if rule.Matches(gvk, name) {
173+
matchingFields = append(matchingFields, rule.Fields...)
174+
}
175+
}
176+
if len(matchingFields) == 0 {
177+
return true
178+
}
179+
180+
oldMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(oldObj)
181+
if err != nil {
182+
return true
183+
}
184+
newMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(newObj)
185+
if err != nil {
186+
return true
187+
}
188+
189+
clearMetadataFields(oldMap)
190+
clearMetadataFields(newMap)
191+
192+
for _, field := range matchingFields {
193+
removeFieldPath(oldMap, field)
194+
removeFieldPath(newMap, field)
195+
}
196+
197+
return !reflect.DeepEqual(oldMap, newMap)
198+
}
199+
200+
func gvkForObject(scheme *runtime.Scheme, obj client.Object) (schema.GroupVersionKind, error) {
201+
gvks, _, err := scheme.ObjectKinds(obj)
202+
if err != nil {
203+
return schema.GroupVersionKind{}, err
204+
}
205+
if len(gvks) == 0 {
206+
return schema.GroupVersionKind{}, fmt.Errorf("no GVK found for object %T", obj)
207+
}
208+
return gvks[0], nil
209+
}
210+
211+
// clearMetadataFields removes standard metadata fields that change on every
212+
// update and should never trigger reconciliation.
213+
func clearMetadataFields(obj map[string]any) {
214+
if metadata, ok := obj["metadata"].(map[string]any); ok {
215+
delete(metadata, "resourceVersion")
216+
delete(metadata, "generation")
217+
delete(metadata, "managedFields")
218+
}
219+
}
220+
221+
// removeFieldPath removes a field from a nested map using dot-separated path
222+
// notation. Array wildcards ([*]) cause the operation to be applied to every
223+
// element of the array.
224+
func removeFieldPath(obj map[string]any, path string) {
225+
segments := strings.Split(path, ".")
226+
removeFieldSegments(obj, segments)
227+
}
228+
229+
func removeFieldSegments(obj map[string]any, segments []string) {
230+
if len(segments) == 0 || obj == nil {
231+
return
232+
}
233+
234+
seg := segments[0]
235+
remaining := segments[1:]
236+
237+
if strings.HasSuffix(seg, "[*]") {
238+
key := strings.TrimSuffix(seg, "[*]")
239+
if len(remaining) == 0 {
240+
// Bare [*] with no remaining segments (e.g. "items[*]") removes
241+
// the entire array field.
242+
delete(obj, key)
243+
return
244+
}
245+
arr, ok := obj[key].([]any)
246+
if !ok {
247+
return
248+
}
249+
for _, item := range arr {
250+
if m, ok := item.(map[string]any); ok {
251+
removeFieldSegments(m, remaining)
252+
}
253+
}
254+
return
255+
}
256+
257+
if len(remaining) == 0 {
258+
delete(obj, seg)
259+
return
260+
}
261+
262+
child, ok := obj[seg].(map[string]any)
263+
if !ok {
264+
return
265+
}
266+
removeFieldSegments(child, remaining)
267+
}

0 commit comments

Comments
 (0)