chore: bumping helm chart sematically

[callum-tait-pbx]

Chart verison needs bumping again, I did it right this time 😅

[callum-tait-pbx]

I've removed the app version config as it isn't really a thing with this chart and is optional anyway. The underlying controller container version maybe defines what app version is being ran and this can be configured in the chart values.yaml and so is independant of the chart itself.

Lints OK outside of the deprecating errors, these are highlighted as needing bumping on #144 and should probably be looked at sooner rather than later @mumoshu 🙏 😅 , this isn't a Helm problem though:

==> Linting actions-runner-controller/
[INFO] Chart.yaml: icon is recommended
[WARNING] templates/webhook_configs.yaml: admissionregistration.k8s.io/v1beta1 MutatingWebhookConfiguration is deprecated in v1.16+, unavailable in v1.22+; use admissionregistration.k8s.io/v1 MutatingWebhookConfiguration
[WARNING] templates/webhook_configs.yaml: admissionregistration.k8s.io/v1beta1 ValidatingWebhookConfiguration is deprecated in v1.16+, unavailable in v1.22+; use admissionregistration.k8s.io/v1 ValidatingWebhookConfiguration

[callum-tait-pbx]

@mumoshu Just noticed you use the .Chart.AppVersion in your webhook work. Could you just use the charts values.yaml as the source for the default value? The AppVersion in the chart is super misleading as the app version isn't really tied to the chart at all so is probably best removed entirely to avoid confusion?

The concept of a static app verison with this chart breaks down quickly becoming meaningless. Defining the app version can be complicated to start with, is it the controller container version? Is it the kube-proxy container version? Is it the dind container version? Even if we agree that the controller container is the version it's a bit meaningless for this project with regards to Helm as the charts AppVersion doesn't tell you anything about what version of the app is actually being ran. The various container verison, including the controller, can be changed without deploying a new version of the chart by deploying a new values.yaml instead. As a result the AppVersion your Helm deployment states can be completely and totally wrong, moreover it is likely to be wrong as most changes don't involve changes to the chart but to the underlying container codes instead.

Additionally, removing it would also simplify bumping the chart version when a release is needed.

EDIT Worded better

[callum-tait-pbx]

@mumoshu any thoughts on this in relation to #302 and #144

[mumoshu]

@callum-tait-pbx Hey! I was thinking that appVersion could be used to tell the minimum controller version that is supposed to work with that version of the chart. Without it, we have no way to say which chart is supported to work with which version of the controller at all, which might make user support harder.

appVersion can be a few versions behind the latest controller release, but it will still serve the purpose.

Thoughts?

[callum-tait-pbx]

I'm totally fine with keeping the appVersion if you want, I removed it because the default comment that is included if you scafold a chart file is:

# This is the version number of the application being deployed. This version number should be
# incremented each time you make changes to the application. Versions are not expected to
# follow Semantic Versioning. They should reflect the version the application is using.

This obviously isn't true for this project as the values.yaml is defining what version of the application is deployed. Happy to re-add it if you think it's needed and just include a comment that says somethig like

# This is the minimum controller version that this version of the chart supports.
# Increment only when the minimum version has changed.

This just might be confusing to anyone who is used to Helm as they won't see that comment if they are deploying from this repository and so will assume the appVersion is the normal definition making helm list commands confusing and probably wrong.

Let me know if you want it re-added, happy to do so. I would however shy away from using it as the source version for any aspect of the templating and instead rely on the values.yaml file for default values. So I would remove the default (cat "v" .Chart.AppVersion entries in the templates. If you disagree I'm happy to leave them in, if not let me know and I'll include the removal of references to the AppVersion as part of this PR?

[mumoshu]

Makes sense! Let's go ahead without appVersion. In the future, we may revisit once anyone asks about which chart version supports which version of controller - but I believe we have no strong reason to do so right now 😃

Thanks for the detailed feedback and the fix, @callum-tait-pbx!