chore: allow pushing only inactive coderd_template versions (#167)

Per some customer feedback, this PR relaxes some of the constraints when
creating template versions via the `coderd_template` resource.
Previously, each update of the resource required that at least one of
the versions in the `versions` list had the `active` attribute set to
true.

This constraint is now only required when:
1. Creating the resource, as every template must have an active version.
2. It's not clear what template version should be set as active. 
- This occurs when the currently active version is marked as inactive,
but no changes are made to the contents of the version (and no version
would be created).
    
Examples of these cases can be seen in the tests.

With this, a workflow that involves:
- Marking an active version in the list as inactive
- Pushing it a few times via the provider
- And then finally marking it as active 

is now possible with just a single item in the list.

Author: ethanndickson

docs/resources/template.md

@@ -77,7 +77,7 @@ resource "coderd_template" "ubuntu-main" {
 - `description` (String) A description of the template.
 - `display_name` (String) The display name of the template. Defaults to the template name.
 - `failure_ttl_ms` (Number) (Enterprise) The max lifetime before Coder stops all resources for failed workspaces created from this template, in milliseconds.
-- `icon` (String) Relative path or external URL that specifes an icon to be displayed in the dashboard.
+- `icon` (String) Relative path or external URL that specifies an icon to be displayed in the dashboard.
 - `max_port_share_level` (String) (Enterprise) The maximum port share level for workspaces created from this template. Defaults to `owner` on an Enterprise deployment, or `public` otherwise.
 - `organization_id` (String) The ID of the organization. Defaults to the provider's default organization
 - `require_active_version` (Boolean) (Enterprise) Whether workspaces must be created from the active version of this template. Defaults to false.

integration/integration.go

@@ -51,9 +51,10 @@ func StartCoder(ctx context.Context, t *testing.T, name string, useLicense bool)
 	ctr, err := cli.ContainerCreate(ctx, &container.Config{
 		Image: coderImg + ":" + coderVersion,
 		Env: []string{
-			"CODER_HTTP_ADDRESS=0.0.0.0:3000",        // Listen on all interfaces inside the container
-			"CODER_ACCESS_URL=http://localhost:3000", // Set explicitly to avoid creating try.coder.app URLs.
-			"CODER_TELEMETRY_ENABLE=false",           // Avoid creating noise.
+			"CODER_HTTP_ADDRESS=0.0.0.0:3000",          // Listen on all interfaces inside the container
+			"CODER_ACCESS_URL=http://localhost:3000",   // Set explicitly to avoid creating try.coder.app URLs.
+			"CODER_TELEMETRY_ENABLE=false",             // Avoid creating noise.
+			"CODER_DANGEROUS_DISABLE_RATE_LIMITS=true", // Avoid hitting file rate limit in tests.
 		},
 		Labels:       map[string]string{},
 		ExposedPorts: map[nat.Port]struct{}{nat.Port("3000/tcp"): {}},

integration/integration_test.go

@@ -166,7 +166,7 @@ func TestIntegration(t *testing.T) {
 			tfCmd.Stderr = &buf
 			tt.preF(t, client)
 			if err := tfCmd.Run(); !assert.NoError(t, err) {
-				t.Logf("%s", buf.String())
+				t.Log(buf.String())
 				t.FailNow()
 			}
 			tt.assertF(t, client)

internal/provider/template_resource.go

@@ -286,7 +286,7 @@ func (r *TemplateResource) Schema(ctx context.Context, req resource.SchemaReques
 				},
 			},
 			"icon": schema.StringAttribute{
-				MarkdownDescription: "Relative path or external URL that specifes an icon to be displayed in the dashboard.",
+				MarkdownDescription: "Relative path or external URL that specifies an icon to be displayed in the dashboard.",
 				Optional:            true,
 				Computed:            true,
 				Default:             stringdefault.StaticString(""),
@@ -404,7 +404,7 @@ 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{
@@ -867,24 +867,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 {
-	return "Validate that exactly one template version has active set to true."
+func (a *versionsValidator) MarkdownDescription(context.Context) string {
+	return "Validate that template version names are unique and that at most one version is active."
 }
 
 // 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) {
 	if req.ConfigValue.IsNull() || req.ConfigValue.IsUnknown() {
 		return
 	}
@@ -908,13 +908,13 @@ func (a *activeVersionValidator) ValidateList(ctx context.Context, req validator
 		uniqueNames[version.Name.ValueString()] = struct{}{}
 	}
 
-	// Check if only one item in Version has active set to true
+	// Ensure at most one version is active
 	active := false
 	for _, version := range data {
-		// `active` is required, so if it's null or unknown, this is Terraform
+		// `active` defaults to false, so if it's null or unknown, this is Terraform
 		// requesting an early validation.
 		if version.Active.IsNull() || version.Active.IsUnknown() {
-			return
+			continue
 		}
 		if version.Active.ValueBool() {
 			if active {
@@ -924,12 +924,9 @@ func (a *activeVersionValidator) ValidateList(ctx context.Context, req validator
 			active = true
 		}
 	}
-	if !active {
-		resp.Diagnostics.AddError("Client Error", "At least one template version must be active.")
-	}
 }
 
-var _ validator.List = &activeVersionValidator{}
+var _ validator.List = &versionsValidator{}
 
 type versionsPlanModifier struct{}
 
@@ -956,6 +953,12 @@ func (d *versionsPlanModifier) PlanModifyList(ctx context.Context, req planmodif
 		return
 	}
 
+	hasActiveVersion, diag := hasOneActiveVersion(configVersions)
+	if diag.HasError() {
+		resp.Diagnostics.Append(diag...)
+		return
+	}
+
 	for i := range planVersions {
 		hash, err := computeDirectoryHash(planVersions[i].Directory.ValueString())
 		if err != nil {
@@ -974,6 +977,13 @@ func (d *versionsPlanModifier) PlanModifyList(ctx context.Context, req planmodif
 	// If this is the first read, init the private state value
 	if lvBytes == nil {
 		lv = make(LastVersionsByHash)
+		// If there's no prior private state, this might be resource creation,
+		// in which case one version must be active.
+		if !hasActiveVersion {
+			resp.Diagnostics.AddError("Client Error", "At least one template version must be active when creating a"+
+				" `coderd_template` resource.\n(Subsequent resource updates can be made without an active template in the list).")
+			return
+		}
 	} else {
 		err := json.Unmarshal(lvBytes, &lv)
 		if err != nil {
@@ -982,9 +992,34 @@ func (d *versionsPlanModifier) PlanModifyList(ctx context.Context, req planmodif
 		}
 	}
 
-	planVersions.reconcileVersionIDs(lv, configVersions)
+	diag = planVersions.reconcileVersionIDs(lv, configVersions, hasActiveVersion)
+	if diag.HasError() {
+		resp.Diagnostics.Append(diag...)
+		return
+	}
 
-	resp.PlanValue, resp.Diagnostics = types.ListValueFrom(ctx, req.PlanValue.ElementType(ctx), planVersions)
+	resp.PlanValue, diag = types.ListValueFrom(ctx, req.PlanValue.ElementType(ctx), planVersions)
+	if diag.HasError() {
+		resp.Diagnostics.Append(diag...)
+	}
+}
+
+func hasOneActiveVersion(data Versions) (hasActiveVersion bool, diags diag.Diagnostics) {
+	active := false
+	for _, version := range data {
+		if version.Active.IsNull() || version.Active.IsUnknown() {
+			// If null or unknown, the value will be defaulted to false
+			continue
+		}
+		if version.Active.ValueBool() {
+			if active {
+				diags.AddError("Client Error", "Only one template version can be active at a time.")
+				return
+			}
+			active = true
+		}
+	}
+	return active, diags
 }
 
 func NewVersionsPlanModifier() planmodifier.List {
@@ -1309,6 +1344,7 @@ type PreviousTemplateVersion struct {
 	ID     uuid.UUID         `json:"id"`
 	Name   string            `json:"name"`
 	TFVars map[string]string `json:"tf_vars"`
+	Active bool              `json:"active"`
 }
 
 type privateState interface {
@@ -1331,13 +1367,15 @@ func (v Versions) setPrivateState(ctx context.Context, ps privateState) (diags d
 				ID:     version.ID.ValueUUID(),
 				Name:   version.Name.ValueString(),
 				TFVars: tfVars,
+				Active: version.Active.ValueBool(),
 			})
 		} else {
 			lv[version.DirectoryHash.ValueString()] = []PreviousTemplateVersion{
 				{
 					ID:     version.ID.ValueUUID(),
 					Name:   version.Name.ValueString(),
 					TFVars: tfVars,
+					Active: version.Active.ValueBool(),
 				},
 			}
 		}
@@ -1350,7 +1388,7 @@ func (v Versions) setPrivateState(ctx context.Context, ps privateState) (diags d
 	return ps.SetKey(ctx, LastVersionsKey, lvBytes)
 }
 
-func (planVersions Versions) reconcileVersionIDs(lv LastVersionsByHash, configVersions Versions) {
+func (planVersions Versions) reconcileVersionIDs(lv LastVersionsByHash, configVersions Versions, hasOneActiveVersion bool) (diag diag.Diagnostics) {
 	// We remove versions that we've matched from `lv`, so make a copy for
 	// resolving tfvar changes at the end.
 	fullLv := make(LastVersionsByHash)
@@ -1420,6 +1458,39 @@ func (planVersions Versions) reconcileVersionIDs(lv LastVersionsByHash, configVe
 			}
 		}
 	}
+
+	// If a version was deactivated, and no active version was set, we need to
+	// return an error to avoid a post-apply plan being non-empty.
+	if !hasOneActiveVersion {
+		for i := range planVersions {
+			if !planVersions[i].ID.IsUnknown() {
+				prevs, ok := fullLv[planVersions[i].DirectoryHash.ValueString()]
+				if !ok {
+					continue
+				}
+				if versionDeactivated(prevs, &planVersions[i]) {
+					diag.AddError("Client Error", "Plan could not determine which version should be active.\n"+
+						"Either specify an active version or modify the contents of the previously active version before marking it as inactive.")
+					return diag
+				}
+			}
+		}
+	}
+	return diag
+}
+
+func versionDeactivated(prevs []PreviousTemplateVersion, planned *TemplateVersion) bool {
+	for _, prev := range prevs {
+		if prev.ID == planned.ID.ValueUUID() {
+			if prev.Active &&
+				!planned.Active.IsNull() &&
+				!planned.Active.IsUnknown() &&
+				!planned.Active.ValueBool() {
+				return true
+			}
+		}
+	}
+	return false
 }
 
 func tfVariablesChanged(prevs []PreviousTemplateVersion, planned *TemplateVersion) bool {