Skip to content

Commit

Permalink
feat: make yaml-update syntax the same as json-update syntax (akuity#…
Browse files Browse the repository at this point in the history 
…3156)

Signed-off-by: Kent Rancourt <[email protected]>
  • Loading branch information
@krancour @fykaa
krancour authored and fykaa committed Jan 16, 2025
1 parent a03d3da commit b74e550
Show file tree
Hide file tree
Showing 12 changed files with 450 additions and 119 deletions.
2 changes: 1 addition & 1 deletion docs/docs/35-references/10-promotion-steps.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ followed by a [`helm-template`](#helm-template) step.
|------|------|----------|-------------|
| `path` | `string` | Y | Path to a YAML file. This path is relative to the temporary workspace that Kargo provisions for use by the promotion process. |
| `updates` | `[]object` | Y | The details of changes to be applied to the file. At least one must be specified. |
| `updates[].key` | `string` | Y | The key to update within the file. For nested values, use a YAML dot notation path. |
| `updates[].key` | `string` | Y | The key to update within the file. For nested values, use dots to delimit key parts. e.g. `image.tag`. The syntax is identical to that supported by the `json-update` step and is documented in more detail [here](https://github.com/tidwall/sjson?tab=readme-ov-file#path-syntax). |
| `updates[].value` | `string` | Y | The new value for the key. Typically specified using an expression. |

#### `yaml-update` Example
Expand Down
15 changes: 9 additions & 6 deletions internal/directives/helm_chart_updater.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,9 @@ func (h *helmChartUpdater) processChartUpdates(
stepCtx *PromotionStepContext,
cfg HelmUpdateChartConfig,
chartDependencies []chartDependency,
) (map[string]string, error) {
changes := make(map[string]string)
for _, update := range cfg.Charts {
) ([]intyaml.Update, error) {
updates := make([]intyaml.Update, len(cfg.Charts))
for i, update := range cfg.Charts {
version := update.Version
if update.Version == "" {
// TODO(krancour): Remove this for v1.3.0
Expand Down Expand Up @@ -169,9 +169,12 @@ func (h *helmChartUpdater) processChartUpdates(
}

var updateUsed bool
for i, dep := range chartDependencies {
for j, dep := range chartDependencies {
if dep.Repository == update.Repository && dep.Name == update.Name {
changes[fmt.Sprintf("dependencies.%d.version", i)] = version
updates[i] = intyaml.Update{
Key: fmt.Sprintf("dependencies.%d.version", j),
Value: version,
}
updateUsed = true
break
}
Expand All @@ -183,7 +186,7 @@ func (h *helmChartUpdater) processChartUpdates(
)
}
}
return changes, nil
return updates, nil
}

func (h *helmChartUpdater) updateDependencies(
Expand Down
54 changes: 43 additions & 11 deletions internal/directives/helm_chart_updater_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
kargoapi "github.com/akuity/kargo/api/v1alpha1"
"github.com/akuity/kargo/internal/credentials"
"github.com/akuity/kargo/internal/helm"
intyaml "github.com/akuity/kargo/internal/yaml"
)

func Test_helmChartUpdater_validate(t *testing.T) {
Expand Down Expand Up @@ -310,7 +311,7 @@ func Test_helmChartUpdater_processChartUpdates(t *testing.T) {
context *PromotionStepContext
cfg HelmUpdateChartConfig
chartDependencies []chartDependency
assertions func(*testing.T, map[string]string, error)
assertions func(*testing.T, []intyaml.Update, error)
}{
{
name: "finds chart update",
Expand Down Expand Up @@ -358,9 +359,13 @@ func Test_helmChartUpdater_processChartUpdates(t *testing.T) {
chartDependencies: []chartDependency{
{Repository: "https://charts.example.com", Name: "test-chart"},
},
assertions: func(t *testing.T, changes map[string]string, err error) {
assertions: func(t *testing.T, updates []intyaml.Update, err error) {
assert.NoError(t, err)
assert.Equal(t, map[string]string{"dependencies.0.version": "1.0.0"}, changes)
assert.Equal(
t,
[]intyaml.Update{{Key: "dependencies.0.version", Value: "1.0.0"}},
updates,
)
},
},
{
Expand All @@ -378,7 +383,7 @@ func Test_helmChartUpdater_processChartUpdates(t *testing.T) {
chartDependencies: []chartDependency{
{Repository: "https://charts.example.com", Name: "non-existent-chart"},
},
assertions: func(t *testing.T, _ map[string]string, err error) {
assertions: func(t *testing.T, _ []intyaml.Update, err error) {
assert.ErrorContains(t, err, "not found in referenced Freight")
},
},
Expand Down Expand Up @@ -430,7 +435,7 @@ func Test_helmChartUpdater_processChartUpdates(t *testing.T) {
{Repository: "https://charts.example.com", Name: "chart1"},
{Repository: "https://charts.example.com", Name: "chart2"},
},
assertions: func(t *testing.T, _ map[string]string, err error) {
assertions: func(t *testing.T, _ []intyaml.Update, err error) {
assert.ErrorContains(t, err, "not found in referenced Freight")
},
},
Expand Down Expand Up @@ -484,9 +489,13 @@ func Test_helmChartUpdater_processChartUpdates(t *testing.T) {
chartDependencies: []chartDependency{
{Repository: "https://charts.example.com", Name: "origin-chart"},
},
assertions: func(t *testing.T, changes map[string]string, err error) {
assertions: func(t *testing.T, updates []intyaml.Update, err error) {
assert.NoError(t, err)
assert.Equal(t, map[string]string{"dependencies.0.version": "2.0.0"}, changes)
assert.Equal(
t,
[]intyaml.Update{{Key: "dependencies.0.version", Value: "2.0.0"}},
updates,
)
},
},
{
Expand All @@ -506,9 +515,32 @@ func Test_helmChartUpdater_processChartUpdates(t *testing.T) {
chartDependencies: []chartDependency{
{Repository: "https://charts.example.com", Name: "origin-chart"},
},
assertions: func(t *testing.T, changes map[string]string, err error) {
assertions: func(t *testing.T, updates []intyaml.Update, err error) {
assert.NoError(t, err)
assert.Equal(t, map[string]string{"dependencies.0.version": "fake-version"}, changes)
assert.Equal(
t,
[]intyaml.Update{{Key: "dependencies.0.version", Value: "fake-version"}},
updates,
)
},
},
{
name: "update specified for non-existent chart dependency",
context: &PromotionStepContext{
Project: "test-project",
},
cfg: HelmUpdateChartConfig{
Charts: []Chart{
{
Repository: "https://charts.example.com",
Name: "origin-chart",
Version: "fake-version",
},
},
},
chartDependencies: []chartDependency{},
assertions: func(t *testing.T, _ []intyaml.Update, err error) {
assert.ErrorContains(t, err, "no dependency in Chart.yaml matched update")
},
},
}
Expand All @@ -523,8 +555,8 @@ func Test_helmChartUpdater_processChartUpdates(t *testing.T) {
stepCtx := tt.context
stepCtx.KargoClient = fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.objects...).Build()

changes, err := runner.processChartUpdates(context.Background(), stepCtx, tt.cfg, tt.chartDependencies)
tt.assertions(t, changes, err)
updates, err := runner.processChartUpdates(context.Background(), stepCtx, tt.cfg, tt.chartDependencies)
tt.assertions(t, updates, err)
})
}
}
Expand Down
17 changes: 10 additions & 7 deletions internal/directives/helm_image_updater.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (

kargoapi "github.com/akuity/kargo/api/v1alpha1"
"github.com/akuity/kargo/internal/controller/freight"
libYAML "github.com/akuity/kargo/internal/yaml"
intyaml "github.com/akuity/kargo/internal/yaml"
)

func init() {
Expand Down Expand Up @@ -99,11 +99,11 @@ func (h *helmImageUpdater) generateImageUpdates(
ctx context.Context,
stepCtx *PromotionStepContext,
cfg HelmUpdateImageConfig,
) (map[string]string, []string, error) {
updates := make(map[string]string, len(cfg.Images))
) ([]intyaml.Update, []string, error) {
updates := make([]intyaml.Update, len(cfg.Images))
fullImageRefs := make([]string, 0, len(cfg.Images))

for _, image := range cfg.Images {
for i, image := range cfg.Images {
desiredOrigin := h.getDesiredOrigin(image.FromOrigin)

targetImage, err := freight.FindImage(
Expand All @@ -124,7 +124,10 @@ func (h *helmImageUpdater) generateImageUpdates(
return nil, nil, err
}

updates[image.Key] = value
updates[i] = intyaml.Update{
Key: image.Key,
Value: value,
}
fullImageRefs = append(fullImageRefs, imageRef)
}
return updates, fullImageRefs, nil
Expand Down Expand Up @@ -157,12 +160,12 @@ func (h *helmImageUpdater) getImageValues(image *kargoapi.Image, valueType strin
}
}

func (h *helmImageUpdater) updateValuesFile(workDir string, path string, changes map[string]string) error {
func (h *helmImageUpdater) updateValuesFile(workDir string, path string, updates []intyaml.Update) error {
absValuesFile, err := securejoin.SecureJoin(workDir, path)
if err != nil {
return fmt.Errorf("error joining path %q: %w", path, err)
}
if err := libYAML.SetStringsInFile(absValuesFile, changes); err != nil {
if err := intyaml.SetStringsInFile(absValuesFile, updates); err != nil {
return fmt.Errorf("error updating image references in values file %q: %w", path, err)
}
return nil
Expand Down
45 changes: 31 additions & 14 deletions internal/directives/helm_image_updater_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"

kargoapi "github.com/akuity/kargo/api/v1alpha1"
intyaml "github.com/akuity/kargo/internal/yaml"
)

func Test_helmImageUpdater_validate(t *testing.T) {
Expand Down Expand Up @@ -334,7 +335,7 @@ func Test_helmImageUpdater_generateImageUpdates(t *testing.T) {
objects []client.Object
stepCtx *PromotionStepContext
cfg HelmUpdateImageConfig
assertions func(*testing.T, map[string]string, []string, error)
assertions func(*testing.T, []intyaml.Update, []string, error)
}{
{
name: "finds image update",
Expand Down Expand Up @@ -378,9 +379,13 @@ func Test_helmImageUpdater_generateImageUpdates(t *testing.T) {
{Key: "image.tag", Image: "docker.io/library/nginx", Value: Tag},
},
},
assertions: func(t *testing.T, changes map[string]string, summary []string, err error) {
assertions: func(t *testing.T, updates []intyaml.Update, summary []string, err error) {
assert.NoError(t, err)
assert.Equal(t, map[string]string{"image.tag": "1.19.0"}, changes)
assert.Equal(
t,
[]intyaml.Update{{Key: "image.tag", Value: "1.19.0"}},
updates,
)
assert.Equal(t, []string{"docker.io/library/nginx:1.19.0"}, summary)
},
},
Expand All @@ -396,7 +401,7 @@ func Test_helmImageUpdater_generateImageUpdates(t *testing.T) {
{Key: "image.tag", Image: "docker.io/library/non-existent", Value: Tag},
},
},
assertions: func(t *testing.T, _ map[string]string, _ []string, err error) {
assertions: func(t *testing.T, _ []intyaml.Update, _ []string, err error) {
assert.ErrorContains(t, err, "not found in referenced Freight")
},
},
Expand Down Expand Up @@ -443,7 +448,7 @@ func Test_helmImageUpdater_generateImageUpdates(t *testing.T) {
{Key: "image2.tag", Image: "docker.io/library/non-existent", Value: Tag},
},
},
assertions: func(t *testing.T, _ map[string]string, _ []string, err error) {
assertions: func(t *testing.T, _ []intyaml.Update, _ []string, err error) {
assert.ErrorContains(t, err, "not found in referenced Freight")
},
},
Expand Down Expand Up @@ -494,9 +499,13 @@ func Test_helmImageUpdater_generateImageUpdates(t *testing.T) {
},
},
},
assertions: func(t *testing.T, changes map[string]string, summary []string, err error) {
assertions: func(t *testing.T, updates []intyaml.Update, summary []string, err error) {
assert.NoError(t, err)
assert.Equal(t, map[string]string{"image.tag": "2.0.0"}, changes)
assert.Equal(
t,
[]intyaml.Update{{Key: "image.tag", Value: "2.0.0"}},
updates,
)
assert.Equal(t, []string{"docker.io/library/origin-image:2.0.0"}, summary)
},
},
Expand All @@ -512,8 +521,8 @@ func Test_helmImageUpdater_generateImageUpdates(t *testing.T) {
stepCtx := tt.stepCtx
stepCtx.KargoClient = fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.objects...).Build()

changes, summary, err := runner.generateImageUpdates(context.Background(), stepCtx, tt.cfg)
tt.assertions(t, changes, summary, err)
updates, summary, err := runner.generateImageUpdates(context.Background(), stepCtx, tt.cfg)
tt.assertions(t, updates, summary, err)
})
}
}
Expand Down Expand Up @@ -640,13 +649,13 @@ func Test_helmImageUpdater_updateValuesFile(t *testing.T) {
tests := []struct {
name string
valuesContent string
changes map[string]string
updates []intyaml.Update
assertions func(*testing.T, string, error)
}{
{
name: "successful update",
valuesContent: "key: value\n",
changes: map[string]string{"key": "newvalue"},
updates: []intyaml.Update{{Key: "key", Value: "newvalue"}},
assertions: func(t *testing.T, valuesFilePath string, err error) {
require.NoError(t, err)

Expand All @@ -659,7 +668,7 @@ func Test_helmImageUpdater_updateValuesFile(t *testing.T) {
{
name: "file does not exist",
valuesContent: "",
changes: map[string]string{"key": "value"},
updates: []intyaml.Update{{Key: "key", Value: "value"}},
assertions: func(t *testing.T, valuesFilePath string, err error) {
require.ErrorContains(t, err, "no such file or directory")
require.NoFileExists(t, valuesFilePath)
Expand All @@ -668,7 +677,7 @@ func Test_helmImageUpdater_updateValuesFile(t *testing.T) {
{
name: "empty changes",
valuesContent: "key: value\n",
changes: map[string]string{},
updates: []intyaml.Update{},
assertions: func(t *testing.T, valuesFilePath string, err error) {
require.NoError(t, err)
require.FileExists(t, valuesFilePath)
Expand All @@ -677,6 +686,14 @@ func Test_helmImageUpdater_updateValuesFile(t *testing.T) {
assert.Equal(t, "key: value\n", string(content))
},
},
{
name: "update specified for non-existent key",
valuesContent: "key: value\n",
updates: []intyaml.Update{{Key: "non-existent-key", Value: "newvalue"}},
assertions: func(t *testing.T, _ string, err error) {
require.ErrorContains(t, err, "key path not found")
},
},
}

runner := &helmImageUpdater{}
Expand All @@ -691,7 +708,7 @@ func Test_helmImageUpdater_updateValuesFile(t *testing.T) {
require.NoError(t, err)
}

err := runner.updateValuesFile(workDir, path.Base(valuesFile), tt.changes)
err := runner.updateValuesFile(workDir, path.Base(valuesFile), tt.updates)
tt.assertions(t, valuesFile, err)
})
}
Expand Down
Loading

0 comments on commit b74e550

Please sign in to comment.