fix: use partially randomised version names

Author: ethanndickson

docs/resources/template.md

@@ -45,14 +45,16 @@ 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_suffix` (String) A suffix for the name of the template version. Prepended by a random string. Must be unique within the list of versions.
 - `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))
 
 Read-Only:
 
 - `directory_hash` (String)
+- `full_name` (String) The full name of the template version, as on the Coder deployment.
 - `id` (String)
+- `revision_num` (Number) The ordinal appended to the name_prefix to generate a unique name for the template version.
 
 <a id="nestedatt--versions--provisioner_tags"></a>
 ### Nested Schema for `versions.provisioner_tags`

go.mod

@@ -16,6 +16,7 @@ require (
 	github.com/hashicorp/terraform-plugin-go v0.23.0
 	github.com/hashicorp/terraform-plugin-log v0.9.0
 	github.com/hashicorp/terraform-plugin-testing v1.9.0
+	github.com/moby/moby v26.1.0+incompatible
 	github.com/stretchr/testify v1.9.0
 )
 

go.sum

@@ -317,6 +317,8 @@ github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zx
 github.com/mitchellh/reflectwalk v1.0.2/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
 github.com/moby/docker-image-spec v1.3.1 h1:jMKff3w6PgbfSa69GfNg+zN/XLhfXJGnEx3Nl2EsFP0=
 github.com/moby/docker-image-spec v1.3.1/go.mod h1:eKmb5VW8vQEh/BAr2yvVNvuiJuY6UIocYsFu/DxxRpo=
+github.com/moby/moby v26.1.0+incompatible h1:mjepCwMH0KpCgPvrXjqqyCeTCHgzO7p9TwZ2nQMI2qU=
+github.com/moby/moby v26.1.0+incompatible/go.mod h1:fDXVQ6+S340veQPv35CzDahGBmHsiclFwfEygB/TWMc=
 github.com/moby/term v0.5.0 h1:xt8Q1nalod/v7BqbG21f8mQPqH+xAaC9C3N3wfWbVP0=
 github.com/moby/term v0.5.0/go.mod h1:8FzsFHVUBGZdbDsJw/ot+X+d5HLUbvklYLJ9uGfcI3Y=
 github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=

integration/integration_test.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"strconv"
 	"testing"
 	"time"
@@ -128,7 +129,7 @@ func TestIntegration(t *testing.T) {
 				})
 				require.NoError(t, err)
 				require.Len(t, versions, 2)
-				require.Equal(t, "latest", versions[0].Name)
+				require.Regexp(t, regexp.MustCompile(`^0_.*\.latest$`), versions[0].Name)
 				require.NotEmpty(t, versions[0].ID)
 				require.Equal(t, templates[0].ID, *versions[0].TemplateID)
 				require.Equal(t, templates[0].ActiveVersionID, versions[0].ID)

integration/template-test/main.tf

