Skip to content

Commit 7d612fb

Browse files
austinvalleaustinvalle
authored
ImportState: Fix panic when identity attribute is null (#499)
* add failing test * actual fix * add identity state check for readability * changelog * move nil check earlier
1 parent a72064c commit 7d612fb

File tree

4 files changed

+166
-0
lines changed

4 files changed

+166
-0
lines changed

‎.changes/unreleased/BUG FIXES-20250507-174510.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
kind: BUG FIXES
2+
body: 'ImportState: Fixes a bug where an identity attribute set to `null` would panic when testing import via identity.'
3+
time: 2025-05-07T17:45:10.221508-04:00
4+
custom:
5+
Issue: "499"

‎helper/resource/importstate/examplecloud_test.go

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,3 +479,120 @@ func examplecloudResourceWithEveryIdentitySchemaType() testprovider.Resource {
479479
},
480480
}
481481
}
482+
483+
func examplecloudResourceWithNullIdentityAttr() testprovider.Resource {
484+
return testprovider.Resource{
485+
CreateResponse: &resource.CreateResponse{
486+
NewState: tftypes.NewValue(
487+
tftypes.Object{
488+
AttributeTypes: map[string]tftypes.Type{
489+
"id": tftypes.String,
490+
"location": tftypes.String,
491+
"name": tftypes.String,
492+
},
493+
},
494+
map[string]tftypes.Value{
495+
"id": tftypes.NewValue(tftypes.String, "westeurope/somevalue"),
496+
"location": tftypes.NewValue(tftypes.String, "westeurope"),
497+
"name": tftypes.NewValue(tftypes.String, "somevalue"),
498+
},
499+
),
500+
NewIdentity: teststep.Pointer(tftypes.NewValue(
501+
tftypes.Object{
502+
AttributeTypes: map[string]tftypes.Type{
503+
"id": tftypes.String,
504+
"value_we_dont_always_need": tftypes.String,
505+
},
506+
},
507+
map[string]tftypes.Value{
508+
"id": tftypes.NewValue(tftypes.String, "westeurope/somevalue"),
509+
"value_we_dont_always_need": tftypes.NewValue(tftypes.String, nil),
510+
},
511+
)),
512+
},
513+
ReadResponse: &resource.ReadResponse{
514+
NewState: tftypes.NewValue(
515+
tftypes.Object{
516+
AttributeTypes: map[string]tftypes.Type{
517+
"id": tftypes.String,
518+
"location": tftypes.String,
519+
"name": tftypes.String,
520+
},
521+
},
522+
map[string]tftypes.Value{
523+
"id": tftypes.NewValue(tftypes.String, "westeurope/somevalue"),
524+
"location": tftypes.NewValue(tftypes.String, "westeurope"),
525+
"name": tftypes.NewValue(tftypes.String, "somevalue"),
526+
},
527+
),
528+
NewIdentity: teststep.Pointer(tftypes.NewValue(
529+
tftypes.Object{
530+
AttributeTypes: map[string]tftypes.Type{
531+
"id": tftypes.String,
532+
"value_we_dont_always_need": tftypes.String,
533+
},
534+
},
535+
map[string]tftypes.Value{
536+
"id": tftypes.NewValue(tftypes.String, "westeurope/somevalue"),
537+
"value_we_dont_always_need": tftypes.NewValue(tftypes.String, nil),
538+
},
539+
)),
540+
},
541+
ImportStateResponse: &resource.ImportStateResponse{
542+
State: tftypes.NewValue(
543+
tftypes.Object{
544+
AttributeTypes: map[string]tftypes.Type{
545+
"id": tftypes.String,
546+
"location": tftypes.String,
547+
"name": tftypes.String,
548+
},
549+
},
550+
map[string]tftypes.Value{
551+
"id": tftypes.NewValue(tftypes.String, "westeurope/somevalue"),
552+
"location": tftypes.NewValue(tftypes.String, "westeurope"),
553+
"name": tftypes.NewValue(tftypes.String, "somevalue"),
554+
},
555+
),
556+
Identity: teststep.Pointer(tftypes.NewValue(
557+
tftypes.Object{
558+
AttributeTypes: map[string]tftypes.Type{
559+
"id": tftypes.String,
560+
"value_we_dont_always_need": tftypes.String,
561+
},
562+
},
563+
map[string]tftypes.Value{
564+
"id": tftypes.NewValue(tftypes.String, "westeurope/somevalue"),
565+
"value_we_dont_always_need": tftypes.NewValue(tftypes.String, nil),
566+
},
567+
)),
568+
},
569+
SchemaResponse: &resource.SchemaResponse{
570+
Schema: &tfprotov6.Schema{
571+
Block: &tfprotov6.SchemaBlock{
572+
Attributes: []*tfprotov6.SchemaAttribute{
573+
ComputedStringAttribute("id"),
574+
RequiredStringAttribute("location"),
575+
RequiredStringAttribute("name"),
576+
},
577+
},
578+
},
579+
},
580+
IdentitySchemaResponse: &resource.IdentitySchemaResponse{
581+
Schema: &tfprotov6.ResourceIdentitySchema{
582+
Version: 1,
583+
IdentityAttributes: []*tfprotov6.ResourceIdentitySchemaAttribute{
584+
{
585+
Name: "id",
586+
Type: tftypes.String,
587+
RequiredForImport: true,
588+
},
589+
{
590+
Name: "value_we_dont_always_need",
591+
Type: tftypes.String,
592+
OptionalForImport: true,
593+
},
594+
},
595+
},
596+
},
597+
}
598+
}

