From 54162d2fa760b5f6e52197a7087ed40eccd4172d Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 7 Mar 2023 14:42:09 +0100 Subject: [PATCH 1/4] feat: import value from legacy variable --- .../resources/coder_parameter/resource.tf | 2 +- .../coder_parameter_migration/resource.tf | 12 + provider/examples_test.go | 16 + provider/parameter.go | 28 +- provider/parameter_test.go | 374 ++++++++++-------- 5 files changed, 252 insertions(+), 180 deletions(-) create mode 100644 examples/resources/coder_parameter_migration/resource.tf diff --git a/examples/resources/coder_parameter/resource.tf b/examples/resources/coder_parameter/resource.tf index 579e0735..4d1206cd 100644 --- a/examples/resources/coder_parameter/resource.tf +++ b/examples/resources/coder_parameter/resource.tf @@ -74,4 +74,4 @@ data "coder_parameter" "cat_lives" { data "coder_parameter" "fairy_tale" { name = "Fairy Tale" type = "string" -} +} \ No newline at end of file diff --git a/examples/resources/coder_parameter_migration/resource.tf b/examples/resources/coder_parameter_migration/resource.tf new file mode 100644 index 00000000..bef7034c --- /dev/null +++ b/examples/resources/coder_parameter_migration/resource.tf @@ -0,0 +1,12 @@ +provider "coder" {} + +variable "old_account_name" { + type = string + default = "fake-user" # for testing purposes, no need to set via env TF_... +} + +data "coder_parameter" "account_name" { + name = "Account Name" + type = "string" + legacy_variable_name = var.old_account_name +} \ No newline at end of file diff --git a/provider/examples_test.go b/provider/examples_test.go index cf4325e3..12f32800 100644 --- a/provider/examples_test.go +++ b/provider/examples_test.go @@ -14,6 +14,8 @@ func TestExamples(t *testing.T) { t.Parallel() t.Run("coder_parameter", func(t *testing.T) { + t.Parallel() + resource.Test(t, resource.TestCase{ Providers: map[string]*schema.Provider{ "coder": provider.New(), @@ -24,6 +26,20 @@ func TestExamples(t *testing.T) { }}, }) }) + + t.Run("coder_parameter_migration", func(t *testing.T) { + t.Parallel() + + resource.Test(t, resource.TestCase{ + Providers: map[string]*schema.Provider{ + "coder": provider.New(), + }, + IsUnitTest: true, + Steps: []resource.TestStep{{ + Config: mustReadFile(t, "../examples/resources/coder_parameter_migration/resource.tf"), + }}, + }) + }) } func mustReadFile(t *testing.T, path string) string { diff --git a/provider/parameter.go b/provider/parameter.go index e839f7ba..29e411d6 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -49,6 +49,8 @@ type Parameter struct { Option []Option Validation []Validation Optional bool + + LegacyVariable string } func parameterDataSource() *schema.Resource { @@ -69,16 +71,28 @@ func parameterDataSource() *schema.Resource { Option interface{} Validation interface{} Optional interface{} + + LegacyVariable interface{} }{ Value: rd.Get("value"), Name: rd.Get("name"), Description: rd.Get("description"), Type: rd.Get("type"), Mutable: rd.Get("mutable"), - Default: rd.Get("default"), - Icon: rd.Get("icon"), - Option: rd.Get("option"), - Validation: rd.Get("validation"), + Default: func() interface{} { + standardMode := rd.GetRawConfig().AsValueMap()["legacy_variable"].IsNull() + if standardMode { + return rd.Get("default") + } + + // legacy variable is linked + legacyVariable := rd.GetRawConfig().AsValueMap()["legacy_variable"].AsString() + rd.Set("default", legacyVariable) + return legacyVariable + }(), + Icon: rd.Get("icon"), + Option: rd.Get("option"), + Validation: rd.Get("validation"), Optional: func() bool { // This hack allows for checking if the "default" field is present in the .tf file. // If "default" is missing or is "null", then it means that this field is required, @@ -87,6 +101,7 @@ func parameterDataSource() *schema.Resource { rd.Set("optional", val) return val }(), + LegacyVariable: rd.Get("legacy_variable"), }, ¶meter) if err != nil { return diag.Errorf("decode parameter: %s", err) @@ -282,6 +297,11 @@ func parameterDataSource() *schema.Resource { Computed: true, Description: "Whether this value is optional.", }, + "legacy_variable": { + Type: schema.TypeString, + Optional: true, + Description: "The name of the Terraform variable used by legacy parameters. Coder will use it to lookup the parameter value.", + }, }, } } diff --git a/provider/parameter_test.go b/provider/parameter_test.go index d7a3c68c..779a9c0e 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -21,29 +21,29 @@ func TestParameter(t *testing.T) { }{{ Name: "FieldsExist", Config: ` -data "coder_parameter" "region" { - name = "Region" - type = "string" - description = <<-EOT - # Select the machine image - See the [registry](https://container.registry.blah/namespace) for options. - EOT - mutable = true - icon = "/icon/region.svg" - option { - name = "US Central" - value = "us-central1-a" - icon = "/icon/central.svg" - description = "Select for central!" - } - option { - name = "US East" - value = "us-east1-a" - icon = "/icon/east.svg" - description = "Select for east!" - } -} -`, + data "coder_parameter" "region" { + name = "Region" + type = "string" + description = <<-EOT + # Select the machine image + See the [registry](https://container.registry.blah/namespace) for options. + EOT + mutable = true + icon = "/icon/region.svg" + option { + name = "US Central" + value = "us-central1-a" + icon = "/icon/central.svg" + description = "Select for central!" + } + option { + name = "US East" + value = "us-east1-a" + icon = "/icon/east.svg" + description = "Select for east!" + } + } + `, Check: func(state *terraform.ResourceState) { attrs := state.Primary.Attributes for key, value := range map[string]interface{}{ @@ -67,100 +67,100 @@ data "coder_parameter" "region" { }, { Name: "ValidationWithOptions", Config: ` -data "coder_parameter" "region" { - name = "Region" - type = "number" - option { - name = "1" - value = "1" - } - validation { - regex = "1" - error = "Not 1" - } -} -`, + data "coder_parameter" "region" { + name = "Region" + type = "number" + option { + name = "1" + value = "1" + } + validation { + regex = "1" + error = "Not 1" + } + } + `, ExpectError: regexp.MustCompile("conflicts with option"), }, { Name: "ValidationRegexMissingError", Config: ` -data "coder_parameter" "region" { - name = "Region" - type = "string" - default = "hello" - validation { - regex = "hello" - } -} -`, + data "coder_parameter" "region" { + name = "Region" + type = "string" + default = "hello" + validation { + regex = "hello" + } + } + `, ExpectError: regexp.MustCompile("an error must be specified"), }, { Name: "NumberValidation", Config: ` -data "coder_parameter" "region" { - name = "Region" - type = "number" - default = 2 - validation { - min = 1 - max = 5 - } -} -`, + data "coder_parameter" "region" { + name = "Region" + type = "number" + default = 2 + validation { + min = 1 + max = 5 + } + } + `, }, { Name: "DefaultNotNumber", Config: ` -data "coder_parameter" "region" { - name = "Region" - type = "number" - default = true -} -`, + data "coder_parameter" "region" { + name = "Region" + type = "number" + default = true + } + `, ExpectError: regexp.MustCompile("is not a number"), }, { Name: "DefaultNotBool", Config: ` -data "coder_parameter" "region" { - name = "Region" - type = "bool" - default = 5 -} -`, + data "coder_parameter" "region" { + name = "Region" + type = "bool" + default = 5 + } + `, ExpectError: regexp.MustCompile("is not a bool"), }, { Name: "OptionNotBool", Config: ` -data "coder_parameter" "region" { - name = "Region" - type = "bool" - option { - value = 1 - name = 1 - } - option { - value = 2 - name = 2 - } -}`, + data "coder_parameter" "region" { + name = "Region" + type = "bool" + option { + value = 1 + name = 1 + } + option { + value = 2 + name = 2 + } + }`, ExpectError: regexp.MustCompile("\"2\" is not a bool"), }, { Name: "MultipleOptions", Config: ` -data "coder_parameter" "region" { - name = "Region" - type = "string" - option { - name = "1" - value = "1" - icon = "/icon/code.svg" - description = "Something!" - } - option { - name = "2" - value = "2" - } -} -`, + data "coder_parameter" "region" { + name = "Region" + type = "string" + option { + name = "1" + value = "1" + icon = "/icon/code.svg" + description = "Something!" + } + option { + name = "2" + value = "2" + } + } + `, Check: func(state *terraform.ResourceState) { for key, expected := range map[string]string{ "name": "Region", @@ -176,22 +176,22 @@ data "coder_parameter" "region" { }, { Name: "ValidDefaultWithOptions", Config: ` -data "coder_parameter" "region" { - name = "Region" - type = "string" - default = "2" - option { - name = "1" - value = "1" - icon = "/icon/code.svg" - description = "Something!" - } - option { - name = "2" - value = "2" - } -} -`, + data "coder_parameter" "region" { + name = "Region" + type = "string" + default = "2" + option { + name = "1" + value = "1" + icon = "/icon/code.svg" + description = "Something!" + } + option { + name = "2" + value = "2" + } + } + `, Check: func(state *terraform.ResourceState) { for key, expected := range map[string]string{ "name": "Region", @@ -207,72 +207,72 @@ data "coder_parameter" "region" { }, { Name: "InvalidDefaultWithOption", Config: ` -data "coder_parameter" "region" { - name = "Region" - default = "hi" - option { - name = "1" - value = "1" - } - option { - name = "2" - value = "2" - } -} -`, + data "coder_parameter" "region" { + name = "Region" + default = "hi" + option { + name = "1" + value = "1" + } + option { + name = "2" + value = "2" + } + } + `, ExpectError: regexp.MustCompile("must be defined as one of options"), }, { Name: "SingleOption", Config: ` -data "coder_parameter" "region" { - name = "Region" - option { - name = "1" - value = "1" - } -} -`, + data "coder_parameter" "region" { + name = "Region" + option { + name = "1" + value = "1" + } + } + `, }, { Name: "DuplicateOptionName", Config: ` -data "coder_parameter" "region" { - name = "Region" - type = "string" - option { - name = "1" - value = "1" - } - option { - name = "1" - value = "2" - } -} -`, + data "coder_parameter" "region" { + name = "Region" + type = "string" + option { + name = "1" + value = "1" + } + option { + name = "1" + value = "2" + } + } + `, ExpectError: regexp.MustCompile("cannot have the same name"), }, { Name: "DuplicateOptionValue", Config: ` -data "coder_parameter" "region" { - name = "Region" - type = "string" - option { - name = "1" - value = "1" - } - option { - name = "2" - value = "1" - } -} -`, + data "coder_parameter" "region" { + name = "Region" + type = "string" + option { + name = "1" + value = "1" + } + option { + name = "2" + value = "1" + } + } + `, ExpectError: regexp.MustCompile("cannot have the same value"), }, { Name: "RequiredParameterNoDefault", Config: ` -data "coder_parameter" "region" { - name = "Region" - type = "string" -}`, + data "coder_parameter" "region" { + name = "Region" + type = "string" + }`, Check: func(state *terraform.ResourceState) { for key, expected := range map[string]string{ "name": "Region", @@ -285,11 +285,11 @@ data "coder_parameter" "region" { }, { Name: "RequiredParameterDefaultNull", Config: ` -data "coder_parameter" "region" { - name = "Region" - type = "string" - default = null -}`, + data "coder_parameter" "region" { + name = "Region" + type = "string" + default = null + }`, Check: func(state *terraform.ResourceState) { for key, expected := range map[string]string{ "name": "Region", @@ -302,11 +302,11 @@ data "coder_parameter" "region" { }, { Name: "OptionalParameterDefaultEmpty", Config: ` -data "coder_parameter" "region" { - name = "Region" - type = "string" - default = "" -}`, + data "coder_parameter" "region" { + name = "Region" + type = "string" + default = "" + }`, Check: func(state *terraform.ResourceState) { for key, expected := range map[string]string{ "name": "Region", @@ -319,16 +319,40 @@ data "coder_parameter" "region" { }, { Name: "OptionalParameterDefaultNotEmpty", Config: ` + data "coder_parameter" "region" { + name = "Region" + type = "string" + default = "us-east-1" + }`, + Check: func(state *terraform.ResourceState) { + for key, expected := range map[string]string{ + "name": "Region", + "type": "string", + "optional": "true", + } { + require.Equal(t, expected, state.Primary.Attributes[key]) + } + }, + }, { + Name: "LegacyVariable", + Config: ` +variable "old_region" { + type = string + default = "fake-region" # for testing purposes, no need to set via env TF_... +} + data "coder_parameter" "region" { name = "Region" type = "string" - default = "us-east-1" + default = "will-be-ignored" + legacy_variable = var.old_region }`, Check: func(state *terraform.ResourceState) { for key, expected := range map[string]string{ - "name": "Region", - "type": "string", - "optional": "true", + "name": "Region", + "type": "string", + "default": "fake-region", + "legacy_variable": "fake-region", } { require.Equal(t, expected, state.Primary.Attributes[key]) } From 8102bd1f6fc378af219e27f012afe20a672811e2 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 7 Mar 2023 14:50:07 +0100 Subject: [PATCH 2/4] make gen --- docs/data-sources/parameter.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/data-sources/parameter.md b/docs/data-sources/parameter.md index 51d95262..0680933f 100644 --- a/docs/data-sources/parameter.md +++ b/docs/data-sources/parameter.md @@ -24,6 +24,7 @@ Use this data source to configure editable options for workspaces. - `default` (String) A default value for the parameter. - `description` (String) Describe what this parameter does. - `icon` (String) A URL to an icon that will display in the dashboard. View built-in icons here: https://github.com/coder/coder/tree/main/site/static/icon. Use a built-in icon with `data.coder_workspace.me.access_url + "/icon/"`. +- `legacy_variable` (String) The name of the Terraform variable used by legacy parameters. Coder will use it to lookup the parameter value. - `mutable` (Boolean) Whether this value can be changed after workspace creation. This can be destructive for values like region, so use with caution! - `option` (Block List, Max: 64) Each "option" block defines a value for a user to select from. (see [below for nested schema](#nestedblock--option)) - `type` (String) The type of this parameter. Must be one of: "number", "string", or "bool". From f60baa6bb8587c9edec4aeca9eef45c477507346 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 7 Mar 2023 14:54:01 +0100 Subject: [PATCH 3/4] Fix: typo --- examples/resources/coder_parameter_migration/resource.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/resources/coder_parameter_migration/resource.tf b/examples/resources/coder_parameter_migration/resource.tf index bef7034c..79040b7a 100644 --- a/examples/resources/coder_parameter_migration/resource.tf +++ b/examples/resources/coder_parameter_migration/resource.tf @@ -8,5 +8,5 @@ variable "old_account_name" { data "coder_parameter" "account_name" { name = "Account Name" type = "string" - legacy_variable_name = var.old_account_name + legacy_variable = var.old_account_name } \ No newline at end of file From 8f50ce21e48a22e21f771bef61fbcee906184bdd Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 7 Mar 2023 14:57:21 +0100 Subject: [PATCH 4/4] fmt --- examples/resources/coder_parameter_migration/resource.tf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/resources/coder_parameter_migration/resource.tf b/examples/resources/coder_parameter_migration/resource.tf index 79040b7a..7e903989 100644 --- a/examples/resources/coder_parameter_migration/resource.tf +++ b/examples/resources/coder_parameter_migration/resource.tf @@ -6,7 +6,7 @@ variable "old_account_name" { } data "coder_parameter" "account_name" { - name = "Account Name" - type = "string" + name = "Account Name" + type = "string" legacy_variable = var.old_account_name } \ No newline at end of file