Skip to content

Commit

Permalink
Use TryFrom for LabelSelector conversion, drop Expression::Invalid
Browse files Browse the repository at this point in the history 
Signed-off-by: Danil-Grigorev <[email protected]>
  • Loading branch information
@Danil-Grigorev
Danil-Grigorev committed Jul 19, 2024
1 parent 4fe0ab5 commit cf91e40
Showing 1 changed file with 131 additions and 29 deletions.
160 changes: 131 additions & 29 deletions kube-core/src/labels.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Expression>;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -160,9 +163,20 @@ impl Selector {
}

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<String, String>) -> bool {
let selector: Selector = self.clone().into();
selector.matches(labels)
let selector: Result<Selector, ParseExpressionError> = self.clone().try_into();
match selector {
Ok(selector) => selector.matches(labels),
Err(_) => false,

Check warning on line 178 in kube-core/src/labels.rs

View check run for this annotation

Codecov / codecov/patch

kube-core/src/labels.rs#L174-L178 

Added lines #L174 - L178 were not covered by tests
}
}
}

Expand Down Expand Up @@ -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,
}
}
}
Expand All @@ -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(()),
}
}
}
Expand Down Expand Up @@ -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<T: IntoIterator<Item = (&'static str, &'static str)>>(iter: T) -> Self {
Self::from_map(
iter.into_iter()
Expand All @@ -280,37 +299,45 @@ impl From<Expression> for Selector {
}
}

impl From<LabelSelector> for Selector {
fn from(value: LabelSelector) -> Self {
impl TryFrom<LabelSelector> for Selector {
type Error = ParseExpressionError;

fn try_from(value: LabelSelector) -> Result<Self, Self::Error> {
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<LabelSelectorRequirement> for Expression {
fn from(requirement: LabelSelectorRequirement) -> Self {
impl TryFrom<LabelSelectorRequirement> for Expression {
type Error = ParseExpressionError;

fn try_from(requirement: LabelSelectorRequirement) -> Result<Self, Self::Error> {
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(),

Check warning on line 329 in kube-core/src/labels.rs

View check run for this annotation

Codecov / codecov/patch

kube-core/src/labels.rs#L328-L329 

Added lines #L328 - L329 were not covered by tests
)),
},
"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(),

Check warning on line 335 in kube-core/src/labels.rs

View check run for this annotation

Codecov / codecov/patch

kube-core/src/labels.rs#L334-L335 

Added lines #L334 - L335 were not covered by tests
)),
},
"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())),

Check warning on line 340 in kube-core/src/labels.rs

View check run for this annotation

Codecov / codecov/patch

kube-core/src/labels.rs#L340 

Added line #L340 was not covered by tests
}
}
}
Expand Down Expand Up @@ -347,7 +374,6 @@ impl From<Selector> for LabelSelector {
operator: "DoesNotExist".into(),
values: None,

Check warning on line 375 in kube-core/src/labels.rs

View check run for this annotation

Codecov / codecov/patch

kube-core/src/labels.rs#L372-L375 

Added lines #L372 - L375 were not covered by tests
}),
Expression::Invalid => (),
}
}

Expand All @@ -365,16 +391,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()),
Expand All @@ -388,6 +428,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()),
Expand All @@ -401,6 +449,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()),
Expand All @@ -414,6 +466,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()),
Expand All @@ -427,6 +487,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()),
Expand All @@ -440,6 +508,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()),
Expand All @@ -453,6 +529,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()),
Expand All @@ -466,6 +550,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()),
Expand All @@ -479,6 +571,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()),
Expand All @@ -489,8 +589,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,
Expand Down Expand Up @@ -527,7 +628,8 @@ 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()));
}
Expand Down

0 comments on commit cf91e40

Please sign in to comment.