Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Tag changes do not cause updates to firehose destinations #26451

Merged
merged 2 commits into from
Aug 23, 2022

Conversation

mbamber
Copy link
Contributor

@mbamber mbamber commented Aug 23, 2022

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #25396

Output from acceptance testing:

$ AWS_DEFAULT_REGION=eu-west-1 AWS_SDK_LOAD_CONFIG=1 AWS_PROFILE=dev make testacc TESTS=TestAccFirehoseDeliveryStream_tagUpdates PKG=firehose
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/firehose/... -v -count 1 -parallel 20 -run='TestAccFirehoseDeliveryStream_tagUpdates'  -timeout 180m
=== RUN   TestAccFirehoseDeliveryStream_tagUpdates
=== PAUSE TestAccFirehoseDeliveryStream_tagUpdates
=== CONT  TestAccFirehoseDeliveryStream_tagUpdates
--- PASS: TestAccFirehoseDeliveryStream_tagUpdates (375.03s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/firehose	377.636s

I don't really know how to test that this fixes the underlying issue, as the redshift password is not returned in API requests (and as such it's not possible to check that it hasn't been changed, AFAIK). But from comments on the ticket this appears to be the correct solution. Any advice here greatly appreciated.

@github-actions
Copy link

Hey @mbamber 👋 Thank you very much for your contribution! At times, our maintainers need to make direct edits to pull requests in order to help get it ready to be merged. Your current settings do not allow maintainers to make such edits. To help facilitate this, update your pull request to allow such edits as described in GitHub's Allowing changes to a pull request branch created from a fork documentation. (If you're using a fork owned by an organization, your organization may not allow you to change this setting. If that is the case, let us know.)

@github-actions github-actions bot added service/firehose Issues and PRs that pertain to the firehose service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. needs-triage Waiting for first response or review from a maintainer. size/L Managed by automation to categorize the size of a PR. labels Aug 23, 2022
@mbamber mbamber marked this pull request as ready for review August 23, 2022 17:19
@justinretzolk justinretzolk added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels Aug 23, 2022
@mbamber
Copy link
Contributor Author

mbamber commented Aug 23, 2022

Your current settings do not allow maintainers to make such edits

I can't see a setting to change this anywhere...

@ewbankkit
Copy link
Contributor

@mbamber I think that's because the PR has been submitted from a fork in an organization's GitHub account, not a personal account; That's to be expected.

.changelog/26451.txt Outdated Show resolved Hide resolved
Co-authored-by: Kit Ewbank <Kit_Ewbank@hotmail.com>
Copy link
Contributor

@ewbankkit ewbankkit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀.

