Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support additional Field Managers in the disallow list #982

Closed
Nalum opened this issue Oct 5, 2023 · 6 comments · Fixed by #1017
Closed

Support additional Field Managers in the disallow list #982

Nalum opened this issue Oct 5, 2023 · 6 comments · Fixed by #1017
Labels
area/server-side-apply SSA related issues and pull requests enhancement New feature or request

Comments

@Nalum
Copy link
Member

Nalum commented Oct 5, 2023

In working with this controller and messing around with kubectl edit and Lens to make changes to resources I have found that Lens will update the list of managedFields with it's own entry where manager is set to node-fetch. As this is not part of the below code Flux does not undo the changes made (assuming they do not conflict with flux managed fields) and we are left with the resource being in an unknown state where the changes have come from outside the cluster.

It would be great to be able to configure the below list so that we can have other managers in this list:

FieldManagers: []ssa.FieldManager{
{
// to undo changes made with 'kubectl apply --server-side --force-conflicts'
Name: "kubectl",
OperationType: metav1.ManagedFieldsOperationApply,
},
{
// to undo changes made with 'kubectl apply'
Name: "kubectl",
OperationType: metav1.ManagedFieldsOperationUpdate,
},
{
// to undo changes made with 'kubectl apply'
Name: "before-first-apply",
OperationType: metav1.ManagedFieldsOperationUpdate,
},
{
// to undo changes made by the controller before SSA
Name: r.ControllerName,
OperationType: metav1.ManagedFieldsOperationUpdate,
},
},

@stefanprodan stefanprodan changed the title Support Additional Field Managers Support additional Field Managers in the disallow list Oct 5, 2023
@stefanprodan stefanprodan added enhancement New feature or request area/server-side-apply SSA related issues and pull requests labels Oct 5, 2023
@Nalum
Copy link
Member Author

Nalum commented Nov 25, 2023

@stefanprodan gonna take a look at doing this, if no one else has already started on it, over the next few days

@stefanprodan
Copy link
Member

stefanprodan commented Nov 25, 2023

I propose we add a string array flag type called something like --override-manager. For all the specified manages, we'll add two rules (ManagedFieldsOperationApply and ManagedFieldsOperationUpdate) to the SSA cleanup list. I guess Lens does a Update operations, but in case they migrate to SSA, we'll still be able to undo it by adding now both operation types.

@Nalum
Copy link
Member Author

Nalum commented Nov 25, 2023

Should it override the list or just add to it?

I was thinking just appending but will follow the existing approach if that is to override the list.

I'll see what I can find on the specifics of what Lens does.

@stefanprodan
Copy link
Member

Should it override the list or just add to it?

It should add to it.

@Nalum
Copy link
Member Author

Nalum commented Nov 25, 2023

Would it be too much to add support for the user to specify the action? e.g. --override-manager "node-fetch:Apply"

Maybe this is putting too much into the users hands?

Would have logic to verify the action and log a warning if not, don't add to the list and continue to run.

@stefanprodan
Copy link
Member

stefanprodan commented Nov 25, 2023

A tool could migrate from client-side apply to server-side apply without users being aware of it. Also this flag will be set by a cluster admin, while the editors like Lens are used by devs, admins may not be aware that Lens has switched the apply procedure, it's a implementation detail. I see no reason why would someone would allow edits via SSA but deny it via client-side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/server-side-apply SSA related issues and pull requests enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants