From 16437c875500bbe2165d2172571673b0b94414e6 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Wed, 12 Feb 2025 16:48:41 +0100 Subject: [PATCH 1/9] feat(stackable-operator/s3connection): Add region field defaulting to us-east-1 --- crates/stackable-operator/src/commons/s3/crd.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/stackable-operator/src/commons/s3/crd.rs b/crates/stackable-operator/src/commons/s3/crd.rs index d1609ccb9..9777b989a 100644 --- a/crates/stackable-operator/src/commons/s3/crd.rs +++ b/crates/stackable-operator/src/commons/s3/crd.rs @@ -56,6 +56,12 @@ pub struct S3ConnectionSpec { #[serde(default, skip_serializing_if = "Option::is_none")] pub port: Option, + /// Region to connect to when using AWS S3. + /// + /// This defaults to us-east-1, and can be ignored if not using AWS S3. + #[serde(default = "s3_region_default")] + pub region: String, + /// Which access style to use. /// Defaults to virtual hosted-style as most of the data products out there. /// Have a look at the [AWS documentation](https://docs.aws.amazon.com/AmazonS3/latest/userguide/VirtualHosting.html). @@ -85,3 +91,7 @@ pub enum S3AccessStyle { #[default] VirtualHosted, } + +fn s3_region_default() -> String { + "us-east-1".to_owned() +} From a8f87a8cae25124ce475b9fc889fb553d847a4f3 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Fri, 14 Feb 2025 11:04:04 +0100 Subject: [PATCH 2/9] refactor(stackable-operator/s3connection: Make the region field more complex to cater to the various use-cases: 1. Using S3 buckets from within AWS (users can choose to auto-discover via IMDS) 2. Using S3 buckets from outside AWS (users can choose the default, or set an optimal region based on the clients location). 3. Not using AWS S3 buckets, eg: MinIO or IONOS (users can ignore setting the field and the default region will be set to keep the AWS SDK happy, even though the region is not used for bucket operations). --- .../stackable-operator/src/commons/s3/crd.rs | 60 +++++++++++++++++-- 1 file changed, 55 insertions(+), 5 deletions(-) diff --git a/crates/stackable-operator/src/commons/s3/crd.rs b/crates/stackable-operator/src/commons/s3/crd.rs index 9777b989a..b5db5dd0d 100644 --- a/crates/stackable-operator/src/commons/s3/crd.rs +++ b/crates/stackable-operator/src/commons/s3/crd.rs @@ -56,11 +56,26 @@ pub struct S3ConnectionSpec { #[serde(default, skip_serializing_if = "Option::is_none")] pub port: Option, - /// Region to connect to when using AWS S3. + /// AWS service API region used by the AWS SDK when using AWS S3 buckets. + /// + /// This defaults to `us-east-1`. + /// + /// NOTE: This is not the bucket region, and is used by the AWS SDK to make + /// endpoints for various AWS service APIs. It is only useful when using AWS + /// S3 buckets. + /// + /// NOTE: This setting is only useful if using AWS S3 buckets. Other S3 + /// implementations _should not_ require this to be set. + /// + /// When using AWS S3 buckets, you can configure optimal AWS service API + /// connections in the following ways: + /// - From **inside** AWS: Use an auto-discovery source (eg: AWS IMDS). + /// - From **outside** AWS, or when IMDS is disabled, explicity set the + /// region name nearest to where the client application is running from. /// /// This defaults to us-east-1, and can be ignored if not using AWS S3. - #[serde(default = "s3_region_default")] - pub region: String, + #[serde(flatten)] + pub region: AwsRegion, /// Which access style to use. /// Defaults to virtual hosted-style as most of the data products out there. @@ -92,6 +107,41 @@ pub enum S3AccessStyle { VirtualHosted, } -fn s3_region_default() -> String { - "us-east-1".to_owned() +#[derive(strum::Display, Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] +#[strum(serialize_all = "snake_case")] +pub enum AwsRegion { + /// Defer region detection to an auto-discovery mechanism. + Source(AwsRegionAutoDiscovery), + + /// An explicit region, eg: eu-central-1 + Name(String), +} + +impl AwsRegion { + /// Get the region. + /// + /// Returns None if an auto-discovery source has been selected. + /// Otherwise, returns the configured region name. + pub fn region(self) -> Option { + match self { + AwsRegion::Name(name) => Some(name), + AwsRegion::Source(_) => None, + } + } +} + +impl Default for AwsRegion { + fn default() -> Self { + Self::Name("us-east-1".to_owned()) + } +} + +#[derive(strum::Display, Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] +#[strum(serialize_all = "kebab-case")] +pub enum AwsRegionAutoDiscovery { + /// AWS Instance Meta Data Service. + /// + /// This variant should result in no region being given to the AWS SDK, + /// which should, in turn, query the AWS IMDS. + AwsImds, } From e32221abf66ba9678344a10e3df72fde259a9eac Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Wed, 19 Feb 2025 15:44:31 +0100 Subject: [PATCH 3/9] test(stackable-operator): Add region new fields in helpers --- crates/stackable-operator/src/commons/s3/helpers.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/stackable-operator/src/commons/s3/helpers.rs b/crates/stackable-operator/src/commons/s3/helpers.rs index 654bee025..1cb13eac3 100644 --- a/crates/stackable-operator/src/commons/s3/helpers.rs +++ b/crates/stackable-operator/src/commons/s3/helpers.rs @@ -201,6 +201,7 @@ mod tests { access_style: Default::default(), credentials: None, tls: TlsClientDetails { tls: None }, + region: Default::default(), }; let (volumes, mounts) = s3.volumes_and_mounts().unwrap(); @@ -226,6 +227,7 @@ mod tests { }), }), }, + region: Default::default(), }; let (mut volumes, mut mounts) = s3.volumes_and_mounts().unwrap(); @@ -278,6 +280,7 @@ mod tests { verification: TlsVerification::None {}, }), }, + region: Default::default(), }; let (volumes, mounts) = s3.volumes_and_mounts().unwrap(); From 09c37a57d0a6eab914509c5933bdb3701c872284 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Wed, 19 Feb 2025 15:52:08 +0100 Subject: [PATCH 4/9] chore(stackable-operator): Remove strum until needed, set serde rename styles --- crates/stackable-operator/src/commons/s3/crd.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/stackable-operator/src/commons/s3/crd.rs b/crates/stackable-operator/src/commons/s3/crd.rs index b5db5dd0d..6aa29df43 100644 --- a/crates/stackable-operator/src/commons/s3/crd.rs +++ b/crates/stackable-operator/src/commons/s3/crd.rs @@ -74,7 +74,7 @@ pub struct S3ConnectionSpec { /// region name nearest to where the client application is running from. /// /// This defaults to us-east-1, and can be ignored if not using AWS S3. - #[serde(flatten)] + #[serde(default)] pub region: AwsRegion, /// Which access style to use. @@ -107,8 +107,8 @@ pub enum S3AccessStyle { VirtualHosted, } -#[derive(strum::Display, Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] -#[strum(serialize_all = "snake_case")] +#[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] pub enum AwsRegion { /// Defer region detection to an auto-discovery mechanism. Source(AwsRegionAutoDiscovery), @@ -136,8 +136,8 @@ impl Default for AwsRegion { } } -#[derive(strum::Display, Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] -#[strum(serialize_all = "kebab-case")] +#[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] +#[serde(rename_all = "PascalCase")] pub enum AwsRegionAutoDiscovery { /// AWS Instance Meta Data Service. /// From b1a0760642166f0ea515d1cc19284305b0b1657b Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Wed, 19 Feb 2025 15:53:17 +0100 Subject: [PATCH 5/9] chore(stackable-operator): Rename associated function to name (since the struct is already representing the "region" --- crates/stackable-operator/src/commons/s3/crd.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stackable-operator/src/commons/s3/crd.rs b/crates/stackable-operator/src/commons/s3/crd.rs index 6aa29df43..792225045 100644 --- a/crates/stackable-operator/src/commons/s3/crd.rs +++ b/crates/stackable-operator/src/commons/s3/crd.rs @@ -122,7 +122,7 @@ impl AwsRegion { /// /// Returns None if an auto-discovery source has been selected. /// Otherwise, returns the configured region name. - pub fn region(self) -> Option { + pub fn name(self) -> Option { match self { AwsRegion::Name(name) => Some(name), AwsRegion::Source(_) => None, From c0f14c483ea19f8c144bd525dcaf507d09b23517 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Wed, 19 Feb 2025 15:53:42 +0100 Subject: [PATCH 6/9] docs(stackable-operator): Improve docs --- .../stackable-operator/src/commons/s3/crd.rs | 36 ++++++++++++------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/crates/stackable-operator/src/commons/s3/crd.rs b/crates/stackable-operator/src/commons/s3/crd.rs index 792225045..ca72231e9 100644 --- a/crates/stackable-operator/src/commons/s3/crd.rs +++ b/crates/stackable-operator/src/commons/s3/crd.rs @@ -58,22 +58,18 @@ pub struct S3ConnectionSpec { /// AWS service API region used by the AWS SDK when using AWS S3 buckets. /// - /// This defaults to `us-east-1`. + /// This defaults to `us-east-1` and can be ignored if not using AWS S3 + /// buckets. /// - /// NOTE: This is not the bucket region, and is used by the AWS SDK to make - /// endpoints for various AWS service APIs. It is only useful when using AWS - /// S3 buckets. - /// - /// NOTE: This setting is only useful if using AWS S3 buckets. Other S3 - /// implementations _should not_ require this to be set. + /// NOTE: This is not the bucket region, and is used by the AWS SDK to + /// construct endpoints for various AWS service APIs. It is only useful when + /// using AWS S3 buckets. /// /// When using AWS S3 buckets, you can configure optimal AWS service API /// connections in the following ways: /// - From **inside** AWS: Use an auto-discovery source (eg: AWS IMDS). /// - From **outside** AWS, or when IMDS is disabled, explicity set the /// region name nearest to where the client application is running from. - /// - /// This defaults to us-east-1, and can be ignored if not using AWS S3. #[serde(default)] pub region: AwsRegion, @@ -107,6 +103,7 @@ pub enum S3AccessStyle { VirtualHosted, } +/// Set a named AWS region, or defer to an auto-discovery mechanism. #[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub enum AwsRegion { @@ -118,10 +115,24 @@ pub enum AwsRegion { } impl AwsRegion { - /// Get the region. + /// Get the AWS region name. + /// + /// Returns `None` if an auto-discovery source has been selected. Otherwise, + /// it returns the configured region name. + /// + /// Example usage: + /// + /// ``` + /// # fn set_property(key: &str, value: String) { + /// # } /// - /// Returns None if an auto-discovery source has been selected. - /// Otherwise, returns the configured region name. + /// # fn example(aws_region: AwsRegion) { + /// aws_region.name().and_then(|region_name| { + /// // set some propery if the region is set, or is the default. + /// set_property("aws.region", region_name); + /// }); + /// # } + /// ``` pub fn name(self) -> Option { match self { AwsRegion::Name(name) => Some(name), @@ -136,6 +147,7 @@ impl Default for AwsRegion { } } +/// AWS region auto-discovery mechanism. #[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] #[serde(rename_all = "PascalCase")] pub enum AwsRegionAutoDiscovery { From b1aa83469f0ec67d0bbb1251acbc7e8463b4e6f4 Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Thu, 20 Feb 2025 11:32:18 +0100 Subject: [PATCH 7/9] docs(stackable-operator): Improve docs --- crates/stackable-operator/src/commons/s3/crd.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/stackable-operator/src/commons/s3/crd.rs b/crates/stackable-operator/src/commons/s3/crd.rs index ca72231e9..2d45efab1 100644 --- a/crates/stackable-operator/src/commons/s3/crd.rs +++ b/crates/stackable-operator/src/commons/s3/crd.rs @@ -123,14 +123,13 @@ impl AwsRegion { /// Example usage: /// /// ``` - /// # fn set_property(key: &str, value: String) { - /// # } - /// + /// # use stackable_operator::commons::s3::AwsRegion; + /// # fn set_property(key: &str, value: String) {} /// # fn example(aws_region: AwsRegion) { - /// aws_region.name().and_then(|region_name| { + /// if let Some(region_name) = aws_region.name() { /// // set some propery if the region is set, or is the default. /// set_property("aws.region", region_name); - /// }); + /// }; /// # } /// ``` pub fn name(self) -> Option { From 4163144ddcd60781fa2f3f032a7ee4bc7d7758ae Mon Sep 17 00:00:00 2001 From: Nick <10092581+NickLarsenNZ@users.noreply.github.com> Date: Fri, 21 Feb 2025 11:21:51 +0100 Subject: [PATCH 8/9] Apply suggestions from code review Co-authored-by: Techassi --- crates/stackable-operator/src/commons/s3/crd.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stackable-operator/src/commons/s3/crd.rs b/crates/stackable-operator/src/commons/s3/crd.rs index 2d45efab1..f378a6176 100644 --- a/crates/stackable-operator/src/commons/s3/crd.rs +++ b/crates/stackable-operator/src/commons/s3/crd.rs @@ -127,7 +127,7 @@ impl AwsRegion { /// # fn set_property(key: &str, value: String) {} /// # fn example(aws_region: AwsRegion) { /// if let Some(region_name) = aws_region.name() { - /// // set some propery if the region is set, or is the default. + /// // set some property if the region is set, or is the default. /// set_property("aws.region", region_name); /// }; /// # } From d7bab128f6d755aa03a74c5027d375499ecb6b6a Mon Sep 17 00:00:00 2001 From: Nick Larsen Date: Fri, 21 Feb 2025 11:24:28 +0100 Subject: [PATCH 9/9] chore(stackable-operator): Update changelog --- crates/stackable-operator/CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index e882a2862..09b8e3595 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Added + +- Add `region` field to S3ConnectionSpec ([#959]). + +[#959]: https://github.com/stackabletech/operator-rs/pull/959 + ## [0.86.0] - 2025-01-30 ### Added