From 45f3cfd320c3b50a45bf90d40871517219ca1cc9 Mon Sep 17 00:00:00 2001 From: "Brad P. Crochet" Date: Fri, 4 Aug 2023 09:44:10 -0400 Subject: [PATCH] Refactor CraneEngine to craneEngine Since we have a New(...) constructor function, there is no need for CraneEngine to be exported. Also, no module outside of the engine package uses the fields of craneEngine, so there is no need for them to be exported either. Signed-off-by: Brad P. Crochet --- internal/engine/engine.go | 76 +++++++++++++++++----------------- internal/engine/engine_test.go | 28 ++++++------- 2 files changed, 52 insertions(+), 52 deletions(-) diff --git a/internal/engine/engine.go b/internal/engine/engine.go index d8b150a5..47ed4e93 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -48,45 +48,45 @@ func New(ctx context.Context, checks []check.Check, kubeconfig []byte, cfg runtime.Config, -) (CraneEngine, error) { - return CraneEngine{ - Kubeconfig: kubeconfig, - DockerConfig: cfg.DockerConfig, - Image: cfg.Image, - Checks: checks, - IsBundle: cfg.Bundle, - IsScratch: cfg.Scratch, - Platform: cfg.Platform, - Insecure: cfg.Insecure, +) (craneEngine, error) { + return craneEngine{ + kubeconfig: kubeconfig, + dockerConfig: cfg.DockerConfig, + image: cfg.Image, + checks: checks, + isBundle: cfg.Bundle, + isScratch: cfg.Scratch, + platform: cfg.Platform, + insecure: cfg.Insecure, }, nil } // CraneEngine implements a certification.CheckEngine, and leverage crane to interact with // the container registry and target image. -type CraneEngine struct { +type craneEngine struct { // Kubeconfig is a byte slice containing a valid Kubeconfig to be used by checks. - Kubeconfig []byte + kubeconfig []byte // DockerConfig is the credential required to pull the image. - DockerConfig string + dockerConfig string // Image is what is being tested, and should contain the // fully addressable path (including registry, namespaces, etc) // to the image - Image string + image string // Checks is an array of all checks to be executed against // the image provided. - Checks []check.Check + checks []check.Check // Platform is the container platform to use. E.g. amd64. - Platform string + platform string // IsBundle is an indicator that the asset is a bundle. - IsBundle bool + isBundle bool // IsScratch is an indicator that the asset is a scratch image - IsScratch bool + isScratch bool // Insecure controls whether to allow an insecure connection to // the registry crane connects with. - Insecure bool + insecure bool imageRef image.ImageReference results certification.Results @@ -98,28 +98,28 @@ func export(img cranev1.Image, w io.Writer) error { return err } -func (c *CraneEngine) CranePlatform() string { - return c.Platform +func (c *craneEngine) CranePlatform() string { + return c.platform } -func (c *CraneEngine) CraneDockerConfig() string { - return c.DockerConfig +func (c *craneEngine) CraneDockerConfig() string { + return c.dockerConfig } -func (c *CraneEngine) CraneInsecure() bool { - return c.Insecure +func (c *craneEngine) CraneInsecure() bool { + return c.insecure } -var _ option.CraneConfig = &CraneEngine{} +var _ option.CraneConfig = &craneEngine{} -func (c *CraneEngine) ExecuteChecks(ctx context.Context) error { +func (c *craneEngine) ExecuteChecks(ctx context.Context) error { logger := logr.FromContextOrDiscard(ctx) - logger.Info("target image", "image", c.Image) + logger.Info("target image", "image", c.image) // pull the image and save to fs logger.V(log.DBG).Info("pulling image from target registry") options := option.GenerateCraneOptions(ctx, c) - img, err := crane.Pull(c.Image, options...) + img, err := crane.Pull(c.image, options...) if err != nil { return fmt.Errorf("failed to pull remote container: %v", err) } @@ -172,14 +172,14 @@ func (c *CraneEngine) ExecuteChecks(ctx context.Context) error { return fmt.Errorf("failed to drain io reader: %v", err) } - reference, err := name.ParseReference(c.Image) + reference, err := name.ParseReference(c.image) if err != nil { return fmt.Errorf("image uri could not be parsed: %v", err) } // store the image internals in the engine image reference to pass to validations. c.imageRef = image.ImageReference{ - ImageURI: c.Image, + ImageURI: c.image, ImageFSPath: containerFSPath, ImageInfo: img, ImageRegistry: reference.Context().RegistryStr(), @@ -191,15 +191,15 @@ func (c *CraneEngine) ExecuteChecks(ctx context.Context) error { return fmt.Errorf("could not write cert image: %v", err) } - if !c.IsScratch { + if !c.isScratch { if err := writeRPMManifest(ctx, containerFSPath); err != nil { return fmt.Errorf("could not write rpm manifest: %v", err) } } - if c.IsBundle { + if c.isBundle { // Record test cluster version - version, err := openshift.GetOpenshiftClusterVersion(ctx, c.Kubeconfig) + version, err := openshift.GetOpenshiftClusterVersion(ctx, c.kubeconfig) if err != nil { logger.Error(err, "could not determine test cluster version") } @@ -211,8 +211,8 @@ func (c *CraneEngine) ExecuteChecks(ctx context.Context) error { // execute checks logger.V(log.DBG).Info("executing checks") - for _, check := range c.Checks { - c.results.TestedImage = c.Image + for _, check := range c.checks { + c.results.TestedImage = c.image logger.V(log.DBG).Info("running check", "check", check.Name()) if check.Metadata().Level == "optional" { @@ -246,7 +246,7 @@ func (c *CraneEngine) ExecuteChecks(ctx context.Context) error { c.results.PassedOverall = true } - if c.IsBundle { // for operators: + if c.isBundle { // for operators: // hash the contents of the bundle. md5sum, err := generateBundleHash(ctx, c.imageRef.ImageFSPath) if err != nil { @@ -354,7 +354,7 @@ func generateBundleHash(ctx context.Context, bundlePath string) (string, error) } // Results will return the results of check execution. -func (c *CraneEngine) Results(ctx context.Context) certification.Results { +func (c *craneEngine) Results(ctx context.Context) certification.Results { return c.results } diff --git a/internal/engine/engine_test.go b/internal/engine/engine_test.go index 9d6aafe7..3d2dfb8e 100644 --- a/internal/engine/engine_test.go +++ b/internal/engine/engine_test.go @@ -31,7 +31,7 @@ import ( var _ = Describe("Execute Checks tests", func() { var src string - var engine CraneEngine + var engine craneEngine var testcontext context.Context var s *httptest.Server var u *url.URL @@ -109,18 +109,18 @@ var _ = Describe("Execute Checks tests", func() { ) emptyConfig := runtime.Config{} - engine = CraneEngine{ - DockerConfig: emptyConfig.DockerConfig, - Image: src, - Checks: []check.Check{ + engine = craneEngine{ + dockerConfig: emptyConfig.DockerConfig, + image: src, + checks: []check.Check{ goodCheck, errorCheck, failedCheck, optionalCheckPassing, optionalCheckFailing, }, - IsBundle: false, - IsScratch: false, + isBundle: false, + isScratch: false, } }) Context("Run the checks", func() { @@ -134,7 +134,7 @@ var _ = Describe("Execute Checks tests", func() { }) Context("it is a bundle", func() { It("should succeed and generate a bundle hash", func() { - engine.IsBundle = true + engine.isBundle = true err := engine.ExecuteChecks(testcontext) Expect(err).ToNot(HaveOccurred()) Expect(engine.results.CertificationHash).ToNot(BeEmpty()) @@ -142,7 +142,7 @@ var _ = Describe("Execute Checks tests", func() { }) Context("the image is invalid", func() { It("should throw a crane error on pull", func() { - engine.Image = "does.not/exist/anywhere:ever" + engine.image = "does.not/exist/anywhere:ever" err := engine.ExecuteChecks(testcontext) Expect(err).To(HaveOccurred()) }) @@ -164,7 +164,7 @@ var _ = Describe("Execute Checks tests", func() { Expect(err).ToNot(HaveOccurred()) }) It("should just hang forever err and hash mean nothing", func() { - engine.IsBundle = true + engine.isBundle = true err := engine.ExecuteChecks(testcontext) Expect(err).ToNot(HaveOccurred()) Expect(engine.results.CertificationHash).ToNot(BeEmpty()) @@ -172,7 +172,7 @@ var _ = Describe("Execute Checks tests", func() { }) Context("it is a bundle made with tar layer", func() { BeforeEach(func() { - engine.IsBundle = true + engine.isBundle = true var buf bytes.Buffer @@ -189,7 +189,7 @@ var _ = Describe("Execute Checks tests", func() { Expect(err).ToNot(HaveOccurred()) }) It("should succeed and generate a bundle hash", func() { - engine.IsBundle = true + engine.isBundle = true err := engine.ExecuteChecks(testcontext) Expect(err).ToNot(HaveOccurred()) Expect(engine.results.CertificationHash).ToNot(BeEmpty()) @@ -197,7 +197,7 @@ var _ = Describe("Execute Checks tests", func() { }) Context("it is a bundle made and one of the layers is not a tar", func() { BeforeEach(func() { - engine.IsBundle = true + engine.isBundle = true want := []byte(`{"foo":"bar"}`) layer := static.NewLayer(want, types.MediaType("application/json")) @@ -210,7 +210,7 @@ var _ = Describe("Execute Checks tests", func() { Expect(err).ToNot(HaveOccurred()) }) It("should throw an EOF error on untar", func() { - engine.IsBundle = true + engine.isBundle = true err := engine.ExecuteChecks(testcontext) Expect(err).To(HaveOccurred()) Expect(engine.results.CertificationHash).To(BeEmpty())