Repository: redhat-certification/chart-verifier (go)

Issue:
Chart verifier error: Missing OpenShift version. "openshiftVersion" not found in 'oc version' output. And the 'openshift-version' flag has not set.
if you run chart verifier after  https://github.com/redhat-certification/chart-verifier/pull/115

and you run the verifer without oc installed:

```docker run -v $(pwd):/charts --rm quay.io/redhat-certification/chart-verifier:278be83 verify /charts/chart-0.1.0-v3.valid.tgz```

you get this, both running the command line and the docker image: 
```
Error: Missing OpenShift version. "openshiftVersion" not found in 'oc version' output. And the 'openshift-version' flag has not set.
Usage:
  chart-verifier verify <chart-uri> [flags]

Flags:
  -S, --chart-set strings           set values for the chart (can specify multiple or separate values with commas: key1=val1,key2=val2)
  -G, --chart-set-file strings      set values from respective files specified via the command line (can specify multiple or separate values with commas: key1=path1,key2=path2)
  -X, --chart-set-string strings    set STRING values for the chart (can specify multiple or separate values with commas: key1=val1,key2=val2)
  -F, --chart-values strings        specify values in a YAML file or a URL (can specify multiple)
      --debug                       enable verbose output
  -x, --disable strings             all checks will be enabled except the informed ones
  -e, --enable strings              only the informed checks will be enabled
  -h, --help                        help for verify
      --kube-apiserver string       the address and the port for the Kubernetes API server
      --kube-as-group stringArray   group to impersonate for the operation, this flag can be repeated to specify multiple groups.
      --kube-as-user string         username to impersonate for the operation
      --kube-ca-file string         the certificate authority file for the Kubernetes API server connection
      --kube-context string         name of the kubeconfig context to use
      --kube-token string           bearer token used for authentication
      --kubeconfig string           path to the kubeconfig file
  -n, --namespace string            namespace scope for this request
  -V, --openshift-version string    version of OpenShift used in the cluster
  -o, --output string               the output format: default, json or yaml
      --registry-config string      path to the registry config file (default "/root/.config/helm/registry.json")
      --repository-cache string     path to the file containing cached repository indexes (default "/root/.cache/helm/repository")
      --repository-config string    path to the file containing repository names and URLs (default "/root/.config/helm/repositories.yaml")
  -s, --set strings                 overrides a configuration, e.g: dummy.ok=false
  -f, --set-values strings          specify application and check configuration values in a YAML file or a URL (can specify multiple)

Global Flags:
      --config string   config file (default is $HOME/.chart-verifier.yaml)

Missing OpenShift version. "openshiftVersion" not found in 'oc version' output. And the 'openshift-version' flag has not set.
```

Pull request:
Add OpenShift version if chart-testing is enabled
* Ensure the OpenShift version is available if the chart-testing
  check is enabled.

* Users should be able to run a check that doesn't require a cluster.

* If the user inputs an OpenShift version that should be a valid
  SemVer version.

Fixes #129
Jira: https://issues.redhat.com/browse/HELM-188

Patch:
diff --git a/pkg/chartverifier/checks/registry.go b/pkg/chartverifier/checks/registry.go
index 6753999..5b93380 100644
--- a/pkg/chartverifier/checks/registry.go
+++ b/pkg/chartverifier/checks/registry.go
@@ -51,6 +51,11 @@ func (r *Result) AddResult(outcome bool, reason string) Result {
 	return *r
 }
 
