Skip to content

refactor:crd deployment#952

Merged
sgaist merged 4 commits into
mainfrom
refactor/crd-deployment
Jul 2, 2025
Merged

refactor:crd deployment#952
sgaist merged 4 commits into
mainfrom
refactor/crd-deployment

Conversation

@sgaist

@sgaist sgaist commented Jun 18, 2025

Copy link
Copy Markdown
Collaborator

Describe your changes

This PR refactors the CRD deployment part of the amalthea and amalthea-session helm charts.

The management of the CRD, while looking simple, is in fact quite complex in its consequences.
Hence we will follow the current recommendation of the Helm maintainers.
More information about the reasoning can be found at:

https://github.com/helm/community/blob/main/hips/hip-0011.md

The RBAC cleanup will also allow for deployment on OpenShift.

@sgaist sgaist requested review from a team and olevski as code owners June 18, 2025 10:01
@sgaist sgaist force-pushed the refactor/crd-deployment branch 2 times, most recently from ef851a8 to 73d79e1 Compare June 24, 2025 12:49

@olevski olevski left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just have one question about the RBAC changes.

Comment thread helm-chart/amalthea/templates/rbac/_rbac_rules.tpl
@olevski olevski self-requested a review July 1, 2025 15:47
sgaist added 4 commits July 1, 2025 17:48
This follow the rational expressed by the helm authors:
- <https://github.com/helm/community/blob/main/hips/hip-0011.md>

TLDR: modifying a CRD (update, deletion) should be
a manual process because the ramifications are
quite complex (CR deletion for example).

CRD deployment can be skipped using the --skip-crds
option.

Some related issues:
- <helm/helm#5871>
- <helm/helm#7735>
- <helm/helm#10585>
This can be done at helm level using the --skip-crds
option since the CRDs have been moved to the crds
folder.
While it's possible to have multiple apiGroups and
resources, the list made would not apply on
OpenShift. The generated lists would contain invalid
entries such as networking items in the apps group
and all the combinations possible.
The original code appends to the file rather than
overwriting its content.
@sgaist sgaist force-pushed the refactor/crd-deployment branch from 73d79e1 to 745fa66 Compare July 1, 2025 15:48
@sgaist sgaist merged commit a6bd2a4 into main Jul 2, 2025
21 of 22 checks passed
@sgaist sgaist deleted the refactor/crd-deployment branch July 2, 2025 09:50
olevski added a commit that referenced this pull request Jul 4, 2025
olevski added a commit that referenced this pull request Jul 4, 2025
olevski added a commit that referenced this pull request Jul 7, 2025
olevski added a commit that referenced this pull request Jul 7, 2025
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.

2 participants