From 605bb90d78b3cca717fe217aca8aec0dff5949e8 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 22 Mar 2024 11:42:20 +0100 Subject: [PATCH 01/10] fix: Fixed wrong schema for `config.affinity.nodeSelector` --- CHANGELOG.md | 4 ++++ src/commons/affinity.rs | 17 +++++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a0be789e..0e6a1bf0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ All notable changes to this project will be documented in this file. - Add `stackable_webhook` crate which provides utilities to create webhooks with TLS termination ([#730]). - Add `ConversionReview` re-export in `stackable_webhook` crate ([#749]). +## Fixed + +- Fixed wrong schema (and thus CRD) for `config.affinity.nodeSelector`, making it unusable ([#XXX]). + [#730]: https://github.com/stackabletech/operator-rs/pull/730 [#749]: https://github.com/stackabletech/operator-rs/pull/749 diff --git a/src/commons/affinity.rs b/src/commons/affinity.rs index 6bbaf8ff0..45bfa10ca 100644 --- a/src/commons/affinity.rs +++ b/src/commons/affinity.rs @@ -6,7 +6,7 @@ use k8s_openapi::{ }, apimachinery::pkg::apis::meta::v1::LabelSelector, }; -use schemars::JsonSchema; +use schemars::{schema::Schema, JsonSchema}; use serde::{Deserialize, Serialize}; use stackable_operator_derive::Fragment; @@ -39,10 +39,19 @@ pub struct StackableAffinity { pub pod_affinity: Option, pub pod_anti_affinity: Option, pub node_affinity: Option, + #[schemars(schema_with = "stackable_node_selector_schema")] + #[fragment_attrs(schemars(schema_with = "stackable_node_selector_schema"))] pub node_selector: Option, } -#[derive(Clone, Debug, Eq, Deserialize, JsonSchema, PartialEq, Serialize)] +/// We can not simply use [`BTreeMap`] in [`StackableAffinity`], as the fields needs to be [`Atomic`]. +/// We can not mark it as [`Atomic`], as [`crate::config::fragment::FromFragment`] is already implemented for +/// [`BTreeMap`]. +/// +/// We `#[serde(flatten)]` the contained [`BTreeMap`], so `serde_yaml` can deserialize everything as +/// expected. However the generated JsonSchema will be wrong, so we need to provide our custom one (see +/// for details). +#[derive(Clone, Debug, Eq, Deserialize, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub struct StackableNodeSelector { #[serde(flatten)] @@ -51,6 +60,10 @@ pub struct StackableNodeSelector { impl Atomic for StackableNodeSelector {} +pub fn stackable_node_selector_schema(gen: &mut schemars::gen::SchemaGenerator) -> Schema { + Option::>::json_schema(gen) +} + /// Creates a `WeightedPodAffinityTerm`, which expresses a affinity towards all Pods of the given product (`app_name`) instance (`cluster_name`) role (`role`). /// This affinity can be used to attract towards (affinity) or away (anti-affinity) from the specified role. /// One common example would be to use this to distribute all the Pods of a certain role, e.g. hdfs datanodes. From 805329030fdea610de45df8f0514a7cf129cebfe Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 22 Mar 2024 11:44:44 +0100 Subject: [PATCH 02/10] changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e6a1bf0c..dadf92d69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,10 +11,11 @@ All notable changes to this project will be documented in this file. ## Fixed -- Fixed wrong schema (and thus CRD) for `config.affinity.nodeSelector`, making it unusable ([#XXX]). +- Fixed wrong schema (and thus CRD) for `config.affinity.nodeSelector`, making it unusable ([#752]). [#730]: https://github.com/stackabletech/operator-rs/pull/730 [#749]: https://github.com/stackabletech/operator-rs/pull/749 +[#752]: https://github.com/stackabletech/operator-rs/pull/752 ## Changed From 91077370dea406168034c76be2c5d7a65210939f Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 22 Mar 2024 11:56:35 +0100 Subject: [PATCH 03/10] Update CHANGELOG.md Co-authored-by: Techassi --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dadf92d69..c36d5b0c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ All notable changes to this project will be documented in this file. ## Fixed -- Fixed wrong schema (and thus CRD) for `config.affinity.nodeSelector`, making it unusable ([#752]). +- Fix wrong schema (and thus CRD) for `config.affinity.nodeSelector` ([#752]). [#730]: https://github.com/stackabletech/operator-rs/pull/730 [#749]: https://github.com/stackabletech/operator-rs/pull/749 From 68f85f37985ef8ab4ed31448c2a8233e630b3170 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 22 Mar 2024 12:03:15 +0100 Subject: [PATCH 04/10] Add docs on stackable_node_selector_schema --- crates/stackable-operator/src/commons/affinity.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/stackable-operator/src/commons/affinity.rs b/crates/stackable-operator/src/commons/affinity.rs index 45bfa10ca..818ae36b7 100644 --- a/crates/stackable-operator/src/commons/affinity.rs +++ b/crates/stackable-operator/src/commons/affinity.rs @@ -60,6 +60,7 @@ pub struct StackableNodeSelector { impl Atomic for StackableNodeSelector {} +/// We need a custom JsonSchema for [`StackableNodeSelector`], please have a look at the documentation there. pub fn stackable_node_selector_schema(gen: &mut schemars::gen::SchemaGenerator) -> Schema { Option::>::json_schema(gen) } From 2ae3ac962ddb36a25722126fae9d285d12f75ab4 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Fri, 22 Mar 2024 12:07:38 +0100 Subject: [PATCH 05/10] Rename stackable_node_selector_schema -> optional_stackable_node_selector_schema --- crates/stackable-operator/src/commons/affinity.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/stackable-operator/src/commons/affinity.rs b/crates/stackable-operator/src/commons/affinity.rs index 818ae36b7..32fde9024 100644 --- a/crates/stackable-operator/src/commons/affinity.rs +++ b/crates/stackable-operator/src/commons/affinity.rs @@ -39,8 +39,8 @@ pub struct StackableAffinity { pub pod_affinity: Option, pub pod_anti_affinity: Option, pub node_affinity: Option, - #[schemars(schema_with = "stackable_node_selector_schema")] - #[fragment_attrs(schemars(schema_with = "stackable_node_selector_schema"))] + #[schemars(schema_with = "optional_stackable_node_selector_schema")] + #[fragment_attrs(schemars(schema_with = "optional_stackable_node_selector_schema"))] pub node_selector: Option, } @@ -61,7 +61,7 @@ pub struct StackableNodeSelector { impl Atomic for StackableNodeSelector {} /// We need a custom JsonSchema for [`StackableNodeSelector`], please have a look at the documentation there. -pub fn stackable_node_selector_schema(gen: &mut schemars::gen::SchemaGenerator) -> Schema { +pub fn optional_stackable_node_selector_schema(gen: &mut schemars::gen::SchemaGenerator) -> Schema { Option::>::json_schema(gen) } From 861bfc26dfd4e1fcff147dfdfa28a078642e55c3 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Mon, 25 Mar 2024 11:56:25 +0100 Subject: [PATCH 06/10] USe #[schemars(deny_unknown_fields)] --- .../stackable-operator/src/commons/affinity.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/crates/stackable-operator/src/commons/affinity.rs b/crates/stackable-operator/src/commons/affinity.rs index 32fde9024..be05da763 100644 --- a/crates/stackable-operator/src/commons/affinity.rs +++ b/crates/stackable-operator/src/commons/affinity.rs @@ -6,7 +6,7 @@ use k8s_openapi::{ }, apimachinery::pkg::apis::meta::v1::LabelSelector, }; -use schemars::{schema::Schema, JsonSchema}; +use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use stackable_operator_derive::Fragment; @@ -39,8 +39,6 @@ pub struct StackableAffinity { pub pod_affinity: Option, pub pod_anti_affinity: Option, pub node_affinity: Option, - #[schemars(schema_with = "optional_stackable_node_selector_schema")] - #[fragment_attrs(schemars(schema_with = "optional_stackable_node_selector_schema"))] pub node_selector: Option, } @@ -49,10 +47,13 @@ pub struct StackableAffinity { /// [`BTreeMap`]. /// /// We `#[serde(flatten)]` the contained [`BTreeMap`], so `serde_yaml` can deserialize everything as -/// expected. However the generated JsonSchema will be wrong, so we need to provide our custom one (see -/// for details). -#[derive(Clone, Debug, Eq, Deserialize, PartialEq, Serialize)] +/// expected. +// FIXME: However, the generated JsonSchema will be wrong, so we need to use `#[schemars(deny_unknown_fields)]`. +// See https://github.com/stackabletech/operator-rs/pull/752#issuecomment-2017630433 and +// https://github.com/GREsau/schemars/issues/259 for details. +#[derive(Clone, Debug, Eq, Deserialize, JsonSchema, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] +#[schemars(deny_unknown_fields)] pub struct StackableNodeSelector { #[serde(flatten)] pub node_selector: BTreeMap, @@ -60,11 +61,6 @@ pub struct StackableNodeSelector { impl Atomic for StackableNodeSelector {} -/// We need a custom JsonSchema for [`StackableNodeSelector`], please have a look at the documentation there. -pub fn optional_stackable_node_selector_schema(gen: &mut schemars::gen::SchemaGenerator) -> Schema { - Option::>::json_schema(gen) -} - /// Creates a `WeightedPodAffinityTerm`, which expresses a affinity towards all Pods of the given product (`app_name`) instance (`cluster_name`) role (`role`). /// This affinity can be used to attract towards (affinity) or away (anti-affinity) from the specified role. /// One common example would be to use this to distribute all the Pods of a certain role, e.g. hdfs datanodes. From ba50416afa9046099b56a3c7051509fc42be1f13 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Mon, 25 Mar 2024 11:57:39 +0100 Subject: [PATCH 07/10] improve wording --- crates/stackable-operator/src/commons/affinity.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/stackable-operator/src/commons/affinity.rs b/crates/stackable-operator/src/commons/affinity.rs index be05da763..cdd17e51c 100644 --- a/crates/stackable-operator/src/commons/affinity.rs +++ b/crates/stackable-operator/src/commons/affinity.rs @@ -48,9 +48,9 @@ pub struct StackableAffinity { /// /// We `#[serde(flatten)]` the contained [`BTreeMap`], so `serde_yaml` can deserialize everything as /// expected. -// FIXME: However, the generated JsonSchema will be wrong, so we need to use `#[schemars(deny_unknown_fields)]`. -// See https://github.com/stackabletech/operator-rs/pull/752#issuecomment-2017630433 and -// https://github.com/GREsau/schemars/issues/259 for details. +// FIXME: The generated JsonSchema will be wrong, so until https://github.com/GREsau/schemars/issues/259 is fixed, we +// need to use `#[schemars(deny_unknown_fields)]`. +// See https://github.com/stackabletech/operator-rs/pull/752#issuecomment-2017630433 for details. #[derive(Clone, Debug, Eq, Deserialize, JsonSchema, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] #[schemars(deny_unknown_fields)] From f05807d7bed5a94b53b9988fbabddfabcf43f8eb Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Mon, 25 Mar 2024 12:02:47 +0100 Subject: [PATCH 08/10] typo --- crates/stackable-operator/src/commons/affinity.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stackable-operator/src/commons/affinity.rs b/crates/stackable-operator/src/commons/affinity.rs index cdd17e51c..75f57a935 100644 --- a/crates/stackable-operator/src/commons/affinity.rs +++ b/crates/stackable-operator/src/commons/affinity.rs @@ -42,7 +42,7 @@ pub struct StackableAffinity { pub node_selector: Option, } -/// We can not simply use [`BTreeMap`] in [`StackableAffinity`], as the fields needs to be [`Atomic`]. +/// We can not simply use [`BTreeMap`] in [`StackableAffinity`], as the fields need to be [`Atomic`]. /// We can not mark it as [`Atomic`], as [`crate::config::fragment::FromFragment`] is already implemented for /// [`BTreeMap`]. /// From 84429466032dd0aacf507ad99f978a9923631c62 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Mon, 25 Mar 2024 15:22:29 +0100 Subject: [PATCH 09/10] changelog --- crates/stackable-operator/CHANGELOG.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index 767508e35..27146b33a 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] +## Fixed + +- Fix wrong schema (and thus CRD) for `config.affinity.nodeSelector` ([#752]). + +[#752]: https://github.com/stackabletech/operator-rs/pull/752 + ## [0.65.0] - 2024-03-25 ## Added @@ -11,13 +17,8 @@ All notable changes to this project will be documented in this file. - Add `stackable_webhook` crate which provides utilities to create webhooks with TLS termination ([#730]). - Add `ConversionReview` re-export in `stackable_webhook` crate ([#749]). -## Fixed - -- Fix wrong schema (and thus CRD) for `config.affinity.nodeSelector` ([#752]). - [#730]: https://github.com/stackabletech/operator-rs/pull/730 [#749]: https://github.com/stackabletech/operator-rs/pull/749 -[#752]: https://github.com/stackabletech/operator-rs/pull/752 ## Changed From 425e7344390cce600474a514ede29fcc8d0abbf3 Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Mon, 25 Mar 2024 15:23:04 +0100 Subject: [PATCH 10/10] doccomment -> normal comment --- crates/stackable-operator/src/commons/affinity.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/stackable-operator/src/commons/affinity.rs b/crates/stackable-operator/src/commons/affinity.rs index 75f57a935..7973f9daa 100644 --- a/crates/stackable-operator/src/commons/affinity.rs +++ b/crates/stackable-operator/src/commons/affinity.rs @@ -42,12 +42,12 @@ pub struct StackableAffinity { pub node_selector: Option, } -/// We can not simply use [`BTreeMap`] in [`StackableAffinity`], as the fields need to be [`Atomic`]. -/// We can not mark it as [`Atomic`], as [`crate::config::fragment::FromFragment`] is already implemented for -/// [`BTreeMap`]. -/// -/// We `#[serde(flatten)]` the contained [`BTreeMap`], so `serde_yaml` can deserialize everything as -/// expected. +// We can not simply use [`BTreeMap`] in [`StackableAffinity`], as the fields need to be [`Atomic`]. +// We can not mark it as [`Atomic`], as [`crate::config::fragment::FromFragment`] is already implemented for +// [`BTreeMap`]. +// +// We `#[serde(flatten)]` the contained [`BTreeMap`], so `serde_yaml` can deserialize everything as +// expected. // FIXME: The generated JsonSchema will be wrong, so until https://github.com/GREsau/schemars/issues/259 is fixed, we // need to use `#[schemars(deny_unknown_fields)]`. // See https://github.com/stackabletech/operator-rs/pull/752#issuecomment-2017630433 for details.