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.

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

fix(services/s3): Return error if credential load fail instead skip #2233

Merged
merged 4 commits into from
May 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ jobs:
check_msrv:
runs-on: ubuntu-latest
env:
# OpenDAL's MSRV is 1.64.
OPENDAL_MSRV: "1.64"
# OpenDAL's MSRV is 1.65.
OPENDAL_MSRV: "1.65"
steps:
- uses: actions/checkout@v3
- name: Setup msrv of rust
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/service_test_s3.yml
Original file line number Diff line number Diff line change
Expand Up @@ -177,3 +177,4 @@ jobs:
OPENDAL_S3_TEST: on
OPENDAL_S3_BUCKET: test
OPENDAL_S3_ENDPOINT: "http://127.0.0.1:9000"
OPENDAL_S3_ALLOW_ANONYMOUS: on
115 changes: 86 additions & 29 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ edition = "2021"
homepage = "https://opendal.apache.org/"
license = "Apache-2.0"
repository = "https://github.com/apache/incubator-opendal"
rust-version = "1.64"
rust-version = "1.65"
version = "0.33.3"

[workspace.dependencies]
Expand Down
2 changes: 1 addition & 1 deletion core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ redis = { version = "0.22", features = [
"tokio-comp",
"connection-manager",
], optional = true }
reqsign = { version = "0.9.1", default-features = false, optional = true }
reqsign = { version = "0.10.0", default-features = false, optional = true }
reqwest = { version = "0.11.13", features = [
"stream",
], default-features = false }
Expand Down
1 change: 1 addition & 0 deletions core/src/raw/http_util/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ pub fn new_request_credential_error(err: anyhow::Error) -> Error {
ErrorKind::Unexpected,
"loading credentail to sign http request",
)
.set_temporary()
.with_operation("reqsign::LoadCredential")
.set_source(err)
}
Expand Down
3 changes: 2 additions & 1 deletion core/src/services/azblob/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use std::fmt;
use std::fmt::Debug;
use std::fmt::Formatter;
use std::fmt::Write;
use std::time::Duration;

use http::header::HeaderName;
use http::header::CONTENT_LENGTH;
Expand Down Expand Up @@ -83,7 +84,7 @@ impl AzblobCore {
let cred = self.load_credential().await?;

self.signer
.sign_query(req, &cred)
.sign_query(req, Duration::from_secs(3600), &cred)
.map_err(new_request_sign_error)
}

Expand Down
14 changes: 13 additions & 1 deletion core/src/services/s3/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ pub struct S3Builder {

disable_config_load: bool,
disable_ec2_metadata: bool,
allow_anonymous: bool,
enable_virtual_host_style: bool,

http_client: Option<HttpClient>,
Expand Down Expand Up @@ -626,6 +627,13 @@ impl S3Builder {
self
}

/// Allow anonymous will allow opendal to send request without signing
/// when credentail is not loaded.
pub fn allow_anonymous(&mut self) -> &mut Self {
self.allow_anonymous = true;
self
}

/// Enable virtual host style so that opendal will send API requests
/// in virtual host style instead of path style.
///
Expand Down Expand Up @@ -756,6 +764,9 @@ impl Builder for S3Builder {
map.get("enable_virtual_host_style")
.filter(|v| *v == "on" || *v == "true")
.map(|_| builder.enable_virtual_host_style());
map.get("allow_anonymous")
.filter(|v| *v == "on" || *v == "true")
.map(|_| builder.allow_anonymous());
map.get("default_storage_class")
.map(|v| builder.default_storage_class(v));

Expand Down Expand Up @@ -876,7 +887,7 @@ impl Builder for S3Builder {
let endpoint = self.build_endpoint(&region);
debug!("backend use endpoint: {endpoint}");

let mut loader = AwsLoader::new(client.client(), cfg).with_allow_anonymous();
let mut loader = AwsLoader::new(client.client(), cfg);
if self.disable_ec2_metadata {
loader = loader.with_disable_ec2_metadata();
}
Expand Down Expand Up @@ -906,6 +917,7 @@ impl Builder for S3Builder {
server_side_encryption_customer_key,
server_side_encryption_customer_key_md5,
default_storage_class,
allow_anonymous: self.allow_anonymous,
signer,
loader,
client,
Expand Down
Loading