apache / opendal

Apache OpenDAL: access data freely.
https://opendal.apache.org
Apache License 2.0
3.25k stars 453 forks source link

new feature: allow inject a customized signer for aws s3 #4960

Open flaneur2020 opened 1 month ago

flaneur2020 commented 1 month ago

Feature Description

background: https://github.com/apache/iceberg-rust/issues/506

It seems we need to implement this signer in iceberg-rust and pass it as a customized signer to opendal.

Problem and Solution

but currently the signer is a struct AwsV4Signer.

we need make the signer in S3Core a trait before allowing it customizable.

maybe we can defined a trait called S3Signer in opendal, and make AwsV4Signer as it's implementation by default.

Additional Context

No response

Are you willing to contribute to the development of this feature?

flaneur2020 commented 1 month ago

to make the Signer customizable, i've got some difficulties to solely add it in the opendal part.

adding a trait called S3Signer in opendal requires a signature to be object safe, but a signature like fn sign<T>(&self, req: &mut Request<T>, cred: &AwsCredential) -> Result<()> is not object safe because it contains a generic parameter inside it.

so i think there might have two ways to make it possible:

1: add a GenericSigner trait with fn sign(&self, req: &mut dyn SignableRequest, cred: &AwsCredential) -> Result<()> in reqsign.

but as Signer accepts &mut impl SignableRequest, we may have to change the api of Signer to allow it accept trait objects.

2: change Signer in reqsign into an enum like:

pub enum Signer {
  V4(V4Signer),
  Custom(Box<dyn CustomSigner>)
}

this approach can make the api of Signer unchanged, but would introduce a small dispatch cost on every sign.

(i'd prefer the second approach, it seems less intrusive)

@Xuanwo any suggestion about this?

Xuanwo commented 1 month ago

Hi, S3::customized_credential_load should allow you to implement your own logic.

flaneur2020 commented 1 month ago

Hi, S3::customized_credential_load should allow you to implement your own logic.

imho customized_credential_load is to customize the credential logic, but here we need customize is the signing logic.

the Signer currently is not a trait like dyn AwsCredentialLoad yet.

Xuanwo commented 1 month ago

Thank you for the clarification. I now understand what the problem is.

My current ideas: