Skip to content

Commit

Permalink
resource/aws_kinesis_firehose_delivery_stream: Address PR #9103 feedback
Browse files Browse the repository at this point in the history 
Reference: #9103

Mainly reverting some of the changes to processing_configuration/error_output_prefix and adding DiffSuppressFunc to handle the processing_configuration configuration block attribute.

Output from acceptance testing:

```
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3_ErrorOutputPrefix (151.81s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_missingProcessingConfiguration (158.88s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3_DataFormatConversionConfiguration_OrcSerDe_Empty (168.93s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3_ExternalUpdate (169.36s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_SplunkConfigUpdates (170.83s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_s3KinesisStreamSource (136.10s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3_DataFormatConversionConfiguration_Deserializer_Update (179.46s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3Updates (180.75s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_importBasic (187.90s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3_DataFormatConversionConfiguration_Enabled (190.90s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3_DataFormatConversionConfiguration_ParquetSerDe_Empty (192.30s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3_DataFormatConversionConfiguration_Serializer_Update (196.54s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3basic (200.98s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3_DataFormatConversionConfiguration_OpenXJsonSerDe_Empty (202.37s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3_DataFormatConversionConfiguration_HiveJsonSerDe_Empty (222.59s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3KmsKeyArn (226.21s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_s3WithCloudwatchLogging (189.02s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_s3basic (136.60s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_s3ConfigUpdates (303.77s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_s3basicWithTags (184.97s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_RedshiftConfigUpdates (808.04s)
--- PASS: TestAccAWSKinesisFirehoseDeliveryStream_ElasticsearchConfigUpdates (970.67s)
```
  • Loading branch information
@bflad
bflad committed Aug 20, 2019
1 parent 5521968 commit f2da6cc
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 28 deletions.
35 changes: 9 additions & 26 deletions aws/resource_aws_kinesis_firehose_delivery_stream.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,10 @@ func s3ConfigurationSchema() *schema.Schema {

func processingConfigurationSchema() *schema.Schema {
return &schema.Schema{
Type: schema.TypeList,
Optional: true,
MaxItems: 1,
Type: schema.TypeList,
Optional: true,
MaxItems: 1,
DiffSuppressFunc: suppressMissingOptionalConfigurationBlock,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"enabled": {
Expand Down Expand Up @@ -336,9 +337,9 @@ func flattenFirehoseDataFormatConversionConfiguration(dfcc *firehose.DataFormatC
// The AWS SDK can represent "no data format conversion configuration" in two ways:
// 1. With a nil value
// 2. With enabled set to false and nil for ALL the config sections.
// These two cases are equivalent so we use the same state description, to avoid diffs.
// We normalize this with an empty configuration in the state due
// to the existing Default: true on the enabled attribute.
if !enabled && len(ifc) == 0 && len(ofc) == 0 && len(sc) == 0 {
log.Printf("Found ambiguous AWS response")
return []map[string]interface{}{}
}

Expand Down Expand Up @@ -545,16 +546,6 @@ func flattenProcessingConfiguration(pc *firehose.ProcessingConfiguration, roleAr
return []map[string]interface{}{}
}

enabled := aws.BoolValue(pc.Enabled)

// The AWS SDK can represent "no processing configuration" in two ways:
// 1. With a nil value
// 2. With an empty processor list and enabled set to false.
// These are equivalent so we use the same state description, to avoid diffs.
if !enabled && len(pc.Processors) == 0 {
return []map[string]interface{}{}
}

processingConfiguration := make([]map[string]interface{}, 1)

// It is necessary to explicitly filter this out
Expand Down Expand Up @@ -595,7 +586,7 @@ func flattenProcessingConfiguration(pc *firehose.ProcessingConfiguration, roleAr
}
}
processingConfiguration[0] = map[string]interface{}{
"enabled": enabled,
"enabled": aws.BoolValue(pc.Enabled),
"processors": processors,
}
return processingConfiguration
Expand Down Expand Up @@ -1426,12 +1417,8 @@ func createExtendedS3Config(d *schema.ResourceData) *firehose.ExtendedS3Destinat
configuration.CloudWatchLoggingOptions = extractCloudWatchLoggingConfiguration(s3)
}

if v, ok := s3["error_output_prefix"]; ok {
if v, ok := s3["error_output_prefix"]; ok && v.(string) != "" {
configuration.ErrorOutputPrefix = aws.String(v.(string))
} else {
// It is possible to just pass nil here, but this seems to be the
// canonical form that AWS uses, and is less likely to produce diffs.
configuration.ErrorOutputPrefix = aws.String("")
}

if s3BackupMode, ok := s3["s3_backup_mode"]; ok {
Expand Down Expand Up @@ -1515,12 +1502,8 @@ func updateExtendedS3Config(d *schema.ResourceData) *firehose.ExtendedS3Destinat
configuration.CloudWatchLoggingOptions = extractCloudWatchLoggingConfiguration(s3)
}

if v, ok := s3["error_output_prefix"]; ok {
if v, ok := s3["error_output_prefix"]; ok && v.(string) != "" {
configuration.ErrorOutputPrefix = aws.String(v.(string))
} else {
// It is possible to just pass nil here, but this seems to be the
// canonical form that AWS uses, and is less likely to produce diffs.
configuration.ErrorOutputPrefix = aws.String("")
}

if s3BackupMode, ok := s3["s3_backup_mode"]; ok {
Expand Down
13 changes: 11 additions & 2 deletions aws/resource_aws_kinesis_firehose_delivery_stream_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,15 @@ func TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3_DataFormatConversionConf
resource.TestCheckResourceAttr(resourceName, "extended_s3_configuration.0.data_format_conversion_configuration.0.enabled", "true"),
),
},
{
Config: testAccKinesisFirehoseDeliveryStreamConfig_ExtendedS3_DataFormatConversionConfiguration_Enabled(rName, rInt, false),
Check: resource.ComposeTestCheckFunc(
testAccCheckKinesisFirehoseDeliveryStreamExists(resourceName, &stream),
resource.TestCheckResourceAttr(resourceName, "extended_s3_configuration.#", "1"),
resource.TestCheckResourceAttr(resourceName, "extended_s3_configuration.0.data_format_conversion_configuration.#", "1"),
resource.TestCheckResourceAttr(resourceName, "extended_s3_configuration.0.data_format_conversion_configuration.0.enabled", "false"),
),
},
},
})
}
Expand All @@ -356,7 +365,7 @@ func TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3_ExternalUpdate(t *testin
testAccCheckKinesisFirehoseDeliveryStreamExists(resourceName, &stream),
resource.TestCheckResourceAttr(resourceName, "extended_s3_configuration.#", "1"),
resource.TestCheckResourceAttr(resourceName, "extended_s3_configuration.0.data_format_conversion_configuration.#", "0"),
resource.TestCheckResourceAttr(resourceName, "extended_s3_configuration.0.processing_configuration.#", "0"),
resource.TestCheckResourceAttr(resourceName, "extended_s3_configuration.0.processing_configuration.#", "1"),
),
},
{
Expand Down Expand Up @@ -386,7 +395,7 @@ func TestAccAWSKinesisFirehoseDeliveryStream_ExtendedS3_ExternalUpdate(t *testin
testAccCheckKinesisFirehoseDeliveryStreamExists(resourceName, &stream),
resource.TestCheckResourceAttr(resourceName, "extended_s3_configuration.#", "1"),
resource.TestCheckResourceAttr(resourceName, "extended_s3_configuration.0.data_format_conversion_configuration.#", "0"),
resource.TestCheckResourceAttr(resourceName, "extended_s3_configuration.0.processing_configuration.#", "0"),
resource.TestCheckResourceAttr(resourceName, "extended_s3_configuration.0.processing_configuration.#", "1"),
),
},
},
Expand Down
2 changes: 2 additions & 0 deletions website/docs/r/kinesis_firehose_delivery_stream.html.markdown
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,8 @@ The `parameters` array objects support the following:

### data_format_conversion_configuration

~> **NOTE:** Once configured, the data format conversion configuration can only be disabled, in which the configuration values will remain, but will not be active. It is not currently possible to completely remove the configuration without recreating the resource.

Example:

```hcl
Expand Down

0 comments on commit f2da6cc

Please sign in to comment.