Fix duplicate options warnings

Also turn the warning into an error in the help pages test.

fixes #1315

Author: mdellweg

CHANGES/1315.bugfix

@@ -0,0 +1 @@
+Fixed warnings about parameters being used multiple times.

src/pulp_cli/generic.py

@@ -399,6 +399,7 @@ def __init__(
     ):
         self.allowed_with_contexts = allowed_with_contexts
         self.needs_plugins = needs_plugins
+        self.option_processors: list[t.Callable[[click.Context], None]] = []
         super().__init__(*args, **kwargs)
 
     def invoke(self, ctx: click.Context) -> t.Any:
@@ -408,6 +409,8 @@ def invoke(self, ctx: click.Context) -> t.Any:
                 assert pulp_ctx is not None
                 for plugin_requirement in self.needs_plugins:
                     pulp_ctx.needs_plugin(plugin_requirement)
+            for processor in self.option_processors:
+                processor(ctx)
             return super().invoke(ctx)
         except PulpException as e:
             raise click.ClickException(str(e))
@@ -471,7 +474,9 @@ def list_commands(self, ctx: click.Context) -> list[str]:
 
 
 def pulp_command(
-    name: str | None = None, **kwargs: t.Any
+    name: str | None = None,
+    cls: t.Type[PulpCommand] = PulpCommand,
+    **kwargs: t.Any,
 ) -> t.Callable[[_AnyCallable], PulpCommand]:
     """
     Pulp command factory.
@@ -480,17 +485,21 @@ def pulp_command(
     `allowed_with_contexts`. It allows rendering the docstring with the values of `ENTITY` and
     `ENTITIES` from the closest entity context.
     """
-    return click.command(name=name, cls=PulpCommand, **kwargs)
+    return click.command(name=name, cls=cls, **kwargs)
 
 
-def pulp_group(name: str | None = None, **kwargs: t.Any) -> t.Callable[[_AnyCallable], PulpGroup]:
+def pulp_group(
+    name: str | None = None,
+    cls: t.Type[PulpGroup] = PulpGroup,
+    **kwargs: t.Any,
+) -> t.Callable[[_AnyCallable], PulpGroup]:
     """
     Pulp command group factory.
 
     Creates a click compatible group command that selects subcommands based on
     `allowed_with_contexts` and creates `PulpCommand` subcommands by default.
     """
-    return click.group(name=name, cls=PulpGroup, **kwargs)
+    return click.group(name=name, cls=cls, **kwargs)
 
 
 class PulpOption(click.Option):
