-
Notifications
You must be signed in to change notification settings - Fork 182
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
GA: Promote Kustomization API to kustomize.toolkit.fluxcd.io/v1
#822
Conversation
Signed-off-by: Stefan Prodan <[email protected]>
a091558
to
67f19f2
Compare
Signed-off-by: Sanskar Jaiswal <[email protected]>
Signed-off-by: Stefan Prodan <[email protected]>
Signed-off-by: Stefan Prodan <[email protected]>
Signed-off-by: Stefan Prodan <[email protected]>
Signed-off-by: Stefan Prodan <[email protected]>
Signed-off-by: Sanskar Jaiswal <[email protected]>
Signed-off-by: Sanskar Jaiswal <[email protected]>
Signed-off-by: Sanskar Jaiswal <[email protected]>
Signed-off-by: Sanskar Jaiswal <[email protected]>
e56c397
to
9764b8d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good enough to me to be released.
In terms of the spec doc, I think it could use another proof read to see if more interlinking between sections is possible, and to align tone of voice between different sections. In addition to reducing the usage of "kustomize-controller" as at some point based on the context it can just be called "controller", and other nits like this. But not blocking to move forward, as structure wise it's 💯.
Thanks @stefanprodan and @aryan9600 🙇 🥇
i completely agree. one particular instance that comes to mind is how can we make the sops guide and the decryption section more compatible/co-existent? |
Another thing I ran into is that we first talk about
but then later have a section on how this default can be changed. I think this should eventually be rewritten in a way which brings the override more to the foreground, as people who read lazily may have had their alarm bells ring before they actually notice this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some suggestions in the API spec doc. There are some references of GitRepository instead of Kustomization which need to be fixed, but the other suggestions can be used or ignored as they may be subjective.
2fc216b
to
a91e6a6
Compare
Signed-off-by: Hidde Beydals <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
API changes
The
Kustomization
kind was promoted from v1beta2 to v1 (GA) and deprecated fields were removed.The
kustomizations.source.toolkit.fluxcd.io
CRD contains the following versions:Upgrade
The
Kustomization
v1 API is backwards compatible with v1beta2 with the following exceptions:.spec.validation
was removed.spec.patchesStrategicMerge
was removed (replaced by.spec.patches
).spec.patchesJson6902
was removed (replaced by.spec.patches
)To upgrade from v1beta2, after deploying the new CRD and controller, set
apiVersion: kustomize.toolkit.fluxcd.io/v1
in the YAML files that containKustomization
definitions and remove the deprecated fields if any. Bumping the API version in manifests can be done gradually. It is advised to not delay this procedure as the beta versions will be removed after 6 months.Closes: #755
Closes: #664