[CORE-15023] Cloud Storage Clients: Add util::find_multipart_boundary #29166

oleiman · 2026-01-07T08:09:29Z

Find the multipart boundary string in a response's content-type header. This code is identical between ABS and GCS versions. Includes unit tests.

PR also improves surrounding unit tests slightly w/ use of gmock matchers.

Backports Required

Release Notes

none

Copilot

Pull request overview

This PR extracts common multipart boundary parsing logic into a new utility function find_multipart_boundary() to reduce code duplication between Azure Blob Storage (ABS) and Google Cloud Storage (GCS) implementations. The function parses the Content-Type header to extract the boundary string from multipart/mixed responses.

Key Changes

Added util::find_multipart_boundary() function to parse multipart boundary strings from HTTP response headers
Updated existing unit tests to use gmock matchers (testing::HasSubstr) instead of manual string contains checks
Added comprehensive unit tests for the new boundary parsing function covering valid cases and error conditions

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/v/cloud_storage_clients/util.h	Declares new `find_multipart_boundary()` function
src/v/cloud_storage_clients/util.cc	Implements boundary parsing with proper error handling for missing headers, incorrect content types, and malformed boundaries
src/v/cloud_storage_clients/tests/util_test.cc	Refactors existing tests to use gmock matchers and adds new test cases for boundary parsing

src/v/cloud_storage_clients/util.cc

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

src/v/cloud_storage_clients/util.cc

src/v/cloud_storage_clients/tests/util_test.cc

This is well-specified and identical between ABS and GCS, so factor it out for easier testing. Signed-off-by: Oren Leiman <[email protected]>

Signed-off-by: Oren Leiman <[email protected]>

vbotbuildovich · 2026-01-07T10:15:13Z

Retry command for Build#78637

please wait until all jobs are finished before running the slash command

/ci-repeat 1
skip-redpanda-build
skip-units
skip-rebase
tests/rptest/tests/compatibility/java_compression_test.py::JavaCompressionTest.test_upgrade_java_compression@{"compression_type":"snappy"}
tests/rptest/tests/write_caching_fi_e2e_test.py::WriteCachingFailureInjectionE2ETest.test_crash_all@{"use_transactions":false}

vbotbuildovich · 2026-01-07T10:36:55Z

CI test results

test results on build#78637

test_class	test_method	test_arguments	test_kind	job_url	test_status	passed	reason	test_history
JavaCompressionTest	test_upgrade_java_compression	{"compression_type": "snappy"}	integration	https://buildkite.com/redpanda/redpanda/builds/78637#019b97c8-c386-43de-b6d9-c31d783c365a	FLAKY	9/11	Test FAILS after retries.Significant increase in flaky rate(baseline=0.0000, p0=0.0000, reject_threshold=0.0100)	https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=JavaCompressionTest&test_method=test_upgrade_java_compression
DatalakeDLQTest	test_dlq_table_for_mixed_records	{"catalog_type": "rest_jdbc", "cloud_storage_type": 1, "query_engine": "spark"}	integration	https://buildkite.com/redpanda/redpanda/builds/78637#019b97c7-d8f9-4dc5-bf78-af75ffab0765	FLAKY	10/11	Test PASSES after retries.No significant increase in flaky rate(baseline=0.0025, p0=1.0000, reject_threshold=0.0100. adj_baseline=0.1000, p1=0.3487, trust_threshold=0.5000)	https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=DatalakeDLQTest&test_method=test_dlq_table_for_mixed_records
MountUnmountIcebergTest	test_simple_remount	{"cloud_storage_type": 1}	integration	https://buildkite.com/redpanda/redpanda/builds/78637#019b97c8-c389-4368-b7b7-e4e6c8f1205b	FLAKY	7/11	Test PASSES after retries.No significant increase in flaky rate(baseline=0.2093, p0=0.3504, reject_threshold=0.0100. adj_baseline=0.5057, p1=0.1628, trust_threshold=0.5000)	https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=MountUnmountIcebergTest&test_method=test_simple_remount
WriteCachingFailureInjectionE2ETest	test_crash_all	{"use_transactions": false}	integration	https://buildkite.com/redpanda/redpanda/builds/78637#019b97c8-c387-4d9d-8762-14c724db9752	FLAKY	12/21	Test FAILS after retries.Significant increase in flaky rate(baseline=0.1147, p0=0.0011, reject_threshold=0.0100)	https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=WriteCachingFailureInjectionE2ETest&test_method=test_crash_all

test results on build#78645

test_class	test_method	test_arguments	test_kind	job_url	test_status	passed	reason	test_history
WriteCachingFailureInjectionE2ETest	test_crash_all	{"use_transactions": false}	integration	https://buildkite.com/redpanda/redpanda/builds/78645#019b992f-a18f-4916-9f82-d96101c208f4	FLAKY	8/11	Test PASSES after retries.No significant increase in flaky rate(baseline=0.1176, p0=0.3326, reject_threshold=0.0100. adj_baseline=0.3130, p1=0.3488, trust_threshold=0.5000)	https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=WriteCachingFailureInjectionE2ETest&test_method=test_crash_all

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

