-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
feat: TBD #3735
base: develop
Are you sure you want to change the base?
feat: TBD #3735
Conversation
samtranslator/model/sam_resources.py
Outdated
""" | ||
Construct the lambda permission associated with the function url resource in a case | ||
Construct the lambda permissions associated with the function url resource in a case | ||
for public access when AuthType is NONE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AI: Check AuthType = "NONE"
is explicit or implicit. We want to avoid creating additional permission if customers do not explicitly tell SAM to do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirm that good to go. AuthType = "NONE"
need to be explicitly specified by customers.
logical_id=url_invoke_permission_logical_id, attributes=lambda_permission_attributes | ||
) | ||
lambda_invoke_permission.Action = FUNCTION_INVOKE_PERMISSION_ACTION | ||
lambda_invoke_permission.Principal = "*" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not new in the PR, but I am curious why the service principal is *
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sync offline, this is by design meant for public (whoever).
samtranslator/model/sam_resources.py
Outdated
""" | ||
Construct the lambda permission associated with the function url resource in a case | ||
Construct the lambda permissions associated with the function url resource in a case | ||
for public access when AuthType is NONE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirm that good to go. AuthType = "NONE"
need to be explicitly specified by customers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The linter is failing on the generated CFN files. That's because InvokedViaFunctionUrl
is not yet available on CloudFormation. To make the linter pass, please add tests/translator/output/**/function_with_function_url_config.json
in cfnlintrc.yaml
file to ignore it. (Should be removed once the feature is GA)
Thanks, I'm kind of swaying between doing that (typically in private repo) or wait for official CFN update (public repo). |
Issue #, if available
Description of changes
Description of how you validated changes
Checklist
Examples?
Please reach out in the comments if you want to add an example. Examples will be
added to
sam init
through aws/aws-sam-cli-app-templates.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.