Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drop basic exec policy from caller role #763

Merged
merged 1 commit into from
Jul 12, 2024

Conversation

eschultink
Copy link
Member

@eschultink eschultink commented Jul 11, 2024

Fixes

  • lambda basic exec role is effectively just logging perms, not needed - as lambdas have their own exec rules with those perms

Change implications

  • dependencies added/changed? no
  • something important to note in future release notes? yes - will see role attachment being destroyed

@eschultink eschultink self-assigned this Jul 11, 2024
Base automatically changed from rc-0.4.57 to main July 11, 2024 21:27
@@ -103,11 +103,6 @@ resource "aws_iam_policy" "execution_lambda_to_caller" {
}
}

resource "aws_iam_role_policy_attachment" "invoker_lambda_execution" {
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure this works? Do lambdas log because they have the rights to do so, or because the caller (that can do InvokeURL) also has logging permissions?
Also this policy is mentioned in various doc files, see https://github.com/search?q=repo%3AWorklytics%2Fpsoxy%20AWSLambdaBasicExecutionRole&type=code . Does it need to be dropped in certain cases?

Copy link
Member Author

Choose a reason for hiding this comment

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

lambdas don't execute as this role

they each have their own execution role; and the lambda execution permission is attached to each here:

resource "aws_iam_role_policy_attachment" "basic" {

Copy link
Member

Choose a reason for hiding this comment

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

got it. Looks good then

@eschultink eschultink changed the base branch from main to rc-v0.4.58 July 11, 2024 21:55
@eschultink eschultink merged commit ca23a2a into rc-v0.4.58 Jul 12, 2024
36 checks passed
@eschultink eschultink deleted the s178-drop-caller-exec-role branch July 12, 2024 19:05
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.

None yet

3 participants