fix: reduce AWS STS inline policy size by merging read+write actions#4228
Open
yushesp wants to merge 1 commit intoapache:mainfrom
Open
fix: reduce AWS STS inline policy size by merging read+write actions#4228yushesp wants to merge 1 commit intoapache:mainfrom
yushesp wants to merge 1 commit intoapache:mainfrom
Conversation
Writable locations previously appeared in two separate statements:
1. Read statement covering every location
2. Write statement covering only writable ones.
This listed the resource ARN twice for every writable location,
which pushed deeply nested namespace paths past the 2048-character
STS policy limit.
This change emits a single combined statement granting both read and write actions
for writable locations, and keep the read-only statement only for
locations not already covered.
Also wraps the assumeRole call to rewrap PackedPolicyTooLargeException
with a descriptive error msg.
Example:
Before (2 statements, ARN duplicated):
{"Action": ["s3:GetObject", "s3:GetObjectVersion"],
"Resource": ["...bucket/path/*"]}
{"Action": ["s3:PutObject", "s3:DeleteObject"],
"Resource": ["...bucket/path/*"]}
After (1 statement):
{"Action": ["s3:GetObject", "s3:GetObjectVersion",
"s3:PutObject", "s3:DeleteObject"],
"Resource": ["...bucket/path/*"]}
Fixes apache#3243
jbonofre
approved these changes
Apr 17, 2026
| try { | ||
| response = stsClient.assumeRole(request.build()); | ||
| } catch (PackedPolicyTooLargeException e) { | ||
| throw new RuntimeException( |
Member
There was a problem hiding this comment.
To be honest, I would expect some more specific than RuntimeException here but as the rest of the storage layer is also using RuntimeException, it's consistent and make sense 😄
| response = stsClient.assumeRole(request.build()); | ||
| } catch (PackedPolicyTooLargeException e) { | ||
| throw new RuntimeException( | ||
| "AWS STS rejected the session policy: it exceeds the 2048-character packed-policy" |
Member
There was a problem hiding this comment.
I was confused when I saw the corresponding testDeepNestedNamespaceStaysWithinPackedPolicyLimit as the assert is on the raw JSON length, not the packed size.
But it's actually correct. I was misleaded by the comment and naming.
github-project-automation
bot
moved this from PRs In Progress
to Ready to merge
in Basic Kanban Board
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
2 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.
Writable locations previously appeared in two separate statements:
This listed the resource ARN twice for every writable location, which pushed deeply nested namespace paths past the 2048-character STS policy limit.
This change emits a single combined statement granting both read and write actions for writable locations, and keep the read-only statement only for locations not already covered.
Also wraps the assumeRole call to rewrap
PackedPolicyTooLargeExceptionwith a descriptive error msg.Example:
Before (2 statements, ARN duplicated):
After (1 statement):
Fixes #3243
Checklist
CHANGELOG.md(if needed)site/content/in-dev/unreleased(if needed)