Skip to content

rgw: add wildcard "*" support for conditional read#64428

Merged
anrao19 merged 1 commit intoceph:mainfrom
sungjoon-koh:add-wildcard-support-cond-read
Sep 17, 2025
Merged

rgw: add wildcard "*" support for conditional read#64428
anrao19 merged 1 commit intoceph:mainfrom
sungjoon-koh:add-wildcard-support-cond-read

Conversation

@sungjoon-koh
Copy link
Copy Markdown
Contributor

@sungjoon-koh sungjoon-koh commented Jul 10, 2025 •
edited
Loading

  • Enhanced If-Match and If-None-Match headers to support the wildcard "*".
  • Updated conditional read behavior:
    • If-Match: * now returns 200 instead of 412 when any object exists.
    • If-None-Match: * now returns 304 instead of 200 when any object exists.
    • If-Match: * + If-None-Match: * now returns 304 instead of 412.
  • Aligns with AWS S3 expected functionality for improved efficiency.

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands

@sungjoon-koh sungjoon-koh requested a review from a team as a code owner July 10, 2025 07:17
@github-actions github-actions bot added the rgw label Jul 10, 2025
@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Jul 10, 2025

@cbodley Hello, could you please check this PR?

Also I would really appreciate if you could share if some other works(ex. document update or creating issue ticket) need to be done.

@sungjoon-koh sungjoon-koh force-pushed the add-wildcard-support-cond-read branch 2 times, most recently from 248a961 to baaed21 Compare July 10, 2025 07:26
@sungjoon-koh sungjoon-koh changed the title Add wildcard "*" support for S3 conditional reads raw: add wildcard "*" support for conditional reads Jul 10, 2025
@sungjoon-koh sungjoon-koh force-pushed the add-wildcard-support-cond-read branch from baaed21 to 9212290 Compare July 10, 2025 07:26
@sungjoon-koh sungjoon-koh changed the title raw: add wildcard "*" support for conditional reads raw: add wildcard "*" support for conditional read Jul 10, 2025
@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Jul 10, 2025 •
edited
Loading

To test:

fallocate test --len 10MB
aws s3api create-bucket --bucket test
aws s3api put-object --bucket test --key test --body test

# 412 -> 200
aws s3api get-object --bucket test --key test --if-match "*" test-res

# 200 -> 304
aws s3api get-object --bucket test --key test --if-none-match "*" test-res

# 412 -> 304
aws s3api get-object --bucket test --key test --if-none-match "*" --if-match "*" test-res

@cbodley cbodley requested a review from AliMasarweh July 10, 2025 14:32
@adamemerson adamemerson changed the title raw: add wildcard "*" support for conditional read rgw: add wildcard "*" support for conditional read Jul 14, 2025
AliMasarweh
AliMasarweh approved these changes Jul 15, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@AliMasarweh AliMasarweh left a comment

Choose a reason for hiding this comment

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

LGTM

@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Jul 28, 2025

@AliMasarweh Thank you for the review! Do I need to do something before merging this commit?

@AliMasarweh
Copy link
Copy Markdown
Member

AliMasarweh commented Jul 28, 2025 •
edited
Loading

@AliMasarweh Thank you for the review! Do I need to do something before merging this commit?

you are welcome :)
you need to run a teuthology run for suite rgw, have you done that?

@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Jul 30, 2025 •
edited
Loading

@AliMasarweh Thank you for the reply. However, I'm a beginner contributor to Ceph and I'm trying to figure out how to run Teuthology tests. I've read that I can run them in the Sepia lab, is that the right way to do it? If so, could you please provide me with access? Thanks!

I filed a ticket.
https://tracker.ceph.com/issues/72332

@AliMasarweh
Copy link
Copy Markdown
Member

AliMasarweh commented Jul 30, 2025

@AliMasarweh Thank you for the reply. However, I'm a beginner contributor to Ceph and I'm trying to figure out how to run Teuthology tests. I've read that I can run them in the Sepia lab, is that the right way to do it? If so, could you please provide me with access? Thanks!

I filed a ticket. https://tracker.ceph.com/issues/72332

I am not responsible for providing access to people, but I will ask someone who can provide you access

@cbodley
Copy link
Copy Markdown
Contributor

cbodley commented Jul 30, 2025

To test:

fallocate test --len 10MB
aws s3api create-bucket --bucket test
aws s3api put-object --bucket test --key test --body test

# 412 -> 200
aws s3api get-object --bucket test --key test --if-match "*" test-res

# 200 -> 304
aws s3api get-object --bucket test --key test --if-none-match "*" test-res

# 412 -> 304
aws s3api get-object --bucket test --key test --if-none-match "*" --if-match "*" test-res

thanks @sungjoon-koh. it would be great if you could turn this into test cases in s3-tests so we can validate this pr

