diff --git a/CHANGES/230.feature b/CHANGES/230.feature new file mode 100644 index 000000000..16b125b8c --- /dev/null +++ b/CHANGES/230.feature @@ -0,0 +1 @@ +Updated the `--requirements` option for ansible remotes to handle both files and strings. diff --git a/pulpcore/cli/ansible/remote.py b/pulpcore/cli/ansible/remote.py index f158247db..77b7f5de0 100644 --- a/pulpcore/cli/ansible/remote.py +++ b/pulpcore/cli/ansible/remote.py @@ -16,6 +16,7 @@ href_option, label_command, list_command, + load_file_or_string_wrapper, name_option, pass_pulp_context, pulp_group, @@ -31,17 +32,17 @@ _ = translation.gettext -def _requirements_callback( +def requirements_file_callback( ctx: click.Context, param: click.Parameter, value: Any ) -> Optional[Union[str, Any]]: if value: - if param.name == "requirements_file": - return f"{yaml.safe_load(value)}" - elif param.name == "requirements": - return yaml.safe_load(f'"{value}"') + return f"{yaml.safe_load(value)}" return value +requirements_callback = load_file_or_string_wrapper(requirements_file_callback) + + @pulp_group() @click.option( "-t", @@ -71,26 +72,26 @@ def remote(ctx: click.Context, pulp_ctx: PulpCLIContext, remote_type: str) -> No collection_options = [ pulp_option( "--requirements-file", - callback=_requirements_callback, + callback=requirements_file_callback, type=click.File(), help=_("Collections only: a Collection requirements yaml"), allowed_with_contexts=collection_context, ), pulp_option( "--requirements", - callback=_requirements_callback, + callback=requirements_callback, help=_("Collections only: a string of a requirements yaml"), allowed_with_contexts=collection_context, ), pulp_option( "--auth-url", - callback=_requirements_callback, + callback=requirements_callback, help=_("Collections only: URL to receive a session token"), allowed_with_contexts=collection_context, ), pulp_option( "--token", - callback=_requirements_callback, + callback=requirements_callback, help=_("Collections only: token key use for authentication"), allowed_with_contexts=collection_context, ), diff --git a/pulpcore/cli/ansible/repository.py b/pulpcore/cli/ansible/repository.py index 79329a274..3a7c35d26 100644 --- a/pulpcore/cli/ansible/repository.py +++ b/pulpcore/cli/ansible/repository.py @@ -145,6 +145,7 @@ def repository(ctx: click.Context, pulp_ctx: PulpCLIContext, repo_type: str) -> ), href_option, ] + content_json_callback = create_content_json_callback(schema=CONTENT_LIST_SCHEMA) modify_options = [ click.option( diff --git a/pulpcore/cli/common/generic.py b/pulpcore/cli/common/generic.py index e80ebb426..9427c5b2a 100644 --- a/pulpcore/cli/common/generic.py +++ b/pulpcore/cli/common/generic.py @@ -1,7 +1,7 @@ import datetime import json import re -from functools import lru_cache +from functools import lru_cache, wraps from typing import IO, Any, Callable, Dict, Iterable, List, Mapping, Optional, Tuple, Type, Union import click @@ -331,59 +331,53 @@ def _version_callback( return value -# TODO: would be great to have enable this to take a validator, rather than having -# to build "on top of" it like I'm doing now w/ json_callback -def load_file_or_string_callback( - ctx: click.Context, param: click.Parameter, value: Optional[str] +def load_file_or_string_wrapper( + handler: Callable[[click.Context, click.Parameter, str], Any] = lambda c, p, x: x ) -> Any: - """Load string from input or from file if string starts with @.""" - the_content: str + """A wrapper that used for chaining or decorating callbacks that manipulate with input data.""" - # pass None and "" verbatim - if not value: - return value + @wraps(handler) + def _load_file_or_string_wrapper( + ctx: click.Context, param: click.Parameter, value: Optional[str] + ) -> Any: + """Load the string from input, or from file if the value starts with @.""" + if not value: + return value - if value.startswith("@"): - the_file = value[1:] - try: - with click.open_file(the_file, "r") as fp: - the_content = fp.read() - except OSError: - raise click.ClickException( - _("Failed to load content from {file}").format(file=the_file) - ) - else: - the_content = value + if value.startswith("@"): + the_file = value[1:] + try: + with click.open_file(the_file, "r") as fp: + the_content = fp.read() + except OSError: + raise click.ClickException( + _("Failed to load content from {file}").format(file=the_file) + ) + else: + the_content = value - return the_content + return handler(ctx, param, the_content) + return _load_file_or_string_wrapper -def load_json_callback(ctx: click.Context, param: click.Parameter, value: Optional[str]) -> Any: - """Load JSON from input string or from file if string starts with @.""" - # None or empty-str are legal - shortcircuit here - if not value: - return value +load_file_or_string_callback = load_file_or_string_wrapper() - # Now try to evaluate legal JSON - json_object: Any - json_string: str = load_file_or_string_callback(ctx, param, value) + +def json_callback(ctx: click.Context, param: click.Parameter, value: str) -> Any: try: - json_object = json.loads(json_string) + json_object = json.loads(value) except json.decoder.JSONDecodeError: raise click.ClickException(_("Failed to decode JSON")) else: return json_object -def labels_callback( - ctx: click.Context, param: click.Parameter, value: Optional[str] -) -> Optional[Dict[str, str]]: - # None is legal - shortcircuit here - if value is None: - return value +load_json_callback = load_file_or_string_wrapper(json_callback) + - value = load_json_callback(ctx, param, value) +def labels_callback(ctx: click.Context, param: click.Parameter, value: str) -> Any: + value = json_callback(ctx, param, value) if isinstance(value, dict) and all( (isinstance(key, str) and isinstance(val, str) for key, val in value.items()) ): @@ -391,14 +385,18 @@ def labels_callback( raise click.ClickException(_("Labels must be provided as a dictionary of strings.")) +load_labels_callback = load_file_or_string_wrapper(labels_callback) + + def create_content_json_callback( context_class: Optional[Type[PulpContentContext]] = None, schema: s.Schema = None ) -> Any: + @load_file_or_string_wrapper def _callback( - ctx: click.Context, param: click.Parameter, value: Optional[str] + ctx: click.Context, param: click.Parameter, value: str ) -> Optional[List[PulpContentContext]]: ctx_class = context_class - new_value = load_json_callback(ctx, param, value) + new_value = json_callback(ctx, param, value) if new_value is not None: if schema is not None: try: @@ -791,7 +789,7 @@ def _type_callback(ctx: click.Context, param: click.Parameter, value: Optional[s "Search for {entities} with these content hrefs in them (JSON list or " "@file containing a JSON list)" ), - callback=load_json_callback, + callback=load_file_or_string_callback, ) chunk_size_option = pulp_option( @@ -842,7 +840,7 @@ def _type_callback(ctx: click.Context, param: click.Parameter, value: Optional[s help=_( "JSON dictionary of labels to set on {entity} (or " "@file containing a JSON dictionary)" ), - callback=labels_callback, + callback=load_labels_callback, ) name_filter_options = [ diff --git a/pulpcore/cli/file/repository.py b/pulpcore/cli/file/repository.py index ad7f491e7..336a93852 100644 --- a/pulpcore/cli/file/repository.py +++ b/pulpcore/cli/file/repository.py @@ -16,10 +16,11 @@ create_content_json_callback, destroy_command, href_option, + json_callback, label_command, label_select_option, list_command, - load_json_callback, + load_file_or_string_wrapper, name_option, pass_pulp_context, pass_repository_context, @@ -69,10 +70,9 @@ def _content_callback(ctx: click.Context, param: click.Parameter, value: Any) -> CONTENT_LIST_SCHEMA = s.Schema([{"sha256": str, "relative_path": s.And(str, len)}]) -def _content_list_callback(ctx: click.Context, param: click.Parameter, value: Any) -> Any: - result = load_json_callback(ctx, param, value) - if result is None: - return None +@load_file_or_string_wrapper +def _content_list_callback(ctx: click.Context, param: click.Parameter, value: str) -> Any: + result = json_callback(ctx, param, value) try: return CONTENT_LIST_SCHEMA.validate(result) except s.SchemaError as e: @@ -125,13 +125,11 @@ def repository(ctx: click.Context, pulp_ctx: PulpCLIContext, repo_type: str) -> callback=_content_callback, ), ] -content_json_callback = create_content_json_callback( - PulpFileContentContext, schema=CONTENT_LIST_SCHEMA -) + modify_options = [ click.option( "--add-content", - callback=content_json_callback, + callback=create_content_json_callback(PulpFileContentContext, schema=CONTENT_LIST_SCHEMA), help=_( """JSON string with a list of objects to add to the repository. Each object must contain the following keys: "sha256", "relative_path". @@ -140,7 +138,7 @@ def repository(ctx: click.Context, pulp_ctx: PulpCLIContext, repo_type: str) -> ), click.option( "--remove-content", - callback=content_json_callback, + callback=create_content_json_callback(PulpFileContentContext, schema=CONTENT_LIST_SCHEMA), help=_( """JSON string with a list of objects to remove from the repository. Each object must contain the following keys: "sha256", "relative_path". diff --git a/pulpcore/cli/python/repository.py b/pulpcore/cli/python/repository.py index fead1216b..05b25486f 100644 --- a/pulpcore/cli/python/repository.py +++ b/pulpcore/cli/python/repository.py @@ -103,11 +103,11 @@ def repository(ctx: click.Context, pulp_ctx: PulpCLIContext, repo_type: str) -> expose_value=False, help=_("Filename of the python package"), ) -content_json_callback = create_content_json_callback(PulpPythonContentContext) + modify_options = [ click.option( "--add-content", - callback=content_json_callback, + callback=create_content_json_callback(PulpPythonContentContext), help=_( """JSON string with a list of objects to add to the repository. Each object should have the key: "filename" @@ -116,7 +116,7 @@ def repository(ctx: click.Context, pulp_ctx: PulpCLIContext, repo_type: str) -> ), click.option( "--remove-content", - callback=content_json_callback, + callback=create_content_json_callback(PulpPythonContentContext), help=_( """JSON string with a list of objects to remove from the repository. Each object should have the key: "filename" diff --git a/pulpcore/cli/rpm/repository.py b/pulpcore/cli/rpm/repository.py index f2ac45365..544406912 100644 --- a/pulpcore/cli/rpm/repository.py +++ b/pulpcore/cli/rpm/repository.py @@ -102,6 +102,7 @@ def repository(ctx: click.Context, pulp_ctx: PulpCLIContext, repo_type: str) -> content_json_callback = create_content_json_callback( PulpRpmPackageContext, schema=CONTENT_LIST_SCHEMA ) + modify_options = [ click.option( "--add-content", diff --git a/tests/scripts/pulp_ansible/test_remote.sh b/tests/scripts/pulp_ansible/test_remote.sh index 207041e3d..f2483cf1a 100755 --- a/tests/scripts/pulp_ansible/test_remote.sh +++ b/tests/scripts/pulp_ansible/test_remote.sh @@ -20,7 +20,8 @@ expect_succ pulp ansible remote -t "role" list expect_succ pulp ansible remote -t "collection" list expect_succ pulp ansible remote -t "role" update --remote "cli_test_ansible_role_remote" --download-concurrency "5" expect_succ pulp ansible remote -t "collection" update --remote "cli_test_ansible_collection_remote" --download-concurrency "5" -expect_fail pulp ansible remote -t "role" update --remote "cli_test_ansible_role_remote" --requirements "collections:\n - robertdebock.ansible_development_environment" +expect_fail pulp ansible remote -t "role" update --remote "cli_test_ansible_role_remote" --requirements "collections: + - robertdebock.ansible_development_environment" expect_succ pulp ansible remote -t "role" destroy --remote "cli_test_ansible_role_remote" expect_succ pulp ansible remote -t "collection" destroy --remote "cli_test_ansible_collection_remote" @@ -31,3 +32,9 @@ collections: - pulp.squeezer" > requirements.yml expect_succ pulp ansible remote create --name "cli_test_ansible_collection_remote" \ --requirements-file requirements.yml --url "$ANSIBLE_COLLECTION_REMOTE_URL" +expect_succ pulp ansible remote -t "collection" update --name "cli_test_ansible_collection_remote" \ + --requirements @requirements.yml +expect_succ pulp ansible remote -t "collection" update --name "cli_test_ansible_collection_remote" \ + --requirements "collections: + - testing.ansible_testing_content + - pulp.squeezer" diff --git a/tests/scripts/pulp_ansible/test_sign.sh b/tests/scripts/pulp_ansible/test_sign.sh index 325389be6..e54a669e1 100755 --- a/tests/scripts/pulp_ansible/test_sign.sh +++ b/tests/scripts/pulp_ansible/test_sign.sh @@ -15,7 +15,8 @@ trap cleanup EXIT # Prepare expect_succ pulp ansible remote -t "collection" create --name "cli_test_ansible_collection_remote" \ ---url "$ANSIBLE_COLLECTION_REMOTE_URL" --requirements "collections:\n - robertdebock.ansible_development_environment" +--url "$ANSIBLE_COLLECTION_REMOTE_URL" --requirements "collections: + - robertdebock.ansible_development_environment" expect_succ pulp ansible repository create --name "cli_test_ansible_repository" HREF="$(echo "$OUTPUT" | jq -r "pulp_href")" expect_succ pulp ansible repository sync --repository "cli_test_ansible_repository" --remote "cli_test_ansible_collection_remote" diff --git a/tests/scripts/pulp_ansible/test_sync.sh b/tests/scripts/pulp_ansible/test_sync.sh index 03d1470e1..e9714a400 100755 --- a/tests/scripts/pulp_ansible/test_sync.sh +++ b/tests/scripts/pulp_ansible/test_sync.sh @@ -18,7 +18,8 @@ cleanup # Prepare expect_succ pulp ansible remote -t "role" create --name "cli_test_ansible_remote" --url "$ANSIBLE_ROLE_REMOTE_URL" expect_succ pulp ansible remote -t "collection" create --name "cli_test_ansible_collection_remote" \ ---url "$ANSIBLE_COLLECTION_REMOTE_URL" --requirements "collections:\n - robertdebock.ansible_development_environment" +--url "$ANSIBLE_COLLECTION_REMOTE_URL" --requirements "collections: + - robertdebock.ansible_development_environment" expect_succ pulp ansible repository create --name "cli_test_ansible_repository" # Test without remote (should fail)