From c681879d0592de9710edc78dfccce391177f933f Mon Sep 17 00:00:00 2001 From: razzle Date: Wed, 31 May 2023 19:13:40 -0500 Subject: [PATCH] Do not check composed filepath (#1758) ## Related Issue Fixes #1756 ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Other (security config, docs update, etc) ## Checklist before merging - [ ] Test, docs, adr added or updated as needed - [ ] [Contributor Guide Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow) followed --------- Signed-off-by: razzle Co-authored-by: Wayne Starr Co-authored-by: Wayne Starr --- src/internal/packager/helm/utils.go | 5 ++- src/pkg/packager/common.go | 14 +++--- src/pkg/packager/compose.go | 68 +++++------------------------ src/pkg/packager/create.go | 36 +++++++-------- src/test/e2e/51_oci_compose_test.go | 14 +++--- src/types/runtime.go | 11 +++++ 6 files changed, 59 insertions(+), 89 deletions(-) diff --git a/src/internal/packager/helm/utils.go b/src/internal/packager/helm/utils.go index 2913ae62b7..13e30d978b 100644 --- a/src/internal/packager/helm/utils.go +++ b/src/internal/packager/helm/utils.go @@ -10,6 +10,7 @@ import ( "strconv" "github.com/defenseunicorns/zarf/src/pkg/message" + "github.com/defenseunicorns/zarf/src/types" "helm.sh/helm/v3/pkg/action" "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/cli" @@ -22,7 +23,7 @@ import ( // loadChartFromTarball returns a helm chart from a tarball. func (h *Helm) loadChartFromTarball() (*chart.Chart, error) { // Get the path the temporary helm chart tarball - sourceFile := StandardName(filepath.Join(h.BasePath, "charts"), h.Chart) + ".tgz" + sourceFile := StandardName(filepath.Join(h.BasePath, types.ChartsFolder), h.Chart) + ".tgz" if h.ChartLoadOverride != "" { sourceFile = h.ChartLoadOverride } @@ -45,7 +46,7 @@ func (h *Helm) parseChartValues() (map[string]any, error) { valueOpts := &values.Options{} for idx, file := range h.Chart.ValuesFiles { - path := StandardName(filepath.Join(h.BasePath, "values"), h.Chart) + "-" + strconv.Itoa(idx) + path := StandardName(filepath.Join(h.BasePath, types.ValuesFolder), h.Chart) + "-" + strconv.Itoa(idx) // If we are overriding the chart path, assuming this is for zarf prepare if h.ChartLoadOverride != "" { path = file diff --git a/src/pkg/packager/common.go b/src/pkg/packager/common.go index 8cef56c9cc..d8e36df618 100644 --- a/src/pkg/packager/common.go +++ b/src/pkg/packager/common.go @@ -148,13 +148,13 @@ func (p *Packager) createOrGetComponentPaths(component types.ZarfComponent) (pat paths = types.ComponentPaths{ Base: base, - Temp: filepath.Join(base, "temp"), - Files: filepath.Join(base, "files"), - Charts: filepath.Join(base, "charts"), - Repos: filepath.Join(base, "repos"), - Manifests: filepath.Join(base, "manifests"), - DataInjections: filepath.Join(base, "data"), - Values: filepath.Join(base, "values"), + Temp: filepath.Join(base, types.TempFolder), + Files: filepath.Join(base, types.FilesFolder), + Charts: filepath.Join(base, types.ChartsFolder), + Repos: filepath.Join(base, types.ReposFolder), + Manifests: filepath.Join(base, types.ManifestsFolder), + DataInjections: filepath.Join(base, types.DataInjectionsFolder), + Values: filepath.Join(base, types.ValuesFolder), } if len(component.Files) > 0 { diff --git a/src/pkg/packager/compose.go b/src/pkg/packager/compose.go index 74737d1448..94182efca8 100644 --- a/src/pkg/packager/compose.go +++ b/src/pkg/packager/compose.go @@ -193,43 +193,28 @@ func (p *Packager) fixComposedFilepaths(pathAncestry string, child types.ZarfCom message.Debugf("packager.fixComposedFilepaths(%+v, %+v)", pathAncestry, child) for fileIdx, file := range child.Files { - composed, err := p.getComposedFilePath(pathAncestry, file.Source) - if err != nil { - return child, err - } + composed := p.getComposedFilePath(pathAncestry, file.Source) child.Files[fileIdx].Source = composed } for chartIdx, chart := range child.Charts { for valuesIdx, valuesFile := range chart.ValuesFiles { - composed, err := p.getComposedFilePath(pathAncestry, valuesFile) - if err != nil { - return child, err - } + composed := p.getComposedFilePath(pathAncestry, valuesFile) child.Charts[chartIdx].ValuesFiles[valuesIdx] = composed } if child.Charts[chartIdx].LocalPath != "" { - composed, err := p.getComposedFilePath(pathAncestry, child.Charts[chartIdx].LocalPath) - if err != nil { - return child, err - } + composed := p.getComposedFilePath(pathAncestry, child.Charts[chartIdx].LocalPath) child.Charts[chartIdx].LocalPath = composed } } for manifestIdx, manifest := range child.Manifests { for fileIdx, file := range manifest.Files { - composed, err := p.getComposedFilePath(pathAncestry, file) - if err != nil { - return child, err - } + composed := p.getComposedFilePath(pathAncestry, file) child.Manifests[manifestIdx].Files[fileIdx] = composed } for kustomizeIdx, kustomization := range manifest.Kustomizations { - composed, err := p.getComposedFilePath(pathAncestry, kustomization) - if err != nil { - return child, err - } + composed := p.getComposedFilePath(pathAncestry, kustomization) // kustomizations can use non-standard urls, so we need to check if the composed path exists on the local filesystem abs, _ := filepath.Abs(composed) invalid := utils.InvalidPath(abs) @@ -240,10 +225,7 @@ func (p *Packager) fixComposedFilepaths(pathAncestry string, child types.ZarfCom } for dataInjectionsIdx, dataInjection := range child.DataInjections { - composed, err := p.getComposedFilePath(pathAncestry, dataInjection.Source) - if err != nil { - return child, err - } + composed := p.getComposedFilePath(pathAncestry, dataInjection.Source) child.DataInjections[dataInjectionsIdx].Source = composed } @@ -265,18 +247,12 @@ func (p *Packager) fixComposedFilepaths(pathAncestry string, child types.ZarfCom totalActions := len(child.Actions.OnCreate.OnSuccess) + len(child.Actions.OnCreate.OnFailure) + len(child.Actions.OnCreate.Before) + len(child.Actions.OnCreate.After) if totalActions > 0 { - composedDefaultDir, err := p.getComposedFilePath(pathAncestry, child.Actions.OnCreate.Defaults.Dir) - if err != nil { - return child, err - } + composedDefaultDir := p.getComposedFilePath(pathAncestry, child.Actions.OnCreate.Defaults.Dir) child.Actions.OnCreate.Defaults.Dir = composedDefaultDir } if child.CosignKeyPath != "" { - composed, err := p.getComposedFilePath(pathAncestry, child.CosignKeyPath) - if err != nil { - return child, err - } + composed := p.getComposedFilePath(pathAncestry, child.CosignKeyPath) child.CosignKeyPath = composed } @@ -286,10 +262,7 @@ func (p *Packager) fixComposedFilepaths(pathAncestry string, child types.ZarfCom func (p *Packager) fixComposedActionFilepaths(pathAncestry string, actions []types.ZarfComponentAction) ([]types.ZarfComponentAction, error) { for actionIdx, action := range actions { if action.Dir != nil { - composedActionDir, err := p.getComposedFilePath(pathAncestry, *action.Dir) - if err != nil { - return actions, err - } + composedActionDir := p.getComposedFilePath(pathAncestry, *action.Dir) actions[actionIdx].Dir = &composedActionDir } } @@ -395,31 +368,14 @@ func (p *Packager) getSubPackage(packagePath string) (importedPackage types.Zarf } // Prefix file path with importPath if original file path is not a url. -func (p *Packager) getComposedFilePath(prefix string, path string) (string, error) { +func (p *Packager) getComposedFilePath(prefix string, path string) string { message.Debugf("packager.getComposedFilePath(%s, %s)", prefix, path) // Return original if it is a remote file. if utils.IsURL(path) { - return path, nil + return path } // Add prefix for local files. - relativeToParent := filepath.Join(prefix, path) - - abs, err := filepath.Abs(relativeToParent) - if err != nil { - return "", err - } - if utils.InvalidPath(abs) { - pathAbs, err := filepath.Abs(path) - if err != nil { - return "", err - } - if !utils.InvalidPath(pathAbs) { - return "", fmt.Errorf("imported path %s does not exist, please update %s to be relative to the imported component", relativeToParent, path) - } - return "", fmt.Errorf("imported path %s does not exist", relativeToParent) - } - - return relativeToParent, nil + return filepath.Join(prefix, path) } diff --git a/src/pkg/packager/create.go b/src/pkg/packager/create.go index c6bfaf9e61..525996644a 100755 --- a/src/pkg/packager/create.go +++ b/src/pkg/packager/create.go @@ -148,7 +148,7 @@ func (p *Packager) Create(baseDir string) error { combinedImageList = append(combinedImageList, component.Images...) // Remove the temp directory for this component before archiving. - err = os.RemoveAll(filepath.Join(p.tmp.Components, component.Name, "temp")) + err = os.RemoveAll(filepath.Join(p.tmp.Components, component.Name, types.TempFolder)) if err != nil { message.Warnf("unable to remove temp directory for component %s, component tarball may contain unused artifacts: %s", component.Name, err.Error()) } @@ -366,12 +366,14 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel } if isSkeleton && chart.URL == "" { - dst := filepath.Join(componentPath.Charts, fmt.Sprintf("%s-%d", chart.Name, chartIdx)) - rel := strings.TrimPrefix(dst, componentPath.Base) + rel := filepath.Join(types.ChartsFolder, fmt.Sprintf("%s-%d", chart.Name, chartIdx)) + dst := filepath.Join(componentPath.Base, rel) + err := utils.CreatePathAndCopy(chart.LocalPath, dst) if err != nil { return err } + p.cfg.Pkg.Components[index].Charts[chartIdx].LocalPath = rel } else if isGitURL { _, err = helmCfg.PackageChartFromGit(componentPath.Charts) @@ -389,7 +391,9 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel } for valuesIdx, path := range chart.ValuesFiles { - dst := fmt.Sprintf("%s-%d", helm.StandardName(componentPath.Values, chart), valuesIdx) + rel := fmt.Sprintf("%s-%d", helm.StandardName(types.ValuesFolder, chart), valuesIdx) + dst := filepath.Join(componentPath.Base, rel) + if utils.IsURL(path) { if isSkeleton { continue @@ -402,7 +406,6 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel return fmt.Errorf("unable to copy chart values file %s: %w", path, err) } if isSkeleton { - rel := strings.TrimPrefix(dst, componentPath.Base) p.cfg.Pkg.Components[index].Charts[chartIdx].ValuesFiles[valuesIdx] = rel } } @@ -412,9 +415,8 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel for filesIdx, file := range component.Files { message.Debugf("Loading %#v", file) - dstIdxPath := filepath.Join(componentPath.Files, strconv.Itoa(filesIdx)) - utils.CreateDirectory(dstIdxPath, 0700) - dst := filepath.Join(dstIdxPath, filepath.Base(file.Target)) + rel := filepath.Join(types.FilesFolder, strconv.Itoa(filesIdx), filepath.Base(file.Target)) + dst := filepath.Join(componentPath.Base, rel) if utils.IsURL(file.Source) { if isSkeleton { @@ -428,7 +430,6 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel return fmt.Errorf("unable to copy file %s: %w", file.Source, err) } if isSkeleton { - rel := strings.TrimPrefix(dst, componentPath.Base) p.cfg.Pkg.Components[index].Files[filesIdx].Source = rel } } @@ -454,9 +455,8 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel for dataIdx, data := range component.DataInjections { spinner.Updatef("Copying data injection %s for %s", data.Target.Path, data.Target.Selector) - dstIdxPath := filepath.Join(componentPath.DataInjections, strconv.Itoa(dataIdx)) - utils.CreateDirectory(dstIdxPath, 0700) - dst := filepath.Join(dstIdxPath, filepath.Base(data.Target.Path)) + rel := filepath.Join(types.DataInjectionsFolder, strconv.Itoa(dataIdx), filepath.Base(data.Target.Path)) + dst := filepath.Join(componentPath.Base, rel) if utils.IsURL(data.Source) { if isSkeleton { @@ -470,7 +470,6 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel return fmt.Errorf("unable to copy data injection %s: %s", data.Source, err.Error()) } if isSkeleton { - rel := strings.TrimPrefix(dst, componentPath.Base) p.cfg.Pkg.Components[index].DataInjections[dataIdx].Source = rel } } @@ -493,7 +492,9 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel // Iterate over all manifests. for manifestIdx, manifest := range component.Manifests { for fileIdx, path := range manifest.Files { - dst := filepath.Join(componentPath.Manifests, fmt.Sprintf("%s-%d.yaml", manifest.Name, fileIdx)) + rel := filepath.Join(types.ManifestsFolder, fmt.Sprintf("%s-%d.yaml", manifest.Name, fileIdx)) + dst := filepath.Join(componentPath.Base, rel) + // Copy manifests without any processing. spinner.Updatef("Copying manifest %s", path) if utils.IsURL(path) { @@ -508,7 +509,6 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel return fmt.Errorf("unable to copy manifest %s: %w", path, err) } if isSkeleton { - rel := strings.TrimPrefix(dst, componentPath.Base) p.cfg.Pkg.Components[index].Manifests[manifestIdx].Files[fileIdx] = rel } } @@ -517,13 +517,15 @@ func (p *Packager) addComponent(index int, component types.ZarfComponent, isSkel for kustomizeIdx, path := range manifest.Kustomizations { // Generate manifests from kustomizations and place in the package. spinner.Updatef("Building kustomization for %s", path) + kname := fmt.Sprintf("kustomization-%s-%d.yaml", manifest.Name, kustomizeIdx) - dst := filepath.Join(componentPath.Manifests, kname) + rel := filepath.Join(types.ManifestsFolder, kname) + dst := filepath.Join(componentPath.Base, rel) + if err := kustomize.Build(path, dst, manifest.KustomizeAllowAnyDirectory); err != nil { return fmt.Errorf("unable to build kustomization %s: %w", path, err) } if isSkeleton { - rel := strings.TrimPrefix(dst, componentPath.Base) p.cfg.Pkg.Components[index].Manifests[manifestIdx].Files = append(p.cfg.Pkg.Components[index].Manifests[manifestIdx].Files, rel) } } diff --git a/src/test/e2e/51_oci_compose_test.go b/src/test/e2e/51_oci_compose_test.go index f5ecfb182c..5702c31cf0 100644 --- a/src/test/e2e/51_oci_compose_test.go +++ b/src/test/e2e/51_oci_compose_test.go @@ -152,13 +152,13 @@ func (suite *SkeletonSuite) verifyComponentPaths(unpackedPath string, components base := filepath.Join(unpackedPath, "components", component.Name) componentPaths := types.ComponentPaths{ Base: base, - Temp: filepath.Join(base, "temp"), - Files: filepath.Join(base, "files"), - Charts: filepath.Join(base, "charts"), - Repos: filepath.Join(base, "repos"), - Manifests: filepath.Join(base, "manifests"), - DataInjections: filepath.Join(base, "data"), - Values: filepath.Join(base, "values"), + Temp: filepath.Join(base, types.TempFolder), + Files: filepath.Join(base, types.FilesFolder), + Charts: filepath.Join(base, types.ChartsFolder), + Repos: filepath.Join(base, types.ReposFolder), + Manifests: filepath.Join(base, types.ManifestsFolder), + DataInjections: filepath.Join(base, types.DataInjectionsFolder), + Values: filepath.Join(base, types.ValuesFolder), } if isSkeleton && component.CosignKeyPath != "" { diff --git a/src/types/runtime.go b/src/types/runtime.go index ab383f44cb..92ee38361a 100644 --- a/src/types/runtime.go +++ b/src/types/runtime.go @@ -9,6 +9,17 @@ import ( "oras.land/oras-go/v2/registry" ) +// Constants to keep track of folders within components +const ( + TempFolder = "temp" + FilesFolder = "files" + ChartsFolder = "charts" + ReposFolder = "repos" + ManifestsFolder = "manifests" + DataInjectionsFolder = "data" + ValuesFolder = "values" +) + // ZarfCommonOptions tracks the user-defined preferences used across commands. type ZarfCommonOptions struct { Confirm bool `json:"confirm" jsonschema:"description=Verify that Zarf should perform an action"`