From 9fc13ee1f0dcfcfb7102a2c0664a44a660628b9e Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 14 Mar 2025 12:40:57 +0100 Subject: [PATCH 1/7] feat: Warn when a non-default region is configured --- CHANGELOG.md | 3 ++- Cargo.lock | 8 ++++---- Cargo.toml | 2 +- docs/modules/druid/partials/s3-note.adoc | 2 +- rust/operator-binary/src/druid_controller.rs | 11 +++++++++++ 5 files changed, 19 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c89a8960..9145af4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,8 @@ All notable changes to this project will be documented in this file. - BREAKING: Adjust default memory limits of coordinator from `512Mi` to `768Mi` and middlemanager from `1Gi` to `1500Mi` ([#685]). - Support configuring JVM arguments ([#693]). - Add `region.name` field in S3Connection. - This field is **ignored** by this operator, see [ingestion] and [deep storage] documentation ([#695]). + This field is **ignored** by this operator, see [ingestion] and [deep storage] documentation. + A warning is emitted when a non-default endpoint is used ([#695], [#XXX]). ### Changed diff --git a/Cargo.lock b/Cargo.lock index 4ede70ee..8c4a6ed8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2432,8 +2432,8 @@ dependencies = [ [[package]] name = "stackable-operator" -version = "0.87.2" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.87.2#bc176bfc23f15533cdb3b7a7e7a773d4f29891e1" +version = "0.87.3" +source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.87.3#5b417a52d69deefc9a2eda9de73bd4c6708d7754" dependencies = [ "chrono", "clap", @@ -2471,7 +2471,7 @@ dependencies = [ [[package]] name = "stackable-operator-derive" version = "0.3.1" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.87.2#bc176bfc23f15533cdb3b7a7e7a773d4f29891e1" +source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.87.3#5b417a52d69deefc9a2eda9de73bd4c6708d7754" dependencies = [ "darling", "proc-macro2", @@ -2482,7 +2482,7 @@ dependencies = [ [[package]] name = "stackable-shared" version = "0.0.1" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.87.2#bc176bfc23f15533cdb3b7a7e7a773d4f29891e1" +source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.87.3#5b417a52d69deefc9a2eda9de73bd4c6708d7754" dependencies = [ "kube", "semver", diff --git a/Cargo.toml b/Cargo.toml index d453fdc8..0b12d078 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,7 @@ repository = "https://github.com/stackabletech/druid-operator" [workspace.dependencies] stackable-versioned = { git = "https://github.com/stackabletech/operator-rs.git", features = ["k8s"], tag = "stackable-versioned-0.6.0" } -stackable-operator = { git = "https://github.com/stackabletech/operator-rs.git", tag = "stackable-operator-0.87.2" } +stackable-operator = { git = "https://github.com/stackabletech/operator-rs.git", tag = "stackable-operator-0.87.3" } product-config = { git = "https://github.com/stackabletech/product-config.git", tag = "0.7.0" } anyhow = "1.0" diff --git a/docs/modules/druid/partials/s3-note.adoc b/docs/modules/druid/partials/s3-note.adoc index 6d9ebaca..0e6a0262 100644 --- a/docs/modules/druid/partials/s3-note.adoc +++ b/docs/modules/druid/partials/s3-note.adoc @@ -3,7 +3,7 @@ You can specify just a connection/bucket for either ingestion or deep storage or for both, but Druid only supports a single S3 connection under the hood. If two connections are specified, they must be the same. This is easiest if a dedicated S3 Connection Resource is used - not defined inline but as a dedicated object. -The connection/bucket `region.name` field is ignored because Druid uses the AWS SDK v1, which ignores the region if the endpoint is set. +The `S3Connection` `region` field is ignored, because Druid uses the AWS SDK v1, which ignores the region if the endpoint is set. The host is a required field, therefore the endpoint will always be set. TLS for S3 is not yet supported. diff --git a/rust/operator-binary/src/druid_controller.rs b/rust/operator-binary/src/druid_controller.rs index 54f49f0b..58086960 100644 --- a/rust/operator-binary/src/druid_controller.rs +++ b/rust/operator-binary/src/druid_controller.rs @@ -734,6 +734,17 @@ fn build_rolegroup_config_map( }; if let Some(s3) = s3_conn { + if !s3.region.is_default_config() { + // Raising this as warning instead of returning an error, better safe than sorry. + // It might still work out for the user. + tracing::warn!( + ?s3.region, + "You configured a non-default region on the S3Connection. + The S3Connection region field is ignored, because Druid uses the AWS SDK v1, which ignores the region if the endpoint is set. \ + The host is a required field, therefore the endpoint will always be set." + ) + } + conf.insert( S3_ENDPOINT_URL.to_string(), Some(s3.endpoint().context(ConfigureS3Snafu)?.to_string()), From af2ea7c8b65b03c2760dad035d3641b7727944fc Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 14 Mar 2025 12:42:07 +0100 Subject: [PATCH 2/7] changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9145af4c..79eaeb4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ All notable changes to this project will be documented in this file. - Support configuring JVM arguments ([#693]). - Add `region.name` field in S3Connection. This field is **ignored** by this operator, see [ingestion] and [deep storage] documentation. - A warning is emitted when a non-default endpoint is used ([#695], [#XXX]). + A warning is emitted when a non-default endpoint is used ([#695], [#700]). ### Changed @@ -29,6 +29,7 @@ All notable changes to this project will be documented in this file. [#693]: https://github.com/stackabletech/druid-operator/pull/693 [#685]: https://github.com/stackabletech/druid-operator/pull/685 [#695]: https://github.com/stackabletech/druid-operator/pull/695 +[#700]: https://github.com/stackabletech/druid-operator/pull/700 [ingestion]: https://docs.stackable.tech/home/nightly/druid/usage-guide/ingestion/ [deep storage]: https://docs.stackable.tech/home/nightly/druid/usage-guide/deep-storage/ From 25ed2c93042e979e0e40b439712417d0a41ac643 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 14 Mar 2025 12:43:10 +0100 Subject: [PATCH 3/7] change warning --- rust/operator-binary/src/druid_controller.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/operator-binary/src/druid_controller.rs b/rust/operator-binary/src/druid_controller.rs index 58086960..870097c2 100644 --- a/rust/operator-binary/src/druid_controller.rs +++ b/rust/operator-binary/src/druid_controller.rs @@ -738,7 +738,7 @@ fn build_rolegroup_config_map( // Raising this as warning instead of returning an error, better safe than sorry. // It might still work out for the user. tracing::warn!( - ?s3.region, + region = ?s3.region, "You configured a non-default region on the S3Connection. The S3Connection region field is ignored, because Druid uses the AWS SDK v1, which ignores the region if the endpoint is set. \ The host is a required field, therefore the endpoint will always be set." From 9071fffc339fa2ffd837390a481e809c3347a811 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 14 Mar 2025 12:44:17 +0100 Subject: [PATCH 4/7] typo --- docs/modules/druid/partials/s3-note.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/modules/druid/partials/s3-note.adoc b/docs/modules/druid/partials/s3-note.adoc index 0e6a0262..23ad4b7b 100644 --- a/docs/modules/druid/partials/s3-note.adoc +++ b/docs/modules/druid/partials/s3-note.adoc @@ -3,7 +3,7 @@ You can specify just a connection/bucket for either ingestion or deep storage or for both, but Druid only supports a single S3 connection under the hood. If two connections are specified, they must be the same. This is easiest if a dedicated S3 Connection Resource is used - not defined inline but as a dedicated object. -The `S3Connection` `region` field is ignored, because Druid uses the AWS SDK v1, which ignores the region if the endpoint is set. +The `S3Connection` `region` field is ignored because Druid uses the AWS SDK v1, which ignores the region if the endpoint is set. The host is a required field, therefore the endpoint will always be set. TLS for S3 is not yet supported. From 4a3990425d2ef749b8b316605ff55210982e87ac Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 14 Mar 2025 12:44:49 +0100 Subject: [PATCH 5/7] typo part 2 --- rust/operator-binary/src/druid_controller.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/operator-binary/src/druid_controller.rs b/rust/operator-binary/src/druid_controller.rs index 870097c2..8ba9e6ee 100644 --- a/rust/operator-binary/src/druid_controller.rs +++ b/rust/operator-binary/src/druid_controller.rs @@ -740,7 +740,7 @@ fn build_rolegroup_config_map( tracing::warn!( region = ?s3.region, "You configured a non-default region on the S3Connection. - The S3Connection region field is ignored, because Druid uses the AWS SDK v1, which ignores the region if the endpoint is set. \ + The S3Connection region field is ignored because Druid uses the AWS SDK v1, which ignores the region if the endpoint is set. \ The host is a required field, therefore the endpoint will always be set." ) } From e835a36d957616fc7ac93192bde2e1e1fde35156 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 14 Mar 2025 12:56:02 +0100 Subject: [PATCH 6/7] make regenerate-nix --- Cargo.nix | 14 +++++++------- crate-hashes.json | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Cargo.nix b/Cargo.nix index 21ea7ea8..a8e8c45f 100644 --- a/Cargo.nix +++ b/Cargo.nix @@ -7534,13 +7534,13 @@ rec { }; "stackable-operator" = rec { crateName = "stackable-operator"; - version = "0.87.2"; + version = "0.87.3"; edition = "2021"; workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "bc176bfc23f15533cdb3b7a7e7a773d4f29891e1"; - sha256 = "0cqz1xmj3vbm5hm9x6wbgg2l265s30j5j5609wmg68p6giywh82a"; + rev = "5b417a52d69deefc9a2eda9de73bd4c6708d7754"; + sha256 = "1vm2ahxpxc7va0jy40g8mxg1pqpv7by2cn2l81ba7an2lhdhyg99"; }; libName = "stackable_operator"; authors = [ @@ -7699,8 +7699,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "bc176bfc23f15533cdb3b7a7e7a773d4f29891e1"; - sha256 = "0cqz1xmj3vbm5hm9x6wbgg2l265s30j5j5609wmg68p6giywh82a"; + rev = "5b417a52d69deefc9a2eda9de73bd4c6708d7754"; + sha256 = "1vm2ahxpxc7va0jy40g8mxg1pqpv7by2cn2l81ba7an2lhdhyg99"; }; procMacro = true; libName = "stackable_operator_derive"; @@ -7734,8 +7734,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech/operator-rs.git"; - rev = "bc176bfc23f15533cdb3b7a7e7a773d4f29891e1"; - sha256 = "0cqz1xmj3vbm5hm9x6wbgg2l265s30j5j5609wmg68p6giywh82a"; + rev = "5b417a52d69deefc9a2eda9de73bd4c6708d7754"; + sha256 = "1vm2ahxpxc7va0jy40g8mxg1pqpv7by2cn2l81ba7an2lhdhyg99"; }; libName = "stackable_shared"; authors = [ diff --git a/crate-hashes.json b/crate-hashes.json index 154e76bd..f9585b1c 100644 --- a/crate-hashes.json +++ b/crate-hashes.json @@ -1,7 +1,7 @@ { - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.87.2#stackable-operator-derive@0.3.1": "0cqz1xmj3vbm5hm9x6wbgg2l265s30j5j5609wmg68p6giywh82a", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.87.2#stackable-operator@0.87.2": "0cqz1xmj3vbm5hm9x6wbgg2l265s30j5j5609wmg68p6giywh82a", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.87.2#stackable-shared@0.0.1": "0cqz1xmj3vbm5hm9x6wbgg2l265s30j5j5609wmg68p6giywh82a", + "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.87.3#stackable-operator-derive@0.3.1": "1vm2ahxpxc7va0jy40g8mxg1pqpv7by2cn2l81ba7an2lhdhyg99", + "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.87.3#stackable-operator@0.87.3": "1vm2ahxpxc7va0jy40g8mxg1pqpv7by2cn2l81ba7an2lhdhyg99", + "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.87.3#stackable-shared@0.0.1": "1vm2ahxpxc7va0jy40g8mxg1pqpv7by2cn2l81ba7an2lhdhyg99", "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-versioned-0.6.0#k8s-version@0.1.2": "0asgwj93drwvqsgd5c563qawjc3avb42nav0i5dgs4zv8bldx6x0", "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-versioned-0.6.0#stackable-versioned-macros@0.6.0": "0asgwj93drwvqsgd5c563qawjc3avb42nav0i5dgs4zv8bldx6x0", "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-versioned-0.6.0#stackable-versioned@0.6.0": "0asgwj93drwvqsgd5c563qawjc3avb42nav0i5dgs4zv8bldx6x0", From 2073a1a9348dea6f0eaa391600cde1ba5ad0eff6 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 14 Mar 2025 13:20:32 +0100 Subject: [PATCH 7/7] clippy --- rust/operator-binary/src/crd/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index ddb38f24..417a67e5 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -389,6 +389,7 @@ impl v1alpha1::DruidCluster { Ok(result) } + #[allow(clippy::type_complexity)] pub fn build_role_properties( &self, ) -> HashMap<