From 26126d0f5a9803e25c818665fb7ababef62a1ed4 Mon Sep 17 00:00:00 2001 From: Eytan Avisror Date: Tue, 12 Nov 2019 14:40:16 -0800 Subject: [PATCH] 0.4.2 - fix existing IAM support (#65) * add instanceProfileName - 0.4.2 * fix test * Update instancegroup_types.go --- CHANGELOG.md | 6 ++ README.md | 2 +- api/v1alpha1/instancegroup_types.go | 38 ++++++++----- .../crd/bases/instance-manager-configmap.yaml | 6 +- ...stancemgr.keikoproj.io_instancegroups.yaml | 4 +- .../ekscloudformation/ekscloudformation.go | 29 +++++++--- .../ekscloudformation_test.go | 57 +++++++++++++------ docs/04_instance-manager.yaml | 11 +++- 8 files changed, 109 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49187c58..f866f20b 100644 --- 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 index e4ac971a..f845442a 100644 --- 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 index 5fe547dd..8e288b5d 100644 --- 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 index e4f94535..5c693b59 100644 --- 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 index 7aafd844..ae59794c 100644 --- 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 index 106a7219..c43d19cd 100644 --- 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,14 +498,15 @@ 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] @@ -509,7 +514,17 @@ func getExistingRoleName(role string) string { 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 index 3fedcd87..42b364ea 100644 --- 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 index e6116e6e..01059390 100644 --- 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