Skip to content

Commit 38cc6df

Browse files
jatcod3rPepziC0lesethanndickson
authored
fix: preserve sensitive tf_vars and unknown version names (#347)
This keeps template version plan updates from rewriting unrelated nested values, which preserves sensitive tf_vars during deferred replans. It also stops reusing a previous version name when the config supplied a non-null expression that is still unknown at plan time, like a random_uuid result. The change adds regressions for both cases and skips the sensitive tf_vars acceptance test on Terraform 1.0, where the CLI panics while formatting that plan. --------- Co-authored-by: ju-pe <jullianpepito@gmail.com> Co-authored-by: Ethan Dickson <ethan@coder.com>
1 parent ab08af2 commit 38cc6df

2 files changed

Lines changed: 200 additions & 3 deletions

File tree

internal/provider/template_resource.go

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1051,7 +1051,43 @@ func (d *versionsPlanModifier) PlanModifyList(ctx context.Context, req planmodif
10511051
return
10521052
}
10531053

1054-
resp.PlanValue, diag = types.ListValueFrom(ctx, req.PlanValue.ElementType(ctx), planVersions)
1054+
// Patch the original plan elements instead of rebuilding the entire list
1055+
// from planVersions. Re-encoding the whole list with ListValueFrom() strips
1056+
// Terraform Core's cty-level marks from nested values (for example sensitive
1057+
// tf_vars entries), which can surface later as an inconsistent final plan
1058+
// during deferred re-plans. Keeping the original attr.Values intact preserves
1059+
// those marks on untouched fields.
1060+
planElements := req.PlanValue.Elements()
1061+
newElements := make([]attr.Value, len(planElements))
1062+
1063+
for i, elem := range planElements {
1064+
obj, ok := elem.(types.Object)
1065+
if !ok {
1066+
resp.Diagnostics.AddError("Client Error", "Expected object element in versions list")
1067+
return
1068+
}
1069+
1070+
// types.Object.Attributes returns a copy of the attribute map, so it is
1071+
// safe to mutate attrs before constructing the replacement object.
1072+
attrs := obj.Attributes()
1073+
attrTypes := obj.AttributeTypes(ctx)
1074+
1075+
// Overwrite only the fields the plan modifier manages
1076+
attrs["id"] = planVersions[i].ID
1077+
attrs["name"] = planVersions[i].Name
1078+
attrs["directory_hash"] = planVersions[i].DirectoryHash
1079+
1080+
// tf_vars, provisioner_tags, directory, active, message — all untouched
1081+
1082+
newObj, objDiag := types.ObjectValue(attrTypes, attrs)
1083+
if objDiag.HasError() {
1084+
resp.Diagnostics.Append(objDiag...)
1085+
return
1086+
}
1087+
newElements[i] = newObj
1088+
}
1089+
1090+
resp.PlanValue, diag = types.ListValue(req.PlanValue.ElementType(ctx), newElements)
10551091
if diag.HasError() {
10561092
resp.Diagnostics.Append(diag...)
10571093
}
@@ -1519,7 +1555,7 @@ func (planVersions Versions) reconcileVersionIDs(lv LastVersionsByHash, configVe
15191555
prevList := lv[planVersions[i].DirectoryHash.ValueString()]
15201556
if len(prevList) > 0 && planVersions[i].ID.IsUnknown() {
15211557
planVersions[i].ID = UUIDValue(prevList[0].ID)
1522-
if planVersions[i].Name.IsUnknown() {
1558+
if configVersions[i].Name.IsNull() {
15231559
planVersions[i].Name = types.StringValue(prevList[0].Name)
15241560
}
15251561
lv[planVersions[i].DirectoryHash.ValueString()] = prevList[1:]

internal/provider/template_resource_test.go

Lines changed: 162 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/hashicorp/terraform-plugin-framework/types"
1515
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
1616
"github.com/hashicorp/terraform-plugin-testing/terraform"
17+
"github.com/hashicorp/terraform-plugin-testing/tfversion"
1718
cp "github.com/otiai10/copy"
1819
"github.com/stretchr/testify/require"
1920

@@ -1070,6 +1071,108 @@ resource "coderd_template" "sample" {
10701071
})
10711072
}
10721073

1074+
func TestAccTemplateResourceSensitiveTFVarsDeferredReplan(t *testing.T) {
1075+
t.Parallel()
1076+
1077+
// Changing the sensitive tf_var forces random_uuid to be replaced, which
1078+
// makes the version name unknown during planning and triggers a deferred
1079+
// re-plan during apply. Before PlanModifyList started patching the original
1080+
// attr.Values instead of rebuilding the whole list, this failed with:
1081+
// "Provider produced inconsistent final plan ... inconsistent values for
1082+
// sensitive attribute".
1083+
cfg := `
1084+
provider coderd {
1085+
url = %q
1086+
token = %q
1087+
}
1088+
1089+
variable "secret_one" {
1090+
type = string
1091+
sensitive = true
1092+
default = %q
1093+
}
1094+
1095+
locals {
1096+
my_secrets = {
1097+
normal = "hi"
1098+
secret = var.secret_one
1099+
}
1100+
}
1101+
1102+
resource "random_uuid" "uuid" {
1103+
keepers = local.my_secrets
1104+
}
1105+
1106+
resource "coderd_template" "test" {
1107+
name = "sensitive-template"
1108+
1109+
versions = [{
1110+
name = random_uuid.uuid.result
1111+
directory = %q
1112+
active = true
1113+
tf_vars = [for k, v in local.my_secrets : {
1114+
name = k
1115+
value = tostring(v)
1116+
}]
1117+
}]
1118+
}
1119+
`
1120+
1121+
ctx := t.Context()
1122+
client := integration.StartCoder(ctx, t, "template_resource_sensitive_tfvars_acc")
1123+
1124+
exTemplateOne := t.TempDir()
1125+
err := cp.Copy("../../integration/template-test/example-template", exTemplateOne)
1126+
require.NoError(t, err)
1127+
1128+
cfg1 := fmt.Sprintf(cfg, client.URL.String(), client.SessionToken(), "no", exTemplateOne)
1129+
cfg2 := fmt.Sprintf(cfg, client.URL.String(), client.SessionToken(), "yes", exTemplateOne)
1130+
1131+
resource.Test(t, resource.TestCase{
1132+
PreCheck: func() { testAccPreCheck(t) },
1133+
IsUnitTest: true,
1134+
// Terraform 1.0 panics while formatting plans that contain marked nested
1135+
// values in this scenario ("value is marked, so must be unmarked first").
1136+
// The provider regression is still covered on Terraform 1.1+.
1137+
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
1138+
tfversion.SkipBelow(tfversion.Version1_1_0),
1139+
},
1140+
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
1141+
ExternalProviders: map[string]resource.ExternalProvider{
1142+
"random": {
1143+
Source: "hashicorp/random",
1144+
VersionConstraint: "~> 3.7",
1145+
},
1146+
},
1147+
Steps: []resource.TestStep{
1148+
{
1149+
Config: cfg1,
1150+
Check: resource.ComposeAggregateTestCheckFunc(
1151+
resource.TestCheckResourceAttr("coderd_template.test", "versions.#", "1"),
1152+
resource.TestMatchTypeSetElemNestedAttrs("coderd_template.test", "versions.*", map[string]*regexp.Regexp{
1153+
"name": regexp.MustCompile(".+"),
1154+
"id": regexp.MustCompile(".+"),
1155+
"directory_hash": regexp.MustCompile(".+"),
1156+
}),
1157+
testAccCheckNumTemplateVersions(ctx, client, 1),
1158+
),
1159+
},
1160+
{
1161+
Config: cfg2,
1162+
Check: resource.ComposeAggregateTestCheckFunc(
1163+
resource.TestCheckResourceAttr("coderd_template.test", "versions.#", "1"),
1164+
resource.TestMatchTypeSetElemNestedAttrs("coderd_template.test", "versions.*", map[string]*regexp.Regexp{
1165+
"name": regexp.MustCompile(".+"),
1166+
"id": regexp.MustCompile(".+"),
1167+
"directory_hash": regexp.MustCompile(".+"),
1168+
}),
1169+
testAccCheckNumTemplateVersions(ctx, client, 2),
1170+
),
1171+
},
1172+
},
1173+
})
1174+
}
1175+
10731176
type testAccTemplateResourceConfig struct {
10741177
URL string
10751178
Token string
@@ -1439,6 +1542,8 @@ func TestReconcileVersionIDs(t *testing.T) {
14391542
},
14401543
},
14411544
{
1545+
// Config name is null (auto-generated), plan name is unknown.
1546+
// Should backfill name from state since the user didn't set one.
14421547
Name: "UnknownUsesStateInOrder",
14431548
planVersions: []TemplateVersion{
14441549
{
@@ -1459,7 +1564,7 @@ func TestReconcileVersionIDs(t *testing.T) {
14591564
Name: types.StringValue("foo"),
14601565
},
14611566
{
1462-
Name: types.StringValue("bar"),
1567+
Name: types.StringNull(),
14631568
},
14641569
},
14651570
inputState: map[string][]PreviousTemplateVersion{
@@ -1491,6 +1596,62 @@ func TestReconcileVersionIDs(t *testing.T) {
14911596
},
14921597
},
14931598
},
1599+
{
1600+
// Config name is non-null (e.g. random_uuid.result), plan name is unknown
1601+
// because the upstream resource is being recreated.
1602+
// Should NOT backfill name — leave it unknown to resolve after apply.
1603+
Name: "UnknownNonNullConfigNameNotBackfilled",
1604+
planVersions: []TemplateVersion{
1605+
{
1606+
Name: types.StringValue("foo"),
1607+
DirectoryHash: types.StringValue("aaa"),
1608+
ID: NewUUIDUnknown(),
1609+
TerraformVariables: []Variable{},
1610+
},
1611+
{
1612+
Name: types.StringUnknown(),
1613+
DirectoryHash: types.StringValue("aaa"),
1614+
ID: NewUUIDUnknown(),
1615+
TerraformVariables: []Variable{},
1616+
},
1617+
},
1618+
configVersions: []TemplateVersion{
1619+
{
1620+
Name: types.StringValue("foo"),
1621+
},
1622+
{
1623+
Name: types.StringValue("bar"),
1624+
},
1625+
},
1626+
inputState: map[string][]PreviousTemplateVersion{
1627+
"aaa": {
1628+
{
1629+
ID: aUUID,
1630+
Name: "qux",
1631+
TFVars: map[string]string{},
1632+
},
1633+
{
1634+
ID: bUUID,
1635+
Name: "baz",
1636+
TFVars: map[string]string{},
1637+
},
1638+
},
1639+
},
1640+
expectedVersions: []TemplateVersion{
1641+
{
1642+
Name: types.StringValue("foo"),
1643+
DirectoryHash: types.StringValue("aaa"),
1644+
ID: UUIDValue(aUUID),
1645+
TerraformVariables: []Variable{},
1646+
},
1647+
{
1648+
Name: types.StringUnknown(),
1649+
DirectoryHash: types.StringValue("aaa"),
1650+
ID: UUIDValue(bUUID),
1651+
TerraformVariables: []Variable{},
1652+
},
1653+
},
1654+
},
14941655
{
14951656
Name: "NewVersionNewRandomName",
14961657
planVersions: []TemplateVersion{

0 commit comments

Comments
 (0)