Flux should override ownership of fields in case of the conflict for SSAed object.

[vespian]

Describe the bug

Flux sometimes needs to "adopt" objects - i.e start managing objects that were already defined in APIserver, after user defines them in the gitrepo. In case when such objects were created using Server Side Apply and field manager is set, Flux will not override such objects and instead will just set labels on it.

This may cause confusion in some cases as the object is not being synchronized with the contents of the repo. What is more - in case when ownership is relinquished, Flux will reconcile the object and will apply the due changes from the repo.

Steps to reproduce

1. Launch flux using tutorial.
2. Define HelmRepository objects as follows:

$ cat ./manual/*
apiVersion: v1
data:
  ca.cert: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUZGVENDQkxxZ0F3SUJBZ0lRQzRWbHNHcGVPRHd1L0NyeVFDMGh5VEFLQmdncWhrak9QUVFEQWpCS01Rc3cKQ1FZRFZRUUdFd0pWVXpFWk1CY0dBMVVFQ2hNUVEyeHZkV1JtYkdGeVpTd2dTVzVqTGpFZ01CNEdBMVVFQXhNWApRMnh2ZFdSbWJHRnlaU0JKYm1NZ1JVTkRJRU5CTFRNd0hoY05NakV3TkRJM01EQXdNREF3V2hjTk1qSXdOREkyCk1qTTFPVFU1V2pCeU1Rc3dDUVlEVlFRR0V3SlZVekVUTUJFR0ExVUVDQk1LUTJGc2FXWnZjbTVwWVRFV01CUUcKQTFVRUJ4TU5VMkZ1SUVaeVlXNWphWE5qYnpFWk1CY0dBMVVFQ2hNUVEyeHZkV1JtYkdGeVpTd2dTVzVqTGpFYgpNQmtHQTFVRUF4TVNZMmhoY25SekxtcGxkSE4wWVdOckxtbHZNRmt3RXdZSEtvWkl6ajBDQVFZSUtvWkl6ajBECkFRY0RRZ0FFN09FUDNDVDFzN0NFWm1VdUFnSzR4MG5SbDcreUNlSGZIUzlHWElZYjV6WThoOHdTd1lMeW5rdmkKd1pWaGV5WDRIc3JVTmJoSGhOR3k5WUo1Mm0yRzBLT0NBMWd3Z2dOVU1COEdBMVVkSXdRWU1CYUFGS1hPTitycgpzSFVPbEdlSXRFWDYyU1FRaDVZZk1CMEdBMVVkRGdRV0JCUmwyT1NUaE1MRnlzWS9INTJ1bThvSjArREZvVEFkCkJnTlZIUkVFRmpBVWdoSmphR0Z5ZEhNdWFtVjBjM1JoWTJzdWFXOHdEZ1lEVlIwUEFRSC9CQVFEQWdlQU1CMEcKQTFVZEpRUVdNQlFHQ0NzR0FRVUZCd01CQmdnckJnRUZCUWNEQWpCN0JnTlZIUjhFZERCeU1EZWdOYUF6aGpGbwpkSFJ3T2k4dlkzSnNNeTVrYVdkcFkyVnlkQzVqYjIwdlEyeHZkV1JtYkdGeVpVbHVZMFZEUTBOQkxUTXVZM0pzCk1EZWdOYUF6aGpGb2RIUndPaTh2WTNKc05DNWthV2RwWTJWeWRDNWpiMjB2UTJ4dmRXUm1iR0Z5WlVsdVkwVkQKUTBOQkxUTXVZM0pzTUQ0R0ExVWRJQVEzTURVd013WUdaNEVNQVFJQ01Da3dKd1lJS3dZQkJRVUhBZ0VXRzJoMApkSEE2THk5M2QzY3VaR2xuYVdObGNuUXVZMjl0TDBOUVV6QjJCZ2dyQmdFRkJRY0JBUVJxTUdnd0pBWUlLd1lCCkJRVUhNQUdHR0doMGRIQTZMeTl2WTNOd0xtUnBaMmxqWlhKMExtTnZiVEJBQmdnckJnRUZCUWN3QW9ZMGFIUjAKY0RvdkwyTmhZMlZ5ZEhNdVpHbG5hV05sY25RdVkyOXRMME5zYjNWa1pteGhjbVZKYm1ORlEwTkRRUzB6TG1OeQpkREFNQmdOVkhSTUJBZjhFQWpBQU1JSUJmd1lLS3dZQkJBSFdlUUlFQWdTQ0FXOEVnZ0ZyQVdrQWR3QkdwVlhyCmRmcVJJREMxb29scDlQTjlFU3hCZEw3OVNiaUZxL0w4Y1A1dFJ3QUFBWGtUNmZkeEFBQUVBd0JJTUVZQ0lRQ0cKZGp3Wkk1MCthVm5tYXNNOU1SV1pNQ1dOeUl6ZTF1OFJiU3VDZlBsMjRRSWhBTFp2a1ZoWHppcmJRbm9maE15OQpiVGgxaUJHS1RyZ2tuZ01RL1BaRFFLb2ZBSFlBSWtWRkIxbFZKRmFXUDZFdjhmZHRodUFqSm1PdHdFdC9YY2FEClhHN2lEd0lBQUFGNUUrbjJiZ0FBQkFNQVJ6QkZBaUJSVTg5TEtDYWp6RklCc3ZaYkIyMXhua250NkkvZ0Y0cGgKdHNQbmhYOEVPd0loQU9DZ0ViNTR1R2N2QndreTQ2ZkcybGFmelZaSmsxL2lremVzNXE3MW5WQ3NBSFlBVWFPdwo5ZjBCZVp4V2JiZzNlSThNcEhyTUd5Zkw5NTZJUXBvTi90U0xCZVVBQUFGNUUrbjRQUUFBQkFNQVJ6QkZBaUVBCjBQeWg1UzAwR2NXZS9OY3BrSkJBb00wZDNFdHFHL0xyS2U0Z09sVnQwU1VDSUdNMmhDaTZ2STVnTkQ5U0FhcFUKUjUwMk0zV3RoQlJrY0kyWlZjSHpXdmt5TUFvR0NDcUdTTTQ5QkFNQ0Ewa0FNRVlDSVFEa1l6UWdjTWhEYlBLcQpKd0VwRS9hT3lYdis2VFJ6VEpvSHhVQ2FBT2crWGdJaEFJZWVqY25YdDdjY1NMTitRSzY4akY0MDZyaXhUZUE3Ck5oenRVOVYyZ1hSWQotLS0tLUVORCBDRVJUSUZJQ0FURS0tLS0tCi0tLS0tQkVHSU4gQ0VSVElGSUNBVEUtLS0tLQpNSUlEelRDQ0FyV2dBd0lCQWdJUUNqZUhaRjVmdEl3aVR2MGI3UlFNUERBTkJna3Foa2lHOXcwQkFRc0ZBREJhCk1Rc3dDUVlEVlFRR0V3SkpSVEVTTUJBR0ExVUVDaE1KUW1Gc2RHbHRiM0psTVJNd0VRWURWUVFMRXdwRGVXSmwKY2xSeWRYTjBNU0l3SUFZRFZRUURFeGxDWVd4MGFXMXZjbVVnUTNsaVpYSlVjblZ6ZENCU2IyOTBNQjRYRFRJdwpNREV5TnpFeU5EZ3dPRm9YRFRJME1USXpNVEl6TlRrMU9Wb3dTakVMTUFrR0ExVUVCaE1DVlZNeEdUQVhCZ05WCkJBb1RFRU5zYjNWa1pteGhjbVVzSUVsdVl5NHhJREFlQmdOVkJBTVRGME5zYjNWa1pteGhjbVVnU1c1aklFVkQKUXlCRFFTMHpNRmt3RXdZSEtvWkl6ajBDQVFZSUtvWkl6ajBEQVFjRFFnQUV1YTFOWnBrVUMwYnNINEhSS2xBZQpuUU1WTHpRU2ZTMld1SWc0bTRWZmo3KzdUZTloUnNUSmM5UWtUK0R1SE01c3MxRnhMMnJ1VEFVSmQ5TnlZcVNiCjE2T0NBV2d3Z2dGa01CMEdBMVVkRGdRV0JCU2x6amZxNjdCMURwUm5pTFJGK3Rra0VJZVdIekFmQmdOVkhTTUUKR0RBV2dCVGxuVmt3Z2tkWXpLejZDRlEyaG5zNnRRUk44REFPQmdOVkhROEJBZjhFQkFNQ0FZWXdIUVlEVlIwbApCQll3RkFZSUt3WUJCUVVIQXdFR0NDc0dBUVVGQndNQ01CSUdBMVVkRXdFQi93UUlNQVlCQWY4Q0FRQXdOQVlJCkt3WUJCUVVIQVFFRUtEQW1NQ1FHQ0NzR0FRVUZCekFCaGhob2RIUndPaTh2YjJOemNDNWthV2RwWTJWeWRDNWoKYjIwd09nWURWUjBmQkRNd01UQXZvQzJnSzRZcGFIUjBjRG92TDJOeWJETXVaR2xuYVdObGNuUXVZMjl0TDA5dApibWx5YjI5ME1qQXlOUzVqY213d2JRWURWUjBnQkdZd1pEQTNCZ2xnaGtnQmh2MXNBUUV3S2pBb0JnZ3JCZ0VGCkJRY0NBUlljYUhSMGNITTZMeTkzZDNjdVpHbG5hV05sY25RdVkyOXRMME5RVXpBTEJnbGdoa2dCaHYxc0FRSXcKQ0FZR1o0RU1BUUlCTUFnR0JtZUJEQUVDQWpBSUJnWm5nUXdCQWdNd0RRWUpLb1pJaHZjTkFRRUxCUUFEZ2dFQgpBQVVrSGQwYnNDcnJtTmFGNHpsTlhtdFhuWUpYL092b01hSlhrR1VGdmhaRU9GcDNBcm5QRUVMRzRaS2s0MFVuCitBQkhMR2lvVnBsVFZJK3Rua0RCMEErMjF3MExPRWhzVUN4SmtBWmJaQjJMekVnd0x0NEk0cHRKSXNDU0RCRmUKbHBLVTFmd2czRlpzNVpLVHYzb2N3RGZqaFVrVitpdmhkRGtZRDdmYTg2SlhXR0JQekk2VUFQeEdlelF4UGsxSApnb0U2eS9TSlhRN3ZUUTF1bkJ1Q0pOMHlKVjBSZUZFUVBhQTFJd1F2WlcrY3dkRkQxOUFlOHpGbldTZmRhOUoxCkNaTVJKQ1FVenltKzVpUER1STl5UCtrSHlDUkVVM3F6dVdGbG9Vd094a2dBeVhWakJZZHdSVktEMDVXZFJlcncKNkRFZGZna2ZDdjQrM2FvOFhuVFNyTEU9Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K
kind: Secret
metadata:
  creationTimestamp: null
  name: tls-root-ca
  namespace: flux-system
---
apiVersion: source.toolkit.fluxcd.io/v1beta1
kind: HelmRepository
metadata:
  name: charts.jetstack.io
  namespace: flux-system
spec:
  interval: 10m
  secretRef:
    name: "tls-root-ca"
  timeout: 1m
  url: https://charts.jetstack.io

$ k apply -f manual --field-manager=some-cli --server-side=true

3. Notice that the created object has the secretRef. It is best to keep watching it as we progress with the remaining steps of the repro:

$ k get helmrepositories.source.toolkit.fluxcd.io -A -o yaml -w

4. In git repository, create similar HelmRepository object, but this time without secretRef specified. Commit to the repo:

$ cat ./fleet-infra/clusters/bugtest/*
---
apiVersion: source.toolkit.fluxcd.io/v1beta1
kind: HelmRepository
metadata:
  name: charts.jetstack.io
  namespace: flux-system
spec:
  interval: 10m
  timeout: 1m
  url: "https://charts.jetstack.io"
---
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- hr.yaml

5. Create a customization pointing to the modified object in the gitrepo:

$ cat ./kustomization.yaml
apiVersion: kustomize.toolkit.fluxcd.io/v1beta2
kind: Kustomization
metadata:
  name: bug-test
  namespace: flux-system
spec:
  force: true
  interval: 10m0s
  path: ./clusters/bugtest/
  prune: true
  sourceRef:
    kind: GitRepository
    name: flux-system

6. Apply the kustomization, note that the HelmRepository object has labels applied but sercretRef field has not been removed by Flux.

$ k apply -f ./kustomization.yaml

Expected behavior

Flux overrides the object in accordance of the git repo contents.

Screenshots and recordings

No response

OS / Distro

Ubuntu 20.04.3 LTS

Flux version

v0.27.3

Flux check

$ flux check ► checking prerequisites ✔ Kubernetes 1.21.1 >=1.20.6-0 ► checking controllers ✔ helm-controller: deployment ready ► ghcr.io/fluxcd/helm-controller:v0.17.1 ✔ kustomize-controller: deployment ready ► ghcr.io/fluxcd/kustomize-controller:v0.21.1 ✔ notification-controller: deployment ready ► ghcr.io/fluxcd/notification-controller:v0.22.2 ✔ source-controller: deployment ready ► ghcr.io/fluxcd/source-controller:v0.21.2 ✔ all checks passed

Git provider

Github

Container Registry provider

Docker

Additional context

No response

Code of Conduct

• [X] I agree to follow this project's Code of Conduct

[stefanprodan]

This is the expected behaviour, Flux only removes fields from objects applied with the default kubectl manager. Docs here: https://fluxcd.io/docs/faq/#why-are-kubectl-edits-rolled-back-by-flux

If we would undo all changes for any random manager then HPA, Flagger and any other controller will not be able to function.

[stefanprodan]

No being able to remove fields is related to https://github.com/kubernetes/kubernetes/issues/99003 I don't think this bug will be solved any time soon in Kubernetes, so we may want to implement an allowlist of managers...

[kingdonb]

I think the 0.28.2 release has now addressed this, ~Flux takes over fields~ the merge behavior is now opt-in:

https://github.com/fluxcd/flux2/releases/tag/v0.28.0

(subheading "Manage cluster addons")

Maybe this is incorrect though, I'm afraid I've missed some context here and jumped the gun to suggest it might be fixed now. Please let me know if this issue seems abated or remains from your perspective @vespian

The clarification I mean to add:

A new annotation (kustomize.toolkit.fluxcd.io/ssa: merge) is available for allowing Flux to patch cluster addons such as CoreDNS without the kustomize-controller removing the kubectl managed fields.

Since these are fields from another SSA manager than kubectl, I think they are only overrided if they are explicitly set by Flux. (Or they are still not overrided by Flux at all.)

That would mean, this issue remains still, although bad behavior from changes made through client-side apply of kubectl should now be fixed.

Corroborated by this line from the thread above:

If we would undo all changes for any random manager then HPA, Flagger and any other controller will not be able to function.