Skip to content

feat: add coderd_template resource #35

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 1 commit into from
Jul 25, 2024

Conversation

ethanndickson
Copy link
Member

@ethanndickson ethanndickson commented Jul 18, 2024

Design Rationale: The RFC previously proposed defining templates and template versions separately. Multiple template version resources for a template could exist, only one of them needed to be marked as the active version (by ID) on the template resource.

This could introduce a circular dependency between the two, as template versions require the ID of a template at creation (it cannot be added later), and templates must know the IDs of all template versions associated with it. To fix this, only the template version would point at the template, and not the other way around.
For example:

resource "coderd_template" "debug" {
  name = "debug"
  [....]
  acl = {
     [....]
  }
}
resource "coderd_template_version" "v1" {
  name = data.git_branch.main.sha
  directory = "./dogfood"
  template_id = resource.coderd_template.debug.id
  active = true
}

The Terraform internal logic would then be:

  1. Identify the template resource as a dependency of a template version.
  2. Create a template resource with no associated version
  3. Create a template version resource with the ID of the template resource. During this creation, modify the template on coderd to include the template version ID. If the template version is marked as active, set it as the active version.

This updating of templates from template version resources feels a bit janky, or at least not Terraform idiomatic.

I therefore propose that the schema for creating templates using Terraform should instead look like:

resource "coderd_template" "dogfood" {
  name = "dogfood"
  versions = [
    {
      name      = "dogfood-old"
      directory = "./dogfood-old"
    },
    {
      name      = "dogfood-new"
      directory = "./dogfood"
      active    = true
    }
  ]
}

This means we'd just do the third step in one resource creation task.

In a Netgru meeting, we decided we'd prioritise uploading template versions by having users supply a path to their directory. It's then pretty trivial to just have Terraform compute a hash of all the contents during planning, and use that to determine whether or not the template version needs updating.
However, we can't update the contents of a template version, we can only create a new version.
This means internally, a terraform representation of a template resource would only represent the state of the latest version, and the previously created versions would be effectively 'orphaned', with no way to modify their state on coderd via Terraform, outside of simply deleting the template resource. I don't believe we can avoid this orphaning of versions, though I'm open to suggestions.

This feels like the only surprising part of the schema; that each version in the versions list doesn't actually represent a single version on coderd. In reality, these versions listed in the resource are more like parallel branches of the template, and what's written in Terraform is merely the head of each branch, and where one of those branches can be marked as 'active'.

(We can also just not support multiple template versions through Terraform as in the above snippet, and instead just have a template resource be given a single directory, the implementation is basically the same either way.)

I think this is our best option, but I'm keen to hear thoughts. With this, template creation is still idempotent, and we don't have any circular dependency issues.

Closes #29 and #30.

Todo:

