Repository: keikoproj/instance-manager (go)

Issue:
IAM Role pass-through
Currently, instance-manager creates the IAM role for the instance group as part of the cloudformation template, which means the controller pods need to get `iam:*` permissions.

Some users may find this inappropriate to allow arbitrary IAM role creation from the controller.
We should allow taking `.spec.eks-cf.configuration.role` which would be an IAM role that is pre-created which would be attached to the scaling group instead of a created one.

This should override other options such as `managedPolicies` etc.

Pull request description:
Support for existing IAM roles
fixes #56 

This implements additional configuration for the EKSCF API called `roleName` which will override the creation of a new IAM role within the cloudformation stack and instead use an existing role for the instance profile + bootstrapping.

This helps with the use case where a user does not wish to create IAM roles from within the cluster and can now provide an externally created IAM role.

In this case the IAM role provided must have the basic minimum roles in order to join the cluster, and `managedPolicies` field is not supported as the role is now essentially managed by the user and not instance-manager.

Added unit tests

Patch:
diff --git a/api/v1alpha1/instancegroup_types.go b/api/v1alpha1/instancegroup_types.go
index f9dbbeb..5fe547d 100644
--- a/api/v1alpha1/instancegroup_types.go
+++ b/api/v1alpha1/instancegroup_types.go
@@ -173,6 +173,7 @@ type EKSCFConfiguration struct {
 	BootstrapArguments string              `json:"bootstrapArguments,omitempty"`
 	SpotPrice          string              `json:"spotPrice,omitempty"`
 	Tags               []map[string]string `json:"tags,omitempty"`
+	ExistingRoleName   string              `json:"roleName,omitempty"`
 	ManagedPolicies    []string            `json:"managedPolicies,omitempty"`
 	MetricsCollection  []string            `json:"metricsCollection,omitempty"`
 }
@@ -419,6 +420,13 @@ func (conf *EKSCFConfiguration) SetBootstrapArgs(args string) {
 	conf.BootstrapArguments = args
 }
 
+func (conf *EKSCFConfiguration) GetRoleName() string {
+	return conf.ExistingRoleName
+}
+
+func (conf *EKSCFConfiguration) SetRoleName(role string) {
+	conf.ExistingRoleName = role
+}
 func (conf *EKSCFConfiguration) GetTags() []map[string]string {
 	return conf.Tags
 }
diff --git a/config/crd/bases/instance-manager-configmap.yaml b/config/crd/bases/instance-manager-configmap.yaml
index 10c15ff..e4f9453 100644
--- a/config/crd/bases/instance-manager-configmap.yaml
+++ b/config/crd/bases/instance-manager-configmap.yaml
@@ -114,6 +114,10 @@ data:
       NodeAutoScalingGroupMetrics:
         Description: The Metrics that an Amazon EC2 ASG sends to Amazon CloudWatch.
         Type: List<String>
+      ExistingRoleName:
+        Description: Optional - an existing iam role name to use for bootstrapping
+        Type: String
+        Default: ""
     Metadata:
       AWS::CloudFormation::Interface:
         ParameterGroups:
@@ -144,12 +148,8 @@ data:
               - VpcId
               - Subnets
     Resources:
-      NodeInstanceProfile:
-        Type: AWS::IAM::InstanceProfile
-        Properties:
-          Path: "/"
-          Roles:
-          - !Ref NodeInstanceRole
+
+      {{if not .Spec.EKSCFSpec.EKSCFConfiguration.ExistingRoleName}}
       NodeInstanceRole:
         Type: AWS::IAM::Role
         Properties:
@@ -164,6 +164,14 @@ data:
               - sts:AssumeRole
           Path: "/"
           ManagedPolicyArns: !Ref ManagedPolicyARNs
+      NodeInstanceProfile:
+        Type: AWS::IAM::InstanceProfile
+        Properties:
+          Path: "/"
+          Roles:
+          - !Ref NodeInstanceRole
+      {{end}}
+
       NodeGroup:
         Type: AWS::AutoScaling::AutoScalingGroup
         Properties:
@@ -214,7 +222,11 @@ data:
         Type: AWS::AutoScaling::LaunchConfiguration
         Properties:
           AssociatePublicIpAddress: 'false'
+          {{if not .Spec.EKSCFSpec.EKSCFConfiguration.ExistingRoleName}}
           IamInstanceProfile: !Ref NodeInstanceProfile
+          {{else}}
+          IamInstanceProfile: !Ref ExistingRoleName
+          {{end}}
           ImageId: !Ref NodeImageId
           InstanceType: !Ref NodeInstanceType
           KeyName: !Ref KeyName
@@ -242,7 +254,16 @@ data:
     Outputs:
       NodeInstanceRole:
         Description: The node instance role
+        {{if not .Spec.EKSCFSpec.EKSCFConfiguration.ExistingRoleName}}
         Value: !GetAtt NodeInstanceRole.Arn
+        {{else}}
+        Value: !Join
+          - ''
+          - - 'arn:aws:iam::'
+            - !Ref 'AWS::AccountId'
+            - ":role/"
+            - !Ref ExistingRoleName
+        {{end}}
       AsgName:
         Description: The ASG name
         Value: !Ref NodeGroup