src/v/cloud_storage_clients/util.h

src/v/cloud_storage_clients/util.cc

oleiman · 2026-01-07T15:58:50Z

/ci-repeat 1
skip-redpanda-build
skip-units
skip-rebase
tests/rptest/tests/compatibility/java_compression_test.py::JavaCompressionTest.test_upgrade_java_compression@{"compression_type":"snappy"}
tests/rptest/tests/write_caching_fi_e2e_test.py::WriteCachingFailureInjectionE2ETest.test_crash_all@{"use_transactions":false}

dotnwat · 2026-01-08T00:27:23Z

src/v/cloud_storage_clients/util.cc

+            bool is_whitespace = (c == ' ' || c == '\t');
+            if (!is_eq && !is_whitespace) {
+                break;
+            }


If we have "multipart/mixed; boundaryfffff", then boundary will be the substr fffff, and then c will be f and we'll break here, and return fffff as the boundary, but I would have expected that the lack of = would mean that it is malformed?

boundary will be the substring fffff... but the lack of = would mean that it is malformed

correct, but a few lines down we blow away the string view if we didn't find exactly one '=' before the first non-whitespace character. there is a unit test for this 🙂

This function could use some documentation. If the diff looks ok to you aside from that, I'll add a nice block comment in the next PR.

oh yeh makes sense i was thinking those checks were inside the while loop

dotnwat

lgtm. why are we returning a std::exception_ptr for std::unexpected case? it would seem like either throwing or returning a formatted error string and letting the caller throw would be better.

oleiman · 2026-01-08T16:39:42Z

lgtm. why are we returning a std::exception_ptr for std::unexpected case? it would seem like either throwing or returning a formatted error string and letting the caller throw would be better.

yeah fair, that sounds more normal. will change in follow up

oleiman self-assigned this Jan 7, 2026

Copilot AI review requested due to automatic review settings January 7, 2026 08:09

github-actions bot added the area/redpanda label Jan 7, 2026

Copilot AI reviewed Jan 7, 2026

View reviewed changes

src/v/cloud_storage_clients/util.cc Outdated Show resolved Hide resolved

src/v/cloud_storage_clients/util.cc Outdated Show resolved Hide resolved

oleiman force-pushed the ct/core-15023/find-boundary branch 2 times, most recently from 1c601fa to b84eb38 Compare January 7, 2026 08:49

oleiman requested a review from Copilot January 7, 2026 08:50

Copilot AI reviewed Jan 7, 2026

View reviewed changes

src/v/cloud_storage_clients/util.cc Outdated Show resolved Hide resolved

src/v/cloud_storage_clients/tests/util_test.cc Outdated Show resolved Hide resolved

oleiman mentioned this pull request Jan 7, 2026

[CORE-15023]: Cloud Storage Clients: Implement batch delete for ABS #29143

Merged

7 tasks

oleiman force-pushed the ct/core-15023/find-boundary branch from b84eb38 to 76bb416 Compare January 7, 2026 08:59

oleiman added 2 commits January 7, 2026 01:04

cs_clients/util: Add find_multipart_boundary

a2200b7

This is well-specified and identical between ABS and GCS, so factor it out for easier testing. Signed-off-by: Oren Leiman <[email protected]>

cs_clients/tests: Add some gmock flavor to util_test

84e1679

Signed-off-by: Oren Leiman <[email protected]>

oleiman force-pushed the ct/core-15023/find-boundary branch from 76bb416 to 84e1679 Compare January 7, 2026 09:05

oleiman requested a review from Copilot January 7, 2026 15:58

Copilot AI reviewed Jan 7, 2026

View reviewed changes

src/v/cloud_storage_clients/util.h Show resolved Hide resolved

src/v/cloud_storage_clients/util.cc Show resolved Hide resolved

oleiman requested a review from dotnwat January 7, 2026 17:11

dotnwat reviewed Jan 8, 2026

View reviewed changes

oleiman requested a review from dotnwat January 8, 2026 16:01

dotnwat approved these changes Jan 8, 2026

View reviewed changes

oleiman merged commit 34b6a34 into redpanda-data:dev Jan 8, 2026
19 checks passed

[CORE-15023] Cloud Storage Clients: Add util::find_multipart_boundary #29166

[CORE-15023] Cloud Storage Clients: Add util::find_multipart_boundary #29166

Uh oh!

Conversation

oleiman commented Jan 7, 2026

Backports Required

Release Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

vbotbuildovich commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Retry command for Build#78637

Uh oh!

vbotbuildovich commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI test results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

oleiman commented Jan 7, 2026

Uh oh!

dotnwat Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

oleiman Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

dotnwat Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

dotnwat left a comment

Choose a reason for hiding this comment

Uh oh!

oleiman commented Jan 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vbotbuildovich commented Jan 7, 2026 •

edited

Loading

vbotbuildovich commented Jan 7, 2026 •

edited

Loading