Repository: redhat-openshift-ecosystem/openshift-preflight
Pull request #706 (2022-06-23 17:27:06)

updating gh actions to login to a given registry
## Motivation
There is sometimes a need to cut a hotfix for a partner and the easiest way to do this is to create a release from a fork, if we have changes in `main` that are not read to be released. In-order to accomplish this, the login steps need to be moved around/added in various steps to guard against failures.

## Testing
This was used in my fork in the `ghactions` [branch](https://github.com/acornett21/openshift-preflight/tree/ghactions) to cut release `1.2.1-rpm-hotfix` found [here](https://github.com/acornett21/openshift-preflight/releases/tag/1.2.1-rpm-hotfix)

Signed-off-by: Adam D. Cornett <adc@redhat.com>

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/706#" title="Author self-approved">acornett21</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]

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/fac4d749-c4bd-4dc2-8d49-760447470ebb/jobStates
 from https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/706
coveralls: 
[![Coverage Status](https://coveralls.io/builds/50308500/badge)](https://coveralls.io/builds/50308500)

Coverage remained the same at 61.67% when pulling **abe2417ef6d3496fd9e7376877d66147cc61befc on acornett21:enhance_gh_actions** into **d4305e841e29fdfdb013cc98d544e5f657e44ce9 on redhat-openshift-ecosystem:main**.


Patch:
diff --git a/.github/workflows/build-multiarch.yml b/.github/workflows/build-multiarch.yml
index 60fb768..18fea53 100644
--- a/.github/workflows/build-multiarch.yml
+++ b/.github/workflows/build-multiarch.yml
@@ -33,6 +33,14 @@ jobs:
     name: create and push a multiarch manifest to the repo
     runs-on: ubuntu-latest
     steps:
+      # Authenticate to container image registry to push the image
+      - name: Podman Login
+        uses: redhat-actions/podman-login@v1
+        with:
+          registry: ${{ secrets.registry }}
+          username: ${{ secrets.user }}
+          password: ${{ secrets.password }}
+
       - name: Create and add to manifest
         run: |
           buildah manifest create ${{ inputs.name }}
@@ -41,14 +49,6 @@ jobs:
           buildah manifest add ${{ inputs.name }} ${{ secrets.registry }}/${{ inputs.name }}:${{ inputs.tag }}-linux-arm64
           buildah manifest add ${{ inputs.name }} ${{ secrets.registry }}/${{ inputs.name }}:${{ inputs.tag }}-linux-s390x
 
-      # Authenticate to container image registry to push the image
-      - name: Podman Login
-        uses: redhat-actions/podman-login@v1
-        with:
-          registry: ${{ secrets.registry }}
-          username: ${{ secrets.user }}
-          password: ${{ secrets.password }}
-          
       - name: Push manifest
         run: |
             podman manifest push ${{ inputs.name }} ${{ secrets.registry }}/${{ inputs.name }}:${{ inputs.tag }}  --all
diff --git a/.github/workflows/build-release.yml b/.github/workflows/build-release.yml
index 4d34c0e..5d15f7e 100644
--- a/.github/workflows/build-release.yml
+++ b/.github/workflows/build-release.yml
@@ -70,12 +70,13 @@ jobs:
   extract-assets:
     needs: build-release
     uses: ./.github/workflows/release-artifacts.yml
-
     with:
       name: ${{ needs.build-release.outputs.imageName }}
       tag: ${{ needs.build-release.outputs.imageVersion }}
     secrets:
       registry: ${{ secrets.IMAGE_REGISTRY }}
+      user: ${{ secrets.REGISTRY_USER }}
+      password: ${{ secrets.REGISTRY_PASSWORD }}
       token: ${{ secrets.GITHUB_TOKEN }}
 
   copy-to-rhisv:
diff --git a/.github/workflows/release-artifacts.yml b/.github/workflows/release-artifacts.yml
index ea5b829..fbd740a 100644
--- a/.github/workflows/release-artifacts.yml
+++ b/.github/workflows/release-artifacts.yml
@@ -12,6 +12,10 @@ on:
     secrets:
       registry:
         required: true
+      user:
+        required: true
+      password:
+        required: true
       token:
         required: true
         
@@ -28,6 +32,13 @@ jobs:
           - ppc64le
           - s390x
     steps:
+    - name: Docker Login
+      uses: docker/login-action@v1
+      with:
+        registry: ${{ secrets.registry }}
+        username: ${{ secrets.user }}
+        password: ${{ secrets.password }}
+
     - name: Extract executable
       uses: shrink/actions-docker-extract@v1
       id: extract
diff --git a/certification/formatters/defaults.go b/certification/formatters/defaults.go
new file mode 100644
index 0000000..aaf6876
--- /dev/null
+++ b/certification/formatters/defaults.go
@@ -0,0 +1,5 @@
+package formatters
+
+const (
+	DefaultFormat = "json"
+)
diff --git a/certification/formatters/errors.go b/certification/formatters/errors.go
new file mode 100644
index 0000000..5c138c8
--- /dev/null
+++ b/certification/formatters/errors.go
@@ -0,0 +1,5 @@
+package formatters
+
+import "errors"
+
+var ErrUnknownFormatter = errors.New("requested format is unknown")
diff --git a/certification/formatters/formatters.go b/certification/formatters/formatters.go
index bceeeae..a32d999 100644
--- a/certification/formatters/formatters.go
+++ b/certification/formatters/formatters.go
@@ -30,11 +30,17 @@ type FormatterFunc = func(context.Context, runtime.Results) (response []byte, fo
 // NewForConfig returns a new formatter based on the user-provided configuration. It relies
 // on config values which should align with known/supported/built-in formatters.
 func NewForConfig(cfg certification.Config) (ResponseFormatter, error) {
-	formatter, defined := availableFormatters[cfg.ResponseFormat()]
+	return NewByName(cfg.ResponseFormat())
+}
+
+// NewByName returns a predefined ResponseFormatter with the given name.
+// TODO: New* funcs in this package may benefit from renaming.
+func NewByName(name string) (ResponseFormatter, error) {
+	formatter, defined := availableFormatters[name]
 	if !defined {
-		return nil, fmt.Errorf(
-			"failed to create a new formatter from config: %s",
-			cfg.ResponseFormat(),
+		return nil, fmt.Errorf("%w: %s",
+			ErrUnknownFormatter,
+			name,
 		)
 	}
 
diff --git a/certification/pyxis/builder.go b/certification/pyxis/builder.go
index 121b611..a20b8fe 100644
--- a/certification/pyxis/builder.go
+++ b/certification/pyxis/builder.go
@@ -13,7 +13,7 @@ import (
 // certificationInputBuilder facilitates the building of CertificationInput for
 // submitting an asset to Pyxis.
 type certificationInputBuilder struct {
-	certificationInput
+	CertificationInput
 }
 
 // NewCertificationInput accepts required values for submitting to Pyxis, and returns a CertificationInputBuilder for
@@ -25,7 +25,7 @@ func NewCertificationInput(project *CertProject) (*certificationInputBuilder, er
 	}
 
 	b := certificationInputBuilder{
-		certificationInput: certificationInput{
+		CertificationInput: CertificationInput{
 			CertProject: project,
 		},
 	}
@@ -39,7 +39,7 @@ func NewCertificationInput(project *CertProject) (*certificationInputBuilder, er
 // unmodifiable CertificationInput.
 //
 // If any required values are not included, an error is thrown.
-func (b *certificationInputBuilder) Finalize() (*certificationInput, error) {
+func (b *certificationInputBuilder) Finalize() (*CertificationInput, error) {
 	// safeguards, make sure things aren't nil for any reason.
 	if b.CertImage == nil {
 		return nil, fmt.Errorf("a CertImage was not provided and is required")
@@ -61,7 +61,7 @@ func (b *certificationInputBuilder) Finalize() (*certificationInput, error) {
 	b.CertImage.ISVPID = b.CertProject.Container.ISVPID
 	b.CertImage.Certified = b.TestResults.Passed
 
-	return &b.certificationInput, nil
+	return &b.CertificationInput, nil
 }
 
 // WithCertImage adds a pyxis.CertImage from the passed io.Reader to the CertificationInput.
diff --git a/certification/pyxis/submit.go b/certification/pyxis/submit.go
index a3e74a2..e1d44de 100644
--- a/certification/pyxis/submit.go
+++ b/certification/pyxis/submit.go
@@ -12,7 +12,7 @@ var defaultRegistryAlias = "docker.io"
 
 // SubmitResults takes certInput and sends requests to Pyxis to create or update entries
 // based on certInput.
-func (p *pyxisClient) SubmitResults(ctx context.Context, certInput *certificationInput) (*CertificationResults, error) {
+func (p *pyxisClient) SubmitResults(ctx context.Context, certInput *CertificationInput) (*CertificationResults, error) {
 	var err error
 
 	certProject := certInput.CertProject
diff --git a/certification/pyxis/types.go b/certification/pyxis/types.go
index 2e74e65..27fe87a 100644
--- a/certification/pyxis/types.go
+++ b/certification/pyxis/types.go
@@ -6,7 +6,7 @@ import (
 	"github.com/redhat-openshift-ecosystem/openshift-preflight/certification/formatters"
 )
 
-type certificationInput struct {
+type CertificationInput struct {
 	CertProject *CertProject
 	CertImage   *CertImage
 	TestResults *TestResults
diff --git a/certification/runtime/result_writer.go b/certification/runtime/result_writer.go
new file mode 100644
index 0000000..9603334
--- /dev/null
+++ b/certification/runtime/result_writer.go
@@ -0,0 +1,33 @@
+package runtime
+
+import (
+	"io"
+	"os"
+)
+
+// ResultWriterFile implements a resultWriter for use at preflight runtime.
+type ResultWriterFile struct {
+	file *os.File
+}
+
+// OpenFile will open the expected file for writing.
+func (f *ResultWriterFile) OpenFile(name string) (io.WriteCloser, error) {
+	file, err := os.OpenFile(
+		name,
+		os.O_WRONLY|os.O_CREATE|os.O_TRUNC,
+		0o600)
+	if err != nil {
+		return nil, err
+	}
+
+	f.file = file // so we can close it later.
+	return f, nil
+}
+
+func (f *ResultWriterFile) Close() error {
+	return f.file.Close()
+}
+
+func (f *ResultWriterFile) Write(p []byte) (int, error) {
+	return f.file.Write(p)
+}
diff --git a/cmd/check_container.go b/cmd/check_container.go
index 8a0a377..7729b58 100644
--- a/cmd/check_container.go
+++ b/cmd/check_container.go
@@ -64,7 +64,7 @@ func checkContainerRunE(cmd *cobra.Command, args []string) error {
 
 	// Set our runtime defaults.
 	cfg.Image = containerImage
-	cfg.ResponseFormat = DefaultOutputFormat
+	cfg.ResponseFormat = formatters.DefaultFormat
 
 	// Run the  container check.
 	cmd.SilenceUsage = true
diff --git a/cmd/check_operator.go b/cmd/check_operator.go
index d1a1330..7274610 100644
--- a/cmd/check_operator.go
+++ b/cmd/check_operator.go
@@ -56,7 +56,7 @@ func checkOperatorRunE(cmd *cobra.Command, args []string) error {
 	}
 
 	cfg.Image = operatorImage
-	cfg.ResponseFormat = DefaultOutputFormat
+	cfg.ResponseFormat = formatters.DefaultFormat
 	cfg.Bundle = true
 	cfg.Scratch = true
 
diff --git a/cmd/defaults.go b/cmd/defaults.go
index d71dd7e..813d749 100644
--- a/cmd/defaults.go
+++ b/cmd/defaults.go
@@ -1,7 +1,6 @@
 package cmd
 
 var (
-	DefaultOutputFormat      = "json"
 	DefaultLogFile           = "preflight.log"
 	DefaultLogLevel          = "info"
 	DefaultNamespace         = "default"
diff --git a/cmd/types.go b/cmd/types.go
new file mode 100644
index 0000000..df45c4c
--- /dev/null
+++ b/cmd/types.go
@@ -0,0 +1,176 @@
+package cmd
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"net/http"
+	"os"
+	"path"
+	"path/filepath"
+	"time"
+
+	"github.com/redhat-openshift-ecosystem/openshift-preflight/certification"
+	"github.com/redhat-openshift-ecosystem/openshift-preflight/certification/artifacts"
+	"github.com/redhat-openshift-ecosystem/openshift-preflight/certification/pyxis"
+	log "github.com/sirupsen/logrus"
+)
+
+// resultWriter defines methods associated with writing check results.
+type resultWriter interface {
+	OpenFile(name string) (io.WriteCloser, error)
+	io.WriteCloser
+}
+
+// resultSubmitter defines methods associated with submitting results to Red HAt.
+type resultSubmitter interface {
+	Submit(context.Context) error
+}
+
+// pyxisClient defines pyxis API interactions that are relevant to check executions in cmd.
+type pyxisClient interface {
+	FindImagesByDigest(ctx context.Context, digests []string) ([]pyxis.CertImage, error)
+	GetProject(context.Context) (*pyxis.CertProject, error)
+	SubmitResults(context.Context, *pyxis.CertificationInput) (*pyxis.CertificationResults, error)
+}
+
+// newPyxisClient initializes a pyxisClient with relevant information from cfg.
+// If the the CertificationProjectID, PyxisAPIToken, or PyxisHost are empty, then nil is returned.
+// Callers should treat a nil pyxis client as an indicator that pyxis calls should not be made.
+func newPyxisClient(ctx context.Context, cfg certification.Config) pyxisClient {
+	if cfg.CertificationProjectID() == "" || cfg.PyxisAPIToken() == "" || cfg.PyxisHost() == "" {
+		return nil
+	}
+
+	return pyxis.NewPyxisClient(
+		cfg.PyxisHost(),
+		cfg.PyxisAPIToken(),
+		cfg.CertificationProjectID(),
+		&http.Client{Timeout: 60 * time.Second},
+	)
+}
+
+// containerCertificationSubmitter submits container results to Pyxis, and implements
+// a resultSubmitter.
+type containerCertificationSubmitter struct {
+	certificationProjectID string
+	pyxis                  pyxisClient
+	dockerConfig           string
+	preflightLogFile       string
+}
+
+func (s *containerCertificationSubmitter) Submit(ctx context.Context) error {
+	log.Info("preparing results that will be submitted to Red Hat")
+
+	// get the project info from pyxis
+	certProject, err := s.pyxis.GetProject(ctx)
+	if err != nil {
+		return fmt.Errorf("could not retrieve project: %w", err)
+	}
+
+	// Ensure that a certProject was returned. In theory we would expect pyxis
+	// to throw an error if no project is returned, but in the event that it doesn't
+	// we need to confirm before we proceed in order to prevent a runtime panic
+	// setting the DockerConfigJSON below.
+	if certProject == nil {
+		return fmt.Errorf("no certification project was returned from pyxis")
+	}
+
+	log.Tracef("CertProject: %+v", certProject)
+
+	// read the provided docker config
+	dockerConfigJsonBytes, err := os.ReadFile(s.dockerConfig)
+	if err != nil {
+		return err
+	}
+
+	certProject.Container.DockerConfigJSON = string(dockerConfigJsonBytes)
+
+	// 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.
+	submission, _ := pyxis.NewCertificationInput(certProject)
+
+	certImage, err := os.Open(path.Join(artifacts.Path(), certification.DefaultCertImageFilename))
+	defer certImage.Close()
+	if err != nil {
+		return fmt.Errorf("could not open file for submission: %s: %w",
+			certification.DefaultCertImageFilename,
+			err,
+		)
+	}
+	preflightResults, err := os.Open(path.Join(artifacts.Path(), certification.DefaultTestResultsFilename))
+	defer preflightResults.Close()
+	if err != nil {
+		return fmt.Errorf(
+			"could not open file for submission: %s: %w",
+			certification.DefaultTestResultsFilename,
+			err,
+		)
+	}
+	rpmManifest, err := os.Open(path.Join(artifacts.Path(), certification.DefaultRPMManifestFilename))
+	defer rpmManifest.Close()
+	if err != nil {
+		return fmt.Errorf(
+			"could not open file for submission: %s: %w",
+			certification.DefaultRPMManifestFilename,
+			err,
+		)
+	}
+	logfile, err := os.Open(s.preflightLogFile)
+	defer logfile.Close()
+	if err != nil {
+		return fmt.Errorf(
+			"could not open file for submission: %s: %w",
+			s.preflightLogFile,
+			err,
+		)
+	}
+	submission.
+		// The engine writes the certified image config to disk in a Pyxis-specific format.
+		WithCertImage(certImage).
+		// Include Preflight's test results in our submission. pyxis.TestResults embeds them.
+		WithPreflightResults(preflightResults).
+		// The certification engine writes the rpmManifest for images not based on scratch.
+		WithRPMManifest(rpmManifest).
+		// Include the preflight execution log file.
+		WithArtifact(logfile, filepath.Base(s.preflightLogFile))
+
+	input, err := submission.Finalize()
+	if err != nil {
+		return fmt.Errorf("unable to finalize data that would be sent to pyxis: %w", err)
+	}
+
+	certResults, err := s.pyxis.SubmitResults(ctx, input)
+	if err != nil {
+		return fmt.Errorf("could not submit to pyxis: %w", err)
+	}
+
+	log.Info("Test results have been submitted to Red Hat.")
+	log.Info("These results will be reviewed by Red Hat for final certification.")
+	log.Infof("The container's image id is: %s.", certResults.CertImage.ID)
+	log.Infof("Please check %s to view scan results.", buildScanResultsURL(s.certificationProjectID, certResults.CertImage.ID))
+	log.Infof("Please check %s to monitor the progress.", buildOverviewURL(s.certificationProjectID))
+
+	return nil
+}
+
+// noopSubmitter is a no-op resultSubmitter that optionally logs a message
+// and a reason as to why results were not submitted.
+type noopSubmitter struct {
+	emitLog bool
+	reason  string
+	log     *log.Logger
+}
+
+func (s *noopSubmitter) Submit(ctx context.Context) error {
+	if s.emitLog {
+		msg := "Results are not being sent for submission."
+		if s.reason != "" {
+			msg = fmt.Sprintf("%s Reason: %s.", msg, s.reason)
+		}
+
+		s.log.Info(msg)
+	}
+
+	return nil
+}


Test patch:
diff --git a/certification/formatters/formatters_test.go b/certification/formatters/formatters_test.go
index 41eb38a..2de88e7 100644
--- a/certification/formatters/formatters_test.go
+++ b/certification/formatters/formatters_test.go
@@ -10,6 +10,12 @@ import (
 )
 
 var _ = Describe("Formatters", func() {
+	Describe("When getting the formatter for the named default format", func() {
+		It("should never fail", func() {
+			_, err := NewByName(DefaultFormat)
+			Expect(err).ToNot(HaveOccurred())
+		})
+	})
 	Describe("When getting a new formatter for a configuration", func() {
 		Context("with a valid configuration", func() {
 			cfg := runtime.Config{
diff --git a/certification/pyxis/builder_test.go b/certification/pyxis/builder_test.go
index 7f4fb63..e9d8f3d 100644
--- a/certification/pyxis/builder_test.go
+++ b/certification/pyxis/builder_test.go
@@ -60,7 +60,7 @@ var _ = Describe("Pyxis Builder tests", func() {
 			})
 
 			It("should return a certification input builder with the project embedded", func() {
-				Expect(builder.certificationInput.CertProject.ID).To(Equal(p.ID))
+				Expect(builder.CertificationInput.CertProject.ID).To(Equal(p.ID))
 			})
 
 			It("should fail to finalize with no cert image", func() {
diff --git a/certification/pyxis/submit_test.go b/certification/pyxis/submit_test.go
index d0005bc..65e8811 100644
--- a/certification/pyxis/submit_test.go
+++ b/certification/pyxis/submit_test.go
@@ -12,7 +12,7 @@ var _ = Describe("Pyxis Submit", func() {
 	ctx := context.Background()
 
 	var pyxisClient *pyxisClient
-	var certInput certificationInput
+	var certInput CertificationInput
 	mux := http.NewServeMux()
 
 	// These go from most explicit to least explicit. They will be check that way by the ServeMux.
@@ -29,7 +29,7 @@ var _ = Describe("Pyxis Submit", func() {
 			"my-awesome-project-id",
 			&http.Client{Transport: localRoundTripper{handler: mux}},
 		)
-		certInput = certificationInput{
+		certInput = CertificationInput{
 			CertProject: &CertProject{CertificationStatus: "Started"},
 			CertImage: &CertImage{
 				Repositories: []Repository{
diff --git a/certification/runtime/result_writer_test.go b/certification/runtime/result_writer_test.go
new file mode 100644
index 0000000..3510c2b
--- /dev/null
+++ b/certification/runtime/result_writer_test.go
@@ -0,0 +1,52 @@
+package runtime
+
+import (
+	"os"
+	"path"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Result Writers", func() {
+	var resultWriterTestDir string
+
+	BeforeEach(func() {
+		// instantiate err to make sure we can equal-assign in the following line.
+		var err error
+		resultWriterTestDir, err = os.MkdirTemp(os.TempDir(), "rw-test-*")
+		Expect(err).ToNot(HaveOccurred())
+		Expect(len(resultWriterTestDir)).ToNot(BeZero())
+	})
+
+	AfterEach(func() {
+		err := os.RemoveAll(resultWriterTestDir)
+		Expect(err).ToNot(HaveOccurred())
+	})
+
+	Context("File-based Result Writer", func() {
+		var rw *ResultWriterFile
+
+		BeforeEach(func() {
+			rw = &ResultWriterFile{}
+		})
+
+		Context("When using the file-based result writer", func() {
+			It("should open, be writeable, and close successfully", func() {
+				p := path.Join(resultWriterTestDir, "foo.txt")
+				f, err := rw.OpenFile(p)
+				defer f.Close()
+				Expect(err).ToNot(HaveOccurred())
+				Expect(p).To(BeAnExistingFile())
+
+				toWrite := []byte("testing")
+				_, err = f.Write(toWrite)
+				Expect(err).ToNot(HaveOccurred())
+
+				contents, err := os.ReadFile(p)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(contents).To(Equal(toWrite))
+			})
+		})
+	})
+})