Skip to content

Commit

Permalink
(Managed)NodeGroups: Handle instanceRoleARN with nested resource path
Browse files Browse the repository at this point in the history
  • Loading branch information
dmcneil committed Sep 29, 2020
1 parent ed665e5 commit c7c2ff0
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 3 deletions.
23 changes: 23 additions & 0 deletions pkg/cfn/builder/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1601,6 +1601,29 @@ var _ = Describe("CloudFormation template builder API", func() {
})
})

Context("NodeGroup with custom role containing a deep resource path is normalized", func() {
cfg, ng := newClusterConfigAndNodegroup(true)

ng.IAM.InstanceRoleARN = "arn:aws:iam::1234567890:role/foo/bar/baz/custom-eks-role"

build(cfg, "eksctl-test-123-cluster", ng)

roundtrip()

It("should have correct instance role and profile", func() {
Expect(ngTemplate.Resources).ToNot(HaveKey("NodeInstanceRole"))
Expect(ngTemplate.Resources).To(HaveKey("NodeInstanceProfile"))

profile := ngTemplate.Resources["NodeInstanceProfile"].Properties

Expect(profile.Path).To(Equal("/"))
Expect(profile.Roles).To(HaveLen(1))
Expect(profile.Roles[0]).To(Equal("arn:aws:iam::1234567890:role/custom-eks-role"))

isFnGetAttOf(getLaunchTemplateData(ngTemplate).IamInstanceProfile.Arn, "NodeInstanceProfile", "Arn")
})
})

Context("NodeGroup with cutom profile", func() {
cfg, ng := newClusterConfigAndNodegroup(true)

Expand Down
6 changes: 4 additions & 2 deletions pkg/cfn/builder/iam.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,17 +144,19 @@ func (n *NodeGroupResourceSet) addResourcesForIAM() error {
n.rs.withIAM = true

if n.spec.IAM.InstanceRoleARN != "" {
roleARN := NormalizeARN(n.spec.IAM.InstanceRoleARN)

// if role is set, but profile isn't - create profile
n.newResource(cfnIAMInstanceProfileName, &gfniam.InstanceProfile{
Path: gfnt.NewString("/"),
Roles: gfnt.NewStringSlice(n.spec.IAM.InstanceRoleARN),
Roles: gfnt.NewStringSlice(roleARN),
})
n.instanceProfileARN = gfnt.MakeFnGetAttString(cfnIAMInstanceProfileName, "Arn")
n.rs.defineOutputFromAtt(outputs.NodeGroupInstanceProfileARN, cfnIAMInstanceProfileName, "Arn", true, func(v string) error {
n.spec.IAM.InstanceProfileARN = v
return nil
})
n.rs.defineOutputWithoutCollector(outputs.NodeGroupInstanceRoleARN, n.spec.IAM.InstanceRoleARN, true)
n.rs.defineOutputWithoutCollector(outputs.NodeGroupInstanceRoleARN, roleARN, true)
return nil
}

Expand Down
16 changes: 16 additions & 0 deletions pkg/cfn/builder/iam_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,3 +318,19 @@ func makeManagedPolicies(iamCluster *api.ClusterIAM, iamConfig *api.NodeGroupIAM
makePolicyARNs(managedPolicyNames.List()...)...,
)...), nil
}

// NormalizeARN returns the ARN with just the last element in the resource path preserved. If the
// input does not contain at least one forward-slash then the input is returned unmodified.
//
// When providing an existing instanceRoleARN that contains a path other than "/", nodes may
// fail to join the cluster as the AWS IAM Authenticator does not recognize such ARNs declared in
// the aws-auth ConfigMap.
//
// See: https://docs.aws.amazon.com/eks/latest/userguide/troubleshooting.html#troubleshoot-container-runtime-network
func NormalizeARN(arn string) string {
parts := strings.Split(arn, "/")
if len(parts) <= 1 {
return arn
}
return fmt.Sprintf("%s/%s", parts[0], parts[len(parts)-1])
}
2 changes: 1 addition & 1 deletion pkg/cfn/builder/managed_nodegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func (m *ManagedNodeGroupResourceSet) AddAllResources() error {
}
nodeRole = gfnt.MakeFnGetAttString(cfnIAMInstanceRoleName, "Arn")
} else {
nodeRole = gfnt.NewString(m.nodeGroup.IAM.InstanceRoleARN)
nodeRole = gfnt.NewString(NormalizeARN(m.nodeGroup.IAM.InstanceRoleARN))
}

subnets, err := AssignSubnets(m.nodeGroup.AvailabilityZones, m.clusterStackName, m.clusterConfig, m.nodeGroup.PrivateNetworking)
Expand Down
12 changes: 12 additions & 0 deletions pkg/cfn/builder/managed_nodegroup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,18 @@ func TestManagedNodeRole(t *testing.T) {
expectedNewRole: false,
expectedNodeRoleARN: gfnt.NewString("arn::DUMMY::DUMMYROLE"), // using the provided role
},
{
description: "InstanceRoleARN is provided and normalized",
nodeGroup: &api.ManagedNodeGroup{
NodeGroupBase: &api.NodeGroupBase{
IAM: &api.NodeGroupIAM{
InstanceRoleARN: "arn:aws:iam::1234567890:role/foo/bar/baz/custom-eks-role",
},
},
},
expectedNewRole: false,
expectedNodeRoleARN: gfnt.NewString("arn:aws:iam::1234567890:role/custom-eks-role"),
},
}

for i, tt := range nodeRoleTests {
Expand Down

0 comments on commit c7c2ff0

Please sign in to comment.