-
Notifications
You must be signed in to change notification settings - Fork 0
Add test for Serverlesspresso AWS sample application #24
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
base: master
Are you sure you want to change the base?
Conversation
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.
PR Summary
This pull request adds a comprehensive scenario test for the Serverlesspresso AWS sample application, implementing a complex serverless coffee ordering system using various AWS services.
- Introduced
ServerlesspressoCoreStack
inserverlesspresso_core_stack.py
, setting up core AWS services like DynamoDB, EventBridge, IoT Core, and Lambda functions - Added multiple service constructs (e.g.,
AuthService
,ConfigService
,OrderManagerService
) in separate files underconstructs/
to modularize the application architecture - Implemented
test_serverlesspresso.py
with extensive test cases covering data population, store operations, user registration, order workflows, and WebSocket communication - Created
ServerlesspressoStack.json
CloudFormation template defining the complete infrastructure for the application - Added Lambda function implementations in
artifacts/functions/
for various application functionalities like order processing, QR code verification, and IoT publishing
29 file(s) reviewed, 50 comment(s)
Edit PR Review Bot Settings
] | ||
} | ||
}, | ||
"ValidatorServiceGetQRcodeFunctionB2637D0B": { |
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.
style: Lambda function code is embedded directly in the template. Consider using a separate file for better maintainability
"OrderManagerStateMachine": { | ||
"Type": "AWS::StepFunctions::StateMachine", | ||
"Properties": { | ||
"DefinitionString": "{\n \"Comment\": \"A description of my state machine\",\n \"StartAt\": \"Decide Action\",\n \"States\": {\n \"Decide Action\": {\n \"Type\": \"Choice\",\n \"Choices\": [\n {\n \"Variable\": \"$.action\",\n \"StringEquals\": \"complete\",\n \"Next\": \"Complete Order\"\n },\n {\n \"Variable\": \"$.action\",\n \"StringEquals\": \"cancel\",\n \"Next\": \"Cancel Order\"\n },\n {\n \"Variable\": \"$.action\",\n \"StringEquals\": \"make\",\n \"Next\": \"Claim Order\"\n },\n {\n \"Variable\": \"$.action\",\n \"StringEquals\": \"unmake\",\n \"Next\": \"Claim Order\"\n }\n ],\n \"Default\": \"Customer Put Order\"\n },\n \"Cancel Order\": {\n \"Type\": \"Pass\",\n \"Next\": \"DynamoDB Update Order Record\",\n \"Result\": {\n \"name\": \"Cancelled\",\n \"state\": \"CANCELLED\"\n },\n \"ResultPath\": \"$.event\"\n },\n \"DynamoDB Update Order Record\": {\n \"Type\": \"Task\",\n \"Resource\": \"arn:aws:states:::dynamodb:updateItem\",\n \"Parameters\": {\n \"TableName\": \"${OMTable}\",\n \"Key\": {\n \"PK\": {\n \"S\": \"orders\"\n },\n \"SK\": {\n \"S.$\": \"$.orderId\"\n }\n },\n \"UpdateExpression\": \"set #OS = :OS\",\n \"ExpressionAttributeNames\": {\n \"#OS\": \"ORDERSTATE\"\n },\n \"ExpressionAttributeValues\": {\n \":OS\": {\n \"S.$\": \"$.event.state\"\n }\n },\n \"ReturnValues\": \"ALL_NEW\"\n },\n \"ResultPath\": \"$.result\",\n \"Next\": \"Construct record (1)\",\n \"ResultSelector\": {\n \"Attributes.$\": \"$.Attributes\"\n }\n },\n \"Construct record (1)\": {\n \"Type\": \"Pass\",\n \"Next\": \"Emit Completed || Cancelled\",\n \"ResultPath\": \"$.detail\",\n \"Parameters\": {\n \"orderId.$\": \"$.orderId\",\n \"userId.$\": \"$.result.Attributes.USERID.S\",\n \"ORDERSTATE.$\": \"$.result.Attributes.ORDERSTATE.S\",\n \"Message\": \"Barrista has cancelled or completed the order\"\n }\n },\n \"Emit Completed || Cancelled\": {\n \"Type\": \"Task\",\n \"Resource\": \"arn:aws:states:::events:putEvents\",\n \"Parameters\": {\n \"Entries\": [\n {\n \"Detail.$\": \"States.JsonToString($.detail)\",\n \"DetailType.$\": \"States.Format('OrderManager.Order{}', $.event.name)\",\n \"EventBusName\": \"Serverlesspresso\",\n \"Source\": \"lstesting.serverlesspresso\",\n \"Time.$\": \"$$.State.EnteredTime\"\n }\n ]\n },\n \"Next\": \"Resume Order Processor 1\",\n \"ResultPath\": \"$.eventEmit\"\n },\n \"Complete Order\": {\n \"Type\": \"Pass\",\n \"Next\": \"DynamoDB Update Order Record\",\n \"Result\": {\n \"name\": \"Completed\",\n \"state\": \"COMPLETED\"\n },\n \"ResultPath\": \"$.event\"\n },\n \"Customer Put Order\": {\n \"Type\": \"Pass\",\n \"Next\": \"get menu\"\n },\n \"get menu\": {\n \"Type\": \"Task\",\n \"Resource\": \"arn:aws:states:::dynamodb:getItem\",\n \"Parameters\": {\n \"TableName\": \"${ConfigTable}\",\n \"Key\": {\n \"PK\": {\n \"S\": \"menu\"\n }\n }\n },\n \"Next\": \"Sanitize order\",\n \"ResultPath\": \"$.menu\"\n },\n \"Sanitize order\": {\n \"Type\": \"Task\",\n \"Resource\": \"arn:aws:states:::lambda:invoke\",\n \"Parameters\": {\n \"Payload.$\": \"$\",\n \"FunctionName\": \"${SanitizeOrderLambda}\"\n },\n \"Retry\": [\n {\n \"ErrorEquals\": [\n \"Lambda.ServiceException\",\n \"Lambda.AWSLambdaException\",\n \"Lambda.SdkClientException\"\n ],\n \"IntervalSeconds\": 2,\n \"MaxAttempts\": 6,\n \"BackoffRate\": 2\n }\n ],\n \"Next\": \"Is Order Valid?\",\n \"ResultPath\": \"$.sanitise\"\n },\n \"Is Order Valid?\": {\n \"Type\": \"Choice\",\n \"Choices\": [\n {\n \"Variable\": \"$.sanitise.Payload\",\n \"BooleanEquals\": false,\n \"Next\": \"not a valid order\"\n }\n ],\n \"Default\": \"Update order\"\n },\n \"not a valid order\": {\n \"Type\": \"Succeed\"\n },\n \"Update order\": {\n \"Type\": \"Task\",\n \"Resource\": \"arn:aws:states:::dynamodb:updateItem\",\n \"Parameters\": {\n \"TableName\": \"serverlesspresso-order-table\",\n \"Key\": {\n \"PK\": {\n \"S\": \"orders\"\n },\n \"SK\": {\n \"S.$\": \"$.orderId\"\n }\n },\n \"UpdateExpression\": \"set #drinkOrder = :drinkOrder\",\n \"ConditionExpression\": \"#userId = :userId AND attribute_exists(TaskToken)\",\n \"ExpressionAttributeNames\": {\n \"#drinkOrder\": \"drinkOrder\",\n \"#userId\": \"USERID\"\n },\n \"ExpressionAttributeValues\": {\n \":drinkOrder\": {\n \"S.$\": \"States.JsonToString($.body)\"\n },\n \":userId\": {\n \"S.$\": \"$.body.userId\"\n }\n },\n \"ReturnValues\": \"ALL_NEW\"\n },\n \"Next\": \"Resume Order Processor 2\",\n \"ResultSelector\": {\n \"TaskToken.$\": \"$.Attributes.TaskToken.S\"\n },\n \"OutputPath\": \"$.record\",\n \"ResultPath\": \"$.record\"\n },\n \"Resume Order Processor 1\": {\n \"Type\": \"Task\",\n \"Parameters\": {\n \"Output\": \"{}\",\n \"TaskToken.$\": \"$.result.Attributes.TaskToken.S\"\n },\n \"Resource\": \"arn:aws:states:::aws-sdk:sfn:sendTaskSuccess\",\n \"End\": true\n },\n \"Resume Order Processor 2\": {\n \"Type\": \"Task\",\n \"Parameters\": {\n \"Output\": \"{}\",\n \"TaskToken.$\": \"$.TaskToken\"\n },\n \"Resource\": \"arn:aws:states:::aws-sdk:sfn:sendTaskSuccess\",\n \"End\": true\n },\n \"Claim Order\": {\n \"Type\": \"Pass\",\n \"Next\": \"Make OR Unmake?\"\n },\n \"Make OR Unmake?\": {\n \"Type\": \"Choice\",\n \"Choices\": [\n {\n \"Variable\": \"$.action\",\n \"StringEquals\": \"unmake\",\n \"Next\": \"Unmake Order\"\n },\n {\n \"Variable\": \"$.action\",\n \"StringEquals\": \"make\",\n \"Next\": \"DynamoDB Update Order\"\n }\n ],\n \"Default\": \"DynamoDB Update Order\"\n },\n \"Unmake Order\": {\n \"Type\": \"Pass\",\n \"Parameters\": {\n \"baristaUserId\": \"\",\n \"orderId.$\": \"$.orderId\",\n \"Message\": \"The barista has pressed the 'UnMake order' button, this Invokes a Lambda function via API Gateway, which updates the order in DynamoDB and emits a new 'make order' Event.\"\n },\n \"Next\": \"DynamoDB Update Order\"\n },\n \"DynamoDB Update Order\": {\n \"Type\": \"Task\",\n \"Resource\": \"arn:aws:states:::dynamodb:updateItem\",\n \"Parameters\": {\n \"TableName\": \"serverlesspresso-order-table\",\n \"Key\": {\n \"PK\": {\n \"S\": \"orders\"\n },\n \"SK\": {\n \"S.$\": \"$.orderId\"\n }\n },\n \"UpdateExpression\": \"set #baristaUserId = :baristaUserId\",\n \"ExpressionAttributeNames\": {\n \"#baristaUserId\": \"baristaUserId\"\n },\n \"ExpressionAttributeValues\": {\n \":baristaUserId\": {\n \"S.$\": \"$.baristaUserId\"\n }\n },\n \"ReturnValues\": \"ALL_NEW\"\n },\n \"ResultSelector\": {\n \"Attributes.$\": \"$.Attributes\"\n },\n \"ResultPath\": \"$.result\",\n \"Next\": \"Construct record\"\n },\n \"Construct record\": {\n \"Type\": \"Pass\",\n \"Next\": \"EventBridge Emit Making Order\",\n \"ResultPath\": \"$.detail\",\n \"Parameters\": {\n \"baristaUserId.$\": \"$.result.Attributes.baristaUserId.S\",\n \"orderId.$\": \"$.orderId\",\n \"userId.$\": \"$.result.Attributes.USERID.S\",\n \"Message\": \"The barista has pressed the 'Make order' button, this Invokes a Lambda function via API Gateway, which updates the order in DynamoDB and emits a new 'make order' Event.\"\n }\n },\n \"EventBridge Emit Making Order\": {\n \"Type\": \"Task\",\n \"Resource\": \"arn:aws:states:::events:putEvents\",\n \"Parameters\": {\n \"Entries\": [\n {\n \"Detail.$\": \"States.JsonToString($.detail)\",\n \"DetailType\": \"OrderManager.MakeOrder\",\n \"EventBusName\": \"Serverlesspresso\",\n \"Source\": \"lstesting.serverlesspresso\",\n \"Time.$\": \"$$.State.EnteredTime\"\n }\n ]\n },\n \"End\": true\n }\n }\n}\n", |
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.
style: State machine definition is embedded as a string. Consider using a separate JSON file for improved readability and maintenance
default: | ||
statusCode: "200" | ||
responseParameters: | ||
method.response.header.Access-Control-Allow-Methods: "'GET,OPTIONS'" |
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.
logic: Access-Control-Allow-Methods should include 'PUT' instead of 'GET' for the store endpoint
application/json: "\n#set($state = $input.params('state'))\n#if( $input.params('maxItems').toString()\ | ||
\ != \"\" )\n #set($maxItems = $input.params('maxItems'))\n#else\n #set($maxItems\ | ||
\ = 100)\n#end\n\n\n{\n \"TableName\": \"serverlesspresso-order-table\"\ | ||
,\n \"IndexName\": \"GSI-status\",\n \"KeyConditionExpression\"\ | ||
: \"#ORDERSTATE = :ORDERSTATE\",\n \"ExpressionAttributeNames\": {\n\ | ||
\ \"#ORDERSTATE\": \"ORDERSTATE\"\n },\n \"ExpressionAttributeValues\"\ | ||
: {\n \":ORDERSTATE\": {\n \"S\": \"$state\"\n }\n },\n\ | ||
\ \"ScanIndexForward\": true,\n \"Limit\": $maxItems,\n \"ProjectionExpression\"\ | ||
: \"PK, SK, orderNumber, robot, drinkOrder, ORDERSTATE, TS, userId, baristaUserId\"\ | ||
\n}" |
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.
style: Consider using a more descriptive name for the DynamoDB table, such as 'serverlesspresso-orders' instead of 'serverlesspresso-order-table'
application/json: "#set($sub = $input.params('sub'))\n#set($subFromJWT =\ | ||
\ $context.authorizer.claims.sub)\n\n{\n \"TableName\": \"serverlesspresso-order-table\"\ | ||
,\n \"IndexName\": \"GSI-userId\",\n \"KeyConditionExpression\"\ | ||
: \"#USERID = :USERID\",\n \"ExpressionAttributeNames\": {\n \"\ | ||
#USERID\": \"USERID\"\n },\n \"ExpressionAttributeValues\": {\n\ | ||
\ \":USERID\": {\n \"S\": \"$subFromJWT\"\n }\n },\n \ | ||
\ \"ScanIndexForward\": true,\n \"ProjectionExpression\": \"PK, SK,\ | ||
\ orderNumber, robot, drinkOrder, ORDERSTATE, TS\"\n}" |
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.
style: The 'sub' parameter is set but not used. Consider removing it if it's unnecessary
source = SERVERLESSPRESSO_SOURCE # Event bus source by application | ||
max_concurrent_capacity = "5" |
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.
style: Consider using an integer instead of a string for max_concurrent_capacity
cdk.aws_iam.PolicyStatement( | ||
actions=["iot:DescribeEndpoint"], resources=["*"], effect=cdk.aws_iam.Effect.ALLOW |
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.
style: Broad IAM permission. Consider restricting to specific resources if possible
}, | ||
timeout=cdk.Duration.seconds(15), | ||
) | ||
validator_table.grant_full_access(get_qr_code_fn) |
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.
style: Granting full access to the Lambda function may be overly permissive. Consider using least privilege principle
default_cors_preflight_options=cdk.aws_apigateway.CorsOptions( | ||
allow_origins=["*"], allow_headers=["*"], allow_methods=["GET", "POST", "OPTIONS"] | ||
), |
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.
style: CORS is set to allow all origins (*). This may pose security risks in production
] | ||
aws_client.events.put_events(Entries=initial_events) | ||
# all 5 should be in "Emit - Workflow Started TT" state waiting for the callback with a token | ||
time.sleep(2) # TODO: instead just check if all 5 are in a waiting state |
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.
style: Replace sleep with a more robust check for waiting state
Motivation
As part of our scenario testing exploration, we also wanted to look into migrating bigger samples to our CDK-based setup and add multiple test scenarios on that infrastructure.
Changes
Testing
Note that the test currently only works against AWS.
TODO
What's left to do: