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.

Sign up for GitHub

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

feat: make yaml-update syntax the same as json-update syntax #3156

Merged
merged 3 commits into from
Dec 19, 2024
Merged

feat: make yaml-update syntax the same as json-update syntax #3156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f6c0113
make yaml updates array-based instead of map based
krancour Dec 18, 2024
ad8f7cc
make syntax supported by yaml pkg consistent with tidwall/sjson
krancour Dec 18, 2024
bed52c4
corresponding doc updates
krancour Dec 19, 2024
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
2 changes: 1 addition & 1 deletion docs/docs/35-references/10-promotion-steps.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading