Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

conditionally calling pyxis.WithRPMManifest based on the policy value in ctx #1012

Merged
merged 1 commit into from
Jun 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions container/check_container.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@ import (
"context"
"fmt"
"net/http"
"time"

goruntime "runtime"
"time"

"github.com/redhat-openshift-ecosystem/openshift-preflight/certification"
preflighterr "github.com/redhat-openshift-ecosystem/openshift-preflight/errors"
Expand Down Expand Up @@ -59,6 +58,9 @@ func (c *containerCheck) Run(ctx context.Context) (certification.Results, error)
return certification.Results{}, fmt.Errorf("%w: %s", preflighterr.ErrCannotResolvePolicyException, err)
}

// adding policy to context to be retrieved later in the submit flow
ctx = policy.NewContext(ctx, override)

pol = override
}

Expand Down
44 changes: 23 additions & 21 deletions internal/lib/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@ import (
"path/filepath"
"time"

"github.com/go-logr/logr"

"github.com/redhat-openshift-ecosystem/openshift-preflight/artifacts"
"github.com/redhat-openshift-ecosystem/openshift-preflight/internal/check"
"github.com/redhat-openshift-ecosystem/openshift-preflight/internal/log"
"github.com/redhat-openshift-ecosystem/openshift-preflight/internal/policy"
"github.com/redhat-openshift-ecosystem/openshift-preflight/internal/pyxis"
"github.com/redhat-openshift-ecosystem/openshift-preflight/internal/viper"

"github.com/go-logr/logr"
)

// ResultWriter defines methods associated with writing check results.
Expand Down Expand Up @@ -141,16 +142,6 @@ func (s *ContainerCertificationSubmitter) Submit(ctx context.Context) error {
}
defer preflightResults.Close()

rpmManifest, err := os.Open(path.Join(artifactWriter.Path(), check.DefaultRPMManifestFilename))
if err != nil {
return fmt.Errorf(
"could not open file for submission: %s: %w",
check.DefaultRPMManifestFilename,
err,
)
}
defer rpmManifest.Close()

logfile, err := os.Open(s.PreflightLogFile)
if err != nil {
return fmt.Errorf(
Expand All @@ -161,18 +152,29 @@ func (s *ContainerCertificationSubmitter) Submit(ctx context.Context) error {
}
defer logfile.Close()

// 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, err := pyxis.NewCertificationInput(ctx, certProject,
// The engine writes the certified image config to disk in a Pyxis-specific format.
options := []pyxis.CertificationInputOption{
pyxis.WithCertImage(certImage),
// Include Preflight's test results in our submission. pyxis.TestResults embeds them.
pyxis.WithPreflightResults(preflightResults),
// The certification engine writes the rpmManifest for images not based on scratch.
pyxis.WithRPMManifest(rpmManifest),
// Include the preflight execution log file.
pyxis.WithArtifact(logfile, filepath.Base(s.PreflightLogFile)),
)
}

// only read the rpm manifest file off of disk if the policy executed is not scratch
// scratch images do not have rpm manifests, the rpm-manifest.json file is not written to disk by the engine during execution
if policy.FromContext(ctx) != policy.PolicyScratch {
rpmManifest, err := os.Open(path.Join(artifactWriter.Path(), check.DefaultRPMManifestFilename))
if err != nil {
return fmt.Errorf(
"could not open file for submission: %s: %w",
check.DefaultRPMManifestFilename,
err,
)
}
defer rpmManifest.Close()

options = append(options, pyxis.WithRPMManifest(rpmManifest))
}

submission, err := pyxis.NewCertificationInput(ctx, certProject, options...)
if err != nil {
return fmt.Errorf("unable to finalize data that would be sent to pyxis: %w", err)
}
Expand Down
39 changes: 33 additions & 6 deletions internal/lib/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,9 @@ var _ = Describe("Container Certification Submitter", func() {
err := os.Remove(dockerConfigPath)
Expect(err).ToNot(HaveOccurred())

err = sbmt.Submit(testcontext)
scratchContext := policy.NewContext(testcontext, policy.PolicyContainer)

err = sbmt.Submit(scratchContext)
Expect(err).ToNot(HaveOccurred())
})
})
Expand All @@ -276,7 +278,9 @@ var _ = Describe("Container Certification Submitter", func() {
fakePC.getProjectsFunc = gpFuncReturnHostedRegistry
})
It("should not throw an error", func() {
err := sbmt.Submit(testcontext)
scratchContext := policy.NewContext(testcontext, policy.PolicyContainer)

err := sbmt.Submit(scratchContext)
Expect(err).ToNot(HaveOccurred())
})
})
Expand Down Expand Up @@ -308,12 +312,29 @@ var _ = Describe("Container Certification Submitter", func() {
err := os.Remove(path.Join(aw.Path(), check.DefaultRPMManifestFilename))
Expect(err).ToNot(HaveOccurred())

err = sbmt.Submit(testcontext)
scratchContext := policy.NewContext(testcontext, policy.PolicyContainer)

err = sbmt.Submit(scratchContext)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring(check.DefaultRPMManifestFilename))
})
})

