Fix roundtrip conversions between v1alpha2 and v1alpha3 #2074
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/M
|
cc @ncdc |
sedefsavas
force-pushed
the
conversion_bastion
branch
from
01e944a to
08f2f0d
Compare
|
Are there some tests we could add that would have picked up on the issues? |
|
@randomvariable yes, we need to make the change in CAPI though |
|
Added a reverse fuzz test: kubernetes-sigs/cluster-api#3877 I could add individual tests as well for the 3 fields I changed here. |
|
If an upstream CAPI change is going to deal with it, that's fine. /lgtm I'll leave @ncdc to do the approve given the greater context you have. |
k8s-ci-robot
added
the
lgtm
| if restored.CloudInit.SecureSecretsBackend != "" { | ||
| if src.CloudInit != nil { | ||
| dst.CloudInit.SecureSecretsBackend = restored.CloudInit.SecureSecretsBackend | ||
| } |
There was a problem hiding this comment.
Back when I added the secure secrets stuff, I had originally also added
before removing it thinking it was excessive.
Do we in fact need to conditionally restore the SecretARN here too?
There was a problem hiding this comment.
SecretARN turned into SecretPrefix + SecretCount.
There was a problem hiding this comment.
And SecretPrefix + SecretCount exist in both a2 and a3
|
/retitle Fix roundtrip conversions between v1alpha2 and v1alpha3 |
k8s-ci-robot
changed the title
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: randomvariable 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 |
k8s-ci-robot
added
the
approved
What this PR does / why we need it:
This PR prevents overriding ControlPlaneLoadBalancer and Bastion changes.
It also avoids creating
CloudInitduring the conversion if it is empty, because if we create an emptyCloudInitduring conversion,out.EnableSecureSecretsManager = !in.InsecureSkipSecretsManagersets a field that should not be there.``
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #2059