Skip to content

feat: allow presets to define prebuilds #373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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._

Expand Down
2 changes: 2 additions & 0 deletions docs/data-sources/workspace.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
21 changes: 16 additions & 5 deletions docs/data-sources/workspace_preset.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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.

<a id="nestedblock--prebuilds"></a>
### Nested Schema for `prebuilds`

Required:

- `instances` (Number) The number of workspaces to keep in reserve for this preset.
9 changes: 5 additions & 4 deletions integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
},
},
{
Expand Down
5 changes: 5 additions & 0 deletions integration/test-data-source/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ data "coder_workspace_preset" "preset" {
parameters = {
(data.coder_parameter.param.name) = "preset param value"
}

prebuilds {
instances = 1
}
}

locals {
Expand All @@ -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),
}
}

Expand Down
22 changes: 22 additions & 0 deletions provider/workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link

@evgeniy-scherbina evgeniy-scherbina Apr 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I'd rather write it like this:

if prebuild == "true" {
	_ = rd.Set("is_prebuild", true)
	_ = rd.Set("prebuild_count", 1)
} else {
	_ = rd.Set("is_prebuild", false)
	_ = rd.Set("prebuild_count", 0)
}

I think it's more logical, but it's not a big deal


name := helpers.OptionalEnvOrDefault("CODER_WORKSPACE_NAME", "default")
rd.Set("name", name)

Expand Down Expand Up @@ -83,6 +91,11 @@ func workspaceDataSource() *schema.Resource {
Computed: true,
Description: "The access port of the Coder deployment provisioning this workspace.",
},
"prebuild_count": {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_prebuild & prebuild_count looks the same to me, what's the point to have both?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be duplication, yes. We could probably get rid of "is_prebuild" and just check the count. Looking at the rest of the code, we are following the pattern that was set by the "transition" and "start_count" parameters. They have the same relationship.

I'm not sure whether to remove "is_prebuild" or keep it.

Copy link

@evgeniy-scherbina evgeniy-scherbina Apr 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like neither prebuild_count nor start_count. But probably there is some reason we're using, I'm not aware of.
My guess is we're using start_count to map transition statuses, like:

  • stop: 0
  • start: 1
    etc...

Also looks like there is some duplication here as well:

  • start_count
  • transition

But I don't know why it's needed.

We can ask original author of this approach with start_count, transition?
Maybe it was done in a rush, and there is no point to continue using this approach.

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,
Expand All @@ -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,
Expand All @@ -121,3 +139,7 @@ func workspaceDataSource() *schema.Resource {
},
}
}

func IsPrebuildEnvironmentVariable() string {
return "CODER_WORKSPACE_IS_PREBUILD"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it can be const instead of func, but up to you

}
47 changes: 35 additions & 12 deletions provider/workspace_preset.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,59 +12,82 @@ 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
Copy link

@evgeniy-scherbina evgeniy-scherbina Apr 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SasSwart btw: I just noticed code in the ReadContext is basically no-op?
It reads data and then forgets them.
Probably it's fine if main goal is to send this data downstream to provisioner

But still what is the purpose of this code?

  • is it for validation purposes? But I'm not sure maybe validations happens earlier?
  • is it for helping debugging in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the decoding is defunct, yes. I've removed it.
I don't think we should rely on this for validation.
We define a schema below, which Terraform already uses for validation.
I've removed the defunct code here: #376

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
},
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),
},
},
},
},
},
}
}
40 changes: 38 additions & 2 deletions provider/workspace_preset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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 {
Expand Down
Loading