Bug: Creates unnecessary AWSLambdaVPCAccessExecutionRole role about serverless-application-model HOT 7 CLOSED

@GavinZZ Got you. Thanks! I will take this workaround into account.

from serverless-application-model.

Thanks for opening this issue. Like you mentioned, it doesn't look like the if is being evaluated here. Transferring to the SAM repo in case the folks there have more insight into this.

from serverless-application-model.

Thanks for creating an issue. I can confirm that I can reproduce this issue.

Unfortunately there's not much we can do because SAM does not resolve complex intrinsics and due to backward compatibility issue. A detailed explanation on why SAM doesn't resolve these intrinsics is explained here. AFAIK, resolving these intrinsics typically happens at a later stage after SAM Transform during CloudFormation create changeset step. During SAM transform, there is no way for us to figure out the value of this condition thus deciding to add the policy or not.

If this behaviour is bothering you, I would recommend providing a custom iam role and policies to the lambda function.

from serverless-application-model.

@GavinZZ What do you mean "...there's not much we can do"? 😀 This behaviour simply escalates privileges. It is a bug and you claim "we can't do anything about it"? Isn't it a program after all what ever it does (parses, transforms, etc... one-pass, two-pass, creates abstract syntax tree....)?
Imagine instead of AWSLambdaVPCAccessExecutionRole it adds

“Statement”: [{
        “Effect”: “Allow”,
        “Action”: “iam:PassRole”,
        “Resource”: “*”
    }] }

The rule of the least privileges (AWS's motto) are violated by AWS themselves?
Well, in this particular case it does not bother too much, but I hope we won't see more severe escalations of privileges.

Thank you. 🙂

from serverless-application-model.

Hi @fade2black I understand that it is frustrating and I acknowledge it breaks the principle of least privilege. Please allow me to explain the reasoning a bit more in detail. This problem arises from a inconsiderate design issue when this is first implemented. There are are underlying issue that blocked us from fixing it.

As described in my previous comment, SAM does not have the capability to figure out the value of Fn::If intrinsics. Without knowing the exact value, we cannot determine whether or not to add the policy or not. The proper solution here would be not, by default, add the policy (or like the pass role example you provided). I absolutely agree that this should have been the way from the first implementation.

However, we cannot proceed with this change as it could be a potentially backward incompatible changes, causing users' lambda function who rely on this behaviour to add the policy to no longer work. SAM promises to always guarantee backward compatibility and that's why we cannot proceed with this change.

One additional workaround that I just think of is to rely on the use of Transform: AWS::Serverless-2016-10-31 transform before SAM Transform, like the following:

Transform:
  - AWS::LanguageExtensions
  - AWS::Serverless-2016-10-31

This would resolve the intrinsics into a static value and when SAM transform the template, it knows the exact value and would hence not add the policy.

from serverless-application-model.

Closing this issue as it's not in a workable state due to backward compatibility issue. Feel free to open another issue if you have additional questions.

from serverless-application-model.

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

from serverless-application-model.