Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

ethanndickson
Copy link
Member

@ethanndicksonethanndickson commentedJul 18, 2024
edited
Loading

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 oncoderd 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 oncoderd via Terraform, outside of simply deleting thetemplate 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 oncoderd. In reality, these versions listed in the resource are more likeparallel 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:

@ethanndicksonethanndicksonforce-pushed the07-17-chore_add_groups_to_integration_test branch from40c3e02 toc0065c3CompareJuly 18, 2024 13:26
@ethanndicksonethanndicksonforce-pushed the07-18-feat_add_coderd_template_resource branch from9d925c0 to534d0c9CompareJuly 18, 2024 13:26
@ethanndicksonethanndicksonforce-pushed the07-17-chore_add_groups_to_integration_test branch fromc0065c3 to3e749a6CompareJuly 18, 2024 13:50
@ethanndicksonethanndicksonforce-pushed the07-18-feat_add_coderd_template_resource branch from534d0c9 to3237486CompareJuly 18, 2024 13:50
@ethanndicksonethanndicksonforce-pushed the07-17-chore_add_groups_to_integration_test branch from3e749a6 to39670c5CompareJuly 19, 2024 12:40
@ethanndicksonethanndicksonforce-pushed the07-18-feat_add_coderd_template_resource branch from3237486 to914db7eCompareJuly 19, 2024 12:40
@ethanndicksonethanndicksonforce-pushed the07-17-chore_add_groups_to_integration_test branch from39670c5 to2fca026CompareJuly 19, 2024 12:43
@ethanndicksonethanndicksonforce-pushed the07-18-feat_add_coderd_template_resource branch from914db7e toeb0c94aCompareJuly 19, 2024 12:43
@ethanndicksonethanndicksonforce-pushed the07-17-chore_add_groups_to_integration_test branch from2fca026 to8222503CompareJuly 19, 2024 12:44
@ethanndicksonethanndicksonforce-pushed the07-18-feat_add_coderd_template_resource branch fromeb0c94a to5f8c0cfCompareJuly 19, 2024 12:45
@ethanndicksonethanndicksonforce-pushed the07-17-chore_add_groups_to_integration_test branch from8222503 to2632693CompareJuly 19, 2024 12:45
@ethanndicksonethanndicksonforce-pushed the07-18-feat_add_coderd_template_resource branch from5f8c0cf toa7601b2CompareJuly 19, 2024 12:45
@ethanndicksonethanndicksonforce-pushed the07-17-chore_add_groups_to_integration_test branch from2632693 tocae8c17CompareJuly 22, 2024 12:59
@ethanndicksonethanndicksonforce-pushed the07-18-feat_add_coderd_template_resource branch froma7601b2 to899e04eCompareJuly 22, 2024 12:59
@ethanndicksonethanndicksonforce-pushed the07-17-chore_add_groups_to_integration_test branch 2 times, most recently from92ecb54 tob35dec9CompareJuly 22, 2024 13:02
@ethanndicksonethanndicksonforce-pushed the07-18-feat_add_coderd_template_resource branch 2 times, most recently from2fa4521 to64ed234CompareJuly 22, 2024 13:02
@sreya
Copy link

I like nesting a template version within a template 👍

@ethanndicksonethanndicksonforce-pushed the07-18-feat_add_coderd_template_resource branch 3 times, most recently from587b480 to509c7c3CompareJuly 23, 2024 11:25
@ethanndicksonethanndickson changed the base branch from07-17-chore_add_groups_to_integration_test tomainJuly 23, 2024 11:25
@ethanndicksonethanndicksonforce-pushed the07-18-feat_add_coderd_template_resource branch from509c7c3 toa30132aCompareJuly 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
MemberAuthor

@ethanndicksonethanndicksonJul 24, 2024
edited
Loading

Choose a reason for hiding this comment

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

I'll getcoder/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 breakingcodersdk changes in main where the respectivecoderd 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
MemberAuthor

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 mockcodersdk.Client, but an alternative approach I've seen used in the wild would involve usinghttpmock 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 reacted with thumbs up emoji
@ethanndicksonethanndickson marked this pull request as ready for reviewJuly 24, 2024 07:44
@ethanndicksonethanndicksonforce-pushed the07-18-feat_add_coderd_template_resource branch froma30132a to37c1724CompareJuly 25, 2024 03:22
@ethanndicksonethanndicksonforce-pushed the07-18-feat_add_coderd_template_resource branch from37c1724 tobd5fbd1CompareJuly 25, 2024 05:27
@ethanndicksonethanndickson merged commit3e8547b intomainJul 25, 2024
13 checks passed
@deansheatherdeansheather deleted the 07-18-feat_add_coderd_template_resource branchJuly 26, 2024 07:14
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@johnstcnjohnstcnjohnstcn approved these changes

@coadlercoadlerAwaiting requested review from coadler

@deansheatherdeansheatherAwaiting requested review from deansheather

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for template resource
4 participants
@ethanndickson@sreya@johnstcn@deansheather

[8]ページ先頭

©2009-2025 Movatter.jp