Skip to content

Commit 6f8b09f

Browse files
ethanndicksonethanndickson
committed
fix: template version replacement & metadata updates
1 parent 5c29d3c commit 6f8b09f

File tree

3 files changed

+123
-39
lines changed

3 files changed

+123
-39
lines changed

‎docs/resources/template.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ Optional:
5454

5555
- `active` (Boolean) Whether this version is the active version of the template. Only one version can be active at a time.
5656
- `message` (String) A message describing the changes in this version of the template. Messages longer than 72 characters will be truncated.
57-
- `name` (String) The name of the template version. Automatically generated if not provided.
57+
- `name` (String) The name of the template version. Automatically generated if not provided. If provided, the name *must* change each time the directory contents are updated.
5858
- `provisioner_tags` (Attributes Set) Provisioner tags for the template version. (see [below for nested schema](#nestedatt--versions--provisioner_tags))
5959
- `tf_vars` (Attributes Set) Terraform variables for the template version. (see [below for nested schema](#nestedatt--versions--tf_vars))
6060

‎internal/provider/template_resource.go

+103-35
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package provider
33
import (
44
"bufio"
55
"context"
6+
"encoding/json"
67
"fmt"
78
"io"
89

@@ -339,7 +340,7 @@ func (r *TemplateResource) Schema(ctx context.Context, req resource.SchemaReques
339340
Computed: true,
340341
},
341342
"name": schema.StringAttribute{
342-
MarkdownDescription: "The name of the template version. Automatically generated if not provided.",
343+
MarkdownDescription: "The name of the template version. Automatically generated if not provided. If provided, the name *must* change each time the directory contents are updated.",
343344
Optional: true,
344345
Computed: true,
345346
},
@@ -495,6 +496,10 @@ func (r *TemplateResource) Create(ctx context.Context, req resource.CreateReques
495496
data.ID = UUIDValue(templateResp.ID)
496497
data.DisplayName = types.StringValue(templateResp.DisplayName)
497498

499+
resp.Diagnostics.Append(data.Versions.writePrivateState(func(key string, value []byte) diag.Diagnostics {
500+
return resp.Private.SetKey(ctx, key, value)
501+
})...)
502+
498503
// Save data into Terraform sutate
499504
resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
500505
}
@@ -562,11 +567,11 @@ func (r *TemplateResource) Read(ctx context.Context, req resource.ReadRequest, r
562567
}
563568

564569
func (r *TemplateResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) {
565-
var planState TemplateResourceModel
570+
var newState TemplateResourceModel
566571
var curState TemplateResourceModel
567572

568573
// Read Terraform plan data into the model
569-
resp.Diagnostics.Append(req.Plan.Get(ctx, &planState)...)
574+
resp.Diagnostics.Append(req.Plan.Get(ctx, &newState)...)
570575

571576
if resp.Diagnostics.HasError() {
572577
return
@@ -578,25 +583,25 @@ func (r *TemplateResource) Update(ctx context.Context, req resource.UpdateReques
578583
return
579584
}
580585

581-
if planState.OrganizationID.IsUnknown() {
582-
planState.OrganizationID = UUIDValue(r.data.DefaultOrganizationID)
586+
if newState.OrganizationID.IsUnknown() {
587+
newState.OrganizationID = UUIDValue(r.data.DefaultOrganizationID)
583588
}
584589

585-
if planState.DisplayName.IsUnknown() {
586-
planState.DisplayName = planState.Name
590+
if newState.DisplayName.IsUnknown() {
591+
newState.DisplayName = newState.Name
587592
}
588593

589-
orgID := planState.OrganizationID.ValueUUID()
594+
orgID := newState.OrganizationID.ValueUUID()
590595

591-
templateID := planState.ID.ValueUUID()
596+
templateID := newState.ID.ValueUUID()
592597

593598
client := r.data.Client
594599

595-
templateMetadataChanged := !planState.EqualTemplateMetadata(curState)
600+
templateMetadataChanged := !newState.EqualTemplateMetadata(curState)
596601
// This is required, as the API will reject no-diff updates.
597602
if templateMetadataChanged {
598603
tflog.Trace(ctx, "change in template metadata detected, updating.")
599-
updateReq := planState.toUpdateRequest(ctx, resp)
604+
updateReq := newState.toUpdateRequest(ctx, resp)
600605
if resp.Diagnostics.HasError() {
601606
return
602607
}
@@ -611,9 +616,9 @@ func (r *TemplateResource) Update(ctx context.Context, req resource.UpdateReques
611616

612617
// Since the everyone group always gets deleted by `DisableEveryoneGroupAccess`, we need to run this even if there
613618
// were no ACL changes but the template metadata was updated.
614-
if !planState.ACL.IsNull() && (!curState.ACL.Equal(planState.ACL) || templateMetadataChanged) {
619+
if !newState.ACL.IsNull() && (!curState.ACL.Equal(newState.ACL) || templateMetadataChanged) {
615620
var acl ACL
616-
resp.Diagnostics.Append(planState.ACL.As(ctx, &acl, basetypes.ObjectAsOptions{})...)
621+
resp.Diagnostics.Append(newState.ACL.As(ctx, &acl, basetypes.ObjectAsOptions{})...)
617622
if resp.Diagnostics.HasError() {
618623
return
619624
}
@@ -625,51 +630,64 @@ func (r *TemplateResource) Update(ctx context.Context, req resource.UpdateReques
625630
tflog.Trace(ctx, "successfully updated template ACL")
626631
}
627632

628-
for idx, plannedVersion := range planState.Versions {
629-
var curVersionID uuid.UUID
630-
// All versions in the state are guaranteed to have known IDs
631-
foundVersion := curState.Versions.ByID(plannedVersion.ID)
632-
// If the version is new, or if the directory hash has changed, create a new version
633-
if foundVersion == nil || foundVersion.DirectoryHash != plannedVersion.DirectoryHash {
633+
// Populate version IDs, based off what we created last `apply`
634+
diags := readPrivateState(newState.Versions, func(key string) ([]byte, diag.Diagnostics) {
635+
return req.Private.GetKey(ctx, key)
636+
})
637+
if diags.HasError() {
638+
resp.Diagnostics.Append(diags...)
639+
return
640+
}
641+
for idx := range newState.Versions {
642+
if newState.Versions[idx].ID.IsUnknown() {
634643
tflog.Trace(ctx, "discovered a new or modified template version")
635-
versionResp, err := newVersion(ctx, client, newVersionRequest{
636-
Version: &plannedVersion,
644+
uploadResp, err := newVersion(ctx, client, newVersionRequest{
645+
Version: &newState.Versions[idx],
637646
OrganizationID: orgID,
638647
TemplateID: &templateID,
639648
})
640649
if err != nil {
641650
resp.Diagnostics.AddError("Client Error", err.Error())
642651
return
643652
}
644-
curVersionID = versionResp.ID
653+
versionResp, err := client.TemplateVersion(ctx, uploadResp.ID)
654+
if err != nil {
655+
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to get template version: %s", err))
656+
return
657+
}
658+
newState.Versions[idx].ID = UUIDValue(versionResp.ID)
645659
} else {
646-
// Or if it's an existing version, get the ID
647-
curVersionID = plannedVersion.ID.ValueUUID()
648-
}
649-
versionResp, err := client.TemplateVersion(ctx, curVersionID)
650-
if err != nil {
651-
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to get template version: %s", err))
652-
return
660+
_, err := client.UpdateTemplateVersion(ctx, newState.Versions[idx].ID.ValueUUID(), codersdk.PatchTemplateVersionRequest{
661+
Name: newState.Versions[idx].Name.ValueString(),
662+
Message: newState.Versions[idx].Message.ValueStringPointer(),
663+
})
664+
if err != nil {
665+
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to update template version metadata: %s", err))
666+
return
667+
}
653668
}
654-
if plannedVersion.Active.ValueBool() {
669+
if newState.Versions[idx].Active.ValueBool() {
655670
tflog.Trace(ctx, "marking template version as active", map[string]any{
656-
"version_id": versionResp.ID,
657-
"template_id": templateID,
671+
"version_id": newState.Versions[idx].ID.ValueString(),
672+
"template_id": templateID.String(),
658673
})
659674
err := client.UpdateActiveTemplateVersion(ctx, templateID, codersdk.UpdateActiveTemplateVersion{
660-
ID: versionResp.ID,
675+
ID: newState.Versions[idx].ID.ValueUUID(),
661676
})
662677
if err != nil {
663678
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to update active template version: %s", err))
664679
return
665680
}
666681
tflog.Trace(ctx, "marked template version as active")
667682
}
668-
planState.Versions[idx].ID = UUIDValue(versionResp.ID)
669683
}
670684

685+
resp.Diagnostics.Append(newState.Versions.writePrivateState(func(key string, value []byte) diag.Diagnostics {
686+
return resp.Private.SetKey(ctx, key, value)
687+
})...)
688+
671689
// Save updated data into Terraform state
672-
resp.Diagnostics.Append(resp.State.Set(ctx, &planState)...)
690+
resp.Diagnostics.Append(resp.State.Set(ctx, &newState)...)
673691
}
674692

675693
func (r *TemplateResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {
@@ -1053,3 +1071,53 @@ func (r *TemplateResourceModel) toCreateRequest(ctx context.Context, resp *resou
10531071
DisableEveryoneGroupAccess: !r.ACL.IsNull(),
10541072
}
10551073
}
1074+
1075+
type PreviousTemplateVersion struct {
1076+
ID uuid.UUID `json:"id"`
1077+
Name string `json:"name"`
1078+
}
1079+
1080+
func (v Versions) writePrivateState(privateStateWriter func(key string, value []byte) diag.Diagnostics) (diags diag.Diagnostics) {
1081+
for _, version := range v {
1082+
prevBytes, err := json.Marshal(PreviousTemplateVersion{ID: version.ID.ValueUUID(), Name: version.Name.ValueString()})
1083+
if err != nil {
1084+
diags.AddError("Client Error", fmt.Sprintf("Failed to marshal name to json bytes: %s", err))
1085+
return diags
1086+
}
1087+
diag := privateStateWriter(version.DirectoryHash.ValueString(), prevBytes)
1088+
if diag.HasError() {
1089+
return diag
1090+
}
1091+
}
1092+
return diags
1093+
}
1094+
1095+
func readPrivateState(v Versions, privateStateReader func(key string) ([]byte, diag.Diagnostics)) (diags diag.Diagnostics) {
1096+
for idx, version := range v {
1097+
jsonBytes, diag := privateStateReader(version.DirectoryHash.ValueString())
1098+
if diag.HasError() {
1099+
return diag
1100+
}
1101+
// If not in state, create it
1102+
if jsonBytes == nil {
1103+
continue
1104+
}
1105+
var prev PreviousTemplateVersion
1106+
err := json.Unmarshal(jsonBytes, &prev)
1107+
if err != nil {
1108+
diags.AddError("Client Error", fmt.Sprintf("Failed to unmarshal name from json bytes: %s", err))
1109+
return diags
1110+
}
1111+
// If in the state, but with a different name, create it
1112+
if prev.Name != version.Name.ValueString() {
1113+
continue
1114+
}
1115+
// If in the state, but with no name, create it
1116+
if prev.Name == "" {
1117+
continue
1118+
}
1119+
// Otherwise, use the ID from last time
1120+
v[idx].ID = UUIDValue(prev.ID)
1121+
}
1122+
return
1123+
}

‎internal/provider/template_resource_test.go

+19-3
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ func TestAccTemplateResource(t *testing.T) {
2929
Name: PtrTo("example-template"),
3030
Versions: []testAccTemplateVersionConfig{
3131
{
32-
Name: PtrTo("main"),
32+
// Auto-generated version name
3333
Directory: PtrTo("../../integration/template-test/example-template/"),
3434
Active: PtrTo(true),
3535
// TODO(ethanndickson): Remove this when we add in `*.tfvars` parsing
@@ -123,8 +123,8 @@ func TestAccTemplateResource(t *testing.T) {
123123
resource.TestCheckResourceAttr("coderd_template.test", "time_til_dormant_autodelete", "0"),
124124
resource.TestCheckResourceAttr("coderd_template.test", "require_active_version", "false"),
125125
resource.TestMatchTypeSetElemNestedAttrs("coderd_template.test", "versions.*", map[string]*regexp.Regexp{
126-
"name": regexp.MustCompile("main"),
127-
"id": regexp.MustCompile(".*"),
126+
"name": regexp.MustCompile(".+"),
127+
"id": regexp.MustCompile(".+"),
128128
"directory_hash": regexp.MustCompile(".+"),
129129
"message": regexp.MustCompile(""),
130130
}),
@@ -137,6 +137,7 @@ func TestAccTemplateResource(t *testing.T) {
137137
ImportState: true,
138138
ImportStateVerify: true,
139139
// In the real world, `versions` needs to be added to the configuration after importing
140+
// We can't import ACL as we can't currently differentiate between managed and unmanaged ACL
140141
ImportStateVerifyIgnore: []string{"versions", "acl"},
141142
},
142143
// Update existing version & metadata
@@ -214,6 +215,21 @@ func TestAccTemplateResource(t *testing.T) {
214215
resource.TestCheckNoResourceAttr("coderd_template.test", "acl"),
215216
),
216217
},
218+
// Check orphaned versions
219+
{
220+
Config: cfg7.String(t),
221+
PreConfig: func() {
222+
templates, err := client.Templates(ctx)
223+
require.NoError(t, err)
224+
require.Len(t, templates, 1)
225+
versions, err := client.TemplateVersionsByTemplate(ctx, codersdk.TemplateVersionsByTemplateRequest{
226+
TemplateID: templates[0].ID,
227+
})
228+
require.NoError(t, err)
229+
require.Len(t, versions, 3)
230+
},
231+
},
232+
// Resource deleted
217233
},
218234
})
219235
}

0 commit comments

Comments
 (0)