From 955bbd177d01e13500a61b4a24433d704f9fdbda Mon Sep 17 00:00:00 2001 From: naveensrinivasan <172697+naveensrinivasan@users.noreply.github.com> Date: Thu, 28 Mar 2024 12:07:39 -0500 Subject: [PATCH] addressed code review comments. Signed-off-by: naveensrinivasan <172697+naveensrinivasan@users.noreply.github.com> --- adr/0023-chart-override.md | 2 +- docs/3-create-a-zarf-package/4-zarf-schema.md | 4 ++++ examples/helm-charts/zarf.yaml | 2 +- src/pkg/packager/deploy.go | 5 +---- src/pkg/packager/deploy_test.go | 12 ++++++------ src/types/component.go | 2 +- zarf.schema.json | 1 + 7 files changed, 15 insertions(+), 13 deletions(-) diff --git a/adr/0023-chart-override.md b/adr/0023-chart-override.md index a9b94072b5..fe05f14a7c 100644 --- a/adr/0023-chart-override.md +++ b/adr/0023-chart-override.md @@ -23,6 +23,6 @@ Key aspects of the proposed implementation include: ## Consequences Adopting this feature would lead to several key improvements: -- **Streamlined Configuration Process**: Centralizing overrides in a single, unified file significantly simplifies the management of configuration settings, aligning more closely with standard Helm practices and reducing the reliance on extensive command-line arguments. +- **Streamlined Configuration Process**: Centralizing overrides in a single, unified file significantly simplifies the management of configuration settings, aligning more closely with standard Helm practices and reducing the reliance on extensive custom `###` templating. Ultimately, this feature is aimed at enhancing the deployment workflow by offering a straightforward and efficient means of customizing Helm chart deployments via command-line inputs. diff --git a/docs/3-create-a-zarf-package/4-zarf-schema.md b/docs/3-create-a-zarf-package/4-zarf-schema.md index c42cc82213..b74c2fafdc 100644 --- a/docs/3-create-a-zarf-package/4-zarf-schema.md +++ b/docs/3-create-a-zarf-package/4-zarf-schema.md @@ -1228,6 +1228,10 @@ Must be one of: | -------- | -------- | | **Type** | `string` | +| Restrictions | | +| --------------------------------- | ----------------------------------------------------------------------------- | +| **Must match regular expression** | ```^[A-Z0-9_]+$``` [Test](https://regex101.com/?regex=%5E%5BA-Z0-9_%5D%2B%24) | + diff --git a/examples/helm-charts/zarf.yaml b/examples/helm-charts/zarf.yaml index 7a02b3d8a1..29e6b3435e 100644 --- a/examples/helm-charts/zarf.yaml +++ b/examples/helm-charts/zarf.yaml @@ -19,7 +19,7 @@ components: # Variables are used to override the default values in the chart # This can be overridden by the user at deployment time with the `--set` flag variables: - - name: replicaCount + - name: REPLICA_COUNT description: "Override the number of pod replicas" path: replicaCount value: 2 diff --git a/src/pkg/packager/deploy.go b/src/pkg/packager/deploy.go index 0892297275..d6e7968a86 100644 --- a/src/pkg/packager/deploy.go +++ b/src/pkg/packager/deploy.go @@ -575,10 +575,7 @@ func generateValuesOverrides(chartVariables []types.ZarfChartVariable, // Apply any direct overrides specified in the deployment options for this component and chart if componentOverrides, ok := deployOpts.ValuesOverridesMap[componentName]; ok { if chartSpecificOverrides, ok := componentOverrides[chartName]; ok { - for k, v := range chartSpecificOverrides { - // Directly apply overrides to valuesOverrides to avoid overwriting by path. - valuesOverrides[k] = v - } + valuesOverrides = chartSpecificOverrides } } diff --git a/src/pkg/packager/deploy_test.go b/src/pkg/packager/deploy_test.go index ae244453b9..81766f2277 100644 --- a/src/pkg/packager/deploy_test.go +++ b/src/pkg/packager/deploy_test.go @@ -69,23 +69,23 @@ func TestGenerateValuesOverrides(t *testing.T) { }, }, { - name: "Multiple variables with nested and non-nested paths", + name: "Multiple variables with nested and non-nested paths, distinct values", chartVariables: []types.ZarfChartVariable{ - {Name: "nestedVar.level2", Path: "nestedVar.level2"}, + {Name: "NESTED_VAR_LEVEL2", Path: "nestedVar.level2"}, {Name: "simpleVar", Path: "simpleVar"}, }, setVariableMap: map[string]*types.ZarfSetVariable{ - "nestedVar.level2": {Value: "nestedValue"}, - "simpleVar": {Value: "simpleValue"}, + "NESTED_VAR_LEVEL2": {Value: "distinctNestedValue"}, + "simpleVar": {Value: "distinctSimpleValue"}, }, deployOpts: types.ZarfDeployOptions{}, componentName: "mixedComponent", chartName: "mixedChart", want: map[string]any{ "nestedVar": map[string]any{ - "level2": "nestedValue", + "level2": "distinctNestedValue", }, - "simpleVar": "simpleValue", + "simpleVar": "distinctSimpleValue", }, }, { diff --git a/src/types/component.go b/src/types/component.go index dc2e28319e..b259b4eb13 100644 --- a/src/types/component.go +++ b/src/types/component.go @@ -120,7 +120,7 @@ type ZarfChart struct { // ZarfChartVariable represents a variable that can be set for a Helm chart overrides. type ZarfChartVariable struct { - Name string `json:"name" jsonschema:"description=The name of the variable"` + Name string `json:"name" jsonschema:"description=The name of the variable,pattern=^[A-Z0-9_]+$"` Description string `json:"description" jsonschema:"description=A brief description of what the variable controls"` Path string `json:"path" jsonschema:"description=The path within the Helm chart values where this variable applies"` } diff --git a/zarf.schema.json b/zarf.schema.json index 33a02cca3e..aeee25bf95 100644 --- a/zarf.schema.json +++ b/zarf.schema.json @@ -280,6 +280,7 @@ ], "properties": { "name": { + "pattern": "^[A-Z0-9_]+$", "type": "string", "description": "The name of the variable" },