Use get-ocp-range to translate Kube to OCP ranges

[mgoerens]

This work had been started as part of commit f08a985

This commit removes the mapping of Kubernetes versions to OCP versions and calls get-ocp-range instead.

closes #418

[mgoerens]

See also how this function is called:

chart-verifier/internal/chartverifier/checks/checks.go

Lines 465 to 479 in 9e4c6cc

	func certifyImages(r Result, opts *CheckOptions, registry string) Result {
	kubeVersion := ""
	kubeConfig := tool.GetClientConfig(opts.HelmEnvSettings)
	kubectl, kubeErr := tool.NewKubectl(kubeConfig)
	if kubeErr == nil {
	serverVersion, versionErr := kubectl.GetServerVersion()
	if versionErr == nil {
	kubeVersion = fmt.Sprintf("%s.%s", serverVersion.Major, serverVersion.Minor)
	}
	}
	images, err := getImageReferences(opts.URI, opts.Values, kubeVersion)
	if err != nil {
	r.SetResult(false, fmt.Sprintf("%s : Failed to get images, error running helm template : %v", ImageCertifyFailed, err))
	}

In my understanding, there is an attempt (with tool.GetClientConfig) to determine the Kubernetes version (that this chart supports ??) based on Helm related environment variables that might be set. I somehow ended here in my investigations.

In case that failed (for instance because the environment variables are not set), then there is a fallback here, that defaults to the latest known Kubernetes version (i.e. the latest set in internal/tool/kubeOpenShiftVersionMap.yaml).

I'm wondering:

1. what exactly does this Kubernetes version represents. Is this supposed to be the supported version for this chart ?
2. if defaulting to the latest Kubernetes version really makes sense here.
3. if the fallback serves a real-world scenario or is just to accommodate the tests, when the Helm env vars are not set.

I'll continue pulling those threads, but in the meantime I wanted to post my thoughts here.

[komish]

Effectively, this kubeVersion value seems to align with the server that's being used for testing, and if the server is not available, then it defaults to the newest version.

The image reference resolution code effectively emulates helm template, and seems to be emulating the design of the upstream action library: https://pkg.go.dev/helm.sh/helm/[email protected]/pkg/action.

As I read it, the summary is that the RenderManifests function that ultimately underpins getImageReferences takes in the user's helm-relevant input values (such as values.yaml), and then uses a template rendering strategy to derive manifests with their values.yaml-provided images rendered out. Otherwise, images would be templated in the manifests being scanned, and they would all fail (for being invalid). It also makes sense for cases where a partner is rendering one value for upstream Kubernetes, and another values for OpenShift.

As far as why a kubeclient is necessary, I'd have to dig deeper. Happy to do that if you'd like. I stopped at this point because I think the end goal is that a cluster being unavailable should not prevent the templating from taking place. I'm presuming that no actual calls to the cluster are made, but the upstream actions being called ask for an actions.Configuration, which requires a client. As such, the latest version is being subbed in to produce a client, regardless of whether calls to a given cluster are made - and only checks that are related to image reference resolution.

Hope that helps, let me know if I can expand.

[mgoerens]

Thanks. That helped.

After giving it a fresh look at it I found don't need to set a kubeVersion at all, hence we can remove the problematic logic from the code.

This kubeVersion variable is in fact not needed to instantiate a new Kubernetes Client. That is done here using a fake client.

It turns out that the kubeVersion that was set only serves an informative purpose. It is fed into the chartutil.Capabilities struct. By using the chartutil.DefaultCapabilities we are already provided with a default kubeVersion. I don't believe we need to set our own.

[komish]

/lgtm