Make the --requirements param to accept file and text

closes pulp#230

CHANGES/230.feature

@@ -0,0 +1 @@
+Updated the `--requirements` option for ansible remotes to handle both files and strings.

pulpcore/cli/ansible/remote.py

@@ -16,6 +16,7 @@
     href_option,
     label_command,
     list_command,
+    load_file_wrapper,
     name_option,
     pass_pulp_context,
     pulp_group,
@@ -31,17 +32,17 @@
 _ = translation.gettext
 
 
-def _requirements_callback(
+def yaml_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
 
 
+yaml_callback = load_file_wrapper(yaml_file_callback)
+
+
 @pulp_group()
 @click.option(
     "-t",
@@ -71,26 +72,27 @@ def remote(ctx: click.Context, pulp_ctx: PulpCLIContext, remote_type: str) -> No
 collection_options = [
     pulp_option(
         "--requirements-file",
-        callback=_requirements_callback,
+        callback=yaml_file_callback,
         type=click.File(),
-        help=_("Collections only: a Collection requirements yaml"),
+        help=_(
+            "(Deprecated) Please use '--requirements' instead\n\n"
+            "Collections only: a Collection requirements yaml"
+        ),
         allowed_with_contexts=collection_context,
     ),
     pulp_option(
         "--requirements",
-        callback=_requirements_callback,
+        callback=yaml_callback,
         help=_("Collections only: a string of a requirements yaml"),
         allowed_with_contexts=collection_context,
     ),
     pulp_option(
         "--auth-url",
-        callback=_requirements_callback,
         help=_("Collections only: URL to receive a session token"),
         allowed_with_contexts=collection_context,
     ),
     pulp_option(
         "--token",
-        callback=_requirements_callback,
         help=_("Collections only: token key use for authentication"),
         allowed_with_contexts=collection_context,
     ),

pulpcore/cli/ansible/repository.py

@@ -26,8 +26,8 @@
     label_command,
     label_select_option,
     list_command,
-    load_file_or_string_callback,
     load_json_callback,
+    load_string_callback,
     name_option,
     pass_pulp_context,
     pass_repository_context,
@@ -108,7 +108,7 @@ def repository(ctx: click.Context, pulp_ctx: PulpCLIContext, repo_type: str) ->
     click.option("--description"),
     pulp_option(
         "--gpgkey",
-        callback=load_file_or_string_callback,
+        callback=load_string_callback,
         needs_plugins=[
             PluginRequirement(
                 "ansible", min="0.15.0.dev", feature="gpgkeys on ansible repositories"

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,55 +331,55 @@ 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]
-) -> Any:
-    """Load string from input or from file if string starts with @."""
-    the_content: str
+def load_file_wrapper(handler: Callable[[click.Context, click.Parameter, str], Any]) -> Any:
+    """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_string_callback = load_file_wrapper(lambda c, p, x: x)
+
+
+def json_callback(ctx: click.Context, param: click.Parameter, value: Optional[str]) -> Any:
+    if value is None:
+        return None
 
-    # Now try to evaluate legal JSON
-    json_object: Any
-    json_string: str = load_file_or_string_callback(ctx, param, value)
     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(
+load_json_callback = load_file_wrapper(json_callback)
+
+
+def load_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
 
@@ -394,11 +394,12 @@ def labels_callback(
 def create_content_json_callback(
     context_class: Optional[Type[PulpContentContext]] = None, schema: s.Schema = None
 ) -> Any:
+    @load_file_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:
@@ -842,7 +843,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 = [
@@ -883,17 +884,17 @@ def _type_callback(ctx: click.Context, param: click.Parameter, value: Optional[s
     click.option(
         "--ca-cert",
         help=_("a PEM encoded CA certificate or @file containing same"),
-        callback=load_file_or_string_callback,
+        callback=load_string_callback,
     ),
     click.option(
         "--client-cert",
         help=_("a PEM encoded client certificate or @file containing same"),
-        callback=load_file_or_string_callback,
+        callback=load_string_callback,
     ),
     click.option(
         "--client-key",
         help=_("a PEM encode private key or @file containing same"),
-        callback=load_file_or_string_callback,
+        callback=load_string_callback,
     ),
     click.option("--connect-timeout", type=float),
     click.option(
@@ -933,17 +934,17 @@ def _type_callback(ctx: click.Context, param: click.Parameter, value: Optional[s
     click.option(
         "--ca-cert",
         help=_("a PEM encoded CA certificate or @file containing same"),
-        callback=load_file_or_string_callback,
+        callback=load_string_callback,
     ),
     click.option(
         "--client-cert",
         help=_("a PEM encoded client certificate or @file containing same"),
-        callback=load_file_or_string_callback,
+        callback=load_string_callback,
     ),
     click.option(
         "--client-key",
         help=_("a PEM encode private key or @file containing same"),
-        callback=load_file_or_string_callback,
+        callback=load_string_callback,
     ),
     click.option("--connect-timeout", type=float_or_empty),
     click.option(

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_wrapper,
     name_option,
     pass_pulp_context,
     pass_repository_context,
@@ -69,10 +70,12 @@ 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:
+@load_file_wrapper
+def _content_list_callback(ctx: click.Context, param: click.Parameter, value: Optional[str]) -> Any:
+    if value is None:
         return None
+
+    result = json_callback(ctx, param, value)
     try:
         return CONTENT_LIST_SCHEMA.validate(result)
     except s.SchemaError as e:

pulpcore/cli/rpm/remote.py

@@ -7,7 +7,7 @@
     href_option,
     label_command,
     list_command,
-    load_file_or_string_callback,
+    load_string_callback,
     name_option,
     pass_pulp_context,
     pulp_group,
@@ -57,17 +57,17 @@ def remote(ctx: click.Context, pulp_ctx: PulpCLIContext, remote_type: str) -> No
     click.option(
         "--ca-cert",
         help=_("a PEM encoded CA certificate or @file containing same"),
-        callback=load_file_or_string_callback,
+        callback=load_string_callback,
     ),
     click.option(
         "--client-cert",
         help=_("a PEM encoded client certificate or @file containing same"),
-        callback=load_file_or_string_callback,
+        callback=load_string_callback,
     ),
     click.option(
         "--client-key",
         help=_("a PEM encode private key or @file containing same"),
-        callback=load_file_or_string_callback,
+        callback=load_string_callback,
     ),
     click.option("--connect-timeout", type=float),
     click.option(

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"

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"

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)