Context("and scratch policy was executed, so no rpmManifest exists on disk", func() {
BeforeEach(func() {
fakePC.setSRFuncSubmitSuccessfully("12345", "12345")
})
It("should not throw an error", func() {
err := os.Remove(path.Join(aw.Path(), check.DefaultRPMManifestFilename))
Expect(err).ToNot(HaveOccurred())

scratchContext := policy.NewContext(testcontext, policy.PolicyScratch)

err = sbmt.Submit(scratchContext)
Expect(err).ToNot(HaveOccurred())
})
})

Context("and the preflight logfile cannot be read from disk", func() {
It("should throw an error", func() {
err := os.Remove(preflightLogPath)
Expand All @@ -331,7 +352,9 @@ var _ = Describe("Container Certification Submitter", func() {
})

It("should throw an error", func() {
err := sbmt.Submit(testcontext)
scratchContext := policy.NewContext(testcontext, policy.PolicyContainer)

err := sbmt.Submit(scratchContext)
Expect(err).To(HaveOccurred())
})
})
Expand All @@ -355,7 +378,9 @@ var _ = Describe("Container Certification Submitter", func() {
})

It("should throw an error finalizing the submission", func() {
err := sbmt.Submit(testcontext)
scratchContext := policy.NewContext(testcontext, policy.PolicyContainer)

err := sbmt.Submit(scratchContext)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("unable to finalize data"))
})
Expand All @@ -366,7 +391,9 @@ var _ = Describe("Container Certification Submitter", func() {
fakePC.setSRFuncSubmitSuccessfully("", "")
})
It("should not throw an error", func() {
err := sbmt.Submit(testcontext)
scratchContext := policy.NewContext(testcontext, policy.PolicyContainer)

err := sbmt.Submit(scratchContext)
Expect(err).ToNot(HaveOccurred())
})
})
Expand Down
22 changes: 22 additions & 0 deletions internal/policy/policy.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package policy

import "context"

type Policy = string

const (
Expand All @@ -8,3 +10,23 @@ const (
PolicyScratch Policy = "scratch"
PolicyRoot Policy = "root"
)

// NewContext adds Policy p to the context ctx.
func NewContext(ctx context.Context, p Policy) context.Context {
return context.WithValue(ctx, policyContextKey, p)
}

// FromContext returns the policy from the context, or empty string.
func FromContext(ctx context.Context) Policy {
p := ctx.Value(policyContextKey)
if policy, ok := p.(Policy); ok {
return policy
}

return ""
}

// contextKey is a key used to store/retrieve Policy in/from context.Context.
type contextKey string

const policyContextKey contextKey = "Policy"
20 changes: 11 additions & 9 deletions internal/pyxis/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"fmt"
"io"
"net/http"

"github.com/redhat-openshift-ecosystem/openshift-preflight/internal/policy"
)

