fix: template version replacement & metadata updates

Author: ethanndickson

docs/resources/template.md

@@ -55,7 +55,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))
 

internal/provider/template_resource.go

@@ -3,6 +3,7 @@ package provider
 import (
 	"bufio"
 	"context"
+	"encoding/json"
 	"fmt"
 	"io"
 
@@ -346,7 +347,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,
 						},
@@ -502,6 +503,17 @@ func (r *TemplateResource) Create(ctx context.Context, req resource.CreateReques
 	data.ID = UUIDValue(templateResp.ID)
 	data.DisplayName = types.StringValue(templateResp.DisplayName)
 
+	// We have to init the private state again since the PlanModifyObject private
+	// state is not accessible in Create
+	resp.Diagnostics.Append(setEmptyPrivateState(ctx, resp.Private)...)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+	resp.Diagnostics.Append(data.Versions.writePrivateState(ctx, resp.Private)...)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
 	// Save data into Terraform sutate
 	resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
 }
@@ -569,11 +581,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
@@ -585,25 +597,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
 		}
@@ -618,9 +630,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
 		}
@@ -632,51 +644,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 {
+	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)
+			newState.Versions[idx].Name = types.StringValue(versionResp.Name)
 		} 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
+			versionResp, 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 the name was not provided on an update we set it to the patch result, which is the previous name.
+			// There's no way to go back to an auto-generated name unless the template version files itself change.
+			newState.Versions[idx].Name = types.StringValue(versionResp.Name)
 		}
-		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)
+	}
+
+	// We only want the previous apply in the state at any given time
+	resp.Diagnostics.Append(setEmptyPrivateState(ctx, resp.Private)...)
+
+	resp.Diagnostics.Append(newState.Versions.writePrivateState(ctx, resp.Private)...)
+	if resp.Diagnostics.HasError() {
+		return
 	}
 
 	// 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) {
@@ -766,25 +791,26 @@ func (d *directoryHashPlanModifier) MarkdownDescription(context.Context) string
 
 // PlanModifyObject implements planmodifier.Object.
 func (d *directoryHashPlanModifier) PlanModifyObject(ctx context.Context, req planmodifier.ObjectRequest, resp *planmodifier.ObjectResponse) {
-	attributes := req.PlanValue.Attributes()
-	directory, ok := attributes["directory"].(types.String)
-	if !ok {
-		resp.Diagnostics.AddError("Client Error", fmt.Sprintf("unexpected type for directory, got: %T", directory))
+	var data TemplateVersion
+	resp.Diagnostics.Append(req.PlanValue.As(ctx, &data, basetypes.ObjectAsOptions{})...)
+	if resp.Diagnostics.HasError() {
 		return
 	}
 
-	hash, err := computeDirectoryHash(directory.ValueString())
+	hash, err := computeDirectoryHash(data.Directory.ValueString())
 	if err != nil {
 		resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to compute directory hash: %s", err))
 		return
 	}
-	attributes["directory_hash"] = types.StringValue(hash)
-	out, diag := types.ObjectValue(req.PlanValue.AttributeTypes(ctx), attributes)
-	if diag.HasError() {
-		resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to create plan object: %s", diag))
+
+	data.DirectoryHash = types.StringValue(hash)
+	// Populate version IDs or mark them as unknown if the hash has changed
+	resp.Diagnostics.Append(data.readFromPrivateState(ctx, req.Private)...)
+	if resp.Diagnostics.HasError() {
 		return
 	}
-	resp.PlanValue = out
+
+	resp.PlanValue, resp.Diagnostics = types.ObjectValueFrom(ctx, req.PlanValue.AttributeTypes(ctx), data)
 }
 
 func NewDirectoryHashPlanModifier() planmodifier.Object {
@@ -1062,3 +1088,95 @@ func (r *TemplateResourceModel) toCreateRequest(ctx context.Context, resp *resou
 		DisableEveryoneGroupAccess:     !r.ACL.IsNull(),
 	}
 }
+
+type LastVersionsByHash map[string]PreviousTemplateVersion
+
+func (lv LastVersionsByHash) MarshalJSON() ([]byte, error) {
+	return json.Marshal(map[string]PreviousTemplateVersion(lv))
+}
+
+func (lv *LastVersionsByHash) UnmarshalJSON(data []byte) error {
+	var m map[string]PreviousTemplateVersion
+	err := json.Unmarshal(data, &m)
+	if err != nil {
+		return err
+	}
+	*lv = LastVersionsByHash(m)
+	return nil
+}
+
+var LastVersionsKey = "last_versions"
+
+type PreviousTemplateVersion struct {
+	ID   uuid.UUID `json:"id"`
+	Name string    `json:"name"`
+}
+
+type privateState interface {
+	GetKey(ctx context.Context, key string) ([]byte, diag.Diagnostics)
+	SetKey(ctx context.Context, key string, value []byte) diag.Diagnostics
+}
+
+func (v Versions) writePrivateState(ctx context.Context, ps privateState) (diags diag.Diagnostics) {
+	var lv LastVersionsByHash
+	lvBytes, diag := ps.GetKey(ctx, LastVersionsKey)
+	if diag.HasError() {
+		return diag
+	}
+	err := lv.UnmarshalJSON(lvBytes)
+	if err != nil {
+		diags.AddError("Client Error", fmt.Sprintf("Failed to unmarshal private state when writing: %s", err))
+		return diags
+	}
+	for _, version := range v {
+		lv[version.DirectoryHash.ValueString()] = PreviousTemplateVersion{
+			ID:   version.ID.ValueUUID(),
+			Name: version.ID.ValueString(),
+		}
+		lvBytes, err = lv.MarshalJSON()
+		if err != nil {
+			diags.AddError("Client Error", fmt.Sprintf("Failed to marshal private state: %s", err))
+			return diags
+		}
+	}
+	return ps.SetKey(ctx, LastVersionsKey, lvBytes)
+}
+
+func (v *TemplateVersion) readFromPrivateState(ctx context.Context, ps privateState) (diags diag.Diagnostics) {
+	var lv LastVersionsByHash
+	lvBytes, diag := ps.GetKey(ctx, LastVersionsKey)
+	if diag.HasError() {
+		diags.Append(diag...)
+		return
+	}
+	// If this is the first read, init the private state value
+	if lvBytes == nil {
+		setEmptyPrivateState(ctx, ps)
+		return
+	}
+	err := lv.UnmarshalJSON(lvBytes)
+	if err != nil {
+		diags.AddError("Client Error", fmt.Sprintf("Failed to unmarshal private state when reading: %s", err))
+		return
+	}
+
+	prev, ok := lv[v.DirectoryHash.ValueString()]
+	// If not in state, mark as known after apply since we'll create a new version.
+	// Versions who's Terraform configuration has not changed will have known
+	// IDs at this point, so we need to set this manually.
+	if !ok {
+		v.ID = NewUUIDUnknown()
+		return
+	}
+	// Otherwise, use the existing ID for this hash
+	v.ID = UUIDValue(prev.ID)
+	return
+}
+
+func setEmptyPrivateState(ctx context.Context, ps privateState) (diags diag.Diagnostics) {
+	pvBytes, err := make(LastVersionsByHash).MarshalJSON()
+	if err != nil {
+		panic("failed to marshal empty private state")
+	}
+	return ps.SetKey(ctx, LastVersionsKey, pvBytes)
+}