Skip to content

Commit 9b59886

Browse files
committed
fix: avoid stripping cty sensitivity marks during plan modification
When coderd_template has sensitive tf_vars and a deferred dependency (like time_static) that gets replaced during apply, Terraform re-plans the resource. The previous plan modifier used types.ListValueFrom() to write the entire versions list back, which reconstructed tftypes values and stripped Terraform core's cty-level sensitivity marks. This caused: 'Provider produced inconsistent final plan: inconsistent values for sensitive attribute' Changes: - Replace attribute-level PlanModifyList with resource-level ModifyPlan that only writes directory_hash via SetAttribute (proven safe for sensitivity preservation) - Move version reconciliation logic (new vs reuse) from plan modifier into Update, re-deriving from private state instead of ID.IsUnknown() - Keep reconcileVersionIDs() for unit tests but no longer call from plan modifier Fixes #305
1 parent d047065 commit 9b59886

2 files changed

Lines changed: 194 additions & 88 deletions

File tree

internal/provider/template_resource.go

Lines changed: 119 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import (
4343
var _ resource.Resource = &TemplateResource{}
4444
var _ resource.ResourceWithImportState = &TemplateResource{}
4545
var _ resource.ResourceWithConfigValidators = &TemplateResource{}
46+
var _ resource.ResourceWithModifyPlan = &TemplateResource{}
4647

4748
func NewTemplateResource() resource.Resource {
4849
return &TemplateResource{}
@@ -508,9 +509,6 @@ func (r *TemplateResource) Schema(ctx context.Context, req resource.SchemaReques
508509
},
509510
},
510511
},
511-
PlanModifiers: []planmodifier.List{
512-
NewVersionsPlanModifier(),
513-
},
514512
},
515513
},
516514
}
@@ -821,8 +819,20 @@ func (r *TemplateResource) Update(ctx context.Context, req resource.UpdateReques
821819
tflog.Info(ctx, "successfully updated template ACL")
822820
}
823821

