Mark chart.spec.version as specific to HelmRepository source in example

[tjanson]

In my opinion it is not visible enough that chart.spec.version is only considered for HelmRepository sources (and silently ignored otherwise.
I'm aware that the text below the example mentions this, but I think it would be benefitial for the code example itself to make that fact clear.

Adding this comment is only a suggestion (which I'm not 100% happy with myself) -- feel free to substitute a different way of pointing out the connection between HelmRepository and the version spec.

---

More generally, have you considered throwing an error when chart.spec.version is specified with e.g. a GitSource containing a different version of the chart (rather than using it despite the version mismatch)? The current behavior is not what I personally would expect.

[kingdonb]

Thanks for your feedback @tjanson ! I am always looking for ways to make the Helm Controller docs more accessible.

I'm not sure if this is the change either, but it might be a step in the right direction. If it is to be merged, please sign off your commit as described in the failing DCO merge check: https://github.com/fluxcd/website/pull/709/checks?check_run_id=4881437581

What people usually struggle more with, IMHO, is that GitRepository sources aren't using the Revision strategy by default. https://fluxcd.io/docs/guides/helmreleases/#release-when-a-source-revision-changes-for-git-and-cloud-storage

Maybe somewhere in the "Manage Helm Releases" guide, we can add something about this too. It's good to shout out all of the ways that Helm Controller can behave differently when it is used with a HelmRepository and without one.

[tjanson]

What people usually struggle more with, IMHO, is that GitRepository sources aren't using the Revision strategy by default.

Indeed, I just learned about that too.

Anything that increases visibility of the fact that chart.spec.version is silently ignored in any case other than a HelmRepository is a good step imho. Maybe the sentence that mentions this should be in an "attention"-styled infobox.

Has this behavior been brought up as an issue before? I assumed it was likely already discussed and decided.
It seems to me that following the principles of least astonishment and failing early, a chart.spec.version should be strictly forbidden unless the source is a HelmRepository.

[kingdonb]

No I don't think anyone has ever raised this issue before, I would suggest that you file it separately on the helm-controller.

I'm sure that ignoring the version is desired behavior, since the only thing that can effectively be deployed from is the thing at the HEAD of the repository ref. What else is surprising though is that HelmCharts, since they are immutable, are only generated once whenever the Chart.Version is first observed being incremented – IFF you're using a GitRepository source – but if a HelmRepository source swaps out an existing chart version for a changed or mutated copy, on the other hand, it WILL result in changes on the cluster. The HelmChart has its own reconciling interval, on which it attempts to re-fetch the chart – and if it finds it has changed, then Helm Controller happily overwrites the old version with a new one AIUI.

What's worse is that if you think about it hard enough, these are both the least surprising behavior, given the medium.

But, accepting a version parameter and not honoring it in any way for GitRepositories is definitely a surprising twist. I think it is fair to call that a bug.

I actually feel like that's a good "read the docs" trigger action – if the user passes in a HelmRelease with a GitRepository source and a spec.chart.spec.version set, that's as clear a signal that you can hope for that they haven't yet read all of the docs, we should balk and link them to the section of the HelmRelease guide I linked above, with a cheeky admonishment along the lines of "There are some things you'll need to know about using HelmRelease with GitRepository sources..."

I'm absolutely not saying we've definitely covered everything as well as we can already in the docs, I just do think there is a decent chance that building the widest funnel (shunting people toward docs most effectively when it's most needed) is as important if not more than getting the actual content of the docs right. 👍