Skip to content
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

Manage labels and annotations for a HelmChart #631

Merged
merged 2 commits into from
Mar 29, 2023

Conversation

raffis
Copy link
Contributor

@raffis raffis commented Mar 6, 2023

Current situation

It is not possible to manage labels and/or annotations for a HelmChart via the owning HelmRelease resource.
I would expect that I can label the chart via the HelmChart template.
Currently I would need to manage the HelmChart separately in order to maintain some labels.

Proposal

Add support for setting metadata.labels and metadata.annotations.
This pr extends the HelmRelease api.

Note: As it is currently in this pr the labels and annotations are overwritten by the metadata defined in the helmrelease https://github.com/fluxcd/helm-controller/compare/main...raffis:helm-controller:feat-chart-metadata?expand=1#diff-6cc8d46119f3c5f338d63957e402c55243c2abd7dc1d349bd057ce55e219e546R83-R84

Meaning changes done to the HelmChart directly would be overwritten by the helm-controller again.

Comment on lines 248 to 252
case !reflect.DeepEqual(template.Annotations, chart.Annotations):
return true
case !reflect.DeepEqual(template.Labels, chart.Labels):
return true
Copy link
Member

Choose a reason for hiding this comment

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

Maps have a random order, and the comparison must thus be semantic. See e.g.

https://github.com/fluxcd/pkg/blob/main/ssa/manager_diff.go#L151

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hiddeco thx for the review. Fixed now. Note this pr is somewhat related to: fluxcd/notification-controller#482 (or at least thats the reason I added this feature in the first place)

@hiddeco hiddeco added the enhancement New feature or request label Mar 7, 2023
@hiddeco
Copy link
Member

hiddeco commented Mar 10, 2023

Please git rebase instead of merging main into your branch.

@raffis raffis force-pushed the feat-chart-metadata branch 3 times, most recently from 0a82fc4 to 9b69872 Compare March 16, 2023 14:50
@raffis raffis requested a review from hiddeco March 20, 2023 16:27
- Assing `ObjectMeta` field in Helm chart template.
- Ensure things are at least lightly mentioned in spec documentation.
- Add two simple test cases.
- Fix broken links to Kubernetes documentation.

Signed-off-by: Hidde Beydals <[email protected]>
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @raffis and @hiddeco

@hiddeco hiddeco merged commit 5a1c513 into fluxcd:main Mar 29, 2023
@hiddeco hiddeco changed the title feat: manage label and annotations for a helmchart Manage labels and annotations for a HelmChart Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants