From 18182a7010a98281717901cc196462f570ead5d6 Mon Sep 17 00:00:00 2001 From: Camila Macedo Date: Tue, 12 May 2020 11:49:33 +0100 Subject: [PATCH 1/4] feat: Add new hidden alpha flag `--json` to output the result of `operator-sdk bundle validate` in JSON format. --- changelog/fragments/json-hidden-flag.yaml | 16 +++ cmd/operator-sdk/alpha/scorecard/cmd.go | 11 +- cmd/operator-sdk/bundle/validate.go | 69 +++++++-- cmd/operator-sdk/scorecard/cmd.go | 11 +- go.mod | 1 + pkg/output/logger.go | 28 ++++ pkg/output/result.go | 162 ++++++++++++++++++++++ 7 files changed, 271 insertions(+), 27 deletions(-) create mode 100644 changelog/fragments/json-hidden-flag.yaml create mode 100644 pkg/output/logger.go create mode 100644 pkg/output/result.go diff --git a/changelog/fragments/json-hidden-flag.yaml b/changelog/fragments/json-hidden-flag.yaml new file mode 100644 index 00000000000..db715577998 --- /dev/null +++ b/changelog/fragments/json-hidden-flag.yaml @@ -0,0 +1,16 @@ +# entries is a list of entries to include in +# release notes and/or the migration guide +entries: + - description: > + Add new hidden alpha flag `--output` to allow output the result of `operator-sdk bundle validate` in JSON format. + + # kind is one of: + # - addition + # - change + # - deprecation + # - removal + # - bugfix + kind: "addition" + + # Is this a breaking change? + breaking: false diff --git a/cmd/operator-sdk/alpha/scorecard/cmd.go b/cmd/operator-sdk/alpha/scorecard/cmd.go index 97db719f810..e599c63f1d7 100644 --- a/cmd/operator-sdk/alpha/scorecard/cmd.go +++ b/cmd/operator-sdk/alpha/scorecard/cmd.go @@ -32,6 +32,7 @@ import ( "github.com/operator-framework/operator-sdk/internal/flags" scorecard "github.com/operator-framework/operator-sdk/internal/scorecard/alpha" "github.com/operator-framework/operator-sdk/pkg/apis/scorecard/v1alpha2" + "github.com/operator-framework/operator-sdk/pkg/output" ) func NewCmd() *cobra.Command { @@ -65,7 +66,7 @@ If the argument holds an image tag, it must be present remotely.`, // Extract bundle image contents if bundle is inferred to be an image. if _, err = os.Stat(bundle); err != nil && errors.Is(err, os.ErrNotExist) { // Discard bundle extraction logs unless user sets verbose mode. - logger := log.NewEntry(discardLogger()) + logger := log.NewEntry(output.NewLoggerTo(ioutil.Discard)) if viper.GetBool(flags.VerboseOpt) { logger = log.WithFields(log.Fields{"bundle": bundle}) } @@ -171,12 +172,4 @@ func printOutput(outputFormat string, output v1alpha2.ScorecardOutput) error { return fmt.Errorf("invalid output format selected") } return nil - -} - -// discardLogger returns a logger that throws away input. -func discardLogger() *log.Logger { - logger := log.New() - logger.SetOutput(ioutil.Discard) - return logger } diff --git a/cmd/operator-sdk/bundle/validate.go b/cmd/operator-sdk/bundle/validate.go index de399416045..c802b0e0f60 100644 --- a/cmd/operator-sdk/bundle/validate.go +++ b/cmd/operator-sdk/bundle/validate.go @@ -31,10 +31,14 @@ import ( "github.com/operator-framework/operator-sdk/internal/flags" internalregistry "github.com/operator-framework/operator-sdk/internal/registry" + "github.com/operator-framework/operator-sdk/pkg/output" + ) type bundleValidateCmd struct { bundleCmd + + outputFormat string } // newValidateCmd returns a command that will validate an operator bundle image. @@ -83,14 +87,17 @@ To build and validate an image: log.SetLevel(log.DebugLevel) } - if err := c.validate(args); err != nil { + // Always print non-output logs to stderr as to not pollute actual command output. + logger := log.NewEntry(output.NewLoggerTo(os.Stderr)) + + if err = c.validate(args); err != nil { return fmt.Errorf("invalid command args: %v", err) } // If the argument isn't a directory, assume it's an image. if isExist(args[0]) { if c.directory, err = relWd(args[0]); err != nil { - log.Fatal(err) + logger.Fatal(err) } } else { c.directory, err = ioutil.TempDir("", "bundle-") @@ -99,19 +106,23 @@ To build and validate an image: } defer func() { if err = os.RemoveAll(c.directory); err != nil { - log.Errorf("Error removing temp bundle dir: %v", err) + logger.Errorf("Error removing temp bundle dir: %v", err) } }() log.Info("Unpacking image layers") if err := c.unpackImageIntoDir(args[0], c.directory); err != nil { - log.Fatalf("Error unpacking image %s: %v", args[0], err) + logger.Fatalf("Error unpacking image %s: %v", args[0], err) } } - if err = c.run(); err != nil { - log.Fatal(err) + result, err := c.run(logger) + if err != nil { + logger.Fatal(err) + } + if err := result.PrintWithFormat(c.outputFormat); err != nil { + logger.Fatal(err) } log.Info("All validation tests have completed successfully") @@ -129,6 +140,9 @@ func (c bundleValidateCmd) validate(args []string) error { if len(args) != 1 { return errors.New("an image tag or directory is a required argument") } + if c.outputFormat != output.JSONAlpha1 && c.outputFormat != output.Text { + return fmt.Errorf("invalid value for output flag: %v", c.outputFormat) + } return nil } @@ -138,11 +152,23 @@ func (c *bundleValidateCmd) addToFlagSet(fs *pflag.FlagSet) { fs.StringVarP(&c.imageBuilder, "image-builder", "b", "docker", "Tool to extract bundle image data. Only used when validating a bundle image. "+ "One of: [docker, podman]") + fs.StringVarP(&c.outputFormat, "output", "o", output.Text, + "Result format for results. One of: [text, json-alpha1]") + + // It is hidden because it is an alpha option + // The idea is the next versions of Operator Registry will return a List of errors + if err := fs.MarkHidden("output"); err != nil { + panic(err) + } } -func (c bundleValidateCmd) run() error { +func (c bundleValidateCmd) run(logger *log.Entry) (res output.Result, err error) { - logger := log.WithFields(log.Fields{ + // Create a new res containing validation errors and info msgs, if any + res = output.Result{} + res.Passed = true // Init without errors + + logger = log.WithFields(log.Fields{ "bundle-dir": c.directory, "container-tool": c.imageBuilder, }) @@ -150,7 +176,7 @@ func (c bundleValidateCmd) run() error { // Validate bundle format. if err := val.ValidateBundleFormat(c.directory); err != nil { - return fmt.Errorf("invalid bundle format: %v", err) + return res, fmt.Errorf("invalid bundle format: %v", err) } // Validate bundle content. @@ -159,13 +185,13 @@ func (c bundleValidateCmd) run() error { manifestsDir := filepath.Join(c.directory, registrybundle.ManifestsDir) results, err := validateBundleContent(logger, manifestsDir) if err != nil { - return fmt.Errorf("error validating %s: %v", manifestsDir, err) + return res, fmt.Errorf("error validating %s: %v", manifestsDir, err) } if checkResults(results) { - return errors.New("invalid bundle content") + return res, errors.New("invalid bundle content") } - return nil + return res, nil } // unpackImageIntoDir writes files in image layers found in image imageTag to dir. @@ -211,3 +237,22 @@ func checkResults(results []apierrors.ManifestResult) (hasErrors bool) { } return hasErrors } + +// addBundleValErrorToOutput will add the output logs results from the Bundle Validation +// NOTE: Currently, the Bundle Validation from Operator Registry only returns error types, +// however, we might have WARN types in the future. +func addBundleValErrorToOutput(err error, res *output.Result) { + verr := bundle.ValidationError{} + if errors.As(err, &verr) { + for _, valErr := range verr.Errors { + res.AddErrors(valErr) + } + } else { + res.AddErrors(err) + } +} + +// isToOutputJSON returns true when it should output json +func (c bundleValidateCmd) isToOutputJSON() bool { + return c.outputFormat == output.JSONAlpha1 +} diff --git a/cmd/operator-sdk/scorecard/cmd.go b/cmd/operator-sdk/scorecard/cmd.go index 487110a1b72..fa5c9ae8744 100644 --- a/cmd/operator-sdk/scorecard/cmd.go +++ b/cmd/operator-sdk/scorecard/cmd.go @@ -20,17 +20,16 @@ import ( "io" "os" + "github.com/mitchellh/mapstructure" + "github.com/pkg/errors" log "github.com/sirupsen/logrus" + "github.com/spf13/cobra" + "github.com/spf13/viper" + "k8s.io/apimachinery/pkg/labels" - "github.com/mitchellh/mapstructure" "github.com/operator-framework/operator-sdk/internal/scorecard" schelpers "github.com/operator-framework/operator-sdk/internal/scorecard/helpers" "github.com/operator-framework/operator-sdk/internal/util/projutil" - "github.com/pkg/errors" - "k8s.io/apimachinery/pkg/labels" - - "github.com/spf13/cobra" - "github.com/spf13/viper" ) const ( diff --git a/go.mod b/go.mod index b3cfc5977ff..28cefacb176 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/fatih/structtag v1.1.0 github.com/go-logr/logr v0.1.0 github.com/go-logr/zapr v0.1.1 + github.com/gobuffalo/logger v1.0.1 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/iancoleman/strcase v0.0.0-20190422225806-e506e3ef7365 github.com/markbates/inflect v1.0.4 diff --git a/pkg/output/logger.go b/pkg/output/logger.go new file mode 100644 index 00000000000..f97e1ddab87 --- /dev/null +++ b/pkg/output/logger.go @@ -0,0 +1,28 @@ +// Copyright 2020 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 output + +import ( + "io" + + log "github.com/sirupsen/logrus" +) + +// NewLoggerTo returns a logger that writes logs to w. +func NewLoggerTo(w io.Writer) *log.Logger { + logger := log.New() + logger.SetOutput(w) + return logger +} diff --git a/pkg/output/result.go b/pkg/output/result.go new file mode 100644 index 00000000000..0a9ee4032a3 --- /dev/null +++ b/pkg/output/result.go @@ -0,0 +1,162 @@ +// Copyright 2020 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 output + +import ( + "encoding/json" + "fmt" + "os" + + "github.com/sirupsen/logrus" + log "github.com/sirupsen/logrus" +) + +const ( + //output types + JSONAlpha1 = "json-alpha1" + Text = "text" +) + +// Result represents the final result +type Result struct { + Passed bool `json:"passed"` + Logs []Log `json:"logs"` +} + +// Log represents the logs which are used to return the final result in the JSON format +type Log struct { + Type string `json:"type"` + Message string `json:"message"` +} + +// AddInfo will add a log to the result with the Info Level +func (o *Result) AddInfo(msg string) { + o.Logs = append(o.Logs, Log{ + Type: logrus.InfoLevel.String(), + Message: msg, + }) +} + +// AddError will add a log to the result with the Error Level +func (o *Result) addError(err error) { + o.Logs = append(o.Logs, Log{ + Type: logrus.ErrorLevel.String(), + Message: err.Error(), + }) + o.Passed = false +} + +// AddError(s) will add a log to the result with the Error Level +func (o *Result) AddErrors(errs ...error) { + // Collect validation errors, if any. + for _, err := range errs { + if err == nil { + continue + } + o.addError(err) + } +} + + + +// AddWarn will add a log to the result with the Warn Level +func (o *Result) AddWarn(err error) { + o.Logs = append(o.Logs, Log{ + Type: logrus.WarnLevel.String(), + Message: err.Error(), + }) +} + +// printText will print the output in human readable format +func (o *Result) printText(logger *logrus.Entry) error { + + for _, obj := range o.Logs { + lvl, err := logrus.ParseLevel(obj.Type) + if err != nil { + return err + } + switch lvl { + case logrus.InfoLevel: + logger.Info(obj.Message) + case logrus.WarnLevel: + logger.Warn(obj.Message) + case logrus.ErrorLevel: + logger.Error(obj.Message) + default: + return fmt.Errorf("unknown output level %q", obj.Type) + } + } + + return nil +} + +// printJSON will print the output in JSON format +func (o *Result) printJSON() error { + prettyJSON, err := json.MarshalIndent(o, "", " ") + if err != nil { + return fmt.Errorf("error marshaling JSON output: %v", err) + } + fmt.Printf("%s\n", string(prettyJSON)) + return nil +} + +// prepare should be used when writing an Result to a non-log writer. +// it will ensure that the passed boolean will properly set +func (o *Result) prepare() error { + o.Passed = true + for i, obj := range o.Logs { + lvl, err := logrus.ParseLevel(obj.Type) + if err != nil { + return err + } + if o.Passed && lvl == logrus.ErrorLevel { + o.Passed = false + } + lvlBytes, _ := lvl.MarshalText() + o.Logs[i].Type = string(lvlBytes) + } + return nil +} + +// PrintWithFormat prints output to w in format, and exits if some object in output +// is not in a passing state. +func (o *Result) PrintWithFormat(format string) (err error) { + if err = o.prepare(); err != nil { + return fmt.Errorf("error to prepare output: %v", err) + } + + printf := o.getPrintFuncFormat(format) + if err = printf(*o); err == nil && !o.Passed { + os.Exit(1) + } + return err +} + +// getPrintFuncFormat returns a function that writes an Result to w in a given +// format, defaulting to "text" if format is not recognized. +func (o *Result) getPrintFuncFormat(format string) func(Result) error { + // PrintWithFormat output in desired format. + switch format { + case JSONAlpha1: + return func(o Result) error { + return o.printJSON() + } + } + + logger := log.NewEntry(NewLoggerTo(os.Stdout)) + return func(o Result) error { + return o.printText(logger) + } +} From 7007291441924732f49f0ebd093e68c8a207817a Mon Sep 17 00:00:00 2001 From: Camila Macedo Date: Sun, 24 May 2020 07:47:39 +0100 Subject: [PATCH 2/4] rebase + cleanup + add tests + address suggestionss --- cmd/operator-sdk/alpha/scorecard/cmd.go | 2 +- cmd/operator-sdk/bundle/validate.go | 79 ++++--- go.mod | 3 +- go.sum | 6 +- {pkg => internal}/output/logger.go | 0 {pkg => internal}/output/result.go | 48 ++-- internal/output/result_test.go | 288 ++++++++++++++++++++++++ 7 files changed, 353 insertions(+), 73 deletions(-) rename {pkg => internal}/output/logger.go (100%) rename {pkg => internal}/output/result.go (78%) create mode 100644 internal/output/result_test.go diff --git a/cmd/operator-sdk/alpha/scorecard/cmd.go b/cmd/operator-sdk/alpha/scorecard/cmd.go index e599c63f1d7..b6155c68d96 100644 --- a/cmd/operator-sdk/alpha/scorecard/cmd.go +++ b/cmd/operator-sdk/alpha/scorecard/cmd.go @@ -30,9 +30,9 @@ import ( "k8s.io/apimachinery/pkg/labels" "github.com/operator-framework/operator-sdk/internal/flags" + "github.com/operator-framework/operator-sdk/internal/output" scorecard "github.com/operator-framework/operator-sdk/internal/scorecard/alpha" "github.com/operator-framework/operator-sdk/pkg/apis/scorecard/v1alpha2" - "github.com/operator-framework/operator-sdk/pkg/output" ) func NewCmd() *cobra.Command { diff --git a/cmd/operator-sdk/bundle/validate.go b/cmd/operator-sdk/bundle/validate.go index c802b0e0f60..5aae197c765 100644 --- a/cmd/operator-sdk/bundle/validate.go +++ b/cmd/operator-sdk/bundle/validate.go @@ -30,9 +30,8 @@ import ( "github.com/spf13/viper" "github.com/operator-framework/operator-sdk/internal/flags" + "github.com/operator-framework/operator-sdk/internal/output" internalregistry "github.com/operator-framework/operator-sdk/internal/registry" - "github.com/operator-framework/operator-sdk/pkg/output" - ) type bundleValidateCmd struct { @@ -88,6 +87,9 @@ To build and validate an image: } // Always print non-output logs to stderr as to not pollute actual command output. + // Note that it allows the JSON result be redirected to the Stdout. E.g + // if we run the command with `| jq . > result.json` the command will print just the logs + // and the file will have only the JSON result. logger := log.NewEntry(output.NewLoggerTo(os.Stderr)) if err = c.validate(args); err != nil { @@ -110,23 +112,21 @@ To build and validate an image: } }() - log.Info("Unpacking image layers") - + logger.Info("Unpacking image layers") if err := c.unpackImageIntoDir(args[0], c.directory); err != nil { logger.Fatalf("Error unpacking image %s: %v", args[0], err) } } - result, err := c.run(logger) - if err != nil { - logger.Fatal(err) + result := c.run() + if result.Passed { // if no errors were added + result.AddInfo("All validation tests have completed successfully") } + if err := result.PrintWithFormat(c.outputFormat); err != nil { logger.Fatal(err) } - log.Info("All validation tests have completed successfully") - return nil }, } @@ -136,6 +136,7 @@ To build and validate an image: return cmd } +// validate verifies the command args func (c bundleValidateCmd) validate(args []string) error { if len(args) != 1 { return errors.New("an image tag or directory is a required argument") @@ -162,21 +163,21 @@ func (c *bundleValidateCmd) addToFlagSet(fs *pflag.FlagSet) { } } -func (c bundleValidateCmd) run(logger *log.Entry) (res output.Result, err error) { +func (c bundleValidateCmd) run() (res output.Result) { + // Create Result to be outputted + res = output.NewResult() - // Create a new res containing validation errors and info msgs, if any - res = output.Result{} - res.Passed = true // Init without errors - - logger = log.WithFields(log.Fields{ + logger := log.WithFields(log.Fields{ "bundle-dir": c.directory, "container-tool": c.imageBuilder, }) + val := registrybundle.NewImageValidator(c.imageBuilder, logger) // Validate bundle format. if err := val.ValidateBundleFormat(c.directory); err != nil { - return res, fmt.Errorf("invalid bundle format: %v", err) + addBundleValidationErrorToResult(err, &res) + res.AddError(errors.New("invalid bundle format")) } // Validate bundle content. @@ -185,13 +186,15 @@ func (c bundleValidateCmd) run(logger *log.Entry) (res output.Result, err error manifestsDir := filepath.Join(c.directory, registrybundle.ManifestsDir) results, err := validateBundleContent(logger, manifestsDir) if err != nil { - return res, fmt.Errorf("error validating %s: %v", manifestsDir, err) - } - if checkResults(results) { - return res, errors.New("invalid bundle content") + addBundleValidationErrorToResult(err, &res) + res.AddError(fmt.Errorf("error validating %s", manifestsDir)) } - return res, nil + // Check the Results will check the []apierrors.ManifestResult returned + // from the ValidateBundleContent to add the output(s) into the result + checkResults(results, &res) + + return res } // unpackImageIntoDir writes files in image layers found in image imageTag to dir. @@ -201,7 +204,6 @@ func (c bundleValidateCmd) unpackImageIntoDir(imageTag, dir string) error { "container-tool": c.imageBuilder, }) val := registrybundle.NewImageValidator(c.imageBuilder, logger) - return val.PullBundleImage(imageTag, dir) } @@ -217,42 +219,39 @@ func validateBundleContent(logger *log.Entry, manifestsDir string) ([]apierrors. if err != nil { return nil, err } - return internalregistry.ValidateBundleContent(logger, bundle, mediaType), nil } // checkResults logs warnings and errors in results, and returns true if at // least one error was encountered. -func checkResults(results []apierrors.ManifestResult) (hasErrors bool) { +func checkResults(results []apierrors.ManifestResult, res *output.Result) { + hasErrors := false for _, r := range results { for _, w := range r.Warnings { - log.Warnf("%s validation: [%s] %s", r.Name, w.Type, w.Detail) + res.AddWarn(w) } for _, e := range r.Errors { - log.Errorf("%s validation: [%s] %s", r.Name, e.Type, e.Detail) - } - if r.HasError() { + res.AddError(e) hasErrors = true } + + } + if hasErrors { + res.AddError(errors.New("invalid bundle content")) } - return hasErrors } -// addBundleValErrorToOutput will add the output logs results from the Bundle Validation +// addBundleValidationErrorToResult will check if is a ValidationError from Registry Operator and set them +// accordingly to the results // NOTE: Currently, the Bundle Validation from Operator Registry only returns error types, -// however, we might have WARN types in the future. -func addBundleValErrorToOutput(err error, res *output.Result) { - verr := bundle.ValidationError{} +// however, it might have WARN and INFO types in the future as well. +func addBundleValidationErrorToResult(err error, res *output.Result) { + verr := registrybundle.ValidationError{} if errors.As(err, &verr) { for _, valErr := range verr.Errors { - res.AddErrors(valErr) + res.AddError(valErr) } } else { - res.AddErrors(err) + res.AddError(err) } } - -// isToOutputJSON returns true when it should output json -func (c bundleValidateCmd) isToOutputJSON() bool { - return c.outputFormat == output.JSONAlpha1 -} diff --git a/go.mod b/go.mod index 28cefacb176..0133c620f5b 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,6 @@ require ( github.com/fatih/structtag v1.1.0 github.com/go-logr/logr v0.1.0 github.com/go-logr/zapr v0.1.1 - github.com/gobuffalo/logger v1.0.1 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/iancoleman/strcase v0.0.0-20190422225806-e506e3ef7365 github.com/markbates/inflect v1.0.4 @@ -24,7 +23,7 @@ require ( github.com/prometheus/client_golang v1.5.1 github.com/rogpeppe/go-internal v1.5.0 github.com/sergi/go-diff v1.0.0 - github.com/sirupsen/logrus v1.5.0 + github.com/sirupsen/logrus v1.6.0 github.com/spf13/afero v1.2.2 github.com/spf13/cobra v1.0.0 github.com/spf13/pflag v1.0.5 diff --git a/go.sum b/go.sum index 80eb2a46c50..a0666486fb1 100644 --- a/go.sum +++ b/go.sum @@ -598,6 +598,8 @@ github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+o github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/konsorten/go-windows-terminal-sequences v1.0.2 h1:DB17ag19krx9CFsz4o3enTrPXyIXCl+2iCXH/aMAp9s= github.com/konsorten/go-windows-terminal-sequences v1.0.2/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= +github.com/konsorten/go-windows-terminal-sequences v1.0.3 h1:CE8S1cTafDpPvMhIxNJKvHsGVBgn1xWYf1NbHQhywc8= +github.com/konsorten/go-windows-terminal-sequences v1.0.3/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc= github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= @@ -879,8 +881,8 @@ github.com/sirupsen/logrus v1.0.4-0.20170822132746-89742aefa4b2/go.mod h1:pMByvH github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo= github.com/sirupsen/logrus v1.4.1/go.mod h1:ni0Sbl8bgC9z8RoU9G6nDWqqs/fq4eDPysMBDgk/93Q= github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE= -github.com/sirupsen/logrus v1.5.0 h1:1N5EYkVAPEywqZRJd7cwnRtCb6xJx7NH3T3WUTF980Q= -github.com/sirupsen/logrus v1.5.0/go.mod h1:+F7Ogzej0PZc/94MaYx/nvG9jOFMD2osvC3s+Squfpo= +github.com/sirupsen/logrus v1.6.0 h1:UBcNElsrwanuuMsnGSlYmtmgbb23qDR5dG+6X6Oo89I= +github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrfsX/uA88= github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d/go.mod h1:OnSkiWE9lh6wB0YB77sQom3nweQdgAjqCqsofrRNTgc= github.com/smartystreets/assertions v1.0.1/go.mod h1:kHHU4qYBaI3q23Pp3VPrmWhuIUrLW/7eUrw0BU5VaoM= github.com/smartystreets/goconvey v0.0.0-20190330032615-68dc04aab96a/go.mod h1:syvi0/a8iFYH4r/RixwvyeAJjdLS9QV7WQ/tjFTllLA= diff --git a/pkg/output/logger.go b/internal/output/logger.go similarity index 100% rename from pkg/output/logger.go rename to internal/output/logger.go diff --git a/pkg/output/result.go b/internal/output/result.go similarity index 78% rename from pkg/output/result.go rename to internal/output/result.go index 0a9ee4032a3..d66121429fe 100644 --- a/pkg/output/result.go +++ b/internal/output/result.go @@ -24,56 +24,47 @@ import ( ) const ( - //output types JSONAlpha1 = "json-alpha1" Text = "text" ) // Result represents the final result type Result struct { - Passed bool `json:"passed"` - Logs []Log `json:"logs"` + Passed bool `json:"passed"` + Outputs []output `json:"outputs"` } -// Log represents the logs which are used to return the final result in the JSON format -type Log struct { +// output represents the logs which are used to return the final result in the JSON format +type output struct { Type string `json:"type"` Message string `json:"message"` } +// NewResult return a new result object which starts with passed == true since has no errors +func NewResult() Result { + return Result{Passed: true} +} + // AddInfo will add a log to the result with the Info Level func (o *Result) AddInfo(msg string) { - o.Logs = append(o.Logs, Log{ + o.Outputs = append(o.Outputs, output{ Type: logrus.InfoLevel.String(), Message: msg, }) } // AddError will add a log to the result with the Error Level -func (o *Result) addError(err error) { - o.Logs = append(o.Logs, Log{ +func (o *Result) AddError(err error) { + o.Outputs = append(o.Outputs, output{ Type: logrus.ErrorLevel.String(), Message: err.Error(), }) o.Passed = false } -// AddError(s) will add a log to the result with the Error Level -func (o *Result) AddErrors(errs ...error) { - // Collect validation errors, if any. - for _, err := range errs { - if err == nil { - continue - } - o.addError(err) - } -} - - - // AddWarn will add a log to the result with the Warn Level func (o *Result) AddWarn(err error) { - o.Logs = append(o.Logs, Log{ + o.Outputs = append(o.Outputs, output{ Type: logrus.WarnLevel.String(), Message: err.Error(), }) @@ -81,8 +72,7 @@ func (o *Result) AddWarn(err error) { // printText will print the output in human readable format func (o *Result) printText(logger *logrus.Entry) error { - - for _, obj := range o.Logs { + for _, obj := range o.Outputs { lvl, err := logrus.ParseLevel(obj.Type) if err != nil { return err @@ -113,10 +103,10 @@ func (o *Result) printJSON() error { } // prepare should be used when writing an Result to a non-log writer. -// it will ensure that the passed boolean will properly set +// it will ensure that the passed boolean will properly set in the case of the setters were not properly used func (o *Result) prepare() error { o.Passed = true - for i, obj := range o.Logs { + for i, obj := range o.Outputs { lvl, err := logrus.ParseLevel(obj.Type) if err != nil { return err @@ -125,7 +115,7 @@ func (o *Result) prepare() error { o.Passed = false } lvlBytes, _ := lvl.MarshalText() - o.Logs[i].Type = string(lvlBytes) + o.Outputs[i].Type = string(lvlBytes) } return nil } @@ -133,13 +123,14 @@ func (o *Result) prepare() error { // PrintWithFormat prints output to w in format, and exits if some object in output // is not in a passing state. func (o *Result) PrintWithFormat(format string) (err error) { + // the prepare will ensure the result data if the setters were not used if err = o.prepare(); err != nil { return fmt.Errorf("error to prepare output: %v", err) } printf := o.getPrintFuncFormat(format) if err = printf(*o); err == nil && !o.Passed { - os.Exit(1) + os.Exit(1) // Exit with error when any Error type was added } return err } @@ -155,6 +146,7 @@ func (o *Result) getPrintFuncFormat(format string) func(Result) error { } } + // Address all to the Stdout when the type is not JSON logger := log.NewEntry(NewLoggerTo(os.Stdout)) return func(o Result) error { return o.printText(logger) diff --git a/internal/output/result_test.go b/internal/output/result_test.go new file mode 100644 index 00000000000..5481c9a0ecd --- /dev/null +++ b/internal/output/result_test.go @@ -0,0 +1,288 @@ +// Copyright 2020 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 output + +import ( + "encoding/json" + "errors" + "io/ioutil" + "os" + "testing" + + . "github.com/onsi/ginkgo" //nolint:golint + . "github.com/onsi/gomega" //nolint:golint + log "github.com/sirupsen/logrus" +) + +func TestResult(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Output Result Tests") +} + +var _ = Describe("Output Result", func() { + var result Result + + BeforeEach(func() { + result = NewResult() + }) + + Describe("Test Result Model Manipulation", func() { + It("should add the error with ErrorLevel and passed should be flagged with false", func() { + result.AddError(errors.New("example of an error")) + + Expect(result).NotTo(BeNil()) + Expect(result.Passed).To(BeFalse()) + Expect(result.Outputs[0].Type).To(Equal(log.ErrorLevel.String())) + Expect(result.Outputs[0].Message).To(Equal("example of an error")) + }) + + It("should add the error with WarnLevel and passed should be flagged with true", func() { + result.AddWarn(errors.New("example of a warn")) + + Expect(result).NotTo(BeNil()) + Expect(result.Passed).To(BeTrue()) + Expect(result.Outputs[0].Type).To(Equal(log.WarnLevel.String())) + Expect(result.Outputs[0].Message).To(Equal("example of a warn")) + }) + + It("should add msg with InfoLevel and passed should be flagged with true", func() { + result.AddInfo("Example of an info") + + Expect(result).NotTo(BeNil()) + Expect(result.Passed).To(BeTrue()) + Expect(result.Outputs[0].Type).To(Equal(log.InfoLevel.String())) + Expect(result.Outputs[0].Message).To(Equal("Example of an info")) + }) + + It("should passed be flagged with false when has many outputs and an error", func() { + result.AddError(errors.New("example of an error")) + result.AddWarn(errors.New("example of a warn")) + result.AddInfo("Example of an info") + + Expect(result).NotTo(BeNil()) + Expect(result.Passed).To(BeFalse()) + Expect(result.Outputs).To(HaveLen(3)) + }) + + }) + + Describe("Test PrintText", func() { + It("should work successfully with valid log levels", func() { + logger := log.NewEntry(NewLoggerTo(os.Stderr)) + result.AddError(errors.New("example of an error")) + result.AddWarn(errors.New("example of a warn")) + result.AddInfo("Example of an info") + + err := result.printText(logger) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should fail when an invalid log level is found", func() { + // This scenario can just occurs if the setters are not used + logger := log.NewEntry(NewLoggerTo(os.Stderr)) + result.Outputs = append(result.Outputs, output{ + Type: log.TraceLevel.String(), + Message: "invalid", + }) + + err := result.printText(logger) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("unknown output level \"trace\"")) + }) + + It("should fail when is not possible parse the log level", func() { + // This scenario can just occurs if the setters are not used + logger := log.NewEntry(NewLoggerTo(os.Stderr)) + result.Outputs = append(result.Outputs, output{ + Type: "invalid", + Message: "invalid", + }) + + err := result.printText(logger) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("not a valid logrus Level: \"invalid\"")) + }) + }) + + Describe("Test prepare()", func() { + It("should finished with passed flagged with false when has an output with the ErrorLevel", func() { + // This scenario can just occurs if the setters are not used + result.Outputs = append(result.Outputs, output{ + Type: log.ErrorLevel.String(), + Message: "error", + }) + + result.Outputs = append(result.Outputs, output{ + Type: log.InfoLevel.String(), + Message: "info", + }) + + result.Outputs = append(result.Outputs, output{ + Type: log.WarnLevel.String(), + Message: "warn", + }) + + err := result.prepare() + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(result.Passed).To(BeFalse()) + Expect(result.Outputs).To(HaveLen(3)) + }) + + It("should fail when an invalid log level is found", func() { + // This scenario can just occurs if the setters are not used + result.Outputs = append(result.Outputs, output{ + Type: "invalid", + Message: "invalid", + }) + + err := result.prepare() + Expect(err).To(HaveOccurred()) + }) + }) + + Describe("Test getPrintFuncFormat()", func() { + It("should return the printJSON func which works successfully", func() { + By("passing the format`json-alpha1`") + printf := result.getPrintFuncFormat(JSONAlpha1) + Expect(printf).ToNot(BeNil()) + + r, w, _ := os.Pipe() + tmp := os.Stdout + defer func() { + os.Stdout = tmp + }() + os.Stdout = w + go func() { + err := printf(result) + Expect(err).NotTo(HaveOccurred()) + w.Close() + }() + stdout, _ := ioutil.ReadAll(r) + res := map[string]interface{}{} + + Expect(json.Unmarshal(stdout, &res)).To(Succeed()) + Expect(res).To(HaveKeyWithValue("passed", true)) + }) + + It("should return the printText func which works successfully", func() { + By("passing ANY value as format") + printf := result.getPrintFuncFormat("ANY") + Expect(printf).ToNot(BeNil()) + + r, w, _ := os.Pipe() + tmp := os.Stdout + defer func() { + os.Stdout = tmp + }() + os.Stdout = w + go func() { + err := printf(result) + Expect(err).NotTo(HaveOccurred()) + w.Close() + }() + stdout, _ := ioutil.ReadAll(r) + res := map[string]interface{}{} + + Expect(json.Unmarshal(stdout, &res)).NotTo(Succeed()) + }) + }) + + Describe("Test printJSON()", func() { + It("should return a pretty JSON", func() { + By("adding an error") + result.AddError(errors.New("example of an error")) + r, w, _ := os.Pipe() + tmp := os.Stdout + defer func() { + os.Stdout = tmp + }() + os.Stdout = w + go func() { + Expect(result.printJSON()).To(Succeed()) + w.Close() + }() + stdout, _ := ioutil.ReadAll(r) + res := map[string]interface{}{} + + By("checking if stdout is an JSON") + Expect(json.Unmarshal(stdout, &res)).To(Succeed()) + + By("checking if the stdout has the expected values") + Expect(res).To(HaveKeyWithValue("passed", false)) + Expect(res).To(HaveKey("outputs")) + Expect(string(stdout)).To(ContainSubstring("example of an error")) + }) + }) + + Describe("Test PrintWithFormat()", func() { + It("should print a JSON", func() { + By("passing the format`json-alpha1`") + result.AddWarn(errors.New("example of an warn")) + r, w, _ := os.Pipe() + tmp := os.Stdout + defer func() { + os.Stdout = tmp + }() + os.Stdout = w + go func() { + Expect(result.PrintWithFormat(JSONAlpha1)).To(Succeed()) + w.Close() + }() + stdout, _ := ioutil.ReadAll(r) + res := map[string]interface{}{} + By("checking if stdout is an JSON") + Expect(json.Unmarshal(stdout, &res)).To(Succeed()) + + By("checking if the stdout has the expected values") + Expect(res).To(HaveKeyWithValue("passed", true)) + Expect(res).To(HaveKey("outputs")) + Expect(string(stdout)).To(ContainSubstring("example of an warn")) + }) + + It("should NOT print a JSON", func() { + By("passing the format`text`") + result.AddWarn(errors.New("example of an warn")) + r, w, _ := os.Pipe() + tmp := os.Stdout + defer func() { + os.Stdout = tmp + }() + os.Stdout = w + go func() { + Expect(result.PrintWithFormat(Text)).To(Succeed()) + w.Close() + }() + stdout, _ := ioutil.ReadAll(r) + res := map[string]interface{}{} + + By("checking that the output is NOT a JSON") + Expect(json.Unmarshal(stdout, &res)).NotTo(Succeed()) + + By("checking if the stdout has the expected values") + Expect(string(stdout)).To(ContainSubstring("example of an warn")) + }) + + It("should return an error to prepare the output", func() { + By("adding an invalid log level`") + result.Outputs = append(result.Outputs, output{ + Type: "invalid", + Message: "invalid", + }) + + Expect(result.PrintWithFormat(Text)).NotTo(Succeed()) + }) + }) +}) From 1fc304e4470f999bec94c9a2b1ca2144dd3472b7 Mon Sep 17 00:00:00 2001 From: Camila Macedo Date: Thu, 28 May 2020 00:31:52 +0100 Subject: [PATCH 3/4] latest review - eric --- changelog/fragments/json-hidden-flag.yaml | 2 +- cmd/operator-sdk/alpha/scorecard/cmd.go | 11 ++++-- .../operator-sdk/bundle/internal}/logger.go | 2 +- .../operator-sdk/bundle/internal}/result.go | 2 +- .../bundle/internal}/result_test.go | 2 +- cmd/operator-sdk/bundle/validate.go | 35 +++++++------------ cmd/operator-sdk/scorecard/cmd.go | 11 +++--- go.mod | 2 +- go.sum | 6 ++-- 9 files changed, 34 insertions(+), 39 deletions(-) rename {internal/output => cmd/operator-sdk/bundle/internal}/logger.go (97%) rename {internal/output => cmd/operator-sdk/bundle/internal}/result.go (99%) rename {internal/output => cmd/operator-sdk/bundle/internal}/result_test.go (99%) diff --git a/changelog/fragments/json-hidden-flag.yaml b/changelog/fragments/json-hidden-flag.yaml index db715577998..d00e75db99f 100644 --- a/changelog/fragments/json-hidden-flag.yaml +++ b/changelog/fragments/json-hidden-flag.yaml @@ -2,7 +2,7 @@ # release notes and/or the migration guide entries: - description: > - Add new hidden alpha flag `--output` to allow output the result of `operator-sdk bundle validate` in JSON format. + Add new hidden alpha flag `--output` to print the result of `operator-sdk bundle validate` in JSON format to stdout. Logs are printed to stderr. # kind is one of: # - addition diff --git a/cmd/operator-sdk/alpha/scorecard/cmd.go b/cmd/operator-sdk/alpha/scorecard/cmd.go index b6155c68d96..97db719f810 100644 --- a/cmd/operator-sdk/alpha/scorecard/cmd.go +++ b/cmd/operator-sdk/alpha/scorecard/cmd.go @@ -30,7 +30,6 @@ import ( "k8s.io/apimachinery/pkg/labels" "github.com/operator-framework/operator-sdk/internal/flags" - "github.com/operator-framework/operator-sdk/internal/output" scorecard "github.com/operator-framework/operator-sdk/internal/scorecard/alpha" "github.com/operator-framework/operator-sdk/pkg/apis/scorecard/v1alpha2" ) @@ -66,7 +65,7 @@ If the argument holds an image tag, it must be present remotely.`, // Extract bundle image contents if bundle is inferred to be an image. if _, err = os.Stat(bundle); err != nil && errors.Is(err, os.ErrNotExist) { // Discard bundle extraction logs unless user sets verbose mode. - logger := log.NewEntry(output.NewLoggerTo(ioutil.Discard)) + logger := log.NewEntry(discardLogger()) if viper.GetBool(flags.VerboseOpt) { logger = log.WithFields(log.Fields{"bundle": bundle}) } @@ -172,4 +171,12 @@ func printOutput(outputFormat string, output v1alpha2.ScorecardOutput) error { return fmt.Errorf("invalid output format selected") } return nil + +} + +// discardLogger returns a logger that throws away input. +func discardLogger() *log.Logger { + logger := log.New() + logger.SetOutput(ioutil.Discard) + return logger } diff --git a/internal/output/logger.go b/cmd/operator-sdk/bundle/internal/logger.go similarity index 97% rename from internal/output/logger.go rename to cmd/operator-sdk/bundle/internal/logger.go index f97e1ddab87..fcba00b2504 100644 --- a/internal/output/logger.go +++ b/cmd/operator-sdk/bundle/internal/logger.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package output +package internal import ( "io" diff --git a/internal/output/result.go b/cmd/operator-sdk/bundle/internal/result.go similarity index 99% rename from internal/output/result.go rename to cmd/operator-sdk/bundle/internal/result.go index d66121429fe..cad41e2b0ec 100644 --- a/internal/output/result.go +++ b/cmd/operator-sdk/bundle/internal/result.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package output +package internal import ( "encoding/json" diff --git a/internal/output/result_test.go b/cmd/operator-sdk/bundle/internal/result_test.go similarity index 99% rename from internal/output/result_test.go rename to cmd/operator-sdk/bundle/internal/result_test.go index 5481c9a0ecd..f47861aa402 100644 --- a/internal/output/result_test.go +++ b/cmd/operator-sdk/bundle/internal/result_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package output +package internal import ( "encoding/json" diff --git a/cmd/operator-sdk/bundle/validate.go b/cmd/operator-sdk/bundle/validate.go index 5aae197c765..1a2987e65d1 100644 --- a/cmd/operator-sdk/bundle/validate.go +++ b/cmd/operator-sdk/bundle/validate.go @@ -21,6 +21,8 @@ import ( "os" "path/filepath" + "github.com/operator-framework/operator-sdk/cmd/operator-sdk/bundle/internal" + apimanifests "github.com/operator-framework/api/pkg/manifests" apierrors "github.com/operator-framework/api/pkg/validation/errors" registrybundle "github.com/operator-framework/operator-registry/pkg/lib/bundle" @@ -30,7 +32,6 @@ import ( "github.com/spf13/viper" "github.com/operator-framework/operator-sdk/internal/flags" - "github.com/operator-framework/operator-sdk/internal/output" internalregistry "github.com/operator-framework/operator-sdk/internal/registry" ) @@ -90,7 +91,7 @@ To build and validate an image: // Note that it allows the JSON result be redirected to the Stdout. E.g // if we run the command with `| jq . > result.json` the command will print just the logs // and the file will have only the JSON result. - logger := log.NewEntry(output.NewLoggerTo(os.Stderr)) + logger := log.NewEntry(internal.NewLoggerTo(os.Stderr)) if err = c.validate(args); err != nil { return fmt.Errorf("invalid command args: %v", err) @@ -119,14 +120,10 @@ To build and validate an image: } result := c.run() - if result.Passed { // if no errors were added - result.AddInfo("All validation tests have completed successfully") - } - if err := result.PrintWithFormat(c.outputFormat); err != nil { logger.Fatal(err) } - + logger.Info("All validation tests have completed successfully") return nil }, } @@ -141,7 +138,7 @@ func (c bundleValidateCmd) validate(args []string) error { if len(args) != 1 { return errors.New("an image tag or directory is a required argument") } - if c.outputFormat != output.JSONAlpha1 && c.outputFormat != output.Text { + if c.outputFormat != internal.JSONAlpha1 && c.outputFormat != internal.Text { return fmt.Errorf("invalid value for output flag: %v", c.outputFormat) } return nil @@ -153,7 +150,7 @@ func (c *bundleValidateCmd) addToFlagSet(fs *pflag.FlagSet) { fs.StringVarP(&c.imageBuilder, "image-builder", "b", "docker", "Tool to extract bundle image data. Only used when validating a bundle image. "+ "One of: [docker, podman]") - fs.StringVarP(&c.outputFormat, "output", "o", output.Text, + fs.StringVarP(&c.outputFormat, "output", "o", internal.Text, "Result format for results. One of: [text, json-alpha1]") // It is hidden because it is an alpha option @@ -163,9 +160,9 @@ func (c *bundleValidateCmd) addToFlagSet(fs *pflag.FlagSet) { } } -func (c bundleValidateCmd) run() (res output.Result) { +func (c bundleValidateCmd) run() (res internal.Result) { // Create Result to be outputted - res = output.NewResult() + res = internal.NewResult() logger := log.WithFields(log.Fields{ "bundle-dir": c.directory, @@ -176,8 +173,7 @@ func (c bundleValidateCmd) run() (res output.Result) { // Validate bundle format. if err := val.ValidateBundleFormat(c.directory); err != nil { - addBundleValidationErrorToResult(err, &res) - res.AddError(errors.New("invalid bundle format")) + addBundleValidationErrorToResult(fmt.Errorf("error validating format in %s: %v", c.directory, err), &res) } // Validate bundle content. @@ -186,8 +182,7 @@ func (c bundleValidateCmd) run() (res output.Result) { manifestsDir := filepath.Join(c.directory, registrybundle.ManifestsDir) results, err := validateBundleContent(logger, manifestsDir) if err != nil { - addBundleValidationErrorToResult(err, &res) - res.AddError(fmt.Errorf("error validating %s", manifestsDir)) + addBundleValidationErrorToResult(fmt.Errorf("error validating content in %s: %v", manifestsDir, err), &res) } // Check the Results will check the []apierrors.ManifestResult returned @@ -224,20 +219,14 @@ func validateBundleContent(logger *log.Entry, manifestsDir string) ([]apierrors. // checkResults logs warnings and errors in results, and returns true if at // least one error was encountered. -func checkResults(results []apierrors.ManifestResult, res *output.Result) { - hasErrors := false +func checkResults(results []apierrors.ManifestResult, res *internal.Result) { for _, r := range results { for _, w := range r.Warnings { res.AddWarn(w) } for _, e := range r.Errors { res.AddError(e) - hasErrors = true } - - } - if hasErrors { - res.AddError(errors.New("invalid bundle content")) } } @@ -245,7 +234,7 @@ func checkResults(results []apierrors.ManifestResult, res *output.Result) { // accordingly to the results // NOTE: Currently, the Bundle Validation from Operator Registry only returns error types, // however, it might have WARN and INFO types in the future as well. -func addBundleValidationErrorToResult(err error, res *output.Result) { +func addBundleValidationErrorToResult(err error, res *internal.Result) { verr := registrybundle.ValidationError{} if errors.As(err, &verr) { for _, valErr := range verr.Errors { diff --git a/cmd/operator-sdk/scorecard/cmd.go b/cmd/operator-sdk/scorecard/cmd.go index fa5c9ae8744..487110a1b72 100644 --- a/cmd/operator-sdk/scorecard/cmd.go +++ b/cmd/operator-sdk/scorecard/cmd.go @@ -20,16 +20,17 @@ import ( "io" "os" - "github.com/mitchellh/mapstructure" - "github.com/pkg/errors" log "github.com/sirupsen/logrus" - "github.com/spf13/cobra" - "github.com/spf13/viper" - "k8s.io/apimachinery/pkg/labels" + "github.com/mitchellh/mapstructure" "github.com/operator-framework/operator-sdk/internal/scorecard" schelpers "github.com/operator-framework/operator-sdk/internal/scorecard/helpers" "github.com/operator-framework/operator-sdk/internal/util/projutil" + "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/labels" + + "github.com/spf13/cobra" + "github.com/spf13/viper" ) const ( diff --git a/go.mod b/go.mod index 0133c620f5b..b3cfc5977ff 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,7 @@ require ( github.com/prometheus/client_golang v1.5.1 github.com/rogpeppe/go-internal v1.5.0 github.com/sergi/go-diff v1.0.0 - github.com/sirupsen/logrus v1.6.0 + github.com/sirupsen/logrus v1.5.0 github.com/spf13/afero v1.2.2 github.com/spf13/cobra v1.0.0 github.com/spf13/pflag v1.0.5 diff --git a/go.sum b/go.sum index a0666486fb1..80eb2a46c50 100644 --- a/go.sum +++ b/go.sum @@ -598,8 +598,6 @@ github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+o github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/konsorten/go-windows-terminal-sequences v1.0.2 h1:DB17ag19krx9CFsz4o3enTrPXyIXCl+2iCXH/aMAp9s= github.com/konsorten/go-windows-terminal-sequences v1.0.2/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= -github.com/konsorten/go-windows-terminal-sequences v1.0.3 h1:CE8S1cTafDpPvMhIxNJKvHsGVBgn1xWYf1NbHQhywc8= -github.com/konsorten/go-windows-terminal-sequences v1.0.3/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc= github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= @@ -881,8 +879,8 @@ github.com/sirupsen/logrus v1.0.4-0.20170822132746-89742aefa4b2/go.mod h1:pMByvH github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo= github.com/sirupsen/logrus v1.4.1/go.mod h1:ni0Sbl8bgC9z8RoU9G6nDWqqs/fq4eDPysMBDgk/93Q= github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE= -github.com/sirupsen/logrus v1.6.0 h1:UBcNElsrwanuuMsnGSlYmtmgbb23qDR5dG+6X6Oo89I= -github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrfsX/uA88= +github.com/sirupsen/logrus v1.5.0 h1:1N5EYkVAPEywqZRJd7cwnRtCb6xJx7NH3T3WUTF980Q= +github.com/sirupsen/logrus v1.5.0/go.mod h1:+F7Ogzej0PZc/94MaYx/nvG9jOFMD2osvC3s+Squfpo= github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d/go.mod h1:OnSkiWE9lh6wB0YB77sQom3nweQdgAjqCqsofrRNTgc= github.com/smartystreets/assertions v1.0.1/go.mod h1:kHHU4qYBaI3q23Pp3VPrmWhuIUrLW/7eUrw0BU5VaoM= github.com/smartystreets/goconvey v0.0.0-20190330032615-68dc04aab96a/go.mod h1:syvi0/a8iFYH4r/RixwvyeAJjdLS9QV7WQ/tjFTllLA= From 0b26b1dd50e9c24b7d487a823917bb6af477f3c6 Mon Sep 17 00:00:00 2001 From: Camila Macedo Date: Thu, 28 May 2020 10:31:07 +0100 Subject: [PATCH 4/4] cleanup since in the review was defined that the output cannot be use for others cmds --- cmd/operator-sdk/bundle/internal/result.go | 20 ++++++++++++---- cmd/operator-sdk/bundle/validate.go | 28 +++++++--------------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/cmd/operator-sdk/bundle/internal/result.go b/cmd/operator-sdk/bundle/internal/result.go index cad41e2b0ec..fbb982bbc5d 100644 --- a/cmd/operator-sdk/bundle/internal/result.go +++ b/cmd/operator-sdk/bundle/internal/result.go @@ -16,9 +16,11 @@ package internal import ( "encoding/json" + "errors" "fmt" "os" + registrybundle "github.com/operator-framework/operator-registry/pkg/lib/bundle" "github.com/sirupsen/logrus" log "github.com/sirupsen/logrus" ) @@ -55,10 +57,20 @@ func (o *Result) AddInfo(msg string) { // AddError will add a log to the result with the Error Level func (o *Result) AddError(err error) { - o.Outputs = append(o.Outputs, output{ - Type: logrus.ErrorLevel.String(), - Message: err.Error(), - }) + verr := registrybundle.ValidationError{} + if errors.As(err, &verr) { + for _, valErr := range verr.Errors { + o.Outputs = append(o.Outputs, output{ + Type: logrus.ErrorLevel.String(), + Message: valErr.Error(), + }) + } + } else { + o.Outputs = append(o.Outputs, output{ + Type: logrus.ErrorLevel.String(), + Message: err.Error(), + }) + } o.Passed = false } diff --git a/cmd/operator-sdk/bundle/validate.go b/cmd/operator-sdk/bundle/validate.go index 1a2987e65d1..f13a566e83d 100644 --- a/cmd/operator-sdk/bundle/validate.go +++ b/cmd/operator-sdk/bundle/validate.go @@ -21,8 +21,6 @@ import ( "os" "path/filepath" - "github.com/operator-framework/operator-sdk/cmd/operator-sdk/bundle/internal" - apimanifests "github.com/operator-framework/api/pkg/manifests" apierrors "github.com/operator-framework/api/pkg/validation/errors" registrybundle "github.com/operator-framework/operator-registry/pkg/lib/bundle" @@ -31,6 +29,7 @@ import ( "github.com/spf13/pflag" "github.com/spf13/viper" + "github.com/operator-framework/operator-sdk/cmd/operator-sdk/bundle/internal" "github.com/operator-framework/operator-sdk/internal/flags" internalregistry "github.com/operator-framework/operator-sdk/internal/registry" ) @@ -114,6 +113,7 @@ To build and validate an image: }() logger.Info("Unpacking image layers") + if err := c.unpackImageIntoDir(args[0], c.directory); err != nil { logger.Fatalf("Error unpacking image %s: %v", args[0], err) } @@ -123,7 +123,9 @@ To build and validate an image: if err := result.PrintWithFormat(c.outputFormat); err != nil { logger.Fatal(err) } + logger.Info("All validation tests have completed successfully") + return nil }, } @@ -168,12 +170,11 @@ func (c bundleValidateCmd) run() (res internal.Result) { "bundle-dir": c.directory, "container-tool": c.imageBuilder, }) - val := registrybundle.NewImageValidator(c.imageBuilder, logger) // Validate bundle format. if err := val.ValidateBundleFormat(c.directory); err != nil { - addBundleValidationErrorToResult(fmt.Errorf("error validating format in %s: %v", c.directory, err), &res) + res.AddError(fmt.Errorf("error validating format in %s: %v", c.directory, err)) } // Validate bundle content. @@ -182,7 +183,7 @@ func (c bundleValidateCmd) run() (res internal.Result) { manifestsDir := filepath.Join(c.directory, registrybundle.ManifestsDir) results, err := validateBundleContent(logger, manifestsDir) if err != nil { - addBundleValidationErrorToResult(fmt.Errorf("error validating content in %s: %v", manifestsDir, err), &res) + res.AddError(fmt.Errorf("error validating content in %s: %v", manifestsDir, err)) } // Check the Results will check the []apierrors.ManifestResult returned @@ -199,6 +200,7 @@ func (c bundleValidateCmd) unpackImageIntoDir(imageTag, dir string) error { "container-tool": c.imageBuilder, }) val := registrybundle.NewImageValidator(c.imageBuilder, logger) + return val.PullBundleImage(imageTag, dir) } @@ -214,6 +216,7 @@ func validateBundleContent(logger *log.Entry, manifestsDir string) ([]apierrors. if err != nil { return nil, err } + return internalregistry.ValidateBundleContent(logger, bundle, mediaType), nil } @@ -229,18 +232,3 @@ func checkResults(results []apierrors.ManifestResult, res *internal.Result) { } } } - -// addBundleValidationErrorToResult will check if is a ValidationError from Registry Operator and set them -// accordingly to the results -// NOTE: Currently, the Bundle Validation from Operator Registry only returns error types, -// however, it might have WARN and INFO types in the future as well. -func addBundleValidationErrorToResult(err error, res *internal.Result) { - verr := registrybundle.ValidationError{} - if errors.As(err, &verr) { - for _, valErr := range verr.Errors { - res.AddError(valErr) - } - } else { - res.AddError(err) - } -}