Panic when creating Cluster using non-semver tag

[tankbusta]

Similar to #551, If you attempt to create a HumioCluster with spec.image set to a tag such as snapshot or preview, it'll panic inside humiocluster_version.go. This is due to the constraint failing to parse and constraint is nil.

humio-operator/controllers/humiocluster_version.go

Lines 67 to 70 in b7c2429

	func (hv *HumioVersion) constraint(constraintStr string) (bool, error) {
	constraint, err := semver.NewConstraint(constraintStr)
	return constraint.Check(hv.version), err
	}

[SaaldjorMike]

I would never recommend you to use tags called snapshot or preview. At least not with the current implementation.
Right now the current implementation assumes it knows what version of LogScale is actually being used for the cluster pods, and if you "bring your own random tags" then we can't properly parse them up and understand if they e.g. are "At least LogScale version 1.70.0" or similar things. Changes happen to the LogScale API and sometimes we need to change how the operator works depending on which version of LogScale it manages.

The current code has special meaning to tags latest and master as they are assumed to always be the very bleeding edge of LogScale.

The nil pointer dereference is obviously not ideal here and is definitely something that needs to be fixed so it is handled better and not crashes the entire operator.

If we assume we merge in this change and configure image: humio/humio-core:snapshot, this would mean that we end up calling https://github.com/humio/humio-operator/blob/master/controllers/humiocluster_version.go#L40-L43
which just calls semver.NewVersion("preview") and semver.NewVersion doesn't understand that tag, which would return nil and an error with the text Invalid Semantic Version. This would mean we end bubbling up the error to where we do things like humioVersion.AtLeast() which will also fail with nil pointer dereference because humioVersion would be nil in that case since we for the most part don't handle error cases for errors returned by HumioVersionFromString() and just try calling AtLeast() no matter what HumioVersionFromString() returns.

Question is more: should we just add snapshot/staging to the list of "known assumed-bleeding-edge" container image tags? Or should we change the default behavior so that it always assumes anything not semver compatible is always assumed to be bleeding edge?

[SaaldjorMike]

Perhaps consider #752
That will effectively treat any "non-standard" image tags that doesn't include version hints as bleeding edge.
WDYT? cc @jswoods

[tankbusta]

I would never recommend you to use tags called snapshot or preview. At least not with the current implementation.
Right now the current implementation assumes it knows what version of LogScale is actually being used for the cluster pods, and if you "bring your own random tags" then we can't properly parse them up and understand if they e.g. are "At least LogScale version 1.70.0" or similar things.

Yeah at first I didn't know this but then when I went to go fix & create the issue I learned how tied the operator is to versions :)

The nil pointer dereference is obviously not ideal here and is definitely something that needs to be fixed so it is handled better and not crashes the entire operator.

Agreed.

I think #752 makes sense but I'd also still consider the changes in #751 alongside it.

This could be dangerous though if you're on a bleeding edge version as it'll be nil.

humio-operator/controllers/humiocluster_version.go

Lines 54 to 56 in 7354ed9

	func (hv *HumioVersion) SemVer() *semver.Version {
	return hv.version
	}