-
Notifications
You must be signed in to change notification settings - Fork 12
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
Support AWS_ENDPOINT_URL #84
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
#!/bin/bash | ||
|
||
trap "echo 'Caught termination signal. Exiting...'; exit 0" SIGINT SIGTERM | ||
|
||
minio server /data & | ||
|
||
minio_pid=$! | ||
|
||
while ! curl $AWS_ENDPOINT_URL; do | ||
echo "Waiting for $AWS_ENDPOINT_URL..." | ||
sleep 1 | ||
done | ||
|
||
# set access key and secret key | ||
mc alias set local $AWS_ENDPOINT_URL $MINIO_ROOT_USER $MINIO_ROOT_PASSWORD | ||
|
||
# create bucket | ||
mc mb local/$AWS_S3_TEST_BUCKET | ||
|
||
wait $minio_pid |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,7 @@ | ||
use std::{sync::Arc, sync::LazyLock}; | ||
|
||
use arrow::datatypes::SchemaRef; | ||
use aws_config::{ | ||
environment::{EnvironmentVariableCredentialsProvider, EnvironmentVariableRegionProvider}, | ||
meta::{credentials::CredentialsProviderChain, region::RegionProviderChain}, | ||
profile::{ProfileFileCredentialsProvider, ProfileFileRegionProvider}, | ||
}; | ||
use aws_config::BehaviorVersion; | ||
use aws_credential_types::provider::ProvideCredentials; | ||
use object_store::{ | ||
aws::{AmazonS3, AmazonS3Builder}, | ||
|
@@ -92,48 +88,40 @@ | |
async fn get_s3_object_store(bucket_name: &str) -> AmazonS3 { | ||
let mut aws_s3_builder = AmazonS3Builder::new().with_bucket_name(bucket_name); | ||
|
||
let is_test_running = std::env::var("PG_PARQUET_TEST").is_ok(); | ||
|
||
if is_test_running { | ||
// use minio for testing | ||
aws_s3_builder = aws_s3_builder.with_endpoint("http://localhost:9000"); | ||
aws_s3_builder = aws_s3_builder.with_allow_http(true); | ||
} | ||
// AWS_ALLOW_HTTP | ||
if let Ok(aws_allow_http) = std::env::var("AWS_ALLOW_HTTP") { | ||
aws_s3_builder = aws_s3_builder | ||
.with_allow_http(aws_allow_http.parse().unwrap_or_else(|e| panic!("{}", e))); | ||
}; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably good to add some context in a comment on why we have this additional logic at this point, I don't have a very good mental model of what object_store does and does not do for us. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added comment to the function. Allowed passing the environment variables to |
||
let aws_profile_name = std::env::var("AWS_PROFILE").unwrap_or("default".to_string()); | ||
// first tries to load the profile files from the environment variables and then from the profile | ||
let sdk_config = aws_config::defaults(BehaviorVersion::v2024_03_28()) | ||
.load() | ||
.await; | ||
|
||
let region_provider = RegionProviderChain::first_try(EnvironmentVariableRegionProvider::new()) | ||
.or_else( | ||
ProfileFileRegionProvider::builder() | ||
.profile_name(aws_profile_name.clone()) | ||
.build(), | ||
); | ||
if let Some(credential_provider) = sdk_config.credentials_provider() { | ||
if let Ok(credentials) = credential_provider.provide_credentials().await { | ||
// AWS_ACCESS_KEY_ID | ||
aws_s3_builder = aws_s3_builder.with_access_key_id(credentials.access_key_id()); | ||
|
||
let region = region_provider.region().await; | ||
// AWS_SECRET_ACCESS_KEY | ||
aws_s3_builder = aws_s3_builder.with_secret_access_key(credentials.secret_access_key()); | ||
|
||
if let Some(region) = region { | ||
aws_s3_builder = aws_s3_builder.with_region(region.to_string()); | ||
if let Some(token) = credentials.session_token() { | ||
// AWS_SESSION_TOKEN | ||
aws_s3_builder = aws_s3_builder.with_token(token); | ||
} | ||
} | ||
} | ||
|
||
let credential_provider = CredentialsProviderChain::first_try( | ||
"Environment", | ||
EnvironmentVariableCredentialsProvider::new(), | ||
) | ||
.or_else( | ||
"Profile", | ||
ProfileFileCredentialsProvider::builder() | ||
.profile_name(aws_profile_name) | ||
.build(), | ||
); | ||
|
||
if let Ok(credentials) = credential_provider.provide_credentials().await { | ||
aws_s3_builder = aws_s3_builder.with_access_key_id(credentials.access_key_id()); | ||
|
||
aws_s3_builder = aws_s3_builder.with_secret_access_key(credentials.secret_access_key()); | ||
// AWS_ENDPOINT_URL | ||
if let Some(aws_endpoint_url) = sdk_config.endpoint_url() { | ||
aws_s3_builder = aws_s3_builder.with_endpoint(aws_endpoint_url); | ||
} | ||
|
||
if let Some(token) = credentials.session_token() { | ||
aws_s3_builder = aws_s3_builder.with_token(token); | ||
} | ||
// AWS_REGION | ||
if let Some(aws_region) = sdk_config.region() { | ||
aws_s3_builder = aws_s3_builder.with_region(aws_region.as_ref()); | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does this code also cover other paths like using the EC2 metadata server to obtain credentials? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When all required environment variables are configured to auth via EC2 server, |
||
aws_s3_builder.build().unwrap_or_else(|e| panic!("{}", e)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can the
unwrap_or_else
beexpect!
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expect(format!())
gives lint errors but we might useunwrap
or give additional context withunwrap_or_else