Skip to content

Conversation

@ChrisJBurns
Copy link
Collaborator

@ChrisJBurns ChrisJBurns commented Nov 29, 2025

Summary

This PR addresses the CRD lifecycle management issues described in stacklok/stacklok-epics#175 by moving CRDs from crds/ into templates/ to take advantage of standard Helm upgrade capability.

Problem: Helm does not upgrade CRDs during helm upgrade operations (#2518). CRDs placed in the crds/ directory are only processed during initial installation - subsequent upgrades silently leave CRDs unchanged, even when the chart contains updates. This led to users having stale CRDs without any indication (#2311).

Solution: Following the common patterns, CRDs are now placed in templates/ instead of crds/. This ensures CRDs are upgraded alongside the chart during helm upgrade. We have additionally wrapped the CRDs with Helm template conditionals to provide flexibility:

  • crds.install - Enable/disable CRD installation (for users who prefer kubectl apply)
  • crds.skip - List of specific CRDs to exclude from installation
  • crds.keep - Adds helm.sh/resource-policy: keep annotation to prevent CRD deletion on uninstall

Implementation:

  • Created a simple Go script in deploy/charts/operator-crds/crd-helm-wrapper/ that wraps raw CRD YAML files with the desired Helm conditionals to allow for feature flagging crd installation, keeping, and skipping.
  • Uses embedded .tpl template files for maintainability

Related issues: #2518, #1779, #2311

Large PR Justification

The changes in this PR are all auto-generated CRD definitions.

Signed-off-by: ChrisJBurns <[email protected]>
Signed-off-by: Chris Burns <[email protected]>
Signed-off-by: Chris Burns <[email protected]>
Signed-off-by: Chris Burns <[email protected]>
@github-actions github-actions bot added the size/XL Extra large PR: 1000+ lines changed label Nov 29, 2025
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.


This review will be automatically dismissed once you add the justification section.

@codecov
Copy link

codecov bot commented Nov 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.53%. Comparing base (ab7f542) to head (02e25f9).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2809   +/-   ##
=======================================
  Coverage   56.53%   56.53%           
=======================================
  Files         319      319           
  Lines       30887    30887           
=======================================
  Hits        17462    17462           
  Misses      11931    11931           
  Partials     1494     1494           

☔ 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large PR: 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants