From 49af4c49be99c1303479deb73991af719722c793 Mon Sep 17 00:00:00 2001 From: Darcy Cleaver Date: Tue, 7 May 2024 12:44:06 -0600 Subject: [PATCH 01/10] feat: valuesFiles override support --- docs/overrides.md | 25 ++++++++++++++++++++-- src/pkg/bundle/create.go | 28 ++++++++++++++++++++++++ src/test/e2e/variable_test.go | 40 +++++++++++++++++++++++++++++++++++ src/types/bundle.go | 15 ++++++------- uds.schema.json | 20 ++++++++++++++++++ zarf.schema.json | 35 ++++++++++++++++++++++++++++++ 6 files changed, 152 insertions(+), 11 deletions(-) diff --git a/docs/overrides.md b/docs/overrides.md index 5722e35c..475221a5 100644 --- a/docs/overrides.md +++ b/docs/overrides.md @@ -59,6 +59,8 @@ packages: overrides: helm-overrides-component: podinfo: + valuesFiles: + - file: values.yaml values: - path: "replicaCount" value: 2 @@ -69,7 +71,13 @@ packages: default: "purple" ``` -This bundle will deploy the `helm-overrides-package` Zarf package and override the `replicaCount` and `ui.color` values in the `podinfo` chart. The `values` can't be modified after the bundle has been created. However, at deploy time, users can override the `UI_COLOR` and other `variables` using a environment variable called `UDS_UI_COLOR` or by specifying it in a `uds-config.yaml` like so: +```yaml +#values.yaml +podAnnotations: + customAnnotation: "customValue" +``` + +This bundle will deploy the `helm-overrides-package` Zarf package and override the `replicaCount`, `ui.color`, and `podAnnotations` values in the `podinfo` chart. The `values` can't be modified after the bundle has been created. However, at deploy time, users can override the `UI_COLOR` and other `variables` using a environment variable called `UDS_UI_COLOR` or by specifying it in a `uds-config.yaml` like so: ```yaml variables: @@ -92,6 +100,8 @@ packages: overrides: helm-overrides-component: # component name inside of the helm-overrides-package Zarf pkg podinfo: # chart name from the helm-overrides-component component + valuesFiles: + - file: values.yaml values: - path: "replicaCount" value: 2 @@ -102,7 +112,18 @@ packages: default: "purple" ``` -In this example, the `helm-overrides-package` Zarf package has a component called `helm-overrides-component` which contains a Helm chart called `podinfo`; note how these names are keys in the `overrides` block. The `podinfo` chart has a `replicaCount` value that is overridden to `2` and a variable called `UI_COLOR` that is overridden to `purple`. +```yaml +#values.yaml +podAnnotations: + customAnnotation: "customValue" +``` +In this example, the `helm-overrides-package` Zarf package has a component called `helm-overrides-component` which contains a Helm chart called `podinfo`; note how these names are keys in the `overrides` block. The `podinfo` chart has a `replicaCount` value that is overridden to `2`, a `podAnnotations` value that is overridden to include `customAnnotation: "customValue"` and a variable called `UI_COLOR` that is overridden to `purple`. + +### ValuesFiles + +The `valuesFiles` in an `overrides` block are a list of `file`'s. It allows users to override multiple values in a Zarf package component's underlying Helm chart, by providing a file with those values instead of having to include them all indiviually in the `overrides` block. + +NOTE: If a value is specified in both a `valuesFile` and as a `value` in the `overrides` block, the value set in the `valuesFile` will take precedence. ### Values diff --git a/src/pkg/bundle/create.go b/src/pkg/bundle/create.go index b8d64653..58f50aaf 100644 --- a/src/pkg/bundle/create.go +++ b/src/pkg/bundle/create.go @@ -11,11 +11,13 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/defenseunicorns/uds-cli/src/config" "github.com/defenseunicorns/uds-cli/src/pkg/bundler" + "github.com/defenseunicorns/uds-cli/src/types" zarfConfig "github.com/defenseunicorns/zarf/src/config" "github.com/defenseunicorns/zarf/src/pkg/interactive" "github.com/defenseunicorns/zarf/src/pkg/message" "github.com/defenseunicorns/zarf/src/pkg/utils" "github.com/pterm/pterm" + "helm.sh/helm/v3/pkg/chartutil" ) // Create creates a bundle @@ -26,6 +28,32 @@ func (b *Bundle) Create() error { return err } + // Populate values from valuesFiles if provided + for i, pkg := range b.bundle.Packages { + for j, overrides := range pkg.Overrides { + for k, bundleChartOverrides := range overrides { + for _, valuesFile := range bundleChartOverrides.ValuesFiles { + // Check relative vs absolute path + fileName := filepath.Join(b.cfg.CreateOpts.SourceDirectory, valuesFile.File) + if filepath.IsAbs(valuesFile.File) { + fileName = valuesFile.File + } + // read values from valuesFile + values, err := chartutil.ReadValuesFile(fileName) + if err != nil { + return err + } + // add values from valuesFile to bundleChartOverrides + for key, value := range values { + bundleChartOverrides.Values = append(bundleChartOverrides.Values, types.BundleChartValue{Path: key, Value: value}) + } + // update bundle with override values + b.bundle.Packages[i].Overrides[j][k] = bundleChartOverrides + } + } + } + } + // confirm creation if ok := b.confirmBundleCreation(); !ok { return fmt.Errorf("bundle creation cancelled") diff --git a/src/test/e2e/variable_test.go b/src/test/e2e/variable_test.go index 69e8466e..049fd5d0 100644 --- a/src/test/e2e/variable_test.go +++ b/src/test/e2e/variable_test.go @@ -157,6 +157,46 @@ func TestBundleWithHelmOverrides(t *testing.T) { remove(t, bundlePath) } +func TestBundleWithHelmOverridesValuesFile(t *testing.T) { + deployZarfInit(t) + e2e.HelmDepUpdate(t, "src/test/packages/helm/unicorn-podinfo") + e2e.CreateZarfPkg(t, "src/test/packages/helm", false) + bundleDir := "src/test/bundles/07-helm-overrides/values-file" + bundlePath := filepath.Join(bundleDir, fmt.Sprintf("uds-bundle-helm-values-file-%s-0.0.1.tar.zst", e2e.Arch)) + err := os.Setenv("UDS_CONFIG", filepath.Join("src/test/bundles/07-helm-overrides", "uds-config.yaml")) + require.NoError(t, err) + + createLocal(t, bundleDir, e2e.Arch) + deploy(t, bundlePath) + + // test values overrides + t.Run("check values overrides", func(t *testing.T) { + cmd := strings.Split("zarf tools kubectl get deploy -n podinfo unicorn-podinfo -o=jsonpath='{.spec.replicas}'", " ") + outputNumReplicas, _, err := e2e.UDS(cmd...) + require.Equal(t, "'2'", outputNumReplicas) + require.NoError(t, err) + }) + + t.Run("check object-type override in values", func(t *testing.T) { + cmd := strings.Split("zarf tools kubectl get deployment -n podinfo unicorn-podinfo -o=jsonpath='{.spec.template.metadata.annotations}'", " ") + annotations, _, err := e2e.UDS(cmd...) + require.Contains(t, annotations, "\"customAnnotation\":\"customValue\"") + require.NoError(t, err) + + }) + + t.Run("check list-type override in values", func(t *testing.T) { + cmd := strings.Split("zarf tools kubectl get deployment -n podinfo unicorn-podinfo -o=jsonpath='{.spec.template.spec.tolerations}'", " ") + tolerations, _, err := e2e.UDS(cmd...) + require.Contains(t, tolerations, "\"key\":\"uds\"") + require.Contains(t, tolerations, "\"value\":\"defense\"") + require.Contains(t, tolerations, "\"key\":\"unicorn\"") + require.Contains(t, tolerations, "\"effect\":\"NoSchedule\"") + require.NoError(t, err) + + }) +} + func TestBundleWithDupPkgs(t *testing.T) { deployZarfInit(t) e2e.SetupDockerRegistry(t, 888) diff --git a/src/types/bundle.go b/src/types/bundle.go index 3e7ef9e8..cd70cf2d 100644 --- a/src/types/bundle.go +++ b/src/types/bundle.go @@ -28,12 +28,10 @@ type Package struct { // BundleChartOverrides represents a Helm chart override to set via UDS variables type BundleChartOverrides struct { - Values []BundleChartValue `json:"values,omitempty" jsonschema:"description=List of Helm chart values to set statically"` - Variables []BundleChartVariable `json:"variables,omitempty" jsonschema:"description=List of Helm chart variables to set via UDS variables"` - Namespace string `json:"namespace,omitempty" jsonschema:"description=The namespace to deploy the Helm chart to"` - - // EXPERIMENTAL, not yet implemented - //ValueFiles []BundleChartValueFile `json:"value-files,omitempty" jsonschema:"description=List of Helm chart value files to set statically"` + Values []BundleChartValue `json:"values,omitempty" jsonschema:"description=List of Helm chart values to set statically"` + Variables []BundleChartVariable `json:"variables,omitempty" jsonschema:"description=List of Helm chart variables to set via UDS variables"` + Namespace string `json:"namespace,omitempty" jsonschema:"description=The namespace to deploy the Helm chart to"` + ValuesFiles []BundleChartValuesFile `json:"valuesFiles,omitempty" jsonschema:"description=List of Helm chart value files to set statically"` } // BundleChartValue represents a Helm chart value to path mapping to set via UDS variables @@ -42,9 +40,8 @@ type BundleChartValue struct { Value interface{} `json:"value" jsonschema:"name=The value to set"` } -// BundleChartValueFile - EXPERIMENTAL - represents a Helm chart value file to override -type BundleChartValueFile struct { - Path string `json:"path" jsonschema:"name=Path to the Helm chart to set. The format is /, example=my-component/my-cool-chart"` +// BundleChartValueFile represents a Helm chart value file to override +type BundleChartValuesFile struct { File string `json:"file" jsonschema:"name=The path to the values file to add to the Helm chart"` } diff --git a/uds.schema.json b/uds.schema.json index ac1e39f9..804771e9 100644 --- a/uds.schema.json +++ b/uds.schema.json @@ -23,6 +23,14 @@ "namespace": { "type": "string", "description": "The namespace to deploy the Helm chart to" + }, + "valuesFiles": { + "items": { + "$schema": "http://json-schema.org/draft-04/schema#", + "$ref": "#/definitions/BundleChartValuesFile" + }, + "type": "array", + "description": "List of Helm chart value files to set statically" } }, "additionalProperties": false, @@ -44,6 +52,18 @@ "additionalProperties": false, "type": "object" }, + "BundleChartValuesFile": { + "required": [ + "file" + ], + "properties": { + "file": { + "type": "string" + } + }, + "additionalProperties": false, + "type": "object" + }, "BundleChartVariable": { "required": [ "path", diff --git a/zarf.schema.json b/zarf.schema.json index f61482b7..69f4aae9 100644 --- a/zarf.schema.json +++ b/zarf.schema.json @@ -256,6 +256,41 @@ }, "type": "array", "description": "List of local values file paths or remote URLs to include in the package; these will be merged together when deployed" + }, + "variables": { + "items": { + "$schema": "http://json-schema.org/draft-04/schema#", + "$ref": "#/definitions/ZarfChartVariable" + }, + "type": "array", + "description": "[alpha] List of variables to set in the Helm chart" + } + }, + "additionalProperties": false, + "type": "object", + "patternProperties": { + "^x-": {} + } + }, + "ZarfChartVariable": { + "required": [ + "name", + "description", + "path" + ], + "properties": { + "name": { + "pattern": "^[A-Z0-9_]+$", + "type": "string", + "description": "The name of the variable" + }, + "description": { + "type": "string", + "description": "A brief description of what the variable controls" + }, + "path": { + "type": "string", + "description": "The path within the Helm chart values where this variable applies" } }, "additionalProperties": false, From d603ae375bd78b752970f5d442e4a9a39220870b Mon Sep 17 00:00:00 2001 From: Darcy Cleaver Date: Tue, 7 May 2024 12:44:25 -0600 Subject: [PATCH 02/10] feat: valuesFiles override support --- .../values-file/uds-bundle.yaml | 40 +++++++++++++++++++ .../07-helm-overrides/values-file/values.yaml | 12 ++++++ 2 files changed, 52 insertions(+) create mode 100644 src/test/bundles/07-helm-overrides/values-file/uds-bundle.yaml create mode 100644 src/test/bundles/07-helm-overrides/values-file/values.yaml diff --git a/src/test/bundles/07-helm-overrides/values-file/uds-bundle.yaml b/src/test/bundles/07-helm-overrides/values-file/uds-bundle.yaml new file mode 100644 index 00000000..4d48631c --- /dev/null +++ b/src/test/bundles/07-helm-overrides/values-file/uds-bundle.yaml @@ -0,0 +1,40 @@ +kind: UDSBundle +metadata: + name: helm-values-file + description: testing a bundle with Helm overrides + version: 0.0.1 + +packages: + - name: helm-overrides + path: "../../../packages/helm" + ref: 0.0.1 + + overrides: + podinfo-component: + unicorn-podinfo: + valuesFiles: + - file: values.yaml + variables: + - name: log_level + path: "podinfo.logLevel" + description: "Set the log level for podinfo" + default: "debug" # not overwritten! + - name: ui_color + path: "podinfo.ui.color" + description: "Set the color for podinfo's UI" + default: "blue" + - name: UI_MSG + path: "podinfo.ui.message" + description: "Set the message for podinfo's UI" + - name: SECRET_VAL + path: "testSecret" + description: "testing a secret value" + - name: SECURITY_CTX + path: "podinfo.securityContext" + description: "testing an object" + default: + runAsUser: 1000 + runAsGroup: 3000 + - name: HOSTS + path: "podinfo.ingress.hosts" + description: "just testing a a list of objects (doesn't actually do ingress things)" diff --git a/src/test/bundles/07-helm-overrides/values-file/values.yaml b/src/test/bundles/07-helm-overrides/values-file/values.yaml new file mode 100644 index 00000000..63476214 --- /dev/null +++ b/src/test/bundles/07-helm-overrides/values-file/values.yaml @@ -0,0 +1,12 @@ +podinfo.replicaCount: 2 +podinfo.tolerations: + - key: "unicorn" + operator: "Equal" + value: "defense" + effect: "NoSchedule" + - key: "uds" + operator: "Equal" + value: "true" + effect: "NoSchedule" +podinfo.podAnnotations: + customAnnotation: "customValue" From b9bb112575d08efa4fdaf37ab3f65b622e1138f8 Mon Sep 17 00:00:00 2001 From: Darcy Cleaver Date: Wed, 15 May 2024 09:30:34 -0600 Subject: [PATCH 03/10] convert bundlechartOverrides values to a map --- src/pkg/bundle/create.go | 15 ++++++++++----- src/pkg/bundle/deploy.go | 6 +++--- .../values-file/uds-bundle.yaml | 4 +++- src/types/bundle.go | 19 ++++--------------- 4 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/pkg/bundle/create.go b/src/pkg/bundle/create.go index 58f50aaf..e2873ab8 100644 --- a/src/pkg/bundle/create.go +++ b/src/pkg/bundle/create.go @@ -11,7 +11,6 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/defenseunicorns/uds-cli/src/config" "github.com/defenseunicorns/uds-cli/src/pkg/bundler" - "github.com/defenseunicorns/uds-cli/src/types" zarfConfig "github.com/defenseunicorns/zarf/src/config" "github.com/defenseunicorns/zarf/src/pkg/interactive" "github.com/defenseunicorns/zarf/src/pkg/message" @@ -34,9 +33,9 @@ func (b *Bundle) Create() error { for k, bundleChartOverrides := range overrides { for _, valuesFile := range bundleChartOverrides.ValuesFiles { // Check relative vs absolute path - fileName := filepath.Join(b.cfg.CreateOpts.SourceDirectory, valuesFile.File) - if filepath.IsAbs(valuesFile.File) { - fileName = valuesFile.File + fileName := filepath.Join(b.cfg.CreateOpts.SourceDirectory, valuesFile) + if filepath.IsAbs(valuesFile) { + fileName = valuesFile } // read values from valuesFile values, err := chartutil.ReadValuesFile(fileName) @@ -44,8 +43,14 @@ func (b *Bundle) Create() error { return err } // add values from valuesFile to bundleChartOverrides + if bundleChartOverrides.Values == nil { + bundleChartOverrides.Values = make(map[string]interface{}) + } for key, value := range values { - bundleChartOverrides.Values = append(bundleChartOverrides.Values, types.BundleChartValue{Path: key, Value: value}) + // only use valuesFile values if they are not already set by overrides value + if bundleChartOverrides.Values[key] == nil { + bundleChartOverrides.Values[key] = value + } } // update bundle with override values b.bundle.Packages[i].Overrides[j][k] = bundleChartOverrides diff --git a/src/pkg/bundle/deploy.go b/src/pkg/bundle/deploy.go index 3f767ebb..6458c82e 100644 --- a/src/pkg/bundle/deploy.go +++ b/src/pkg/bundle/deploy.go @@ -345,10 +345,10 @@ func (b *Bundle) processOverrideNamespaces(overrideMap sources.NamespaceOverride } // processOverrideValues processes a bundles values overrides and adds them to the override map -func (b *Bundle) processOverrideValues(overrideMap *map[string]map[string]*values.Options, values *[]types.BundleChartValue, componentName string, chartName string, pkgVars map[string]string) error { - for _, v := range *values { +func (b *Bundle) processOverrideValues(overrideMap *map[string]map[string]*values.Options, values *map[string]interface{}, componentName string, chartName string, pkgVars map[string]string) error { + for k, v := range *values { // Add the override to the map, or return an error if the path is invalid - if err := addOverrideValue(*overrideMap, componentName, chartName, v.Path, v.Value, pkgVars); err != nil { + if err := addOverrideValue(*overrideMap, componentName, chartName, k, v, pkgVars); err != nil { return err } } diff --git a/src/test/bundles/07-helm-overrides/values-file/uds-bundle.yaml b/src/test/bundles/07-helm-overrides/values-file/uds-bundle.yaml index 4d48631c..3b3804c4 100644 --- a/src/test/bundles/07-helm-overrides/values-file/uds-bundle.yaml +++ b/src/test/bundles/07-helm-overrides/values-file/uds-bundle.yaml @@ -13,7 +13,9 @@ packages: podinfo-component: unicorn-podinfo: valuesFiles: - - file: values.yaml + - values.yaml + values: + podinfo.replicaCount: 3 variables: - name: log_level path: "podinfo.logLevel" diff --git a/src/types/bundle.go b/src/types/bundle.go index cd70cf2d..a1aa6eaa 100644 --- a/src/types/bundle.go +++ b/src/types/bundle.go @@ -28,21 +28,10 @@ type Package struct { // BundleChartOverrides represents a Helm chart override to set via UDS variables type BundleChartOverrides struct { - Values []BundleChartValue `json:"values,omitempty" jsonschema:"description=List of Helm chart values to set statically"` - Variables []BundleChartVariable `json:"variables,omitempty" jsonschema:"description=List of Helm chart variables to set via UDS variables"` - Namespace string `json:"namespace,omitempty" jsonschema:"description=The namespace to deploy the Helm chart to"` - ValuesFiles []BundleChartValuesFile `json:"valuesFiles,omitempty" jsonschema:"description=List of Helm chart value files to set statically"` -} - -// BundleChartValue represents a Helm chart value to path mapping to set via UDS variables -type BundleChartValue struct { - Path string `json:"path" jsonschema:"name=Path to the Helm chart value to set. The format is , example=controller.service.type"` - Value interface{} `json:"value" jsonschema:"name=The value to set"` -} - -// BundleChartValueFile represents a Helm chart value file to override -type BundleChartValuesFile struct { - File string `json:"file" jsonschema:"name=The path to the values file to add to the Helm chart"` + Values map[string]interface{} `json:"values,omitempty" jsonschema:"description=Map of Helm chart values to set statically"` + Variables []BundleChartVariable `json:"variables,omitempty" jsonschema:"description=List of Helm chart variables to set via UDS variables"` + Namespace string `json:"namespace,omitempty" jsonschema:"description=The namespace to deploy the Helm chart to"` + ValuesFiles []string `json:"valuesFiles,omitempty" jsonschema:"description=List of Helm chart value file paths to set statically"` } // BundleChartVariable - EXPERIMENTAL - represents a Helm chart variable and its path From 9f3f3528b8960c1c55e4ebc402c8c4b9549f01fa Mon Sep 17 00:00:00 2001 From: Darcy Cleaver Date: Wed, 15 May 2024 14:52:12 -0600 Subject: [PATCH 04/10] updates without breaking changes --- docs/overrides.md | 8 +- src/pkg/bundle/create.go | 89 +++++++++++++------ src/pkg/bundle/deploy.go | 6 +- .../values-file/uds-bundle.yaml | 3 +- .../07-helm-overrides/values-file/values.yaml | 2 +- src/types/bundle.go | 14 +-- 6 files changed, 81 insertions(+), 41 deletions(-) diff --git a/docs/overrides.md b/docs/overrides.md index 475221a5..5ff32c29 100644 --- a/docs/overrides.md +++ b/docs/overrides.md @@ -123,8 +123,6 @@ In this example, the `helm-overrides-package` Zarf package has a component calle The `valuesFiles` in an `overrides` block are a list of `file`'s. It allows users to override multiple values in a Zarf package component's underlying Helm chart, by providing a file with those values instead of having to include them all indiviually in the `overrides` block. -NOTE: If a value is specified in both a `valuesFile` and as a `value` in the `overrides` block, the value set in the `valuesFile` will take precedence. - ### Values The `values` in an `overrides` block are a list of `path` and `value` pairs. They allow users to override values in a Zarf package component's underlying Helm chart. Note that values are specified by bundle authors and **cannot be modified** after the bundle has been created. @@ -181,6 +179,12 @@ packages: value: ${COLOR} ``` +#### Value Precedence +Value precedence is as follows: +1. The `values` in an `overrides` block +1. `values` set in the first `valuesFile` (if specified) +1. `values` set in the next `valuesFile` (if specified) + ### Variables Variables are similar to [values](#values) in that they allow users to override values in a Zarf package component's underlying Helm chart; they also share a similar syntax. However, unlike `values`, `variables` can be overridden at deploy time. For example, consider the `variables` key in the following `uds-bundle.yaml`: diff --git a/src/pkg/bundle/create.go b/src/pkg/bundle/create.go index e2873ab8..dff30d7c 100644 --- a/src/pkg/bundle/create.go +++ b/src/pkg/bundle/create.go @@ -11,6 +11,7 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/defenseunicorns/uds-cli/src/config" "github.com/defenseunicorns/uds-cli/src/pkg/bundler" + "github.com/defenseunicorns/uds-cli/src/types" zarfConfig "github.com/defenseunicorns/zarf/src/config" "github.com/defenseunicorns/zarf/src/pkg/interactive" "github.com/defenseunicorns/zarf/src/pkg/message" @@ -28,35 +29,8 @@ func (b *Bundle) Create() error { } // Populate values from valuesFiles if provided - for i, pkg := range b.bundle.Packages { - for j, overrides := range pkg.Overrides { - for k, bundleChartOverrides := range overrides { - for _, valuesFile := range bundleChartOverrides.ValuesFiles { - // Check relative vs absolute path - fileName := filepath.Join(b.cfg.CreateOpts.SourceDirectory, valuesFile) - if filepath.IsAbs(valuesFile) { - fileName = valuesFile - } - // read values from valuesFile - values, err := chartutil.ReadValuesFile(fileName) - if err != nil { - return err - } - // add values from valuesFile to bundleChartOverrides - if bundleChartOverrides.Values == nil { - bundleChartOverrides.Values = make(map[string]interface{}) - } - for key, value := range values { - // only use valuesFile values if they are not already set by overrides value - if bundleChartOverrides.Values[key] == nil { - bundleChartOverrides.Values[key] = value - } - } - // update bundle with override values - b.bundle.Packages[i].Overrides[j][k] = bundleChartOverrides - } - } - } + if err := b.processValuesFiles(); err != nil { + return err } // confirm creation @@ -140,3 +114,60 @@ func (b *Bundle) confirmBundleCreation() (confirm bool) { } return true } + +// processValuesFiles reads values from valuesFiles and updates the bundle with the override values +func (b *Bundle) processValuesFiles() error { + // Populate values from valuesFiles if provided + for i, pkg := range b.bundle.Packages { + for j, overrides := range pkg.Overrides { + for k, bundleChartOverrides := range overrides { + for _, valuesFile := range bundleChartOverrides.ValuesFiles { + // Check relative vs absolute path + fileName := filepath.Join(b.cfg.CreateOpts.SourceDirectory, valuesFile) + if filepath.IsAbs(valuesFile) { + fileName = valuesFile + } + // read values from valuesFile + values, err := chartutil.ReadValuesFile(fileName) + if err != nil { + return err + } + if len(values) > 0 { + // populate BundleChartValue slice to use for merging existing values + valuesFileValues := make([]types.BundleChartValue, 0, len(values)) + for key, value := range values { + valuesFileValues = append(valuesFileValues, types.BundleChartValue{Path: key, Value: value}) + } + // update bundle with override values + overrides := b.bundle.Packages[i].Overrides[j][k] + // Merge values from valuesFile and existing values + overrides.Values = mergeBundleChartValues(overrides.Values, valuesFileValues) + b.bundle.Packages[i].Overrides[j][k] = overrides + } + } + } + } + } + return nil +} + +// mergeBundleChartValues merges two lists of BundleChartValue using the values from list1 if there are duplicates +func mergeBundleChartValues(list1, list2 []types.BundleChartValue) []types.BundleChartValue { + merged := make([]types.BundleChartValue, 0) + paths := make(map[string]bool) + + // Add entries from list1 to the merged list + for _, bundleChartValue := range list1 { + merged = append(merged, bundleChartValue) + paths[bundleChartValue.Path] = true + } + + // Add entries from list2 to the merged list, if they don't already exist + for _, bundleChartValue := range list2 { + if _, ok := paths[bundleChartValue.Path]; !ok { + merged = append(merged, bundleChartValue) + } + } + + return merged +} diff --git a/src/pkg/bundle/deploy.go b/src/pkg/bundle/deploy.go index 6458c82e..3f767ebb 100644 --- a/src/pkg/bundle/deploy.go +++ b/src/pkg/bundle/deploy.go @@ -345,10 +345,10 @@ func (b *Bundle) processOverrideNamespaces(overrideMap sources.NamespaceOverride } // processOverrideValues processes a bundles values overrides and adds them to the override map -func (b *Bundle) processOverrideValues(overrideMap *map[string]map[string]*values.Options, values *map[string]interface{}, componentName string, chartName string, pkgVars map[string]string) error { - for k, v := range *values { +func (b *Bundle) processOverrideValues(overrideMap *map[string]map[string]*values.Options, values *[]types.BundleChartValue, componentName string, chartName string, pkgVars map[string]string) error { + for _, v := range *values { // Add the override to the map, or return an error if the path is invalid - if err := addOverrideValue(*overrideMap, componentName, chartName, k, v, pkgVars); err != nil { + if err := addOverrideValue(*overrideMap, componentName, chartName, v.Path, v.Value, pkgVars); err != nil { return err } } diff --git a/src/test/bundles/07-helm-overrides/values-file/uds-bundle.yaml b/src/test/bundles/07-helm-overrides/values-file/uds-bundle.yaml index 3b3804c4..c643cbf0 100644 --- a/src/test/bundles/07-helm-overrides/values-file/uds-bundle.yaml +++ b/src/test/bundles/07-helm-overrides/values-file/uds-bundle.yaml @@ -15,7 +15,8 @@ packages: valuesFiles: - values.yaml values: - podinfo.replicaCount: 3 + - path: "podinfo.replicaCount" + value: 2 variables: - name: log_level path: "podinfo.logLevel" diff --git a/src/test/bundles/07-helm-overrides/values-file/values.yaml b/src/test/bundles/07-helm-overrides/values-file/values.yaml index 63476214..d8ea4aef 100644 --- a/src/test/bundles/07-helm-overrides/values-file/values.yaml +++ b/src/test/bundles/07-helm-overrides/values-file/values.yaml @@ -1,4 +1,4 @@ -podinfo.replicaCount: 2 +# podinfo.replicaCount: 3 podinfo.tolerations: - key: "unicorn" operator: "Equal" diff --git a/src/types/bundle.go b/src/types/bundle.go index a1aa6eaa..8ff296c6 100644 --- a/src/types/bundle.go +++ b/src/types/bundle.go @@ -28,13 +28,17 @@ type Package struct { // BundleChartOverrides represents a Helm chart override to set via UDS variables type BundleChartOverrides struct { - Values map[string]interface{} `json:"values,omitempty" jsonschema:"description=Map of Helm chart values to set statically"` - Variables []BundleChartVariable `json:"variables,omitempty" jsonschema:"description=List of Helm chart variables to set via UDS variables"` - Namespace string `json:"namespace,omitempty" jsonschema:"description=The namespace to deploy the Helm chart to"` - ValuesFiles []string `json:"valuesFiles,omitempty" jsonschema:"description=List of Helm chart value file paths to set statically"` + Values []BundleChartValue `json:"values,omitempty" jsonschema:"description=List of Helm chart values to set statically"` + Variables []BundleChartVariable `json:"variables,omitempty" jsonschema:"description=List of Helm chart variables to set via UDS variables"` + Namespace string `json:"namespace,omitempty" jsonschema:"description=The namespace to deploy the Helm chart to"` + ValuesFiles []string `json:"valuesFiles,omitempty" jsonschema:"description=List of Helm chart value file paths to set statically"` +} + +type BundleChartValue struct { + Path string `json:"path" jsonschema:"name=Path to the Helm chart value to set. The format is , example=controller.service.type"` + Value interface{} `json:"value" jsonschema:"name=The value to set"` } -// BundleChartVariable - EXPERIMENTAL - represents a Helm chart variable and its path type BundleChartVariable struct { Path string `json:"path" jsonschema:"name=Path to the Helm chart value to set. The format is , example=controller.service.type"` Name string `json:"name" jsonschema:"name=Name of the variable to set"` From 4f5c876c7aa72da686bafd60c8758990be0fb833 Mon Sep 17 00:00:00 2001 From: Darcy Cleaver Date: Wed, 15 May 2024 14:58:22 -0600 Subject: [PATCH 05/10] schema updates --- uds.schema.json | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/uds.schema.json b/uds.schema.json index 804771e9..1030a301 100644 --- a/uds.schema.json +++ b/uds.schema.json @@ -26,11 +26,10 @@ }, "valuesFiles": { "items": { - "$schema": "http://json-schema.org/draft-04/schema#", - "$ref": "#/definitions/BundleChartValuesFile" + "type": "string" }, "type": "array", - "description": "List of Helm chart value files to set statically" + "description": "List of Helm chart value file paths to set statically" } }, "additionalProperties": false, @@ -52,18 +51,6 @@ "additionalProperties": false, "type": "object" }, - "BundleChartValuesFile": { - "required": [ - "file" - ], - "properties": { - "file": { - "type": "string" - } - }, - "additionalProperties": false, - "type": "object" - }, "BundleChartVariable": { "required": [ "path", From c0c3c787af72ec223d64f063627ddbcaaec281ac Mon Sep 17 00:00:00 2001 From: decleaver <85503726+decleaver@users.noreply.github.com> Date: Mon, 20 May 2024 09:19:15 -0600 Subject: [PATCH 06/10] Update docs/overrides.md Co-authored-by: UncleGedd <42304551+UncleGedd@users.noreply.github.com> --- docs/overrides.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/overrides.md b/docs/overrides.md index 5ff32c29..2aa3e14d 100644 --- a/docs/overrides.md +++ b/docs/overrides.md @@ -119,7 +119,7 @@ podAnnotations: ``` In this example, the `helm-overrides-package` Zarf package has a component called `helm-overrides-component` which contains a Helm chart called `podinfo`; note how these names are keys in the `overrides` block. The `podinfo` chart has a `replicaCount` value that is overridden to `2`, a `podAnnotations` value that is overridden to include `customAnnotation: "customValue"` and a variable called `UI_COLOR` that is overridden to `purple`. -### ValuesFiles +### Values Files The `valuesFiles` in an `overrides` block are a list of `file`'s. It allows users to override multiple values in a Zarf package component's underlying Helm chart, by providing a file with those values instead of having to include them all indiviually in the `overrides` block. From 2e91283675df6c98522e2e6d21503ff1bea2aa83 Mon Sep 17 00:00:00 2001 From: decleaver <85503726+decleaver@users.noreply.github.com> Date: Mon, 20 May 2024 09:36:14 -0600 Subject: [PATCH 07/10] Update src/test/e2e/variable_test.go Co-authored-by: UncleGedd <42304551+UncleGedd@users.noreply.github.com> --- src/test/e2e/variable_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/e2e/variable_test.go b/src/test/e2e/variable_test.go index 049fd5d0..10402196 100644 --- a/src/test/e2e/variable_test.go +++ b/src/test/e2e/variable_test.go @@ -182,7 +182,6 @@ func TestBundleWithHelmOverridesValuesFile(t *testing.T) { annotations, _, err := e2e.UDS(cmd...) require.Contains(t, annotations, "\"customAnnotation\":\"customValue\"") require.NoError(t, err) - }) t.Run("check list-type override in values", func(t *testing.T) { From e79c82162c92bb5ad76a5828146528f3fbd59d7a Mon Sep 17 00:00:00 2001 From: Darcy Cleaver Date: Mon, 20 May 2024 11:01:15 -0600 Subject: [PATCH 08/10] pr comment updates --- docs/overrides.md | 4 +-- src/pkg/bundle/create.go | 26 ++++++++++--------- .../values-file/uds-bundle.yaml | 1 + .../07-helm-overrides/values-file/values.yaml | 2 +- src/test/e2e/variable_test.go | 2 +- 5 files changed, 19 insertions(+), 16 deletions(-) diff --git a/docs/overrides.md b/docs/overrides.md index 2aa3e14d..4bf74734 100644 --- a/docs/overrides.md +++ b/docs/overrides.md @@ -182,8 +182,8 @@ packages: #### Value Precedence Value precedence is as follows: 1. The `values` in an `overrides` block -1. `values` set in the first `valuesFile` (if specified) -1. `values` set in the next `valuesFile` (if specified) +1. `values` set in the last `valuesFile` (if more than one specified) +1. `values` set in the previous `valuesFile` (if more than one specified) ### Variables Variables are similar to [values](#values) in that they allow users to override values in a Zarf package component's underlying Helm chart; they also share a similar syntax. However, unlike `values`, `variables` can be overridden at deploy time. For example, consider the `variables` key in the following `uds-bundle.yaml`: diff --git a/src/pkg/bundle/create.go b/src/pkg/bundle/create.go index dff30d7c..5ab280b3 100644 --- a/src/pkg/bundle/create.go +++ b/src/pkg/bundle/create.go @@ -119,9 +119,11 @@ func (b *Bundle) confirmBundleCreation() (confirm bool) { func (b *Bundle) processValuesFiles() error { // Populate values from valuesFiles if provided for i, pkg := range b.bundle.Packages { - for j, overrides := range pkg.Overrides { - for k, bundleChartOverrides := range overrides { - for _, valuesFile := range bundleChartOverrides.ValuesFiles { + for componentName, overrides := range pkg.Overrides { + for chartName, bundleChartOverrides := range overrides { + // Iterate over valuesFiles in reverse order to ensure subsequent value files takes precedence over previous ones + for index := len(bundleChartOverrides.ValuesFiles) - 1; index >= 0; index-- { + valuesFile := bundleChartOverrides.ValuesFiles[index] // Check relative vs absolute path fileName := filepath.Join(b.cfg.CreateOpts.SourceDirectory, valuesFile) if filepath.IsAbs(valuesFile) { @@ -139,10 +141,10 @@ func (b *Bundle) processValuesFiles() error { valuesFileValues = append(valuesFileValues, types.BundleChartValue{Path: key, Value: value}) } // update bundle with override values - overrides := b.bundle.Packages[i].Overrides[j][k] + override := b.bundle.Packages[i].Overrides[componentName][chartName] // Merge values from valuesFile and existing values - overrides.Values = mergeBundleChartValues(overrides.Values, valuesFileValues) - b.bundle.Packages[i].Overrides[j][k] = overrides + override.Values = mergeBundleChartValues(valuesFileValues, override.Values) + b.bundle.Packages[i].Overrides[componentName][chartName] = override } } } @@ -151,19 +153,19 @@ func (b *Bundle) processValuesFiles() error { return nil } -// mergeBundleChartValues merges two lists of BundleChartValue using the values from list1 if there are duplicates -func mergeBundleChartValues(list1, list2 []types.BundleChartValue) []types.BundleChartValue { +// mergeBundleChartValues merges two lists of BundleChartValue using the values from overrides if there are duplicates +func mergeBundleChartValues(original, overrides []types.BundleChartValue) []types.BundleChartValue { merged := make([]types.BundleChartValue, 0) paths := make(map[string]bool) - // Add entries from list1 to the merged list - for _, bundleChartValue := range list1 { + // Add entries from overrides to the merged list + for _, bundleChartValue := range overrides { merged = append(merged, bundleChartValue) paths[bundleChartValue.Path] = true } - // Add entries from list2 to the merged list, if they don't already exist - for _, bundleChartValue := range list2 { + // Add entries from original to the merged list, if they don't already exist + for _, bundleChartValue := range original { if _, ok := paths[bundleChartValue.Path]; !ok { merged = append(merged, bundleChartValue) } diff --git a/src/test/bundles/07-helm-overrides/values-file/uds-bundle.yaml b/src/test/bundles/07-helm-overrides/values-file/uds-bundle.yaml index c643cbf0..46550cfa 100644 --- a/src/test/bundles/07-helm-overrides/values-file/uds-bundle.yaml +++ b/src/test/bundles/07-helm-overrides/values-file/uds-bundle.yaml @@ -14,6 +14,7 @@ packages: unicorn-podinfo: valuesFiles: - values.yaml + - values2.yaml values: - path: "podinfo.replicaCount" value: 2 diff --git a/src/test/bundles/07-helm-overrides/values-file/values.yaml b/src/test/bundles/07-helm-overrides/values-file/values.yaml index d8ea4aef..8905cc5b 100644 --- a/src/test/bundles/07-helm-overrides/values-file/values.yaml +++ b/src/test/bundles/07-helm-overrides/values-file/values.yaml @@ -1,4 +1,4 @@ -# podinfo.replicaCount: 3 +podinfo.replicaCount: 3 podinfo.tolerations: - key: "unicorn" operator: "Equal" diff --git a/src/test/e2e/variable_test.go b/src/test/e2e/variable_test.go index 10402196..f79772ec 100644 --- a/src/test/e2e/variable_test.go +++ b/src/test/e2e/variable_test.go @@ -180,7 +180,7 @@ func TestBundleWithHelmOverridesValuesFile(t *testing.T) { t.Run("check object-type override in values", func(t *testing.T) { cmd := strings.Split("zarf tools kubectl get deployment -n podinfo unicorn-podinfo -o=jsonpath='{.spec.template.metadata.annotations}'", " ") annotations, _, err := e2e.UDS(cmd...) - require.Contains(t, annotations, "\"customAnnotation\":\"customValue\"") + require.Contains(t, annotations, "\"customAnnotation\":\"customValue2\"") require.NoError(t, err) }) From 529c146cde40848ff5e7c1cc9abde9cb9745901f Mon Sep 17 00:00:00 2001 From: Darcy Cleaver Date: Mon, 20 May 2024 15:24:13 -0600 Subject: [PATCH 09/10] update merge function to take in a variable amount of lists to merge --- src/pkg/bundle/create.go | 43 +++++++++++-------- .../values-file/values2.yaml | 3 ++ 2 files changed, 28 insertions(+), 18 deletions(-) create mode 100644 src/test/bundles/07-helm-overrides/values-file/values2.yaml diff --git a/src/pkg/bundle/create.go b/src/pkg/bundle/create.go index 5ab280b3..c0560cc8 100644 --- a/src/pkg/bundle/create.go +++ b/src/pkg/bundle/create.go @@ -121,9 +121,9 @@ func (b *Bundle) processValuesFiles() error { for i, pkg := range b.bundle.Packages { for componentName, overrides := range pkg.Overrides { for chartName, bundleChartOverrides := range overrides { + valuesFilesToMerge := make([][]types.BundleChartValue, 0) // Iterate over valuesFiles in reverse order to ensure subsequent value files takes precedence over previous ones - for index := len(bundleChartOverrides.ValuesFiles) - 1; index >= 0; index-- { - valuesFile := bundleChartOverrides.ValuesFiles[index] + for _, valuesFile := range bundleChartOverrides.ValuesFiles { // Check relative vs absolute path fileName := filepath.Join(b.cfg.CreateOpts.SourceDirectory, valuesFile) if filepath.IsAbs(valuesFile) { @@ -140,34 +140,41 @@ func (b *Bundle) processValuesFiles() error { for key, value := range values { valuesFileValues = append(valuesFileValues, types.BundleChartValue{Path: key, Value: value}) } - // update bundle with override values - override := b.bundle.Packages[i].Overrides[componentName][chartName] - // Merge values from valuesFile and existing values - override.Values = mergeBundleChartValues(valuesFileValues, override.Values) - b.bundle.Packages[i].Overrides[componentName][chartName] = override + valuesFilesToMerge = append(valuesFilesToMerge, valuesFileValues) } } + override := b.bundle.Packages[i].Overrides[componentName][chartName] + // add override values to the end of the list of values to merge since we want them to take precedence + valuesFilesToMerge = append(valuesFilesToMerge, override.Values) + override.Values = mergeBundleChartValues(valuesFilesToMerge...) + b.bundle.Packages[i].Overrides[componentName][chartName] = override } } } return nil } -// mergeBundleChartValues merges two lists of BundleChartValue using the values from overrides if there are duplicates -func mergeBundleChartValues(original, overrides []types.BundleChartValue) []types.BundleChartValue { +// mergeBundleChartValues merges lists of BundleChartValue using the values from the last list if there are any duplicates +// such that values from the last list will take precedence over the values from previous lists +func mergeBundleChartValues(bundleChartValueLists ...[]types.BundleChartValue) []types.BundleChartValue { merged := make([]types.BundleChartValue, 0) paths := make(map[string]bool) - // Add entries from overrides to the merged list - for _, bundleChartValue := range overrides { - merged = append(merged, bundleChartValue) - paths[bundleChartValue.Path] = true - } - - // Add entries from original to the merged list, if they don't already exist - for _, bundleChartValue := range original { - if _, ok := paths[bundleChartValue.Path]; !ok { + // Iterate over each list in order + for _, bundleChartValues := range bundleChartValueLists { + // Add entries from the current list to the merged list, overwriting any existing entries + for _, bundleChartValue := range bundleChartValues { + if _, ok := paths[bundleChartValue.Path]; ok { + // Remove the existing entry from the merged list + for i, mergedValue := range merged { + if mergedValue.Path == bundleChartValue.Path { + merged = append(merged[:i], merged[i+1:]...) + break + } + } + } merged = append(merged, bundleChartValue) + paths[bundleChartValue.Path] = true } } diff --git a/src/test/bundles/07-helm-overrides/values-file/values2.yaml b/src/test/bundles/07-helm-overrides/values-file/values2.yaml new file mode 100644 index 00000000..e6e8d4fd --- /dev/null +++ b/src/test/bundles/07-helm-overrides/values-file/values2.yaml @@ -0,0 +1,3 @@ +podinfo.replicaCount: 4 +podinfo.podAnnotations: + customAnnotation: "customValue2" From ce15dbf0c759d78707162e8d99e51bca0fb2cad5 Mon Sep 17 00:00:00 2001 From: Darcy Cleaver Date: Mon, 20 May 2024 20:14:07 -0600 Subject: [PATCH 10/10] fix potential panic in merge function --- src/pkg/bundle/create.go | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/pkg/bundle/create.go b/src/pkg/bundle/create.go index c0560cc8..e06b56b6 100644 --- a/src/pkg/bundle/create.go +++ b/src/pkg/bundle/create.go @@ -157,26 +157,21 @@ func (b *Bundle) processValuesFiles() error { // mergeBundleChartValues merges lists of BundleChartValue using the values from the last list if there are any duplicates // such that values from the last list will take precedence over the values from previous lists func mergeBundleChartValues(bundleChartValueLists ...[]types.BundleChartValue) []types.BundleChartValue { - merged := make([]types.BundleChartValue, 0) - paths := make(map[string]bool) + mergedMap := make(map[string]types.BundleChartValue) // Iterate over each list in order for _, bundleChartValues := range bundleChartValueLists { - // Add entries from the current list to the merged list, overwriting any existing entries + // Add entries from the current list to the merged map, overwriting any existing entries for _, bundleChartValue := range bundleChartValues { - if _, ok := paths[bundleChartValue.Path]; ok { - // Remove the existing entry from the merged list - for i, mergedValue := range merged { - if mergedValue.Path == bundleChartValue.Path { - merged = append(merged[:i], merged[i+1:]...) - break - } - } - } - merged = append(merged, bundleChartValue) - paths[bundleChartValue.Path] = true + mergedMap[bundleChartValue.Path] = bundleChartValue } } + // Convert the map to a slice + merged := make([]types.BundleChartValue, 0, len(mergedMap)) + for _, value := range mergedMap { + merged = append(merged, value) + } + return merged }