-
Notifications
You must be signed in to change notification settings - Fork 2
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
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @ethanndickson and the rest of your teammates on |
40c3e02
to
c0065c3
Compare
9d925c0
to
534d0c9
Compare
c0065c3
to
3e749a6
Compare
534d0c9
to
3237486
Compare
3e749a6
to
39670c5
Compare
3237486
to
914db7e
Compare
39670c5
to
2fca026
Compare
914db7e
to
eb0c94a
Compare
2fca026
to
8222503
Compare
eb0c94a
to
5f8c0cf
Compare
8222503
to
2632693
Compare
5f8c0cf
to
a7601b2
Compare
2632693
to
cae8c17
Compare
a7601b2
to
899e04e
Compare
92ecb54
to
b35dec9
Compare
2fa4521
to
64ed234
Compare
I like nesting a template version within a template 👍 |
587b480
to
509c7c3
Compare
509c7c3
to
a30132a
Compare
// varFiles, err := codersdk.DiscoverVarsFiles(directory) | ||
// if err != nil { | ||
// return nil, fmt.Errorf("failed to discover vars files: %s", err) | ||
// } |
There was a problem hiding this comment.
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/"), |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
a30132a
to
37c1724
Compare
37c1724
to
bd5fbd1
Compare
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:
The Terraform internal logic would then be:
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:
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 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 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: