Use pre defined temp storage location name

Snowflake-Labs · Oct 21, 2024 · 15b8738 · 15b8738
1 parent 2c09c7d
commit 15b8738
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 22 deletions.
diff --git a/docs/resources/external_volume.md b/docs/resources/external_volume.md
@@ -39,7 +39,7 @@ Resource used to manage external volume objects. For more information, check [ex
 Required:
 
 - `storage_base_url` (String) Specifies the base URL for your cloud storage location.
-- `storage_location_name` (String) Name of the storage location. Must be unique for the external volume. Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `(`, `)`, `"`
+- `storage_location_name` (String) Name of the storage location. Must be unique for the external volume. Do not use the name `terraform_provider_sentinel_storage_location` - this is reserved for the provider for performing update operations. Due to technical limitations (read more [here](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/docs/technical-documentation/identifiers_rework_design_decisions.md#known-limitations-and-identifier-recommendations)), avoid using the following characters: `|`, `.`, `(`, `)`, `"`
 - `storage_provider` (String) Specifies the cloud storage provider that stores your data files. Valid values are (case-insensitive): `GCS` | `AZURE` | `S3` | `S3GOV`.
 
 Optional:

diff --git a/pkg/resources/external_volume.go b/pkg/resources/external_volume.go
@@ -38,7 +38,7 @@ var externalVolumeSchema = map[string]*schema.Schema{
 				"storage_location_name": {
 					Type:             schema.TypeString,
 					Required:         true,
-					Description:      blocklistedCharactersFieldDescription("Name of the storage location. Must be unique for the external volume."),
+					Description:      blocklistedCharactersFieldDescription("Name of the storage location. Must be unique for the external volume. Do not use the name `terraform_provider_sentinel_storage_location` - this is reserved for the provider for performing update operations."),
 					DiffSuppressFunc: suppressIdentifierQuoting,
 				},
 				"storage_provider": {
@@ -378,16 +378,6 @@ func UpdateContextExternalVolume(ctx context.Context, d *schema.ResourceData, me
 			// would otherwise be necessary as a minimum of 1 storage location per external volume is required.
 			// The alternative solution of adding volumes before removing them isn't possible as
 			// name must be unique for storage locations
-			if len(removedLocations) > 1 {
-				// To ensure the name of the temporary storage location is unique,
-				// first remove all but 1 storage locations from the list
-				for _, removedStorageLocation := range removedLocations[1:] {
-					err = removeStorageLocation(removedStorageLocation, client, ctx, id)
-					if err != nil {
-						return diag.FromErr(err)
-					}
-				}
-			}
 
 			temp_storage_location, err := CopyStorageLocationWithTempName(removedLocations[0])
 			if err != nil {
@@ -405,7 +395,7 @@ func UpdateContextExternalVolume(ctx context.Context, d *schema.ResourceData, me
 				}
 			}()
 
-			updateErr := updateStorageLocations([]sdk.ExternalVolumeStorageLocation{removedLocations[0]}, addedLocations, client, ctx, id)
+			updateErr := updateStorageLocations(removedLocations, addedLocations, client, ctx, id)
 			if updateErr != nil {
 				return diag.FromErr(updateErr)
 			}

diff --git a/pkg/resources/helpers.go b/pkg/resources/helpers.go
@@ -341,12 +341,7 @@ func CopyStorageLocationWithTempName(
 		return sdk.ExternalVolumeStorageLocation{}, err
 	}
 
-	currName, err := GetStorageLocationName(storageLocation)
-	if err != nil {
-		return sdk.ExternalVolumeStorageLocation{}, err
-	}
-
-	newName := fmt.Sprintf("temp_%s", currName)
+	newName := "terraform_provider_sentinel_storage_location"
 	var tempNameStorageLocation sdk.ExternalVolumeStorageLocation
 	switch storageProvider {
 	case sdk.StorageProviderS3, sdk.StorageProviderS3GOV:

diff --git a/pkg/resources/helpers_test.go b/pkg/resources/helpers_test.go
@@ -632,6 +632,7 @@ func Test_GetStorageLocationStorageProvider(t *testing.T) {
 var s3StorageAwsExternalId = "1234567890"
 
 func Test_CopyStorageLocationWithTempName(t *testing.T) {
+	tempStorageLocationName := "terraform_provider_sentinel_storage_location"
 	s3StorageLocationName := "s3Test"
 	s3StorageBaseUrl := "s3://my_example_bucket"
 	s3StorageAwsRoleArn := "arn:aws:iam::123456789012:role/myrole"
@@ -676,7 +677,7 @@ func Test_CopyStorageLocationWithTempName(t *testing.T) {
 		storageLocationInput := sdk.ExternalVolumeStorageLocation{S3StorageLocationParams: &s3StorageLocationA}
 		copiedStorageLocation, err := resources.CopyStorageLocationWithTempName(storageLocationInput)
 		require.NoError(t, err)
-		assert.Equal(t, copiedStorageLocation.S3StorageLocationParams.Name, fmt.Sprintf("temp_%s", s3StorageLocationA.Name))
+		assert.Equal(t, copiedStorageLocation.S3StorageLocationParams.Name, tempStorageLocationName)
 		assert.Equal(t, copiedStorageLocation.S3StorageLocationParams.StorageProvider, s3StorageLocationA.StorageProvider)
 		assert.Equal(t, copiedStorageLocation.S3StorageLocationParams.StorageBaseUrl, s3StorageLocationA.StorageBaseUrl)
 		assert.Equal(t, copiedStorageLocation.S3StorageLocationParams.StorageAwsRoleArn, s3StorageLocationA.StorageAwsRoleArn)
@@ -689,7 +690,7 @@ func Test_CopyStorageLocationWithTempName(t *testing.T) {
 		storageLocationInput := sdk.ExternalVolumeStorageLocation{GCSStorageLocationParams: &gcsStorageLocationA}
 		copiedStorageLocation, err := resources.CopyStorageLocationWithTempName(storageLocationInput)
 		require.NoError(t, err)
-		assert.Equal(t, copiedStorageLocation.GCSStorageLocationParams.Name, fmt.Sprintf("temp_%s", gcsStorageLocationA.Name))
+		assert.Equal(t, copiedStorageLocation.GCSStorageLocationParams.Name, tempStorageLocationName)
 		assert.Equal(t, copiedStorageLocation.GCSStorageLocationParams.StorageBaseUrl, gcsStorageLocationA.StorageBaseUrl)
 		assert.Equal(t, copiedStorageLocation.GCSStorageLocationParams.Encryption.Type, gcsStorageLocationA.Encryption.Type)
 		assert.Equal(t, *copiedStorageLocation.GCSStorageLocationParams.Encryption.KmsKeyId, *gcsStorageLocationA.Encryption.KmsKeyId)
@@ -699,7 +700,7 @@ func Test_CopyStorageLocationWithTempName(t *testing.T) {
 		storageLocationInput := sdk.ExternalVolumeStorageLocation{AzureStorageLocationParams: &azureStorageLocationA}
 		copiedStorageLocation, err := resources.CopyStorageLocationWithTempName(storageLocationInput)
 		require.NoError(t, err)
-		assert.Equal(t, copiedStorageLocation.AzureStorageLocationParams.Name, fmt.Sprintf("temp_%s", azureStorageLocationA.Name))
+		assert.Equal(t, copiedStorageLocation.AzureStorageLocationParams.Name, tempStorageLocationName)
 		assert.Equal(t, copiedStorageLocation.AzureStorageLocationParams.StorageBaseUrl, azureStorageLocationA.StorageBaseUrl)
 		assert.Equal(t, copiedStorageLocation.AzureStorageLocationParams.AzureTenantId, azureStorageLocationA.AzureTenantId)
 	})