From ccc80ae3c3ff32d434bedde1009e1c265379f1c2 Mon Sep 17 00:00:00 2001 From: Austin Abro <37223396+AustinAbro321@users.noreply.github.com> Date: Thu, 25 Apr 2024 13:48:10 -0400 Subject: [PATCH] fix: error on create if an index sha is used (#2429) ## Description Errors on create if an index sha is used and gives users options of all the other platforms they can use ## Related Issue Relates to #2425, #2394 ## Checklist before merging - [X] Test, docs, adr added or updated as needed - [ ] [Contributor Guide Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow) followed --------- Co-authored-by: razzle --- go.mod | 4 +- go.sum | 6 ++- .../docs/commands/zarf_tools_yq_eval.md | 2 +- src/config/config.go | 9 ++-- src/config/lang/english.go | 11 ++--- src/internal/packager/images/pull.go | 26 +++++++++++- src/pkg/packager/creator/normal.go | 9 +++- src/test/e2e/00_use_cli_test.go | 28 +++++++++---- src/test/e2e/14_create_sha_index_test.go | 41 +++++++++++++++++++ src/test/e2e/36_custom_retries_test.go | 3 +- .../14-index-sha/image-index/zarf.yaml | 9 ++++ .../14-index-sha/manifest-list/zarf.yaml | 9 ++++ 12 files changed, 129 insertions(+), 28 deletions(-) create mode 100644 src/test/e2e/14_create_sha_index_test.go create mode 100644 src/test/packages/14-index-sha/image-index/zarf.yaml create mode 100644 src/test/packages/14-index-sha/manifest-list/zarf.yaml diff --git a/go.mod b/go.mod index 9e67298f72..4be2e73cf0 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/anchore/clio v0.0.0-20240307182142-fb5fc4c9db3c github.com/anchore/stereoscope v0.0.1 github.com/anchore/syft v0.100.0 - github.com/defenseunicorns/pkg/helpers v1.0.0 + github.com/defenseunicorns/pkg/helpers v1.1.1 github.com/defenseunicorns/pkg/oci v0.0.1 github.com/derailed/k9s v0.31.7 github.com/distribution/reference v0.5.0 @@ -377,7 +377,7 @@ require ( github.com/opencontainers/selinux v1.11.0 // indirect github.com/opentracing/opentracing-go v1.2.0 // indirect github.com/openvex/go-vex v0.2.5 // indirect - github.com/otiai10/copy v1.14.0 // indirect + github.com/otiai10/copy v1.14.0 github.com/owenrumney/go-sarif v1.1.2-0.20231003122901-1000f5e05554 // indirect github.com/package-url/packageurl-go v0.1.1 // indirect github.com/pborman/indent v1.2.1 // indirect diff --git a/go.sum b/go.sum index 5a5ab0acbf..a45dbd3c96 100644 --- a/go.sum +++ b/go.sum @@ -595,8 +595,10 @@ github.com/daviddengcn/go-colortext v1.0.0 h1:ANqDyC0ys6qCSvuEK7l3g5RaehL/Xck9EX github.com/daviddengcn/go-colortext v1.0.0/go.mod h1:zDqEI5NVUop5QPpVJUxE9UO10hRnmkD5G4Pmri9+m4c= github.com/defenseunicorns/gojsonschema v0.0.0-20231116163348-e00f069122d6 h1:gwevOZ0fxT2nzM9hrtdPbsiOHjFqDRIYMzJHba3/G6Q= github.com/defenseunicorns/gojsonschema v0.0.0-20231116163348-e00f069122d6/go.mod h1:StKLYMmPj1R5yIs6CK49EkcW1TvUYuw5Vri+LRk7Dy8= -github.com/defenseunicorns/pkg/helpers v1.0.0 h1:0o3Rs+J/g0UemZHcENBS1Z2Qw2y4FIUUrGs75iEyPb4= -github.com/defenseunicorns/pkg/helpers v1.0.0/go.mod h1:F4S5VZLDrlNWQKklzv4v9tFWjjZNhxJ1gT79j4XiLwk= +github.com/defenseunicorns/pkg/helpers v1.1.0 h1:VU8Cr3IGFEDuZrfavxmo6YXsh5FZXhehy4clKXrHNGk= +github.com/defenseunicorns/pkg/helpers v1.1.0/go.mod h1:F4S5VZLDrlNWQKklzv4v9tFWjjZNhxJ1gT79j4XiLwk= +github.com/defenseunicorns/pkg/helpers v1.1.1 h1:p3pKeK5SeFaoZUJZIX9sEsJqX1CGGMS8OpQMPgJtSqM= +github.com/defenseunicorns/pkg/helpers v1.1.1/go.mod h1:F4S5VZLDrlNWQKklzv4v9tFWjjZNhxJ1gT79j4XiLwk= github.com/defenseunicorns/pkg/oci v0.0.1 h1:EFRp3NeiwzhOWKpQ6mAxi0l9chnrAvDcIgjMr0o0fkM= github.com/defenseunicorns/pkg/oci v0.0.1/go.mod h1:zVBgRjckEAhfdvbnQrnfOP/3M/GYJkIgWtJtY7pjYdo= github.com/deitch/magic v0.0.0-20230404182410-1ff89d7342da h1:ZOjWpVsFZ06eIhnh4mkaceTiVoktdU67+M7KDHJ268M= diff --git a/site/src/content/docs/commands/zarf_tools_yq_eval.md b/site/src/content/docs/commands/zarf_tools_yq_eval.md index 58921184ab..215184cf00 100644 --- a/site/src/content/docs/commands/zarf_tools_yq_eval.md +++ b/site/src/content/docs/commands/zarf_tools_yq_eval.md @@ -41,7 +41,7 @@ cat file2.yml | zarf tools yq e '.a.b' file1.yml - file3.yml ## Note that editing an empty file does not work. zarf tools yq e -n '.a.b.c = "cat"' -# Update a file inplace +# Update a file in place zarf tools yq e '.a.b = "cool"' -i file.yaml ``` diff --git a/src/config/config.go b/src/config/config.go index b8e3e2d386..63396255b0 100644 --- a/src/config/config.go +++ b/src/config/config.go @@ -134,12 +134,11 @@ func GetCraneOptions(insecure bool, archs ...string) []crane.Option { options = append(options, crane.Insecure, crane.WithTransport(roundTripper)) } - // Add the image platform info + if archs != nil { + options = append(options, crane.WithPlatform(&v1.Platform{OS: "linux", Architecture: GetArch(archs...)})) + } + options = append(options, - crane.WithPlatform(&v1.Platform{ - OS: "linux", - Architecture: GetArch(archs...), - }), crane.WithUserAgent("zarf"), crane.WithNoClobber(true), // TODO: (@WSTARR) this is set to limit pushes to registry pods and reduce the likelihood that crane will get stuck. diff --git a/src/config/lang/english.go b/src/config/lang/english.go index b5350deb7f..1d1847d207 100644 --- a/src/config/lang/english.go +++ b/src/config/lang/english.go @@ -519,7 +519,7 @@ cat file2.yml | zarf tools yq e '.a.b' file1.yml - file3.yml ## Note that editing an empty file does not work. zarf tools yq e -n '.a.b.c = "cat"' -# Update a file inplace +# Update a file in place zarf tools yq e '.a.b = "cool"' -i file.yaml ` CmdToolsMonitorShort = "Launches a terminal UI to monitor the connected cluster using K9s." @@ -728,10 +728,11 @@ const ( // Collection of reusable error messages. var ( - ErrInitNotFound = errors.New("this command requires a zarf-init package, but one was not found on the local system. Re-run the last command again without '--confirm' to download the package") - ErrUnableToCheckArch = errors.New("unable to get the configured cluster's architecture") - ErrInterrupt = errors.New("execution cancelled due to an interrupt") - ErrUnableToGetPackages = errors.New("unable to load the Zarf Package data from the cluster") + ErrInitNotFound = errors.New("this command requires a zarf-init package, but one was not found on the local system. Re-run the last command again without '--confirm' to download the package") + ErrUnableToCheckArch = errors.New("unable to get the configured cluster's architecture") + ErrInterrupt = errors.New("execution cancelled due to an interrupt") + ErrUnableToGetPackages = errors.New("unable to load the Zarf Package data from the cluster") + ErrUnsupportedImageType = errors.New("zarf does not currently support image indexes or docker manifest lists") ) // Collection of reusable warn messages. diff --git a/src/internal/packager/images/pull.go b/src/internal/packager/images/pull.go index df839e4f07..2db0200d53 100644 --- a/src/internal/packager/images/pull.go +++ b/src/internal/packager/images/pull.go @@ -6,12 +6,14 @@ package images import ( "context" + "encoding/json" "fmt" "path/filepath" "strings" "github.com/defenseunicorns/pkg/helpers" "github.com/defenseunicorns/zarf/src/config" + "github.com/defenseunicorns/zarf/src/config/lang" "github.com/defenseunicorns/zarf/src/pkg/layout" "github.com/defenseunicorns/zarf/src/pkg/message" "github.com/defenseunicorns/zarf/src/pkg/transform" @@ -25,6 +27,7 @@ import ( "github.com/google/go-containerregistry/pkg/v1/empty" clayout "github.com/google/go-containerregistry/pkg/v1/layout" "github.com/google/go-containerregistry/pkg/v1/partial" + ctypes "github.com/google/go-containerregistry/pkg/v1/types" "github.com/moby/moby/client" ) @@ -271,7 +274,7 @@ func (i *ImageConfig) PullImage(src string, spinner *message.Spinner) (img v1.Im if err != nil { return nil, false, err } - } else if _, err := crane.Manifest(src, config.GetCraneOptions(i.Insecure, i.Architectures...)...); err != nil { + } else if desc, err := crane.Get(src, config.GetCraneOptions(i.Insecure)...); err != nil { // If crane is unable to pull the image, try to load it from the local docker daemon. message.Notef("Falling back to local 'docker' images, failed to find the manifest on a remote: %s", err.Error()) @@ -308,6 +311,27 @@ func (i *ImageConfig) PullImage(src string, spinner *message.Spinner) (img v1.Im return nil, false, fmt.Errorf("failed to load image from docker daemon: %w", err) } } else { + refInfo, err := transform.ParseImageRef(src) + if err != nil { + return nil, false, err + } + // Check if we have an image index or manifest list and if so error out + if refInfo.Digest != "" && (desc.MediaType == ctypes.OCIImageIndex || desc.MediaType == ctypes.DockerManifestList) { + var idx v1.IndexManifest + if err := json.Unmarshal(desc.Manifest, &idx); err != nil { + return nil, false, fmt.Errorf("%w: %w", lang.ErrUnsupportedImageType, err) + } + imageOptions := "please select one of the images below based on your platform to use instead" + imageBaseName := refInfo.Name + if refInfo.Tag != "" { + imageBaseName = fmt.Sprintf("%s:%s", imageBaseName, refInfo.Tag) + } + for _, manifest := range idx.Manifests { + imageOptions = fmt.Sprintf("%s\n %s@%s for platform %s", imageOptions, imageBaseName, manifest.Digest, manifest.Platform) + } + return nil, false, fmt.Errorf("%w: %s", lang.ErrUnsupportedImageType, imageOptions) + } + // Manifest was found, so use crane to pull the image. if img, err = crane.Pull(src, config.GetCraneOptions(i.Insecure, i.Architectures...)...); err != nil { return nil, false, fmt.Errorf("failed to pull image: %w", err) diff --git a/src/pkg/packager/creator/normal.go b/src/pkg/packager/creator/normal.go index 9e742d6cee..fed5003380 100644 --- a/src/pkg/packager/creator/normal.go +++ b/src/pkg/packager/creator/normal.go @@ -177,6 +177,7 @@ func (pc *PackageCreator) Assemble(dst *layout.PackagePaths, components []types. var pulled []images.ImgInfo var err error + ctx, cancel := context.WithCancel(context.TODO()) doPull := func() error { imgConfig := images.ImageConfig{ ImagesPath: dst.Images.Base, @@ -187,11 +188,15 @@ func (pc *PackageCreator) Assemble(dst *layout.PackagePaths, components []types. } pulled, err = imgConfig.PullAll() + if errors.Is(err, lang.ErrUnsupportedImageType) { + cancel() + } + return err } - if err := helpers.Retry(doPull, 3, 5*time.Second, message.Warnf); err != nil { - return fmt.Errorf("unable to pull images after 3 attempts: %w", err) + if err := helpers.RetryWithContext(ctx, doPull, 3, 5*time.Second, message.Warnf); err != nil { + return fmt.Errorf("unable to pull images: %w", err) } for _, imgInfo := range pulled { diff --git a/src/test/e2e/00_use_cli_test.go b/src/test/e2e/00_use_cli_test.go index efdd0f3f82..6b5a0e387c 100644 --- a/src/test/e2e/00_use_cli_test.go +++ b/src/test/e2e/00_use_cli_test.go @@ -13,6 +13,7 @@ import ( "testing" "github.com/defenseunicorns/pkg/helpers" + "github.com/otiai10/copy" "github.com/stretchr/testify/require" ) @@ -216,21 +217,32 @@ func TestUseCLI(t *testing.T) { t.Run("zarf tools yq should function appropriately across different uses", func(t *testing.T) { t.Parallel() - file := "src/test/packages/00-yq-checks/file1.yaml" - otherFile := "src/test/packages/00-yq-checks/file2.yaml" + tmpdir := t.TempDir() + originalPath := "src/test/packages/00-yq-checks" + + originalFile := filepath.Join(originalPath, "file1.yaml") + originalOtherFile := filepath.Join(originalPath, "file2.yaml") + + file := filepath.Join(tmpdir, "file1.yaml") + otherFile := filepath.Join(tmpdir, "file2.yaml") + + err := copy.Copy(originalFile, file) + require.NoError(t, err) + err = copy.Copy(originalOtherFile, otherFile) + require.NoError(t, err) // Test that yq can eval properly _, stdErr, err := e2e.Zarf("tools", "yq", "eval", "-i", `.items[1].name = "renamed-item"`, file) require.NoError(t, err, stdErr) - stdOut, stdErr, err := e2e.Zarf("tools", "yq", ".items[1].name", file) - require.NoError(t, err, stdErr) + stdOut, _, err := e2e.Zarf("tools", "yq", ".items[1].name", file) + require.NoError(t, err) require.Contains(t, stdOut, "renamed-item") // Test that yq ea can be used properly - _, stdErr, err = e2e.Zarf("tools", "yq", "eval-all", "-i", `. as $doc ireduce ({}; .items += $doc.items)`, file, otherFile) - require.NoError(t, err, stdErr) - stdOut, stdErr, err = e2e.Zarf("tools", "yq", "e", ".items | length", file) - require.NoError(t, err, stdErr) + _, _, err = e2e.Zarf("tools", "yq", "eval-all", "-i", `. as $doc ireduce ({}; .items += $doc.items)`, file, otherFile) + require.NoError(t, err) + stdOut, _, err = e2e.Zarf("tools", "yq", "e", ".items | length", file) + require.NoError(t, err) require.Equal(t, "4\n", stdOut) }) } diff --git a/src/test/e2e/14_create_sha_index_test.go b/src/test/e2e/14_create_sha_index_test.go new file mode 100644 index 0000000000..9dee622c85 --- /dev/null +++ b/src/test/e2e/14_create_sha_index_test.go @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: 2021-Present The Zarf Authors + +// Package test provides e2e tests for Zarf. +package test + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestCreateIndexShaErrors(t *testing.T) { + t.Log("E2E: CreateIndexShaErrors") + + testCases := []struct { + name string + packagePath string + expectedImageInStderr string + }{ + { + name: "Image Index", + packagePath: "src/test/packages/14-index-sha/image-index", + expectedImageInStderr: "ghcr.io/defenseunicorns/zarf/agent:v0.32.6@sha256:b3fabdc7d4ecd0f396016ef78da19002c39e3ace352ea0ae4baa2ce9d5958376", + }, + { + name: "Manifest List", + packagePath: "src/test/packages/14-index-sha/manifest-list", + expectedImageInStderr: "docker.io/defenseunicorns/zarf-game@sha256:f78e442f0f3eb3e9459b5ae6b1a8fda62f8dfe818112e7d130a4e8ae72b3cbff", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + _, stderr, err := e2e.Zarf("package", "create", tc.packagePath, "--confirm") + require.Error(t, err) + require.Contains(t, stderr, tc.expectedImageInStderr) + }) + } + +} diff --git a/src/test/e2e/36_custom_retries_test.go b/src/test/e2e/36_custom_retries_test.go index e66a6ff435..a0f33b94ac 100644 --- a/src/test/e2e/36_custom_retries_test.go +++ b/src/test/e2e/36_custom_retries_test.go @@ -27,7 +27,6 @@ func TestRetries(t *testing.T) { stdOut, stdErr, err = e2e.Zarf("package", "deploy", path.Join(tmpDir, pkgName), "--retries", "2", "--timeout", "3s", "--tmpdir", tmpDir, "--confirm") require.Error(t, err, stdOut, stdErr) - require.Contains(t, stdErr, "Retrying (1/2) in 5s:") - require.Contains(t, stdErr, "Retrying (2/2) in 10s:") + require.Contains(t, stdErr, "Retrying in 5s") require.Contains(t, stdErr, "unable to install chart after 2 attempts") } diff --git a/src/test/packages/14-index-sha/image-index/zarf.yaml b/src/test/packages/14-index-sha/image-index/zarf.yaml new file mode 100644 index 0000000000..a29c4380b9 --- /dev/null +++ b/src/test/packages/14-index-sha/image-index/zarf.yaml @@ -0,0 +1,9 @@ +kind: ZarfPackageConfig +metadata: + name: image-index + +components: + - name: baseline + required: true + images: + - ghcr.io/defenseunicorns/zarf/agent:v0.32.6@sha256:05a82656df5466ce17c3e364c16792ae21ce68438bfe06eeab309d0520c16b48 diff --git a/src/test/packages/14-index-sha/manifest-list/zarf.yaml b/src/test/packages/14-index-sha/manifest-list/zarf.yaml new file mode 100644 index 0000000000..9d7ad76b20 --- /dev/null +++ b/src/test/packages/14-index-sha/manifest-list/zarf.yaml @@ -0,0 +1,9 @@ +kind: ZarfPackageConfig +metadata: + name: manifest-list + +components: + - name: baseline + required: true + images: + - defenseunicorns/zarf-game@sha256:0b694ca1c33afae97b7471488e07968599f1d2470c629f76af67145ca64428af