From d7c2690d24b3b34640e6574aaf86cb8cbe573f5c Mon Sep 17 00:00:00 2001 From: shortwavedave Date: Sat, 22 Jun 2024 14:32:11 -0600 Subject: [PATCH] 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 | 8 +-- pkg/relocator/chart.go | 47 +++++++++--------- pkg/relocator/imagelock.go | 12 ++--- pkg/utils/utils.go | 49 +++++++++++++++++++ pkg/utils/utils_test.go | 18 ++++++- .../scenarios/chart1/charts/common/Chart.yaml | 5 ++ .../charts/common/charts/common2/Chart.yaml | 12 +++++ 7 files changed, 113 insertions(+), 38 deletions(-) create mode 100644 testdata/scenarios/chart1/charts/common/charts/common2/Chart.yaml diff --git a/pkg/chartutils/values.go b/pkg/chartutils/values.go index 27cf373..3783587 100644 --- a/pkg/chartutils/values.go +++ b/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) } diff --git a/pkg/relocator/chart.go b/pkg/relocator/chart.go index 2ccb8e5..78bc170 100644 --- a/pkg/relocator/chart.go +++ b/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,31 +87,22 @@ 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) @@ -111,7 +110,7 @@ func relocateCarvelBundle(chartRoot string, prefix string) error { 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 diff --git a/pkg/relocator/imagelock.go b/pkg/relocator/imagelock.go index cf2118a..2b37bb5 100644 --- a/pkg/relocator/imagelock.go +++ b/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 } diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 749f577..acb8e27 100644 --- a/pkg/utils/utils.go +++ b/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 diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index a449649..13ea807 100644 --- a/pkg/utils/utils_test.go +++ b/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 { diff --git a/testdata/scenarios/chart1/charts/common/Chart.yaml b/testdata/scenarios/chart1/charts/common/Chart.yaml index 3e80239..2dc4369 100644 --- a/testdata/scenarios/chart1/charts/common/Chart.yaml +++ b/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 + diff --git a/testdata/scenarios/chart1/charts/common/charts/common2/Chart.yaml b/testdata/scenarios/chart1/charts/common/charts/common2/Chart.yaml new file mode 100644 index 0000000..3e80239 --- /dev/null +++ b/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