% make testacc TESTARGS='-run=TestAccFirehoseDeliveryStream_' PKG=firehose ACCTEST_PARALLELISM=2
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/firehose/... -v -count 1 -parallel 2  -run=TestAccFirehoseDeliveryStream_ -timeout 180m
=== RUN   TestAccFirehoseDeliveryStream_basic
=== PAUSE TestAccFirehoseDeliveryStream_basic
=== RUN   TestAccFirehoseDeliveryStream_disappears
=== PAUSE TestAccFirehoseDeliveryStream_disappears
=== RUN   TestAccFirehoseDeliveryStream_s3basic
=== PAUSE TestAccFirehoseDeliveryStream_s3basic
=== RUN   TestAccFirehoseDeliveryStream_s3basicWithPrefixes
=== PAUSE TestAccFirehoseDeliveryStream_s3basicWithPrefixes
=== RUN   TestAccFirehoseDeliveryStream_s3basicWithSSE
=== PAUSE TestAccFirehoseDeliveryStream_s3basicWithSSE
=== RUN   TestAccFirehoseDeliveryStream_s3basicWithSSEAndKeyARN
=== PAUSE TestAccFirehoseDeliveryStream_s3basicWithSSEAndKeyARN
=== RUN   TestAccFirehoseDeliveryStream_s3basicWithSSEAndKeyType
=== PAUSE TestAccFirehoseDeliveryStream_s3basicWithSSEAndKeyType
=== RUN   TestAccFirehoseDeliveryStream_s3basicWithTags
=== PAUSE TestAccFirehoseDeliveryStream_s3basicWithTags
=== RUN   TestAccFirehoseDeliveryStream_s3KinesisStreamSource
=== PAUSE TestAccFirehoseDeliveryStream_s3KinesisStreamSource
=== RUN   TestAccFirehoseDeliveryStream_s3WithCloudWatchLogging
=== PAUSE TestAccFirehoseDeliveryStream_s3WithCloudWatchLogging
=== RUN   TestAccFirehoseDeliveryStream_s3Updates
=== PAUSE TestAccFirehoseDeliveryStream_s3Updates
=== RUN   TestAccFirehoseDeliveryStream_extendedS3basic
=== PAUSE TestAccFirehoseDeliveryStream_extendedS3basic
=== RUN   TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversion_enabled
=== PAUSE TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversion_enabled
=== RUN   TestAccFirehoseDeliveryStream_ExtendedS3_externalUpdate
=== PAUSE TestAccFirehoseDeliveryStream_ExtendedS3_externalUpdate
=== RUN   TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionDeserializer_update
=== PAUSE TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionDeserializer_update
=== RUN   TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionHiveJSONSerDe_empty
=== PAUSE TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionHiveJSONSerDe_empty
=== RUN   TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionOpenXJSONSerDe_empty
=== PAUSE TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionOpenXJSONSerDe_empty
=== RUN   TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionOrcSerDe_empty
=== PAUSE TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionOrcSerDe_empty
=== RUN   TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionParquetSerDe_empty
=== PAUSE TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionParquetSerDe_empty
=== RUN   TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionSerializer_update
=== PAUSE TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionSerializer_update
=== RUN   TestAccFirehoseDeliveryStream_ExtendedS3_errorOutputPrefix
=== PAUSE TestAccFirehoseDeliveryStream_ExtendedS3_errorOutputPrefix
=== RUN   TestAccFirehoseDeliveryStream_ExtendedS3_S3BackupConfiguration_ErrorOutputPrefix
=== PAUSE TestAccFirehoseDeliveryStream_ExtendedS3_S3BackupConfiguration_ErrorOutputPrefix
=== RUN   TestAccFirehoseDeliveryStream_ExtendedS3Processing_empty
=== PAUSE TestAccFirehoseDeliveryStream_ExtendedS3Processing_empty
=== RUN   TestAccFirehoseDeliveryStream_extendedS3KMSKeyARN
=== PAUSE TestAccFirehoseDeliveryStream_extendedS3KMSKeyARN
=== RUN   TestAccFirehoseDeliveryStream_extendedS3DynamicPartitioning
=== PAUSE TestAccFirehoseDeliveryStream_extendedS3DynamicPartitioning
=== RUN   TestAccFirehoseDeliveryStream_extendedS3Updates
=== PAUSE TestAccFirehoseDeliveryStream_extendedS3Updates
=== RUN   TestAccFirehoseDeliveryStream_ExtendedS3_kinesisStreamSource
=== PAUSE TestAccFirehoseDeliveryStream_ExtendedS3_kinesisStreamSource
=== RUN   TestAccFirehoseDeliveryStream_redshiftUpdates
=== PAUSE TestAccFirehoseDeliveryStream_redshiftUpdates
=== RUN   TestAccFirehoseDeliveryStream_tagUpdates
=== PAUSE TestAccFirehoseDeliveryStream_tagUpdates
=== RUN   TestAccFirehoseDeliveryStream_splunkUpdates
=== PAUSE TestAccFirehoseDeliveryStream_splunkUpdates
=== RUN   TestAccFirehoseDeliveryStream_Splunk_ErrorOutputPrefix
=== PAUSE TestAccFirehoseDeliveryStream_Splunk_ErrorOutputPrefix
=== RUN   TestAccFirehoseDeliveryStream_httpEndpoint
=== PAUSE TestAccFirehoseDeliveryStream_httpEndpoint
=== RUN   TestAccFirehoseDeliveryStream_HTTPEndpoint_ErrorOutputPrefix
=== PAUSE TestAccFirehoseDeliveryStream_HTTPEndpoint_ErrorOutputPrefix
=== RUN   TestAccFirehoseDeliveryStream_HTTPEndpoint_retryDuration
=== PAUSE TestAccFirehoseDeliveryStream_HTTPEndpoint_retryDuration
=== RUN   TestAccFirehoseDeliveryStream_elasticSearchUpdates
=== PAUSE TestAccFirehoseDeliveryStream_elasticSearchUpdates
=== RUN   TestAccFirehoseDeliveryStream_elasticSearchEndpointUpdates
=== PAUSE TestAccFirehoseDeliveryStream_elasticSearchEndpointUpdates
=== RUN   TestAccFirehoseDeliveryStream_elasticSearchWithVPCUpdates
=== PAUSE TestAccFirehoseDeliveryStream_elasticSearchWithVPCUpdates
=== RUN   TestAccFirehoseDeliveryStream_Elasticsearch_ErrorOutputPrefix
=== PAUSE TestAccFirehoseDeliveryStream_Elasticsearch_ErrorOutputPrefix
=== RUN   TestAccFirehoseDeliveryStream_missingProcessing
=== PAUSE TestAccFirehoseDeliveryStream_missingProcessing
=== CONT  TestAccFirehoseDeliveryStream_basic
=== CONT  TestAccFirehoseDeliveryStream_ExtendedS3_errorOutputPrefix
--- PASS: TestAccFirehoseDeliveryStream_basic (97.79s)
=== CONT  TestAccFirehoseDeliveryStream_s3Updates
--- PASS: TestAccFirehoseDeliveryStream_ExtendedS3_errorOutputPrefix (159.23s)
=== CONT  TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionSerializer_update
--- PASS: TestAccFirehoseDeliveryStream_s3Updates (154.91s)
=== CONT  TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionParquetSerDe_empty
--- PASS: TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionSerializer_update (224.90s)
=== CONT  TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionOrcSerDe_empty
--- PASS: TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionParquetSerDe_empty (133.67s)
=== CONT  TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionOpenXJSONSerDe_empty
--- PASS: TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionOpenXJSONSerDe_empty (113.61s)
=== CONT  TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionHiveJSONSerDe_empty
--- PASS: TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionOrcSerDe_empty (131.43s)
=== CONT  TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionDeserializer_update
--- PASS: TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionHiveJSONSerDe_empty (112.40s)
=== CONT  TestAccFirehoseDeliveryStream_ExtendedS3_externalUpdate
--- PASS: TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversionDeserializer_update (131.83s)
=== CONT  TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversion_enabled
--- PASS: TestAccFirehoseDeliveryStream_ExtendedS3_externalUpdate (113.23s)
=== CONT  TestAccFirehoseDeliveryStream_extendedS3basic
--- PASS: TestAccFirehoseDeliveryStream_ExtendedS3DataFormatConversion_enabled (143.78s)
=== CONT  TestAccFirehoseDeliveryStream_Splunk_ErrorOutputPrefix
--- PASS: TestAccFirehoseDeliveryStream_extendedS3basic (174.20s)
=== CONT  TestAccFirehoseDeliveryStream_missingProcessing
--- PASS: TestAccFirehoseDeliveryStream_Splunk_ErrorOutputPrefix (147.19s)
=== CONT  TestAccFirehoseDeliveryStream_Elasticsearch_ErrorOutputPrefix
--- PASS: TestAccFirehoseDeliveryStream_missingProcessing (144.51s)
=== CONT  TestAccFirehoseDeliveryStream_elasticSearchWithVPCUpdates
=== CONT  TestAccFirehoseDeliveryStream_Elasticsearch_ErrorOutputPrefix
    delivery_stream_test.go:1565: Step 1/5 error: Error running apply: exit status 1
        
        Error: error waiting for Kinesis Firehose Delivery Stream (tf-acc-test-1009201887386327658) create: timeout while waiting for state to become 'ACTIVE' (last state: 'CREATING', timeout: 20m0s)
        
          with aws_kinesis_firehose_delivery_stream.test,
          on terraform_plugin_test.tf line 129, in resource "aws_kinesis_firehose_delivery_stream" "test":
         129: resource "aws_kinesis_firehose_delivery_stream" "test" {
        
--- FAIL: TestAccFirehoseDeliveryStream_Elasticsearch_ErrorOutputPrefix (2455.31s)
=== CONT  TestAccFirehoseDeliveryStream_elasticSearchEndpointUpdates
--- PASS: TestAccFirehoseDeliveryStream_elasticSearchWithVPCUpdates (3181.78s)
=== CONT  TestAccFirehoseDeliveryStream_elasticSearchUpdates
--- PASS: TestAccFirehoseDeliveryStream_elasticSearchEndpointUpdates (1402.76s)
=== CONT  TestAccFirehoseDeliveryStream_HTTPEndpoint_retryDuration
--- PASS: TestAccFirehoseDeliveryStream_HTTPEndpoint_retryDuration (210.15s)
=== CONT  TestAccFirehoseDeliveryStream_HTTPEndpoint_ErrorOutputPrefix
--- PASS: TestAccFirehoseDeliveryStream_HTTPEndpoint_ErrorOutputPrefix (161.22s)
=== CONT  TestAccFirehoseDeliveryStream_httpEndpoint
--- PASS: TestAccFirehoseDeliveryStream_httpEndpoint (175.61s)
=== CONT  TestAccFirehoseDeliveryStream_s3basicWithSSEAndKeyARN
--- PASS: TestAccFirehoseDeliveryStream_elasticSearchUpdates (1490.50s)
=== CONT  TestAccFirehoseDeliveryStream_s3WithCloudWatchLogging
--- PASS: TestAccFirehoseDeliveryStream_s3basicWithSSEAndKeyARN (384.00s)
=== CONT  TestAccFirehoseDeliveryStream_s3KinesisStreamSource
--- PASS: TestAccFirehoseDeliveryStream_s3KinesisStreamSource (58.43s)
=== CONT  TestAccFirehoseDeliveryStream_s3basicWithTags
--- PASS: TestAccFirehoseDeliveryStream_s3WithCloudWatchLogging (151.61s)
=== CONT  TestAccFirehoseDeliveryStream_s3basicWithSSEAndKeyType
--- PASS: TestAccFirehoseDeliveryStream_s3basicWithTags (173.94s)
=== CONT  TestAccFirehoseDeliveryStream_tagUpdates
--- PASS: TestAccFirehoseDeliveryStream_s3basicWithSSEAndKeyType (263.72s)
=== CONT  TestAccFirehoseDeliveryStream_splunkUpdates
--- PASS: TestAccFirehoseDeliveryStream_tagUpdates (383.38s)
=== CONT  TestAccFirehoseDeliveryStream_extendedS3KMSKeyARN
--- PASS: TestAccFirehoseDeliveryStream_splunkUpdates (219.95s)
=== CONT  TestAccFirehoseDeliveryStream_extendedS3Updates
--- PASS: TestAccFirehoseDeliveryStream_extendedS3KMSKeyARN (121.64s)
=== CONT  TestAccFirehoseDeliveryStream_redshiftUpdates
--- PASS: TestAccFirehoseDeliveryStream_extendedS3Updates (152.63s)
=== CONT  TestAccFirehoseDeliveryStream_ExtendedS3_kinesisStreamSource
--- PASS: TestAccFirehoseDeliveryStream_ExtendedS3_kinesisStreamSource (71.04s)
=== CONT  TestAccFirehoseDeliveryStream_extendedS3DynamicPartitioning
--- PASS: TestAccFirehoseDeliveryStream_extendedS3DynamicPartitioning (142.56s)
=== CONT  TestAccFirehoseDeliveryStream_ExtendedS3Processing_empty
--- PASS: TestAccFirehoseDeliveryStream_ExtendedS3Processing_empty (111.29s)
=== CONT  TestAccFirehoseDeliveryStream_ExtendedS3_S3BackupConfiguration_ErrorOutputPrefix
--- PASS: TestAccFirehoseDeliveryStream_redshiftUpdates (379.82s)
=== CONT  TestAccFirehoseDeliveryStream_s3basicWithPrefixes
--- PASS: TestAccFirehoseDeliveryStream_s3basicWithPrefixes (146.94s)
=== CONT  TestAccFirehoseDeliveryStream_s3basicWithSSE
--- PASS: TestAccFirehoseDeliveryStream_ExtendedS3_S3BackupConfiguration_ErrorOutputPrefix (167.36s)
=== CONT  TestAccFirehoseDeliveryStream_disappears
--- PASS: TestAccFirehoseDeliveryStream_disappears (149.73s)
=== CONT  TestAccFirehoseDeliveryStream_s3basic
--- PASS: TestAccFirehoseDeliveryStream_s3basic (103.80s)
--- PASS: TestAccFirehoseDeliveryStream_s3basicWithSSE (278.92s)
FAIL
FAIL	github.com/hashicorp/terraform-provider-aws/internal/service/firehose	7275.161s

Failure is unrelated to this change.

@ewbankkit
Copy link
Contributor

@mbamber Thanks for the contribution 🎉 👏.

@ewbankkit ewbankkit merged commit d0dab16 into hashicorp:main Aug 23, 2022
@github-actions github-actions bot added this to the v4.28.0 milestone Aug 23, 2022
@github-actions
Copy link

This functionality has been released in v4.28.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 26, 2022
@mbamber mbamber deleted the fix-25396 branch September 27, 2022 10:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/firehose Issues and PRs that pertain to the firehose service. size/L Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updating tags for aws_kinesis_firehose_delivery_stream also updates the Redshift delivery settings
3 participants