From fa7b856a49ac785a82ac27fddad7361709c8239b Mon Sep 17 00:00:00 2001 From: Danil-Grigorev Date: Fri, 19 Jul 2024 11:24:01 +0200 Subject: [PATCH] Use TryFrom for LabelSelector conversion, drop Expression::Invalid Signed-off-by: Danil-Grigorev --- kube-core/src/labels.rs | 163 ++++++++++++++++++++++++++++++++-------- kube-core/src/params.rs | 4 - 2 files changed, 130 insertions(+), 37 deletions(-) diff --git a/kube-core/src/labels.rs b/kube-core/src/labels.rs index 488d66da4..9872a7f70 100644 --- a/kube-core/src/labels.rs +++ b/kube-core/src/labels.rs @@ -9,9 +9,15 @@ use std::{ iter::FromIterator, option::IntoIter, }; +use thiserror::Error; use crate::ResourceExt; +#[derive(Debug, Error)] +#[error("failed to parse value as expression: {0}")] +/// Indicates failure of conversion to Expression +pub struct ParseExpressionError(pub String); + // local type aliases type Expressions = Vec; @@ -115,9 +121,6 @@ pub enum Expression { /// assert_eq!(exp, "!foo") /// ``` DoesNotExist(String), - - /// Invalid combination. Always evaluates to false. - Invalid, } /// Perform selection on a list of expressions @@ -147,22 +150,33 @@ impl Selector { /// use kube_core::Expression; /// use k8s_openapi::apimachinery::pkg::apis::meta::v1::LabelSelector; /// - /// let label_selector: Selector = LabelSelector::default().into(); - /// let mut selector = Selector::default(); - /// selector.extend(Expression::Equal("environment".into(), "production".into())); + /// let label_selector: Selector = LabelSelector::default().try_into().unwrap(); + /// let mut selector = &mut Selector::default(); + /// selector = selector.extend(Expression::Equal("environment".into(), "production".into())); /// selector.extend([Expression::Exists("bar".into()), Expression::Exists("foo".into())].into_iter()); /// selector.extend(label_selector); /// ``` - pub fn extend(&mut self, exprs: impl IntoIterator) -> &Self { + pub fn extend(&mut self, exprs: impl IntoIterator) -> &mut Self { self.0.extend(exprs); self } } impl Matcher for LabelSelector { + /// Perform best effort match on label selector + /// + /// ``` + /// use k8s_openapi::apimachinery::pkg::apis::meta::v1::LabelSelector; + /// use kube_core::Matcher; + /// + /// LabelSelector::default().matches(&Default::default()); + /// ``` fn matches(&self, labels: &BTreeMap) -> bool { - let selector: Selector = self.clone().into(); - selector.matches(labels) + let selector: Result = self.clone().try_into(); + match selector { + Ok(selector) => selector.matches(labels), + Err(_) => false, + } } } @@ -193,7 +207,6 @@ impl Matcher for Expression { Expression::DoesNotExist(key) => !labels.contains_key(key), Expression::Equal(key, value) => labels.get(key) == Some(value), Expression::NotEqual(key, value) => labels.get(key) != Some(value), - Expression::Invalid => false, } } } @@ -220,7 +233,6 @@ impl Display for Expression { Expression::NotEqual(key, value) => write!(f, "{key}!={value}"), Expression::Exists(key) => write!(f, "{key}"), Expression::DoesNotExist(key) => write!(f, "!{key}"), - Expression::Invalid => Ok(()), } } } @@ -259,6 +271,13 @@ impl FromIterator<(String, String)> for Selector { } impl FromIterator<(&'static str, &'static str)> for Selector { + /// ``` + /// use kube_core::{Selector, Expression}; + /// + /// let sel: Selector = Some(("foo", "bar")).into_iter().collect(); + /// let equal: Selector = Expression::Equal("foo".into(), "bar".into()).into(); + /// assert_eq!(sel, equal) + /// ``` fn from_iter>(iter: T) -> Self { Self::from_map( iter.into_iter() @@ -280,37 +299,41 @@ impl From for Selector { } } -impl From for Selector { - fn from(value: LabelSelector) -> Self { +impl TryFrom for Selector { + type Error = ParseExpressionError; + + fn try_from(value: LabelSelector) -> Result { let expressions = match value.match_expressions { - Some(requirements) => requirements.into_iter().map(Into::into).collect(), - None => vec![], - }; + Some(requirements) => requirements.into_iter().map(TryInto::try_into).collect(), + None => Ok(vec![]), + }?; let mut equality: Selector = value .match_labels .map(|labels| labels.into_iter().collect()) .unwrap_or_default(); - equality.0.extend(expressions); - equality + equality.extend(expressions); + Ok(equality) } } -impl From for Expression { - fn from(requirement: LabelSelectorRequirement) -> Self { +impl TryFrom for Expression { + type Error = ParseExpressionError; + + fn try_from(requirement: LabelSelectorRequirement) -> Result { let key = requirement.key; let values = requirement.values.map(|values| values.into_iter().collect()); match requirement.operator.as_str() { "In" => match values { - Some(values) => Expression::In(key, values), - None => Expression::Invalid, + Some(values) => Ok(Expression::In(key, values)), + None => Err(ParseExpressionError("Expected values for In operator, got none".into())), }, "NotIn" => match values { - Some(values) => Expression::NotIn(key, values), - None => Expression::Invalid, + Some(values) => Ok(Expression::NotIn(key, values)), + None => Err(ParseExpressionError("Expected values for In operator, got none".into())), }, - "Exists" => Expression::Exists(key), - "DoesNotExist" => Expression::DoesNotExist(key), - _ => Expression::Invalid, + "Exists" => Ok(Expression::Exists(key)), + "DoesNotExist" => Ok(Expression::DoesNotExist(key)), + _ => Err(ParseExpressionError("Invalid expression operator".into())), } } } @@ -347,7 +370,6 @@ impl From for LabelSelector { operator: "DoesNotExist".into(), values: None, }), - Expression::Invalid => (), } } @@ -365,16 +387,30 @@ mod tests { #[test] fn test_raw_matches() { - for (selector, labels, matches, msg) in &[ - (Selector::default(), Default::default(), true, "empty match"), + for (selector, label_selector, labels, matches, msg) in &[ + ( + Selector::default(), + LabelSelector::default(), + Default::default(), + true, + "empty match", + ), ( Selector::from_iter(Some(("foo", "bar"))), + LabelSelector { + match_labels: Some([("foo".into(), "bar".into())].into()), + match_expressions: Default::default(), + }, [("foo".to_string(), "bar".to_string())].into(), true, "exact label match", ), ( Selector::from_iter(Some(("foo", "bar"))), + LabelSelector { + match_labels: Some([("foo".to_string(), "bar".to_string())].into()), + match_expressions: None, + }, [ ("foo".to_string(), "bar".to_string()), ("bah".to_string(), "baz".to_string()), @@ -388,6 +424,14 @@ mod tests { "foo".into(), Some("bar".to_string()).into_iter().collect(), ))), + LabelSelector { + match_labels: None, + match_expressions: Some(vec![LabelSelectorRequirement { + key: "foo".into(), + operator: "In".to_string(), + values: Some(vec!["bar".into()]), + }]), + }, [ ("foo".to_string(), "bar".to_string()), ("bah".to_string(), "baz".to_string()), @@ -401,6 +445,10 @@ mod tests { "foo".into(), Some("bar".to_string()).into_iter().collect(), ))), + LabelSelector { + match_labels: Some([("foo".into(), "bar".into())].into()), + match_expressions: None, + }, [ ("foo".to_string(), "bar".to_string()), ("bah".to_string(), "baz".to_string()), @@ -414,6 +462,14 @@ mod tests { "foo".into(), Some("bar".to_string()).into_iter().collect(), ))), + LabelSelector { + match_labels: None, + match_expressions: Some(vec![LabelSelectorRequirement { + key: "foo".into(), + operator: "NotIn".into(), + values: Some(vec!["bar".into()]), + }]), + }, [ ("foo".to_string(), "bar".to_string()), ("bah".to_string(), "baz".to_string()), @@ -427,6 +483,14 @@ mod tests { "foo".into(), Some("bar".to_string()).into_iter().collect(), ))), + LabelSelector { + match_labels: None, + match_expressions: Some(vec![LabelSelectorRequirement { + key: "foo".into(), + operator: "In".into(), + values: Some(vec!["bar".into()]), + }]), + }, [ ("foo".to_string(), "bar".to_string()), ("bah".to_string(), "baz".to_string()), @@ -440,6 +504,14 @@ mod tests { "foo".into(), Some("quux".to_string()).into_iter().collect(), ))), + LabelSelector { + match_labels: None, + match_expressions: Some(vec![LabelSelectorRequirement { + key: "foo".into(), + operator: "NotIn".into(), + values: Some(vec!["quux".into()]), + }]), + }, [ ("foo".to_string(), "bar".to_string()), ("bah".to_string(), "baz".to_string()), @@ -453,6 +525,14 @@ mod tests { "foo".into(), Some("bar".to_string()).into_iter().collect(), ))), + LabelSelector { + match_labels: None, + match_expressions: Some(vec![LabelSelectorRequirement { + key: "foo".into(), + operator: "NotIn".into(), + values: Some(vec!["bar".into()]), + }]), + }, [ ("foo".to_string(), "bar".to_string()), ("bah".to_string(), "baz".to_string()), @@ -466,6 +546,14 @@ mod tests { Expression::Equal("foo".to_string(), "bar".to_string()), Expression::In("bah".into(), Some("bar".to_string()).into_iter().collect()), ]), + LabelSelector { + match_labels: Some([("foo".into(), "bar".into())].into()), + match_expressions: Some(vec![LabelSelectorRequirement { + key: "bah".into(), + operator: "In".into(), + values: Some(vec!["bar".into()]), + }]), + }, [ ("foo".to_string(), "bar".to_string()), ("bah".to_string(), "baz".to_string()), @@ -479,6 +567,14 @@ mod tests { Expression::Equal("foo".to_string(), "bar".to_string()), Expression::In("bah".into(), Some("bar".to_string()).into_iter().collect()), ]), + LabelSelector { + match_labels: Some([("foo".into(), "bar".into())].into()), + match_expressions: Some(vec![LabelSelectorRequirement { + key: "bah".into(), + operator: "In".into(), + values: Some(vec!["bar".into()]), + }]), + }, [ ("foo".to_string(), "bar".to_string()), ("bah".to_string(), "bar".to_string()), @@ -489,8 +585,9 @@ mod tests { ), ] { assert_eq!(selector.matches(labels), *matches, "{}", msg); - let label_selector: LabelSelector = selector.clone().into(); - let converted_selector: Selector = label_selector.into(); + let converted: LabelSelector = selector.clone().into(); + assert_eq!(&converted, label_selector); + let converted_selector: Selector = converted.try_into().unwrap(); assert_eq!( converted_selector.matches(labels), *matches, @@ -527,7 +624,7 @@ mod tests { ]), match_labels: Some([("foo".into(), "bar".into())].into()), } - .into(); + .try_into().unwrap(); assert!(selector.matches(&[("foo".into(), "bar".into())].into())); assert!(!selector.matches(&Default::default())); } diff --git a/kube-core/src/params.rs b/kube-core/src/params.rs index 116cb57b7..7cd100d94 100644 --- a/kube-core/src/params.rs +++ b/kube-core/src/params.rs @@ -177,8 +177,6 @@ impl ListParams { /// let selector: Selector = Expression::In("env".into(), ["development".into(), "sandbox".into()].into()).into(); /// let lp = ListParams::default().labels_from(&selector); /// let lp = ListParams::default().labels_from(&Expression::Exists("foo".into()).into()); - /// // Alternatively the raw LabelSelector is accepted - /// let lp = ListParams::default().labels_from(&LabelSelector::default().into()); ///``` #[must_use] pub fn labels_from(mut self, selector: &Selector) -> Self { @@ -460,8 +458,6 @@ impl WatchParams { /// let selector: Selector = Expression::In("env".into(), ["development".into(), "sandbox".into()].into()).into(); /// let wp = WatchParams::default().labels_from(&selector); /// let wp = WatchParams::default().labels_from(&Expression::Exists("foo".into()).into()); - /// // Alternatively the raw LabelSelector is accepted - /// let wp = WatchParams::default().labels_from(&LabelSelector::default().into()); ///``` #[must_use] pub fn labels_from(mut self, selector: &Selector) -> Self {