From ef92eea9d88a63ecda165a3f17f891845d3a7da2 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Mon, 7 Apr 2025 08:20:00 +0200 Subject: [PATCH 1/4] feat: allow presets to define prebuilds (#373) * feat: allow presets to define prebuilds * document prebuild parameters * remove todo * make gen * Add integration testing * Allow presets to define 0 parameters * additional documentation for prebuilds --- README.md | 2 +- docs/data-sources/workspace.md | 2 ++ docs/data-sources/workspace_preset.md | 21 +++++++++--- integration/integration_test.go | 9 ++--- integration/test-data-source/main.tf | 5 +++ provider/workspace.go | 22 +++++++++++++ provider/workspace_preset.go | 47 ++++++++++++++++++++------- provider/workspace_preset_test.go | 40 +++++++++++++++++++++-- 8 files changed, 124 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index b8ee8840..f055961e 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,7 @@ to setup your local Terraform to use your local version rather than the registry } ``` 2. Run `terraform init` and observe a warning like `Warning: Provider development overrides are in effect` -4. Run `go build -o terraform-provider-coder` to build the provider binary, which Terraform will try locate and execute +4. Run `make build` to build the provider binary, which Terraform will try locate and execute 5. All local Terraform runs will now use your local provider! 6. _**NOTE**: we vendor in this provider into `github.com/coder/coder`, so if you're testing with a local clone then you should also run `go mod edit -replace github.com/coder/terraform-provider-coder=/path/to/terraform-provider-coder` in your clone._ diff --git a/docs/data-sources/workspace.md b/docs/data-sources/workspace.md index 26396ba1..29fd9179 100644 --- a/docs/data-sources/workspace.md +++ b/docs/data-sources/workspace.md @@ -69,7 +69,9 @@ resource "docker_container" "workspace" { - `access_port` (Number) The access port of the Coder deployment provisioning this workspace. - `access_url` (String) The access URL of the Coder deployment provisioning this workspace. - `id` (String) UUID of the workspace. +- `is_prebuild` (Boolean) Similar to `prebuild_count`, but a boolean value instead of a count. This is set to true if the workspace is a currently unassigned prebuild. Once the workspace is assigned, this value will be false. - `name` (String) Name of the workspace. +- `prebuild_count` (Number) A computed count, equal to 1 if the workspace is a currently unassigned prebuild. Use this to conditionally act on the status of a prebuild. Actions that do not require user identity can be taken when this value is set to 1. Actions that should only be taken once the workspace has been assigned to a user may be taken when this value is set to 0. - `start_count` (Number) A computed count based on `transition` state. If `start`, count will equal 1. - `template_id` (String) ID of the workspace's template. - `template_name` (String) Name of the workspace's template. diff --git a/docs/data-sources/workspace_preset.md b/docs/data-sources/workspace_preset.md index 28f90faa..edd61f18 100644 --- a/docs/data-sources/workspace_preset.md +++ b/docs/data-sources/workspace_preset.md @@ -3,12 +3,12 @@ page_title: "coder_workspace_preset Data Source - terraform-provider-coder" subcategory: "" description: |- - Use this data source to predefine common configurations for workspaces. + Use this data source to predefine common configurations for coder workspaces. Users will have the option to select a defined preset, which will automatically apply the selected configuration. Any parameters defined in the preset will be applied to the workspace. Parameters that are not defined by the preset will still be configurable when creating a workspace. --- # coder_workspace_preset (Data Source) -Use this data source to predefine common configurations for workspaces. +Use this data source to predefine common configurations for coder workspaces. Users will have the option to select a defined preset, which will automatically apply the selected configuration. Any parameters defined in the preset will be applied to the workspace. Parameters that are not defined by the preset will still be configurable when creating a workspace. ## Example Usage @@ -34,9 +34,20 @@ data "coder_workspace_preset" "example" { ### Required -- `name` (String) Name of the workspace preset. -- `parameters` (Map of String) Parameters of the workspace preset. +- `name` (String) The name of the workspace preset. + +### Optional + +- `parameters` (Map of String) Workspace parameters that will be set by the workspace preset. For simple templates that only need prebuilds, you may define a preset with zero parameters. Because workspace parameters may change between Coder template versions, preset parameters are allowed to define values for parameters that do not exist in the current template version. +- `prebuilds` (Block Set, Max: 1) Prebuilt workspace configuration related to this workspace preset. Coder will build and maintain workspaces in reserve based on this configuration. When a user creates a new workspace using a preset, they will be assigned a prebuilt workspace, instead of waiting for a new workspace to build. (see [below for nested schema](#nestedblock--prebuilds)) ### Read-Only -- `id` (String) ID of the workspace preset. +- `id` (String) The preset ID is automatically generated and may change between runs. It is recommended to use the `name` attribute to identify the preset. + + +### Nested Schema for `prebuilds` + +Required: + +- `instances` (Number) The number of workspaces to keep in reserve for this preset. diff --git a/integration/integration_test.go b/integration/integration_test.go index 65aa5aed..9803aa41 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -90,10 +90,11 @@ func TestIntegration(t *testing.T) { // TODO (sasswart): the cli doesn't support presets yet. // once it does, the value for workspace_parameter.value // will be the preset value. - "workspace_parameter.value": `param value`, - "workspace_parameter.icon": `param icon`, - "workspace_preset.name": `preset`, - "workspace_preset.parameters.param": `preset param value`, + "workspace_parameter.value": `param value`, + "workspace_parameter.icon": `param icon`, + "workspace_preset.name": `preset`, + "workspace_preset.parameters.param": `preset param value`, + "workspace_preset.prebuilds.instances": `1`, }, }, { diff --git a/integration/test-data-source/main.tf b/integration/test-data-source/main.tf index 5fb2e0e6..f18fa347 100644 --- a/integration/test-data-source/main.tf +++ b/integration/test-data-source/main.tf @@ -24,6 +24,10 @@ data "coder_workspace_preset" "preset" { parameters = { (data.coder_parameter.param.name) = "preset param value" } + + prebuilds { + instances = 1 + } } locals { @@ -47,6 +51,7 @@ locals { "workspace_parameter.icon" : data.coder_parameter.param.icon, "workspace_preset.name" : data.coder_workspace_preset.preset.name, "workspace_preset.parameters.param" : data.coder_workspace_preset.preset.parameters.param, + "workspace_preset.prebuilds.instances" : tostring(one(data.coder_workspace_preset.preset.prebuilds).instances), } } diff --git a/provider/workspace.go b/provider/workspace.go index fde742b6..5ddd3ee8 100644 --- a/provider/workspace.go +++ b/provider/workspace.go @@ -27,6 +27,14 @@ func workspaceDataSource() *schema.Resource { } _ = rd.Set("start_count", count) + prebuild := helpers.OptionalEnv(IsPrebuildEnvironmentVariable()) + prebuildCount := 0 + if prebuild == "true" { + prebuildCount = 1 + _ = rd.Set("is_prebuild", true) + } + _ = rd.Set("prebuild_count", prebuildCount) + name := helpers.OptionalEnvOrDefault("CODER_WORKSPACE_NAME", "default") rd.Set("name", name) @@ -83,6 +91,11 @@ func workspaceDataSource() *schema.Resource { Computed: true, Description: "The access port of the Coder deployment provisioning this workspace.", }, + "prebuild_count": { + Type: schema.TypeInt, + Computed: true, + Description: "A computed count, equal to 1 if the workspace is a currently unassigned prebuild. Use this to conditionally act on the status of a prebuild. Actions that do not require user identity can be taken when this value is set to 1. Actions that should only be taken once the workspace has been assigned to a user may be taken when this value is set to 0.", + }, "start_count": { Type: schema.TypeInt, Computed: true, @@ -98,6 +111,11 @@ func workspaceDataSource() *schema.Resource { Computed: true, Description: "UUID of the workspace.", }, + "is_prebuild": { + Type: schema.TypeBool, + Computed: true, + Description: "Similar to `prebuild_count`, but a boolean value instead of a count. This is set to true if the workspace is a currently unassigned prebuild. Once the workspace is assigned, this value will be false.", + }, "name": { Type: schema.TypeString, Computed: true, @@ -121,3 +139,7 @@ func workspaceDataSource() *schema.Resource { }, } } + +func IsPrebuildEnvironmentVariable() string { + return "CODER_WORKSPACE_IS_PREBUILD" +} diff --git a/provider/workspace_preset.go b/provider/workspace_preset.go index cd56c980..004489e7 100644 --- a/provider/workspace_preset.go +++ b/provider/workspace_preset.go @@ -12,33 +12,39 @@ import ( type WorkspacePreset struct { Name string `mapstructure:"name"` Parameters map[string]string `mapstructure:"parameters"` + Prebuilds WorkspacePrebuild `mapstructure:"prebuilds"` +} + +type WorkspacePrebuild struct { + Instances int `mapstructure:"instances"` } func workspacePresetDataSource() *schema.Resource { return &schema.Resource{ SchemaVersion: 1, - Description: "Use this data source to predefine common configurations for workspaces.", + Description: "Use this data source to predefine common configurations for coder workspaces. Users will have the option to select a defined preset, which will automatically apply the selected configuration. Any parameters defined in the preset will be applied to the workspace. Parameters that are not defined by the preset will still be configurable when creating a workspace.", ReadContext: func(ctx context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { var preset WorkspacePreset err := mapstructure.Decode(struct { Name interface{} Parameters interface{} + Prebuilds struct { + Instances interface{} + } }{ Name: rd.Get("name"), Parameters: rd.Get("parameters"), + Prebuilds: struct { + Instances interface{} + }{ + Instances: rd.Get("prebuilds.0.instances"), + }, }, &preset) if err != nil { return diag.Errorf("decode workspace preset: %s", err) } - // MinItems doesn't work with maps, so we need to check the length - // of the map manually. All other validation is handled by the - // schema. - if len(preset.Parameters) == 0 { - return diag.Errorf("expected \"parameters\" to not be an empty map") - } - rd.SetId(preset.Name) return nil @@ -46,25 +52,42 @@ func workspacePresetDataSource() *schema.Resource { Schema: map[string]*schema.Schema{ "id": { Type: schema.TypeString, - Description: "ID of the workspace preset.", + Description: "The preset ID is automatically generated and may change between runs. It is recommended to use the `name` attribute to identify the preset.", Computed: true, }, "name": { Type: schema.TypeString, - Description: "Name of the workspace preset.", + Description: "The name of the workspace preset.", Required: true, ValidateFunc: validation.StringIsNotEmpty, }, "parameters": { Type: schema.TypeMap, - Description: "Parameters of the workspace preset.", - Required: true, + Description: "Workspace parameters that will be set by the workspace preset. For simple templates that only need prebuilds, you may define a preset with zero parameters. Because workspace parameters may change between Coder template versions, preset parameters are allowed to define values for parameters that do not exist in the current template version.", + Optional: true, Elem: &schema.Schema{ Type: schema.TypeString, Required: true, ValidateFunc: validation.StringIsNotEmpty, }, }, + "prebuilds": { + Type: schema.TypeSet, + Description: "Prebuilt workspace configuration related to this workspace preset. Coder will build and maintain workspaces in reserve based on this configuration. When a user creates a new workspace using a preset, they will be assigned a prebuilt workspace, instead of waiting for a new workspace to build.", + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "instances": { + Type: schema.TypeInt, + Description: "The number of workspaces to keep in reserve for this preset.", + Required: true, + ForceNew: true, + ValidateFunc: validation.IntAtLeast(0), + }, + }, + }, + }, }, } } diff --git a/provider/workspace_preset_test.go b/provider/workspace_preset_test.go index 876e2044..aa1ca0ce 100644 --- a/provider/workspace_preset_test.go +++ b/provider/workspace_preset_test.go @@ -84,7 +84,7 @@ func TestWorkspacePreset(t *testing.T) { }`, // This validation is done by Terraform, but it could still break if we misconfigure the schema. // So we test it here to make sure we don't regress. - ExpectError: regexp.MustCompile("The argument \"parameters\" is required, but no definition was found"), + ExpectError: nil, }, { Name: "Parameters field is empty", @@ -95,7 +95,7 @@ func TestWorkspacePreset(t *testing.T) { }`, // This validation is *not* done by Terraform, because MinItems doesn't work with maps. // We've implemented the validation in ReadContext, so we test it here to make sure we don't regress. - ExpectError: regexp.MustCompile("expected \"parameters\" to not be an empty map"), + ExpectError: nil, }, { Name: "Parameters field is not a map", @@ -108,6 +108,42 @@ func TestWorkspacePreset(t *testing.T) { // So we test it here to make sure we don't regress. ExpectError: regexp.MustCompile("Inappropriate value for attribute \"parameters\": map of string required"), }, + { + Name: "Prebuilds is set, but not its required fields", + Config: ` + data "coder_workspace_preset" "preset_1" { + name = "preset_1" + parameters = { + "region" = "us-east1-a" + } + prebuilds {} + }`, + ExpectError: regexp.MustCompile("The argument \"instances\" is required, but no definition was found."), + }, + { + Name: "Prebuilds is set, and so are its required fields", + Config: ` + data "coder_workspace_preset" "preset_1" { + name = "preset_1" + parameters = { + "region" = "us-east1-a" + } + prebuilds { + instances = 1 + } + }`, + ExpectError: nil, + Check: func(state *terraform.State) error { + require.Len(t, state.Modules, 1) + require.Len(t, state.Modules[0].Resources, 1) + resource := state.Modules[0].Resources["data.coder_workspace_preset.preset_1"] + require.NotNil(t, resource) + attrs := resource.Primary.Attributes + require.Equal(t, attrs["name"], "preset_1") + require.Equal(t, attrs["prebuilds.0.instances"], "1") + return nil + }, + }, } for _, testcase := range testcases { From 3a2c18dab13ebd36ddab31a97e2c566696b1d3e3 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Mon, 7 Apr 2025 09:55:38 +0200 Subject: [PATCH 2/4] Correct the prebuilds type (#376) --- provider/workspace_preset.go | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/provider/workspace_preset.go b/provider/workspace_preset.go index 004489e7..2fab0b8a 100644 --- a/provider/workspace_preset.go +++ b/provider/workspace_preset.go @@ -12,7 +12,12 @@ import ( type WorkspacePreset struct { Name string `mapstructure:"name"` Parameters map[string]string `mapstructure:"parameters"` - Prebuilds WorkspacePrebuild `mapstructure:"prebuilds"` + // There should always be only one prebuild block, but Terraform's type system + // still parses them as a slice, so we need to handle it as such. We could use + // an anonymous type and rd.Get to avoid a slice here, but that would not be possible + // for utilities that parse our terraform output using this type. To remain compatible + // with those cases, we use a slice here. + Prebuilds []WorkspacePrebuild `mapstructure:"prebuilds"` } type WorkspacePrebuild struct { @@ -27,19 +32,9 @@ func workspacePresetDataSource() *schema.Resource { ReadContext: func(ctx context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { var preset WorkspacePreset err := mapstructure.Decode(struct { - Name interface{} - Parameters interface{} - Prebuilds struct { - Instances interface{} - } + Name interface{} }{ - Name: rd.Get("name"), - Parameters: rd.Get("parameters"), - Prebuilds: struct { - Instances interface{} - }{ - Instances: rd.Get("prebuilds.0.instances"), - }, + Name: rd.Get("name"), }, &preset) if err != nil { return diag.Errorf("decode workspace preset: %s", err) From 8804c44f070b366bb29e0e4b8d44b6eba9473b0c Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 8 Apr 2025 13:04:02 -0500 Subject: [PATCH 3/4] feat: `form_type` and `styling` metadata arguments added (#375) Adds `form_type` as a field to parameters. By default, legacy behavior is maintained by deducing the default `form_type` value from the existing inputs. `styling` argument also added for cosmetic usage on the frontend form --- docs/data-sources/parameter.md | 2 + provider/formtype.go | 148 ++++++++++++ provider/formtype_test.go | 428 +++++++++++++++++++++++++++++++++ provider/parameter.go | 94 ++++++-- provider/parameter_test.go | 17 +- 5 files changed, 664 insertions(+), 25 deletions(-) create mode 100644 provider/formtype.go create mode 100644 provider/formtype_test.go diff --git a/docs/data-sources/parameter.md b/docs/data-sources/parameter.md index e46cf86d..934ae77a 100644 --- a/docs/data-sources/parameter.md +++ b/docs/data-sources/parameter.md @@ -145,10 +145,12 @@ data "coder_parameter" "home_volume_size" { - `description` (String) Describe what this parameter does. - `display_name` (String) The displayed name of the parameter as it will appear in the interface. - `ephemeral` (Boolean) The value of an ephemeral parameter will not be preserved between consecutive workspace builds. +- `form_type` (String) The type of this parameter. Must be one of: [radio, slider, input, dropdown, checkbox, switch, multi-select, tag-select, textarea, error]. - `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/"`. - `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) Each `option` block defines a value for a user to select from. (see [below for nested schema](#nestedblock--option)) - `order` (Number) The order determines the position of a template parameter in the UI/CLI presentation. The lowest order is shown first and parameters with equal order are sorted by name (ascending order). +- `styling` (String) JSON encoded string containing the metadata for controlling the appearance of this parameter in the UI. This option is purely cosmetic and does not affect the function of the parameter in terraform. - `type` (String) The type of this parameter. Must be one of: `"number"`, `"string"`, `"bool"`, or `"list(string)"`. - `validation` (Block List, Max: 1) Validate the input of a parameter. (see [below for nested schema](#nestedblock--validation)) diff --git a/provider/formtype.go b/provider/formtype.go new file mode 100644 index 00000000..48fbeed3 --- /dev/null +++ b/provider/formtype.go @@ -0,0 +1,148 @@ +package provider + +import ( + "slices" + + "golang.org/x/xerrors" +) + +// OptionType is a type of option that can be used in the 'type' argument of +// a parameter. These should match types as defined in terraform: +// +// https://developer.hashicorp.com/terraform/language/expressions/types +// +// The value have to be string literals, as type constraint keywords are not +// supported in providers. +type OptionType string + +const ( + OptionTypeString OptionType = "string" + OptionTypeNumber OptionType = "number" + OptionTypeBoolean OptionType = "bool" + OptionTypeListString OptionType = "list(string)" +) + +func OptionTypes() []OptionType { + return []OptionType{ + OptionTypeString, + OptionTypeNumber, + OptionTypeBoolean, + OptionTypeListString, + } +} + +// ParameterFormType is the list of supported form types for display in +// the Coder "create workspace" form. These form types are functional as well +// as cosmetic. Refer to `formTypeTruthTable` for the allowed pairings. +// For example, "multi-select" has the type "list(string)" but the option +// values are "string". +type ParameterFormType string + +const ( + ParameterFormTypeDefault ParameterFormType = "" + ParameterFormTypeRadio ParameterFormType = "radio" + ParameterFormTypeSlider ParameterFormType = "slider" + ParameterFormTypeInput ParameterFormType = "input" + ParameterFormTypeDropdown ParameterFormType = "dropdown" + ParameterFormTypeCheckbox ParameterFormType = "checkbox" + ParameterFormTypeSwitch ParameterFormType = "switch" + ParameterFormTypeMultiSelect ParameterFormType = "multi-select" + ParameterFormTypeTagSelect ParameterFormType = "tag-select" + ParameterFormTypeTextArea ParameterFormType = "textarea" + ParameterFormTypeError ParameterFormType = "error" +) + +// ParameterFormTypes should be kept in sync with the enum list above. +func ParameterFormTypes() []ParameterFormType { + return []ParameterFormType{ + // Intentionally omit "ParameterFormTypeDefault" from this set. + // It is a valid enum, but will always be mapped to a real value when + // being used. + ParameterFormTypeRadio, + ParameterFormTypeSlider, + ParameterFormTypeInput, + ParameterFormTypeDropdown, + ParameterFormTypeCheckbox, + ParameterFormTypeSwitch, + ParameterFormTypeMultiSelect, + ParameterFormTypeTagSelect, + ParameterFormTypeTextArea, + ParameterFormTypeError, + } +} + +// formTypeTruthTable is a map of [`type`][`optionCount` > 0] to `form_type`. +// The first value in the slice is the default value assuming `form_type` is +// not specified. +// +// The boolean key indicates whether the `options` field is specified. +// | Type | Options | Specified Form Type | form_type | Notes | +// |-------------------|---------|---------------------|----------------|--------------------------------| +// | `string` `number` | Y | | `radio` | | +// | `string` `number` | Y | `dropdown` | `dropdown` | | +// | `string` `number` | N | | `input` | | +// | `string` | N | 'textarea' | `textarea` | | +// | `number` | N | 'slider' | `slider` | min/max validation | +// | `bool` | Y | | `radio` | | +// | `bool` | N | | `checkbox` | | +// | `bool` | N | `switch` | `switch` | | +// | `list(string)` | Y | | `radio` | | +// | `list(string)` | N | | `tag-select` | | +// | `list(string)` | Y | `multi-select` | `multi-select` | Option values will be `string` | +var formTypeTruthTable = map[OptionType]map[bool][]ParameterFormType{ + OptionTypeString: { + true: {ParameterFormTypeRadio, ParameterFormTypeDropdown}, + false: {ParameterFormTypeInput, ParameterFormTypeTextArea}, + }, + OptionTypeNumber: { + true: {ParameterFormTypeRadio, ParameterFormTypeDropdown}, + false: {ParameterFormTypeInput, ParameterFormTypeSlider}, + }, + OptionTypeBoolean: { + true: {ParameterFormTypeRadio}, + false: {ParameterFormTypeCheckbox, ParameterFormTypeSwitch}, + }, + OptionTypeListString: { + true: {ParameterFormTypeRadio, ParameterFormTypeMultiSelect}, + false: {ParameterFormTypeTagSelect}, + }, +} + +// ValidateFormType handles the truth table for the valid set of `type` and +// `form_type` options. +// The OptionType is also returned because it is possible the 'type' of the +// 'value' & 'default' fields is different from the 'type' of the options. +// The use case is when using multi-select. The options are 'string' and the +// value is 'list(string)'. +func ValidateFormType(paramType OptionType, optionCount int, specifiedFormType ParameterFormType) (OptionType, ParameterFormType, error) { + allowed, ok := formTypeTruthTable[paramType][optionCount > 0] + if !ok || len(allowed) == 0 { + return paramType, specifiedFormType, xerrors.Errorf("value type %q is not supported for 'form_types'", paramType) + } + + if specifiedFormType == ParameterFormTypeDefault { + // handle the default case + specifiedFormType = allowed[0] + } + + if !slices.Contains(allowed, specifiedFormType) { + return paramType, specifiedFormType, xerrors.Errorf("value type %q is not supported for 'form_types'", specifiedFormType) + } + + // This is the only current special case. If 'multi-select' is selected, the type + // of 'value' and an options 'value' are different. The type of the parameter is + // `list(string)` but the type of the individual options is `string`. + if paramType == OptionTypeListString && specifiedFormType == ParameterFormTypeMultiSelect { + return OptionTypeString, ParameterFormTypeMultiSelect, nil + } + + return paramType, specifiedFormType, nil +} + +func toStrings[A ~string](l []A) []string { + var r []string + for _, v := range l { + r = append(r, string(v)) + } + return r +} diff --git a/provider/formtype_test.go b/provider/formtype_test.go new file mode 100644 index 00000000..599c2e41 --- /dev/null +++ b/provider/formtype_test.go @@ -0,0 +1,428 @@ +package provider_test + +import ( + "encoding/json" + "fmt" + "regexp" + "strconv" + "strings" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/coder/terraform-provider-coder/v2/provider" +) + +// formTypeTestCase is the config for a single test case. +type formTypeTestCase struct { + name string + config formTypeCheck + assert paramAssert + expectError *regexp.Regexp +} + +// paramAssert is asserted on the provider's parsed terraform state. +type paramAssert struct { + FormType provider.ParameterFormType + Type provider.OptionType + Styling json.RawMessage +} + +// formTypeCheck is a struct that helps build the terraform config +type formTypeCheck struct { + formType provider.ParameterFormType + optionType provider.OptionType + options bool + + // optional to inform the assert + customOptions []string + defValue string + styling json.RawMessage +} + +func (c formTypeCheck) String() string { + return fmt.Sprintf("%s_%s_%t", c.formType, c.optionType, c.options) +} + +func TestValidateFormType(t *testing.T) { + t.Parallel() + + // formTypesChecked keeps track of all checks run. It will be used to + // ensure all combinations of form_type and option_type are tested. + // All untested options are assumed to throw an error. + formTypesChecked := make(map[string]struct{}) + + expectType := func(expected provider.ParameterFormType, opts formTypeCheck) formTypeTestCase { + ftname := opts.formType + if ftname == "" { + ftname = "default" + } + + if opts.styling == nil { + // Try passing arbitrary data in, as anything should be accepted + opts.styling, _ = json.Marshal(map[string]any{ + "foo": "bar", + "disabled": true, + "nested": map[string]any{ + "foo": "bar", + }, + }) + } + + return formTypeTestCase{ + name: fmt.Sprintf("%s_%s_%t", + ftname, + opts.optionType, + opts.options, + ), + config: opts, + assert: paramAssert{ + FormType: expected, + Type: opts.optionType, + Styling: opts.styling, + }, + expectError: nil, + } + } + + // expectSameFormType just assumes the FormType in the check is the expected + // FormType. Using `expectType` these fields can differ + expectSameFormType := func(opts formTypeCheck) formTypeTestCase { + return expectType(opts.formType, opts) + } + + cases := []formTypeTestCase{ + { + // When nothing is specified + name: "defaults", + config: formTypeCheck{}, + assert: paramAssert{ + FormType: provider.ParameterFormTypeInput, + Type: provider.OptionTypeString, + Styling: []byte("{}"), + }, + }, + // All default behaviors. Essentially legacy behavior. + // String + expectType(provider.ParameterFormTypeRadio, formTypeCheck{ + options: true, + optionType: provider.OptionTypeString, + }), + expectType(provider.ParameterFormTypeInput, formTypeCheck{ + options: false, + optionType: provider.OptionTypeString, + }), + // Number + expectType(provider.ParameterFormTypeRadio, formTypeCheck{ + options: true, + optionType: provider.OptionTypeNumber, + }), + expectType(provider.ParameterFormTypeInput, formTypeCheck{ + options: false, + optionType: provider.OptionTypeNumber, + }), + // Boolean + expectType(provider.ParameterFormTypeRadio, formTypeCheck{ + options: true, + optionType: provider.OptionTypeBoolean, + }), + expectType(provider.ParameterFormTypeCheckbox, formTypeCheck{ + options: false, + optionType: provider.OptionTypeBoolean, + }), + // List(string) + expectType(provider.ParameterFormTypeRadio, formTypeCheck{ + options: true, + optionType: provider.OptionTypeListString, + }), + expectType(provider.ParameterFormTypeTagSelect, formTypeCheck{ + options: false, + optionType: provider.OptionTypeListString, + }), + + // ---- New Behavior + // String + expectSameFormType(formTypeCheck{ + options: true, + optionType: provider.OptionTypeString, + formType: provider.ParameterFormTypeDropdown, + }), + expectSameFormType(formTypeCheck{ + options: true, + optionType: provider.OptionTypeString, + formType: provider.ParameterFormTypeRadio, + }), + expectSameFormType(formTypeCheck{ + options: false, + optionType: provider.OptionTypeString, + formType: provider.ParameterFormTypeInput, + }), + expectSameFormType(formTypeCheck{ + options: false, + optionType: provider.OptionTypeString, + formType: provider.ParameterFormTypeTextArea, + }), + // Number + expectSameFormType(formTypeCheck{ + options: true, + optionType: provider.OptionTypeNumber, + formType: provider.ParameterFormTypeDropdown, + }), + expectSameFormType(formTypeCheck{ + options: true, + optionType: provider.OptionTypeNumber, + formType: provider.ParameterFormTypeRadio, + }), + expectSameFormType(formTypeCheck{ + options: false, + optionType: provider.OptionTypeNumber, + formType: provider.ParameterFormTypeInput, + }), + expectSameFormType(formTypeCheck{ + options: false, + optionType: provider.OptionTypeNumber, + formType: provider.ParameterFormTypeSlider, + }), + // Boolean + expectSameFormType(formTypeCheck{ + options: true, + optionType: provider.OptionTypeBoolean, + formType: provider.ParameterFormTypeRadio, + }), + expectSameFormType(formTypeCheck{ + options: false, + optionType: provider.OptionTypeBoolean, + formType: provider.ParameterFormTypeSwitch, + }), + expectSameFormType(formTypeCheck{ + options: false, + optionType: provider.OptionTypeBoolean, + formType: provider.ParameterFormTypeCheckbox, + }), + // List(string) + expectSameFormType(formTypeCheck{ + options: true, + optionType: provider.OptionTypeListString, + formType: provider.ParameterFormTypeRadio, + }), + expectSameFormType(formTypeCheck{ + options: true, + optionType: provider.OptionTypeListString, + formType: provider.ParameterFormTypeMultiSelect, + customOptions: []string{"red", "blue", "green"}, + defValue: `["red", "blue"]`, + }), + expectSameFormType(formTypeCheck{ + options: false, + optionType: provider.OptionTypeListString, + formType: provider.ParameterFormTypeTagSelect, + }), + + // Some manual test cases + { + name: "list_string_bad_default", + config: formTypeCheck{ + formType: provider.ParameterFormTypeMultiSelect, + optionType: provider.OptionTypeListString, + customOptions: []string{"red", "blue", "green"}, + defValue: `["red", "yellow"]`, + styling: nil, + }, + expectError: regexp.MustCompile("is not a valid option"), + }, + } + + passed := t.Run("TabledTests", func(t *testing.T) { + // TabledCases runs through all the manual test cases + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + t.Parallel() + if _, ok := formTypesChecked[c.config.String()]; ok { + t.Log("Duplicated form type check, delete this extra test case") + t.Fatalf("form type %q already checked", c.config.String()) + } + + formTypesChecked[c.config.String()] = struct{}{} + formTypeTest(t, c) + }) + } + }) + + if !passed { + // Do not run additional tests and pollute the output + t.Log("Tests failed, will not run the assumed error cases") + return + } + + // AssumeErrorCases assumes any uncovered test will return an error. Not covered + // cases in the truth table are assumed to be invalid. So if the tests above + // cover all valid cases, this asserts all the invalid cases. + // + // This test consequentially ensures all valid cases are covered manually above. + t.Run("AssumeErrorCases", func(t *testing.T) { + // requiredChecks loops through all possible form_type and option_type + // combinations. + requiredChecks := make([]formTypeCheck, 0) + for _, ft := range append(provider.ParameterFormTypes(), "") { + for _, ot := range provider.OptionTypes() { + requiredChecks = append(requiredChecks, formTypeCheck{ + formType: ft, + optionType: ot, + options: false, + }) + requiredChecks = append(requiredChecks, formTypeCheck{ + formType: ft, + optionType: ot, + options: true, + }) + } + } + + for _, check := range requiredChecks { + if _, alreadyChecked := formTypesChecked[check.String()]; alreadyChecked { + continue + } + + ftName := check.formType + if ftName == "" { + ftName = "default" + } + fc := formTypeTestCase{ + name: fmt.Sprintf("%s_%s_%t", + ftName, + check.optionType, + check.options, + ), + config: check, + assert: paramAssert{}, + expectError: regexp.MustCompile("is not supported"), + } + + t.Run(fc.name, func(t *testing.T) { + t.Parallel() + + // This is just helpful log output to give the boilerplate + // to write the manual test. + tcText := fmt.Sprintf(` + expectSameFormType(%s, ezconfigOpts{ + Options: %t, + OptionType: %q, + FormType: %q, + }), + //`, "", check.options, check.optionType, check.formType) + + logDebugInfo := formTypeTest(t, fc) + if !logDebugInfo { + t.Logf("To construct this test case:\n%s", tcText) + } + }) + + } + }) +} + +// createTF converts a formTypeCheck into a terraform config string. +func createTF(paramName string, cfg formTypeCheck) (defaultValue string, tf string) { + options := cfg.customOptions + if cfg.options && len(cfg.customOptions) == 0 { + switch cfg.optionType { + case provider.OptionTypeString: + options = []string{"foo"} + defaultValue = "foo" + case provider.OptionTypeBoolean: + options = []string{"true", "false"} + defaultValue = "true" + case provider.OptionTypeNumber: + options = []string{"1"} + defaultValue = "1" + case provider.OptionTypeListString: + options = []string{`["red", "blue"]`} + defaultValue = `["red", "blue"]` + default: + panic(fmt.Sprintf("unknown option type %q when generating options", cfg.optionType)) + } + } + + if cfg.defValue == "" { + cfg.defValue = defaultValue + } + + var body strings.Builder + if cfg.defValue != "" { + body.WriteString(fmt.Sprintf("default = %q\n", cfg.defValue)) + } + if cfg.formType != "" { + body.WriteString(fmt.Sprintf("form_type = %q\n", cfg.formType)) + } + if cfg.optionType != "" { + body.WriteString(fmt.Sprintf("type = %q\n", cfg.optionType)) + } + if cfg.styling != nil { + body.WriteString(fmt.Sprintf("styling = %s\n", strconv.Quote(string(cfg.styling)))) + } + + for i, opt := range options { + body.WriteString("option {\n") + body.WriteString(fmt.Sprintf("name = \"val_%d\"\n", i)) + body.WriteString(fmt.Sprintf("value = %q\n", opt)) + body.WriteString("}\n") + } + + return cfg.defValue, fmt.Sprintf(` + provider "coder" { + } + data "coder_parameter" "%s" { + name = "%s" + %s + } + `, paramName, paramName, body.String()) +} + +func formTypeTest(t *testing.T, c formTypeTestCase) bool { + t.Helper() + const paramName = "test_param" + // logDebugInfo is just a guess used for logging. It's not important. It cannot + // determine for sure if the test passed because the terraform test runner is a + // black box. It does not indicate if the test passed or failed. Since this is + // just used for logging, this is good enough. + logDebugInfo := true + + def, tf := createTF(paramName, c.config) + checkFn := func(state *terraform.State) error { + require.Len(t, state.Modules, 1) + require.Len(t, state.Modules[0].Resources, 1) + + key := strings.Join([]string{"data", "coder_parameter", paramName}, ".") + param := state.Modules[0].Resources[key] + + logDebugInfo = logDebugInfo && assert.Equal(t, def, param.Primary.Attributes["default"], "default value") + logDebugInfo = logDebugInfo && assert.Equal(t, string(c.assert.FormType), param.Primary.Attributes["form_type"], "form_type") + logDebugInfo = logDebugInfo && assert.Equal(t, string(c.assert.Type), param.Primary.Attributes["type"], "type") + logDebugInfo = logDebugInfo && assert.JSONEq(t, string(c.assert.Styling), param.Primary.Attributes["styling"], "styling") + + return nil + } + if c.expectError != nil { + checkFn = nil + } + + resource.Test(t, resource.TestCase{ + IsUnitTest: true, + ProviderFactories: coderFactory(), + Steps: []resource.TestStep{ + { + Config: tf, + Check: checkFn, + ExpectError: c.expectError, + }, + }, + }) + + if !logDebugInfo { + t.Logf("Terraform config:\n%s", tf) + } + return logDebugInfo +} diff --git a/provider/parameter.go b/provider/parameter.go index 1345f4d6..4eb8a282 100644 --- a/provider/parameter.go +++ b/provider/parameter.go @@ -50,7 +50,8 @@ type Parameter struct { Name string DisplayName string `mapstructure:"display_name"` Description string - Type string + Type OptionType + FormType ParameterFormType Mutable bool Default string Icon string @@ -86,6 +87,7 @@ func parameterDataSource() *schema.Resource { DisplayName interface{} Description interface{} Type interface{} + FormType interface{} Mutable interface{} Default interface{} Icon interface{} @@ -100,6 +102,7 @@ func parameterDataSource() *schema.Resource { DisplayName: rd.Get("display_name"), Description: rd.Get("description"), Type: rd.Get("type"), + FormType: rd.Get("form_type"), Mutable: rd.Get("mutable"), Default: rd.Get("default"), Icon: rd.Get("icon"), @@ -149,6 +152,20 @@ func parameterDataSource() *schema.Resource { } } + // Validate options + + // optionType might differ from parameter.Type. This is ok, and parameter.Type + // should be used for the value type, and optionType for options. + var optionType OptionType + optionType, parameter.FormType, err = ValidateFormType(parameter.Type, len(parameter.Option), parameter.FormType) + if err != nil { + return diag.FromErr(err) + } + // Set the form_type back in case the value was changed. + // Eg via a default. If a user does not specify, a default value + // is used and saved. + rd.Set("form_type", parameter.FormType) + if len(parameter.Option) > 0 { names := map[string]interface{}{} values := map[string]interface{}{} @@ -161,7 +178,7 @@ func parameterDataSource() *schema.Resource { if exists { return diag.Errorf("multiple options cannot have the same value %q", option.Value) } - err := valueIsType(parameter.Type, option.Value) + err := valueIsType(optionType, option.Value) if err != nil { return err } @@ -170,9 +187,36 @@ func parameterDataSource() *schema.Resource { } if parameter.Default != "" { - _, defaultIsValid := values[parameter.Default] - if !defaultIsValid { - return diag.Errorf("default value %q must be defined as one of options", parameter.Default) + if parameter.Type == OptionTypeListString && optionType == OptionTypeString { + // If the type is list(string) and optionType is string, we have + // to ensure all elements of the default exist as options. + var defaultValues []string + // TODO: We do this unmarshal in a few spots. It should be standardized. + err = json.Unmarshal([]byte(parameter.Default), &defaultValues) + if err != nil { + return diag.Errorf("default value %q is not a list of strings", parameter.Default) + } + + // missing is used to construct a more helpful error message + var missing []string + for _, defaultValue := range defaultValues { + _, defaultIsValid := values[defaultValue] + if !defaultIsValid { + missing = append(missing, defaultValue) + } + } + + if len(missing) > 0 { + return diag.Errorf( + "default value %q is not a valid option, values %q are missing from the option", + parameter.Default, strings.Join(missing, ", "), + ) + } + } else { + _, defaultIsValid := values[parameter.Default] + if !defaultIsValid { + return diag.Errorf("%q default value %q must be defined as one of options", parameter.FormType, parameter.Default) + } } } } @@ -203,9 +247,23 @@ func parameterDataSource() *schema.Resource { Type: schema.TypeString, Default: "string", Optional: true, - ValidateFunc: validation.StringInSlice([]string{"number", "string", "bool", "list(string)"}, false), + ValidateFunc: validation.StringInSlice(toStrings(OptionTypes()), false), Description: "The type of this parameter. Must be one of: `\"number\"`, `\"string\"`, `\"bool\"`, or `\"list(string)\"`.", }, + "form_type": { + Type: schema.TypeString, + Default: ParameterFormTypeDefault, + Optional: true, + ValidateFunc: validation.StringInSlice(toStrings(ParameterFormTypes()), false), + Description: fmt.Sprintf("The type of this parameter. Must be one of: [%s].", strings.Join(toStrings(ParameterFormTypes()), ", ")), + }, + "styling": { + Type: schema.TypeString, + Default: `{}`, + Description: "JSON encoded string containing the metadata for controlling the appearance of this parameter in the UI. " + + "This option is purely cosmetic and does not affect the function of the parameter in terraform.", + Optional: true, + }, "mutable": { Type: schema.TypeBool, Optional: true, @@ -375,25 +433,25 @@ func fixValidationResourceData(rawConfig cty.Value, validation interface{}) (int return vArr, nil } -func valueIsType(typ, value string) diag.Diagnostics { +func valueIsType(typ OptionType, value string) diag.Diagnostics { switch typ { - case "number": + case OptionTypeNumber: _, err := strconv.ParseFloat(value, 64) if err != nil { return diag.Errorf("%q is not a number", value) } - case "bool": + case OptionTypeBoolean: _, err := strconv.ParseBool(value) if err != nil { return diag.Errorf("%q is not a bool", value) } - case "list(string)": + case OptionTypeListString: var items []string err := json.Unmarshal([]byte(value), &items) if err != nil { return diag.Errorf("%q is not an array of strings", value) } - case "string": + case OptionTypeString: // Anything is a string! default: return diag.Errorf("invalid type %q", typ) @@ -401,8 +459,8 @@ func valueIsType(typ, value string) diag.Diagnostics { return nil } -func (v *Validation) Valid(typ, value string) error { - if typ != "number" { +func (v *Validation) Valid(typ OptionType, value string) error { + if typ != OptionTypeNumber { if !v.MinDisabled { return fmt.Errorf("a min cannot be specified for a %s type", typ) } @@ -413,16 +471,16 @@ func (v *Validation) Valid(typ, value string) error { return fmt.Errorf("monotonic validation can only be specified for number types, not %s types", typ) } } - if typ != "string" && v.Regex != "" { + if typ != OptionTypeString && v.Regex != "" { return fmt.Errorf("a regex cannot be specified for a %s type", typ) } switch typ { - case "bool": + case OptionTypeBoolean: if value != "true" && value != "false" { return fmt.Errorf(`boolean value can be either "true" or "false"`) } return nil - case "string": + case OptionTypeString: if v.Regex == "" { return nil } @@ -437,7 +495,7 @@ func (v *Validation) Valid(typ, value string) error { if !matched { return fmt.Errorf("%s (value %q does not match %q)", v.Error, value, regex) } - case "number": + case OptionTypeNumber: num, err := strconv.Atoi(value) if err != nil { return takeFirstError(v.errorRendered(value), fmt.Errorf("value %q is not a number", value)) @@ -451,7 +509,7 @@ func (v *Validation) Valid(typ, value string) error { if v.Monotonic != "" && v.Monotonic != ValidationMonotonicIncreasing && v.Monotonic != ValidationMonotonicDecreasing { return fmt.Errorf("number monotonicity can be either %q or %q", ValidationMonotonicIncreasing, ValidationMonotonicDecreasing) } - case "list(string)": + case OptionTypeListString: var listOfStrings []string err := json.Unmarshal([]byte(value), &listOfStrings) if err != nil { diff --git a/provider/parameter_test.go b/provider/parameter_test.go index 7bcea8fd..f817280e 100644 --- a/provider/parameter_test.go +++ b/provider/parameter_test.go @@ -27,6 +27,7 @@ func TestParameter(t *testing.T) { name = "region" display_name = "Region" type = "string" + form_type = "dropdown" description = <<-EOT # Select the machine image See the [registry](https://container.registry.blah/namespace) for options. @@ -56,6 +57,7 @@ func TestParameter(t *testing.T) { "name": "region", "display_name": "Region", "type": "string", + "form_type": "dropdown", "description": "# Select the machine image\nSee the [registry](https://container.registry.blah/namespace) for options.\n", "mutable": "true", "icon": "/icon/region.svg", @@ -137,6 +139,7 @@ func TestParameter(t *testing.T) { for key, expected := range map[string]string{ "name": "Region", "type": "number", + "form_type": "input", "validation.#": "1", "default": "2", "validation.0.min": "1", @@ -686,13 +689,13 @@ data "coder_parameter" "region" { func TestValueValidatesType(t *testing.T) { t.Parallel() for _, tc := range []struct { - Name, - Type, - Value, - Regex, - RegexError string - Min, - Max int + Name string + Type provider.OptionType + Value string + Regex string + RegexError string + Min int + Max int MinDisabled, MaxDisabled bool Monotonic string Error *regexp.Regexp From d90c1817de41ff9768ce7be861a66ea02773c2ee Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 9 Apr 2025 09:04:53 -0500 Subject: [PATCH 4/4] chore: change OptionType to alias for usage as string (#377) To be backwards compatible using this repo as a library, use a string alias. This allows using the 'type' as a `string` or the new enum. Since enum validation is quite weak in Go, this is baiscally functionally equvialent. --- provider/formtype.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/provider/formtype.go b/provider/formtype.go index 48fbeed3..753f8945 100644 --- a/provider/formtype.go +++ b/provider/formtype.go @@ -13,7 +13,7 @@ import ( // // The value have to be string literals, as type constraint keywords are not // supported in providers. -type OptionType string +type OptionType = string const ( OptionTypeString OptionType = "string"