Helm version verification fails when using RedHat's Helm build

[etherealy]

Hello there,

My infrastructure team recently updated the docker image we use for most of our CI/CD and substituted Helm's official binary for RedHat's. (We use Redhat's Openshift)
Since then, CT has been unusable and returns Invalid Semantic Version errors.

We could have it just rolled back, but I thought I'd have a look into it...

How to reproduce

• Grab any of the RedHat's build from their repository

For example at the time of this writing the latest version returns:
version.BuildInfo{Version:"v3.9.0+3.el8", GitCommit:"e09b16a5119e20607a4a7ae9884a96331ffff1de", GitTreeState:"clean", GoVersion:"go1.17.7"}

• Make this version the one resolved by your path

• Run, for example, ct lint

This will produce the following output:

> ct lint
Linting charts...
Error: Invalid Semantic Version
Invalid Semantic Version

Diagnostic

What is happening?

The error occurs here:

chart-testing/pkg/chart/chart.go

Lines 279 to 282 in 1d3feac

	version, err := semver.NewVersion(versionString)
	if err != nil {
	return testing, err
	}

While debugging, it appears that the versionString passed is equal to v3.9.0+3.el8+ge09b16a.

Which indeed does not match the SemVer regex found here:
^v?([0-9]+)(\.[0-9]+)?(\.[0-9]+)?(-([0-9A-Za-z\-]+(\.[0-9A-Za-z\-]+)*))?(\+([0-9A-Za-z\-]+(\.[0-9A-Za-z\-]+)*))?$

Looking at https://semver.org/, multiple + does not seem to be a valid syntax.

Why is the version rendered as such?

Looking at the following:

chart-testing/pkg/tool/helm.go

Lines 92 to 94 in 1d3feac

	func (h Helm) Version() (string, error) {
	return h.exec.RunProcessAndCaptureStdout("helm", "version", "--short")
	}

Helm's short version is used, which is rendered by Helm as seen here, by concatenating the version followed by +g and then the first 7 characters of the GitCommit.

As stated above, the full version is version.BuildInfo{Version:"v3.9.0+3.el8", GitCommit:"e09b16a5119e20607a4a7ae9884a96331ffff1de", GitTreeState:"clean", GoVersion:"go1.17.7"}, hence the incorrect version format.

How to fix this?

This is not for me to say, obviously. :)

The options I identify, are the following:

Change chart-testing's Helm.Version() implementation

One thing that could be done is to parse Helm's long version to only retrieve the Version part instead of using Helm's short version.
It seems like only the Major component is used, to make sure that Helm's version is > 3

+ Easy and quick to fix, only involves changes in CT
- Does not fix the fact that Helm returns an improper SemVer with the --short flag

Change Helm's formatVersion to handle the special case of the Version already containing a "build" component

A fix could be operated in Helm's formatVersion function to handle this case of the Version already containing a "build" component ((\+([0-9A-Za-z\-]+(\.[0-9A-Za-z\-]+)*))).
It could for example, in this case, use . as a separator instead of a +, as it is supported by the SemVer spec.

+ Properly fixes the problem
- Requires to make changes in Helm and then have to wait for new version and RH build

My preference goes for the second one, but I'd love to have your take on this!

I'd happily open a PR if needed! :)

[dperaza4dustbit]

Interesting, we are looking into this.