Refactor cmd/ to have less coupling

No more inits are being used in the individual command files. The
remaining init is to initialize cobra itself.

This should increase coverage slightly, and make the commands less
interdependent.

Signed-off-by: Brad P. Crochet <brad@redhat.com>

cmd/check.go

@@ -13,13 +13,13 @@ import (
 	"github.com/spf13/viper"
 )
 
-var 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.",
-}
+func checkCmd() *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.",
+	}
 
-func init() {
 	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)")
@@ -28,7 +28,10 @@ func init() {
 	checkCmd.PersistentFlags().String("artifacts", "", "Where check-specific artifacts will be written. (env: PFLT_ARTIFACTS)")
 	viper.BindPFlag("artifacts", checkCmd.PersistentFlags().Lookup("artifacts"))
 
-	rootCmd.AddCommand(checkCmd)
+	checkCmd.AddCommand(checkContainerCmd())
+	checkCmd.AddCommand(checkOperatorCmd())
+
+	return checkCmd
 }
 
 // writeJUnit will write results as JUnit XML using the built-in formatter.
@@ -61,7 +64,8 @@ func resultsFilenameWithExtension(ext string) string {
 func buildConnectURL(projectID string) string {
 	connectURL := fmt.Sprintf("https://connect.redhat.com/projects/%s", projectID)
 
-	if viper.GetString("pyxis_env") != "prod" {
+	pyxis_env := viper.GetString("pyxis_env")
+	if len(pyxis_env) > 0 && pyxis_env != "prod" {
 		connectURL = fmt.Sprintf("https://connect.%s.redhat.com/projects/%s", viper.GetString("pyxis_env"), projectID)
 	}
 

cmd/check_container.go

@@ -17,14 +17,35 @@ import (
 
 var submit bool
 
-var 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,
-	// 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"),
-	RunE:    checkContainerRunE,
+func checkContainerCmd() *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,
+		// 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"),
+		RunE:    checkContainerRunE,
+	}
+
+	checkContainerCmd.Flags().BoolVarP(&submit, "submit", "s", false, "submit check container results to red hat")
+	viper.BindPFlag("submit", checkContainerCmd.Flags().Lookup("submit"))
+
+	checkContainerCmd.Flags().String("pyxis-api-token", "", "API token for Pyxis authentication (env: PFLT_PYXIS_API_TOKEN)")
+	viper.BindPFlag("pyxis_api_token", checkContainerCmd.Flags().Lookup("pyxis-api-token"))
+
+	checkContainerCmd.Flags().String("pyxis-host", "", fmt.Sprintf("Host to use for Pyxis submissions. This will override Pyxis Env. Only set this if you know what you are doing.\n"+
+		"If you do set it, it should include just the host, and the URI path. (env: PFLT_PYXIS_HOST)"))
+	viper.BindPFlag("pyxis_host", checkContainerCmd.Flags().Lookup("pyxis-host"))
+
+	checkContainerCmd.Flags().String("pyxis-env", certification.DefaultPyxisEnv, "Env to use for Pyxis submissions.")
+	viper.BindPFlag("pyxis_env", checkContainerCmd.Flags().Lookup("pyxis-env"))
+
+	checkContainerCmd.Flags().String("certification-project-id", "", fmt.Sprintf("Certification Project ID from connect.redhat.com/projects/{certification-project-id}/overview\n"+
+		"URL paramater. This value may differ from the PID on the overview page. (env: PFLT_CERTIFICATION_PROJECT_ID)"))
+	viper.BindPFlag("certification_project_id", checkContainerCmd.Flags().Lookup("certification-project-id"))
+
+	return checkContainerCmd
 }
 
 // checkContainerRunner contains all of the components necessary to run checkContainer.
@@ -162,24 +183,3 @@ func checkContainerPositionalArgs(cmd *cobra.Command, args []string) error {
 
 	return nil
 }
-
-func init() {
-	checkContainerCmd.Flags().BoolVarP(&submit, "submit", "s", false, "submit check container results to red hat")
-	viper.BindPFlag("submit", checkContainerCmd.Flags().Lookup("submit"))
-
-	checkContainerCmd.Flags().String("pyxis-api-token", "", "API token for Pyxis authentication (env: PFLT_PYXIS_API_TOKEN)")
-	viper.BindPFlag("pyxis_api_token", checkContainerCmd.Flags().Lookup("pyxis-api-token"))
-
-	checkContainerCmd.Flags().String("pyxis-host", "", fmt.Sprintf("Host to use for Pyxis submissions. This will override Pyxis Env. Only set this if you know what you are doing.\n"+
-		"If you do set it, it should include just the host, and the URI path. (env: PFLT_PYXIS_HOST)"))
-	viper.BindPFlag("pyxis_host", checkContainerCmd.Flags().Lookup("pyxis-host"))
-
-	checkContainerCmd.Flags().String("pyxis-env", certification.DefaultPyxisEnv, "Env to use for Pyxis submissions.")
-	viper.BindPFlag("pyxis_env", checkContainerCmd.Flags().Lookup("pyxis-env"))
-
-	checkContainerCmd.Flags().String("certification-project-id", "", fmt.Sprintf("Certification Project ID from connect.redhat.com/projects/{certification-project-id}/overview\n"+
-		"URL paramater. This value may differ from the PID on the overview page. (env: PFLT_CERTIFICATION_PROJECT_ID)"))
-	viper.BindPFlag("certification_project_id", checkContainerCmd.Flags().Lookup("certification-project-id"))
-
-	checkCmd.AddCommand(checkContainerCmd)
-}

cmd/check_container_test.go

@@ -26,7 +26,7 @@ var _ = Describe("Check Container Command", func() {
 	Context("when running the check container subcommand", func() {
 		Context("With all of the required parameters", func() {
 			It("should reach the core logic, but throw an error because of the placeholder values for the container image", func() {
-				_, err := executeCommand(rootCmd, "check", "container", "example.com/example/image:mytag")
+				_, err := executeCommand(checkContainerCmd(), "example.com/example/image:mytag")
 				Expect(err).To(HaveOccurred())
 			})
 		})
@@ -457,21 +457,21 @@ var _ = Describe("Check Container Command", func() {
 	Context("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(rootCmd, "check", "container", "foo", "bar")
+				_, err := executeCommand(checkContainerCmd(), "foo", "bar")
 				Expect(err).To(HaveOccurred())
 			})
 		})
 
 		Context("and the user provided less than 1 positional arg", func() {
 			It("should fail to run", func() {
-				_, err := executeCommand(rootCmd, "check", "container")
+				_, err := executeCommand(checkContainerCmd())
 				Expect(err).To(HaveOccurred())
 			})
 		})
 
 		Context("and the user has enabled the submit flag", func() {
 			It("should cause the certification-project-id and pyxis-api-token flag to be required", func() {
-				out, err := executeCommand(rootCmd, "check", "container", "--submit", "foo")
+				out, err := executeCommand(checkContainerCmd(), "--submit", "foo")
 				Expect(err).To(HaveOccurred())
 				Expect(out).To(ContainSubstring("required flag(s) \"%s\", \"%s\" not set", "certification-project-id", "pyxis-api-token"))
 			})

cmd/check_operator.go

@@ -15,14 +15,36 @@ import (
 	"github.com/spf13/viper"
 )
 
-var 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,
+func checkOperatorCmd() *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,
+	}
+	checkOperatorCmd.Flags().String("namespace", "", "The namespace to use when running OperatorSDK Scorecard. (env: PFLT_NAMESPACE)")
+	viper.BindPFlag("namespace", checkOperatorCmd.Flags().Lookup("namespace"))
+
+	checkOperatorCmd.Flags().String("serviceaccount", "", "The service account to use when running OperatorSDK Scorecard. (env: PFLT_SERVICEACCOUNT)")
+	viper.BindPFlag("serviceaccount", checkOperatorCmd.Flags().Lookup("serviceaccount"))
+
+	checkOperatorCmd.Flags().String("scorecard-image", "", "A uri that points to the scorecard image digest, used in disconnected environments.\n"+
+		"It should only be used in a disconnected environment. Use preflight runtime-assets on a connected \n"+
+		"workstation to generate the digest that needs to be mirrored. (env: PFLT_SCORECARD_IMAGE)")
+	viper.BindPFlag("scorecard_image", checkOperatorCmd.Flags().Lookup("scorecard-image"))
+
+	checkOperatorCmd.Flags().String("scorecard-wait-time", "", "A time value that will be passed to scorecard's --wait-time environment variable.\n"+
+		"(env: PFLT_SCORECARD_WAIT_TIME)")
+	viper.BindPFlag("scorecard_wait_time", checkOperatorCmd.Flags().Lookup("scorecard-wait-time"))
+
+	checkOperatorCmd.Flags().String("channel", "", "The name of the operator channel which is used by DeployableByOLM to deploy the operator.\n"+
+		"If empty, the default operator channel in bundle's annotations file is used.. (env: PFLT_CHANNEL)")
+	viper.BindPFlag("channel", checkOperatorCmd.Flags().Lookup("channel"))
+
+	return checkOperatorCmd
 }
 
 // checkOperatorRunner contains all of the components necessary to run checkOperator.
@@ -125,26 +147,3 @@ func checkOperatorPositionalArgs(cmd *cobra.Command, args []string) error {
 
 	return nil
 }
-
-func init() {
-	checkOperatorCmd.Flags().String("namespace", "", "The namespace to use when running OperatorSDK Scorecard. (env: PFLT_NAMESPACE)")
-	viper.BindPFlag("namespace", checkOperatorCmd.Flags().Lookup("namespace"))
-
-	checkOperatorCmd.Flags().String("serviceaccount", "", "The service account to use when running OperatorSDK Scorecard. (env: PFLT_SERVICEACCOUNT)")
-	viper.BindPFlag("serviceaccount", checkOperatorCmd.Flags().Lookup("serviceaccount"))
-
-	checkOperatorCmd.Flags().String("scorecard-image", "", "A uri that points to the scorecard image digest, used in disconnected environments.\n"+
-		"It should only be used in a disconnected environment. Use preflight runtime-assets on a connected \n"+
-		"workstation to generate the digest that needs to be mirrored. (env: PFLT_SCORECARD_IMAGE)")
-	viper.BindPFlag("scorecard_image", checkOperatorCmd.Flags().Lookup("scorecard-image"))
-
-	checkOperatorCmd.Flags().String("scorecard-wait-time", "", "A time value that will be passed to scorecard's --wait-time environment variable.\n"+
-		"(env: PFLT_SCORECARD_WAIT_TIME)")
-	viper.BindPFlag("scorecard_wait_time", checkOperatorCmd.Flags().Lookup("scorecard-wait-time"))
-
-	checkOperatorCmd.Flags().String("channel", "", "The name of the operator channel which is used by DeployableByOLM to deploy the operator.\n"+
-		"If empty, the default operator channel in bundle's annotations file is used.. (env: PFLT_CHANNEL)")
-	viper.BindPFlag("channel", checkOperatorCmd.Flags().Lookup("channel"))
-
-	checkCmd.AddCommand(checkOperatorCmd)
-}

cmd/check_operator_test.go

@@ -18,14 +18,14 @@ var _ = Describe("Check Operator", func() {
 		BeforeEach(createAndCleanupDirForArtifactsAndLogs)
 		Context("without the operator bundle image being provided", func() {
 			It("should return an error", func() {
-				_, err := executeCommand(rootCmd, "check", "operator")
+				_, err := executeCommand(checkOperatorCmd())
 				Expect(err).To(HaveOccurred())
 			})
 		})
 
 		Context("without having set the KUBECONFIG environment variable", func() {
 			It("should return an error", func() {
-				out, err := executeCommand(rootCmd, "check", "operator", "quay.io/example/image:mytag")
+				out, err := executeCommand(checkOperatorCmd(), "quay.io/example/image:mytag")
 				Expect(err).To(HaveOccurred())
 				Expect(out).To(ContainSubstring("KUBECONFIG could not"))
 			})
@@ -35,7 +35,7 @@ var _ = Describe("Check Operator", func() {
 			BeforeEach(func() { os.Setenv("KUBECONFIG", "foo") })
 			AfterEach(func() { os.Unsetenv("KUBECONFIG") })
 			It("should return an error", func() {
-				out, err := executeCommand(rootCmd, "check", "operator", "quay.io/example/image:mytag")
+				out, err := executeCommand(checkOperatorCmd(), "quay.io/example/image:mytag")
 				Expect(err).To(HaveOccurred())
 				Expect(out).To(ContainSubstring("PFLT_INDEXIMAGE could not"))
 			})
@@ -53,7 +53,7 @@ var _ = Describe("Check Operator", func() {
 			})
 
 			It("should reach the core logic, but throw an error because of the placeholder values", func() {
-				_, err := executeCommand(rootCmd, "check", "operator", "quay.io/example/image:mytag")
+				_, err := executeCommand(checkOperatorCmd(), "quay.io/example/image:mytag")
 				Expect(err).To(HaveOccurred())
 			})
 		})
@@ -164,7 +164,7 @@ var _ = Describe("Check Operator", func() {
 			os.Unsetenv("KUBECONFIG")
 		})
 		It("should succeed when all positional arg constraints and environment constraints are correct", func() {
-			err := checkOperatorPositionalArgs(checkOperatorCmd, posArgs)
+			err := checkOperatorPositionalArgs(checkOperatorCmd(), posArgs)
 			Expect(err).ToNot(HaveOccurred())
 		})
 	})

cmd/list_checks.go

@@ -10,11 +10,14 @@ import (
 	"github.com/spf13/cobra"
 )
 
-var listChecksCmd = &cobra.Command{
-	Use:   "list-checks",
-	Short: "List all checks that will be executed for each policy",
-	Long:  "This command will list all checks that preflight uses against an asset by policy type",
-	Run:   listChecksRunFunc,
+func listChecksCmd() *cobra.Command {
+	listChecksCmd := &cobra.Command{
+		Use:   "list-checks",
+		Short: "List all checks that will be executed for each policy",
+		Long:  "This command will list all checks that preflight uses against an asset by policy type",
+		Run:   listChecksRunFunc,
+	}
+	return listChecksCmd
 }
 
 // listChecksRunFunc binds printChecks to cobra's Run function
@@ -57,7 +60,3 @@ func formatList(list []string) string {
 func dashPrefix(s string) string {
 	return fmt.Sprintf("- %s", s)
 }
-
-func init() {
-	rootCmd.AddCommand(listChecksCmd)
-}

cmd/list_checks_test.go

@@ -82,7 +82,7 @@ var _ = Describe("list checks subcommand", func() {
 
 			// Run the command. Because we bind this command to the
 			// root command in init, we must pass rootCmd to executeCommand.
-			out, err := executeCommand(rootCmd, "list-checks")
+			out, err := executeCommand(listChecksCmd())
 			Expect(len(out) > 0).To(BeTrue())
 
 			Expect(err).ToNot(HaveOccurred())

cmd/root.go

@@ -15,30 +15,36 @@ import (
 
 var configFileUsed bool
 
-var 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,
-}
-
 func init() {
 	cobra.OnInitialize(initConfig)
+}
+
+func rootCmd() *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,
+	}
 
 	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(listChecksCmd())
+	rootCmd.AddCommand(runtimeAssetsCmd())
+	rootCmd.AddCommand(supportCmd())
+
+	return rootCmd
 }
 
-func Execute() {
-	if err := rootCmd.ExecuteContext(context.Background()); err != nil {
-		log.Fatal(err)
-		os.Exit(-1)
-	}
+func Execute() error {
+	return rootCmd().ExecuteContext(context.Background())
 }
 
 func initConfig() {

cmd/root_test.go

@@ -29,6 +29,16 @@ func executeCommand(root *cobra.Command, args ...string) (output string, err err
 }
 
 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()
+				Expect(cmd).ToNot(BeNil())
+				Expect(cmd.Commands()).ToNot(BeEmpty())
+			})
+		})
+	})
+
 	DescribeTable("Determine filename to which to write test results",
 		func(extension, expected string) {
 			// Ensure resultsFilenameWithExtension accurately joins the
@@ -79,7 +89,6 @@ var _ = Describe("cmd package utility functions", func() {
 				PersistentPreRun: preRunConfig,
 				Run:              func(cmd *cobra.Command, args []string) {},
 			}
-			cobra.OnInitialize(initConfig)
 		})
 		Context("configuring a Cobra Command", func() {
 			var tmpDir string