diff --git a/e2e/test/e2e_test.go b/e2e/test/e2e_test.go index cbc31485e..bd6785909 100644 --- a/e2e/test/e2e_test.go +++ b/e2e/test/e2e_test.go @@ -387,7 +387,11 @@ var _ = Describe("Core E2E Tests", Label("core"), Ordered, func() { Expect(err).NotTo(HaveOccurred(), out) Expect(out).To(ContainSubstring("!nonexistent")) - out, err = Jmp("get", "leases", "--selector", "example.com/board=sa,!production") + out, err = Jmp("get", "leases", "--selector", "example.com/board=sa,!production", "-o", "yaml") + Expect(err).NotTo(HaveOccurred(), out) + Expect(out).To(ContainSubstring("example.com/board=sa")) + + out, err = Jmp("get", "leases", "--selector", "example.com/board=sa,!example.com/board") Expect(err).NotTo(HaveOccurred(), out) Expect(out).To(Equal("No resources found.")) diff --git a/python/packages/jumpstarter/jumpstarter/client/grpc.py b/python/packages/jumpstarter/jumpstarter/client/grpc.py index 6e2b575cf..7d1f6aab0 100644 --- a/python/packages/jumpstarter/jumpstarter/client/grpc.py +++ b/python/packages/jumpstarter/jumpstarter/client/grpc.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +import logging from collections import OrderedDict from dataclasses import InitVar, dataclass, field from datetime import datetime, timedelta @@ -16,6 +17,8 @@ from jumpstarter.common import ExporterStatus from jumpstarter.common.grpc import translate_grpc_exceptions +logger = logging.getLogger(__name__) + @dataclass class WithOptions: @@ -370,7 +373,13 @@ def filter_by_selector(self, filter_selector: str | None) -> LeaseList: """ if not filter_selector: return self - filtered = [lease for lease in self.leases if selector_contains(lease.selector, filter_selector)] + filtered = [] + for lease in self.leases: + try: + if selector_contains(lease.selector, filter_selector): + filtered.append(lease) + except ValueError: + logger.warning("skipping lease %s: unable to evaluate selector %r", lease.name, lease.selector) return LeaseList(leases=filtered, next_page_token=None) def filter_by_client(self, client_name: str) -> LeaseList: diff --git a/python/packages/jumpstarter/jumpstarter/client/grpc_test.py b/python/packages/jumpstarter/jumpstarter/client/grpc_test.py index 20f4e904f..84195d109 100644 --- a/python/packages/jumpstarter/jumpstarter/client/grpc_test.py +++ b/python/packages/jumpstarter/jumpstarter/client/grpc_test.py @@ -10,6 +10,7 @@ ClientService, Exporter, Lease, + LeaseList, WithOptions, add_display_columns, add_exporter_row, @@ -532,6 +533,34 @@ def test_rich_display_empty_tags(self): assert "TAGS" in columns +class TestLeaseListFilterBySelector: + def create_lease(self, name="test-lease", selector="board=rpi"): + return Lease( + namespace="default", + name=name, + selector=selector, + duration=timedelta(hours=1), + client="test-client", + exporter="test-exporter", + conditions=[], + ) + + def test_filter_keeps_matching_and_excludes_erroring_leases(self): + leases = LeaseList( + leases=[ + self.create_lease(name="good", selector="board=rpi"), + self.create_lease(name="bad", selector="board=jetson"), + ], + next_page_token=None, + ) + with patch( + "jumpstarter.client.grpc.selector_contains", + side_effect=[True, ValueError("unknown label selector operator: 'bogus'")], + ): + result = leases.filter_by_selector("board=rpi") + assert [lease.name for lease in result.leases] == ["good"] + + @pytest.mark.asyncio async def test_create_lease_sets_tags_on_protobuf(): from jumpstarter_protocol import client_pb2 diff --git a/python/packages/jumpstarter/jumpstarter/client/selectors.py b/python/packages/jumpstarter/jumpstarter/client/selectors.py index d7225fec9..9490b7db0 100644 --- a/python/packages/jumpstarter/jumpstarter/client/selectors.py +++ b/python/packages/jumpstarter/jumpstarter/client/selectors.py @@ -67,11 +67,26 @@ def extract_match_labels_filter(selector: str | None) -> str | None: return ",".join(f"{k}={v}" for k, v in match_labels.items()) +def _label_satisfies_expression(sel_labels: dict[str, str], key: str, operator: str, values: list[str]) -> bool: + if operator == "!exists": + return key not in sel_labels + if operator in ("notin", "!="): + return key not in sel_labels or sel_labels[key] not in values + if key not in sel_labels: + return False + if operator == "in": + return sel_labels[key] in values + if operator == "exists": + return True + raise ValueError(f"unknown label selector operator: {operator!r}") + + def selector_contains(selector: str, requirements: str) -> bool: - """Check if selector contains all criteria from requirements. + """Check if selector satisfies all criteria from requirements. - Returns True if all matchLabels and matchExpressions in `requirements` - are present in `selector`. + Returns True if all matchLabels in `requirements` are present in `selector` + and all matchExpressions in `requirements` are satisfied by `selector` + (either by exact match in matchExpressions or by evaluation against matchLabels). """ if not requirements or not requirements.strip(): return True @@ -84,13 +99,12 @@ def selector_contains(selector: str, requirements: str) -> bool: if sel_labels.get(key) != value: return False - # All required matchExpressions must be in selector's matchExpressions + # All required matchExpressions must be satisfied by selector's + # matchExpressions or matchLabels for r_key, r_op, r_vals in req_exprs: - found = False - for s_key, s_op, s_vals in sel_exprs: - if s_key == r_key and s_op == r_op and set(s_vals) == set(r_vals): - found = True - break + found = any(s_key == r_key and s_op == r_op and set(s_vals) == set(r_vals) for s_key, s_op, s_vals in sel_exprs) + if not found: + found = _label_satisfies_expression(sel_labels, r_key, r_op, r_vals) if not found: return False diff --git a/python/packages/jumpstarter/jumpstarter/client/selectors_test.py b/python/packages/jumpstarter/jumpstarter/client/selectors_test.py index 23f629aae..3839ddd84 100644 --- a/python/packages/jumpstarter/jumpstarter/client/selectors_test.py +++ b/python/packages/jumpstarter/jumpstarter/client/selectors_test.py @@ -1,10 +1,74 @@ """Tests for label selector matching.""" -from jumpstarter.client.selectors import selector_contains +import pytest + +from jumpstarter.client.selectors import _label_satisfies_expression, selector_contains + + +class TestLabelSatisfiesExpressionIn: + def test_key_present_value_matches(self): + assert _label_satisfies_expression({"board": "rpi"}, "board", "in", ["rpi", "jetson"]) is True + + def test_key_present_value_does_not_match(self): + assert _label_satisfies_expression({"board": "rpi"}, "board", "in", ["jetson", "nano"]) is False + + def test_key_absent(self): + assert _label_satisfies_expression({}, "board", "in", ["rpi"]) is False + + +class TestLabelSatisfiesExpressionNotIn: + def test_key_present_value_not_in_set(self): + assert _label_satisfies_expression({"board": "rpi"}, "board", "notin", ["jetson", "nano"]) is True + + def test_key_present_value_in_set(self): + assert _label_satisfies_expression({"board": "rpi"}, "board", "notin", ["rpi", "nano"]) is False + + def test_key_absent(self): + assert _label_satisfies_expression({}, "board", "notin", ["rpi"]) is True + + +class TestLabelSatisfiesExpressionExists: + def test_key_present(self): + assert _label_satisfies_expression({"board": "rpi"}, "board", "exists", []) is True + + def test_key_absent(self): + assert _label_satisfies_expression({}, "board", "exists", []) is False + + +class TestLabelSatisfiesExpressionDoesNotExist: + def test_key_present(self): + assert _label_satisfies_expression({"board": "rpi"}, "board", "!exists", []) is False + + def test_key_absent(self): + assert _label_satisfies_expression({}, "board", "!exists", []) is True + + +class TestLabelSatisfiesExpressionNotEqual: + def test_key_present_value_differs(self): + assert _label_satisfies_expression({"board": "rpi"}, "board", "!=", ["jetson"]) is True + + def test_key_present_value_same(self): + assert _label_satisfies_expression({"board": "rpi"}, "board", "!=", ["rpi"]) is False + + def test_key_absent(self): + assert _label_satisfies_expression({}, "board", "!=", ["rpi"]) is True + + +class TestLabelSatisfiesExpressionUnknownOperator: + def test_raises_value_error(self): + with pytest.raises(ValueError, match="unknown label selector operator"): + _label_satisfies_expression({"key": "val"}, "key", "bogus", ["val"]) + + def test_error_message_includes_operator(self): + with pytest.raises(ValueError, match="'bogus'"): + _label_satisfies_expression({"key": "val"}, "key", "bogus", ["val"]) + + def test_empty_string_operator_raises(self): + with pytest.raises(ValueError, match="unknown label selector operator"): + _label_satisfies_expression({"key": "val"}, "key", "", ["val"]) class TestSelectorContains: - """Tests for checking if a lease's selector contains a filter's criteria.""" def test_exact_match_labels(self): assert selector_contains("board=rpi", "board=rpi") is True @@ -12,6 +76,12 @@ def test_exact_match_labels(self): def test_subset_match_labels(self): assert selector_contains("board=rpi,env=prod", "board=rpi") is True + def test_double_equals_match(self): + assert selector_contains("board=rpi", "board==rpi") is True + + def test_double_equals_in_selector(self): + assert selector_contains("board==rpi", "board=rpi") is True + def test_no_match_labels(self): assert selector_contains("board=jetson", "board=rpi") is False @@ -27,15 +97,43 @@ def test_match_mixed(self): def test_no_match_expression(self): assert selector_contains("board=rpi", "firmware in (v2, v3)") is False - def test_filter_not_exists(self): + def test_filter_not_exists_present_in_selector(self): assert selector_contains("board=rpi,!experimental", "!experimental") is True - assert selector_contains("board=rpi", "!experimental") is False + + def test_filter_not_exists_absent_from_selector(self): + assert selector_contains("board=rpi", "!experimental") is True + + def test_filter_not_exists_key_present_in_labels(self): + assert selector_contains("experimental=true", "!experimental") is False def test_empty_filter_matches_all(self): assert selector_contains("board=rpi,firmware in (v2, v3)", "") is True + def test_match_label_satisfies_in_expression(self): + assert selector_contains("board=rpi", "board in (rpi, jetson)") is True + + def test_match_label_does_not_satisfy_in_expression(self): + assert selector_contains("board=rpi", "board in (jetson, nano)") is False + + def test_match_label_satisfies_notin_expression(self): + assert selector_contains("board=rpi", "board notin (jetson, nano)") is True + + def test_match_label_does_not_satisfy_notin_expression(self): + assert selector_contains("board=rpi", "board notin (rpi, nano)") is False + + def test_notin_key_absent_from_selector(self): + assert selector_contains("board=rpi", "env notin (prod)") is True + + def test_not_equal_key_absent_from_selector(self): + assert selector_contains("board=rpi", "env!=prod") is True + + def test_exists_key_present_in_selector(self): + assert selector_contains("board=rpi", "board") is True + + def test_exists_key_absent_from_selector(self): + assert selector_contains("board=rpi", "env") is False + def test_whitespace_tolerance(self): - """Whitespace around operators should be tolerated (matching Go behavior).""" assert selector_contains("board=rpi", "board = rpi") is True assert selector_contains("board=rpi", "board =rpi") is True assert selector_contains("board=rpi", "board= rpi") is True