From ef4b57782f6db38afbf74d735aaa294bc59ab973 Mon Sep 17 00:00:00 2001 From: decleaver <85503726+decleaver@users.noreply.github.com> Date: Tue, 20 Feb 2024 14:12:47 -0700 Subject: [PATCH] fix: use tmpdir if provided (#431) Co-authored-by: unclegedd --- src/cmd/root.go | 4 +- src/cmd/uds.go | 6 +- src/pkg/bundle/common.go | 9 +- src/pkg/bundle/create.go | 20 +--- src/pkg/bundler/bundler.go | 5 +- src/pkg/bundler/fetcher/local.go | 4 +- src/pkg/bundler/fetcher/remote.go | 2 +- src/pkg/bundler/localbundle.go | 14 +-- src/pkg/bundler/pusher/remote.go | 4 +- src/pkg/runner/runner.go | 2 +- .../03-local-and-remote/uds-bundle.yaml | 2 +- src/test/bundles/04-init/uds-bundle.yaml | 2 +- src/test/bundles/05-gitrepo/uds-bundle.yaml | 2 +- .../bundles/07-helm-overrides/uds-bundle.yaml | 2 +- .../bundles/08-var-precedence/uds-bundle.yaml | 4 +- .../bundles/09-uds-bundle-yml/uds-bundle.yml | 2 +- .../bundles/11-real-simple/uds-bundle.yaml | 2 +- src/test/e2e/bundle_test.go | 105 ++++++++++++++++++ 18 files changed, 139 insertions(+), 52 deletions(-) diff --git a/src/cmd/root.go b/src/cmd/root.go index 576769ac..55faf571 100644 --- a/src/cmd/root.go +++ b/src/cmd/root.go @@ -31,7 +31,7 @@ var ( var rootCmd = &cobra.Command{ Use: "uds COMMAND", - PersistentPreRun: func(cmd *cobra.Command, args []string) { + PersistentPreRun: func(cmd *cobra.Command, _ []string) { // Skip for vendor-only commands if common.CheckVendorOnlyFromPath(cmd) { return @@ -46,7 +46,7 @@ var rootCmd = &cobra.Command{ cliSetup() }, Short: lang.RootCmdShort, - Run: func(cmd *cobra.Command, args []string) { + Run: func(cmd *cobra.Command, _ []string) { _, _ = fmt.Fprintln(os.Stderr) err := cmd.Help() if err != nil { diff --git a/src/cmd/uds.go b/src/cmd/uds.go index ff0b6955..b55b16db 100644 --- a/src/cmd/uds.go +++ b/src/cmd/uds.go @@ -26,7 +26,7 @@ var createCmd = &cobra.Command{ Aliases: []string{"c"}, Args: cobra.MaximumNArgs(1), Short: lang.CmdBundleCreateShort, - PreRun: func(cmd *cobra.Command, args []string) { + PreRun: func(_ *cobra.Command, args []string) { pathToBundleFile := "" if len(args) > 0 { if !zarfUtils.IsDir(args[0]) { @@ -44,7 +44,7 @@ var createCmd = &cobra.Command{ message.Fatalf(err, "Neither %s or %s found", config.BundleYAML, bundleYml) } }, - Run: func(cmd *cobra.Command, args []string) { + Run: func(_ *cobra.Command, args []string) { srcDir, err := os.Getwd() if err != nil { message.Fatalf(err, "error reading the current working directory") @@ -69,7 +69,7 @@ var deployCmd = &cobra.Command{ Aliases: []string{"d"}, Short: lang.CmdBundleDeployShort, Args: cobra.MaximumNArgs(1), - Run: func(cmd *cobra.Command, args []string) { + Run: func(_ *cobra.Command, args []string) { bundleCfg.DeployOpts.Source = chooseBundle(args) configureZarf() diff --git a/src/pkg/bundle/common.go b/src/pkg/bundle/common.go index f4648779..ef4941aa 100644 --- a/src/pkg/bundle/common.go +++ b/src/pkg/bundle/common.go @@ -49,7 +49,7 @@ func New(cfg *types.BundleConfig) (*Bundle, error) { } ) - tmp, err := utils.MakeTempDir("") + tmp, err := utils.MakeTempDir(config.CommonOptions.TempDirectory) if err != nil { return nil, fmt.Errorf("bundler unable to create temp directory: %w", err) } @@ -102,11 +102,6 @@ func (b *Bundle) ValidateBundleResources(bundle *types.UDSBundle, spinner *messa return fmt.Errorf("error validating bundle vars: %s", err) } - tmp, err := utils.MakeTempDir("") - if err != nil { - return err - } - // validate access to packages as well as components referenced in the package for idx, pkg := range bundle.Packages { spinner.Updatef("Validating Bundle Package: %s", pkg.Name) @@ -180,8 +175,6 @@ func (b *Bundle) ValidateBundleResources(bundle *types.UDSBundle, spinner *messa message.Debug("Validating package:", message.JSONValue(pkg)) - defer os.RemoveAll(tmp) - // todo: need to packager.ValidatePackageSignature (or come up with a bundle-level signature scheme) publicKeyPath := filepath.Join(b.tmp, config.PublicKeyFile) if pkg.PublicKey != "" { diff --git a/src/pkg/bundle/create.go b/src/pkg/bundle/create.go index cfbada05..f801f31d 100644 --- a/src/pkg/bundle/create.go +++ b/src/pkg/bundle/create.go @@ -6,7 +6,6 @@ package bundle import ( "fmt" - "os" "path/filepath" "github.com/AlecAivazis/survey/v2" @@ -21,25 +20,9 @@ import ( // Create creates a bundle func (b *Bundle) Create() error { - // get the current working directory - cwd, err := os.Getwd() - if err != nil { - return err - } - - // cd into base - if err := os.Chdir(b.cfg.CreateOpts.SourceDirectory); err != nil { - return err - } - defer func() { - err := os.Chdir(cwd) - if err != nil { - fmt.Println("Error changing back to the original directory:", err) - } - }() // read the bundle's metadata into memory - if err := utils.ReadYaml(b.cfg.CreateOpts.BundleFile, &b.bundle); err != nil { + if err := utils.ReadYaml(filepath.Join(b.cfg.CreateOpts.SourceDirectory, b.cfg.CreateOpts.BundleFile), &b.bundle); err != nil { return err } @@ -94,6 +77,7 @@ func (b *Bundle) Create() error { Bundle: &b.bundle, Output: b.cfg.CreateOpts.Output, TmpDstDir: b.tmp, + SourceDir: b.cfg.CreateOpts.SourceDirectory, } bundlerClient := bundler.NewBundler(&opts) return bundlerClient.Create() diff --git a/src/pkg/bundler/bundler.go b/src/pkg/bundler/bundler.go index 3e94e69a..008af960 100644 --- a/src/pkg/bundler/bundler.go +++ b/src/pkg/bundler/bundler.go @@ -12,6 +12,7 @@ type Bundler struct { bundle *types.UDSBundle output string tmpDstDir string + sourceDir string } type Pusher interface { @@ -21,6 +22,7 @@ type Options struct { Bundle *types.UDSBundle Output string TmpDstDir string + SourceDir string } func NewBundler(opts *Options) *Bundler { @@ -28,13 +30,14 @@ func NewBundler(opts *Options) *Bundler { bundle: opts.Bundle, output: opts.Output, tmpDstDir: opts.TmpDstDir, + sourceDir: opts.SourceDir, } return &b } func (b *Bundler) Create() error { if b.output == "" { - localBundle := NewLocalBundle(&LocalBundleOpts{Bundle: b.bundle, TmpDstDir: b.tmpDstDir}) + localBundle := NewLocalBundle(&LocalBundleOpts{Bundle: b.bundle, TmpDstDir: b.tmpDstDir, SourceDir: b.sourceDir}) err := localBundle.create(nil) if err != nil { return err diff --git a/src/pkg/bundler/fetcher/local.go b/src/pkg/bundler/fetcher/local.go index 8383dc46..9a50298b 100644 --- a/src/pkg/bundler/fetcher/local.go +++ b/src/pkg/bundler/fetcher/local.go @@ -37,7 +37,7 @@ type localFetcher struct { func (f *localFetcher) Fetch() ([]ocispec.Descriptor, error) { fetchSpinner := message.NewProgressSpinner("Fetching package %s", f.pkg.Name) defer fetchSpinner.Stop() - pkgTmp, err := zarfUtils.MakeTempDir("") + pkgTmp, err := zarfUtils.MakeTempDir(config.CommonOptions.TempDirectory) defer os.RemoveAll(pkgTmp) if err != nil { return nil, err @@ -64,7 +64,7 @@ func (f *localFetcher) Fetch() ([]ocispec.Descriptor, error) { // GetPkgMetadata grabs metadata from a local Zarf package's zarf.yaml func (f *localFetcher) GetPkgMetadata() (zarfTypes.ZarfPackage, error) { - tmpDir, err := zarfUtils.MakeTempDir("") + tmpDir, err := zarfUtils.MakeTempDir(config.CommonOptions.TempDirectory) if err != nil { return zarfTypes.ZarfPackage{}, err } diff --git a/src/pkg/bundler/fetcher/remote.go b/src/pkg/bundler/fetcher/remote.go index 9095417a..edf31ed0 100644 --- a/src/pkg/bundler/fetcher/remote.go +++ b/src/pkg/bundler/fetcher/remote.go @@ -199,7 +199,7 @@ func (f *remoteFetcher) GetPkgMetadata() (zarfTypes.ZarfPackage, error) { if err != nil { return zarfTypes.ZarfPackage{}, err } - tmpDir, err := zarfUtils.MakeTempDir("") + tmpDir, err := zarfUtils.MakeTempDir(config.CommonOptions.TempDirectory) if err != nil { return zarfTypes.ZarfPackage{}, fmt.Errorf("bundler unable to create temp directory: %w", err) } diff --git a/src/pkg/bundler/localbundle.go b/src/pkg/bundler/localbundle.go index 63de7fb8..8399a210 100644 --- a/src/pkg/bundler/localbundle.go +++ b/src/pkg/bundler/localbundle.go @@ -29,17 +29,20 @@ import ( type LocalBundleOpts struct { Bundle *types.UDSBundle TmpDstDir string + SourceDir string } type LocalBundle struct { bundle *types.UDSBundle tmpDstDir string + sourceDir string } func NewLocalBundle(opts *LocalBundleOpts) *LocalBundle { return &LocalBundle{ bundle: opts.Bundle, tmpDstDir: opts.TmpDstDir, + sourceDir: opts.SourceDir, } } @@ -153,7 +156,7 @@ func (lo *LocalBundle) create(signature []byte) error { } // tarball the bundle - err = writeTarball(bundle, artifactPathMap) + err = writeTarball(bundle, artifactPathMap, lo.sourceDir) if err != nil { return err } @@ -199,17 +202,14 @@ func pushManifestConfig(store *ocistore.Store, metadata types.UDSMetadata, build } // writeTarball builds and writes a bundle tarball to disk based on a file map -func writeTarball(bundle *types.UDSBundle, artifactPathMap types.PathMap) error { +func writeTarball(bundle *types.UDSBundle, artifactPathMap types.PathMap, sourceDir string) error { format := archiver.CompressedArchive{ Compression: archiver.Zstd{}, Archival: archiver.Tar{}, } filename := fmt.Sprintf("%s%s-%s-%s.tar.zst", config.BundlePrefix, bundle.Metadata.Name, bundle.Metadata.Architecture, bundle.Metadata.Version) - cwd, err := os.Getwd() - if err != nil { - return err - } - dst := filepath.Join(cwd, filename) + + dst := filepath.Join(sourceDir, filename) _ = os.RemoveAll(dst) diff --git a/src/pkg/bundler/pusher/remote.go b/src/pkg/bundler/pusher/remote.go index 37cae6cb..9abca0df 100644 --- a/src/pkg/bundler/pusher/remote.go +++ b/src/pkg/bundler/pusher/remote.go @@ -1,6 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: 2023-Present The UDS Authors +// Package pusher contains functionality to push Zarf pkgs to remote bundles package pusher import ( @@ -23,6 +24,7 @@ type RemotePusher struct { cfg Config } +// Config contains the configuration for the remote pusher type Config struct { PkgRootManifest *oci.ZarfOCIManifest RemoteSrc *oci.OrasRemote @@ -81,7 +83,7 @@ func (p *RemotePusher) PushManifest() (ocispec.Descriptor, error) { return zarfManifestDesc, nil } -// todo: what does this fn return? +// LayersToRemoteBundle pushes the Zarf pkg's layers to a remote bundle func (p *RemotePusher) LayersToRemoteBundle(spinner *message.Spinner, currentPackageIter int, totalPackages int) ([]ocispec.Descriptor, error) { spinner.Updatef("Fetching %s package layer metadata (package %d of %d)", p.pkg.Name, currentPackageIter, totalPackages) // get only the layers that are required by the components diff --git a/src/pkg/runner/runner.go b/src/pkg/runner/runner.go index ccf81b6c..16178a7c 100644 --- a/src/pkg/runner/runner.go +++ b/src/pkg/runner/runner.go @@ -103,7 +103,7 @@ func (r *Runner) importTasks(includes []map[string]string, dir string, setVariab // check if included file is a url if helpers.IsURL(includeFilename) { // If file is a url download it to a tmp directory - tmpDir, err := zarfUtils.MakeTempDir("") + tmpDir, err := zarfUtils.MakeTempDir(config.CommonOptions.TempDirectory) defer os.RemoveAll(tmpDir) if err != nil { return err diff --git a/src/test/bundles/03-local-and-remote/uds-bundle.yaml b/src/test/bundles/03-local-and-remote/uds-bundle.yaml index feb0adc1..d9eea763 100644 --- a/src/test/bundles/03-local-and-remote/uds-bundle.yaml +++ b/src/test/bundles/03-local-and-remote/uds-bundle.yaml @@ -9,5 +9,5 @@ packages: repository: ghcr.io/defenseunicorns/uds-cli/nginx ref: 0.0.1 - name: podinfo - path: "../../packages/podinfo" + path: "src/test/packages/podinfo" ref: 0.0.1 diff --git a/src/test/bundles/04-init/uds-bundle.yaml b/src/test/bundles/04-init/uds-bundle.yaml index 45641682..72588984 100644 --- a/src/test/bundles/04-init/uds-bundle.yaml +++ b/src/test/bundles/04-init/uds-bundle.yaml @@ -8,7 +8,7 @@ metadata: # cannot do uds remove on this pkg due to having the same name packages: - name: init - path: "../../packages" + path: "src/test/packages" # renovate: datasource=github-tags depName=defenseunicorns/zarf ref: v0.32.3 optionalComponents: diff --git a/src/test/bundles/05-gitrepo/uds-bundle.yaml b/src/test/bundles/05-gitrepo/uds-bundle.yaml index 835039dd..7cd86d97 100644 --- a/src/test/bundles/05-gitrepo/uds-bundle.yaml +++ b/src/test/bundles/05-gitrepo/uds-bundle.yaml @@ -6,5 +6,5 @@ metadata: packages: - name: gitrepo - path: "../../packages/gitrepo" + path: "src/test/packages/gitrepo" ref: 0.0.1 diff --git a/src/test/bundles/07-helm-overrides/uds-bundle.yaml b/src/test/bundles/07-helm-overrides/uds-bundle.yaml index a0019dc3..73689822 100644 --- a/src/test/bundles/07-helm-overrides/uds-bundle.yaml +++ b/src/test/bundles/07-helm-overrides/uds-bundle.yaml @@ -6,7 +6,7 @@ metadata: packages: - name: helm-overrides - path: "../../packages/helm" + path: "src/test/packages/helm" ref: 0.0.1 overrides: diff --git a/src/test/bundles/08-var-precedence/uds-bundle.yaml b/src/test/bundles/08-var-precedence/uds-bundle.yaml index 3608b58d..1b98f24d 100644 --- a/src/test/bundles/08-var-precedence/uds-bundle.yaml +++ b/src/test/bundles/08-var-precedence/uds-bundle.yaml @@ -6,7 +6,7 @@ metadata: packages: - name: helm-overrides - path: "../../packages/helm" + path: "src/test/packages/helm" ref: 0.0.1 overrides: podinfo-component: @@ -22,5 +22,5 @@ packages: default: "uds.dev" - name: output-var - path: "../../packages/no-cluster/output-var" + path: "src/test/packages/no-cluster/output-var" ref: 0.0.1 diff --git a/src/test/bundles/09-uds-bundle-yml/uds-bundle.yml b/src/test/bundles/09-uds-bundle-yml/uds-bundle.yml index ae397f99..49726eae 100644 --- a/src/test/bundles/09-uds-bundle-yml/uds-bundle.yml +++ b/src/test/bundles/09-uds-bundle-yml/uds-bundle.yml @@ -6,5 +6,5 @@ metadata: packages: - name: nginx - path: "../../packages/nginx" + path: "src/test/packages/nginx" ref: 0.0.1 diff --git a/src/test/bundles/11-real-simple/uds-bundle.yaml b/src/test/bundles/11-real-simple/uds-bundle.yaml index a5aa4406..6e17114f 100644 --- a/src/test/bundles/11-real-simple/uds-bundle.yaml +++ b/src/test/bundles/11-real-simple/uds-bundle.yaml @@ -5,5 +5,5 @@ metadata: packages: - name: real-simple - path: "../../packages/no-cluster/real-simple" + path: "src/test/packages/no-cluster/real-simple" ref: 0.0.1 diff --git a/src/test/e2e/bundle_test.go b/src/test/e2e/bundle_test.go index ed271a07..d89ed309 100644 --- a/src/test/e2e/bundle_test.go +++ b/src/test/e2e/bundle_test.go @@ -10,7 +10,9 @@ import ( "path/filepath" "strings" "testing" + "time" + "github.com/fsnotify/fsnotify" ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/stretchr/testify/require" "oras.land/oras-go/v2/registry" @@ -115,6 +117,10 @@ func TestBundle(t *testing.T) { os.Setenv("UDS_CONFIG", filepath.Join("src/test/bundles/01-uds-bundle", "uds-config.yaml")) deploy(t, bundlePath) remove(t, bundlePath) + + //Test create using custom tmpDir + runCmd(t, "create "+bundleDir+" --tmpdir ./customtmp --confirm --insecure") + } func TestPackagesFlag(t *testing.T) { @@ -474,3 +480,102 @@ func validateMultiArchIndex(t *testing.T, index ocispec.Index) { require.True(t, checkedAMD) require.True(t, checkedARM) } + +func TestBundleTmpDir(t *testing.T) { + deployZarfInit(t) + + e2e.CreateZarfPkg(t, "src/test/packages/nginx", false) + e2e.CreateZarfPkg(t, "src/test/packages/podinfo", false) + + e2e.SetupDockerRegistry(t, 888) + defer e2e.TeardownRegistry(t, 888) + e2e.SetupDockerRegistry(t, 889) + defer e2e.TeardownRegistry(t, 889) + + pkg := fmt.Sprintf("src/test/packages/nginx/zarf-package-nginx-%s-0.0.1.tar.zst", e2e.Arch) + zarfPublish(t, pkg, "localhost:888") + + pkg = fmt.Sprintf("src/test/packages/podinfo/zarf-package-podinfo-%s-0.0.1.tar.zst", e2e.Arch) + zarfPublish(t, pkg, "localhost:889") + + bundleDir := "src/test/bundles/01-uds-bundle" + bundlePath := filepath.Join(bundleDir, fmt.Sprintf("uds-bundle-example-%s-0.0.1.tar.zst", e2e.Arch)) + + //Test create using custom tmpDir + tmpDirName := "customtmp" + tmpDir := fmt.Sprintf("%s/%s", bundleDir, tmpDirName) + + err := os.Mkdir(tmpDir, 0755) + if err != nil { + t.Fatalf("error creating directory: %v", err) + } + defer os.RemoveAll(tmpDir) + + // create a file watcher for tmpDir + watcher, err := fsnotify.NewWatcher() + if err != nil { + t.Fatalf("error creating file watcher: %v", err) + } + defer watcher.Close() + + // Add the temporary directory to the watcher + err = watcher.Add(tmpDir) + if err != nil { + t.Fatalf("error adding directory to watcher: %v", err) + } + + // Channel to receive file change events + done := make(chan bool) + // Channel to receive errors + errCh := make(chan error) + + // Watch for file creation in the temporary directory + go func() { + for { + select { + case event, ok := <-watcher.Events: + if !ok { + return + } + if event.Op&fsnotify.Create == fsnotify.Create { + // Check if the directory is populated + files, err := os.ReadDir(tmpDir) + if err != nil { + // Send error to the main test goroutine + errCh <- fmt.Errorf("error reading directory: %v", err) + return + } + if len(files) > 0 { + // Directory is populated, send success signal to the main test goroutine + done <- true + return + } + } + case err, ok := <-watcher.Errors: + if !ok { + return + } + // Send error to the main test goroutine + errCh <- fmt.Errorf("error watching directory: %v", err) + return + } + } + }() + + // run create command with tmpDir + runCmd(t, "create "+bundleDir+" --tmpdir "+tmpDir+" --confirm --insecure") + // run deploy command with tmpDir + runCmd(t, "deploy "+bundlePath+" --tmpdir "+tmpDir+" --confirm --insecure") + // run remove command with tmpDir + runCmd(t, "remove "+bundlePath+" --tmpdir "+tmpDir+" --confirm --insecure") + + // handle errors and failures + select { + case err := <-errCh: + t.Fatalf("error: %v", err) + case <-done: + t.Log("Directory is populated") + case <-time.After(10 * time.Second): // Timeout after 10 seconds + t.Fatal("timeout waiting for directory to get populated") + } +}