Support datafusion-cli access to public S3 buckets that do not require authentication #16300

alamb · 2025-06-06T12:52:06Z

Which issue does this PR close?

Closes Support datafusion-cli access to public S3 buckets that do not require authentication #16299
Related to [DISCUSSION] Make it easier and faster to query remote files (S3, iceberg, etc) #13456

Rationale for this change

I want to be able to access public s3 buckets without providing (valid) s3 credentials

What changes are included in this PR?

Add skip_signature option to datafusion-cli CREATE EXTERNAL TABLE
Default to skip_signature when other credentials are not provided
Update documentation

Before this PR:

DataFusion CLI v47.0.0
> CREATE EXTERNAL TABLE nyc_taxi_rides
STORED AS PARQUET LOCATION 's3://altinity-clickhouse-data/nyc_taxi_rides/data/tripdata_parquet/';
Object Store error: Generic S3 error: the credential provider was not enabled

After this PR:

DataFusion CLI v48.0.0
> CREATE EXTERNAL TABLE nyc_taxi_rides
STORED AS PARQUET LOCATION 's3://altinity-clickhouse-data/nyc_taxi_rides/data/tripdata_parquet/';
selec0 row(s) fetched.
Elapsed 1.575 seconds.

> select count(*) from nyc_taxi_rides;
+------------+
| count(*)   |
+------------+
| 1310903963 |
+------------+
1 row(s) fetched.
Elapsed 3.011 seconds.

Are these changes tested?

Yes, new unit tests are added and I tested it manually

For example, if you provide credentials, they take precidence over the signature:

AWS_ACCESS_KEY_ID=foo AWS_SECRET_ACCESS_KEY=bar  cargo run -p datafusion-cli

> CREATE EXTERNAL TABLE nyc_taxi_rides
STORED AS PARQUET LOCATION 's3://altinity-clickhouse-data/nyc_taxi_rides/data/tripdata_parquet/';
Object Store error: Generic S3 error: Error performing list request: Error performing GET https://s3.us-east-1.amazonaws.com/altinity-clickhouse-data?list-type=2&prefix=nyc_taxi_rides%2Fdata%2Ftripdata_parquet%2F in 134.200375ms - Server returned non-2xx status code: 403 Forbidden: <?xml version="1.0" encoding="UTF-8"?>
<Error><Code>InvalidAccessKeyId</Code><Message>The AWS Access Key Id you provided does not exist in our records.</Message><AWSAccessKeyId>foo</AWSAccessKeyId><RequestId>ZAEM63Q02FQXYMTA</RequestId><HostId>mYh2PUtKzDxjrPA4vQm4d+Qae9TiNpCUDDTS5BP4jTayKVE4BQbSpT/+HSIAdzt3lne6G0sxNmE=</HostId></Error>

But you can override this with SKIP_SIGNATURE

> CREATE EXTERNAL TABLE nyc_taxi_rides
STORED AS PARQUET LOCATION 's3://altinity-clickhouse-data/nyc_taxi_rides/data/tripdata_parquet/' OPTIONS(AWS.SKIP_SIGNATURE 'true');
0 row(s) fetched.
Elapsed 1.455 seconds.

Are there any user-facing changes?

Easier to use datafusion-cli

…e authentication

alamb · 2025-06-06T12:52:48Z

datafusion-cli/src/object_storage.rs

