From 195c605f80bb8d34355b8f73362710d114228c98 Mon Sep 17 00:00:00 2001 From: Helena Greebe Date: Thu, 6 Nov 2025 08:20:06 -0500 Subject: [PATCH 1/5] Add GetFunction and GetPolicy permissions to the build image cleanup role --- cli/src/pcluster/imagebuilder_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/src/pcluster/imagebuilder_utils.py b/cli/src/pcluster/imagebuilder_utils.py index 9057231fd3..7c35f1720b 100644 --- a/cli/src/pcluster/imagebuilder_utils.py +++ b/cli/src/pcluster/imagebuilder_utils.py @@ -140,7 +140,7 @@ def _expected_inline_policy(account_id: str, partition: str): {"Action": "ec2:CreateTags", "Resource": f"arn:{partition}:ec2:*::image/*", "Effect": "Allow"}, {"Action": "tag:TagResources", "Resource": "*", "Effect": "Allow"}, { - "Action": ["lambda:DeleteFunction", "lambda:RemovePermission"], + "Action": ["lambda:DeleteFunction", "lambda:RemovePermission", "lambda:GetFunction", "lambda:GetPolicy"], "Resource": f"arn:{partition}:lambda:*:{account_id}:function:ParallelClusterImage-*", "Effect": "Allow", }, From 66356de4a3c1ab1d58f86bdd77c0d6802dbbb37c Mon Sep 17 00:00:00 2001 From: Helena Greebe Date: Thu, 6 Nov 2025 11:25:06 -0500 Subject: [PATCH 2/5] Increase pcluster build image cleanup role revision number --- cli/src/pcluster/constants.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/src/pcluster/constants.py b/cli/src/pcluster/constants.py index 9e8f33b53e..a5d37a0818 100644 --- a/cli/src/pcluster/constants.py +++ b/cli/src/pcluster/constants.py @@ -343,7 +343,7 @@ class Operation(Enum): PCLUSTER_BUILD_IMAGE_CLEANUP_ROLE_PREFIX = "PClusterBuildImageCleanupRole" # Tag key & expected revision (increment when policy widens) -PCLUSTER_BUILD_IMAGE_CLEANUP_ROLE_REVISION = 1 +PCLUSTER_BUILD_IMAGE_CLEANUP_ROLE_REVISION = 2 PCLUSTER_BUILD_IMAGE_CLEANUP_ROLE_BOOTSTRAP_TAG_KEY = "parallelcluster:build-image-cleanup-role-bootstrapped" P6E_GB200 = "p6e-gb200" From 4617b4756a20f4c718610dcea078b8d812ae6b6e Mon Sep 17 00:00:00 2001 From: Helena Greebe Date: Thu, 6 Nov 2025 14:15:21 -0500 Subject: [PATCH 3/5] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f47c3c11f..d1ab65543b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ CHANGELOG **BUG FIXES** - Reduce EFA installation time for Ubuntu by ~20 minutes by only holding kernel packages for the installed kernel. +- Add GetFunction and GetPolicy permissions to PClusterBuildImageCleanupRole to prevent AccessDenied errors during build image stack deletion. 3.14.1 ------ From 469197ff24569aeb5f1d45b454eae16d300d6035 Mon Sep 17 00:00:00 2001 From: Helena Greebe Date: Fri, 7 Nov 2025 11:17:44 -0500 Subject: [PATCH 4/5] Add unit test for actions in CleanupRole policy --- cli/tests/pcluster/cli/test_build_image.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/cli/tests/pcluster/cli/test_build_image.py b/cli/tests/pcluster/cli/test_build_image.py index fe42fd003b..d93135c326 100644 --- a/cli/tests/pcluster/cli/test_build_image.py +++ b/cli/tests/pcluster/cli/test_build_image.py @@ -283,19 +283,25 @@ def test_ensure_default_build_image_stack_cleanup_role_permission_denied(self, a aws_api_mock.iam.tag_role.assert_not_called() @pytest.mark.parametrize( - "account_id, partition", + "account_id, partition, actions", [ - ("123456789012", "aws"), - ("000000000000", "aws-us-gov"), + ("123456789012", "aws", ["lambda:DeleteFunction", "lambda:RemovePermission", "lambda:GetFunction", "lambda:GetPolicy"]), + ("000000000000", "aws-us-gov", ["lambda:DeleteFunction", "lambda:RemovePermission", "lambda:GetFunction", "lambda:GetPolicy"]), ], ) - def test_expected_inline_policy_dynamic_fields(self, account_id, partition): + def test_expected_inline_policy_dynamic_fields(self, account_id, partition, actions): raw = _expected_inline_policy(account_id, partition) policy = json.loads(raw) assert policy["Version"] == "2012-10-17" assert len(policy["Statement"]) == 13 for statement in policy["Statement"]: resources = statement["Resource"] + action = statement["Action"] + action = action if isinstance(action, list) else [action] + for act in action: + if act in actions: + actions.remove(act) + resources = resources if isinstance(resources, list) else [resources] for res in resources: if res == "*": @@ -303,6 +309,8 @@ def test_expected_inline_policy_dynamic_fields(self, account_id, partition): assert f"arn:{partition}" in res if not res == f"arn:{partition}:ec2:*::image/*": assert f":{account_id}:" in res + if len(actions) != 0: + assert False, f"Actions {actions} are not in the policy" def _build_args(self, args): args = [[k, v] if v is not None else [k] for k, v in args.items()] From de1e865b8352064f2421031d80243e3125c21e66 Mon Sep 17 00:00:00 2001 From: Helena Greebe Date: Mon, 10 Nov 2025 11:10:20 -0500 Subject: [PATCH 5/5] Fix linter errors --- cli/src/pcluster/imagebuilder_utils.py | 7 ++++++- cli/tests/pcluster/cli/test_build_image.py | 14 +++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/cli/src/pcluster/imagebuilder_utils.py b/cli/src/pcluster/imagebuilder_utils.py index 7c35f1720b..858cc27f15 100644 --- a/cli/src/pcluster/imagebuilder_utils.py +++ b/cli/src/pcluster/imagebuilder_utils.py @@ -140,7 +140,12 @@ def _expected_inline_policy(account_id: str, partition: str): {"Action": "ec2:CreateTags", "Resource": f"arn:{partition}:ec2:*::image/*", "Effect": "Allow"}, {"Action": "tag:TagResources", "Resource": "*", "Effect": "Allow"}, { - "Action": ["lambda:DeleteFunction", "lambda:RemovePermission", "lambda:GetFunction", "lambda:GetPolicy"], + "Action": [ + "lambda:DeleteFunction", + "lambda:RemovePermission", + "lambda:GetFunction", + "lambda:GetPolicy", + ], "Resource": f"arn:{partition}:lambda:*:{account_id}:function:ParallelClusterImage-*", "Effect": "Allow", }, diff --git a/cli/tests/pcluster/cli/test_build_image.py b/cli/tests/pcluster/cli/test_build_image.py index d93135c326..1e920f34f2 100644 --- a/cli/tests/pcluster/cli/test_build_image.py +++ b/cli/tests/pcluster/cli/test_build_image.py @@ -285,8 +285,16 @@ def test_ensure_default_build_image_stack_cleanup_role_permission_denied(self, a @pytest.mark.parametrize( "account_id, partition, actions", [ - ("123456789012", "aws", ["lambda:DeleteFunction", "lambda:RemovePermission", "lambda:GetFunction", "lambda:GetPolicy"]), - ("000000000000", "aws-us-gov", ["lambda:DeleteFunction", "lambda:RemovePermission", "lambda:GetFunction", "lambda:GetPolicy"]), + ( + "123456789012", + "aws", + ["lambda:DeleteFunction", "lambda:RemovePermission", "lambda:GetFunction", "lambda:GetPolicy"], + ), + ( + "000000000000", + "aws-us-gov", + ["lambda:DeleteFunction", "lambda:RemovePermission", "lambda:GetFunction", "lambda:GetPolicy"], + ), ], ) def test_expected_inline_policy_dynamic_fields(self, account_id, partition, actions): @@ -310,7 +318,7 @@ def test_expected_inline_policy_dynamic_fields(self, account_id, partition, acti if not res == f"arn:{partition}:ec2:*::image/*": assert f":{account_id}:" in res if len(actions) != 0: - assert False, f"Actions {actions} are not in the policy" + raise AssertionError(f"Actions {actions} are not in the policy") def _build_args(self, args): args = [[k, v] if v is not None else [k] for k, v in args.items()]