Skip to content

Conversation

@hgreebe
Copy link
Contributor

@hgreebe hgreebe commented Nov 6, 2025

Description of changes

  • Add GetFunction and GetPolicy permissions to the build image cleanup role
  • AccessDenied Errors were encountered when trying to delete the DeleteFunction Lambda function
  • Cloudtrail events were encountering the following error:
User: arn:aws:sts::<ACCOUNT_ID>:assumed-role/PClusterBuildImageCleanupRole-*-v1/ParallelClusterImage-* is not authorized to perform: lambda:GetFunction on resource: arn:aws:lambda:us-west-2:<ACCOUNT_ID>:function:ParallelClusterImage-* because no identity-based policy allows the lambda:GetFunction action
User: arn:aws:sts::<ACCOUNT_ID>:assumed-role/PClusterBuildImageCleanupRole-*-v1/ParallelClusterImage-* is not authorized to perform: lambda:GetPolicy on resource: arn:aws:lambda:us-west-2:<ACCOUNT_ID>:function:ParallelClusterImage-* because no identity-based policy allows the lambda:GetPolicy action

Tests

  • Running test_build_image and checked CloudTrail to validate that GetFunction and GetPolicy actions no longer had AccessDenied errors.

Please review the guidelines for contributing and Pull Request Instructions.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@hgreebe hgreebe marked this pull request as ready for review November 6, 2025 20:04
@hgreebe hgreebe requested review from a team as code owners November 6, 2025 20:04
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC we need to increment only when we introduce breaking changes to the policy. This is an additive fix, not a breaking change. So I think we can keep the version to 1.

Can you please double check?
If this is the case, then please also fix the comment, as it could be misleading.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, I was wrong. We need to increase the version at every change, because we do not modify the exisitng role
See code

if already_bootstrapped:
return role_arn

{"Action": "tag:TagResources", "Resource": "*", "Effect": "Allow"},
{
"Action": ["lambda:DeleteFunction", "lambda:RemovePermission"],
"Action": ["lambda:DeleteFunction", "lambda:RemovePermission", "lambda:GetFunction", "lambda:GetPolicy"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Test] I would expect a unit test to be adapted accordingly. If there is no unit test covering this part, can we add it to cover this change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants