kube2iam
kube2iam copied to clipboard
jtblin
don't add trailing slash to provided --base-role-arn
The current implementation always adds a trailing slash to the baseRoleArn if the last character ist not already a slash. This behavior breaks my use-case.
Here's an example of my use case: full arn:
- A) arn:aws:iam::1234567890:role/helloworld-foo
- B) arn:aws:iam::1234567890:role/helloworld-bar
base-role-arn: arn:aws:iam::1234567890:role/helloworld-
pod annotation for iam A: iam.amazonaws.com/role: foo
pod annotation for iam B: iam.amazonaws.com/role: foo
Everything else in the repository would already allow my use-case to work, but the fact that the slash is currently always enforced breaks it completely.
As a sidenote the current regex for validating is ^arn:(\w|-)*:iam::\d+:role\/?(\w+|-|\/|\.)*$
But according to the AWS IAM reference slashes are not allowed as a part of the role-name
Names of users, groups, roles, policies, instance profiles, and server certificates must be alphanumeric, including the following common characters: plus (+), equal (=), comma (,), period (.), at (@), underscore (_), and hyphen (-).
Therefore I think the correct regex should also be: ^arn:(\w|-)*:iam::\d+:role\/?(\w+|\+|@|-|\.|\,|\=|\_)*$
Although I did not touch that as I have not verified if the AWS documentation is correct in that regard.
Coverage remained the same at 19.786% when pulling 5ac8264937d55a4fe2adb03936cb42d46c470f48 on Bobonium:allow_base_role_arn_prefix into 80efbb12e3650ab35de87230a0a300ffdc5cbe84 on jtblin:master.
I was actually planning to make this change too! Glad you have made it!
My use case is similar as well.
Use case:
base-role-arn: arn:aws:iam::1234567890:role/acme-prod-us-east-2-k8s-
pod annotation for some role: iam.amazonaws.com/role: user-reader
This is required so that our configs do not need to know the full prefix (acme-prod-us-east-2-k8s-) which is a combination of namespace, environment, and region. Currently I have some magic taking care of this, but would love to remove that and simplify our configs for our multi region deployments.
We may however want to make the this configurable, so we don't break existing usage for everyone that may have forgot to put a trailing slash on the arn.
This project is still in it's 0.X.X so there should be no need to enforce backwards compatibility. Especially since the adding of the slash is so far in no way implied based on the documentation. Additionally even the regex is built in a way that it will fail the initial check without it. Therefore I would not add another Parameter, that no one really needs because that would just add up in the future.
@mwhittington21 @jtblin
Is it possible to integrate the change upstream? Any further requirements/tasks from your side in order to include it in one of the next releases?
