diff --git a/cmd/dt/pull.go b/cmd/dt/pull.go index 4c38dd7..fafefed 100644 --- a/cmd/dt/pull.go +++ b/cmd/dt/pull.go @@ -2,13 +2,11 @@ package main import ( "context" - "errors" "fmt" "github.com/spf13/cobra" "github.com/vmware-labs/distribution-tooling-for-helm/internal/log" "github.com/vmware-labs/distribution-tooling-for-helm/pkg/chartutils" - "github.com/vmware-labs/distribution-tooling-for-helm/pkg/imagelock" "github.com/vmware-labs/distribution-tooling-for-helm/pkg/utils" "github.com/vmware-labs/distribution-tooling-for-helm/pkg/wrapping" ) @@ -16,16 +14,14 @@ import ( var pullCmd = newPullCommand() func pullChartImages(chart wrapping.Lockable, imagesDir string, opts ...chartutils.Option) error { - lockFile := chart.LockFilePath() - - lock, err := imagelock.FromYAMLFile(lockFile) + lock, err := chart.GetImagesLock() if err != nil { - return fmt.Errorf("failed to read Images.lock file") + return fmt.Errorf("failed to read Images.lock file: %v", err) } if err := chartutils.PullImages(lock, imagesDir, opts..., ); err != nil { - return fmt.Errorf("failed to pull images: %w", err) + return fmt.Errorf("failed to pull images: %v", err) } return nil } @@ -64,28 +60,30 @@ func newPullCommand() *cobra.Command { if imagesDir == "" { imagesDir = chart.ImagesDir() } + lock, err := chart.GetImagesLock() + if err != nil { + return l.Failf("Failed to load Images.lock: %v", err) + } - var imagesPulled bool - if err := l.Section(fmt.Sprintf("Pulling images into %q", chart.ImagesDir()), func(childLog log.SectionLogger) error { - if err := pullChartImages( - chart, - imagesDir, - chartutils.WithLog(childLog), - chartutils.WithContext(ctx), - chartutils.WithProgressBar(childLog.ProgressBar()), - chartutils.WithArtifactsDir(chart.ImageArtifactsDir()), - ); err != nil { - if errors.Is(err, chartutils.ErrNoImagesFound) { - childLog.Warnf("No images found in Images.lock") - return nil + if len(lock.Images) == 0 { + l.Warnf("No images found in Images.lock") + } else { + if err := l.Section(fmt.Sprintf("Pulling images into %q", chart.ImagesDir()), func(childLog log.SectionLogger) error { + if err := pullChartImages( + chart, + imagesDir, + chartutils.WithLog(childLog), + chartutils.WithContext(ctx), + chartutils.WithProgressBar(childLog.ProgressBar()), + chartutils.WithArtifactsDir(chart.ImageArtifactsDir()), + ); err != nil { + return childLog.Failf("%v", err) } - return childLog.Failf("%v", err) + childLog.Infof("All images pulled successfully") + return nil + }); err != nil { + return l.Failf("%w", err) } - imagesPulled = true - childLog.Infof("All images pulled successfully") - return nil - }); err != nil { - return l.Failf("%w", err) } if outputFile != "" { @@ -108,20 +106,8 @@ func newPullCommand() *cobra.Command { successMessage = fmt.Sprintf("All images pulled successfully into %q", chart.ImagesDir()) } - var warningMessage string - if outputFile != "" { - warningMessage = fmt.Sprintf("No images found in Images.lock. Chart compressed into %q", outputFile) - } else { - warningMessage = "No images found in Images.lock" - } - l.Printf(terminalSpacer) - - if imagesPulled { - l.Successf(successMessage) - } else { - l.Warnf(warningMessage) - } + l.Successf(successMessage) return nil }, diff --git a/cmd/dt/pull_test.go b/cmd/dt/pull_test.go index ece783b..e42f4b9 100644 --- a/cmd/dt/pull_test.go +++ b/cmd/dt/pull_test.go @@ -80,7 +80,7 @@ func (suite *CmdSuite) TestPullCommand() { scenarioDir = fmt.Sprintf("../../testdata/scenarios/%s", scenarioName) chartDir := createSampleChart(sb.TempFile()) dt("images", "pull", chartDir).AssertSuccessMatch(t, "No images found in Images.lock") - verifyChartDir(chartDir) + require.NoDirExists(filepath.Join(chartDir, "images")) }) t.Run("Errors", func(t *testing.T) { @@ -88,7 +88,7 @@ func (suite *CmdSuite) TestPullCommand() { chartDir := createSampleChart(sb.TempFile()) require.NoError(os.RemoveAll(filepath.Join(chartDir, "Images.lock"))) - dt("images", "pull", chartDir).AssertErrorMatch(t, `(?s).*failed to read Images\.lock file.*`) + dt("images", "pull", chartDir).AssertErrorMatch(t, `Failed to load Images\.lock.*`) }) }) } diff --git a/cmd/dt/push.go b/cmd/dt/push.go index 15a3cfc..33ad1f3 100644 --- a/cmd/dt/push.go +++ b/cmd/dt/push.go @@ -3,27 +3,17 @@ package main import ( "context" "fmt" - "os" "github.com/spf13/cobra" "github.com/vmware-labs/distribution-tooling-for-helm/internal/log" "github.com/vmware-labs/distribution-tooling-for-helm/pkg/chartutils" - "github.com/vmware-labs/distribution-tooling-for-helm/pkg/imagelock" "github.com/vmware-labs/distribution-tooling-for-helm/pkg/wrapping" ) var pushCmd = newPushCmd() func pushChartImages(wrap wrapping.Wrap, imagesDir string, opts ...chartutils.Option) error { - lockFile := wrap.LockFilePath() - - fh, err := os.Open(lockFile) - if err != nil { - return fmt.Errorf("failed to open Images.lock file: %v", err) - } - defer fh.Close() - - lock, err := imagelock.FromYAML(fh) + lock, err := wrap.GetImagesLock() if err != nil { return fmt.Errorf("failed to load Images.lock: %v", err) } diff --git a/cmd/dt/unwrap.go b/cmd/dt/unwrap.go index cdcf857..57064c6 100644 --- a/cmd/dt/unwrap.go +++ b/cmd/dt/unwrap.go @@ -12,7 +12,6 @@ import ( "github.com/vmware-labs/distribution-tooling-for-helm/internal/widgets" "github.com/vmware-labs/distribution-tooling-for-helm/pkg/artifacts" "github.com/vmware-labs/distribution-tooling-for-helm/pkg/chartutils" - "github.com/vmware-labs/distribution-tooling-for-helm/pkg/imagelock" "github.com/vmware-labs/distribution-tooling-for-helm/pkg/relocator" "github.com/vmware-labs/distribution-tooling-for-helm/pkg/utils" "github.com/vmware-labs/distribution-tooling-for-helm/pkg/wrapping" @@ -155,7 +154,7 @@ func pushChartImagesAndVerify(ctx context.Context, wrap wrapping.Wrap, l log.Sec } func showImagesSummary(wrap wrapping.Lockable, l log.SectionLogger) int { - lock, err := imagelock.FromYAMLFile(wrap.LockFilePath()) + lock, err := wrap.GetImagesLock() if err != nil { l.Debugf("failed to load list of images: failed to load lock file: %v", err) return 0 diff --git a/cmd/dt/wrap.go b/cmd/dt/wrap.go index c9e9676..6bc1eb8 100644 --- a/cmd/dt/wrap.go +++ b/cmd/dt/wrap.go @@ -2,7 +2,6 @@ package main import ( "context" - "errors" "fmt" glog "log" "path/filepath" @@ -112,27 +111,32 @@ func wrapChart(ctx context.Context, inputPath string, outputFile string, platfor } } - if err := l.Section(fmt.Sprintf("Pulling images into %q", chart.ImagesDir()), func(childLog log.SectionLogger) error { - fetchArtifacts, _ := flags.GetBool("fetch-artifacts") - if err := pullChartImages( - wrap, - wrap.ImagesDir(), - chartutils.WithLog(childLog), - chartutils.WithContext(ctx), - chartutils.WithFetchArtifacts(fetchArtifacts), - chartutils.WithArtifactsDir(wrap.ImageArtifactsDir()), - chartutils.WithProgressBar(childLog.ProgressBar()), - ); err != nil { - if errors.Is(err, chartutils.ErrNoImagesFound) { - childLog.Warnf("No images found in Images.lock") - return nil + lock, err := chart.GetImagesLock() + if err != nil { + return l.Failf("Failed to load Images.lock: %v", err) + } + + if len(lock.Images) == 0 { + l.Warnf("No images found in Images.lock") + } else { + if err := l.Section(fmt.Sprintf("Pulling images into %q", chart.ImagesDir()), func(childLog log.SectionLogger) error { + fetchArtifacts, _ := flags.GetBool("fetch-artifacts") + if err := pullChartImages( + wrap, + wrap.ImagesDir(), + chartutils.WithLog(childLog), + chartutils.WithContext(ctx), + chartutils.WithFetchArtifacts(fetchArtifacts), + chartutils.WithArtifactsDir(wrap.ImageArtifactsDir()), + chartutils.WithProgressBar(childLog.ProgressBar()), + ); err != nil { + return childLog.Failf("%v", err) } - return childLog.Failf("%v", err) + childLog.Infof("All images pulled successfully") + return nil + }); err != nil { + return err } - childLog.Infof("All images pulled successfully") - return nil - }); err != nil { - return err } if shouldCarvelize(flags) { diff --git a/cmd/dt/wrap_test.go b/cmd/dt/wrap_test.go index 4f8fd46..e810355 100644 --- a/cmd/dt/wrap_test.go +++ b/cmd/dt/wrap_test.go @@ -123,11 +123,15 @@ func testChartWrap(t *testing.T, sb *tu.Sandbox, inputChart string, expectedLock require.NoError(t, utils.Untar(expectedWrapFile, tmpDir, utils.TarConfig{StripComponents: 1})) imagesDir := filepath.Join(tmpDir, "images") - require.DirExists(t, imagesDir) - for _, imgData := range cfg.Images { - for _, digestData := range imgData.Digests { - imgDir := filepath.Join(imagesDir, fmt.Sprintf("%s.layout", digestData.Digest.Encoded())) - assert.DirExists(t, imgDir) + if len(cfg.Images) == 0 { + require.NoDirExists(t, imagesDir) + } else { + require.DirExists(t, imagesDir) + for _, imgData := range cfg.Images { + for _, digestData := range imgData.Digests { + imgDir := filepath.Join(imagesDir, fmt.Sprintf("%s.layout", digestData.Digest.Encoded())) + assert.DirExists(t, imgDir) + } } } wrappedChartDir := filepath.Join(tmpDir, "chart") diff --git a/pkg/chartutils/chart.go b/pkg/chartutils/chart.go index f4677b9..4f2ba61 100644 --- a/pkg/chartutils/chart.go +++ b/pkg/chartutils/chart.go @@ -59,6 +59,18 @@ func (c *Chart) LockFilePath() string { return c.AbsFilePath(imagelock.DefaultImagesLockFileName) } +// GetImagesLock returns the chart's ImagesLock object +func (c *Chart) GetImagesLock() (*imagelock.ImagesLock, error) { + lockFile := c.LockFilePath() + + lock, err := imagelock.FromYAMLFile(lockFile) + if err != nil { + return nil, err + } + + return lock, nil +} + // ImageArtifactsDir returns the imags artifacts directory func (c *Chart) ImageArtifactsDir() string { return filepath.Join(c.RootDir(), artifacts.HelmArtifactsFolder, "images") diff --git a/pkg/chartutils/images.go b/pkg/chartutils/images.go index 0d359fe..60bd16e 100644 --- a/pkg/chartutils/images.go +++ b/pkg/chartutils/images.go @@ -2,7 +2,6 @@ package chartutils import ( "context" - "errors" "fmt" "os" "path/filepath" @@ -21,9 +20,6 @@ import ( "github.com/vmware-labs/distribution-tooling-for-helm/pkg/utils" ) -// ErrNoImagesFound is returned when no images are found in the Images.lock file -var ErrNoImagesFound = errors.New("no images found in Images.lock") - func getNumberOfArtifacts(images imagelock.ImageList) int { n := 0 for _, imgDesc := range images { @@ -54,8 +50,8 @@ func PullImages(lock *imagelock.ImagesLock, imagesDir string, opts ...Option) er } l := cfg.Log - if getNumberOfArtifacts(lock.Images) == 0 { - return ErrNoImagesFound + if len(lock.Images) == 0 { + return fmt.Errorf("no images found in Images.lock") } p, _ := cfg.ProgressBar.WithTotal(getNumberOfArtifacts(lock.Images)).UpdateTitle("Pulling Images").Start() diff --git a/pkg/chartutils/images_test.go b/pkg/chartutils/images_test.go index 878e072..4b003d1 100644 --- a/pkg/chartutils/images_test.go +++ b/pkg/chartutils/images_test.go @@ -67,7 +67,7 @@ func (suite *ChartUtilsTestSuite) TestPullImages() { } }) - t.Run("Warning when no images in Images.lock", func(t *testing.T) { + t.Run("Error when no images in Images.lock", func(t *testing.T) { chartDir := sb.TempFile() images := []tu.ImageData{} @@ -82,7 +82,7 @@ func (suite *ChartUtilsTestSuite) TestPullImages() { lock, err := imagelock.FromYAMLFile(filepath.Join(chartDir, "Images.lock")) require.NoError(err) - require.ErrorIs(PullImages(lock, imagesDir), ErrNoImagesFound) + require.Error(PullImages(lock, imagesDir)) require.DirExists(imagesDir) diff --git a/pkg/imagelock/lock.go b/pkg/imagelock/lock.go index 2b63130..077f019 100644 --- a/pkg/imagelock/lock.go +++ b/pkg/imagelock/lock.go @@ -112,7 +112,7 @@ func FromYAML(r io.Reader) (*ImagesLock, error) { func FromYAMLFile(file string) (*ImagesLock, error) { fh, err := os.Open(file) if err != nil { - return nil, fmt.Errorf("failed to open Images.lock file: %v", err) + return nil, fmt.Errorf("failed to open Images.lock file: %w", err) } defer fh.Close() return FromYAML(fh) diff --git a/pkg/wrapping/wrap.go b/pkg/wrapping/wrap.go index 9fd0433..4d20429 100644 --- a/pkg/wrapping/wrap.go +++ b/pkg/wrapping/wrap.go @@ -18,6 +18,7 @@ import ( type Lockable interface { LockFilePath() string ImagesDir() string + GetImagesLock() (*imagelock.ImagesLock, error) } // Wrap defines the interface to implement a Helm chart wrap @@ -55,6 +56,11 @@ func (w *wrap) ImagesDir() string { return w.AbsFilePath("images") } +// GetImagesLock returns the chart's ImagesLock object +func (w *wrap) GetImagesLock() (*imagelock.ImagesLock, error) { + return w.chart.GetImagesLock() +} + // AbsFilePath returns the absolute path to the Chart relative file name func (w *wrap) AbsFilePath(name string) string { return filepath.Join(w.rootDir, name)