+type AnnotationHolder interface {
+	SetCertifiedOpenShiftVersion(version string)
+	GetCertifiedOpenShiftVersionFlag() string
+}
+
 type CheckType string
 
 type Check struct {
@@ -70,6 +75,8 @@ type CheckOptions struct {
 	Values map[string]interface{}
 	// HelmEnvSettings contains the Helm related environment settings.
 	HelmEnvSettings *helmcli.EnvSettings
+	// AnnotationHolder provides and API to set the OpenShift Version
+	AnnotationHolder AnnotationHolder
 }
 
 type CheckFunc func(options *CheckOptions) (Result, error)
diff --git a/pkg/chartverifier/verifier.go b/pkg/chartverifier/verifier.go
index ff1fa1b..4e963d2 100644
--- a/pkg/chartverifier/verifier.go
+++ b/pkg/chartverifier/verifier.go
@@ -17,9 +17,7 @@
 package chartverifier
 
 import (
-	"github.com/Masterminds/semver"
 	"github.com/redhat-certification/chart-verifier/pkg/chartverifier/checks"
-	"github.com/redhat-certification/chart-verifier/pkg/tool"
 	"github.com/spf13/viper"
 	helmcli "helm.sh/helm/v3/pkg/cli"
 )
@@ -36,25 +34,21 @@ func (e CheckErr) Error() string {
 	return "check error: " + string(e)
 }
 
-type OpenShiftVersionErr string
-
-func (e OpenShiftVersionErr) Error() string {
-	return "Missing OpenShift version. " + string(e) + ". And the 'openshift-version' flag has not set."
+func NewCheckErr(err error) error {
+	return CheckErr(err.Error())
 }
 
-type OpenShiftSemVerErr string
-
-func (e OpenShiftSemVerErr) Error() string {
-	return "OpenShift version is not following SemVer spec. " + string(e)
+type AnnotationHolder struct {
+	Holder                        ReportBuilder
+	CertifiedOpenShiftVersionFlag string
 }
 
-func NewCheckErr(err error) error {
-	return CheckErr(err.Error())
+func (holder *AnnotationHolder) SetCertifiedOpenShiftVersion(version string) {
+	holder.Holder.SetCertifiedOpenShiftVersion(version)
 }
 
-// Versioner provides OpenShift version
-type Versioner interface {
-	getVersion(debug bool) (string, error)
+func (holder *AnnotationHolder) GetCertifiedOpenShiftVersionFlag() string {
+	return holder.CertifiedOpenShiftVersionFlag
 }
 
 type verifier struct {
@@ -65,7 +59,6 @@ type verifier struct {
 	toolVersion      string
 	openshiftVersion string
 	values           map[string]interface{}
-	version          Versioner
 }
 
 func (c *verifier) subConfig(name string) *viper.Viper {
@@ -76,18 +69,6 @@ func (c *verifier) subConfig(name string) *viper.Viper {
 	}
 }
 
-type version struct{}
-
-func (ver *version) getVersion(debug bool) (string, error) {
-
-	procExec := tool.NewProcessExecutor(debug)
-	oc := tool.NewOc(procExec)
-
-	// oc.GetVersion() returns an error both in case the oc command can't be executed and
-	// the value for the OpenShift version key not present.
-	return oc.GetVersion()
-}
-
 func (c *verifier) Verify(uri string) (*Report, error) {
 
 	chrt, _, err := checks.LoadChartFromURI(uri)
@@ -95,33 +76,10 @@ func (c *verifier) Verify(uri string) (*Report, error) {
 		return nil, err
 	}
 
-	// oc.GetVersion() returns an error both in case the oc command can't be executed and
-	// the value for the OpenShift version key not present.
-	osVersion, getVersionErr := c.version.getVersion(c.settings.Debug)
-
-	// From this point on, an error is set and osVersion is empty.
-	if getVersionErr != nil && c.openshiftVersion != "" {
-		osVersion = c.openshiftVersion
-	}
-
-	// osVersion is empty only if an error happened and a default value
-	// informed by the user hasn't been informed.
-	if osVersion == "" {
-		return nil, OpenShiftVersionErr(getVersionErr.Error())
-	}
-
-	// osVersion is guaranteed to have a value, not yet validated as a
-	// semver value.
-	if _, err := semver.NewVersion(osVersion); err != nil {
-		return nil, OpenShiftSemVerErr(err.Error())
-	}
-	// osVersion is guaranteed to a valid value from here onwards.
-
 	result := NewReportBuilder().
 		SetToolVersion(c.toolVersion).
 		SetChartUri(uri).
-		SetChart(chrt).
-		SetCertifiedOpenShiftVersion(osVersion)
+		SetChart(chrt)
 
 	for _, name := range c.requiredChecks {
 		check, ok := c.registry.Get(name)
@@ -129,11 +87,15 @@ func (c *verifier) Verify(uri string) (*Report, error) {
 			return nil, CheckNotFoundErr(name)
 		}
 
+		holder := AnnotationHolder{Holder: result,
+			CertifiedOpenShiftVersionFlag: c.openshiftVersion}
+
 		r, checkErr := check.Func(&checks.CheckOptions{
-			HelmEnvSettings: c.settings,
-			URI:             uri,
-			Values:          c.values,
-			ViperConfig:     c.subConfig(name),
+			HelmEnvSettings:  c.settings,
+			URI:              uri,
+			Values:           c.values,
+			ViperConfig:      c.subConfig(name),
+			AnnotationHolder: &holder,
 		})
 
 		if checkErr != nil {
diff --git a/pkg/chartverifier/verifierbuilder.go b/pkg/chartverifier/verifierbuilder.go
index 9adbdf9..7f9ada8 100644
--- a/pkg/chartverifier/verifierbuilder.go
+++ b/pkg/chartverifier/verifierbuilder.go
@@ -122,7 +122,6 @@ func (b *verifierBuilder) Build() (Vertifier, error) {
 		parts := strings.Split(val, "=")
 		b.config.Set(parts[0], parts[1])
 	}
-	ver := &version{}
 	return &verifier{
 		config:           b.config,
 		registry:         b.registry,
@@ -131,7 +130,6 @@ func (b *verifierBuilder) Build() (Vertifier, error) {
 		toolVersion:      b.toolVersion,
 		openshiftVersion: b.openshiftVersion,
 		values:           b.values,
-		version:          ver,
 	}, nil
 }
 
diff --git a/pkg/tool/oc.go b/pkg/tool/oc.go
index 5616771..4b44edf 100644
--- a/pkg/tool/oc.go
+++ b/pkg/tool/oc.go
@@ -16,7 +16,18 @@ func NewOc(exec ProcessExecutor) Oc {
 	}
 }
 
-const osVersionKey = "openshiftVersion"
+const osVersionKey = "serverVersion"
+
+// Based on https://access.redhat.com/solutions/4870701
+var kubeOpenShiftVersionMap map[string]string = map[string]string{
+	"1.20": "4.7.0",
+	"1.19": "4.6.0",
+	"1.18": "4.5.0",
+	"1.17": "4.4.0",
+	"1.16": "4.3.0",
+	"1.14": "4.2.0",
+	"1.13": "4.1.0",
+}
 
 func (o Oc) GetVersion() (string, error) {
 	rawOutput, err := o.RunProcessAndCaptureOutput("oc", "version", "-o", "yaml")
@@ -28,16 +39,14 @@ func (o Oc) GetVersion() (string, error) {
 	if err != nil {
 		return "", err
 	}
-
-	version := out[osVersionKey]
-	if version == nil {
-		return "", fmt.Errorf("%q not found in 'oc version' output", osVersionKey)
-	}
-
-	v, ok := version.(string)
+	// Relying on Kubernetes version can be replaced after fixing this issue:
+	// https://bugzilla.redhat.com/show_bug.cgi?id=1850656
+	kubeServerVersion := out[osVersionKey].(map[string]string)
+	kubeVersion := kubeServerVersion["major"] + "." + kubeServerVersion["minor"]
+	osVersion, ok := kubeOpenShiftVersionMap[kubeVersion]
 	if !ok {
-		return "", fmt.Errorf("%q does not contain a string: %v", osVersionKey, version)
+		return "", fmt.Errorf("Internal error: %q not found in Kubernetes-OpenShift version map", kubeVersion)
 	}
 
-	return v, nil
+	return osVersion, nil
 }


Test patch:
diff --git a/pkg/chartverifier/checks/charttesting.go b/pkg/chartverifier/checks/charttesting.go
index f79d477..e3b0319 100644
--- a/pkg/chartverifier/checks/charttesting.go
+++ b/pkg/chartverifier/checks/charttesting.go
@@ -7,6 +7,7 @@ import (
 	"path"
 	"strings"
 
+	"github.com/Masterminds/semver"
 	"github.com/helm/chart-testing/v3/pkg/chart"
 	"github.com/helm/chart-testing/v3/pkg/config"
 	"github.com/helm/chart-testing/v3/pkg/util"
@@ -15,6 +16,31 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
+// Versioner provides OpenShift version
+type Versioner func() (string, error)
+
+func getVersion() (string, error) {
+
+	procExec := tool.NewProcessExecutor(false)
+	oc := tool.NewOc(procExec)
+
+	// oc.GetVersion() returns an error both in case the oc command can't be executed and
+	// the value for the OpenShift version key not present.
+	return oc.GetVersion()
+}
+
+type OpenShiftVersionErr string
+
+func (e OpenShiftVersionErr) Error() string {
+	return "Missing OpenShift version. " + string(e) + ". And the 'openshift-version' flag has not set."
+}
+
+type OpenShiftSemVerErr string
+
+func (e OpenShiftSemVerErr) Error() string {
+	return "OpenShift version is not following SemVer spec. " + string(e)
+}
+
 // buildChartTestingConfiguration computes the chart testing related
 // configuration from the given check options.
 func buildChartTestingConfiguration(opts *CheckOptions) config.Configuration {
@@ -106,6 +132,10 @@ func ChartTesting(opts *CheckOptions) (Result, error) {
 		}
 	}
 
+	if versionError := setOCVersion(opts.AnnotationHolder, getVersion); versionError != nil {
+		return NewResult(false, versionError.Error()), nil
+	}
+
 	return NewResult(true, ChartTestingSuccess), nil
 }
 
@@ -347,3 +377,32 @@ func installAndTestChartRelease(
 
 	return result
 }
+
+func setOCVersion(holder AnnotationHolder, versioner Versioner) error {
+	// oc.GetVersion() returns an error both in case the oc command can't be executed and
+	// the value for the OpenShift version key not present.
+	osVersion, getVersionErr := versioner()
+
+	// From this point on, an error is set and osVersion is empty.
+	if getVersionErr != nil && holder.GetCertifiedOpenShiftVersionFlag() != "" {
+		osVersion = holder.GetCertifiedOpenShiftVersionFlag()
+	}
+
+	// osVersion is empty only if an error happened and a default value
+	// informed by the user hasn't been informed.
+	if osVersion == "" {
+		holder.SetCertifiedOpenShiftVersion("N/A")
+		return OpenShiftVersionErr(getVersionErr.Error())
+	}
+
+	// osVersion is guaranteed to have a value, not yet validated as a
+	// semver value.
+	if _, err := semver.NewVersion(osVersion); err != nil {
+		holder.SetCertifiedOpenShiftVersion("N/A")
+		return OpenShiftSemVerErr(err.Error())
+	}
+
+	holder.SetCertifiedOpenShiftVersion(osVersion)
+
+	return nil
+}
diff --git a/pkg/chartverifier/checks/charttesting_test.go b/pkg/chartverifier/checks/charttesting_test.go
index 901603e..c1340d2 100644
--- a/pkg/chartverifier/checks/charttesting_test.go
+++ b/pkg/chartverifier/checks/charttesting_test.go
@@ -1,6 +1,7 @@
 package checks
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -117,3 +118,80 @@ func TestChartTesting(t *testing.T) {
 		})
 	}
 }
+
+func getVersionError() (string, error) {
+	return "", errors.New("error")
+}
+
+func getVersionGood() (string, error) {
+	return "4.7.9", nil
+}
+
+type testAnnotationHolder struct {
+	OpenShiftVersion              string
+	CertifiedOpenShiftVersionFlag string
+}
+
+func (holder *testAnnotationHolder) SetCertifiedOpenShiftVersion(version string) {
+	holder.OpenShiftVersion = version
+}
+
+func (holder *testAnnotationHolder) GetCertifiedOpenShiftVersionFlag() string {
+	return holder.CertifiedOpenShiftVersionFlag
+}
+
+func TestVersionSetting(t *testing.T) {
+	type testCase struct {
+		description string
+		holder      *testAnnotationHolder
+		versioner   Versioner
+		version     string
+		error       string
+	}
+
+	testCases := []testCase{
+		{
+			description: "oc.Version returns 4.7.9",
+			holder:      &testAnnotationHolder{},
+			versioner:   getVersionGood,
+			version:     "4.7.9",
+		},
+		{
+			description: "oc.Version returns error, flag set to 4.7.8",
+			holder:      &testAnnotationHolder{CertifiedOpenShiftVersionFlag: "4.7.8"},
+			versioner:   getVersionError,
+			version:     "4.7.8",
+		},
+		{
+			description: "oc.Version returns semantic error, flag set to fourseveneight",
+			holder:      &testAnnotationHolder{CertifiedOpenShiftVersionFlag: "fourseveneight"},
+			versioner:   getVersionError,
+			error:       "OpenShift version is not following SemVer spec. Invalid Semantic Version",
+		},
+		{
+			description: "oc.Version returns error, flag not set",
+			holder:      &testAnnotationHolder{},
+			versioner:   getVersionError,
+			error:       "Missing OpenShift version. error. And the 'openshift-version' flag has not set.",
+		},
+	}
+
+	for _, tc := range testCases {
+
+		t.Run(tc.description, func(t *testing.T) {
+
+			err := setOCVersion(tc.holder, tc.versioner)
+
+			if len(tc.error) > 0 {
+				require.Error(t, err)
+				require.Equal(t, tc.error, err.Error())
+			} else {
+				require.NoError(t, err)
+				require.Equal(t, tc.version, tc.holder.OpenShiftVersion)
+			}
+
+		})
+
+	}
+
+}
diff --git a/pkg/chartverifier/verifier_test.go b/pkg/chartverifier/verifier_test.go
index 86e8799..9d0e132 100644
--- a/pkg/chartverifier/verifier_test.go
+++ b/pkg/chartverifier/verifier_test.go
@@ -29,18 +29,6 @@ import (
 	"github.com/redhat-certification/chart-verifier/pkg/testutil"
 )
 
-type ocVersionError struct{}
-
-func (v *ocVersionError) getVersion(debug bool) (string, error) {
-	return "", errors.New("error")
-}
-
-type ocVersionWithoutError struct{}
-
-func (v *ocVersionWithoutError) getVersion(debug bool) (string, error) {
-	return "4.9.7", nil
-}
-
 func (c *Report) isOk() bool {
 	outcome := true
 	for _, check := range c.Results {
@@ -75,16 +63,12 @@ func TestVerifier_Verify(t *testing.T) {
 
 	validChartUri := "http://" + addr + "/charts/chart-0.1.0-v3.valid.tgz"
 
-	verocVersionWithoutError := &ocVersionWithoutError{}
-	verocVersionError := &ocVersionError{}
-
 	t.Run("Should return error if check does not exist", func(t *testing.T) {
 		c := &verifier{
 			settings:       cli.New(),
 			config:         viper.New(),
 			registry:       checks.NewRegistry(),
 			requiredChecks: []string{dummyCheckName},
-			version:        verocVersionWithoutError,
 		}
 
 		r, err := c.Verify(validChartUri)
@@ -98,7 +82,6 @@ func TestVerifier_Verify(t *testing.T) {
 			config:         viper.New(),
 			registry:       checks.NewRegistry().Add(checks.Check{Name: dummyCheckName, Type: MandatoryCheckType, Func: erroredCheck}),
 			requiredChecks: []string{dummyCheckName},
-			version:        verocVersionWithoutError,
 		}
 
 		r, err := c.Verify(validChartUri)
@@ -114,7 +97,6 @@ func TestVerifier_Verify(t *testing.T) {
 			registry:         checks.NewRegistry().Add(checks.Check{Name: dummyCheckName, Type: MandatoryCheckType, Func: negativeCheck}),
 			requiredChecks:   []string{dummyCheckName},
 			openshiftVersion: "4.9",
-			version:          verocVersionWithoutError,
 		}
 
 		r, err := c.Verify(validChartUri)
@@ -125,98 +107,16 @@ func TestVerifier_Verify(t *testing.T) {
 
 	t.Run("Result should be positive if check exists and returns positive", func(t *testing.T) {
 		c := &verifier{
-			settings:         cli.New(),
-			config:           viper.New(),
-			registry:         checks.NewRegistry().Add(checks.Check{Name: dummyCheckName, Type: MandatoryCheckType, Func: positiveCheck}),
-			requiredChecks:   []string{dummyCheckName},
-			openshiftVersion: "4.9",
-			version:          verocVersionWithoutError,
-		}
-
-		r, err := c.Verify(validChartUri)
-		require.NoError(t, err)
-		require.NotNil(t, r)
-		require.True(t, r.isOk())
-	})
-
-	t.Run("oc version error and wrong user input", func(t *testing.T) {
-		c := &verifier{
-			settings:         cli.New(),
-			config:           viper.New(),
-			registry:         checks.NewRegistry().Add(checks.Check{Name: dummyCheckName, Type: MandatoryCheckType, Func: positiveCheck}),
-			requiredChecks:   []string{dummyCheckName},
-			openshiftVersion: "NaN",
-			version:          verocVersionError,
-		}
-		r, err := c.Verify(validChartUri)
-		require.Error(t, err)
-		require.Nil(t, r)
-	})
-
-	t.Run("oc version error and correct user input", func(t *testing.T) {
-		c := &verifier{
-			settings:         cli.New(),
-			config:           viper.New(),
-			registry:         checks.NewRegistry().Add(checks.Check{Name: dummyCheckName, Type: MandatoryCheckType, Func: positiveCheck}),
-			requiredChecks:   []string{dummyCheckName},
-			openshiftVersion: "4.9.7",
-			version:          verocVersionError,
-		}
-
-		r, err := c.Verify(validChartUri)
-		require.NoError(t, err)
-		require.NotNil(t, r)
-		require.True(t, r.isOk())
-		require.Equal(t, "4.9.7", r.Metadata.ToolMetadata.CertifiedOpenShiftVersions)
-	})
-
-	t.Run("oc version and wrong user input", func(t *testing.T) {
-		c := &verifier{
-			settings:         cli.New(),
-			config:           viper.New(),
-			registry:         checks.NewRegistry().Add(checks.Check{Name: dummyCheckName, Type: MandatoryCheckType, Func: positiveCheck}),
-			requiredChecks:   []string{dummyCheckName},
-			openshiftVersion: "NaN",
-			version:          verocVersionWithoutError,
-		}
-
-		r, err := c.Verify(validChartUri)
-		require.NoError(t, err)
-		require.NotNil(t, r)
-		require.True(t, r.isOk())
-		require.Equal(t, "4.9.7", r.Metadata.ToolMetadata.CertifiedOpenShiftVersions)
-	})
-
-	t.Run("oc version and correct user input", func(t *testing.T) {
-		c := &verifier{
-			settings:         cli.New(),
-			config:           viper.New(),
-			registry:         checks.NewRegistry().Add(checks.Check{Name: dummyCheckName, Type: MandatoryCheckType, Func: positiveCheck}),
-			requiredChecks:   []string{dummyCheckName},
-			openshiftVersion: "5.6.8",
-			version:          verocVersionWithoutError,
+			settings:       cli.New(),
+			config:         viper.New(),
+			registry:       checks.NewRegistry().Add(checks.Check{Name: dummyCheckName, Type: MandatoryCheckType, Func: positiveCheck}),
+			requiredChecks: []string{dummyCheckName},
 		}
 
 		r, err := c.Verify(validChartUri)
 		require.NoError(t, err)
 		require.NotNil(t, r)
 		require.True(t, r.isOk())
-		require.Equal(t, "4.9.7", r.Metadata.ToolMetadata.CertifiedOpenShiftVersions)
-	})
-
-	t.Run("oc version error and empty user input", func(t *testing.T) {
-		c := &verifier{
-			settings:         cli.New(),
-			config:           viper.New(),
-			registry:         checks.NewRegistry().Add(checks.Check{Name: dummyCheckName, Type: MandatoryCheckType, Func: positiveCheck}),
-			requiredChecks:   []string{dummyCheckName},
-			openshiftVersion: "",
-			version:          verocVersionError,
-		}
-
-		r, err := c.Verify(validChartUri)
-		require.Error(t, err)
-		require.Nil(t, r)
 	})
 
 	cancel()