From 8ba2c1a7ff57b8aab3afa07e87ed3fd066231283 Mon Sep 17 00:00:00 2001 From: Lyle Nel Date: Wed, 22 Jan 2020 12:16:43 +0200 Subject: [PATCH] #8000 #8009 Optional fields should not be set unless it has a default or is explicitly populated. --- aws/resource_aws_dms_endpoint.go | 120 +++--- aws/resource_aws_dms_endpoint_test.go | 555 ++++++++++++++------------ 2 files changed, 361 insertions(+), 314 deletions(-) diff --git a/aws/resource_aws_dms_endpoint.go b/aws/resource_aws_dms_endpoint.go index e4f1bc91d34e..1f7afbbbf05d 100644 --- a/aws/resource_aws_dms_endpoint.go +++ b/aws/resource_aws_dms_endpoint.go @@ -206,8 +206,7 @@ func resourceAwsDmsEndpoint() *schema.Resource { }, "bucket_name": { Type: schema.TypeString, - Optional: true, - Default: "", + Required: true, }, "compression_type": { Type: schema.TypeString, @@ -217,27 +216,22 @@ func resourceAwsDmsEndpoint() *schema.Resource { "timestamp_column_name": { Type: schema.TypeString, Optional: true, - Default: "", }, "data_format": { Type: schema.TypeString, Optional: true, - Default: "", }, "parquet_version": { Type: schema.TypeString, Optional: true, - Default: "", }, "encryption_mode": { Type: schema.TypeString, Optional: true, - Default: "", }, "server_side_encryption_kms_key_id": { Type: schema.TypeString, Optional: true, - Default: "", }, }, }, @@ -246,6 +240,15 @@ func resourceAwsDmsEndpoint() *schema.Resource { } } +func getStringRefOrNil(d *schema.ResourceData, key string) *string { + value, exists := d.GetOk(key) + if !exists { + return nil + } + valueString := value.(string) + return &valueString +} + func resourceAwsDmsEndpointCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).dmsconn @@ -287,18 +290,18 @@ func resourceAwsDmsEndpointCreate(d *schema.ResourceData, meta interface{}) erro request.DatabaseName = aws.String(d.Get("database_name").(string)) case "s3": request.S3Settings = &dms.S3Settings{ - ServiceAccessRoleArn: aws.String(d.Get("s3_settings.0.service_access_role_arn").(string)), - ExternalTableDefinition: aws.String(d.Get("s3_settings.0.external_table_definition").(string)), - CsvRowDelimiter: aws.String(d.Get("s3_settings.0.csv_row_delimiter").(string)), - CsvDelimiter: aws.String(d.Get("s3_settings.0.csv_delimiter").(string)), - BucketFolder: aws.String(d.Get("s3_settings.0.bucket_folder").(string)), - BucketName: aws.String(d.Get("s3_settings.0.bucket_name").(string)), - CompressionType: aws.String(d.Get("s3_settings.0.compression_type").(string)), - TimestampColumnName: aws.String(d.Get("s3_settings.0.timestamp_column_name").(string)), - DataFormat: aws.String(d.Get("s3_settings.0.data_format").(string)), - ParquetVersion: aws.String(d.Get("s3_settings.0.parquet_version").(string)), - EncryptionMode: aws.String(d.Get("s3_settings.0.encryption_mode").(string)), - ServerSideEncryptionKmsKeyId: aws.String(d.Get("s3_settings.0.server_side_encryption_kms_key_id").(string)), + ServiceAccessRoleArn: getStringRefOrNil(d, "s3_settings.0.service_access_role_arn"), + ExternalTableDefinition: getStringRefOrNil(d, "s3_settings.0.external_table_definition"), + CsvRowDelimiter: getStringRefOrNil(d, "s3_settings.0.csv_row_delimiter"), + CsvDelimiter: getStringRefOrNil(d, "s3_settings.0.csv_delimiter"), + BucketFolder: getStringRefOrNil(d, "s3_settings.0.bucket_folder"), + BucketName: getStringRefOrNil(d, "s3_settings.0.bucket_name"), + CompressionType: getStringRefOrNil(d, "s3_settings.0.compression_type"), + TimestampColumnName: getStringRefOrNil(d, "s3_settings.0.timestamp_column_name"), + DataFormat: getStringRefOrNil(d, "s3_settings.0.data_format"), + ParquetVersion: getStringRefOrNil(d, "s3_settings.0.parquet_version"), + EncryptionMode: getStringRefOrNil(d, "s3_settings.0.encryption_mode"), + ServerSideEncryptionKmsKeyId: getStringRefOrNil(d, "s3_settings.0.server_side_encryption_kms_key_id"), } default: request.Password = aws.String(d.Get("password").(string)) @@ -346,13 +349,13 @@ func resourceAwsDmsEndpointCreate(d *schema.ResourceData, meta interface{}) erro } d.SetId(d.Get("endpoint_id").(string)) + response, _ := describeEndpointResponse(conn, d) + log.Println("[DEBUG] DMS response from create endpoint:", response) return resourceAwsDmsEndpointRead(d, meta) } -func resourceAwsDmsEndpointRead(d *schema.ResourceData, meta interface{}) error { - conn := meta.(*AWSClient).dmsconn - - response, err := conn.DescribeEndpoints(&dms.DescribeEndpointsInput{ +func describeEndpointResponse(conn *dms.DatabaseMigrationService, d *schema.ResourceData) (*dms.DescribeEndpointsOutput, error) { + response, err2 := conn.DescribeEndpoints(&dms.DescribeEndpointsInput{ Filters: []*dms.Filter{ { Name: aws.String("endpoint-id"), @@ -360,6 +363,13 @@ func resourceAwsDmsEndpointRead(d *schema.ResourceData, meta interface{}) error }, }, }) + return response, err2 +} + +func resourceAwsDmsEndpointRead(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*AWSClient).dmsconn + + response, err := describeEndpointResponse(conn, d) if err != nil { if dmserr, ok := err.(awserr.Error); ok && dmserr.Code() == "ResourceNotFoundFault" { log.Printf("[DEBUG] DMS Replication Endpoint %q Not Found", d.Id()) @@ -495,24 +505,23 @@ func resourceAwsDmsEndpointUpdate(d *schema.ResourceData, meta interface{}) erro d.HasChange("s3_settings.0.bucket_folder") || d.HasChange("s3_settings.0.bucket_name") || d.HasChange("s3_settings.0.compression_type") || - d.HasChange("s3_settings.0.timestamp_column_name") || d.HasChange("s3_settings.0.data_format") || d.HasChange("s3_settings.0.parquet_version") || d.HasChange("s3_settings.0.encryption_mode") || d.HasChange("s3_settings.0.server_side_encryption_kms_key_id") { request.S3Settings = &dms.S3Settings{ - ServiceAccessRoleArn: aws.String(d.Get("s3_settings.0.service_access_role_arn").(string)), - ExternalTableDefinition: aws.String(d.Get("s3_settings.0.external_table_definition").(string)), - CsvRowDelimiter: aws.String(d.Get("s3_settings.0.csv_row_delimiter").(string)), - CsvDelimiter: aws.String(d.Get("s3_settings.0.csv_delimiter").(string)), - BucketFolder: aws.String(d.Get("s3_settings.0.bucket_folder").(string)), - BucketName: aws.String(d.Get("s3_settings.0.bucket_name").(string)), - CompressionType: aws.String(d.Get("s3_settings.0.compression_type").(string)), - TimestampColumnName: aws.String(d.Get("s3_settings.0.timestamp_column_name").(string)), - DataFormat: aws.String(d.Get("s3_settings.0.data_format").(string)), - ParquetVersion: aws.String(d.Get("s3_settings.0.parquet_version").(string)), - EncryptionMode: aws.String(d.Get("s3_settings.0.encryption_mode").(string)), - ServerSideEncryptionKmsKeyId: aws.String(d.Get("s3_settings.0.server_side_encryption_kms_key_id").(string)), + ServiceAccessRoleArn: getStringRefOrNil(d, "s3_settings.0.service_access_role_arn"), + ExternalTableDefinition: getStringRefOrNil(d, "s3_settings.0.external_table_definition"), + CsvRowDelimiter: getStringRefOrNil(d, "s3_settings.0.csv_row_delimiter"), + CsvDelimiter: getStringRefOrNil(d, "s3_settings.0.csv_delimiter"), + BucketFolder: getStringRefOrNil(d, "s3_settings.0.bucket_folder"), + BucketName: getStringRefOrNil(d, "s3_settings.0.bucket_name"), + TimestampColumnName: getStringRefOrNil(d, "s3_settings.0.timestamp_column_name"), + CompressionType: getStringRefOrNil(d, "s3_settings.0.compression_type"), + DataFormat: getStringRefOrNil(d, "s3_settings.0.data_format"), + ParquetVersion: getStringRefOrNil(d, "s3_settings.0.parquet_version"), + EncryptionMode: getStringRefOrNil(d, "s3_settings.0.encryption_mode"), + ServerSideEncryptionKmsKeyId: getStringRefOrNil(d, "s3_settings.0.server_side_encryption_kms_key_id"), } request.EngineName = aws.String(d.Get("engine_name").(string)) // Must be included (should be 's3') hasChanges = true @@ -639,24 +648,27 @@ func flattenDmsMongoDbSettings(settings *dms.MongoDbSettings) []map[string]inter } func flattenDmsS3Settings(settings *dms.S3Settings) []map[string]interface{} { - if settings == nil { - return []map[string]interface{}{} + result := make([]map[string]interface{}, 0, 1) + + mapWithNils := map[string]interface{}{ + "service_access_role_arn": settings.ServiceAccessRoleArn, + "external_table_definition": settings.ExternalTableDefinition, + "csv_row_delimiter": settings.CsvRowDelimiter, + "csv_delimiter": settings.CsvDelimiter, + "bucket_folder": settings.BucketFolder, + "bucket_name": settings.BucketName, + "compression_type": settings.CompressionType, + "timestamp_column_name": settings.TimestampColumnName, + "data_format": settings.DataFormat, + "parquet_version": settings.ParquetVersion, + "encryption_mode": settings.EncryptionMode, + "server_side_encryption_kms_key_id": settings.ServerSideEncryptionKmsKeyId, } - - m := map[string]interface{}{ - "service_access_role_arn": aws.StringValue(settings.ServiceAccessRoleArn), - "external_table_definition": aws.StringValue(settings.ExternalTableDefinition), - "csv_row_delimiter": aws.StringValue(settings.CsvRowDelimiter), - "csv_delimiter": aws.StringValue(settings.CsvDelimiter), - "bucket_folder": aws.StringValue(settings.BucketFolder), - "bucket_name": aws.StringValue(settings.BucketName), - "compression_type": aws.StringValue(settings.CompressionType), - "timestamp_column_name": aws.StringValue(settings.TimestampColumnName), - "data_format": aws.StringValue(settings.DataFormat), - "parquet_version": aws.StringValue(settings.ParquetVersion), - "encryption_mode": aws.StringValue(settings.EncryptionMode), - "server_side_encryption_kms_key_id": aws.StringValue(settings.ServerSideEncryptionKmsKeyId), + mapWithoutNills := make(map[string]interface{}) + for key := range mapWithNils { + if mapWithNils[key].(*string) != nil { + mapWithoutNills[key] = aws.StringValue(mapWithNils[key].(*string)) + } } - - return []map[string]interface{}{m} + return append(result, mapWithoutNills) } diff --git a/aws/resource_aws_dms_endpoint_test.go b/aws/resource_aws_dms_endpoint_test.go index de5f83fe1273..3b72f31dc830 100644 --- a/aws/resource_aws_dms_endpoint_test.go +++ b/aws/resource_aws_dms_endpoint_test.go @@ -2,13 +2,13 @@ package aws import ( "fmt" - "testing" - "github.com/aws/aws-sdk-go/aws" - dms "github.com/aws/aws-sdk-go/service/databasemigrationservice" "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/terraform" + "testing" + + dms "github.com/aws/aws-sdk-go/service/databasemigrationservice" ) func TestAccAwsDmsEndpoint_Basic(t *testing.T) { @@ -50,6 +50,8 @@ func TestAccAwsDmsEndpoint_Basic(t *testing.T) { }) } +type TestCheckFunc func(*terraform.State) error + func TestAccAwsDmsEndpoint_S3_Csv(t *testing.T) { resourceName := "aws_dms_endpoint.dms_endpoint" randId := acctest.RandString(8) + "-s3" @@ -60,7 +62,8 @@ func TestAccAwsDmsEndpoint_S3_Csv(t *testing.T) { CheckDestroy: dmsEndpointDestroy, Steps: []resource.TestStep{ { - Config: dmsEndpointS3Config(randId), + PreventDiskCleanup: true, + Config: dmsEndpointS3Config(randId), Check: resource.ComposeTestCheckFunc( checkDmsEndpointExists(resourceName), resource.TestCheckResourceAttr(resourceName, "s3_settings.#", "1"), @@ -70,6 +73,8 @@ func TestAccAwsDmsEndpoint_S3_Csv(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "s3_settings.0.bucket_folder", ""), resource.TestCheckResourceAttr(resourceName, "s3_settings.0.bucket_name", "bucket_name"), resource.TestCheckResourceAttr(resourceName, "s3_settings.0.compression_type", "NONE"), + resource.TestCheckNoResourceAttr(resourceName, "s3_settings.0.enable_statistics"), + resource.TestCheckNoResourceAttr(resourceName, "s3_settings.0.timestamp_column_name"), ), }, { @@ -95,7 +100,6 @@ func TestAccAwsDmsEndpoint_S3_Csv(t *testing.T) { }, }) } - func TestAccAwsDmsEndpoint_S3_Parquet(t *testing.T) { resourceName := "aws_dms_endpoint.dms_endpoint" randId := acctest.RandString(8) + "-s3" @@ -113,6 +117,12 @@ func TestAccAwsDmsEndpoint_S3_Parquet(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "s3_settings.0.bucket_folder", ""), resource.TestCheckResourceAttr(resourceName, "s3_settings.0.bucket_name", "bucket_name"), resource.TestCheckResourceAttr(resourceName, "s3_settings.0.compression_type", "NONE"), + resource.TestCheckNoResourceAttr(resourceName, "s3_settings.0.enable_statistics"), + resource.TestCheckNoResourceAttr(resourceName, "s3_settings.0.timestamp_column_name"), + resource.TestCheckNoResourceAttr(resourceName, "s3_settings.0.enable_statistics"), + resource.TestCheckNoResourceAttr(resourceName, "s3_settings.0.data_format"), + resource.TestCheckNoResourceAttr(resourceName, "s3_settings.0.parquet_version"), + resource.TestCheckNoResourceAttr(resourceName, "s3_settings.0.server_side_encryption_kms_key_id"), ), }, { @@ -135,6 +145,7 @@ func TestAccAwsDmsEndpoint_S3_Parquet(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "s3_settings.0.data_format", "parquet"), resource.TestCheckResourceAttr(resourceName, "s3_settings.0.parquet_version", "parquet-2-0"), resource.TestCheckResourceAttr(resourceName, "s3_settings.0.encryption_mode", "SSE_S3"), + resource.TestCheckResourceAttrSet(resourceName, "s3_settings.0.data_format"), resource.TestCheckResourceAttrSet(resourceName, "s3_settings.0.server_side_encryption_kms_key_id"), ), }, @@ -346,23 +357,23 @@ func checkDmsEndpointExists(n string) resource.TestCheckFunc { func dmsEndpointBasicConfig(randId string) string { return fmt.Sprintf(` resource "aws_dms_endpoint" "dms_endpoint" { - database_name = "tf-test-dms-db" - endpoint_id = "tf-test-dms-endpoint-%[1]s" - endpoint_type = "source" - engine_name = "aurora" - extra_connection_attributes = "" - password = "tftest" - port = 3306 - server_name = "tftest" - ssl_mode = "none" - - tags = { - Name = "tf-test-dms-endpoint-%[1]s" - Update = "to-update" - Remove = "to-remove" - } - - username = "tftest" + database_name = "tf-test-dms-db" + endpoint_id = "tf-test-dms-endpoint-%[1]s" + endpoint_type = "source" + engine_name = "aurora" + extra_connection_attributes = "" + password = "tftest" + port = 3306 + server_name = "tftest" + ssl_mode = "none" + + tags = { + Name = "tf-test-dms-endpoint-%[1]s" + Update = "to-update" + Remove = "to-remove" + } + + username = "tftest" } `, randId) } @@ -370,23 +381,23 @@ resource "aws_dms_endpoint" "dms_endpoint" { func dmsEndpointBasicConfigUpdate(randId string) string { return fmt.Sprintf(` resource "aws_dms_endpoint" "dms_endpoint" { - database_name = "tf-test-dms-db-updated" - endpoint_id = "tf-test-dms-endpoint-%[1]s" - endpoint_type = "source" - engine_name = "aurora" - extra_connection_attributes = "extra" - password = "tftestupdate" - port = 3303 - server_name = "tftestupdate" - ssl_mode = "none" - - tags = { - Name = "tf-test-dms-endpoint-%[1]s" - Update = "updated" - Add = "added" - } - - username = "tftestupdate" + database_name = "tf-test-dms-db-updated" + endpoint_id = "tf-test-dms-endpoint-%[1]s" + endpoint_type = "source" + engine_name = "aurora" + extra_connection_attributes = "extra" + password = "tftestupdate" + port = 3303 + server_name = "tftestupdate" + ssl_mode = "none" + + tags = { + Name = "tf-test-dms-endpoint-%[1]s" + Update = "updated" + Add = "added" + } + + username = "tftestupdate" } `, randId) } @@ -394,25 +405,25 @@ resource "aws_dms_endpoint" "dms_endpoint" { func dmsEndpointDynamoDbConfig(randId string) string { return fmt.Sprintf(` resource "aws_dms_endpoint" "dms_endpoint" { - endpoint_id = "tf-test-dms-endpoint-%[1]s" - endpoint_type = "target" - engine_name = "dynamodb" - service_access_role = "${aws_iam_role.iam_role.arn}" - ssl_mode = "none" + endpoint_id = "tf-test-dms-endpoint-%[1]s" + endpoint_type = "target" + engine_name = "dynamodb" + service_access_role = "${aws_iam_role.iam_role.arn}" + ssl_mode = "none" - tags = { - Name = "tf-test-dynamodb-endpoint-%[1]s" - Update = "to-update" - Remove = "to-remove" - } + tags = { + Name = "tf-test-dynamodb-endpoint-%[1]s" + Update = "to-update" + Remove = "to-remove" + } - depends_on = ["aws_iam_role_policy.dms_dynamodb_access"] + depends_on = ["aws_iam_role_policy.dms_dynamodb_access"] } resource "aws_iam_role" "iam_role" { - name = "tf-test-iam-dynamodb-role-%[1]s" + name = "tf-test-iam-dynamodb-role-%[1]s" - assume_role_policy = <