Skip to content

Commit 6cf7cdf

Browse files
authored
Fix bug in updating user.spec.inlinePolicies (#83)
Issue aws-controllers-k8s/community#1906 The bug occurs when updating an existing item in `spec.inlinePolicies`. The code incorrectly includes both the old(to remove) and new(to add)values when using `lo.Difference`. This causes the code to first attempt to replace the xisting policies with a call to `iam::PutUserPolicy` and then mistakenly "delete" them right after. This patch fixes this bug by calling `lo.Find` to double check whether we need to keep an inline policy (if it only needs to be updated) By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent b10b5d9 commit 6cf7cdf

File tree

2 files changed

+53
-2
lines changed

2 files changed

+53
-2
lines changed

pkg/resource/user/hooks.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,11 @@ func (rm *resourceManager) syncInlinePolicies(
234234
}
235235
}
236236
for _, pair := range toDelete {
237+
// do not remove elements we just updated with `addInlinePolicy`
238+
if _, ok := lo.Find(toAdd, func(entry lo.Entry[string, string]) bool { return entry.Key == pair.Key }); ok {
239+
continue
240+
}
241+
237242
polName := pair.Key
238243
rlog.Debug(
239244
"removing inline policy from user",

test/e2e/tests/test_user.py

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,10 +147,20 @@ def test_crud(self, simple_user):
147147
"Resource": ["*"]
148148
}]
149149
}'''
150+
inline_doc_2 = '''{
151+
"Version": "2012-10-17",
152+
"Statement": [{
153+
"Effect": "Allow",
154+
"Action": ["s3:Get*"],
155+
"Resource": ["*"]
156+
}]
157+
}'''
158+
150159
updates = {
151160
"spec": {
152161
"inlinePolicies": {
153162
"ec2get": inline_doc,
163+
"s3get": inline_doc_2,
154164
},
155165
},
156166
}
@@ -159,23 +169,59 @@ def test_crud(self, simple_user):
159169

160170
expect_inline_policies = {
161171
'ec2get': inline_doc,
172+
's3get': inline_doc_2,
162173
}
163174
cr = k8s.get_resource(ref)
164175
assert cr is not None
165176
assert 'spec' in cr
166177
assert 'inlinePolicies' in cr['spec']
167-
assert len(cr['spec']['inlinePolicies']) == 1
178+
assert len(cr['spec']['inlinePolicies']) == 2
168179
assert expect_inline_policies == cr['spec']['inlinePolicies']
169180

170181
latest_inline_policies = user.get_inline_policies(user_name)
171-
assert len(latest_inline_policies) == 1
182+
assert len(latest_inline_policies) == 2
172183
assert 'ec2get' in latest_inline_policies
184+
173185
got_pol_doc = latest_inline_policies['ec2get']
174186
nospace_got_doc = "".join(c for c in got_pol_doc if not c.isspace())
175187
nospace_exp_doc = "".join(c for c in inline_doc if not c.isspace())
188+
assert nospace_exp_doc == nospace_got_doc
176189

190+
got_pol_doc = latest_inline_policies['s3get']
191+
nospace_got_doc = "".join(c for c in got_pol_doc if not c.isspace())
192+
nospace_exp_doc = "".join(c for c in inline_doc_2 if not c.isspace())
177193
assert nospace_exp_doc == nospace_got_doc
178194

195+
inline_doc_s3_get_object = '''{
196+
"Version": "2012-10-17",
197+
"Statement": [{
198+
"Effect": "Allow",
199+
"Action": ["s3:GetObject"],
200+
"Resource": ["*"]
201+
}]
202+
}'''
203+
# update s3get policy document
204+
updates = {
205+
"spec": {
206+
"inlinePolicies": {
207+
"ec2get": inline_doc,
208+
"s3get": inline_doc_s3_get_object,
209+
},
210+
},
211+
}
212+
k8s.patch_custom_resource(ref, updates)
213+
time.sleep(MODIFY_WAIT_AFTER_SECONDS)
214+
215+
latest_inline_policies = user.get_inline_policies(user_name)
216+
assert len(latest_inline_policies) == 2
217+
assert 's3get' in latest_inline_policies
218+
assert 'ec2get' in latest_inline_policies
219+
220+
# expect s3get policy document to change into inline_doc_s3_get_object
221+
got_pol_doc = latest_inline_policies['s3get']
222+
nospace_got_doc = "".join(c for c in got_pol_doc if not c.isspace())
223+
nospace_exp_doc = "".join(c for c in inline_doc_s3_get_object if not c.isspace())
224+
179225
# Remove the inline policy we just added and check the updates are
180226
# reflected in the IAM API
181227
updates = {

0 commit comments

Comments
 (0)