Skip to content

Commit

Permalink
Clean up and call out existing issue
Browse files Browse the repository at this point in the history
  • Loading branch information
@jdoldis
jdoldis committed Sep 29, 2024
1 parent 1704a50 commit afbd4a0
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 43 deletions.
92 changes: 49 additions & 43 deletions pkg/resources/external_volume.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func ImportExternalVolume(ctx context.Context, d *schema.ResourceData, meta any)
return []*schema.ResourceData{d}, nil
}

func extractStorageLocations(v any, withAzureEncryptionCheck bool) ([]sdk.ExternalVolumeStorageLocation, error) {
func extractStorageLocations(v any, withAzureEncryptionTypeCheck bool) ([]sdk.ExternalVolumeStorageLocation, error) {
_, ok := v.([]any)
if v == nil || !ok {
return nil, fmt.Errorf("unable to extract storage locations, input is either nil or non expected type (%T): %v", v, v)
Expand All @@ -212,7 +212,7 @@ func extractStorageLocations(v any, withAzureEncryptionCheck bool) ([]sdk.Extern
return nil, fmt.Errorf("unable to extract storage location, missing storage_provider key in storage location")
}

storage_base_url, ok := storageLocationConfig["storage_base_url"].(string)
storageBaseUrl, ok := storageLocationConfig["storage_base_url"].(string)
if !ok {
return nil, fmt.Errorf("unable to extract storage location, missing storage_base_url key in storage location")
}
Expand All @@ -227,13 +227,13 @@ func extractStorageLocations(v any, withAzureEncryptionCheck bool) ([]sdk.Extern
case sdk.StorageProviderS3, sdk.StorageProviderS3GOV:
// Test that azure_tenant_id is not given
// If given non empty plans will be produced
azure_tenant_id, ok := storageLocationConfig["azure_tenant_id"].(string)
if ok && len(azure_tenant_id) > 0 {
azureTenantId, ok := storageLocationConfig["azure_tenant_id"].(string)
if ok && len(azureTenantId) > 0 {
return nil, fmt.Errorf("unable to extract storage location, azure_tenant_id provided for s3 storage location")
}

storage_aws_role_arn, ok := storageLocationConfig["storage_aws_role_arn"].(string)
if !ok || len(storage_aws_role_arn) == 0 {
storageAwsRoleArn, ok := storageLocationConfig["storage_aws_role_arn"].(string)
if !ok || len(storageAwsRoleArn) == 0 {
return nil, fmt.Errorf("unable to extract storage location, missing storage_aws_role_arn key in an s3 storage location")
}

Expand All @@ -245,22 +245,22 @@ func extractStorageLocations(v any, withAzureEncryptionCheck bool) ([]sdk.Extern
s3StorageLocation := &sdk.S3StorageLocationParams{
Name: name,
StorageProvider: s3StorageProvider,
StorageBaseUrl: storage_base_url,
StorageAwsRoleArn: storage_aws_role_arn,
StorageBaseUrl: storageBaseUrl,
StorageAwsRoleArn: storageAwsRoleArn,
}

encryption_type, ok := storageLocationConfig["encryption_type"].(string)
if ok && len(encryption_type) > 0 {
encryptionTypeParsed, err := sdk.ToS3EncryptionType(encryption_type)
encryptionType, ok := storageLocationConfig["encryption_type"].(string)
if ok && len(encryptionType) > 0 {
encryptionTypeParsed, err := sdk.ToS3EncryptionType(encryptionType)
if err != nil {
return nil, err
}

encryption_kms_key_id, ok := storageLocationConfig["encryption_kms_key_id"].(string)
if ok && len(encryption_kms_key_id) > 0 {
encryptionKmsKeyId, ok := storageLocationConfig["encryption_kms_key_id"].(string)
if ok && len(encryptionKmsKeyId) > 0 {
s3StorageLocation.Encryption = &sdk.ExternalVolumeS3Encryption{
Type: encryptionTypeParsed,
KmsKeyId: &encryption_kms_key_id,
KmsKeyId: &encryptionKmsKeyId,
}
} else {
s3StorageLocation.Encryption = &sdk.ExternalVolumeS3Encryption{
Expand All @@ -275,30 +275,31 @@ func extractStorageLocations(v any, withAzureEncryptionCheck bool) ([]sdk.Extern
case sdk.StorageProviderGCS:
// Test that azure_tenant_id and storage_aws_role_arn are not given
// If given non empty plans will be produced
azure_tenant_id, ok := storageLocationConfig["azure_tenant_id"].(string)
storage_aws_role_arn, ok := storageLocationConfig["storage_aws_role_arn"].(string)
if ok && len(azure_tenant_id) > 0 {
azureTenantId, ok := storageLocationConfig["azure_tenant_id"].(string)
if ok && len(azureTenantId) > 0 {
return nil, fmt.Errorf("unable to extract storage location, azure_tenant_id provided for gcs storage location")
}
if ok && len(storage_aws_role_arn) > 0 {

storageAwsRoleArn, ok := storageLocationConfig["storage_aws_role_arn"].(string)
if ok && len(storageAwsRoleArn) > 0 {
return nil, fmt.Errorf("unable to extract storage location, storage_aws_role_arn provided for gcs storage location")
}

gcsStorageLocation := &sdk.GCSStorageLocationParams{
Name: name,
StorageBaseUrl: storage_base_url,
StorageBaseUrl: storageBaseUrl,
}
encryption_type, ok := storageLocationConfig["encryption_type"].(string)
if ok && len(encryption_type) > 0 {
encryptionTypeParsed, err := sdk.ToGCSEncryptionType(encryption_type)
encryptionType, ok := storageLocationConfig["encryption_type"].(string)
if ok && len(encryptionType) > 0 {
encryptionTypeParsed, err := sdk.ToGCSEncryptionType(encryptionType)
if err != nil {
return nil, err
}
encryption_kms_key_id, ok := storageLocationConfig["encryption_kms_key_id"].(string)
if ok && len(encryption_kms_key_id) > 0 {
encryptionKmsKeyId, ok := storageLocationConfig["encryption_kms_key_id"].(string)
if ok && len(encryptionKmsKeyId) > 0 {
gcsStorageLocation.Encryption = &sdk.ExternalVolumeGCSEncryption{
Type: encryptionTypeParsed,
KmsKeyId: &encryption_kms_key_id,
KmsKeyId: &encryptionKmsKeyId,
}
} else {
gcsStorageLocation.Encryption = &sdk.ExternalVolumeGCSEncryption{
Expand All @@ -311,37 +312,42 @@ func extractStorageLocations(v any, withAzureEncryptionCheck bool) ([]sdk.Extern
GCSStorageLocationParams: gcsStorageLocation,
}
case sdk.StorageProviderAzure:
// Test that storage_aws_role_arn is not given
// Test that storage_aws_role_arn and encryption_kms_key_id is not given
// If given non empty plans will be produced
storage_aws_role_arn, ok := storageLocationConfig["storage_aws_role_arn"].(string)
if ok && len(storage_aws_role_arn) > 0 {
storageAwsRolArn, ok := storageLocationConfig["storage_aws_role_arn"].(string)
if ok && len(storageAwsRolArn) > 0 {
return nil, fmt.Errorf("unable to extract storage location, storage_aws_role_arn provided for azure storage location")
}

// Encryption fields are set on describe requests, although they are not a valid parameter.
// Allowing optional checking of these fields as they will be present in some cases.
if withAzureEncryptionCheck {
encryption_type, ok := storageLocationConfig["encryption_type"].(string)
if ok && len(encryption_type) > 0 {
return nil, fmt.Errorf("unable to extract storage location, encryption_type provided for azure storage location")
}
encryptionKmsKeyId, ok := storageLocationConfig["encryption_kms_key_id"].(string)
if ok && len(encryptionKmsKeyId) > 0 {
return nil, fmt.Errorf("unable to extract storage location, encryption_kms_key_id provided for azure storage location")
}

encryption_kms_key_id, ok := storageLocationConfig["encryption_kms_key_id"].(string)
if ok && len(encryption_kms_key_id) > 0 {
return nil, fmt.Errorf("unable to extract storage location, encryption_kms_key_id provided for azure storage location")
// TODO
// For some reason, the new storage_locations list in the update function has NONE as the value
// for encryption_type for azure storage locations, even when it is not specified in the config.
// To get the acceptance tests passing for now the check has been turned off for this case,
// but why it's happning in the first place needs to be investigated.
// To reproduce, turn on the check for extracting new external volumes in the update function and
// run the TestAcc_External_Volume_Multiple acceptance test.
if withAzureEncryptionTypeCheck {
encryptionType, ok := storageLocationConfig["encryption_type"].(string)
if ok && len(encryptionType) > 0 {
return nil, fmt.Errorf("unable to extract storage location, encryption_type provided for azure storage location")
}
}

azure_tenant_id, ok := storageLocationConfig["azure_tenant_id"].(string)
if !ok || len(azure_tenant_id) == 0 {
azureTenantId, ok := storageLocationConfig["azure_tenant_id"].(string)
if !ok || len(azureTenantId) == 0 {
return nil, fmt.Errorf("unable to extract storage location, missing azure_tenant_id provider key in an azure storage location")
}

storageLocation = sdk.ExternalVolumeStorageLocation{
AzureStorageLocationParams: &sdk.AzureStorageLocationParams{
Name: name,
AzureTenantId: azure_tenant_id,
StorageBaseUrl: storage_base_url,
AzureTenantId: azureTenantId,
StorageBaseUrl: storageBaseUrl,
},
}
}
Expand Down Expand Up @@ -607,7 +613,7 @@ func UpdateContextExternalVolume(ctx context.Context, d *schema.ResourceData, me

if d.HasChange("storage_location") {
old, new := d.GetChange("storage_location")
oldLocations, err := extractStorageLocations(old, false)
oldLocations, err := extractStorageLocations(old, true)
if err != nil {
return diag.FromErr(err)
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/resources/external_volume_acceptance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1913,11 +1913,13 @@ func TestAcc_External_Volume_Invalid_Cases(t *testing.T) {
ConfigVariables: externalVolume(config.ListVariable(azureStorageLocationAllParams), externalVolumeName, "", ""),
ExpectError: regexp.MustCompile("unable to extract storage location, storage_aws_role_arn provided for azure storage location"),
},
// encryption_type specified for azure storage location
{
ConfigDirectory: acc.ConfigurationDirectory("TestAcc_ExternalVolume/azure/with-encryption-type"),
ConfigVariables: externalVolume(config.ListVariable(azureStorageLocationAllParams), externalVolumeName, "", ""),
ExpectError: regexp.MustCompile("unable to extract storage location, encryption_type provided for azure storage location"),
},
// encryption_kms_key_id specified for azure storage location
{
ConfigDirectory: acc.ConfigurationDirectory("TestAcc_ExternalVolume/azure/with-encryption-kms-key-id"),
ConfigVariables: externalVolume(config.ListVariable(azureStorageLocationAllParams), externalVolumeName, "", ""),
Expand Down

0 comments on commit afbd4a0

Please sign in to comment.