Merge pull request #37611 from acwwat/b-aws_storagegateway_smb_file_s…

…hare-fix_cache_attributes_crash

fix: Fix crash on update when cache_attributes block is removed for aws_storagegateway_smb_file_share

.changelog/37611.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_storagegateway_smb_file_share: Fix crash when `cache_attributes` is removed on update
+```

internal/service/storagegateway/cache.go

@@ -16,15 +16,17 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-provider-aws/internal/conns"
 	"github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag"
+	"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
 	"github.com/hashicorp/terraform-provider-aws/internal/verify"
 )
 
-// @SDKResource("aws_storagegateway_cache")
-func ResourceCache() *schema.Resource {
+// @SDKResource("aws_storagegateway_cache", name="Cache")
+func resourceCache() *schema.Resource {
 	return &schema.Resource{
 		CreateWithoutTimeout: resourceCacheCreate,
 		ReadWithoutTimeout:   resourceCacheRead,
 		DeleteWithoutTimeout: schema.NoopContext,
+
 		Importer: &schema.ResourceImporter{
 			StateContext: schema.ImportStatePassthroughContext,
 		},
@@ -51,57 +53,52 @@ func resourceCacheCreate(ctx context.Context, d *schema.ResourceData, meta inter
 
 	diskID := d.Get("disk_id").(string)
 	gatewayARN := d.Get("gateway_arn").(string)
-
-	input := &storagegateway.AddCacheInput{
-		DiskIds:    []*string{aws.String(diskID)},
+	id := cacheCreateResourceID(gatewayARN, diskID)
+	inputAC := &storagegateway.AddCacheInput{
+		DiskIds:    aws.StringSlice([]string{diskID}),
 		GatewayARN: aws.String(gatewayARN),
 	}
 
-	_, err := conn.AddCacheWithContext(ctx, input)
+	_, err := conn.AddCacheWithContext(ctx, inputAC)
+
 	if err != nil {
-		return sdkdiag.AppendErrorf(diags, "creating Storage Gateway Cache: %s", err)
+		return sdkdiag.AppendErrorf(diags, "creating Storage Gateway Cache (%s): %s", id, err)
 	}
 
-	d.SetId(fmt.Sprintf("%s:%s", gatewayARN, diskID))
+	d.SetId(id)
 
 	// Depending on the Storage Gateway software, it will sometimes relabel a local DiskId
 	// with a UUID if previously unlabeled, e.g.
 	//   Before conn.AddCache(): "DiskId": "/dev/xvdb",
 	//   After conn.AddCache(): "DiskId": "112764d7-7e83-42ce-9af3-d482985a31cc",
 	// This prevents us from successfully reading the disk after creation.
 	// Here we try to refresh the local disks to see if we can find a new DiskId.
-
-	listLocalDisksInput := &storagegateway.ListLocalDisksInput{
+	inputLLD := &storagegateway.ListLocalDisksInput{
 		GatewayARN: aws.String(gatewayARN),
 	}
-
-	log.Printf("[DEBUG] Reading Storage Gateway Local Disk: %s", listLocalDisksInput)
-	output, err := conn.ListLocalDisksWithContext(ctx, listLocalDisksInput)
-	if err != nil {
-		return sdkdiag.AppendErrorf(diags, "reading Storage Gateway Local Disk: %s", err)
-	}
-
-	if output != nil {
-		for _, disk := range output.Disks {
-			if aws.StringValue(disk.DiskId) == diskID || aws.StringValue(disk.DiskNode) == diskID || aws.StringValue(disk.DiskPath) == diskID {
-				diskID = aws.StringValue(disk.DiskId)
-				break
-			}
-		}
+	disk, err := findLocalDisk(ctx, conn, inputLLD, func(v *storagegateway.Disk) bool {
+		return aws.StringValue(v.DiskId) == diskID || aws.StringValue(v.DiskNode) == diskID || aws.StringValue(v.DiskPath) == diskID
+	})
+
+	switch {
+	case tfresource.NotFound(err):
+	case err != nil:
+		return sdkdiag.AppendErrorf(diags, "reading Storage Gateway local disk: %s", err)
+	default:
+		id = cacheCreateResourceID(gatewayARN, aws.StringValue(disk.DiskId))
+		d.SetId(id)
 	}
 
-	d.SetId(fmt.Sprintf("%s:%s", gatewayARN, diskID))
-
 	return append(diags, resourceCacheRead(ctx, d, meta)...)
 }
 
 func resourceCacheRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	var diags diag.Diagnostics
 	conn := meta.(*conns.AWSClient).StorageGatewayConn(ctx)
 
-	gatewayARN, diskID, err := DecodeCacheID(d.Id())
+	gatewayARN, diskID, err := cacheParseResourceID(d.Id())
 	if err != nil {
-		return sdkdiag.AppendErrorf(diags, "reading Storage Gateway Cache (%s): %s", d.Id(), err)
+		return sdkdiag.AppendFromErr(diags, err)
 	}
 
 	input := &storagegateway.DescribeCacheInput{
@@ -144,9 +141,18 @@ func resourceCacheRead(ctx context.Context, d *schema.ResourceData, meta interfa
 	return diags
 }
 
-func DecodeCacheID(id string) (string, string, error) {
+const cacheResourceIDSeparator = ":"
+
+func cacheCreateResourceID(gatewayARN, diskID string) string {
+	parts := []string{gatewayARN, diskID}
+	id := strings.Join(parts, cacheResourceIDSeparator)
+
+	return id
+}
+
+func cacheParseResourceID(id string) (string, string, error) {
 	// id = arn:aws:storagegateway:us-east-1:123456789012:gateway/sgw-12345678:pci-0000:03:00.0-scsi-0:0:0:0
-	idFormatErr := fmt.Errorf("expected ID in form of GatewayARN:DiskId, received: %s", id)
+	idFormatErr := fmt.Errorf("unexpected format for ID (%[1]s), expected GatewayARN%[2]sDiskID", id, cacheResourceIDSeparator)
 	gatewayARNAndDisk, err := arn.Parse(id)
 	if err != nil {
 		return "", "", idFormatErr
@@ -158,10 +164,10 @@ func DecodeCacheID(id string) (string, string, error) {
 	}
 	// resourceParts = ["gateway/sgw-12345678", "pci-0000:03:00.0-scsi-0:0:0:0"]
 	gatewayARN := &arn.ARN{
-		AccountID: gatewayARNAndDisk.AccountID,
 		Partition: gatewayARNAndDisk.Partition,
-		Region:    gatewayARNAndDisk.Region,
 		Service:   gatewayARNAndDisk.Service,
+		Region:    gatewayARNAndDisk.Region,
+		AccountID: gatewayARNAndDisk.AccountID,
 		Resource:  resourceParts[0],
 	}
 	return gatewayARN.String(), resourceParts[1], nil

internal/service/storagegateway/cache_test.go

@@ -61,7 +61,7 @@ func TestDecodeCacheID(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		gatewayARN, diskID, err := tfstoragegateway.DecodeCacheID(tc.Input)
+		gatewayARN, diskID, err := tfstoragegateway.CacheParseResourceID(tc.Input)
 		if tc.ErrCount == 0 && err != nil {
 			t.Fatalf("expected %q not to trigger an error, received: %s", tc.Input, err)
 		}
@@ -148,7 +148,7 @@ func testAccCheckCacheExists(ctx context.Context, resourceName string) resource.
 
 		conn := acctest.Provider.Meta().(*conns.AWSClient).StorageGatewayConn(ctx)
 
-		gatewayARN, diskID, err := tfstoragegateway.DecodeCacheID(rs.Primary.ID)
+		gatewayARN, diskID, err := tfstoragegateway.CacheParseResourceID(rs.Primary.ID)
 		if err != nil {
 			return err
 		}

internal/service/storagegateway/cached_iscsi_volume.go

@@ -28,12 +28,13 @@ import (
 
 // @SDKResource("aws_storagegateway_cached_iscsi_volume", name="Cached iSCSI Volume")
 // @Tags(identifierAttribute="arn")
-func ResourceCachediSCSIVolume() *schema.Resource {
+func resourceCachediSCSIVolume() *schema.Resource {
 	return &schema.Resource{
 		CreateWithoutTimeout: resourceCachediSCSIVolumeCreate,
 		ReadWithoutTimeout:   resourceCachediSCSIVolumeRead,
 		UpdateWithoutTimeout: resourceCachediSCSIVolumeUpdate,
 		DeleteWithoutTimeout: resourceCachediSCSIVolumeDelete,
+
 		Importer: &schema.ResourceImporter{
 			StateContext: schema.ImportStatePassthroughContext,
 		},

internal/service/storagegateway/errors.go

@@ -4,10 +4,9 @@
 package storagegateway
 
 import (
-	"errors"
-
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/storagegateway"
+	"github.com/hashicorp/terraform-provider-aws/internal/errs"
 )
 
 // Operation error code constants missing from AWS Go SDK: https://docs.aws.amazon.com/sdk-for-go/api/service/storagegateway/#pkg-constants.
@@ -24,12 +23,12 @@ const (
 //
 // See https://docs.aws.amazon.com/storagegateway/latest/userguide/AWSStorageGatewayAPI.html#APIErrorResponses for details.
 func operationErrorCode(err error) string {
-	if inner := (*storagegateway.InternalServerError)(nil); errors.As(err, &inner) && inner.Error_ != nil {
-		return aws.StringValue(inner.Error_.ErrorCode)
+	if v, ok := errs.As[*storagegateway.InternalServerError](err); ok && v.Error_ != nil {
+		return aws.StringValue(v.Error_.ErrorCode)
 	}
 
-	if inner := (*storagegateway.InvalidGatewayRequestException)(nil); errors.As(err, &inner) && inner.Error_ != nil {
-		return aws.StringValue(inner.Error_.ErrorCode)
+	if v, ok := errs.As[*storagegateway.InvalidGatewayRequestException](err); ok && v.Error_ != nil {
+		return aws.StringValue(v.Error_.ErrorCode)
 	}
 
 	return ""

internal/service/storagegateway/exports_test.go

@@ -5,5 +5,15 @@ package storagegateway
 
 // Exports for use in tests only.
 var (
-	Valid4ByteASN = valid4ByteASN
+	ResourceCache                 = resourceCache
+	ResourceCachediSCSIVolume     = resourceCachediSCSIVolume
+	ResourceFileSystemAssociation = resourceFileSystemAssociation
+	ResourceGateway               = resourceGateway
+	ResourceNFSFileShare          = resourceNFSFileShare
+	ResourceSMBFileShare          = resourceSMBFileShare
+	ResourceStorediSCSIVolume     = resourceStorediSCSIVolume
+	ResourceTapePool              = resourceTapePool
+	ResourceUploadBuffer          = resourceUploadBuffer
+
+	CacheParseResourceID = cacheParseResourceID
 )

internal/service/storagegateway/file_system_association.go

@@ -25,16 +25,19 @@ import (
 
 // @SDKResource("aws_storagegateway_file_system_association", name="File System Association")
 // @Tags(identifierAttribute="arn")
-func ResourceFileSystemAssociation() *schema.Resource {
+func resourceFileSystemAssociation() *schema.Resource {
 	return &schema.Resource{
 		CreateWithoutTimeout: resourceFileSystemAssociationCreate,
 		ReadWithoutTimeout:   resourceFileSystemAssociationRead,
 		UpdateWithoutTimeout: resourceFileSystemAssociationUpdate,
 		DeleteWithoutTimeout: resourceFileSystemAssociationDelete,
-		CustomizeDiff:        customdiff.Sequence(verify.SetTagsDiff),
+
+		CustomizeDiff: customdiff.Sequence(verify.SetTagsDiff),
+
 		Importer: &schema.ResourceImporter{
 			StateContext: schema.ImportStatePassthroughContext,
 		},
+
 		Schema: map[string]*schema.Schema{
 			names.AttrARN: {
 				Type:     schema.TypeString,

internal/service/storagegateway/find.go

@@ -10,55 +10,52 @@ import (
 	"github.com/aws/aws-sdk-go/service/storagegateway"
 	"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
+	tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices"
 	"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
 )
 
-func FindLocalDiskByDiskID(ctx context.Context, conn *storagegateway.StorageGateway, gatewayARN string, diskID string) (*storagegateway.Disk, error) {
-	input := &storagegateway.ListLocalDisksInput{
-		GatewayARN: aws.String(gatewayARN),
+func findLocalDisk(ctx context.Context, conn *storagegateway.StorageGateway, input *storagegateway.ListLocalDisksInput, filter tfslices.Predicate[*storagegateway.Disk]) (*storagegateway.Disk, error) {
+	output, err := findLocalDisks(ctx, conn, input, filter)
+
+	if err != nil {
+		return nil, err
 	}
 
+	return tfresource.AssertSinglePtrResult(output)
+}
+
+func findLocalDisks(ctx context.Context, conn *storagegateway.StorageGateway, input *storagegateway.ListLocalDisksInput, filter tfslices.Predicate[*storagegateway.Disk]) ([]*storagegateway.Disk, error) {
 	output, err := conn.ListLocalDisksWithContext(ctx, input)
 
 	if err != nil {
 		return nil, err
 	}
 
 	if output == nil {
-		return nil, nil
-	}
-
-	for _, disk := range output.Disks {
-		if aws.StringValue(disk.DiskId) == diskID {
-			return disk, nil
-		}
+		return nil, tfresource.NewEmptyResultError(input)
 	}
 
-	return nil, nil
+	return tfslices.Filter(output.Disks, filter), nil
 }
 
-func FindLocalDiskByDiskPath(ctx context.Context, conn *storagegateway.StorageGateway, gatewayARN string, diskPath string) (*storagegateway.Disk, error) {
+func findLocalDiskByGatewayARNAndDiskID(ctx context.Context, conn *storagegateway.StorageGateway, gatewayARN, diskID string) (*storagegateway.Disk, error) {
 	input := &storagegateway.ListLocalDisksInput{
 		GatewayARN: aws.String(gatewayARN),
 	}
 
-	output, err := conn.ListLocalDisksWithContext(ctx, input)
-
-	if err != nil {
-		return nil, err
-	}
-
-	if output == nil {
-		return nil, nil
-	}
+	return findLocalDisk(ctx, conn, input, func(v *storagegateway.Disk) bool {
+		return aws.StringValue(v.DiskId) == diskID
+	})
+}
 
-	for _, disk := range output.Disks {
-		if aws.StringValue(disk.DiskPath) == diskPath {
-			return disk, nil
-		}
+func findLocalDiskByGatewayARNAndDiskPath(ctx context.Context, conn *storagegateway.StorageGateway, gatewayARN, diskPath string) (*storagegateway.Disk, error) {
+	input := &storagegateway.ListLocalDisksInput{
+		GatewayARN: aws.String(gatewayARN),
 	}
 
-	return nil, nil
+	return findLocalDisk(ctx, conn, input, func(v *storagegateway.Disk) bool {
+		return aws.StringValue(v.DiskPath) == diskPath
+	})
 }
 
 func FindUploadBufferDisk(ctx context.Context, conn *storagegateway.StorageGateway, gatewayARN string, diskID string) (*string, error) {

internal/service/storagegateway/gateway.go

@@ -34,7 +34,7 @@ import (
 
 // @SDKResource("aws_storagegateway_gateway", name="Gateway")
 // @Tags(identifierAttribute="arn")
-func ResourceGateway() *schema.Resource {
+func resourceGateway() *schema.Resource {
 	return &schema.Resource{
 		CreateWithoutTimeout: resourceGatewayCreate,
 		ReadWithoutTimeout:   resourceGatewayRead,

internal/service/storagegateway/local_disk_data_source.go

@@ -16,8 +16,8 @@ import (
 	"github.com/hashicorp/terraform-provider-aws/internal/verify"
 )
 
-// @SDKDataSource("aws_storagegateway_local_disk")
-func DataSourceLocalDisk() *schema.Resource {
+// @SDKDataSource("aws_storagegateway_local_disk", name="Local Disk")
+func dataSourceLocalDisk() *schema.Resource {
 	return &schema.Resource{
 		ReadWithoutTimeout: dataSourceLocalDiskRead,
 

internal/service/storagegateway/nfs_file_share.go

@@ -27,7 +27,7 @@ import (
 
 // @SDKResource("aws_storagegateway_nfs_file_share", name="NFS File Share")
 // @Tags(identifierAttribute="arn")
-func ResourceNFSFileShare() *schema.Resource {
+func resourceNFSFileShare() *schema.Resource {
 	return &schema.Resource{
 		CreateWithoutTimeout: resourceNFSFileShareCreate,
 		ReadWithoutTimeout:   resourceNFSFileShareRead,
@@ -152,13 +152,13 @@ func ResourceNFSFileShare() *schema.Resource {
 							Type:         schema.TypeString,
 							Optional:     true,
 							Default:      "65534",
-							ValidateFunc: valid4ByteASN,
+							ValidateFunc: verify.Valid4ByteASN,
 						},
 						names.AttrOwnerID: {
 							Type:         schema.TypeString,
 							Optional:     true,
 							Default:      "65534",
-							ValidateFunc: valid4ByteASN,
+							ValidateFunc: verify.Valid4ByteASN,
 						},
 					},
 				},