Merge pull request #85 from vmware-labs/rename-new-image-options

Renames skip-image-refs to skip-relocaiton and skip-images to skip-pull-images

cmd/dt/relocate/relocate.go

@@ -12,7 +12,7 @@ import (
 // NewCmd builds a new relocate command
 func NewCmd(cfg *config.Config) *cobra.Command {
 	valuesFiles := []string{"values.yaml"}
-	skipImageRefs := false
+	skipImageRelocation := false
 	cmd := &cobra.Command{
 		Use:   "relocate CHART_PATH OCI_URI",
 		Short: "Relocates a Helm chart",
@@ -36,7 +36,7 @@ func NewCmd(cfg *config.Config) *cobra.Command {
 					relocator.WithLog(l), relocator.Recursive,
 					relocator.WithAnnotationsKey(cfg.AnnotationsKey),
 					relocator.WithValuesFiles(valuesFiles...),
-					relocator.WithSkipImageRefs(skipImageRefs),
+					relocator.WithSkipImageRelocation(skipImageRelocation),
 				)
 			}); err != nil {
 				return l.Failf("failed to relocate Helm chart %q: %w", chartPath, err)
@@ -48,7 +48,7 @@ func NewCmd(cfg *config.Config) *cobra.Command {
 	}
 
 	cmd.PersistentFlags().StringSliceVar(&valuesFiles, "values", valuesFiles, "values files to relocate images (can specify multiple)")
-	cmd.PersistentFlags().BoolVar(&skipImageRefs, "skip-image-refs", skipImageRefs, "Skip updating image references in values.yaml and Images.lock")
+	cmd.PersistentFlags().BoolVar(&skipImageRelocation, "skip-relocation", skipImageRelocation, "skip relocating image references in the different files")
 
 	return cmd
 }

cmd/dt/relocate_test.go

@@ -67,3 +67,51 @@ func (suite *CmdSuite) TestRelocateCommand() {
 		}
 	})
 }
+
+func (suite *CmdSuite) TestRelocateCommandRecursively() {
+	s, err := tu.NewTestServer()
+	suite.Require().NoError(err)
+	defer s.Close()
+
+	images, err := s.LoadImagesFromFile("../../testdata/images.json")
+	suite.Require().NoError(err)
+
+	sb := suite.sb
+	require := suite.Require()
+	serverURL := s.ServerURL
+	scenarioName := "recursive-chart"
+	chartName := "test"
+
+	scenarioDir := fmt.Sprintf("../../testdata/scenarios/%s", scenarioName)
+
+	renderLockedChart := func(chartDir string, _ string, serverURL string) string {
+
+		require.NoError(tu.RenderScenario(scenarioDir, chartDir,
+			map[string]interface{}{"ServerURL": serverURL, "Images": images, "Name": chartName, "RepositoryURL": serverURL},
+		))
+
+		return chartDir
+	}
+	suite.T().Run("Relocate Helm chart", func(t *testing.T) {
+		relocateURL := "custom.repo.example.com"
+		originChart := renderLockedChart(sb.TempFile(), scenarioName, serverURL)
+
+		cmd := dt("charts", "relocate", originChart, relocateURL)
+		cmd.AssertSuccess(suite.T())
+
+		err := filepath.Walk(originChart, func(path string, info os.FileInfo, err error) error {
+			if err != nil {
+				return err
+			}
+			if !info.IsDir() && filepath.Base(path) == "Chart.yaml" {
+				content, err := os.ReadFile(path)
+				if err != nil {
+					return err
+				}
+				suite.Assert().Contains(string(content), relocateURL, "File %s does not contain relocated URL", path)
+			}
+			return nil
+		})
+		suite.Require().NoError(err)
+	})
+}

cmd/dt/unwrap/unwrap.go

@@ -42,8 +42,8 @@ type Config struct {
 	TempDirectory         string
 	Version               string
 	Carvelize             bool
-	SkipImageRefs         bool
-	SkipImages            bool
+	SkipImageRelocation   bool
+	SkipPullImages        bool
 	KeepArtifacts         bool
 	FetchArtifacts        bool
 	Auth                  Auth
@@ -141,17 +141,17 @@ func WithCarvelize(carvelize bool) func(c *Config) {
 	}
 }
 
