-
Notifications
You must be signed in to change notification settings - Fork 666
fix: ensure IAM Role name length does not exceed 64 characters #4696
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
base: main
Are you sure you want to change the base?
fix: ensure IAM Role name length does not exceed 64 characters #4696
Conversation
Worth noting that there is one caveat to this approach: if the |
When using a long enough `prefix`, the IAM Roles can exceed the maximum length allowed by AWS. For example: ``` │ Error: expected length of name to be in the range (1 - 64), got github-runners-prod-xxxxxxxxxxxxxx-prod-action-scale-down-lambda-role │ │ with module.multi_runner.module.runners["xxxxxxxxxxxxxx-prod"].aws_iam_role.scale_down, │ on .terraform/modules/multi_runner/modules/runners/scale-down.tf line 88, in resource "aws_iam_role" "scale_down": │ 88: name = "${var.prefix}-action-scale-down-lambda-role" ``` There is nowhere to override this, so your only options are to change the prefix for the entire module. This commit resolves this by truncating the name to fit under the maximum length. This primarily happens on the scale-up and scale-down Lambdas, but I've added it everywhere for consistency. Fixes: github-aws-runners#3973
f7768eb
to
2bdcd44
Compare
Thx for you contribution, need a bit of time to check what would be the best direction. There are many resources that have the prefix as part of the name and not only the iam roleas are bounded by 64 chars. Have a both the prefix and a name of the role is important for me, it helps to quicly understand what the role is all about. Technically we could have a prefix and a generated part. But the will make the roles less understanable in my point of view. Some for other resources. We can also shorten the role names, wich gives also more space for the prefix, which can be in the case of the multi runner long. Maybe for future improvment (breaking) is to limit to prefix lenght as well to avoid problems like this. |
I am currently deploying this branch and it was only the scale-up and scale-down roles that were failing with a name of
I also thought about something like this, maybe a resource precondition on the IAM Role resource that asserts that the IAM Role names aren't going to completely conflict, or something along those lines. Also to consider - is it a breaking change if it's not currently possible? |
For this PR let's stick to this improvement. However also looked some time back to pre/post condtions. I think it is not truly breaking but it drops supports for older terraform version. I prefer to keep breaking changes limited. But I see not real reason to keep supporting old terraform version ver ever. So if you are happ to prepare a change in a sperate PR to see how pre / post can help we can introdcue this in a new major release. |
When using a long enough
prefix
, the IAM Roles can exceed the maximum length allowed by AWS.For example:
There is nowhere to override this, so your only options are to change the prefix for the entire module. This commit resolves this by truncating the name to fit under the maximum length.
This primarily happens on the scale-up and scale-down Lambdas, but I've added it everywhere for consistency.
Fixes: #3973