@sungjoon-koh sungjoon-koh force-pushed the add-wildcard-support-cond-read branch from 9212290 to 71d21b1 Compare August 9, 2025 17:42
@sungjoon-koh sungjoon-koh mentioned this pull request Aug 9, 2025
Open
@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Aug 9, 2025

s3 tests for this pr: ceph/s3-tests#681

@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Aug 17, 2025

Many test cases seem to be failing, but most of them appear unrelated to my modifications.
Is there any consensus regarding these test failures? Or, am I missing some configurations?

https://pulpito.ceph.com/sungjoon_koh-2025-08-15_01:04:30-rgw-wip-sungjoon-250815-distro-default-smithi/

@AliMasarweh
Copy link
Copy Markdown
Member

AliMasarweh commented Aug 17, 2025

Many test cases seem to be failing, but most of them appear unrelated to my modifications. Is there any consensus regarding these test failures? Or, am I missing some configurations?

https://pulpito.ceph.com/sungjoon_koh-2025-08-15_01:04:30-rgw-wip-sungjoon-250815-distro-default-smithi/

Lots of failures on test test_get_account_summary_root
I don't see any open tracker regarding this failure
however it doesn't look like something that your code caused
so it might be that you need to rebase your build to the latest branch

@sungjoon-koh sungjoon-koh force-pushed the add-wildcard-support-cond-read branch from 71d21b1 to e96f968 Compare August 18, 2025 00:36
@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Aug 18, 2025

Many test cases seem to be failing, but most of them appear unrelated to my modifications. Is there any consensus regarding these test failures? Or, am I missing some configurations?
https://pulpito.ceph.com/sungjoon_koh-2025-08-15_01:04:30-rgw-wip-sungjoon-250815-distro-default-smithi/

Lots of failures on test test_get_account_summary_root I don't see any open tracker regarding this failure however it doesn't look like something that your code caused so it might be that you need to rebase your build to the latest branch

Thank you for checking. I will rebase and check it again.

@sungjoon-koh sungjoon-koh force-pushed the add-wildcard-support-cond-read branch from e96f968 to 6e28ac3 Compare August 18, 2025 18:02
@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Aug 19, 2025

jenkins test make check

@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Aug 19, 2025

jenkins test make check arm64

@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Aug 19, 2025

jenkins test windows

@sungjoon-koh
rgw: add wildcard "*" support for conditional read
beba28f 
- Enhanced If-Match and If-None-Match headers to support the wildcard "*".
- Updated conditional read behavior:
  - If-Match: * now returns 200 instead of 412 when any object exists.
  - If-None-Match: * now returns 304 instead of 200 when any object exists.
  - If-Match: * + If-None-Match: * now returns 304 instead of 412.
- Aligns with AWS S3 expected functionality for improved efficiency.

Signed-off-by: sungjoon_koh <sungjoon_koh@linecorp.com>
@sungjoon-koh sungjoon-koh force-pushed the add-wildcard-support-cond-read branch from 6e28ac3 to beba28f Compare August 19, 2025 15:52
@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Aug 20, 2025

jenkins test make check

@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Aug 20, 2025

jenkins test make check arm64

@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Aug 28, 2025

@cbodley

Ali Masarweh suggested I reach out to you for confirmation regarding some RGW teuthology test results.
I noticed some test failures in the testing environment, even though the same tests pass locally.

After discussing with Ali, he mentioned that:

  1. RGW suite typically has around 15 expected failures that are constant on main and unrelated to PR changes
  2. The specific failure I was concerned about (db-store related) is an expected failure with an existing tracker
  3. He believes my teuthology run is actually passing overall
    Here's the link to my test results for reference:
    https://pulpito.ceph.com/sungjoon_koh-2025-08-21_15:22:48-rgw-wip-sungjoon-conditional-read-distro-default-smithi/

Could you please help confirm whether these results look acceptable for my PR?
I want to make sure I'm not missing any critical issues before proceeding.

Thank you.

@anrao19
Copy link
Copy Markdown
Contributor

anrao19 commented Sep 16, 2025

pr testing completed : https://tracker.ceph.com/issues/73008 and got approved by @ivancich
@sungjoon-koh post pr approval if no further testing is need then this pr could be merged

@sungjoon-koh
Copy link
Copy Markdown
Contributor Author

sungjoon-koh commented Sep 16, 2025

@anrao19 Thank you for the test! Who do I need to ask for PR approval?

@anrao19
Copy link
Copy Markdown
Contributor

anrao19 commented Sep 16, 2025

@anrao19 Thank you for the test! Who do I need to ask for PR approval?

I have asked for review, will wait for some time post approval we can merge

@anrao19 anrao19 merged commit 0f53297 into ceph:main Sep 17, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AliMasarweh AliMasarweh AliMasarweh approved these changes

Assignees

No one assigned

Labels

needs-qa ready-to-merge rgw TESTED

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@sungjoon-koh @AliMasarweh @cbodley @anrao19 @adamemerson