feat(services/s3): support default_region, server_side_encryption_bucket_key and skip_signature for Arrow object_store compatibility #6531

jackye1995 · 2025-08-23T04:40:24Z

This PR adds 2 simple features that are available in Arrow object_store, but missing in OpenDAL:

default_region when the default region cannot be discovered
server_side_encryption_bucket_key for setting the X_AMZ_SERVER_SIDE_ENCRYPTION_BUCKET_KEY_ENABLED header

It also sets skip_signature and aws_skip_signature as aliases for allow_anonymous

…ementation Implements six new S3 service features to improve compatibility and functionality: - DefaultRegion: Fallback region when auto-detection fails - ImdsV1Fallback: Enable IMDSv1 for credential retrieval (prepared for reqsign support) - UnsignedPayload: Skip payload checksum for improved performance - SkipSignature: Disable request signing for public buckets - DisableTagging: Prepared for future S3 tagging implementation - BucketKeyEnabled: Control S3 bucket key usage for KMS encryption All features include: ✅ Configuration options in S3Config ✅ Builder methods in S3Builder ✅ Core implementation in signing and request logic ✅ Comprehensive unit tests ✅ Full clippy compliance with -D warnings ✅ Proper code formatting 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Add serde aliases for all new S3 configuration options to ensure compatibility with object_store library configuration naming: - default_region: aws_default_region, default_region - imdsv1_fallback: aws_imdsv1_fallback, imdsv1_fallback - unsigned_payload: aws_unsigned_payload, unsigned_payload - skip_signature: aws_skip_signature, skip_signature - disable_tagging: aws_disable_tagging, disable_tagging - bucket_key_enabled: aws_sse_bucket_key_enabled, bucket_key_enabled This allows users to configure OpenDAL S3 service using the same configuration keys as object_store, enabling easier migration and consistent configuration across Rust storage libraries. ✅ Added comprehensive tests for both AWS-prefixed and short aliases ✅ All tests pass with proper type handling ✅ Maintains backward compatibility with existing configurations 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Refactor the monolithic test_new_s3_features test into individual focused tests for better maintainability and clarity: - test_default_region_config: Tests DefaultRegion feature and fallback behavior - test_imdsv1_fallback_config: Tests IMDSv1Fallback feature enable/disable - test_unsigned_payload_config: Tests UnsignedPayload feature enable/disable - test_skip_signature_config: Tests SkipSignature feature enable/disable - test_disable_tagging_config: Tests DisableTagging feature enable/disable - test_bucket_key_enabled_config: Tests BucketKeyEnabled with enable/disable/none Benefits: ✅ Each test focuses on a single feature ✅ Easier to debug failing tests ✅ Better test isolation and clarity ✅ Tests both default values and enabled states ✅ All 12 tests pass (was 7, now 12 individual tests) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

…ging - Remove redundant aliases that duplicate config field names - Add individual alias tests for each AWS-prefixed configuration - Remove disable_tagging flag completely from config, builder, and core - Maintain 5 core features: default_region, imdsv1_fallback, unsigned_payload, skip_signature, bucket_key_enabled 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Remove TODO comments about S3 tagging functionality that are no longer needed. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Replace assert_eq! with boolean literals to use assert! instead as suggested by clippy. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Xuanwo · 2025-08-25T17:21:07Z

core/src/services/s3/backend.rs

        self
    }

+    /// Enable IMDSv1 fallback for retrieving credentials from instance metadata.


reqsign doesn't support this so far.

will remove, created a tracking issue at apache/opendal-reqsign#599

Xuanwo · 2025-08-25T17:22:45Z

core/src/services/s3/backend.rs

