Tpt commented on code in PR #707:
URL:
https://github.com/apache/arrow-rs-object-store/pull/707#discussion_r3335410924
##########
.github/workflows/ci.yml:
##########
@@ -39,6 +39,10 @@ jobs:
- uses: actions/checkout@v6
- name: Setup Clippy
run: rustup component add clippy
+ - name: Check no crypto crates
+ run:
+ cargo tree --features
gcp-no-crypto,aws-no-crypto,azure-no-crypto,http-no-crypto \
+ | grep -qE '\b(ring|openssl)\b' && { echo "❌ disallowed crate
found"; exit 1; } || echo "✅ no disallowed crates"
Review Comment:
I guess this test should also add aws-lc-rs?
```suggestion
| grep -qE '\b(ring|openssl|aws-lc-rs)\b' && { echo "❌ disallowed
crate found"; exit 1; } || echo "✅ no disallowed crates"
```
##########
src/gcp/credential.rs:
##########
@@ -123,44 +104,64 @@ pub struct GcpSigningCredential {
}
/// A private RSA key for a service account
-#[derive(Debug)]
-pub struct ServiceAccountKey(RsaKeyPair);
+pub struct ServiceAccountKey(Box<dyn Signer>);
+
+impl std::fmt::Debug for ServiceAccountKey {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ f.debug_tuple("ServiceAccountKey").finish_non_exhaustive()
+ }
+}
impl ServiceAccountKey {
+ /// Creates a [`ServiceAccountKey`] from the provided [`Signer`]
+ pub fn new(signer: Box<dyn Signer>) -> Self {
+ Self(signer)
+ }
+
/// Parses a pem-encoded RSA key
- pub fn from_pem(encoded: &[u8]) -> Result<Self> {
- use rustls_pki_types::PrivateKeyDer;
- use rustls_pki_types::pem::PemObject;
-
- match PrivateKeyDer::from_pem_slice(encoded) {
- Ok(PrivateKeyDer::Pkcs8(key)) =>
Self::from_pkcs8(key.secret_pkcs8_der()),
- Ok(PrivateKeyDer::Pkcs1(key)) =>
Self::from_der(key.secret_pkcs1_der()),
- Ok(_) => Err(Error::MissingKey),
- Err(source) => Err(Error::ReadPem { source }),
- }
+ #[cfg(feature = "aws-lc-rs")]
Review Comment:
hyper-nit (please ignore): moving the `cfg` switch inside of the function
might help ensuring the signature does not diverge between aws-lc and rustls
(same for the other functions)
##########
src/client/mod.rs:
##########
@@ -260,7 +301,7 @@ impl FromStr for ClientConfigKey {
"timeout" => Ok(Self::Timeout),
"user_agent" => Ok(Self::UserAgent),
_ => Err(super::Error::UnknownConfigurationKey {
- store: "HTTP",
+ store: "http-no-crypto",
Review Comment:
find/replace error?
```suggestion
store: "HTTP",
```
--
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]
