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

fix: Remove opinionated tag #381

Conversation

robertalexa
Copy link

Description

This PR is removing the opinionated tag residing in locals.tags tags = merge(var.tags, { terraform-aws-modules = "alb" })

Motivation and Context

This goes against expected and predictable behaviour, and can also not be removed or overwritten. Thus is should not exist at all, and give the users the option to add it if needed.

Looking at other terraform-aws-modules, I haven't seen any others doing this?

Breaking Changes

Should not cause any breaking changes. It should also not trigger resource recreation as this is impacting tags only.

May present an inconvenience to people that had this tag passively added to their resources, and made use of it in other areas of AWS (reporting, costing etc)

@robertalexa robertalexa changed the title Remove opinionated tag fix: Remove opinionated tag Aug 29, 2024
@bryantbiggs
Copy link
Member

thank you but we are not removing that at this time

@robertalexa
Copy link
Author

Hi @bryantbiggs

Thanks for your prompt reply. Could you please provide some justification of why it is included though?

@bryantbiggs
Copy link
Member

Terraform modules do not have anything similar to a user-agent header - the user-agent header that is used is the Terraform user-agent, which means there is no distinction between which resources are created by this module versus another module versus a custom module or bare resources. You can think of the tag as a psuedo user-agent string

@bryantbiggs
Copy link
Member

and yes, we have started rolling it out to other modules - we try to minimize its impact so its not added to every resource where tags are provided, only the core resource for the module (usually)

@robertalexa
Copy link
Author

Thanks for the clarification. And i 100% stand by your logic there.

In reality, the reason this cropped up on my plate is because that is exactly what we are doing, but doing it ourselves. Because the modules have inputs for both parent resources and "children" (dependencies).

And once you start doing it yourself, precisely to keep track of the information you have presented. This tag is irrelevant. Or more importantly, doesn't match the tagging structure enforced e.g. PascalCase.

It is from this angle that I believe it should not be forcefully added by the module (and you have now clarified that this is now being rolled out, rather than this module being an "exception"), and instead the responsibility should be left with the user.

Alternatively there could be an input, even if defaulting to True, to state "include_default_module_tags". This would align with your intentions, but would also provide more flexibility to other users who would rather not have them.

Any thoughts?

@bryantbiggs
Copy link
Member

Think of it as a very, very small price to pay for our body of work which is completely free from a monetary value.

If we make it configurable where users can remove, then what is the point - we're back to having spent time and effort on something that we cannot show the value it provides if we wish to seek funding to continue our efforts and provide for our families. Ideally this would be transparent to users via some metadata sent with the API requests, but we are working with the facilities we have and trying to do so in the most minimal way possible

@robertalexa
Copy link
Author

Right. I am happy "to pay the price" if you either add it to all the resources created by the module, so that indeed you have a FULL picture of ALL the resources created by the module, or none at all.

An inbetween means that we can't reliable use that tag to track the resources, since as you said, they dont all have it. And if we create our own set of tracking tags, then your one becomes obsolete. And this is the situation I am at now.

I get your point, and I could fully stand by it, but only if you were to do it across all resources. As for the funding part of your argument, the majority of corpos have internal policies against political affiliations. So removing the war related parts of the readme, and more importantly the code flags would give you a better chance of the module being onboarded and funded. But i guess that is just my unpopular opinion

Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants