drop ceph/radosgw backend support and block ceph-style endpoints#2614
drop ceph/radosgw backend support and block ceph-style endpoints#2614bert-e merged 2 commits intodevelopment/8.4from
Conversation
Hello benzekrimaha,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Review by Claude Code |
In this commit we : removed explicit Ceph location type handling from location patching logic removed CI_CEPH path-style override behavior removed Ceph-specific unit test coverage added defensive validation to reject Ceph/RadosGW-style endpoints even when configured through generic S3 location types added unit tests validating endpoint rejection for ceph and radosgw patterns Issue: ARSN-573
b2fb8c1 to
1504695
Compare
|
LGTM |
|
LGTM |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development/8.4 #2614 +/- ##
================================================
Coverage 73.47% 73.48%
================================================
Files 222 222
Lines 18177 18183 +6
Branches 3762 3763 +1
================================================
+ Hits 13356 13361 +5
- Misses 4816 4817 +1
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| supportsVersioning = true; // fallthrough | ||
| case 'location-do-spaces-v1': | ||
| if (isUnsupportedCephEndpoint(l.details?.endpoint)) { | ||
| log.warn('unsupported ceph endpoint for location type', { |
There was a problem hiding this comment.
Maybe adding the word "deprecated" inside would provide more context
SylvainSenechal
left a comment
There was a problem hiding this comment.
Can you explain a bit the pr :
in zenko when we removed ceph, we just completely removed everything.
Here it seems that instead of just removing any reference to ceph, we add tests that will assert that ceph is not supported ? its kinda weird because later someone might read this and wonder why would we do this for ceph but not for any other s3 backend ?
I mean, i guess the pr is ok but just lacking context, like one of the location (location-do-spaces-v1) is just removed from the switch statement, while the location-do-spaces-v1 is not removed bu gets an extra isUnsupported ceph endpoint logic
Maybe there is some coupling with other codebase because it's Arsenal, forcing us to do this ?
|
LGTM |
|
/approve |
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve |
|
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue ARSN-573. Goodbye benzekrimaha. |
Issue: ARSN-573