Embedding role identifier in secret name precludes use of paths

[seh]

At present, kube-aws-iam-controller allows pod authors to nominate an IAM role by way of appending that role's name to a sentinel prefix, forming a Kubernetes Secret name. kube-aws-iam-controller forms the role's complete ARN by concatenating either a designated or discovered base ARN with the suffix of the Secret name. Since Kubernetes Secret names can't contain forward slash characters, users can't designate IAM roles with paths that contain such slashes.

It's possible for a user to specify an IAM role ARN prefix—including a path—by using the --base-role-arn command-line flag. If all the roles nominated by pods happen to all live under that same path, the current concatenation scheme does allow forming role ARNs with paths. However, if pod authors will want to nominate IAM roles under different paths, they can't embed those paths in Secret names.

kiam doesn't have this problem because the annotation value it reads for nominating an IAM role ARN has no syntactic constraints. Secret names—and all Kubernetes object names—are too limited for this purpose.

Consider an IAM role with an ARN like arn:aws:iam::123456789012:role/division/department/team/ReadAssets. We can discover the base role ARN as arn:aws:iam::123456789012:role, and we can include the role name "ReadAssets" in a Secret name ("aws-iam-ReadAssets"), but concatenating the two parts gives us the wrong ARN: arn:aws:iam::123456789012:role/ReadAssets. Against this, we can't name our Secret "aws-iam-role/division/department/team/ReadAssets."

If we specified a --base-role-arn flag value of "arn:aws:iam::123456789012:role/division/department/team/" then we could name our Secret "aws-iam-ReadAssets" and get the right ARN for that role, but another pod author couldn't then nominate a role with an ARN under a different path like arn:aws:iam::123456789012:role/division/project/mint/CreateMoney.

The project documentation currently bears this promise or threat:

This way of specifying the role on pod specs are subject to change.

I read the rest of the paragraph as a concern for the pod authors' convenience, but there's also this gap in capability. I do like the current design, so perhaps we could augment it just enough by accepting an optional annotation on the pods to designate an ARN path. In the example above, the Secret name would be "aws-iam-ReadAssets," and the yet-to-be-named annotation value would be "division/department/team." IAM does have this odd identifier model, where the object names are global within the containing AWS account, yet the ARNs can include this path, and I don't think that you can omit a path from an ARN.

Roles also have unique IDs that a Secret name could accommodate syntactically. Unfortunately, the STS AssumeRoleInput type doesn't accept such a unique ID in lieu of a role ARN.

Have you considered this problem before? What do you think about the proposed optional annotation for specifying an ARN path?

[seh]

More thoughts concerning annotations: We could allow a pod author to specify the role assumption duration (which I think you're referring to as "TTL" in the code at present), though it's also possible that session length should instead be restricted (perhaps just clamping these other duration values) by the controller's operator.

Also, it's possible that two different pods could specify the same Secret name, but with a different ARN path. I'm pretty sure that no more than one of those could form the ARN of an actual role, because two roles can't share the same name, despite their paths.

It is possible to for two IAM roles in different accounts to have the same name. Thinking about that problem, perhaps the annotation for the path I proposed should be treated more like how kiam does it: check whether the ARN suffix looks like a whole ARN (starting with "arn:"), and if so, don't include the base ARN prefix when forming the role ARN.

The question is whether the controller's base ARN is a convenience to allow the pod authors to omit unnecessary detail when nominating their roles, or whether it's a deliberate confinement to only allow pod authors to nominate roles under that AWS account (and possibly under a given path).

[seh]

Having thought about this more overnight, I think we are trying to cram too much information into the Secret name, and we're going to find that this approach is convenient but too limiting in practice.

Bitnami's Sealed Secrets controller is a good precedent for another design we could consider, albeit more complicated: Introduce a CRD called something like IAMSessionSecret, with a schema that captures things like the IAM role ARN, a session duration, and maybe a session name. The controller will own namesake Secret objects for each of these IAMSessionSecret objects, just as it does today. The difference is that the Secret name suffixes no longer designate the IAM role ARN; these names can take any form allowed by Kubernetes, as the controller is only paying attention to the specifications in the IAMSessionSecret objects. The controller could also post status updates to these objects, indicating when they were last updated and when they're expected to expire.

This approach requires pod authors to create a separate object manually, but I don't think that's much more onerous than adding annotations to the pods that wish to consume the corresponding Secrets.

[mikkeloscar]

Thanks a lot for opening this issue! I'm sorry that I didn't get back to you before now. I basically had similar ideas as you already and I have now implemented a PoC for a custom resource like you suggest, which I also think is the best solution. It obviously doesn't support everything you mentioned right now, but I would be happy to know what you think about it so far: #13

[seh]

This is fantastic news! I've been itching to get started on an implementation after my current project wraps up, but that opportunity won't come for several more weeks, so I'm delighted to hear of your progress. I'll take a look at your patch in the next day or so.

I know I proposed many details and features above. If we have the right place to hang them, we can add them incrementally. Starting with the right bones is most important.

[mikkeloscar]

I know I proposed many details and features above. If we have the right place to hang them, we can add them incrementally. Starting with the right bones is most important.

I totally agree. I also have not dealt with #11 yet, but this we can always do and it can be split up etc.

[seh]

Now that #13 landed, I think we can close this. There was a suggestion about controlling session length, but that's fit for a separate issue if it proves to be important enough.

[mikkeloscar]

Closing. Reopen/create new issue if something is missing.