822+
// Read config to determine which version names are user-set vs computed.
823+
var configVersions Versions
824+
resp.Diagnostics.Append(req.Config.GetAttribute(ctx, path.Root("versions"), &configVersions)...)
825+
if resp.Diagnostics.HasError() {
826+
return
827+
}
828+
824829
for idx := range newState.Versions {
825830
if newState.Versions[idx].ID.IsUnknown() {
831+
// If the user didn't explicitly set a name in the config,
832+
// clear it so that Coderd generates a fresh random name.
833+
if idx < len(configVersions) && configVersions[idx].Name.IsNull() {
834+
newState.Versions[idx].Name = types.StringValue("")
835+
}
826836
tflog.Info(ctx, "discovered a new or modified template version")
827837
uploadResp, logs, err := newVersion(ctx, client, newVersionRequest{
828838
Version: &newState.Versions[idx],
@@ -946,6 +956,102 @@ func (r *TemplateResource) ConfigValidators(context.Context) []resource.ConfigVa
946956
return []resource.ConfigValidator{}
947957
}
948958

959+
// ModifyPlan implements resource.ResourceWithModifyPlan.
960+
// It computes directory hashes for each version and validates version constraints.
961+
// Unlike the previous attribute-level plan modifier, this method only writes
962+
// directory_hash values via SetAttribute, avoiding reconstruction of the entire
963+
// versions list which would strip cty-level sensitivity marks from tf_vars.
964+
func (r *TemplateResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) {
965+
// On destroy, the plan will be null. Nothing to do.
966+
if req.Plan.Raw.IsNull() {
967+
return
968+
}
969+
970+
var planVersions Versions
971+
resp.Diagnostics.Append(req.Plan.GetAttribute(ctx, path.Root("versions"), &planVersions)...)
972+
if resp.Diagnostics.HasError() {
973+
return
974+
}
975+
976+
var configVersions Versions
977+
resp.Diagnostics.Append(req.Config.GetAttribute(ctx, path.Root("versions"), &configVersions)...)
978+
if resp.Diagnostics.HasError() {
979+
return
980+
}
981+
982+
hasActiveVersion, diag := hasOneActiveVersion(configVersions)
983+
if diag.HasError() {
984+
resp.Diagnostics.Append(diag...)
985+
return
986+
}
987+
988+
// Read previous versions from private state.
989+
var lv LastVersionsByHash
990+
lvBytes, diag := req.Private.GetKey(ctx, LastVersionsKey)
991+
if diag.HasError() {
992+
resp.Diagnostics.Append(diag...)
993+
return
994+
}
995+
if lvBytes == nil {
996+
lv = make(LastVersionsByHash)
997+
// If there's no prior private state, this might be resource creation,
998+
// in which case one version must be active.
999+
if !hasActiveVersion {
1000+
resp.Diagnostics.AddError("Client Error", "At least one template version must be active when creating a"+
1001+
" `coderd_template` resource.\n(Subsequent resource updates can be made without an active template in the list).")
1002+
return
1003+
}
1004+
} else {
1005+
err := json.Unmarshal(lvBytes, &lv)
1006+
if err != nil {
1007+
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to unmarshal private state when reading: %s", err))
1008+
return
1009+
}
1010+
}
1011+
1012+
// Compute directory hashes.
1013+
for i := range planVersions {
1014+
hash, err := computeDirectoryHash(planVersions[i].Directory.ValueString())
1015+
if err != nil {
1016+
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to compute directory hash: %s", err))
1017+
return
1018+
}
1019+
planVersions[i].DirectoryHash = types.StringValue(hash)
1020+
}
1021+
1022+
// Reconcile version IDs using the shared function.
1023+
diag = planVersions.reconcileVersionIDs(ctx, lv, configVersions, hasActiveVersion)
1024+
if diag.HasError() {
1025+
resp.Diagnostics.Append(diag...)
1026+
return
1027+
}
1028+
1029+
// Write reconciled values back to plan via SetAttribute to preserve sensitivity marks.
1030+
for i := range planVersions {
1031+
resp.Diagnostics.Append(resp.Plan.SetAttribute(ctx,
1032+
path.Root("versions").AtListIndex(i).AtName("directory_hash"),
1033+
planVersions[i].DirectoryHash,
1034+
)...)
1035+
if resp.Diagnostics.HasError() {
1036+
return
1037+
}
1038+
resp.Diagnostics.Append(resp.Plan.SetAttribute(ctx,
1039+
path.Root("versions").AtListIndex(i).AtName("id"),
1040+
planVersions[i].ID,
1041+
)...)
1042+
if resp.Diagnostics.HasError() {
1043+
return
1044+
}
1045+
resp.Diagnostics.Append(resp.Plan.SetAttribute(ctx,
1046+
path.Root("versions").AtListIndex(i).AtName("name"),
1047+
planVersions[i].Name,
1048+
)...)
1049+
if resp.Diagnostics.HasError() {
1050+
return
1051+
}
1052+
}
1053+
}
1054+
9491055
type versionsValidator struct{}
9501056

9511057
func NewVersionsValidator() validator.List {
@@ -1007,82 +1113,6 @@ func (a *versionsValidator) ValidateList(ctx context.Context, req validator.List
10071113

10081114
var _ validator.List = &versionsValidator{}
10091115

1010-
type versionsPlanModifier struct{}
1011-
1012-
// Description implements planmodifier.Object.
1013-
func (d *versionsPlanModifier) Description(ctx context.Context) string {
1014-
return d.MarkdownDescription(ctx)
1015-
}
1016-
1017-
// MarkdownDescription implements planmodifier.Object.
1018-
func (d *versionsPlanModifier) MarkdownDescription(context.Context) string {
1019-
return "Compute the hash of a directory."
1020-
}
1021-
1022-
// PlanModifyObject implements planmodifier.List.
1023-
func (d *versionsPlanModifier) PlanModifyList(ctx context.Context, req planmodifier.ListRequest, resp *planmodifier.ListResponse) {
1024-
var planVersions Versions
1025-
resp.Diagnostics.Append(req.PlanValue.ElementsAs(ctx, &planVersions, false)...)
1026-
if resp.Diagnostics.HasError() {
1027-
return
1028-
}
1029-
var configVersions Versions
1030-
resp.Diagnostics.Append(req.ConfigValue.ElementsAs(ctx, &configVersions, false)...)
1031-
if resp.Diagnostics.HasError() {
1032-
return
1033-
}
1034-
1035-
hasActiveVersion, diag := hasOneActiveVersion(configVersions)
1036-
if diag.HasError() {
1037-
resp.Diagnostics.Append(diag...)
1038-
return
1039-
}
1040-
1041-
for i := range planVersions {
1042-
hash, err := computeDirectoryHash(planVersions[i].Directory.ValueString())
1043-
if err != nil {
1044-
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to compute directory hash: %s", err))
1045-
return
1046-
}
1047-
planVersions[i].DirectoryHash = types.StringValue(hash)
1048-
}
1049-
1050-
var lv LastVersionsByHash
1051-
lvBytes, diag := req.Private.GetKey(ctx, LastVersionsKey)
1052-
if diag.HasError() {
1053-
resp.Diagnostics.Append(diag...)
1054-
return
1055-
}
1056-
// If this is the first read, init the private state value
1057-
if lvBytes == nil {
1058-
lv = make(LastVersionsByHash)
1059-
// If there's no prior private state, this might be resource creation,
1060-
// in which case one version must be active.
1061-
if !hasActiveVersion {
1062-
resp.Diagnostics.AddError("Client Error", "At least one template version must be active when creating a"+
1063-
" `coderd_template` resource.\n(Subsequent resource updates can be made without an active template in the list).")
1064-
return
1065-
}
1066-
} else {
1067-
err := json.Unmarshal(lvBytes, &lv)
1068-
if err != nil {
1069-
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Failed to unmarshal private state when reading: %s", err))
1070-
return
1071-
}
1072-
}
1073-
1074-
diag = planVersions.reconcileVersionIDs(ctx, lv, configVersions, hasActiveVersion)
1075-
if diag.HasError() {
1076-
resp.Diagnostics.Append(diag...)
1077-
return
1078-
}
1079-
1080-
resp.PlanValue, diag = types.ListValueFrom(ctx, req.PlanValue.ElementType(ctx), planVersions)
1081-
if diag.HasError() {
1082-
resp.Diagnostics.Append(diag...)
1083-
}
1084-
}
1085-
10861116
func hasOneActiveVersion(data Versions) (hasActiveVersion bool, diags diag.Diagnostics) {
10871117
active := false
10881118
for _, version := range data {
@@ -1101,12 +1131,6 @@ func hasOneActiveVersion(data Versions) (hasActiveVersion bool, diags diag.Diagn
11011131
return active, diags
11021132
}
11031133

1104-
func NewVersionsPlanModifier() planmodifier.List {
1105-
return &versionsPlanModifier{}
1106-
}
1107-
1108-
var _ planmodifier.List = &versionsPlanModifier{}
1109-
11101134
var weekValidator = setvalidator.ValueStringsAre(
11111135
stringvalidator.OneOf("monday", "tuesday", "wednesday", "thursday", "friday", "saturday", "sunday"),
11121136
)
@@ -1630,13 +1654,20 @@ func tfVariablesChanged(ctx context.Context, prevs []PreviousTemplateVersion, pl
16301654
if prev.TFVars == nil {
16311655
return true
16321656
}
1657+
// If the set is unknown, we cannot compare and must treat it as changed.
1658+
if planned.TerraformVariables.IsUnknown() {
1659+
return true
1660+
}
1661+
// If the set is null (tf_vars not specified), treat as no variables.
1662+
// Only consider this a change if the previous version had variables.
1663+
if planned.TerraformVariables.IsNull() {
1664+
return len(prev.TFVars) > 0
1665+
}
16331666
plannedVars, diags := varsFromSet(ctx, planned.TerraformVariables)
16341667
if diags.HasError() {
16351668
return true
16361669
}
1637-
// If the set is unknown or null, we cannot compare and
1638-
// must treat it as changed.
1639-
if planned.TerraformVariables.IsUnknown() || planned.TerraformVariables.IsNull() {
1670+
if len(plannedVars) != len(prev.TFVars) {
16401671
return true
16411672
}
16421673
for _, tfVar := range plannedVars {

internal/provider/template_resource_test.go

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,12 @@ import (
1111
"text/template"
1212

1313
"github.com/google/uuid"
14+
"github.com/hashicorp/go-version"
1415
"github.com/hashicorp/terraform-plugin-framework/attr"
1516
"github.com/hashicorp/terraform-plugin-framework/types"
1617
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
1718
"github.com/hashicorp/terraform-plugin-testing/terraform"
19+
"github.com/hashicorp/terraform-plugin-testing/tfversion"
1820
cp "github.com/otiai10/copy"
1921
"github.com/stretchr/testify/require"
2022

@@ -1036,6 +1038,79 @@ resource "coderd_template" "sample" {
10361038
})
10371039
}
10381040

1041+
func TestAccTemplateResourceSensitiveTFVars(t *testing.T) {
1042+
t.Parallel()
1043+
cfg := `
1044+
provider coderd {
1045+
url = %q
1046+
token = %q
1047+
}
1048+
1049+
variable "secret" {
1050+
sensitive = true
1051+
default = %q
1052+
}
1053+
1054+
resource "time_static" "trigger" {
1055+
triggers = {
1056+
secret = var.secret
1057+
}
1058+
}
1059+
1060+
resource "coderd_template" "sample" {
1061+
name = "sensitive-tfvars-test"
1062+
versions = [
1063+
{
1064+
name = "v_${time_static.trigger.unix}"
1065+
directory = %q
1066+
active = true
1067+
tf_vars = [
1068+
{
1069+
name = "secret"
1070+
value = var.secret
1071+
}
1072+
]
1073+
}
1074+
]
1075+
}
1076+
`
1077+
1078+
ctx := t.Context()
1079+
client := integration.StartCoder(ctx, t, "template_sensitive_tfvars_acc")
1080+
1081+
exTemplate := t.TempDir()
1082+
err := cp.Copy("../../integration/template-test/example-template", exTemplate)
1083+
require.NoError(t, err)
1084+
1085+
cfgStep1 := fmt.Sprintf(cfg, client.URL.String(), client.SessionToken(), "initial-secret", exTemplate)
1086+
cfgStep2 := fmt.Sprintf(cfg, client.URL.String(), client.SessionToken(), "changed-secret", exTemplate)
1087+
1088+
resource.Test(t, resource.TestCase{
1089+
PreCheck: func() { testAccPreCheck(t) },
1090+
IsUnitTest: true,
1091+
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
1092+
// Terraform < 1.1 panics rendering plan diffs with sensitivity marks
1093+
// on nested attributes ("value is marked, so must be unmarked first").
1094+
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
1095+
tfversion.SkipBelow(version.Must(version.NewVersion("1.1.0"))),
1096+
},
1097+
ExternalProviders: map[string]resource.ExternalProvider{
1098+
"time": {
1099+
Source: "hashicorp/time",
1100+
VersionConstraint: ">=0.9.0",
1101+
},
1102+
},
1103+
Steps: []resource.TestStep{
1104+
{
1105+
Config: cfgStep1,
1106+
},
1107+
{
1108+
Config: cfgStep2,
1109+
},
1110+
},
1111+
})
1112+
}
1113+
10391114
type testAccTemplateResourceConfig struct {
10401115
URL string
10411116
Token string

0 commit comments

Comments
 (0)