This branch adds support for nested helm dependencies

The previous implementation had a recursive function but it terminated after one level
This branch adds support for nested repository urls - the previous implementation only supported a single level

Resolves: #67

pkg/chartutils/values.go

@@ -181,8 +181,8 @@ func parseValuesImageElement(data map[string]interface{}) *ValuesImageElement {
 	for _, k := range imageElementKeys {
 		v, ok := data[k]
 		if !ok {
-			// digest is optional
-			if k == "digest" {
+			// digest and registry are optional
+			if k == "digest" || k == "registry" {
 				continue
 			}
 			return nil
@@ -193,9 +193,5 @@ func parseValuesImageElement(data map[string]interface{}) *ValuesImageElement {
 		}
 		elemData[k] = vStr
 	}
-	// An empty registry may be acceptable, but not an empty repository
-	if elemData["repository"] == "" {
-		return nil
-	}
 	return valuesImageElementFromMap(elemData)
 }

pkg/relocator/chart.go

@@ -25,8 +25,8 @@ type RelocationResult struct {
 	Count int
 }
 
-func relocateChart(chart *cu.Chart, prefix string, cfg *RelocateConfig) error {
-	valuesReplRes, err := relocateValues(chart, prefix)
+func relocateChart(chart *cu.Chart, newRegistry string, cfg *RelocateConfig) error {
+	valuesReplRes, err := relocateValues(chart, newRegistry)
 	if err != nil {
 		return fmt.Errorf("failed to relocate chart: %v", err)
 	}
@@ -42,7 +42,7 @@ func relocateChart(chart *cu.Chart, prefix string, cfg *RelocateConfig) error {
 	var allErrors error
 
 	// TODO: Compare annotations with values replacements
-	annotationsRelocResult, err := relocateAnnotations(chart, prefix)
+	annotationsRelocResult, err := relocateAnnotations(chart, newRegistry)
 	if err != nil {
 		allErrors = errors.Join(allErrors, fmt.Errorf("failed to relocate Helm chart: %v", err))
 	} else {
@@ -58,19 +58,27 @@ func relocateChart(chart *cu.Chart, prefix string, cfg *RelocateConfig) error {
 
 	lockFile := chart.LockFilePath()
 	if utils.FileExists(lockFile) {
-		err = RelocateLockFile(lockFile, prefix)
+		err = RelocateLockFile(lockFile, newRegistry)
 		if err != nil {
 			allErrors = errors.Join(allErrors, fmt.Errorf("failed to relocate Images.lock file: %v", err))
 		}
 	}
 
+	if cfg.Recursive {
+		for _, dep := range chart.Dependencies() {
+			if err := relocateChart(dep, newRegistry, cfg); err != nil {
+				allErrors = errors.Join(allErrors, fmt.Errorf("failed to relocate Helm SubChart %q: %v", dep.Chart().ChartFullPath(), err))
+			}
+		}
+	}
+
 	return allErrors
 }
 
 // RelocateChartDir relocates the chart (Chart.yaml annotations, Images.lock and values.yaml) specified
 // by chartPath using the provided prefix
-func RelocateChartDir(chartPath string, prefix string, opts ...RelocateOption) error {
-	prefix = normalizeRelocateURL(prefix)
+func RelocateChartDir(chartPath string, newRegistry string, opts ...RelocateOption) error {
+	newRegistry = normalizeRelocateURL(newRegistry)
 
 	cfg := NewRelocateConfig(opts...)
 
@@ -79,39 +87,30 @@ func RelocateChartDir(chartPath string, prefix string, opts ...RelocateOption) e
 		return fmt.Errorf("failed to load Helm chart: %v", err)
 	}
 
-	err = relocateChart(chart, prefix, cfg)
+	err = relocateChart(chart, newRegistry, cfg)
 	if err != nil {
 		return err
 	}
 	if utils.FileExists(filepath.Join(chartPath, carvel.CarvelImagesFilePath)) {
-		err = relocateCarvelBundle(chartPath, prefix)
+		err = relocateCarvelBundle(chartPath, newRegistry)
 
 		if err != nil {
 			return err
 		}
 	}
 
-	var allErrors error
-
-	if cfg.Recursive {
-		for _, dep := range chart.Dependencies() {
-			if err := relocateChart(dep, prefix, cfg); err != nil {
-				allErrors = errors.Join(allErrors, fmt.Errorf("failed to reloacte Helm SubChart %q: %v", dep.Chart().ChartFullPath(), err))
-			}
-		}
-	}
-	return allErrors
+	return err
 }
 
-func relocateCarvelBundle(chartRoot string, prefix string) error {
+func relocateCarvelBundle(chartRoot string, newRegistry string) error {
 
 	//TODO: Do better detection here, imgpkg probably has something
 	carvelImagesFile := filepath.Join(chartRoot, carvel.CarvelImagesFilePath)
 	lock, err := lockconfig.NewImagesLockFromPath(carvelImagesFile)
 	if err != nil {
 		return fmt.Errorf("failed to load Carvel images lock: %v", err)
 	}
-	result, err := RelocateCarvelImagesLock(&lock, prefix)
+	result, err := RelocateCarvelImagesLock(&lock, newRegistry)
 	if err != nil {
 		return err
 	}
@@ -125,9 +124,9 @@ func relocateCarvelBundle(chartRoot string, prefix string) error {
 }
 
 // RelocateCarvelImagesLock rewrites the images urls in the provided lock using prefix
-func RelocateCarvelImagesLock(lock *lockconfig.ImagesLock, prefix string) (*RelocationResult, error) {
+func RelocateCarvelImagesLock(lock *lockconfig.ImagesLock, newRegistry string) (*RelocationResult, error) {
 
-	count, err := relocateCarvelImages(lock.Images, prefix)
+	count, err := relocateCarvelImages(lock.Images, newRegistry)
 	if err != nil {
 		return nil, fmt.Errorf("failed to relocate Carvel images lock file: %v", err)
 	}
@@ -141,10 +140,10 @@ func RelocateCarvelImagesLock(lock *lockconfig.ImagesLock, prefix string) (*Relo
 
 }
 
-func relocateCarvelImages(images []lockconfig.ImageRef, prefix string) (count int, err error) {
+func relocateCarvelImages(images []lockconfig.ImageRef, newRegistry string) (count int, err error) {
 	var allErrors error
 	for i, img := range images {
-		norm, err := utils.RelocateImageURL(img.Image, prefix, true)
+		norm, err := utils.RelocateImageURL(img.Image, newRegistry, true)
 		if err != nil {
 			allErrors = errors.Join(allErrors, err)
 			continue

pkg/relocator/imagelock.go

@@ -9,10 +9,10 @@ import (
 	"github.com/vmware-labs/distribution-tooling-for-helm/pkg/utils"
 )
 
-func relocateImages(images imagelock.ImageList, prefix string) (count int, err error) {
+func relocateImages(images imagelock.ImageList, newRegistry string) (count int, err error) {
 	var allErrors error
 	for _, img := range images {
-		norm, err := utils.RelocateImageURL(img.Image, prefix, true)
+		norm, err := utils.RelocateImageRegistry(img.Image, newRegistry, true)
 		if err != nil {
 			allErrors = errors.Join(allErrors, err)
 			continue
@@ -24,8 +24,8 @@ func relocateImages(images imagelock.ImageList, prefix string) (count int, err e
 }
 
 // RelocateLock rewrites the images urls in the provided lock using prefix
-func RelocateLock(lock *imagelock.ImagesLock, prefix string) (*RelocationResult, error) {
-	count, err := relocateImages(lock.Images, prefix)
+func RelocateLock(lock *imagelock.ImagesLock, newRegistry string) (*RelocationResult, error) {
+	count, err := relocateImages(lock.Images, newRegistry)
 	if err != nil {
 		return nil, fmt.Errorf("failed to relocate Images.lock file: %v", err)
 	}
@@ -37,12 +37,12 @@ func RelocateLock(lock *imagelock.ImagesLock, prefix string) (*RelocationResult,
 }
 
 // RelocateLockFile relocates images urls in the provided Images.lock using prefix
-func RelocateLockFile(file string, prefix string) error {
+func RelocateLockFile(file string, newRegistry string) error {
 	lock, err := imagelock.FromYAMLFile(file)
 	if err != nil {
 		return fmt.Errorf("failed to load Images.lock: %v", err)
 	}
-	result, err := RelocateLock(lock, prefix)
+	result, err := RelocateLock(lock, newRegistry)
 	if err != nil {
 		return err
 	}

pkg/utils/utils.go

@@ -147,6 +147,55 @@ func RelocateImageURL(url string, prefix string, includeIndentifier bool) (strin
 	return newURL, nil
 }
 
+// RelocateImageRegistry rewrites the provided image URL by replacing its
+// registry with the newRegistry. If includeIdentifier is true, the tag or
+// digest of the image is included in the returned URL. The function returns
+// an error if the URL cannot be parsed or the new repository cannot be created.
+func RelocateImageRegistry(url string, newRegistry string,
+	includeIndentifier bool) (string, error) {
+	ref, err := name.ParseReference(url)
+	if err != nil {
+		return "", fmt.Errorf("failed to relocate url: %v", err)
+	}
+
+	// Create a new repository with the new registry and the repository path
+	// from the parsed reference
+	newRepo, err := name.NewRepository(fmt.Sprintf("%s/%s", newRegistry,
+		ref.Context().RepositoryStr()))
+	if err != nil {
+		return "", fmt.Errorf("failed to create new repository: %v", err)
+	}
+
+	var newRef name.Reference
+	switch v := ref.(type) {
+	case name.Tag:
+		// If the parsed reference is a Tag, create a new Tag with the new
+		// repository and the tag from the parsed reference
+		newRef, err = name.NewTag(fmt.Sprintf("%s:%s", newRepo.Name(),
+			v.TagStr()), name.WeakValidation)
+	case name.Digest:
+		// If the parsed reference is a Digest, create a new Digest with the
+		// new repository and the digest from the parsed reference
+		newRef, err = name.NewDigest(fmt.Sprintf("%s@%s", newRepo.Name(),
+			v.DigestStr()), name.WeakValidation)
+	}
+	if err != nil {
+		return "", fmt.Errorf("failed to create new reference: %v", err)
+	}
+
+	newURL := newRef.Context().Name()
+	if includeIndentifier && newRef.Identifier() != "" {
+		separator := ":"
+		if _, ok := newRef.(name.Digest); ok {
+			separator = "@"
+		}
+		newURL = fmt.Sprintf("%s%s%s", newURL, separator, newRef.Identifier())
+	}
+
+	// Return the full name of the new reference, which includes the tag or digest
+	return newURL, nil
+}
+
 // ExecuteWithRetry executes a function retrying until it succeeds or the number of retries is reached
 func ExecuteWithRetry(retries int, cb func(try int, prevErr error) error) error {
 	retry := 0

pkg/utils/utils_test.go

@@ -194,7 +194,21 @@ func TestRelocateImageURL(t *testing.T) {
 				url:    "example.com:80/foo/bar/bitnami/app",
 				prefix: newReg,
 			},
-			want: fmt.Sprintf("%s/bitnami/app", newReg),
+			want: fmt.Sprintf("%s/foo/bar/bitnami/app", newReg),
+		},
+		"Replaces registry with doubly nested repository prefex": {
+			args: args{
+				url:    "www.example.com/docker/abc/imagename",
+				prefix: newReg,
+			},
+			want: fmt.Sprintf("%s/docker/abc/imagename", newReg),
+		},
+		"Replaces registry with triply nested repository prefix": {
+			args: args{
+				url:    "www.example.com/docker/abc/testimages/imagename",
+				prefix: newReg,
+			},
+			want: fmt.Sprintf("%s/docker/abc/testimages/imagename", newReg),
 		},
 		"Replaces library repositoy": {
 			args: args{
@@ -213,7 +227,7 @@ func TestRelocateImageURL(t *testing.T) {
 	}
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
-			got, err := RelocateImageURL(tt.args.url, tt.args.prefix, tt.args.includeIndentifier)
+			got, err := RelocateImageRegistry(tt.args.url, tt.args.prefix, tt.args.includeIndentifier)
 			validateError(t, tt.expectedErr, err)
 
 			if got != tt.want {

testdata/scenarios/chart1/charts/common/Chart.yaml

@@ -10,3 +10,8 @@ icon: https://bitnami.com/downloads/logos/bitnami-mark.png
 name: common
 type: library
 version: 2.6.0
+dependencies:
+  - name: common2
+    repository: oci://registry-1.docker.io/bitnamicharts
+    version: 2.x.x
+

testdata/scenarios/chart1/charts/common/charts/common2/Chart.yaml

@@ -0,0 +1,12 @@
+annotations:
+  category: Infrastructure
+  licenses: Apache-2.0
+apiVersion: v2
+appVersion: 2.6.0
+description: A Library Helm chart for grouping common logic between bitnami charts.
+  This chart is not deployable by itself.
+home: https://bitnami.com
+icon: https://bitnami.com/downloads/logos/bitnami-mark.png
+name: common
+type: library
+version: 2.6.0