diff --git a/.github/workflows/binaries.yml b/.github/workflows/binaries.yml index ce0281eb..029de3e7 100644 --- a/.github/workflows/binaries.yml +++ b/.github/workflows/binaries.yml @@ -387,7 +387,7 @@ jobs: id-token: write contents: read env: - CLOUDSMITH_ORG: ${{ vars.CLOUDSMITH_NAMESPACE }} + CLOUDSMITH_WORKSPACE: ${{ vars.CLOUDSMITH_NAMESPACE }} CLOUDSMITH_SERVICE_SLUG: ${{ vars.CLOUDSMITH_SERVICE_SLUG }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 diff --git a/.github/workflows/dev-release.yml b/.github/workflows/dev-release.yml index 3ec1a97a..4cda232e 100644 --- a/.github/workflows/dev-release.yml +++ b/.github/workflows/dev-release.yml @@ -33,7 +33,7 @@ jobs: env: CLOUDSMITH_NAMESPACE: ${{ vars.CLOUDSMITH_NAMESPACE }} CLOUDSMITH_DEV_REPO: ${{ vars.CLOUDSMITH_DEV_REPO }} - CLOUDSMITH_ORG: ${{ vars.CLOUDSMITH_NAMESPACE }} + CLOUDSMITH_WORKSPACE: ${{ vars.CLOUDSMITH_NAMESPACE }} CLOUDSMITH_SERVICE_SLUG: ${{ vars.CLOUDSMITH_SERVICE_SLUG }} BRANCH: ${{ github.ref_name }} steps: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 563fa741..731e2158 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -242,7 +242,7 @@ jobs: env: CLOUDSMITH_NAMESPACE: ${{ vars.CLOUDSMITH_NAMESPACE }} CLOUDSMITH_REPO: ${{ vars.CLOUDSMITH_REPO }} - CLOUDSMITH_ORG: ${{ vars.CLOUDSMITH_NAMESPACE }} + CLOUDSMITH_WORKSPACE: ${{ vars.CLOUDSMITH_NAMESPACE }} CLOUDSMITH_SERVICE_SLUG: ${{ vars.CLOUDSMITH_SERVICE_SLUG }} VERSION: ${{ needs.validate.outputs.version }} steps: @@ -443,7 +443,7 @@ jobs: CLOUDSMITH_NAMESPACE: ${{ vars.CLOUDSMITH_NAMESPACE }} CLOUDSMITH_REPO: ${{ vars.CLOUDSMITH_REPO }} CLOUDSMITH_SERVICE_SLUG: ${{ vars.CLOUDSMITH_SERVICE_SLUG }} - CLOUDSMITH_ORG: ${{ vars.CLOUDSMITH_NAMESPACE }} + CLOUDSMITH_WORKSPACE: ${{ vars.CLOUDSMITH_NAMESPACE }} DOCKERHUB_USER: ${{ secrets.DOCKERHUB_USER }} HAS_DOCKERHUB: ${{ secrets.DOCKERHUB_PAT != '' }} VERSION: ${{ needs.validate.outputs.version }} diff --git a/CHANGELOG.md b/CHANGELOG.md index 7104cb05..38ed7fb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,31 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Using `#!/usr/bin/env sh` as shebang path for unix credential helpers. - The minimum version of `cloudsmith-api` is now 2.0.42. -### Fixed +### Deprecated + +- The old names of renamed options, environment variables, and `config.ini` keys are deprecated, and version 2.0.0 will remove them. Until then, they work as before, but the CLI prints a warning to stderr, once per run, that names the replacement. For example: ``Warning: `--org` is deprecated and will be removed in version 2.0.0. Use `--workspace` instead.`` The old names no longer show in `--help`. + + | Setting | Deprecated name | Replacement | + | -- | -- | -- | + | Proxy | `--api-proxy` | `--proxy` | + | | `CLOUDSMITH_API_PROXY` | `CLOUDSMITH_PROXY` | + | | `api_proxy` config key | `proxy` | + | | `--allowed-api-proxy-suffixes`, `CLOUDSMITH_ALLOWED_API_PROXY_SUFFIXES` (hidden) | `--allowed-proxy-suffixes`, `CLOUDSMITH_ALLOWED_PROXY_SUFFIXES` | + | SSL verification | `--without-api-ssl-verify`, `-S` | `--without-ssl-verify` | + | | `CLOUDSMITH_WITHOUT_API_SSL_VERIFY` | `CLOUDSMITH_SSL_VERIFY=0` | + | | `api_ssl_verify` config key | `ssl_verify` | + | Workspace | `--org`, `--organization`, `--oidc-org`, `--owner` | `--workspace` | + | | `-o` | `-w` | + | | `CLOUDSMITH_ORG` | `CLOUDSMITH_WORKSPACE` | + | | `org`, `organization`, `oidc_org` config keys | `workspace` | + | Pagination | `--show-all` | `--page-all` | + | `cloudsmith auth` | `--token`, `--force` | `--request-api-key` | + | | `--json` | `--output-format json` | + + - The CLI warns only when an old name changes the result. It does not warn for a config key that the new key in the same section overrides, for an environment variable that the new variable overrides, for an empty value, or for `api_ssl_verify = true`, which is the default. + - The credential helpers that Docker, Cargo, pnpm, Terraform, and other tools run (`cloudsmith credential-helper docker`, `cargo`, `pnpm`, `terraform`, and `generic`) do not print the warning. This includes a `~/.terraformrc` block with `--org` that an earlier version of `cloudsmith credential-helper install terraform` wrote. To update the block, run the install again. + - `cloudsmith auth --token`, `--force`, and `--json`, and the `cloudsmith login` and `cloudsmith token` commands, now use the same warning format. The warning prefix is now `Warning:` instead of `DEPRECATION WARNING:`. `cloudsmith login` still stops working on November 20, 2026. + - When you give an old and a new option on the same command line, such as `--org a --workspace b`, the new option now wins. Previously, the last option won. - `cloudsmith credential-helper install pnpm` now ends `.npmrc` with a newline. Previously, a line that a shell appended to the file, such as `echo '//host/:_authToken=...' >> ~/.npmrc`, joined the last `tokenHelper` line. This made the `tokenHelper` path incorrect, and npm did not read the appended token. The install now also shows each `tokenHelper` line that it sets and the file that it writes to. To add the newline to an existing `.npmrc`, run the install again. If a line is already joined, split it into two lines before you run the install. If you do not, the install removes the appended token. - OIDC authentication now works when the CLI cannot create its config directory, for example in a CI container with a read-only home directory. Previously, the CLI did not exchange the OIDC token and ran the command without credentials. Now the CLI exchanges the token. If the CLI cannot write the token to the disk cache, it continues without the cache. A malformed entry in the disk cache no longer stops the token exchange. diff --git a/README.md b/README.md index 66ba338c..3aea42df 100644 --- a/README.md +++ b/README.md @@ -323,10 +323,11 @@ See the [default config](https://raw.githubusercontent.com/cloudsmith-io/cloudsm You can specify the following configuration options: - `api_host`: The API host to connect to. -- `proxy`: The proxy to connect through for all HTTP requests, including API, SAML, upload, download, MCP, update, credential helper, OIDC, and error report connections. Use `--proxy` or `CLOUDSMITH_PROXY` to set it for one command. The config key `api_proxy`, flag `--api-proxy`, and environment variable `CLOUDSMITH_API_PROXY` remain supported for compatibility. In config and environment variables, the new name takes precedence over the alias. Without `proxy`, the CLI uses the `HTTPS_PROXY`, `HTTP_PROXY`, and `NO_PROXY` environment variables. When `proxy` is set, it takes precedence over them. -- `ssl_verify`: Whether or not to verify SSL certificates for HTTPS requests, including API, SAML, uploads, downloads, and MCP (default: `true`). Use `CLOUDSMITH_SSL_VERIFY=1` to enable verification or `CLOUDSMITH_SSL_VERIFY=0` / `--without-ssl-verify` to disable it. Disabling verification is dangerous; prefer disabling only strict checks where possible. The config key `api_ssl_verify`, flag `--without-api-ssl-verify` / `-S`, and environment variable `CLOUDSMITH_WITHOUT_API_SSL_VERIFY` remain supported for compatibility. Command-line settings override environment variables, which override config; within the same source, the new spelling takes precedence. +- `proxy`: The proxy to connect through for all HTTP requests, including API, SAML, upload, download, MCP, update, credential helper, OIDC, and error report connections. Use `--proxy` or `CLOUDSMITH_PROXY` to set it for one command. The config key `api_proxy`, flag `--api-proxy`, and environment variable `CLOUDSMITH_API_PROXY` still work, but they are deprecated and will be removed in version 2.0.0; the CLI prints a warning when you use them. The new name takes precedence over the old one. Without `proxy`, the CLI uses the `HTTPS_PROXY`, `HTTP_PROXY`, and `NO_PROXY` environment variables. When `proxy` is set, it takes precedence over them. +- `ssl_verify`: Whether or not to verify SSL certificates for HTTPS requests, including API, SAML, uploads, downloads, and MCP (default: `true`). Use `CLOUDSMITH_SSL_VERIFY=1` to enable verification or `CLOUDSMITH_SSL_VERIFY=0` / `--without-ssl-verify` to disable it. Disabling verification is dangerous; prefer disabling only strict checks where possible. The config key `api_ssl_verify`, flag `--without-api-ssl-verify` / `-S`, and environment variable `CLOUDSMITH_WITHOUT_API_SSL_VERIFY` still work, but they are deprecated and will be removed in version 2.0.0; the CLI prints a warning when you use them. Command-line settings override environment variables, which override config; within the same source, the new spelling takes precedence. - `ssl_verify_strict`: Whether or not to apply strict X.509 certificate verification (`VERIFY_X509_STRICT`, enabled by default on Python 3.13+). Set to `false` (or use `--without-ssl-verify-strict` / `CLOUDSMITH_SSL_VERIFY_STRICT=0`) if a TLS-intercepting proxy issues certificates that fail these checks. `CLOUDSMITH_SSL_VERIFY_STRICT=1` keeps strict verification enabled (the default). Certificates are still verified. - `api_user_agent`: The user agent to use for requests. +- `workspace`: The Cloudsmith Workspace slug. Use `-w`/`--workspace` or `CLOUDSMITH_WORKSPACE` to set it for one command. The config keys `org`, `organization`, and `oidc_org`, the flags `--org`, `--organization`, `--oidc-org`, `--owner`, and `-o`, and the environment variable `CLOUDSMITH_ORG` still work, but they are deprecated and will be removed in version 2.0.0; the CLI prints a warning when you use them. ### Credentials (credentials.ini) @@ -345,7 +346,7 @@ You'll need to provide authentication to Cloudsmith for any CLI actions that res You can authenticate using your organization's SAML provider, if configured, with the `cloudsmith auth` command: ``` -cloudsmith auth --owner example +cloudsmith auth --workspace example Beginning authentication for the example org ... Your organization's SAML IDP URL is: https://example.com/some-saml-idp @@ -357,7 +358,7 @@ Authentication complete The CLI opens the IDP URL in your browser automatically. If it can't (for example in Cygwin or a headless shell), it tells you to open the URL above manually and carries on waiting for the callback. Use `--no-browser` to skip the automatic launch entirely: ``` -cloudsmith auth --owner example --no-browser +cloudsmith auth --workspace example --no-browser ``` *Note:* The authentication callback is served on `127.0.0.1`, on the first free port between `12400` and `12404`. The browser you open the URL in must run on the same machine as the CLI. diff --git a/cloudsmith_cli/cli/commands/auth.py b/cloudsmith_cli/cli/commands/auth.py index 81dfbeea..a482cb1e 100644 --- a/cloudsmith_cli/cli/commands/auth.py +++ b/cloudsmith_cli/cli/commands/auth.py @@ -10,7 +10,7 @@ get_access_token_expiry, renew_sso_session, ) -from .. import decorators, utils, validators +from .. import decorators, deprecation, utils, validators from ..exceptions import handle_api_exceptions from ..saml import get_idp_url from ..webserver import AuthenticationWebRequestHandler, AuthenticationWebServer @@ -248,28 +248,13 @@ def authenticate( use_stderr = request_api_key_flag or json or utils.should_use_stderr(opts) if token: - click.secho( - "DEPRECATION WARNING: The `--token` flag is deprecated and will be removed in a future release. " - "Please use `--request-api-key` instead.", - fg="yellow", - err=True, - ) + deprecation.warn_option("--token", "--request-api-key", ctx=ctx) if force: - click.secho( - "DEPRECATION WARNING: The `--force` flag is deprecated and will be removed in a future release. " - "Please use `--request-api-key` instead (force is implied).", - fg="yellow", - err=True, - ) + deprecation.warn_option("--force", "--request-api-key", ctx=ctx) if json and not utils.should_use_stderr(opts): - click.secho( - "DEPRECATION WARNING: The `--json` flag is deprecated and will be removed in a future release. " - "Please use `--output-format json` instead.", - fg="yellow", - err=True, - ) + deprecation.warn_option("--json", "--output-format json", ctx=ctx) if not force and not token and not request_api_key_flag: session_result = _renew_existing_sso_session( diff --git a/cloudsmith_cli/cli/commands/credential_helper/cargo.py b/cloudsmith_cli/cli/commands/credential_helper/cargo.py index 3169014e..f52b8a8c 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/cargo.py +++ b/cloudsmith_cli/cli/commands/credential_helper/cargo.py @@ -37,7 +37,7 @@ def cargo(opts, cargo_plugin, provider_args): # pylint: disable=unused-argument Provides credentials for all Cloudsmith Cargo registries: ``*.cloudsmith.io``, ``*.cloudsmith.com``, and any custom domains configured for the Workspace (requires a Workspace - ``--workspace``, CLOUDSMITH_WORKSPACE, or - ``workspace`` in ``config.ini``; legacy aliases are also accepted - and a + ``workspace`` in ``config.ini``; deprecated aliases still work - and a valid API key/token). A registry that is not a Cloudsmith one is answered with @@ -76,7 +76,7 @@ def cargo(opts, cargo_plugin, provider_args): # pylint: disable=unused-argument \b Environment variables: CLOUDSMITH_API_KEY: API key for authentication (optional) - CLOUDSMITH_WORKSPACE: Workspace slug (CLOUDSMITH_ORG is also accepted) + CLOUDSMITH_WORKSPACE: Workspace slug (the deprecated CLOUDSMITH_ORG still works) """ # `provider_args` collects the extra arguments Cargo appends from the # credential-provider config entry. This provider takes no configuration diff --git a/cloudsmith_cli/cli/commands/credential_helper/docker.py b/cloudsmith_cli/cli/commands/credential_helper/docker.py index b64bb478..ad8345af 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/docker.py +++ b/cloudsmith_cli/cli/commands/credential_helper/docker.py @@ -30,7 +30,7 @@ def docker(opts, operation): Provides credentials for all Cloudsmith Docker registries: ``*.cloudsmith.io``, ``*.cloudsmith.com``, and any custom domains configured for the Workspace (requires a Workspace - ``--workspace``, CLOUDSMITH_WORKSPACE, or - ``workspace`` in ``config.ini``; legacy aliases are also accepted - and a + ``workspace`` in ``config.ini``; deprecated aliases still work - and a valid API key/token). \b @@ -59,7 +59,7 @@ def docker(opts, operation): \b Environment variables: CLOUDSMITH_API_KEY: API key for authentication (optional) - CLOUDSMITH_WORKSPACE: Workspace slug (CLOUDSMITH_ORG is also accepted) + CLOUDSMITH_WORKSPACE: Workspace slug (the deprecated CLOUDSMITH_ORG still works) """ exit_code, stdout, stderr = execute( operation, diff --git a/cloudsmith_cli/cli/commands/credential_helper/pnpm.py b/cloudsmith_cli/cli/commands/credential_helper/pnpm.py index 7cccbe63..603f5e5c 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/pnpm.py +++ b/cloudsmith_cli/cli/commands/credential_helper/pnpm.py @@ -31,7 +31,7 @@ def pnpm(opts: Options, repo: str) -> NoReturn: Provides credentials for all Cloudsmith npm registries: ``*.cloudsmith.io``, ``*.cloudsmith.com``, and any custom domains configured for the Workspace (requires a Workspace - ``--workspace``, CLOUDSMITH_WORKSPACE, or - ``workspace`` in ``config.ini``; legacy aliases are also accepted - and a + ``workspace`` in ``config.ini``; deprecated aliases still work - and a valid API key/token). \b @@ -59,7 +59,7 @@ def pnpm(opts: Options, repo: str) -> NoReturn: \b Environment variables: CLOUDSMITH_API_KEY: API key for authentication (optional) - CLOUDSMITH_WORKSPACE: Workspace slug (CLOUDSMITH_ORG is also accepted) + CLOUDSMITH_WORKSPACE: Workspace slug (the deprecated CLOUDSMITH_ORG still works) """ exit_code, stdout, stderr = execute( diff --git a/cloudsmith_cli/cli/commands/login.py b/cloudsmith_cli/cli/commands/login.py index 245f47a4..d1654a4e 100644 --- a/cloudsmith_cli/cli/commands/login.py +++ b/cloudsmith_cli/cli/commands/login.py @@ -6,7 +6,7 @@ from ...core.api.exceptions import TwoFactorRequiredException from ...core.api.user import get_user_token from ...core.config import create_config_files, new_config_messaging -from .. import decorators, utils +from .. import decorators, deprecation, utils from ..exceptions import handle_api_exceptions from ..utils import maybe_spinner from .main import main @@ -26,9 +26,12 @@ def validate_login(ctx, param, value): "https://docs.cloudsmith.com/developer-tools/cli#getting-your-api-key" ) -DEPRECATION_WARNING = ( - "Warning: `cloudsmith {name}` is deprecated and will stop working on " - "November 20, 2026, when Cloudsmith disables username and password login. " +DEPRECATION_REMOVAL = ( + "will stop working on November 20, 2026, when Cloudsmith disables username " + "and password login" +) + +DEPRECATION_DETAIL = ( "See https://cloudsmith.com/changelog\n" "Use an API key or SAML instead: set `CLOUDSMITH_API_KEY` to an API key from " "the Cloudsmith web app (your user menu, then Personal API Keys), or run " @@ -64,11 +67,13 @@ class DeprecatedLoginCommand(click.Command): def parse_args(self, ctx, args): # Click prompts for --login and --password while parsing, so the # warning must be printed here rather than in the command body. - # Shell completion also parses (resiliently) and must stay quiet. - if not ctx.resilient_parsing: - click.secho( - DEPRECATION_WARNING.format(name=ctx.info_name), fg="yellow", err=True - ) + # Shell completion also parses (resiliently); the warning skips it. + deprecation.warn( + f"`cloudsmith {ctx.info_name}`", + removal=DEPRECATION_REMOVAL, + detail=DEPRECATION_DETAIL, + ctx=ctx, + ) return super().parse_args(ctx, args) diff --git a/cloudsmith_cli/cli/config.py b/cloudsmith_cli/cli/config.py index 982c6977..28e1e663 100644 --- a/cloudsmith_cli/cli/config.py +++ b/cloudsmith_cli/cli/config.py @@ -11,7 +11,7 @@ from click_configfile import ConfigFileReader, Param, SectionSchema, matches_section from ..core.utils import get_data_path, read_file -from . import utils, validators +from . import deprecation, utils, validators OPTIONS = threading.local() @@ -289,6 +289,14 @@ def _load_values_into_opts(opts, values): else: if v is None: continue + setting = deprecation.CONFIG_KEY_REPLACEMENTS.get(k, k) + if setting in deprecation.CONFIG_KEY_REPLACEMENTS.values(): + # Record which key set the value last, so a profile that sets + # the new key clears a deprecated key from [default]. The + # schema defaults api_ssl_verify to true, so only false shows + # that the key is set, and only false changes the result. + is_alias = k != setting and v is not True + opts.config_keys_in_effect[setting] = k if is_alias else None setattr(opts, k, v) @@ -412,9 +420,15 @@ def __init__(self, *args, **kwargs): """Initialise a new Options object.""" super().__init__(*args, **kwargs) self.opts = {} + self.config_keys_in_effect = {} for k, v in kwargs.items(): setattr(self, k, v) + @property + def deprecated_config_keys(self): + """The deprecated config.ini keys whose values are in effect.""" + return [key for key in self.config_keys_in_effect.values() if key] + @staticmethod def get_config_reader(): """Get the non-credentials config reader class.""" diff --git a/cloudsmith_cli/cli/decorators.py b/cloudsmith_cli/cli/decorators.py index e2022716..8f790458 100644 --- a/cloudsmith_cli/cli/decorators.py +++ b/cloudsmith_cli/cli/decorators.py @@ -19,7 +19,7 @@ disabled_detectors_from_env, registered_detectors, ) -from . import config, utils +from . import config, deprecation, utils def _expired_saml_renewal_message(error): @@ -191,6 +191,18 @@ def wrapper(ctx, *args, **kwargs): return wrapper +def _warn_deprecated_config_keys(ctx, opts): + """Warn about the deprecated config.ini keys that are in effect. + + Only the command that runs warns, not its parent groups, so a credential + helper subcommand can stay quiet. A subcommand that inherits the config + from its group, such as `mcp start`, warns from its session setup. + """ + if not ctx.invoked_subcommand: + for key in opts.deprecated_config_keys: + deprecation.warn_config_key(key, ctx=ctx) + + def common_cli_config_options(f): """Add common CLI config options to commands.""" @@ -231,6 +243,7 @@ def wrapper(ctx, *args, **kwargs): opts.load_config_file(path=config_file, profile=profile) opts.load_creds_file(path=creds_file, profile=profile) opts.api_key_from_file = opts.api_key + _warn_deprecated_config_keys(ctx, opts) kwargs["opts"] = opts return ctx.invoke(f, *args, **kwargs) @@ -336,17 +349,24 @@ def common_cli_list_options(f): ) @click.option( "--page-all", - "--show-all", default=False, is_flag=True, help="Show all results. Cannot be used with --page (-p) or --page-size (-l).", ) # Validation performed post-parse for order-independence. + @deprecation.alias_options(deprecation.PAGE_ALL_ALIASES, is_flag=True) @click.pass_context @functools.wraps(f) def wrapper(ctx, *args, **kwargs): # pylint: disable=missing-docstring opts = config.get_or_create_options(ctx) + show_all = deprecation.pop_alias_options( + ctx, kwargs, deprecation.PAGE_ALL_ALIASES + ) + page_all = kwargs["page_all"] = bool(kwargs.get("page_all") or show_all) + # `list repos` forwards ctx.params, which no longer holds --show-all. + ctx.params["page_all"] = page_all + # Handle wildcard page_size (converts to page_all behavior) page_size = kwargs.get("page_size") if page_size == -1: @@ -355,9 +375,8 @@ def wrapper(ctx, *args, **kwargs): validators.enforce_page_all_exclusive(ctx, wildcard_used=True) # Order-independent validation: perform after all options parsed. - page_all = kwargs.get("page_all") if page_all: - validators.enforce_page_all_exclusive(ctx) + validators.enforce_page_all_exclusive(ctx, page_all=page_all) kwargs["opts"] = opts return ctx.invoke(f, *args, **kwargs) @@ -404,6 +423,28 @@ def wrapper(ctx, *args, **kwargs): "CLOUDSMITH_ALLOWED_PROXY_SUFFIXES", "CLOUDSMITH_ALLOWED_API_PROXY_SUFFIXES", ) +SSL_VERIFY_LEGACY_ENVVAR = "CLOUDSMITH_WITHOUT_API_SSL_VERIFY" +WORKSPACE_ENVVARS = ("CLOUDSMITH_WORKSPACE", "CLOUDSMITH_ORG") + + +def _resolve_alias(ctx, kwargs, name, envvars, aliases): + """Pop option `name` and its deprecated spellings, warning for each one used. + + The current spelling wins on the command line, then a deprecated spelling, + then the environment (where the current variable name wins). The resolved + value is stored in `ctx.params`, so `ctx.forward` passes it on. + """ + value = kwargs.pop(name) + alias_value = deprecation.pop_alias_options(ctx, kwargs, aliases) + if ( + alias_value is not None + and ctx.get_parameter_source(name) != ParameterSource.COMMANDLINE + ): + value = alias_value + else: + deprecation.warn_deprecated_envvar(ctx, name, envvars) + ctx.params[name] = value + return value def _parse_suffixes(value): @@ -413,14 +454,17 @@ def _parse_suffixes(value): ) -def _guard_untrusted_endpoints(ctx, opts, host_suffixes, proxy_suffixes): +def _guard_untrusted_endpoints( + ctx, opts, host_suffixes, proxy_suffixes, proxy_trusted=False +): """Reject api_host/proxy redirections from directory-relative config. Enforcement applies only when the effective value originated from an untrusted (relative) config file and was not supplied via a CLI flag or environment variable. Both must match their allow-listed host suffixes; proxy has no public default suffixes, so it is rejected unless one is - supplied via the environment. + supplied via the environment. `proxy_trusted` marks a proxy given by the + deprecated `--api-proxy` flag, which is a separate parameter. """ trusted_sources = (ParameterSource.COMMANDLINE, ParameterSource.ENVIRONMENT) profile = ctx.meta.get("profile") @@ -432,7 +476,7 @@ def _guard_untrusted_endpoints(ctx, opts, host_suffixes, proxy_suffixes): if relative_host and opts.api_host == relative_host: validators.validate_untrusted_api_host(opts.api_host, host_suffixes) - if ctx.get_parameter_source("proxy") not in trusted_sources: + if not proxy_trusted and ctx.get_parameter_source("proxy") not in trusted_sources: _guard_untrusted_proxy(ctx, opts, proxy_suffixes) @@ -485,12 +529,11 @@ def initialise_session(f): ) @click.option( "--proxy", - "--api-proxy", "proxy", envvar=list(PROXY_ENVVARS), - help="The proxy to connect through for all HTTP requests. " - "--api-proxy and CLOUDSMITH_API_PROXY are compatibility aliases.", + help="The proxy to connect through for all HTTP requests.", ) + @deprecation.alias_options(deprecation.PROXY_ALIASES) @click.option( "--ssl-verify/--without-ssl-verify", default=None, @@ -499,14 +542,10 @@ def initialise_session(f): "Disabling verification is dangerous; prefer --without-ssl-verify-strict " "if a TLS-intercepting proxy issues certificates that fail strict checks.", ) - @click.option( - "-S", - "--without-api-ssl-verify", - default=None, + @deprecation.alias_options( + deprecation.SSL_VERIFY_ALIASES, + envvars={"--without-api-ssl-verify": SSL_VERIFY_LEGACY_ENVVAR}, is_flag=True, - envvar="CLOUDSMITH_WITHOUT_API_SSL_VERIFY", - help="Compatibility alias for --without-ssl-verify. Disabling SSL " - "certificate verification is dangerous.", ) @click.option( "--ssl-verify-strict/--without-ssl-verify-strict", @@ -536,12 +575,12 @@ def initialise_session(f): ) @click.option( "--allowed-proxy-suffixes", - "--allowed-api-proxy-suffixes", "allowed_proxy_suffixes", envvar=list(PROXY_SUFFIX_ENVVARS), hidden=True, help="Comma-separated trusted proxy host suffixes (internal use).", ) + @deprecation.alias_options(deprecation.PROXY_SUFFIXES_ALIASES) @click.pass_context @functools.wraps(f) def wrapper(ctx, *args, **kwargs): @@ -550,26 +589,51 @@ def wrapper(ctx, *args, **kwargs): from ..core.session import set_default_proxy opts = config.get_or_create_options(ctx) + _warn_deprecated_config_keys(ctx, opts) host_suffixes = _parse_suffixes(kwargs.pop("allowed_api_host_suffixes")) - proxy_suffixes = _parse_suffixes(kwargs.pop("allowed_proxy_suffixes")) + proxy_suffixes = _parse_suffixes( + _resolve_alias( + ctx, + kwargs, + "allowed_proxy_suffixes", + PROXY_SUFFIX_ENVVARS, + deprecation.PROXY_SUFFIXES_ALIASES, + ) + ) opts.api_host = kwargs.pop("api_host") - opts.proxy = kwargs.pop("proxy") + opts.proxy = _resolve_alias( + ctx, kwargs, "proxy", PROXY_ENVVARS, deprecation.PROXY_ALIASES + ) + proxy_from_alias = deprecation.used_on_command_line( + ctx, deprecation.PROXY_ALIASES + ) ssl_verify = kwargs.pop("ssl_verify") - legacy_ssl_verify = _pop_boolean_flag( - kwargs, "without_api_ssl_verify", invert=True + legacy_without_ssl_verify = deprecation.pop_alias_options( + ctx, kwargs, deprecation.SSL_VERIFY_ALIASES + ) + legacy_ssl_verify = ( + None if legacy_without_ssl_verify is None else not legacy_without_ssl_verify ) if ( ctx.get_parameter_source("ssl_verify") != ParameterSource.COMMANDLINE - and ctx.get_parameter_source("without_api_ssl_verify") - == ParameterSource.COMMANDLINE + and deprecation.used_on_command_line(ctx, deprecation.SSL_VERIFY_ALIASES) ) or ssl_verify is None: ssl_verify = legacy_ssl_verify + # Warn about the variable only when its value is the one used. + legacy_envvar = deprecation.alias_envvar( + ctx, deprecation.SSL_VERIFY_ALIASES + ) + if legacy_envvar: + deprecation.warn_envvar(legacy_envvar, "CLOUDSMITH_SSL_VERIFY=0") + ctx.params["ssl_verify"] = ssl_verify opts.ssl_verify = ssl_verify opts.ssl_verify_strict = kwargs.pop("ssl_verify_strict") opts.api_user_agent = kwargs.pop("api_user_agent") opts.api_headers = kwargs.pop("api_headers") - _guard_untrusted_endpoints(ctx, opts, host_suffixes, proxy_suffixes) + _guard_untrusted_endpoints( + ctx, opts, host_suffixes, proxy_suffixes, proxy_trusted=proxy_from_alias + ) set_default_proxy(opts.proxy) opts.session = _create_session( @@ -644,15 +708,11 @@ def resolve_credentials(f): @click.option( "-w", "--workspace", - "--org", - "--organization", - "--oidc-org", - "-o", - "--owner", "org", - envvar=["CLOUDSMITH_WORKSPACE", "CLOUDSMITH_ORG"], + envvar=list(WORKSPACE_ENVVARS), help="The Cloudsmith Workspace slug.", ) + @deprecation.alias_options(deprecation.WORKSPACE_ALIASES) @click.option( "--oidc-service-slug", envvar="CLOUDSMITH_SERVICE_SLUG", @@ -678,7 +738,9 @@ def wrapper(ctx, *args, **kwargs): opts = config.get_or_create_options(ctx) oidc_audience = kwargs.pop("oidc_audience") - org = kwargs.pop("org") + org = _resolve_alias( + ctx, kwargs, "org", WORKSPACE_ENVVARS, deprecation.WORKSPACE_ALIASES + ) oidc_service_slug = kwargs.pop("oidc_service_slug") oidc_discovery_disabled = _pop_boolean_flag(kwargs, "oidc_discovery_disabled") oidc_detector_order = kwargs.pop("oidc_detector_order") diff --git a/cloudsmith_cli/cli/deprecation.py b/cloudsmith_cli/cli/deprecation.py new file mode 100644 index 00000000..5ee43cba --- /dev/null +++ b/cloudsmith_cli/cli/deprecation.py @@ -0,0 +1,199 @@ +"""CLI - Deprecation warnings for old command, option and setting names.""" + +import os + +import click +from click.core import ParameterSource + +#: The release that removes the deprecated aliases. +REMOVAL_VERSION = "2.0.0" + +#: Credential helper commands that package managers run. Their users cannot act +#: on a warning in the middle of a `docker pull` or `terraform init`, so these +#: commands print none. +CREDENTIAL_HELPER_RUNTIMES = frozenset( + {"cargo", "docker", "generic", "pnpm", "terraform"} +) + +#: Deprecated config.ini keys and the keys that replace them. +CONFIG_KEY_REPLACEMENTS = { + "api_proxy": "proxy", + "api_ssl_verify": "ssl_verify", + "org": "workspace", + "organization": "workspace", + "oidc_org": "workspace", +} + +#: Deprecated option spellings and the spellings that replace them. +PROXY_ALIASES = {"--api-proxy": "--proxy"} +PROXY_SUFFIXES_ALIASES = {"--allowed-api-proxy-suffixes": "--allowed-proxy-suffixes"} +SSL_VERIFY_ALIASES = { + "--without-api-ssl-verify": "--without-ssl-verify", + "-S": "--without-ssl-verify", +} +WORKSPACE_ALIASES = { + "--org": "--workspace", + "--organization": "--workspace", + "--oidc-org": "--workspace", + "--owner": "--workspace", + "-o": "-w", +} +PAGE_ALL_ALIASES = {"--show-all": "--page-all"} + +_WARNED_META_KEY = "cloudsmith_cli.deprecation_warnings" + + +def format_warning(subject, replacement=None, *, removal=None, detail=None): + """Return the shared deprecation warning for `subject`.""" + removal = removal or f"will be removed in version {REMOVAL_VERSION}" + message = f"Warning: {subject} is deprecated and {removal}." + if replacement: + message += f" Use {replacement} instead." + if detail: + message += f"\n{detail}" + return message + + +def _is_credential_helper_runtime(ctx): + parent = ctx.parent + return ( + parent is not None + and parent.info_name == "credential-helper" + and ctx.info_name in CREDENTIAL_HELPER_RUNTIMES + ) + + +def warn(subject, replacement=None, *, removal=None, detail=None, ctx=None): + """Print the deprecation warning for `subject` to stderr, once per run.""" + message = format_warning(subject, replacement, removal=removal, detail=detail) + ctx = ctx or click.get_current_context(silent=True) + if ctx is not None: + if ctx.resilient_parsing or _is_credential_helper_runtime(ctx): + return + warned = ctx.find_root().meta.setdefault(_WARNED_META_KEY, set()) + if message in warned: + return + warned.add(message) + click.secho(message, fg="yellow", err=True) + + +def warn_option(spelling, replacement, ctx=None): + """Warn that the option `spelling` is deprecated.""" + warn(f"`{spelling}`", f"`{replacement}`", ctx=ctx) + + +def warn_config_key(key, ctx=None): + """Warn that the config.ini key `key` is deprecated.""" + warn( + f"The `{key}` key in config.ini", + f"`{CONFIG_KEY_REPLACEMENTS[key]}`", + ctx=ctx, + ) + + +def warn_envvar(name, replacement, ctx=None): + """Warn that the environment variable `name` is deprecated.""" + warn(f"`{name}`", f"`{replacement}`", ctx=ctx) + + +def warn_deprecated_envvar(ctx, param_name, envvars): + """Warn if a parameter took its value from a deprecated environment variable. + + `envvars` lists the variables in the order Click reads them, with the + current name first; Click takes the first one that is set and not empty. + """ + if ctx.get_parameter_source(param_name) != ParameterSource.ENVIRONMENT: + return + used = next((name for name in envvars if os.environ.get(name)), None) + if used and used != envvars[0]: + warn_envvar(used, envvars[0], ctx=ctx) + + +def alias_param_name(spelling): + """Return the parameter name of the hidden option for a deprecated spelling.""" + return "deprecated_" + spelling.lstrip("-").replace("-", "_") + + +def alias_options(aliases, envvars=None, **attrs): + """Add each deprecated spelling in `aliases` as its own hidden option. + + A separate option per spelling lets the CLI name the spelling that was used + in the warning, and keeps the old spellings out of the help text. `envvars` + maps a spelling to the deprecated environment variable that also sets it. + Use `pop_alias_options` in the command to read the values. + """ + envvars = envvars or {} + + def decorator(f): + for spelling in reversed(list(aliases)): + f = click.option( + spelling, + alias_param_name(spelling), + envvar=envvars.get(spelling), + hidden=True, + default=None, + **attrs, + )(f) + return f + + return decorator + + +def pop_alias_options(ctx, kwargs, aliases): + """Pop the hidden options of `aliases`, warning for each one on the command line. + + Return the value of the deprecated spelling given last on the command line, + else the value from a deprecated environment variable, else None. The + caller warns about the environment variable (see `alias_envvar`) once it + knows that the value is used. + + The options are also removed from `ctx.params`, so `ctx.forward` passes the + resolved value of the current option instead of the alias, which would + lose its parameter source on the way. + """ + names = {alias_param_name(spelling): spelling for spelling in aliases} + order = list(ctx.params) + value = None + from_command_line = [] + for name, spelling in names.items(): + given = kwargs.pop(name) + ctx.params.pop(name, None) + source = ctx.get_parameter_source(name) + if source == ParameterSource.COMMANDLINE: + warn_option(spelling, aliases[spelling], ctx=ctx) + from_command_line.append((name, given)) + elif source == ParameterSource.ENVIRONMENT: + value = given + if from_command_line: + # Click records parameters in command-line order, so the last one + # given wins, as it did when the spellings were one option. + value = max( + from_command_line, + key=lambda item: order.index(item[0]) if item[0] in order else -1, + )[1] + return value + + +def alias_envvar(ctx, aliases): + """Return the deprecated environment variable that set one of `aliases`. + + Return None if no alias came from the environment, or if a deprecated + spelling on the command line took precedence over it. + """ + if used_on_command_line(ctx, aliases): + return None + for spelling in aliases: + name = alias_param_name(spelling) + if ctx.get_parameter_source(name) == ParameterSource.ENVIRONMENT: + param = next(p for p in ctx.command.params if p.name == name) + return param.envvar + return None + + +def used_on_command_line(ctx, aliases): + """Return True if any of the deprecated spellings was given on the command line.""" + return any( + ctx.get_parameter_source(alias_param_name(spelling)) + == ParameterSource.COMMANDLINE + for spelling in aliases + ) diff --git a/cloudsmith_cli/cli/tests/commands/test_auth.py b/cloudsmith_cli/cli/tests/commands/test_auth.py index bd97b830..76bda075 100644 --- a/cloudsmith_cli/cli/tests/commands/test_auth.py +++ b/cloudsmith_cli/cli/tests/commands/test_auth.py @@ -11,6 +11,7 @@ from ....core.api.exceptions import ApiException from ....core.sso import SsoRenewalResult, SsoRenewalStatus +from ... import deprecation from ...commands.auth import _renew_existing_sso_session, authenticate from ...commands.main import main from .conftest import MockToken @@ -118,6 +119,30 @@ def test_workspace_environment_aliases( assert result.exit_code == 0, result.output assert mock_auth_server.call_args.kwargs["owner"] == "test-workspace" + @pytest.mark.parametrize( + "flag,replacement", + [ + ("--token", "--request-api-key"), + ("--force", "--request-api-key"), + ("--json", "--output-format json"), + ], + ) + def test_deprecated_flags_use_the_shared_warning( + self, + flag, + replacement, + runner, + mock_saml_session, + mock_get_idp_url, + mock_webbrowser, + mock_auth_server, + ): + result = runner.invoke(authenticate, ["-w", "acme", "--no-browser", flag]) + + assert ( + deprecation.format_warning(f"`{flag}`", f"`{replacement}`") in result.stderr + ) + def test_auth_command_invokes_webserver( self, runner, diff --git a/cloudsmith_cli/cli/tests/test_deprecation.py b/cloudsmith_cli/cli/tests/test_deprecation.py new file mode 100644 index 00000000..711ffc2d --- /dev/null +++ b/cloudsmith_cli/cli/tests/test_deprecation.py @@ -0,0 +1,428 @@ +# Copyright 2026 Cloudsmith Ltd +"""Tests for the deprecation warnings of old option, variable and config names.""" + +import json +from unittest.mock import patch + +import click +import click.testing +import pytest + +from .. import deprecation +from ..decorators import ( + common_cli_config_options, + common_cli_list_options, + common_cli_output_options, + resolve_credentials, +) + +ENVVARS = ( + "CLOUDSMITH_WORKSPACE", + "CLOUDSMITH_ORG", + "CLOUDSMITH_PROXY", + "CLOUDSMITH_API_PROXY", + "CLOUDSMITH_SSL_VERIFY", + "CLOUDSMITH_WITHOUT_API_SSL_VERIFY", + "CLOUDSMITH_ALLOWED_PROXY_SUFFIXES", + "CLOUDSMITH_ALLOWED_API_PROXY_SUFFIXES", + "CLOUDSMITH_CONFIG_FILE", + "CLOUDSMITH_PROFILE", +) + + +def _report_command(): + @click.command() + @common_cli_config_options + @common_cli_output_options + @common_cli_list_options + @resolve_credentials + def report(opts, page, page_size, page_all): # pylint: disable=unused-argument + click.echo( + json.dumps( + { + "org": opts.org, + "proxy": opts.proxy, + "ssl_verify": opts.ssl_verify, + "page_all": page_all, + } + ) + ) + + return report + + +@pytest.fixture(autouse=True) +def isolated(monkeypatch, telemetry_config_dir): + for name in ENVVARS: + monkeypatch.delenv(name, raising=False) + with patch( + "cloudsmith_cli.cli.decorators.CredentialProviderChain.resolve", + return_value=None, + ): + yield telemetry_config_dir + + +def invoke(args=(), env=None, command=None): + result = click.testing.CliRunner().invoke( + command or _report_command(), list(args), env=env, catch_exceptions=False + ) + assert result.exit_code == 0, result.output + return result + + +def values(result): + return json.loads(result.stdout) + + +def test_format_warning(): + assert deprecation.format_warning("`--org`", "`--workspace`") == ( + "Warning: `--org` is deprecated and will be removed in version " + f"{deprecation.REMOVAL_VERSION}. Use `--workspace` instead." + ) + + +@pytest.mark.parametrize( + "args,env", + [ + (["-w", "acme", "--proxy", "http://proxy:3128", "--without-ssl-verify"], {}), + (["--workspace", "acme", "--page-all"], {}), + ( + [], + { + "CLOUDSMITH_WORKSPACE": "acme", + "CLOUDSMITH_PROXY": "http://proxy:3128", + "CLOUDSMITH_SSL_VERIFY": "0", + "CLOUDSMITH_ALLOWED_PROXY_SUFFIXES": "example.com", + }, + ), + ], +) +def test_current_names_do_not_warn(args, env): + assert invoke(args, env=env).stderr == "" + + +@pytest.mark.parametrize( + "flag,replacement", + [ + ("--org", "--workspace"), + ("-o", "-w"), + ], +) +def test_workspace_flag_aliases_warn_and_still_work(flag, replacement): + result = invoke([flag, "acme"]) + + assert values(result)["org"] == "acme" + assert ( + result.stderr + == deprecation.format_warning(f"`{flag}`", f"`{replacement}`") + "\n" + ) + + +def test_proxy_flag_alias_warns_and_still_works(): + result = invoke(["--api-proxy", "http://proxy:3128"]) + + assert values(result)["proxy"] == "http://proxy:3128" + assert "Warning: `--api-proxy` is deprecated" in result.stderr + assert "Use `--proxy` instead." in result.stderr + + +def test_ssl_flag_alias_warns_and_still_works(): + result = invoke(["-S"]) + + assert values(result)["ssl_verify"] is False + assert "Warning: `-S` is deprecated" in result.stderr + assert "Use `--without-ssl-verify` instead." in result.stderr + + +def test_show_all_warns_and_still_works(): + result = invoke(["--show-all"]) + + assert values(result)["page_all"] is True + assert "Warning: `--show-all` is deprecated" in result.stderr + assert "Use `--page-all` instead." in result.stderr + + +def test_show_all_still_conflicts_with_page(): + result = click.testing.CliRunner().invoke( + _report_command(), ["--show-all", "--page", "2"] + ) + + assert result.exit_code == 2 + assert "Cannot be used with --page (-p) or --page-size (-l)." in result.output + + +@pytest.mark.parametrize("flag", ["--page-all", "--show-all"]) +def test_show_all_survives_list_repos_forwarding(flag): + """`list repos` forwards its parameters to `repos get`.""" + from ..commands.list_ import repos + + with patch( + "cloudsmith_cli.cli.commands.repos.paginate_results", return_value=([], None) + ) as paginate: + result = invoke( + ["acme", flag, "--api-host", "https://api.example.invalid"], command=repos + ) + + assert paginate.call_args.args[1] is True + assert result.stderr.count("is deprecated") == (flag == "--show-all") + + +def test_page_size_wildcard_is_not_a_conflict(): + result = invoke(["--page-size", "*"]) + + assert values(result)["page_all"] is True + assert result.stderr == "" + + +@pytest.mark.parametrize( + "args,expected", + [ + (["--workspace", "new", "--org", "old"], "new"), + (["--org", "old", "--workspace", "new"], "new"), + ], +) +def test_current_flag_wins_over_alias(args, expected): + result = invoke(args) + + assert values(result)["org"] == expected + assert "`--org` is deprecated" in result.stderr + + +@pytest.mark.parametrize( + "args,expected", + [ + (["--org", "first", "--owner", "last"], "last"), + (["--owner", "first", "--org", "last"], "last"), + ], +) +def test_last_deprecated_spelling_wins(args, expected): + """The spellings were one option, where the last value given won.""" + assert values(invoke(args))["org"] == expected + + +def test_current_flag_still_wins_after_forwarding(): + @click.command() + @common_cli_config_options + @resolve_credentials + @click.pass_context + def outer(ctx, opts): # pylint: disable=unused-argument + ctx.forward(inner) + + @click.command() + @common_cli_config_options + @resolve_credentials + def inner(opts): + click.echo(json.dumps({"org": opts.org})) + + result = invoke(["--workspace", "new", "--org", "old"], command=outer) + + assert values(result)["org"] == "new" + + +def test_alias_flag_wins_over_environment(): + result = invoke(["--org", "flag"], env={"CLOUDSMITH_WORKSPACE": "env"}) + + assert values(result)["org"] == "flag" + + +@pytest.mark.parametrize( + "env,key,expected,warning", + [ + ( + {"CLOUDSMITH_ORG": "acme"}, + "org", + "acme", + "`CLOUDSMITH_ORG` is deprecated and will be removed in version 2.0.0. " + "Use `CLOUDSMITH_WORKSPACE` instead.", + ), + ( + {"CLOUDSMITH_API_PROXY": "http://proxy:3128"}, + "proxy", + "http://proxy:3128", + "`CLOUDSMITH_API_PROXY` is deprecated and will be removed in version 2.0.0. " + "Use `CLOUDSMITH_PROXY` instead.", + ), + ( + {"CLOUDSMITH_WITHOUT_API_SSL_VERIFY": "1"}, + "ssl_verify", + False, + "`CLOUDSMITH_WITHOUT_API_SSL_VERIFY` is deprecated and will be removed in " + "version 2.0.0. Use `CLOUDSMITH_SSL_VERIFY=0` instead.", + ), + ], +) +def test_environment_aliases_warn_and_still_work(env, key, expected, warning): + result = invoke(env=env) + + assert values(result)[key] == expected + assert warning in result.stderr + + +@pytest.mark.parametrize( + "args,env", + [ + ([], {"CLOUDSMITH_SSL_VERIFY": "1"}), + (["--ssl-verify"], {}), + ], +) +def test_overridden_ssl_environment_alias_does_not_warn(args, env): + result = invoke(args, env={"CLOUDSMITH_WITHOUT_API_SSL_VERIFY": "1", **env}) + + assert values(result)["ssl_verify"] is True + assert result.stderr == "" + + +def test_proxy_suffixes_environment_alias_warns(): + result = invoke(env={"CLOUDSMITH_ALLOWED_API_PROXY_SUFFIXES": "example.com"}) + + assert "`CLOUDSMITH_ALLOWED_API_PROXY_SUFFIXES` is deprecated" in result.stderr + assert "Use `CLOUDSMITH_ALLOWED_PROXY_SUFFIXES` instead." in result.stderr + + +def test_unused_environment_alias_does_not_warn(): + result = invoke(env={"CLOUDSMITH_ORG": "old", "CLOUDSMITH_WORKSPACE": "new"}) + + assert values(result)["org"] == "new" + assert result.stderr == "" + + +@pytest.mark.parametrize( + "body,replacement", + [ + ("org = acme", "workspace"), + ("api_proxy = http://proxy:3128", "proxy"), + ("api_ssl_verify = false", "ssl_verify"), + ], +) +def test_config_key_aliases_warn(isolated, body, replacement): + (isolated / "config.ini").write_text(f"[default]\n{body}\n") + key = body.split(" ", 1)[0] + + result = invoke() + + assert ( + result.stderr + == deprecation.format_warning( + f"The `{key}` key in config.ini", f"`{replacement}`" + ) + + "\n" + ) + + +@pytest.mark.parametrize( + "body", + [ + "workspace = acme", + "api_ssl_verify = true", + "org =", + "workspace = new\norg = old", + "proxy = http://new:3128\napi_proxy = http://old:3128", + ], +) +def test_config_keys_that_change_nothing_do_not_warn(isolated, body): + (isolated / "config.ini").write_text(f"[default]\n{body}\n") + + assert invoke().stderr == "" + + +@pytest.mark.parametrize( + "body,key,expected", + [ + ("[default]\norg = old\n[profile:ci]\nworkspace = new\n", "org", "new"), + ("[default]\napi_ssl_verify = false\n[profile:ci]\n", "ssl_verify", True), + ], +) +def test_profile_override_clears_default_alias_warning(isolated, body, key, expected): + (isolated / "config.ini").write_text(body) + + result = invoke(["-P", "ci"]) + + assert values(result)[key] == expected + assert result.stderr == "" + + +def test_subcommand_inheriting_config_still_warns(isolated): + """Like `mcp start`, which takes its config options from the `mcp` group.""" + (isolated / "config.ini").write_text("[default]\norg = acme\n") + + @click.group() + @common_cli_config_options + def group(opts): # pylint: disable=unused-argument + pass + + @group.command() + @resolve_credentials + def start(opts): + click.echo(json.dumps({"org": opts.org})) + + result = invoke(["start"], command=group) + + assert values(result)["org"] == "acme" + assert "The `org` key in config.ini is deprecated" in result.stderr + + +def test_each_warning_prints_once_per_run(isolated): + (isolated / "config.ini").write_text("[default]\norg = acme\n") + + @click.group(invoke_without_command=True) + @common_cli_config_options + def group(opts): # pylint: disable=unused-argument + pass + + group.add_command(_report_command(), name="report") + + result = invoke(["report", "--org", "acme", "--org", "acme"], command=group) + + assert result.stderr.count("`--org` is deprecated") == 1 + assert result.stderr.count("The `org` key in config.ini is deprecated") == 1 + + +def test_credential_helper_runtimes_do_not_warn(isolated): + (isolated / "config.ini").write_text("[default]\norg = acme\n") + + @click.group(name="credential-helper") + def credential_helper(): + pass + + credential_helper.add_command(_report_command(), name="terraform") + command = click.Group(commands={"credential-helper": credential_helper}) + + result = invoke( + ["credential-helper", "terraform", "--org", "acme"], + env={"CLOUDSMITH_API_PROXY": "http://proxy:3128"}, + command=command, + ) + + assert values(result)["org"] == "acme" + assert result.stderr == "" + + +def test_credential_helper_management_commands_warn(): + @click.group(name="credential-helper") + def credential_helper(): + pass + + credential_helper.add_command(_report_command(), name="install") + command = click.Group(commands={"credential-helper": credential_helper}) + + result = invoke(["credential-helper", "install", "--org", "acme"], command=command) + + assert "`--org` is deprecated" in result.stderr + + +def test_help_hides_the_deprecated_spellings(): + result = invoke(["--help"]) + + for spelling in ( + "--org", + "--organization", + "--oidc-org", + "--owner", + "-o,", + "--api-proxy", + "--without-api-ssl-verify", + "-S,", + "--show-all", + ): + assert spelling not in result.stdout + assert "--workspace" in result.stdout diff --git a/cloudsmith_cli/cli/tests/test_org_option.py b/cloudsmith_cli/cli/tests/test_org_option.py index ce2c9187..a9227261 100644 --- a/cloudsmith_cli/cli/tests/test_org_option.py +++ b/cloudsmith_cli/cli/tests/test_org_option.py @@ -2,8 +2,8 @@ """Tests for the Workspace option and its accepted spellings. ``--workspace`` and ``-w`` use current Cloudsmith terminology. ``--org``, -``--organization``, ``--oidc-org``, ``--owner``, and ``-o`` remain available -for compatibility. All seven flags are one option. +``--organization``, ``--oidc-org``, ``--owner``, and ``-o`` are deprecated but +still set the same value (see test_deprecation.py for their warnings). """ from unittest.mock import patch diff --git a/cloudsmith_cli/cli/validators.py b/cloudsmith_cli/cli/validators.py index 81a360ea..47b53735 100644 --- a/cloudsmith_cli/cli/validators.py +++ b/cloudsmith_cli/cli/validators.py @@ -262,7 +262,7 @@ def validate_page_size(ctx, param, value): return value -def enforce_page_all_exclusive(ctx, wildcard_used=False): +def enforce_page_all_exclusive(ctx, wildcard_used=False, page_all=None): """Order-independent mutual exclusivity check for pagination options. Raises click.BadParameter bound to the --page-all option if it was used @@ -273,8 +273,11 @@ def enforce_page_all_exclusive(ctx, wildcard_used=False): ctx: Click context wildcard_used: If True, validates even if --page-all wasn't explicitly passed (used when --page-size '*' or -1 was used) + page_all: The resolved --page-all value, if it can differ from the + parsed parameter (the deprecated --show-all also sets it) """ - page_all = ctx.params.get("page_all") + if page_all is None: + page_all = ctx.params.get("page_all") if not page_all and not wildcard_used: return @@ -299,7 +302,7 @@ def enforce_page_all_exclusive(ctx, wildcard_used=False): page_all_param = next( (p for p in ctx.command.params if p.name == "page_all"), None ) - error_msg = "Cannot be used with --page (-p) or --page-size (-l). (--show-all is an alias for --page-all)" + error_msg = "Cannot be used with --page (-p) or --page-size (-l)." if wildcard_used: error_msg = "Wildcard '*' or -1 in --page-size cannot be used with --page (-p). Use --page-all instead." raise click.BadParameter( diff --git a/cloudsmith_cli/data/config.ini b/cloudsmith_cli/data/config.ini index efdf794b..c45434de 100644 --- a/cloudsmith_cli/data/config.ini +++ b/cloudsmith_cli/data/config.ini @@ -4,11 +4,11 @@ api_host= # The proxy to connect through for all HTTP requests (default: None). -# api_proxy is accepted as a compatibility alias. +# The api_proxy key is deprecated and will be removed in version 2.0.0. proxy= # Whether to verify SSL certificates for HTTPS connections (default: True). -# api_ssl_verify is accepted as a compatibility alias. +# The api_ssl_verify key is deprecated and will be removed in version 2.0.0. ssl_verify= # Whether to apply strict X.509 certificate verification (VERIFY_X509_STRICT),