@@ -631,7 +640,9 @@ def _version_callback(ctx: click.Context, param: click.Parameter, value: int | N
     return value
 
 
-def load_file_wrapper(handler: t.Callable[[click.Context, click.Parameter, str], t.Any]) -> t.Any:
+def load_file_wrapper(
+    handler: t.Callable[[click.Context, click.Parameter, str], t.Any],
+) -> t.Any:
     """
     A wrapper that is used for chaining or decorating callbacks that manipulate input data.
 
@@ -799,7 +810,11 @@ def remote_header_callback(
 # Decorator common options
 
 
-def pulp_option(*args: t.Any, **kwargs: t.Any) -> t.Callable[[FC], FC]:
+def pulp_option(
+    *args: t.Any,
+    cls: t.Type[PulpOption] = PulpOption,
+    **kwargs: t.Any,
+) -> t.Callable[[FC], FC]:
     """
     Factory of [`PulpOption`][pulp_cli.generic.PulpOption] objects.
 
@@ -820,8 +835,72 @@ def pulp_option(*args: t.Any, **kwargs: t.Any) -> t.Callable[[FC], FC]:
         )
         ```
     """
-    kwargs["cls"] = PulpOption
-    return click.option(*args, **kwargs)
+    return click.option(*args, cls=cls, **kwargs)
+
+
+def option_processor(
+    *, callback: t.Callable[[click.Context], None]
+) -> t.Callable[[PulpCommand], PulpCommand]:
+    """
+    Add a callback i.e. for option processing at a rather generic place before calling the command.
+    """
+
+    def _inner(cmd: PulpCommand) -> PulpCommand:
+        cmd.option_processors.append(callback)
+        return cmd
+
+    return _inner
+
+
+def option_group(
+    name: str,
+    options: t.List[str],
+    /,
+    callback: t.Callable[[click.Context, t.Dict[str, t.Any]], t.Any] | None = None,
+    require_all: bool = True,
+    expose_value: bool = True,
+) -> t.Callable[[PulpCommand], PulpCommand]:
+    def _group_callback(ctx: click.Context) -> None:
+        """
+        Group a list of options into a group represented as a dictionary.
+        This allows to add a `callback` function for further processing.
+        `expose_value` allows to hide the value from the command callback.
+        """
+        value = {k: v for k, v in ((k, ctx.params.pop(k, None)) for k in options) if v is not None}
+        if value:
+            if require_all and (missing_options := set(options) - set(value.keys())):
+                raise click.UsageError(
+                    _(
+                        "Illegal usage, please specify all options in the group: {option_list}\n"
+                        "missing: {missing_options}"
+                    ).format(
+                        option_list=", ".join(options),
+                        missing_options=", ".join(missing_options),
+                    )
+                )
+
+            if callback is not None:
+                value = callback(ctx, value)
+            if expose_value:
+                ctx.params[name] = value
+
+    def _inner(cmd: PulpCommand) -> PulpCommand:
+        for param in cmd.params:
+            if param.name in options:
+                other_options = [o for o in options if o != param.name]
+                if other_options:
+                    help_text = (
+                        (getattr(param, "help") or "")
+                        + "\n"
+                        + _("Option is grouped with {options}.").format(
+                            options=", ".join(other_options)
+                        )
+                    ).strip()
+                    setattr(param, "help", help_text)
+        cmd.option_processors.append(_group_callback)
+        return cmd
+
+    return _inner
 
 
 def resource_lookup_option(*args: t.Any, **kwargs: t.Any) -> t.Callable[[FC], FC]:
@@ -1013,7 +1092,11 @@ def _option_callback(
                         _(
                             "The type '{plugin}:{resource_type}' "
                             "does not support the '{capability}' capability."
-                        ).format(plugin=plugin, resource_type=resource_type, capability=capability)
+                        ).format(
+                            plugin=plugin,
+                            resource_type=resource_type,
+                            capability=capability,
+                        )
                     )
 
         if parent_resource_lookup:
@@ -1044,7 +1127,7 @@ def _multi_option_callback(
             "{plugin_default}{type_default}{multiple_note}"
         ).format(
             plugin_form=_("[<plugin>:]") if default_plugin else _("<plugin>:"),
-            type_form=_("[<resource_type>:]") if default_type else _("<resource_type>:"),
+            type_form=(_("[<resource_type>:]") if default_type else _("<resource_type>:")),
             lookup_key=lookup_key,
             plugin_default=(
                 _("'<plugin>' defaults to {plugin}. ").format(plugin=default_plugin)
@@ -1376,7 +1459,9 @@ def _type_callback(ctx: click.Context, param: click.Parameter, value: str | None
         help=_("total number of simultaneous connections"),
     ),
     click.option(
-        "--rate-limit", type=int_or_empty, help=_("limit download rate in requests per second")
+        "--rate-limit",
+        type=int_or_empty,
+        help=_("limit download rate in requests per second"),
     ),
     click.option("--sock-connect-timeout", type=float_or_empty),
     click.option("--sock-read-timeout", type=float_or_empty),

src/pulpcore/cli/ansible/remote.py

@@ -67,8 +67,6 @@ def remote(ctx: click.Context, pulp_ctx: PulpCLIContext, /, remote_type: str) ->
 nested_lookup_options = [remote_lookup_option]
 remote_options = [
     click.option("--policy", help=_("policy to use when downloading")),
-]
-collection_options = [
     pulp_option(
         "--requirements-file",
         callback=yaml_callback,
@@ -96,9 +94,8 @@ def remote(ctx: click.Context, pulp_ctx: PulpCLIContext, /, remote_type: str) ->
         allowed_with_contexts=collection_context,
     ),
 ]
-ansible_remote_options = remote_options + collection_options
-create_options = common_remote_create_options + remote_options + ansible_remote_options
-update_options = common_remote_update_options + remote_options + ansible_remote_options
+create_options = common_remote_create_options + remote_options
+update_options = common_remote_update_options + remote_options
 
 remote.add_command(list_command(decorators=remote_filter_options))
 remote.add_command(show_command(decorators=lookup_options))

src/pulpcore/cli/container/content.py

@@ -3,20 +3,20 @@
 
 import click
 
-from pulp_glue.common.context import PluginRequirement, PulpEntityContext
+from pulp_glue.common.context import PluginRequirement, PulpContentContext
 from pulp_glue.container.context import (
     PulpContainerBlobContext,
     PulpContainerManifestContext,
     PulpContainerTagContext,
 )
 
 from pulp_cli.generic import (
-    GroupOption,
     PulpCLIContext,
     content_filter_options,
     href_option,
     label_command,
     list_command,
+    option_group,
     pass_pulp_context,
     pulp_group,
     pulp_option,
@@ -26,14 +26,13 @@
 _ = gettext.gettext
 
 
-def _content_callback(ctx: click.Context, param: click.Parameter, value: t.Any) -> None:
-    if value is not None:
-        entity_ctx = ctx.find_object(PulpEntityContext)
+def _content_callback(ctx: click.Context, value: t.Dict[str, t.Any]) -> None:
+    if value:
+        entity_ctx = ctx.find_object(PulpContentContext)
         assert entity_ctx is not None
-        if isinstance(entity_ctx, PulpContainerTagContext):
-            entity_ctx.entity = value
-        else:
-            entity_ctx.entity = {"digest": value}
+        if isinstance(entity_ctx, PulpContainerTagContext) and len(value) != 2:
+            raise click.UsageError(_("Both 'name' and 'digest' are needed to describe a tag."))
+        entity_ctx.entity = value
 
 
 @pulp_group()
@@ -64,7 +63,10 @@ def content(ctx: click.Context, pulp_ctx: PulpCLIContext, /, content_type: str)
     ),
     pulp_option("--name", "name", allowed_with_contexts=(PulpContainerTagContext,)),
     pulp_option(
-        "--name-in", "name__in", multiple=True, allowed_with_contexts=(PulpContainerTagContext,)
+        "--name-in",
+        "name__in",
+        multiple=True,
+        allowed_with_contexts=(PulpContainerTagContext,),
     ),
     pulp_option(
         "--digest",
@@ -98,33 +100,21 @@ def content(ctx: click.Context, pulp_ctx: PulpCLIContext, /, content_type: str)
 lookup_options = [
     pulp_option(
         "--digest",
-        expose_value=False,
         help=_("Digest associated with {entity}"),
-        callback=_content_callback,
-        allowed_with_contexts=(PulpContainerBlobContext, PulpContainerManifestContext),
     ),
-    click.option(
-        "--digest",
-        expose_value=False,
-        help=_("Digest associated with {entity}"),
-        callback=_content_callback,
-        allowed_with_contexts=(PulpContainerTagContext,),
-        group=[
-            "name",
-        ],
-        cls=GroupOption,
-    ),
-    click.option(
+    pulp_option(
         "--name",
-        expose_value=False,
         help=_("Name of {entity}"),
         allowed_with_contexts=(PulpContainerTagContext,),
-        group=[
-            "digest",
-        ],
-        cls=GroupOption,
     ),
     href_option,
+    option_group(
+        "content",
+        ["name", "digest"],
+        callback=_content_callback,
+        require_all=False,
+        expose_value=False,
+    ),
 ]
 
 content.add_command(list_command(decorators=list_options))

src/pulpcore/cli/core/content_guard.py

@@ -203,7 +203,6 @@ def x509(ctx: click.Context, pulp_ctx: PulpCLIContext, /) -> None:
 
 
 certguard_create_options = common_create_options + [
-    click.option("--description"),
     click.option(
         "--ca-certificate",
         help=_("a PEM encoded CA certificate or @file containing same"),
@@ -212,14 +211,17 @@ def x509(ctx: click.Context, pulp_ctx: PulpCLIContext, /) -> None:
     ),
 ]
 
-certguard_update_options = lookup_options + [
-    click.option("--description"),
-    click.option(
-        "--ca-certificate",
-        help=_("a PEM encoded CA certificate or @file containing same"),
-        callback=load_string_callback,
-    ),
-]
+certguard_update_options = (
+    lookup_options
+    + common_update_options
+    + [
+        click.option(
+            "--ca-certificate",
+            help=_("a PEM encoded CA certificate or @file containing same"),
+            callback=load_string_callback,
+        ),
+    ]
+)
 
 
 x509.add_command(list_command(decorators=filter_options))