Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[v23.3.x] cloud_storage: correct list_object() request headers and parameters (manual backport) #18447

Merged
merged 8 commits into from
May 13, 2024
Merged

[v23.3.x] cloud_storage: correct list_object() request headers and parameters (manual backport) #18447

merged 8 commits into from
May 13, 2024

Commits on May 13, 2024

  1. cloud_storage: respect max_keys in s3_client::list_objects() 

    Previously, the `max_keys` value was erroneously added to the header
of a `list_objects` request in the `s3_client`. This is, in fact,
a URI request parameter, not a request header.

See:
 * https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjects.html
 * https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjectsV2.html

If `max_keys` is specified in a call to `remote::list_objects()`,
the onus is now on the user to deal with a possibly truncated value
at the call site, likely in a while loop.

The idea is that the user will be able to check for this case using
`list_result.is_truncated`, and then pass `list_result.next_continuation_token`
to `remote::list_objects()` in future requests.

(cherry picked from commit 39c48f4)
    @WillemKauf
    WillemKauf committed May 13, 2024
    Configuration menu
    Copy the full SHA
    d46264e View commit details
    Browse the repository at this point in the history

  2. cloud_storage: add continuation_token parameter 

    This commit adds an optional `continuation_token` parameter to
`remote::list_objects()` in order to support future work around
re-factoring of the `list_objects()` remote API.

This allows users to handle results that are truncated at the call site
to `list_objects()`. The `remote::list_result` will have `is_truncated`
set to true, and `next_continuation_token` set appropriately. To allow use
of this `next_continuation_token` in future requests, we have to expose
it as a parameter in `list_objects()`.

(cherry picked from commit eae9f43)
    @WillemKauf
    WillemKauf committed May 13, 2024
    Configuration menu
    Copy the full SHA
    82f52ee View commit details
    Browse the repository at this point in the history

  3. cloud_storage: fix s3 request parameters 

    In `ListObjectsV2` for the `s3_client`, `prefix`, `delimiter`,
`start_after`, and `continuation-token` are all URI parameters,
not request headers.

(cherry picked from commit 15e3ec6)
    @WillemKauf
    WillemKauf committed May 13, 2024
    Configuration menu
    Copy the full SHA
    267a202 View commit details
    Browse the repository at this point in the history

  4. cloud_storage: correct s3_client_test header usage 

    There are a few places in our cloud storage testing where
we previously expected things like `prefix` and `continuation-token`
in the headers of a `list_objects` request, despite the fact S3 expects
these as URI parameters.

This commit corrects those checks by query parameters.

(cherry picked from commit 745eb7c)
    @WillemKauf
    WillemKauf committed May 13, 2024
    Configuration menu
    Copy the full SHA
    d38b8a6 View commit details
    Browse the repository at this point in the history

  5. cloud_storage: fix abs URI parameters 

    The `continuation-token` (called `marker` in ABS) was not being passed
into `abs_request_creator::make_list_blob_request()`, despite being
properly set in `remote::list_objects()`.

This seems like dangerous behavior, as the `while()` loop
will continue to issue `list_objects()` requests to the `abs_client`
sans `marker`.

The `continuation-token` (`marker`) is now being passed to the request
creator and respected in the `abs_client`.

Also fixes an existing bug where `max_results` should be `maxresults`
in the URI parameters.

Also renames parameters to reflect ABS API naming.

See:
  * https://learn.microsoft.com/en-us/rest/api/storageservices/list-blobs#uri-parameters
(cherry picked from commit e6e595c)
    @WillemKauf
    WillemKauf committed May 13, 2024
    Configuration menu
    Copy the full SHA
    6235666 View commit details
    Browse the repository at this point in the history

  6. cloud_storage: correct list_objects_resp behavior 

    Fixes behavior with `max_keys` and `continuation_token` within
`list_objects_resp()`. Fixture tests that use the `s3_imposter`
for requests can now expect proper behavior around these parameters.

(cherry picked from commit 96961f7)
    @WillemKauf
    WillemKauf committed May 13, 2024
    Configuration menu
    Copy the full SHA
    6c01b0e View commit details
    Browse the repository at this point in the history

  7. cloud_storage: default is_truncated to false 

    To avoid undefined behavior due to accessing uninitialized memory,
default construct `list_bucket_result::is_truncated` with `false`.

(cherry picked from commit 13ecfd3)
    @WillemKauf
    WillemKauf committed May 13, 2024
    Configuration menu
    Copy the full SHA
    9be9d5b View commit details
    Browse the repository at this point in the history

  8. cloud_storage: add test_list_bucket_with_max_keys 

    (cherry picked from commit 699176e)
    @WillemKauf
    WillemKauf committed May 13, 2024
    Configuration menu
    Copy the full SHA
    70efcad View commit details
    Browse the repository at this point in the history