APIGW: add CRUD support for Stage.AccessLogSettings#13849
Open
APIGW: add CRUD support for Stage.AccessLogSettings#13849
Conversation
Test Results - Alternative Providers573 tests 312 ✅ 17m 45s ⏱️ Results for commit 3f5b7e2. |
Test Results (amd64) - Integration, Bootstrap 5 files 5 suites 2h 41m 40s ⏱️ Results for commit 3f5b7e2. |
simonrw
approved these changes
Feb 26, 2026
Contributor
simonrw
left a comment
There was a problem hiding this comment.
I have only reviewed the CFn changes, but they seem sound. Thanks!
|
|
||
| # TODO: add methodSettings with the same principle | ||
| patch_operations = [] | ||
| if access_log_settings := model.get("AccessLogSetting"): |
cloutierMat
approved these changes
Feb 26, 2026
Member
cloutierMat
left a comment
There was a problem hiding this comment.
Thanks for adding support for the access log settings. Hopefully, we get time to add integration with CW 🙏
Really nice and thorough testing of the update method and kudos on thinking ahead and ensuring CFN support! 🚀
Comment on lines
+490
to
+499
| # See https://docs.aws.amazon.com/apigateway/latest/api/patch-operations.html#UpdateStage-Patch | ||
| # not everything is right on the table, for example no path supports `remove` accept the root path | ||
| # TODO: validate destinationArn is a valid ARN (does not have to validate it exists) | ||
| valid_paths = ["/accessLogSettings/destinationArn", "/accessLogSettings/format"] | ||
| if op == "remove": | ||
| if path != "/accessLogSettings": | ||
| stripped_path = path.removeprefix("/") | ||
| raise BadRequestException( | ||
| f"Cannot remove method setting {stripped_path} because there is no method setting for this method " | ||
| ) |
|
|
||
| class TestApiGatewayStage: | ||
| @pytest.fixture | ||
| def create_api_for_deployment(self, apigw_create_rest_api, aws_client): |
Member
There was a problem hiding this comment.
Thanks for creating this small fixture! makes reading the test much easier! 👏
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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
We got a request to add CRUD support for
AccessLogSettingsin APIGW. This PR adds support forUpdateStageand for theAccessLogSettingsin CFNSee https://docs.aws.amazon.com/apigateway/latest/developerguide/set-up-logging.html
Funny enough, you cannot specify that in
CreateStage, it has to go through update.This PR does not add support for properly sending the logs to CloudWatch or Firehose.
Changes
AccessLogSettings, there was partial support in Moto so it only needed a bit of validation and adaptation to what we send to itAWS::ApiGateway::Stageresource to properly update the Stage with the valuesTests
Related