Repository: redhat-openshift-ecosystem/openshift-preflight
Pull request #188 (2021-08-31 12:28:49)

Add Validate Operator Bundle check to CraneEngine
The Validate Operator Bundle will now look for a bundle path, instead of a bundle image.

Signed-off-by: Brad P. Crochet <brad@redhat.com>

bcrochet: I will validate that this does not require privilege before merging.

/hold
openshift-ci[bot]: New changes are detected. LGTM label has been removed.
bcrochet: /hold cancel

This is passing without privilege now.
openshift-ci[bot]: @bcrochet: PR needs rebase.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>

Patch:
diff --git a/certification/engine/engine.go b/certification/engine/engine.go
index 9e1f072..527847c 100644
--- a/certification/engine/engine.go
+++ b/certification/engine/engine.go
@@ -129,10 +129,9 @@ var underLayerMaxCheck certification.Check = &shell.UnderLayerMaxCheck{}
 var hasRequiredLabelCheck certification.Check = &shell.HasRequiredLabelsCheck{}
 var basedOnUbiCheck certification.Check = &shell.BaseOnUBICheck{}
 var deprecatedHasLicenseCheck certification.Check = &shell.HasLicenseCheck{}
-var hasMinimalVulnerabilitiesCheck certification.Check = &shell.HasMinimalVulnerabilitiesCheck{}
 var hasUniqueTagCheck certification.Check = &shell.HasUniqueTagCheck{}
 var hasNoProhibitedCheck certification.Check = &shell.HasNoProhibitedPackagesCheck{}
-var validateOperatorBundle certification.Check = &shell.ValidateOperatorBundleCheck{}
+var deprecatedValidateOperatorBundle certification.Check = &shell.ValidateOperatorBundleCheck{}
 var scorecardBasicSpecCheck certification.Check = &shell.ScorecardBasicSpecCheck{}
 var scorecardOlmSuiteCheck certification.Check = &shell.ScorecardOlmSuiteCheck{}
 var hasNoProhibitedMountedCheck certification.Check = &shell.HasNoProhibitedPackagesMountedCheck{}
@@ -143,11 +142,15 @@ var hasMinimalVulnerabilitiesUnshareCheck certification.Check = &shell.HasMinima
 var deprecatedDeployableByOlmCheck certification.Check = &k8s.DeployableByOlmCheck{}
 var deprecatedDeployableByOlmMountedCheck certification.Check = &k8s.DeployableByOlmMountedCheck{}
 
+// Disabled due to issue #99 and discussions in community meeting
+// var hasMinimalVulnerabilitiesCheck certification.Check = &shell.HasMinimalVulnerabilitiesCheck{}
+
 // new checks for CraneEngine
 var hasLicenseCheck certification.Check = &containerpol.HasLicenseCheck{}
 var relatedImageManifestSchemaVersionCheck certification.Check = &operatorpol.RelatedImagesAreSchemaVersion2Check{}
 var deployableByOlmCheck certification.Check = &operatorpol.DeployableByOlmCheck{}
 var operatorPkgNameIsUniqueCheck certification.Check = &operatorpol.OperatorPkgNameIsUniqueCheck{}
