incorporating runPreflight func call into check_operator cmd to allow…

… for better testability

Signed-off-by: Adam D. Cornett <adc@redhat.com>

cmd/preflight/cmd/check.go

@@ -22,7 +22,7 @@ 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())
+	checkCmd.AddCommand(checkOperatorCmd(cli.RunPreflight))
 	checkCmd.AddCommand(checkContainerCmd(cli.RunPreflight))
 
 	return checkCmd

cmd/preflight/cmd/check_container_test.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"bytes"
 	"context"
+	"errors"
 	"os"
 	"path/filepath"
 
@@ -160,3 +161,7 @@ certification_project_id: mycertid`
 func mockRunPreflight(context.Context, func(ctx context.Context) (certification.Results, error), cli.CheckConfig, formatters.ResponseFormatter, lib.ResultWriter, lib.ResultSubmitter) error {
 	return nil
 }
+
+func mockRunPreflightReturnErr(context.Context, func(ctx context.Context) (certification.Results, error), cli.CheckConfig, formatters.ResponseFormatter, lib.ResultWriter, lib.ResultSubmitter) error {
+	return errors.New("random error")
+}

cmd/preflight/cmd/check_operator.go

@@ -19,15 +19,17 @@ import (
 	"github.com/spf13/viper"
 )
 
-func checkOperatorCmd() *cobra.Command {
+func checkOperatorCmd(runpreflight runPreflight) *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,
 		// 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:    checkOperatorRunE,
+		RunE: func(cmd *cobra.Command, args []string) error {
+			return checkOperatorRunE(cmd, args, runpreflight)
+		},
 	}
 	checkOperatorCmd.Flags().String("namespace", "", "The namespace to use when running OperatorSDK Scorecard. (env: PFLT_NAMESPACE)")
 	_ = viper.BindPFlag("namespace", checkOperatorCmd.Flags().Lookup("namespace"))
@@ -71,7 +73,7 @@ func ensureIndexImageConfigIsSet() error {
 }
 
 // checkOperatorRunE executes checkOperator using the user args to inform the execution.
-func checkOperatorRunE(cmd *cobra.Command, args []string) error {
+func checkOperatorRunE(cmd *cobra.Command, args []string, runpreflight runPreflight) error {
 	ctx := cmd.Context()
 	logger, err := logr.FromContext(ctx)
 	if err != nil {
@@ -114,7 +116,7 @@ func checkOperatorRunE(cmd *cobra.Command, args []string) error {
 	checkoperator := operator.NewCheck(operatorImage, cfg.IndexImage, kubeconfig, opts...)
 
 	cmd.SilenceUsage = true
-	return cli.RunPreflight(
+	return runpreflight(
 		ctx,
 		checkoperator.Run,
 		cli.CheckConfig{

cmd/preflight/cmd/check_operator_test.go

@@ -2,18 +2,21 @@ package cmd
 
 import (
 	"os"
+	"path/filepath"
 
+	"github.com/go-logr/logr"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/spf13/viper"
 )
 
 var _ = Describe("Check Operator", func() {
+	BeforeEach(createAndCleanupDirForArtifactsAndLogs)
+
 	Context("when running the check operator subcommand", func() {
-		BeforeEach(createAndCleanupDirForArtifactsAndLogs)
 		Context("without the operator bundle image being provided", func() {
 			It("should return an error", func() {
-				_, err := executeCommand(checkOperatorCmd())
+				_, err := executeCommand(checkOperatorCmd(mockRunPreflight))
 				Expect(err).To(HaveOccurred())
 			})
 		})
@@ -26,7 +29,7 @@ var _ = Describe("Check Operator", func() {
 				os.Unsetenv("KUBECONFIG")
 			})
 			It("should return an error", func() {
-				out, err := executeCommand(checkOperatorCmd(), "quay.io/example/image:mytag")
+				out, err := executeCommand(checkOperatorCmd(mockRunPreflight), "quay.io/example/image:mytag")
 				Expect(err).To(HaveOccurred())
 				Expect(out).To(ContainSubstring("KUBECONFIG could not"))
 			})
@@ -46,7 +49,7 @@ var _ = Describe("Check Operator", func() {
 				os.Setenv("KUBECONFIG", "foo")
 			})
 			It("should return an error", func() {
-				out, err := executeCommand(checkOperatorCmd(), "quay.io/example/image:mytag")
+				out, err := executeCommand(checkOperatorCmd(mockRunPreflight), "quay.io/example/image:mytag")
 				Expect(err).To(HaveOccurred())
 				Expect(out).To(ContainSubstring("PFLT_INDEXIMAGE could not"))
 			})
@@ -61,11 +64,65 @@ var _ = Describe("Check Operator", func() {
 				} else {
 					DeferCleanup(os.Unsetenv, "KUBECONFIG")
 				}
+
+				tmpDir, err := os.MkdirTemp("", "preflight-operator-test-*")
+				Expect(err).ToNot(HaveOccurred())
+				DeferCleanup(os.RemoveAll, tmpDir)
+
+				// creating an empty kubeconfig file in the tmpDir so we don't fail for a missing file
+				f1, err := os.Create(filepath.Join(tmpDir, "kubeconfig"))
+				Expect(err).ToNot(HaveOccurred())
+				defer f1.Close()
+
+				os.Setenv("KUBECONFIG", f1.Name())
+			})
+			It("should reach the core logic, and execute the mocked RunPreflight", func() {
+				out, err := executeCommandWithLogger(checkOperatorCmd(mockRunPreflight), 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")
+				Expect(err).To(HaveOccurred())
+				Expect(out).To(ContainSubstring("random error"))
+			})
+		})
+
+		Context("With an invalid KUBECONFIG file location", func() {
+			BeforeEach(func() {
+				DeferCleanup(viper.Set, "indexImage", viper.GetString("indexImage"))
+				viper.Set("indexImage", "foo")
+				if val, isSet := os.LookupEnv("KUBECONFIG"); isSet {
+					DeferCleanup(os.Setenv, "KUBECONFIG", val)
+				} else {
+					DeferCleanup(os.Unsetenv, "KUBECONFIG")
+				}
+
 				os.Setenv("KUBECONFIG", "foo")
 			})
-			It("should reach the core logic, but throw an error because of the placeholder values", func() {
-				_, err := executeCommand(checkOperatorCmd(), "quay.io/example/image:mytag")
+			It("should return a no such file error", func() {
+				out, err := executeCommandWithLogger(checkOperatorCmd(mockRunPreflight), 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() {
+			BeforeEach(func() {
+				DeferCleanup(viper.Set, "indexImage", viper.GetString("indexImage"))
+				viper.Set("indexImage", "foo")
+				if val, isSet := os.LookupEnv("KUBECONFIG"); isSet {
+					DeferCleanup(os.Setenv, "KUBECONFIG", val)
+				} else {
+					DeferCleanup(os.Unsetenv, "KUBECONFIG")
+				}
+
+				os.Setenv("KUBECONFIG", ".")
+			})
+			It("should return an is a directory error", func() {
+				out, err := executeCommandWithLogger(checkOperatorCmd(mockRunPreflight), logr.Discard(), "quay.io/example/image:mytag")
 				Expect(err).To(HaveOccurred())
+				Expect(out).To(ContainSubstring(": is a directory"))
 			})
 		})
 	})
@@ -127,7 +184,7 @@ var _ = Describe("Check Operator", func() {
 		})
 
 		It("should succeed when all positional arg constraints and environment constraints are correct", func() {
-			err := checkOperatorPositionalArgs(checkOperatorCmd(), posArgs)
+			err := checkOperatorPositionalArgs(checkOperatorCmd(mockRunPreflight), posArgs)
 			Expect(err).ToNot(HaveOccurred())
 		})
 	})