Add KDP - Mutate Existing Resources

[realshuting]

Signed-off-by: ShutingZhao [email protected]

This KDP addresses:

• Mutate target resource which is different from watched resource kyverno#2139
• Post mutation kyverno#1722
• Mutate existing resource on policy update kyverno#1607

[chipzoller]

What I don't see here is a pseudo policy on the background mutation use case (i.e., where there is no triggering resource). I think that's the primary use case for background mutation here. Couple suggestions on that:

1. needs to be a way in a policy to differentiate select between "new" resources which can be mutated (CREATE or UPDATE) and "old" resources which should be mutated (those which already exist)
2. needs parallel support in the Kyverno CLI with the apply command, similar to background generation, to allow users to understand the impact such a policy may have on existing resources.

[realshuting]

@chipzoller - sorry I missed your comments. For some reason the notification goes into spam :(

To answer your questions:

1. needs to be a way in a policy to differentiate select between "new" resources which can be mutated (CREATE or UPDATE) and "old" resources which should be mutated (those which already exist)

1.1. ""new" resources which can be mutated" - can be defined in spec.rules.mutate.targets;

1.2. ""old" resources which should be mutated (those which already exist)" - I assume old and new resources are the same in this case? If so, such use cases can be addressed by example 3, when the policy is installed, Kyverno will mutate existing resources.

2. needs parallel support in the Kyverno CLI with the apply command, similar to background generation, to allow users to understand the impact such a policy may have on existing resources.

Good point! I've added it in the requirement.

[chipzoller]

@chipzoller - sorry I missed your comments. For some reason the notification goes into spam :(

To answer your questions:

1. needs to be a way in a policy to differentiate select between "new" resources which can be mutated (CREATE or UPDATE) and "old" resources which should be mutated (those which already exist)

1.1. ""new" resources which can be mutated" - can be defined in spec.rules.mutate.targets;

But doesn't mutate.targets imply the use case where the target is different from the trigger? What if that's not the case and trigger = target? Seems that the existing way to write a mutate policy would suffice here for "new" resources.

1.2. ""old" resources which should be mutated (those which already exist)" - I assume old and new resources are the same in this case? If so, such use cases can be addressed by example 3, when the policy is installed, Kyverno will mutate existing resources.

I think there needs to be a clear way to distinguish between the two. There are three permutations here:

1. I want to mutate only "new" (incoming) resources but not pre-existing ones.
2. I want to mutate both "new" and "old" resources.
3. I want to mutate only "old" resources.

[realshuting]

1. I want to mutate only "new" (incoming) resources but not pre-existing ones.

The existing mutate policy can support this.

2. I want to mutate both "new" and "old" resources.

2.1 "new" and "old" are the same - example 3, setting mutateExisting=true

2.2 "new" and "old" are different - example 1 or 2

3. I want to mutate only "old" resources.

Good point, we can let the user set mutateExisting=false to support this. And we set the default value to nil/null.

[chipzoller]

I went back and studied this a bit more and will add more thoughts and comments. I identify three use cases for this enhancement. Let me know if these sound right:

1. Mutate existing resources once
2. Mutate existing resources upon policy update (sync mutation)
3. Mutate resources which are different from the triggering resource

Should we provide for distinction between use cases 1 and 2? Use case for 1 is a one-time mutation whereas use case for 2 is a continual mutation, similar to a generate rule with sync: true enabled.

Also at least one use case that we might want to call as out of scope:

1. Rolling back mutations in any of the above use cases

Couple additions I thought of we should consider:

1. Generate an event on existing resources if they were mutated.
2. Have a new (multiple?) metric corresponding to existing resources that were mutated if not covered by a current metric.

[realshuting]

Should we provide for distinction between use cases 1 and 2? Use case for 1 is a one-time mutation whereas use case for 2 is a continual mutation, similar to a generate rule with sync: true enabled.

Just a clarification of the current proposal, there's no support as of now to mutate resources once. Like the existing mutate policies, resources will be mutated whenever there's an admission operation. The only difference with this feature is that the trigger can be from two sources:

• trigger resource events via admission requests
• policy events

If one-time mutation is something we want to support, we can extend this flag spec.rules.mutate.mutateExisting to accept different modes, say "Always" and "Once", instead of true and flase.

[realshuting]

Also at least one use case that we might want to call as out of scope:

Rolling back mutations in any of the above use cases

This is an interesting use case, I'm not sure if the previous state is stored and we can use that for rolling back since mutation happens during admission review. Do you know if kubectl rollout can revert admission review changes?

Generate an event on existing resources if they were mutated.

Yes, will support.

Have a new (multiple?) metric corresponding to existing resources that were mutated if not covered by a current metric.

Good catch, will need to check this during implementation.

[realshuting]

I've updated mutate.target structure to re-use the same target in generate policies. Otherwise changes were completed.

Can we merge this @prateekpandey14 @chipzoller @JimBugwadia ?

[chipzoller]

I think so. I have resolved my comments.

[realshuting]

@prateekpandey14 @JimBugwadia - needs another approval :)

[JimBugwadia]

Should we provide for distinction between use cases 1 and 2? Use case for 1 is a one-time mutation whereas use case for 2 is a continual mutation, similar to a generate rule with sync: true enabled.

Just a clarification of the current proposal, there's no support as of now to mutate resources once. Like the existing mutate policies, resources will be mutated whenever there's an admission operation. The only difference with this feature is that the trigger can be from two sources:

• trigger resource events via admission requests
• policy events

If one-time mutation is something we want to support, we can extend this flag spec.rules.mutate.mutateExisting to accept different modes, say "Always" and "Once", instead of true and flase.

I don't follow....if the user wants to apply the rule on existing resources in the cluster, will we not apply it as a background operation when the rule is created or modified?

[realshuting]

I don't follow....if the user wants to apply the rule on existing resources in the cluster, will we not apply it as a background operation when the rule is created or modified?

As stated here:

This feature is can be triggered from two sources:

• trigger resource events via admission requests
• policy events

The "policy events" means when a policy/rule is added/updated/removed.

[chipzoller]

@realshuting what do we do with this now it's implemented yet the PR was never merged? Do we merge for posterity's sake or close?

[realshuting]

@realshuting what do we do with this now it's implemented yet the PR was never merged? Do we merge for posterity's sake or close?

I'll update to the current implementation today and we can then merge.

[realshuting]

@chipzoller - I have updated the KDP to reflect the current implementation.

[chipzoller]

Needs second review by @JimBugwadia