Skip to content

Commit 586f314

Browse files
uturunku1uturunku1
authored
Merge pull request #1658 from hashicorp/refactor
update tfe_variable and tfe_test_variable to use helpers.NewWriteOnly…
2 parents 58d1c0e + 8facf2f commit 586f314

File tree

4 files changed

+61
-226
lines changed

4 files changed

+61
-226
lines changed

‎internal/provider/attribute_helpers.go‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,12 @@ package provider
55

66
import (
77
"context"
8-
"crypto/sha256"
9-
"encoding/hex"
108

119
"github.com/hashicorp/terraform-plugin-framework/diag"
1210
"github.com/hashicorp/terraform-plugin-framework/path"
1311
"github.com/hashicorp/terraform-plugin-framework/types"
1412
)
1513

16-
const (
17-
ValueWOHashedPrivateKey = "value_wo_hashed"
18-
)
19-
2014
// AttrGettable is a small enabler for helper functions that need to read one
2115
// attribute of a Configuration, Plan, or State.
2216
type AttrGettable interface {
@@ -45,9 +39,3 @@ func (c *ConfiguredClient) dataOrDefaultOrganization(ctx context.Context, data A
4539

4640
return diags
4741
}
48-
49-
func generateSHA256Hash(data string) string {
50-
hasher := sha256.New()
51-
hasher.Write([]byte(data))
52-
return hex.EncodeToString(hasher.Sum(nil))
53-
}

‎internal/provider/resource_tfe_organization_run_task.go‎

Lines changed: 20 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,8 @@ package provider
55

66
import (
77
"context"
8-
"encoding/json"
98
"errors"
109
"fmt"
11-
"log"
1210
"strings"
1311

1412
tfe "github.com/hashicorp/go-tfe"
@@ -25,6 +23,8 @@ import (
2523
"github.com/hashicorp/terraform-plugin-framework/types"
2624
"github.com/hashicorp/terraform-plugin-log/tflog"
2725

26+
"github.com/hashicorp/terraform-provider-tfe/internal/provider/helpers"
27+
"github.com/hashicorp/terraform-provider-tfe/internal/provider/planmodifiers"
2828
customValidators "github.com/hashicorp/terraform-provider-tfe/internal/provider/validators"
2929
)
3030

@@ -154,7 +154,7 @@ func (r *resourceOrgRunTask) Schema(ctx context.Context, req resource.SchemaRequ
154154
stringvalidator.ConflictsWith(path.MatchRoot("hmac_key")),
155155
},
156156
PlanModifiers: []planmodifier.String{
157-
&replaceHMACKeyWOPlanModifier{},
157+
planmodifiers.NewReplaceForWriteOnlyStringValue("hmac_key_wo"),
158158
},
159159
},
160160
"enabled": schema.BoolAttribute{
@@ -171,67 +171,6 @@ func (r *resourceOrgRunTask) Schema(ctx context.Context, req resource.SchemaRequ
171171
}
172172
}
173173

174-
func (r *resourceOrgRunTask) isWriteOnlyHMACKeyInPrivateState(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) bool {
175-
storedValueWO, diags := req.Private.GetKey(ctx, "hmac_key_wo")
176-
resp.Diagnostics.Append(diags...)
177-
return len(storedValueWO) != 0
178-
}
179-
180-
type replaceHMACKeyWOPlanModifier struct{}
181-
182-
func (v *replaceHMACKeyWOPlanModifier) Description(ctx context.Context) string {
183-
return "The resource will be replaced when the value of hmac_key_wo has changed"
184-
}
185-
186-
func (v *replaceHMACKeyWOPlanModifier) MarkdownDescription(ctx context.Context) string {
187-
return v.Description(ctx)
188-
}
189-
190-
func (v *replaceHMACKeyWOPlanModifier) PlanModifyString(ctx context.Context, request planmodifier.StringRequest, response *planmodifier.StringResponse) {
191-
// Write-only argument values cannot produce a Terraform plan difference. The prior state value for a write-only argument will always be null and the planned state value will also be null, therefore, it cannot produce a diff on its own. The one exception to this case is if the write-only argument is added to requires_replace during Plan Modification, in that case, the write-only argument will always cause a diff/trigger a resource recreation.
192-
var configHMACKeyWO types.String
193-
diag := request.Config.GetAttribute(ctx, path.Root("hmac_key_wo"), &configHMACKeyWO)
194-
response.Diagnostics.Append(diag...)
195-
if response.Diagnostics.HasError() {
196-
return
197-
}
198-
199-
storedHMACWO, diags := request.Private.GetKey(ctx, "hmac_key_wo")
200-
response.Diagnostics.Append(diags...)
201-
if response.Diagnostics.HasError() {
202-
return
203-
}
204-
205-
if configHMACKeyWO.IsNull() {
206-
if len(storedHMACWO) != 0 {
207-
response.RequiresReplace = true
208-
}
209-
return
210-
}
211-
212-
if len(storedHMACWO) == 0 {
213-
log.Printf("[DEBUG] Replacing resource because `hmac_key_wo` attribute has been added to a pre-existing variable resource")
214-
response.RequiresReplace = true
215-
return
216-
}
217-
218-
var hashedStoredHMACWO string
219-
err := json.Unmarshal(storedHMACWO, &hashedStoredHMACWO)
220-
if err != nil {
221-
response.Diagnostics.AddError("Error unmarshalling stored hmac_key_wo", err.Error())
222-
return
223-
}
224-
225-
hashedConfigHMACKeyWO := generateSHA256Hash(configHMACKeyWO.ValueString())
226-
227-
// when an ephemeral value is being used, they will generate a new token on every run.
228-
// So the previous hmac_key_wo will not match the current one.
229-
if hashedStoredHMACWO != hashedConfigHMACKeyWO {
230-
log.Printf("[DEBUG] Replacing resource because the value of `hmac_key_wo` attribute has changed")
231-
response.RequiresReplace = true
232-
}
233-
}
234-
235174
func (r *resourceOrgRunTask) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) {
236175
var state modelTFEOrganizationRunTaskV0
237176

@@ -254,12 +193,13 @@ func (r *resourceOrgRunTask) Read(ctx context.Context, req resource.ReadRequest,
254193
return
255194
}
256195

257-
isWriteOnlyValue := r.isWriteOnlyHMACKeyInPrivateState(ctx, req, resp) // to avoid reading from written-only values
258-
if resp.Diagnostics.HasError() {
196+
isWriteOnly, diags := r.writeOnlyValueStore(resp.Private).PriorValueExists(ctx)
197+
resp.Diagnostics.Append(diags...)
198+
if diags.HasError() {
259199
return
260200
}
261201
// update state
262-
result := modelFromTFEOrganizationRunTask(task, state.HMACKey, isWriteOnlyValue)
202+
result := modelFromTFEOrganizationRunTask(task, state.HMACKey, isWriteOnly)
263203
// Save updated data into Terraform state
264204
resp.Diagnostics.Append(resp.State.Set(ctx, &result)...)
265205
}
@@ -294,7 +234,7 @@ func (r *resourceOrgRunTask) Create(ctx context.Context, req resource.CreateRequ
294234
Enabled: plan.Enabled.ValueBoolPointer(),
295235
Description: plan.Description.ValueStringPointer(),
296236
}
297-
237+
// Set Value from "hmac_key_wo" if set, otherwise use the normal value
298238
if !config.HMACKeyWO.IsNull() {
299239
options.HMACKey = config.HMACKeyWO.ValueStringPointer()
300240
} else {
@@ -309,16 +249,9 @@ func (r *resourceOrgRunTask) Create(ctx context.Context, req resource.CreateRequ
309249
}
310250

311251
result := modelFromTFEOrganizationRunTask(task, plan.HMACKey, !config.HMACKeyWO.IsNull())
312-
if !config.HMACKeyWO.IsNull() {
313-
// Use the resource's private state to store secure hashes of write-only argument values, the provider during planmodify will use the hash to determine if a write-only argument value has changed in later Terraform runs.
314-
hashedValue := generateSHA256Hash(config.HMACKeyWO.ValueString())
315-
diags := resp.Private.SetKey(ctx, "hmac_key_wo", fmt.Appendf(nil, `"%s"`, hashedValue))
316-
resp.Diagnostics.Append(diags...)
317-
} else {
318-
// if the key is not configured as write-only, then remove HMACKeyWO key from private state. Setting a key with an empty byte slice is interpreted by the framework as a request to remove the key from the ProviderData map.
319-
diags := resp.Private.SetKey(ctx, "hmac_key_wo", []byte(""))
320-
resp.Diagnostics.Append(diags...)
321-
}
252+
// Store the hashed write-only value in the private state
253+
store := r.writeOnlyValueStore(resp.Private)
254+
resp.Diagnostics.Append(store.SetPriorValue(ctx, config.HMACKeyWO)...)
322255

323256
// Save data into Terraform state
324257
resp.Diagnostics.Append(resp.State.Set(ctx, &result)...)
@@ -371,9 +304,14 @@ func (r *resourceOrgRunTask) Update(ctx context.Context, req resource.UpdateRequ
371304
return
372305
}
373306

374-
result := modelFromTFEOrganizationRunTask(task, plan.HMACKey, !config.HMACKeyWO.IsNull())
375-
r.updatePrivateState(ctx, resp, config.HMACKeyWO)
307+
// Store the hashed write-only value in the private state
308+
store := r.writeOnlyValueStore(resp.Private)
309+
resp.Diagnostics.Append(store.SetPriorValue(ctx, config.HMACKeyWO)...)
310+
if resp.Diagnostics.HasError() {
311+
return
312+
}
376313

314+
result := modelFromTFEOrganizationRunTask(task, plan.HMACKey, !config.HMACKeyWO.IsNull())
377315
// Save data into Terraform state
378316
resp.Diagnostics.Append(resp.State.Set(ctx, &result)...)
379317
}
@@ -430,17 +368,6 @@ func (r *resourceOrgRunTask) ImportState(ctx context.Context, req resource.Impor
430368
}
431369
}
432370

433-
func (r *resourceOrgRunTask) updatePrivateState(ctx context.Context, resp *resource.UpdateResponse, configHMACKeyWO types.String) {
434-
if !configHMACKeyWO.IsNull() {
435-
// Use the resource's private state to store secure hashes of write-only argument values, planModify will use the hash to determine if a write-only argument value has changed in later Terraform runs.
436-
hashedValue := generateSHA256Hash(configHMACKeyWO.ValueString())
437-
diags := resp.Private.SetKey(ctx, "hmac_key_wo", fmt.Appendf(nil, `"%s"`, hashedValue))
438-
resp.Diagnostics.Append(diags...)
439-
} else {
440-
// if key is not configured as write-only, remove hmacKeyWO key from private state
441-
diags := resp.Private.SetKey(ctx, "hmac_key_wo", []byte(""))
442-
resp.Diagnostics.Append(diags...)
443-
}
371+
func (r *resourceOrgRunTask) writeOnlyValueStore(private helpers.PrivateState) *helpers.WriteOnlyValueStore {
372+
return helpers.NewWriteOnlyValueStore(private, "hmac_key_wo")
444373
}
445-
446-
var _ planmodifier.String = &replaceHMACKeyWOPlanModifier{}

‎internal/provider/resource_tfe_test_variable.go‎

Lines changed: 18 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import (
2222
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
2323
"github.com/hashicorp/terraform-plugin-framework/types"
2424
"github.com/hashicorp/terraform-plugin-log/tflog"
25+
"github.com/hashicorp/terraform-provider-tfe/internal/provider/helpers"
26+
"github.com/hashicorp/terraform-provider-tfe/internal/provider/planmodifiers"
2527
)
2628

2729
type resourceTFETestVariable struct {
@@ -152,7 +154,7 @@ func (r *resourceTFETestVariable) Schema(ctx context.Context, req resource.Schem
152154
stringvalidator.ConflictsWith(path.MatchRoot("value")),
153155
},
154156
PlanModifiers: []planmodifier.String{
155-
&replaceValueWOPlanModifier{},
157+
planmodifiers.NewReplaceForWriteOnlyStringValue("value_wo"),
156158
},
157159
},
158160
"category": schema.StringAttribute{
@@ -250,7 +252,7 @@ func (r *resourceTFETestVariable) Create(ctx context.Context, req resource.Creat
250252
Sensitive: data.Sensitive.ValueBoolPointer(),
251253
Description: data.Description.ValueStringPointer(),
252254
}
253-
255+
// Set Value from `value_wo` if set, otherwise use the normal value
254256
if !config.ValueWO.IsNull() {
255257
options.Value = config.ValueWO.ValueStringPointer()
256258
} else {
@@ -270,17 +272,9 @@ func (r *resourceTFETestVariable) Create(ctx context.Context, req resource.Creat
270272
// We got a variable, so set state to new values
271273
result := modelFromTFETestVariable(*variable, data.Value, moduleID, !config.ValueWO.IsNull())
272274

273-
if !config.ValueWO.IsNull() {
274-
// Use the resource's private state to store secure hashes of write-only argument values, the provider during planmodify will use the hash to determine if a write-only argument value has changed in later Terraform runs.
275-
hashedValue := generateSHA256Hash(config.ValueWO.ValueString())
276-
diags := resp.Private.SetKey(ctx, ValueWOHashedPrivateKey, fmt.Appendf(nil, `"%s"`, hashedValue))
277-
resp.Diagnostics.Append(diags...)
278-
} else {
279-
// if the value is not configured as write-only, then remove valueWO key from private state. Setting a key with an empty byte slice is interpreted by the framework as a request to remove the key from the ProviderData map.
280-
diags := resp.Private.SetKey(ctx, ValueWOHashedPrivateKey, []byte(""))
281-
resp.Diagnostics.Append(diags...)
282-
}
283-
275+
// Store the hashed write-only value in the private state
276+
store := r.writeOnlyValueStore(resp.Private)
277+
resp.Diagnostics.Append(store.SetPriorValue(ctx, config.ValueWO)...)
284278
if resp.Diagnostics.HasError() {
285279
return
286280
}
@@ -322,13 +316,14 @@ func (r *resourceTFETestVariable) Read(ctx context.Context, req resource.ReadReq
322316
return
323317
}
324318

325-
isWriteOnlyValue := isWriteOnlyValueInPrivateState(req, resp) // to avoid reading from written-only values
326-
if resp.Diagnostics.HasError() {
319+
isWriteOnly, diags := r.writeOnlyValueStore(resp.Private).PriorValueExists(ctx)
320+
resp.Diagnostics.Append(diags...)
321+
if diags.HasError() {
327322
return
328323
}
329324

330325
// We got a variable, so update state:
331-
result := modelFromTFETestVariable(*variable, data.Value, moduleID, isWriteOnlyValue)
326+
result := modelFromTFETestVariable(*variable, data.Value, moduleID, isWriteOnly)
332327
diags = resp.State.Set(ctx, &result)
333328
resp.Diagnostics.Append(diags...)
334329
}
@@ -383,12 +378,14 @@ func (r *resourceTFETestVariable) Update(ctx context.Context, req resource.Updat
383378
)
384379
return
385380
}
386-
// Update state
387-
result := modelFromTFETestVariable(*variable, plan.Value, moduleID, !config.ValueWO.IsNull())
388-
r.updatePrivateState(ctx, resp, config.ValueWO)
381+
// Store the hashed write-only value in the private state
382+
store := r.writeOnlyValueStore(resp.Private)
383+
resp.Diagnostics.Append(store.SetPriorValue(ctx, config.ValueWO)...)
389384
if resp.Diagnostics.HasError() {
390385
return
391386
}
387+
// Update state
388+
result := modelFromTFETestVariable(*variable, plan.Value, moduleID, !config.ValueWO.IsNull())
392389
diags = resp.State.Set(ctx, &result)
393390
resp.Diagnostics.Append(diags...)
394391
}
@@ -421,15 +418,6 @@ func (r *resourceTFETestVariable) Delete(ctx context.Context, req resource.Delet
421418
// Resource is implicitly deleted from resp.State if diagnostics have no errors.
422419
}
423420

424-
func (r *resourceTFETestVariable) updatePrivateState(ctx context.Context, resp *resource.UpdateResponse, configValueWO types.String) {
425-
if !configValueWO.IsNull() {
426-
// Use the resource's private state to store secure hashes of write-only argument values, planModify will use the hash to determine if a write-only argument value has changed in later Terraform runs.
427-
hashedValue := generateSHA256Hash(configValueWO.ValueString())
428-
diags := resp.Private.SetKey(ctx, ValueWOHashedPrivateKey, fmt.Appendf(nil, `"%s"`, hashedValue))
429-
resp.Diagnostics.Append(diags...)
430-
} else {
431-
// if value is not configured as write-only, remove valueWO key from private state
432-
diags := resp.Private.SetKey(ctx, ValueWOHashedPrivateKey, []byte(""))
433-
resp.Diagnostics.Append(diags...)
434-
}
421+
func (r *resourceTFETestVariable) writeOnlyValueStore(private helpers.PrivateState) *helpers.WriteOnlyValueStore {
422+
return helpers.NewWriteOnlyValueStore(private, "value_wo")
435423
}

0 commit comments

Comments
 (0)