‎helper/resource/importstate/import_block_with_resource_identity_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import (
1111

1212
"github.com/hashicorp/terraform-plugin-testing/internal/testing/testprovider"
1313
"github.com/hashicorp/terraform-plugin-testing/internal/testing/testsdk/providerserver"
14+
"github.com/hashicorp/terraform-plugin-testing/knownvalue"
15+
"github.com/hashicorp/terraform-plugin-testing/statecheck"
1416
"github.com/hashicorp/terraform-plugin-testing/tfversion"
1517

1618
r "github.com/hashicorp/terraform-plugin-testing/helper/resource"
@@ -47,6 +49,43 @@ func TestImportBlock_WithResourceIdentity(t *testing.T) {
4749
})
4850
}
4951

52+
func TestImportBlock_WithResourceIdentity_NullAttribute(t *testing.T) {
53+
t.Parallel()
54+
55+
r.UnitTest(t, r.TestCase{
56+
TerraformVersionChecks: []tfversion.TerraformVersionCheck{
57+
tfversion.SkipBelow(tfversion.Version1_12_0), // ImportBlockWithResourceIdentity requires Terraform 1.12.0 or later
58+
},
59+
ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){
60+
"examplecloud": providerserver.NewProviderServer(testprovider.Provider{
61+
Resources: map[string]testprovider.Resource{
62+
"examplecloud_container": examplecloudResourceWithNullIdentityAttr(),
63+
},
64+
}),
65+
},
66+
Steps: []r.TestStep{
67+
{
68+
Config: `
69+
resource "examplecloud_container" "test" {
70+
location = "westeurope"
71+
name = "somevalue"
72+
}`,
73+
ConfigStateChecks: []statecheck.StateCheck{
74+
statecheck.ExpectIdentity("examplecloud_container.test", map[string]knownvalue.Check{
75+
"id": knownvalue.StringExact("westeurope/somevalue"),
76+
"value_we_dont_always_need": knownvalue.Null(), // This value will not be brought over to import config
77+
}),
78+
},
79+
},
80+
{
81+
ResourceName: "examplecloud_container.test",
82+
ImportState: true,
83+
ImportStateKind: r.ImportBlockWithResourceIdentity,
84+
},
85+
},
86+
})
87+
}
88+
5089
func TestImportBlock_WithResourceIdentity_WithEveryType(t *testing.T) {
5190
t.Parallel()
5291

‎helper/resource/testing_new_import_state.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,11 @@ func appendImportBlockWithIdentity(config teststep.Config, resourceName string,
445445
resourceName))
446446

447447
for k, v := range identityValues {
448+
// It's valid for identity attributes to be null, we can just omit it from config
449+
if v == nil {
450+
continue
451+
}
452+
448453
switch v := v.(type) {
449454
case bool:
450455
configBuilder.WriteString(fmt.Sprintf(` %q = %t`+"\n", k, v))

0 commit comments

Comments
 (0)