Skip to content

Commit

Permalink
Switch the bundle validate to use the API
Browse files Browse the repository at this point in the history
This uses the validators directly, but still has the same usage
semantics, for the most part.

Signed-off-by: Brad P. Crochet <brad@redhat.com>
  • Loading branch information
bcrochet committed Aug 18, 2022
1 parent 936664e commit ad6b03e
Show file tree
Hide file tree
Showing 11 changed files with 120 additions and 132 deletions.
4 changes: 2 additions & 2 deletions certification/engine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ func initializeChecks(ctx context.Context, p policy.Policy, cfg certification.Co
return []certification.Check{
operatorpol.NewScorecardBasicSpecCheck(operatorsdk.New(cfg.ScorecardImage(), exec.Command), cfg.Namespace(), cfg.ServiceAccount(), cfg.Kubeconfig(), cfg.ScorecardWaitTime()),
operatorpol.NewScorecardOlmSuiteCheck(operatorsdk.New(cfg.ScorecardImage(), exec.Command), cfg.Namespace(), cfg.ServiceAccount(), cfg.Kubeconfig(), cfg.ScorecardWaitTime()),
operatorpol.NewDeployableByOlmCheck(operatorsdk.New(cfg.ScorecardImage(), exec.Command), cfg.IndexImage(), cfg.DockerConfig(), cfg.Channel()),
operatorpol.NewValidateOperatorBundleCheck(operatorsdk.New(cfg.ScorecardImage(), exec.Command)),
operatorpol.NewDeployableByOlmCheck(cfg.IndexImage(), cfg.DockerConfig(), cfg.Channel()),
operatorpol.NewValidateOperatorBundleCheck(),
operatorpol.NewCertifiedImagesCheck(pyxis.NewPyxisClient(
certification.DefaultPyxisHost,
"",
Expand Down
42 changes: 25 additions & 17 deletions certification/internal/bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,15 @@ import (

"github.com/blang/semver"
"github.com/operator-framework/api/pkg/manifests"
"github.com/redhat-openshift-ecosystem/openshift-preflight/certification/internal/operatorsdk"
"github.com/operator-framework/api/pkg/validation"
olmvalidation "github.com/redhat-openshift-ecosystem/ocp-olm-catalog-validator/pkg/validation"
log "github.com/sirupsen/logrus"
"sigs.k8s.io/yaml"
)

const ocpVerV1beta1Unsupported = "4.9"

type operatorSdk interface {
BundleValidate(context.Context, string, operatorsdk.OperatorSdkBundleValidateOptions) (*operatorsdk.OperatorSdkBundleValidateReport, error)
}

func Validate(ctx context.Context, operatorSdk operatorSdk, imagePath string) (*operatorsdk.OperatorSdkBundleValidateReport, error) {
selector := []string{"community", "operatorhub"}
opts := operatorsdk.OperatorSdkBundleValidateOptions{
Selector: selector,
Verbose: true,
ContainerEngine: "none",
OutputFormat: "json-alpha1",
}

func Validate(ctx context.Context, imagePath string) (*Report, error) {
log.Trace("reading annotations file from the bundle")
log.Debug("image extraction directory is ", imagePath)
// retrieve the operator metadata from bundle image
Expand All @@ -45,16 +34,35 @@ func Validate(ctx context.Context, operatorSdk operatorSdk, imagePath string) (*
return nil, fmt.Errorf("unable to get annotations.yaml from the bundle: %v", err)
}

optionalValues := make(map[string]string)
if annotations.OpenshiftVersions != "" {
// Check that the label range contains >= 4.9
if isTarget49OrGreater(annotations.OpenshiftVersions) {
log.Debug("OpenShift 4.9 detected in annotations. Running with additional checks enabled.")
opts.OptionalValues = make(map[string]string)
opts.OptionalValues["k8s-version"] = "1.22"
optionalValues["k8s-version"] = "1.22"
}
}

bundle, err := manifests.GetBundleFromDir(imagePath)
if err != nil {
return nil, fmt.Errorf("could not load bundle from path: %s: %v", imagePath, err)
}
validators := validation.DefaultBundleValidators.WithValidators(
validation.AlphaDeprecatedAPIsValidator,
validation.OperatorHubValidator,
olmvalidation.OpenShiftValidator,
)

results := validators.Validate(bundle, optionalValues)
passed := true
for _, v := range results {
if v.HasError() {
passed = false
break
}
}

return operatorSdk.BundleValidate(ctx, imagePath, opts)
return &Report{Results: results, Passed: passed}, nil
}

func isTarget49OrGreater(ocpLabelIndex string) bool {
Expand Down
12 changes: 0 additions & 12 deletions certification/internal/bundle/bundle_suite_test.go
Original file line number Diff line number Diff line change
@@ -1,30 +1,18 @@
package bundle

import (
"context"
"errors"
"testing"

. "github.com/onsi/ginkgo/v2/dsl/core"
. "github.com/onsi/gomega"

"github.com/redhat-openshift-ecosystem/openshift-preflight/certification/internal/operatorsdk"
)

func TestBundle(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Bundle Utils Suite")
}

type FakeOperatorSdk struct {
OperatorSdkReport operatorsdk.OperatorSdkScorecardReport
OperatorSdkBVReport operatorsdk.OperatorSdkBundleValidateReport
}

func (f FakeOperatorSdk) BundleValidate(ctx context.Context, image string, opts operatorsdk.OperatorSdkBundleValidateOptions) (*operatorsdk.OperatorSdkBundleValidateReport, error) {
return &f.OperatorSdkBVReport, nil
}

// In order to test some negative paths, this io.Reader will just throw an error
type errReader int

Expand Down
25 changes: 14 additions & 11 deletions certification/internal/bundle/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import (
"os"
"path/filepath"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/ginkgo/v2/dsl/core"
. "github.com/onsi/ginkgo/v2/dsl/table"
. "github.com/onsi/gomega"
"github.com/redhat-openshift-ecosystem/openshift-preflight/certification"
)
Expand All @@ -22,7 +23,9 @@ var _ = Describe("BundleValidateCheck", func() {
operators.operatorframework.io.bundle.package.v1: testPackage
operators.operatorframework.io.bundle.channel.default.v1: testChannel
`
csvContents = `spec:
csvContents = `apiVersion: operators.coreos.com/v1alpha1
kind: ClusterServiceVersion
spec:
install:
spec:
deployments:
Expand All @@ -40,10 +43,7 @@ var _ = Describe("BundleValidateCheck", func() {
)

Describe("Bundle validation", func() {
var (
imageRef certification.ImageReference
fakeEngine operatorSdk
)
var imageRef certification.ImageReference

BeforeEach(func() {
// mock bundle directory
Expand All @@ -56,11 +56,14 @@ var _ = Describe("BundleValidateCheck", func() {
err = os.Mkdir(filepath.Join(tmpDir, manifestsDir), 0o755)
Expect(err).ToNot(HaveOccurred())

err = os.WriteFile(filepath.Join(tmpDir, manifestsDir, clusterServiceVersionFilename), []byte(csvContents), 0o644)
Expect(err).ToNot(HaveOccurred())

err = os.WriteFile(filepath.Join(tmpDir, metadataDir, annotationFilename), []byte(annotations), 0o644)
Expect(err).ToNot(HaveOccurred())

imageRef.ImageFSPath = tmpDir
fakeEngine = FakeOperatorSdk{}
DeferCleanup(os.RemoveAll, tmpDir)
})

AfterEach(func() {
Expand All @@ -70,7 +73,7 @@ var _ = Describe("BundleValidateCheck", func() {

Context("the annotations file is valid", func() {
It("should pass", func() {
report, err := Validate(context.Background(), fakeEngine, imageRef.ImageFSPath)
report, err := Validate(context.Background(), imageRef.ImageFSPath)
Expect(err).ToNot(HaveOccurred())
Expect(report).ToNot(BeNil())
})
Expand All @@ -82,7 +85,7 @@ var _ = Describe("BundleValidateCheck", func() {
Expect(err).ToNot(HaveOccurred())
})
It("should error", func() {
report, err := Validate(context.Background(), fakeEngine, imageRef.ImageFSPath)
report, err := Validate(context.Background(), imageRef.ImageFSPath)
Expect(err).To(HaveOccurred())
Expect(report).To(BeNil())
})
Expand All @@ -94,7 +97,7 @@ var _ = Describe("BundleValidateCheck", func() {
Expect(err).ToNot(HaveOccurred())
})
It("should error", func() {
report, err := Validate(context.Background(), fakeEngine, imageRef.ImageFSPath)
report, err := Validate(context.Background(), imageRef.ImageFSPath)
Expect(err).To(HaveOccurred())
Expect(report).To(BeNil())
})
Expand All @@ -106,7 +109,7 @@ var _ = Describe("BundleValidateCheck", func() {
Expect(err).ToNot(HaveOccurred())
})
It("should fail gracefully", func() {
report, err := Validate(context.Background(), fakeEngine, imageRef.ImageFSPath)
report, err := Validate(context.Background(), imageRef.ImageFSPath)
Expect(err).ToNot(HaveOccurred())
Expect(report).ToNot(BeNil())
})
Expand Down
10 changes: 9 additions & 1 deletion certification/internal/bundle/types.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package bundle

import "github.com/operator-framework/api/pkg/manifests"
import (
"github.com/operator-framework/api/pkg/manifests"
validationerrors "github.com/operator-framework/api/pkg/validation/errors"
)

type AnnotationsFile struct {
Annotations Annotations `json:"annotations" yaml:"annotations"`
Expand All @@ -11,3 +14,8 @@ type Annotations struct {

OpenshiftVersions string `json:"com.redhat.openshift.versions,omitempty" yaml:"com.redhat.openshift.versions,omitempty"`
}

type Report struct {
Results []validationerrors.ManifestResult
Passed bool
}
7 changes: 2 additions & 5 deletions certification/internal/policy/operator/deployable_by_olm.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ type DeployableByOlmCheck struct {
// channel is optional. If empty, we will introspect.
channel string

OperatorSdk operatorSdk
openshiftClient openshift.Client
client crclient.Client
csvReady bool
Expand Down Expand Up @@ -94,13 +93,11 @@ func (p *DeployableByOlmCheck) initOpenShifeEngine() error {
// in scope are public. An empty channel value implies that the check should
// introspect the channel from the bundle. indexImage is required.
func NewDeployableByOlmCheck(
operatorSdk operatorSdk,
indexImage,
dockerConfig,
channel string,
) *DeployableByOlmCheck {
return &DeployableByOlmCheck{
OperatorSdk: operatorSdk,
dockerConfig: dockerConfig,
indexImage: indexImage,
channel: channel,
Expand All @@ -110,7 +107,7 @@ func NewDeployableByOlmCheck(
func (p *DeployableByOlmCheck) Validate(ctx context.Context, bundleRef certification.ImageReference) (bool, error) {
p.initClient()
p.initOpenShifeEngine()
if report, err := bundle.Validate(ctx, p.OperatorSdk, bundleRef.ImageFSPath); err != nil || !report.Passed {
if report, err := bundle.Validate(ctx, bundleRef.ImageFSPath); err != nil || !report.Passed {
return false, fmt.Errorf("%v", err)
}

Expand Down Expand Up @@ -640,6 +637,6 @@ func (p *DeployableByOlmCheck) Metadata() certification.Metadata {
func (p *DeployableByOlmCheck) Help() certification.HelpText {
return certification.HelpText{
Message: "It is required that your operator could be deployed by OLM",
Suggestion: "Follow the guidelines on the operatorsdk website to learn how to package your operator https://sdk.operatorframework.io/docs/olm-integration/cli-overview/",
Suggestion: "Follow the guidelines on the operator-sdk website to learn how to package your operator https://sdk.operatorframework.io/docs/olm-integration/cli-overview/",
}
}
23 changes: 5 additions & 18 deletions certification/internal/policy/operator/deployable_by_olm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,11 @@ import (
"github.com/redhat-openshift-ecosystem/openshift-preflight/certification/artifacts"

"github.com/redhat-openshift-ecosystem/openshift-preflight/certification/internal/openshift"
"github.com/redhat-openshift-ecosystem/openshift-preflight/certification/internal/operatorsdk"
)

var _ = Describe("DeployableByOLMCheck", func() {
var (
deployableByOLMCheck DeployableByOlmCheck
fakeEngine operatorSdk
imageRef certification.ImageReference
tmpDockerDir string
client crclient.Client
Expand Down Expand Up @@ -53,6 +51,9 @@ var _ = Describe("DeployableByOLMCheck", func() {
csvStr = `apiVersion: operators.coreos.com/v1alpha1
kind: ClusterServiceVersion
spec:
provider:
name: Provider
version: 0.0.1
installModes:
- supported: false
type: OwnNamespace
Expand Down Expand Up @@ -96,17 +97,9 @@ spec:
imageRef.ImageInfo = &fakeImage
imageRef.ImageFSPath = tmpDir

report := operatorsdk.OperatorSdkBundleValidateReport{
Passed: true,
Outputs: []operatorsdk.OperatorSdkBundleValidateOutput{},
}
fakeEngine = FakeOperatorSdk{
OperatorSdkBVReport: report,
}

now := metav1.Now()
og.Status.LastUpdated = &now
deployableByOLMCheck = *NewDeployableByOlmCheck(fakeEngine, "test_indeximage", "", "")
deployableByOLMCheck = *NewDeployableByOlmCheck("test_indeximage", "", "")
scheme := apiruntime.NewScheme()
Expect(openshift.AddSchemes(scheme)).To(Succeed())
client = fake.NewClientBuilder().
Expand All @@ -118,6 +111,7 @@ spec:

artifacts.SetDir(tmpDir)
DeferCleanup(os.RemoveAll, tmpDir)
DeferCleanup(os.RemoveAll, tmpDockerDir)
DeferCleanup(artifacts.Reset)
})

Expand Down Expand Up @@ -199,11 +193,4 @@ spec:
Entry("registry.access.redhat.com", []string{"registry.access.redhat.com/ubi8/ubi"}, true),
Entry("quay.io", []string{"quay.io/rocrisp/preflight-operator-bundle:v1"}, false),
)
AfterEach(func() {
err := os.RemoveAll(imageRef.ImageFSPath)
Expect(err).ToNot(HaveOccurred())

err = os.RemoveAll(tmpDockerDir)
Expect(err).ToNot(HaveOccurred())
})
})
37 changes: 13 additions & 24 deletions certification/internal/policy/operator/validate_operator_bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,26 +8,19 @@ import (
"github.com/operator-framework/api/pkg/validation"
"github.com/redhat-openshift-ecosystem/openshift-preflight/certification"
"github.com/redhat-openshift-ecosystem/openshift-preflight/certification/internal/bundle"
"github.com/redhat-openshift-ecosystem/openshift-preflight/certification/internal/operatorsdk"
log "github.com/sirupsen/logrus"
)

var _ certification.Check = &ValidateOperatorBundleCheck{}

// ValidateOperatorBundleCheck evaluates the image and ensures that it passes bundle validation
// as executed by `operator-sdk bundle validate`
type ValidateOperatorBundleCheck struct {
OperatorSdk operatorSdk
}
type ValidateOperatorBundleCheck struct{}

func NewValidateOperatorBundleCheck(operatorSdk operatorSdk) *ValidateOperatorBundleCheck {
return &ValidateOperatorBundleCheck{
OperatorSdk: operatorSdk,
}
func NewValidateOperatorBundleCheck() *ValidateOperatorBundleCheck {
return &ValidateOperatorBundleCheck{}
}

const ocpVerV1beta1Unsupported = "4.9"

func (p *ValidateOperatorBundleCheck) Validate(ctx context.Context, bundleRef certification.ImageReference) (bool, error) {
report, err := p.getDataToValidate(ctx, bundleRef.ImageFSPath)
if err != nil {
Expand All @@ -53,23 +46,19 @@ func (p *ValidateOperatorBundleCheck) dataToValidate(ctx context.Context, imageP
return valid, nil
}

func (p *ValidateOperatorBundleCheck) getDataToValidate(ctx context.Context, imagePath string) (*operatorsdk.OperatorSdkBundleValidateReport, error) {
return bundle.Validate(ctx, p.OperatorSdk, imagePath)
func (p *ValidateOperatorBundleCheck) getDataToValidate(ctx context.Context, imagePath string) (*bundle.Report, error) {
return bundle.Validate(ctx, imagePath)
}

func (p *ValidateOperatorBundleCheck) validate(ctx context.Context, report *operatorsdk.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
func (p *ValidateOperatorBundleCheck) validate(ctx context.Context, report *bundle.Report) (bool, error) {
if !report.Passed || len(report.Results) > 0 {
for _, output := range report.Results {
for _, result := range output.Errors {
log.Error(result.Error())
}
for _, result := range output.Warnings {
log.Warn(result.Error())
}
logFn(output.Message)
}
}
return report.Passed, nil
Expand Down
Loading

0 comments on commit ad6b03e

Please sign in to comment.