+var validateOperatorBundle certification.Check = operatorpol.NewValidateOperatorBundleCheck(NewOperatorSdkEngine())
 
 var operatorPolicy = map[string]certification.Check{
 	operatorPkgNameIsUniqueCheck.Name():           operatorPkgNameIsUniqueCheck,
@@ -155,6 +158,7 @@ var operatorPolicy = map[string]certification.Check{
 	scorecardBasicSpecCheck.Name():                scorecardBasicSpecCheck,
 	scorecardOlmSuiteCheck.Name():                 scorecardOlmSuiteCheck,
 	deployableByOlmCheck.Name():                   deployableByOlmCheck,
+	validateOperatorBundle.Name():                 validateOperatorBundle,
 }
 
 var containerPolicy = map[string]certification.Check{
@@ -174,7 +178,7 @@ var oldContainerPolicy = map[string]certification.Check{
 }
 
 var oldOperatorPolicy = map[string]certification.Check{
-	validateOperatorBundle.Name():                           validateOperatorBundle,
+	deprecatedValidateOperatorBundle.Name():                 deprecatedValidateOperatorBundle,
 	scorecardBasicSpecCheck.Name():                          scorecardBasicSpecCheck,
 	scorecardOlmSuiteCheck.Name():                           scorecardOlmSuiteCheck,
 	deprecatedRelatedImageManifestSchemaVersionCheck.Name(): deprecatedRelatedImageManifestSchemaVersionCheck,
diff --git a/certification/engine/operatorsdk.go b/certification/engine/operatorsdk.go
new file mode 100644
index 0000000..9112075
--- /dev/null
+++ b/certification/engine/operatorsdk.go
@@ -0,0 +1,171 @@
+package engine
+
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"os"
+	"os/exec"
+	"strings"
+
+	"github.com/redhat-openshift-ecosystem/openshift-preflight/certification/errors"
+	fileutils "github.com/redhat-openshift-ecosystem/openshift-preflight/certification/internal/utils/file"
+	"github.com/redhat-openshift-ecosystem/openshift-preflight/cli"
+	log "github.com/sirupsen/logrus"
+)
+
+func NewOperatorSdkEngine() *cli.OperatorSdkEngine {
+	var engine cli.OperatorSdkEngine = operatorSdkEngine{}
+	return &engine
+}
+
+type operatorSdkEngine struct{}
+
+func (o operatorSdkEngine) Scorecard(image string, opts cli.OperatorSdkScorecardOptions) (*cli.OperatorSdkScorecardReport, error) {
+	cmdArgs := []string{"scorecard"}
+	if opts.OutputFormat == "" {
+		opts.OutputFormat = "json"
+	}
+	cmdArgs = append(cmdArgs, "--output", opts.OutputFormat)
+	if opts.Selector != nil {
+		for _, selector := range opts.Selector {
+			cmdArgs = append(cmdArgs, fmt.Sprintf("--selector=%s", selector))
+		}
+	}
+	if opts.Kubeconfig != "" {
+		cmdArgs = append(cmdArgs, "--kubeconfig", opts.Kubeconfig)
+	}
+	if opts.Namespace != "" {
+		cmdArgs = append(cmdArgs, "--namespace", opts.Namespace)
+	}
+	if opts.ServiceAccount != "" {
+		cmdArgs = append(cmdArgs, "--service-account", opts.ServiceAccount)
+	}
+
+	configFile, err := createScorecardConfigFile()
+	defer os.Remove(configFile)
+	if err != nil {
+		log.Error("could not create scorecard config file", err)
+		return nil, err
+	}
+	cmdArgs = append(cmdArgs, "--config", configFile)
+
+	cmdArgs = append(cmdArgs, image)
+
+	cmd := exec.Command("operator-sdk", cmdArgs...)
+	log.Trace("running scorecard with the following invocation", cmd.Args)
+	var stdout, stderr bytes.Buffer
+	cmd.Stdout = &stdout
+	cmd.Stderr = &stderr
+	if err := cmd.Run(); err != nil {
+		// This is a workaround due to operator-sdk scorecard always returning a 1 exit code
+		// whether a test failed or the tool encountered a fatal error.
+		//
+		// Until resolved, we are concluding/assuming that non-zero exit codes with len(stderr) == 0
+		// means that we failed a test, but the command execution succeeded.
+		//
+		// We also conclude/assume that anything being in stderr would indicate an error in the
+		// check execution itself.
+		if stderr.Len() != 0 {
+			log.Error("stderr: ", stdout.String())
+			return nil, fmt.Errorf("%w: %s", errors.ErrOperatorSdkScorecardFailed, err)
+		}
+	}
+
+	if err := o.writeScorecardFile(opts.ResultFile, stdout.String()); err != nil {
+		log.Error("unable to copy result to artifacts directory: ", err)
+		return nil, err
+	}
+
+	var scorecardData cli.OperatorSdkScorecardReport
+	if err := json.Unmarshal(stdout.Bytes(), &scorecardData); err != nil {
+		return nil, fmt.Errorf("%w: %s", errors.ErrOperatorSdkScorecardFailed, err)
+	}
+	scorecardData.Stdout = stdout.String()
+	scorecardData.Stderr = stderr.String()
+	return &scorecardData, nil
+}
+
+func (o operatorSdkEngine) BundleValidate(image string, opts cli.OperatorSdkBundleValidateOptions) (*cli.OperatorSdkBundleValidateReport, error) {
+	cmdArgs := []string{"bundle", "validate"}
+	if opts.ContainerEngine == "" {
+		opts.ContainerEngine = "podman"
+	}
+	cmdArgs = append(cmdArgs, "-b", opts.ContainerEngine)
+	if opts.OutputFormat == "" {
+		opts.OutputFormat = "json-alpha1"
+	}
+	cmdArgs = append(cmdArgs, "--output", opts.OutputFormat)
+	if opts.Selector != nil {
+		for _, selector := range opts.Selector {
+			cmdArgs = append(cmdArgs, "--select-optional", fmt.Sprintf("name=%s", selector))
+		}
+	}
+	if opts.Verbose {
+		cmdArgs = append(cmdArgs, "--verbose")
+	}
+	cmdArgs = append(cmdArgs, image)
+
+	cmd := exec.Command("operator-sdk", cmdArgs...)
+	log.Debugf("Command being run: %s", cmd.Args)
+	var stdout, stderr bytes.Buffer
+	cmd.Stdout = &stdout
+	cmd.Stderr = &stderr
+	if err := cmd.Run(); err != nil {
+		log.Debugf("Stderr: %s", stderr.String())
+		return &cli.OperatorSdkBundleValidateReport{Stderr: stderr.String()}, fmt.Errorf("%w: %s", errors.ErrOperatorSdkBundleValidateFailed, err)
+	}
+
+	var bundleValidateData cli.OperatorSdkBundleValidateReport
+	if strings.Contains(opts.OutputFormat, "json") {
+		if err := json.Unmarshal(stdout.Bytes(), &bundleValidateData); err != nil {
+			return nil, fmt.Errorf("%w: %s", errors.ErrOperatorSdkBundleValidateFailed, err)
+		}
+	} else {
+		if strings.Contains(stdout.String(), "ERRO") || strings.Contains(stdout.String(), "FATA") {
+			bundleValidateData.Passed = false
+		}
+	}
+	bundleValidateData.Stdout = stdout.String()
+	bundleValidateData.Stderr = stderr.String()
+
+	return &bundleValidateData, nil
+}
+
+func (o operatorSdkEngine) writeScorecardFile(resultFile, stdout string) error {
+	_, err := fileutils.WriteFileToArtifactsPath(resultFile, stdout)
+	return err
+}
+
+func createScorecardConfigFile() (string, error) {
+	configTemplate := `kind: Configuration
+	apiversion: scorecard.operatorframework.io/v1alpha3
+	metadata:
+	  name: config
+	stages:
+	  - parallel: true
+		tests:
+		  - image: quay.io/operator-framework/scorecard-test:v1.9.0
+			entrypoint:
+			  - scorecard-test
+			  - basic-check-spec
+			labels:
+			  suite: basic
+			  test: basic-check-spec-test
+		  - image: quay.io/operator-framework/scorecard-test:v1.9.0
+			entrypoint:
+			  - scorecard-test
+			  - olm-bundle-validation
+			labels:
+			  suite: olm
+			  test: olm-bundle-validation-test
+`
+
+	tempConfigFile, err := os.CreateTemp("", "scorecard-test-config-*.yaml")
+	if err != nil {
+		log.Error("could not create temp config file", err)
+		return "", err
+	}
+	_, err = tempConfigFile.WriteString(configTemplate)
+	return tempConfigFile.Name(), err
+}
diff --git a/certification/internal/policy/operator/validate_operator_bundle.go b/certification/internal/policy/operator/validate_operator_bundle.go
new file mode 100644
index 0000000..8718a05
--- /dev/null
+++ b/certification/internal/policy/operator/validate_operator_bundle.go
@@ -0,0 +1,79 @@
+package operator
+
+import (
+	"github.com/redhat-openshift-ecosystem/openshift-preflight/certification"
+	"github.com/redhat-openshift-ecosystem/openshift-preflight/cli"
+	log "github.com/sirupsen/logrus"
+)
+
+// ValidateOperatorBundleCheck evaluates the image and ensures that it passes bundle validation
+// as executed by `operator-sdk bundle validate`
+type ValidateOperatorBundleCheck struct {
+	OperatorSdkEngine cli.OperatorSdkEngine
+}
+
+func NewValidateOperatorBundleCheck(operatorSdkEngine *cli.OperatorSdkEngine) *ValidateOperatorBundleCheck {
+	return &ValidateOperatorBundleCheck{
+		OperatorSdkEngine: *operatorSdkEngine,
+	}
+}
+
+func (p ValidateOperatorBundleCheck) Validate(bundleRef certification.ImageReference) (bool, error) {
+	report, err := p.getDataToValidate(bundleRef.ImageFSPath)
+	if err != nil {
+		log.Error("Error while executing operator-sdk bundle validate: ", err)
+		return false, err
+	}
+
+	return p.validate(report)
+}
+
+func (p ValidateOperatorBundleCheck) getDataToValidate(imagePath string) (*cli.OperatorSdkBundleValidateReport, error) {
+	selector := []string{"community", "operatorhub"}
+	opts := cli.OperatorSdkBundleValidateOptions{
+		Selector:        selector,
+		Verbose:         true,
+		ContainerEngine: "podman",
+		OutputFormat:    "json-alpha1",
+	}
+
+	return p.OperatorSdkEngine.BundleValidate(imagePath, opts)
+}
+
+func (p ValidateOperatorBundleCheck) validate(report *cli.OperatorSdkBundleValidateReport) (bool, error) {
+	if !report.Passed || len(report.Outputs) > 0 {
+		for _, output := range report.Outputs {
+			var logFn func(...interface{})
+			switch output.Type {
+			case "warning":
+				logFn = log.Warn
+			case "error":
+				logFn = log.Error
+			default:
+				logFn = log.Debug
+			}
+			logFn(output.Message)
+		}
+	}
+	return report.Passed, nil
+}
+
+func (p ValidateOperatorBundleCheck) Name() string {
+	return "ValidateOperatorBundle"
+}
+
+func (p ValidateOperatorBundleCheck) Metadata() certification.Metadata {
+	return certification.Metadata{
+		Description:      "Validating Bundle image that checks if it can validate the content and format of the operator bundle",
+		Level:            "best",
+		KnowledgeBaseURL: "https://connect.redhat.com/zones/containers/container-certification-policy-guide",
+		CheckURL:         "https://connect.redhat.com/zones/containers/container-certification-policy-guide",
+	}
+}
+
+func (p ValidateOperatorBundleCheck) Help() certification.HelpText {
+	return certification.HelpText{
+		Message:    "Check ValidateOperatorBundle encountered an error. Please review the preflight.log file for more information.",
+		Suggestion: "Valid bundles are defined by bundle spec, so make sure that this bundle conforms to that spec. More Information: https://github.com/operator-framework/operator-registry/blob/master/docs/design/operator-bundle.md",
+	}
+}
diff --git a/certification/internal/utils/file/helper.go b/certification/internal/utils/file/helper.go
index 097afc5..f1c0a68 100644
--- a/certification/internal/utils/file/helper.go
+++ b/certification/internal/utils/file/helper.go
@@ -5,6 +5,7 @@ import (
 	"compress/bzip2"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -86,6 +87,16 @@ func ArtifactPath(artifact string) string {
 	return filepath.Join(artifactDir, artifact)
 }
 
+func WriteFileToArtifactsPath(filename, contents string) (string, error) {
+	fullFilePath := ArtifactPath(filename)
+
+	err := ioutil.WriteFile(fullFilePath, []byte(contents), 0644)
+	if err != nil {
+		return fullFilePath, err
+	}
+	return fullFilePath, nil
+}
+
 // Untar takes a destination path and a reader; a tar reader loops over the tarfile
 // creating the file structure at 'dst' along the way, and writing any files
 func Untar(dst string, r io.Reader) error {


Test patch:
diff --git a/certification/internal/policy/operator/validate_operator_bundle_test.go b/certification/internal/policy/operator/validate_operator_bundle_test.go
new file mode 100644
index 0000000..c06f178
--- /dev/null
+++ b/certification/internal/policy/operator/validate_operator_bundle_test.go
@@ -0,0 +1,108 @@
+package operator
+
+import (
+	"errors"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"github.com/redhat-openshift-ecosystem/openshift-preflight/certification/internal/utils/migration"
+	"github.com/redhat-openshift-ecosystem/openshift-preflight/cli"
+)
+
+type FakeOperatorSdkEngine struct {
+	OperatorSdkReport   cli.OperatorSdkScorecardReport
+	OperatorSdkBVReport cli.OperatorSdkBundleValidateReport
+}
+
+func (f FakeOperatorSdkEngine) BundleValidate(image string, opts cli.OperatorSdkBundleValidateOptions) (*cli.OperatorSdkBundleValidateReport, error) {
+	return &f.OperatorSdkBVReport, nil
+}
+
+func (f FakeOperatorSdkEngine) Scorecard(image string, opts cli.OperatorSdkScorecardOptions) (*cli.OperatorSdkScorecardReport, error) {
+	return &f.OperatorSdkReport, nil
+}
+
+type BadOperatorSdkEngine struct{}
+
+func (bose BadOperatorSdkEngine) Scorecard(bundleImage string, opts cli.OperatorSdkScorecardOptions) (*cli.OperatorSdkScorecardReport, error) {
+	operatorSdkReport := cli.OperatorSdkScorecardReport{
+		Stdout: "Bad Stdout",
+		Stderr: "Bad Stderr",
+		Items:  []cli.OperatorSdkScorecardItem{},
+	}
+	return &operatorSdkReport, errors.New("the Operator Sdk Scorecard has failed")
+}
+
+func (bose BadOperatorSdkEngine) BundleValidate(bundleImage string, opts cli.OperatorSdkBundleValidateOptions) (*cli.OperatorSdkBundleValidateReport, error) {
+	operatorSdkReport := cli.OperatorSdkBundleValidateReport{
+		Stdout:  "Bad Stdout",
+		Stderr:  "Bad Stderr",
+		Passed:  false,
+		Outputs: []cli.OperatorSdkBundleValidateOutput{},
+	}
+	return &operatorSdkReport, errors.New("the Operator Sdk Bundle Validate has failed")
+}
+
+var _ = Describe("BundleValidateCheck", func() {
+	var (
+		bundleValidateCheck ValidateOperatorBundleCheck
+		fakeEngine          cli.OperatorSdkEngine
+	)
+
+	BeforeEach(func() {
+		stdout := `{
+	"passed": true,
+	"outputs": null
+}`
+		stderr := ""
+		report := cli.OperatorSdkBundleValidateReport{
+			Stdout:  stdout,
+			Stderr:  stderr,
+			Passed:  true,
+			Outputs: []cli.OperatorSdkBundleValidateOutput{},
+		}
+		fakeEngine = FakeOperatorSdkEngine{
+			OperatorSdkBVReport: report,
+		}
+		bundleValidateCheck = *NewValidateOperatorBundleCheck(&fakeEngine)
+	})
+	Describe("Operator Bundle Validate", func() {
+		Context("When Operator Bundle Validate passes", func() {
+			It("Should pass Validate", func() {
+				ok, err := bundleValidateCheck.Validate(migration.ImageToImageReference("dummy/image"))
+				Expect(err).ToNot(HaveOccurred())
+				Expect(ok).To(BeTrue())
+			})
+		})
+		Context("When Operator Bundle Validate does not Pass", func() {
+			BeforeEach(func() {
+				engine := fakeEngine.(FakeOperatorSdkEngine)
+				engine.OperatorSdkBVReport.Passed = false
+				engine.OperatorSdkBVReport.Outputs = []cli.OperatorSdkBundleValidateOutput{
+					{Type: "warning", Message: "This is a warning"},
+					{Type: "error", Message: "This is an error"},
+				}
+				fakeEngine = engine
+				bundleValidateCheck = *NewValidateOperatorBundleCheck(&fakeEngine)
+			})
+			It("Should not pass Validate", func() {
+				ok, err := bundleValidateCheck.Validate(migration.ImageToImageReference("dummy/image"))
+				Expect(err).ToNot(HaveOccurred())
+				Expect(ok).To(BeFalse())
+			})
+		})
+	})
+	Describe("Checking that OperatorSdkEngine errors are handled correctly", func() {
+		BeforeEach(func() {
+			fakeEngine = BadOperatorSdkEngine{}
+			bundleValidateCheck = *NewValidateOperatorBundleCheck(&fakeEngine)
+		})
+		Context("When OperatorSdk throws an error", func() {
+			It("should fail Validate and return an error", func() {
+				ok, err := bundleValidateCheck.Validate(migration.ImageToImageReference("dummy/image"))
+				Expect(err).To(HaveOccurred())
+				Expect(ok).To(BeFalse())
+			})
+		})
+	})
+})