+            builder = builder.with_credentials(credentials);
+        } else {
+            debug!("No credentials found, defaulting to skip signature ");
+            builder = builder.with_skip_signature(true);


this changes the default so we don't try and create a signature if no credentials can be extracted

alamb · 2025-06-06T12:53:03Z

datafusion-cli/src/object_storage.rs

@@ -256,6 +305,9 @@ impl ExtensionOptions for AwsOptions {
            "allow_http" => {
                self.allow_http.set(rem, value)?;
            }
+            "skip_signature" | "nosign" => {


here is support for passing through skip_signature

alamb · 2025-06-06T12:54:00Z

datafusion-cli/src/object_storage.rs

@@ -493,29 +576,27 @@ mod tests {
        );

        let ctx = SessionContext::new();


Looking at this with whitespace blind diff https://github.com/apache/datafusion/pull/16300/files?w=1

makes it easier to see what is happening

Specifically, I just refactored the parsing / table_options extraction into a separate function to reduce repetition.

Otherwise the existing tests are the same

blaginin

🙌🏻

datafusion-cli/Cargo.toml

blaginin · 2025-06-09T19:55:58Z

datafusion-cli/src/object_storage.rs

+        // we can call `provide_credentials` here.
+        let credentials = match credentials.provide_credentials().await {
+            Ok(_) => Some(credentials),
+            Err(e) => {


nit: does it make sense to expect specific errors? For example, for CredentialsError::InvalidConfiguration or InvalidConfiguration::ProviderTimedOut, we probably still want to raise since those mean the creds could be set?

That is a great idea -- fixed in 37d1132

Co-authored-by: Dmitrii Blaginin <[email protected]>

alamb · 2025-06-11T13:02:49Z

Thanks @blaginin !

alamb · 2025-06-11T17:38:08Z

There appears to be some problem with this code in PR:

CI failure on Datafusion extended tests / cargo test hash collisions (amd64) (push) #16378
I have another PR up to add some better error messages to debug:
Add more context to error message for datafusion-cli config failure #16379

Support datafusion-cli access to public S3 buckets that do not requir…

63b976d

…e authentication

github-actions bot added the documentation Improvements or additions to documentation label Jun 6, 2025

alamb commented Jun 6, 2025

View reviewed changes

alamb marked this pull request as ready for review June 6, 2025 12:54

improve docs

659f6b1

alamb mentioned this pull request Jun 6, 2025

Improved experience when remote object store URL does not end in / #16302

Open

alamb added 3 commits June 6, 2025 12:35

clippy and test

2358874

Update test

7183016

Merge remote-tracking branch 'apache/main' into alamb/nosign-cli

b4cd32d

blaginin self-requested a review June 9, 2025 19:56

blaginin approved these changes Jun 9, 2025

View reviewed changes

alamb and others added 2 commits June 11, 2025 00:25

Update datafusion-cli/Cargo.toml

dd62c2c

Co-authored-by: Dmitrii Blaginin <[email protected]>

Merge remote-tracking branch 'apache/main' into alamb/nosign-cli

add7d71

alamb mentioned this pull request Jun 11, 2025

Improve performance of datafusion-cli when reading from remote storage #16365

Open

Only use unsigned requests on CredentialsError::CredentialsNotLoaded

37d1132

blaginin merged commit a91e042 into apache:main Jun 11, 2025
31 checks passed

alamb deleted the alamb/nosign-cli branch June 11, 2025 13:02

This was referenced Jun 11, 2025

CI failure on Datafusion extended tests / cargo test hash collisions (amd64) (push) #16378

Closed

Add more context to error message for datafusion-cli config failure #16379

Merged

alamb mentioned this pull request Jun 11, 2025

Disable datafusion-cli tests for hash_collision tests, fix extended CI #16382

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Support datafusion-cli access to public S3 buckets that do not require authentication #16300

Support datafusion-cli access to public S3 buckets that do not require authentication #16300

Uh oh!

alamb commented Jun 6, 2025

Uh oh!

alamb Jun 6, 2025

Uh oh!

alamb Jun 6, 2025

Uh oh!

alamb Jun 6, 2025

Uh oh!

blaginin left a comment

Uh oh!

Uh oh!

blaginin Jun 9, 2025

Uh oh!

alamb Jun 11, 2025

Uh oh!

Uh oh!

alamb commented Jun 11, 2025

Uh oh!

alamb commented Jun 11, 2025

Uh oh!

Uh oh!

		@@ -493,29 +576,27 @@ mod tests {
		);

		let ctx = SessionContext::new();

Support datafusion-cli access to public S3 buckets that do not require authentication #16300

Support datafusion-cli access to public S3 buckets that do not require authentication #16300

Uh oh!

Conversation

alamb commented Jun 6, 2025

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

alamb Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

alamb Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

alamb Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

blaginin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

blaginin Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

alamb Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alamb commented Jun 11, 2025

Uh oh!

alamb commented Jun 11, 2025

Uh oh!

Uh oh!