refactor: Adjust S3 region field structure

crates/stackable-certs/src/ca/mod.rs

@@ -380,7 +380,7 @@ where
         key_certificate: &str,
         key_private_key: &str,
     ) -> Result<Self, SecretError<S::Error>> {
-        if !secret.type_.as_ref().is_some_and(|s| s == TLS_SECRET_TYPE) {
+        if secret.type_.as_ref().is_none_or(|s| s != TLS_SECRET_TYPE) {
             return InvalidSecretTypeSnafu.fail();
         }
 

crates/stackable-operator/CHANGELOG.md

@@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+### Changed
+
+- Refactor `region` field in S3ConnectionSpec ([#976]).
+
+[#976]: https://github.com/stackabletech/operator-rs/pull/976
+
 ## [0.87.0] - 2025-02-28
 
 ### Changed
@@ -24,7 +30,7 @@ All notable changes to this project will be documented in this file.
 
 ### Added
 
-- Add `region` field to S3ConnectionSpec ([#959]).
+- BREAKING: Add `region` field to S3ConnectionSpec (defaults to `us-east-1`) ([#959]).
 
 [#959]: https://github.com/stackabletech/operator-rs/pull/959
 

crates/stackable-operator/src/commons/rbac.rs

@@ -30,7 +30,6 @@ pub enum Error {
 /// Build RBAC objects for the product workloads.
 /// The `product_name` is meant to be the product name, for example: zookeeper, airflow, etc.
 /// and it is a assumed that a ClusterRole named `{product_name}-clusterrole` exists.
-
 pub fn build_rbac_resources<T: Clone + Resource<DynamicType = ()>>(
     resource: &T,
     // 'product_name' is not used to build the names of the serviceAccount and roleBinding objects,

crates/stackable-operator/src/commons/s3/crd.rs

@@ -56,22 +56,13 @@ pub struct S3ConnectionSpec {
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub port: Option<u16>,
 
-    /// AWS service API region used by the AWS SDK when using AWS S3 buckets.
+    /// Bucket region used for signing headers (sigv4).
     ///
-    /// This defaults to `us-east-1` and can be ignored if not using AWS S3
-    /// buckets.
+    /// This defaults to `us-east-1` which is compatible with other implementations such as Minio.
     ///
-    /// 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.
+    /// WARNING: Some products use the Hadoop S3 implementation which falls back to us-east-2.
     #[serde(default)]
-    pub region: AwsRegion,
+    pub region: Region,
 
     /// Which access style to use.
     /// Defaults to virtual hosted-style as most of the data products out there.
@@ -103,56 +94,22 @@ pub enum S3AccessStyle {
     VirtualHosted,
 }
 
-/// Set a named AWS region, or defer to an auto-discovery mechanism.
+/// Set a named S3 Bucket region.
 #[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),
-
-    /// An explicit region, eg: eu-central-1
-    Name(String),
-}
-
-impl AwsRegion {
-    /// Get the AWS region name.
-    ///
-    /// Returns `None` if an auto-discovery source has been selected. Otherwise,
-    /// it returns the configured region name.
-    ///
-    /// Example usage:
-    ///
-    /// ```
-    /// # use stackable_operator::commons::s3::AwsRegion;
-    /// # fn set_property(key: &str, value: &str) {}
-    /// # fn example(aws_region: AwsRegion) {
-    /// if let Some(region_name) = aws_region.name() {
-    ///     // set some property if the region is set, or is the default.
-    ///     set_property("aws.region", region_name);
-    /// };
-    /// # }
-    /// ```
-    pub fn name(&self) -> Option<&str> {
-        match self {
-            AwsRegion::Name(name) => Some(name),
-            AwsRegion::Source(_) => None,
-        }
-    }
+pub struct Region {
+    #[serde(default = "default_region_name")]
+    name: String,
 }
 
-impl Default for AwsRegion {
+impl Default for Region {
     fn default() -> Self {
-        Self::Name("us-east-1".to_owned())
+        Self {
+            name: default_region_name(),
+        }
     }
 }
 
-/// AWS region auto-discovery mechanism.
-#[derive(Clone, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)]
-#[serde(rename_all = "PascalCase")]
-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,
+fn default_region_name() -> String {
+    "us-east-1".into()
 }

crates/stackable-operator/src/config/fragment.rs

@@ -26,7 +26,7 @@ pub struct Validator<'a> {
     parent: Option<&'a Validator<'a>>,
 }
 
-impl<'a> Validator<'a> {
+impl Validator<'_> {
     /// Creates a `Validator` for a subfield of the current object
     pub fn field<'b>(&'b self, ident: &'b dyn Display) -> Validator<'b> {
         Validator {

crates/stackable-operator/src/config/merge.rs

@@ -146,7 +146,7 @@ impl Atomic for bool {}
 impl Atomic for String {}
 impl Atomic for Quantity {}
 impl Atomic for Duration {}
-impl<'a> Atomic for &'a str {}
+impl Atomic for &str {}
 impl Atomic for LabelSelector {}
 impl Atomic for PodAffinity {}
 impl Atomic for PodAntiAffinity {}

crates/stackable-operator/src/cpu.rs

@@ -70,7 +70,7 @@ impl<'de> Deserialize<'de> for CpuQuantity {
     {
         struct CpuQuantityVisitor;
 
-        impl<'de> Visitor<'de> for CpuQuantityVisitor {
+        impl Visitor<'_> for CpuQuantityVisitor {
             type Value = CpuQuantity;
 
             fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {

crates/stackable-operator/src/memory.rs

@@ -347,7 +347,7 @@ impl<'de> Deserialize<'de> for MemoryQuantity {
     {
         struct MemoryQuantityVisitor;
 
-        impl<'de> Visitor<'de> for MemoryQuantityVisitor {
+        impl Visitor<'_> for MemoryQuantityVisitor {
             type Value = MemoryQuantity;
 
             fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {

crates/stackable-operator/src/status/condition/operations.rs

@@ -13,7 +13,7 @@ pub struct ClusterOperationsConditionBuilder<'a> {
     cluster_operation: &'a ClusterOperation,
 }
 
-impl<'a> ConditionBuilder for ClusterOperationsConditionBuilder<'a> {
+impl ConditionBuilder for ClusterOperationsConditionBuilder<'_> {
     fn build_conditions(&self) -> ClusterConditionSet {
         vec![self.reconciliation_paused(), self.cluster_stopped()].into()
     }

crates/stackable-operator/src/time/serde_impl.rs

@@ -4,7 +4,7 @@ use crate::time::Duration;
 
 struct DurationVisitor;
 
-impl<'de> Visitor<'de> for DurationVisitor {
+impl Visitor<'_> for DurationVisitor {
     type Value = Duration;
 
     fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {

crates/stackable-telemetry/src/instrumentation/axum/extractor.rs

@@ -18,7 +18,7 @@ use opentelemetry::{propagation::Extractor, Context};
 /// [4]: https://docs.rs/opentelemetry-http/latest/opentelemetry_http/struct.HeaderExtractor.html
 pub struct HeaderExtractor<'a>(pub(crate) &'a HeaderMap);
 
-impl<'a> Extractor for HeaderExtractor<'a> {
+impl Extractor for HeaderExtractor<'_> {
     fn get(&self, key: &str) -> Option<&str> {
         self.0
             .get(key)

crates/stackable-telemetry/src/instrumentation/axum/injector.rs

@@ -20,7 +20,7 @@ use opentelemetry::{propagation::Injector, Context};
 /// [5]: https://docs.rs/opentelemetry-http/latest/opentelemetry_http/struct.HeaderInjector.html
 pub struct HeaderInjector<'a>(pub(crate) &'a mut HeaderMap);
 
-impl<'a> Injector for HeaderInjector<'a> {
+impl Injector for HeaderInjector<'_> {
     fn set(&mut self, key: &str, value: String) {
         if let Ok(header_name) = HeaderName::from_bytes(key.as_bytes()) {
             if let Ok(header_value) = HeaderValue::from_str(&value) {

crates/stackable-versioned-macros/src/codegen/changes.rs

@@ -66,7 +66,7 @@ where
     where
         F: Fn(&V) -> bool,
     {
-        self.get(key).map_or(false, f)
+        self.get(key).is_some_and(f)
     }
 
     fn lo_bound(&self, bound: Bound<&K>) -> Option<(&K, &V)> {

crates/stackable-versioned-macros/src/codegen/container/enum.rs

@@ -34,7 +34,7 @@ impl Container {
                 .common
                 .options
                 .skip
-                .map_or(false, |s| s.from.is_present()),
+                .is_some_and(|s| s.from.is_present()),
         };
 
         let idents = ContainerIdents::from(item_enum.ident, None);
@@ -68,10 +68,7 @@ impl Container {
 
         let options = ContainerOptions {
             kubernetes_options: None,
-            skip_from: attributes
-                .options
-                .skip
-                .map_or(false, |s| s.from.is_present()),
+            skip_from: attributes.options.skip.is_some_and(|s| s.from.is_present()),
         };
 
         let idents = ContainerIdents::from(item_enum.ident, None);
@@ -209,7 +206,7 @@ impl Enum {
         // cannot be deprecated). Then we retrieve the status of the variant and
         // ensure it is deprecated.
         self.variants.iter().any(|f| {
-            f.changes.as_ref().map_or(false, |c| {
+            f.changes.as_ref().is_some_and(|c| {
                 c.value_is(&version.inner, |a| {
                     matches!(
                         a,

crates/stackable-versioned-macros/src/codegen/container/mod.rs

@@ -302,7 +302,7 @@ impl From<KubernetesArguments> for KubernetesOptions {
                 .map_or_else(KubernetesCrateOptions::default, |crates| crates.into()),
             status: args.status,
             shortnames: args.shortnames,
-            skip_merged_crd: args.skip.map_or(false, |s| s.merged_crd.is_present()),
+            skip_merged_crd: args.skip.is_some_and(|s| s.merged_crd.is_present()),
         }
     }
 }

crates/stackable-versioned-macros/src/codegen/container/struct.rs

@@ -47,7 +47,7 @@ impl Container {
                 .common
                 .options
                 .skip
-                .map_or(false, |s| s.from.is_present()),
+                .is_some_and(|s| s.from.is_present()),
             kubernetes_options,
         };
 
@@ -90,10 +90,7 @@ impl Container {
         }
 
         let options = ContainerOptions {
-            skip_from: attributes
-                .options
-                .skip
-                .map_or(false, |s| s.from.is_present()),
+            skip_from: attributes.options.skip.is_some_and(|s| s.from.is_present()),
             kubernetes_options,
         };
 
@@ -245,7 +242,7 @@ impl Struct {
         // cannot be deprecated). Then we retrieve the status of the field and
         // ensure it is deprecated.
         self.fields.iter().any(|f| {
-            f.changes.as_ref().map_or(false, |c| {
+            f.changes.as_ref().is_some_and(|c| {
                 c.value_is(&version.inner, |a| {
                     matches!(
                         a,

crates/stackable-versioned-macros/src/codegen/mod.rs

@@ -36,7 +36,7 @@ impl From<&StandaloneContainerAttributes> for Vec<VersionDefinition> {
             .versions
             .iter()
             .map(|v| VersionDefinition {
-                skip_from: v.skip.as_ref().map_or(false, |s| s.from.is_present()),
+                skip_from: v.skip.as_ref().is_some_and(|s| s.from.is_present()),
                 ident: format_ident!("{version}", version = v.name.to_string()).into(),
                 deprecated: v.deprecated.as_ref().map(|r#override| {
                     r#override
@@ -57,7 +57,7 @@ impl From<&ModuleAttributes> for Vec<VersionDefinition> {
             .versions
             .iter()
             .map(|v| VersionDefinition {
-                skip_from: v.skip.as_ref().map_or(false, |s| s.from.is_present()),
+                skip_from: v.skip.as_ref().is_some_and(|s| s.from.is_present()),
                 ident: format_ident!("{version}", version = v.name.to_string()).into(),
                 deprecated: v.deprecated.as_ref().map(|r#override| {
                     r#override

crates/stackable-versioned-macros/src/codegen/module.rs

@@ -53,7 +53,7 @@ impl Module {
             .options
             .skip
             .as_ref()
-            .map_or(false, |opts| opts.from.is_present());
+            .is_some_and(|opts| opts.from.is_present());
 
         let mut errors = Error::accumulator();
         let mut submodules = HashMap::new();