Repository: redhat-openshift-ecosystem/openshift-preflight
Pull request #715 (2022-06-28 16:37:11)

fix bug with undefined default formatter
This fixes a bug where the formatter being used was undefined, and so `check container` would fail indicating that the value was empty. I forgot that this value isn't defined in viper's defaulting logic, or an environment variable.

I've added a test to see that we reach the "happy path" and then fail because of staged values when running check_container.

Signed-off-by: Jose R. Gonzalez <jose@flutes.dev>

openshift-ci[bot]: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/715#" title="Author self-approved">komish</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=redhat-openshift-ecosystem%2Fopenshift-preflight).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/redhat-openshift-ecosystem/openshift-preflight/blob/main/OWNERS)~~ [komish]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
openshift-ci[bot]: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/715#pullrequestreview-1022086804" title="Approved">acornett21</a>*, *<a href="https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/715#pullrequestreview-1022084759" title="Approved">jomkz</a>*, *<a href="https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/715#" title="Author self-approved">komish</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=redhat-openshift-ecosystem%2Fopenshift-preflight).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/redhat-openshift-ecosystem/openshift-preflight/blob/main/OWNERS)~~ [acornett21,jomkz,komish]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
openshift-ci[bot]: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/715#pullrequestreview-1022086804" title="Approved">acornett21</a>*, *<a href="https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/715#pullrequestreview-1022084759" title="Approved">jomkz</a>*, *<a href="https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/715#" title="Author self-approved">komish</a>*, *<a href="https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/715#pullrequestreview-1022092825" title="Approved">skattoju</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=redhat-openshift-ecosystem%2Fopenshift-preflight).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/redhat-openshift-ecosystem/openshift-preflight/blob/main/OWNERS)~~ [acornett21,jomkz,komish,skattoju]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
dcibot: -  SUCCESS https://www.distributed-ci.io/jobs/5fc4e3f2-b674-4379-81e6-3f172685715c/jobStates
 from https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/715

Patch:
diff --git a/cmd/check_container.go b/cmd/check_container.go
index 13be46b..4f6fd37 100644
--- a/cmd/check_container.go
+++ b/cmd/check_container.go
@@ -87,6 +87,8 @@ func checkContainerRunE(cmd *cobra.Command, args []string) error {
 	}
 
 	cfg.Image = containerImage
+	cfg.ResponseFormat = formatters.DefaultFormat
+
 	checkContainer, err := newCheckContainerRunner(ctx, cfg)
 	if err != nil {
 		return err
diff --git a/cmd/types.go b/cmd/types.go
index df45c4c..cf173a9 100644
--- a/cmd/types.go
+++ b/cmd/types.go
@@ -78,13 +78,28 @@ func (s *containerCertificationSubmitter) Submit(ctx context.Context) error {
 
 	log.Tracef("CertProject: %+v", certProject)
 
-	// read the provided docker config
-	dockerConfigJsonBytes, err := os.ReadFile(s.dockerConfig)
-	if err != nil {
-		return err
+	// only read the dockerfile if the user provides a location for the file
+	// at this point in the flow, if `cfg.DockerConfig` is empty we know the repo is public and can continue the submission flow
+	if s.dockerConfig != "" {
+		dockerConfigJsonBytes, err := os.ReadFile(s.dockerConfig)
+		if err != nil {
+			return fmt.Errorf("could not open file for submission: %s: %w",
+				s.dockerConfig,
+				err,
+			)
+		}
+
+		certProject.Container.DockerConfigJSON = string(dockerConfigJsonBytes)
 	}
 
-	certProject.Container.DockerConfigJSON = string(dockerConfigJsonBytes)
+	// the below code is for the edge case where a partner has a DockerConfig in pyxis, but does not send one to preflight.
+	// when we call pyxis's GetProject API, we get back the DockerConfig as a PGP encrypted string and not JSON,
+	// if we were to send what pyixs just sent us in a update call, pyxis would throw a validation error saying it's not valid json
+	// the below code aims to set the DockerConfigJSON to an empty string, and since this field is `omitempty` when we marshall it
+	// we will not get a validation error
+	if s.dockerConfig == "" {
+		certProject.Container.DockerConfigJSON = ""
+	}
 
 	// prepare submission. We ignore the error because nil checks for the certProject
 	// are done earlier to prevent panics, and that's the only error case for this function.


Test patch:
diff --git a/cmd/check_container_test.go b/cmd/check_container_test.go
index 4d50181..a849efb 100644
--- a/cmd/check_container_test.go
+++ b/cmd/check_container_test.go
@@ -40,6 +40,15 @@ var _ = Describe("Check Container Command", func() {
 		artifacts.Reset()
 	})
 
+	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")
+				Expect(err).To(HaveOccurred())
+			})
+		})
+	})
+
 	Context("When determining container policy exceptions", func() {
 		var fakePC *fakePyxisClient
 		BeforeEach(func() {
@@ -156,6 +165,20 @@ var _ = Describe("Check Container Command", func() {
 			})
 		})
 
+		Context("and no docker config command argument was provided", func() {
+			BeforeEach(func() {
+				fakePC.setSRFuncSubmitSuccessfully("", "")
+			})
+			It("should not throw an error", func() {
+				sbmt.dockerConfig = ""
+				err := os.Remove(dockerConfigPath)
+				Expect(err).ToNot(HaveOccurred())
+
+				err = sbmt.Submit(context.TODO())
+				Expect(err).ToNot(HaveOccurred())
+			})
+		})
+
 		Context("and the cert image cannot be read from disk", func() {
 			It("should throw an error", func() {
 				err := os.Remove(path.Join(artifactsDir, certification.DefaultCertImageFilename))
@@ -426,6 +449,7 @@ var _ = Describe("Check Container Command", func() {
 
 				expectedEngine, err := engine.NewForConfig(context.TODO(), cfg.ReadOnly())
 				Expect(runner.eng).To(BeEquivalentTo(expectedEngine))
+				Expect(err).ToNot(HaveOccurred())
 			})
 		})
 		// NOTE(): There's no way to test policy exceptions here because