From 15b87382a13d970e130afe7a3a3a3570d0bdac96 Mon Sep 17 00:00:00 2001 From: James Doldissen Date: Mon, 21 Oct 2024 13:27:28 +0200 Subject: [PATCH] Use pre defined temp storage location name --- docs/resources/external_volume.md | 2 +- pkg/resources/external_volume.go | 14 ++------------ pkg/resources/helpers.go | 7 +------ pkg/resources/helpers_test.go | 7 ++++--- 4 files changed, 8 insertions(+), 22 deletions(-) diff --git a/docs/resources/external_volume.md b/docs/resources/external_volume.md index 473e92edcf..7e068ba7b5 100644 --- 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 index ab1fd2ef20..d631bc2977 100644 --- 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 index 8874ebf13d..ada59b0a9f 100644 --- 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 index 0cade24726..e35b436bd8 100644 --- 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) })