Xuanwo commented on code in PR #5268:
URL: https://github.com/apache/arrow-rs/pull/5268#discussion_r1442569651
##########
object_store/src/aws/credential.rs:
##########
@@ -659,6 +685,56 @@ async fn task_credential(
})
}
+/// A session provider as used by S3 Express One Zone
+///
+/// <https://docs.aws.amazon.com/AmazonS3/latest/API/API_CreateSession.html>
+#[derive(Debug)]
+pub struct SessionProvider {
+ pub endpoint: String,
+ pub region: String,
+ pub credentials: AwsCredentialProvider,
+}
+
+#[async_trait]
+impl TokenProvider for SessionProvider {
+ type Credential = AwsCredential;
+
+ async fn fetch_token(
+ &self,
+ client: &Client,
+ retry: &RetryConfig,
+ ) -> Result<TemporaryToken<Arc<Self::Credential>>> {
+ let creds = self.credentials.get_credential().await?;
+ let authorizer = AwsAuthorizer::new(&creds, "s3", &self.region);
+
+ let bytes = client
+ .get(format!("{}?session", self.endpoint))
+ .with_aws_sigv4(Some(authorizer), None)
+ .send_retry(retry)
+ .await
+ .context(CreateSessionRequestSnafu)?
+ .bytes()
+ .await
+ .context(CreateSessionResponseSnafu)?;
+
+ let resp: CreateSessionOutput =
+
quick_xml::de::from_reader(bytes.reader()).context(CreateSessionOutputSnafu)?;
+
+ let creds = resp.credentials;
+ Ok(TemporaryToken {
+ token: Arc::new(creds.into()),
+ // Credentials last 5 minutes
+ expiry: Some(Instant::now() + Duration::from_secs(5 * 60)),
Review Comment:
I'm a bit confused. Is there another consideration preventing us from using
something like `SessionCredentials.expiration - Duration::from_secs(600)`?
Default of the expiration could last 3600 seconds.
##########
object_store/src/aws/credential.rs:
##########
@@ -555,20 +581,20 @@ struct AssumeRoleResponse {
#[derive(Debug, Deserialize)]
#[serde(rename_all = "PascalCase")]
struct AssumeRoleResult {
- credentials: AssumeRoleCredentials,
+ credentials: SessionCredentials,
Review Comment:
Nice change, since `AssumeRoleWithWebIdentity` returns the same struct.
##########
object_store/src/aws/credential.rs:
##########
@@ -75,13 +99,14 @@ pub struct AwsAuthorizer<'a> {
credential: &'a AwsCredential,
service: &'a str,
region: &'a str,
+ token_header: Option<HeaderName>,
sign_payload: bool,
}
-const DATE_HEADER: &str = "x-amz-date";
-const HASH_HEADER: &str = "x-amz-content-sha256";
-const TOKEN_HEADER: &str = "x-amz-security-token";
-const AUTH_HEADER: &str = "authorization";
+static DATE_HEADER: HeaderName = HeaderName::from_static("x-amz-date");
+static HASH_HEADER: HeaderName =
HeaderName::from_static("x-amz-content-sha256");
+static TOKEN_HEADER: HeaderName =
HeaderName::from_static("x-amz-security-token");
+static AUTH_HEADER: HeaderName = HeaderName::from_static("authorization");
Review Comment:
nit: Maybe we can use `http::header::AUTHORIZATION`?
##########
object_store/src/aws/mod.rs:
##########
@@ -255,6 +255,16 @@ impl ObjectStore for AmazonS3 {
prefix: Option<&Path>,
offset: &Path,
) -> BoxStream<'_, Result<ObjectMeta>> {
+ if self.client.config.session_provider.is_some() {
Review Comment:
The relationship between `session_provider` and S3 Express is not
immediately clear at first glance. I'm not sure if it worth to add an
`is_s3_express_bucket`, or more formally, `is_s3_directory_bucket`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
