Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: Remove default subcommand hack #642

Merged
merged 5 commits into from
May 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
67 changes: 21 additions & 46 deletions sigstore/_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
11 changes: 9 additions & 2 deletions test/integration/sigstore-python-conformance
Original file line number Diff line number Diff line change
Expand Up @@ -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)