Skip to content

Commit 805da5a

Browse files
stephybunstephybun
committed
run plan and check for no-op when importing via a block
1 parent 6cb9373 commit 805da5a

File tree

2 files changed

+171
-30
lines changed

2 files changed

+171
-30
lines changed

‎helper/resource/testing_new_import_block_id_test.go‎

Lines changed: 129 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44
package resource
55

66
import (
7-
"context"
87
"fmt"
98
"github.com/hashicorp/terraform-plugin-testing/internal/testing/testsdk/datasource"
109
"github.com/hashicorp/terraform-plugin-testing/terraform"
10+
"regexp"
1111
"testing"
1212

1313
"github.com/hashicorp/terraform-plugin-go/tfprotov6"
@@ -46,6 +46,126 @@ func TestTest_TestStep_ImportBlockId(t *testing.T) {
4646
},
4747
),
4848
},
49+
ReadResponse: &resource.ReadResponse{
50+
NewState: tftypes.NewValue(
51+
tftypes.Object{
52+
AttributeTypes: map[string]tftypes.Type{
53+
"id": tftypes.String,
54+
"location": tftypes.String,
55+
"name": tftypes.String,
56+
},
57+
},
58+
map[string]tftypes.Value{
59+
"id": tftypes.NewValue(tftypes.String, "westeurope/somevalue"),
60+
"location": tftypes.NewValue(tftypes.String, "westeurope"),
61+
"name": tftypes.NewValue(tftypes.String, "somevalue"),
62+
},
63+
),
64+
},
65+
ImportStateResponse: &resource.ImportStateResponse{
66+
State: tftypes.NewValue(
67+
tftypes.Object{
68+
AttributeTypes: map[string]tftypes.Type{
69+
"id": tftypes.String,
70+
"location": tftypes.String,
71+
"name": tftypes.String,
72+
},
73+
},
74+
map[string]tftypes.Value{
75+
"id": tftypes.NewValue(tftypes.String, "westeurope/somevalue"),
76+
"location": tftypes.NewValue(tftypes.String, "westeurope"),
77+
"name": tftypes.NewValue(tftypes.String, "somevalue"),
78+
},
79+
),
80+
},
81+
SchemaResponse: &resource.SchemaResponse{
82+
Schema: &tfprotov6.Schema{
83+
Block: &tfprotov6.SchemaBlock{
84+
Attributes: []*tfprotov6.SchemaAttribute{
85+
{
86+
Name: "id",
87+
Type: tftypes.String,
88+
Computed: true,
89+
},
90+
{
91+
Name: "location",
92+
Type: tftypes.String,
93+
Required: true,
94+
},
95+
{
96+
Name: "name",
97+
Type: tftypes.String,
98+
Required: true,
99+
},
100+
},
101+
},
102+
},
103+
},
104+
},
105+
},
106+
}),
107+
},
108+
Steps: []TestStep{
109+
{
110+
Config: `
111+
resource "examplecloud_container" "test" {
112+
location = "westeurope"
113+
name = "somevalue"
114+
}`,
115+
},
116+
{
117+
ResourceName: "examplecloud_container.test",
118+
ImportState: true,
119+
ImportStateKind: ImportBlockWithId,
120+
ImportStateVerify: true,
121+
},
122+
},
123+
})
124+
}
125+
126+
func TestTest_TestStep_ImportBlockId_ExpectError(t *testing.T) {
127+
t.Parallel()
128+
129+
UnitTest(t, TestCase{
130+
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
131+
tfversion.SkipBelow(tfversion.Version1_5_0), // ProtoV6ProviderFactories
132+
},
133+
ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){
134+
"examplecloud": providerserver.NewProviderServer(testprovider.Provider{
135+
Resources: map[string]testprovider.Resource{
136+
"examplecloud_container": {
137+
CreateResponse: &resource.CreateResponse{
138+
NewState: tftypes.NewValue(
139+
tftypes.Object{
140+
AttributeTypes: map[string]tftypes.Type{
141+
"id": tftypes.String,
142+
"location": tftypes.String,
143+
"name": tftypes.String,
144+
},
145+
},
146+
map[string]tftypes.Value{
147+
"id": tftypes.NewValue(tftypes.String, "westeurope/somevalue"),
148+
"location": tftypes.NewValue(tftypes.String, "westeurope"),
149+
"name": tftypes.NewValue(tftypes.String, "somevalue"),
150+
},
151+
),
152+
},
153+
ReadResponse: &resource.ReadResponse{
154+
NewState: tftypes.NewValue(
155+
tftypes.Object{
156+
AttributeTypes: map[string]tftypes.Type{
157+
"id": tftypes.String,
158+
"location": tftypes.String,
159+
"name": tftypes.String,
160+
},
161+
},
162+
map[string]tftypes.Value{
163+
"id": tftypes.NewValue(tftypes.String, "westeurope/somevalue"),
164+
"location": tftypes.NewValue(tftypes.String, "westeurope"),
165+
"name": tftypes.NewValue(tftypes.String, "somevalue"),
166+
},
167+
),
168+
},
49169
ImportStateResponse: &resource.ImportStateResponse{
50170
State: tftypes.NewValue(
51171
tftypes.Object{
@@ -98,10 +218,16 @@ func TestTest_TestStep_ImportBlockId(t *testing.T) {
98218
}`,
99219
},
100220
{
221+
Config: `
222+
resource "examplecloud_container" "test" {
223+
location = "eastus"
224+
name = "somevalue"
225+
}`,
101226
ResourceName: "examplecloud_container.test",
102227
ImportState: true,
103228
ImportStateKind: ImportBlockWithId,
104229
ImportStateVerify: true,
230+
ExpectError: regexp.MustCompile(`importing resource examplecloud_container.test should be a no-op, but got action update with plan(.?)`),
105231
},
106232
},
107233
})
@@ -211,6 +337,8 @@ func TestTest_TestStep_ImportBlockId_SkipDataSourceState(t *testing.T) {
211337
})
212338
}
213339

340+
// These tests currently pass but only because the `getState` function which is used on the imported resource
341+
// to do the state comparison doesn't return an error if there is no state in the working directory
214342
func TestTest_TestStep_ImportBlockId_ImportStateVerifyIgnore_Real_Example(t *testing.T) {
215343
/*
216344
This test tries to imitate a real world example of behaviour we often see in the AzureRM provider which requires
@@ -283,33 +411,6 @@ func TestTest_TestStep_ImportBlockId_ImportStateVerifyIgnore_Real_Example(t *tes
283411
},
284412
),
285413
},
286-
PlanChangeFunc: func(ctx context.Context, request resource.PlanChangeRequest, response *resource.PlanChangeResponse) {
287-
/*
288-
Returning a nil for another attribute to simulate a situation where `ImportStateVerifyIgnore`
289-
should be used results in the error below from Terraform
290-
291-
Error: Provider returned invalid result object after apply
292-
293-
After the apply operation, the provider still indicated an unknown value for
294-
examplecloud_container.test.id. All values must be known after apply, so this
295-
is always a bug in the provider and should be reported in the provider's own
296-
repository. Terraform will still save the other known object values in the
297-
state.
298-
299-
Modifying the plan to set the id to a known value appears to be the only way to
300-
circumvent this behaviour, the cause of which I don't fully understand.
301-
302-
This doesn't seem great, because this gets applied to all Plans that happen in this
303-
test - so we're modifying plans in steps that we might not want to.
304-
*/
305-
306-
objVal := map[string]tftypes.Value{}
307-
308-
if !response.PlannedState.IsNull() {
309-
_ = response.PlannedState.As(&objVal)
310-
objVal["id"] = tftypes.NewValue(tftypes.String, "sometestid")
311-
}
312-
},
313414
SchemaResponse: &resource.SchemaResponse{
314415
Schema: &tfprotov6.Schema{
315416
Block: &tfprotov6.SchemaBlock{

‎helper/resource/testing_new_import_state.go‎

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package resource
66
import (
77
"context"
88
"fmt"
9+
tfjson "github.com/hashicorp/terraform-json"
910
"reflect"
1011
"strings"
1112

@@ -166,14 +167,53 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest
166167
}
167168

168169
if step.ImportStateKind == ImportBlockWithResourceIdentity || step.ImportStateKind == ImportBlockWithId {
169-
var opts []tfexec.ApplyOption
170+
var opts []tfexec.PlanOption
170171

171172
err = runProviderCommand(ctx, t, func() error {
172-
return importWd.Apply(ctx, opts...)
173+
return importWd.CreatePlan(ctx, opts...)
173174
}, importWd, providers)
174175
if err != nil {
175176
return err
176177
}
178+
179+
var plan *tfjson.Plan
180+
err = runProviderCommand(ctx, t, func() error {
181+
var err error
182+
plan, err = importWd.SavedPlan(ctx)
183+
return err
184+
}, importWd, providers)
185+
if err != nil {
186+
return err
187+
}
188+
189+
if plan.ResourceChanges != nil {
190+
for _, rc := range plan.ResourceChanges {
191+
if rc.Address != step.ResourceName {
192+
// we're only interested in the changes for the resource being imported
193+
continue
194+
}
195+
if rc.Change != nil && rc.Change.Actions != nil {
196+
// should this be length checked and used as a condition, if it's a no-op then there shouldn't be any other changes here
197+
for _, action := range rc.Change.Actions {
198+
if action != "no-op" {
199+
var stdout string
200+
err = runProviderCommand(ctx, t, func() error {
201+
var err error
202+
stdout, err = importWd.SavedPlanRawStdout(ctx)
203+
return err
204+
}, importWd, providers)
205+
if err != nil {
206+
return fmt.Errorf("retrieving formatted plan output: %w", err)
207+
}
208+
209+
return fmt.Errorf("importing resource %s should be a no-op, but got action %s with plan \\nstdout:\\n\\n%s", rc.Address, action, stdout)
210+
}
211+
}
212+
}
213+
}
214+
}
215+
216+
// TODO compare plan to state from previous step
177217
} else {
178218
err = runProviderCommand(ctx, t, func() error {
179219
return importWd.Import(ctx, step.ResourceName, importId)

0 commit comments

Comments
 (0)