@@ -41,9 +41,9 @@ resource "coderd_template" "sample" {
   }
   versions = [
     {
-      name      = "latest"
-      directory = "./example-template"
-      active    = true
+      name_suffix = "latest"
+      directory   = "./version-one"
+      active      = true
       tf_vars = [
         {
           name  = "name"
@@ -52,9 +52,9 @@ resource "coderd_template" "sample" {
       ]
     },
     {
-      name      = "legacy"
-      directory = "./example-template-2"
-      active    = false
+      name_suffix = "legacy"
+      directory   = "./version-two"
+      active      = false
       tf_vars = [
         {
           name  = "name"

integration/template-test/example-template-2/main.tf renamed to integration/template-test/version-one/main.tf

@@ -31,9 +31,9 @@ func TestAccTemplateDataSource(t *testing.T) {
 	version, err := newVersion(ctx, client, newVersionRequest{
 		OrganizationID: orgID,
 		Version: &TemplateVersion{
-			Name:      types.StringValue("main"),
-			Message:   types.StringValue("Initial commit"),
-			Directory: types.StringValue("../../integration/template-test/example-template/"),
+			NameSuffix: types.StringValue("version-one"),
+			Message:    types.StringValue("Initial commit"),
+			Directory:  types.StringValue("../../integration/template-test/version-one/"),
 			TerraformVariables: []Variable{
 				{
 					Name:  types.StringValue("name"),

integration/template-test/example-template/terraform.tfvars renamed to integration/template-test/version-one/terraform.tfvars

@@ -5,6 +5,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"strings"
 
 	"cdr.dev/slog"
 	"github.com/coder/coder/v2/codersdk"
@@ -25,6 +26,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/types"
 	"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
 	"github.com/hashicorp/terraform-plugin-log/tflog"
+	"github.com/moby/moby/pkg/namesgenerator"
 )
 
 // Ensure provider defined types fully satisfy framework interfaces.
@@ -53,6 +55,7 @@ type TemplateResourceModel struct {
 	AllowUserAutoStart types.Bool   `tfsdk:"allow_user_auto_start"`
 	AllowUserAutoStop  types.Bool   `tfsdk:"allow_user_auto_stop"`
 
+	// If null, we are not managing ACL via Terraform (such as for AGPL).
 	ACL      types.Object `tfsdk:"acl"`
 	Versions Versions     `tfsdk:"versions"`
 }
@@ -69,21 +72,25 @@ func (m TemplateResourceModel) EqualTemplateMetadata(other TemplateResourceModel
 }
 
 type TemplateVersion struct {
-	ID                 UUID         `tfsdk:"id"`
-	Name               types.String `tfsdk:"name"`
+	ID UUID `tfsdk:"id"`
+
+	NameSuffix         types.String `tfsdk:"name_suffix"`
 	Message            types.String `tfsdk:"message"`
 	Directory          types.String `tfsdk:"directory"`
 	DirectoryHash      types.String `tfsdk:"directory_hash"`
 	Active             types.Bool   `tfsdk:"active"`
 	TerraformVariables []Variable   `tfsdk:"tf_vars"`
 	ProvisionerTags    []Variable   `tfsdk:"provisioner_tags"`
+
+	RevisionNum types.Int64  `tfsdk:"revision_num"`
+	FullName    types.String `tfsdk:"full_name"`
 }
 
 type Versions []TemplateVersion
 
-func (v Versions) ByID(id UUID) *TemplateVersion {
+func (v Versions) ByNameSuffix(nameSuffix types.String) *TemplateVersion {
 	for _, m := range v {
-		if m.ID.Equal(id) {
+		if m.NameSuffix.Equal(nameSuffix) {
 			return &m
 		}
 	}
@@ -219,18 +226,20 @@ func (r *TemplateResource) Schema(ctx context.Context, req resource.SchemaReques
 				Required: true,
 				Validators: []validator.List{
 					listvalidator.SizeAtLeast(1),
-					NewActiveVersionValidator(),
+					NewVersionsValidator(),
 				},
 				NestedObject: schema.NestedAttributeObject{
 					Attributes: map[string]schema.Attribute{
 						"id": schema.StringAttribute{
 							CustomType: UUIDType,
 							Computed:   true,
+							// This ID may change as the version is re-created.
 						},
-						"name": schema.StringAttribute{
-							MarkdownDescription: "The name of the template version. Automatically generated if not provided.",
+						"name_suffix": schema.StringAttribute{
+							MarkdownDescription: "A suffix for the name of the template version. Prepended by a random string. Must be unique within the list of versions.",
 							Optional:            true,
 							Computed:            true,
+							Default:             stringdefault.StaticString(""),
 						},
 						"message": schema.StringAttribute{
 							MarkdownDescription: "A message describing the changes in this version of the template. Messages longer than 72 characters will be truncated.",
@@ -261,6 +270,14 @@ func (r *TemplateResource) Schema(ctx context.Context, req resource.SchemaReques
 							Optional:            true,
 							NestedObject:        variableNestedObject,
 						},
+						"revision_num": schema.Int64Attribute{
+							MarkdownDescription: "The ordinal appended to the name_prefix to generate a unique name for the template version.",
+							Computed:            true,
+						},
+						"full_name": schema.StringAttribute{
+							MarkdownDescription: "The full name of the template version, as on the Coder deployment.",
+							Computed:            true,
+						},
 					},
 					PlanModifiers: []planmodifier.Object{
 						NewDirectoryHashPlanModifier(),
@@ -316,6 +333,7 @@ func (r *TemplateResource) Create(ctx context.Context, req resource.CreateReques
 		newVersionRequest := newVersionRequest{
 			Version:        &version,
 			OrganizationID: orgID,
+			RevisionNum:    0,
 		}
 		if idx > 0 {
 			newVersionRequest.TemplateID = &templateResp.ID
@@ -376,8 +394,9 @@ func (r *TemplateResource) Create(ctx context.Context, req resource.CreateReques
 			}
 			tflog.Trace(ctx, "marked template version as active")
 		}
+		data.Versions[idx].FullName = types.StringValue(versionResp.Name)
 		data.Versions[idx].ID = UUIDValue(versionResp.ID)
-		data.Versions[idx].Name = types.StringValue(versionResp.Name)
+		data.Versions[idx].RevisionNum = types.Int64Value(newVersionRequest.RevisionNum)
 	}
 	data.ID = UUIDValue(templateResp.ID)
 	data.DisplayName = types.StringValue(templateResp.DisplayName)
@@ -437,7 +456,8 @@ func (r *TemplateResource) Read(ctx context.Context, req resource.ReadRequest, r
 			resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to get template version: %s", err))
 			return
 		}
-		data.Versions[idx].Name = types.StringValue(versionResp.Name)
+		data.Versions[idx].FullName = types.StringValue(versionResp.Name)
+		data.Versions[idx].NameSuffix = types.StringValue(extractNameSuffix(versionResp.Name))
 		data.Versions[idx].Message = types.StringValue(versionResp.Message)
 		active := false
 		if versionResp.ID == template.ActiveVersionID {
@@ -481,7 +501,8 @@ func (r *TemplateResource) Update(ctx context.Context, req resource.UpdateReques
 
 	client := r.data.Client
 
-	if !planState.EqualTemplateMetadata(curState) {
+	templateMetadataChanged := !planState.EqualTemplateMetadata(curState)
+	if templateMetadataChanged {
 		tflog.Trace(ctx, "change in template metadata detected, updating.")
 		_, err := client.UpdateTemplateMeta(ctx, templateID, codersdk.UpdateTemplateMeta{
 			Name:                       planState.Name.ValueString(),
@@ -499,8 +520,9 @@ func (r *TemplateResource) Update(ctx context.Context, req resource.UpdateReques
 		tflog.Trace(ctx, "successfully updated template metadata")
 	}
 
-	// If there's a change, and we're still managing ACL
-	if !planState.ACL.Equal(curState.ACL) && !planState.ACL.IsNull() {
+	// Since the everyone group always gets deleted by `DisableEveryoneGroupAccess`, we need to run this even if there
+	// were no ACL changes, in case the template metadata was updated.
+	if !planState.ACL.IsNull() && (!curState.ACL.Equal(planState.ACL) || templateMetadataChanged) {
 		var acl ACL
 		resp.Diagnostics.Append(planState.ACL.As(ctx, &acl, basetypes.ObjectAsOptions{})...)
 		if resp.Diagnostics.HasError() {
@@ -515,31 +537,51 @@ func (r *TemplateResource) Update(ctx context.Context, req resource.UpdateReques
 	}
 
 	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 {
+		var curVersionName string
+		// All versions in the state are guaranteed to have known name suffixes
+		foundVersion := curState.Versions.ByNameSuffix(plannedVersion.NameSuffix)
+		// If the version is new (name suffix doesn't exist already), or if the directory hash has changed, create a
+		// new version.
+		if foundVersion == nil || !foundVersion.DirectoryHash.Equal(plannedVersion.DirectoryHash) {
 			tflog.Trace(ctx, "discovered a new or modified template version")
+			var curRevs int64 = 0
+			if foundVersion != nil {
+				curRevs = foundVersion.RevisionNum.ValueInt64() + 1
+			}
 			versionResp, err := newVersion(ctx, client, newVersionRequest{
 				Version:        &plannedVersion,
 				OrganizationID: orgID,
 				TemplateID:     &templateID,
+				RevisionNum:    curRevs,
 			})
 			if err != nil {
 				resp.Diagnostics.AddError("Client Error", err.Error())
 				return
 			}
-			curVersionID = versionResp.ID
+			planState.Versions[idx].RevisionNum = types.Int64Value(curRevs)
+			curVersionName = versionResp.Name
 		} else {
-			// Or if it's an existing version, get the ID
-			curVersionID = plannedVersion.ID.ValueUUID()
+			// Or if it's an existing version, get the full name to look it up
+			planState.Versions[idx].RevisionNum = foundVersion.RevisionNum
+			curVersionName = foundVersion.FullName.ValueString()
 		}
-		versionResp, err := client.TemplateVersion(ctx, curVersionID)
+		versionResp, err := client.TemplateVersionByName(ctx, templateID, curVersionName)
 		if err != nil {
 			resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to get template version: %s", err))
 			return
 		}
+
+		if versionResp.Message != plannedVersion.Message.ValueString() {
+			_, err := client.UpdateTemplateVersion(ctx, versionResp.ID, codersdk.PatchTemplateVersionRequest{
+				Name:    versionResp.Name,
+				Message: plannedVersion.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() {
 			tflog.Trace(ctx, "marking template version as active", map[string]any{
 				"version_id":  versionResp.ID,
@@ -555,6 +597,7 @@ 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)
+		planState.Versions[idx].FullName = types.StringValue(versionResp.Name)
 	}
 
 	// Save updated data into Terraform state
@@ -592,24 +635,24 @@ func (r *TemplateResource) ConfigValidators(context.Context) []resource.ConfigVa
 	return []resource.ConfigValidator{}
 }
 
-type activeVersionValidator struct{}
+type versionsValidator struct{}
 
-func NewActiveVersionValidator() validator.List {
-	return &activeVersionValidator{}
+func NewVersionsValidator() validator.List {
+	return &versionsValidator{}
 }
 
 // Description implements validator.List.
-func (a *activeVersionValidator) Description(ctx context.Context) string {
+func (a *versionsValidator) Description(ctx context.Context) string {
 	return a.MarkdownDescription(ctx)
 }
 
 // MarkdownDescription implements validator.List.
-func (a *activeVersionValidator) MarkdownDescription(context.Context) string {
+func (a *versionsValidator) MarkdownDescription(context.Context) string {
 	return "Validate that exactly one template version has active set to true."
 }
 
 // ValidateList implements validator.List.
-func (a *activeVersionValidator) ValidateList(ctx context.Context, req validator.ListRequest, resp *validator.ListResponse) {
+func (a *versionsValidator) ValidateList(ctx context.Context, req validator.ListRequest, resp *validator.ListResponse) {
 	var data []TemplateVersion
 	resp.Diagnostics.Append(req.ConfigValue.ElementsAs(ctx, &data, false)...)
 	if resp.Diagnostics.HasError() {
@@ -630,9 +673,20 @@ func (a *activeVersionValidator) ValidateList(ctx context.Context, req validator
 	if !active {
 		resp.Diagnostics.AddError("Client Error", "At least one template version must be active.")
 	}
+
+	// Check if all versions have unique name suffixes
+	nameSuffixes := make(map[string]bool)
+	for _, version := range data {
+		nameSuffix := version.NameSuffix.ValueString()
+		if _, ok := nameSuffixes[nameSuffix]; ok {
+			resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Template version name suffixes must be unique, found duplicate: `%s`", nameSuffix))
+			return
+		}
+		nameSuffixes[nameSuffix] = true
+	}
 }
 
-var _ validator.List = &activeVersionValidator{}
+var _ validator.List = &versionsValidator{}
 
 type directoryHashPlanModifier struct{}
 
@@ -731,6 +785,7 @@ type newVersionRequest struct {
 	OrganizationID uuid.UUID
 	Version        *TemplateVersion
 	TemplateID     *uuid.UUID
+	RevisionNum    int64
 }
 
 func newVersion(ctx context.Context, client *codersdk.Client, req newVersionRequest) (*codersdk.TemplateVersion, error) {
@@ -761,8 +816,12 @@ func newVersion(ctx context.Context, client *codersdk.Client, req newVersionRequ
 			Value: variable.Value.ValueString(),
 		})
 	}
+	versionName := fmt.Sprintf("%d_%s", req.RevisionNum, namesgenerator.GetRandomName(1))
+	if req.Version.NameSuffix.ValueString() != "" {
+		versionName = fmt.Sprintf("%s.%s", versionName, req.Version.NameSuffix.ValueString())
+	}
 	tmplVerReq := codersdk.CreateTemplateVersionRequest{
-		Name:               req.Version.Name.ValueString(),
+		Name:               versionName,
 		Message:            req.Version.Message.ValueString(),
 		StorageMethod:      codersdk.ProvisionerStorageMethodFile,
 		Provisioner:        codersdk.ProvisionerTypeTerraform,
@@ -821,3 +880,11 @@ func convertResponseToACL(acl codersdk.TemplateACL) ACL {
 		GroupPermissions: groupPerms,
 	}
 }
+
+func extractNameSuffix(name string) string {
+	parts := strings.Split(name, ".")
+	if len(parts) == 1 {
+		return ""
+	}
+	return parts[1]
+}