+
+    /// Enable unsigned payload for request signing to avoid computing body checksum.
+    /// This can improve performance for large uploads but may not be suitable for all use cases.
+    pub fn enable_unsigned_payload(mut self) -> Self {


We are using unsigned_payload by default, if we do need this, we might use disable_unsigned_payload. However, this can also have overlap with the checksum support we want to add in opendal: #5549

Xuanwo · 2025-08-25T17:23:39Z

core/src/services/s3/backend.rs

+    }
+
+    /// Skip request signing. Useful for public buckets or when using pre-signed URLs.
+    pub fn enable_skip_signature(mut self) -> Self {


This is allow_anonymous

Xuanwo · 2025-08-25T17:23:57Z

core/src/services/s3/backend.rs

+
+    /// Enable the use of S3 bucket keys for server-side encryption.
+    /// This can reduce costs when using KMS encryption by using fewer KMS API calls.
+    pub fn enable_bucket_key(mut self) -> Self {


This is server_side_encryption

Xuanwo · 2025-08-25T17:24:02Z

core/src/services/s3/backend.rs

+    }
+
+    /// Disable the use of S3 bucket keys for server-side encryption.
+    pub fn disable_bucket_key(mut self) -> Self {


This is server_side_encryption

…skip_signature an alias - Removed enable_imdsv1_fallback() method and imdsv1_fallback config field - Made enable_skip_signature() an alias for allow_anonymous() - Added serde aliases for skip_signature configs to map to allow_anonymous - Updated tests to reflect these changes 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

…ed_payload - Completely removed enable_skip_signature() method - Completely removed enable_unsigned_payload() method and all related logic - Removed unsigned_payload field from S3Config and S3Core - Fixed aliases to keep only aws_skip_signature and skip_signature for allow_anonymous - Updated tests to reflect these changes 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

…ket_key_enabled - Removed skip_signature from S3Core as it duplicates allow_anonymous - Updated sign methods to directly use allow_anonymous instead - Removed bucket_key_enabled from S3Config as it duplicates server_side_encryption_bucket_key_enabled - Consolidated bucket key configuration under server_side_encryption_bucket_key_enabled - Updated tests to use the correct field names 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

…sary skip_signature handling - Renamed enable_bucket_key() to enable_server_side_encryption_bucket_key() - Renamed disable_bucket_key() to disable_server_side_encryption_bucket_key() - Changed server_side_encryption_bucket_key_enabled from Option<bool> to bool - Removed unnecessary skip_signature handling in core.rs sign methods - Removed bucket_key_enabled alias, keeping only aws_sse_bucket_key_enabled - Updated tests to use the new method names 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Xuanwo · 2025-09-02T10:00:45Z

core/src/services/s3/backend.rs


+    /// Enable the use of S3 bucket keys for server-side encryption.
+    /// This can reduce costs when using KMS encryption by using fewer KMS API calls.
+    pub fn enable_server_side_encryption_bucket_key(mut self) -> Self {


I'm hesitant about this because OpenDAL tends to maintain configuration values in an orthogonal way, meaning each configuration value controls only a single feature. Users don't need to worry about the interactions between configuration values.

But I do understand the value of be compatible to those object_store options. Is it a good idea to add those mapping in to object_store_opendal? What do you think?

I think for the 3 configs here, they are a bit different:

enable_server_side_encryption_bucket_key seems to be a missing feature currently in OpenDAL S3, it adds the header x-amz-server-side-encryption-bucket-key-enabled to the request, where user can directly configure a bucket key for the entire bucket, and then use this API key to use that pre-configured key.

default_region gives a default in region resolution process, so it is only used when there is no other environment configurations that can resolve a region

These 2 are independent configs orthogonal to other existing configurations in OpenDAL-S3.

skip_signature as you suggested is just an alias of allow_anonymous so it was just a missed configuration mapping in the last PR.

jackye1995 requested a review from Xuanwo as a code owner August 23, 2025 04:40

dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. releases-note/feat The PR implements a new feature or has a title that begins with "feat" labels Aug 23, 2025

jackye1995 and others added 5 commits August 22, 2025 21:45

chore(services/s3): Remove S3 tagging TODO comments

477e7f5

Remove TODO comments about S3 tagging functionality that are no longer needed. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

fix(services/s3): Fix clippy bool-assert-comparison warnings

f6a1b6a

Replace assert_eq! with boolean literals to use assert! instead as suggested by clippy. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

Xuanwo reviewed Aug 25, 2025

View reviewed changes

jackye1995 and others added 5 commits August 25, 2025 15:17

fmt

0b0808f

jackye1995 changed the title ~~feat(services/s3): Add missing S3 features based on object_store implementation~~ feat(services/s3): support default_region, server_side_encryption_bucket_key and skip_signature for Arrow object_store compatibility Aug 25, 2025

jackye1995 requested a review from Xuanwo August 26, 2025 00:04

jackye1995 force-pushed the s3-missing-features branch from 5366a3e to 0b0808f Compare August 26, 2025 06:22

Xuanwo reviewed Sep 2, 2025

View reviewed changes

Xuanwo mentioned this pull request Sep 2, 2025

feat(services/azblob): support use_fabric_endpoint config #6530

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(services/s3): support default_region, server_side_encryption_bucket_key and skip_signature for Arrow object_store compatibility #6531

feat(services/s3): support default_region, server_side_encryption_bucket_key and skip_signature for Arrow object_store compatibility #6531

Uh oh!

jackye1995 commented Aug 23, 2025 •

edited

Loading

Uh oh!

Xuanwo Aug 25, 2025

Uh oh!

jackye1995 Aug 25, 2025

Uh oh!

Xuanwo Aug 25, 2025

Uh oh!

Xuanwo Aug 25, 2025

Uh oh!

Xuanwo Aug 25, 2025

Uh oh!

Xuanwo Aug 25, 2025

Uh oh!

Xuanwo Sep 2, 2025

Uh oh!

jackye1995 Sep 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

feat(services/s3): support default_region, server_side_encryption_bucket_key and skip_signature for Arrow object_store compatibility #6531

Are you sure you want to change the base?

feat(services/s3): support default_region, server_side_encryption_bucket_key and skip_signature for Arrow object_store compatibility #6531

Uh oh!

Conversation

jackye1995 commented Aug 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jackye1995 commented Aug 23, 2025 •

edited

Loading