diff --git a/docs/resources/template.md b/docs/resources/template.md index d7d47fa..f30a0d9 100644 --- a/docs/resources/template.md +++ b/docs/resources/template.md @@ -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)) diff --git a/internal/provider/template_resource.go b/internal/provider/template_resource.go index df5114a..eb864e6 100644 --- a/internal/provider/template_resource.go +++ b/internal/provider/template_resource.go @@ -3,6 +3,7 @@ package provider import ( "bufio" "context" + "encoding/json" "fmt" "io" @@ -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, }, @@ -495,6 +496,11 @@ 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(ctx, resp.Private)...) + if resp.Diagnostics.HasError() { + return + } + // Save data into Terraform sutate resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) } @@ -562,11 +568,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 @@ -578,25 +584,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 } @@ -611,9 +617,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 } @@ -625,15 +631,16 @@ 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 previously created template versions stored in private state. + resp.Diagnostics.Append(readPrivateState(ctx, newState.Versions, req.Private)...) + if resp.Diagnostics.HasError() { + 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, }) @@ -641,23 +648,29 @@ func (r *TemplateResource) Update(ctx context.Context, req resource.UpdateReques 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)) @@ -665,11 +678,15 @@ func (r *TemplateResource) Update(ctx context.Context, req resource.UpdateReques } tflog.Trace(ctx, "marked template version as active") } - planState.Versions[idx].ID = UUIDValue(versionResp.ID) + } + + 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) { @@ -1053,3 +1070,50 @@ 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"` +} + +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) { + 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 := ps.SetKey(ctx, version.DirectoryHash.ValueString(), prevBytes) + if diag.HasError() { + return diag + } + } + return diags +} + +func readPrivateState(ctx context.Context, v Versions, ps privateState) (diags diag.Diagnostics) { + for idx, version := range v { + jsonBytes, diag := ps.GetKey(ctx, 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 + } + // Otherwise, use the existing ID + v[idx].ID = UUIDValue(prev.ID) + } + return +} diff --git a/internal/provider/template_resource_test.go b/internal/provider/template_resource_test.go index 937141f..1292030 100644 --- a/internal/provider/template_resource_test.go +++ b/internal/provider/template_resource_test.go @@ -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 @@ -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(""), }), @@ -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 @@ -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, 2) + }, + }, + // Resource deleted }, }) }