From b6bb55a968161642d24500a74bb9fcc304b92269 Mon Sep 17 00:00:00 2001 From: Mauricio Alvarez Leon <65101411+BBBmau@users.noreply.github.com> Date: Mon, 24 Feb 2025 10:42:19 -0800 Subject: [PATCH] `set.value`: `required` -> `optional` (#1572) * required -> optional * add resource test * add data source test * set_list.value is required * docs set_list required * changelog entry --- .changelog/1572.txt | 3 ++ docs/data-sources/template.md | 2 +- docs/resources/release.md | 5 +-- helm/data_helm_template.go | 4 +- helm/data_helm_template_test.go | 46 +++++++++++++++++++++ helm/resource_helm_release.go | 2 +- helm/resource_helm_release_test.go | 66 ++++++++++++++++++++++++++++++ 7 files changed, 121 insertions(+), 7 deletions(-) create mode 100644 .changelog/1572.txt diff --git a/.changelog/1572.txt b/.changelog/1572.txt new file mode 100644 index 000000000..732570d6b --- /dev/null +++ b/.changelog/1572.txt @@ -0,0 +1,3 @@ +```release-note:bug +change `set.value` && `set_list.value` to optional instead of required +``` \ No newline at end of file diff --git a/docs/data-sources/template.md b/docs/data-sources/template.md index b956a9e73..8bdd71e9c 100644 --- a/docs/data-sources/template.md +++ b/docs/data-sources/template.md @@ -85,11 +85,11 @@ Required: Required: - `name` (String) -- `value` (String) Optional: - `type` (String) +- `value` (String) diff --git a/docs/resources/release.md b/docs/resources/release.md index 59fd7fdda..aa1ca5394 100644 --- a/docs/resources/release.md +++ b/docs/resources/release.md @@ -86,12 +86,11 @@ Optional: Required: - `name` (String) -- `value` (String) Optional: - `type` (String) - +- `value` (String) ### Nested Schema for `set_list` @@ -289,7 +288,7 @@ resource "helm_release" "example" { The `set`, `set_list`, and `set_sensitive` blocks support: * `name` - (Required) full name of the variable to be set. -* `value` - (Required) value of the variable to be set. +* `value` - (Required; Optional for `set`) value of the variable to be set. * `type` - (Optional) type of the variable to be set. Valid options are `auto` and `string`. Since Terraform Utilizes HCL as well as Helm using the Helm Template Language, it's necessary to escape the `{}`, `[]`, `.`, and `,` characters twice in order for it to be parsed. `name` should also be set to the `value path`, and `value` is the desired value that will be set. diff --git a/helm/data_helm_template.go b/helm/data_helm_template.go index f4fdd2dda..4c1ad0127 100644 --- a/helm/data_helm_template.go +++ b/helm/data_helm_template.go @@ -293,7 +293,7 @@ func (d *HelmTemplate) Schema(ctx context.Context, req datasource.SchemaRequest, Required: true, }, "value": schema.StringAttribute{ - Required: true, + Optional: true, }, "type": schema.StringAttribute{ Optional: true, @@ -311,7 +311,7 @@ func (d *HelmTemplate) Schema(ctx context.Context, req datasource.SchemaRequest, NestedObject: schema.NestedAttributeObject{ Attributes: map[string]schema.Attribute{ "name": schema.StringAttribute{ - Required: true, + Optional: true, }, "value": schema.ListAttribute{ Required: true, diff --git a/helm/data_helm_template_test.go b/helm/data_helm_template_test.go index f2611f083..138d99b63 100644 --- a/helm/data_helm_template_test.go +++ b/helm/data_helm_template_test.go @@ -235,6 +235,24 @@ func TestAccDataTemplate_kubeVersion(t *testing.T) { }) } +func TestAccDataTemplate_configSetNull(t *testing.T) { + name := randName("basic") + namespace := randName(testNamespacePrefix) + + datasourceAddress := fmt.Sprintf("data.helm_template.%s", testResourceName) + + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: protoV6ProviderFactories(), + Steps: []resource.TestStep{{ + Config: testAccDataHelmTemplateConfigNullSet(testResourceName, namespace, name, "1.2.3"), + Check: resource.ComposeAggregateTestCheckFunc( + checkResourceAttrNotSet(datasourceAddress, "set.0.value"), + checkResourceAttrNotSet(datasourceAddress, "set.1.value"), + ), + }}, + }) +} + func testAccDataHelmTemplateConfigBasic(resource, ns, name, version string) string { return fmt.Sprintf(` data "helm_template" "%s" { @@ -290,6 +308,34 @@ func testAccDataHelmTemplateConfigTemplates(resource, ns, name, version string) `, resource, name, ns, testRepositoryURL, version) } +func testAccDataHelmTemplateConfigNullSet(resource, ns, name, version string) string { + return fmt.Sprintf(` + data "helm_template" "%s" { + name = %q + namespace = %q + description = "Test" + repository = %q + chart = "test-chart" + version = %q + + set = [ + { + name = "foo" + }, + { + name = "fizz" + value = null + } + ] + + show_only = [ + "templates/configmaps.yaml", + "" + ] + } + `, resource, name, ns, testRepositoryURL, version) +} + func testAccDataHelmTemplateKubeVersion(resource, ns, name, version, kubeVersion string) string { return fmt.Sprintf(` data "helm_template" "%s" { diff --git a/helm/resource_helm_release.go b/helm/resource_helm_release.go index 4e34e8f2b..4130087cc 100644 --- a/helm/resource_helm_release.go +++ b/helm/resource_helm_release.go @@ -518,7 +518,7 @@ func (r *HelmRelease) Schema(ctx context.Context, req resource.SchemaRequest, re Required: true, }, "value": schema.StringAttribute{ - Required: true, + Optional: true, }, "type": schema.StringAttribute{ Optional: true, diff --git a/helm/resource_helm_release_test.go b/helm/resource_helm_release_test.go index 05a664f45..3fe628917 100644 --- a/helm/resource_helm_release_test.go +++ b/helm/resource_helm_release_test.go @@ -531,6 +531,29 @@ func TestAccResourceRelease_updateExistingFailed(t *testing.T) { }, }) } +func TestAccResourceRelease_SetNull(t *testing.T) { + name := randName("test-update-set-value") + namespace := createRandomNamespace(t) + defer deleteNamespace(t, namespace) + + // Ensure that value is null + resource.Test(t, resource.TestCase{ + //PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: protoV6ProviderFactories(), + //CheckDestroy: testAccCheckHelmReleaseDestroy(namespace), + Steps: []resource.TestStep{ + { + Config: testAccHelmReleaseConfigSetNull( + testResourceName, namespace, name, "1.2.3", + ), + Check: resource.ComposeAggregateTestCheckFunc( + checkResourceAttrNotSet("helm_release.test", "set.0.value"), + checkResourceAttrNotSet("helm_release.test", "set.1.value"), + ), + }, + }, + }) +} func TestAccResourceRelease_updateSetValue(t *testing.T) { name := randName("test-update-set-value") namespace := createRandomNamespace(t) @@ -602,6 +625,26 @@ func checkResourceAttrExists(name, key string) resource.TestCheckFunc { return fmt.Errorf("%s: Attribute '%s' expected to be set", name, key) } } +func checkResourceAttrNotSet(name, key string) resource.TestCheckFunc { + return func(s *terraform.State) error { + ms := s.RootModule() + rs, ok := ms.Resources[name] + if !ok { + return fmt.Errorf("Not found: %s in %s", name, ms.Path) + } + + is := rs.Primary + if is == nil { + return fmt.Errorf("No primary instance: %s in %s", name, ms.Path) + } + + if _, ok := is.Attributes[key]; !ok { + return nil // Success - attribute does not exist + } + return fmt.Errorf("%s: Attribute '%s' exists but was expected to be unset", name, key) + } +} + func TestAccResourceRelease_postrender(t *testing.T) { // TODO: Add Test Fixture to return real YAML here @@ -898,6 +941,29 @@ func testAccHelmReleaseConfigSet(resource, ns, name, version, setValue string) s `, resource, name, ns, testRepositoryURL, version, setValue) } +func testAccHelmReleaseConfigSetNull(resource, ns, name, version string) string { + return fmt.Sprintf(` + resource "helm_release" "%s" { + name = %q + namespace = %q + description = "Test" + repository = %q + chart = "test-chart" + version = %q + + set = [ + { + name = "foo" + value = null + }, + { + name = "fizz" + } + ] + } + `, resource, name, ns, testRepositoryURL, version) +} + // func TestGetValues(t *testing.T) { // // Initialize a new HelmReleaseResource // r := NewHelmReleaseResource().Data(nil)