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

add option to pass tags as parameter to apply to aws resources #10

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

Conversation

vamsinm
Copy link
Contributor

@vamsinm vamsinm commented Apr 26, 2023

when using terraform module the resources created will not have any resource tags. in most cases organizations need certain mandatory tagging for billing or categorizng resources to particular application or team. this PR will in clude tags as parameter so they can be passed to module.

#8

@vamsinm
Copy link
Contributor Author

vamsinm commented Apr 27, 2023

@bardielle @gdbranco would you mind reviewing this PR? thank you!

README.md Outdated Show resolved Hide resolved
variables.tf Outdated Show resolved Hide resolved
@vamsinm
Copy link
Contributor Author

vamsinm commented Apr 27, 2023

@bardielle thank you for the review. updated as suggested.

@bardielle
Copy link
Member

@gdbranco please review it also

Copy link
Member

@bardielle bardielle left a comment

Choose a reason for hiding this comment

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

/lgtm

@romfreiman
Copy link

Lets have jira ticket prefixing the title

@vamsinm
Copy link
Contributor Author

vamsinm commented May 1, 2023

@bardielle @romfreiman just checking to see if there is anything needed on this to approve merge?

@bardielle
Copy link
Member

bardielle commented May 1, 2023

@arendej asked to discuss this PR on our next sync meeting.

"Currently, ROSA only accepts tags at install time. We await a feature from OCP in order to have the cluster reconcile tags as day2 parameters.
We should discuss this one.
It makes sense to allow tagging for resources part of the AWS TF provider resources, but not quite the same for ROSA/OCM resources, where tagging is reconciled with OCM." 

I will update you @vamsinm with the conclusions

@vamsinm
Copy link
Contributor Author

vamsinm commented May 1, 2023

thank you @bardielle ! this merge request does not refer any OCM resources. this terrafrom module is only generating AWS resources. is there a plan to include OCM resources in here?

@openshift-merge-robot
Copy link
Contributor

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

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.

4 participants