CLDSRV-893: Return ARN for assumed-role requesters in access logs#6152
Conversation
Hello dvasilas,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
... and 1 file with indirect coverage changes @@ Coverage Diff @@
## development/9.2 #6152 +/- ##
===================================================
- Coverage 84.34% 84.29% -0.05%
===================================================
Files 204 204
Lines 13143 13147 +4
===================================================
- Hits 11085 11082 -3
- Misses 2058 2065 +7
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command: Alternatively, the |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
LGTM — clean, well-scoped fix with good test coverage. One minor nit posted inline (double |
| return `REST.${req.method}.${resourceType}`; | ||
| } | ||
|
|
||
| const assumedRoleArnRegex = /^arn:aws:sts::[0-9]{12}:assumed-role\/.*$/; |
There was a problem hiding this comment.
You may want to be more restrictive in the last part after / to ensure there is no space or invalid character that would break the log formatting (not that it is likely to happen, but just in case)
There was a problem hiding this comment.
Makes sense, I went with \S+
The getRequester() function fell through to the canonical ID for assumed-role sessions, producing a space-containing string that broke space-delimited log parsers.
dvasilas
force-pushed
the
improvement/CLDSRV-893-return-arn-for-assumed-role-in-access-logs
branch
from
7a84f96 to
46a99ae
Compare
|
LGTM |
|
/create_integration_branches |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option The following options are set: create_integration_branches |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following options are set: create_integration_branches |
|
/approve |
Build failedThe build for commit did not succeed in branch w/9.3/improvement/CLDSRV-893-return-arn-for-assumed-role-in-access-logs The following options are set: approve, create_integration_branches |
|
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue CLDSRV-893. Goodbye dvasilas. The following options are set: approve, create_integration_branches |
bert-e
deleted the
improvement/CLDSRV-893-return-arn-for-assumed-role-in-access-logs
branch
5 participants
For assumed-role sessions, the requester field in server access logs currently contains spaces (e.g.
[assumedRole] lifecycle-role:backbeat-lifecycle:scality-internal-services). Since the log format is space-delimited and this field is unquoted, the space shifts all subsequent fields for log parsers.Per the AWS S3 Server Access Log Format, the Requester field for assumed-role sessions should be an ARN:
Fixes: S3C-11117