diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 9f0393b..b7717a6 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -12,6 +12,9 @@ Raise `ValueError` when `start_time` passed to `create()` is neither a `datetime +- `dispatch-cli` accepts `FREQUENZ_API_KEY` and `FREQUENZ_API_SECRET` as a + fallback pair for `DISPATCH_API_AUTH_KEY` and `DISPATCH_API_SIGN_SECRET`. + ## Bug Fixes - `DispatchApiClient.create()`: Passing an invalid `start_time` (not a `datetime` or `"NOW"`) previously silently created a dispatch with an epoch timestamp (1970-01-01). It now raises `ValueError` immediately. diff --git a/src/frequenz/client/dispatch/__main__.py b/src/frequenz/client/dispatch/__main__.py index 9b18b60..c313e78 100644 --- a/src/frequenz/client/dispatch/__main__.py +++ b/src/frequenz/client/dispatch/__main__.py @@ -6,6 +6,7 @@ import asyncio import os import shlex +from collections.abc import Mapping from datetime import datetime, timezone from pprint import pformat from typing import Any, List @@ -163,6 +164,47 @@ def format_line(key: str, value: str, color: str = "cyan") -> str: ) +def _resolve_credentials( + api_key: str | None, + auth_key: str | None, + sign_secret: str | None, + env: Mapping[str, str], +) -> tuple[str | None, str | None, bool]: + """Resolve credentials without mixing key and secret sources. + + Explicit options override the service-specific environment variables, which + override the generic Frequenz API variables. Each source is selected as a + unit so a key is never combined with a secret from another source. + + Args: + api_key: Key passed through the deprecated `--api-key` option. + auth_key: Key passed through the canonical `--auth-key` option. + sign_secret: Secret passed through `--sign-secret`. + env: Environment variables to resolve against. + + Returns: + The auth key, signing secret, and whether the deprecated key name was used. + """ + if api_key is not None or auth_key is not None or sign_secret is not None: + return auth_key or api_key, sign_secret, api_key is not None and not auth_key + + specific_vars = ( + "DISPATCH_API_AUTH_KEY", + "DISPATCH_API_KEY", + "DISPATCH_API_SIGN_SECRET", + ) + if any(var in env for var in specific_vars): + specific_auth_key = env.get("DISPATCH_API_AUTH_KEY") + specific_api_key = env.get("DISPATCH_API_KEY") + return ( + specific_auth_key or specific_api_key, + env.get("DISPATCH_API_SIGN_SECRET"), + specific_api_key is not None and not specific_auth_key, + ) + + return env.get("FREQUENZ_API_KEY"), env.get("FREQUENZ_API_SECRET"), False + + # Click command groups @click.group(invoke_without_command=True) @click.option( @@ -173,23 +215,17 @@ def format_line(key: str, value: str, color: str = "cyan") -> str: ) @click.option( "--api-key", - help="API key for authentication (deprecated, use --auth-key or DISPATCH_API_AUTH_KEY)", - envvar="DISPATCH_API_KEY", - show_envvar=True, + help="API key for authentication (deprecated, use --auth-key)", required=False, ) @click.option( "--auth-key", help="API auth key for authentication", - envvar="DISPATCH_API_AUTH_KEY", - show_envvar=True, required=False, ) @click.option( "--sign-secret", help="API signing secret for authentication", - envvar="DISPATCH_API_SIGN_SECRET", - show_envvar=True, required=False, default=None, ) @@ -213,16 +249,18 @@ async def cli( # pylint: disable=too-many-arguments, too-many-positional-argume if ctx.obj is None: ctx.obj = {} - key = auth_key or api_key + auth_key, sign_secret, used_deprecated_key = _resolve_credentials( + api_key, auth_key, sign_secret, os.environ + ) - if not key: + if not auth_key: raise click.BadParameter( - "You must provide an API auth key using --auth-key or " - "the DISPATCH_API_AUTH_KEY environment variable." + "You must provide an API auth key using --auth-key, " + "DISPATCH_API_AUTH_KEY, or FREQUENZ_API_KEY." ) click.echo(f"Using API URL: {url}", err=True) - click.echo(f"Using API Auth Key: {key[:4]}{'*' * 8}", err=True) + click.echo(f"Using API Auth Key: {auth_key[:4]}{'*' * 8}", err=True) if sign_secret: if len(sign_secret) > 8: @@ -232,7 +270,7 @@ async def cli( # pylint: disable=too-many-arguments, too-many-positional-argume else: click.echo("Using API Signing Secret (not shown).", err=True) - if api_key and auth_key is None: + if used_deprecated_key: click.echo( click.style( "Deprecation Notice: The --api-key option and the DISPATCH_API_KEY environment " @@ -246,14 +284,14 @@ async def cli( # pylint: disable=too-many-arguments, too-many-positional-argume ctx.obj["client"] = DispatchApiClient( server_url=url, - auth_key=key, + auth_key=auth_key, sign_secret=sign_secret, connect=True, ) ctx.obj["params"] = { "url": url, - "auth_key": key, + "auth_key": auth_key, "sign_secret": sign_secret, } @@ -261,7 +299,7 @@ async def cli( # pylint: disable=too-many-arguments, too-many-positional-argume # Check if a subcommand was given if ctx.invoked_subcommand is None: - await interactive_mode(url, key, sign_secret) + await interactive_mode(url, auth_key, sign_secret) @cli.command("list") diff --git a/tests/test_cli.py b/tests/test_cli.py index 75fa93e..00c0fa3 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -16,7 +16,7 @@ from frequenz.client.common.microgrid.electrical_components import ( ElectricalComponentCategory, ) -from frequenz.client.dispatch.__main__ import cli +from frequenz.client.dispatch.__main__ import _resolve_credentials, cli from frequenz.client.dispatch._cli_types import FuzzyDateTime from frequenz.client.dispatch.recurrence import ( EndCriteria, @@ -39,6 +39,65 @@ ENVIRONMENT_VARIABLES = {"DISPATCH_API_KEY": "test_key"} +def test_resolve_credentials_uses_generic_pair() -> None: + """Generic credentials are used when no higher-precedence source is set.""" + env = { + "FREQUENZ_API_KEY": "generic-key", + "FREQUENZ_API_SECRET": "generic-secret", + } + + assert _resolve_credentials(None, None, None, env) == ( + "generic-key", + "generic-secret", + False, + ) + + +def test_resolve_credentials_service_pair_overrides_generic_pair() -> None: + """Dispatch-specific credentials override generic credentials as a pair.""" + env = { + "DISPATCH_API_AUTH_KEY": "dispatch-key", + "DISPATCH_API_SIGN_SECRET": "dispatch-secret", + "FREQUENZ_API_KEY": "generic-key", + "FREQUENZ_API_SECRET": "generic-secret", + } + + assert _resolve_credentials(None, None, None, env) == ( + "dispatch-key", + "dispatch-secret", + False, + ) + + +def test_resolve_credentials_partial_service_pair_does_not_mix() -> None: + """A partial Dispatch pair is not completed from generic credentials.""" + env = { + "DISPATCH_API_AUTH_KEY": "dispatch-key", + "FREQUENZ_API_KEY": "generic-key", + "FREQUENZ_API_SECRET": "generic-secret", + } + + assert _resolve_credentials(None, None, None, env) == ( + "dispatch-key", + None, + False, + ) + + +def test_resolve_credentials_explicit_pair_overrides_environment() -> None: + """Explicit options override environment credentials as a pair.""" + env = { + "DISPATCH_API_AUTH_KEY": "dispatch-key", + "DISPATCH_API_SIGN_SECRET": "dispatch-secret", + } + + assert _resolve_credentials("explicit-key", None, "explicit-secret", env) == ( + "explicit-key", + "explicit-secret", + True, + ) + + @pytest.fixture def runner() -> CliRunner: """Fixture for CLI Runner."""