feat: Add legacy_variable_name to parameter #110
Conversation
docs/data-sources/parameter.md
Outdated
| @@ -24,7 +24,8 @@ 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/<path>"`. | |||
| - `legacy_variable` (String) The name of the Terraform variable used by legacy parameters. Coder will use it to lookup the parameter value. | |||
| - `legacy_variable` (String) Reference to the legacy parameter. Coder will use it to lookup the default value. | |||
There was a problem hiding this comment.
Maybe we could call this legacy_variable_{,default_}value to clearly indicate the difference between name/default value?
PS. It might be a bit confusing to users to call these variable when the description mentions parameters. Perhaps reword the description to only mention variables and not parameters? (Same below.)
There was a problem hiding this comment.
I rephrased the sentence to refer only to variables.
Maybe we could call this legacy_variable_{,default_}value to clearly indicate the difference between name/default value?
I'm hesitant as I'd rather see template authors referring to variables. I have a feeling that legacy_variable_{,default_}value may suggest inputting a number instead of ${var.foobar}. Actually, it's unfortunate that we need a field for a variable name, but Terraform can't operate on metadata (resource name).
Besides that, this is only for migration purposes. Those legacy_* should be present only for one update cycle of a template. I think we're good...
There was a problem hiding this comment.
I agree with your reasoning for keeping the name as-is. 👍🏻
| "legacy_variable_name": { | ||
| Type: schema.TypeString, | ||
| Optional: true, | ||
| RequiredWith: []string{"legacy_variable"}, |
There was a problem hiding this comment.
Great that you added this requirement! 🤩
Related: coder/coder#6368
This PR adds new property to
coder_parameter-legacy_variable_name.It's outcome of the research here.