-
-
Notifications
You must be signed in to change notification settings - Fork 110
Add helm chart #240
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
base: main
Are you sure you want to change the base?
Add helm chart #240
Conversation
that wasnt supposed to be there
|
Whenever you're ready for review and merge, feel free to change it off of a draft and I'll take a look. Take your time though, we have a long time to go before 0.6.1! |
|
Hey, added some few things and I think it's looking good, already using this chart on my personal and work clusters, headplane is so nice, thanks for bringing this awesome app to the community :) |
This is only applicable if the user choose policy mode file, but without this value the configmap is not being created
Co-Authored-By: definitelynobody <[email protected]>
|
Thank you for all the hard work, I'll try to get it merged into 0.6.1 ASAP. |
Co-Authored-By: definitelynobody <[email protected]>
tale
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from nits related to the packaging and distribution of the charts, this looks good to me.
| ### Install the Chart | ||
| ```sh | ||
| # Install with default values | ||
| helm install headplane oci://harbor.lag0.com.br/library/headplane |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise a good overall PR, wondering why we're defaulting to the Harbor hosted instance, is there not a way to just pull it directly from the GitHub repository or publish a chart repository using GitHub pages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use Harbor just because I like self-hosting stuff lol, no objections for creating a workflow to push this to another chart repository, maybe you can setup a workflow based on this one: https://github.com/antoniolago/headplane-chart/blob/main/.github/workflows/publish-helm-chart.yml (tests are included in this PR already)
| Consider using a secrets management solution in production like external-secrets. | ||
|
|
||
| ## License | ||
| Copyright © 2025 antoniolago |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer if this isn't licensed, especially as Apache because its language doesn't necessarily fit well with MIT the best.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure we can remove Apache license since the code is forked from https://github.com/nbcloudio/headplane-chart unless all contributors agrees, or am I wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then in that case, we do need their permission yes, and I can't technically merge this in-tree until it's been addressed.
| Note: Make sure to keep your secrets secure and never commit them to version control. | ||
| Consider using a secrets management solution in production like external-secrets. | ||
|
|
||
| ## License |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same concerns as earlier
Co-Authored-By: Stephan Deumier <[email protected]>
Co-Authored-By: Stephan Deumier <[email protected]>
|
@antoniolago @tale - I have been using this helm chart locally but saw you were contributing a fixed version of the existing one so just kept it to myself. If licensing is a concern, I am happy to provide that under MIT as an option - no hard feelings at all if this one works out and you decide to close my PR If you find it helpful, @antoniolago I would love to collaborate with you on bringing this up to standard |
Hello, nice work you're doing, havent got the opportunity to test yet but it looks like you know what you are doing, at the moment I've reached a goal state to >my specific requirements< with headplane's infra, this PR and the chart repository is a way to share this but it's not necessarely the best chart option, tho we could ask the 4 people involved if they agree to change license theres a certainty on your solution to the license issue, so maybe we should actually go for it! At the moment I'm changing obsessions lol, but I'll keep an eye on this for sure. |
lucasfcnunes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*multiple files have missing new line at EOF
| cp /headscale-config/config.yaml /headscale-data/config.yaml | ||
| cp /headscale-acl/acl.hujson /headscale-data/acl.hujson |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't this going to overwrite changes made from the ui with the values in the configmap and secret?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only if headscale.config.policy.mode is 'file'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about dns configs?
As discussed in #55, users should have helm chart as option for deploying headplane, I forked an existing chart and fixed a few things.
I will keep https://github.com/antoniolago/headplane-chart/ as is, but the mantainers may want the helm's code into this repo tree so other devs can use and enhance it in a more official way.
I used my personal OCI registry on README, feel free to change this or any other thing.
I have deployed and tested >some< features and it worked (basic admin access, add device, edit acl via values.yaml) so it definetly should be more tested.
Edit:
Using this chart at a production environment and everything looks good