// certificationInputBuilder facilitates the building of CertificationInput for
Expand All @@ -18,7 +20,7 @@ type certificationInputBuilder struct {
// NewCertificationInput accepts required values for submitting to Pyxis, and returns a CertificationInputBuilder for
// adding additional files as artifacts to the submission. The caller must call Finalize() in order to receive
// a *CertificationInput.
func NewCertificationInput(ctx context.Context, project *CertProject, opts ...certificationInputOption) (*CertificationInput, error) {
func NewCertificationInput(ctx context.Context, project *CertProject, opts ...CertificationInputOption) (*CertificationInput, error) {
if project == nil {
return nil, fmt.Errorf("a certification project was not provided and is required")
}
Expand All @@ -35,7 +37,7 @@ func NewCertificationInput(ctx context.Context, project *CertProject, opts ...ce
}
}

return b.finalize()
return b.finalize(ctx)
}

// Finalize runs a collection of safeguards to try to ensure we get a reliable
Expand All @@ -44,7 +46,7 @@ func NewCertificationInput(ctx context.Context, project *CertProject, opts ...ce
// unmodifiable CertificationInput.
//
// If any required values are not included, an error is thrown.
func (b *certificationInputBuilder) finalize() (*CertificationInput, error) {
func (b *certificationInputBuilder) finalize(ctx context.Context) (*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")
Expand All @@ -53,7 +55,7 @@ func (b *certificationInputBuilder) finalize() (*CertificationInput, error) {
return nil, fmt.Errorf("test results were not provided and are required")
}

if b.RpmManifest == nil {
if b.RpmManifest == nil && policy.FromContext(ctx) != policy.PolicyScratch {
return nil, fmt.Errorf("the RPM manifest was not provided and is required")
}

Expand All @@ -69,11 +71,11 @@ func (b *certificationInputBuilder) finalize() (*CertificationInput, error) {
return &b.CertificationInput, nil
}

type certificationInputOption func(*certificationInputBuilder) error
type CertificationInputOption func(*certificationInputBuilder) error

// WithCertImage adds a pyxis.CertImage from the passed io.Reader to the CertificationInput.
// Errors are logged, but will not halt execution.
func WithCertImage(r io.Reader) certificationInputOption {
func WithCertImage(r io.Reader) CertificationInputOption {
return func(b *certificationInputBuilder) error {
if err := b.storeCertImage(r); err != nil {
return fmt.Errorf("cert image could not be stored: %v", err)
Expand All @@ -84,7 +86,7 @@ func WithCertImage(r io.Reader) certificationInputOption {

// WithPreflightResults adds formatters.UserResponse from the passed io.Reader to the CertificationInput.
// Errors are logged, but will not halt execution.
func WithPreflightResults(r io.Reader) certificationInputOption {
func WithPreflightResults(r io.Reader) CertificationInputOption {
return func(b *certificationInputBuilder) error {
if err := b.storePreflightResults(r); err != nil {
return fmt.Errorf("preflight results could not be stored: %v", err)
Expand All @@ -95,7 +97,7 @@ func WithPreflightResults(r io.Reader) certificationInputOption {

// WithRPMManifest adds the pyxis.RPMManifest from the passed io.Reader to the CertificationInput.
// Errors are logged, but will not halt execution.
func WithRPMManifest(r io.Reader) certificationInputOption {
func WithRPMManifest(r io.Reader) CertificationInputOption {
return func(b *certificationInputBuilder) error {
if err := b.storeRPMManifest(r); err != nil {
return fmt.Errorf("rpm manifest could not be stored: %v", err)
Expand All @@ -109,7 +111,7 @@ func WithRPMManifest(r io.Reader) certificationInputOption {
// but will not halt execution. The filename parameter will be used as the Filename
// field in the Artifact struct. It will be sent as is. It should prepresent only the
// base filename.
func WithArtifact(r io.Reader, filename string) certificationInputOption {
func WithArtifact(r io.Reader, filename string) CertificationInputOption {
return func(b *certificationInputBuilder) error {
bts, err := io.ReadAll(r)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion test/containerfiles/container-passes.Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ RUN useradd preflightuser

COPY --chown=preflightuser:preflightuser example-license.txt /licenses/

LABEL name="preflight test image" \
LABEL name="preflight test image container-policy" \
vendor="preflight test vendor" \
version="1" \
release="1" \
Expand Down