Skip to content

HDDS-14847. [STS] Expose ExpiredToken Error#9935

Merged
sodonnel merged 4 commits into
apache:HDDS-13323-stsfrom
fmorg-git:HDDS-14847
Apr 14, 2026
Merged

HDDS-14847. [STS] Expose ExpiredToken Error#9935
sodonnel merged 4 commits into
apache:HDDS-13323-stsfrom
fmorg-git:HDDS-14847

Conversation

@fmorg-git
Copy link
Copy Markdown
Contributor

@fmorg-git fmorg-git commented Mar 17, 2026

Please describe your PR in detail:

  • Currently, when an STS token expires and it is attempted to be used, an AccessDenied error occurs. When testing with AWS, it generates an ExpiredToken error code with the token in the body, so this ticket updates the implementation to have a similar response.
  • Separately, while debugging other issues, it was found that while iterating in BucketEndpoint, if an acl check gave PermissionDenied, a RuntimeException in OzoneBucket$KeyIterator.hasNext() which was not caught and bubbled up to the end user as an Internal Server Error http code 500. So a commit is made here to catch the RuntimeException and if it is of type OMException, then handle it the same way the code that handles expired token does.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-14847

How was this patch tested?

manual test waiting for expiration, unit tests

@fmorg-git fmorg-git marked this pull request as ready for review April 1, 2026 18:12
@Russole
Copy link
Copy Markdown
Contributor

Russole commented Apr 4, 2026

Thanks @fmorg-git for the patch. Overall, this looks good to me, but I have a few questions.

@fmorg-git
Copy link
Copy Markdown
Contributor Author

Thanks @fmorg-git for the patch. Overall, this looks good to me, but I have a few questions.

hi @Russole - what questions do you have? I didn't see them on the comment.

Comment on lines +259 to +262
if (ex.getCause() instanceof OMException) {
final OMException omException = (OMException) ex.getCause();
handleOMException(omException, bucketName, prefix);
} else {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment for ResultCodes.FILE_NOT_FOUND says "continue and send normal response with 0 keyCount", which makes sense for the outer case (before iteration starts).
However, in the new iterator-side runtime path, FILE_NOT_FOUND will stop the loop and return the current response.
So the semantics seem different. Should iterator-side FILE_NOT_FOUND really be treated the same way?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch - updated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @Russole - I think your comment might have been reversed. Before this PR, the inner loop would have thrown on FILE_NOT_FOUND but my initial commit would have swallowed it. So I excluded the FILE_NOT_FOUND case so it will behave the same as before and still handle the AccessDenied and ExpiredToken cases.

@Russole
Copy link
Copy Markdown
Contributor

Russole commented Apr 5, 2026

Hi @fmorg-git , I've submitted my pending review comments.

@fmorg-git fmorg-git requested a review from Russole April 7, 2026 19:34
Copy link
Copy Markdown
Contributor

@sodonnel sodonnel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sodonnel sodonnel merged commit 9b3577e into apache:HDDS-13323-sts Apr 14, 2026
88 of 89 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants