Lambda: improve function name and qualifier validation #11366
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LocalStack Community integration with Pro 2 files ± 0 2 suites ±0 1h 13m 55s ⏱️ - 21m 54s Results for commit b6f8a0e. ± Comparison against base commit 0063912. This pull request removes 916 and adds 72 tests. Note that renamed tests count towards both.This pull request removes 97 skipped tests and adds 4 skipped tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
dfangl
requested changes
Aug 22, 2024
2979749 to
4b4362b
Compare
4b4362b to
b6f8a0e
Compare
dfangl
approved these changes
Aug 26, 2024
There was a problem hiding this comment.
LGTM now! Thanks for adding those changes!
FYI: We squash commits anyway on merging, so it is not necessary to force push for changes, and keep the commit count low. Personally, I find it is easier, especially for review, if you can see the changes done after a review in separate commits. Others might disagree with this, though 😅
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Current validation checks for Lambda function names did not account for some invalid regex combinations as well as length limits for
FunctionNamediffering between request types. This will add tests ensuring a wider range of invalidFunctionNameandQualfiercombinations are caught.Changes
The Lambda Provider:
create_functionin favour ofapi_utils.get_name_and_qualifierDeleteFunctionThe API utils
api_utils.py:get_name_and_qualifier-- which is ubiquitously used across the Lambda provider and should enact these checks.validate_function_name(accounting specifically forCreateFunction,DeleteFunction,Invoke, andGetFunction) with sensible defaults being used elsewhere.validate_qualifierandconstruct_validation_exception_messageThe Lambda API tests:
test_function_name_and_qualifier_validationwhich accounts for a wide range of possible exception cases when attempting validation.Testing
The below is a test table showing all scenarios tested for
CreateFunction,DeleteFunctionGetFunctionandInvoketest types.Some considerations:
create_functionrequest.create_functionrequest -- withResourceNotFoundExceptionbeing thrown for othe types. None are skipped to ensure parity with AWS.my-function!my-functioninvalid!invalid-account:function:my-functionarn:aws:lambda:invalid-region:{account_id}:function:my-functionarn:aws:ec2:{region_name}:{account_id}:instance:i-1234567890abcdef0"a" * max_lengtharn:aws:lambda:invalid-region:{account_id}:function:my-function["a" * max_length]arn:aws:lambda:invalid-region:{account_id}:function:my-function-["a" * max_length]"a" * max_lengthmy-function"a" * max_lengtharn:aws:lambda:{region_name}:{account_id}:function:my-function:1:2my-function-1create_functionarn:aws:lambda:{region_name}:{account_id}:functioncreate_functionfunction:my-function:$LATEST:extraarn:aws:lambda:{region_name}:{account_id}:function:my-function:$LATEST1arn:aws:lambda:{region_name}:{account_id}:function:my-function$latestarn:aws:lambda::{account_id}:function:my-functionarn:aws:lambda:{region_name}::function:my-functionarn:aws:lambda:{region_name}:{account_id}:function:my-function:$LATESAdditional Pipelines
LocalStack Pro: IAM Policies Enforced
ext: Partial run against tests inaws/tests/lambda_and withENFORCE_IAMenabled at GitHub workflowENV: ENFORCE_IAM=1LocalStack Community: Multi-Account and Multi-Region
multi: Multi-region/account build at CircleCI pipeline.TODO
What's left to do:
Potential future works
CreateFunction,DeleteFunction,GetFunction, andInvoke