resource/aws_kinesis_firehose_delivery_stream: Fix for s3 destinations #2970
Conversation
- import doesn't work for s3 destination
bflad
added
bug
|
Works for me on a newly created Delivery Stream, also verified that a subsequent |
radeksimko
changed the title
Flattener was previously broken, hence drifts were not detected. Adding an error check should prevent breaking it in the future.
Firehose API automatically sets some default parameters which we need to filter out to prevent diffs during routine use of the resource.
|
Hi @ApsOps I didn't pile up just random commits. The guiding line for me were failing acceptance tests which we already have in place. The goal was to get them all pass again which I managed to do with all attached changes. I hope my commit messages are self-explanatory - if not I'm happy to explain/update. There's still one pending bug in this resource, but it's apparently not covered in any test, so we can address it in a separate PR. Test results |
|
@radeksimko Thanks so much! It would be awesome if we could have acceptance tests running on travis. I think I wasn't able to get them working properly, and it costs more if tests fail which leaves some resources lying around sometimes. |
| "parameter_value": params.ParameterValue, | ||
| name, value := *params.ParameterName, *params.ParameterValue | ||
|
|
||
| if t == "Lambda" { |
There was a problem hiding this comment.
Nitpick: Constant available: firehose.ProcessorTypeLambda
| @@ -269,31 +298,36 @@ func flattenKinesisFirehoseDeliveryStream(d *schema.ResourceData, s *firehose.De | |||
| elasticsearchConfList[0] = elasticsearchConfiguration | |||
| d.Set("elasticsearch_configuration", elasticsearchConfList) | |||
| d.Set("s3_configuration", flattenFirehoseS3Configuration(*destination.ElasticsearchDestinationDescription.S3DestinationDescription)) | |||
| } else if destination.S3DestinationDescription != nil { | |||
| } else if d.Get("destination").(string) == "s3" { | |||
There was a problem hiding this comment.
Question: Should we create constants for these?
There was a problem hiding this comment.
As discussed on Slack we will eventually phase out this field as it likely doesn't need to be there at all.
|
Could it be related to #2997 ? |
|
@pecigonzalo Yes, this should fix #2997 |
|
This has been released in terraform-provider-aws version 1.7.1. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. Please note we are tracking one additional crash relating to the (lack of) |
hashicorp#2970) * Fix kinesis firehose stream breaking for s3 destinations - import doesn't work for s3 destination * Run tests for extended_s3 config * Fix invalid test config * Avoid crash on empty redshift's S3BackupDescription Fixes hashicorp#3006 * Fix flattenProcessingConfiguration Flattener was previously broken, hence drifts were not detected. Adding an error check should prevent breaking it in the future. * Filter out default params in processing configuration Firehose API automatically sets some default parameters which we need to filter out to prevent diffs during routine use of the resource. * Suppress diff for redshift_configuration.0.password * Replace string with constant
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
ghost
locked and limited conversation to collaborators
Fixes the breaking functionality caused by #2082