-// WithSkipImageRefs configures the WithSkipImageRefs of the WrapConfig
-func WithSkipImageRefs(skipimagerefs bool) func(c *Config) {
+// WithSkipImageRelocation configures the WithSkipImageRelocation of the WrapConfig
+func WithSkipImageRelocation(skipImageRelocation bool) func(c *Config) {
 	return func(c *Config) {
-		c.SkipImageRefs = skipimagerefs
+		c.SkipImageRelocation = skipImageRelocation
 	}
 }
 
-// WithSkipImages configures the WithSkipImages of the WrapConfig
-func WithSkipImages(skipimages bool) func(c *Config) {
+// WithSkipPullImages configures the WithSkipPullImages of the WrapConfig
+func WithSkipPullImages(skipPullImages bool) func(c *Config) {
 	return func(c *Config) {
-		c.SkipImages = skipimages
+		c.SkipPullImages = skipPullImages
 	}
 }
 
@@ -295,7 +295,7 @@ func unwrapChart(inputChart, registryURL, pushChartURL string, opts ...Option) (
 		return relocator.RelocateChartDir(
 			wrap.ChartDir(), registryURL, relocator.WithLog(l),
 			relocator.Recursive, relocator.WithAnnotationsKey(cfg.AnnotationsKey), relocator.WithValuesFiles(cfg.ValuesFiles...),
-			relocator.WithSkipImageRefs(cfg.SkipImageRefs),
+			relocator.WithSkipImageRelocation(cfg.SkipImageRelocation),
 		)
 	}); err != nil {
 		return "", l.Failf("failed to relocate %q: %w", chartPath, err)
@@ -304,7 +304,7 @@ func unwrapChart(inputChart, registryURL, pushChartURL string, opts ...Option) (
 
 	images := getImageList(wrap, l)
 
-	if len(images) > 0 && !cfg.SkipImages {
+	if len(images) > 0 && !cfg.SkipPullImages {
 		// If we are not in interactive mode, we do not show the list of images
 		if cfg.Interactive {
 			showImagesSummary(images, l)
@@ -452,11 +452,11 @@ func pushChart(ctx context.Context, wrap wrapping.Wrap, pushChartURL string, cfg
 // NewCmd returns a new unwrap command
 func NewCmd(cfg *config.Config) *cobra.Command {
 	var (
-		sayYes        bool
-		pushChartURL  string
-		version       string
-		skipImageRefs bool
-		skipImages    bool
+		sayYes              bool
+		pushChartURL        string
+		version             string
+		skipImageRelocation bool
+		skipPullImages      bool
 	)
 	valuesFiles := []string{"values.yaml"}
 	cmd := &cobra.Command{
@@ -490,8 +490,8 @@ func NewCmd(cfg *config.Config) *cobra.Command {
 				WithTempDirectory(tempDir),
 				WithUsePlainHTTP(cfg.UsePlainHTTP),
 				WithValuesFiles(valuesFiles...),
-				WithSkipImageRefs(skipImageRefs),
-				WithSkipImages(skipImages),
+				WithSkipImageRelocation(skipImageRelocation),
+				WithSkipPullImages(skipPullImages),
 			)
 			if err != nil {
 				return err
@@ -510,8 +510,8 @@ func NewCmd(cfg *config.Config) *cobra.Command {
 	cmd.PersistentFlags().StringVar(&pushChartURL, "push-chart-url", pushChartURL, "push the unwrapped Helm chart to the given URL")
 	cmd.PersistentFlags().BoolVar(&sayYes, "yes", sayYes, "respond 'yes' to any yes/no question")
 	cmd.PersistentFlags().StringSliceVar(&valuesFiles, "values", valuesFiles, "values files to relocate images (can specify multiple)")
-	cmd.PersistentFlags().BoolVar(&skipImageRefs, "skip-image-refs", skipImageRefs, "Skip updating image references in values.yaml and Images.lock")
-	cmd.PersistentFlags().BoolVar(&skipImages, "skip-images", skipImageRefs, "Skip pushing images")
+	cmd.PersistentFlags().BoolVar(&skipImageRelocation, "skip-image-relocation", skipImageRelocation, "Skip relocating image references in the different files")
+	cmd.PersistentFlags().BoolVar(&skipPullImages, "skip-pull-images", skipPullImages, "Skip pulling images")
 
 	return cmd
 }

cmd/dt/wrap/wrap.go

@@ -44,7 +44,7 @@ type Config struct {
 	Carvelize             bool
 	KeepArtifacts         bool
 	FetchArtifacts        bool
-	SkipImages            bool
+	SkipPullImages        bool
 	Auth                  Auth
 	ContainerRegistryAuth Auth
 	OutputFile            string
@@ -130,10 +130,10 @@ func WithFetchArtifacts(fetchArtifacts bool) func(c *Config) {
 	}
 }
 
-// WithSkipImages configures the WithSkipImages of the WrapConfig
-func WithSkipImages(skipimages bool) func(c *Config) {
+// WithSkipPullImages configures the WithSkipPullImages of the WrapConfig
+func WithSkipPullImages(skipPullImages bool) func(c *Config) {
 	return func(c *Config) {
-		c.SkipImages = skipimages
+		c.SkipPullImages = skipPullImages
 	}
 }
 
@@ -408,7 +408,7 @@ func wrapChart(inputPath string, opts ...Option) (string, error) {
 			outputFile = filepath.Join(filepath.Dir(chartRoot), outputBaseName)
 		}
 	}
-	if !cfg.SkipImages {
+	if !cfg.SkipPullImages {
 		if err := pullImages(wrap, subCfg); err != nil {
 			return "", err
 		}
@@ -448,7 +448,7 @@ func NewCmd(cfg *config.Config) *cobra.Command {
 	var platforms []string
 	var fetchArtifacts bool
 	var carvelize bool
-	var skipImages bool
+	var skipPullImages bool
 	var examples = `  # Wrap a Helm chart from a local folder
   $ dt wrap examples/mariadb
 
@@ -485,7 +485,7 @@ This command will pull all the container images and wrap it into a single tarbal
 				WithUsePlainHTTP(cfg.UsePlainHTTP), WithInsecure(cfg.Insecure),
 				WithOutputFile(outputFile),
 				WithTempDirectory(tmpDir),
-				WithSkipImages(skipImages),
+				WithSkipPullImages(skipPullImages),
 			)
 			if err != nil {
 				if _, ok := err.(*log.LoggedError); ok {
@@ -507,7 +507,7 @@ This command will pull all the container images and wrap it into a single tarbal
 	cmd.PersistentFlags().StringSliceVar(&platforms, "platforms", platforms, "platforms to include in the Images.lock file")
 	cmd.PersistentFlags().BoolVar(&carvelize, "add-carvel-bundle", carvelize, "whether the wrap should include a Carvel bundle or not")
 	cmd.PersistentFlags().BoolVar(&fetchArtifacts, "fetch-artifacts", fetchArtifacts, "fetch remote metadata and signature artifacts")
-	cmd.PersistentFlags().BoolVar(&skipImages, "skip-images", skipImages, "skip fetching images")
+	cmd.PersistentFlags().BoolVar(&skipPullImages, "skip-pull-images", skipPullImages, "skip pulling images when wrapping a Helm Chart")
 
 	return cmd
 }

pkg/relocator/chart.go

@@ -27,7 +27,7 @@ type RelocationResult struct {
 
 func relocateChart(chart *cu.Chart, prefix string, cfg *RelocateConfig) error {
 	var allErrors error
-	if cfg.SkipImageRefs {
+	if cfg.SkipImageRelocation {
 		return allErrors
 	}
 
@@ -98,11 +98,12 @@ func RelocateChartDir(chartPath string, prefix string, opts ...RelocateOption) e
 
 	if cfg.Recursive {
 		for _, dep := range chart.Dependencies() {
-			if err := relocateChart(dep, prefix, cfg); err != nil {
+			if err := RelocateChartDir(dep.ChartDir(), prefix, opts...); err != nil {
 				allErrors = errors.Join(allErrors, fmt.Errorf("failed to relocate Helm SubChart %q: %v", dep.Chart().ChartFullPath(), err))
 			}
 		}
 	}
+
 	return allErrors
 }
 

pkg/relocator/chart_test.go

@@ -85,13 +85,13 @@ func TestRelocateChartDir(t *testing.T) {
 
 	})
 
-	// create a new chart dir to reset for the SkipImageRef tests
+	// create a new chart dir to reset for the SkipImageRelocation tests
 	chartDir = sb.TempFile()
 	require.NoError(t, tu.RenderScenario(scenarioDir, chartDir, map[string]interface{}{"ServerURL": serverURL}))
-	err = RelocateChartDir(chartDir, "", WithValuesFiles(valuesFiles...), WithSkipImageRefs(true))
+	err = RelocateChartDir(chartDir, "", WithValuesFiles(valuesFiles...), WithSkipImageRelocation(true))
 	require.NoError(t, err)
 
-	t.Run("Values Relocated SkipImageRef", func(t *testing.T) {
+	t.Run("Values Relocated SkipImageRelocation", func(t *testing.T) {
 		for _, valuesFile := range valuesFiles {
 			t.Logf("checking %s file", valuesFile)
 			data, err := os.ReadFile(filepath.Join(chartDir, valuesFile))
@@ -125,7 +125,7 @@ func TestRelocateChartDir(t *testing.T) {
 		assert.Equal(t, expectedAnnotations, relocatedAnnotations)
 	})
 
-	t.Run("ImageLock Relocated SkipImageRef", func(t *testing.T) {
+	t.Run("ImageLock Relocated SkipImageRelocation", func(t *testing.T) {
 		data, err := os.ReadFile(filepath.Join(chartDir, "Images.lock"))
 		assert.NoError(t, err)
 		var lockData map[string]interface{}

pkg/relocator/options.go

@@ -9,22 +9,22 @@ import (
 
 // RelocateConfig defines the configuration used in the relocator functions
 type RelocateConfig struct {
-	ImageLockConfig  imagelock.Config
-	Log              log.Logger
-	RelocateLockFile bool
-	Recursive        bool
-	SkipImageRefs    bool
-	ValuesFiles      []string
+	ImageLockConfig     imagelock.Config
+	Log                 log.Logger
+	RelocateLockFile    bool
+	Recursive           bool
+	SkipImageRelocation bool
+	ValuesFiles         []string
 }
 
 // NewRelocateConfig returns a new RelocateConfig with default settings
 func NewRelocateConfig(opts ...RelocateOption) *RelocateConfig {
 	cfg := &RelocateConfig{
-		Log:              silentLog.NewLogger(),
-		SkipImageRefs:    false,
-		RelocateLockFile: true,
-		ImageLockConfig:  *imagelock.NewImagesLockConfig(),
-		ValuesFiles:      []string{"values.yaml"},
+		Log:                 silentLog.NewLogger(),
+		SkipImageRelocation: false,
+		RelocateLockFile:    true,
+		ImageLockConfig:     *imagelock.NewImagesLockConfig(),
+		ValuesFiles:         []string{"values.yaml"},
 	}
 	for _, opt := range opts {
 		opt(cfg)
@@ -48,10 +48,10 @@ func WithAnnotationsKey(str string) func(rc *RelocateConfig) {
 	}
 }
 
-// WithSkipImageRefs configures the skipImageRefs configuration
-func WithSkipImageRefs(skipimagerefs bool) func(rc *RelocateConfig) {
+// WithSkipImageRelocation configures the SkipImageRelocation configuration
+func WithSkipImageRelocation(skipImageRelocation bool) func(rc *RelocateConfig) {
 	return func(rc *RelocateConfig) {
-		rc.SkipImageRefs = skipimagerefs
+		rc.SkipImageRelocation = skipImageRelocation
 	}
 }
 

testdata/scenarios/recursive-chart/Chart.yaml

@@ -0,0 +1,13 @@
+name: chartA
+version: 1.0.0
+annotations:
+  category: CMS
+  licenses: Apache-2.0
+  images: |
+    - name: imagea
+      image: registry-1.docker.io/bitnami/imagea:1.0.8-debian-12-r7
+
+dependencies:
+  - name: chartB
+    repository: oci://registry-1.docker.io/bitnamicharts
+    version: 2.0.0

testdata/scenarios/recursive-chart/charts/chartB/Chart.yaml

@@ -0,0 +1,12 @@
+name: chartB
+version: 2.0.0
+annotations:
+  category: CMS
+  licenses: Apache-2.0
+  images: |
+    - name: imageb
+      image: registry-1.docker.io/bitnami/imageb:1.0.8-debian-12-r7
+dependencies:
+  - name: chartC
+    repository: oci://registry-1.docker.io/bitnamicharts
+    version: 3.0.0

testdata/scenarios/recursive-chart/charts/chartB/charts/chartC/Chart.yaml

@@ -0,0 +1,8 @@
+name: chartC
+version: 3.0.0
+annotations:
+  category: CMS
+  licenses: Apache-2.0
+  images: |
+    - name: imagec
+      image: registry-1.docker.io/bitnami/imagec:1.0.8-debian-12-r7