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

Refactor and add new services #10

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

m-radzikowski
Copy link

Hey @ilayanambi86
the trouble with CloudFormation tags not being propagated is real and I like the approach you took with this plugin!

I needed it to support more services. I refactored the code to enable it. The changes I made:

  1. Introduce separate lists of resources that accept tags as object, as list, and those that need a programmatic SDK call (not supported by the CloudFormation).
  2. Remove code to tag the API Gateway stage - I tested creating the REST API Gtw. and the stage was always tagged with the same tags as the API. Maybe I'm not seeing some edge case - in such a case, I will be happy to restore it.
  3. Refactor code in few places to use new array methods and lambda functions to simplify the code, remove unnecessary dependencies.

Please let me know what do you think.

@ilayanambi86
Copy link
Owner

Will check the PR and merge. Thanks.

@markandersontrocme
Copy link

@ilayanambi86 Any update on this PR?

@ilayanambi86
Copy link
Owner

@ilayanambi86 Any update on this PR?

Hi, didn't get time to check. Will try this weekend. Thanks for your support.

@RichiCoder1
Copy link

Would also love to see this! Added a very specific resource I'm looking into, namely CloudWatch Log Groups.

@ArsenyYankovsky
Copy link

Would be nice to see this merged and get tags on our SNS topics.

@ilayanambi86
Copy link
Owner

@m-radzikowski m-radzikowski can resolve the conflicts. I will create a beta version for this to validate the change.

@m-radzikowski
Copy link
Author

@ilayanambi86 updated.

I tested it, for some reason HTTP API Gateway tags were not added, despite them being defined in the CloudFormation template. Take a look if you have a minute or let me know, I will investigate it further.

@OJFord
Copy link

OJFord commented Jul 14, 2022

Not to suggest that additional requests should block merging at all, but I just wondered if there's a reason to omit the CloudFormation stack itself - is it not possible?

@OJFord
Copy link

OJFord commented Jul 14, 2022

Also, why have the hard-coded list at all? Couldn't we just try it for each resource - no check if it's a supported one - and if the error is 'no such operation TagResource' or whatever, just ignore it and continue?

@tveal
Copy link

tveal commented Oct 26, 2022

This project seems severely outdated, so had to roll a new tagging plugin with more features and resource type support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants