Skip to content

Conversation

@enrichman
Copy link
Collaborator

@enrichman enrichman commented Nov 10, 2025

Helm doesn't handle CRDs updates automatically. To handle this different strategies are possible. The simplest is to treat CRDs like normal resources, putting them in the templates folder, and adding the helm.sh/resource-policy: keep annotation, to prevent their deletion.

This is the same strategy adopted by cert-manager or kubewarden.

If someone is upgrading from a previous release the CRDs will not have the annotations used by Helm to track the managed resource, and the upgrade will fail. To prevent this the user should upgrade specifying the --take-ownership flag, or adding manually the needed labels and annotations:

# assuming the release name is "k3k" and the namespace "k3k-system"

kubectl label    crd clusters.k3k.io --overwrite "app.kubernetes.io/managed-by=Helm"
kubectl annotate crd clusters.k3k.io --overwrite "meta.helm.sh/release-name=k3k"
kubectl annotate crd clusters.k3k.io --overwrite "meta.helm.sh/release-namespace=k3k-system"

kubectl label    crd virtualclusterpolicies.k3k.io --overwrite "app.kubernetes.io/managed-by=Helm"
kubectl annotate crd virtualclusterpolicies.k3k.io --overwrite "meta.helm.sh/release-name=k3k"
kubectl annotate crd virtualclusterpolicies.k3k.io --overwrite "meta.helm.sh/release-namespace=k3k-system"

The PR also updates the generate script to output CRDs to the correct directory and include the keep resource policy annotation.

Updated the generate script to output CRDs to the correct directory and include the keep resource policy annotation.
@codecov-commenter
Copy link

codecov-commenter commented Nov 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.63%. Comparing base (a84c49f) to head (0900230).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #552      +/-   ##
==========================================
+ Coverage   59.58%   59.63%   +0.05%     
==========================================
  Files          56       56              
  Lines        5151     5151              
==========================================
+ Hits         3069     3072       +3     
+ Misses       1798     1797       -1     
+ Partials      284      282       -2     
Flag Coverage Δ
cli 53.59% <ø> (+0.46%) ⬆️
controller 58.32% <ø> (-0.14%) ⬇️
e2e 58.32% <ø> (-0.14%) ⬇️
unit 41.52% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@enrichman enrichman marked this pull request as ready for review November 11, 2025 10:51
@enrichman enrichman self-assigned this Nov 11, 2025
@raulcabello
Copy link

kubewarden uses a separate helm chart for crds. Have you consider following the same approach?

@enrichman
Copy link
Collaborator Author

kubewarden uses a separate helm chart for crds. Have you consider following the same approach?

Yes, @bigkevmcd also told me about the two approaches. Speaking with @fabriziosestito he pointed me to the kubewarden/sbomscanner repo. Handling two charts could be very annoying, especially considering the AppCo mirroring, or the UI.

Copy link

@raulcabello raulcabello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand your point that having 2 helm chart if more complicated for AppCo and for installing k3k. The proposed approach looks good to me!

Copy link

@bigkevmcd bigkevmcd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm broadly fine with this, it would definitely be worthwhile looking at how we're doing this across products with a view to some standardisation?

@enrichman
Copy link
Collaborator Author

I'm broadly fine with this, it would definitely be worthwhile looking at how we're doing this across products with a view to some standardisation?

It would definitely makes sense, even though I have the feeling it will depends a lot by the use case.

But we can try to have at least some common patterns for similar approaches.

@enrichman enrichman merged commit 7144cf9 into rancher:main Nov 11, 2025
9 checks passed
@enrichman enrichman deleted the crds-templates branch November 11, 2025 15:23
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.

4 participants