diff --git a/cmd/preflight/cmd/check.go b/cmd/preflight/cmd/check.go index 26b1a929..31637a06 100644 --- a/cmd/preflight/cmd/check.go +++ b/cmd/preflight/cmd/check.go @@ -2,19 +2,18 @@ package cmd import ( "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/cli" - "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/viper" "github.com/spf13/cobra" + "github.com/spf13/viper" ) -func checkCmd() *cobra.Command { +func checkCmd(viper *viper.Viper) *cobra.Command { checkCmd := &cobra.Command{ Use: "check", Short: "Run checks for an operator or container", Long: "This command will allow you to execute the Red Hat Certification tests for an operator or a container.", } - viper := viper.Instance() checkCmd.PersistentFlags().StringP("docker-config", "d", "", "Path to docker config.json file. This value is optional for publicly accessible images.\n"+ "However, it is strongly encouraged for public Docker Hub images,\n"+ "due to the rate limit imposed for unauthenticated requests. (env: PFLT_DOCKERCONFIG)") @@ -23,8 +22,8 @@ func checkCmd() *cobra.Command { checkCmd.PersistentFlags().String("artifacts", "", "Where check-specific artifacts will be written. (env: PFLT_ARTIFACTS)") _ = viper.BindPFlag("artifacts", checkCmd.PersistentFlags().Lookup("artifacts")) - checkCmd.AddCommand(checkOperatorCmd(cli.RunPreflight)) - checkCmd.AddCommand(checkContainerCmd(cli.RunPreflight)) + checkCmd.AddCommand(checkOperatorCmd(cli.RunPreflight, viper)) + checkCmd.AddCommand(checkContainerCmd(cli.RunPreflight, viper)) return checkCmd } diff --git a/cmd/preflight/cmd/check_container.go b/cmd/preflight/cmd/check_container.go index e30c0e4c..435376b0 100644 --- a/cmd/preflight/cmd/check_container.go +++ b/cmd/preflight/cmd/check_container.go @@ -22,7 +22,6 @@ import ( "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/log" "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/option" "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/runtime" - "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/viper" "github.com/redhat-openshift-ecosystem/openshift-preflight/version" "github.com/go-logr/logr" @@ -31,30 +30,32 @@ import ( "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/spf13/cobra" "github.com/spf13/pflag" + "github.com/spf13/viper" ) var submit bool // runPreflight is introduced to make testing of this command possible, it has the same method signature as cli.RunPreflight. -type runPreflight func(context.Context, func(ctx context.Context) (certification.Results, error), cli.CheckConfig, formatters.ResponseFormatter, lib.ResultWriter, lib.ResultSubmitter) error +type runPreflight func(context.Context, func(ctx context.Context) (certification.Results, error), cli.CheckConfig, formatters.ResponseFormatter, lib.ResultWriter, lib.ResultSubmitter, string) error -func checkContainerCmd(runpreflight runPreflight) *cobra.Command { +func checkContainerCmd(runpreflight runPreflight, viper *viper.Viper) *cobra.Command { checkContainerCmd := &cobra.Command{ Use: "container", Short: "Run checks for a container", Long: `This command will run the Certification checks for a container image. `, - Args: checkContainerPositionalArgs, + Args: func(cmd *cobra.Command, args []string) error { + return checkContainerPositionalArgs(cmd, args, viper) + }, // this fmt.Sprintf is in place to keep spacing consistent with cobras two spaces that's used in: Usage, Flags, etc Example: fmt.Sprintf(" %s", "preflight check container quay.io/repo-name/container-name:version"), - PreRunE: validateCertificationProjectID, + PreRunE: checkContainerPreRunE, RunE: func(cmd *cobra.Command, args []string) error { - return checkContainerRunE(cmd, args, runpreflight) + return checkContainerRunE(cmd, args, runpreflight, viper) }, } flags := checkContainerCmd.Flags() - viper := viper.Instance() flags.BoolVarP(&submit, "submit", "s", false, "submit check container results to Red Hat") _ = viper.BindPFlag("submit", flags.Lookup("submit")) @@ -93,7 +94,7 @@ func checkContainerCmd(runpreflight runPreflight) *cobra.Command { } // checkContainerRunE executes checkContainer using the user args to inform the execution. -func checkContainerRunE(cmd *cobra.Command, args []string, runpreflight runPreflight) error { +func checkContainerRunE(cmd *cobra.Command, args []string, runpreflight runPreflight, viper *viper.Viper) error { ctx := cmd.Context() logger, err := logr.FromContext(ctx) if err != nil { @@ -104,7 +105,7 @@ func checkContainerRunE(cmd *cobra.Command, args []string, runpreflight runPrefl containerImage := args[0] // Render the Viper configuration as a runtime.Config - cfg, err := runtime.NewConfigFrom(*viper.Instance()) + cfg, err := runtime.NewConfigFrom(*viper) if err != nil { return fmt.Errorf("invalid configuration: %w", err) } @@ -155,6 +156,7 @@ func checkContainerRunE(cmd *cobra.Command, args []string, runpreflight runPrefl formatter, &runtime.ResultWriterFile{}, resultSubmitter, + viper.GetString("pyxis_env"), ); err != nil { return err } @@ -197,7 +199,7 @@ func checkContainerRunE(cmd *cobra.Command, args []string, runpreflight runPrefl return nil } -func checkContainerPositionalArgs(cmd *cobra.Command, args []string) error { +func checkContainerPositionalArgs(cmd *cobra.Command, args []string, viper *viper.Viper) error { if len(args) != 1 { return fmt.Errorf("a container image positional argument is required") } @@ -211,7 +213,6 @@ func checkContainerPositionalArgs(cmd *cobra.Command, args []string) error { }) // --submit was specified - viper := viper.Instance() if submit { // If the flag is not marked as changed AND viper hasn't gotten it from environment, it's an error if !cmd.Flag("certification-project-id").Changed && !viper.IsSet("certification_project_id") { @@ -238,24 +239,34 @@ func checkContainerPositionalArgs(cmd *cobra.Command, args []string) error { return nil } +func checkContainerPreRunE(cmd *cobra.Command, args []string) error { + certificationProjectID := viper.GetString("certification_project_id") + validatedCertificationProjectID, err := validateCertificationProjectID(certificationProjectID) + if err != nil { + return err + } + if validatedCertificationProjectID != certificationProjectID { + viper.Set("certification_project_id", validatedCertificationProjectID) + } + return nil +} + // validateCertificationProjectID validates that the certification project id is in the proper format // and throws an error if the value provided is in a legacy format that is not usable to query pyxis -func validateCertificationProjectID(cmd *cobra.Command, args []string) error { - viper := viper.Instance() - certificationProjectID := viper.GetString("certification_project_id") +func validateCertificationProjectID(certificationProjectID string) (string, error) { // splitting the certification project id into parts. if there are more than 2 elements in the array, // we know they inputted a legacy project id, which can not be used to query pyxis parts := strings.Split(certificationProjectID, "-") if len(parts) > 2 { - return fmt.Errorf("certification project id: %s is improperly formatted see help command for instructions on obtaining proper value", certificationProjectID) + return certificationProjectID, fmt.Errorf("certification project id: %s is improperly formatted see help command for instructions on obtaining proper value", certificationProjectID) } if parts[0] == "ospid" { - viper.Set("certification_project_id", parts[1]) + return parts[1], nil } - return nil + return certificationProjectID, nil } // generateContainerCheckOptions returns appropriate container.Options based on cfg. diff --git a/cmd/preflight/cmd/check_container_test.go b/cmd/preflight/cmd/check_container_test.go index b4454743..0b759aee 100644 --- a/cmd/preflight/cmd/check_container_test.go +++ b/cmd/preflight/cmd/check_container_test.go @@ -13,13 +13,11 @@ import ( "path/filepath" "runtime" - "github.com/redhat-openshift-ecosystem/openshift-preflight/artifacts" "github.com/redhat-openshift-ecosystem/openshift-preflight/certification" "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/check" "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/cli" "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/formatters" "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/lib" - "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/viper" "github.com/go-logr/logr" "github.com/google/go-containerregistry/pkg/crane" @@ -33,6 +31,8 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/onsi/gomega/types" + "github.com/spf13/afero" + "github.com/spf13/viper" ) func createPlatformImage(arch string, addlLayers int) cranev1.Image { @@ -122,8 +122,14 @@ var _ = Describe("Check Container Command", func() { When("a manifest list is passed", func() { When("default params otherwise", func() { + var v *viper.Viper + + BeforeEach(func() { + v = viper.New() + }) + It("should not error", func() { - _, err := executeCommandWithLogger(checkContainerCmd(mockRunPreflightReturnNil), logr.Discard(), manifestListSrc) + _, err := executeCommandWithLogger(checkContainerCmd(mockRunPreflightReturnNil, v), logr.Discard(), manifestListSrc) Expect(err).ToNot(HaveOccurred()) }) }) @@ -131,11 +137,12 @@ var _ = Describe("Check Container Command", func() { DescribeTable("--platform tests", func(manifestKey string, platform string, match types.GomegaMatcher, includePlatformArg bool) { + v := viper.New() args := []string{manifests[manifestKey]} if includePlatformArg { args = append(args, "--platform", platform) } - _, err := executeCommandWithLogger(checkContainerCmd(mockRunPreflightReturnNil), logr.Discard(), args...) + _, err := executeCommandWithLogger(checkContainerCmd(mockRunPreflightReturnNil, v), logr.Discard(), args...) Expect(err).To(match) }, Entry("image manifest, valid platform", "image", "amd64", Not(HaveOccurred()), true), @@ -145,24 +152,24 @@ var _ = Describe("Check Container Command", func() { Entry("index manifest, invalid platform", "index", "none", HaveOccurred(), true), ) - Context("When validating check container arguments and flags", func() { + When("validating check container arguments and flags", func() { Context("and the user provided more than 1 positional arg", func() { It("should fail to run", func() { - _, err := executeCommand(checkContainerCmd(mockRunPreflightReturnNil), "foo", "bar") + _, err := executeCommand(checkContainerCmd(mockRunPreflightReturnNil, viper.New()), "foo", "bar") Expect(err).To(HaveOccurred()) }) }) - Context("and the user provided less than 1 positional arg", func() { + When("the user provided less than 1 positional arg", func() { It("should fail to run", func() { - _, err := executeCommand(checkContainerCmd(mockRunPreflightReturnNil)) + _, err := executeCommand(checkContainerCmd(mockRunPreflightReturnNil, viper.New())) Expect(err).To(HaveOccurred()) }) }) DescribeTable("and the user has enabled the submit flag", func(errString string, args []string) { - out, err := executeCommand(checkContainerCmd(mockRunPreflightReturnNil), args...) + out, err := executeCommand(checkContainerCmd(mockRunPreflightReturnNil, viper.New()), args...) Expect(err).To(HaveOccurred()) Expect(out).To(ContainSubstring(errString)) }, @@ -177,6 +184,11 @@ var _ = Describe("Check Container Command", func() { ) When("the user enables the submit flag", func() { + var v *viper.Viper + + BeforeEach(func() { + v = viper.New() + }) When("environment variables are used for certification ID and api token", func() { BeforeEach(func() { os.Setenv("PFLT_CERTIFICATION_PROJECT_ID", "certid") @@ -185,101 +197,75 @@ var _ = Describe("Check Container Command", func() { DeferCleanup(os.Unsetenv, "PFLT_PYXIS_API_TOKEN") }) It("should still execute with no error", func() { + initConfig(v) submit = true - - err := checkContainerPositionalArgs(checkContainerCmd(mockRunPreflightReturnNil), []string{"foo"}) + err := checkContainerPositionalArgs(checkContainerCmd(mockRunPreflightReturnNil, v), []string{"foo"}, v) Expect(err).ToNot(HaveOccurred()) - Expect(viper.Instance().GetString("pyxis_api_token")).To(Equal("tokenid")) - Expect(viper.Instance().GetString("certification_project_id")).To(Equal("certid")) + Expect(v.GetString("pyxis_api_token")).To(Equal("tokenid")) + Expect(v.GetString("certification_project_id")).To(Equal("certid")) }) }) When("a config file is used", func() { BeforeEach(func() { + fs := afero.NewMemMapFs() + v.SetFs(fs) config := `pyxis_api_token: mytoken certification_project_id: mycertid` - tempDir, err := os.MkdirTemp("", "check-container-submit-*") - Expect(err).ToNot(HaveOccurred()) - err = os.WriteFile(filepath.Join(tempDir, "config.yaml"), bytes.NewBufferString(config).Bytes(), 0o644) - Expect(err).ToNot(HaveOccurred()) - viper.Instance().AddConfigPath(tempDir) - DeferCleanup(os.RemoveAll, tempDir) + Expect(afero.WriteFile(fs, "/config/config.yaml", bytes.NewBufferString(config).Bytes(), 0o644)).To(Succeed()) + v.AddConfigPath("/config") }) It("should still execute with no error", func() { // Make sure that we've read the config file - initConfig(viper.Instance()) + initConfig(v) submit = true - err := checkContainerPositionalArgs(checkContainerCmd(mockRunPreflightReturnNil), []string{"foo"}) + err := checkContainerPositionalArgs(checkContainerCmd(mockRunPreflightReturnNil, v), []string{"foo"}, v) Expect(err).ToNot(HaveOccurred()) - Expect(viper.Instance().GetString("pyxis_api_token")).To(Equal("mytoken")) - Expect(viper.Instance().GetString("certification_project_id")).To(Equal("mycertid")) + Expect(v.GetString("pyxis_api_token")).To(Equal("mytoken")) + Expect(v.GetString("certification_project_id")).To(Equal("mycertid")) }) }) }) }) - Context("When validating the certification-project-id flag", func() { - Context("and the flag is set properly", func() { - BeforeEach(func() { - viper.Instance().Set("certification_project_id", "123456789") - DeferCleanup(viper.Instance().Set, "certification_project_id", "") - }) - It("should not change the flag value", func() { - err := validateCertificationProjectID(checkContainerCmd(mockRunPreflightReturnNil), []string{"foo"}) + DescribeTable("validating the certification-project-id flag", + func(value, expected string, succeed bool) { + updated, err := validateCertificationProjectID(value) + if succeed { Expect(err).ToNot(HaveOccurred()) - Expect(viper.Instance().GetString("certification_project_id")).To(Equal("123456789")) - }) - }) - Context("and a valid ospid format is provided", func() { - BeforeEach(func() { - viper.Instance().Set("certification_project_id", "ospid-123456789") - DeferCleanup(viper.Instance().Set, "certification_project_id", "") - }) - It("should strip ospid- from the flag value", func() { - err := validateCertificationProjectID(checkContainerCmd(mockRunPreflightReturnNil), []string{"foo"}) - Expect(err).ToNot(HaveOccurred()) - Expect(viper.Instance().GetString("certification_project_id")).To(Equal("123456789")) - }) - }) - Context("and a legacy format with ospid is provided", func() { - BeforeEach(func() { - viper.Instance().Set("certification_project_id", "ospid-62423-f26c346-6cc1dc7fae92") - DeferCleanup(viper.Instance().Set, "certification_project_id", "") - }) - It("should throw an error", func() { - err := validateCertificationProjectID(checkContainerCmd(mockRunPreflightReturnNil), []string{"foo"}) - Expect(err).To(HaveOccurred()) - }) - }) - Context("and a legacy format without ospid is provided", func() { - BeforeEach(func() { - viper.Instance().Set("certification_project_id", "62423-f26c346-6cc1dc7fae92") - DeferCleanup(viper.Instance().Set, "certification_project_id", "") - }) - It("should throw an error", func() { - err := validateCertificationProjectID(checkContainerCmd(mockRunPreflightReturnNil), []string{"foo"}) - Expect(err).To(HaveOccurred()) - }) - }) - }) + Expect(updated).To(Equal(expected)) + return + } + Expect(err).To(HaveOccurred()) + }, + Entry("the flag is set properly should not change value", "123456789", "123456789", true), + Entry("a valid ospid should strip ospid-", "ospid-123456789", "123456789", true), + Entry("legacy format with ospid", "ospid-62423-f26c346-6cc1dc7fae92", "", false), + Entry("legacy format without ospid", "62423-f26c346-6cc1dc7fae92", "", false), + ) - Context("when running the check container subcommand with a logger provided", func() { - Context("with all of the required parameters", func() { + When("running the check container subcommand with a logger provided", func() { + When("with all of the required parameters", func() { It("should reach the core logic, and execute the mocked RunPreflight", func() { - _, err := executeCommandWithLogger(checkContainerCmd(mockRunPreflightReturnNil), logr.Discard(), src) + _, err := executeCommandWithLogger(checkContainerCmd(mockRunPreflightReturnNil, viper.New()), logr.Discard(), src) Expect(err).ToNot(HaveOccurred()) }) }) - Context("with all of the required parameters with error mocked", func() { + When("with all of the required parameters with error mocked", func() { It("should reach the core logic, and execute the mocked RunPreflight and return error", func() { - _, err := executeCommandWithLogger(checkContainerCmd(mockRunPreflightReturnErr), logr.Discard(), src) + _, err := executeCommandWithLogger(checkContainerCmd(mockRunPreflightReturnErr, viper.New()), logr.Discard(), src) Expect(err).To(HaveOccurred()) }) }) }) - Context("when running the check container subcommand with a offline config provided", func() { - Context("with all of the required parameters", func() { + When("running the check container subcommand with a offline config provided", func() { + var v *viper.Viper + + BeforeEach(func() { + v = viper.New() + }) + When("with all of the required parameters", func() { BeforeEach(func() { tmpDir, err := os.MkdirTemp("", "preflight-submit-test-*") Expect(err).ToNot(HaveOccurred()) @@ -297,19 +283,16 @@ certification_project_id: mycertid` Expect(err).ToNot(HaveOccurred()) defer f2.Close() - viper.Instance().Set("artifacts", tmpDir) - DeferCleanup(viper.Instance().Set, "artifacts", artifacts.DefaultArtifactsDir) - - viper.Instance().Set("offline", true) - DeferCleanup(viper.Instance().Set, "offline", false) + v.Set("artifacts", tmpDir) + v.Set("offline", true) }) It("should reach core logic, and the additional offline logic", func() { - out, err := executeCommandWithLogger(checkContainerCmd(mockRunPreflightReturnNil), logr.Discard(), src) + out, err := executeCommandWithLogger(checkContainerCmd(mockRunPreflightReturnNil, v), logr.Discard(), src) Expect(err).ToNot(HaveOccurred()) Expect(out).ToNot(BeNil()) }) }) - Context("when an existing artifacts.tar already on disk", func() { + When("an existing artifacts.tar already on disk", func() { BeforeEach(func() { tmpDir, err := os.MkdirTemp("", "preflight-submit-test-*") Expect(err).ToNot(HaveOccurred()) @@ -332,41 +315,38 @@ certification_project_id: mycertid` Expect(err).ToNot(HaveOccurred()) defer f3.Close() - viper.Instance().Set("artifacts", tmpDir) - DeferCleanup(viper.Instance().Set, "artifacts", artifacts.DefaultArtifactsDir) - - viper.Instance().Set("offline", true) - DeferCleanup(viper.Instance().Set, "offline", false) + v.Set("artifacts", tmpDir) + v.Set("offline", true) }) It("should reach the additional offline logic, and remove existing tar file", func() { - out, err := executeCommandWithLogger(checkContainerCmd(mockRunPreflightReturnNil), logr.Discard(), src) + out, err := executeCommandWithLogger(checkContainerCmd(mockRunPreflightReturnNil, v), logr.Discard(), src) Expect(err).ToNot(HaveOccurred()) Expect(out).ToNot(BeNil()) }) }) }) - Context("when artifactsTar is called directly", func() { - Context("and the src does not exist", func() { + When("artifactsTar is called directly", func() { + When("the src does not exist", func() { It("should get an unable to tar files error", func() { err := artifactsTar(context.Background(), "", nil) Expect(err).To(HaveOccurred()) }) }) - Context("and a bad writer is passed", func() { + When("a bad writer is passed", func() { It("should get an error with trying to write the header", func() { err := artifactsTar(context.Background(), ".", errWriter(0)) Expect(err).To(HaveOccurred()) }) }) - Context("and a src has no permissions", func() { + When("a src has no permissions", func() { var tmpDir string var err error var buf bytes.Buffer JustBeforeEach(func() { tmpDir, err = os.MkdirTemp("", "preflight-submit-test-*") Expect(err).ToNot(HaveOccurred()) - err = os.Chmod(tmpDir, 0o00) + err = os.Chmod(tmpDir, 0o000) Expect(err).ToNot(HaveOccurred()) DeferCleanup(os.RemoveAll, tmpDir) }) @@ -375,7 +355,7 @@ certification_project_id: mycertid` Expect(err).To(HaveOccurred()) }) }) - Context("and src only contains another directory", func() { + When("and src only contains another directory", func() { var tmpDir string var err error var buf bytes.Buffer @@ -394,10 +374,10 @@ certification_project_id: mycertid` }) }) -func mockRunPreflightReturnNil(context.Context, func(ctx context.Context) (certification.Results, error), cli.CheckConfig, formatters.ResponseFormatter, lib.ResultWriter, lib.ResultSubmitter) error { +func mockRunPreflightReturnNil(context.Context, func(ctx context.Context) (certification.Results, error), cli.CheckConfig, formatters.ResponseFormatter, lib.ResultWriter, lib.ResultSubmitter, string) error { return nil } -func mockRunPreflightReturnErr(context.Context, func(ctx context.Context) (certification.Results, error), cli.CheckConfig, formatters.ResponseFormatter, lib.ResultWriter, lib.ResultSubmitter) error { +func mockRunPreflightReturnErr(context.Context, func(ctx context.Context) (certification.Results, error), cli.CheckConfig, formatters.ResponseFormatter, lib.ResultWriter, lib.ResultSubmitter, string) error { return errors.New("random error") } diff --git a/cmd/preflight/cmd/check_operator.go b/cmd/preflight/cmd/check_operator.go index 098a855a..7bd14a8a 100644 --- a/cmd/preflight/cmd/check_operator.go +++ b/cmd/preflight/cmd/check_operator.go @@ -11,28 +11,29 @@ import ( "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/formatters" "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/lib" "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/runtime" - "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/viper" "github.com/redhat-openshift-ecosystem/openshift-preflight/operator" "github.com/redhat-openshift-ecosystem/openshift-preflight/version" "github.com/go-logr/logr" "github.com/spf13/cobra" + "github.com/spf13/viper" ) -func checkOperatorCmd(runpreflight runPreflight) *cobra.Command { +func checkOperatorCmd(runpreflight runPreflight, viper *viper.Viper) *cobra.Command { checkOperatorCmd := &cobra.Command{ Use: "operator", Short: "Run checks for an Operator", Long: `This command will run the Certification checks for an Operator bundle image. `, - Args: checkOperatorPositionalArgs, + Args: func(cmd *cobra.Command, args []string) error { + return checkOperatorPositionalArgs(args, viper) + }, // this fmt.Sprintf is in place to keep spacing consistent with cobras two spaces that's used in: Usage, Flags, etc Example: fmt.Sprintf(" %s", "preflight check operator quay.io/repo-name/operator-bundle:version"), RunE: func(cmd *cobra.Command, args []string) error { - return checkOperatorRunE(cmd, args, runpreflight) + return checkOperatorRunE(cmd, args, runpreflight, viper) }, } - viper := viper.Instance() checkOperatorCmd.Flags().String("namespace", "", "The namespace to use when running OperatorSDK Scorecard. (env: PFLT_NAMESPACE)") _ = viper.BindPFlag("namespace", checkOperatorCmd.Flags().Lookup("namespace")) @@ -66,8 +67,8 @@ func ensureKubeconfigIsSet() error { // ensureIndexImageConfigIsSet ensures that the PFLT_INDEXIMAGE environment variable has // a value. -func ensureIndexImageConfigIsSet() error { - if catalogImage := viper.Instance().GetString("indexImage"); len(catalogImage) == 0 { +func ensureIndexImageConfigIsSet(viper *viper.Viper) error { + if catalogImage := viper.GetString("indexImage"); len(catalogImage) == 0 { return fmt.Errorf("environment variable PFLT_INDEXIMAGE could not be found") } @@ -75,7 +76,7 @@ func ensureIndexImageConfigIsSet() error { } // checkOperatorRunE executes checkOperator using the user args to inform the execution. -func checkOperatorRunE(cmd *cobra.Command, args []string, runpreflight runPreflight) error { +func checkOperatorRunE(cmd *cobra.Command, args []string, runpreflight runPreflight, viper *viper.Viper) error { ctx := cmd.Context() logger, err := logr.FromContext(ctx) if err != nil { @@ -86,7 +87,7 @@ func checkOperatorRunE(cmd *cobra.Command, args []string, runpreflight runPrefli operatorImage := args[0] // Render the Viper configuration as a runtime.Config - cfg, err := runtime.NewConfigFrom(*viper.Instance()) + cfg, err := runtime.NewConfigFrom(*viper) if err != nil { return fmt.Errorf("invalid configuration: %w", err) } @@ -128,10 +129,11 @@ func checkOperatorRunE(cmd *cobra.Command, args []string, runpreflight runPrefli formatter, &runtime.ResultWriterFile{}, &lib.NoopSubmitter{}, + viper.GetString("pyxis_env"), ) } -func checkOperatorPositionalArgs(cmd *cobra.Command, args []string) error { +func checkOperatorPositionalArgs(args []string, viper *viper.Viper) error { if len(args) != 1 { return fmt.Errorf("an operator bundle image positional argument is required") } @@ -140,7 +142,7 @@ func checkOperatorPositionalArgs(cmd *cobra.Command, args []string) error { return err } - if err := ensureIndexImageConfigIsSet(); err != nil { + if err := ensureIndexImageConfigIsSet(viper); err != nil { return err } diff --git a/cmd/preflight/cmd/check_operator_test.go b/cmd/preflight/cmd/check_operator_test.go index 241771de..397e129a 100644 --- a/cmd/preflight/cmd/check_operator_test.go +++ b/cmd/preflight/cmd/check_operator_test.go @@ -7,22 +7,21 @@ import ( "github.com/go-logr/logr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - - "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/viper" + "github.com/spf13/viper" ) var _ = Describe("Check Operator", func() { BeforeEach(createAndCleanupDirForArtifactsAndLogs) - Context("when running the check operator subcommand", func() { - Context("without the operator bundle image being provided", func() { + When("running the check operator subcommand", func() { + When("without the operator bundle image being provided", func() { It("should return an error", func() { - _, err := executeCommand(checkOperatorCmd(mockRunPreflightReturnNil)) + _, err := executeCommand(checkOperatorCmd(mockRunPreflightReturnNil, viper.New())) Expect(err).To(HaveOccurred()) }) }) - Context("without having set the KUBECONFIG environment variable", func() { + When("without having set the KUBECONFIG environment variable", func() { BeforeEach(func() { if val, isSet := os.LookupEnv("KUBECONFIG"); isSet { DeferCleanup(os.Setenv, "KUBECONFIG", val) @@ -30,13 +29,13 @@ var _ = Describe("Check Operator", func() { os.Unsetenv("KUBECONFIG") }) It("should return an error", func() { - out, err := executeCommand(checkOperatorCmd(mockRunPreflightReturnNil), "quay.io/example/image:mytag") + out, err := executeCommand(checkOperatorCmd(mockRunPreflightReturnNil, viper.New()), "quay.io/example/image:mytag") Expect(err).To(HaveOccurred()) Expect(out).To(ContainSubstring("KUBECONFIG could not")) }) }) - Context("without having set the PFLT_INDEXIMAGE environment variable", func() { + When("without having set the PFLT_INDEXIMAGE environment variable", func() { BeforeEach(func() { if val, isSet := os.LookupEnv("PFLT_INDEXIMAGE"); isSet { DeferCleanup(os.Setenv, "PFLT_INDEXIMAGE", val) @@ -50,16 +49,17 @@ var _ = Describe("Check Operator", func() { os.Setenv("KUBECONFIG", "foo") }) It("should return an error", func() { - out, err := executeCommand(checkOperatorCmd(mockRunPreflightReturnNil), "quay.io/example/image:mytag") + out, err := executeCommand(checkOperatorCmd(mockRunPreflightReturnNil, viper.New()), "quay.io/example/image:mytag") Expect(err).To(HaveOccurred()) Expect(out).To(ContainSubstring("PFLT_INDEXIMAGE could not")) }) }) - Context("With all of the required parameters", func() { + When("with all of the required parameters", func() { + var v *viper.Viper BeforeEach(func() { - DeferCleanup(viper.Instance().Set, "indexImage", viper.Instance().GetString("indexImage")) - viper.Instance().Set("indexImage", "foo") + v = viper.New() + v.Set("indexImage", "foo") if val, isSet := os.LookupEnv("KUBECONFIG"); isSet { DeferCleanup(os.Setenv, "KUBECONFIG", val) } else { @@ -78,21 +78,22 @@ var _ = Describe("Check Operator", func() { os.Setenv("KUBECONFIG", f1.Name()) }) It("should reach the core logic, and execute the mocked RunPreflight", func() { - out, err := executeCommandWithLogger(checkOperatorCmd(mockRunPreflightReturnNil), logr.Discard(), "quay.io/example/image:mytag") + out, err := executeCommandWithLogger(checkOperatorCmd(mockRunPreflightReturnNil, v), logr.Discard(), "quay.io/example/image:mytag") Expect(err).ToNot(HaveOccurred()) Expect(out).ToNot(BeNil()) }) It("should reach the core logic, and execute the mocked RunPreflight and return error", func() { - out, err := executeCommandWithLogger(checkOperatorCmd(mockRunPreflightReturnErr), logr.Discard(), "quay.io/example/image:mytag") + out, err := executeCommandWithLogger(checkOperatorCmd(mockRunPreflightReturnErr, v), logr.Discard(), "quay.io/example/image:mytag") Expect(err).To(HaveOccurred()) Expect(out).To(ContainSubstring("random error")) }) }) - Context("With an invalid KUBECONFIG file location", func() { + When("with an invalid KUBECONFIG file location", func() { + var v *viper.Viper BeforeEach(func() { - DeferCleanup(viper.Instance().Set, "indexImage", viper.Instance().GetString("indexImage")) - viper.Instance().Set("indexImage", "foo") + v = viper.New() + v.Set("indexImage", "foo") if val, isSet := os.LookupEnv("KUBECONFIG"); isSet { DeferCleanup(os.Setenv, "KUBECONFIG", val) } else { @@ -102,16 +103,17 @@ var _ = Describe("Check Operator", func() { os.Setenv("KUBECONFIG", "foo") }) It("should return a no such file error", func() { - out, err := executeCommandWithLogger(checkOperatorCmd(mockRunPreflightReturnNil), logr.Discard(), "quay.io/example/image:mytag") + out, err := executeCommandWithLogger(checkOperatorCmd(mockRunPreflightReturnNil, v), logr.Discard(), "quay.io/example/image:mytag") Expect(err).To(HaveOccurred()) Expect(out).To(ContainSubstring(": open foo: no such file or directory")) }) }) - Context("With a KUBECONFIG file location that is a directory", func() { + When("with a KUBECONFIG file location that is a directory", func() { + var v *viper.Viper BeforeEach(func() { - DeferCleanup(viper.Instance().Set, "indexImage", viper.Instance().GetString("indexImage")) - viper.Instance().Set("indexImage", "foo") + v = viper.New() + v.Set("indexImage", "foo") if val, isSet := os.LookupEnv("KUBECONFIG"); isSet { DeferCleanup(os.Setenv, "KUBECONFIG", val) } else { @@ -121,15 +123,15 @@ var _ = Describe("Check Operator", func() { os.Setenv("KUBECONFIG", ".") }) It("should return an is a directory error", func() { - out, err := executeCommandWithLogger(checkOperatorCmd(mockRunPreflightReturnNil), logr.Discard(), "quay.io/example/image:mytag") + out, err := executeCommandWithLogger(checkOperatorCmd(mockRunPreflightReturnNil, v), logr.Discard(), "quay.io/example/image:mytag") Expect(err).To(HaveOccurred()) Expect(out).To(ContainSubstring(": is a directory")) }) }) }) - Context("When checking for required environment variables", func() { - Context("specifically, KUBECONFIG", func() { + When("checking for required environment variables", func() { + When("specifically, KUBECONFIG", func() { BeforeEach(func() { if val, isSet := os.LookupEnv("KUBECONIFG"); isSet { DeferCleanup(os.Setenv, "KUBECONFIG", val) @@ -150,32 +152,36 @@ var _ = Describe("Check Operator", func() { }) }) - Context("specifically, PFLT_INDEXIMAGE", func() { + When("specifically, PFLT_INDEXIMAGE", func() { + var v *viper.Viper BeforeEach(func() { - DeferCleanup(viper.Instance().Set, "indexImage", viper.Instance().GetString("indexImage")) - viper.Instance().Set("indexImage", "foo") + v = viper.New() + v.Set("indexImage", "foo") }) It("should not encounter an error if the value is set", func() { - err := ensureIndexImageConfigIsSet() + err := ensureIndexImageConfigIsSet(v) Expect(err).ToNot(HaveOccurred()) }) It("should encounter an error if the value is not set", func() { - viper.Instance().Set("indexImage", "") - err := ensureIndexImageConfigIsSet() + v.Set("indexImage", "") + err := ensureIndexImageConfigIsSet(v) Expect(err).To(HaveOccurred()) }) }) }) - Context("When testing positional arg parsing", func() { + When("testing positional arg parsing", func() { + var v *viper.Viper + var posArgs []string + // failure cases are tested earlier in this file by running executeCommand. // This tests the success case using the standalone function in order // to prevent trying to run the entire RunE func in previous cases. - posArgs := []string{"firstparam"} BeforeEach(func() { - DeferCleanup(viper.Instance().Set, "indexImage", viper.Instance().GetString("indexImage")) - viper.Instance().Set("indexImage", "foo") + v = viper.New() + posArgs = append(posArgs, "first") + v.Set("indexImage", "foo") if val, isSet := os.LookupEnv("KUBECONIFG"); isSet { DeferCleanup(os.Setenv, "KUBECONFIG", val) } else { @@ -185,7 +191,7 @@ var _ = Describe("Check Operator", func() { }) It("should succeed when all positional arg constraints and environment constraints are correct", func() { - err := checkOperatorPositionalArgs(checkOperatorCmd(mockRunPreflightReturnNil), posArgs) + err := checkOperatorPositionalArgs(posArgs, v) Expect(err).ToNot(HaveOccurred()) }) }) diff --git a/cmd/preflight/cmd/check_test.go b/cmd/preflight/cmd/check_test.go index a735515b..836ec680 100644 --- a/cmd/preflight/cmd/check_test.go +++ b/cmd/preflight/cmd/check_test.go @@ -4,7 +4,6 @@ import ( "os" "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/lib" - "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/viper" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -19,28 +18,17 @@ var _ = Describe("cmd package check command", func() { imageID = "my-image-id" resultsID = "my-results-id" ) - BeforeEach(func() { - viper.Instance().SetEnvPrefix("pflt") - viper.Instance().AutomaticEnv() - }) - AfterEach(func() { - os.Unsetenv("PFLT_PYXIS_ENV") - os.Unsetenv("PFLT_PYXIS_HOST") - }) Context("Regular Connect URL", func() { It("should return a URL with just a project ID", func() { expected := "https://connect.redhat.com/component/view/this-is-my-project-id" - actual := lib.BuildConnectURL(projectID) + actual := lib.BuildConnectURL(projectID, "prod") Expect(expected).To(Equal(actual)) }) }) Context("QA Connect URL", func() { - BeforeEach(func() { - os.Setenv("PFLT_PYXIS_ENV", "qa") - }) It("should return a URL for QA", func() { expected := "https://connect.qa.redhat.com/component/view/this-is-my-project-id" - actual := lib.BuildConnectURL(projectID) + actual := lib.BuildConnectURL(projectID, "qa") Expect(expected).To(Equal(actual)) }) }) @@ -50,7 +38,7 @@ var _ = Describe("cmd package check command", func() { }) It("should return a URL for UAT", func() { expected := "https://connect.uat.redhat.com/component/view/this-is-my-project-id/certification/test-results/my-results-id" - actual := lib.BuildTestResultsURL(projectID, resultsID) + actual := lib.BuildTestResultsURL(projectID, resultsID, "uat") Expect(expected).To(Equal(actual)) }) }) @@ -60,7 +48,7 @@ var _ = Describe("cmd package check command", func() { }) It("should return a URL for QA", func() { expected := "https://connect.qa.redhat.com/component/view/this-is-my-project-id/images" - actual := lib.BuildImagesURL(projectID) + actual := lib.BuildImagesURL(projectID, "qa") Expect(expected).To(Equal(actual)) }) }) @@ -70,17 +58,17 @@ var _ = Describe("cmd package check command", func() { }) It("should return a Prod Images URL", func() { expected := "https://connect.redhat.com/component/view/this-is-my-project-id/images" - actual := lib.BuildImagesURL(projectID) + actual := lib.BuildImagesURL(projectID, "prod") Expect(expected).To(Equal(actual)) }) It("should return a Prod Test Results URL", func() { expected := "https://connect.redhat.com/component/view/this-is-my-project-id/certification/test-results/my-results-id" - actual := lib.BuildTestResultsURL(projectID, resultsID) + actual := lib.BuildTestResultsURL(projectID, resultsID, "prod") Expect(expected).To(Equal(actual)) }) It("should return a Prod Vulnerabilities URL", func() { expected := "https://connect.redhat.com/component/view/this-is-my-project-id/security/vulnerabilities/my-image-id" - actual := lib.BuildVulnerabilitiesURL(projectID, imageID) + actual := lib.BuildVulnerabilitiesURL(projectID, imageID, "prod") Expect(expected).To(Equal(actual)) }) }) diff --git a/cmd/preflight/cmd/root.go b/cmd/preflight/cmd/root.go index 6559bcab..c0817640 100644 --- a/cmd/preflight/cmd/root.go +++ b/cmd/preflight/cmd/root.go @@ -8,43 +8,40 @@ import ( "path/filepath" "github.com/redhat-openshift-ecosystem/openshift-preflight/artifacts" - "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/viper" "github.com/redhat-openshift-ecosystem/openshift-preflight/version" "github.com/bombsimon/logrusr/v4" "github.com/go-logr/logr" "github.com/sirupsen/logrus" "github.com/spf13/cobra" - spfviper "github.com/spf13/viper" + "github.com/spf13/viper" ctrl "sigs.k8s.io/controller-runtime" ) var configFileUsed bool -func init() { - cobra.OnInitialize(func() { initConfig(viper.Instance()) }) -} - -func rootCmd() *cobra.Command { +func rootCmd(viper *viper.Viper) *cobra.Command { rootCmd := &cobra.Command{ - Use: "preflight", - Short: "Preflight Red Hat certification prep tool.", - Long: "A utility that allows you to pre-test your bundles, operators, and container before submitting for Red Hat Certification.", - Version: version.Version.String(), - Args: cobra.MinimumNArgs(1), - PersistentPreRun: preRunConfig, + Use: "preflight", + Short: "Preflight Red Hat certification prep tool.", + Long: "A utility that allows you to pre-test your bundles, operators, and container before submitting for Red Hat Certification.", + Version: version.Version.String(), + Args: cobra.MinimumNArgs(1), + PersistentPreRun: func(cmd *cobra.Command, args []string) { + preRunConfig(cmd, viper) + }, } - viper := viper.Instance() rootCmd.PersistentFlags().String("config", "", "A preflight config file. The default is config.yaml (env: PFLT_CONFIG)") _ = viper.BindPFlag("config", rootCmd.PersistentFlags().Lookup("config")) + rootCmd.PersistentFlags().String("logfile", "", "Where the execution logfile will be written. (env: PFLT_LOGFILE)") _ = viper.BindPFlag("logfile", rootCmd.PersistentFlags().Lookup("logfile")) rootCmd.PersistentFlags().String("loglevel", "", "The verbosity of the preflight tool itself. Ex. warn, debug, trace, info, error. (env: PFLT_LOGLEVEL)") _ = viper.BindPFlag("loglevel", rootCmd.PersistentFlags().Lookup("loglevel")) - rootCmd.AddCommand(checkCmd()) + rootCmd.AddCommand(checkCmd(viper)) rootCmd.AddCommand(listChecksCmd()) rootCmd.AddCommand(runtimeAssetsCmd()) rootCmd.AddCommand(supportCmd()) @@ -52,47 +49,47 @@ func rootCmd() *cobra.Command { return rootCmd } -func Execute() error { - return rootCmd().ExecuteContext(context.Background()) +func Execute(ctx context.Context, viper *viper.Viper) error { + initConfig(viper) + return rootCmd(viper).ExecuteContext(ctx) } -func initConfig(viper *spfviper.Viper) { +func initConfig(v *viper.Viper) { // set up ENV var support - viper.SetEnvPrefix("pflt") - viper.AutomaticEnv() + v.SetEnvPrefix("pflt") + v.AutomaticEnv() // set up optional config file support - viper.SetConfigName("config") - viper.SetConfigType("yaml") - viper.AddConfigPath(".") + v.SetConfigName("config") + v.SetConfigType("yaml") + v.AddConfigPath(".") configFileUsed = true - if viper.GetString("config") != "" { - viper.SetConfigFile(viper.GetString("config")) + if v.GetString("config") != "" { + v.SetConfigFile(v.GetString("config")) } - if err := viper.ReadInConfig(); err != nil { - if _, ok := err.(spfviper.ConfigFileNotFoundError); ok { + if err := v.ReadInConfig(); err != nil { + if _, ok := err.(viper.ConfigFileNotFoundError); ok { configFileUsed = false } } // Set up logging config defaults - viper.SetDefault("logfile", DefaultLogFile) - viper.SetDefault("loglevel", DefaultLogLevel) - viper.SetDefault("artifacts", artifacts.DefaultArtifactsDir) + v.SetDefault("logfile", DefaultLogFile) + v.SetDefault("loglevel", DefaultLogLevel) + v.SetDefault("artifacts", artifacts.DefaultArtifactsDir) // Set up cluster defaults - viper.SetDefault("namespace", DefaultNamespace) - viper.SetDefault("serviceaccount", DefaultServiceAccount) + v.SetDefault("namespace", DefaultNamespace) + v.SetDefault("serviceaccount", DefaultServiceAccount) // Set up scorecard wait time default - viper.SetDefault("scorecard_wait_time", DefaultScorecardWaitTime) + v.SetDefault("scorecard_wait_time", DefaultScorecardWaitTime) } // preRunConfig is used by cobra.PreRun in all non-root commands to load all necessary configurations -func preRunConfig(cmd *cobra.Command, args []string) { - viper := viper.Instance() +func preRunConfig(cmd *cobra.Command, viper *viper.Viper) { l := logrus.New() l.SetFormatter(&logrus.TextFormatter{DisableColors: true}) diff --git a/cmd/preflight/cmd/root_test.go b/cmd/preflight/cmd/root_test.go index 9c4222e7..9bab90a6 100644 --- a/cmd/preflight/cmd/root_test.go +++ b/cmd/preflight/cmd/root_test.go @@ -8,14 +8,13 @@ import ( "github.com/redhat-openshift-ecosystem/openshift-preflight/artifacts" "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/cli" - "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/viper" "github.com/go-logr/logr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/spf13/afero" "github.com/spf13/cobra" - spfviper "github.com/spf13/viper" + "github.com/spf13/viper" ) // executeCommand is used for cobra command testing. It is effectively what's seen here: @@ -53,7 +52,7 @@ var _ = Describe("cmd package utility functions", func() { Describe("Get the root command", func() { Context("when calling the root command function", func() { It("should return a root command", func() { - cmd := rootCmd() + cmd := rootCmd(viper.New()) Expect(cmd).ToNot(BeNil()) Expect(cmd.Commands()).ToNot(BeEmpty()) }) @@ -73,11 +72,11 @@ var _ = Describe("cmd package utility functions", func() { ) Describe("Initialize Viper configuration", func() { - var testViper *spfviper.Viper + var testViper *viper.Viper BeforeEach(func() { - testViper = spfviper.New() + testViper = viper.New() }) - Context("when initConfig is called", func() { + When("initConfig() is called", func() { Context("and no envvars are set", func() { It("should have defaults set correctly", func() { initConfig(testViper) @@ -105,7 +104,9 @@ var _ = Describe("cmd package utility functions", func() { }) }) When("a config file is present", func() { + var testViper *viper.Viper BeforeEach(func() { + testViper = viper.New() fs := afero.NewMemMapFs() testViper.SetFs(fs) configFile := `namespace: configspace @@ -119,9 +120,8 @@ loglevel: configloglevel` cwd, err := os.Getwd() Expect(err).ToNot(HaveOccurred()) Expect(afero.WriteFile(fs, filepath.Join(cwd, "config.yaml"), bytes.NewBufferString(configFile).Bytes(), 0o644)).To(Succeed()) - DeferCleanup(testViper.SetFs, afero.NewOsFs()) }) - It("should read all of the config", func() { + It("should read config from a file", func() { initConfig(testViper) Expect(testViper.GetString("namespace")).To(Equal("configspace")) Expect(testViper.GetString("artifacts")).To(Equal("configartifacts")) @@ -134,22 +134,30 @@ loglevel: configloglevel` Describe("Pre-run configuration", func() { var cmd *cobra.Command + var testViper *viper.Viper + var tmpDir string BeforeEach(func() { + testViper = viper.New() cmd = &cobra.Command{ - PersistentPreRun: preRunConfig, - Run: func(cmd *cobra.Command, args []string) {}, + PersistentPreRun: func(cmd *cobra.Command, args []string) { + preRunConfig(cmd, testViper) + }, + Run: func(cmd *cobra.Command, args []string) {}, } + var err error + tmpDir, err = os.MkdirTemp("", "prerun-config-*") + Expect(err).ToNot(HaveOccurred()) + DeferCleanup(os.RemoveAll, tmpDir) }) Context("configuring a Cobra Command", func() { - var tmpDir string BeforeEach(func() { var err error tmpDir, err = os.MkdirTemp("", "prerun-config-*") Expect(err).ToNot(HaveOccurred()) DeferCleanup(os.RemoveAll, tmpDir) - viper.Instance().Set("logfile", filepath.Join(tmpDir, "foo.log")) - DeferCleanup(viper.Instance().Set, "logfile", "preflight.log") + testViper.Set("logfile", filepath.Join(tmpDir, "foo.log")) + DeferCleanup(testViper.Set, "logfile", "preflight.log") }) It("should create the logfile", func() { Expect(cmd.ExecuteContext(context.TODO())).To(Succeed()) @@ -158,7 +166,6 @@ loglevel: configloglevel` }) }) Context("with the offline flag", func() { - var tmpDir string BeforeEach(func() { var err error tmpDir, err = os.MkdirTemp("", "prerun-artifacts-*") @@ -169,14 +176,9 @@ loglevel: configloglevel` Expect(err).ToNot(HaveOccurred()) DeferCleanup(os.RemoveAll, tmpLogDir) - viper.Instance().Set("artifacts", tmpDir) - DeferCleanup(viper.Instance().Set, "artifacts", artifacts.DefaultArtifactsDir) - - viper.Instance().Set("logfile", filepath.Join(tmpLogDir, "preflight.log")) - DeferCleanup(viper.Instance().Set, "logfile", DefaultLogFile) - - viper.Instance().Set("offline", true) - DeferCleanup(viper.Instance().Set, "offline", false) + testViper.Set("artifacts", tmpDir) + testViper.Set("logfile", filepath.Join(tmpLogDir, "preflight.log")) + testViper.Set("offline", true) }) It("should create the logfile in the artifacts directory", func() { Expect(cmd.ExecuteContext(context.TODO())).To(Succeed()) diff --git a/cmd/preflight/main.go b/cmd/preflight/main.go index 5468d282..2956001f 100644 --- a/cmd/preflight/main.go +++ b/cmd/preflight/main.go @@ -1,13 +1,16 @@ package main import ( + "context" "log" "github.com/redhat-openshift-ecosystem/openshift-preflight/cmd/preflight/cmd" + + "github.com/spf13/viper" ) func main() { - if err := cmd.Execute(); err != nil { + if err := cmd.Execute(context.Background(), viper.New()); err != nil { log.Fatal(err) } } diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 955fa3d4..ad1fe63d 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -31,6 +31,7 @@ func RunPreflight( formatter formatters.ResponseFormatter, rw lib.ResultWriter, rs lib.ResultSubmitter, + pyxisEnv string, ) error { logger := logr.FromContextOrDiscard(ctx) @@ -77,7 +78,7 @@ func RunPreflight( } if cfg.SubmitResults { - if err := rs.Submit(ctx); err != nil { + if err := rs.Submit(ctx, pyxisEnv); err != nil { return err } } diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 12c70bbf..64080330 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -28,7 +28,7 @@ var _ = Describe("CLI Library function", func() { When("invoking preflight using the CLI library", func() { Context("without passing in an artifact writer ", func() { It("should throw an error", func() { - err := RunPreflight(context.TODO(), func(ctx context.Context) (certification.Results, error) { return certification.Results{}, nil }, CheckConfig{}, nil, nil, nil) + err := RunPreflight(context.TODO(), func(ctx context.Context) (certification.Results, error) { return certification.Results{}, nil }, CheckConfig{}, nil, nil, nil, "prod") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("no artifact writer")) }) @@ -54,7 +54,7 @@ var _ = Describe("CLI Library function", func() { It("Should return an error if unable to successfully check execution encounters an error", func() { err := RunPreflight(testcontext, func(ctx context.Context) (certification.Results, error) { return certification.Results{}, errors.New("some error") - }, CheckConfig{}, testFormatter, &runtime.ResultWriterFile{}, nil) + }, CheckConfig{}, testFormatter, &runtime.ResultWriterFile{}, nil, "prod") Expect(err).To(HaveOccurred()) }) @@ -65,7 +65,7 @@ var _ = Describe("CLI Library function", func() { }) Expect(err).ToNot(HaveOccurred()) - err = RunPreflight(testcontext, func(ctx context.Context) (certification.Results, error) { return certification.Results{}, nil }, CheckConfig{}, testFormatter, &runtime.ResultWriterFile{}, nil) + err = RunPreflight(testcontext, func(ctx context.Context) (certification.Results, error) { return certification.Results{}, nil }, CheckConfig{}, testFormatter, &runtime.ResultWriterFile{}, nil, "prod") Expect(err).To(HaveOccurred()) }) @@ -93,7 +93,7 @@ var _ = Describe("CLI Library function", func() { Failed: []certification.Result{}, Errors: []certification.Result{}, }, nil - }, c, testFormatter, &runtime.ResultWriterFile{}, nil) + }, c, testFormatter, &runtime.ResultWriterFile{}, nil, "prod") Expect(err).ToNot(HaveOccurred()) expectedJUnitResultFile := filepath.Join(artifactWriter.Path(), "results-junit.xml") Expect(expectedJUnitResultFile).To(BeAnExistingFile()) @@ -128,7 +128,7 @@ var _ = Describe("CLI Library function", func() { Failed: []certification.Result{}, Errors: []certification.Result{}, }, nil - }, c, testFormatter, &runtime.ResultWriterFile{}, testSubmitter) + }, c, testFormatter, &runtime.ResultWriterFile{}, testSubmitter, "prod") Expect(err).ToNot(HaveOccurred()) contents, err := io.ReadAll(buf) @@ -161,7 +161,7 @@ var _ = Describe("CLI Library function", func() { Failed: []certification.Result{}, Errors: []certification.Result{}, }, nil - }, c, testFormatter, &runtime.ResultWriterFile{}, &badResultSubmitter{submissionError}) + }, c, testFormatter, &runtime.ResultWriterFile{}, &badResultSubmitter{submissionError}, "prod") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring(submissionError)) }) diff --git a/internal/cli/fakes_test.go b/internal/cli/fakes_test.go index bd4646b7..8b311690 100644 --- a/internal/cli/fakes_test.go +++ b/internal/cli/fakes_test.go @@ -10,6 +10,6 @@ type badResultSubmitter struct { errmsg string } -func (brs *badResultSubmitter) Submit(ctx context.Context) error { +func (brs *badResultSubmitter) Submit(ctx context.Context, _ string) error { return errors.New(brs.errmsg) } diff --git a/internal/lib/types.go b/internal/lib/types.go index 2481ca6c..2c9914de 100644 --- a/internal/lib/types.go +++ b/internal/lib/types.go @@ -16,7 +16,6 @@ import ( "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/log" "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/policy" "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/pyxis" - "github.com/redhat-openshift-ecosystem/openshift-preflight/internal/viper" "github.com/go-logr/logr" ) @@ -29,7 +28,7 @@ type ResultWriter interface { // ResultSubmitter defines methods associated with submitting results to Red HAt. type ResultSubmitter interface { - Submit(context.Context) error + Submit(context.Context, string) error } // PyxisClient defines pyxis API interactions that are relevant to check executions in cmd. @@ -64,7 +63,7 @@ type ContainerCertificationSubmitter struct { PreflightLogFile string } -func (s *ContainerCertificationSubmitter) Submit(ctx context.Context) error { +func (s *ContainerCertificationSubmitter) Submit(ctx context.Context, pyxisEnv string) error { logger := logr.FromContextOrDiscard(ctx) logger.Info("preparing results that will be submitted to Red Hat") @@ -193,9 +192,9 @@ func (s *ContainerCertificationSubmitter) Submit(ctx context.Context) error { logger.Info("Test results have been submitted to Red Hat.") logger.Info("These results will be reviewed by Red Hat for final certification.") logger.Info(fmt.Sprintf("The container's image id is: %s.", certResults.CertImage.ID)) - logger.Info(fmt.Sprintf("Please check %s to view test results.", BuildTestResultsURL(s.CertificationProjectID, certResults.TestResults.ID))) - logger.Info(fmt.Sprintf("Please check %s to view security vulnerabilities.", BuildVulnerabilitiesURL(s.CertificationProjectID, certResults.CertImage.ID))) - logger.Info(fmt.Sprintf("Please check %s to monitor the progress.", BuildImagesURL(s.CertificationProjectID))) + logger.Info(fmt.Sprintf("Please check %s to view test results.", BuildTestResultsURL(s.CertificationProjectID, certResults.TestResults.ID, pyxisEnv))) + logger.Info(fmt.Sprintf("Please check %s to view security vulnerabilities.", BuildVulnerabilitiesURL(s.CertificationProjectID, certResults.CertImage.ID, pyxisEnv))) + logger.Info(fmt.Sprintf("Please check %s to monitor the progress.", BuildImagesURL(s.CertificationProjectID, pyxisEnv))) return nil } @@ -217,7 +216,7 @@ func NewNoopSubmitter(emitLog bool, log *logr.Logger) *NoopSubmitter { var _ ResultSubmitter = &NoopSubmitter{} -func (s *NoopSubmitter) Submit(ctx context.Context) error { +func (s *NoopSubmitter) Submit(ctx context.Context, _ string) error { if s.emitLog { msg := "Results are not being sent for submission." if s.reason != "" { @@ -238,25 +237,24 @@ func (s *NoopSubmitter) SetReason(reason string) { s.reason = reason } -func BuildConnectURL(projectID string) string { +func BuildConnectURL(projectID, pyxisEnv string) string { connectURL := fmt.Sprintf("https://connect.redhat.com/component/view/%s", projectID) - pyxisEnv := viper.Instance().GetString("pyxis_env") if len(pyxisEnv) > 0 && pyxisEnv != "prod" { - connectURL = fmt.Sprintf("https://connect.%s.redhat.com/component/view/%s", viper.Instance().GetString("pyxis_env"), projectID) + connectURL = fmt.Sprintf("https://connect.%s.redhat.com/component/view/%s", pyxisEnv, projectID) } return connectURL } -func BuildImagesURL(projectID string) string { - return fmt.Sprintf("%s/images", BuildConnectURL(projectID)) +func BuildImagesURL(projectID, pyxisEnv string) string { + return fmt.Sprintf("%s/images", BuildConnectURL(projectID, pyxisEnv)) } -func BuildTestResultsURL(projectID string, testResultsID string) string { - return fmt.Sprintf("%s/certification/test-results/%s", BuildConnectURL(projectID), testResultsID) +func BuildTestResultsURL(projectID, testResultsID, pyxisEnv string) string { + return fmt.Sprintf("%s/certification/test-results/%s", BuildConnectURL(projectID, pyxisEnv), testResultsID) } -func BuildVulnerabilitiesURL(projectID string, imageID string) string { - return fmt.Sprintf("%s/security/vulnerabilities/%s", BuildConnectURL(projectID), imageID) +func BuildVulnerabilitiesURL(projectID, imageID, pyxisEnv string) string { + return fmt.Sprintf("%s/security/vulnerabilities/%s", BuildConnectURL(projectID, pyxisEnv), imageID) } diff --git a/internal/lib/types_test.go b/internal/lib/types_test.go index 46de4fa2..797aff0b 100644 --- a/internal/lib/types_test.go +++ b/internal/lib/types_test.go @@ -153,13 +153,13 @@ var _ = Describe("The NoopSubmitter", func() { It("should include the reason in the emitted log if specified", func() { testReason := "test reason" noop.SetReason(testReason) - err := noop.Submit(context.TODO()) + err := noop.Submit(context.TODO(), "prod") Expect(err).ToNot(HaveOccurred()) Expect(bf.String()).To(ContainSubstring(testReason)) }) It("should emit logs when calling submit", func() { - err := noop.Submit(context.TODO()) + err := noop.Submit(context.TODO(), "prod") Expect(err).ToNot(HaveOccurred()) Expect(bf.String()).To(ContainSubstring("Results are not being sent for submission.")) }) @@ -168,7 +168,7 @@ var _ = Describe("The NoopSubmitter", func() { Context("and disabling log emitting", func() { It("should not emit logs when calling submit", func() { noop.SetEmitLog(false) - err := noop.Submit(context.TODO()) + err := noop.Submit(context.TODO(), "prod") Expect(err).ToNot(HaveOccurred()) Expect(bf.String()).To(BeEmpty()) }) @@ -247,7 +247,7 @@ var _ = Describe("Container Certification Submitter", func() { fakePC.getProjectsFunc = gpFuncReturnError }) It("should throw an error", func() { - err := sbmt.Submit(testcontext) + err := sbmt.Submit(testcontext, "prod") Expect(err).To(HaveOccurred()) }) }) @@ -257,7 +257,7 @@ var _ = Describe("Container Certification Submitter", func() { err := os.Remove(dockerConfigPath) Expect(err).ToNot(HaveOccurred()) - err = sbmt.Submit(testcontext) + err = sbmt.Submit(testcontext, "prod") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring(dockerconfigFilename)) }) @@ -273,7 +273,7 @@ var _ = Describe("Container Certification Submitter", func() { err := os.Remove(dockerConfigPath) Expect(err).ToNot(HaveOccurred()) - err = sbmt.Submit(testcontext) + err = sbmt.Submit(testcontext, "prod") Expect(err).ToNot(HaveOccurred()) }) }) @@ -284,7 +284,7 @@ var _ = Describe("Container Certification Submitter", func() { fakePC.getProjectsFunc = gpFuncReturnHostedRegistry }) It("should not throw an error", func() { - err := sbmt.Submit(testcontext) + err := sbmt.Submit(testcontext, "prod") Expect(err).ToNot(HaveOccurred()) }) }) @@ -294,7 +294,7 @@ var _ = Describe("Container Certification Submitter", func() { err := os.Remove(path.Join(aw.Path(), check.DefaultCertImageFilename)) Expect(err).ToNot(HaveOccurred()) - err = sbmt.Submit(testcontext) + err = sbmt.Submit(testcontext, "prod") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring(check.DefaultCertImageFilename)) }) @@ -305,7 +305,7 @@ var _ = Describe("Container Certification Submitter", func() { err := os.Remove(path.Join(aw.Path(), check.DefaultTestResultsFilename)) Expect(err).ToNot(HaveOccurred()) - err = sbmt.Submit(testcontext) + err = sbmt.Submit(testcontext, "prod") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring(check.DefaultTestResultsFilename)) }) @@ -316,7 +316,7 @@ var _ = Describe("Container Certification Submitter", func() { err := os.Remove(path.Join(aw.Path(), check.DefaultRPMManifestFilename)) Expect(err).ToNot(HaveOccurred()) - err = sbmt.Submit(testcontext) + err = sbmt.Submit(testcontext, "prod") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring(check.DefaultRPMManifestFilename)) }) @@ -331,7 +331,7 @@ var _ = Describe("Container Certification Submitter", func() { err := os.Remove(path.Join(aw.Path(), check.DefaultRPMManifestFilename)) Expect(err).ToNot(HaveOccurred()) - err = sbmt.Submit(testcontext) + err = sbmt.Submit(testcontext, "prod") Expect(err).ToNot(HaveOccurred()) }) }) @@ -341,7 +341,7 @@ var _ = Describe("Container Certification Submitter", func() { err := os.Remove(preflightLogPath) Expect(err).ToNot(HaveOccurred()) - err = sbmt.Submit(testcontext) + err = sbmt.Submit(testcontext, "prod") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring(preflightLogFilename)) }) @@ -354,7 +354,7 @@ var _ = Describe("Container Certification Submitter", func() { }) It("should throw an error", func() { - err := sbmt.Submit(testcontext) + err := sbmt.Submit(testcontext, "prod") Expect(err).To(HaveOccurred()) }) }) @@ -365,7 +365,7 @@ var _ = Describe("Container Certification Submitter", func() { }) It("should throw an error", func() { - err := sbmt.Submit(testcontext) + err := sbmt.Submit(testcontext, "prod") Expect(err).To(HaveOccurred()) }) }) @@ -378,7 +378,7 @@ var _ = Describe("Container Certification Submitter", func() { }) It("should throw an error finalizing the submission", func() { - err := sbmt.Submit(testcontext) + err := sbmt.Submit(testcontext, "prod") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("unable to finalize data")) }) @@ -390,7 +390,7 @@ var _ = Describe("Container Certification Submitter", func() { fakePC.getProjectsFunc = gpFuncReturnScratchException }) It("should not throw an error", func() { - err := sbmt.Submit(testcontext) + err := sbmt.Submit(testcontext, "prod") Expect(err).ToNot(HaveOccurred()) }) }) diff --git a/internal/viper/viper.go b/internal/viper/viper.go deleted file mode 100644 index 9815ab88..00000000 --- a/internal/viper/viper.go +++ /dev/null @@ -1,29 +0,0 @@ -// Package viper provides a package-specific instance of Viper to avoid -// the use of Viper's global instance, which can cause conflicts -package viper - -import ( - "sync" - - spfviper "github.com/spf13/viper" -) - -var ( - instance *spfviper.Viper - mu = sync.Mutex{} -) - -// Instance provides the instance of Viper, or lazy-loads a new one -// if one has not been defined. -func Instance() *spfviper.Viper { - if instance != nil { - return instance - } - - mu.Lock() - defer mu.Unlock() - if instance == nil { - instance = spfviper.New() - } - return instance -} diff --git a/internal/viper/viper_suite_test.go b/internal/viper/viper_suite_test.go deleted file mode 100644 index b409fd9d..00000000 --- a/internal/viper/viper_suite_test.go +++ /dev/null @@ -1,13 +0,0 @@ -package viper - -import ( - "testing" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -func TestViper(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "Package-Local Viper") -} diff --git a/internal/viper/viper_test.go b/internal/viper/viper_test.go deleted file mode 100644 index 148d1a17..00000000 --- a/internal/viper/viper_test.go +++ /dev/null @@ -1,28 +0,0 @@ -package viper - -import ( - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -var _ = Describe("Viper tests", func() { - Context("Lazy Loading Viper", func() { - When("the viper instance hasn't been initialized", func() { - instance = nil - It("should be initialized when calling for it", func() { - _ = Instance() // we don't care about this return value for this test. - Expect(instance).ToNot(BeNil()) - }) - }) - }) - - Context("Getting the project-specific Viper instance", func() { - When("Requesting the viper instance for the project", func() { - It("Should return a non-empty viper instance", func() { - packageV := Instance() - packageV.Set("foo", "bar") - Expect(Instance().Get("foo")).To(Equal("bar")) - }) - }) - }) -})