diff --git a/config/crd/bases/instancemgr.keikoproj.io_instancegroups.yaml b/config/crd/bases/instancemgr.keikoproj.io_instancegroups.yaml
index f06b347..7aafd84 100644
--- a/config/crd/bases/instancemgr.keikoproj.io_instancegroups.yaml
+++ b/config/crd/bases/instancemgr.keikoproj.io_instancegroups.yaml
@@ -443,6 +443,8 @@ spec:
                       items:
                         type: string
                       type: array
+                    roleName:
+                      type: string
                     securityGroups:
                       items:
                         type: string
diff --git a/config/samples/instancemgr_v1alpha1_instancegroup_with_existing_iam.yaml b/config/samples/instancemgr_v1alpha1_instancegroup_with_existing_iam.yaml
new file mode 100644
index 0000000..df560e1
--- /dev/null
+++ b/config/samples/instancemgr_v1alpha1_instancegroup_with_existing_iam.yaml
@@ -0,0 +1,24 @@
+apiVersion: instancemgr.keikoproj.io/v1alpha1
+kind: InstanceGroup
+metadata:
+  labels:
+    controller-tools.k8s.io: "1.0"
+  name: hello-world
+spec:
+  provisioner: eks-cf
+  strategy:
+      type: rollingUpdate
+  eks-cf:
+    maxSize: 3
+    minSize: 1
+    configuration:
+      keyPairName: mykey
+      image: ami-0923e4b35a30a5f53
+      instanceType: m5.large
+      volSize: 20
+      roleName: basic-eks-role
+      securityGroups:
+      - sg-0de3bb02a40b915e6
+      tags:
+        - key: mykey
+          value: myvalue
\ No newline at end of file
diff --git a/controllers/provisioners/ekscloudformation/bootstrap.go b/controllers/provisioners/ekscloudformation/bootstrap.go
index 0e1cbdb..dd65712 100644
--- a/controllers/provisioners/ekscloudformation/bootstrap.go
+++ b/controllers/provisioners/ekscloudformation/bootstrap.go
@@ -78,6 +78,9 @@ func (ctx *EksCfInstanceGroupContext) updateAuthConfigMap() error {
 
 	// Upsert ARNs from discovered state
 	for _, instanceGroup := range instanceGroups.Items {
+		if instanceGroup.ARN == "" {
+			continue
+		}
 		err = authMap.Upsert(getNodeUpsert(instanceGroup.ARN))
 		if err != nil {
 			return err
@@ -85,13 +88,16 @@ func (ctx *EksCfInstanceGroupContext) updateAuthConfigMap() error {
 	}
 	// Upsert ARNs from controller config
 	for _, arn := range ctx.DefaultARNList {
+		if arn == "" {
+			continue
+		}
 		err = authMap.Upsert(getNodeUpsert(arn))
 		if err != nil {
 			return err
 		}
 	}
 	// Remove selfARN in case of deletion event
-	if ctx.GetState() == v1alpha1.ReconcileInitDelete {
+	if ctx.GetState() == v1alpha1.ReconcileInitDelete && selfARN != "" {
 		err = authMap.Remove(getNodeRemove(selfARN))
 		if err != nil {
 			return err
diff --git a/controllers/provisioners/ekscloudformation/ekscloudformation.go b/controllers/provisioners/ekscloudformation/ekscloudformation.go
index 85ca126..106a721 100644
--- a/controllers/provisioners/ekscloudformation/ekscloudformation.go
+++ b/controllers/provisioners/ekscloudformation/ekscloudformation.go
@@ -155,6 +155,7 @@ func (ctx *EksCfInstanceGroupContext) Delete() error {
 		log.Errorf("failed to submit DeleteStack: %v", err)
 		return err
 	}
+
 	instanceGroup.SetState(v1alpha1.ReconcileDeleting)
 	ctx.reloadDiscoveryCache()
 	return nil
@@ -477,6 +478,7 @@ func (ctx *EksCfInstanceGroupContext) processParameters() error {
 		"VpcId":                       ctx.VpcID,
 		"ManagedPolicyARNs":           getManagedPolicyARNs(specConfig.ManagedPolicies),
 		"NodeAutoScalingGroupMetrics": getNodeAutoScalingGroupMetrics(specConfig.GetMetricsCollection()),
+		"ExistingRoleName":            getExistingRoleName(specConfig.GetRoleName()),
 	}
 
 	var parameters []*cloudformation.Parameter
@@ -492,6 +494,24 @@ func (ctx *EksCfInstanceGroupContext) processParameters() error {
 	return nil
 }
 
+func getExistingRoleName(role string) string {
+	var (
+		rolePrefix       = ":role/"
+		existingRoleName string
+	)
+
+	if role == "" {
+		return role
+	} else if strings.Contains(role, rolePrefix) {
+		trim := strings.Split(role, rolePrefix)
+		existingRoleName = trim[1]
+	} else {
+		existingRoleName = role
+	}
+
+	return existingRoleName
+}
+
 // getManagedPolicyARNs constructs managed policy arns
 func getManagedPolicyARNs(pNames []string) string {
 	// This is for Managed Policy ARN list.
diff --git a/docs/04_instance-manager.yaml b/docs/04_instance-manager.yaml
index 703ee71..e6116e6 100644
--- a/docs/04_instance-manager.yaml
+++ b/docs/04_instance-manager.yaml
@@ -447,6 +447,8 @@ spec:
                       items:
                         type: string
                       type: array
+                    roleName:
+                      type: string
                     securityGroups:
                       items:
                         type: string
@@ -782,6 +784,10 @@ data:
       NodeAutoScalingGroupMetrics:
         Description: The Metrics that an Amazon EC2 ASG sends to Amazon CloudWatch.
         Type: List<String>
+      ExistingRoleName:
+        Description: Optional - an existing iam role name to use for bootstrapping
+        Type: String
+        Default: ""
     Metadata:
       AWS::CloudFormation::Interface:
         ParameterGroups:
@@ -812,12 +818,8 @@ data:
               - VpcId
               - Subnets
     Resources:
-      NodeInstanceProfile:
-        Type: AWS::IAM::InstanceProfile
-        Properties:
-          Path: "/"
-          Roles:
-          - !Ref NodeInstanceRole
+
+      {{if not .Spec.EKSCFSpec.EKSCFConfiguration.ExistingRoleName}}
       NodeInstanceRole:
         Type: AWS::IAM::Role
         Properties:
@@ -832,6 +834,14 @@ data:
               - sts:AssumeRole
           Path: "/"
           ManagedPolicyArns: !Ref ManagedPolicyARNs
+      NodeInstanceProfile:
+        Type: AWS::IAM::InstanceProfile
+        Properties:
+          Path: "/"
+          Roles:
+          - !Ref NodeInstanceRole
+      {{end}}
+
       NodeGroup:
         Type: AWS::AutoScaling::AutoScalingGroup
         Properties:
@@ -858,7 +868,7 @@ data:
           - Granularity: "1Minute"
           {{else}}
           - Metrics:
-            !Ref NodeAutoScalingGroupMetrics
+              !Ref NodeAutoScalingGroupMetrics
             Granularity: "1Minute"
           {{end}}
           {{end}}
@@ -882,7 +892,11 @@ data:
         Type: AWS::AutoScaling::LaunchConfiguration
         Properties:
           AssociatePublicIpAddress: 'false'
+          {{if not .Spec.EKSCFSpec.EKSCFConfiguration.ExistingRoleName}}
           IamInstanceProfile: !Ref NodeInstanceProfile
+          {{else}}
+          IamInstanceProfile: !Ref ExistingRoleName
+          {{end}}
           ImageId: !Ref NodeImageId
           InstanceType: !Ref NodeInstanceType
           KeyName: !Ref KeyName
@@ -910,7 +924,16 @@ data:
     Outputs:
       NodeInstanceRole:
         Description: The node instance role
+        {{if not .Spec.EKSCFSpec.EKSCFConfiguration.ExistingRoleName}}
         Value: !GetAtt NodeInstanceRole.Arn
+        {{else}}
+        Value: !Join
+          - ''
+          - - 'arn:aws:iam::'
+            - !Ref 'AWS::AccountId'
+            - ":role/"
+            - !Ref ExistingRoleName
+        {{end}}
       AsgName:
         Description: The ASG name
         Value: !Ref NodeGroup
@@ -941,7 +964,7 @@ spec:
         app: instance-manager
     spec:
       containers:
-      - image: keikoproj/instance-manager:latest
+      - image: keikoproj/instance-manager:master
         imagePullPolicy: Always
         name: instance-manager
         resources:


Test patch:
diff --git a/controllers/provisioners/ekscloudformation/ekscloudformation_test.go b/controllers/provisioners/ekscloudformation/ekscloudformation_test.go
index e3a0a3b..3fedcd8 100644
--- a/controllers/provisioners/ekscloudformation/ekscloudformation_test.go
+++ b/controllers/provisioners/ekscloudformation/ekscloudformation_test.go
@@ -1205,6 +1205,36 @@ defaultArns:
 	}
 }
 
+func TestGetExistingRoleName(t *testing.T) {
+	tt := []struct {
+		testCase string
+		input    string
+		expected string
+	}{
+		{
+			testCase: "custom resource creation with no/empty role",
+			input:    "",
+			expected: "",
+		},
+		{
+			testCase: "custom resource creation with an existing role with prefix",
+			input:    "arn:aws:iam::0123456789123:role/some-role",
+			expected: "some-role",
+		},
+		{
+			testCase: "custom resource creation with an existing role without prefix",
+			input:    "some-role",
+			expected: "some-role",
+		},
+	}
+
+	for _, tc := range tt {
+		resp := getExistingRoleName(tc.input)
+		if !reflect.DeepEqual(resp, tc.expected) {
+			t.Fatalf("Test Case [%s] Failed Expected [%s] Got [%s]\n", tc.testCase, tc.expected, resp)
+		}
+	}
+}
 func TestGetManagedPolicyARNs(t *testing.T) {
 	tt := []struct {
 		testCase string