From d1a824f8d3c8b5bd624c7721ade90abb3eff180a Mon Sep 17 00:00:00 2001 From: Bartosz Blizniak Date: Thu, 4 Jun 2026 11:58:11 +0100 Subject: [PATCH 1/2] fix: metadata enum update --- CHANGELOG.md | 7 ++ cloudsmith_cli/cli/commands/metadata.py | 20 +---- .../cli/tests/commands/test_metadata.py | 23 +++--- cloudsmith_cli/core/api/metadata.py | 75 +++---------------- cloudsmith_cli/core/tests/test_metadata.py | 70 ++++------------- 5 files changed, 50 insertions(+), 145 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8009ae30..a6648a8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] +### Fixed + +- `metadata list` filters (`--source-kind`, `--classification`) now send the enum name the v2 API expects instead of an integer, fixing +an HTTP 400 on every filtered list. Valid source kinds: `unknown, system, upstream, custom, third_party`; classifications: `unknown, +intrinsic, security, provenance, sbom, generic`. + + ## [1.17.0] - 2026-05-18 ### Added diff --git a/cloudsmith_cli/cli/commands/metadata.py b/cloudsmith_cli/cli/commands/metadata.py index d80c3ab1..6fb80c07 100644 --- a/cloudsmith_cli/cli/commands/metadata.py +++ b/cloudsmith_cli/cli/commands/metadata.py @@ -9,8 +9,6 @@ delete_metadata as api_delete_metadata, get_metadata as api_get_metadata, list_metadata as api_list_metadata, - normalise_classification, - normalise_source_kind, update_metadata as api_update_metadata, ) from ...core.api.packages import get_package_slug_perm as api_get_package_slug_perm @@ -121,9 +119,8 @@ def metadata_(ctx, opts): # pylint: disable=unused-argument "source_kind", default=None, help=( - "Filter by source kind. Accepts an integer or name " - "(for example, 'customer' or 'third_party'). Ignored when " - "METADATA_SLUG_PERM is given." + "Filter by source kind name (one of: unknown, system, upstream, " + "custom, third_party). Ignored when METADATA_SLUG_PERM is given." ), ) @click.option( @@ -131,9 +128,8 @@ def metadata_(ctx, opts): # pylint: disable=unused-argument "classification", default=None, help=( - "Filter by classification. Accepts an integer or name " - "(for example, 'provenance' or 'sbom'). Ignored when " - "METADATA_SLUG_PERM is given." + "Filter by classification name (one of: unknown, intrinsic, security, " + "provenance, sbom, generic). Ignored when METADATA_SLUG_PERM is given." ), ) @click.pass_context @@ -187,14 +183,6 @@ def list_metadata( _print_metadata_entry(opts, entry) return - # Validate filter values up-front for a friendlier error than what the - # API would return (the normalisers raise ValueError on invalid values). - try: - normalise_source_kind(source_kind) - normalise_classification(classification) - except ValueError as exc: - raise click.UsageError(str(exc)) from exc - _echo_action( "Listing metadata for %(package)s ... " % {"package": click.style(package, bold=True)}, diff --git a/cloudsmith_cli/cli/tests/commands/test_metadata.py b/cloudsmith_cli/cli/tests/commands/test_metadata.py index f1bfe0b9..56fd4f17 100644 --- a/cloudsmith_cli/cli/tests/commands/test_metadata.py +++ b/cloudsmith_cli/cli/tests/commands/test_metadata.py @@ -111,16 +111,16 @@ def test_list_passes_filters(self, mock_resolve, mock_list): "list", "myorg/myrepo/mypkg", "--source-kind", - "customer", + "custom", "--classification", - "4", + "generic", ], ) self.assertEqual(result.exit_code, 0, msg=result.output) kwargs = mock_list.call_args.kwargs - self.assertEqual(kwargs["source_kind"], "customer") - self.assertEqual(kwargs["classification"], "4") + self.assertEqual(kwargs["source_kind"], "custom") + self.assertEqual(kwargs["classification"], "generic") @patch("cloudsmith_cli.cli.commands.metadata.api_list_metadata") @patch("cloudsmith_cli.cli.commands.metadata.api_get_package_slug_perm") @@ -132,7 +132,7 @@ def test_list_json_output(self, mock_resolve, mock_list): "slug_perm": "abc", "content_type": "application/json", "classification": "GENERIC", - "source_kind": "CUSTOMER", + "source_kind": "CUSTOM", "source_identity": "cloudsmith-cli@1.16.0", } ], @@ -150,17 +150,22 @@ def test_list_json_output(self, mock_resolve, mock_list): @patch("cloudsmith_cli.cli.commands.metadata.api_list_metadata") @patch("cloudsmith_cli.cli.commands.metadata.api_get_package_slug_perm") - def test_list_invalid_filter_value_is_usage_error(self, mock_resolve, mock_list): + def test_list_forwards_filter_name_without_client_validation( + self, mock_resolve, mock_list + ): + # Filter values are no longer validated client-side; an unknown name is + # forwarded verbatim (the backend is the source of truth and 4xxs it). mock_resolve.return_value = "pkg-slug-perm" + mock_list.return_value = ([], _empty_page_info()) result = self.runner.invoke( metadata_, ["list", "myorg/myrepo/mypkg", "--source-kind", "not-a-kind"], ) - self.assertNotEqual(result.exit_code, 0) - self.assertIn("source_kind", result.output.lower()) - mock_list.assert_not_called() + self.assertEqual(result.exit_code, 0, msg=result.output) + mock_list.assert_called_once() + self.assertEqual(mock_list.call_args.kwargs["source_kind"], "not-a-kind") @patch("cloudsmith_cli.cli.commands.metadata.api_list_metadata") @patch("cloudsmith_cli.cli.commands.metadata.api_get_package_slug_perm") diff --git a/cloudsmith_cli/core/api/metadata.py b/cloudsmith_cli/core/api/metadata.py index f3df8a60..58e11a53 100644 --- a/cloudsmith_cli/core/api/metadata.py +++ b/cloudsmith_cli/core/api/metadata.py @@ -10,60 +10,6 @@ from ..rest import RestClient from .exceptions import catch_raise_api_exception -SOURCE_KIND_VALUES = { - "unknown": 0, - "system": 1, - "ecosystem": 2, - "customer": 3, - "third_party": 4, -} - -CLASSIFICATION_VALUES = { - "unknown": 0, - "intrinsic": 1, - "upstream": 2, - "security": 3, - "provenance": 4, - "sbom": 5, - "generic": 6, -} - - -def _normalise_enum(value, mapping, name): - if value is None: - return None - if isinstance(value, bool): - raise ValueError(f"Invalid {name} value: {value!r}") - if isinstance(value, int): - return value - if isinstance(value, str): - text = value.strip() - if not text: - raise ValueError(f"Invalid {name} value: {value!r}") - try: - return int(text) - except ValueError: - pass - key = text.lower().replace("-", "_") - try: - return mapping[key] - except KeyError: - valid = ", ".join(sorted(mapping)) - raise ValueError( - f"Invalid {name} {value!r}. Expected an integer or one of: {valid}." - ) - raise ValueError(f"Invalid {name} type: {type(value).__name__}") - - -def normalise_source_kind(value): - """Coerce a MetadataSourceKind name or integer to its integer value.""" - return _normalise_enum(value, SOURCE_KIND_VALUES, "source_kind") - - -def normalise_classification(value): - """Coerce a MetadataClassification name or integer to its integer value.""" - return _normalise_enum(value, CLASSIFICATION_VALUES, "classification") - class _MetadataApi: """Small client for metadata endpoints not yet present in cloudsmith_api.""" @@ -136,29 +82,26 @@ def _response_json(response): def list_metadata( package_slug_perm: str, *, - source_kind: int | str | None = None, - classification: int | str | None = None, + source_kind: str | None = None, + classification: str | None = None, page: int | None = None, page_size: int | None = None, ): """List metadata entries attached to a package. - `source_kind` and `classification` may be supplied as either an integer - or the matching enum name (case-insensitive); both are converted to the - integer the v2 API expects before the request is issued. + `source_kind` and `classification` are sent as the lowercased enum name + the v2 API expects; the authoritative backend validates the value and + surfaces a 4xx for anything it does not recognise. Returns a (results, PageInfo) tuple. """ client = get_metadata_api() api_kwargs = {} - source_kind_value = normalise_source_kind(source_kind) - if source_kind_value is not None: - api_kwargs["source_kind"] = source_kind_value - - classification_value = normalise_classification(classification) - if classification_value is not None: - api_kwargs["classification"] = classification_value + if source_kind: + api_kwargs["source_kind"] = str(source_kind).strip().lower() + if classification: + api_kwargs["classification"] = str(classification).strip().lower() api_kwargs.update(utils.get_page_kwargs(page=page, page_size=page_size)) diff --git a/cloudsmith_cli/core/tests/test_metadata.py b/cloudsmith_cli/core/tests/test_metadata.py index d0c326f3..778af47a 100644 --- a/cloudsmith_cli/core/tests/test_metadata.py +++ b/cloudsmith_cli/core/tests/test_metadata.py @@ -53,52 +53,6 @@ def _last_request(): return httpretty.last_request() -class TestNormalisers: - @pytest.mark.parametrize( - "value, expected", - [ - (None, None), - (3, 3), - ("3", 3), - ("customer", 3), - ("CUSTOMER", 3), - ("Third-Party", 4), - ("third_party", 4), - ], - ) - def test_source_kind(self, value, expected): - assert metadata.normalise_source_kind(value) == expected - - @pytest.mark.parametrize( - "value, expected", - [ - (None, None), - (6, 6), - ("generic", 6), - ("GENERIC", 6), - ("PROVENANCE", 4), - ], - ) - def test_classification(self, value, expected): - assert metadata.normalise_classification(value) == expected - - def test_invalid_source_kind_name(self): - with pytest.raises(ValueError, match="Invalid source_kind"): - metadata.normalise_source_kind("not-a-kind") - - def test_invalid_classification_name(self): - with pytest.raises(ValueError, match="Invalid classification"): - metadata.normalise_classification("nope") - - def test_invalid_type(self): - with pytest.raises(ValueError): - metadata.normalise_source_kind(3.14) - - def test_bool_rejected(self): - with pytest.raises(ValueError): - metadata.normalise_source_kind(True) - - class TestListMetadata: @httpretty.activate(allow_net_connect=False) def test_success_returns_results_and_page_info(self): @@ -128,7 +82,7 @@ def test_success_returns_results_and_page_info(self): assert sent.headers.get("Accept") == "application/json" @httpretty.activate(allow_net_connect=False) - def test_filters_normalised_to_integers(self): + def test_filters_sent_as_lowercased_names(self): httpretty.register_uri( httpretty.GET, LIST_URL, @@ -138,12 +92,12 @@ def test_filters_normalised_to_integers(self): ) metadata.list_metadata( - PKG, source_kind="customer", classification="GENERIC", page=2, page_size=50 + PKG, source_kind="custom", classification="GENERIC", page=2, page_size=50 ) qs = _last_request().querystring # pylint: disable=no-member - assert qs["source_kind"] == ["3"] - assert qs["classification"] == ["6"] + assert qs["source_kind"] == ["custom"] + assert qs["classification"] == ["generic"] assert qs["page"] == ["2"] assert qs["page_size"] == ["50"] @@ -180,10 +134,16 @@ def test_404_raises_api_exception(self): assert exc_info.value.detail == "Not found." @httpretty.activate(allow_net_connect=False) - def test_422_raises_with_fields(self): + def test_invalid_filter_name_surfaces_backend_error(self): + # The CLI no longer validates filter names client-side; an unknown + # name is forwarded verbatim and the backend rejects it via EnumFieldV2. + message = ( + "bogus is not valid for source_kind - must be one of " + "['UNKNOWN', 'SYSTEM', 'UPSTREAM', 'CUSTOM', 'THIRD_PARTY']" + ) body = { "detail": "Invalid query parameters.", - "fields": {"source_kind": ["Not a valid choice."]}, + "fields": {"source_kind": [message]}, } httpretty.register_uri( httpretty.GET, @@ -194,10 +154,12 @@ def test_422_raises_with_fields(self): ) with pytest.raises(ApiException) as exc_info: - metadata.list_metadata(PKG, source_kind=3) + metadata.list_metadata(PKG, source_kind="bogus") + qs = _last_request().querystring # pylint: disable=no-member + assert qs["source_kind"] == ["bogus"] assert exc_info.value.status == 422 - assert exc_info.value.fields == {"source_kind": ["Not a valid choice."]} + assert exc_info.value.fields == {"source_kind": [message]} class TestGetMetadata: From 21f2c6d3fa51edab18a16ad6b7aed7e674c50f25 Mon Sep 17 00:00:00 2001 From: Bartosz Blizniak Date: Thu, 4 Jun 2026 12:38:11 +0100 Subject: [PATCH 2/2] fix: copilot feedback --- CHANGELOG.md | 4 +--- cloudsmith_cli/core/api/metadata.py | 10 ++++++---- cloudsmith_cli/core/tests/test_metadata.py | 23 ++++++++++++++++++++-- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6648a8e..415e2c1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,9 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed -- `metadata list` filters (`--source-kind`, `--classification`) now send the enum name the v2 API expects instead of an integer, fixing -an HTTP 400 on every filtered list. Valid source kinds: `unknown, system, upstream, custom, third_party`; classifications: `unknown, -intrinsic, security, provenance, sbom, generic`. +- `metadata list` filters (`--source-kind`, `--classification`) now send the enum name the v2 API expects instead of an integer, fixing an HTTP 400 on every filtered list. Valid source kinds: `unknown, system, upstream, custom, third_party`; classifications: `unknown, intrinsic, security, provenance, sbom, generic`. ## [1.17.0] - 2026-05-18 diff --git a/cloudsmith_cli/core/api/metadata.py b/cloudsmith_cli/core/api/metadata.py index 58e11a53..5ce1fc1f 100644 --- a/cloudsmith_cli/core/api/metadata.py +++ b/cloudsmith_cli/core/api/metadata.py @@ -98,10 +98,12 @@ def list_metadata( client = get_metadata_api() api_kwargs = {} - if source_kind: - api_kwargs["source_kind"] = str(source_kind).strip().lower() - if classification: - api_kwargs["classification"] = str(classification).strip().lower() + source_kind_value = str(source_kind).strip().lower() if source_kind else "" + if source_kind_value: + api_kwargs["source_kind"] = source_kind_value + classification_value = str(classification).strip().lower() if classification else "" + if classification_value: + api_kwargs["classification"] = classification_value api_kwargs.update(utils.get_page_kwargs(page=page, page_size=page_size)) diff --git a/cloudsmith_cli/core/tests/test_metadata.py b/cloudsmith_cli/core/tests/test_metadata.py index 778af47a..2c241857 100644 --- a/cloudsmith_cli/core/tests/test_metadata.py +++ b/cloudsmith_cli/core/tests/test_metadata.py @@ -117,6 +117,24 @@ def test_non_positive_page_options_omitted(self): assert "page" not in qs assert "page_size" not in qs + @httpretty.activate(allow_net_connect=False) + def test_blank_filters_omitted(self): + # A whitespace-only filter normalises to an empty string, which must be + # dropped rather than sent as an empty query param (the backend 4xxs it). + httpretty.register_uri( + httpretty.GET, + LIST_URL, + body=json.dumps({"results": []}), + status=200, + content_type="application/json", + ) + + metadata.list_metadata(PKG, source_kind=" ", classification="") + + qs = _last_request().querystring # pylint: disable=no-member + assert "source_kind" not in qs + assert "classification" not in qs + @httpretty.activate(allow_net_connect=False) def test_404_raises_api_exception(self): httpretty.register_uri( @@ -135,8 +153,9 @@ def test_404_raises_api_exception(self): @httpretty.activate(allow_net_connect=False) def test_invalid_filter_name_surfaces_backend_error(self): - # The CLI no longer validates filter names client-side; an unknown - # name is forwarded verbatim and the backend rejects it via EnumFieldV2. + # The API client lowercases/strips filter values but does not validate + # them; an unknown name is forwarded and the backend rejects it via + # EnumFieldV2. message = ( "bogus is not valid for source_kind - must be one of " "['UNKNOWN', 'SYSTEM', 'UPSTREAM', 'CUSTOM', 'THIRD_PARTY']"