Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Recursively update images on nested subcharts regardless of depth (#67) #83

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions cmd/dt/relocate/relocate.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
// NewCmd builds a new relocate command
func NewCmd(cfg *config.Config) *cobra.Command {
valuesFiles := []string{"values.yaml"}
skipImageRefs := false
cmd := &cobra.Command{
Use: "relocate CHART_PATH OCI_URI",
Short: "Relocates a Helm chart",
Expand All @@ -35,6 +36,7 @@ func NewCmd(cfg *config.Config) *cobra.Command {
relocator.WithLog(l), relocator.Recursive,
relocator.WithAnnotationsKey(cfg.AnnotationsKey),
relocator.WithValuesFiles(valuesFiles...),
relocator.WithSkipImageRefs(skipImageRefs),
)
}); err != nil {
return l.Failf("failed to relocate Helm chart %q: %w", chartPath, err)
Expand All @@ -46,6 +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")

return cmd
}
48 changes: 48 additions & 0 deletions cmd/dt/relocate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
31 changes: 27 additions & 4 deletions cmd/dt/unwrap/unwrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ type Config struct {
TempDirectory string
Version string
Carvelize bool
SkipImageRefs bool
SkipImages bool
KeepArtifacts bool
FetchArtifacts bool
Auth Auth
Expand Down Expand Up @@ -139,6 +141,20 @@ func WithCarvelize(carvelize bool) func(c *Config) {
}
}

// WithSkipImageRefs configures the WithSkipImageRefs of the WrapConfig
juan131 marked this conversation as resolved.
Show resolved Hide resolved
func WithSkipImageRefs(skipimagerefs bool) func(c *Config) {
return func(c *Config) {
c.SkipImageRefs = skipimagerefs
}
}

// WithSkipImages configures the WithSkipImages of the WrapConfig
juan131 marked this conversation as resolved.
Show resolved Hide resolved
func WithSkipImages(skipimages bool) func(c *Config) {
return func(c *Config) {
c.SkipImages = skipimages
}
}

// WithFetchArtifacts configures the FetchArtifacts of the WrapConfig
func WithFetchArtifacts(fetchArtifacts bool) func(c *Config) {
return func(c *Config) {
Expand Down Expand Up @@ -279,6 +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),
)
}); err != nil {
return "", l.Failf("failed to relocate %q: %w", chartPath, err)
Expand All @@ -287,7 +304,7 @@ func unwrapChart(inputChart, registryURL, pushChartURL string, opts ...Option) (

images := getImageList(wrap, l)

if len(images) > 0 {
if len(images) > 0 && !cfg.SkipImages {
// If we are not in interactive mode, we do not show the list of images
if cfg.Interactive {
showImagesSummary(images, l)
Expand Down Expand Up @@ -435,9 +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
sayYes bool
pushChartURL string
version string
skipImageRefs bool
skipImages bool
)
valuesFiles := []string{"values.yaml"}
cmd := &cobra.Command{
Expand Down Expand Up @@ -471,6 +490,8 @@ func NewCmd(cfg *config.Config) *cobra.Command {
WithTempDirectory(tempDir),
WithUsePlainHTTP(cfg.UsePlainHTTP),
WithValuesFiles(valuesFiles...),
WithSkipImageRefs(skipImageRefs),
WithSkipImages(skipImages),
)
if err != nil {
return err
Expand All @@ -489,6 +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")

return cmd
}
20 changes: 16 additions & 4 deletions cmd/dt/wrap/wrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type Config struct {
Carvelize bool
KeepArtifacts bool
FetchArtifacts bool
SkipImages bool
Auth Auth
ContainerRegistryAuth Auth
OutputFile string
Expand Down Expand Up @@ -129,6 +130,13 @@ func WithFetchArtifacts(fetchArtifacts bool) func(c *Config) {
}
}

// WithSkipImages configures the WithSkipImages of the WrapConfig
juan131 marked this conversation as resolved.
Show resolved Hide resolved
func WithSkipImages(skipimages bool) func(c *Config) {
return func(c *Config) {
c.SkipImages = skipimages
}
}

// WithVersion configures the Version of the WrapConfig
func WithVersion(version string) func(c *Config) {
return func(c *Config) {
Expand Down Expand Up @@ -400,10 +408,11 @@ func wrapChart(inputPath string, opts ...Option) (string, error) {
outputFile = filepath.Join(filepath.Dir(chartRoot), outputBaseName)
}
}
if err := pullImages(wrap, subCfg); err != nil {
return "", err
if !cfg.SkipImages {
if err := pullImages(wrap, subCfg); err != nil {
return "", err
}
}

if cfg.Carvelize {
if err := l.Section(fmt.Sprintf("Generating Carvel bundle for Helm chart %q", chartPath), func(childLog log.SectionLogger) error {
return carvelize.GenerateBundle(
Expand Down Expand Up @@ -439,7 +448,8 @@ func NewCmd(cfg *config.Config) *cobra.Command {
var platforms []string
var fetchArtifacts bool
var carvelize bool
examples := ` # Wrap a Helm chart from a local folder
var skipImages bool
var examples = ` # Wrap a Helm chart from a local folder
$ dt wrap examples/mariadb

# Wrap a Helm chart in an OCI registry
Expand Down Expand Up @@ -475,6 +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),
)
if err != nil {
if _, ok := err.(*log.LoggedError); ok {
Expand All @@ -496,6 +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")

return cmd
}
10 changes: 7 additions & 3 deletions pkg/relocator/chart.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ type RelocationResult struct {
}

func relocateChart(chart *cu.Chart, prefix string, cfg *RelocateConfig) error {
var allErrors error
if cfg.SkipImageRefs {
return allErrors
}
juan131 marked this conversation as resolved.
Show resolved Hide resolved

valuesReplRes, err := relocateValues(chart, prefix)
if err != nil {
return fmt.Errorf("failed to relocate chart: %v", err)
Expand All @@ -39,8 +44,6 @@ func relocateChart(chart *cu.Chart, prefix string, cfg *RelocateConfig) error {
}
}

var allErrors error

// TODO: Compare annotations with values replacements
annotationsRelocResult, err := relocateAnnotations(chart, prefix)
if err != nil {
Expand Down Expand Up @@ -95,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
}

Expand Down
62 changes: 62 additions & 0 deletions pkg/relocator/chart_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,66 @@ func TestRelocateChartDir(t *testing.T) {
assert.Equal(t, expectedData, relocatedImagesData)

})

// create a new chart dir to reset for the SkipImageRef tests
chartDir = sb.TempFile()
require.NoError(t, tu.RenderScenario(scenarioDir, chartDir, map[string]interface{}{"ServerURL": serverURL}))
err = RelocateChartDir(chartDir, "", WithValuesFiles(valuesFiles...), WithSkipImageRefs(true))
require.NoError(t, err)

t.Run("Values Relocated SkipImageRef", func(t *testing.T) {
for _, valuesFile := range valuesFiles {
t.Logf("checking %s file", valuesFile)
data, err := os.ReadFile(filepath.Join(chartDir, valuesFile))
require.NoError(t, err)
relocatedValues, err := tu.NormalizeYAML(string(data))
require.NoError(t, err)

expectedData, err := tu.RenderTemplateFile(filepath.Join(scenarioDir, fmt.Sprintf("%s.tmpl", valuesFile)), map[string]string{"ServerURL": serverURL})
require.NoError(t, err)

expectedValues, err := tu.NormalizeYAML(expectedData)
require.NoError(t, err)
assert.Equal(t, expectedValues, relocatedValues)
}
})

t.Run("Annotations Relocated ", func(t *testing.T) {
c, err := loader.Load(chartDir)
require.NoError(t, err)

relocatedAnnotations, err := tu.NormalizeYAML(c.Metadata.Annotations["images"])
require.NoError(t, err)

require.NotEqual(t, relocatedAnnotations, "")

expectedData, err := tu.RenderTemplateFile(filepath.Join(scenarioDir, "images.partial.tmpl"), map[string]string{"ServerURL": serverURL})
require.NoError(t, err)

expectedAnnotations, err := tu.NormalizeYAML(expectedData)
require.NoError(t, err)
assert.Equal(t, expectedAnnotations, relocatedAnnotations)
})

t.Run("ImageLock Relocated SkipImageRef", func(t *testing.T) {
data, err := os.ReadFile(filepath.Join(chartDir, "Images.lock"))
assert.NoError(t, err)
var lockData map[string]interface{}

require.NoError(t, yaml.Unmarshal(data, &lockData))

imagesElemData, err := yaml.Marshal(lockData["images"])
require.NoError(t, err)

relocatedImagesData, err := tu.NormalizeYAML(string(imagesElemData))
require.NoError(t, err)

expectedData, err := tu.RenderTemplateFile(filepath.Join(scenarioDir, "lock_images.partial.tmpl"), map[string]string{"ServerURL": serverURL})
require.NoError(t, err)
expectedData, err = tu.NormalizeYAML(expectedData)
require.NoError(t, err)

assert.Equal(t, expectedData, relocatedImagesData)

})
}
9 changes: 9 additions & 0 deletions pkg/relocator/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@ type RelocateConfig struct {
Log log.Logger
RelocateLockFile bool
Recursive bool
SkipImageRefs 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"},
Expand All @@ -46,6 +48,13 @@ func WithAnnotationsKey(str string) func(rc *RelocateConfig) {
}
}

// WithSkipImageRefs configures the skipImageRefs configuration
func WithSkipImageRefs(skipimagerefs bool) func(rc *RelocateConfig) {
return func(rc *RelocateConfig) {
rc.SkipImageRefs = skipimagerefs
}
}

// WithRelocateLockFile configures the RelocateLockFile configuration
func WithRelocateLockFile(relocateLock bool) func(rc *RelocateConfig) {
return func(rc *RelocateConfig) {
Expand Down
13 changes: 13 additions & 0 deletions testdata/scenarios/recursive-chart/Chart.yaml
Original file line number Diff line number Diff line change
@@ -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
12 changes: 12 additions & 0 deletions testdata/scenarios/recursive-chart/charts/chartB/Chart.yaml
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Loading