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

r/aws_kinesis_stream: Add Support enforce_consumer_deletion attribute #8682

Conversation

teraken0509
Copy link
Contributor

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" comments, they generate extra noise for pull request followers and do not help prioritize the request

Fixes #8420

Release note for CHANGELOG:

r/aws_kinesis_stream: Add Support enforce_consumer_deletion attribute

Output from acceptance testing:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSKinesisStream_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSKinesisStream_ -timeout 120m
=== RUN   TestAccAWSKinesisStream_basic
=== PAUSE TestAccAWSKinesisStream_basic
=== RUN   TestAccAWSKinesisStream_createMultipleConcurrentStreams
=== PAUSE TestAccAWSKinesisStream_createMultipleConcurrentStreams
=== RUN   TestAccAWSKinesisStream_encryptionWithoutKmsKeyThrowsError
=== PAUSE TestAccAWSKinesisStream_encryptionWithoutKmsKeyThrowsError
=== RUN   TestAccAWSKinesisStream_encryption
=== PAUSE TestAccAWSKinesisStream_encryption
=== RUN   TestAccAWSKinesisStream_importBasic
=== PAUSE TestAccAWSKinesisStream_importBasic
=== RUN   TestAccAWSKinesisStream_shardCount
=== PAUSE TestAccAWSKinesisStream_shardCount
=== RUN   TestAccAWSKinesisStream_retentionPeriod
=== PAUSE TestAccAWSKinesisStream_retentionPeriod
=== RUN   TestAccAWSKinesisStream_shardLevelMetrics
=== PAUSE TestAccAWSKinesisStream_shardLevelMetrics
=== RUN   TestAccAWSKinesisStream_enforceConsumerDeletion
=== PAUSE TestAccAWSKinesisStream_enforceConsumerDeletion
=== RUN   TestAccAWSKinesisStream_Tags
=== PAUSE TestAccAWSKinesisStream_Tags
=== CONT  TestAccAWSKinesisStream_basic
=== CONT  TestAccAWSKinesisStream_retentionPeriod
=== CONT  TestAccAWSKinesisStream_shardCount
=== CONT  TestAccAWSKinesisStream_encryption
=== CONT  TestAccAWSKinesisStream_importBasic
=== CONT  TestAccAWSKinesisStream_encryptionWithoutKmsKeyThrowsError
=== CONT  TestAccAWSKinesisStream_createMultipleConcurrentStreams
=== CONT  TestAccAWSKinesisStream_enforceConsumerDeletion
=== CONT  TestAccAWSKinesisStream_shardLevelMetrics
=== CONT  TestAccAWSKinesisStream_Tags
--- PASS: TestAccAWSKinesisStream_encryptionWithoutKmsKeyThrowsError (86.01s)
--- PASS: TestAccAWSKinesisStream_basic (103.23s)
--- PASS: TestAccAWSKinesisStream_enforceConsumerDeletion (103.77s)
--- PASS: TestAccAWSKinesisStream_Tags (134.00s)
--- PASS: TestAccAWSKinesisStream_importBasic (157.76s)
--- PASS: TestAccAWSKinesisStream_retentionPeriod (177.53s)
--- PASS: TestAccAWSKinesisStream_shardCount (216.48s)
--- PASS: TestAccAWSKinesisStream_createMultipleConcurrentStreams (266.76s)
--- PASS: TestAccAWSKinesisStream_encryption (279.71s)
--- PASS: TestAccAWSKinesisStream_shardLevelMetrics (311.63s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	311.714s

@ghost ghost added size/M Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. service/kinesis Issues and PRs that pertain to the kinesis service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels May 18, 2019
@bflad bflad added the enhancement Requests to existing resources that expand the functionality or scope. label May 23, 2019
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Hi @kterada0509 👋 Thanks for contributing this! 🚀 This was implemented well, especially with the new testing.

The only odd part about "virtual" attributes (ones that are Terraform-only or not available in the API, generally used for update/delete handling) is that they require a state migration to prevent a difference during the first Terraform plan where the provider has been upgraded, e.g. to prevent this difference in Terraform 0.11 for example:

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  ~ aws_kinesis_stream.test
      enforce_consumer_deletion: "" => "false"


Plan: 0 to add, 1 to change, 0 to destroy.

We have not even got to documenting the process for how this is now handled in version 0.12 of the Terraform Provider SDK yet though 😅 so spent some time this morning writing up a followup commit (42069b4) and testing it with "real" Terraform 0.11 and 0.12 environments. All good now and we'll get that documentation updated on the Terraform website shortly. 👍

Output from AWS Standard acceptance testing:

--- PASS: TestAccAWSKinesisStream_encryptionWithoutKmsKeyThrowsError (61.96s)
--- PASS: TestAccAWSKinesisStream_basic (73.32s)
--- PASS: TestAccAWSKinesisStream_enforceConsumerDeletion (73.70s)
--- PASS: TestAccAWSKinesisStream_importBasic (73.96s)
--- PASS: TestAccAWSKinesisStream_Tags (127.42s)
--- PASS: TestAccAWSKinesisStream_shardLevelMetrics (134.23s)
--- PASS: TestAccAWSKinesisStream_retentionPeriod (136.24s)
--- PASS: TestAccAWSKinesisStream_createMultipleConcurrentStreams (194.19s)
--- PASS: TestAccAWSKinesisStream_encryption (213.62s)
--- PASS: TestAccAWSKinesisStream_shardCount (265.32s)

Output from AWS GovCloud (US) acceptance testing:

--- PASS: TestAccAWSKinesisStream_encryptionWithoutKmsKeyThrowsError (61.96s)
--- PASS: TestAccAWSKinesisStream_basic (73.32s)
--- PASS: TestAccAWSKinesisStream_enforceConsumerDeletion (73.70s)
--- PASS: TestAccAWSKinesisStream_importBasic (73.96s)
--- PASS: TestAccAWSKinesisStream_Tags (127.42s)
--- PASS: TestAccAWSKinesisStream_shardLevelMetrics (134.23s)
--- PASS: TestAccAWSKinesisStream_retentionPeriod (136.24s)
--- PASS: TestAccAWSKinesisStream_createMultipleConcurrentStreams (194.19s)
--- PASS: TestAccAWSKinesisStream_encryption (213.62s)
--- PASS: TestAccAWSKinesisStream_shardCount (265.32s)

func testAccKinesisStreamConfigWithEnforceConsumerDeletion(rInt int) string {
return fmt.Sprintf(`
resource "aws_kinesis_stream" "test_stream" {
name = "terraform-kinesis-test-%d"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: The whitespace here looks like tabs instead of spaces.

@bflad bflad added this to the v2.12.0 milestone May 23, 2019
@bflad bflad merged commit be79359 into hashicorp:master May 23, 2019
bflad added a commit that referenced this pull request May 23, 2019
@bflad
Copy link
Contributor

bflad commented May 24, 2019

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

@teraken0509 teraken0509 deleted the feature/add-enforce_consumer_deletion-attribute-for-aws_kinesis_stream-resource branch March 5, 2020 14:01
@ghost
Copy link

ghost commented Mar 5, 2020

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 ghost locked and limited conversation to collaborators Mar 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/kinesis Issues and PRs that pertain to the kinesis service. size/M 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.

Kinesis stream enforce deletion
2 participants