Adds Kinesis Stream Consumer resource and data source (third time) #17149

anthonyroach · 2021-01-16T22:55:43Z

This adds support for AWS Kinesis Stream Consumers, that allow for a dedicated consumer using enhanced fan-out.

Changes proposed in this pull request:

Adds aws_kinesis_stream_consumer as new resource
Adds aws_kinesis_stream_consumer as new data source

It is based on the original contribution of #10487

Changes on top of #10487:

rebased onto master, and squashed
added warning on resource removal on not found during read
set the resource id immediately after creation
fixed import id syntax to match the documentation
added an import test
removed Optional from arn output in schema

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 #7611

Release note for CHANGELOG:

FEATURES:
New Data Source: aws_kinesis_stream_consumer
New Resource: aws_kinesis_stream_consumer

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSKinesisStreamConsumer'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSKinesisStreamConsumer -timeout 120m
=== RUN   TestAccAWSKinesisStreamConsumerDataSource_basic
=== PAUSE TestAccAWSKinesisStreamConsumerDataSource_basic
=== RUN   TestAccAWSKinesisStreamConsumer_basic
=== PAUSE TestAccAWSKinesisStreamConsumer_basic
=== RUN   TestAccAWSKinesisStreamConsumer_createMultipleConcurrentStreamConsumers
=== PAUSE TestAccAWSKinesisStreamConsumer_createMultipleConcurrentStreamConsumers
=== CONT  TestAccAWSKinesisStreamConsumerDataSource_basic
=== CONT  TestAccAWSKinesisStreamConsumer_basic
=== CONT  TestAccAWSKinesisStreamConsumer_createMultipleConcurrentStreamConsumers
--- PASS: TestAccAWSKinesisStreamConsumer_createMultipleConcurrentStreamConsumers (75.59s)
--- PASS: TestAccAWSKinesisStreamConsumerDataSource_basic (76.44s)
--- PASS: TestAccAWSKinesisStreamConsumer_basic (77.05s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	77.125s

anthonyroach · 2021-01-16T23:00:53Z

I hope this can get merged soon. Issue #7611 that requested this change is almost two years old now, and the original PR #6565 is even older. I only made very minor changes to the previous PR #10487 to satisfy the contribution requirements and fix a bug in the import implementation.

github-actions

Welcome @anthonyroach 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

anGie44 · 2021-03-18T00:38:37Z

Hi @anthonyroach, thank you for this PR! Since it extends #10487, I've merged in commits from that PR into here so that the contributor gets credit for their work, nevertheless your commits should appear unchanged (if you have any questions please reach out!). Quick note, to get this highly requested PR into an upcoming release, I'm going to provide a review and may make some changes to bring the work inline with current provider conventions if needed.

anthonyroach · 2021-03-18T00:54:07Z

Hi @anthonyroach, thank you for this PR! Since it extends #10487, I've merged in commits from that PR into here so that the contributor gets credit for their work, nevertheless your commits should appear unchanged (if you have any questions please reach out!). Quick note, to get this highly requested PR into an upcoming release, I'm going to provide a review and may make some changes to bring the work inline with current provider conventions if needed.

Thanks. Unfortunately merging the branches seems to have broken some things due to conflicting changes. I suppose in retrospect I should have not squashed the change like I did. Not sure what the best way to fix this is.

anGie44 · 2021-03-18T00:59:09Z

Ohh @anthonyroach are you referring to the This branch cannot be rebased due to conflicts message or the failing CI check, the latter of which seems to be coming from the main branch. I'll take a look at the rebase related conflicts and see if it can be easily updated 👍

anthonyroach · 2021-03-18T01:03:33Z

Ohh @anthonyroach are you referring to the This branch cannot be rebased due to conflicts message or the failing CI check, the latter of which seems to be coming from the main branch. I'll take a look at the rebase related conflicts and see if it can be easily updated

Yeah the failing CI checks, but looking more closely those aren't lines the original author or I changed, and I'm not able to reproduce the failure locally:

aws/resource_aws_ami.go:245:4: duplicate key "owner_id" in map literal
	previous key at aws/resource_aws_ami.go:189:4
aws/resource_aws_ami_copy.go:217:4: duplicate key "owner_id" in map literal
	previous key at aws/resource_aws_ami_copy.go:158:4
aws/resource_aws_ami_from_instance.go:204:4: duplicate key "owner_id" in map literal
	previous key at aws/resource_aws_ami_from_instance.go:145:4
Error: Process completed with exit code 2.

anthonyroach · 2021-03-18T01:05:35Z

Oh, I see, the failures are coming from main which this branch is being rebased onto as part of the CI checks. Makes sense.

@rockycore

Adapted from original commits from @rockycore at yomagroup:feature/kinesis-stream-consumer. Changes on top of the original commits: * rebased onto master, and squashed * added warning resource removal on not found during read * set the resource id immediately after creation * fixed import id syntax to match the documentation * added an import test * removed Optional from arn output in schema

anGie44 · 2021-03-18T06:46:01Z

aws/data_source_aws_kinesis_stream_consumer.go

+	cn := d.Get("name").(string)
+	sa := d.Get("stream_arn").(string)
+
+	state, err := readKinesisStreamConsumerState(conn, cn, sa)


we can expand the search functionality (by name and/or arn, as well as neither) by using the ListStreamConsumers API method, so I'll update here w/usage

anGie44 · 2021-03-18T06:48:13Z

aws/data_source_aws_kinesis_stream_consumer_test.go

+func TestAccAWSKinesisStreamConsumerDataSource_basic(t *testing.T) {
+	var stream kinesis.StreamDescription
+	var consumer kinesis.ConsumerDescription
+	config := createAccKinesisStreamConsumerConfig()


interesting usage! just for maintainability, and current recommendations, we should however prefer to reference functions that are more explicit when defining resources and/or data-sources. I'll update these tests with updated examples.

anGie44 · 2021-03-18T06:49:52Z

aws/resource_aws_kinesis_stream_consumer.go

+			Delete: schema.DefaultTimeout(120 * time.Minute),
+		},
+
+		Importer: &schema.ResourceImporter{


since the DescribeStreamConsumer and Deregister API methods support searching by ARN, we can set the ID with the determined ARN for easier lookup throughout 👍

anGie44 · 2021-03-18T06:51:28Z

aws/resource_aws_kinesis_stream_consumer.go

+		Read:   resourceAwsKinesisStreamConsumerRead,
+		Delete: resourceAwsKinesisStreamConsumerDelete,
+
+		Timeouts: &schema.ResourceTimeout{


generally, we prefer to handle timeout settings within the status and waiter functions on create/delete in place of operator-defined timeouts

anGie44 · 2021-03-18T06:52:43Z

aws/resource_aws_kinesis_stream_consumer.go

+	d.Set("arn", arn)
+
+	// No error, wait for ACTIVE state
+	stateConf := &resource.StateChangeConf{


this is great! just moving this out into the service package

anGie44 · 2021-03-18T06:53:41Z

aws/resource_aws_kinesis_stream_consumer.go

+	cn := d.Get("name").(string)
+	sa := d.Get("stream_arn").(string)
+
+	state, err := readKinesisStreamConsumerState(conn, cn, sa)


since we'll have the ARN stored as the ID, resource lookup will be updated to use that attribute and will be relocated in the service package as well

anGie44

Thanks again @anthonyroach 🚀 !

Output of acceptance tests (commercial):

--- PASS: TestAccAWSKinesisStreamConsumer_disappears (80.92s)
--- PASS: TestAccAWSKinesisStreamConsumerDataSource_Arn (83.21s)
--- PASS: TestAccAWSKinesisStreamConsumerDataSource_Name (83.12s)
--- PASS: TestAccAWSKinesisStreamConsumer_basic (84.88s)
--- PASS: TestAccAWSKinesisStream_shardLevelMetrics (129.77s)
--- PASS: TestAccAWSKinesisStreamConsumerDataSource_basic (135.51s)
--- PASS: TestAccAWSKinesisStream_encryptionWithoutKmsKeyThrowsError (149.59s)
--- PASS: TestAccAWSKinesisStream_retentionPeriod (150.75s)
--- PASS: TestAccAWSKinesisStream_shardCount (163.92s)
--- PASS: TestAccAWSKinesisStream_basic (171.69s)
--- PASS: TestAccAWSKinesisStream_enforceConsumerDeletion (173.03s)
--- PASS: TestAccAWSKinesisStream_UpdateKmsKeyId (174.11s)
--- PASS: TestAccAWSKinesisStreamConsumer_MaxConcurrentConsumers (178.19s)
--- PASS: TestAccAWSKinesisStream_encryption (183.20s)
--- PASS: TestAccAWSKinesisStream_createMultipleConcurrentStreams (192.18s)
--- PASS: TestAccAWSKinesisStreamConsumer_ExceedMaxConcurrentConsumers (200.52s)
--- PASS: TestAccAWSKinesisStream_Tags (217.47s)
--- PASS: TestAccAWSKinesisStreamDataSource_basic (224.17s)

Gov cloud:

--- PASS: TestAccAWSKinesisStreamConsumerDataSource_Name (74.85s)
--- PASS: TestAccAWSKinesisStreamConsumer_MaxConcurrentConsumers (81.56s)
--- PASS: TestAccAWSKinesisStreamConsumerDataSource_basic (82.01s)
--- PASS: TestAccAWSKinesisStream_encryptionWithoutKmsKeyThrowsError (83.29s)
--- PASS: TestAccAWSKinesisStream_basic (85.14s)
--- PASS: TestAccAWSKinesisStreamConsumerDataSource_Arn (90.26s)
--- PASS: TestAccAWSKinesisStreamConsumer_disappears (91.88s)
--- PASS: TestAccAWSKinesisStreamConsumer_ExceedMaxConcurrentConsumers (98.85s)
--- PASS: TestAccAWSKinesisStream_retentionPeriod (99.19s)
--- PASS: TestAccAWSKinesisStreamConsumer_basic (103.76s)
--- PASS: TestAccAWSKinesisStreamDataSource_basic (105.31s)
--- PASS: TestAccAWSKinesisStream_shardCount (114.16s)
--- PASS: TestAccAWSKinesisStream_shardLevelMetrics (119.68s)
--- PASS: TestAccAWSKinesisStream_createMultipleConcurrentStreams (120.71s)
--- PASS: TestAccAWSKinesisStream_enforceConsumerDeletion (49.79s)
--- PASS: TestAccAWSKinesisStream_encryption (137.24s)
--- PASS: TestAccAWSKinesisStream_Tags (94.82s)
--- PASS: TestAccAWSKinesisStream_UpdateKmsKeyId (94.31s)

anGie44 · 2021-03-18T14:12:13Z

aws/resource_aws_kinesis_stream_consumer.go

+				Type:         schema.TypeString,
+				Required:     true,
+				ForceNew:     true,
+				ValidateFunc: validateArn,


we can add additional validation here with the method validateArn

ghost · 2021-03-18T23:52:47Z

This has been released in version 3.33.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 for triage. Thanks!

anthonyroach · 2021-03-19T14:29:33Z

Awesome! Thanks @anGie44

ghost · 2021-04-17T17:10:12Z

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!

anthonyroach requested a review from a team as a code owner January 16, 2021 22:55

github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Jan 16, 2021

anthonyroach mentioned this pull request Jan 16, 2021

Kinesis Stream Consumer #7611

Closed

anthonyroach mentioned this pull request Jan 16, 2021

Adds Kinesis Stream Consumer resource and data source (Continued) #10487

Closed

github-actions bot reviewed Jan 16, 2021

View reviewed changes

anthonyroach changed the title ~~add aws_kinesis_stream_consumer resource and data source~~ Adds Kinesis Stream Consumer resource and data source (third time) Jan 17, 2021

Base automatically changed from master to main January 23, 2021 01:00

anGie44 self-assigned this Mar 17, 2021

anGie44 removed the needs-triage Waiting for first response or review from a maintainer. label Mar 18, 2021

anGie44 force-pushed the f-kinesis_stream_consumer branch 2 times, most recently from 045c510 to f4d9746 Compare March 18, 2021 01:32

Anthony Roach and others added 3 commits March 18, 2021 02:43

lint fixes

0694153

add aws_kinesis_stream_consumer documentation

b39b230

anGie44 reviewed Mar 18, 2021

View reviewed changes

anGie44 force-pushed the f-kinesis_stream_consumer branch from f4d9746 to 897d5a8 Compare March 18, 2021 06:55

ghost added size/XXL Managed by automation to categorize the size of a PR. and removed size/XL Managed by automation to categorize the size of a PR. labels Mar 18, 2021

anGie44 force-pushed the f-kinesis_stream_consumer branch from 897d5a8 to 46a1b6f Compare March 18, 2021 07:01

ghost added size/XL Managed by automation to categorize the size of a PR. size/XXL Managed by automation to categorize the size of a PR. and removed size/XXL Managed by automation to categorize the size of a PR. size/XL Managed by automation to categorize the size of a PR. labels Mar 18, 2021

anGie44 added this to the v3.33.0 milestone Mar 18, 2021

CR updates; refactor to service package and update test coverage

48bf260

anGie44 force-pushed the f-kinesis_stream_consumer branch from 46a1b6f to 48bf260 Compare March 18, 2021 07:08

anGie44 approved these changes Mar 18, 2021

View reviewed changes

anGie44 reviewed Mar 18, 2021

View reviewed changes

anGie44 merged commit 436f62c into hashicorp:main Mar 18, 2021

ghost locked as resolved and limited conversation to collaborators Apr 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds Kinesis Stream Consumer resource and data source (third time) #17149

Adds Kinesis Stream Consumer resource and data source (third time) #17149

anthonyroach commented Jan 16, 2021 •

edited

Loading

anthonyroach commented Jan 16, 2021 •

edited

Loading

github-actions bot left a comment

anGie44 commented Mar 18, 2021

anthonyroach commented Mar 18, 2021

anGie44 commented Mar 18, 2021

anthonyroach commented Mar 18, 2021 •

edited

Loading

anthonyroach commented Mar 18, 2021

anGie44 Mar 18, 2021

anGie44 Mar 18, 2021

anGie44 Mar 18, 2021 •

edited

Loading

anGie44 Mar 18, 2021

anGie44 Mar 18, 2021

anGie44 Mar 18, 2021

anGie44 left a comment •

edited

Loading

anGie44 Mar 18, 2021

ghost commented Mar 18, 2021

anthonyroach commented Mar 19, 2021

ghost commented Apr 17, 2021

Adds Kinesis Stream Consumer resource and data source (third time) #17149

Adds Kinesis Stream Consumer resource and data source (third time) #17149

Conversation

anthonyroach commented Jan 16, 2021 • edited Loading

Community Note

anthonyroach commented Jan 16, 2021 • edited Loading

github-actions bot left a comment

Choose a reason for hiding this comment

anGie44 commented Mar 18, 2021

anthonyroach commented Mar 18, 2021

anGie44 commented Mar 18, 2021

anthonyroach commented Mar 18, 2021 • edited Loading

anthonyroach commented Mar 18, 2021

anGie44 Mar 18, 2021

Choose a reason for hiding this comment

anGie44 Mar 18, 2021

Choose a reason for hiding this comment

anGie44 Mar 18, 2021 • edited Loading

Choose a reason for hiding this comment

anGie44 Mar 18, 2021

Choose a reason for hiding this comment

anGie44 Mar 18, 2021

Choose a reason for hiding this comment

anGie44 Mar 18, 2021

Choose a reason for hiding this comment

anGie44 left a comment • edited Loading

Choose a reason for hiding this comment

anGie44 Mar 18, 2021

Choose a reason for hiding this comment

ghost commented Mar 18, 2021

anthonyroach commented Mar 19, 2021

ghost commented Apr 17, 2021

anthonyroach commented Jan 16, 2021 •

edited

Loading

anthonyroach commented Jan 16, 2021 •

edited

Loading

anthonyroach commented Mar 18, 2021 •

edited

Loading

anGie44 Mar 18, 2021 •

edited

Loading

anGie44 left a comment •

edited

Loading