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

enhancement: provider level tags #241

Closed
wants to merge 6 commits into from
Closed

Conversation

pvlbnkl
Copy link

@pvlbnkl pvlbnkl commented Jun 9, 2023

Add provider level tags to examples according to whitepaper here: https://docs.aws.amazon.com/whitepapers/latest/tagging-best-practices/tagging-best-practices.html

Also ran terraform fmt

Issue: #232

@pvlbnkl pvlbnkl requested a review from wallhided June 9, 2023 14:27
@pvlbnkl pvlbnkl self-assigned this Jun 9, 2023
@keywordlabeler keywordlabeler bot added docs enhancement New feature or request issue labels Jun 9, 2023
@wallhided
Copy link
Contributor

I'll test it out.

@wallhided
Copy link
Contributor

I've checked terraform code on branch.
A list of things to cover in that PR:

  1. Terraform code is not valid, the declaration of default_tags, the proper blocks declaration could be found here.
  2. The idea of having this block - is to stop adding same tags for each terraform resource. For instance, there are some duplicates of these tags (see provider.tf and "vpc" module in example/argocd). Please remove setting of same tags from terraform resources.
  3. Comments in code.

@@ -1,5 +1,11 @@
provider "aws" {
region = var.region
default_tags = {
Environment = "dev"
Copy link
Contributor

Choose a reason for hiding this comment

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

"Environment" and "Project" values could be retrieved from variables.

Copy link
Contributor

Choose a reason for hiding this comment

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

Still not fixed

@@ -1,5 +1,11 @@
provider "aws" {
region = var.region
default_tags = {
Copy link
Contributor

Choose a reason for hiding this comment

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

It is incorrect declaration of default_tags for my terraform version. Does it work for your terraform version?

@wallhided wallhided self-requested a review July 20, 2023 10:23
Copy link
Contributor

@wallhided wallhided left a comment

Choose a reason for hiding this comment

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

Good job, the main problem is fixed. One small change is required - some tags could be retrieved from terraform vars.

@pvlbnkl pvlbnkl closed this Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs enhancement New feature or request issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants