From be354887eae1370f978b4de7fd96e8be1b96c9c3 Mon Sep 17 00:00:00 2001 From: Philip Laine Date: Wed, 7 Aug 2024 16:20:59 +0200 Subject: [PATCH] refactor: FindImages to return errors immediatly Signed-off-by: Philip Laine --- src/pkg/packager/prepare.go | 51 ++++++++--------------------- src/test/e2e/13_find_images_test.go | 3 +- 2 files changed, 15 insertions(+), 39 deletions(-) diff --git a/src/pkg/packager/prepare.go b/src/pkg/packager/prepare.go index 214258634a..91cab2457b 100644 --- a/src/pkg/packager/prepare.go +++ b/src/pkg/packager/prepare.go @@ -54,21 +54,19 @@ func (p *Packager) FindImages(ctx context.Context) (map[string][]string, error) } message.Note(fmt.Sprintf("Using build directory %s", p.cfg.CreateOpts.BaseDir)) - c := creator.NewPackageCreator(p.cfg.CreateOpts, cwd) - if err := helpers.CreatePathAndCopy(layout.ZarfYAML, p.layout.ZarfYAML); err != nil { return nil, err } + c := creator.NewPackageCreator(p.cfg.CreateOpts, cwd) pkg, warnings, err := c.LoadPackageDefinition(ctx, p.layout) if err != nil { return nil, err } - p.cfg.Pkg = pkg - for _, warning := range warnings { message.Warn(warning) } + p.cfg.Pkg = pkg return p.findImages(ctx) } @@ -79,8 +77,6 @@ func (p *Packager) findImages(ctx context.Context) (imgMap map[string][]string, whyImage := p.cfg.FindImagesOpts.Why imagesMap := make(map[string][]string) - erroredCharts := []string{} - erroredCosignLookups := []string{} whyResources := []string{} for _, component := range p.cfg.Pkg.Components { @@ -128,8 +124,7 @@ func (p *Packager) findImages(ctx context.Context) (imgMap map[string][]string, for _, repo := range component.Repos { matches := strings.Split(repo, "@") if len(matches) < 2 { - message.Warnf("Cannot convert git repo %s to helm chart without a version tag", repo) - continue + return nil, fmt.Errorf("cannot convert the Git repository %s to a Helm chart without a version tag", repo) } // Trim the first char to match how the packager expects it, this is messy,need to clean up better @@ -185,9 +180,7 @@ func (p *Packager) findImages(ctx context.Context) (imgMap map[string][]string, // Generate helm templates for this chart chartTemplate, chartValues, err := helmCfg.TemplateChart(ctx) if err != nil { - message.WarnErrf(err, "Problem rendering the helm template for %s: %s", chart.Name, err.Error()) - erroredCharts = append(erroredCharts, chart.Name) - continue + return nil, fmt.Errorf("could not render the Helm template for chart %s: %w", chart.Name, err) } // Break the template into separate resources @@ -198,9 +191,7 @@ func (p *Packager) findImages(ctx context.Context) (imgMap map[string][]string, annotatedImages, err := helm.FindAnnotatedImagesForChart(chartTarball, chartValues) if err != nil { - message.WarnErrf(err, "Problem looking for image annotations for %s: %s", chart.URL, err.Error()) - erroredCharts = append(erroredCharts, chart.URL) - continue + return nil, fmt.Errorf("could not look up image annotations for chart URL %s: %w", chart.URL, err) } for _, image := range annotatedImages { matchedImages[image] = true @@ -210,7 +201,7 @@ func (p *Packager) findImages(ctx context.Context) (imgMap map[string][]string, if whyImage != "" { whyResourcesChart, err := findWhyResources(yamls, whyImage, component.Name, chart.Name, true) if err != nil { - message.WarnErrf(err, "Error finding why resources for chart %s: %s", chart.Name, err.Error()) + return nil, fmt.Errorf("could not determine why resource for the chart %s: %w", chart.Name, err) } whyResources = append(whyResources, whyResourcesChart...) } @@ -250,20 +241,21 @@ func (p *Packager) findImages(ctx context.Context) (imgMap map[string][]string, // Read the contents of each file contents, err := os.ReadFile(f) if err != nil { - message.WarnErrf(err, "Unable to read the file %s", f) - continue + return nil, fmt.Errorf("could not read the file %s: %w", f, err) } // Break the manifest into separate resources - // TODO: Do not dogsled error - yamls, _ := utils.SplitYAML(contents) + yamls, err := utils.SplitYAML(contents) + if err != nil { + return nil, err + } resources = append(resources, yamls...) // Check if the --why flag is set and if it is process the manifests if whyImage != "" { whyResourcesManifest, err := findWhyResources(yamls, whyImage, component.Name, manifest.Name, false) if err != nil { - message.WarnErrf(err, "Error finding why resources for manifest %s: %s", manifest.Name, err.Error()) + return nil, fmt.Errorf("could not find why resources for manifest %s: %w", manifest.Name, err) } whyResources = append(whyResources, whyResourcesManifest...) } @@ -275,7 +267,7 @@ func (p *Packager) findImages(ctx context.Context) (imgMap map[string][]string, for _, resource := range resources { if matchedImages, maybeImages, err = p.processUnstructuredImages(resource, matchedImages, maybeImages); err != nil { - message.WarnErrf(err, "Problem processing K8s resource %s", resource.GetName()) + return nil, fmt.Errorf("could not process the Kubernetes resource %s: %w", resource.GetName(), err) } } @@ -325,8 +317,7 @@ func (p *Packager) findImages(ctx context.Context) (imgMap map[string][]string, spinner.Updatef("Looking up cosign artifacts for discovered images (%d/%d)", idx+1, len(imagesMap[component.Name])) cosignArtifacts, err := utils.GetCosignArtifacts(image) if err != nil { - message.WarnErrf(err, "Problem looking up cosign artifacts for %s: %s", image, err.Error()) - erroredCosignLookups = append(erroredCosignLookups, image) + return nil, fmt.Errorf("could not lookup the cosing artifacts for image %s: %w", image, err) } cosignArtifactList = append(cosignArtifactList, cosignArtifacts...) } @@ -353,20 +344,6 @@ func (p *Packager) findImages(ctx context.Context) (imgMap map[string][]string, fmt.Println(componentDefinition) - if len(erroredCharts) > 0 || len(erroredCosignLookups) > 0 { - errMsg := "" - if len(erroredCharts) > 0 { - errMsg = fmt.Sprintf("the following charts had errors: %s", erroredCharts) - } - if len(erroredCosignLookups) > 0 { - if errMsg != "" { - errMsg += "\n" - } - errMsg += fmt.Sprintf("the following images errored on cosign lookups: %s", erroredCosignLookups) - } - return imagesMap, fmt.Errorf(errMsg) - } - return imagesMap, nil } diff --git a/src/test/e2e/13_find_images_test.go b/src/test/e2e/13_find_images_test.go index 39f6691b16..6d01bdd569 100644 --- a/src/test/e2e/13_find_images_test.go +++ b/src/test/e2e/13_find_images_test.go @@ -42,8 +42,7 @@ func TestFindImages(t *testing.T) { // Test `zarf prepare find-images` with `--kube-version` specified and less than than the declared minimum (v1.21.0) stdOut, stdErr, err = e2e.Zarf(t, "prepare", "find-images", "--kube-version=v1.20.0", "src/test/packages/00-kube-version-override") require.Error(t, err, stdOut, stdErr) - require.Contains(t, stdErr, "Problem rendering the helm template for cert-manager", "The kubeVersion declaration should prevent this from templating") - require.Contains(t, stdErr, "following charts had errors: [cert-manager]", "Zarf should print an ending error message") + require.Contains(t, stdErr, "could not render the Helm template for chart cert-manager", "The kubeVersion declaration should prevent this from templating") }) t.Run("zarf dev find-images with helm or manifest vars", func(t *testing.T) {