Repository: redhat-openshift-ecosystem/openshift-preflight
Pull request #695 (2022-06-14 21:17:29)

updating check container to only read the docker file from disk if present
- Fixes: #685 
- updating to only read the docker file from dish if the user passes in a value
- updating to not set pyxis docker config json if a user did not pass the value, for the case where a docker config is already on the project
- adding in a warning if a user does not provide a docker config file for a github repo 
   - I'm on the fence about if we want to do this or not, it almost seems that Clair should create tokens for our backend systems for dockerhub

Signed-off-by: Adam D. Cornett <adc@redhat.com>

dcibot: -  SUCCESS https://www.distributed-ci.io/jobs/714e67b5-9e78-40d6-909d-89ec737b13b2/jobStates
 from https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/695
coveralls: 
[![Coverage Status](https://coveralls.io/builds/50028957/badge)](https://coveralls.io/builds/50028957)

Coverage decreased (-0.2%) to 60.637% when pulling **b6bb6e6cafed2ef44ee07551353302e8ea390c1c on acornett21:optional_docker_config** into **f5f30cc1f591dc0870613b3adc7356a90e5f03f7 on redhat-openshift-ecosystem:main**.

dcibot: -  SUCCESS https://www.distributed-ci.io/jobs/ad90c8d5-5c67-41a2-92ed-61c096a31b84/jobStates
 from https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/695
dcibot: -  SUCCESS https://www.distributed-ci.io/jobs/aeb12505-5ec2-4e97-8af8-efed4b6bb17c/jobStates
 from https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/695
dcibot: -  SUCCESS https://www.distributed-ci.io/jobs/666381db-94ad-4ae3-8aff-6f55f446e7a5/jobStates
 from https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/695
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/695#" title="Author self-approved">acornett21</a>*, *<a href="https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/695#pullrequestreview-1017452625" 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,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":[]} -->
openshift-ci[bot]: New changes are detected. LGTM label has been removed.
bcrochet: Needs a rebase, and unit tests.
dcibot: -  FAILURE https://www.distributed-ci.io/jobs/8beefd7c-b9c7-4bd8-8813-80f42a45e7f0/jobStates
 from https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/695
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/695#" title="Author self-approved">acornett21</a>*, *<a href="https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/695#pullrequestreview-1022068488" title="LGTM">bcrochet</a>*, *<a href="https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/695#pullrequestreview-1017452625" 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,bcrochet,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":[]} -->

Patch:
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..22ed4d7 100644
--- a/cmd/check_container_test.go
+++ b/cmd/check_container_test.go
@@ -156,6 +156,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))