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

feat: add support of Kinesis EFO Consumers #1073

Merged
merged 2 commits into from
Oct 11, 2023

Conversation

jbleduigou
Copy link
Contributor

@jbleduigou jbleduigou commented Oct 10, 2023

Problem

So far EFO consumers were note shown in the inventory.
It could make sense to show them, because each EFO consumer incurs additional charges.

Solution

Use the ListStreamConsumers method provide by AWS to least the consumers for each Kinesis Data Stream.

Changes Made

  • Modified streams.go so that EFO consumers are retrieved.
  • Added unit tests with 100% coverage on the new code added.
  • Added a mock for Kinesis Client to be used in unit tests
  • Updated the IAM policy so that kinesis:ListStreamConsumers is allowed
  • Updated the list of supported resources in the documentation

How to Test

In order to test the changes you first need to create a Kinesis Data Stream.
Then you need to register an EFO consumer, for instance using the AWS cli:
aws kinesis register-stream-consumer --stream-arn arn:aws:kinesis:us-east-1:0123456789:stream/my-awesome-data-stream --consumer-name my-awesome-efo-consumer

Screenshots

image
image
image

Checklist

  • Code follows the contributing guidelines
  • Changes have been thoroughly tested
  • Documentation has been updated, if necessary
  • Any dependencies have been added to the project, if necessary

Reviewers

@mlabouardy
@jakepage91

Copy link
Collaborator

@AvineshTripathi AvineshTripathi left a comment

Choose a reason for hiding this comment

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

Can we also add cost calutations too here so that we would not need to revisit this again? cc @Azanul

@jbleduigou
Copy link
Contributor Author

Can we also add cost calutations too here so that we would not need to revisit this again? cc @Azanul

I was thinking of doing it in a separate PR.
Would that be ok?

@Azanul
Copy link
Collaborator

Azanul commented Oct 11, 2023

Can we also add cost calutations too here so that we would not need to revisit this again? cc @Azanul

I was thinking of doing it in a separate PR. Would that be ok?

That's fine but why do something in two steps if you can do in one, unless you've a stepcounter. It's fine either way.

@mlabouardy mlabouardy added this to the v3.1.2 milestone Oct 11, 2023
@mlabouardy mlabouardy added the aws label Oct 11, 2023
@mlabouardy mlabouardy merged commit 72bb651 into tailwarden:develop Oct 11, 2023
2 checks passed
@jbleduigou jbleduigou deleted the kinesis-efo-consumer branch October 23, 2023 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants