V1alpha2 api - scorecard output changes for v1alpha2 version (#1995)

* initial set of v1alpha2 api files and output formatting

* add simple v1alpha2 scorecard test that can be run on demand

* add v1alpha2 version docs updates

* update CHANGELOG to mention new version flag added

* adjust json output for v1alpha2 rendering to show failed required tests

* refactor a bit of basic_tests

* tweak output for text v1alpha2

* remove erroneous v1alpha2 reference in v1alpha1 test

* initial set of v1alpha2 api files and output formatting

* add simple v1alpha2 scorecard test that can be run on demand

* add v1alpha2 version docs updates

* update CHANGELOG to mention new version flag added

* adjust json output for v1alpha2 rendering to show failed required tests

* refactor a bit of basic_tests

* tweak output for text v1alpha2

* remove erroneous v1alpha2 reference in v1alpha1 test

* add code changes per PR review comments

* fix merge

* refactor api output formatting implementation to use an interface

* change interface function name for json output to avoid conflict found by go vet

* make v1alpha2 the default scorecard version

* refactor

* updates

* fix ci scorecard test

* minor tweaks per review comments

* add labels field to v1alpha2 ScorecardTestResult type

* remove Suite from the v1alpha2 API, adjust text output to print suite label

* removed required test status, add omitempty to v1alpha2 types

Author: Jeff McCormick

CHANGELOG.md

@@ -22,6 +22,7 @@
 - Ansible based operators now gather and serve metrics about each custom resource on port 8686 of the metrics service. ([#1723](https://github.com/operator-framework/operator-sdk/pull/1723))
 - Added the Go version, OS, and architecture to the output of `operator-sdk version` ([#1863](https://github.com/operator-framework/operator-sdk/pull/1863))
 - Added support for `ppc64le-linux` for the `operator-sdk` binary and the Helm operator base image. ([#1533](https://github.com/operator-framework/operator-sdk/pull/1533))
+- Added new `--version` flag to the `operator-sdk scorecard` command to support a new output format for the scorecard. ([#1916](https://github.com/operator-framework/operator-sdk/pull/1916)
 
 ### Changed
 
@@ -65,6 +66,7 @@
 - [`pkg/test.FrameworkClient`](https://github.com/operator-framework/operator-sdk/blob/master/pkg/test/client.go#L33) methods `List()` and `Delete()` have new signatures corresponding to the homonymous methods of `sigs.k8s.io/controller-runtime/pkg/client.Client`. ([#1876](https://github.com/operator-framework/operator-sdk/pull/1876))
 - CRD file names were previously of the form `<group>_<version>_<kind>_crd.yaml`. Now that CRD manifest `spec.version` is deprecated in favor of `spec.versions`, i.e. multiple versions can be specified in one CRD, CRD file names have the form `<full group>_<resource>_crd.yaml`. `<full group>` is the full group name of your CRD while `<group>` is the last subdomain of `<full group>`, ex. `foo.bar.com` vs `foo`. `<resource>` is the plural lower-case CRD Kind found at `spec.names.plural`. ([#1876](https://github.com/operator-framework/operator-sdk/pull/1876))
 - Upgrade Python version from `2.7` to `3.6`, Ansible version from `2.8.0` to `~=2.8` and ansible-runner from `1.2` to `1.3.4` in the Ansible based images. ([#1947](https://github.com/operator-framework/operator-sdk/pull/1947)) 
+- Made the default scorecard version `v1alpha2` which is new for this release and could break users that were parsing the older scorecard output (`v1alpha1`).  Users can still specify version `v1alpha1` on the scorecard configuration to use the older style for some period of time until `v1alpha1` is removed.
 - Replaced `pkg/kube-metrics.NewCollectors()` with `pkg/kube-metrics.NewMetricsStores()` and changed exported function signature for `pkg/kube-metrics.ServeMetrics()` due to a [breaking change in kube-state-metrics](https://github.com/kubernetes/kube-state-metrics/pull/786). ([#1943](https://github.com/operator-framework/operator-sdk/pull/1943))
 
 ### Deprecated

ci/tests/subcommand-scorecard.sh

@@ -24,11 +24,11 @@ trap_add 'cp $ROOTDIR/backup2.yaml $CONFIG_PATH && rm $ROOTDIR/backup2.yaml' EXI
 sed 's|REPLACE_IMAGE|'$IMAGE'|g' -i $CONFIG_PATH
 
 # basic test with specified config location
-commandoutput="$(operator-sdk scorecard --config "$CONFIG_PATH" 2>&1)"
+commandoutput="$(operator-sdk scorecard --version v1alpha1 --config "$CONFIG_PATH" 2>&1)"
 echo $commandoutput | grep "Total Score: 67%"
 
 # test json output and default config path
-commandoutput2="$(operator-sdk scorecard 2>&1)"
+commandoutput2="$(operator-sdk scorecard --version v1alpha1 2>&1)"
 # check basic suite
 echo $commandoutput2 | grep '^.*"error": 0,[[:space:]]"pass": 3,[[:space:]]"partialPass": 0,[[:space:]]"fail": 0,[[:space:]]"totalTests": 3,[[:space:]]"totalScorePercent": 100,.*$'
 # check olm suite
@@ -43,7 +43,7 @@ echo $commandoutput2 | grep '^.*"name": "Flags",[[:space:]]"description": "Test
 echo $commandoutput2 | grep '^.*"name": "Environment",[[:space:]]"description": "Test plugin with kubeconfig set via env var",[[:space:]]"earnedPoints": 2,[[:space:]]"maximumPoints": 5,.*$'
 
 # test kubeconfig flag (kubeconfig shouldn't exist so internal plugins should instantly fail)
-commandoutput3="$(operator-sdk scorecard --kubeconfig=/kubeconfig 2>&1)"
+commandoutput3="$(operator-sdk scorecard --version v1alpha1 --kubeconfig=/kubeconfig 2>&1)"
 # check basic suite
 echo $commandoutput3 | grep '^.*"name": "Basic Tests",[[:space:]]"description": "",[[:space:]]"error": 1,.*$'
 # check olm suite
@@ -58,6 +58,6 @@ echo $commandoutput3 | grep '^.*"name": "Different Env and flag",[[:space:]]"des
 echo $commandoutput3 | grep '^.*"name": "Environment",[[:space:]]"description": "Test plugin with kubeconfig set via env var",[[:space:]]"earnedPoints": 2,[[:space:]]"maximumPoints": 5,.*$'
 
 # Test invalid config
-operator-sdk scorecard --config "$CONFIG_PATH_INVALID" |& grep '^.*invalid keys.*$'
+operator-sdk scorecard --version v1alpha1 --config "$CONFIG_PATH_INVALID" |& grep '^.*invalid keys.*$'
 
 popd

doc/sdk-cli-reference.md

@@ -449,6 +449,7 @@ Run scorecard tests on an operator
 * `proxy-image` string - Image name for scorecard proxy (default "quay.io/operator-framework/scorecard-proxy")
 * `proxy-pull-policy` string - Pull policy for scorecard proxy image (default "Always")
 * `-h, --help` - help for scorecard
+* `version` string - The scorecard version to run (default v1alpha1), the tech preview version is v1alpha2.
 
 ### Example
 

doc/test-framework/scorecard.md

@@ -106,6 +106,7 @@ While most configuration is done via a config file, there are a few important ar
 | `--config`  | string | Path to config file (default `<project_dir>/.osdk-scorecard`; file type and extension can be any of `.yaml`, `.json`, or `.toml`). If a config file is not provided and a config file is not found at the default location, the scorecard will exit with an error. |
 | `--output`, `-o`  | string | Output format. Valid options are: `text` and `json`. The default format is `text`, which is designed to be a simpler human readable format. The `json` format uses the JSON schema output format used for plugins defined later in this document. |
 | `--kubeconfig`, `-o`  | string |  path to kubeconfig. It sets the kubeconfig internally for internal plugins and sets the `KUBECONFIG` env var to the provided value for external plugins. If an external plugin specifically sets the `KUBECONFIG` env var, the kubeconfig from the specified env var will be used for that plugin instead. |
+| `--version`  | string |  The version of scorecard to run, v1alpha1 is the default, whereas v1alpha2 is the tech preview. |
 
 ### Config File Options
 
@@ -405,4 +406,4 @@ Running with a CSV alone requires both the `--csv-path=<CSV manifest path>` and
 [olm-csv-alm-examples]:https://github.com/operator-framework/operator-lifecycle-manager/blob/master/doc/design/building-your-csv.md#crd-templates
 [olm]:https://github.com/operator-framework/operator-lifecycle-manager
 [olm-deploy-operator]:https://github.com/operator-framework/community-operators/blob/master/docs/testing-operators.md
-[okd]:https://www.okd.io/
+[okd]:https://www.okd.io/

go.mod

@@ -41,6 +41,7 @@ require (
 	github.com/markbates/inflect v1.0.4
 	github.com/martinlindhe/base36 v0.0.0-20180729042928-5cda0030da17
 	github.com/mattbaird/jsonpatch v0.0.0-20171005235357-81af80346b1a
+	github.com/mattn/go-isatty v0.0.8
 	github.com/mitchellh/go-homedir v1.1.0
 	github.com/mitchellh/go-wordwrap v1.0.0 // indirect
 	github.com/mitchellh/mapstructure v1.1.2

go.sum

@@ -289,6 +289,7 @@ github.com/martinlindhe/base36 v0.0.0-20180729042928-5cda0030da17/go.mod h1:+AtE
 github.com/mattbaird/jsonpatch v0.0.0-20171005235357-81af80346b1a h1:+J2gw7Bw77w/fbK7wnNJJDKmw1IbWft2Ul5BzrG1Qm8=
 github.com/mattbaird/jsonpatch v0.0.0-20171005235357-81af80346b1a/go.mod h1:M1qoD/MqPgTZIk0EWKB38wE28ACRfVcn+cU08jyArI0=
 github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE=
+github.com/mattn/go-isatty v0.0.8 h1:HLtExJ+uU2HOZ+wI0Tt5DtUDrx8yhUqDcp7fYERX4CE=
 github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s=
 github.com/mattn/go-sqlite3 v1.9.0/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc=
 github.com/mattn/go-sqlite3 v1.10.0 h1:jbhqpg7tQe4SupckyijYiy0mJJ/pRyHvXf7JdWK860o=

hack/tests/subcommand-scorecard.sh

@@ -2,6 +2,7 @@
 
 DEST_IMAGE="quay.io/example/scorecard-proxy"
 CONFIG_PATH=".test-osdk-scorecard.yaml"
+CONFIG_PATH_V1ALPHA1=".test-osdk-scorecard-v1alpha1.yaml"
 CONFIG_PATH_DISABLE=".osdk-scorecard-disable.yaml"
 CONFIG_PATH_INVALID=".osdk-scorecard-invalid.yaml"
 
@@ -12,11 +13,26 @@ set -ex
 
 # the test framework directory has all the manifests needed to run the cluster
 pushd test/test-framework
-commandoutput="$(operator-sdk scorecard --config "$CONFIG_PATH" 2>&1)"
+
+# test to see if v1alpha2 is used from the command line
+commandoutput="$(operator-sdk scorecard --version v1alpha2 --config "$CONFIG_PATH" 2>&1)"
+failCount=`echo $commandoutput | grep -o ": fail" | wc -l`
+expectedFailCount=7
+if [ $failCount -ne $expectedFailCount ]
+then
+	echo "expected fail count $expectedFailCount, got $failCount"
+	exit 1
+fi
+
+# test to see if version in config file allows v1alpha1 to be specified
+commandoutput="$(operator-sdk scorecard --config "$CONFIG_PATH_V1ALPHA1" 2>&1)"
+echo $commandoutput | grep "Total Score: 67%"
+
+commandoutput="$(operator-sdk scorecard --version v1alpha1 --config "$CONFIG_PATH" 2>&1)"
 echo $commandoutput | grep "Total Score: 67%"
 
 # test json output and default config path
-commandoutput2="$(operator-sdk scorecard 2>&1)"
+commandoutput2="$(operator-sdk scorecard --version v1alpha1 2>&1)"
 # check basic suite
 echo $commandoutput2 | grep '^.*"error": 0,[[:space:]]"pass": 3,[[:space:]]"partialPass": 0,[[:space:]]"fail": 0,[[:space:]]"totalTests": 3,[[:space:]]"totalScorePercent": 100,.*$'
 # check olm suite
@@ -31,7 +47,7 @@ echo $commandoutput2 | grep '^.*"name": "Flags",[[:space:]]"description": "Test
 echo $commandoutput2 | grep '^.*"name": "Environment",[[:space:]]"description": "Test plugin with kubeconfig set via env var",[[:space:]]"earnedPoints": 2,[[:space:]]"maximumPoints": 5,.*$'
 
 # test kubeconfig flag (kubeconfig shouldn't exist so internal plugins should instantly fail)
-commandoutput3="$(operator-sdk scorecard --kubeconfig=/kubeconfig 2>&1)"
+commandoutput3="$(operator-sdk scorecard --version v1alpha1 --kubeconfig=/kubeconfig 2>&1)"
 # check basic suite
 echo $commandoutput3 | grep '^.*"name": "Basic Tests",[[:space:]]"description": "",[[:space:]]"error": 1,.*$'
 # check olm suite
@@ -46,6 +62,7 @@ echo $commandoutput3 | grep '^.*"name": "Different Env and flag",[[:space:]]"des
 echo $commandoutput3 | grep '^.*"name": "Environment",[[:space:]]"description": "Test plugin with kubeconfig set via env var",[[:space:]]"earnedPoints": 2,[[:space:]]"maximumPoints": 5,.*$'
 
 # Test invalid config
-operator-sdk scorecard --config "$CONFIG_PATH_INVALID" |& grep '^.*invalid keys.*$'
+operator-sdk scorecard --version v1alpha1 --config "$CONFIG_PATH_INVALID" |& grep '^.*invalid keys.*$'
+
 
 popd

internal/pkg/scorecard/helpers/helpers.go

@@ -139,6 +139,7 @@ func UpdateState(res scapiv1alpha1.ScorecardTestResult) scapiv1alpha1.ScorecardT
 	if res.State == scapiv1alpha1.ErrorState {
 		return res
 	}
+
 	if res.EarnedPoints == 0 {
 		res.State = scapiv1alpha1.FailState
 	} else if res.EarnedPoints < res.MaximumPoints {
@@ -177,13 +178,3 @@ func UpdateSuiteStates(suite scapiv1alpha1.ScorecardSuiteResult) scapiv1alpha1.S
 	}
 	return suite
 }
-
-func CombineScorecardOutput(outputs []scapiv1alpha1.ScorecardOutput, log string) scapiv1alpha1.ScorecardOutput {
-	output := scapiv1alpha1.ScorecardOutput{
-		Log: log,
-	}
-	for _, item := range outputs {
-		output.Results = append(output.Results, item.Results...)
-	}
-	return output
-}

internal/pkg/scorecard/helpers/versions.go

@@ -16,17 +16,17 @@ package schelpers
 
 import (
 	"fmt"
-	"github.com/spf13/viper"
 	"strings"
 )
 
 const v1alpha1 = "v1alpha1"
 const v1alpha2 = "v1alpha2"
-const DefaultScorecardVersion = v1alpha1
+
+const DefaultScorecardVersion = v1alpha2
 const LatestScorecardVersion = v1alpha2
 const VersionOpt = "version"
 
-var ScorecardVersions = []string{DefaultScorecardVersion, LatestScorecardVersion}
+var ScorecardVersions = []string{v1alpha1, v1alpha2}
 
 func ValidateVersion(version string) error {
 	for _, a := range ScorecardVersions {
@@ -38,16 +38,6 @@ func ValidateVersion(version string) error {
 
 }
 
-func IsV1alpha2() bool {
-	if viper.Sub("scorecard").GetString(VersionOpt) == v1alpha2 {
-		return true
-	}
-	return false
-}
-
-func IsLatestVersion() bool {
-	if viper.Sub("scorecard").GetString(VersionOpt) == LatestScorecardVersion {
-		return true
-	}
-	return false
+func IsV1alpha2(version string) bool {
+	return version == v1alpha2
 }

internal/pkg/scorecard/plugins/basic_tests.go

@@ -114,12 +114,12 @@ func (t *CheckSpecTest) Run(ctx context.Context) *schelpers.TestResult {
 		res.Errors = append(res.Errors, fmt.Errorf("error getting custom resource: %v", err))
 		return res
 	}
-	if t.CR.Object["spec"] != nil {
-		res.EarnedPoints++
-	}
-	if res.EarnedPoints != 1 {
+
+	if t.CR.Object["spec"] == nil {
 		res.Suggestions = append(res.Suggestions, "Add a 'spec' field to your Custom Resource")
+		return res
 	}
+	res.EarnedPoints++
 	return res
 }
 
@@ -131,12 +131,11 @@ func (t *CheckStatusTest) Run(ctx context.Context) *schelpers.TestResult {
 		res.Errors = append(res.Errors, fmt.Errorf("error getting custom resource: %v", err))
 		return res
 	}
-	if t.CR.Object["status"] != nil {
-		res.EarnedPoints++
-	}
-	if res.EarnedPoints != 1 {
+	if t.CR.Object["status"] == nil {
 		res.Suggestions = append(res.Suggestions, "Add a 'status' field to your Custom Resource")
+		return res
 	}
+	res.EarnedPoints++
 	return res
 }
 

internal/pkg/scorecard/scorecard.go

@@ -16,10 +16,8 @@ package scorecard
 
 import (
 	"bytes"
-	"encoding/json"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"strings"
 
@@ -116,64 +114,18 @@ func ScorecardTests(cmd *cobra.Command, args []string) error {
 	for _, plugin := range plugins {
 		pluginOutputs = append(pluginOutputs, plugin.Run())
 	}
-	totalScore := 0.0
+
 	// Update the state for the tests
 	for _, suite := range pluginOutputs {
 		for idx, res := range suite.Results {
 			suite.Results[idx] = schelpers.UpdateSuiteStates(res)
 		}
 	}
-	if scViper.GetString(OutputFormatOpt) == TextOutputFormat {
-		numSuites := 0
-		for _, plugin := range pluginOutputs {
-			for _, suite := range plugin.Results {
-				fmt.Printf("%s:\n", suite.Name)
-				for _, result := range suite.Tests {
-					fmt.Printf("\t%s: %d/%d\n", result.Name, result.EarnedPoints, result.MaximumPoints)
-				}
-				totalScore += float64(suite.TotalScore)
-				numSuites++
-			}
-		}
-		totalScore = totalScore / float64(numSuites)
-		fmt.Printf("\nTotal Score: %.0f%%\n", totalScore)
-		// TODO: We can probably use some helper functions to clean up these quadruple nested loops
-		// Print suggestions
-		for _, plugin := range pluginOutputs {
-			for _, suite := range plugin.Results {
-				for _, result := range suite.Tests {
-					for _, suggestion := range result.Suggestions {
-						// 33 is yellow (specifically, the same shade of yellow that logrus uses for warnings)
-						fmt.Printf("\x1b[%dmSUGGESTION:\x1b[0m %s\n", 33, suggestion)
-					}
-				}
-			}
-		}
-		// Print errors
-		for _, plugin := range pluginOutputs {
-			for _, suite := range plugin.Results {
-				for _, result := range suite.Tests {
-					for _, err := range result.Errors {
-						// 31 is red (specifically, the same shade of red that logrus uses for errors)
-						fmt.Printf("\x1b[%dmERROR:\x1b[0m %s\n", 31, err)
-					}
-				}
-			}
-		}
-	}
-	if scViper.GetString(OutputFormatOpt) == JSONOutputFormat {
-		log, err := ioutil.ReadAll(logReadWriter)
-		if err != nil {
-			return fmt.Errorf("failed to read log buffer: %v", err)
-		}
-		scTest := schelpers.CombineScorecardOutput(pluginOutputs, string(log))
-		// Pretty print so users can also read the json output
-		bytes, err := json.MarshalIndent(scTest, "", "  ")
-		if err != nil {
-			return err
-		}
-		fmt.Printf("%s\n", string(bytes))
+
+	if err := printPluginOutputs(scViper.GetString(schelpers.VersionOpt), pluginOutputs); err != nil {
+		return err
 	}
+
 	return nil
 }
 

internal/pkg/scorecard/scorecard_output.go

@@ -0,0 +1,82 @@
+// Copyright 2019 The Operator-SDK Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package scorecard
+
+import (
+	"encoding/json"
+	"fmt"
+	schelpers "github.com/operator-framework/operator-sdk/internal/pkg/scorecard/helpers"
+	scapi "github.com/operator-framework/operator-sdk/pkg/apis/scorecard"
+	scapiv1alpha1 "github.com/operator-framework/operator-sdk/pkg/apis/scorecard/v1alpha1"
+	"io/ioutil"
+)
+
+func printPluginOutputs(version string, pluginOutputs []scapiv1alpha1.ScorecardOutput) error {
+
+	var list scapi.ScorecardFormatter
+	var err error
+	list, err = combinePluginOutput(pluginOutputs)
+	if err != nil {
+		return err
+	}
+
+	if schelpers.IsV1alpha2(version) {
+		list = scapi.ConvertScorecardOutputV1ToV2(list.(scapiv1alpha1.ScorecardOutput))
+	}
+
+	// produce text output
+	if scViper.GetString(OutputFormatOpt) == TextOutputFormat {
+		output, err := list.MarshalText()
+		if err != nil {
+			return err
+		}
+		fmt.Printf("%s\n", output)
+
+		return nil
+	}
+
+	// produce json output
+	if scViper.GetString(OutputFormatOpt) == JSONOutputFormat {
+		bytes, err := json.MarshalIndent(list, "", "  ")
+		if err != nil {
+			return err
+		}
+		fmt.Printf("%s\n", string(bytes))
+		return nil
+
+	}
+
+	return nil
+}
+
+func combinePluginOutput(pluginOutputs []scapiv1alpha1.ScorecardOutput) (scapiv1alpha1.ScorecardOutput, error) {
+	output := scapiv1alpha1.ScorecardOutput{}
+	output.Results = make([]scapiv1alpha1.ScorecardSuiteResult, 0)
+	for _, v := range pluginOutputs {
+		for _, r := range v.Results {
+			output.Results = append(output.Results, r)
+		}
+	}
+
+	if scViper.GetString(OutputFormatOpt) == JSONOutputFormat {
+		log, err := ioutil.ReadAll(logReadWriter)
+		if err != nil {
+			return output, fmt.Errorf("failed to read log buffer: %v", err)
+		}
+		output.Log = string(log)
+	}
+
+	return output, nil
+}