Skip to content

Commit

Permalink
addressed code review comments.
Browse files Browse the repository at this point in the history
Signed-off-by: naveensrinivasan <172697+naveensrinivasan@users.noreply.github.com>
  • Loading branch information
naveensrinivasan committed Mar 28, 2024
1 parent 7d36da4 commit 955bbd1
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 13 deletions.
2 changes: 1 addition & 1 deletion adr/0023-chart-override.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
4 changes: 4 additions & 0 deletions docs/3-create-a-zarf-package/4-zarf-schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -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) |

</blockquote>
</details>

Expand Down
2 changes: 1 addition & 1 deletion examples/helm-charts/zarf.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 1 addition & 4 deletions src/pkg/packager/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
12 changes: 6 additions & 6 deletions src/pkg/packager/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
},
{
Expand Down
2 changes: 1 addition & 1 deletion src/types/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
Expand Down
1 change: 1 addition & 0 deletions zarf.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@
],
"properties": {
"name": {
"pattern": "^[A-Z0-9_]+$",
"type": "string",
"description": "The name of the variable"
},
Expand Down

0 comments on commit 955bbd1

Please sign in to comment.