Implement a configurable pattern for the IAM Role Name

.github/DEVELOPER.md

@@ -8,15 +8,33 @@
 * kubebuilder
 
 ### Quickstart
-```
-$ git clone https://github.com/keikoproj/iam-manager
-$ cd iam-manager
-$ go mod
-$ go vendor
-```
 
+First, go and fork the Github repo to your own personal project. Once that's
+done, set up a local build environment off of the original Github repo. Then we
+add in your fork'ed repo as a new target for doing git pushes.
+
+    $ go clean -modcache
+    $ go get -v github.com/keikoproj/iam-manager
+    $ cd "$(go env GOPATH)/src/github.com/keikoproj/iam-manager"
+    $ make test
+    $ go mod vendor
+    $ git remote add myfork <your fork>
+
+### Install Kubebuilder
+
+Kubebuilder is a requirement for the testsuite.. you can install it quickly
+on your own or with our make target:
+
+    $ make kubebuilder
 
 ### Build project
-```
-$ make
-```
+
+    $ make
+
+### Running Tests
+
+There are several environment variables that must be set in order for the
+test suite to work. The [Makefile](/Makefile) sets these for you, so please
+use the `make test` target to run tests:
+
+    $ make test

.gitignore

@@ -15,7 +15,8 @@
 vendor/
 mocks/
 gomock*/
+kubebuilder*
 
 #IDE files
 .idea/
-bin/
+bin/

.travis.yml

@@ -19,9 +19,7 @@ before_install: skip
 
 install:
   # install kube builder so BDD test cases can work
-  - wget https://github.com/kubernetes-sigs/kubebuilder/releases/download/v2.2.0/kubebuilder_2.2.0_linux_amd64.tar.gz
-  - tar -zxvf  kubebuilder_2.2.0_linux_amd64.tar.gz
-  - sudo mv kubebuilder_2.2.0_linux_amd64 /usr/local/kubebuilder
+  - make kubebuilder
 
 jobs:
   include:

Makefile

@@ -1,30 +1,43 @@
-
 # Image URL to use all building/pushing image targets
-IMG ?= keikoproj/iam-manager:latest
+IMG         ?= keikoproj/iam-manager:latest
+
+# Tools required to run the full suite of tests properly
+OSNAME           ?= $(shell uname -s | tr A-Z a-z)
+KUBEBUILDER_VER  ?= 2.2.0
+KUBEBUILDER_ARCH ?= amd64
+
 # Produce CRDs that work back to Kubernetes 1.11 (no version conversion)
 CRD_OPTIONS ?= "crd:trivialVersions=true"
 
+KUBECONFIG                  ?= $(HOME)/.kube/config
+LOCAL                       ?= true
+ALLOWED_POLICY_ACTION       ?= s3:,sts:,ec2:Describe,acm:Describe,acm:List,acm:Get,route53:Get,route53:List,route53:Create,route53:Delete,route53:Change,kms:Decrypt,kms:Encrypt,kms:ReEncrypt,kms:GenerateDataKey,kms:DescribeKey,dynamodb:,secretsmanager:GetSecretValue,es:,sqs:SendMessage,sqs:ReceiveMessage,sqs:DeleteMessage,SNS:Publish,sqs:GetQueueAttributes,sqs:GetQueueUrl
+RESTRICTED_POLICY_RESOURCES ?= policy-resource
+RESTRICTED_S3_RESOURCES     ?= s3-resource
+AWS_ACCOUNT_ID              ?= 123456789012
+AWS_REGION                  ?= us-west-2
+MANAGED_POLICIES            ?= arn:aws:iam::123456789012:policy/SOMETHING
+MANAGED_PERMISSION_BOUNDARY_POLICY ?= arn:aws:iam::1123456789012:role/iam-manager-permission-boundary
+CLUSTER_NAME                ?= k8s_test_keiko
+CLUSTER_OIDC_ISSUER_URL     ?= https://google.com/OIDC
+DEFAULT_TRUST_POLICY        ?= '{"Version": "2012-10-17", "Statement": [{"Effect": "Allow","Principal": {"Federated": "arn:aws:iam::AWS_ACCOUNT_ID:oidc-provider/OIDC_PROVIDER"},"Action": "sts:AssumeRoleWithWebIdentity","Condition": {"StringEquals": {"OIDC_PROVIDER:sub": "system:serviceaccount:{{.NamespaceName}}:SERVICE_ACCOUNT_NAME"}}}, {"Effect": "Allow","Principal": {"AWS": ["arn:aws:iam::{{.AccountID}}:role/trust_role"]},"Action": "sts:AssumeRole"}]}'
+
 # Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set)
 ifeq (,$(shell go env GOBIN))
