Skip to content

Allow diff to include crds #762

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

Merged
merged 2 commits into from
Apr 8, 2025

Conversation

hubertbits
Copy link
Contributor

Goal

This PR allows usage of helm template flag --include-crds during a helm diff upgrade.

Before

When using helmfile with helm diff upgrade command you have to use post-renderer to get the fully rendered manifest and allow rendering of all resources including CRDs that are deployed via a sub-chart i.e. kyverno.

Use Case

In order to be able to move a way from post-renderer and use kustomization builtin support of helmfile without breaking the manifest that you can get via helm get manifest -n namespace release it is needed to allow rendering the included crds if the chart decided to render these via a dependency.

It also makes three-way-merge more usefull as you now also get the diff for included crd changes.

After

Merging this PR into an upcoming release would allow anyone to be able to catch changes to included crd changes during a helm diff upgrade.

Breaking Changes?

None to be expected, the default is set to false which reflects the current behavior. You have to pass --include-crds excplitictly to also render included crds when calling template before diff when using three-way-merge
We used helm diff upgrade command in conjunction with helmfile with kustomize postrenderer.

Follow up

helmfile maintainers could add this flag to helmfile diff command in future versions. Using just the patched version would need you to add - "--include-crds" to helmDefaults.diffArgs array.

(at)Others who read this - please upvote if you find this PR useful.

@AMOKANDY
Copy link

AMOKANDY commented Apr 4, 2025

Fine!. But please add the new option to “readme.md”.

@yxxhero
Copy link
Collaborator

yxxhero commented Apr 8, 2025

@obirhuppertz did you test this feature?

@hubertbits
Copy link
Contributor Author

@yxxhero yes, I did also made sure its non breaking by setting default to false i.e. not include the crds as it was before.

@yxxhero yxxhero merged commit 1cf06fa into databus23:master Apr 8, 2025
16 checks passed
@yxxhero
Copy link
Collaborator

yxxhero commented Apr 8, 2025

@obirhuppertz done. could add feature PR for helmfile?

@hubertbits
Copy link
Contributor Author

@yxxhero thanks! Will do.

@hubertbits hubertbits deleted the template-includeCRDs-flag branch April 9, 2025 08:43
yxxhero pushed a commit that referenced this pull request May 9, 2025
* Allow diff to include crds

* Add new include-crds flag to README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants