APIGW: add Canary Deployment logic in invocation layer #12695
Conversation
LocalStack Community integration with Pro 2 files 2 suites 18m 24s ⏱️ Results for commit 1239e1b. ♻️ This comment has been updated with latest results. |
Test Results - Alternative Providers977 tests 547 ✅ 21m 46s ⏱️ Results for commit 1239e1b. ♻️ This comment has been updated with latest results. |
Test Results (amd64) - Integration, Bootstrap 5 files 5 suites 2h 21m 34s ⏱️ Results for commit 1239e1b. ♻️ This comment has been updated with latest results. |
4010c7c to
1239e1b
Compare
| @@ -622,7 +622,7 @@ def update_integration_response( | |||
| param = param.replace("~1", "/") | |||
| if op == "remove": | |||
| integration_response.response_templates.pop(param) | |||
| elif op == "add": | |||
| elif op in ("add", "replace"): | |||
There was a problem hiding this comment.
question: What is this fixing? Is it a leftover from the previous pr?
There was a problem hiding this comment.
Somewhat, this is because now that the test for the actual logic has been enabled, the test couldn't work because the update_integration_response method was not properly updating...
Here in TestCanaryDeployments.test_invoking_canary_deployment:
aws_client.apigateway.update_integration_response(
restApiId=api_id,
resourceId=resource_id,
httpMethod="GET",
statusCode="200",
patchOperations=[
{
"op": "replace",
"path": "/responseTemplates/application~1json",
"value": json.dumps(
{
"statusCode": 200,
"message": "canary deployment",
"variable": "$stageVariables.testVar",
"nonExistingDefault": "$stageVariables.noStageVar",
"nonOverridden": "$stageVariables.defaultVar",
"isCanary": "$context.isCanaryRequest",
}
),
}
],
)So I just made that work for now 😅
Motivation
Follow-up from #12694
This is the actual implementation of canary dispatching between deployments.
Not a lot to add, the code is pretty descriptive. I wasn't sure where to add the logic to "select" a canary deployment, but I think getting this in the router somewhat makes sense, as it is where we are deciding which deployment to use. One of the limitation is that we only know the account id and region from the active deployment itself.
We could also add a handler first in the chain to fetch the stage config and overwrite the deployment?
Also added a small fix in
UpdateIntegrationResponse, this will be a good revamp at some point 😅\cc @HarshCasper, I know you've been wanting this for a long time now!
Changes