Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not check composed filepath #1758

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/internal/packager/helm/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
}
Expand All @@ -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
Expand Down
14 changes: 7 additions & 7 deletions src/pkg/packager/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
68 changes: 12 additions & 56 deletions src/pkg/packager/compose.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}

Expand All @@ -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
}

Expand All @@ -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
}
}
Expand Down Expand Up @@ -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.
Racer159 marked this conversation as resolved.
Show resolved Hide resolved
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)
}
36 changes: 19 additions & 17 deletions src/pkg/packager/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
}
}
Expand All @@ -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 {
Expand All @@ -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
}
}
Expand All @@ -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 {
Expand All @@ -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
}
}
Expand All @@ -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) {
Expand All @@ -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
}
}
Expand All @@ -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)
}
}
Expand Down
14 changes: 7 additions & 7 deletions src/test/e2e/51_oci_compose_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand Down
11 changes: 11 additions & 0 deletions src/types/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down