Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion crates/rattler-bin/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ rustls-tls = [
"rattler_networking/rustls-tls",
"rattler_cache/rustls-tls",
]
s3 = ["rattler_networking/s3", "rattler_upload/s3"]
s3 = ["rattler_networking/s3", "rattler_networking/rattler_config", "rattler_upload/s3"]
gcs = ["rattler_networking/gcs"]

[dependencies]
Expand All @@ -50,6 +50,7 @@ rattler_networking = { workspace = true, default-features = false, features = [
"system-integration",
"netrc-rs",
] }
rattler_config = { workspace = true, default-features = false }
rattler_repodata_gateway = { workspace = true, default-features = false, features = [
"gateway",
] }
Expand Down
26 changes: 22 additions & 4 deletions crates/rattler-bin/src/commands/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ use rattler_conda_types::{
Channel, ChannelConfig, GenericVirtualPackage, MatchSpec, Matches, PackageName,
ParseMatchSpecOptions, Platform, PrefixRecord, RepoDataRecord, Version,
};
#[cfg(feature = "s3")]
use rattler_networking::s3_middleware::compute_s3_config;
use rattler_networking::AuthenticationMiddleware;
#[cfg(feature = "s3")]
use rattler_networking::AuthenticationStorage;
Expand All @@ -42,6 +44,9 @@ pub struct Opt {
#[clap(required = true)]
specs: Vec<String>,

#[clap(long)]
config: Option<PathBuf>,

#[clap(long)]
dry_run: bool,

Expand Down Expand Up @@ -178,10 +183,23 @@ pub async fn create(opt: Opt) -> miette::Result<()> {
))
.with(rattler_networking::OciMiddleware::new(download_client));
#[cfg(feature = "s3")]
let download_client = download_client.with(rattler_networking::S3Middleware::new(
HashMap::new(),
AuthenticationStorage::from_env_and_defaults().into_diagnostic()?,
));
let download_client = {
use rattler_networking::s3_middleware::S3Config;
let s3_config: std::collections::HashMap<String, S3Config> = opt
.config
.as_ref()
.and_then(|config_path| {
rattler_config::config::ConfigBase::<()>::load_from_files([config_path]).ok()
})
.map(|config| compute_s3_config(&config.s3_options.0))
.unwrap_or_default();
download_client.with(rattler_networking::S3Middleware::new(
s3_config,
AuthenticationStorage::from_env_and_defaults().into_diagnostic()?,
))
};
#[cfg(not(feature = "s3"))]
let download_client = download_client;
#[cfg(feature = "gcs")]
let download_client = download_client.with(rattler_networking::GCSMiddleware::default());
let download_client = download_client.build();
Expand Down
9 changes: 8 additions & 1 deletion crates/rattler_index/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ struct Cli {

/// The path to the config file to use to configure rattler-index.
/// Uses the same configuration format as pixi, see `https://pixi.sh/latest/reference/pixi_configuration`.
#[arg(long)]
#[arg(long, global = true)]
config: Option<PathBuf>,
}

Expand Down Expand Up @@ -178,6 +178,13 @@ async fn main() -> anyhow::Result<()> {
credentials.endpoint_url = credentials
.endpoint_url
.or(s3_config.map(|c| c.endpoint_url.clone()));
if let Some(s3_config) = s3_config {
credentials.addressing_style = if s3_config.force_path_style {
rattler_s3::clap::S3AddressingStyleOpts::Path
} else {
rattler_s3::clap::S3AddressingStyleOpts::VirtualHost
};
}
Comment on lines +181 to +187
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should be using credentials.addressing_style.or(...).

this also means that https://github.com/pavelzw/rattler/blob/a8364172f90f752eed7e49f3be73c28878240631/crates/rattler_s3/src/clap.rs#L61 should be a Option<S3AddressingStyleOpts>.

@baszalmstra was there a reason why you made this not an Option back then?
Image

}

// Resolve the credentials
Expand Down
12 changes: 0 additions & 12 deletions crates/rattler_networking/src/s3_middleware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,18 +170,6 @@ impl S3 {
// Set the region from the default provider chain.
s3_config_builder.set_region(sdk_config.region().cloned());

// Infer if we expect path-style addressing from the endpoint URL.
if let Some(endpoint_url) = sdk_config.endpoint_url() {
// If the endpoint URL is localhost, we probably have to use path-style
// addressing. xref: https://github.com/awslabs/aws-sdk-rust/issues/1230
if endpoint_url.starts_with("http://localhost") {
s3_config_builder = s3_config_builder.force_path_style(true);
}
// same with cloudflare R2
if endpoint_url.starts_with("r2.cloudflarestorage.com") {
s3_config_builder = s3_config_builder.force_path_style(true);
}
}
Ok(aws_sdk_s3::Client::from_conf(s3_config_builder.build()))
}
}
Expand Down
15 changes: 11 additions & 4 deletions scripts/e2e/s3-minio.nu
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,19 @@ let data_dir = $"($tmp)/minio-data"
let log_file = $"($tmp)/minio.log"
let pid_file = $"($tmp)/minio.pid"
let bucket_name = $"tmp-(random int 0..1000000)"
let config_path = $"($tmp)/rattler.toml"

# Create directories
mkdir $bin_dir $data_dir

# Create rattler config with S3 options for MinIO (path-style addressing)
let config_content = $"[s3-options.($bucket_name)]
endpoint-url = \"http://localhost:9000\"
force-path-style = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while we're at it, i think we should rename this to addressing-style: path | virtual-host. we should print a warning when force-path-style is still used

region = \"us-east-1\"
"
$config_content | save $config_path

# Credentials
let root_user = ($env.MINIO_ACCESS_KEY? | default "minio")
let root_password = ($env.MINIO_SECRET_KEY? | default "minio123")
Expand Down Expand Up @@ -56,11 +65,9 @@ print "== Index the channel"
(^rattler-index
s3
$"s3://($bucket_name)"
--config $config_path
--access-key-id $root_user
--secret-access-key $root_password
--region "us-east-1"
--endpoint-url "http://localhost:9000"
--addressing-style path
Comment on lines -61 to -63
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is changing this necessary? CLI args should already result in S3CredentialsOpts being set, no?

)

print "== Verify cache control headers are set correctly"
Expand Down Expand Up @@ -113,11 +120,11 @@ with-env {
AWS_ACCESS_KEY_ID: $root_user
AWS_SECRET_ACCESS_KEY: $root_password
AWS_REGION: "us-east-1"
Comment on lines 120 to 122
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should be using the S3_ variants (that are not loaded through aws-sdk: https://github.com/pavelzw/rattler/blob/a8364172f90f752eed7e49f3be73c28878240631/crates/rattler_s3/src/clap.rs#L52

passing them as CLI flags is also fine

AWS_ENDPOINT_URL: "http://localhost:9000"
} {
(^rattler
create
--dry-run
--config $config_path
-c $"s3://($bucket_name)"
empty==0.1.0
)
Expand Down
Loading