From 4c1dd1c144943ad9a1778835361b264abe6b6e6e Mon Sep 17 00:00:00 2001 From: Alex Pavel Date: Mon, 1 Apr 2019 15:13:31 -0700 Subject: [PATCH 1/6] cmd/.../scorecard: move scorecard libraries out of cmd --- cmd/operator-sdk/scorecard/basic_tests.go | 86 ----- cmd/operator-sdk/scorecard/cmd.go | 35 +- .../scorecard/test_definitions.go | 324 ------------------ internal/pkg/scorecard/basic_tests.go | 169 +++++++++ internal/pkg/scorecard/helpers.go | 39 +++ .../pkg}/scorecard/olm_tests.go | 142 +++++++- .../pkg}/scorecard/resource_handler.go | 4 +- .../pkg}/scorecard/scorecard.go | 3 +- pkg/scorecard/lib/test_definitions.go | 107 ++++++ 9 files changed, 469 insertions(+), 440 deletions(-) delete mode 100644 cmd/operator-sdk/scorecard/basic_tests.go delete mode 100644 cmd/operator-sdk/scorecard/test_definitions.go create mode 100644 internal/pkg/scorecard/basic_tests.go create mode 100644 internal/pkg/scorecard/helpers.go rename {cmd/operator-sdk => internal/pkg}/scorecard/olm_tests.go (69%) rename {cmd/operator-sdk => internal/pkg}/scorecard/resource_handler.go (100%) rename {cmd/operator-sdk => internal/pkg}/scorecard/scorecard.go (99%) create mode 100644 pkg/scorecard/lib/test_definitions.go diff --git a/cmd/operator-sdk/scorecard/basic_tests.go b/cmd/operator-sdk/scorecard/basic_tests.go deleted file mode 100644 index 53339cc73e8..00000000000 --- a/cmd/operator-sdk/scorecard/basic_tests.go +++ /dev/null @@ -1,86 +0,0 @@ -// 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 ( - "context" - "encoding/json" - "fmt" - "strings" - - "k8s.io/apimachinery/pkg/types" -) - -// Run - implements Test interface -func (t *CheckSpecTest) Run(ctx context.Context) *TestResult { - res := &TestResult{Test: t, MaximumPoints: 1} - err := t.Client.Get(ctx, types.NamespacedName{Namespace: t.CR.GetNamespace(), Name: t.CR.GetName()}, t.CR) - if err != nil { - 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 { - res.Suggestions = append(res.Suggestions, "Add a 'spec' field to your Custom Resource") - } - return res -} - -// Run - implements Test interface -func (t *CheckStatusTest) Run(ctx context.Context) *TestResult { - res := &TestResult{Test: t, MaximumPoints: 1} - err := t.Client.Get(ctx, types.NamespacedName{Namespace: t.CR.GetNamespace(), Name: t.CR.GetName()}, t.CR) - if err != nil { - 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 { - res.Suggestions = append(res.Suggestions, "Add a 'status' field to your Custom Resource") - } - return res -} - -// Run - implements Test interface -func (t *WritingIntoCRsHasEffectTest) Run(ctx context.Context) *TestResult { - res := &TestResult{Test: t, MaximumPoints: 1} - logs, err := getProxyLogs(t.ProxyPod) - if err != nil { - res.Errors = append(res.Errors, fmt.Errorf("error getting proxy logs: %v", err)) - return res - } - msgMap := make(map[string]interface{}) - for _, msg := range strings.Split(logs, "\n") { - if err := json.Unmarshal([]byte(msg), &msgMap); err != nil { - continue - } - method, ok := msgMap["method"].(string) - if !ok { - continue - } - if method == "PUT" || method == "POST" { - res.EarnedPoints = 1 - break - } - } - if res.EarnedPoints != 1 { - res.Suggestions = append(res.Suggestions, "The operator should write into objects to update state. No PUT or POST requests from the operator were recorded by the scorecard.") - } - return res -} diff --git a/cmd/operator-sdk/scorecard/cmd.go b/cmd/operator-sdk/scorecard/cmd.go index 7acc21d9b0a..ebd77497c4a 100644 --- a/cmd/operator-sdk/scorecard/cmd.go +++ b/cmd/operator-sdk/scorecard/cmd.go @@ -19,6 +19,7 @@ import ( "strings" "github.com/operator-framework/operator-sdk/internal/pkg/scaffold" + "github.com/operator-framework/operator-sdk/internal/pkg/scorecard" "github.com/operator-framework/operator-sdk/version" log "github.com/sirupsen/logrus" @@ -53,25 +54,25 @@ func NewCmd() *cobra.Command { Short: "Run scorecard tests", Long: `Runs blackbox scorecard tests on an operator `, - RunE: ScorecardTests, + RunE: scorecard.ScorecardTests, } - scorecardCmd.Flags().StringVar(&ScorecardConf, ConfigOpt, "", "config file (default is /.osdk-yaml)") - scorecardCmd.Flags().StringVar(&scConf.namespace, NamespaceOpt, "", "Namespace of custom resource created in cluster") - scorecardCmd.Flags().StringVar(&scConf.kubeconfigPath, KubeconfigOpt, "", "Path to kubeconfig of custom resource created in cluster") - scorecardCmd.Flags().IntVar(&scConf.initTimeout, InitTimeoutOpt, 10, "Timeout for status block on CR to be created in seconds") - scorecardCmd.Flags().BoolVar(&scConf.olmDeployed, OlmDeployedOpt, false, "The OLM has deployed the operator. Use only the CSV for test data") - scorecardCmd.Flags().StringVar(&scConf.csvPath, CSVPathOpt, "", "Path to CSV being tested") - scorecardCmd.Flags().BoolVar(&scConf.basicTests, BasicTestsOpt, true, "Enable basic operator checks") - scorecardCmd.Flags().BoolVar(&scConf.olmTests, OLMTestsOpt, true, "Enable OLM integration checks") - scorecardCmd.Flags().BoolVar(&scConf.tenantTests, TenantTestsOpt, false, "Enable good tenant checks") - scorecardCmd.Flags().StringVar(&scConf.namespacedManifest, NamespacedManifestOpt, "", "Path to manifest for namespaced resources (e.g. RBAC and Operator manifest)") - scorecardCmd.Flags().StringVar(&scConf.globalManifest, GlobalManifestOpt, "", "Path to manifest for Global resources (e.g. CRD manifests)") - scorecardCmd.Flags().StringVar(&scConf.crManifest, CRManifestOpt, "", "Path to manifest for Custom Resource (required)") - scorecardCmd.Flags().StringVar(&scConf.proxyImage, ProxyImageOpt, fmt.Sprintf("quay.io/operator-framework/scorecard-proxy:%s", strings.TrimSuffix(version.Version, "+git")), "Image name for scorecard proxy") - scorecardCmd.Flags().StringVar(&scConf.proxyPullPolicy, ProxyPullPolicyOpt, "Always", "Pull policy for scorecard proxy image") - scorecardCmd.Flags().StringVar(&scConf.crdsDir, "crds-dir", scaffold.CRDsDir, "Directory containing CRDs (all CRD manifest filenames must have the suffix 'crd.yaml')") - scorecardCmd.Flags().BoolVar(&scConf.verbose, VerboseOpt, false, "Enable verbose logging") + scorecardCmd.Flags().StringVar(&scorecard.ScorecardConf, scorecard.ConfigOpt, "", "config file (default is /.osdk-yaml)") + scorecardCmd.Flags().StringVar(&scConf.namespace, scorecard.NamespaceOpt, "", "Namespace of custom resource created in cluster") + scorecardCmd.Flags().StringVar(&scConf.kubeconfigPath, scorecard.KubeconfigOpt, "", "Path to kubeconfig of custom resource created in cluster") + scorecardCmd.Flags().IntVar(&scConf.initTimeout, scorecard.InitTimeoutOpt, 10, "Timeout for status block on CR to be created in seconds") + scorecardCmd.Flags().BoolVar(&scConf.olmDeployed, scorecard.OlmDeployedOpt, false, "The OLM has deployed the operator. Use only the CSV for test data") + scorecardCmd.Flags().StringVar(&scConf.csvPath, scorecard.CSVPathOpt, "", "Path to CSV being tested") + scorecardCmd.Flags().BoolVar(&scConf.basicTests, scorecard.BasicTestsOpt, true, "Enable basic operator checks") + scorecardCmd.Flags().BoolVar(&scConf.olmTests, scorecard.OLMTestsOpt, true, "Enable OLM integration checks") + scorecardCmd.Flags().BoolVar(&scConf.tenantTests, scorecard.TenantTestsOpt, false, "Enable good tenant checks") + scorecardCmd.Flags().StringVar(&scConf.namespacedManifest, scorecard.NamespacedManifestOpt, "", "Path to manifest for namespaced resources (e.g. RBAC and Operator manifest)") + scorecardCmd.Flags().StringVar(&scConf.globalManifest, scorecard.GlobalManifestOpt, "", "Path to manifest for Global resources (e.g. CRD manifests)") + scorecardCmd.Flags().StringVar(&scConf.crManifest, scorecard.CRManifestOpt, "", "Path to manifest for Custom Resource (required)") + scorecardCmd.Flags().StringVar(&scConf.proxyImage, scorecard.ProxyImageOpt, fmt.Sprintf("quay.io/operator-framework/scorecard-proxy:%s", strings.TrimSuffix(version.Version, "+git")), "Image name for scorecard proxy") + scorecardCmd.Flags().StringVar(&scConf.proxyPullPolicy, scorecard.ProxyPullPolicyOpt, "Always", "Pull policy for scorecard proxy image") + scorecardCmd.Flags().StringVar(&scConf.crdsDir, scorecard.CRDsDirOpt, scaffold.CRDsDir, "Directory containing CRDs (all CRD manifest filenames must have the suffix 'crd.yaml')") + scorecardCmd.Flags().BoolVar(&scConf.verbose, scorecard.VerboseOpt, false, "Enable verbose logging") if err := viper.BindPFlags(scorecardCmd.Flags()); err != nil { log.Fatalf("Failed to bind scorecard flags to viper: %v", err) diff --git a/cmd/operator-sdk/scorecard/test_definitions.go b/cmd/operator-sdk/scorecard/test_definitions.go deleted file mode 100644 index 216e1a6752d..00000000000 --- a/cmd/operator-sdk/scorecard/test_definitions.go +++ /dev/null @@ -1,324 +0,0 @@ -// 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 ( - "context" - - olmapiv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1" - v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -// Type Definitions - -// Test provides methods for running scorecard tests -type Test interface { - GetName() string - GetDescription() string - IsCumulative() bool - Run(context.Context) *TestResult -} - -// TestResult contains a test's points, suggestions, and errors -type TestResult struct { - Test Test - EarnedPoints int - MaximumPoints int - Suggestions []string - Errors []error -} - -// TestInfo contains information about the scorecard test -type TestInfo struct { - Name string - Description string - // If a test is set to cumulative, the scores of multiple runs of the same test on separate CRs are added together for the total score. - // If cumulative is false, if any test failed, the total score is 0/1. Otherwise 1/1. - Cumulative bool -} - -// GetName return the test name -func (i TestInfo) GetName() string { return i.Name } - -// GetDescription returns the test description -func (i TestInfo) GetDescription() string { return i.Description } - -// IsCumulative returns true if the test's scores are intended to be cumulative -func (i TestInfo) IsCumulative() bool { return i.Cumulative } - -// BasicTestConfig contains all variables required by the BasicTest TestSuite -type BasicTestConfig struct { - Client client.Client - CR *unstructured.Unstructured - ProxyPod *v1.Pod -} - -// OLMTestConfig contains all variables required by the OLMTest TestSuite -type OLMTestConfig struct { - Client client.Client - CR *unstructured.Unstructured - CSV *olmapiv1alpha1.ClusterServiceVersion - CRDsDir string - ProxyPod *v1.Pod -} - -// TestSuite contains a list of tests and results, along with the relative weights of each test -type TestSuite struct { - TestInfo - Tests []Test - TestResults []*TestResult - Weights map[string]float64 -} - -// Test definitions - -// CheckSpecTest is a scorecard test that verifies that the CR has a spec block -type CheckSpecTest struct { - TestInfo - BasicTestConfig -} - -// NewCheckSpecTest returns a new CheckSpecTest object -func NewCheckSpecTest(conf BasicTestConfig) *CheckSpecTest { - return &CheckSpecTest{ - BasicTestConfig: conf, - TestInfo: TestInfo{ - Name: "Spec Block Exists", - Description: "Custom Resource has a Spec Block", - Cumulative: false, - }, - } -} - -// CheckStatusTest is a scorecard test that verifies that the CR has a status block -type CheckStatusTest struct { - TestInfo - BasicTestConfig -} - -// NewCheckStatusTest returns a new CheckStatusTest object -func NewCheckStatusTest(conf BasicTestConfig) *CheckStatusTest { - return &CheckStatusTest{ - BasicTestConfig: conf, - TestInfo: TestInfo{ - Name: "Status Block Exists", - Description: "Custom Resource has a Status Block", - Cumulative: false, - }, - } -} - -// WritingIntoCRsHasEffectTest is a scorecard test that verifies that the operator is making PUT and/or POST requests to the API server -type WritingIntoCRsHasEffectTest struct { - TestInfo - BasicTestConfig -} - -// NewWritingIntoCRsHasEffectTest returns a new WritingIntoCRsHasEffectTest object -func NewWritingIntoCRsHasEffectTest(conf BasicTestConfig) *WritingIntoCRsHasEffectTest { - return &WritingIntoCRsHasEffectTest{ - BasicTestConfig: conf, - TestInfo: TestInfo{ - Name: "Writing into CRs has an effect", - Description: "A CR sends PUT/POST requests to the API server to modify resources in response to spec block changes", - Cumulative: false, - }, - } -} - -// CRDsHaveValidationTest is a scorecard test that verifies that all CRDs have a validation section -type CRDsHaveValidationTest struct { - TestInfo - OLMTestConfig -} - -// NewCRDsHaveValidationTest returns a new CRDsHaveValidationTest object -func NewCRDsHaveValidationTest(conf OLMTestConfig) *CRDsHaveValidationTest { - return &CRDsHaveValidationTest{ - OLMTestConfig: conf, - TestInfo: TestInfo{ - Name: "Provided APIs have validation", - Description: "All CRDs have an OpenAPI validation subsection", - Cumulative: true, - }, - } -} - -// CRDsHaveResourcesTest is a scorecard test that verifies that the CSV lists used resources in its owned CRDs secyion -type CRDsHaveResourcesTest struct { - TestInfo - OLMTestConfig -} - -// NewCRDsHaveResourcesTest returns a new CRDsHaveResourcesTest object -func NewCRDsHaveResourcesTest(conf OLMTestConfig) *CRDsHaveResourcesTest { - return &CRDsHaveResourcesTest{ - OLMTestConfig: conf, - TestInfo: TestInfo{ - Name: "Owned CRDs have resources listed", - Description: "All Owned CRDs contain a resources subsection", - Cumulative: true, - }, - } -} - -// AnnotationsContainExamplesTest is a scorecard test that verifies that the CSV contains examples via the alm-examples annotation -type AnnotationsContainExamplesTest struct { - TestInfo - OLMTestConfig -} - -// NewAnnotationsContainExamplesTest returns a new AnnotationsContainExamplesTest object -func NewAnnotationsContainExamplesTest(conf OLMTestConfig) *AnnotationsContainExamplesTest { - return &AnnotationsContainExamplesTest{ - OLMTestConfig: conf, - TestInfo: TestInfo{ - Name: "CRs have at least 1 example", - Description: "The CSV's metadata contains an alm-examples section", - Cumulative: true, - }, - } -} - -// SpecDescriptorsTest is a scorecard test that verifies that all spec fields have descriptors -type SpecDescriptorsTest struct { - TestInfo - OLMTestConfig -} - -// NewSpecDescriptorsTest returns a new SpecDescriptorsTest object -func NewSpecDescriptorsTest(conf OLMTestConfig) *SpecDescriptorsTest { - return &SpecDescriptorsTest{ - OLMTestConfig: conf, - TestInfo: TestInfo{ - Name: "Spec fields with descriptors", - Description: "All spec fields have matching descriptors in the CSV", - Cumulative: true, - }, - } -} - -// StatusDescriptorsTest is a scorecard test that verifies that all status fields have descriptors -type StatusDescriptorsTest struct { - TestInfo - OLMTestConfig -} - -// NewStatusDescriptorsTest returns a new StatusDescriptorsTest object -func NewStatusDescriptorsTest(conf OLMTestConfig) *StatusDescriptorsTest { - return &StatusDescriptorsTest{ - OLMTestConfig: conf, - TestInfo: TestInfo{ - Name: "Status fields with descriptors", - Description: "All status fields have matching descriptors in the CSV", - Cumulative: true, - }, - } -} - -// Test Suite Declarations - -// NewBasicTestSuite returns a new TestSuite object containing basic, functional operator tests -func NewBasicTestSuite(conf BasicTestConfig) *TestSuite { - ts := NewTestSuite( - "Basic Tests", - "Test suite that runs basic, functional operator tests", - ) - ts.AddTest(NewCheckSpecTest(conf), 1.5) - ts.AddTest(NewCheckStatusTest(conf), 1) - ts.AddTest(NewWritingIntoCRsHasEffectTest(conf), 1) - - return ts -} - -// NewOLMTestSuite returns a new TestSuite object containing CSV best practice checks -func NewOLMTestSuite(conf OLMTestConfig) *TestSuite { - ts := NewTestSuite( - "OLM Tests", - "Test suite checks if an operator's CSV follows best practices", - ) - - ts.AddTest(NewCRDsHaveValidationTest(conf), 1.25) - ts.AddTest(NewCRDsHaveResourcesTest(conf), 1) - ts.AddTest(NewAnnotationsContainExamplesTest(conf), 1) - ts.AddTest(NewSpecDescriptorsTest(conf), 1) - ts.AddTest(NewStatusDescriptorsTest(conf), 1) - - return ts -} - -// Helper functions - -// ResultsPassFail will be used when multiple CRs are supported -func ResultsPassFail(results []TestResult) (earned, max int) { - for _, result := range results { - if result.EarnedPoints != result.MaximumPoints { - return 0, 1 - } - } - return 1, 1 -} - -// ResultsCumulative will be used when multiple CRs are supported -func ResultsCumulative(results []TestResult) (earned, max int) { - for _, result := range results { - earned += result.EarnedPoints - max += result.MaximumPoints - } - return earned, max -} - -// AddTest adds a new Test to a TestSuite along with a relative weight for the new Test -func (ts *TestSuite) AddTest(t Test, weight float64) { - ts.Tests = append(ts.Tests, t) - ts.Weights[t.GetName()] = weight -} - -// TotalScore calculates and returns the total score of all run Tests in a TestSuite -func (ts *TestSuite) TotalScore() (score int) { - floatScore := 0.0 - for _, result := range ts.TestResults { - if result.MaximumPoints != 0 { - floatScore += (float64(result.EarnedPoints) / float64(result.MaximumPoints)) * ts.Weights[result.Test.GetName()] - } - } - // scale to a percentage - addedWeights := 0.0 - for _, weight := range ts.Weights { - addedWeights += weight - } - floatScore = floatScore * (100 / addedWeights) - return int(floatScore) -} - -// Run runs all Tests in a TestSuite -func (ts *TestSuite) Run(ctx context.Context) { - for _, test := range ts.Tests { - ts.TestResults = append(ts.TestResults, test.Run(ctx)) - } -} - -// NewTestSuite returns a new TestSuite with a given name and description -func NewTestSuite(name, description string) *TestSuite { - return &TestSuite{ - TestInfo: TestInfo{ - Name: name, - Description: description, - }, - Weights: make(map[string]float64), - } -} diff --git a/internal/pkg/scorecard/basic_tests.go b/internal/pkg/scorecard/basic_tests.go new file mode 100644 index 00000000000..8fca4c0200a --- /dev/null +++ b/internal/pkg/scorecard/basic_tests.go @@ -0,0 +1,169 @@ +// 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 ( + "context" + "encoding/json" + "fmt" + "strings" + + sclib "github.com/operator-framework/operator-sdk/pkg/scorecard/lib" + + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// BasicTestConfig contains all variables required by the BasicTest TestSuite +type BasicTestConfig struct { + Client client.Client + CR *unstructured.Unstructured + ProxyPod *v1.Pod +} + +// Test Defintions + +// CheckSpecTest is a scorecard test that verifies that the CR has a spec block +type CheckSpecTest struct { + sclib.TestInfo + BasicTestConfig +} + +// NewCheckSpecTest returns a new CheckSpecTest object +func NewCheckSpecTest(conf BasicTestConfig) *CheckSpecTest { + return &CheckSpecTest{ + BasicTestConfig: conf, + TestInfo: sclib.TestInfo{ + Name: "Spec Block Exists", + Description: "Custom Resource has a Spec Block", + Cumulative: false, + }, + } +} + +// CheckStatusTest is a scorecard test that verifies that the CR has a status block +type CheckStatusTest struct { + sclib.TestInfo + BasicTestConfig +} + +// NewCheckStatusTest returns a new CheckStatusTest object +func NewCheckStatusTest(conf BasicTestConfig) *CheckStatusTest { + return &CheckStatusTest{ + BasicTestConfig: conf, + TestInfo: sclib.TestInfo{ + Name: "Status Block Exists", + Description: "Custom Resource has a Status Block", + Cumulative: false, + }, + } +} + +// WritingIntoCRsHasEffectTest is a scorecard test that verifies that the operator is making PUT and/or POST requests to the API server +type WritingIntoCRsHasEffectTest struct { + sclib.TestInfo + BasicTestConfig +} + +// NewWritingIntoCRsHasEffectTest returns a new WritingIntoCRsHasEffectTest object +func NewWritingIntoCRsHasEffectTest(conf BasicTestConfig) *WritingIntoCRsHasEffectTest { + return &WritingIntoCRsHasEffectTest{ + BasicTestConfig: conf, + TestInfo: sclib.TestInfo{ + Name: "Writing into CRs has an effect", + Description: "A CR sends PUT/POST requests to the API server to modify resources in response to spec block changes", + Cumulative: false, + }, + } +} + +// NewBasicTestSuite returns a new TestSuite object containing basic, functional operator tests +func NewBasicTestSuite(conf BasicTestConfig) *sclib.TestSuite { + ts := sclib.NewTestSuite( + "Basic Tests", + "Test suite that runs basic, functional operator tests", + ) + ts.AddTest(NewCheckSpecTest(conf), 1.5) + ts.AddTest(NewCheckStatusTest(conf), 1) + ts.AddTest(NewWritingIntoCRsHasEffectTest(conf), 1) + + return ts +} + +// Test Implementations + +// Run - implements Test interface +func (t *CheckSpecTest) Run(ctx context.Context) *sclib.TestResult { + res := &sclib.TestResult{Test: t, MaximumPoints: 1} + err := t.Client.Get(ctx, types.NamespacedName{Namespace: t.CR.GetNamespace(), Name: t.CR.GetName()}, t.CR) + if err != nil { + 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 { + res.Suggestions = append(res.Suggestions, "Add a 'spec' field to your Custom Resource") + } + return res +} + +// Run - implements Test interface +func (t *CheckStatusTest) Run(ctx context.Context) *sclib.TestResult { + res := &sclib.TestResult{Test: t, MaximumPoints: 1} + err := t.Client.Get(ctx, types.NamespacedName{Namespace: t.CR.GetNamespace(), Name: t.CR.GetName()}, t.CR) + if err != nil { + 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 { + res.Suggestions = append(res.Suggestions, "Add a 'status' field to your Custom Resource") + } + return res +} + +// Run - implements Test interface +func (t *WritingIntoCRsHasEffectTest) Run(ctx context.Context) *sclib.TestResult { + res := &sclib.TestResult{Test: t, MaximumPoints: 1} + logs, err := getProxyLogs(t.ProxyPod) + if err != nil { + res.Errors = append(res.Errors, fmt.Errorf("error getting proxy logs: %v", err)) + return res + } + msgMap := make(map[string]interface{}) + for _, msg := range strings.Split(logs, "\n") { + if err := json.Unmarshal([]byte(msg), &msgMap); err != nil { + continue + } + method, ok := msgMap["method"].(string) + if !ok { + continue + } + if method == "PUT" || method == "POST" { + res.EarnedPoints = 1 + break + } + } + if res.EarnedPoints != 1 { + res.Suggestions = append(res.Suggestions, "The operator should write into objects to update state. No PUT or POST requests from the operator were recorded by the scorecard.") + } + return res +} diff --git a/internal/pkg/scorecard/helpers.go b/internal/pkg/scorecard/helpers.go new file mode 100644 index 00000000000..808623b82ab --- /dev/null +++ b/internal/pkg/scorecard/helpers.go @@ -0,0 +1,39 @@ +// 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 sclib "github.com/operator-framework/operator-sdk/pkg/scorecard/lib" + +// These functions should be in the public test definitions file, but they are not complete/stable, +// so we'll keep these here until they get fully implemented + +// ResultsPassFail will be used when multiple CRs are supported +func ResultsPassFail(results []sclib.TestResult) (earned, max int) { + for _, result := range results { + if result.EarnedPoints != result.MaximumPoints { + return 0, 1 + } + } + return 1, 1 +} + +// ResultsCumulative will be used when multiple CRs are supported +func ResultsCumulative(results []sclib.TestResult) (earned, max int) { + for _, result := range results { + earned += result.EarnedPoints + max += result.MaximumPoints + } + return earned, max +} diff --git a/cmd/operator-sdk/scorecard/olm_tests.go b/internal/pkg/scorecard/olm_tests.go similarity index 69% rename from cmd/operator-sdk/scorecard/olm_tests.go rename to internal/pkg/scorecard/olm_tests.go index 252b2655fd7..eec7576e5ac 100644 --- a/cmd/operator-sdk/scorecard/olm_tests.go +++ b/internal/pkg/scorecard/olm_tests.go @@ -21,15 +21,119 @@ import ( "strings" "github.com/operator-framework/operator-sdk/internal/util/k8sutil" + sclib "github.com/operator-framework/operator-sdk/pkg/scorecard/lib" olmapiv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1" log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" ) +// OLMTestConfig contains all variables required by the OLMTest TestSuite +type OLMTestConfig struct { + Client client.Client + CR *unstructured.Unstructured + CSV *olmapiv1alpha1.ClusterServiceVersion + CRDsDir string + ProxyPod *v1.Pod +} + +// Test Defintions + +// CRDsHaveValidationTest is a scorecard test that verifies that all CRDs have a validation section +type CRDsHaveValidationTest struct { + sclib.TestInfo + OLMTestConfig +} + +// NewCRDsHaveValidationTest returns a new CRDsHaveValidationTest object +func NewCRDsHaveValidationTest(conf OLMTestConfig) *CRDsHaveValidationTest { + return &CRDsHaveValidationTest{ + OLMTestConfig: conf, + TestInfo: sclib.TestInfo{ + Name: "Provided APIs have validation", + Description: "All CRDs have an OpenAPI validation subsection", + Cumulative: true, + }, + } +} + +// CRDsHaveResourcesTest is a scorecard test that verifies that the CSV lists used resources in its owned CRDs secyion +type CRDsHaveResourcesTest struct { + sclib.TestInfo + OLMTestConfig +} + +// NewCRDsHaveResourcesTest returns a new CRDsHaveResourcesTest object +func NewCRDsHaveResourcesTest(conf OLMTestConfig) *CRDsHaveResourcesTest { + return &CRDsHaveResourcesTest{ + OLMTestConfig: conf, + TestInfo: sclib.TestInfo{ + Name: "Owned CRDs have resources listed", + Description: "All Owned CRDs contain a resources subsection", + Cumulative: true, + }, + } +} + +// AnnotationsContainExamplesTest is a scorecard test that verifies that the CSV contains examples via the alm-examples annotation +type AnnotationsContainExamplesTest struct { + sclib.TestInfo + OLMTestConfig +} + +// NewAnnotationsContainExamplesTest returns a new AnnotationsContainExamplesTest object +func NewAnnotationsContainExamplesTest(conf OLMTestConfig) *AnnotationsContainExamplesTest { + return &AnnotationsContainExamplesTest{ + OLMTestConfig: conf, + TestInfo: sclib.TestInfo{ + Name: "CRs have at least 1 example", + Description: "The CSV's metadata contains an alm-examples section", + Cumulative: true, + }, + } +} + +// SpecDescriptorsTest is a scorecard test that verifies that all spec fields have descriptors +type SpecDescriptorsTest struct { + sclib.TestInfo + OLMTestConfig +} + +// NewSpecDescriptorsTest returns a new SpecDescriptorsTest object +func NewSpecDescriptorsTest(conf OLMTestConfig) *SpecDescriptorsTest { + return &SpecDescriptorsTest{ + OLMTestConfig: conf, + TestInfo: sclib.TestInfo{ + Name: "Spec fields with descriptors", + Description: "All spec fields have matching descriptors in the CSV", + Cumulative: true, + }, + } +} + +// StatusDescriptorsTest is a scorecard test that verifies that all status fields have descriptors +type StatusDescriptorsTest struct { + sclib.TestInfo + OLMTestConfig +} + +// NewStatusDescriptorsTest returns a new StatusDescriptorsTest object +func NewStatusDescriptorsTest(conf OLMTestConfig) *StatusDescriptorsTest { + return &StatusDescriptorsTest{ + OLMTestConfig: conf, + TestInfo: sclib.TestInfo{ + Name: "Status fields with descriptors", + Description: "All status fields have matching descriptors in the CSV", + Cumulative: true, + }, + } +} + func matchKind(kind1, kind2 string) bool { singularKind1, err := restMapper.ResourceSingularizer(kind1) if err != nil { @@ -44,6 +148,24 @@ func matchKind(kind1, kind2 string) bool { return strings.EqualFold(singularKind1, singularKind2) } +// NewOLMTestSuite returns a new TestSuite object containing CSV best practice checks +func NewOLMTestSuite(conf OLMTestConfig) *sclib.TestSuite { + ts := sclib.NewTestSuite( + "OLM Tests", + "Test suite checks if an operator's CSV follows best practices", + ) + + ts.AddTest(NewCRDsHaveValidationTest(conf), 1.25) + ts.AddTest(NewCRDsHaveResourcesTest(conf), 1) + ts.AddTest(NewAnnotationsContainExamplesTest(conf), 1) + ts.AddTest(NewSpecDescriptorsTest(conf), 1) + ts.AddTest(NewStatusDescriptorsTest(conf), 1) + + return ts +} + +// Test Implentations + // matchVersion checks if a CRD contains a specified version in a case insensitive manner func matchVersion(version string, crd *apiextv1beta1.CustomResourceDefinition) bool { if strings.EqualFold(version, crd.Spec.Version) { @@ -59,8 +181,8 @@ func matchVersion(version string, crd *apiextv1beta1.CustomResourceDefinition) b } // Run - implements Test interface -func (t *CRDsHaveValidationTest) Run(ctx context.Context) *TestResult { - res := &TestResult{Test: t} +func (t *CRDsHaveValidationTest) Run(ctx context.Context) *sclib.TestResult { + res := &sclib.TestResult{Test: t} crds, err := k8sutil.GetCRDs(t.CRDsDir) if err != nil { res.Errors = append(res.Errors, fmt.Errorf("failed to get CRDs in %s directory: %v", t.CRDsDir, err)) @@ -112,8 +234,8 @@ func (t *CRDsHaveValidationTest) Run(ctx context.Context) *TestResult { } // Run - implements Test interface -func (t *CRDsHaveResourcesTest) Run(ctx context.Context) *TestResult { - res := &TestResult{Test: t} +func (t *CRDsHaveResourcesTest) Run(ctx context.Context) *sclib.TestResult { + res := &sclib.TestResult{Test: t} for _, crd := range t.CSV.Spec.CustomResourceDefinitions.Owned { res.MaximumPoints++ gvk := t.CR.GroupVersionKind() @@ -235,8 +357,8 @@ func getUsedResources(proxyPod *v1.Pod) ([]schema.GroupVersionKind, error) { } // Run - implements Test interface -func (t *AnnotationsContainExamplesTest) Run(ctx context.Context) *TestResult { - res := &TestResult{Test: t, MaximumPoints: 1} +func (t *AnnotationsContainExamplesTest) Run(ctx context.Context) *sclib.TestResult { + res := &sclib.TestResult{Test: t, MaximumPoints: 1} if t.CSV.Annotations != nil && t.CSV.Annotations["alm-examples"] != "" { res.EarnedPoints = 1 } @@ -247,8 +369,8 @@ func (t *AnnotationsContainExamplesTest) Run(ctx context.Context) *TestResult { } // Run - implements Test interface -func (t *StatusDescriptorsTest) Run(ctx context.Context) *TestResult { - res := &TestResult{Test: t} +func (t *StatusDescriptorsTest) Run(ctx context.Context) *sclib.TestResult { + res := &sclib.TestResult{Test: t} err := t.Client.Get(ctx, types.NamespacedName{Namespace: t.CR.GetNamespace(), Name: t.CR.GetName()}, t.CR) if err != nil { res.Errors = append(res.Errors, err) @@ -285,8 +407,8 @@ func (t *StatusDescriptorsTest) Run(ctx context.Context) *TestResult { } // Run - implements Test interface -func (t *SpecDescriptorsTest) Run(ctx context.Context) *TestResult { - res := &TestResult{Test: t} +func (t *SpecDescriptorsTest) Run(ctx context.Context) *sclib.TestResult { + res := &sclib.TestResult{Test: t} err := t.Client.Get(ctx, types.NamespacedName{Namespace: t.CR.GetNamespace(), Name: t.CR.GetName()}, t.CR) if err != nil { res.Errors = append(res.Errors, err) diff --git a/cmd/operator-sdk/scorecard/resource_handler.go b/internal/pkg/scorecard/resource_handler.go similarity index 100% rename from cmd/operator-sdk/scorecard/resource_handler.go rename to internal/pkg/scorecard/resource_handler.go index 2262dcf4f06..dc2fe27435d 100644 --- a/cmd/operator-sdk/scorecard/resource_handler.go +++ b/internal/pkg/scorecard/resource_handler.go @@ -26,11 +26,10 @@ import ( "github.com/operator-framework/operator-sdk/internal/util/yamlutil" proxyConf "github.com/operator-framework/operator-sdk/pkg/ansible/proxy/kubeconfig" "github.com/operator-framework/operator-sdk/pkg/k8sutil" - "github.com/spf13/viper" - "sigs.k8s.io/controller-runtime/pkg/client" "github.com/ghodss/yaml" log "github.com/sirupsen/logrus" + "github.com/spf13/viper" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -41,6 +40,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" + "sigs.k8s.io/controller-runtime/pkg/client" ) type cleanupFn func() error diff --git a/cmd/operator-sdk/scorecard/scorecard.go b/internal/pkg/scorecard/scorecard.go similarity index 99% rename from cmd/operator-sdk/scorecard/scorecard.go rename to internal/pkg/scorecard/scorecard.go index fe15b4d1706..a6353881194 100644 --- a/cmd/operator-sdk/scorecard/scorecard.go +++ b/internal/pkg/scorecard/scorecard.go @@ -26,6 +26,7 @@ import ( k8sInternal "github.com/operator-framework/operator-sdk/internal/util/k8sutil" "github.com/operator-framework/operator-sdk/internal/util/projutil" "github.com/operator-framework/operator-sdk/internal/util/yamlutil" + sclib "github.com/operator-framework/operator-sdk/pkg/scorecard/lib" "github.com/ghodss/yaml" olmapiv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1" @@ -249,7 +250,7 @@ func ScorecardTests(cmd *cobra.Command, args []string) error { if err := waitUntilCRStatusExists(obj); err != nil { return fmt.Errorf("failed waiting to check if CR status exists: %v", err) } - var suites []*TestSuite + var suites []*sclib.TestSuite // Run tests. if viper.GetBool(BasicTestsOpt) { diff --git a/pkg/scorecard/lib/test_definitions.go b/pkg/scorecard/lib/test_definitions.go new file mode 100644 index 00000000000..9fc5a845178 --- /dev/null +++ b/pkg/scorecard/lib/test_definitions.go @@ -0,0 +1,107 @@ +// 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 sclib + +import ( + "context" +) + +// Type Definitions + +// Test provides methods for running scorecard tests +type Test interface { + GetName() string + GetDescription() string + IsCumulative() bool + Run(context.Context) *TestResult +} + +// TestResult contains a test's points, suggestions, and errors +type TestResult struct { + Test Test + EarnedPoints int + MaximumPoints int + Suggestions []string + Errors []error +} + +// TestInfo contains information about the scorecard test +type TestInfo struct { + Name string + Description string + // If a test is set to cumulative, the scores of multiple runs of the same test on separate CRs are added together for the total score. + // If cumulative is false, if any test failed, the total score is 0/1. Otherwise 1/1. + Cumulative bool +} + +// GetName return the test name +func (i TestInfo) GetName() string { return i.Name } + +// GetDescription returns the test description +func (i TestInfo) GetDescription() string { return i.Description } + +// IsCumulative returns true if the test's scores are intended to be cumulative +func (i TestInfo) IsCumulative() bool { return i.Cumulative } + +// TestSuite contains a list of tests and results, along with the relative weights of each test +type TestSuite struct { + TestInfo + Tests []Test + TestResults []*TestResult + Weights map[string]float64 +} + +// Helper functions + +// AddTest adds a new Test to a TestSuite along with a relative weight for the new Test +func (ts *TestSuite) AddTest(t Test, weight float64) { + ts.Tests = append(ts.Tests, t) + ts.Weights[t.GetName()] = weight +} + +// TotalScore calculates and returns the total score of all run Tests in a TestSuite +func (ts *TestSuite) TotalScore() (score int) { + floatScore := 0.0 + for _, result := range ts.TestResults { + if result.MaximumPoints != 0 { + floatScore += (float64(result.EarnedPoints) / float64(result.MaximumPoints)) * ts.Weights[result.Test.GetName()] + } + } + // scale to a percentage + addedWeights := 0.0 + for _, weight := range ts.Weights { + addedWeights += weight + } + floatScore = floatScore * (100 / addedWeights) + return int(floatScore) +} + +// Run runs all Tests in a TestSuite +func (ts *TestSuite) Run(ctx context.Context) { + for _, test := range ts.Tests { + ts.TestResults = append(ts.TestResults, test.Run(ctx)) + } +} + +// NewTestSuite returns a new TestSuite with a given name and description +func NewTestSuite(name, description string) *TestSuite { + return &TestSuite{ + TestInfo: TestInfo{ + Name: name, + Description: description, + }, + Weights: make(map[string]float64), + } +} From 689f021d011dba6cfee4e261634ee87afcac3b91 Mon Sep 17 00:00:00 2001 From: Alex Pavel Date: Wed, 3 Apr 2019 10:00:19 -0700 Subject: [PATCH 2/6] pkg/scorecard: move pkg/scorecard/lib to pkg/scorecard --- internal/pkg/scorecard/basic_tests.go | 2 +- internal/pkg/scorecard/helpers.go | 2 +- internal/pkg/scorecard/olm_tests.go | 2 +- internal/pkg/scorecard/scorecard.go | 2 +- pkg/scorecard/{lib => }/test_definitions.go | 0 5 files changed, 4 insertions(+), 4 deletions(-) rename pkg/scorecard/{lib => }/test_definitions.go (100%) diff --git a/internal/pkg/scorecard/basic_tests.go b/internal/pkg/scorecard/basic_tests.go index 8fca4c0200a..5c415b7f884 100644 --- a/internal/pkg/scorecard/basic_tests.go +++ b/internal/pkg/scorecard/basic_tests.go @@ -20,7 +20,7 @@ import ( "fmt" "strings" - sclib "github.com/operator-framework/operator-sdk/pkg/scorecard/lib" + sclib "github.com/operator-framework/operator-sdk/pkg/scorecard" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" diff --git a/internal/pkg/scorecard/helpers.go b/internal/pkg/scorecard/helpers.go index 808623b82ab..6442126de6f 100644 --- a/internal/pkg/scorecard/helpers.go +++ b/internal/pkg/scorecard/helpers.go @@ -14,7 +14,7 @@ package scorecard -import sclib "github.com/operator-framework/operator-sdk/pkg/scorecard/lib" +import sclib "github.com/operator-framework/operator-sdk/pkg/scorecard" // These functions should be in the public test definitions file, but they are not complete/stable, // so we'll keep these here until they get fully implemented diff --git a/internal/pkg/scorecard/olm_tests.go b/internal/pkg/scorecard/olm_tests.go index eec7576e5ac..16be3e58004 100644 --- a/internal/pkg/scorecard/olm_tests.go +++ b/internal/pkg/scorecard/olm_tests.go @@ -21,7 +21,7 @@ import ( "strings" "github.com/operator-framework/operator-sdk/internal/util/k8sutil" - sclib "github.com/operator-framework/operator-sdk/pkg/scorecard/lib" + sclib "github.com/operator-framework/operator-sdk/pkg/scorecard" olmapiv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1" log "github.com/sirupsen/logrus" diff --git a/internal/pkg/scorecard/scorecard.go b/internal/pkg/scorecard/scorecard.go index a6353881194..ac2fef0c5d4 100644 --- a/internal/pkg/scorecard/scorecard.go +++ b/internal/pkg/scorecard/scorecard.go @@ -26,7 +26,7 @@ import ( k8sInternal "github.com/operator-framework/operator-sdk/internal/util/k8sutil" "github.com/operator-framework/operator-sdk/internal/util/projutil" "github.com/operator-framework/operator-sdk/internal/util/yamlutil" - sclib "github.com/operator-framework/operator-sdk/pkg/scorecard/lib" + sclib "github.com/operator-framework/operator-sdk/pkg/scorecard" "github.com/ghodss/yaml" olmapiv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1" diff --git a/pkg/scorecard/lib/test_definitions.go b/pkg/scorecard/test_definitions.go similarity index 100% rename from pkg/scorecard/lib/test_definitions.go rename to pkg/scorecard/test_definitions.go From c466da66bda1ddaee1cc80449a49271647e84205 Mon Sep 17 00:00:00 2001 From: Alex Pavel Date: Wed, 3 Apr 2019 10:11:26 -0700 Subject: [PATCH 3/6] cmd/.../scorecard: don't store cobra vars We can use viper directly now, so we can ignore the return values for cobra flags --- cmd/operator-sdk/scorecard/cmd.go | 53 +++++++++-------------------- internal/pkg/scorecard/scorecard.go | 6 ++-- 2 files changed, 19 insertions(+), 40 deletions(-) diff --git a/cmd/operator-sdk/scorecard/cmd.go b/cmd/operator-sdk/scorecard/cmd.go index ebd77497c4a..735ff0237b8 100644 --- a/cmd/operator-sdk/scorecard/cmd.go +++ b/cmd/operator-sdk/scorecard/cmd.go @@ -27,27 +27,6 @@ import ( "github.com/spf13/viper" ) -// scorecardConfig stores all scorecard config passed as flags -type scorecardConfig struct { - namespace string - kubeconfigPath string - initTimeout int - olmDeployed bool - csvPath string - basicTests bool - olmTests bool - tenantTests bool - namespacedManifest string - globalManifest string - crManifest string - proxyImage string - proxyPullPolicy string - crdsDir string - verbose bool -} - -var scConf scorecardConfig - func NewCmd() *cobra.Command { scorecardCmd := &cobra.Command{ Use: "scorecard", @@ -57,22 +36,22 @@ func NewCmd() *cobra.Command { RunE: scorecard.ScorecardTests, } - scorecardCmd.Flags().StringVar(&scorecard.ScorecardConf, scorecard.ConfigOpt, "", "config file (default is /.osdk-yaml)") - scorecardCmd.Flags().StringVar(&scConf.namespace, scorecard.NamespaceOpt, "", "Namespace of custom resource created in cluster") - scorecardCmd.Flags().StringVar(&scConf.kubeconfigPath, scorecard.KubeconfigOpt, "", "Path to kubeconfig of custom resource created in cluster") - scorecardCmd.Flags().IntVar(&scConf.initTimeout, scorecard.InitTimeoutOpt, 10, "Timeout for status block on CR to be created in seconds") - scorecardCmd.Flags().BoolVar(&scConf.olmDeployed, scorecard.OlmDeployedOpt, false, "The OLM has deployed the operator. Use only the CSV for test data") - scorecardCmd.Flags().StringVar(&scConf.csvPath, scorecard.CSVPathOpt, "", "Path to CSV being tested") - scorecardCmd.Flags().BoolVar(&scConf.basicTests, scorecard.BasicTestsOpt, true, "Enable basic operator checks") - scorecardCmd.Flags().BoolVar(&scConf.olmTests, scorecard.OLMTestsOpt, true, "Enable OLM integration checks") - scorecardCmd.Flags().BoolVar(&scConf.tenantTests, scorecard.TenantTestsOpt, false, "Enable good tenant checks") - scorecardCmd.Flags().StringVar(&scConf.namespacedManifest, scorecard.NamespacedManifestOpt, "", "Path to manifest for namespaced resources (e.g. RBAC and Operator manifest)") - scorecardCmd.Flags().StringVar(&scConf.globalManifest, scorecard.GlobalManifestOpt, "", "Path to manifest for Global resources (e.g. CRD manifests)") - scorecardCmd.Flags().StringVar(&scConf.crManifest, scorecard.CRManifestOpt, "", "Path to manifest for Custom Resource (required)") - scorecardCmd.Flags().StringVar(&scConf.proxyImage, scorecard.ProxyImageOpt, fmt.Sprintf("quay.io/operator-framework/scorecard-proxy:%s", strings.TrimSuffix(version.Version, "+git")), "Image name for scorecard proxy") - scorecardCmd.Flags().StringVar(&scConf.proxyPullPolicy, scorecard.ProxyPullPolicyOpt, "Always", "Pull policy for scorecard proxy image") - scorecardCmd.Flags().StringVar(&scConf.crdsDir, scorecard.CRDsDirOpt, scaffold.CRDsDir, "Directory containing CRDs (all CRD manifest filenames must have the suffix 'crd.yaml')") - scorecardCmd.Flags().BoolVar(&scConf.verbose, scorecard.VerboseOpt, false, "Enable verbose logging") + scorecardCmd.Flags().String(scorecard.ConfigOpt, "", "config file (default is /.osdk-yaml)") + scorecardCmd.Flags().String(scorecard.NamespaceOpt, "", "Namespace of custom resource created in cluster") + scorecardCmd.Flags().String(scorecard.KubeconfigOpt, "", "Path to kubeconfig of custom resource created in cluster") + scorecardCmd.Flags().Int(scorecard.InitTimeoutOpt, 10, "Timeout for status block on CR to be created in seconds") + scorecardCmd.Flags().Bool(scorecard.OlmDeployedOpt, false, "The OLM has deployed the operator. Use only the CSV for test data") + scorecardCmd.Flags().String(scorecard.CSVPathOpt, "", "Path to CSV being tested") + scorecardCmd.Flags().Bool(scorecard.BasicTestsOpt, true, "Enable basic operator checks") + scorecardCmd.Flags().Bool(scorecard.OLMTestsOpt, true, "Enable OLM integration checks") + scorecardCmd.Flags().Bool(scorecard.TenantTestsOpt, false, "Enable good tenant checks") + scorecardCmd.Flags().String(scorecard.NamespacedManifestOpt, "", "Path to manifest for namespaced resources (e.g. RBAC and Operator manifest)") + scorecardCmd.Flags().String(scorecard.GlobalManifestOpt, "", "Path to manifest for Global resources (e.g. CRD manifests)") + scorecardCmd.Flags().String(scorecard.CRManifestOpt, "", "Path to manifest for Custom Resource (required)") + scorecardCmd.Flags().String(scorecard.ProxyImageOpt, fmt.Sprintf("quay.io/operator-framework/scorecard-proxy:%s", strings.TrimSuffix(version.Version, "+git")), "Image name for scorecard proxy") + scorecardCmd.Flags().String(scorecard.ProxyPullPolicyOpt, "Always", "Pull policy for scorecard proxy image") + scorecardCmd.Flags().String(scorecard.CRDsDirOpt, scaffold.CRDsDir, "Directory containing CRDs (all CRD manifest filenames must have the suffix 'crd.yaml')") + scorecardCmd.Flags().Bool(scorecard.VerboseOpt, false, "Enable verbose logging") if err := viper.BindPFlags(scorecardCmd.Flags()); err != nil { log.Fatalf("Failed to bind scorecard flags to viper: %v", err) diff --git a/internal/pkg/scorecard/scorecard.go b/internal/pkg/scorecard/scorecard.go index ac2fef0c5d4..fdc757cf5e3 100644 --- a/internal/pkg/scorecard/scorecard.go +++ b/internal/pkg/scorecard/scorecard.go @@ -79,7 +79,6 @@ var ( deploymentName string proxyPodGlobal *v1.Pod cleanupFns []cleanupFn - ScorecardConf string ) const ( @@ -307,9 +306,10 @@ func ScorecardTests(cmd *cobra.Command, args []string) error { } func initConfig() error { - if ScorecardConf != "" { + // viper/cobra already has flags parsed at this point; we can check is a config file flag is set + if viper.GetString(ConfigOpt) != "" { // Use config file from the flag. - viper.SetConfigFile(ScorecardConf) + viper.SetConfigFile(viper.GetString(ConfigOpt)) } else { viper.AddConfigPath(projutil.MustGetwd()) // using SetConfigName allows users to use a .yaml, .json, or .toml file From 025c020ea36e0526aa67bc1f5d547e5608474102 Mon Sep 17 00:00:00 2001 From: Alex Pavel Date: Wed, 3 Apr 2019 10:25:33 -0700 Subject: [PATCH 4/6] (internal)/pkg/scorecard: update package and import names --- internal/pkg/scorecard/basic_tests.go | 30 ++++++++--------- internal/pkg/scorecard/helpers.go | 6 ++-- internal/pkg/scorecard/olm_tests.go | 46 +++++++++++++-------------- internal/pkg/scorecard/scorecard.go | 4 +-- pkg/scorecard/test_definitions.go | 2 +- 5 files changed, 44 insertions(+), 44 deletions(-) diff --git a/internal/pkg/scorecard/basic_tests.go b/internal/pkg/scorecard/basic_tests.go index 5c415b7f884..4225849a0e2 100644 --- a/internal/pkg/scorecard/basic_tests.go +++ b/internal/pkg/scorecard/basic_tests.go @@ -20,7 +20,7 @@ import ( "fmt" "strings" - sclib "github.com/operator-framework/operator-sdk/pkg/scorecard" + "github.com/operator-framework/operator-sdk/pkg/scorecard" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -39,7 +39,7 @@ type BasicTestConfig struct { // CheckSpecTest is a scorecard test that verifies that the CR has a spec block type CheckSpecTest struct { - sclib.TestInfo + scorecard.TestInfo BasicTestConfig } @@ -47,7 +47,7 @@ type CheckSpecTest struct { func NewCheckSpecTest(conf BasicTestConfig) *CheckSpecTest { return &CheckSpecTest{ BasicTestConfig: conf, - TestInfo: sclib.TestInfo{ + TestInfo: scorecard.TestInfo{ Name: "Spec Block Exists", Description: "Custom Resource has a Spec Block", Cumulative: false, @@ -57,7 +57,7 @@ func NewCheckSpecTest(conf BasicTestConfig) *CheckSpecTest { // CheckStatusTest is a scorecard test that verifies that the CR has a status block type CheckStatusTest struct { - sclib.TestInfo + scorecard.TestInfo BasicTestConfig } @@ -65,7 +65,7 @@ type CheckStatusTest struct { func NewCheckStatusTest(conf BasicTestConfig) *CheckStatusTest { return &CheckStatusTest{ BasicTestConfig: conf, - TestInfo: sclib.TestInfo{ + TestInfo: scorecard.TestInfo{ Name: "Status Block Exists", Description: "Custom Resource has a Status Block", Cumulative: false, @@ -75,7 +75,7 @@ func NewCheckStatusTest(conf BasicTestConfig) *CheckStatusTest { // WritingIntoCRsHasEffectTest is a scorecard test that verifies that the operator is making PUT and/or POST requests to the API server type WritingIntoCRsHasEffectTest struct { - sclib.TestInfo + scorecard.TestInfo BasicTestConfig } @@ -83,7 +83,7 @@ type WritingIntoCRsHasEffectTest struct { func NewWritingIntoCRsHasEffectTest(conf BasicTestConfig) *WritingIntoCRsHasEffectTest { return &WritingIntoCRsHasEffectTest{ BasicTestConfig: conf, - TestInfo: sclib.TestInfo{ + TestInfo: scorecard.TestInfo{ Name: "Writing into CRs has an effect", Description: "A CR sends PUT/POST requests to the API server to modify resources in response to spec block changes", Cumulative: false, @@ -92,8 +92,8 @@ func NewWritingIntoCRsHasEffectTest(conf BasicTestConfig) *WritingIntoCRsHasEffe } // NewBasicTestSuite returns a new TestSuite object containing basic, functional operator tests -func NewBasicTestSuite(conf BasicTestConfig) *sclib.TestSuite { - ts := sclib.NewTestSuite( +func NewBasicTestSuite(conf BasicTestConfig) *scorecard.TestSuite { + ts := scorecard.NewTestSuite( "Basic Tests", "Test suite that runs basic, functional operator tests", ) @@ -107,8 +107,8 @@ func NewBasicTestSuite(conf BasicTestConfig) *sclib.TestSuite { // Test Implementations // Run - implements Test interface -func (t *CheckSpecTest) Run(ctx context.Context) *sclib.TestResult { - res := &sclib.TestResult{Test: t, MaximumPoints: 1} +func (t *CheckSpecTest) Run(ctx context.Context) *scorecard.TestResult { + res := &scorecard.TestResult{Test: t, MaximumPoints: 1} err := t.Client.Get(ctx, types.NamespacedName{Namespace: t.CR.GetNamespace(), Name: t.CR.GetName()}, t.CR) if err != nil { res.Errors = append(res.Errors, fmt.Errorf("error getting custom resource: %v", err)) @@ -124,8 +124,8 @@ func (t *CheckSpecTest) Run(ctx context.Context) *sclib.TestResult { } // Run - implements Test interface -func (t *CheckStatusTest) Run(ctx context.Context) *sclib.TestResult { - res := &sclib.TestResult{Test: t, MaximumPoints: 1} +func (t *CheckStatusTest) Run(ctx context.Context) *scorecard.TestResult { + res := &scorecard.TestResult{Test: t, MaximumPoints: 1} err := t.Client.Get(ctx, types.NamespacedName{Namespace: t.CR.GetNamespace(), Name: t.CR.GetName()}, t.CR) if err != nil { res.Errors = append(res.Errors, fmt.Errorf("error getting custom resource: %v", err)) @@ -141,8 +141,8 @@ func (t *CheckStatusTest) Run(ctx context.Context) *sclib.TestResult { } // Run - implements Test interface -func (t *WritingIntoCRsHasEffectTest) Run(ctx context.Context) *sclib.TestResult { - res := &sclib.TestResult{Test: t, MaximumPoints: 1} +func (t *WritingIntoCRsHasEffectTest) Run(ctx context.Context) *scorecard.TestResult { + res := &scorecard.TestResult{Test: t, MaximumPoints: 1} logs, err := getProxyLogs(t.ProxyPod) if err != nil { res.Errors = append(res.Errors, fmt.Errorf("error getting proxy logs: %v", err)) diff --git a/internal/pkg/scorecard/helpers.go b/internal/pkg/scorecard/helpers.go index 6442126de6f..0675520e533 100644 --- a/internal/pkg/scorecard/helpers.go +++ b/internal/pkg/scorecard/helpers.go @@ -14,13 +14,13 @@ package scorecard -import sclib "github.com/operator-framework/operator-sdk/pkg/scorecard" +import "github.com/operator-framework/operator-sdk/pkg/scorecard" // These functions should be in the public test definitions file, but they are not complete/stable, // so we'll keep these here until they get fully implemented // ResultsPassFail will be used when multiple CRs are supported -func ResultsPassFail(results []sclib.TestResult) (earned, max int) { +func ResultsPassFail(results []scorecard.TestResult) (earned, max int) { for _, result := range results { if result.EarnedPoints != result.MaximumPoints { return 0, 1 @@ -30,7 +30,7 @@ func ResultsPassFail(results []sclib.TestResult) (earned, max int) { } // ResultsCumulative will be used when multiple CRs are supported -func ResultsCumulative(results []sclib.TestResult) (earned, max int) { +func ResultsCumulative(results []scorecard.TestResult) (earned, max int) { for _, result := range results { earned += result.EarnedPoints max += result.MaximumPoints diff --git a/internal/pkg/scorecard/olm_tests.go b/internal/pkg/scorecard/olm_tests.go index 16be3e58004..9a4e7d6b7d4 100644 --- a/internal/pkg/scorecard/olm_tests.go +++ b/internal/pkg/scorecard/olm_tests.go @@ -21,7 +21,7 @@ import ( "strings" "github.com/operator-framework/operator-sdk/internal/util/k8sutil" - sclib "github.com/operator-framework/operator-sdk/pkg/scorecard" + "github.com/operator-framework/operator-sdk/pkg/scorecard" olmapiv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1" log "github.com/sirupsen/logrus" @@ -46,7 +46,7 @@ type OLMTestConfig struct { // CRDsHaveValidationTest is a scorecard test that verifies that all CRDs have a validation section type CRDsHaveValidationTest struct { - sclib.TestInfo + scorecard.TestInfo OLMTestConfig } @@ -54,7 +54,7 @@ type CRDsHaveValidationTest struct { func NewCRDsHaveValidationTest(conf OLMTestConfig) *CRDsHaveValidationTest { return &CRDsHaveValidationTest{ OLMTestConfig: conf, - TestInfo: sclib.TestInfo{ + TestInfo: scorecard.TestInfo{ Name: "Provided APIs have validation", Description: "All CRDs have an OpenAPI validation subsection", Cumulative: true, @@ -64,7 +64,7 @@ func NewCRDsHaveValidationTest(conf OLMTestConfig) *CRDsHaveValidationTest { // CRDsHaveResourcesTest is a scorecard test that verifies that the CSV lists used resources in its owned CRDs secyion type CRDsHaveResourcesTest struct { - sclib.TestInfo + scorecard.TestInfo OLMTestConfig } @@ -72,7 +72,7 @@ type CRDsHaveResourcesTest struct { func NewCRDsHaveResourcesTest(conf OLMTestConfig) *CRDsHaveResourcesTest { return &CRDsHaveResourcesTest{ OLMTestConfig: conf, - TestInfo: sclib.TestInfo{ + TestInfo: scorecard.TestInfo{ Name: "Owned CRDs have resources listed", Description: "All Owned CRDs contain a resources subsection", Cumulative: true, @@ -82,7 +82,7 @@ func NewCRDsHaveResourcesTest(conf OLMTestConfig) *CRDsHaveResourcesTest { // AnnotationsContainExamplesTest is a scorecard test that verifies that the CSV contains examples via the alm-examples annotation type AnnotationsContainExamplesTest struct { - sclib.TestInfo + scorecard.TestInfo OLMTestConfig } @@ -90,7 +90,7 @@ type AnnotationsContainExamplesTest struct { func NewAnnotationsContainExamplesTest(conf OLMTestConfig) *AnnotationsContainExamplesTest { return &AnnotationsContainExamplesTest{ OLMTestConfig: conf, - TestInfo: sclib.TestInfo{ + TestInfo: scorecard.TestInfo{ Name: "CRs have at least 1 example", Description: "The CSV's metadata contains an alm-examples section", Cumulative: true, @@ -100,7 +100,7 @@ func NewAnnotationsContainExamplesTest(conf OLMTestConfig) *AnnotationsContainEx // SpecDescriptorsTest is a scorecard test that verifies that all spec fields have descriptors type SpecDescriptorsTest struct { - sclib.TestInfo + scorecard.TestInfo OLMTestConfig } @@ -108,7 +108,7 @@ type SpecDescriptorsTest struct { func NewSpecDescriptorsTest(conf OLMTestConfig) *SpecDescriptorsTest { return &SpecDescriptorsTest{ OLMTestConfig: conf, - TestInfo: sclib.TestInfo{ + TestInfo: scorecard.TestInfo{ Name: "Spec fields with descriptors", Description: "All spec fields have matching descriptors in the CSV", Cumulative: true, @@ -118,7 +118,7 @@ func NewSpecDescriptorsTest(conf OLMTestConfig) *SpecDescriptorsTest { // StatusDescriptorsTest is a scorecard test that verifies that all status fields have descriptors type StatusDescriptorsTest struct { - sclib.TestInfo + scorecard.TestInfo OLMTestConfig } @@ -126,7 +126,7 @@ type StatusDescriptorsTest struct { func NewStatusDescriptorsTest(conf OLMTestConfig) *StatusDescriptorsTest { return &StatusDescriptorsTest{ OLMTestConfig: conf, - TestInfo: sclib.TestInfo{ + TestInfo: scorecard.TestInfo{ Name: "Status fields with descriptors", Description: "All status fields have matching descriptors in the CSV", Cumulative: true, @@ -149,8 +149,8 @@ func matchKind(kind1, kind2 string) bool { } // NewOLMTestSuite returns a new TestSuite object containing CSV best practice checks -func NewOLMTestSuite(conf OLMTestConfig) *sclib.TestSuite { - ts := sclib.NewTestSuite( +func NewOLMTestSuite(conf OLMTestConfig) *scorecard.TestSuite { + ts := scorecard.NewTestSuite( "OLM Tests", "Test suite checks if an operator's CSV follows best practices", ) @@ -181,8 +181,8 @@ func matchVersion(version string, crd *apiextv1beta1.CustomResourceDefinition) b } // Run - implements Test interface -func (t *CRDsHaveValidationTest) Run(ctx context.Context) *sclib.TestResult { - res := &sclib.TestResult{Test: t} +func (t *CRDsHaveValidationTest) Run(ctx context.Context) *scorecard.TestResult { + res := &scorecard.TestResult{Test: t} crds, err := k8sutil.GetCRDs(t.CRDsDir) if err != nil { res.Errors = append(res.Errors, fmt.Errorf("failed to get CRDs in %s directory: %v", t.CRDsDir, err)) @@ -234,8 +234,8 @@ func (t *CRDsHaveValidationTest) Run(ctx context.Context) *sclib.TestResult { } // Run - implements Test interface -func (t *CRDsHaveResourcesTest) Run(ctx context.Context) *sclib.TestResult { - res := &sclib.TestResult{Test: t} +func (t *CRDsHaveResourcesTest) Run(ctx context.Context) *scorecard.TestResult { + res := &scorecard.TestResult{Test: t} for _, crd := range t.CSV.Spec.CustomResourceDefinitions.Owned { res.MaximumPoints++ gvk := t.CR.GroupVersionKind() @@ -357,8 +357,8 @@ func getUsedResources(proxyPod *v1.Pod) ([]schema.GroupVersionKind, error) { } // Run - implements Test interface -func (t *AnnotationsContainExamplesTest) Run(ctx context.Context) *sclib.TestResult { - res := &sclib.TestResult{Test: t, MaximumPoints: 1} +func (t *AnnotationsContainExamplesTest) Run(ctx context.Context) *scorecard.TestResult { + res := &scorecard.TestResult{Test: t, MaximumPoints: 1} if t.CSV.Annotations != nil && t.CSV.Annotations["alm-examples"] != "" { res.EarnedPoints = 1 } @@ -369,8 +369,8 @@ func (t *AnnotationsContainExamplesTest) Run(ctx context.Context) *sclib.TestRes } // Run - implements Test interface -func (t *StatusDescriptorsTest) Run(ctx context.Context) *sclib.TestResult { - res := &sclib.TestResult{Test: t} +func (t *StatusDescriptorsTest) Run(ctx context.Context) *scorecard.TestResult { + res := &scorecard.TestResult{Test: t} err := t.Client.Get(ctx, types.NamespacedName{Namespace: t.CR.GetNamespace(), Name: t.CR.GetName()}, t.CR) if err != nil { res.Errors = append(res.Errors, err) @@ -407,8 +407,8 @@ func (t *StatusDescriptorsTest) Run(ctx context.Context) *sclib.TestResult { } // Run - implements Test interface -func (t *SpecDescriptorsTest) Run(ctx context.Context) *sclib.TestResult { - res := &sclib.TestResult{Test: t} +func (t *SpecDescriptorsTest) Run(ctx context.Context) *scorecard.TestResult { + res := &scorecard.TestResult{Test: t} err := t.Client.Get(ctx, types.NamespacedName{Namespace: t.CR.GetNamespace(), Name: t.CR.GetName()}, t.CR) if err != nil { res.Errors = append(res.Errors, err) diff --git a/internal/pkg/scorecard/scorecard.go b/internal/pkg/scorecard/scorecard.go index fdc757cf5e3..ceb4cb2cb87 100644 --- a/internal/pkg/scorecard/scorecard.go +++ b/internal/pkg/scorecard/scorecard.go @@ -26,7 +26,7 @@ import ( k8sInternal "github.com/operator-framework/operator-sdk/internal/util/k8sutil" "github.com/operator-framework/operator-sdk/internal/util/projutil" "github.com/operator-framework/operator-sdk/internal/util/yamlutil" - sclib "github.com/operator-framework/operator-sdk/pkg/scorecard" + "github.com/operator-framework/operator-sdk/pkg/scorecard" "github.com/ghodss/yaml" olmapiv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1" @@ -249,7 +249,7 @@ func ScorecardTests(cmd *cobra.Command, args []string) error { if err := waitUntilCRStatusExists(obj); err != nil { return fmt.Errorf("failed waiting to check if CR status exists: %v", err) } - var suites []*sclib.TestSuite + var suites []*scorecard.TestSuite // Run tests. if viper.GetBool(BasicTestsOpt) { diff --git a/pkg/scorecard/test_definitions.go b/pkg/scorecard/test_definitions.go index 9fc5a845178..b289e769685 100644 --- a/pkg/scorecard/test_definitions.go +++ b/pkg/scorecard/test_definitions.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package sclib +package scorecard import ( "context" From cd1abc6c0a8d961525a2351f01c6d6911d0fc09d Mon Sep 17 00:00:00 2001 From: Alex Pavel Date: Fri, 5 Apr 2019 10:56:56 -0700 Subject: [PATCH 5/6] (internal)/pkg/scorecard: keep everything internal for now --- internal/pkg/scorecard/basic_tests.go | 30 ++++++------- internal/pkg/scorecard/helpers.go | 6 +-- internal/pkg/scorecard/olm_tests.go | 45 +++++++++---------- internal/pkg/scorecard/scorecard.go | 3 +- .../pkg}/scorecard/test_definitions.go | 0 5 files changed, 39 insertions(+), 45 deletions(-) rename {pkg => internal/pkg}/scorecard/test_definitions.go (100%) diff --git a/internal/pkg/scorecard/basic_tests.go b/internal/pkg/scorecard/basic_tests.go index 4225849a0e2..6d4c7d98647 100644 --- a/internal/pkg/scorecard/basic_tests.go +++ b/internal/pkg/scorecard/basic_tests.go @@ -20,8 +20,6 @@ import ( "fmt" "strings" - "github.com/operator-framework/operator-sdk/pkg/scorecard" - v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" @@ -39,7 +37,7 @@ type BasicTestConfig struct { // CheckSpecTest is a scorecard test that verifies that the CR has a spec block type CheckSpecTest struct { - scorecard.TestInfo + TestInfo BasicTestConfig } @@ -47,7 +45,7 @@ type CheckSpecTest struct { func NewCheckSpecTest(conf BasicTestConfig) *CheckSpecTest { return &CheckSpecTest{ BasicTestConfig: conf, - TestInfo: scorecard.TestInfo{ + TestInfo: TestInfo{ Name: "Spec Block Exists", Description: "Custom Resource has a Spec Block", Cumulative: false, @@ -57,7 +55,7 @@ func NewCheckSpecTest(conf BasicTestConfig) *CheckSpecTest { // CheckStatusTest is a scorecard test that verifies that the CR has a status block type CheckStatusTest struct { - scorecard.TestInfo + TestInfo BasicTestConfig } @@ -65,7 +63,7 @@ type CheckStatusTest struct { func NewCheckStatusTest(conf BasicTestConfig) *CheckStatusTest { return &CheckStatusTest{ BasicTestConfig: conf, - TestInfo: scorecard.TestInfo{ + TestInfo: TestInfo{ Name: "Status Block Exists", Description: "Custom Resource has a Status Block", Cumulative: false, @@ -75,7 +73,7 @@ func NewCheckStatusTest(conf BasicTestConfig) *CheckStatusTest { // WritingIntoCRsHasEffectTest is a scorecard test that verifies that the operator is making PUT and/or POST requests to the API server type WritingIntoCRsHasEffectTest struct { - scorecard.TestInfo + TestInfo BasicTestConfig } @@ -83,7 +81,7 @@ type WritingIntoCRsHasEffectTest struct { func NewWritingIntoCRsHasEffectTest(conf BasicTestConfig) *WritingIntoCRsHasEffectTest { return &WritingIntoCRsHasEffectTest{ BasicTestConfig: conf, - TestInfo: scorecard.TestInfo{ + TestInfo: TestInfo{ Name: "Writing into CRs has an effect", Description: "A CR sends PUT/POST requests to the API server to modify resources in response to spec block changes", Cumulative: false, @@ -92,8 +90,8 @@ func NewWritingIntoCRsHasEffectTest(conf BasicTestConfig) *WritingIntoCRsHasEffe } // NewBasicTestSuite returns a new TestSuite object containing basic, functional operator tests -func NewBasicTestSuite(conf BasicTestConfig) *scorecard.TestSuite { - ts := scorecard.NewTestSuite( +func NewBasicTestSuite(conf BasicTestConfig) *TestSuite { + ts := NewTestSuite( "Basic Tests", "Test suite that runs basic, functional operator tests", ) @@ -107,8 +105,8 @@ func NewBasicTestSuite(conf BasicTestConfig) *scorecard.TestSuite { // Test Implementations // Run - implements Test interface -func (t *CheckSpecTest) Run(ctx context.Context) *scorecard.TestResult { - res := &scorecard.TestResult{Test: t, MaximumPoints: 1} +func (t *CheckSpecTest) Run(ctx context.Context) *TestResult { + res := &TestResult{Test: t, MaximumPoints: 1} err := t.Client.Get(ctx, types.NamespacedName{Namespace: t.CR.GetNamespace(), Name: t.CR.GetName()}, t.CR) if err != nil { res.Errors = append(res.Errors, fmt.Errorf("error getting custom resource: %v", err)) @@ -124,8 +122,8 @@ func (t *CheckSpecTest) Run(ctx context.Context) *scorecard.TestResult { } // Run - implements Test interface -func (t *CheckStatusTest) Run(ctx context.Context) *scorecard.TestResult { - res := &scorecard.TestResult{Test: t, MaximumPoints: 1} +func (t *CheckStatusTest) Run(ctx context.Context) *TestResult { + res := &TestResult{Test: t, MaximumPoints: 1} err := t.Client.Get(ctx, types.NamespacedName{Namespace: t.CR.GetNamespace(), Name: t.CR.GetName()}, t.CR) if err != nil { res.Errors = append(res.Errors, fmt.Errorf("error getting custom resource: %v", err)) @@ -141,8 +139,8 @@ func (t *CheckStatusTest) Run(ctx context.Context) *scorecard.TestResult { } // Run - implements Test interface -func (t *WritingIntoCRsHasEffectTest) Run(ctx context.Context) *scorecard.TestResult { - res := &scorecard.TestResult{Test: t, MaximumPoints: 1} +func (t *WritingIntoCRsHasEffectTest) Run(ctx context.Context) *TestResult { + res := &TestResult{Test: t, MaximumPoints: 1} logs, err := getProxyLogs(t.ProxyPod) if err != nil { res.Errors = append(res.Errors, fmt.Errorf("error getting proxy logs: %v", err)) diff --git a/internal/pkg/scorecard/helpers.go b/internal/pkg/scorecard/helpers.go index 0675520e533..f5bd0a9d04a 100644 --- a/internal/pkg/scorecard/helpers.go +++ b/internal/pkg/scorecard/helpers.go @@ -14,13 +14,11 @@ package scorecard -import "github.com/operator-framework/operator-sdk/pkg/scorecard" - // These functions should be in the public test definitions file, but they are not complete/stable, // so we'll keep these here until they get fully implemented // ResultsPassFail will be used when multiple CRs are supported -func ResultsPassFail(results []scorecard.TestResult) (earned, max int) { +func ResultsPassFail(results []TestResult) (earned, max int) { for _, result := range results { if result.EarnedPoints != result.MaximumPoints { return 0, 1 @@ -30,7 +28,7 @@ func ResultsPassFail(results []scorecard.TestResult) (earned, max int) { } // ResultsCumulative will be used when multiple CRs are supported -func ResultsCumulative(results []scorecard.TestResult) (earned, max int) { +func ResultsCumulative(results []TestResult) (earned, max int) { for _, result := range results { earned += result.EarnedPoints max += result.MaximumPoints diff --git a/internal/pkg/scorecard/olm_tests.go b/internal/pkg/scorecard/olm_tests.go index 9a4e7d6b7d4..9982229e13e 100644 --- a/internal/pkg/scorecard/olm_tests.go +++ b/internal/pkg/scorecard/olm_tests.go @@ -21,7 +21,6 @@ import ( "strings" "github.com/operator-framework/operator-sdk/internal/util/k8sutil" - "github.com/operator-framework/operator-sdk/pkg/scorecard" olmapiv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1" log "github.com/sirupsen/logrus" @@ -46,7 +45,7 @@ type OLMTestConfig struct { // CRDsHaveValidationTest is a scorecard test that verifies that all CRDs have a validation section type CRDsHaveValidationTest struct { - scorecard.TestInfo + TestInfo OLMTestConfig } @@ -54,7 +53,7 @@ type CRDsHaveValidationTest struct { func NewCRDsHaveValidationTest(conf OLMTestConfig) *CRDsHaveValidationTest { return &CRDsHaveValidationTest{ OLMTestConfig: conf, - TestInfo: scorecard.TestInfo{ + TestInfo: TestInfo{ Name: "Provided APIs have validation", Description: "All CRDs have an OpenAPI validation subsection", Cumulative: true, @@ -64,7 +63,7 @@ func NewCRDsHaveValidationTest(conf OLMTestConfig) *CRDsHaveValidationTest { // CRDsHaveResourcesTest is a scorecard test that verifies that the CSV lists used resources in its owned CRDs secyion type CRDsHaveResourcesTest struct { - scorecard.TestInfo + TestInfo OLMTestConfig } @@ -72,7 +71,7 @@ type CRDsHaveResourcesTest struct { func NewCRDsHaveResourcesTest(conf OLMTestConfig) *CRDsHaveResourcesTest { return &CRDsHaveResourcesTest{ OLMTestConfig: conf, - TestInfo: scorecard.TestInfo{ + TestInfo: TestInfo{ Name: "Owned CRDs have resources listed", Description: "All Owned CRDs contain a resources subsection", Cumulative: true, @@ -82,7 +81,7 @@ func NewCRDsHaveResourcesTest(conf OLMTestConfig) *CRDsHaveResourcesTest { // AnnotationsContainExamplesTest is a scorecard test that verifies that the CSV contains examples via the alm-examples annotation type AnnotationsContainExamplesTest struct { - scorecard.TestInfo + TestInfo OLMTestConfig } @@ -90,7 +89,7 @@ type AnnotationsContainExamplesTest struct { func NewAnnotationsContainExamplesTest(conf OLMTestConfig) *AnnotationsContainExamplesTest { return &AnnotationsContainExamplesTest{ OLMTestConfig: conf, - TestInfo: scorecard.TestInfo{ + TestInfo: TestInfo{ Name: "CRs have at least 1 example", Description: "The CSV's metadata contains an alm-examples section", Cumulative: true, @@ -100,7 +99,7 @@ func NewAnnotationsContainExamplesTest(conf OLMTestConfig) *AnnotationsContainEx // SpecDescriptorsTest is a scorecard test that verifies that all spec fields have descriptors type SpecDescriptorsTest struct { - scorecard.TestInfo + TestInfo OLMTestConfig } @@ -108,7 +107,7 @@ type SpecDescriptorsTest struct { func NewSpecDescriptorsTest(conf OLMTestConfig) *SpecDescriptorsTest { return &SpecDescriptorsTest{ OLMTestConfig: conf, - TestInfo: scorecard.TestInfo{ + TestInfo: TestInfo{ Name: "Spec fields with descriptors", Description: "All spec fields have matching descriptors in the CSV", Cumulative: true, @@ -118,7 +117,7 @@ func NewSpecDescriptorsTest(conf OLMTestConfig) *SpecDescriptorsTest { // StatusDescriptorsTest is a scorecard test that verifies that all status fields have descriptors type StatusDescriptorsTest struct { - scorecard.TestInfo + TestInfo OLMTestConfig } @@ -126,7 +125,7 @@ type StatusDescriptorsTest struct { func NewStatusDescriptorsTest(conf OLMTestConfig) *StatusDescriptorsTest { return &StatusDescriptorsTest{ OLMTestConfig: conf, - TestInfo: scorecard.TestInfo{ + TestInfo: TestInfo{ Name: "Status fields with descriptors", Description: "All status fields have matching descriptors in the CSV", Cumulative: true, @@ -149,8 +148,8 @@ func matchKind(kind1, kind2 string) bool { } // NewOLMTestSuite returns a new TestSuite object containing CSV best practice checks -func NewOLMTestSuite(conf OLMTestConfig) *scorecard.TestSuite { - ts := scorecard.NewTestSuite( +func NewOLMTestSuite(conf OLMTestConfig) *TestSuite { + ts := NewTestSuite( "OLM Tests", "Test suite checks if an operator's CSV follows best practices", ) @@ -181,8 +180,8 @@ func matchVersion(version string, crd *apiextv1beta1.CustomResourceDefinition) b } // Run - implements Test interface -func (t *CRDsHaveValidationTest) Run(ctx context.Context) *scorecard.TestResult { - res := &scorecard.TestResult{Test: t} +func (t *CRDsHaveValidationTest) Run(ctx context.Context) *TestResult { + res := &TestResult{Test: t} crds, err := k8sutil.GetCRDs(t.CRDsDir) if err != nil { res.Errors = append(res.Errors, fmt.Errorf("failed to get CRDs in %s directory: %v", t.CRDsDir, err)) @@ -234,8 +233,8 @@ func (t *CRDsHaveValidationTest) Run(ctx context.Context) *scorecard.TestResult } // Run - implements Test interface -func (t *CRDsHaveResourcesTest) Run(ctx context.Context) *scorecard.TestResult { - res := &scorecard.TestResult{Test: t} +func (t *CRDsHaveResourcesTest) Run(ctx context.Context) *TestResult { + res := &TestResult{Test: t} for _, crd := range t.CSV.Spec.CustomResourceDefinitions.Owned { res.MaximumPoints++ gvk := t.CR.GroupVersionKind() @@ -357,8 +356,8 @@ func getUsedResources(proxyPod *v1.Pod) ([]schema.GroupVersionKind, error) { } // Run - implements Test interface -func (t *AnnotationsContainExamplesTest) Run(ctx context.Context) *scorecard.TestResult { - res := &scorecard.TestResult{Test: t, MaximumPoints: 1} +func (t *AnnotationsContainExamplesTest) Run(ctx context.Context) *TestResult { + res := &TestResult{Test: t, MaximumPoints: 1} if t.CSV.Annotations != nil && t.CSV.Annotations["alm-examples"] != "" { res.EarnedPoints = 1 } @@ -369,8 +368,8 @@ func (t *AnnotationsContainExamplesTest) Run(ctx context.Context) *scorecard.Tes } // Run - implements Test interface -func (t *StatusDescriptorsTest) Run(ctx context.Context) *scorecard.TestResult { - res := &scorecard.TestResult{Test: t} +func (t *StatusDescriptorsTest) Run(ctx context.Context) *TestResult { + res := &TestResult{Test: t} err := t.Client.Get(ctx, types.NamespacedName{Namespace: t.CR.GetNamespace(), Name: t.CR.GetName()}, t.CR) if err != nil { res.Errors = append(res.Errors, err) @@ -407,8 +406,8 @@ func (t *StatusDescriptorsTest) Run(ctx context.Context) *scorecard.TestResult { } // Run - implements Test interface -func (t *SpecDescriptorsTest) Run(ctx context.Context) *scorecard.TestResult { - res := &scorecard.TestResult{Test: t} +func (t *SpecDescriptorsTest) Run(ctx context.Context) *TestResult { + res := &TestResult{Test: t} err := t.Client.Get(ctx, types.NamespacedName{Namespace: t.CR.GetNamespace(), Name: t.CR.GetName()}, t.CR) if err != nil { res.Errors = append(res.Errors, err) diff --git a/internal/pkg/scorecard/scorecard.go b/internal/pkg/scorecard/scorecard.go index ceb4cb2cb87..899c8c82664 100644 --- a/internal/pkg/scorecard/scorecard.go +++ b/internal/pkg/scorecard/scorecard.go @@ -26,7 +26,6 @@ import ( k8sInternal "github.com/operator-framework/operator-sdk/internal/util/k8sutil" "github.com/operator-framework/operator-sdk/internal/util/projutil" "github.com/operator-framework/operator-sdk/internal/util/yamlutil" - "github.com/operator-framework/operator-sdk/pkg/scorecard" "github.com/ghodss/yaml" olmapiv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1" @@ -249,7 +248,7 @@ func ScorecardTests(cmd *cobra.Command, args []string) error { if err := waitUntilCRStatusExists(obj); err != nil { return fmt.Errorf("failed waiting to check if CR status exists: %v", err) } - var suites []*scorecard.TestSuite + var suites []*TestSuite // Run tests. if viper.GetBool(BasicTestsOpt) { diff --git a/pkg/scorecard/test_definitions.go b/internal/pkg/scorecard/test_definitions.go similarity index 100% rename from pkg/scorecard/test_definitions.go rename to internal/pkg/scorecard/test_definitions.go From f63e2a436393eb41a0e36d99c7adb3177634ec6f Mon Sep 17 00:00:00 2001 From: Alex Pavel Date: Fri, 5 Apr 2019 12:28:32 -0700 Subject: [PATCH 6/6] internal/pkg/scorecard: fix small typo in a comment --- internal/pkg/scorecard/scorecard.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/scorecard/scorecard.go b/internal/pkg/scorecard/scorecard.go index 899c8c82664..f261e6ddccf 100644 --- a/internal/pkg/scorecard/scorecard.go +++ b/internal/pkg/scorecard/scorecard.go @@ -305,7 +305,7 @@ func ScorecardTests(cmd *cobra.Command, args []string) error { } func initConfig() error { - // viper/cobra already has flags parsed at this point; we can check is a config file flag is set + // viper/cobra already has flags parsed at this point; we can check if a config file flag is set if viper.GetString(ConfigOpt) != "" { // Use config file from the flag. viper.SetConfigFile(viper.GetString(ConfigOpt))