Skip to content

Commit e969b88

Browse files
authored
Add GetFunction and GetPolicy permissions to the build image cleanup role (#7087)
* Add GetFunction and GetPolicy permissions to the build image cleanup role * Increase pcluster build image cleanup role revision number * Update CHANGELOG * Add unit test for actions in CleanupRole policy * Fix linter errors
1 parent 7f65bd7 commit e969b88

File tree

4 files changed

+28
-6
lines changed

4 files changed

+28
-6
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ CHANGELOG
1111

1212
**BUG FIXES**
1313
- Reduce EFA installation time for Ubuntu by ~20 minutes by only holding kernel packages for the installed kernel.
14+
- Add GetFunction and GetPolicy permissions to PClusterBuildImageCleanupRole to prevent AccessDenied errors during build image stack deletion.
1415

1516
3.14.1
1617
------

cli/src/pcluster/constants.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ class Operation(Enum):
343343

344344
PCLUSTER_BUILD_IMAGE_CLEANUP_ROLE_PREFIX = "PClusterBuildImageCleanupRole"
345345
# Tag key & expected revision (increment when policy widens)
346-
PCLUSTER_BUILD_IMAGE_CLEANUP_ROLE_REVISION = 1
346+
PCLUSTER_BUILD_IMAGE_CLEANUP_ROLE_REVISION = 2
347347
PCLUSTER_BUILD_IMAGE_CLEANUP_ROLE_BOOTSTRAP_TAG_KEY = "parallelcluster:build-image-cleanup-role-bootstrapped"
348348

349349
P6E_GB200 = "p6e-gb200"

cli/src/pcluster/imagebuilder_utils.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,12 @@ def _expected_inline_policy(account_id: str, partition: str):
140140
{"Action": "ec2:CreateTags", "Resource": f"arn:{partition}:ec2:*::image/*", "Effect": "Allow"},
141141
{"Action": "tag:TagResources", "Resource": "*", "Effect": "Allow"},
142142
{
143-
"Action": ["lambda:DeleteFunction", "lambda:RemovePermission"],
143+
"Action": [
144+
"lambda:DeleteFunction",
145+
"lambda:RemovePermission",
146+
"lambda:GetFunction",
147+
"lambda:GetPolicy",
148+
],
144149
"Resource": f"arn:{partition}:lambda:*:{account_id}:function:ParallelClusterImage-*",
145150
"Effect": "Allow",
146151
},

cli/tests/pcluster/cli/test_build_image.py

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -283,26 +283,42 @@ def test_ensure_default_build_image_stack_cleanup_role_permission_denied(self, a
283283
aws_api_mock.iam.tag_role.assert_not_called()
284284

285285
@pytest.mark.parametrize(
286-
"account_id, partition",
286+
"account_id, partition, actions",
287287
[
288-
("123456789012", "aws"),
289-
("000000000000", "aws-us-gov"),
288+
(
289+
"123456789012",
290+
"aws",
291+
["lambda:DeleteFunction", "lambda:RemovePermission", "lambda:GetFunction", "lambda:GetPolicy"],
292+
),
293+
(
294+
"000000000000",
295+
"aws-us-gov",
296+
["lambda:DeleteFunction", "lambda:RemovePermission", "lambda:GetFunction", "lambda:GetPolicy"],
297+
),
290298
],
291299
)
292-
def test_expected_inline_policy_dynamic_fields(self, account_id, partition):
300+
def test_expected_inline_policy_dynamic_fields(self, account_id, partition, actions):
293301
raw = _expected_inline_policy(account_id, partition)
294302
policy = json.loads(raw)
295303
assert policy["Version"] == "2012-10-17"
296304
assert len(policy["Statement"]) == 13
297305
for statement in policy["Statement"]:
298306
resources = statement["Resource"]
307+
action = statement["Action"]
308+
action = action if isinstance(action, list) else [action]
309+
for act in action:
310+
if act in actions:
311+
actions.remove(act)
312+
299313
resources = resources if isinstance(resources, list) else [resources]
300314
for res in resources:
301315
if res == "*":
302316
continue
303317
assert f"arn:{partition}" in res
304318
if not res == f"arn:{partition}:ec2:*::image/*":
305319
assert f":{account_id}:" in res
320+
if len(actions) != 0:
321+
raise AssertionError(f"Actions {actions} are not in the policy")
306322

307323
def _build_args(self, args):
308324
args = [[k, v] if v is not None else [k] for k, v in args.items()]

0 commit comments

Comments
 (0)