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

Why no name set for IAM policies / roles? #92

Open
jeroenhabets opened this issue May 19, 2023 · 6 comments
Open

Why no name set for IAM policies / roles? #92

jeroenhabets opened this issue May 19, 2023 · 6 comments

Comments

@jeroenhabets
Copy link
Contributor

Not sure if this is an issue/improvement or just a question...

After successfully and happy using your module for 1+ year, I noticed many IAM policies / roles created with names like terraform-20220315113738630100000001.

This because in iam.tf several resources do not have the name property set:

resource "aws_iam_policy" "cluster_instance_policy" {
  description = "cluster-instance-policy-${var.component}-${var.deployment_identifier}-${var.cluster_name}"
  policy      = local.cluster_instance_policy_contents
}

Is this by design? Or should I change this issue into a "Feature Request" (or "Bugfix") and provide a PR with the name set like the description?

@jeroenhabets
Copy link
Contributor Author

@jonassvalin is this by design? If not, it's trivial to fix (I can contribute a PR if desired).

@jeroenhabets
Copy link
Contributor Author

Hi @infrablocks-maintainers, @tobyclemson, @Gryff, @jonassvalin, an update on the issues/questions and PRs (#95, #94, #93) would be much appreciated!
Sorry to bother you this way but I haven't seen any activity since Feb so you're getting me worried here 😅 .

@Gryff
Copy link
Contributor

Gryff commented Jun 22, 2023

Hi @jeroenhabets, apologies we are really busy at the moment, we're going to get to these next week.

@jonassvalin
Copy link
Contributor

@jeroenhabets Yes this is by design. The reason is that we want to be able to deploy multiple instances of the module into the same account. This causes issues if you hardcode a name, and the other option would be to template a name based on certain passed properties, but this is difficult to accomplish in a consistent way without breaking the AWS maximum character length requirements. We're open to suggestions for how to handle it differently to give you more visibility, but yes, it's by design.

@jeroenhabets
Copy link
Contributor Author

Hi @jonassvalin, thanks but the module is naming other objects already, e.g.:

capacity_provider.tf: name = "cp-${var.component}-${var.deployment_identifier}-${var.cluster_name}"
and even in iam.tf e.g:
name = "cluster-instance-profile-${var.component}-${var.deployment_identifier}-${var.cluster_name}"

and using the ${var.deployment_identifier} and ${var.cluster_name} there for identification (and ensure uniqueness).

So I don't see why we couldn't and shouldn't do the same for resource "aws_iam_policy" "cluster_instance_policy", resource "aws_iam_role" "cluster_service_role" and "aws_iam_policy" "cluster_service_policy" (using what's now in their description or if you're worried about the length, abbreviate the prefixes, e.g. "cip- like "cp-).

Or am I making a critical thinking error here?

Sorry, to be adamant but for Compliance / Auditability / Policy enforcement, naming in AWS is rather important to us.

@jeroenhabets
Copy link
Contributor Author

@jonassvalin, @Gryff would you be open to a PR here? P.S. there are 3 PRs pending.

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

No branches or pull requests

3 participants