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 4 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) Whether the workspace is a prebuild.
- `name` (String) Name of the workspace.
- `prebuild_count` (Number) A computed count, equal to 1 if the workspace was prebuilt.
- `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
11 changes: 11 additions & 0 deletions docs/data-sources/workspace_preset.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,17 @@ data "coder_workspace_preset" "example" {
- `name` (String) Name of the workspace preset.
- `parameters` (Map of String) Parameters of the workspace preset.

### Optional

- `prebuilds` (Block Set, Max: 1) Prebuilds of the workspace preset. (see [below for nested schema](#nestedblock--prebuilds))

### Read-Only

- `id` (String) ID of the workspace preset.

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

Required:

- `instances` (Number)
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 was prebuilt.",
},
"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: "Whether the workspace is a prebuild.",
},
"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

}
35 changes: 33 additions & 2 deletions provider/workspace_preset.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,13 @@ import (
)

type WorkspacePreset struct {
Name string `mapstructure:"name"`
Parameters map[string]string `mapstructure:"parameters"`
Name string `mapstructure:"name"`
Parameters map[string]string `mapstructure:"parameters"`
Prebuild []WorkspacePrebuild `mapstructure:"prebuilds"`
}

type WorkspacePrebuild struct {
Instances int `mapstructure:"instances"`
}

func workspacePresetDataSource() *schema.Resource {
Expand All @@ -24,9 +29,19 @@ func workspacePresetDataSource() *schema.Resource {
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)
Expand Down Expand Up @@ -65,6 +80,22 @@ func workspacePresetDataSource() *schema.Resource {
ValidateFunc: validation.StringIsNotEmpty,
},
},
"prebuilds": {
Type: schema.TypeSet,
Description: "Prebuilds of the workspace preset.",
Optional: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"instances": {
Type: schema.TypeInt,
Required: true,
ForceNew: true,
ValidateFunc: validation.IntAtLeast(0),
},
},
},
},
},
}
}
36 changes: 36 additions & 0 deletions provider/workspace_preset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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