Simplify managed fields upgrade from CSA to SSA #319
Conversation
|
Skipping CI for Draft Pull Request. |
jetstack-bot
added
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
jetstack-bot
added
the
size/L
erikgb
force-pushed
the
simplify-ssa-upgrade
branch
2 times, most recently
from
199b81c
to
8cf4122
Compare
|
/test all |
erikgb
changed the title
Signed-off-by: Erik Godding Boye <egboye@gmail.com>
erikgb
force-pushed
the
simplify-ssa-upgrade
branch
from
8cf4122
to
f7403b7
Compare
cert-manager-prow
bot
removed
the
do-not-merge/work-in-progress
|
/unhold |
cert-manager-prow
bot
removed
the
do-not-merge/hold
|
/test pull-trust-manager-verify |
|
Thanks @erikgb. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: inteon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
cert-manager-prow
bot
added
the
approved
| if !needsUpdate { | ||
| return false, nil | ||
| if patch != nil { | ||
| return true, b.client.Patch(ctx, obj, client.RawPatch(types.JSONPatchType, patch)) |
There was a problem hiding this comment.
This is quite a neat trick: instead of update, patch (JSON instead of ssa) is used so there is no extra rbac necessary.
This PR simplifies the upgrade from CSA to SSA by using the helper provided by upstream Kubernetes and also used by
kubectl. There is a theoretical change in behavior when upgrading after this PR, since the helper does not take the target configmap key into account when upgrading. Which means there could potentially be another operator/controller using the c/r regression field manager. I still think the simplification of code and RBAC is worth this minor risk.