0.4.2 - fix existing IAM support (#65)

* add instanceProfileName - 0.4.2 * fix test * Update instancegroup_types.go
keikoproj · Nov 12, 2019 · 26126d0 · 26126d0
1 parent 3c9d62f
commit 26126d0
Show file tree

Hide file tree

Showing 8 changed files with 109 additions and 44 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
+## [v0.4.2-alpha] - 2019-12-07
+
+### Fixed
+
+- Support scenarios where existing IAM role is different than existing instance profile (#64)
+
 ## [v0.4.1-alpha] - 2019-11-07
 
 ### Added

diff --git a/README.md b/README.md
@@ -3,7 +3,7 @@
 [![codecov](https://codecov.io/gh/keikoproj/instance-manager/branch/master/graph/badge.svg)](https://codecov.io/gh/keikoproj/instance-manager)
 [![Go Report Card](https://goreportcard.com/badge/github.com/keikoproj/instance-manager)](https://goreportcard.com/report/github.com/keikoproj/instance-manager)
 [![slack](https://img.shields.io/badge/slack-join%20the%20conversation-ff69b4.svg)][SlackUrl]
-![version](https://img.shields.io/badge/version-0.4.1-blue.svg?cacheSeconds=2592000)
+![version](https://img.shields.io/badge/version-0.4.2-blue.svg?cacheSeconds=2592000)
 > Create and manage instance groups with Kubernetes.
 
 instance-manager simplifies the creation of worker nodes from within a Kubernetes cluster, create `InstanceGroup` objects in your cluster and instance-manager will provision the actual machines and bootstrap them to the cluster.

diff --git a/api/v1alpha1/instancegroup_types.go b/api/v1alpha1/instancegroup_types.go
@@ -163,19 +163,20 @@ type EKSCFSpec struct {
 
 // EKSCFConfiguration defines the context of an AWS Instance Group using EKSCF
 type EKSCFConfiguration struct {
-	EksClusterName     string              `json:"clusterName,omitempty"`
-	KeyPairName        string              `json:"keyPairName"`
-	Image              string              `json:"image"`
-	InstanceType       string              `json:"instanceType"`
-	NodeSecurityGroups []string            `json:"securityGroups"`
-	VolSize            int32               `json:"volSize,omitempty"`
-	Subnets            []string            `json:"subnets,omitempty"`
-	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"`
+	EksClusterName              string              `json:"clusterName,omitempty"`
+	KeyPairName                 string              `json:"keyPairName"`
+	Image                       string              `json:"image"`
+	InstanceType                string              `json:"instanceType"`
+	NodeSecurityGroups          []string            `json:"securityGroups"`
+	VolSize                     int32               `json:"volSize,omitempty"`
+	Subnets                     []string            `json:"subnets,omitempty"`
+	BootstrapArguments          string              `json:"bootstrapArguments,omitempty"`
+	SpotPrice                   string              `json:"spotPrice,omitempty"`
+	Tags                        []map[string]string `json:"tags,omitempty"`
+	ExistingRoleName            string              `json:"roleName,omitempty"`
+	ExistingInstanceProfileName string              `json:"instanceProfileName,omitempty"`
+	ManagedPolicies             []string            `json:"managedPolicies,omitempty"`
+	MetricsCollection           []string            `json:"metricsCollection,omitempty"`
 }
 
 // InstanceGroupStatus defines the schema of resource Status
@@ -188,7 +189,7 @@ type InstanceGroupStatus struct {
 	ActiveScalingGroupName        string `json:"activeScalingGroupName,omitempty"`
 	NodesArn                      string `json:"nodesInstanceRoleArn,omitempty"`
 	StrategyResourceName          string `json:"strategyResourceName,omitempty"`
-	UsingSpotRecommendation       bool   `json:"usingSpotRecommendation"`
+	UsingSpotRecommendation       bool   `json:"usingSpotRecommendation,omitempty"`
 	Lifecycle                     string `json:"lifecycle,omitempty"`
 }
 
@@ -427,6 +428,15 @@ func (conf *EKSCFConfiguration) GetRoleName() string {
 func (conf *EKSCFConfiguration) SetRoleName(role string) {
 	conf.ExistingRoleName = role
 }
+
+func (conf *EKSCFConfiguration) GetInstanceProfileName() string {
+	return conf.ExistingInstanceProfileName
+}
+
+func (conf *EKSCFConfiguration) SetInstanceProfileName(profile string) {
+	conf.ExistingInstanceProfileName = profile
+}
+
 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
@@ -118,6 +118,10 @@ data:
         Description: Optional - an existing iam role name to use for bootstrapping
         Type: String
         Default: ""
+      ExistingInstanceProfileName:
+        Description: Optional - an existing iam instance profile name to use for bootstrapping
+        Type: String
+        Default: ""
     Metadata:
       AWS::CloudFormation::Interface:
         ParameterGroups:
@@ -225,7 +229,7 @@ data:
           {{if not .Spec.EKSCFSpec.EKSCFConfiguration.ExistingRoleName}}
           IamInstanceProfile: !Ref NodeInstanceProfile
           {{else}}
-          IamInstanceProfile: !Ref ExistingRoleName
+          IamInstanceProfile: !Ref ExistingInstanceProfileName
           {{end}}
           ImageId: !Ref NodeImageId
           InstanceType: !Ref NodeInstanceType

diff --git a/config/crd/bases/instancemgr.keikoproj.io_instancegroups.yaml b/config/crd/bases/instancemgr.keikoproj.io_instancegroups.yaml
@@ -431,6 +431,8 @@ spec:
                       type: string
                     image:
                       type: string
+                    instanceProfileName:
+                      type: string
                     instanceType:
                       type: string
                     keyPairName:
@@ -549,8 +551,6 @@ spec:
               type: string
             usingSpotRecommendation:
               type: boolean
-          required:
-          - usingSpotRecommendation
           type: object
       required:
       - metadata

diff --git a/controllers/provisioners/ekscloudformation/ekscloudformation.go b/controllers/provisioners/ekscloudformation/ekscloudformation.go
@@ -22,13 +22,14 @@ import (
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
+	"regexp"
+
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/cloudformation"
 	"github.com/keikoproj/instance-manager/api/v1alpha1"
 	"github.com/keikoproj/instance-manager/controllers/common"
 	awsprovider "github.com/keikoproj/instance-manager/controllers/providers/aws"
 	"github.com/sirupsen/logrus"
-	"regexp"
 )
 
 var (
@@ -463,6 +464,8 @@ func (ctx *EksCfInstanceGroupContext) processParameters() error {
 		specConfig.SetVolSize(32)
 	}
 
+	existingRole, existingProfile := getExistingIAM(specConfig.GetRoleName(), specConfig.GetInstanceProfileName())
+
 	rawParameters := map[string]string{
 		"KeyName":                     specConfig.GetKeyName(),
 		"NodeImageId":                 specConfig.GetImage(),
@@ -478,7 +481,8 @@ func (ctx *EksCfInstanceGroupContext) processParameters() error {
 		"VpcId":                       ctx.VpcID,
 		"ManagedPolicyARNs":           getManagedPolicyARNs(specConfig.ManagedPolicies),
 		"NodeAutoScalingGroupMetrics": getNodeAutoScalingGroupMetrics(specConfig.GetMetricsCollection()),
-		"ExistingRoleName":            getExistingRoleName(specConfig.GetRoleName()),
+		"ExistingRoleName":            existingRole,
+		"ExistingInstanceProfileName": existingProfile,
 	}
 
 	var parameters []*cloudformation.Parameter
@@ -494,22 +498,33 @@ func (ctx *EksCfInstanceGroupContext) processParameters() error {
 	return nil
 }
 
-func getExistingRoleName(role string) string {
+func getExistingIAM(role, profile string) (string, string) {
 	var (
-		rolePrefix       = ":role/"
-		existingRoleName string
+		rolePrefix                                    = ":role/"
+		instanceProfilePrefix                         = ":instance-profile/"
+		existingRoleName, existingInstanceProfileName string
 	)
 
 	if role == "" {
-		return role
+		return "", ""
 	} else if strings.Contains(role, rolePrefix) {
 		trim := strings.Split(role, rolePrefix)
 		existingRoleName = trim[1]
 	} else {
 		existingRoleName = role
 	}
 
-	return existingRoleName
+	if profile == "" {
+		// use role name if profile not provided
+		existingInstanceProfileName = existingRoleName
+	} else if strings.Contains(profile, instanceProfilePrefix) {
+		trim := strings.Split(profile, instanceProfilePrefix)
+		existingInstanceProfileName = trim[1]
+	} else {
+		existingInstanceProfileName = profile
+	}
+
+	return existingRoleName, existingInstanceProfileName
 }
 
 // getManagedPolicyARNs constructs managed policy arns

diff --git a/controllers/provisioners/ekscloudformation/ekscloudformation_test.go b/controllers/provisioners/ekscloudformation/ekscloudformation_test.go
@@ -1205,33 +1205,58 @@ defaultArns:
 	}
 }
 
-func TestGetExistingRoleName(t *testing.T) {
+func TestGetExistingIAM(t *testing.T) {
 	tt := []struct {
-		testCase string
-		input    string
-		expected string
+		testCase        string
+		inputRole       string
+		inputProfile    string
+		expectedRole    string
+		expectedProfile string
 	}{
 		{
-			testCase: "custom resource creation with no/empty role",
-			input:    "",
-			expected: "",
+			testCase:        "custom resource creation with no/empty role",
+			inputRole:       "",
+			inputProfile:    "",
+			expectedRole:    "",
+			expectedProfile: "",
+		},
+		{
+			testCase:        "custom resource creation with profile only",
+			inputRole:       "",
+			inputProfile:    "arn:aws:iam::0123456789123:instance-profile/some-profile",
+			expectedRole:    "",
+			expectedProfile: "",
 		},
 		{
-			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 with prefix",
+			inputRole:       "arn:aws:iam::0123456789123:role/some-role",
+			inputProfile:    "",
+			expectedRole:    "some-role",
+			expectedProfile: "some-role",
 		},
 		{
-			testCase: "custom resource creation with an existing role without prefix",
-			input:    "some-role",
-			expected: "some-role",
+			testCase:        "custom resource creation with an existing role with prefix",
+			inputRole:       "arn:aws:iam::0123456789123:role/some-role",
+			inputProfile:    "arn:aws:iam::0123456789123:instance-profile/some-profile",
+			expectedRole:    "some-role",
+			expectedProfile: "some-profile",
+		},
+		{
+			testCase:        "custom resource creation with an existing role without prefix",
+			inputRole:       "some-role",
+			inputProfile:    "some-profile",
+			expectedRole:    "some-role",
+			expectedProfile: "some-profile",
 		},
 	}
 
 	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)
+		respRole, respProfile := getExistingIAM(tc.inputRole, tc.inputProfile)
+		if !reflect.DeepEqual(respRole, tc.expectedRole) {
+			t.Fatalf("Test Case [%s] Failed Expected Role [%s] Got [%s]\n", tc.testCase, tc.expectedRole, respRole)
+		}
+		if !reflect.DeepEqual(respProfile, tc.expectedProfile) {
+			t.Fatalf("Test Case [%s] Failed Expected Profile [%s] Got [%s]\n", tc.testCase, tc.expectedProfile, respProfile)
 		}
 	}
 }

diff --git a/docs/04_instance-manager.yaml b/docs/04_instance-manager.yaml
@@ -435,6 +435,8 @@ spec:
                       type: string
                     image:
                       type: string
+                    instanceProfileName:
+                      type: string
                     instanceType:
                       type: string
                     keyPairName:
@@ -553,8 +555,6 @@ spec:
               type: string
             usingSpotRecommendation:
               type: boolean
-          required:
-          - usingSpotRecommendation
           type: object
       required:
       - metadata
@@ -788,6 +788,10 @@ data:
         Description: Optional - an existing iam role name to use for bootstrapping
         Type: String
         Default: ""
+      ExistingInstanceProfileName:
+        Description: Optional - an existing iam instance profile name to use for bootstrapping
+        Type: String
+        Default: ""
     Metadata:
       AWS::CloudFormation::Interface:
         ParameterGroups:
@@ -895,7 +899,7 @@ data:
           {{if not .Spec.EKSCFSpec.EKSCFConfiguration.ExistingRoleName}}
           IamInstanceProfile: !Ref NodeInstanceProfile
           {{else}}
-          IamInstanceProfile: !Ref ExistingRoleName
+          IamInstanceProfile: !Ref ExistingInstanceProfileName
           {{end}}
           ImageId: !Ref NodeImageId
           InstanceType: !Ref NodeInstanceType
@@ -940,6 +944,7 @@ data:
       LaunchConfigName:
         Description: The launch config name
         Value: !Ref NodeLaunchConfig
+
 ---
 apiVersion: extensions/v1beta1
 kind: Deployment