Skip to content

Commit

Permalink
refactor: FindImages to return errors immediatly
Browse files Browse the repository at this point in the history 
Signed-off-by: Philip Laine <[email protected]>
  • Loading branch information
@phillebaba
phillebaba committed Aug 8, 2024
1 parent 02f2932 commit 29be298
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 81 deletions.
130 changes: 51 additions & 79 deletions src/pkg/packager/prepare.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,53 +54,42 @@ 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)
}

func (p *Packager) findImages(ctx context.Context) (imgMap map[string][]string, err error) {
repoHelmChartPath := p.cfg.FindImagesOpts.RepoHelmChartPath
kubeVersionOverride := p.cfg.FindImagesOpts.KubeVersionOverride
whyImage := p.cfg.FindImagesOpts.Why

imagesMap := make(map[string][]string)
erroredCharts := []string{}
erroredCosignLookups := []string{}
whyResources := []string{}

// TODO: Refactor to return output string instead of printing inside of function.
func (p *Packager) findImages(ctx context.Context) (map[string][]string, error) {
for _, component := range p.cfg.Pkg.Components {
if len(component.Repos) > 0 && repoHelmChartPath == "" {
if len(component.Repos) > 0 && p.cfg.FindImagesOpts.RepoHelmChartPath == "" {
message.Note("This Zarf package contains git repositories, " +
"if any repos contain helm charts you want to template and " +
"search for images, make sure to specify the helm chart path " +
"via the --repo-chart-path flag")
break
}
}

componentDefinition := "\ncomponents:\n"

if err := p.populatePackageVariableConfig(); err != nil {
return nil, fmt.Errorf("unable to set the active variables: %w", err)
}

// Set default builtin values so they exist in case any helm charts rely on them
registryInfo := types.RegistryInfo{Address: p.cfg.FindImagesOpts.RegistryURL}
err = registryInfo.FillInEmptyValues()
err := registryInfo.FillInEmptyValues()
if err != nil {
return nil, err
}
Expand All @@ -117,41 +106,34 @@ func (p *Packager) findImages(ctx context.Context) (imgMap map[string][]string,
ArtifactServer: artifactServer,
}

componentDefinition := "\ncomponents:\n"
imagesMap := map[string][]string{}
whyResources := []string{}
for _, component := range p.cfg.Pkg.Components {
if len(component.Charts)+len(component.Manifests)+len(component.Repos) < 1 {
// Skip if it doesn't have what we need
continue
}

if repoHelmChartPath != "" {
if p.cfg.FindImagesOpts.RepoHelmChartPath != "" {
// Also process git repos that have helm charts
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
repoHelmChartPath = strings.TrimPrefix(repoHelmChartPath, "/")

// If a repo helm chart path is specified,
component.Charts = append(component.Charts, v1alpha1.ZarfChart{
Name: repo,
URL: matches[0],
Version: matches[1],
GitPath: repoHelmChartPath,
// Trim the first char to match how the packager expects it, this is messy,need to clean up better
GitPath: strings.TrimPrefix(p.cfg.FindImagesOpts.RepoHelmChartPath, "/"),
})
}
}

// matchedImages holds the collection of images, reset per-component
matchedImages := make(imageMap)
maybeImages := make(imageMap)

// resources are a slice of generic structs that represent parsed K8s resources
var resources []*unstructured.Unstructured

componentPaths, err := p.layout.Components.Create(component)
if err != nil {
return nil, err
Expand All @@ -161,56 +143,60 @@ func (p *Packager) findImages(ctx context.Context) (imgMap map[string][]string,
return nil, err
}

resources := []*unstructured.Unstructured{}
matchedImages := imageMap{}
maybeImages := imageMap{}
for _, chart := range component.Charts {
valuesFilePaths, err := helpers.RecursiveFileList(componentPaths.Values, nil, false)
if err != nil {
return nil, err
}
for _, valueFilePath := range valuesFilePaths {
err := p.variableConfig.ReplaceTextTemplate(valueFilePath)
if err != nil {
return nil, err
}
}

// Generate helm templates for this chart
helmCfg := helm.New(
chart,
componentPaths.Charts,
componentPaths.Values,
helm.WithKubeVersion(kubeVersionOverride),
helm.WithKubeVersion(p.cfg.FindImagesOpts.KubeVersionOverride),
helm.WithVariableConfig(p.variableConfig),
)

err = helmCfg.PackageChart(ctx, component.DeprecatedCosignKeyPath)
if err != nil {
return nil, fmt.Errorf("unable to package the chart %s: %w", chart.Name, err)
}

valuesFilePaths, _ := helpers.RecursiveFileList(componentPaths.Values, nil, false)
for _, valueFilePath := range valuesFilePaths {
if err := p.variableConfig.ReplaceTextTemplate(valueFilePath); err != nil {
return nil, err
}
}

// 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
yamls, _ := utils.SplitYAML([]byte(chartTemplate))
yamls, err := utils.SplitYAML([]byte(chartTemplate))
if err != nil {
return nil, err
}
resources = append(resources, yamls...)

chartTarball := helm.StandardName(componentPaths.Charts, chart) + ".tgz"

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
}

// Check if the --why flag is set
if whyImage != "" {
whyResourcesChart, err := findWhyResources(yamls, whyImage, component.Name, chart.Name, true)
if p.cfg.FindImagesOpts.Why != "" {
whyResourcesChart, err := findWhyResources(yamls, p.cfg.FindImagesOpts.Why, 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...)
}
Expand Down Expand Up @@ -250,20 +236,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 p.cfg.FindImagesOpts.Why != "" {
whyResourcesManifest, err := findWhyResources(yamls, p.cfg.FindImagesOpts.Why, 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...)
}
Expand All @@ -275,7 +262,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)
}
}

Expand Down Expand Up @@ -325,8 +312,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...)
}
Expand All @@ -344,29 +330,15 @@ func (p *Packager) findImages(ctx context.Context) (imgMap map[string][]string,
}
}

if whyImage != "" {
if p.cfg.FindImagesOpts.Why != "" {
if len(whyResources) == 0 {
message.Warnf("image %q not found in any charts or manifests", whyImage)
message.Warnf("image %q not found in any charts or manifests", p.cfg.FindImagesOpts.Why)
}
return nil, nil
}

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
}

Expand Down
3 changes: 1 addition & 2 deletions src/test/e2e/13_find_images_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 29be298

Please sign in to comment.