Skip to content

Commit b7a57ce

Browse files
yozelyozel
committed
Add --override-manager flag for server-side apply drift detection
This flag allows specifying field managers whose ownership should be transferred to the helm-controller before performing drift detection. When a disallowed field manager is detected on a managed resource, its field ownership is replaced with the helm-controller's field owner, enabling proper drift detection for fields that were previously modified by other controllers or tools (e.g., kubectl). This feature mirrors the --override-manager flag available in FluxCD's kustomize-controller, providing consistent behavior across Flux components for managing server-side apply field ownership conflicts.
1 parent 2d23374 commit b7a57ce

File tree

6 files changed

+193
-24
lines changed

6 files changed

+193
-24
lines changed

‎internal/action/diff.go‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,16 @@ import (
2727
helmaction "helm.sh/helm/v3/pkg/action"
2828
helmrelease "helm.sh/helm/v3/pkg/release"
2929
apierrors "k8s.io/apimachinery/pkg/api/errors"
30+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
31+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
3032
"k8s.io/apimachinery/pkg/types"
3133
apierrutil "k8s.io/apimachinery/pkg/util/errors"
3234
"k8s.io/utils/ptr"
3335
"sigs.k8s.io/controller-runtime/pkg/client"
3436
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
3537

3638
"github.com/fluxcd/cli-utils/pkg/object"
39+
3740
"github.com/fluxcd/pkg/ssa"
3841
"github.com/fluxcd/pkg/ssa/jsondiff"
3942
ssanormalize "github.com/fluxcd/pkg/ssa/normalize"
@@ -45,7 +48,7 @@ import (
4548

4649
// Diff returns a jsondiff.DiffSet of the changes between the state of the
4750
// cluster and the Helm release.Release manifest.
48-
func Diff(ctx context.Context, config *helmaction.Configuration, rls *helmrelease.Release, fieldOwner string, ignore ...v2.IgnoreRule) (jsondiff.DiffSet, error) {
51+
func Diff(ctx context.Context, config *helmaction.Configuration, rls *helmrelease.Release, fieldOwner string, disallowedFieldManagers []string, ignore ...v2.IgnoreRule) (jsondiff.DiffSet, error) {
4952
// Create a dry-run only client to use solely for diffing.
5053
cfg, err := config.RESTClientGetter.ToRESTConfig()
5154
if err != nil {
@@ -96,6 +99,19 @@ func Diff(ctx context.Context, config *helmaction.Configuration, rls *helmreleas
9699
}
97100
}
98101

102+
if len(disallowedFieldManagers) > 0 {
103+
c, err := client.New(cfg, client.Options{})
104+
if err != nil {
105+
return nil, err
106+
}
107+
for _, obj := range objects {
108+
err = replaceDisallowedFieldManagers(ctx, c, fieldOwner, disallowedFieldManagers, obj)
109+
if err != nil {
110+
return nil, fmt.Errorf("failed to clean-up disallowed field managers: %w", err)
111+
}
112+
}
113+
}
114+
99115
// Base configuration for the diffing of the object.
100116
diffOpts := []jsondiff.ListOption{
101117
jsondiff.FieldOwner(fieldOwner),
@@ -278,3 +294,39 @@ func (s sortableDiffs) Less(i, j int) bool {
278294

279295
return iDiff.GetName() < jDiff.GetName()
280296
}
297+
298+
func replaceDisallowedFieldManagers(ctx context.Context, c client.Client, fieldOwner string, disallowedFieldManagers []string, obj *unstructured.Unstructured) error {
299+
existingObj := &unstructured.Unstructured{}
300+
existingObj.SetGroupVersionKind(obj.GroupVersionKind())
301+
if err := c.Get(ctx, client.ObjectKeyFromObject(obj), existingObj); client.IgnoreNotFound(err) != nil {
302+
return err
303+
}
304+
305+
fieldManagers := []ssa.FieldManager{}
306+
for _, fieldManager := range disallowedFieldManagers {
307+
fieldManagers = append(fieldManagers, ssa.FieldManager{
308+
Name: fieldManager,
309+
OperationType: metav1.ManagedFieldsOperationApply,
310+
})
311+
fieldManagers = append(fieldManagers, ssa.FieldManager{
312+
Name: fieldManager,
313+
OperationType: metav1.ManagedFieldsOperationUpdate,
314+
})
315+
}
316+
317+
patches, err := ssa.PatchReplaceFieldsManagers(existingObj, fieldManagers, fieldOwner)
318+
if err != nil {
319+
return err
320+
}
321+
if len(patches) > 0 {
322+
rawPatch, err := json.Marshal(patches)
323+
if err != nil {
324+
return err
325+
}
326+
err = c.Patch(ctx, existingObj, client.RawPatch(types.JSONPatchType, rawPatch), client.FieldOwner(fieldOwner))
327+
if err != nil {
328+
return err
329+
}
330+
}
331+
return nil
332+
}

‎internal/action/diff_test.go‎

Lines changed: 111 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,15 @@ func TestDiff(t *testing.T) {
7272
}
7373

7474
const testOwner = "helm-controller"
75+
const ownerToOverride = "kubectl"
76+
const ownerToKeep = "kube-controller-manager"
7577

7678
tests := []struct {
7779
name string
7880
manifest string
7981
ignoreRules []v2.IgnoreRule
8082
mutateCluster func(objs []*unstructured.Unstructured, namespace string) ([]*unstructured.Unstructured, error)
83+
updateCluster func(objs []*unstructured.Unstructured, namespace string) ([]*unstructured.Unstructured, client.FieldOwner, error)
8184
want func(namespace string, desired, cluster []*unstructured.Unstructured) jsondiff.DiffSet
8285
wantErr bool
8386
}{
@@ -151,6 +154,91 @@ data:
151154
}
152155
},
153156
},
157+
{
158+
name: "detects drift after kubectl edit",
159+
manifest: `---
160+
apiVersion: v1
161+
kind: ConfigMap
162+
metadata:
163+
name: changed
164+
data:
165+
key: value`,
166+
mutateCluster: func(objs []*unstructured.Unstructured, namespace string) ([]*unstructured.Unstructured, error) {
167+
var clusterObjs []*unstructured.Unstructured
168+
for _, obj := range objs {
169+
obj := obj.DeepCopy()
170+
obj.SetNamespace(namespace)
171+
clusterObjs = append(clusterObjs, obj)
172+
}
173+
return clusterObjs, nil
174+
},
175+
updateCluster: func(objs []*unstructured.Unstructured, namespace string) ([]*unstructured.Unstructured, client.FieldOwner, error) {
176+
var clusterObjs []*unstructured.Unstructured
177+
for _, obj := range objs {
178+
obj := obj.DeepCopy()
179+
if err := unstructured.SetNestedField(obj.Object, "changed", "data", "anotherKey"); err != nil {
180+
return nil, "", fmt.Errorf("failed to set nested field: %w", err)
181+
}
182+
clusterObjs = append(clusterObjs, obj)
183+
}
184+
return clusterObjs, ownerToOverride, nil
185+
},
186+
want: func(namespace string, desired, cluster []*unstructured.Unstructured) jsondiff.DiffSet {
187+
return jsondiff.DiffSet{
188+
{
189+
Type: jsondiff.DiffTypeUpdate,
190+
DesiredObject: namespacedUnstructured(desired[0], namespace),
191+
ClusterObject: cluster[0],
192+
Patch: extjsondiff.Patch{
193+
{
194+
Type: extjsondiff.OperationRemove,
195+
Path: "/data/anotherKey",
196+
OldValue: "changed",
197+
},
198+
},
199+
},
200+
}
201+
},
202+
},
203+
{
204+
name: "detect no drift if edited by kube-controller-manager",
205+
manifest: `---
206+
apiVersion: v1
207+
kind: ConfigMap
208+
metadata:
209+
name: changed
210+
data:
211+
key: value`,
212+
mutateCluster: func(objs []*unstructured.Unstructured, namespace string) ([]*unstructured.Unstructured, error) {
213+
var clusterObjs []*unstructured.Unstructured
214+
for _, obj := range objs {
215+
obj := obj.DeepCopy()
216+
obj.SetNamespace(namespace)
217+
clusterObjs = append(clusterObjs, obj)
218+
}
219+
return clusterObjs, nil
220+
},
221+
updateCluster: func(objs []*unstructured.Unstructured, namespace string) ([]*unstructured.Unstructured, client.FieldOwner, error) {
222+
var clusterObjs []*unstructured.Unstructured
223+
for _, obj := range objs {
224+
obj := obj.DeepCopy()
225+
if err := unstructured.SetNestedField(obj.Object, "changed", "data", "anotherKey"); err != nil {
226+
return nil, "", fmt.Errorf("failed to set nested field: %w", err)
227+
}
228+
clusterObjs = append(clusterObjs, obj)
229+
}
230+
return clusterObjs, ownerToKeep, nil
231+
},
232+
want: func(namespace string, desired, cluster []*unstructured.Unstructured) jsondiff.DiffSet {
233+
return jsondiff.DiffSet{
234+
{
235+
Type: jsondiff.DiffTypeNone,
236+
DesiredObject: namespacedUnstructured(desired[0], namespace),
237+
ClusterObject: cluster[0],
238+
},
239+
}
240+
},
241+
},
154242
{
155243
name: "empty release manifest",
156244
manifest: "",
@@ -440,12 +528,34 @@ data:
440528
}
441529
}
442530

443-
got, err := Diff(ctx, &helmaction.Configuration{RESTClientGetter: getter}, rls, testOwner, tt.ignoreRules...)
531+
if tt.updateCluster != nil {
532+
// tt.updateCluster emulates out-of-band modifications like `kubectl edit`
533+
var (
534+
fieldOwner client.FieldOwner
535+
)
536+
if clusterObjs, fieldOwner, err = tt.updateCluster(clusterObjs, ns.Name); err != nil {
537+
t.Fatalf("Failed to modify cluster resource: %v", err)
538+
}
539+
for _, obj := range clusterObjs {
540+
if err := c.Update(ctx, obj, fieldOwner); err != nil {
541+
t.Fatalf("Failed to update object: %v", err)
542+
}
543+
}
544+
}
545+
546+
got, err := Diff(ctx, &helmaction.Configuration{RESTClientGetter: getter}, rls, testOwner, []string{ownerToOverride}, tt.ignoreRules...)
444547
if (err != nil) != tt.wantErr {
445548
t.Errorf("Diff() error = %v, wantErr %v", err, tt.wantErr)
446549
return
447550
}
448551

552+
// Refresh all objects since Diff might do mutations and this would change resourceVersion
553+
for _, obj := range clusterObjs {
554+
if err := c.Get(ctx, client.ObjectKeyFromObject(obj), obj); err != nil {
555+
t.Fatalf("Failed to create object: %v", err)
556+
}
557+
}
558+
449559
var want jsondiff.DiffSet
450560
if tt.want != nil {
451561
want = tt.want(ns.Name, objs, clusterObjs)

‎internal/controller/helmrelease_controller.go‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -89,13 +89,14 @@ type HelmReleaseReconciler struct {
8989

9090
// Kubernetes configuration
9191

92-
FieldManager string
93-
DefaultServiceAccount string
94-
GetClusterConfig func() (*rest.Config, error)
95-
ClientOpts runtimeClient.Options
96-
KubeConfigOpts runtimeClient.KubeConfigOptions
97-
APIReader client.Reader
98-
TokenCache *cache.TokenCache
92+
FieldManager string
93+
DisallowedFieldManagers []string
94+
DefaultServiceAccount string
95+
GetClusterConfig func() (*rest.Config, error)
96+
ClientOpts runtimeClient.Options
97+
KubeConfigOpts runtimeClient.KubeConfigOptions
98+
APIReader client.Reader
99+
TokenCache *cache.TokenCache
99100

100101
// Retry and requeue configuration
101102

@@ -393,7 +394,7 @@ func (r *HelmReleaseReconciler) reconcileRelease(ctx context.Context, patchHelpe
393394
}
394395

395396
// Off we go!
396-
if err = intreconcile.NewAtomicRelease(patchHelper, cfg, r.EventRecorder, r.FieldManager).Reconcile(ctx, &intreconcile.Request{
397+
if err = intreconcile.NewAtomicRelease(patchHelper, cfg, r.EventRecorder, r.FieldManager, r.DisallowedFieldManagers).Reconcile(ctx, &intreconcile.Request{
397398
Object: obj,
398399
Chart: loadedChart,
399400
Values: values,

‎internal/reconcile/atomic_release.go‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -113,22 +113,24 @@ var (
113113
// For more information on the individual ActionReconcilers, refer to their
114114
// documentation.
115115
type AtomicRelease struct {
116-
patchHelper *patch.SerialPatcher
117-
configFactory *action.ConfigFactory
118-
eventRecorder record.EventRecorder
119-
strategy releaseStrategy
120-
fieldManager string
116+
patchHelper *patch.SerialPatcher
117+
configFactory *action.ConfigFactory
118+
eventRecorder record.EventRecorder
119+
strategy releaseStrategy
120+
fieldManager string
121+
disallowedFieldManagers []string
121122
}
122123

123124
// NewAtomicRelease returns a new AtomicRelease reconciler configured with the
124125
// provided values.
125-
func NewAtomicRelease(patchHelper *patch.SerialPatcher, cfg *action.ConfigFactory, recorder record.EventRecorder, fieldManager string) *AtomicRelease {
126+
func NewAtomicRelease(patchHelper *patch.SerialPatcher, cfg *action.ConfigFactory, recorder record.EventRecorder, fieldManager string, disallowedFieldManagers []string) *AtomicRelease {
126127
return &AtomicRelease{
127-
patchHelper: patchHelper,
128-
eventRecorder: recorder,
129-
configFactory: cfg,
130-
strategy: &cleanReleaseStrategy{},
131-
fieldManager: fieldManager,
128+
patchHelper: patchHelper,
129+
eventRecorder: recorder,
130+
configFactory: cfg,
131+
strategy: &cleanReleaseStrategy{},
132+
fieldManager: fieldManager,
133+
disallowedFieldManagers: disallowedFieldManagers,
132134
}
133135
}
134136

@@ -188,7 +190,7 @@ func (r *AtomicRelease) Reconcile(ctx context.Context, req *Request) error {
188190
default:
189191
// Determine the current state of the Helm release.
190192
log.V(logger.DebugLevel).Info("determining current state of Helm release")
191-
state, err := DetermineReleaseState(ctx, r.configFactory, req)
193+
state, err := DetermineReleaseState(ctx, r.configFactory, req, r.disallowedFieldManagers)
192194
if err != nil {
193195
conditions.MarkFalse(req.Object, meta.ReadyCondition, "StateError", "Could not determine release state: %s", err)
194196
return fmt.Errorf("cannot determine release state: %w", err)

‎internal/reconcile/state.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ type ReleaseState struct {
8989
// DetermineReleaseState determines the state of the Helm release as compared
9090
// to the v2.HelmRelease object. It returns a ReleaseState that indicates
9191
// the status of the release, and an error if the state could not be determined.
92-
func DetermineReleaseState(ctx context.Context, cfg *action.ConfigFactory, req *Request) (ReleaseState, error) {
92+
func DetermineReleaseState(ctx context.Context, cfg *action.ConfigFactory, req *Request, disallowedFieldManagers []string) (ReleaseState, error) {
9393
rls, err := action.LastRelease(cfg.Build(nil), req.Object.GetReleaseName())
9494
if err != nil {
9595
if errors.Is(err, action.ErrReleaseNotFound) {
@@ -188,7 +188,7 @@ func DetermineReleaseState(ctx context.Context, cfg *action.ConfigFactory, req *
188188

189189
// Confirm the cluster state matches the desired config.
190190
if diffOpts := req.Object.GetDriftDetection(); diffOpts.MustDetectChanges() {
191-
diffSet, err := action.Diff(ctx, cfg.Build(nil), rls, kube.ManagedFieldsManager, req.Object.GetDriftDetection().Ignore...)
191+
diffSet, err := action.Diff(ctx, cfg.Build(nil), rls, kube.ManagedFieldsManager, disallowedFieldManagers, req.Object.GetDriftDetection().Ignore...)
192192
hasChanges := diffSet.HasChanges()
193193
if err != nil {
194194
if !hasChanges {

‎main.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ func main() {
105105
oomWatchMaxMemoryPath string
106106
oomWatchCurrentMemoryPath string
107107
snapshotDigestAlgo string
108+
disallowedFieldManagers []string
108109
tokenCacheOptions cache.TokenFlags
109110
defaultKubeConfigServiceAccount string
110111
)
@@ -137,6 +138,8 @@ func main() {
137138
"The path to the cgroup current memory usage file. Requires feature gate 'OOMWatch' to be enabled. If not set, the path will be automatically detected.")
138139
flag.StringVar(&snapshotDigestAlgo, "snapshot-digest-algo", intdigest.Canonical.String(),
139140
"The algorithm to use to calculate the digest of Helm release storage snapshots.")
141+
flag.StringArrayVar(&disallowedFieldManagers, "override-manager", []string{},
142+
"Field manager disallowed to perform changes on managed resources.")
140143

141144
clientOptions.BindFlags(flag.CommandLine)
142145
logOptions.BindFlags(flag.CommandLine)
@@ -354,6 +357,7 @@ func main() {
354357
DependencyRequeueInterval: requeueDependency,
355358
ArtifactFetchRetries: httpRetry,
356359
AllowExternalArtifact: allowExternalArtifact,
360+
DisallowedFieldManagers: disallowedFieldManagers,
357361
}).SetupWithManager(ctx, mgr, controller.HelmReleaseReconcilerOptions{
358362
RateLimiter: helper.GetRateLimiter(rateLimiterOptions),
359363
WatchConfigs: watchConfigs,

0 commit comments

Comments
 (0)