Add AWSIAMRole CRD for flexible role assignment

[mikkeloscar]

This implements support for a simple AWSIAMRole CRD which makes it possible to reference a role name or full arn and get a corresponding secret with IAM role credentials generated, which can be mounted into a pod.

It works like this:

The controller will look for resources of type AWSIAMRole which looks like this:

apiVersion: amazonaws.com/v1
kind: AWSIAMRole
metadata:
  name: my-app-iam-role
spec:
  roleReference: <my-iam-role-name-or-arn>

And it will create a matching secret my-app-iam-role with credentials for the referenced role.

This should solve most of the issues raised in #12

Additionally to being able to referencing a role in the AWSIAMRole resources, I also think about making it possible to specify a full role definition and have the controller provision the role in AWS. This would make it possible to provision AWS IAM roles completely through Kubernetes and would make it possible to hide the complexity of specifying assumeRole relationships between roles from the user. I.e. users would no longer need to define that the kube-aws-iam-controller role should be able to assume the user role as this could be injected transparently.

The reason I think we need to support both a simple roleReference and role provisioning is that it makes it more flexible and potentially easier to migrate from other solutions to kube-aws-iam-controller.

It also allows providing nicer feedback for users, like a status section of the CRD showing the full role arn and expiration date:

$ kubectl get awsiamroles.amazonaws.com                            
NAME              ROLEARN                                          EXPIRATION
my-app-iam-role   arn:aws:iam::012345678910:role/my-app-iam-role   2019-01-29T21:58:57Z

/cc @seh

[coveralls]

Pull Request Test Coverage Report for Build 85

• 257 of 368 (69.84%) changed or added relevant lines in 5 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage increased (+6.6%) to 67.627%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pod_watcher.go	2	3	66.67%
secrets_controller.go	6	7	85.71%
main.go	0	20	0.0%
awsiamrole_controller.go	236	325	72.62%

Files with Coverage Reduction	New Missed Lines	%
main.go	1	0.0%

Totals
Change from base Build 53:	6.6%
Covered Lines:	493
Relevant Lines:	729

---

💛 - Coveralls

[seh]

A few things to consider, that I couldn't comment on at the line level since this patch didn't touch the pertinent areas of the code:

• Update Secrets more carefully
  When updating a Secret consider constraining the update on matching the resource version, and if it fails due to a version mismatch, reread the Secret and try again (no more than some fixed number of times).
• Do we watch pods only to post events to them?
  If so, consider removing that capability, so that your controller doesn't need a pod cache (a scaling limitation that presents a headache to cluster operators).
• Have you considered integrating kubebuilder?
  Though you've already paid the cost of arranging for code generation, integrating kubebuilder could remove some of the boilerplate code here.

Have you been running this controller since posting this patch? How is it working so far?

[seh]

s/tole/role/
s/defaults/Defaults/

[seh]

What happens if this value is greater than "MaxSessionDuration?" Does it get clamped to that value, or does something fail?

[mikkeloscar]

I assume the API call to sts assumeRole would throw an error which would propagate as an event on the AWSIAMRole resource

[seh]

What's the format of this value? Presumably it's a time instant formatted as a string.

[mikkeloscar]

Yes, RFC3339 formatted string in UTC (I'm not sure if kubectl can somehow show the local time given a timestamp string?)

[seh]

Is this CRD defining a new IAM role, or is this a snapshot of an existing IAM role as read from AWS?

[mikkeloscar]

Yes, the idea, which is not yet implemented and won't be in this PR, is to allow users to define an IAM role via the CRD as an alternative to referencing it. This would then take care of provisioning the IAM role with the correct assume role configuration etc.

I will remove this from this PR, was just easier to design the whole thing with this idea in mind.

[seh]

Can we really claim this group name? I think it's best left for software actually published by the trademark holder. Perhaps something involving "kube," "iam," and "aws" would be safer.

[mikkeloscar]

Good point, I also thought about this but went with it anyway since kube2iam does the same and no complaints from AWS :) But you are right, let's use a less trademark infringing value.

[szuecs]

mikkel.labs/aws-iam?

[seh]

Why does the controller watch pods? Is this to help clean up orphaned (no longer referenced) Secrets? Is it to decide whether to keep renewing credentials for Secrets that are no longer referenced?

[mikkeloscar]

The old support for generating secrets based on the aws-iam- prefixed secret mount is still supported here. I would remove it in a later PR to not break existing users without a possibility of an upgrade path.

[seh]

Again, I suggest reconsidering use of this name for which we're not the trademark holder.

[mikkeloscar]

Thanks a lot for the review, let me answer your questions inline.

A few things to consider, that I couldn't comment on at the line level since this patch didn't touch the pertinent areas of the code:

• Update Secrets more carefully
  When updating a Secret consider constraining the update on matching the resource version, and if it fails due to a version mismatch, reread the Secret and try again (no more than some fixed number of times).

This should already be handled by client-go. If we get any error on update then it will be retried in the next iteration, IMO no need to add retries directly to this call unless it becomes a problem.

• Do we watch pods only to post events to them?
  If so, consider removing that capability, so that your controller doesn't need a pod cache (a scaling limitation that presents a headache to cluster operators).

It's to support the old aws-iam- secret prefix behavior for a little longer.

• Have you considered integrating kubebuilder?
  Though you've already paid the cost of arranging for code generation, integrating kubebuilder could remove some of the boilerplate code here.

I have not really considered it. The code generation as-is is fairly simple IMO and I would avoid adding another build dependency unless it really provides a benefit.

Have you been running this controller since posting this patch? How is it working so far?

Not really, have only been running it for testing but nothing real so far. This is mostly due to other priorities right now, but I didn't find any issues in my tests, just didn't get around to merging yet :)