cli: Remove default subcommand hack (#642)

* _cli: remove optional command hack Closes #636. Signed-off-by: William Woodruff <[email protected]> * _cli: improved `--help` output Signed-off-by: William Woodruff <[email protected]> * README: `sigstore --help` Signed-off-by: William Woodruff <[email protected]> * CHANGELOG: record changes Signed-off-by: William Woodruff <[email protected]> * test/integration: fixup subcommand handling Signed-off-by: William Woodruff <[email protected]> --------- Signed-off-by: William Woodruff <[email protected]>
sigstore · May 2, 2023 · d9b1c59 · d9b1c59
1 parent bce2bb4
commit d9b1c59
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 54 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,6 +21,10 @@ All versions prior to 0.9.0 are untracked.
   bundle, rather than falling back to deprecated individual targets
   ([#626](https://github.com/sigstore/sigstore-python/pull/626))
 
+* `sigstore verify` is not longer a backwards-compatible alias for
+  `sigstore verify identity`, as it was during the 1.0 release series
+  ([#642](https://github.com/sigstore/sigstore-python/pull/642))
+
 ### Fixed
 
 * Removed an unnecessary and backwards-incompatible parameter from the

diff --git a/README.md b/README.md
@@ -98,12 +98,16 @@ Top-level:
 ```
 usage: sigstore [-h] [-V] [-v] [--staging] [--rekor-url URL]
                 [--rekor-root-pubkey FILE]
-                {sign,verify,get-identity-token} ...
+                COMMAND ...
 
 a tool for signing and verifying Python package distributions
 
 positional arguments:
-  {sign,verify,get-identity-token}
+  COMMAND               the operation to perform
+    sign                sign one or more inputs
+    verify              verify one or more inputs
+    get-identity-token  retrieve and return a Sigstore-compatible OpenID
+                        Connect token
 
 optional arguments:
   -h, --help            show this help message and exit
@@ -258,10 +262,6 @@ Sigstore instance options:
 ```
 <!-- @end-sigstore-verify-identity-help@ -->
 
-For backwards compatibility, `sigstore verify [args ...]` is equivalent to
-`sigstore verify identity [args ...]`, but the latter form is **strongly**
-preferred.
-
 #### Signatures from GitHub Actions
 
 If your signatures are coming from GitHub Actions (e.g., a workflow

diff --git a/sigstore/_cli.py b/sigstore/_cli.py
@@ -85,42 +85,6 @@ def _boolify_env(envvar: str) -> bool:
         raise ValueError(f"can't coerce '{val}' to a boolean")
 
 
-def _set_default_verify_subparser(parser: argparse.ArgumentParser, name: str) -> None:
-    """
-    An argparse patch for configuring a default subparser for `sigstore verify`.
-
-    Adapted from <https://stackoverflow.com/a/26379693>
-    """
-    subparser_found = False
-    for arg in sys.argv[1:]:
-        if arg in ["-h", "--help"]:  # global help if no subparser
-            break
-    else:
-        for x in parser._subparsers._actions:  # type: ignore[union-attr]
-            if not isinstance(x, argparse._SubParsersAction):
-                continue
-            for sp_name in x._name_parser_map.keys():
-                if sp_name in sys.argv[1:]:
-                    subparser_found = True
-        if not subparser_found:
-            try:
-                # If `sigstore verify identity` wasn't passed explicitly, we need
-                # to insert the `identity` subcommand into the correct position
-                # within `sys.argv`. To do that, we get the index of the `verify`
-                # subcommand, and insert it directly after it.
-                verify_idx = sys.argv.index("verify")
-                sys.argv.insert(verify_idx + 1, name)
-                logger.warning(
-                    "`sigstore verify` without a subcommand will be treated as "
-                    "`sigstore verify identity`, but this behavior will be deprecated "
-                    "in a future release"
-                )
-            except ValueError:
-                # This happens when we invoke `sigstore sign`, since there's no
-                # `verify` subcommand to insert under. We do nothing in this case.
-                pass
-
-
 def _add_shared_instance_options(group: argparse._ArgumentGroup) -> None:
     """
     Common Sigstore instance options, shared between all `sigstore` subcommands.
@@ -288,11 +252,18 @@ def _parser() -> argparse.ArgumentParser:
         default=os.getenv("SIGSTORE_REKOR_ROOT_PUBKEY"),
     )
 
-    subcommands = parser.add_subparsers(required=True, dest="subcommand")
+    subcommands = parser.add_subparsers(
+        required=True,
+        dest="subcommand",
+        metavar="COMMAND",
+        help="the operation to perform",
+    )
 
     # `sigstore sign`
     sign = subcommands.add_parser(
-        "sign", formatter_class=argparse.ArgumentDefaultsHelpFormatter
+        "sign",
+        help="sign one or more inputs",
+        formatter_class=argparse.ArgumentDefaultsHelpFormatter,
     )
 
     oidc_options = sign.add_argument_group("OpenID Connect options")
@@ -388,9 +359,15 @@ def _parser() -> argparse.ArgumentParser:
     # `sigstore verify`
     verify = subcommands.add_parser(
         "verify",
+        help="verify one or more inputs",
         formatter_class=argparse.ArgumentDefaultsHelpFormatter,
     )
-    verify_subcommand = verify.add_subparsers(dest="verify_subcommand")
+    verify_subcommand = verify.add_subparsers(
+        required=True,
+        dest="verify_subcommand",
+        metavar="COMMAND",
+        help="the kind of verification to perform",
+    )
 
     # `sigstore verify identity`
     verify_identity = verify_subcommand.add_parser(
@@ -489,12 +466,12 @@ def _parser() -> argparse.ArgumentParser:
         ),
     )
 
-    # `sigstore verify` defaults to `sigstore verify identity`, for backwards
-    # compatibility.
-    _set_default_verify_subparser(verify, "identity")
-
     # `sigstore get-identity-token`
-    get_identity_token = subcommands.add_parser("get-identity-token")
+    get_identity_token = subcommands.add_parser(
+        "get-identity-token",
+        help="retrieve and return a Sigstore-compatible OpenID Connect token",
+        formatter_class=argparse.ArgumentDefaultsHelpFormatter,
+    )
     _add_shared_oidc_options(get_identity_token)
 
     return parser
@@ -547,8 +524,6 @@ def main() -> None:
                 _verify_identity(args)
             elif args.verify_subcommand == "github":
                 _verify_github(args)
-            else:
-                parser.error(f"Unknown verify subcommand: {args.verify_subcommand}")
         elif args.subcommand == "get-identity-token":
             token = _get_identity_token(args)
             if token:

diff --git a/test/integration/sigstore-python-conformance b/test/integration/sigstore-python-conformance
@@ -23,7 +23,14 @@ fixed_args = [
     ARG_REPLACEMENTS[arg] if arg in ARG_REPLACEMENTS else arg for arg in fixed_args
 ]
 
-# Prepend the `sigstore-python` executable name.
-fixed_args = ["sigstore"] + fixed_args
+# Fix-up the subcommand: the conformance suite uses `verify`, but
+# `sigstore` requires `verify identity` for identity based verifications.
+subcommand, *fixed_args = fixed_args
+if subcommand == "sign":
+    fixed_args = ["sigstore", "sign", *fixed_args]
+elif subcommand == "verify":
+    fixed_args = ["sigstore", "verify", "identity", *fixed_args]
+else:
+    raise ValueError(f"unsupported subcommand: {subcommand}")
 
 subprocess.run(fixed_args, text=True, check=True)