Skip to content

Commit 8d285a8

Browse files
adammwadammw
authored
Add AttachPolicy to NodeGroupIAM configuration (#4331)
* Add AttachPolicy to NodeGroupIAM configuration * Add NodeGroup AttachPolicy tests * fix * Add nodegroup IAM attachPolicy field to userdocs * Add assertion for attachPolicy policy document * Compare JSON encoded policy due to interface differences
1 parent f958457 commit 8d285a8

File tree

11 files changed

+144
-6
lines changed

11 files changed

+144
-6
lines changed

‎pkg/apis/eksctl.io/v1alpha5/assets/schema.json‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1519,11 +1519,18 @@
15191519
},
15201520
"NodeGroupIAM": {
15211521
"properties": {
1522+
"attachPolicy": {
1523+
"$ref": "#/definitions/InlineDocument",
1524+
"description": "holds a policy document to attach",
1525+
"x-intellij-html-description": "holds a policy document to attach"
1526+
},
15221527
"attachPolicyARNs": {
15231528
"items": {
15241529
"type": "string"
15251530
},
1526-
"type": "array"
1531+
"type": "array",
1532+
"description": "list of ARNs of the IAM policies to attach",
1533+
"x-intellij-html-description": "list of ARNs of the IAM policies to attach"
15271534
},
15281535
"instanceProfileARN": {
15291536
"type": "string"
@@ -1542,6 +1549,7 @@
15421549
}
15431550
},
15441551
"preferredOrder": [
1552+
"attachPolicy",
15451553
"attachPolicyARNs",
15461554
"instanceProfileARN",
15471555
"instanceRoleARN",

‎pkg/apis/eksctl.io/v1alpha5/types.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1116,6 +1116,10 @@ type (
11161116
}
11171117
// NodeGroupIAM holds all IAM attributes of a NodeGroup
11181118
NodeGroupIAM struct {
1119+
// AttachPolicy holds a policy document to attach
1120+
// +optional
1121+
AttachPolicy InlineDocument `json:"attachPolicy,omitempty"`
1122+
// list of ARNs of the IAM policies to attach
11191123
// +optional
11201124
AttachPolicyARNs []string `json:"attachPolicyARNs,omitempty"`
11211125
// +optional

‎pkg/apis/eksctl.io/v1alpha5/validation.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -601,6 +601,9 @@ func validateNodeGroupIAM(iam *NodeGroupIAM, value, fieldName, path string) erro
601601
if iam.InstanceRoleName != "" {
602602
return fmtFieldConflictErr("instanceRoleName")
603603
}
604+
if iam.AttachPolicy != nil {
605+
return fmtFieldConflictErr("attachPolicy")
606+
}
604607
if len(iam.AttachPolicyARNs) != 0 {
605608
return fmtFieldConflictErr("attachPolicyARNs")
606609
}

‎pkg/apis/eksctl.io/v1alpha5/validation_test.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
. "github.com/onsi/gomega"
1010

1111
api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
12+
cft "github.com/weaveworks/eksctl/pkg/cfn/template"
1213
"github.com/weaveworks/eksctl/pkg/utils/strings"
1314
)
1415

@@ -228,6 +229,15 @@ var _ = Describe("ClusterConfig validation", func() {
228229
ng0 := cfg.NewNodeGroup()
229230
ng0.Name = "ng0"
230231

232+
ng0.IAM.AttachPolicy = cft.MakePolicyDocument(
233+
cft.MapOfInterfaces{
234+
"Effect": "Allow",
235+
"Action": []string{
236+
"s3:Get*",
237+
},
238+
"Resource": "*",
239+
},
240+
)
231241
ng0.IAM.AttachPolicyARNs = []string{
232242
"arn:aws:iam::aws:policy/Foo",
233243
"arn:aws:iam::aws:policy/Bar",
@@ -308,6 +318,23 @@ var _ = Describe("ClusterConfig validation", func() {
308318
Expect(err.Error()).To(Equal("nodeGroups[1].iam.instanceRoleARN and nodeGroups[1].iam.instanceRoleName cannot be set at the same time"))
309319
})
310320

321+
It("should not allow setting instanceRoleARN and attachPolicy", func() {
322+
ng1.IAM.InstanceRoleARN = "r1"
323+
ng1.IAM.AttachPolicy = cft.MakePolicyDocument(
324+
cft.MapOfInterfaces{
325+
"Effect": "Allow",
326+
"Action": []string{
327+
"s3:Get*",
328+
},
329+
"Resource": "*",
330+
},
331+
)
332+
333+
err = api.ValidateNodeGroup(1, ng1)
334+
Expect(err).To(HaveOccurred())
335+
Expect(err.Error()).To(Equal("nodeGroups[1].iam.instanceRoleARN and nodeGroups[1].iam.attachPolicy cannot be set at the same time"))
336+
})
337+
311338
It("should not allow setting instanceRoleARN and attachPolicyARNs", func() {
312339
ng1.IAM.InstanceRoleARN = "r1"
313340
ng1.IAM.AttachPolicyARNs = []string{

‎pkg/apis/eksctl.io/v1alpha5/zz_generated.deepcopy.go‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/cfn/builder/fakes/fake_cfn_template.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ type Properties struct {
3838
AssumeRolePolicyDocument interface{}
3939

4040
PolicyDocument struct {
41+
Version string
4142
Statement []struct {
4243
Action []string
4344
Effect string

‎pkg/cfn/builder/iam.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,14 @@ func (c *resourceSet) attachAllowPolicy(name string, refRole *gfnt.Value, statem
4949
})
5050
}
5151

52+
func (c *resourceSet) attachAllowPolicyDocument(name string, refRole *gfnt.Value, document api.InlineDocument) {
53+
c.newResource(name, &gfniam.Policy{
54+
PolicyName: makeName(name),
55+
Roles: gfnt.NewSlice(refRole),
56+
PolicyDocument: document,
57+
})
58+
}
59+
5260
// WithIAM states, if IAM roles will be created or not
5361
func (c *ClusterResourceSet) WithIAM() bool {
5462
return c.rs.withIAM

‎pkg/cfn/builder/iam_helper.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515

1616
type cfnTemplate interface {
1717
attachAllowPolicy(name string, refRole *gfnt.Value, statements []cft.MapOfInterfaces)
18+
attachAllowPolicyDocument(name string, refRole *gfnt.Value, document api.InlineDocument)
1819
newResource(name string, resource gfn.Resource) *gfnt.Value
1920
}
2021

@@ -97,6 +98,10 @@ func createRole(cfnTemplate cfnTemplate, clusterIAMConfig *api.ClusterIAM, iamCo
9798

9899
refIR := cfnTemplate.newResource(cfnIAMInstanceRoleName, &role)
99100

101+
if iamConfig.AttachPolicy != nil {
102+
cfnTemplate.attachAllowPolicyDocument("Policy1", refIR, iamConfig.AttachPolicy)
103+
}
104+
100105
if api.IsEnabled(iamConfig.WithAddonPolicies.AutoScaler) {
101106
cfnTemplate.attachAllowPolicy("PolicyAutoScaling", refIR, autoScalerStatements())
102107
}

‎pkg/cfn/builder/managed_nodegroup_test.go‎

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
package builder
22

33
import (
4+
"encoding/json"
45
"fmt"
56
"testing"
67

78
"github.com/stretchr/testify/require"
89
api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
10+
cft "github.com/weaveworks/eksctl/pkg/cfn/template"
911
"github.com/weaveworks/eksctl/pkg/nodebootstrap"
1012
"github.com/weaveworks/eksctl/pkg/nodebootstrap/fakes"
1113
"github.com/weaveworks/eksctl/pkg/testutils/mockprovider"
@@ -18,7 +20,9 @@ import (
1820
func TestManagedPolicyResources(t *testing.T) {
1921
iamRoleTests := []struct {
2022
addons api.NodeGroupIAMAddonPolicies
23+
attachPolicy api.InlineDocument
2124
attachPolicyARNs []string
25+
expectedNewPolicies []string
2226
expectedManagedPolicies []*gfnt.Value
2327
description string
2428
}{
@@ -42,16 +46,36 @@ func TestManagedPolicyResources(t *testing.T) {
4246
"AmazonEC2ContainerRegistryReadOnly", "AmazonSSMManagedInstanceCore", "CloudWatchAgentServerPolicy"),
4347
description: "CloudWatch enabled",
4448
},
49+
{
50+
addons: api.NodeGroupIAMAddonPolicies{
51+
AutoScaler: api.Enabled(),
52+
},
53+
expectedNewPolicies: []string{"PolicyAutoScaling"},
54+
expectedManagedPolicies: makePartitionedPolicies("AmazonEKSWorkerNodePolicy", "AmazonEKS_CNI_Policy", "AmazonEC2ContainerRegistryReadOnly", "AmazonSSMManagedInstanceCore"),
55+
description: "AutoScaler enabled",
56+
},
57+
{
58+
attachPolicy: cft.MakePolicyDocument(cft.MapOfInterfaces{
59+
"Effect": "Allow",
60+
"Action": []string{
61+
"s3:Get*",
62+
},
63+
"Resource": "*",
64+
}),
65+
expectedNewPolicies: []string{"Policy1"},
66+
expectedManagedPolicies: makePartitionedPolicies("AmazonEKSWorkerNodePolicy", "AmazonEKS_CNI_Policy", "AmazonEC2ContainerRegistryReadOnly", "AmazonSSMManagedInstanceCore"),
67+
description: "Custom inline policies",
68+
},
4569
{
4670
attachPolicyARNs: []string{"AmazonEKSWorkerNodePolicy", "AmazonEKS_CNI_Policy"},
4771
expectedManagedPolicies: subs(prefixPolicies("AmazonEKSWorkerNodePolicy", "AmazonEKS_CNI_Policy")),
48-
description: "Custom policies",
72+
description: "Custom managed policies",
4973
},
5074
// should not attach any additional policies
5175
{
5276
attachPolicyARNs: []string{"CloudWatchAgentServerPolicy"},
5377
expectedManagedPolicies: subs(prefixPolicies("CloudWatchAgentServerPolicy")),
54-
description: "Custom policies",
78+
description: "Custom managed policies",
5579
},
5680
// no duplicate values
5781
{
@@ -81,6 +105,7 @@ func TestManagedPolicyResources(t *testing.T) {
81105
ng := api.NewManagedNodeGroup()
82106
api.SetManagedNodeGroupDefaults(ng, clusterConfig.Metadata)
83107
ng.IAM.WithAddonPolicies = tt.addons
108+
ng.IAM.AttachPolicy = tt.attachPolicy
84109
ng.IAM.AttachPolicyARNs = prefixPolicies(tt.attachPolicyARNs...)
85110

86111
p := mockprovider.NewMockProvider()
@@ -99,11 +124,29 @@ func TestManagedPolicyResources(t *testing.T) {
99124
template, err := goformation.ParseJSON(bytes)
100125
require.NoError(err)
101126

102-
role, ok := template.GetAllIAMRoleResources()["NodeInstanceRole"]
103-
require.True(ok)
127+
role, err := template.GetIAMRoleWithName(cfnIAMInstanceRoleName)
128+
require.NoError(err)
104129

105130
require.ElementsMatch(tt.expectedManagedPolicies, role.ManagedPolicyArns.Raw().(gfnt.Slice))
106131

132+
policyNames := make([]string, 0)
133+
for name := range template.GetAllIAMPolicyResources() {
134+
policyNames = append(policyNames, name)
135+
}
136+
require.ElementsMatch(tt.expectedNewPolicies, policyNames)
137+
138+
// assert custom inline policy matches
139+
if tt.attachPolicy != nil {
140+
policy, err := template.GetIAMPolicyWithName("Policy1")
141+
require.NoError(err)
142+
143+
// convert to json for comparison since interfaces are not identical
144+
expectedPolicy, err := json.Marshal(tt.attachPolicy)
145+
require.NoError(err)
146+
actualPolicy, err := json.Marshal(policy.PolicyDocument)
147+
require.NoError(err)
148+
require.Equal(string(expectedPolicy), string(actualPolicy))
149+
}
107150
})
108151
}
109152

@@ -167,7 +210,7 @@ func TestManagedNodeRole(t *testing.T) {
167210

168211
template, err := goformation.ParseJSON(bytes)
169212
require.NoError(err)
170-
ngResource, ok := template.Resources["ManagedNodeGroup"]
213+
ngResource, ok := template.Resources[ManagedNodeGroupResourceName]
171214
require.True(ok)
172215
ng, ok := ngResource.(*gfneks.Nodegroup)
173216
require.True(ok)

‎pkg/cfn/builder/nodegroup_test.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/weaveworks/eksctl/pkg/cfn/builder"
1515
"github.com/weaveworks/eksctl/pkg/cfn/builder/fakes"
1616
"github.com/weaveworks/eksctl/pkg/cfn/outputs"
17+
cft "github.com/weaveworks/eksctl/pkg/cfn/template"
1718
"github.com/weaveworks/eksctl/pkg/eks/mocks"
1819
bootstrapfakes "github.com/weaveworks/eksctl/pkg/nodebootstrap/fakes"
1920
vpcfakes "github.com/weaveworks/eksctl/pkg/vpc/fakes"
@@ -225,6 +226,28 @@ var _ = Describe("Unmanaged NodeGroup Template Builder", func() {
225226
})
226227

227228
// TODO move into IAM tests?
229+
Context("attach policy is set", func() {
230+
PolicyDocument := cft.MakePolicyDocument(cft.MapOfInterfaces{
231+
"Effect": "Allow",
232+
"Action": []string{
233+
"s3:Get*",
234+
},
235+
"Resource": "*",
236+
})
237+
238+
BeforeEach(func() {
239+
ng.IAM.AttachPolicy = PolicyDocument
240+
})
241+
242+
It("adds a custom policy to the role", func() {
243+
Expect(ngTemplate.Resources).To(HaveKey("Policy1"))
244+
Expect(ngTemplate.Resources["Policy1"].Properties.PolicyDocument.Statement).To(HaveLen(1))
245+
Expect(ngTemplate.Resources["Policy1"].Properties.PolicyDocument.Statement[0].Action).To(Equal([]string{"s3:Get*"}))
246+
Expect(ngTemplate.Resources["Policy1"].Properties.Roles).To(HaveLen(1))
247+
Expect(isRefTo(ngTemplate.Resources["Policy1"].Properties.Roles[0], "NodeInstanceRole")).To(BeTrue())
248+
})
249+
})
250+
228251
Context("attach policy arns are set", func() {
229252
BeforeEach(func() {
230253
ng.IAM.AttachPolicyARNs = []string{"arn:aws:iam::1234567890:role/foo"}

0 commit comments

Comments
 (0)