From 7d62fc7f2b448e692f16afad656c8bea626cc972 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 11 Oct 2024 15:02:25 +0200 Subject: [PATCH 01/12] Replace KeyValuePairs' internal BTreeSet with BTreeMap Fixes #887 --- .../src/kvp/annotation/mod.rs | 5 +- .../stackable-operator/src/kvp/label/mod.rs | 4 +- crates/stackable-operator/src/kvp/mod.rs | 58 ++++++++++++------- 3 files changed, 41 insertions(+), 26 deletions(-) diff --git a/crates/stackable-operator/src/kvp/annotation/mod.rs b/crates/stackable-operator/src/kvp/annotation/mod.rs index 295a4ec88..ed9ee4215 100644 --- a/crates/stackable-operator/src/kvp/annotation/mod.rs +++ b/crates/stackable-operator/src/kvp/annotation/mod.rs @@ -293,15 +293,14 @@ impl Annotations { pub fn contains_key(&self, key: impl TryInto) -> bool; /// Returns an [`Iterator`] over [`Annotations`] yielding a reference to every [`Annotation`] contained within. - pub fn iter(&self) -> impl Iterator>; - + pub fn iter(&self) -> impl Iterator> + '_; } } } impl IntoIterator for Annotations { type Item = KeyValuePair; - type IntoIter = std::collections::btree_set::IntoIter; + type IntoIter = as IntoIterator>::IntoIter; /// Returns a consuming [`Iterator`] over [`Annotations`] moving every [`Annotation`] out. /// The [`Annotations`] cannot be used again after calling this. diff --git a/crates/stackable-operator/src/kvp/label/mod.rs b/crates/stackable-operator/src/kvp/label/mod.rs index 7895e9c5f..ae38c12d5 100644 --- a/crates/stackable-operator/src/kvp/label/mod.rs +++ b/crates/stackable-operator/src/kvp/label/mod.rs @@ -385,7 +385,7 @@ impl Labels { pub fn contains_key(&self, key: impl TryInto) -> bool; /// Returns an [`Iterator`] over [`Labels`] yielding a reference to every [`Label`] contained within. - pub fn iter(&self) -> impl Iterator>; + pub fn iter(&self) -> impl Iterator> + '_; } } @@ -393,7 +393,7 @@ impl Labels { impl IntoIterator for Labels { type Item = KeyValuePair; - type IntoIter = std::collections::btree_set::IntoIter; + type IntoIter = as IntoIterator>::IntoIter; /// Returns a consuming [`Iterator`] over [`Labels`] moving every [`Label`] out. /// The [`Labels`] cannot be used again after calling this. diff --git a/crates/stackable-operator/src/kvp/mod.rs b/crates/stackable-operator/src/kvp/mod.rs index c37ce71c0..fff75b363 100644 --- a/crates/stackable-operator/src/kvp/mod.rs +++ b/crates/stackable-operator/src/kvp/mod.rs @@ -178,7 +178,7 @@ pub enum KeyValuePairsError { /// which ultimately prevent unncessary reconciliations due to changes /// in item order. #[derive(Clone, Debug, Default)] -pub struct KeyValuePairs(BTreeSet>); +pub struct KeyValuePairs(BTreeMap); impl TryFrom> for KeyValuePairs where @@ -224,7 +224,7 @@ where T: Value, { fn from_iter>>(iter: I) -> Self { - Self(iter.into_iter().collect()) + Self(iter.into_iter().map(|kvp| (kvp.key, kvp.value)).collect()) } } @@ -242,7 +242,7 @@ where .map(KeyValuePair::try_from) .collect::, KeyValuePairError>>()?; - Ok(Self(pairs)) + Ok(Self::from_iter(pairs)) } } @@ -253,7 +253,7 @@ where fn from(value: KeyValuePairs) -> Self { value .iter() - .map(|pair| (pair.key().to_string(), pair.value().to_string())) + .map(|(k, v)| (k.to_string(), v.to_string())) .collect() } } @@ -262,7 +262,7 @@ impl Deref for KeyValuePairs where T: Value, { - type Target = BTreeSet>; + type Target = BTreeMap; fn deref(&self) -> &Self::Target { &self.0 @@ -280,7 +280,7 @@ where /// Creates a new list of [`KeyValuePair`]s from `pairs`. pub fn new_with(pairs: BTreeSet>) -> Self { - Self(pairs) + Self::from_iter(pairs) } /// Extends `self` with `other`. @@ -295,16 +295,16 @@ where /// [`KeyValuePairs::contains_key`] before inserting or try to insert /// fallible via [`KeyValuePairs::try_insert`]. pub fn insert(&mut self, kvp: KeyValuePair) -> &mut Self { - self.0.insert(kvp); + self.0.insert(kvp.key, kvp.value); self } /// Tries to insert a new [`KeyValuePair`] into the list of pairs. /// - /// If the list already had this pair present, nothing is updated, and an + /// If the list already had this key present, nothing is updated, and an /// error is returned. pub fn try_insert(&mut self, kvp: KeyValuePair) -> Result<(), KeyValuePairsError> { - ensure!(!self.0.contains(&kvp), PairAlreadyExistsSnafu); + ensure!(!self.0.contains_key(&kvp.key), PairAlreadyExistsSnafu); self.insert(kvp); Ok(()) } @@ -314,7 +314,10 @@ where let Ok(kvp) = kvp.try_into() else { return false; }; - self.0.contains(&kvp) + let Some(value) = self.get(&kvp.key) else { + return false; + }; + value == &kvp.value } /// Returns if the list contains a key/value pair with a specific [`Key`]. @@ -323,18 +326,15 @@ where return false; }; - for kvp in &self.0 { - if kvp.key == key { - return true; - } - } - - false + self.0.contains_key(&key) } /// Returns an [`Iterator`] over [`KeyValuePairs`] yielding a reference to every [`KeyValuePair`] contained within. - pub fn iter(&self) -> impl Iterator> { - self.0.iter() + pub fn iter(&self) -> impl Iterator> + '_ { + self.0.iter().map(|(k, v)| KeyValuePair { + key: k.clone(), + value: v.clone(), + }) } } @@ -343,12 +343,15 @@ where T: Value, { type Item = KeyValuePair; - type IntoIter = std::collections::btree_set::IntoIter; + type IntoIter = + std::iter::Map, fn((Key, T)) -> Self::Item>; /// Returns a consuming [`Iterator`] over [`KeyValuePairs`] moving every [`KeyValuePair`] out. /// The [`KeyValuePairs`] cannot be used again after calling this. fn into_iter(self) -> Self::IntoIter { - self.0.into_iter() + self.0 + .into_iter() + .map(|(key, value)| KeyValuePair { key, value }) } } @@ -488,4 +491,17 @@ mod test { let report = Report::from_error(err); println!("{report}") } + + #[test] + fn merge() { + let mut merged_labels = + Labels::try_from_iter([("a", "b"), ("b", "a"), ("c", "c")]).unwrap(); + merged_labels.extend(Labels::try_from_iter([("a", "a"), ("b", "b"), ("d", "d")]).unwrap()); + assert_eq!( + BTreeMap::from(merged_labels), + BTreeMap::from( + Labels::try_from_iter([("a", "a"), ("b", "b"), ("c", "c"), ("d", "d")]).unwrap() + ) + ) + } } From f102e4d8ac665442bdcd04863e10bec054ceb03d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 11 Oct 2024 15:27:46 +0200 Subject: [PATCH 02/12] Rename PairAlreadyExists => KeyAlreadyExists --- crates/stackable-operator/src/kvp/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/stackable-operator/src/kvp/mod.rs b/crates/stackable-operator/src/kvp/mod.rs index fff75b363..5a233c603 100644 --- a/crates/stackable-operator/src/kvp/mod.rs +++ b/crates/stackable-operator/src/kvp/mod.rs @@ -152,8 +152,8 @@ where #[derive(Debug, PartialEq, Snafu)] pub enum KeyValuePairsError { - #[snafu(display("key/value pair already exists"))] - PairAlreadyExists, + #[snafu(display("key already exists"))] + KeyAlreadyExists, } /// A validated set/list of Kubernetes key/value pairs. @@ -304,7 +304,7 @@ where /// If the list already had this key present, nothing is updated, and an /// error is returned. pub fn try_insert(&mut self, kvp: KeyValuePair) -> Result<(), KeyValuePairsError> { - ensure!(!self.0.contains_key(&kvp.key), PairAlreadyExistsSnafu); + ensure!(!self.0.contains_key(&kvp.key), KeyAlreadyExistsSnafu); self.insert(kvp); Ok(()) } From f7ac175a8d8d929835954ce023a7146e84ce1920 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Mon, 14 Oct 2024 11:03:50 +0200 Subject: [PATCH 03/12] Reorganize kvp so that KeyValuePairs is an alias for BTreeMap instead of newtyping it --- crates/stackable-operator/src/builder/meta.rs | 15 +- crates/stackable-operator/src/builder/pdb.rs | 10 +- .../stackable-operator/src/builder/pod/mod.rs | 14 +- .../src/builder/pod/volume.rs | 42 ++- .../src/cluster_resources.rs | 8 +- .../src/commons/product_image_selection.rs | 4 +- .../src/kvp/annotation/mod.rs | 327 ++++------------- crates/stackable-operator/src/kvp/key.rs | 10 +- .../stackable-operator/src/kvp/label/mod.rs | 347 ++++-------------- crates/stackable-operator/src/kvp/mod.rs | 347 +++++------------- 10 files changed, 284 insertions(+), 840 deletions(-) diff --git a/crates/stackable-operator/src/builder/meta.rs b/crates/stackable-operator/src/builder/meta.rs index fbf23e793..009d6633e 100644 --- a/crates/stackable-operator/src/builder/meta.rs +++ b/crates/stackable-operator/src/builder/meta.rs @@ -3,7 +3,10 @@ use kube::{Resource, ResourceExt}; use snafu::{ResultExt, Snafu}; use tracing::warn; -use crate::kvp::{Annotation, Annotations, Label, LabelError, Labels, ObjectLabels}; +use crate::kvp::{ + label, Annotation, Annotations, KeyValuePairs, KeyValuePairsExt, Label, LabelError, Labels, + ObjectLabels, +}; type Result = std::result::Result; @@ -106,7 +109,7 @@ impl ObjectMetaBuilder { pub fn with_annotation(&mut self, annotation: Annotation) -> &mut Self { self.annotations .get_or_insert(Annotations::new()) - .insert(annotation); + .extend([annotation]); self } @@ -128,7 +131,7 @@ impl ObjectMetaBuilder { /// This adds a single label to the existing labels. /// It'll override a label with the same key. pub fn with_label(&mut self, label: Label) -> &mut Self { - self.labels.get_or_insert(Labels::new()).insert(label); + self.labels.get_or_insert(Labels::new()).extend([label]); self } @@ -154,7 +157,7 @@ impl ObjectMetaBuilder { object_labels: ObjectLabels, ) -> Result<&mut Self> { let recommended_labels = - Labels::recommended(object_labels).context(RecommendedLabelsSnafu)?; + label::sets::recommended(object_labels).context(RecommendedLabelsSnafu)?; self.labels .get_or_insert(Labels::new()) @@ -185,8 +188,8 @@ impl ObjectMetaBuilder { .ownerreference .as_ref() .map(|ownerreference| vec![ownerreference.clone()]), - labels: self.labels.clone().map(|l| l.into()), - annotations: self.annotations.clone().map(|a| a.into()), + labels: self.labels.as_ref().map(KeyValuePairs::to_unvalidated), + annotations: self.annotations.as_ref().map(KeyValuePairs::to_unvalidated), ..ObjectMeta::default() } } diff --git a/crates/stackable-operator/src/builder/pdb.rs b/crates/stackable-operator/src/builder/pdb.rs index d6ff52778..489da5689 100644 --- a/crates/stackable-operator/src/builder/pdb.rs +++ b/crates/stackable-operator/src/builder/pdb.rs @@ -10,7 +10,7 @@ use snafu::{ResultExt, Snafu}; use crate::{ builder::meta::ObjectMetaBuilder, - kvp::{Label, Labels}, + kvp::{label, KeyValuePairsExt}, }; type Result = std::result::Result; @@ -86,9 +86,9 @@ impl PodDisruptionBudgetBuilder<(), (), ()> { controller_name: &str, ) -> Result> { let role_selector_labels = - Labels::role_selector(owner, app_name, role).context(RoleSelectorLabelsSnafu)?; - let managed_by_label = - Label::managed_by(operator_name, controller_name).context(ManagedByLabelSnafu)?; + label::sets::role_selector(owner, app_name, role).context(RoleSelectorLabelsSnafu)?; + let managed_by_label = label::well_known::managed_by(operator_name, controller_name) + .context(ManagedByLabelSnafu)?; let metadata = ObjectMetaBuilder::new() .namespace_opt(owner.namespace()) .name(format!("{}-{}", owner.name_any(), role)) @@ -102,7 +102,7 @@ impl PodDisruptionBudgetBuilder<(), (), ()> { metadata, selector: LabelSelector { match_expressions: None, - match_labels: Some(role_selector_labels.into()), + match_labels: Some(role_selector_labels.to_unvalidated()), }, ..PodDisruptionBudgetBuilder::default() }) diff --git a/crates/stackable-operator/src/builder/pod/mod.rs b/crates/stackable-operator/src/builder/pod/mod.rs index 54b34bd3a..d8b23c942 100644 --- a/crates/stackable-operator/src/builder/pod/mod.rs +++ b/crates/stackable-operator/src/builder/pod/mod.rs @@ -331,16 +331,17 @@ impl PodBuilder { /// ``` /// # use stackable_operator::builder::pod::PodBuilder; /// # use stackable_operator::builder::pod::container::ContainerBuilder; + /// # use stackable_operator::iter::TryFromIterator; /// # use stackable_operator::kvp::Labels; /// # use k8s_openapi::{ /// # apimachinery::pkg::apis::meta::v1::ObjectMeta, /// # }; /// # use std::collections::BTreeMap; /// - /// let labels: Labels = Labels::try_from( - /// BTreeMap::from([("app.kubernetes.io/component", "test-role"), + /// let labels: Labels = Labels::try_from_iter( + /// [("app.kubernetes.io/component", "test-role"), /// ("app.kubernetes.io/instance", "test"), - /// ("app.kubernetes.io/name", "test")])) + /// ("app.kubernetes.io/name", "test")]) /// .unwrap(); /// /// let pod = PodBuilder::new() @@ -418,16 +419,17 @@ impl PodBuilder { /// ``` /// # use stackable_operator::builder::pod::PodBuilder; /// # use stackable_operator::builder::pod::container::ContainerBuilder; + /// # use stackable_operator::iter::TryFromIterator; /// # use stackable_operator::kvp::Labels; /// # use k8s_openapi::{ /// # apimachinery::pkg::apis::meta::v1::ObjectMeta, /// # }; /// # use std::collections::BTreeMap; /// - /// let labels: Labels = Labels::try_from( - /// BTreeMap::from([("app.kubernetes.io/component", "test-role"), + /// let labels: Labels = Labels::try_from_iter( + /// [("app.kubernetes.io/component", "test-role"), /// ("app.kubernetes.io/instance", "test"), - /// ("app.kubernetes.io/name", "test")])) + /// ("app.kubernetes.io/name", "test")]) /// .unwrap(); /// /// let pod = PodBuilder::new() diff --git a/crates/stackable-operator/src/builder/pod/volume.rs b/crates/stackable-operator/src/builder/pod/volume.rs index 8f160d1ec..2d5a7fa2b 100644 --- a/crates/stackable-operator/src/builder/pod/volume.rs +++ b/crates/stackable-operator/src/builder/pod/volume.rs @@ -14,7 +14,7 @@ use tracing::warn; use crate::{ builder::meta::ObjectMetaBuilder, - kvp::{Annotation, AnnotationError, Annotations, LabelError, Labels}, + kvp::{annotation, Annotation, AnnotationError, Annotations, LabelError, Labels}, }; /// A builder to build [`Volume`] objects. May only contain one `volume_source` @@ -333,24 +333,32 @@ impl SecretOperatorVolumeSourceBuilder { pub fn build(&self) -> Result { let mut annotations = Annotations::new(); - annotations - .insert(Annotation::secret_class(&self.secret_class).context(ParseAnnotationSnafu)?); + annotations.extend([annotation::well_known::secret_volume::secret_class( + &self.secret_class, + ) + .context(ParseAnnotationSnafu)?]); if !self.scopes.is_empty() { - annotations - .insert(Annotation::secret_scope(&self.scopes).context(ParseAnnotationSnafu)?); + annotations.extend([ + annotation::well_known::secret_volume::secret_scope(&self.scopes) + .context(ParseAnnotationSnafu)?, + ]); } if let Some(format) = &self.format { - annotations - .insert(Annotation::secret_format(format.as_ref()).context(ParseAnnotationSnafu)?); + annotations.extend([annotation::well_known::secret_volume::secret_format( + format.as_ref(), + ) + .context(ParseAnnotationSnafu)?]); } if !self.kerberos_service_names.is_empty() { - annotations.insert( - Annotation::kerberos_service_names(&self.kerberos_service_names) - .context(ParseAnnotationSnafu)?, - ); + annotations.extend([ + annotation::well_known::secret_volume::kerberos_service_names( + &self.kerberos_service_names, + ) + .context(ParseAnnotationSnafu)?, + ]); } if let Some(password) = &self.tls_pkcs12_password { @@ -358,9 +366,10 @@ impl SecretOperatorVolumeSourceBuilder { if Some(SecretFormat::TlsPkcs12) != self.format { warn!(format.actual = ?self.format, format.expected = ?Some(SecretFormat::TlsPkcs12), "A TLS PKCS12 password was set but ignored because another format was requested") } else { - annotations.insert( - Annotation::tls_pkcs12_password(password).context(ParseAnnotationSnafu)?, - ); + annotations.extend([annotation::well_known::secret_volume::tls_pkcs12_password( + password, + ) + .context(ParseAnnotationSnafu)?]); } } @@ -450,7 +459,7 @@ pub enum ListenerOperatorVolumeSourceBuilderError { /// # use std::collections::BTreeMap; /// let mut pod_builder = PodBuilder::new(); /// -/// let labels: Labels = Labels::try_from(BTreeMap::::new()).unwrap(); +/// let labels: Labels = Labels::new(); /// /// let volume_source = /// ListenerOperatorVolumeSourceBuilder::new( @@ -555,7 +564,6 @@ impl ListenerOperatorVolumeSourceBuilder { mod tests { use super::*; use k8s_openapi::apimachinery::pkg::api::resource::Quantity; - use std::collections::BTreeMap; #[test] fn builder() { @@ -615,7 +623,7 @@ mod tests { #[test] fn listener_operator_volume_source_builder() { - let labels: Labels = Labels::try_from(BTreeMap::::new()).unwrap(); + let labels: Labels = Labels::new(); let builder = ListenerOperatorVolumeSourceBuilder::new( &ListenerReference::ListenerClass("public".into()), diff --git a/crates/stackable-operator/src/cluster_resources.rs b/crates/stackable-operator/src/cluster_resources.rs index 8cce2b099..62021971a 100644 --- a/crates/stackable-operator/src/cluster_resources.rs +++ b/crates/stackable-operator/src/cluster_resources.rs @@ -12,7 +12,7 @@ use crate::{ }, kvp::{ consts::{K8S_APP_INSTANCE_KEY, K8S_APP_MANAGED_BY_KEY, K8S_APP_NAME_KEY}, - Label, LabelError, Labels, + label, LabelError, Labels, }, utils::format_full_controller_name, }; @@ -464,12 +464,12 @@ impl ClusterResources { /// Return required labels for cluster resources to be uniquely identified for clean up. // TODO: This is a (quick-fix) helper method but should be replaced by better label handling pub fn get_required_labels(&self) -> Result { - let mut labels = Labels::common(&self.app_name, &self.app_instance)?; + let mut labels = label::sets::common(&self.app_name, &self.app_instance)?; - labels.insert(Label::managed_by( + labels.extend([label::well_known::managed_by( &self.operator_name, &self.controller_name, - )?); + )?]); Ok(labels) } diff --git a/crates/stackable-operator/src/commons/product_image_selection.rs b/crates/stackable-operator/src/commons/product_image_selection.rs index d5e02de3d..8399ba175 100644 --- a/crates/stackable-operator/src/commons/product_image_selection.rs +++ b/crates/stackable-operator/src/commons/product_image_selection.rs @@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize}; use strum::AsRefStr; #[cfg(doc)] -use crate::kvp::Labels; +use crate::kvp::label; pub const STACKABLE_DOCKER_REPO: &str = "docker.stackable.tech/stackable"; @@ -67,7 +67,7 @@ pub struct ResolvedProductImage { /// Version of the product, e.g. `1.4.1`. pub product_version: String, - /// App version as formatted for [`Labels::recommended`] + /// App version as formatted for [`label::sets::recommended`] pub app_version_label: String, /// Image to be used for the product image e.g. `docker.stackable.tech/stackable/superset:1.4.1-stackable2.1.0` diff --git a/crates/stackable-operator/src/kvp/annotation/mod.rs b/crates/stackable-operator/src/kvp/annotation/mod.rs index ed9ee4215..49d388712 100644 --- a/crates/stackable-operator/src/kvp/annotation/mod.rs +++ b/crates/stackable-operator/src/kvp/annotation/mod.rs @@ -9,26 +9,14 @@ //! //! See //! for more information on Kubernetes annotations. -use std::{ - collections::{BTreeMap, BTreeSet}, - convert::Infallible, - fmt::Display, -}; +use std::convert::Infallible; -use delegate::delegate; - -use crate::{ - builder::pod::volume::SecretOperatorVolumeScope, - iter::TryFromIterator, - kvp::{Key, KeyValuePair, KeyValuePairError, KeyValuePairs, KeyValuePairsError}, -}; +use crate::kvp::{KeyValuePair, KeyValuePairError, KeyValuePairs}; mod value; pub use value::*; -pub type AnnotationsError = KeyValuePairsError; - /// A type alias for errors returned when construction or manipulation of a set /// of annotations fails. pub type AnnotationError = KeyValuePairError; @@ -42,102 +30,7 @@ pub type AnnotationError = KeyValuePairError; /// /// See /// for more information on Kubernetes annotations. -#[derive(Debug)] -pub struct Annotation(KeyValuePair); - -impl TryFrom<(K, V)> for Annotation -where - K: AsRef, - V: AsRef, -{ - type Error = AnnotationError; - - fn try_from(value: (K, V)) -> Result { - let kvp = KeyValuePair::try_from(value)?; - Ok(Self(kvp)) - } -} - -impl Display for Annotation { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - self.0.fmt(f) - } -} - -impl Annotation { - /// Returns an immutable reference to the annotation's [`Key`]. - pub fn key(&self) -> &Key { - self.0.key() - } - - /// Returns an immutable reference to the annotation's value. - pub fn value(&self) -> &AnnotationValue { - self.0.value() - } - - /// Consumes self and returns the inner [`KeyValuePair`]. - pub fn into_inner(self) -> KeyValuePair { - self.0 - } - - /// Constructs a `secrets.stackable.tech/class` annotation. - pub fn secret_class(secret_class: &str) -> Result { - let kvp = KeyValuePair::try_from(("secrets.stackable.tech/class", secret_class))?; - Ok(Self(kvp)) - } - - /// Constructs a `secrets.stackable.tech/scope` annotation. - pub fn secret_scope( - scopes: impl AsRef<[SecretOperatorVolumeScope]>, - ) -> Result { - let mut value = String::new(); - - for scope in scopes.as_ref() { - if !value.is_empty() { - value.push(','); - } - - match scope { - SecretOperatorVolumeScope::Node => value.push_str("node"), - SecretOperatorVolumeScope::Pod => value.push_str("pod"), - SecretOperatorVolumeScope::Service { name } => { - value.push_str("service="); - value.push_str(name); - } - SecretOperatorVolumeScope::ListenerVolume { name } => { - value.push_str("listener-volume="); - value.push_str(name); - } - } - } - - let kvp = KeyValuePair::try_from(("secrets.stackable.tech/scope", value))?; - Ok(Self(kvp)) - } - - /// Constructs a `secrets.stackable.tech/format` annotation. - pub fn secret_format(format: &str) -> Result { - let kvp = KeyValuePair::try_from(("secrets.stackable.tech/format", format))?; - Ok(Self(kvp)) - } - - /// Constructs a `secrets.stackable.tech/kerberos.service.names` annotation. - pub fn kerberos_service_names(names: impl AsRef<[String]>) -> Result { - let names = names.as_ref().join(","); - let kvp = KeyValuePair::try_from(("secrets.stackable.tech/kerberos.service.names", names))?; - Ok(Self(kvp)) - } - - /// Constructs a `secrets.stackable.tech/format.compatibility.tls-pkcs12.password` - /// annotation. - pub fn tls_pkcs12_password(password: &str) -> Result { - let kvp = KeyValuePair::try_from(( - "secrets.stackable.tech/format.compatibility.tls-pkcs12.password", - password, - ))?; - Ok(Self(kvp)) - } -} +pub type Annotation = KeyValuePair; /// A validated set/list of Kubernetes annotations. /// @@ -150,181 +43,91 @@ impl Annotation { /// /// ``` /// # use std::collections::BTreeMap; +/// # use stackable_operator::iter::TryFromIterator; /// # use stackable_operator::kvp::Annotations; /// let map = BTreeMap::from([ /// ("stackable.tech/managed-by", "stackablectl"), /// ("stackable.tech/vendor", "Stäckable"), /// ]); /// -/// let labels = Annotations::try_from(map).unwrap(); +/// let labels = Annotations::try_from_iter(map).unwrap(); /// ``` /// /// ### Creating a list of labels from an array /// /// ``` +/// # use stackable_operator::iter::TryFromIterator; /// # use stackable_operator::kvp::Annotations; -/// let labels = Annotations::try_from([ +/// let labels = Annotations::try_from_iter([ /// ("stackable.tech/managed-by", "stackablectl"), /// ("stackable.tech/vendor", "Stäckable"), /// ]).unwrap(); /// ``` -#[derive(Clone, Debug, Default)] -pub struct Annotations(KeyValuePairs); - -impl TryFrom> for Annotations -where - K: AsRef, - V: AsRef, -{ - type Error = AnnotationError; - - fn try_from(map: BTreeMap) -> Result { - Self::try_from_iter(map) - } -} - -impl TryFrom<&BTreeMap> for Annotations -where - K: AsRef, - V: AsRef, -{ - type Error = AnnotationError; - - fn try_from(map: &BTreeMap) -> Result { - Self::try_from_iter(map) - } -} - -impl TryFrom<[(K, V); N]> for Annotations -where - K: AsRef, - V: AsRef, -{ - type Error = AnnotationError; - - fn try_from(array: [(K, V); N]) -> Result { - Self::try_from_iter(array) - } -} - -impl FromIterator> for Annotations { - fn from_iter>>(iter: T) -> Self { - let kvps = KeyValuePairs::from_iter(iter); - Self(kvps) - } -} - -impl TryFromIterator<(K, V)> for Annotations -where - K: AsRef, - V: AsRef, -{ - type Error = AnnotationError; - - fn try_from_iter>(iter: I) -> Result { - let kvps = KeyValuePairs::try_from_iter(iter)?; - Ok(Self(kvps)) - } -} - -impl From for BTreeMap { - fn from(value: Annotations) -> Self { - value.0.into() - } -} - -impl Annotations { - /// Creates a new empty list of [`Annotations`]. - pub fn new() -> Self { - Self::default() - } - - /// Creates a new list of [`Annotations`] from `pairs`. - pub fn new_with(pairs: BTreeSet>) -> Self { - Self(KeyValuePairs::new_with(pairs)) - } - - /// Tries to insert a new annotation by first parsing `annotation` as an - /// [`Annotation`] and then inserting it into the list. This function will - /// overwrite any existing annotation already present. - pub fn parse_insert( - &mut self, - annotation: impl TryInto, - ) -> Result<(), AnnotationError> { - self.0.insert(annotation.try_into()?.0); - Ok(()) - } - - /// Inserts a new [`Annotation`]. This function will overwrite any existing - /// annotation already present. - pub fn insert(&mut self, annotation: Annotation) -> &mut Self { - self.0.insert(annotation.0); - self - } - - // This forwards / delegates associated functions to the inner field. In - // this case self.0 which is of type KeyValuePairs. So calling - // Annotations::len() will be delegated to KeyValuePair::len() without - // the need to write boilerplate code. - delegate! { - to self.0 { - /// Tries to insert a new [`Annotation`]. It ensures there are no duplicate - /// entries. Trying to insert duplicated data returns an error. If no such - /// check is required, use [`Annotations::insert`] instead. - pub fn try_insert(&mut self, #[newtype] annotation: Annotation) -> Result<(), AnnotationsError>; - - /// Extends `self` with `other`. - pub fn extend(&mut self, #[newtype] other: Self); - - /// Returns the number of labels. - pub fn len(&self) -> usize; - - /// Returns if the set of labels is empty. - pub fn is_empty(&self) -> bool; - - /// Returns if the set of annotations contains the provided - /// `annotation`. Failure to parse/validate the [`KeyValuePair`] - /// will return `false`. - pub fn contains(&self, annotation: impl TryInto>) -> bool; - - /// Returns if the set of annotations contains a label with the - /// provided `key`. Failure to parse/validate the [`Key`] will - /// return `false`. - pub fn contains_key(&self, key: impl TryInto) -> bool; - - /// Returns an [`Iterator`] over [`Annotations`] yielding a reference to every [`Annotation`] contained within. - pub fn iter(&self) -> impl Iterator> + '_; +pub type Annotations = KeyValuePairs; + +/// Well-known annotations used by other tools or standard conventions. +pub mod well_known { + /// Annotations applicable to Stackable Secret Operator volumes + pub mod secret_volume { + use crate::{ + builder::pod::volume::SecretOperatorVolumeScope, + kvp::{Annotation, AnnotationError}, + }; + + /// Constructs a `secrets.stackable.tech/class` annotation. + pub fn secret_class(secret_class: &str) -> Result { + Annotation::try_from(("secrets.stackable.tech/class", secret_class)) } - } -} -impl IntoIterator for Annotations { - type Item = KeyValuePair; - type IntoIter = as IntoIterator>::IntoIter; + /// Constructs a `secrets.stackable.tech/scope` annotation. + pub fn secret_scope( + scopes: &[SecretOperatorVolumeScope], + ) -> Result { + let mut value = String::new(); - /// Returns a consuming [`Iterator`] over [`Annotations`] moving every [`Annotation`] out. - /// The [`Annotations`] cannot be used again after calling this. - fn into_iter(self) -> Self::IntoIter { - self.0.into_iter() - } -} + for scope in scopes { + if !value.is_empty() { + value.push(','); + } -#[cfg(test)] -mod test { - use super::*; + match scope { + SecretOperatorVolumeScope::Node => value.push_str("node"), + SecretOperatorVolumeScope::Pod => value.push_str("pod"), + SecretOperatorVolumeScope::Service { name } => { + value.push_str("service="); + value.push_str(name); + } + SecretOperatorVolumeScope::ListenerVolume { name } => { + value.push_str("listener-volume="); + value.push_str(name); + } + } + } - #[test] - fn parse_insert() { - let mut annotations = Annotations::new(); + Annotation::try_from(("secrets.stackable.tech/scope", value.as_str())) + } - annotations - .parse_insert(("stackable.tech/managed-by", "stackablectl")) - .unwrap(); + /// Constructs a `secrets.stackable.tech/format` annotation. + pub fn secret_format(format: &str) -> Result { + Annotation::try_from(("secrets.stackable.tech/format", format)) + } - annotations - .parse_insert(("stackable.tech/vendor", "Stäckable")) - .unwrap(); + /// Constructs a `secrets.stackable.tech/kerberos.service.names` annotation. + pub fn kerberos_service_names(names: &[String]) -> Result { + let names = names.join(","); + Annotation::try_from(( + "secrets.stackable.tech/kerberos.service.names", + names.as_str(), + )) + } - assert_eq!(annotations.len(), 2); + /// Constructs a `secrets.stackable.tech/format.compatibility.tls-pkcs12.password` + /// annotation. + pub fn tls_pkcs12_password(password: &str) -> Result { + Annotation::try_from(( + "secrets.stackable.tech/format.compatibility.tls-pkcs12.password", + password, + )) + } } } diff --git a/crates/stackable-operator/src/kvp/key.rs b/crates/stackable-operator/src/kvp/key.rs index a7a6e0790..da317716b 100644 --- a/crates/stackable-operator/src/kvp/key.rs +++ b/crates/stackable-operator/src/kvp/key.rs @@ -109,6 +109,12 @@ impl Display for Key { } } +impl From<&Key> for String { + fn from(value: &Key) -> Self { + value.to_string() + } +} + impl Key { /// Retrieves the key's prefix. /// @@ -413,7 +419,7 @@ mod test { let label = Label::try_from((key, "zookeeper")).unwrap(); let is_valid = label - .key() + .key .prefix() .is_some_and(|prefix| *prefix == "app.kubernetes.io"); @@ -427,7 +433,7 @@ mod test { #[case("foo", false)] fn key_name_deref(#[case] key: &str, #[case] expected: bool) { let label = Label::try_from((key, "zookeeper")).unwrap(); - let is_valid = *label.key().name() == "name"; + let is_valid = *label.key.name() == "name"; assert_eq!(is_valid, expected); } diff --git a/crates/stackable-operator/src/kvp/label/mod.rs b/crates/stackable-operator/src/kvp/label/mod.rs index ae38c12d5..00edcf118 100644 --- a/crates/stackable-operator/src/kvp/label/mod.rs +++ b/crates/stackable-operator/src/kvp/label/mod.rs @@ -9,26 +9,8 @@ //! //! See //! for more information on Kubernetes labels. -use std::{ - collections::{BTreeMap, BTreeSet}, - fmt::Display, -}; -use delegate::delegate; -use kube::{Resource, ResourceExt}; - -use crate::{ - iter::TryFromIterator, - kvp::{ - consts::{ - K8S_APP_COMPONENT_KEY, K8S_APP_INSTANCE_KEY, K8S_APP_MANAGED_BY_KEY, K8S_APP_NAME_KEY, - K8S_APP_ROLE_GROUP_KEY, K8S_APP_VERSION_KEY, STACKABLE_VENDOR_KEY, - STACKABLE_VENDOR_VALUE, - }, - Key, KeyValuePair, KeyValuePairError, KeyValuePairs, KeyValuePairsError, ObjectLabels, - }, - utils::format_full_controller_name, -}; +use crate::kvp::{KeyValuePair, KeyValuePairError, KeyValuePairs}; mod selector; mod value; @@ -36,8 +18,6 @@ mod value; pub use selector::*; pub use value::*; -pub type LabelsError = KeyValuePairsError; - /// A type alias for errors returned when construction or manipulation of a set /// of labels fails. pub type LabelError = KeyValuePairError; @@ -55,87 +35,7 @@ pub type LabelError = KeyValuePairError; /// only contain a limited set and combination of ASCII characters. See /// /// for more information on Kubernetes labels. -#[derive(Clone, Debug)] -pub struct Label(KeyValuePair); - -impl TryFrom<(K, V)> for Label -where - K: AsRef, - V: AsRef, -{ - type Error = LabelError; - - fn try_from(value: (K, V)) -> Result { - let kvp = KeyValuePair::try_from(value)?; - Ok(Self(kvp)) - } -} - -impl Display for Label { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - self.0.fmt(f) - } -} - -impl Label { - /// Returns an immutable reference to the label's [`Key`]. - /// - /// ``` - /// # use stackable_operator::kvp::Label; - /// let label = Label::try_from(("stackable.tech/vendor", "Stackable")).unwrap(); - /// assert_eq!(label.key().to_string(), "stackable.tech/vendor"); - /// ``` - pub fn key(&self) -> &Key { - self.0.key() - } - - /// Returns an immutable reference to the label's value. - pub fn value(&self) -> &LabelValue { - self.0.value() - } - - /// Consumes self and returns the inner [`KeyValuePair`]. - pub fn into_inner(self) -> KeyValuePair { - self.0 - } - - /// Creates the `app.kubernetes.io/component` label with `role` as the - /// value. This function will return an error if `role` violates the required - /// Kubernetes restrictions. - pub fn component(component: &str) -> Result { - let kvp = KeyValuePair::try_from((K8S_APP_COMPONENT_KEY, component))?; - Ok(Self(kvp)) - } - - /// Creates the `app.kubernetes.io/role-group` label with `role_group` as - /// the value. This function will return an error if `role_group` violates - /// the required Kubernetes restrictions. - pub fn role_group(role_group: &str) -> Result { - let kvp = KeyValuePair::try_from((K8S_APP_ROLE_GROUP_KEY, role_group))?; - Ok(Self(kvp)) - } - - /// Creates the `app.kubernetes.io/managed-by` label with the formated - /// full controller name based on `operator_name` and `controller_name` as - /// the value. This function will return an error if the formatted controller - /// name violates the required Kubernetes restrictions. - pub fn managed_by(operator_name: &str, controller_name: &str) -> Result { - let kvp = KeyValuePair::try_from(( - K8S_APP_MANAGED_BY_KEY, - format_full_controller_name(operator_name, controller_name).as_str(), - ))?; - Ok(Self(kvp)) - } - - /// Creates the `app.kubernetes.io/version` label with `version` as the - /// value. This function will return an error if `role_group` violates the - /// required Kubernetes restrictions. - pub fn version(version: &str) -> Result { - // NOTE (Techassi): Maybe use semver::Version - let kvp = KeyValuePair::try_from((K8S_APP_VERSION_KEY, version))?; - Ok(Self(kvp)) - } -} +pub type Label = KeyValuePair; /// A validated set/list of Kubernetes labels. /// @@ -148,117 +48,88 @@ impl Label { /// /// ``` /// # use std::collections::BTreeMap; +/// # use stackable_operator::iter::TryFromIterator; /// # use stackable_operator::kvp::Labels; /// let map = BTreeMap::from([ /// ("stackable.tech/managed-by", "stackablectl"), /// ("stackable.tech/vendor", "Stackable"), /// ]); /// -/// let labels = Labels::try_from(map).unwrap(); +/// let labels = Labels::try_from_iter(map).unwrap(); /// ``` /// /// ### Creating a list of labels from an array /// /// ``` +/// # use stackable_operator::iter::TryFromIterator; /// # use stackable_operator::kvp::Labels; -/// let labels = Labels::try_from([ +/// let labels = Labels::try_from_iter([ /// ("stackable.tech/managed-by", "stackablectl"), /// ("stackable.tech/vendor", "Stackable"), /// ]).unwrap(); /// ``` -#[derive(Clone, Debug, Default)] -pub struct Labels(KeyValuePairs); +pub type Labels = KeyValuePairs; + +/// Well-known labels used by other tools or standard conventions. +pub mod well_known { + use crate::{ + kvp::consts::{ + K8S_APP_COMPONENT_KEY, K8S_APP_MANAGED_BY_KEY, K8S_APP_ROLE_GROUP_KEY, + K8S_APP_VERSION_KEY, STACKABLE_VENDOR_KEY, STACKABLE_VENDOR_VALUE, + }, + utils::format_full_controller_name, + }; -impl TryFrom> for Labels -where - K: AsRef, - V: AsRef, -{ - type Error = LabelError; + use super::{Label, LabelError}; - fn try_from(map: BTreeMap) -> Result { - Self::try_from_iter(map) + /// Creates the `app.kubernetes.io/component` label with `role` as the + /// value. This function will return an error if `role` violates the required + /// Kubernetes restrictions. + pub fn component(component: &str) -> Result { + Label::try_from((K8S_APP_COMPONENT_KEY, component)) } -} -impl TryFrom<&BTreeMap> for Labels -where - K: AsRef, - V: AsRef, -{ - type Error = LabelError; - - fn try_from(map: &BTreeMap) -> Result { - Self::try_from_iter(map) - } -} - -impl TryFrom<[(K, V); N]> for Labels -where - K: AsRef, - V: AsRef, -{ - type Error = LabelError; - - fn try_from(array: [(K, V); N]) -> Result { - Self::try_from_iter(array) + /// Creates the `app.kubernetes.io/role-group` label with `role_group` as + /// the value. This function will return an error if `role_group` violates + /// the required Kubernetes restrictions. + pub fn role_group(role_group: &str) -> Result { + Label::try_from((K8S_APP_ROLE_GROUP_KEY, role_group)) } -} -impl FromIterator> for Labels { - fn from_iter>>(iter: T) -> Self { - let kvps = KeyValuePairs::from_iter(iter); - Self(kvps) + /// Creates the `app.kubernetes.io/managed-by` label with the formated + /// full controller name based on `operator_name` and `controller_name` as + /// the value. This function will return an error if the formatted controller + /// name violates the required Kubernetes restrictions. + pub fn managed_by(operator_name: &str, controller_name: &str) -> Result { + Label::try_from(( + K8S_APP_MANAGED_BY_KEY, + format_full_controller_name(operator_name, controller_name).as_str(), + )) } -} - -impl TryFromIterator<(K, V)> for Labels -where - K: AsRef, - V: AsRef, -{ - type Error = LabelError; - fn try_from_iter>(iter: I) -> Result { - let kvps = KeyValuePairs::try_from_iter(iter)?; - Ok(Self(kvps)) + /// Creates the `app.kubernetes.io/version` label with `version` as the + /// value. This function will return an error if `role_group` violates the + /// required Kubernetes restrictions. + pub fn version(version: &str) -> Result { + Label::try_from((K8S_APP_VERSION_KEY, version)) } -} -impl From for BTreeMap { - fn from(value: Labels) -> Self { - value.0.into() + pub fn vendor_stackable() -> Label { + Label::try_from((STACKABLE_VENDOR_KEY, STACKABLE_VENDOR_VALUE)) + .expect("failed to parse hard-coded Stackable vendor label") } } -impl Labels { - /// Creates a new empty list of [`Labels`]. - pub fn new() -> Self { - Self::default() - } - - /// Creates a new list of [`Labels`] from `pairs`. - pub fn new_with(pairs: BTreeSet>) -> Self { - Self(KeyValuePairs::new_with(pairs)) - } +/// Common sets of labels that apply for different use-cases. +pub mod sets { + use kube::{Resource, ResourceExt}; - /// Tries to insert a new label by first parsing `label` as a [`Label`] - /// and then inserting it into the list. This function will overwrite any - /// existing label already present. - pub fn parse_insert( - &mut self, - label: impl TryInto, - ) -> Result<(), LabelError> { - self.0.insert(label.try_into()?.0); - Ok(()) - } + use crate::kvp::{ + consts::{K8S_APP_INSTANCE_KEY, K8S_APP_NAME_KEY}, + ObjectLabels, + }; - /// Inserts a new [`Label`]. This function will overwrite any existing label - /// already present. - pub fn insert(&mut self, label: Label) -> &mut Self { - self.0.insert(label.0); - self - } + use super::{well_known, Label, LabelError, Labels}; /// Returns the recommended set of labels. The set includes these well-known /// Kubernetes labels: @@ -277,63 +148,60 @@ impl Labels { /// This function returns a result, because the parameter `object_labels` /// can contain invalid data or can exceed the maximum allowed number of /// characters. - pub fn recommended(object_labels: ObjectLabels) -> Result + pub fn recommended(object_labels: ObjectLabels) -> Result where R: Resource, { // Well-known Kubernetes labels - let mut labels = Self::role_group_selector( + let mut labels = role_group_selector( object_labels.owner, object_labels.app_name, object_labels.role, object_labels.role_group, )?; - let managed_by = - Label::managed_by(object_labels.operator_name, object_labels.controller_name)?; - let version = Label::version(object_labels.app_version)?; - - labels.insert(managed_by); - labels.insert(version); - - // Stackable-specific labels - labels.parse_insert((STACKABLE_VENDOR_KEY, STACKABLE_VENDOR_VALUE))?; + labels.extend([ + well_known::managed_by(object_labels.operator_name, object_labels.controller_name)?, + well_known::version(object_labels.app_version)?, + // Stackable-specific labels + well_known::vendor_stackable(), + ]); Ok(labels) } /// Returns the set of labels required to select the resource based on the /// role group. The set contains role selector labels, see - /// [`Labels::role_selector`] for more details. Additionally, it contains + /// [`role_selector`] for more details. Additionally, it contains /// the `app.kubernetes.io/role-group` label with `role_group` as the value. pub fn role_group_selector( owner: &R, app_name: &str, role: &str, role_group: &str, - ) -> Result + ) -> Result where R: Resource, { - let mut labels = Self::role_selector(owner, app_name, role)?; - labels.insert(Label::role_group(role_group)?); + let mut labels = role_selector(owner, app_name, role)?; + labels.extend([well_known::role_group(role_group)?]); Ok(labels) } /// Returns the set of labels required to select the resource based on the - /// role. The set contains the common labels, see [`Labels::common`] for + /// role. The set contains the common labels, see [`common`] for /// more details. Additionally, it contains the `app.kubernetes.io/component` /// label with `role` as the value. /// /// This function returns a result, because the parameters `owner`, `app_name`, /// and `role` can contain invalid data or can exceed the maximum allowed /// number fo characters. - pub fn role_selector(owner: &R, app_name: &str, role: &str) -> Result + pub fn role_selector(owner: &R, app_name: &str, role: &str) -> Result where R: Resource, { - let mut labels = Self::common(app_name, owner.name_any().as_str())?; - labels.insert(Label::component(role)?); + let mut labels = common(app_name, owner.name_any().as_str())?; + labels.extend([well_known::component(role)?]); Ok(labels) } @@ -347,77 +215,10 @@ impl Labels { /// This function returns a result, because the parameters `app_name` and /// `app_instance` can contain invalid data or can exceed the maximum /// allowed number of characters. - pub fn common(app_name: &str, app_instance: &str) -> Result { - let mut labels = Self::new(); - - labels.insert((K8S_APP_INSTANCE_KEY, app_instance).try_into()?); - labels.insert((K8S_APP_NAME_KEY, app_name).try_into()?); - - Ok(labels) - } - - // This forwards / delegates associated functions to the inner field. In - // this case self.0 which is of type KeyValuePairs. So calling - // Labels::len() will be delegated to KeyValuePair::len() without the - // need to write boilerplate code. - delegate! { - to self.0 { - /// Tries to insert a new [`Label`]. It ensures there are no duplicate - /// entries. Trying to insert duplicated data returns an error. If no such - /// check is required, use [`Labels::insert`] instead. - pub fn try_insert(&mut self, #[newtype] label: Label) -> Result<(), LabelsError>; - - /// Extends `self` with `other`. - pub fn extend(&mut self, #[newtype] other: Self); - - /// Returns the number of labels. - pub fn len(&self) -> usize; - - /// Returns if the set of labels is empty. - pub fn is_empty(&self) -> bool; - - /// Returns if the set of labels contains the provided `label`. Failure to - /// parse/validate the [`KeyValuePair`] will return `false`. - pub fn contains(&self, label: impl TryInto>) -> bool; - - /// Returns if the set of labels contains a label with the provided `key`. - /// Failure to parse/validate the [`Key`] will return `false`. - pub fn contains_key(&self, key: impl TryInto) -> bool; - - /// Returns an [`Iterator`] over [`Labels`] yielding a reference to every [`Label`] contained within. - pub fn iter(&self) -> impl Iterator> + '_; - - } - } -} - -impl IntoIterator for Labels { - type Item = KeyValuePair; - type IntoIter = as IntoIterator>::IntoIter; - - /// Returns a consuming [`Iterator`] over [`Labels`] moving every [`Label`] out. - /// The [`Labels`] cannot be used again after calling this. - fn into_iter(self) -> Self::IntoIter { - self.0.into_iter() - } -} - -#[cfg(test)] -mod test { - use super::*; - - #[test] - fn parse_insert() { - let mut labels = Labels::new(); - - labels - .parse_insert(("stackable.tech/managed-by", "stackablectl")) - .unwrap(); - - labels - .parse_insert(("stackable.tech/vendor", "Stackable")) - .unwrap(); - - assert_eq!(labels.len(), 2); + pub fn common(app_name: &str, app_instance: &str) -> Result { + Ok(Labels::from_iter([ + Label::try_from((K8S_APP_INSTANCE_KEY, app_instance))?, + Label::try_from((K8S_APP_NAME_KEY, app_name))?, + ])) } } diff --git a/crates/stackable-operator/src/kvp/mod.rs b/crates/stackable-operator/src/kvp/mod.rs index 5a233c603..15361b1a5 100644 --- a/crates/stackable-operator/src/kvp/mod.rs +++ b/crates/stackable-operator/src/kvp/mod.rs @@ -1,27 +1,31 @@ //! Utility functions and data structures the create and manage Kubernetes //! key/value pairs, like labels and annotations. use std::{ - collections::{BTreeMap, BTreeSet}, - fmt::Display, - ops::Deref, + collections::BTreeMap, + fmt::{Debug, Display}, str::FromStr, }; -use snafu::{ensure, ResultExt, Snafu}; +use snafu::{ResultExt, Snafu}; -use crate::iter::TryFromIterator; - -mod annotation; +pub mod annotation; pub mod consts; mod key; -mod label; +pub mod label; mod value; -pub use annotation::*; +pub use annotation::{Annotation, AnnotationError, AnnotationValue, Annotations}; pub use key::*; -pub use label::*; +pub use label::{Label, LabelError, LabelSelectorExt, LabelValue, Labels, SelectorError}; pub use value::*; +use crate::iter::TryFromIterator; + +#[cfg(doc)] +use k8s_openapi::apimachinery::pkg::apis::meta::v1::ObjectMeta; +#[cfg(doc)] +use std::ops::Deref; + /// The error type for key/value pair parsing/validating operations. #[derive(Debug, PartialEq, Snafu)] pub enum KeyValuePairError @@ -90,268 +94,102 @@ where /// /// - /// - -#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Clone, PartialEq, Eq, PartialOrd, Ord)] pub struct KeyValuePair where T: Value, { - key: Key, - value: T, + pub key: Key, + pub value: T, } -impl TryFrom<(K, V)> for KeyValuePair +impl TryFrom<(&str, &str)> for KeyValuePair where - K: AsRef, - V: AsRef, - T: Value, + V: Value, { - type Error = KeyValuePairError; - - fn try_from(value: (K, V)) -> Result { - let key = Key::from_str(value.0.as_ref()).context(InvalidKeySnafu { - key: value.0.as_ref(), - })?; - - let value = T::from_str(value.1.as_ref()).context(InvalidValueSnafu { - key: key.to_string(), - value: value.1.as_ref(), - })?; + type Error = KeyValuePairError; + fn try_from((key, value): (&str, &str)) -> Result { + let key = Key::from_str(key).context(InvalidKeySnafu { key })?; + let value = V::from_str(value).context(InvalidValueSnafu { key: &key, value })?; Ok(Self { key, value }) } } -impl Display for KeyValuePair -where - T: Value, -{ - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}={}", self.key, self.value) +impl From> for (Key, V) { + fn from(KeyValuePair { key, value }: KeyValuePair) -> Self { + (key, value) } } -impl KeyValuePair -where - T: Value, -{ - /// Creates a new [`KeyValuePair`] from a validated [`Key`] and value. - pub fn new(key: Key, value: T) -> Self { - Self { key, value } - } - - /// Returns an immutable reference to the pair's [`Key`]. - pub fn key(&self) -> &Key { - &self.key - } - - /// Returns an immutable reference to the pair's value. - pub fn value(&self) -> &T { - &self.value +impl Display for KeyValuePair { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}={}", self.key, self.value) } } -#[derive(Debug, PartialEq, Snafu)] -pub enum KeyValuePairsError { - #[snafu(display("key already exists"))] - KeyAlreadyExists, +impl Debug for KeyValuePair { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{:?}: {:?}", self.key, self.value) + } } /// A validated set/list of Kubernetes key/value pairs. /// -/// It implements various traits which allows conversion from and to different -/// data types. Traits to construct [`KeyValuePairs`] from other data types are: -/// -/// - `TryFrom<&BTreeMap>` -/// - `TryFrom>` -/// - `FromIterator>` -/// - `TryFrom<[(K, V); N]>` -/// -/// Traits to convert [`KeyValuePairs`] into a different data type are: -/// -/// - `From> for BTreeMap` -/// -/// See [`Labels`] and [`Annotations`] on how these traits can be used. +/// See [`Annotations`] and [`Labels`] for actual instantiations. /// -/// # Note -/// -/// A [`BTreeSet`] is used as the inner collection to preserve order of items -/// which ultimately prevent unncessary reconciliations due to changes -/// in item order. -#[derive(Clone, Debug, Default)] -pub struct KeyValuePairs(BTreeMap); - -impl TryFrom> for KeyValuePairs -where - K: AsRef, - V: AsRef, - T: Value, -{ - type Error = KeyValuePairError; +/// See [`KeyValuePairsExt`] for kvp-specific convenience helpers. +pub type KeyValuePairs = BTreeMap; - fn try_from(map: BTreeMap) -> Result { - Self::try_from_iter(map) +impl Extend> for KeyValuePairs { + fn extend>>(&mut self, iter: T) { + self.extend(iter.into_iter().map(<(Key, V)>::from)); } } -impl TryFrom<&BTreeMap> for KeyValuePairs -where - K: AsRef, - V: AsRef, - T: Value, -{ - type Error = KeyValuePairError; - - fn try_from(map: &BTreeMap) -> Result { - Self::try_from_iter(map) +impl FromIterator> for KeyValuePairs { + fn from_iter>>(iter: T) -> Self { + Self::from_iter(iter.into_iter().map(<(Key, V)>::from)) } } -impl TryFrom<[(K, V); N]> for KeyValuePairs -where - K: AsRef, - V: AsRef, - T: Value + std::default::Default, -{ - type Error = KeyValuePairError; - - fn try_from(array: [(K, V); N]) -> Result { - Self::try_from_iter(array) - } -} - -impl FromIterator> for KeyValuePairs -where - T: Value, -{ - fn from_iter>>(iter: I) -> Self { - Self(iter.into_iter().map(|kvp| (kvp.key, kvp.value)).collect()) - } -} - -impl TryFromIterator<(K, V)> for KeyValuePairs -where - K: AsRef, - V: AsRef, - T: Value, -{ - type Error = KeyValuePairError; - - fn try_from_iter>(iter: I) -> Result { - let pairs = iter - .into_iter() - .map(KeyValuePair::try_from) - .collect::, KeyValuePairError>>()?; +/// Helpers for [`KeyValuePairs`]. +pub trait KeyValuePairsExt { + /// Clones `self` into a type without validation types, ready for use in [`ObjectMeta::annotations`]/[`ObjectMeta::labels`]. + fn to_unvalidated(&self) -> BTreeMap; - Ok(Self::from_iter(pairs)) - } + /// Returns whether the list contains a key/value pair with a specific [`Key`]. + /// + /// Returns `false` if `key` cannot be parsed as a valid [`Key`]. + // TODO: Does anyone actually use this API? + fn contains_str_key(&self, key: &str) -> bool; } - -impl From> for BTreeMap -where - T: Value, -{ - fn from(value: KeyValuePairs) -> Self { - value - .iter() - .map(|(k, v)| (k.to_string(), v.to_string())) +impl KeyValuePairsExt for KeyValuePairs { + fn to_unvalidated(&self) -> BTreeMap { + self.iter() + .map(|(key, value)| (key.to_string(), value.to_string())) .collect() } -} - -impl Deref for KeyValuePairs -where - T: Value, -{ - type Target = BTreeMap; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -impl KeyValuePairs -where - T: Value + std::default::Default, -{ - /// Creates a new empty list of [`KeyValuePair`]s. - pub fn new() -> Self { - Self::default() - } - - /// Creates a new list of [`KeyValuePair`]s from `pairs`. - pub fn new_with(pairs: BTreeSet>) -> Self { - Self::from_iter(pairs) - } - - /// Extends `self` with `other`. - pub fn extend(&mut self, other: Self) { - self.0.extend(other.0); - } - /// Inserts a new [`KeyValuePair`] into the list of pairs. - /// - /// This function overwrites any existing key/value pair. To avoid - /// overwriting existing pairs, either use [`KeyValuePairs::contains`] or - /// [`KeyValuePairs::contains_key`] before inserting or try to insert - /// fallible via [`KeyValuePairs::try_insert`]. - pub fn insert(&mut self, kvp: KeyValuePair) -> &mut Self { - self.0.insert(kvp.key, kvp.value); - self - } - - /// Tries to insert a new [`KeyValuePair`] into the list of pairs. - /// - /// If the list already had this key present, nothing is updated, and an - /// error is returned. - pub fn try_insert(&mut self, kvp: KeyValuePair) -> Result<(), KeyValuePairsError> { - ensure!(!self.0.contains_key(&kvp.key), KeyAlreadyExistsSnafu); - self.insert(kvp); - Ok(()) - } - - /// Returns if the list contains a specific [`KeyValuePair`]. - pub fn contains(&self, kvp: impl TryInto>) -> bool { - let Ok(kvp) = kvp.try_into() else { + fn contains_str_key(&self, key: &str) -> bool { + // We could avoid this clone by providing an UnvalidatedKeyRef and ensure that Key: Borrow + let Ok(key) = key.parse::() else { + // If the key cannot be parsed then it cannot, by definition, possibly exist in the map return false; }; - let Some(value) = self.get(&kvp.key) else { - return false; - }; - value == &kvp.value - } - - /// Returns if the list contains a key/value pair with a specific [`Key`]. - pub fn contains_key(&self, key: impl TryInto) -> bool { - let Ok(key) = key.try_into() else { - return false; - }; - - self.0.contains_key(&key) - } - - /// Returns an [`Iterator`] over [`KeyValuePairs`] yielding a reference to every [`KeyValuePair`] contained within. - pub fn iter(&self) -> impl Iterator> + '_ { - self.0.iter().map(|(k, v)| KeyValuePair { - key: k.clone(), - value: v.clone(), - }) + self.contains_key(&key) } } -impl IntoIterator for KeyValuePairs -where - T: Value, -{ - type Item = KeyValuePair; - type IntoIter = - std::iter::Map, fn((Key, T)) -> Self::Item>; - - /// Returns a consuming [`Iterator`] over [`KeyValuePairs`] moving every [`KeyValuePair`] out. - /// The [`KeyValuePairs`] cannot be used again after calling this. - fn into_iter(self) -> Self::IntoIter { - self.0 - .into_iter() - .map(|(key, value)| KeyValuePair { key, value }) +impl<'a, T: Value> TryFromIterator<(&'a str, &'a str)> for KeyValuePairs { + type Error = KeyValuePairError; + + fn try_from_iter>( + iter: I, + ) -> Result { + iter.into_iter() + .map(KeyValuePair::try_from) + .collect::>>() } } @@ -404,28 +242,15 @@ mod test { #[test] fn try_from_tuple() { - let label = Label::try_from(("stackable.tech/vendor", "Stackable")).unwrap(); + let label = + KeyValuePair::::try_from(("stackable.tech/vendor", "Stackable")).unwrap(); - assert_eq!( - label.key(), - &Key::from_str("stackable.tech/vendor").unwrap() - ); - assert_eq!(label.value(), &LabelValue::from_str("Stackable").unwrap()); + assert_eq!(label.key, Key::from_str("stackable.tech/vendor").unwrap()); + assert_eq!(label.value, LabelValue::from_str("Stackable").unwrap()); assert_eq!(label.to_string(), "stackable.tech/vendor=Stackable"); } - #[test] - fn labels_from_array() { - let labels = Labels::try_from([ - ("stackable.tech/managed-by", "stackablectl"), - ("stackable.tech/vendor", "Stackable"), - ]) - .unwrap(); - - assert_eq!(labels.len(), 2); - } - #[test] fn labels_from_iter() { let labels = Labels::from_iter([ @@ -443,28 +268,26 @@ mod test { ("stackable.tech/vendor", "Stackable"), ]); - let labels = Labels::try_from(map).unwrap(); + let labels = Labels::try_from_iter(map).unwrap(); assert_eq!(labels.len(), 2); } #[test] - fn labels_into_map() { - let labels = Labels::try_from([ - ("stackable.tech/managed-by", "stackablectl"), - ("stackable.tech/vendor", "Stackable"), - ]) - .unwrap(); + fn labels_to_unvalidated() { + let labels = Labels::from_iter([ + KeyValuePair::try_from(("stackable.tech/managed-by", "stackablectl")).unwrap(), + KeyValuePair::try_from(("stackable.tech/vendor", "Stackable")).unwrap(), + ]); - let map: BTreeMap = labels.into(); + let map: BTreeMap = labels.to_unvalidated(); assert_eq!(map.len(), 2); } #[test] fn contains() { - let labels = Labels::common("test", "test-01").unwrap(); + let labels = label::sets::common("test", "test-01").unwrap(); - assert!(labels.contains(("app.kubernetes.io/name", "test"))); - assert!(labels.contains_key("app.kubernetes.io/instance")) + assert!(labels.contains_str_key("app.kubernetes.io/instance")) } #[test] @@ -498,10 +321,8 @@ mod test { Labels::try_from_iter([("a", "b"), ("b", "a"), ("c", "c")]).unwrap(); merged_labels.extend(Labels::try_from_iter([("a", "a"), ("b", "b"), ("d", "d")]).unwrap()); assert_eq!( - BTreeMap::from(merged_labels), - BTreeMap::from( - Labels::try_from_iter([("a", "a"), ("b", "b"), ("c", "c"), ("d", "d")]).unwrap() - ) + merged_labels, + Labels::try_from_iter([("a", "a"), ("b", "b"), ("c", "c"), ("d", "d")]).unwrap() ) } } From b215e8e47068d0eac9e45b42f09f8db0f6b29a26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Mon, 14 Oct 2024 11:23:03 +0200 Subject: [PATCH 04/12] Changelog --- crates/stackable-operator/CHANGELOG.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index 7404e2561..eb76c1b15 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -12,6 +12,20 @@ All notable changes to this project will be documented in this file. - BREAKING: The `CustomResourceExt` trait is now re-exported from the `stackable-shared` crate. The trait functions use the same parameters but return a different error type ([#883]). +- BREAKING: `KeyValuePairs` (and `Annotations`/`Labels`) is now an alias for `BTreeMap` ([#889]). + - Generally, this means that the API now behaves like a map rather than a set. For example, duplicate keys are no longer allowed (as was already documented before). + - Some `KeyValuePairs` methods have been renamed for certain use cases: + - `KeyValuePairs::insert(&mut self, kvp)`: use `::extend(&mut self, [kvp])` instead. + - `KeyValuePairs::try_from`: you may need to use `::try_from_iter` instead. + - `KeyValuePairs::contains_key`: unvalidated keys will need to use `::contains_str_key` instead. + - `Into>`: use `::to_unvalidated` instead. + - Well-known annotations have been moved from `kvp::Annotation` to `kvp::annotation::well_known`. + - Well-known labels have been moved from `kvp::Label` to `kvp::label::well_known`. + - Well-known label sets have been moved from `kvp::Labels` to `kvp::label::sets`. + +### Fixed + +- `KeyValuePairs` will now consistently use the last-written value for a given key ([#889]). ### Removed @@ -20,6 +34,7 @@ All notable changes to this project will be documented in this file. use it as a `String`. [#883]: https://github.com/stackabletech/operator-rs/pull/883 +[#889]: https://github.com/stackabletech/operator-rs/pull/889 ## [0.78.0] - 2024-09-30 From 41ce00ee264a490b2016a02763a0e7f42b827122 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Mon, 14 Oct 2024 11:24:53 +0200 Subject: [PATCH 05/12] Drop unused delegate dependency --- Cargo.lock | 12 ------------ Cargo.toml | 1 - crates/stackable-operator/Cargo.toml | 1 - 3 files changed, 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 60d7eba67..e0480db60 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -685,17 +685,6 @@ dependencies = [ "syn 2.0.77", ] -[[package]] -name = "delegate" -version = "0.13.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5060bb0febb73fa907273f8a7ed17ab4bf831d585eac835b28ec24a1e2460956" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.77", -] - [[package]] name = "der" version = "0.7.9" @@ -3007,7 +2996,6 @@ dependencies = [ "chrono", "clap", "const_format", - "delegate", "derivative", "dockerfile-parser", "either", diff --git a/Cargo.toml b/Cargo.toml index 46b6941a6..7556ded22 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,7 +18,6 @@ const_format = "0.2.33" const-oid = "0.9.6" convert_case = "0.6.0" darling = "0.20.10" -delegate = "0.13.0" derivative = "2.2.0" dockerfile-parser = "0.8.0" ecdsa = { version = "0.16.9", features = ["digest", "pem"] } diff --git a/crates/stackable-operator/Cargo.toml b/crates/stackable-operator/Cargo.toml index e6f182fc7..055efcd88 100644 --- a/crates/stackable-operator/Cargo.toml +++ b/crates/stackable-operator/Cargo.toml @@ -17,7 +17,6 @@ stackable-shared = { path = "../stackable-shared" } chrono.workspace = true clap.workspace = true const_format.workspace = true -delegate.workspace = true derivative.workspace = true dockerfile-parser.workspace = true either.workspace = true From 1e4cffe158ee825f332b5f5af2a29f4ec912857c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Wed, 16 Oct 2024 15:08:34 +0200 Subject: [PATCH 06/12] Use typed Key in errors --- crates/stackable-operator/src/kvp/key.rs | 5 +++-- crates/stackable-operator/src/kvp/mod.rs | 8 ++------ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/crates/stackable-operator/src/kvp/key.rs b/crates/stackable-operator/src/kvp/key.rs index da317716b..dbde78d1c 100644 --- a/crates/stackable-operator/src/kvp/key.rs +++ b/crates/stackable-operator/src/kvp/key.rs @@ -109,9 +109,10 @@ impl Display for Key { } } -impl From<&Key> for String { +// Allows SNAFU context selectors to clone the key implicitly +impl From<&Key> for Key { fn from(value: &Key) -> Self { - value.to_string() + value.clone() } } diff --git a/crates/stackable-operator/src/kvp/mod.rs b/crates/stackable-operator/src/kvp/mod.rs index 15361b1a5..35bc809c4 100644 --- a/crates/stackable-operator/src/kvp/mod.rs +++ b/crates/stackable-operator/src/kvp/mod.rs @@ -38,12 +38,8 @@ where InvalidKey { source: KeyError, key: String }, /// Indicates that the value failed to parse. - #[snafu(display("failed to parse value {value:?} of key {key:?}"))] - InvalidValue { - source: E, - key: String, - value: String, - }, + #[snafu(display("failed to parse value {value:?} for key {key:?}", key = key.to_string()))] + InvalidValue { source: E, key: Key, value: String }, } /// A validated Kubernetes key/value pair. From a6101a4c195623f8350f15c6f49d3641743a107e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Wed, 16 Oct 2024 15:12:04 +0200 Subject: [PATCH 07/12] Docs for `label::well_known::vendor_stackable` --- crates/stackable-operator/src/kvp/label/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/stackable-operator/src/kvp/label/mod.rs b/crates/stackable-operator/src/kvp/label/mod.rs index 00edcf118..4eb42d47b 100644 --- a/crates/stackable-operator/src/kvp/label/mod.rs +++ b/crates/stackable-operator/src/kvp/label/mod.rs @@ -114,6 +114,8 @@ pub mod well_known { Label::try_from((K8S_APP_VERSION_KEY, version)) } + /// Creates the `stackable.tech/vendor: Stackable` label, tagging the object as + /// created by a Stackable operator. pub fn vendor_stackable() -> Label { Label::try_from((STACKABLE_VENDOR_KEY, STACKABLE_VENDOR_VALUE)) .expect("failed to parse hard-coded Stackable vendor label") From e21a8f544007251142ea434c74bca384e6bfe52c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Wed, 16 Oct 2024 15:13:17 +0200 Subject: [PATCH 08/12] Separate public modules --- crates/stackable-operator/src/kvp/mod.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/stackable-operator/src/kvp/mod.rs b/crates/stackable-operator/src/kvp/mod.rs index 35bc809c4..f59d31d2a 100644 --- a/crates/stackable-operator/src/kvp/mod.rs +++ b/crates/stackable-operator/src/kvp/mod.rs @@ -8,10 +8,13 @@ use std::{ use snafu::{ResultExt, Snafu}; +use crate::iter::TryFromIterator; + pub mod annotation; pub mod consts; -mod key; pub mod label; + +mod key; mod value; pub use annotation::{Annotation, AnnotationError, AnnotationValue, Annotations}; @@ -19,8 +22,6 @@ pub use key::*; pub use label::{Label, LabelError, LabelSelectorExt, LabelValue, Labels, SelectorError}; pub use value::*; -use crate::iter::TryFromIterator; - #[cfg(doc)] use k8s_openapi::apimachinery::pkg::apis::meta::v1::ObjectMeta; #[cfg(doc)] From 17458e1afd86e674a63cd99c3fabcc3261786192 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Wed, 16 Oct 2024 15:16:50 +0200 Subject: [PATCH 09/12] Rename Value generics from T -> V --- crates/stackable-operator/src/kvp/mod.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/stackable-operator/src/kvp/mod.rs b/crates/stackable-operator/src/kvp/mod.rs index f59d31d2a..bbfaeb1d8 100644 --- a/crates/stackable-operator/src/kvp/mod.rs +++ b/crates/stackable-operator/src/kvp/mod.rs @@ -92,12 +92,12 @@ where /// - /// - #[derive(Clone, PartialEq, Eq, PartialOrd, Ord)] -pub struct KeyValuePair +pub struct KeyValuePair where - T: Value, + V: Value, { pub key: Key, - pub value: T, + pub value: V, } impl TryFrom<(&str, &str)> for KeyValuePair @@ -119,13 +119,13 @@ impl From> for (Key, V) { } } -impl Display for KeyValuePair { +impl Display for KeyValuePair { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{}={}", self.key, self.value) } } -impl Debug for KeyValuePair { +impl Debug for KeyValuePair { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{:?}: {:?}", self.key, self.value) } @@ -178,15 +178,15 @@ impl KeyValuePairsExt for KeyValuePairs { } } -impl<'a, T: Value> TryFromIterator<(&'a str, &'a str)> for KeyValuePairs { - type Error = KeyValuePairError; +impl<'a, V: Value> TryFromIterator<(&'a str, &'a str)> for KeyValuePairs { + type Error = KeyValuePairError; fn try_from_iter>( iter: I, ) -> Result { iter.into_iter() .map(KeyValuePair::try_from) - .collect::>>() + .collect::>>() } } From 8f6daa0333b01541fa428d6a98dd49ad3bc85d33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Wed, 16 Oct 2024 15:31:32 +0200 Subject: [PATCH 10/12] Clean up tests --- crates/stackable-operator/src/kvp/mod.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/stackable-operator/src/kvp/mod.rs b/crates/stackable-operator/src/kvp/mod.rs index bbfaeb1d8..555195df3 100644 --- a/crates/stackable-operator/src/kvp/mod.rs +++ b/crates/stackable-operator/src/kvp/mod.rs @@ -239,8 +239,7 @@ mod test { #[test] fn try_from_tuple() { - let label = - KeyValuePair::::try_from(("stackable.tech/vendor", "Stackable")).unwrap(); + let label = Label::try_from(("stackable.tech/vendor", "Stackable")).unwrap(); assert_eq!(label.key, Key::from_str("stackable.tech/vendor").unwrap()); assert_eq!(label.value, LabelValue::from_str("Stackable").unwrap()); @@ -276,7 +275,7 @@ mod test { KeyValuePair::try_from(("stackable.tech/vendor", "Stackable")).unwrap(), ]); - let map: BTreeMap = labels.to_unvalidated(); + let map = labels.to_unvalidated(); assert_eq!(map.len(), 2); } From 464fdda94dda4713bd7eb29b39df62596f37a5a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Wed, 16 Oct 2024 16:11:57 +0200 Subject: [PATCH 11/12] Rename label::sets to label::well_known::sets --- crates/stackable-operator/CHANGELOG.md | 2 +- crates/stackable-operator/src/builder/meta.rs | 2 +- crates/stackable-operator/src/builder/pdb.rs | 4 +- .../src/cluster_resources.rs | 2 +- .../stackable-operator/src/kvp/label/mod.rs | 202 +++++++++--------- crates/stackable-operator/src/kvp/mod.rs | 2 +- 6 files changed, 107 insertions(+), 107 deletions(-) diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index eb76c1b15..f46abb1c8 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -21,7 +21,7 @@ All notable changes to this project will be documented in this file. - `Into>`: use `::to_unvalidated` instead. - Well-known annotations have been moved from `kvp::Annotation` to `kvp::annotation::well_known`. - Well-known labels have been moved from `kvp::Label` to `kvp::label::well_known`. - - Well-known label sets have been moved from `kvp::Labels` to `kvp::label::sets`. + - Well-known label sets have been moved from `kvp::Labels` to `kvp::label::well_known::sets`. ### Fixed diff --git a/crates/stackable-operator/src/builder/meta.rs b/crates/stackable-operator/src/builder/meta.rs index 009d6633e..3c3b06a39 100644 --- a/crates/stackable-operator/src/builder/meta.rs +++ b/crates/stackable-operator/src/builder/meta.rs @@ -157,7 +157,7 @@ impl ObjectMetaBuilder { object_labels: ObjectLabels, ) -> Result<&mut Self> { let recommended_labels = - label::sets::recommended(object_labels).context(RecommendedLabelsSnafu)?; + label::well_known::sets::recommended(object_labels).context(RecommendedLabelsSnafu)?; self.labels .get_or_insert(Labels::new()) diff --git a/crates/stackable-operator/src/builder/pdb.rs b/crates/stackable-operator/src/builder/pdb.rs index 489da5689..92e772b59 100644 --- a/crates/stackable-operator/src/builder/pdb.rs +++ b/crates/stackable-operator/src/builder/pdb.rs @@ -85,8 +85,8 @@ impl PodDisruptionBudgetBuilder<(), (), ()> { operator_name: &str, controller_name: &str, ) -> Result> { - let role_selector_labels = - label::sets::role_selector(owner, app_name, role).context(RoleSelectorLabelsSnafu)?; + let role_selector_labels = label::well_known::sets::role_selector(owner, app_name, role) + .context(RoleSelectorLabelsSnafu)?; let managed_by_label = label::well_known::managed_by(operator_name, controller_name) .context(ManagedByLabelSnafu)?; let metadata = ObjectMetaBuilder::new() diff --git a/crates/stackable-operator/src/cluster_resources.rs b/crates/stackable-operator/src/cluster_resources.rs index 62021971a..ee1cf3f0b 100644 --- a/crates/stackable-operator/src/cluster_resources.rs +++ b/crates/stackable-operator/src/cluster_resources.rs @@ -464,7 +464,7 @@ impl ClusterResources { /// Return required labels for cluster resources to be uniquely identified for clean up. // TODO: This is a (quick-fix) helper method but should be replaced by better label handling pub fn get_required_labels(&self) -> Result { - let mut labels = label::sets::common(&self.app_name, &self.app_instance)?; + let mut labels = label::well_known::sets::common(&self.app_name, &self.app_instance)?; labels.extend([label::well_known::managed_by( &self.operator_name, diff --git a/crates/stackable-operator/src/kvp/label/mod.rs b/crates/stackable-operator/src/kvp/label/mod.rs index 4eb42d47b..e7a428439 100644 --- a/crates/stackable-operator/src/kvp/label/mod.rs +++ b/crates/stackable-operator/src/kvp/label/mod.rs @@ -120,107 +120,107 @@ pub mod well_known { Label::try_from((STACKABLE_VENDOR_KEY, STACKABLE_VENDOR_VALUE)) .expect("failed to parse hard-coded Stackable vendor label") } -} - -/// Common sets of labels that apply for different use-cases. -pub mod sets { - use kube::{Resource, ResourceExt}; - - use crate::kvp::{ - consts::{K8S_APP_INSTANCE_KEY, K8S_APP_NAME_KEY}, - ObjectLabels, - }; - - use super::{well_known, Label, LabelError, Labels}; - - /// Returns the recommended set of labels. The set includes these well-known - /// Kubernetes labels: - /// - /// - `app.kubernetes.io/role-group` - /// - `app.kubernetes.io/managed-by` - /// - `app.kubernetes.io/component` - /// - `app.kubernetes.io/instance` - /// - `app.kubernetes.io/version` - /// - `app.kubernetes.io/name` - /// - /// Additionally, it includes Stackable-specific labels. These are: - /// - /// - `stackable.tech/vendor` - /// - /// This function returns a result, because the parameter `object_labels` - /// can contain invalid data or can exceed the maximum allowed number of - /// characters. - pub fn recommended(object_labels: ObjectLabels) -> Result - where - R: Resource, - { - // Well-known Kubernetes labels - let mut labels = role_group_selector( - object_labels.owner, - object_labels.app_name, - object_labels.role, - object_labels.role_group, - )?; - - labels.extend([ - well_known::managed_by(object_labels.operator_name, object_labels.controller_name)?, - well_known::version(object_labels.app_version)?, - // Stackable-specific labels - well_known::vendor_stackable(), - ]); - - Ok(labels) - } - - /// Returns the set of labels required to select the resource based on the - /// role group. The set contains role selector labels, see - /// [`role_selector`] for more details. Additionally, it contains - /// the `app.kubernetes.io/role-group` label with `role_group` as the value. - pub fn role_group_selector( - owner: &R, - app_name: &str, - role: &str, - role_group: &str, - ) -> Result - where - R: Resource, - { - let mut labels = role_selector(owner, app_name, role)?; - labels.extend([well_known::role_group(role_group)?]); - Ok(labels) - } - - /// Returns the set of labels required to select the resource based on the - /// role. The set contains the common labels, see [`common`] for - /// more details. Additionally, it contains the `app.kubernetes.io/component` - /// label with `role` as the value. - /// - /// This function returns a result, because the parameters `owner`, `app_name`, - /// and `role` can contain invalid data or can exceed the maximum allowed - /// number fo characters. - pub fn role_selector(owner: &R, app_name: &str, role: &str) -> Result - where - R: Resource, - { - let mut labels = common(app_name, owner.name_any().as_str())?; - labels.extend([well_known::component(role)?]); - Ok(labels) - } - /// Returns a common set of labels, which are required to identify resources - /// that belong to a certain owner object, for example a `ZookeeperCluster`. - /// The set contains these well-known labels: - /// - /// - `app.kubernetes.io/instance` and - /// - `app.kubernetes.io/name` - /// - /// This function returns a result, because the parameters `app_name` and - /// `app_instance` can contain invalid data or can exceed the maximum - /// allowed number of characters. - pub fn common(app_name: &str, app_instance: &str) -> Result { - Ok(Labels::from_iter([ - Label::try_from((K8S_APP_INSTANCE_KEY, app_instance))?, - Label::try_from((K8S_APP_NAME_KEY, app_name))?, - ])) + /// Common sets of labels that apply for different use-cases. + pub mod sets { + use kube::{Resource, ResourceExt}; + + use crate::kvp::{ + consts::{K8S_APP_INSTANCE_KEY, K8S_APP_NAME_KEY}, + ObjectLabels, + }; + + use super::super::{Label, LabelError, Labels}; + + /// Returns the recommended set of labels. The set includes these well-known + /// Kubernetes labels: + /// + /// - `app.kubernetes.io/role-group` + /// - `app.kubernetes.io/managed-by` + /// - `app.kubernetes.io/component` + /// - `app.kubernetes.io/instance` + /// - `app.kubernetes.io/version` + /// - `app.kubernetes.io/name` + /// + /// Additionally, it includes Stackable-specific labels. These are: + /// + /// - `stackable.tech/vendor` + /// + /// This function returns a result, because the parameter `object_labels` + /// can contain invalid data or can exceed the maximum allowed number of + /// characters. + pub fn recommended(object_labels: ObjectLabels) -> Result + where + R: Resource, + { + // Well-known Kubernetes labels + let mut labels = role_group_selector( + object_labels.owner, + object_labels.app_name, + object_labels.role, + object_labels.role_group, + )?; + + labels.extend([ + super::managed_by(object_labels.operator_name, object_labels.controller_name)?, + super::version(object_labels.app_version)?, + // Stackable-specific labels + super::vendor_stackable(), + ]); + + Ok(labels) + } + + /// Returns the set of labels required to select the resource based on the + /// role group. The set contains role selector labels, see + /// [`role_selector`] for more details. Additionally, it contains + /// the `app.kubernetes.io/role-group` label with `role_group` as the value. + pub fn role_group_selector( + owner: &R, + app_name: &str, + role: &str, + role_group: &str, + ) -> Result + where + R: Resource, + { + let mut labels = role_selector(owner, app_name, role)?; + labels.extend([super::role_group(role_group)?]); + Ok(labels) + } + + /// Returns the set of labels required to select the resource based on the + /// role. The set contains the common labels, see [`common`] for + /// more details. Additionally, it contains the `app.kubernetes.io/component` + /// label with `role` as the value. + /// + /// This function returns a result, because the parameters `owner`, `app_name`, + /// and `role` can contain invalid data or can exceed the maximum allowed + /// number fo characters. + pub fn role_selector(owner: &R, app_name: &str, role: &str) -> Result + where + R: Resource, + { + let mut labels = common(app_name, owner.name_any().as_str())?; + labels.extend([super::component(role)?]); + Ok(labels) + } + + /// Returns a common set of labels, which are required to identify resources + /// that belong to a certain owner object, for example a `ZookeeperCluster`. + /// The set contains these well-known labels: + /// + /// - `app.kubernetes.io/instance` and + /// - `app.kubernetes.io/name` + /// + /// This function returns a result, because the parameters `app_name` and + /// `app_instance` can contain invalid data or can exceed the maximum + /// allowed number of characters. + pub fn common(app_name: &str, app_instance: &str) -> Result { + Ok(Labels::from_iter([ + Label::try_from((K8S_APP_INSTANCE_KEY, app_instance))?, + Label::try_from((K8S_APP_NAME_KEY, app_name))?, + ])) + } } } diff --git a/crates/stackable-operator/src/kvp/mod.rs b/crates/stackable-operator/src/kvp/mod.rs index 555195df3..fb0088f19 100644 --- a/crates/stackable-operator/src/kvp/mod.rs +++ b/crates/stackable-operator/src/kvp/mod.rs @@ -281,7 +281,7 @@ mod test { #[test] fn contains() { - let labels = label::sets::common("test", "test-01").unwrap(); + let labels = label::well_known::sets::common("test", "test-01").unwrap(); assert!(labels.contains_str_key("app.kubernetes.io/instance")) } From 98bdabf8a70bb3eb2b937f21fdbcb0863a1ba75d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Wed, 16 Oct 2024 16:16:38 +0200 Subject: [PATCH 12/12] Oops --- crates/stackable-operator/CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index ad1f49e8c..db62ad414 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -38,7 +38,6 @@ All notable changes to this project will be documented in this file. [#885]: https://github.com/stackabletech/operator-rs/pull/885 [#889]: https://github.com/stackabletech/operator-rs/pull/889 - ## [0.78.0] - 2024-09-30 ### Added