-GOBIN=$(shell go env GOPATH)/bin
+	GOBIN := $(shell go env GOPATH)/bin
 else
-GOBIN=$(shell go env GOBIN)
+	GOBIN := $(shell go env GOBIN)
 endif
 
 all: manager
 
-setup: ; $(info $(M) setting up env variables for test…) @ ## Setup env variables
-export LOCAL=true
-export ALLOWED_POLICY_ACTION=s3:,sts:,ec2:Describe,acm:Describe,acm:List,acm:Get,route53:Get,route53:List,route53:Create,route53:Delete,route53:Change,kms:Decrypt,kms:Encrypt,kms:ReEncrypt,kms:GenerateDataKey,kms:DescribeKey,dynamodb:,secretsmanager:GetSecretValue,es:,sqs:SendMessage,sqs:ReceiveMessage,sqs:DeleteMessage,SNS:Publish,sqs:GetQueueAttributes,sqs:GetQueueUrl
-export RESTRICTED_POLICY_RESOURCES=policy-resource
-export RESTRICTED_S3_RESOURCES=s3-resource
-export AWS_ACCOUNT_ID=123456789012
-export AWS_REGION=us-west-2
-export MANAGED_POLICIES=arn:aws:iam::123456789012:policy/SOMETHING
-export MANAGED_PERMISSION_BOUNDARY_POLICY=arn:aws:iam::1123456789012:role/iam-manager-permission-boundary
-export CLUSTER_NAME=k8s_test_keiko
-export CLUSTER_OIDC_ISSUER_URL=https://google.com/OIDC
-export DEFAULT_TRUST_POLICY={"Version": "2012-10-17", "Statement": [{"Effect": "Allow","Principal": {"Federated": "arn:aws:iam::AWS_ACCOUNT_ID:oidc-provider/OIDC_PROVIDER"},"Action": "sts:AssumeRoleWithWebIdentity","Condition": {"StringEquals": {"OIDC_PROVIDER:sub": "system:serviceaccount:{{.NamespaceName}}:SERVICE_ACCOUNT_NAME"}}}, {"Effect": "Allow","Principal": {"AWS": ["arn:aws:iam::{{.AccountID}}:role/trust_role"]},"Action": "sts:AssumeRole"}]}
+.PHONY: kubebuilder
+kubebuilder:
+	@echo "Downloading and installing Kubebuilder - this requires sudo privileges"
+	curl -fsSL -O "https://github.com/kubernetes-sigs/kubebuilder/releases/download/v$(KUBEBUILDER_VER)/kubebuilder_$(KUBEBUILDER_VER)_$(OSNAME)_$(KUBEBUILDER_ARCH).tar.gz"
+	rm -rf kubebuilder && mkdir -p kubebuilder
+	tar -zxvf kubebuilder_$(KUBEBUILDER_VER)_$(OSNAME)_$(KUBEBUILDER_ARCH).tar.gz --strip-components 1 -C kubebuilder
+	sudo cp -rf kubebuilder /usr/local
 
 mock:
 	go get -u github.com/golang/mock/mockgen
@@ -34,7 +47,19 @@ mock:
 	done
 
 # Run tests
-test: setup mock generate fmt manifests
+test: mock generate fmt manifests
+	KUBECONFIG=$(KUBECONFIG) \
+	LOCAL=$(LOCAL) \
+	ALLOWED_POLICY_ACTION=$(ALLOWED_POLICY_ACTION) \
+	RESTRICTED_POLICY_RESOURCES=$(RESTRICTED_POLICY_RESOURCES) \
+	RESTRICTED_S3_RESOURCES=$(RESTRICTED_S3_RESOURCES) \
+	AWS_ACCOUNT_ID=$(AWS_ACCOUNT_ID) \
+	AWS_REGION=$(AWS_REGION) \
+	MANAGED_POLICIES=$(MANAGED_POLICIES) \
+	MANAGED_PERMISSION_BOUNDARY_POLICY=$(MANAGED_PERMISSION_BOUNDARY_POLICY) \
+	CLUSTER_NAME=$(CLUSTER_NAME) \
+	CLUSTER_OIDC_ISSUER_URL="$(CLUSTER_OIDC_ISSUER_URL)" \
+	DEFAULT_TRUST_POLICY=$(DEFAULT_TRUST_POLICY) \
 	go test ./... -coverprofile cover.out
 
 # Build manager binary

