Skip to content

V1alpha2 api - scorecard output changes for v1alpha2 version #1995

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 34 commits into from
Oct 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
17841d3
initial set of v1alpha2 api files and output formatting
Sep 26, 2019
50feda0
add simple v1alpha2 scorecard test that can be run on demand
Sep 30, 2019
dd0bf33
add v1alpha2 version docs updates
Sep 30, 2019
6c30748
update CHANGELOG to mention new version flag added
Sep 30, 2019
6c4731f
adjust json output for v1alpha2 rendering to show failed required tests
Sep 30, 2019
3e5ab12
refactor a bit of basic_tests
Sep 30, 2019
b391538
tweak output for text v1alpha2
Sep 30, 2019
477b53f
remove erroneous v1alpha2 reference in v1alpha1 test
Sep 30, 2019
8e420e1
initial set of v1alpha2 api files and output formatting
Sep 26, 2019
f6cd0f7
add simple v1alpha2 scorecard test that can be run on demand
Sep 30, 2019
3a8ae98
add v1alpha2 version docs updates
Sep 30, 2019
146fa90
update CHANGELOG to mention new version flag added
Sep 30, 2019
54b8f50
adjust json output for v1alpha2 rendering to show failed required tests
Sep 30, 2019
68ac6d3
refactor a bit of basic_tests
Sep 30, 2019
8f20945
tweak output for text v1alpha2
Sep 30, 2019
fba804e
remove erroneous v1alpha2 reference in v1alpha1 test
Sep 30, 2019
178e17e
add code changes per PR review comments
Oct 1, 2019
5c0d199
after rebase
Oct 1, 2019
456d337
fix merge
Oct 1, 2019
394ab82
Merge branch 'master' into v1alpha2-api
Oct 1, 2019
3ba185a
refactor api output formatting implementation to use an interface
Oct 2, 2019
ee143ad
Merge branch 'v1alpha2-api' of github.com:jmccormick2001/operator-sdk…
Oct 2, 2019
cf74e91
change interface function name for json output to avoid conflict foun…
Oct 2, 2019
6767e66
make v1alpha2 the default scorecard version
Oct 3, 2019
b61eca7
refactor
Oct 3, 2019
5740698
Merge branch 'master' into v1alpha2-api
Oct 4, 2019
2830cc2
updates
Oct 7, 2019
b961f88
fix ci scorecard test
Oct 7, 2019
83b27d2
minor tweaks per review comments
Oct 8, 2019
f09d64d
add labels field to v1alpha2 ScorecardTestResult type
Oct 8, 2019
7ce3a9d
remove Suite from the v1alpha2 API, adjust text output to print suite…
Oct 9, 2019
611375f
Merge branch 'master' of github.com:operator-framework/operator-sdk i…
Oct 9, 2019
d31e808
Merge remote-tracking branch 'upstream/master' into v1alpha2-api
Oct 15, 2019
9903139
removed required test status, add omitempty to v1alpha2 types
Oct 15, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,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)
Copy link
Contributor

@camilamacedo86 camilamacedo86 Oct 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked that k8s uses the nomenclature --experimental-x for this kind of impl. WDYT about we adopted the same? As for example --experimental-version

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's meant more for enabling or configuring experimental features. In this case, this will control core features of the scorecard (output formats, support for external plugins, exit code handling, label selection, etc.).

So I think we should leave --version as the flag name.


### Changed

Expand Down Expand Up @@ -63,6 +64,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
Expand Down
8 changes: 4 additions & 4 deletions ci/tests/subcommand-scorecard.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
1 change: 1 addition & 0 deletions doc/sdk-cli-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion doc/test-framework/scorecard.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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/
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
25 changes: 21 additions & 4 deletions hack/tests/subcommand-scorecard.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
11 changes: 1 addition & 10 deletions internal/pkg/scorecard/helpers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
20 changes: 5 additions & 15 deletions internal/pkg/scorecard/helpers/versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
15 changes: 7 additions & 8 deletions internal/pkg/scorecard/plugins/basic_tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
}

Expand Down
58 changes: 5 additions & 53 deletions internal/pkg/scorecard/scorecard.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,8 @@ package scorecard

import (
"bytes"
"encoding/json"
"fmt"
"io"
"io/ioutil"
"os"
"strings"

Expand Down Expand Up @@ -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
}

Expand Down
82 changes: 82 additions & 0 deletions internal/pkg/scorecard/scorecard_output.go
Original file line number Diff line number Diff line change
@@ -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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: This is out of scope for this PR, but I'm not a huge fan of the global variables used throughout scorecard (e.g. scViper) because they cause non-obvious side effects if you're looking at the call site of the functions that use the global variables.

In a follow-on, I think it would be worth it to refactor things so that viper is used only in the cmd/operator-sdk/scorecard package to parse the config from flags, env, config files, etc. and then populate a scorecard struct with the resulting config, so that we don't have to constantly interrogate a global viper instance.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, I could write up a new Jira story for refactoring that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

@jmccormick2001 jmccormick2001 Oct 8, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://jira.coreos.com/browse/OSDK-587
another one that falls into the refactoring camp that would be an improvement.

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
}
Loading