Re: [PR] feat: add EKS Pod Identity support (#282) [arrow-rs-object-store]
tustvold merged PR #333: URL: https://github.com/apache/arrow-rs-object-store/pull/333 -- 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]
Re: [PR] feat: add EKS Pod Identity support (#282) [arrow-rs-object-store]
andreasbros commented on PR #333: URL: https://github.com/apache/arrow-rs-object-store/pull/333#issuecomment-2853857977 > I took the liberty of pushing a quick fix thanks @tustvold -- 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]
Re: [PR] feat: add EKS Pod Identity support (#282) [arrow-rs-object-store]
tustvold commented on PR #333: URL: https://github.com/apache/arrow-rs-object-store/pull/333#issuecomment-2848669611 I took the liberty of pushing a quick fix to avoid doing blocking IO within the credential provider, using the same trick we use for LocalFilesystem (which is also what tokio::fs does under the hood). I don't have a means to test this, but if you are happy it is working I'm happy to get this merged -- 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]
Re: [PR] feat: add EKS Pod Identity support (#282) [arrow-rs-object-store]
andreasbros commented on PR #333: URL: https://github.com/apache/arrow-rs-object-store/pull/333#issuecomment-2821935531 @tustvold I changed to `std::fs` based on existing `web_identity` implementation, but I agree it should be async ideally: https://github.com/apache/arrow-rs-object-store/blob/main/src/aws/credential.rs#L639 > it just needs to be appropriately gated My understanding from the README doc is that entire feature `aws` is not available in WASM. Can you point me to how to gate it? -- 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]
Re: [PR] feat: add EKS Pod Identity support (#282) [arrow-rs-object-store]
tustvold commented on PR #333: URL: https://github.com/apache/arrow-rs-object-store/pull/333#issuecomment-2821677452 Can we please not rebase PRs after review, I will now have to do a fresh review on this PR. > I now changed it to std::fs We should use tokio:fs, std::fs shouldn't be used in async contexts, it just needs to be appropriately gated - WASM contexts don't have filesystem access -- 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]
Re: [PR] feat: add EKS Pod Identity support (#282) [arrow-rs-object-store]
andreasbros commented on PR #333: URL: https://github.com/apache/arrow-rs-object-store/pull/333#issuecomment-2821648818 Thanks @alamb and @tustvold for checking, and indeed WASM build was failing, it was because I used async `tokio::fs`, I now changed it to `std::fs` and this should resolve the issue, just waiting for GHA workflows to get approved and executed. -- 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]
Re: [PR] feat: add EKS Pod Identity support (#282) [arrow-rs-object-store]
tustvold commented on code in PR #333:
URL:
https://github.com/apache/arrow-rs-object-store/pull/333#discussion_r2052561273
##
src/aws/credential.rs:
##
@@ -719,6 +720,68 @@ async fn task_credential(
})
}
+/// EKS Pod Identity credential provider.
+///
+/// Uses the endpoint in `AWS_CONTAINER_CREDENTIALS_FULL_URI`
+/// and the bearer token in `AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE`
+/// to fetch ephemeral AWS credentials from an EKS pod.
+#[derive(Debug)]
+pub(crate) struct EKSPodCredentialProvider {
+pub url: String,
+pub token_file: String,
+pub retry: RetryConfig,
+pub client: HttpClient,
+pub cache: TokenCache>,
+}
+
+#[async_trait]
+impl CredentialProvider for EKSPodCredentialProvider {
+type Credential = AwsCredential;
+
+async fn get_credential(&self) -> Result> {
+self.cache
+.get_or_insert_with(|| {
+eks_credential(&self.client, &self.retry, &self.url,
&self.token_file)
+})
+.await
+.map_err(|source| crate::Error::Generic {
+store: STORE,
+source,
+})
+}
+}
+
+/// Performs the actual credential retrieval and parsing for
`EKSPodCredentialProvider`.
+///
+///
+async fn eks_credential(
+client: &HttpClient,
+retry: &RetryConfig,
+url: &str,
+token_file: &str,
+) -> std::result::Result>, Box>
Review Comment:
```suggestion
) -> Result>, StdError>
```
--
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]