controllers/iamrole_controller.go

@@ -73,20 +73,28 @@ func (r *IamroleReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	log := log.Logger(ctx, "controllers", "iamrole_controller", "Reconcile")
 	log.WithValues("iamrole", req.NamespacedName)
 	log.Info("Start of the request")
+
 	//Get the resource
 	var iamRole iammanagerv1alpha1.Iamrole
 
 	if err := r.Get(ctx, req.NamespacedName, &iamRole); err != nil {
 		return ctrl.Result{}, ignoreNotFound(err)
 	}
 
-	roleName := fmt.Sprintf("k8s-%s", iamRole.ObjectMeta.Name)
-
-	if config.Props.DeriveNameFromNamespace() && config.Props.MaxRolesAllowed() == 1 {
-		roleName = fmt.Sprintf("k8s-%s", iamRole.ObjectMeta.Namespace)
+	// TODO: Remove the need to do this here.
+	//
+	// We are generating the roleName here for the potential call to the
+	// delete function. The right way to do this is to record the created
+	// IAM role ARN in the IamRole object itself, so that we can then
+	// guarantee the delete even if the operator has changed their
+	// iam.role.pattern setting.
+	roleName, err := utils.GenerateRoleName(ctx, iamRole, *config.Props)
+	if err != nil {
+		r.Recorder.Event(&iamRole, v1.EventTypeWarning, string(iammanagerv1alpha1.Error), "Unable to construct iam role name to error "+err.Error())
+		return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
 	}
 
-	// Isit being deleted?
+	// Is it being deleted?
 	if iamRole.ObjectMeta.DeletionTimestamp.IsZero() {
 		//Good. This is not Delete use case
 		//Lets check if this is very first time use case
@@ -132,12 +140,15 @@ func (r *IamroleReconciler) HandleReconcile(ctx context.Context, req ctrl.Reques
 	log = log.WithValues("iam_role_cr", iamRole.Name)
 	log.Info("state of the custom resource ", "state", iamRole.Status.State)
 
-	roleName := fmt.Sprintf("k8s-%s", iamRole.ObjectMeta.Name)
+	roleName, err := utils.GenerateRoleName(ctx, *iamRole, *config.Props)
 
-	if config.Props.DeriveNameFromNamespace() && config.Props.MaxRolesAllowed() == 1 {
-		roleName = fmt.Sprintf("k8s-%s", iamRole.ObjectMeta.Namespace)
-	}
 	log.V(1).Info("roleName constructed successfully", "roleName", roleName)
+	if err != nil {
+		r.Recorder.Event(iamRole, v1.EventTypeWarning, string(iammanagerv1alpha1.Error), "Unable to construct iam role name to error "+err.Error())
+		// It is not clear to me that we want to requeue here - as this is a fairly permanent
+		// error. Is there a better pattern here?
+		return ctrl.Result{RequeueAfter: 30 * time.Second}, nil
+	}
 
 	input, status, err := r.ConstructCreateIAMRoleInput(ctx, iamRole, roleName)
 	if err != nil {

controllers/iamrole_controller_test.go

@@ -9,7 +9,6 @@ import (
 )
 
 var _ = Describe("IamroleController", func() {
-
 	Describe("When checking a StatusUpdatePredicate", func() {
 		instance := StatusUpdatePredicate{}
 

docs/Configmap_Properties.md

@@ -8,12 +8,48 @@ This document explains configmap variables.
 | aws.accountId                     | AWS account ID where IAM roles are created|        |Optional            |
 | iam.managed.policies              | User managed IAM policies     |                    |Optional            |
 | iam.managed.permission.boundary.policy| User managed permission boundary policy|k8s-iam-manager-cluster-permission-boundary       |Required            |
-| webhook.enabled                   |  Enable webhook?              | false              | Required           |
-| iam.role.max.limit.per.namespace      | Maximum number of roles per namespace |   1        | Required |
-| aws.region                        | AWS Region                    | us-west-2          | Required |
-| iam.default.trust.policy| Default trust policy role. This must follow v1alpha1.AssumeRolePolicyDocument syntax|           | Optional |
-| iam.role.derive.from.namespace    | Derive iam role name from namespace? if true it will be k8s-<namespace> | false | Optional|
+| webhook.enabled                   |  Enable webhook?              | `false             | Required           |
+| iam.role.max.limit.per.namespace  | Maximum number of roles per namespace |   1        | Required |
+| aws.region                        | AWS Region                    | `us-west-2`        | Required |
+| iam.default.trust.policy          | Default trust policy role. This must follow v1alpha1.AssumeRolePolicyDocument syntax|           | Optional |
+| [iam.role.pattern](#iamrolepattern) | See docs below...           | `k8s-{{ .ObjectMeta.Name }}` | Optional           |
 | controller.desired.frequency      | Controller frequency to check the state of the world (in seconds) | 300  | Optional |
 | k8s.cluster.name                  | Name of the cluster           |                    | Optional | 
 | k8s.cluster.oidc.issuer.url       | OIDC issuer of the cluster    |                    | Optional |
-| iam.irsa.enabled                  | Enable IRSA option?           | false              | Optional |
+| iam.irsa.enabled                  | Enable IRSA option?           | `false`            | Optional |
+
+
+## `iam.role.pattern`
+
+[template]: https://golang.org/pkg/text/template/
+[iamrole]: /api/v1alpha1/iamrole_types.go
+
+_Default_: `k8s-{{ .ObjectMeta.Name }}`
+
+All IAM roles created by the controller will use this [GoLang template][template]
+to generate the final IAM Role Name. The default setting works fine if you have
+a single cluster - but if you want to operate multiple clusters in the same AWS
+account you will need to make sure the controllers do not conflict.
+
+The [`Iamrole`][iamrole] object is passed into the Go templating engine, enabling
+you to use any object field found in that role. For example 
+`mycluster-{{ .ObjectMeta.Namespace }}-{{ .ObjectMeta.Name }}`.
+
+All IAM roles created and managed by the controller will use this prefix. This
+helps organize IAM roles within your AWS Account, and can be used to ensure
+uniqueness between different EKS Clusters within the same AWS account.
+
+**Critical Note: Read [this](#note-about-changing-iamroleprefix-and-iamroleseparator)
+before changing this setting**
+
+**Note: Changes to your IAM Policy may be required if you customize this**
+
+## Note about changing `iam.role.pattern`
+
+If you have existing `IAMRole` resources in your cluster, and you make a change to
+the `iam.role.pattern` setting - the controller will reconcile the situation by
+creating NEW IAM roles. It will _not_ however clean up the old roles - thus you
+will have left over unused IAM roles in your account.
+
+Get these settings right from the beginning, or be prepared to clean up the left
+over roles.

go.mod

@@ -5,17 +5,18 @@ go 1.12
 require (
 	github.com/aws/aws-sdk-go v1.25.38
 	github.com/go-logr/logr v0.1.0
-	github.com/golang/mock v1.4.3
+	github.com/golang/mock v1.4.4
 	github.com/onsi/ginkgo v1.11.0
 	github.com/onsi/gomega v1.8.1
 	github.com/pborman/uuid v1.2.0
 	github.com/pkg/errors v0.8.1
-	golang.org/x/mod v0.3.0 // indirect
-	golang.org/x/tools v0.0.0-20200609164405-eb789aa7ce50 // indirect
+	golang.org/x/mod v0.4.1 // indirect
+	golang.org/x/tools v0.0.0-20210119195117-a46736d9d962 // indirect
 	gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15
 	k8s.io/api v0.17.2
 	k8s.io/apimachinery v0.17.2
 	k8s.io/client-go v0.17.2
 	k8s.io/klog v1.0.0
+	rsc.io/quote/v3 v3.1.0 // indirect
 	sigs.k8s.io/controller-runtime v0.5.2
 )