Skip to content

Commit c4a2f74

Browse files
jbardinjbardin
authored
Merge pull request #34567 from hashicorp/jbardin/handling-instance-value-marks
apply schema marks to returned instance values
2 parents 9658f9d + 8994e91 commit c4a2f74

File tree

9 files changed

+238
-126
lines changed

9 files changed

+238
-126
lines changed

‎internal/command/testdata/show-json-sensitive/output.json‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@
2828
"password": "secret"
2929
},
3030
"sensitive_values": {
31-
"ami": true
31+
"ami": true,
32+
"password": true
3233
}
3334
},
3435
{
@@ -44,7 +45,8 @@
4445
"password": "secret"
4546
},
4647
"sensitive_values": {
47-
"ami": true
48+
"ami": true,
49+
"password": true
4850
}
4951
},
5052
{
@@ -60,7 +62,8 @@
6062
"password": "secret"
6163
},
6264
"sensitive_values": {
63-
"ami": true
65+
"ami": true,
66+
"password": true
6467
}
6568
}
6669
]

‎internal/command/testdata/show-json/conditions/for-refresh.tfstate‎

Lines changed: 48 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,55 @@
11
{
2-
"version": 4,
3-
"terraform_version": "1.2.0-dev",
4-
"serial": 1,
5-
"lineage": "no",
6-
"outputs": {},
7-
"resources": [
2+
"version": 4,
3+
"terraform_version": "1.2.0-dev",
4+
"serial": 1,
5+
"lineage": "no",
6+
"outputs": {},
7+
"resources": [
8+
{
9+
"mode": "managed",
10+
"type": "test_instance",
11+
"name": "foo",
12+
"provider": "provider[\"registry.terraform.io/hashicorp/test\"]",
13+
"instances": [
814
{
9-
"mode": "managed",
10-
"type": "test_instance",
11-
"name": "foo",
12-
"provider": "provider[\"registry.terraform.io/hashicorp/test\"]",
13-
"instances": [
14-
{
15-
"schema_version": 0,
16-
"attributes": {
17-
"ami": "ami-test",
18-
"id": "placeholder"
19-
}
20-
}
15+
"schema_version": 0,
16+
"attributes": {
17+
"ami": "ami-test",
18+
"id": "placeholder"
19+
},
20+
"sensitive_attributes": [
21+
[
22+
{
23+
"type": "get_attr",
24+
"value": "password"
25+
}
2126
]
22-
},
27+
]
28+
}
29+
]
30+
},
31+
{
32+
"mode": "managed",
33+
"type": "test_instance",
34+
"name": "bar",
35+
"provider": "provider[\"registry.terraform.io/hashicorp/test\"]",
36+
"instances": [
2337
{
24-
"mode": "managed",
25-
"type": "test_instance",
26-
"name": "bar",
27-
"provider": "provider[\"registry.terraform.io/hashicorp/test\"]",
28-
"instances": [
29-
{
30-
"schema_version": 0,
31-
"attributes": {
32-
"ami": "ami-test",
33-
"id": "placeheld"
34-
}
35-
}
38+
"schema_version": 0,
39+
"attributes": {
40+
"ami": "ami-test",
41+
"id": "placeheld"
42+
},
43+
"sensitive_attributes": [
44+
[
45+
{
46+
"type": "get_attr",
47+
"value": "password"
48+
}
3649
]
50+
]
3751
}
38-
]
52+
]
53+
}
54+
]
3955
}

‎internal/command/testdata/show-json/conditions/output-refresh-only.json‎

Lines changed: 65 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
2-
"format_version": "1.1",
3-
"terraform_version": "1.2.0-dev",
2+
"format_version": "1.2",
3+
"terraform_version": "1.8.0-dev",
44
"variables": {
55
"ami": {
66
"value": "bad-ami"
@@ -33,13 +33,13 @@
3333
},
3434
"prior_state": {
3535
"format_version": "1.0",
36-
"terraform_version": "1.1.0",
36+
"terraform_version": "1.8.0",
3737
"values": {
3838
"outputs": {
3939
"foo_id": {
4040
"sensitive": false,
41-
"type": "string",
42-
"value": "placeholder"
41+
"value": "placeholder",
42+
"type": "string"
4343
}
4444
},
4545
"root_module": {
@@ -146,26 +146,70 @@
146146
]
147147
}
148148
],
149-
"condition_results": [
149+
"checks": [
150150
{
151-
"address": "output.foo_id",
152-
"condition_type": "OutputPrecondition",
153-
"result": true,
154-
"unknown": false
151+
"address": {
152+
"kind": "output_value",
153+
"name": "foo_id",
154+
"to_display": "output.foo_id"
155+
},
156+
"status": "pass",
157+
"instances": [
158+
{
159+
"address": {
160+
"to_display": "output.foo_id"
161+
},
162+
"status": "pass"
163+
}
164+
]
155165
},
156166
{
157-
"address": "test_instance.bar",
158-
"condition_type": "ResourcePostcondition",
159-
"result": false,
160-
"unknown": false,
161-
"error_message": "Resource ID is unacceptably short (9 characters)."
167+
"address": {
168+
"kind": "resource",
169+
"mode": "managed",
170+
"name": "bar",
171+
"to_display": "test_instance.bar",
172+
"type": "test_instance"
173+
},
174+
"status": "fail",
175+
"instances": [
176+
{
177+
"address": {
178+
"to_display": "test_instance.bar"
179+
},
180+
"status": "fail",
181+
"problems": [
182+
{
183+
"message": "Resource ID is unacceptably short (9 characters)."
184+
}
185+
]
186+
}
187+
]
162188
},
163189
{
164-
"address": "test_instance.foo",
165-
"condition_type": "ResourcePrecondition",
166-
"result": false,
167-
"unknown": false,
168-
"error_message": "Invalid AMI ID: must start with \"ami-\"."
190+
"address": {
191+
"kind": "resource",
192+
"mode": "managed",
193+
"name": "foo",
194+
"to_display": "test_instance.foo",
195+
"type": "test_instance"
196+
},
197+
"status": "fail",
198+
"instances": [
199+
{
200+
"address": {
201+
"to_display": "test_instance.foo"
202+
},
203+
"status": "fail",
204+
"problems": [
205+
{
206+
"message": "Invalid AMI ID: must start with \"ami-\"."
207+
}
208+
]
209+
}
210+
]
169211
}
170-
]
212+
],
213+
"timestamp": "2024-01-24T18:33:05Z",
214+
"errored": false
171215
}

‎internal/terraform/context_apply_test.go‎

Lines changed: 48 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -11971,15 +11971,22 @@ resource "test_resource" "foo" {
1197111971
}
1197211972

1197311973
verifySensitiveValue := func(pvms []cty.PathValueMarks) {
11974-
if len(pvms) != 1 {
11975-
t.Fatalf("expected 1 sensitive path, got %d", len(pvms))
11974+
if len(pvms) != 2 {
11975+
t.Fatalf("expected 2 sensitive paths, got %d", len(pvms))
1197611976
}
11977-
pvm := pvms[0]
11978-
if gotPath, wantPath := pvm.Path, cty.GetAttrPath("value"); !gotPath.Equals(wantPath) {
11979-
t.Errorf("wrong path\n got: %#v\nwant: %#v", gotPath, wantPath)
11980-
}
11981-
if gotMarks, wantMarks := pvm.Marks, cty.NewValueMarks(marks.Sensitive); !gotMarks.Equal(wantMarks) {
11982-
t.Errorf("wrong marks\n got: %#v\nwant: %#v", gotMarks, wantMarks)
11977+
11978+
for _, pvm := range pvms {
11979+
switch {
11980+
case pvm.Path.Equals(cty.GetAttrPath("value")):
11981+
case pvm.Path.Equals(cty.GetAttrPath("sensitive_value")):
11982+
default:
11983+
t.Errorf("unexpected path mark: %#v", pvm)
11984+
return
11985+
}
11986+
11987+
if want := cty.NewValueMarks(marks.Sensitive); !pvm.Marks.Equal(want) {
11988+
t.Errorf("wrong marks\n got: %#v\nwant: %#v", pvm.Marks, want)
11989+
}
1198311990
}
1198411991
}
1198511992

@@ -12034,15 +12041,19 @@ resource "test_resource" "baz" {
1203412041
}
1203512042

1203612043
verifySensitiveValue := func(pvms []cty.PathValueMarks) {
12037-
if len(pvms) != 1 {
12038-
t.Fatalf("expected 1 sensitive path, got %d", len(pvms))
12039-
}
12040-
pvm := pvms[0]
12041-
if gotPath, wantPath := pvm.Path, cty.GetAttrPath("value"); !gotPath.Equals(wantPath) {
12042-
t.Errorf("wrong path\n got: %#v\nwant: %#v", gotPath, wantPath)
12043-
}
12044-
if gotMarks, wantMarks := pvm.Marks, cty.NewValueMarks(marks.Sensitive); !gotMarks.Equal(wantMarks) {
12045-
t.Errorf("wrong marks\n got: %#v\nwant: %#v", gotMarks, wantMarks)
12044+
for _, pvm := range pvms {
12045+
switch {
12046+
case pvm.Path.Equals(cty.GetAttrPath("value")):
12047+
case pvm.Path.Equals(cty.GetAttrPath("sensitive_value")):
12048+
case pvm.Path.Equals(cty.GetAttrPath("nesting_single").GetAttr("sensitive_value")):
12049+
default:
12050+
t.Errorf("unexpected path mark: %#v", pvm)
12051+
return
12052+
}
12053+
12054+
if want := cty.NewValueMarks(marks.Sensitive); !pvm.Marks.Equal(want) {
12055+
t.Errorf("wrong marks\n got: %#v\nwant: %#v", pvm.Marks, want)
12056+
}
1204612057
}
1204712058
}
1204812059

@@ -12120,17 +12131,22 @@ resource "test_resource" "foo" {
1212012131

1212112132
fooState := state.ResourceInstance(addr)
1212212133

12123-
if len(fooState.Current.AttrSensitivePaths) != 1 {
12124-
t.Fatalf("wrong number of sensitive paths, expected 1, got, %v", len(fooState.Current.AttrSensitivePaths))
12125-
}
12126-
got := fooState.Current.AttrSensitivePaths[0]
12127-
want := cty.PathValueMarks{
12128-
Path: cty.GetAttrPath("value"),
12129-
Marks: cty.NewValueMarks(marks.Sensitive),
12134+
if len(fooState.Current.AttrSensitivePaths) != 2 {
12135+
t.Fatalf("wrong number of sensitive paths, expected 2, got, %v", len(fooState.Current.AttrSensitivePaths))
1213012136
}
1213112137

12132-
if !got.Equal(want) {
12133-
t.Fatalf("wrong value marks; got:\n%#v\n\nwant:\n%#v\n", got, want)
12138+
for _, pvm := range fooState.Current.AttrSensitivePaths {
12139+
switch {
12140+
case pvm.Path.Equals(cty.GetAttrPath("value")):
12141+
case pvm.Path.Equals(cty.GetAttrPath("sensitive_value")):
12142+
default:
12143+
t.Errorf("unexpected path mark: %#v", pvm)
12144+
return
12145+
}
12146+
12147+
if want := cty.NewValueMarks(marks.Sensitive); !pvm.Marks.Equal(want) {
12148+
t.Errorf("wrong marks\n got: %#v\nwant: %#v", pvm.Marks, want)
12149+
}
1213412150
}
1213512151

1213612152
m2 := testModuleInline(t, map[string]string{
@@ -12145,33 +12161,22 @@ resource "test_resource" "foo" {
1214512161
}`,
1214612162
})
1214712163

12148-
ctx2 := testContext2(t, &ContextOpts{
12164+
ctx = testContext2(t, &ContextOpts{
1214912165
Providers: map[addrs.Provider]providers.Factory{
1215012166
addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
1215112167
},
1215212168
})
1215312169

12154-
// NOTE: Prior to our refactoring to make the state an explicit argument
12155-
// of Plan, as opposed to hidden state inside Context, this test was
12156-
// calling ctx.Apply instead of ctx2.Apply and thus using the previous
12157-
// plan instead of this new plan. "Fixing" it to use the new plan seems
12158-
// to break the test, so we've preserved that oddity here by saving the
12159-
// old plan as oldPlan and essentially discarding the new plan entirely,
12160-
// but this seems rather suspicious and we should ideally figure out what
12161-
// this test was originally intending to do and make it do that.
12162-
oldPlan := plan
12163-
_, diags = ctx2.Plan(m2, state, SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables)))
12170+
plan, diags = ctx.Plan(m2, state, SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables)))
1216412171
assertNoErrors(t, diags)
12165-
stateWithoutSensitive, diags := ctx.Apply(oldPlan, m, nil)
12172+
stateWithoutSensitive, diags := ctx.Apply(plan, m2, nil)
1216612173
assertNoErrors(t, diags)
1216712174

1216812175
fooState2 := stateWithoutSensitive.ResourceInstance(addr)
12169-
if len(fooState2.Current.AttrSensitivePaths) > 0 {
12170-
t.Fatalf(
12171-
"wrong number of sensitive paths, expected 0, got, %v\n%s",
12176+
if len(fooState2.Current.AttrSensitivePaths) != 1 {
12177+
t.Fatalf("wrong number of sensitive paths, expected 1, got, %v\n%#v\n",
1217212178
len(fooState2.Current.AttrSensitivePaths),
12173-
spew.Sdump(fooState2.Current.AttrSensitivePaths),
12174-
)
12179+
fooState2.Current.AttrSensitivePaths)
1217512180
}
1217612181
}
1217712182

‎internal/terraform/context_plan2_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2932,7 +2932,7 @@ output "output" {
29322932
}
29332933

29342934
if res.Addr.Resource.Resource.Mode == addrs.DataResourceMode && res.Action != plans.NoOp {
2935-
t.Errorf("unexpected %s plan for %s", res.Action, res.Addr)
2935+
t.Errorf("unexpected %s/%s plan for %s", res.Action, res.ActionReason, res.Addr)
29362936
}
29372937
}
29382938
}

0 commit comments

Comments
 (0)