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 <51100439+definitelynobody@users.noreply.github.com>
|
Thank you for all the hard work, I'll try to get it merged into 0.6.1 ASAP. |
Co-Authored-By: definitelynobody <51100439+definitelynobody@users.noreply.github.com>
tale
left a comment
tale
left a comment
There was a problem hiding this comment.
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.
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.
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.
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.
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.
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 |
Co-Authored-By: Stephan Deumier <github@deumier.dev>
Co-Authored-By: Stephan Deumier <github@deumier.dev>
|
@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
lucasfcnunes
left a comment
There was a problem hiding this comment.
*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.
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.
Only if headscale.config.policy.mode is 'file'
4 participants
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