Skip to content

Commit

Permalink
fix: template version replacement & metadata updates
Browse files Browse the repository at this point in the history
  • Loading branch information
@ethanndickson
ethanndickson committed Aug 1, 2024
1 parent 5c29d3c commit 6f8b09f
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 39 deletions.
2 changes: 1 addition & 1 deletion docs/resources/template.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ Optional:

- `active` (Boolean) Whether this version is the active version of the template. Only one version can be active at a time.
- `message` (String) A message describing the changes in this version of the template. Messages longer than 72 characters will be truncated.
- `name` (String) The name of the template version. Automatically generated if not provided.
- `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.
- `provisioner_tags` (Attributes Set) Provisioner tags for the template version. (see [below for nested schema](#nestedatt--versions--provisioner_tags))
- `tf_vars` (Attributes Set) Terraform variables for the template version. (see [below for nested schema](#nestedatt--versions--tf_vars))

Expand Down
138 changes: 103 additions & 35 deletions internal/provider/template_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package provider
import (
"bufio"
"context"
"encoding/json"
"fmt"
"io"

Expand Down Expand Up @@ -339,7 +340,7 @@ func (r *TemplateResource) Schema(ctx context.Context, req resource.SchemaReques
Computed: true,
},
"name": schema.StringAttribute{
MarkdownDescription: "The name of the template version. Automatically generated if not provided.",
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.",
Optional: true,
Computed: true,
},
Expand Down Expand Up @@ -495,6 +496,10 @@ func (r *TemplateResource) Create(ctx context.Context, req resource.CreateReques
data.ID = UUIDValue(templateResp.ID)
data.DisplayName = types.StringValue(templateResp.DisplayName)

resp.Diagnostics.Append(data.Versions.writePrivateState(func(key string, value []byte) diag.Diagnostics {
return resp.Private.SetKey(ctx, key, value)
})...)

// Save data into Terraform sutate
resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
}
Expand Down Expand Up @@ -562,11 +567,11 @@ func (r *TemplateResource) Read(ctx context.Context, req resource.ReadRequest, r
}

func (r *TemplateResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) {
var planState TemplateResourceModel
var newState TemplateResourceModel
var curState TemplateResourceModel

// Read Terraform plan data into the model
resp.Diagnostics.Append(req.Plan.Get(ctx, &planState)...)
resp.Diagnostics.Append(req.Plan.Get(ctx, &newState)...)

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

if planState.OrganizationID.IsUnknown() {
planState.OrganizationID = UUIDValue(r.data.DefaultOrganizationID)
if newState.OrganizationID.IsUnknown() {
newState.OrganizationID = UUIDValue(r.data.DefaultOrganizationID)
}

if planState.DisplayName.IsUnknown() {
planState.DisplayName = planState.Name
if newState.DisplayName.IsUnknown() {
newState.DisplayName = newState.Name
}

orgID := planState.OrganizationID.ValueUUID()
orgID := newState.OrganizationID.ValueUUID()

templateID := planState.ID.ValueUUID()
templateID := newState.ID.ValueUUID()

client := r.data.Client

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

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

for idx, plannedVersion := range planState.Versions {
var curVersionID uuid.UUID
// All versions in the state are guaranteed to have known IDs
foundVersion := curState.Versions.ByID(plannedVersion.ID)
// If the version is new, or if the directory hash has changed, create a new version
if foundVersion == nil || foundVersion.DirectoryHash != plannedVersion.DirectoryHash {
// Populate version IDs, based off what we created last `apply`
diags := readPrivateState(newState.Versions, func(key string) ([]byte, diag.Diagnostics) {
return req.Private.GetKey(ctx, key)
})
if diags.HasError() {
resp.Diagnostics.Append(diags...)
return
}
for idx := range newState.Versions {
if newState.Versions[idx].ID.IsUnknown() {
tflog.Trace(ctx, "discovered a new or modified template version")
versionResp, err := newVersion(ctx, client, newVersionRequest{
Version: &plannedVersion,
uploadResp, err := newVersion(ctx, client, newVersionRequest{
Version: &newState.Versions[idx],
OrganizationID: orgID,
TemplateID: &templateID,
})
if err != nil {
resp.Diagnostics.AddError("Client Error", err.Error())
return
}
curVersionID = versionResp.ID
versionResp, err := client.TemplateVersion(ctx, uploadResp.ID)
if err != nil {
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to get template version: %s", err))
return
}
newState.Versions[idx].ID = UUIDValue(versionResp.ID)
} else {
// Or if it's an existing version, get the ID
curVersionID = plannedVersion.ID.ValueUUID()
}
versionResp, err := client.TemplateVersion(ctx, curVersionID)
if err != nil {
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to get template version: %s", err))
return
_, err := client.UpdateTemplateVersion(ctx, newState.Versions[idx].ID.ValueUUID(), codersdk.PatchTemplateVersionRequest{
Name: newState.Versions[idx].Name.ValueString(),
Message: newState.Versions[idx].Message.ValueStringPointer(),
})
if err != nil {
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to update template version metadata: %s", err))
return
}
}
if plannedVersion.Active.ValueBool() {
if newState.Versions[idx].Active.ValueBool() {
tflog.Trace(ctx, "marking template version as active", map[string]any{
"version_id": versionResp.ID,
"template_id": templateID,
"version_id": newState.Versions[idx].ID.ValueString(),
"template_id": templateID.String(),
})
err := client.UpdateActiveTemplateVersion(ctx, templateID, codersdk.UpdateActiveTemplateVersion{
ID: versionResp.ID,
ID: newState.Versions[idx].ID.ValueUUID(),
})
if err != nil {
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to update active template version: %s", err))
return
}
tflog.Trace(ctx, "marked template version as active")
}
planState.Versions[idx].ID = UUIDValue(versionResp.ID)
}

resp.Diagnostics.Append(newState.Versions.writePrivateState(func(key string, value []byte) diag.Diagnostics {
return resp.Private.SetKey(ctx, key, value)
})...)

// Save updated data into Terraform state
resp.Diagnostics.Append(resp.State.Set(ctx, &planState)...)
resp.Diagnostics.Append(resp.State.Set(ctx, &newState)...)
}

func (r *TemplateResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {
Expand Down Expand Up @@ -1053,3 +1071,53 @@ func (r *TemplateResourceModel) toCreateRequest(ctx context.Context, resp *resou
DisableEveryoneGroupAccess: !r.ACL.IsNull(),
}
}

type PreviousTemplateVersion struct {
ID uuid.UUID `json:"id"`
Name string `json:"name"`
}

func (v Versions) writePrivateState(privateStateWriter func(key string, value []byte) diag.Diagnostics) (diags diag.Diagnostics) {
for _, version := range v {
prevBytes, err := json.Marshal(PreviousTemplateVersion{ID: version.ID.ValueUUID(), Name: version.Name.ValueString()})
if err != nil {
diags.AddError("Client Error", fmt.Sprintf("Failed to marshal name to json bytes: %s", err))
return diags
}
diag := privateStateWriter(version.DirectoryHash.ValueString(), prevBytes)
if diag.HasError() {
return diag
}
}
return diags
}

func readPrivateState(v Versions, privateStateReader func(key string) ([]byte, diag.Diagnostics)) (diags diag.Diagnostics) {
for idx, version := range v {
jsonBytes, diag := privateStateReader(version.DirectoryHash.ValueString())
if diag.HasError() {
return diag
}
// If not in state, create it
if jsonBytes == nil {
continue
}
var prev PreviousTemplateVersion
err := json.Unmarshal(jsonBytes, &prev)
if err != nil {
diags.AddError("Client Error", fmt.Sprintf("Failed to unmarshal name from json bytes: %s", err))
return diags
}
// If in the state, but with a different name, create it
if prev.Name != version.Name.ValueString() {
continue
}
// If in the state, but with no name, create it
if prev.Name == "" {
continue
}
// Otherwise, use the ID from last time
v[idx].ID = UUIDValue(prev.ID)
}
return
}
22 changes: 19 additions & 3 deletions internal/provider/template_resource_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func TestAccTemplateResource(t *testing.T) {
Name: PtrTo("example-template"),
Versions: []testAccTemplateVersionConfig{
{
Name: PtrTo("main"),
// Auto-generated version name
Directory: PtrTo("../../integration/template-test/example-template/"),
Active: PtrTo(true),
// TODO(ethanndickson): Remove this when we add in `*.tfvars` parsing
Expand Down Expand Up @@ -123,8 +123,8 @@ func TestAccTemplateResource(t *testing.T) {
resource.TestCheckResourceAttr("coderd_template.test", "time_til_dormant_autodelete", "0"),
resource.TestCheckResourceAttr("coderd_template.test", "require_active_version", "false"),
resource.TestMatchTypeSetElemNestedAttrs("coderd_template.test", "versions.*", map[string]*regexp.Regexp{
"name": regexp.MustCompile("main"),
"id": regexp.MustCompile(".*"),
"name": regexp.MustCompile(".+"),
"id": regexp.MustCompile(".+"),
"directory_hash": regexp.MustCompile(".+"),
"message": regexp.MustCompile(""),
}),
Expand All @@ -137,6 +137,7 @@ func TestAccTemplateResource(t *testing.T) {
ImportState: true,
ImportStateVerify: true,
// In the real world, `versions` needs to be added to the configuration after importing
// We can't import ACL as we can't currently differentiate between managed and unmanaged ACL
ImportStateVerifyIgnore: []string{"versions", "acl"},
},
// Update existing version & metadata
Expand Down Expand Up @@ -214,6 +215,21 @@ func TestAccTemplateResource(t *testing.T) {
resource.TestCheckNoResourceAttr("coderd_template.test", "acl"),
),
},
// Check orphaned versions
{
Config: cfg7.String(t),
PreConfig: func() {
templates, err := client.Templates(ctx)
require.NoError(t, err)
require.Len(t, templates, 1)
versions, err := client.TemplateVersionsByTemplate(ctx, codersdk.TemplateVersionsByTemplateRequest{
TemplateID: templates[0].ID,
})
require.NoError(t, err)
require.Len(t, versions, 3)
},
},
// Resource deleted
},
})
}
Expand Down

0 comments on commit 6f8b09f

Please sign in to comment.