@ethanndickson ethanndickson force-pushed the 07-17-chore_add_groups_to_integration_test branch from 40c3e02 to c0065c3 Compare July 18, 2024 13:26
@ethanndickson ethanndickson force-pushed the 07-18-feat_add_coderd_template_resource branch from 9d925c0 to 534d0c9 Compare July 18, 2024 13:26
@ethanndickson ethanndickson force-pushed the 07-17-chore_add_groups_to_integration_test branch from c0065c3 to 3e749a6 Compare July 18, 2024 13:50
@ethanndickson ethanndickson force-pushed the 07-18-feat_add_coderd_template_resource branch from 534d0c9 to 3237486 Compare July 18, 2024 13:50
@ethanndickson ethanndickson force-pushed the 07-17-chore_add_groups_to_integration_test branch from 3e749a6 to 39670c5 Compare July 19, 2024 12:40
@ethanndickson ethanndickson force-pushed the 07-18-feat_add_coderd_template_resource branch from 3237486 to 914db7e Compare July 19, 2024 12:40
@ethanndickson ethanndickson force-pushed the 07-17-chore_add_groups_to_integration_test branch from 39670c5 to 2fca026 Compare July 19, 2024 12:43
@ethanndickson ethanndickson force-pushed the 07-18-feat_add_coderd_template_resource branch from 914db7e to eb0c94a Compare July 19, 2024 12:43
@ethanndickson ethanndickson force-pushed the 07-17-chore_add_groups_to_integration_test branch from 2fca026 to 8222503 Compare July 19, 2024 12:44
@ethanndickson ethanndickson force-pushed the 07-18-feat_add_coderd_template_resource branch from eb0c94a to 5f8c0cf Compare July 19, 2024 12:45
@ethanndickson ethanndickson force-pushed the 07-17-chore_add_groups_to_integration_test branch from 8222503 to 2632693 Compare July 19, 2024 12:45
@ethanndickson ethanndickson force-pushed the 07-18-feat_add_coderd_template_resource branch from 5f8c0cf to a7601b2 Compare July 19, 2024 12:45
@ethanndickson ethanndickson force-pushed the 07-17-chore_add_groups_to_integration_test branch from 2632693 to cae8c17 Compare July 22, 2024 12:59
@ethanndickson ethanndickson force-pushed the 07-18-feat_add_coderd_template_resource branch from a7601b2 to 899e04e Compare July 22, 2024 12:59
@ethanndickson ethanndickson force-pushed the 07-17-chore_add_groups_to_integration_test branch 2 times, most recently from 92ecb54 to b35dec9 Compare July 22, 2024 13:02
@ethanndickson ethanndickson force-pushed the 07-18-feat_add_coderd_template_resource branch 2 times, most recently from 2fa4521 to 64ed234 Compare July 22, 2024 13:02
@sreya
Copy link

sreya commented Jul 22, 2024

I like nesting a template version within a template 👍

@ethanndickson ethanndickson force-pushed the 07-18-feat_add_coderd_template_resource branch 3 times, most recently from 587b480 to 509c7c3 Compare July 23, 2024 11:25
@ethanndickson ethanndickson changed the base branch from 07-17-chore_add_groups_to_integration_test to main July 23, 2024 11:25
@ethanndickson ethanndickson force-pushed the 07-18-feat_add_coderd_template_resource branch from 509c7c3 to a30132a Compare July 24, 2024 07:30
// varFiles, err := codersdk.DiscoverVarsFiles(directory)
// if err != nil {
// return nil, fmt.Errorf("failed to discover vars files: %s", err)
// }
Copy link
Member Author

@ethanndickson ethanndickson Jul 24, 2024

Choose a reason for hiding this comment

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

I'll get coder/coder#13984 cherry-picked into the next release, and then we can add this.
We can't use a go mod replace because there's breaking codersdk changes in main where the respective coderd changes aren't in the docker image we use for tests.

Versions: []testAccTemplateVersionConfig{
{
Name: PtrTo("main"),
Directory: PtrTo("../../integration/template-test/example-template/"),
Copy link
Member Author

Choose a reason for hiding this comment

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

Open to suggestions on better ways to do these acceptance tests.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure it's possible to pass in a mock codersdk.Client, but an alternative approach I've seen used in the wild would involve using httpmock in conjunction with responses as fixtures (example).

This has the benefit of not crossing the 'streams' between the acceptance and integration tests. But I think the current approach is OK for now.

@ethanndickson ethanndickson marked this pull request as ready for review July 24, 2024 07:44
@ethanndickson ethanndickson force-pushed the 07-18-feat_add_coderd_template_resource branch from a30132a to 37c1724 Compare July 25, 2024 03:22
@ethanndickson ethanndickson force-pushed the 07-18-feat_add_coderd_template_resource branch from 37c1724 to bd5fbd1 Compare July 25, 2024 05:27
@ethanndickson ethanndickson merged commit 3e8547b into main Jul 25, 2024
13 checks passed
@deansheather deansheather deleted the 07-18-feat_add_coderd_template_resource branch July 26, 2024 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for template resource
4 participants