diff --git a/config/constants/__init__.py b/config/constants/__init__.py index 156e01232a..dfc5d77138 100644 --- a/config/constants/__init__.py +++ b/config/constants/__init__.py @@ -8,6 +8,22 @@ USAGE_SECRET_ENV, WEBAPP_URL_ENV, ) +from config.constants.coralogix import ( + CORALOGIX_API_KEY_ENV, + CORALOGIX_APPLICATION_NAME_ENV, + CORALOGIX_BASE_URL_ENV, + CORALOGIX_SUBSYSTEM_NAME_ENV, +) +from config.constants.datadog import ( + DATADOG_API_KEY_ENV, + DATADOG_APP_KEY_ENV, + DATADOG_SITE_ENV, +) +from config.constants.honeycomb import ( + HONEYCOMB_API_KEY_ENV, + HONEYCOMB_BASE_URL_ENV, + HONEYCOMB_DATASET_ENV, +) from config.constants.investigation import MAX_INVESTIGATION_LOOPS from config.constants.llm import ( AZURE_OPENAI_API_KEY_ENV, @@ -44,9 +60,19 @@ "AZURE_OPENAI_API_KEY_ENV", "AZURE_OPENAI_API_VERSION_ENV", "AZURE_OPENAI_BASE_URL_ENV", + "CORALOGIX_API_KEY_ENV", + "CORALOGIX_APPLICATION_NAME_ENV", + "CORALOGIX_BASE_URL_ENV", + "CORALOGIX_SUBSYSTEM_NAME_ENV", "CREDITS_HTTP_TIMEOUT_SECONDS", + "DATADOG_API_KEY_ENV", + "DATADOG_APP_KEY_ENV", + "DATADOG_SITE_ENV", "DEFAULT_POSTHOG_TIMEOUT_SECONDS", "DEFAULT_POSTHOG_URL", + "HONEYCOMB_API_KEY_ENV", + "HONEYCOMB_BASE_URL_ENV", + "HONEYCOMB_DATASET_ENV", "INTEGRATIONS_STORE_PATH", "IS_WINDOWS", "MAX_INVESTIGATION_LOOPS", diff --git a/config/constants/coralogix.py b/config/constants/coralogix.py new file mode 100644 index 0000000000..bd8c6537ea --- /dev/null +++ b/config/constants/coralogix.py @@ -0,0 +1,16 @@ +"""Coralogix environment variable names.""" + +from __future__ import annotations + +CORALOGIX_API_KEY_ENV = "CORALOGIX_API_KEY" +# Mirrors the ``base_url`` credential; the names deliberately differ. +CORALOGIX_BASE_URL_ENV = "CORALOGIX_API_URL" +CORALOGIX_APPLICATION_NAME_ENV = "CORALOGIX_APPLICATION_NAME" +CORALOGIX_SUBSYSTEM_NAME_ENV = "CORALOGIX_SUBSYSTEM_NAME" + +__all__ = [ + "CORALOGIX_API_KEY_ENV", + "CORALOGIX_APPLICATION_NAME_ENV", + "CORALOGIX_BASE_URL_ENV", + "CORALOGIX_SUBSYSTEM_NAME_ENV", +] diff --git a/config/constants/datadog.py b/config/constants/datadog.py new file mode 100644 index 0000000000..e1470b1af5 --- /dev/null +++ b/config/constants/datadog.py @@ -0,0 +1,13 @@ +"""Datadog environment variable names.""" + +from __future__ import annotations + +DATADOG_API_KEY_ENV = "DD_API_KEY" +DATADOG_APP_KEY_ENV = "DD_APP_KEY" +DATADOG_SITE_ENV = "DD_SITE" + +__all__ = [ + "DATADOG_API_KEY_ENV", + "DATADOG_APP_KEY_ENV", + "DATADOG_SITE_ENV", +] diff --git a/config/constants/honeycomb.py b/config/constants/honeycomb.py new file mode 100644 index 0000000000..97685c1b3a --- /dev/null +++ b/config/constants/honeycomb.py @@ -0,0 +1,14 @@ +"""Honeycomb environment variable names.""" + +from __future__ import annotations + +HONEYCOMB_API_KEY_ENV = "HONEYCOMB_API_KEY" +HONEYCOMB_DATASET_ENV = "HONEYCOMB_DATASET" +# Mirrors the ``base_url`` credential; the names deliberately differ. +HONEYCOMB_BASE_URL_ENV = "HONEYCOMB_API_URL" + +__all__ = [ + "HONEYCOMB_API_KEY_ENV", + "HONEYCOMB_BASE_URL_ENV", + "HONEYCOMB_DATASET_ENV", +] diff --git a/integrations/_catalog_impl.py b/integrations/_catalog_impl.py index eeecf81755..ddb94928ed 100644 --- a/integrations/_catalog_impl.py +++ b/integrations/_catalog_impl.py @@ -9,6 +9,22 @@ from typing import Any from config.config import get_tracer_base_url +from config.constants.coralogix import ( + CORALOGIX_API_KEY_ENV, + CORALOGIX_APPLICATION_NAME_ENV, + CORALOGIX_BASE_URL_ENV, + CORALOGIX_SUBSYSTEM_NAME_ENV, +) +from config.constants.datadog import ( + DATADOG_API_KEY_ENV, + DATADOG_APP_KEY_ENV, + DATADOG_SITE_ENV, +) +from config.constants.honeycomb import ( + HONEYCOMB_API_KEY_ENV, + HONEYCOMB_BASE_URL_ENV, + HONEYCOMB_DATASET_ENV, +) from config.llm_credentials import resolve_env_credential from integrations.airflow.config import airflow_config_from_env from integrations.airflow.config import classify as _classify_airflow @@ -22,6 +38,7 @@ from integrations.betterstack import classify as _classify_betterstack from integrations.bitbucket import classify as _classify_bitbucket from integrations.config_models import ( + DEFAULT_DATADOG_SITE, AlertmanagerIntegrationConfig, ArgoCDIntegrationConfig, AWSIntegrationConfig, @@ -434,9 +451,11 @@ def load_env_integrations() -> list[dict[str, Any]]: datadog_app_key = "" datadog_site = "" else: - datadog_api_key = resolve_env_credential("DD_API_KEY") - datadog_app_key = resolve_env_credential("DD_APP_KEY") - datadog_site = os.getenv("DD_SITE", "datadoghq.com").strip() or "datadoghq.com" + datadog_api_key = resolve_env_credential(DATADOG_API_KEY_ENV) + datadog_app_key = resolve_env_credential(DATADOG_APP_KEY_ENV) + datadog_site = ( + os.getenv(DATADOG_SITE_ENV, DEFAULT_DATADOG_SITE).strip() or DEFAULT_DATADOG_SITE + ) if datadog_api_key and datadog_app_key: try: datadog_config = DatadogIntegrationConfig.model_validate( @@ -493,14 +512,14 @@ def load_env_integrations() -> list[dict[str, Any]]: integrations.append(honeycomb_multi) honeycomb_api_key = "" else: - honeycomb_api_key = resolve_env_credential("HONEYCOMB_API_KEY") + honeycomb_api_key = resolve_env_credential(HONEYCOMB_API_KEY_ENV) if honeycomb_api_key: try: honeycomb_config = HoneycombIntegrationConfig.model_validate( { "api_key": honeycomb_api_key, - "dataset": os.getenv("HONEYCOMB_DATASET", "").strip(), - "base_url": os.getenv("HONEYCOMB_API_URL", "").strip(), + "dataset": os.getenv(HONEYCOMB_DATASET_ENV, "").strip(), + "base_url": os.getenv(HONEYCOMB_BASE_URL_ENV, "").strip(), } ) except Exception as exc: @@ -518,15 +537,15 @@ def load_env_integrations() -> list[dict[str, Any]]: integrations.append(coralogix_multi) coralogix_api_key = "" else: - coralogix_api_key = resolve_env_credential("CORALOGIX_API_KEY") + coralogix_api_key = resolve_env_credential(CORALOGIX_API_KEY_ENV) if coralogix_api_key: try: coralogix_config = CoralogixIntegrationConfig.model_validate( { "api_key": coralogix_api_key, - "base_url": os.getenv("CORALOGIX_API_URL", "").strip(), - "application_name": os.getenv("CORALOGIX_APPLICATION_NAME", "").strip(), - "subsystem_name": os.getenv("CORALOGIX_SUBSYSTEM_NAME", "").strip(), + "base_url": os.getenv(CORALOGIX_BASE_URL_ENV, "").strip(), + "application_name": os.getenv(CORALOGIX_APPLICATION_NAME_ENV, "").strip(), + "subsystem_name": os.getenv(CORALOGIX_SUBSYSTEM_NAME_ENV, "").strip(), } ) except Exception as exc: diff --git a/integrations/cli.py b/integrations/cli.py index 3f6ec661d5..33a87d2de4 100644 --- a/integrations/cli.py +++ b/integrations/cli.py @@ -193,14 +193,9 @@ def _setup_grafana() -> None: def _setup_datadog() -> None: - api_key = _p("API key", secret=True) - app_key = _p("Application key", secret=True) - site = _p("Site", default="datadoghq.com") - if not api_key or not app_key: - _die("api_key and app_key are required.") - upsert_integration( - "datadog", {"credentials": {"api_key": api_key, "app_key": app_key, "site": site}} - ) + from integrations.datadog.setup import DATADOG_SETUP + + _run_spec_setup(DATADOG_SETUP) def _setup_groundcover() -> None: @@ -224,35 +219,15 @@ def _setup_groundcover() -> None: def _setup_honeycomb() -> None: - api_key = _p("Configuration API key", secret=True) - dataset = _p("Dataset slug or __all__", default="__all__") - base_url = _p("API URL", default="https://api.honeycomb.io") - if not api_key: - _die("api_key is required.") - upsert_integration( - "honeycomb", - {"credentials": {"api_key": api_key, "dataset": dataset, "base_url": base_url}}, - ) + from integrations.honeycomb.setup import HONEYCOMB_SETUP + + _run_spec_setup(HONEYCOMB_SETUP) def _setup_coralogix() -> None: - api_key = _p("DataPrime API key", secret=True) - base_url = _p("API URL", default="https://api.coralogix.com") - application_name = _p("Application name (optional)") - subsystem_name = _p("Subsystem name (optional)") - if not api_key or not base_url: - _die("api_key and base_url are required.") - upsert_integration( - "coralogix", - { - "credentials": { - "api_key": api_key, - "base_url": base_url, - "application_name": application_name, - "subsystem_name": subsystem_name, - } - }, - ) + from integrations.coralogix.setup import CORALOGIX_SETUP + + _run_spec_setup(CORALOGIX_SETUP) def _setup_aws() -> None: @@ -880,8 +855,10 @@ def _run_spec_setup(spec: IntegrationSetupSpec) -> None: values: dict[str, str | None] = {} for field in spec.fields: - value = _p(field.question, secret=field.secret) - if not value and field.required: + value = _p(field.question, default=field.default, secret=field.secret) + # A field with a default is never missing — apply_setup substitutes it — + # so only a defaultless required field can fail here. + if not value and field.required and not field.default: _die(f"{field.label} is required.") values[field.name] = value diff --git a/integrations/config_models.py b/integrations/config_models.py index 5f572ab576..bbca695c39 100644 --- a/integrations/config_models.py +++ b/integrations/config_models.py @@ -22,6 +22,7 @@ _LOCAL_GRAFANA_HOSTS = {"localhost", "127.0.0.1", "0.0.0.0"} DEFAULT_GROUNDCOVER_MCP_URL = "https://mcp.groundcover.com/api/mcp" DEFAULT_GROUNDCOVER_TIMEZONE = "UTC" +DEFAULT_DATADOG_SITE = "datadoghq.com" DEFAULT_HONEYCOMB_BASE_URL = "https://api.honeycomb.io" DEFAULT_HONEYCOMB_DATASET = "__all__" DEFAULT_CORALOGIX_BASE_URL = "https://api.coralogix.com" @@ -122,11 +123,11 @@ class DatadogIntegrationConfig(StrictConfigModel): api_key: str app_key: str - site: str = "datadoghq.com" + site: str = DEFAULT_DATADOG_SITE integration_id: str = "" _normalize_site = field_validator("site", mode="before")( - normalize_with_default("datadoghq.com") + normalize_with_default(DEFAULT_DATADOG_SITE) ) @property diff --git a/integrations/coralogix/setup.py b/integrations/coralogix/setup.py new file mode 100644 index 0000000000..38cbd2a54a --- /dev/null +++ b/integrations/coralogix/setup.py @@ -0,0 +1,66 @@ +"""What Coralogix needs before it is considered configured. + +Only the DataPrime key is required. ``base_url`` is regional and defaults to the +US host. The application and subsystem names are optional query filters — left +blank, queries span the whole account. +""" + +from __future__ import annotations + +from config.constants.coralogix import ( + CORALOGIX_API_KEY_ENV, + CORALOGIX_APPLICATION_NAME_ENV, + CORALOGIX_BASE_URL_ENV, + CORALOGIX_SUBSYSTEM_NAME_ENV, +) +from integrations.config_models import DEFAULT_CORALOGIX_BASE_URL +from integrations.coralogix.verifier import verify_coralogix +from integrations.setup_flow import IntegrationSetupSpec, SetupField + +API_KEY_FIELD = "api_key" +BASE_URL_FIELD = "base_url" +APPLICATION_NAME_FIELD = "application_name" +SUBSYSTEM_NAME_FIELD = "subsystem_name" + +CORALOGIX_SETUP = IntegrationSetupSpec( + service="coralogix", + fields=( + SetupField( + name=API_KEY_FIELD, + label="Coralogix API key", + prompt="DataPrime API key", + env_var=CORALOGIX_API_KEY_ENV, + secret=True, + ), + SetupField( + name=BASE_URL_FIELD, + label="Coralogix API URL", + prompt="API URL", + env_var=CORALOGIX_BASE_URL_ENV, + default=DEFAULT_CORALOGIX_BASE_URL, + ), + SetupField( + name=APPLICATION_NAME_FIELD, + label="Coralogix application name", + prompt="Application name (optional)", + env_var=CORALOGIX_APPLICATION_NAME_ENV, + required=False, + ), + SetupField( + name=SUBSYSTEM_NAME_FIELD, + label="Coralogix subsystem name", + prompt="Subsystem name (optional)", + env_var=CORALOGIX_SUBSYSTEM_NAME_ENV, + required=False, + ), + ), + verify=verify_coralogix, +) + +__all__ = [ + "API_KEY_FIELD", + "APPLICATION_NAME_FIELD", + "BASE_URL_FIELD", + "CORALOGIX_SETUP", + "SUBSYSTEM_NAME_FIELD", +] diff --git a/integrations/datadog/setup.py b/integrations/datadog/setup.py new file mode 100644 index 0000000000..e391b3eb72 --- /dev/null +++ b/integrations/datadog/setup.py @@ -0,0 +1,58 @@ +"""What Datadog needs before it is considered configured. + +Both keys are required and neither substitutes for the other: the API key +authenticates ingestion, the application key authorizes the read endpoints every +Datadog tool calls. A setup with only one verifies against nothing useful. + +``site`` decides which regional API host is contacted, so a US-default account +in the EU fails every query with a confusing 403 rather than a routing error. +It is prompted with a default rather than inferred. +""" + +from __future__ import annotations + +from config.constants.datadog import ( + DATADOG_API_KEY_ENV, + DATADOG_APP_KEY_ENV, + DATADOG_SITE_ENV, +) +from integrations.config_models import DEFAULT_DATADOG_SITE +from integrations.datadog.verifier import verify_datadog +from integrations.setup_flow import IntegrationSetupSpec, SetupField + +API_KEY_FIELD = "api_key" +APP_KEY_FIELD = "app_key" +SITE_FIELD = "site" + +DATADOG_SETUP = IntegrationSetupSpec( + service="datadog", + fields=( + SetupField( + name=API_KEY_FIELD, + label="Datadog API key", + env_var=DATADOG_API_KEY_ENV, + secret=True, + ), + SetupField( + name=APP_KEY_FIELD, + label="Datadog application key", + env_var=DATADOG_APP_KEY_ENV, + secret=True, + ), + SetupField( + name=SITE_FIELD, + label="Datadog site", + prompt="Site (e.g. datadoghq.com, datadoghq.eu)", + env_var=DATADOG_SITE_ENV, + default=DEFAULT_DATADOG_SITE, + ), + ), + verify=verify_datadog, +) + +__all__ = [ + "API_KEY_FIELD", + "APP_KEY_FIELD", + "DATADOG_SETUP", + "SITE_FIELD", +] diff --git a/integrations/honeycomb/setup.py b/integrations/honeycomb/setup.py new file mode 100644 index 0000000000..02d39fa494 --- /dev/null +++ b/integrations/honeycomb/setup.py @@ -0,0 +1,56 @@ +"""What Honeycomb needs before it is considered configured. + +Only the key is required. ``dataset`` defaults to ``__all__`` — the environment- +wide scope — because narrowing it is a preference, not a prerequisite, and +``base_url`` moves only for EU tenants. +""" + +from __future__ import annotations + +from config.constants.honeycomb import ( + HONEYCOMB_API_KEY_ENV, + HONEYCOMB_BASE_URL_ENV, + HONEYCOMB_DATASET_ENV, +) +from integrations.config_models import DEFAULT_HONEYCOMB_BASE_URL, DEFAULT_HONEYCOMB_DATASET +from integrations.honeycomb.verifier import verify_honeycomb +from integrations.setup_flow import IntegrationSetupSpec, SetupField + +API_KEY_FIELD = "api_key" +DATASET_FIELD = "dataset" +BASE_URL_FIELD = "base_url" + +HONEYCOMB_SETUP = IntegrationSetupSpec( + service="honeycomb", + fields=( + SetupField( + name=API_KEY_FIELD, + label="Honeycomb API key", + prompt="Configuration API key", + env_var=HONEYCOMB_API_KEY_ENV, + secret=True, + ), + SetupField( + name=DATASET_FIELD, + label="Honeycomb dataset", + prompt="Dataset slug or __all__", + env_var=HONEYCOMB_DATASET_ENV, + default=DEFAULT_HONEYCOMB_DATASET, + ), + SetupField( + name=BASE_URL_FIELD, + label="Honeycomb API URL", + prompt="API URL", + env_var=HONEYCOMB_BASE_URL_ENV, + default=DEFAULT_HONEYCOMB_BASE_URL, + ), + ), + verify=verify_honeycomb, +) + +__all__ = [ + "API_KEY_FIELD", + "BASE_URL_FIELD", + "DATASET_FIELD", + "HONEYCOMB_SETUP", +] diff --git a/integrations/setup_flow.py b/integrations/setup_flow.py index 066517959f..5360504171 100644 --- a/integrations/setup_flow.py +++ b/integrations/setup_flow.py @@ -75,6 +75,16 @@ class SetupField: else to ``.env``. Fields do not get to choose. """ + default: str = "" + """Value to use when the field is submitted blank. + + Applied in :func:`apply_setup`, not just offered as a prompt prefill, so a + surface that never prompts — the wizard reusing a stored value, an agent + filling fields from a conversation — lands on the same credentials as + someone pressing enter at the CLI. A field with a default is therefore + never missing, whatever *required* says. + """ + required: bool = True """When true, a blank value fails setup instead of being stored as ``None``.""" @@ -136,7 +146,7 @@ def _collect_credentials( """ credentials: dict[str, str | None] = {} for field in spec.fields: - value = (values.get(field.name) or "").strip() + value = (values.get(field.name) or "").strip() or field.default if not value and field.required: return {}, f"{field.label} is required." credentials[field.name] = value or None diff --git a/surfaces/cli/wizard/configurators/chat_notifications.py b/surfaces/cli/wizard/configurators/chat_notifications.py index 4fcce9a95d..c03c2c4ddf 100644 --- a/surfaces/cli/wizard/configurators/chat_notifications.py +++ b/surfaces/cli/wizard/configurators/chat_notifications.py @@ -3,7 +3,6 @@ from __future__ import annotations from config.env_file import sync_env_secret, sync_env_values -from integrations.setup_flow import apply_setup from integrations.store import upsert_integration from integrations.telegram.setup import TELEGRAM_SETUP from platform.terminal.theme import ERROR, GLYPH_ERROR, SECONDARY, WARNING @@ -16,13 +15,13 @@ _render_integration_result, _string_value, ) +from surfaces.cli.wizard.configurators.spec_configurator import configure_from_spec from surfaces.cli.wizard.integration_health import ( validate_discord_bot, validate_rocketchat, validate_rocketchat_webhook, validate_slack_webhook, ) -from surfaces.cli.wizard.integration_validators.shared import IntegrationHealthResult def _configure_slack() -> tuple[str, str]: @@ -228,35 +227,16 @@ def _configure_rocketchat() -> tuple[str, str]: def _configure_telegram() -> tuple[str, str]: - _, credentials = _integration_defaults("telegram") - _console.print( - "\n[bold]Telegram Integration[/bold]\n" - f"[{SECONDARY}]Create a bot with @BotFather, then add it to the chat it should post " - "in. For a public channel the @name is enough; otherwise find the numeric chat id " - "via getUpdates. See docs/messaging/telegram for details.\n" - "Both answers are required — Telegram cannot deliver without a chat. Press Ctrl+C to " - "skip Telegram and continue onboarding; `opensre integrations setup telegram` picks it " - "up later.[/]\n" + return configure_from_spec( + TELEGRAM_SETUP, + title="Telegram", + intro=( + "\n[bold]Telegram Integration[/bold]\n" + f"[{SECONDARY}]Create a bot with @BotFather, then add it to the chat it should post " + "in. For a public channel the @name is enough; otherwise find the numeric chat id " + "via getUpdates. See docs/messaging/telegram for details.\n" + "Both answers are required — Telegram cannot deliver without a chat. Press Ctrl+C to " + "skip Telegram and continue onboarding; `opensre integrations setup telegram` picks " + "it up later.[/]\n" + ), ) - while True: - values = { - field.name: _prompt_value( - field.question, - default=_string_value(credentials.get(field.name)), - secret=field.secret, - allow_empty=not field.required, - ) - for field in TELEGRAM_SETUP.fields - } - with _console.status("Validating Telegram credentials...", spinner="dots"): - outcome = apply_setup(TELEGRAM_SETUP, values) - _render_integration_result( - "Telegram", IntegrationHealthResult(ok=outcome.ok, detail=outcome.detail) - ) - if outcome.ok: - # apply_setup always resolves an .env path on success; narrow for mypy - # and fail loudly rather than returning the string "None" if it ever - # stops doing so. - assert outcome.env_path is not None, "apply_setup returned ok=True without an env_path" - return "Telegram", str(outcome.env_path) - _console.print(f"[{SECONDARY}]Try again or press Ctrl+C to cancel.[/]") diff --git a/surfaces/cli/wizard/configurators/observability.py b/surfaces/cli/wizard/configurators/observability.py index 1f697b8870..911f6fd00e 100644 --- a/surfaces/cli/wizard/configurators/observability.py +++ b/surfaces/cli/wizard/configurators/observability.py @@ -3,6 +3,9 @@ from __future__ import annotations from config.env_file import sync_env_secret, sync_env_values +from integrations.coralogix.setup import CORALOGIX_SETUP +from integrations.datadog.setup import DATADOG_SETUP +from integrations.honeycomb.setup import HONEYCOMB_SETUP from integrations.store import remove_integration, upsert_integration from platform.terminal.theme import DIM, ERROR, GLYPH_ERROR, HIGHLIGHT, SECONDARY from surfaces.cli.wizard._ui import ( @@ -15,11 +18,9 @@ _render_integration_result, _string_value, ) +from surfaces.cli.wizard.configurators.spec_configurator import configure_from_spec from surfaces.cli.wizard.integration_health import ( - validate_coralogix_integration, - validate_datadog_integration, validate_grafana_integration, - validate_honeycomb_integration, validate_opensearch_integration, validate_splunk_integration, validate_tempo_integration, @@ -147,124 +148,15 @@ def _configure_grafana_local() -> tuple[str, str]: def _configure_datadog() -> tuple[str, str]: - _, credentials = _integration_defaults("datadog") - while True: - api_key = _prompt_value( - "Datadog API key", - default=_string_value(credentials.get("api_key")), - secret=True, - ) - app_key = _prompt_value( - "Datadog application key", - default=_string_value(credentials.get("app_key")), - secret=True, - ) - site = _prompt_value( - "Datadog site", - default=_string_value(credentials.get("site"), "datadoghq.com"), - ) - with _console.status("Validating Datadog integration...", spinner="dots"): - result = validate_datadog_integration(api_key=api_key, app_key=app_key, site=site) - _render_integration_result("Datadog", result) - if result.ok: - upsert_integration( - "datadog", - {"credentials": {"api_key": api_key, "app_key": app_key, "site": site}}, - ) - env_path = sync_env_values({}) - return "Datadog", str(env_path) - _console.print(f"[{SECONDARY}]Try again or press Ctrl+C to cancel.[/]") + return configure_from_spec(DATADOG_SETUP, title="Datadog") def _configure_honeycomb() -> tuple[str, str]: - _, credentials = _integration_defaults("honeycomb") - while True: - api_key = _prompt_value( - "Honeycomb configuration API key", - default=_string_value(credentials.get("api_key")), - secret=True, - ) - dataset = _prompt_value( - "Honeycomb dataset slug or __all__", - default=_string_value(credentials.get("dataset"), "__all__"), - ) - base_url = _prompt_value( - "Honeycomb API URL", - default=_string_value(credentials.get("base_url"), "https://api.honeycomb.io"), - ) - with _console.status("Validating Honeycomb integration...", spinner="dots"): - result = validate_honeycomb_integration( - api_key=api_key, - dataset=dataset, - base_url=base_url, - ) - _render_integration_result("Honeycomb", result) - if result.ok: - upsert_integration( - "honeycomb", - {"credentials": {"api_key": api_key, "dataset": dataset, "base_url": base_url}}, - ) - env_path = sync_env_values( - { - "HONEYCOMB_DATASET": dataset, - "HONEYCOMB_API_URL": base_url, - } - ) - return "Honeycomb", str(env_path) - _console.print(f"[{SECONDARY}]Try again or press Ctrl+C to cancel.[/]") + return configure_from_spec(HONEYCOMB_SETUP, title="Honeycomb") def _configure_coralogix() -> tuple[str, str]: - _, credentials = _integration_defaults("coralogix") - while True: - api_key = _prompt_value( - "Coralogix DataPrime API key", - default=_string_value(credentials.get("api_key")), - secret=True, - ) - base_url = _prompt_value( - "Coralogix API URL", - default=_string_value(credentials.get("base_url"), "https://api.coralogix.com"), - ) - application_name = _prompt_value( - "Coralogix application name (optional)", - default=_string_value(credentials.get("application_name")), - allow_empty=True, - ) - subsystem_name = _prompt_value( - "Coralogix subsystem name (optional)", - default=_string_value(credentials.get("subsystem_name")), - allow_empty=True, - ) - with _console.status("Validating Coralogix integration...", spinner="dots"): - result = validate_coralogix_integration( - api_key=api_key, - base_url=base_url, - application_name=application_name, - subsystem_name=subsystem_name, - ) - _render_integration_result("Coralogix", result) - if result.ok: - upsert_integration( - "coralogix", - { - "credentials": { - "api_key": api_key, - "base_url": base_url, - "application_name": application_name, - "subsystem_name": subsystem_name, - } - }, - ) - env_path = sync_env_values( - { - "CORALOGIX_API_URL": base_url, - "CORALOGIX_APPLICATION_NAME": application_name, - "CORALOGIX_SUBSYSTEM_NAME": subsystem_name, - } - ) - return "Coralogix", str(env_path) - _console.print(f"[{SECONDARY}]Try again or press Ctrl+C to cancel.[/]") + return configure_from_spec(CORALOGIX_SETUP, title="Coralogix") def _configure_tempo() -> tuple[str, str]: diff --git a/surfaces/cli/wizard/configurators/spec_configurator.py b/surfaces/cli/wizard/configurators/spec_configurator.py new file mode 100644 index 0000000000..8773973856 --- /dev/null +++ b/surfaces/cli/wizard/configurators/spec_configurator.py @@ -0,0 +1,67 @@ +"""The onboarding wizard's configurator for any spec-driven integration. + +This is the *collection* half of setup, and the counterpart to +:mod:`integrations.setup_flow`: that module takes values someone already +gathered and decides where they are persisted, while this one is what asks the +user for them. + +Every configurator built on an :class:`~integrations.setup_flow.IntegrationSetupSpec` +does the same three things — prompt for each field (prefilled from whatever is +already stored, so re-running onboarding is not a retype), hand the answers to +:func:`~integrations.setup_flow.apply_setup`, and re-ask on failure instead of +dropping the user out of the wizard. Only the heading and the introductory +guidance differ, so those are the arguments. +""" + +from __future__ import annotations + +from integrations.setup_flow import IntegrationSetupSpec, apply_setup +from platform.terminal.theme import SECONDARY +from surfaces.cli.wizard._ui import ( + _console, + _integration_defaults, + _prompt_value, + _render_integration_result, + _string_value, +) +from surfaces.cli.wizard.integration_validators.shared import IntegrationHealthResult + + +def configure_from_spec( + spec: IntegrationSetupSpec, *, title: str, intro: str = "" +) -> tuple[str, str]: + """Prompt for *spec*'s fields until they verify, then persist them. + + Returns the pair the wizard's configurator table expects: the display name + and the ``.env`` path that was written. + """ + _, credentials = _integration_defaults(spec.service) + if intro: + _console.print(intro) + while True: + values = { + field.name: _prompt_value( + field.question, + # A stored value wins over the spec's default, so re-running + # onboarding is a series of enters rather than a retype. + default=_string_value(credentials.get(field.name), field.default), + secret=field.secret, + # Only reached when the field has no default to fall back on: + # _prompt_value substitutes the default before it consults this, + # so a defaulted field never re-prompts and never returns blank. + allow_empty=not field.required, + ) + for field in spec.fields + } + with _console.status(f"Validating {title} credentials...", spinner="dots"): + outcome = apply_setup(spec, values) + _render_integration_result( + title, IntegrationHealthResult(ok=outcome.ok, detail=outcome.detail) + ) + if outcome.ok: + # apply_setup always resolves an .env path on success; narrow for mypy + # and fail loudly rather than returning the string "None" if it ever + # stops doing so. + assert outcome.env_path is not None, "apply_setup returned ok=True without an env_path" + return title, str(outcome.env_path) + _console.print(f"[{SECONDARY}]Try again or press Ctrl+C to cancel.[/]") diff --git a/surfaces/cli/wizard/integration_health.py b/surfaces/cli/wizard/integration_health.py index ce51000a11..2d67c3b8b1 100644 --- a/surfaces/cli/wizard/integration_health.py +++ b/surfaces/cli/wizard/integration_health.py @@ -29,10 +29,7 @@ validate_sentry_mcp_integration, ) from surfaces.cli.wizard.integration_validators.observability import ( - validate_coralogix_integration, - validate_datadog_integration, validate_grafana_integration, - validate_honeycomb_integration, validate_opensearch_integration, validate_splunk_integration, validate_tempo_integration, @@ -50,15 +47,12 @@ "validate_alertmanager_integration", "validate_aws_integration", "validate_betterstack_integration", - "validate_coralogix_integration", "validate_dagster_integration", - "validate_datadog_integration", "validate_discord_bot", "validate_github_mcp_integration", "validate_gitlab_integration", "validate_google_docs_integration", "validate_grafana_integration", - "validate_honeycomb_integration", "validate_incident_io_integration", "validate_jenkins_integration", "validate_jira_integration", diff --git a/surfaces/cli/wizard/integration_validators/observability.py b/surfaces/cli/wizard/integration_validators/observability.py index 72263e17ec..856feb9bcc 100644 --- a/surfaces/cli/wizard/integration_validators/observability.py +++ b/surfaces/cli/wizard/integration_validators/observability.py @@ -3,15 +3,10 @@ from __future__ import annotations from integrations.config_models import ( - CoralogixIntegrationConfig, GrafanaIntegrationConfig, - HoneycombIntegrationConfig, ) -from integrations.coralogix.client import CoralogixClient -from integrations.datadog.client import DatadogClient, DatadogConfig from integrations.elasticsearch.client import ElasticsearchClient, ElasticsearchConfig from integrations.grafana.client import get_grafana_client_from_credentials -from integrations.honeycomb.client import HoneycombClient from integrations.splunk.client import SplunkClient, SplunkConfig from integrations.tempo import build_tempo_config, validate_tempo_config @@ -58,111 +53,6 @@ def validate_grafana_integration( return IntegrationHealthResult(ok=False, detail=f"Grafana validation failed: {err}") -def validate_datadog_integration( - *, api_key: str, app_key: str, site: str -) -> IntegrationHealthResult: - """Validate Datadog credentials with a monitor list request.""" - client = DatadogClient(DatadogConfig(api_key=api_key, app_key=app_key, site=site)) - result = client.list_monitors() - if result.get("success"): - return IntegrationHealthResult( - ok=True, - detail=f"Datadog validated against {site}; fetched {result.get('total', 0)} monitors.", - ) - return IntegrationHealthResult( - ok=False, - detail=f"Datadog validation failed: {result.get('error', 'unknown error')}", - ) - - -def validate_honeycomb_integration( - *, - api_key: str, - dataset: str, - base_url: str, -) -> IntegrationHealthResult: - """Validate Honeycomb credentials with auth and a lightweight query.""" - try: - honeycomb_config = HoneycombIntegrationConfig.model_validate( - { - "api_key": api_key, - "dataset": dataset, - "base_url": base_url, - } - ) - except Exception as err: - return IntegrationHealthResult(ok=False, detail=str(err)) - - client = HoneycombClient(honeycomb_config) - auth_result = client.validate_access() - if not auth_result.get("success"): - return IntegrationHealthResult( - ok=False, - detail=f"Honeycomb auth failed: {auth_result.get('error', 'unknown error')}", - ) - - query_result = client.run_query( - {"calculations": [{"op": "COUNT"}], "time_range": 900}, - limit=1, - ) - if not query_result.get("success"): - return IntegrationHealthResult( - ok=False, - detail=f"Honeycomb query failed: {query_result.get('error', 'unknown error')}", - ) - - return IntegrationHealthResult( - ok=True, - detail=( - f"Honeycomb validated against dataset {honeycomb_config.dataset} " - f"at {honeycomb_config.base_url}." - ), - ) - - -def validate_coralogix_integration( - *, - api_key: str, - base_url: str, - application_name: str = "", - subsystem_name: str = "", -) -> IntegrationHealthResult: - """Validate Coralogix access with a lightweight DataPrime query.""" - try: - coralogix_config = CoralogixIntegrationConfig.model_validate( - { - "api_key": api_key, - "base_url": base_url, - "application_name": application_name, - "subsystem_name": subsystem_name, - } - ) - except Exception as err: - return IntegrationHealthResult(ok=False, detail=str(err)) - - client = CoralogixClient(coralogix_config) - result = client.validate_access() - if not result.get("success"): - return IntegrationHealthResult( - ok=False, - detail=f"Coralogix validation failed: {result.get('error', 'unknown error')}", - ) - - scope: list[str] = [] - if coralogix_config.application_name: - scope.append(f"application {coralogix_config.application_name}") - if coralogix_config.subsystem_name: - scope.append(f"subsystem {coralogix_config.subsystem_name}") - scope_suffix = f" ({', '.join(scope)})" if scope else "" - return IntegrationHealthResult( - ok=True, - detail=( - f"Coralogix validated against {coralogix_config.base_url}{scope_suffix}; " - f"DataPrime returned {result.get('total', 0)} row(s)." - ), - ) - - def validate_splunk_integration( *, base_url: str, diff --git a/tests/cli/test_smoke.py b/tests/cli/test_smoke.py index 83d2d8ebb1..0b46a2e64d 100644 --- a/tests/cli/test_smoke.py +++ b/tests/cli/test_smoke.py @@ -802,7 +802,16 @@ def test_onboard_interactive_smoke_cli_provider_repick_when_unauthenticated( @pytest.mark.skipif(os.name == "nt", reason="interactive smoke uses POSIX PTYs") -def test_integrations_setup_datadog_interactive_smoke(cli_sandbox: CliSandbox) -> None: +def test_integrations_setup_datadog_rejects_credentials_that_do_not_verify( + cli_sandbox: CliSandbox, +) -> None: + """Placeholder keys must leave nothing behind, on any tier. + + This used to save first and verify afterwards, so a typo'd key overwrote a + working integration and the command still reported ``Saved``. The shared + setup flow verifies before it persists; with keys the Datadog API rejects, + the store and ``.env`` are expected to stay untouched. + """ result = _run_cli_pty( cli_sandbox, "integrations", @@ -810,22 +819,17 @@ def test_integrations_setup_datadog_interactive_smoke(cli_sandbox: CliSandbox) - "datadog", actions=[ PtyAction(expect="API key", send=b"dd-api-key\r"), - PtyAction(expect="Application key", send=b"dd-app-key\r"), + PtyAction(expect="application key", send=b"dd-app-key\r"), PtyAction(expect="Site", send=b"\r"), ], # Setup runs verify against the Datadog API; CI runners can exceed 20s. timeout=45.0, ) - assert "Saved" in result.stdout - # Setup saves credentials then runs verify; placeholder keys fail the Datadog API check. - assert result.exit_code in (0, 1) - - integrations = cli_sandbox.read_integrations() - assert len(integrations) == 1 - assert integrations[0]["service"] == "datadog" - # v2 store shape: credentials live inside the default instance. - assert integrations[0]["instances"][0]["credentials"]["site"] == "datadoghq.com" + assert result.exit_code == 1 + assert "Saved" not in result.stdout + assert cli_sandbox.read_integrations() == [] + assert "DD_SITE" not in cli_sandbox.read_project_env() @pytest.mark.skipif(os.name == "nt", reason="interactive smoke uses POSIX PTYs") diff --git a/tests/cli/wizard/test_configurators_chat_notifications.py b/tests/cli/wizard/test_configurators_chat_notifications.py index 12b573e7e3..78b7488e62 100644 --- a/tests/cli/wizard/test_configurators_chat_notifications.py +++ b/tests/cli/wizard/test_configurators_chat_notifications.py @@ -27,6 +27,7 @@ import integrations.setup_flow as setup_flow import surfaces.cli.wizard.configurators.chat_notifications as chat_notifications +import surfaces.cli.wizard.configurators.spec_configurator as spec_configurator _TOKEN = "123456789:AAExampleSecretTokenValue" _CHAT_REFERENCE = "@acme_alerts" @@ -104,10 +105,10 @@ def _fake_resolve(credentials: dict[str, str | None]) -> setup_flow.ResolvedCred note="Delivering to Acme Alerts (channel).", ) - monkeypatch.setattr(chat_notifications, "_console", run.console) - monkeypatch.setattr(chat_notifications, "_integration_defaults", lambda _s: ({}, {})) - monkeypatch.setattr(chat_notifications, "_prompt_value", _fake_prompt) - monkeypatch.setattr(chat_notifications, "_render_integration_result", lambda *_a: None) + monkeypatch.setattr(spec_configurator, "_console", run.console) + monkeypatch.setattr(spec_configurator, "_integration_defaults", lambda _s: ({}, {})) + monkeypatch.setattr(spec_configurator, "_prompt_value", _fake_prompt) + monkeypatch.setattr(spec_configurator, "_render_integration_result", lambda *_a: None) monkeypatch.setattr( chat_notifications, "TELEGRAM_SETUP", diff --git a/tests/cli/wizard/test_flow.py b/tests/cli/wizard/test_flow.py index 9805d32b1c..2705ff418a 100644 --- a/tests/cli/wizard/test_flow.py +++ b/tests/cli/wizard/test_flow.py @@ -1,5 +1,6 @@ from __future__ import annotations +import dataclasses import json import os from unittest.mock import MagicMock @@ -297,6 +298,7 @@ def test_run_wizard_configures_honeycomb(monkeypatch, tmp_path) -> None: text_responses = iter(["prod-api", "https://api.honeycomb.io"]) saved_integrations: list[tuple[str, dict]] = [] synced_env_values: list[dict[str, str]] = [] + synced_env_secrets: list[tuple[str, str]] = [] def _mock_select(*_args, **_kwargs): m = MagicMock() @@ -320,8 +322,11 @@ def _mock_text(*_args, **_kwargs): monkeypatch.setattr(flow, "probe_local_target", lambda _path: ProbeResult("local", True, "ok")) monkeypatch.setattr( _observability_configurator, - "validate_honeycomb_integration", - lambda **_kwargs: flow.IntegrationHealthResult(ok=True, detail="Honeycomb ok"), + "HONEYCOMB_SETUP", + dataclasses.replace( + _observability_configurator.HONEYCOMB_SETUP, + verify=lambda _source, _config: {"status": "passed", "detail": "Honeycomb ok"}, + ), ) monkeypatch.setattr(flow, "save_local_config", lambda **_kwargs: tmp_path / "opensre.json") monkeypatch.setattr(flow, "sync_provider_env", lambda **_kwargs: tmp_path / ".env") @@ -331,9 +336,12 @@ def _sync_env_values(values: dict[str, str], **_kwargs): synced_env_values.append(values) return tmp_path / ".env" - monkeypatch.setattr(_observability_configurator, "sync_env_values", _sync_env_values) + monkeypatch.setattr(_setup_flow, "sync_env_values", _sync_env_values) monkeypatch.setattr( - _observability_configurator, + _setup_flow, "sync_env_secret", lambda key, value: synced_env_secrets.append((key, value)) + ) + monkeypatch.setattr( + _setup_flow, "upsert_integration", lambda service, payload: saved_integrations.append((service, payload)), ) @@ -359,6 +367,8 @@ def _sync_env_values(values: dict[str, str], **_kwargs): "HONEYCOMB_API_URL": "https://api.honeycomb.io", } ] + # The wizard previously wrote the dataset and URL but dropped the key entirely. + assert synced_env_secrets == [("HONEYCOMB_API_KEY", "hny_test")] def test_run_wizard_configures_coralogix(monkeypatch, tmp_path) -> None: @@ -373,6 +383,7 @@ def test_run_wizard_configures_coralogix(monkeypatch, tmp_path) -> None: ) saved_integrations: list[tuple[str, dict]] = [] synced_env_values: list[dict[str, str]] = [] + synced_env_secrets: list[tuple[str, str]] = [] def _mock_select(*_args, **_kwargs): m = MagicMock() @@ -396,8 +407,11 @@ def _mock_text(*_args, **_kwargs): monkeypatch.setattr(flow, "probe_local_target", lambda _path: ProbeResult("local", True, "ok")) monkeypatch.setattr( _observability_configurator, - "validate_coralogix_integration", - lambda **_kwargs: flow.IntegrationHealthResult(ok=True, detail="Coralogix ok"), + "CORALOGIX_SETUP", + dataclasses.replace( + _observability_configurator.CORALOGIX_SETUP, + verify=lambda _source, _config: {"status": "passed", "detail": "Coralogix ok"}, + ), ) monkeypatch.setattr(flow, "save_local_config", lambda **_kwargs: tmp_path / "opensre.json") monkeypatch.setattr(flow, "sync_provider_env", lambda **_kwargs: tmp_path / ".env") @@ -407,9 +421,12 @@ def _sync_env_values(values: dict[str, str], **_kwargs): synced_env_values.append(values) return tmp_path / ".env" - monkeypatch.setattr(_observability_configurator, "sync_env_values", _sync_env_values) + monkeypatch.setattr(_setup_flow, "sync_env_values", _sync_env_values) monkeypatch.setattr( - _observability_configurator, + _setup_flow, "sync_env_secret", lambda key, value: synced_env_secrets.append((key, value)) + ) + monkeypatch.setattr( + _setup_flow, "upsert_integration", lambda service, payload: saved_integrations.append((service, payload)), ) @@ -437,6 +454,8 @@ def _sync_env_values(values: dict[str, str], **_kwargs): "CORALOGIX_SUBSYSTEM_NAME": "worker", } ] + # The wizard previously wrote the URL and filters but dropped the key entirely. + assert synced_env_secrets == [("CORALOGIX_API_KEY", "cx_test")] def test_run_wizard_configures_dagster(monkeypatch, tmp_path) -> None: diff --git a/tests/cli/wizard/test_integration_health.py b/tests/cli/wizard/test_integration_health.py index 7361ccd5fd..48f4843569 100644 --- a/tests/cli/wizard/test_integration_health.py +++ b/tests/cli/wizard/test_integration_health.py @@ -12,13 +12,10 @@ from surfaces.cli.wizard.integration_health import ( validate_aws_integration, validate_betterstack_integration, - validate_coralogix_integration, validate_dagster_integration, - validate_datadog_integration, validate_discord_bot, validate_github_mcp_integration, validate_grafana_integration, - validate_honeycomb_integration, validate_incident_io_integration, validate_sentry_integration, validate_servicenow_integration, @@ -35,15 +32,12 @@ def test_legacy_integration_health_import_surface_still_exports_validators() -> "validate_alertmanager_integration", "validate_aws_integration", "validate_betterstack_integration", - "validate_coralogix_integration", "validate_dagster_integration", - "validate_datadog_integration", "validate_discord_bot", "validate_github_mcp_integration", "validate_gitlab_integration", "validate_google_docs_integration", "validate_grafana_integration", - "validate_honeycomb_integration", "validate_incident_io_integration", "validate_jenkins_integration", "validate_jira_integration", @@ -78,14 +72,6 @@ def discover_datasource_uids(self) -> dict[str, str]: return self._discovered -class _FakeDatadogClient: - def __init__(self, result: dict[str, object]) -> None: - self._result = result - - def list_monitors(self) -> dict[str, object]: - return self._result - - def test_validate_grafana_integration_succeeds_when_datasources_are_discovered(monkeypatch) -> None: monkeypatch.setattr( "surfaces.cli.wizard.integration_validators.observability.get_grafana_client_from_credentials", @@ -110,50 +96,6 @@ def test_validate_grafana_integration_fails_when_no_datasources_are_found(monkey assert "no datasources" in result.detail.lower() -def test_validate_datadog_integration_succeeds(monkeypatch) -> None: - monkeypatch.setattr( - "surfaces.cli.wizard.integration_validators.observability.DatadogClient", - lambda _config: _FakeDatadogClient({"success": True, "total": 7}), - ) - - result = validate_datadog_integration(api_key="dd-api", app_key="dd-app", site="datadoghq.com") - - assert result.ok is True - assert "fetched 7 monitors" in result.detail.lower() - - -def test_validate_datadog_integration_fails(monkeypatch) -> None: - monkeypatch.setattr( - "surfaces.cli.wizard.integration_validators.observability.DatadogClient", - lambda _config: _FakeDatadogClient({"success": False, "error": "HTTP 403"}), - ) - - result = validate_datadog_integration(api_key="dd-api", app_key="dd-app", site="datadoghq.com") - - assert result.ok is False - assert "http 403" in result.detail.lower() - - -def test_validate_honeycomb_integration_succeeds(monkeypatch) -> None: - monkeypatch.setattr( - "surfaces.cli.wizard.integration_validators.observability.HoneycombClient.validate_access", - lambda _self: {"success": True, "environment": {"slug": "prod"}}, - ) - monkeypatch.setattr( - "surfaces.cli.wizard.integration_validators.observability.HoneycombClient.run_query", - lambda _self, *_args, **_kwargs: {"success": True, "results": [{}]}, - ) - - result = validate_honeycomb_integration( - api_key="hny_test", - dataset="prod-api", - base_url="https://api.honeycomb.io", - ) - - assert result.ok is True - assert "dataset prod-api" in result.detail.lower() - - def test_validate_incident_io_integration_succeeds(monkeypatch) -> None: class _FakeIncidentIoClient: def __init__(self, _config) -> None: @@ -179,23 +121,6 @@ def list_incidents(self, **_kwargs): assert "api key accepted" in result.detail.lower() -def test_validate_coralogix_integration_fails(monkeypatch) -> None: - monkeypatch.setattr( - "surfaces.cli.wizard.integration_validators.observability.CoralogixClient.validate_access", - lambda _self: {"success": False, "error": "HTTP 401"}, - ) - - result = validate_coralogix_integration( - api_key="cx_test", - base_url="https://api.coralogix.com", - application_name="payments", - subsystem_name="worker", - ) - - assert result.ok is False - assert "http 401" in result.detail.lower() - - @pytest.mark.parametrize("status_code", [200, 400, 403, 405]) def test_validate_slack_webhook_succeeds_for_allowed_probe_statuses( monkeypatch, diff --git a/tests/cli/wizard/test_spec_configurator.py b/tests/cli/wizard/test_spec_configurator.py new file mode 100644 index 0000000000..afe644ec0a --- /dev/null +++ b/tests/cli/wizard/test_spec_configurator.py @@ -0,0 +1,137 @@ +"""Behavior of the wizard's shared spec-driven collection loop. + +``configure_from_spec`` is what every spec-backed configurator delegates to, so +the prompt-level rules live here rather than being re-asserted per vendor: what +each field is prefilled with, that a blank answer to a defaulted field is +accepted rather than re-prompted, and that a failed verification re-asks instead +of dropping the user out of onboarding. +""" + +from __future__ import annotations + +import dataclasses +from pathlib import Path +from typing import Any + +import pytest + +import integrations.setup_flow as setup_flow +import surfaces.cli.wizard.configurators.spec_configurator as spec_configurator + +_ENV_PATH = Path("/tmp/opensre-test/.env") + +_SPEC = setup_flow.IntegrationSetupSpec( + service="demo", + fields=( + setup_flow.SetupField( + name="api_key", label="Demo API key", env_var="DEMO_API_KEY", secret=True + ), + setup_flow.SetupField( + name="site", label="Demo site", env_var="DEMO_SITE", default="demo.example.com" + ), + setup_flow.SetupField(name="note", label="Demo note", env_var="DEMO_NOTE", required=False), + ), + verify=lambda _source, _config: {"status": "passed", "detail": "Demo connected."}, +) + + +@dataclasses.dataclass +class _Run: + """What the loop asked for, and what it answered with.""" + + answers: dict[str, str] = dataclasses.field(default_factory=dict) + stored: dict[str, Any] = dataclasses.field(default_factory=dict) + asked: list[dict[str, Any]] = dataclasses.field(default_factory=list) + + +@pytest.fixture +def run(monkeypatch: pytest.MonkeyPatch) -> _Run: + state = _Run() + + def _fake_prompt_value( + label: str, + *, + default: str = "", + secret: bool = False, + allow_empty: bool = False, + **_kw: Any, + ) -> str: + state.asked.append( + {"label": label, "default": default, "secret": secret, "allow_empty": allow_empty} + ) + # Mirror the real _prompt_value: a blank answer falls back to the default. + return state.answers.get(label, "") or default + + monkeypatch.setattr(spec_configurator, "_prompt_value", _fake_prompt_value) + monkeypatch.setattr(spec_configurator, "_integration_defaults", lambda _s: ({}, state.stored)) + monkeypatch.setattr(spec_configurator, "_render_integration_result", lambda *_a: None) + monkeypatch.setattr(setup_flow, "upsert_integration", lambda *_a: None) + monkeypatch.setattr(setup_flow, "sync_env_secret", lambda *_a: None) + monkeypatch.setattr(setup_flow, "sync_env_values", lambda *_a, **_kw: _ENV_PATH) + return state + + +def test_blank_answer_to_a_defaulted_field_is_accepted(run: _Run) -> None: + """The rule the ``allow_empty`` argument depends on. + + ``allow_empty=False`` is passed for required fields, defaulted or not, and is + only consulted when there is no default to fall back on. Pressing enter on a + defaulted field must therefore succeed rather than loop on "Required.". + """ + run.answers = {"Demo API key": "key-1"} + + title, env_path = spec_configurator.configure_from_spec(_SPEC, title="Demo") + + assert (title, env_path) == ("Demo", str(_ENV_PATH)) + site = next(entry for entry in run.asked if entry["label"] == "Demo site") + assert site["default"] == "demo.example.com" + assert site["allow_empty"] is False + + +def test_optional_field_without_a_default_may_be_left_empty(run: _Run) -> None: + run.answers = {"Demo API key": "key-1"} + + spec_configurator.configure_from_spec(_SPEC, title="Demo") + + note = next(entry for entry in run.asked if entry["label"] == "Demo note") + assert note["allow_empty"] is True + assert note["default"] == "" + + +def test_a_stored_value_is_prefilled_over_the_spec_default(run: _Run) -> None: + """Re-running onboarding should be a series of enters, not a retype.""" + run.stored = {"api_key": "stored-key", "site": "stored.example.com"} + run.answers = {} + + spec_configurator.configure_from_spec(_SPEC, title="Demo") + + prefilled = {entry["label"]: entry["default"] for entry in run.asked} + assert prefilled["Demo API key"] == "stored-key" + assert prefilled["Demo site"] == "stored.example.com" + + +def test_secret_fields_are_marked_for_masking(run: _Run) -> None: + run.answers = {"Demo API key": "key-1"} + + spec_configurator.configure_from_spec(_SPEC, title="Demo") + + assert {entry["label"]: entry["secret"] for entry in run.asked} == { + "Demo API key": True, + "Demo site": False, + "Demo note": False, + } + + +def test_failed_verification_re_asks_instead_of_leaving_the_wizard(run: _Run) -> None: + """Onboarding must survive a typo; the user gets another go at the prompts.""" + outcomes = iter([("failed", "Demo rejected the key."), ("passed", "Demo connected.")]) + spec = dataclasses.replace( + _SPEC, verify=lambda _source, _config: dict(zip(("status", "detail"), next(outcomes))) + ) + run.answers = {"Demo API key": "key-1"} + + title, _env_path = spec_configurator.configure_from_spec(spec, title="Demo") + + assert title == "Demo" + # Three fields asked twice: the first round failed, the second succeeded. + assert len(run.asked) == 6 diff --git a/tests/integrations/test_cli_spec_setup.py b/tests/integrations/test_cli_spec_setup.py new file mode 100644 index 0000000000..7537c59529 --- /dev/null +++ b/tests/integrations/test_cli_spec_setup.py @@ -0,0 +1,189 @@ +"""Behavior of the spec-driven ``opensre integrations setup `` handlers. + +One parametrized suite rather than a file per vendor: the handlers are now a +two-line delegation to :func:`integrations.cli._run_spec_setup`, so what is +worth pinning is the same for each — the prompt order and which answers are +masked, that nothing is written until verification passes, and that the +credentials reach the keyring and ``.env`` rather than the store alone. + +That last one is the migration's point. These handlers previously called +``upsert_integration`` and stopped, which reads fine at runtime (the store is +resolved first) but leaves the deploy preflight — which reads env vars — +declaring a working integration missing. + +Vendor-specific behavior stays with the vendor: +:mod:`tests.integrations.telegram.test_cli_setup_characterization` covers +Telegram's chat-id resolution, and +:mod:`tests.integrations.test_setup_spec_env_round_trip` covers the env var +names themselves. +""" + +from __future__ import annotations + +import dataclasses +from typing import Any + +import pytest + +import integrations.cli as cli +import integrations.coralogix.setup as coralogix_setup +import integrations.datadog.setup as datadog_setup +import integrations.honeycomb.setup as honeycomb_setup +import integrations.setup_flow as setup_flow + +_ANSWERS: dict[str, dict[str, str]] = { + "datadog": {"api_key": "dd-api-key", "app_key": "dd-app-key", "site": "datadoghq.eu"}, + "honeycomb": { + "api_key": "hc-api-key", + "dataset": "checkout-prod", + "base_url": "https://api.eu1.honeycomb.io", + }, + "coralogix": { + "api_key": "cx-api-key", + "base_url": "https://api.eu2.coralogix.com", + "application_name": "checkout", + "subsystem_name": "api", + }, +} + +# (spec module, spec attribute, CLI handler) — the attribute is patched rather +# than the spec object because ``_setup_*`` imports it inside the function body. +_CASES = [ + pytest.param(datadog_setup, "DATADOG_SETUP", cli._setup_datadog, id="datadog"), + pytest.param(honeycomb_setup, "HONEYCOMB_SETUP", cli._setup_honeycomb, id="honeycomb"), + pytest.param(coralogix_setup, "CORALOGIX_SETUP", cli._setup_coralogix, id="coralogix"), +] + + +@dataclasses.dataclass +class _Run: + """Scripted verifier outcome for one run, plus everything the run did.""" + + verify_status: str = "passed" + verify_detail: str = "Connected." + + asked: list[tuple[str, str, bool]] = dataclasses.field(default_factory=list) + verified: list[dict[str, Any]] = dataclasses.field(default_factory=list) + store: list[tuple[str, dict[str, Any]]] = dataclasses.field(default_factory=list) + keyring: list[tuple[str, str]] = dataclasses.field(default_factory=list) + env: list[dict[str, str]] = dataclasses.field(default_factory=list) + + +@pytest.fixture +def run(monkeypatch: pytest.MonkeyPatch) -> _Run: + state = _Run() + monkeypatch.setattr( + setup_flow, + "upsert_integration", + lambda service, payload: state.store.append((service, payload)), + ) + monkeypatch.setattr( + setup_flow, "sync_env_secret", lambda key, value: state.keyring.append((key, value)) + ) + monkeypatch.setattr( + setup_flow, "sync_env_values", lambda values, **_kw: state.env.append(dict(values)) + ) + return state + + +def _install( + monkeypatch: pytest.MonkeyPatch, module: Any, attr: str, state: _Run, blank: str = "" +) -> setup_flow.IntegrationSetupSpec: + """Swap in a stub verifier and script ``_p`` with this vendor's answers. + + Pass *blank* to answer one field with an empty string instead. + """ + + def _fake_verify(_source: str, config: dict[str, Any]) -> dict[str, str]: + state.verified.append(dict(config)) + return {"status": state.verify_status, "detail": state.verify_detail} + + spec = dataclasses.replace(getattr(module, attr), verify=_fake_verify) + monkeypatch.setattr(module, attr, spec) + + answers = _ANSWERS[spec.service] + queue = ["" if field.name == blank else answers[field.name] for field in spec.fields] + + def _fake_p(label: str, default: str = "", secret: bool = False) -> str: + state.asked.append((label, default, secret)) + return queue.pop(0) + + monkeypatch.setattr(cli, "_p", _fake_p) + return spec + + +@pytest.mark.parametrize(("module", "attr", "handler"), _CASES) +def test_prompts_follow_the_spec_and_mask_only_secret_fields( + monkeypatch: pytest.MonkeyPatch, run: _Run, module: Any, attr: str, handler: Any +) -> None: + spec = _install(monkeypatch, module, attr, run) + + handler() + + assert [label for label, _default, _secret in run.asked] == [ + field.question for field in spec.fields + ] + assert [secret for _label, _default, secret in run.asked] == [ + field.secret for field in spec.fields + ] + + +@pytest.mark.parametrize(("module", "attr", "handler"), _CASES) +def test_defaults_are_offered_as_prompt_prefills( + monkeypatch: pytest.MonkeyPatch, run: _Run, module: Any, attr: str, handler: Any +) -> None: + """A user pressing enter should land on the documented default, not blank.""" + spec = _install(monkeypatch, module, attr, run) + + handler() + + assert [default for _label, default, _secret in run.asked] == [ + field.default for field in spec.fields + ] + + +@pytest.mark.parametrize(("module", "attr", "handler"), _CASES) +def test_credentials_reach_the_keyring_and_env_not_just_the_store( + monkeypatch: pytest.MonkeyPatch, run: _Run, module: Any, attr: str, handler: Any +) -> None: + spec = _install(monkeypatch, module, attr, run) + answers = _ANSWERS[spec.service] + + handler() + + assert run.store == [(spec.service, {"credentials": dict(answers)})] + secret_fields = {f.env_var: answers[f.name] for f in spec.fields if f.secret} + assert dict(run.keyring) == secret_fields + plain_fields = {f.env_var: answers[f.name] for f in spec.fields if f.env_var and not f.secret} + assert run.env == [plain_fields] + + +@pytest.mark.parametrize(("module", "attr", "handler"), _CASES) +def test_failed_verification_exits_without_saving( + monkeypatch: pytest.MonkeyPatch, run: _Run, module: Any, attr: str, handler: Any +) -> None: + """A bad credential must not overwrite a working integration.""" + run.verify_status = "failed" + run.verify_detail = "Rejected." + _install(monkeypatch, module, attr, run) + + with pytest.raises(SystemExit): + handler() + + assert (run.store, run.keyring, run.env) == ([], [], []) + + +@pytest.mark.parametrize(("module", "attr", "handler"), _CASES) +def test_blank_required_field_exits_before_the_next_prompt( + monkeypatch: pytest.MonkeyPatch, run: _Run, module: Any, attr: str, handler: Any +) -> None: + """Fail on the field that is blank, not after working through the rest.""" + spec = getattr(module, attr) + first_required = next(f for f in spec.fields if f.required and not f.default) + _install(monkeypatch, module, attr, run, blank=first_required.name) + + with pytest.raises(SystemExit): + handler() + + assert len(run.asked) == 1 + [f.name for f in spec.fields].index(first_required.name) + assert (run.verified, run.store) == ([], []) diff --git a/tests/integrations/test_setup_flow.py b/tests/integrations/test_setup_flow.py index 1548ea210a..1ba126e0a8 100644 --- a/tests/integrations/test_setup_flow.py +++ b/tests/integrations/test_setup_flow.py @@ -97,6 +97,64 @@ def test_optional_field_left_blank_is_stored_as_none(recorder: _Recorder) -> Non assert recorder.saved[0][1]["credentials"]["note"] is None +def test_blank_field_falls_back_to_its_default(recorder: _Recorder) -> None: + """The default applies in the flow, not only as a prompt prefill. + + A surface that never prompts — an agent filling fields from a conversation — + must land on the same credentials as someone pressing enter at the CLI. + """ + spec = dataclasses.replace( + _SPEC, + fields=( + _FIELDS[0], + setup_flow.SetupField( + name="room", label="Demo room", env_var="DEMO_ROOM", default="general" + ), + ), + ) + + setup_flow.apply_setup(spec, {"api_token": "tok-1", "room": ""}) + + assert recorder.saved[0][1]["credentials"]["room"] == "general" + assert recorder.env_values == [{"DEMO_ROOM": "general"}] + + +def test_a_submitted_value_wins_over_the_default(recorder: _Recorder) -> None: + spec = dataclasses.replace( + _SPEC, + fields=( + _FIELDS[0], + setup_flow.SetupField( + name="room", label="Demo room", env_var="DEMO_ROOM", default="general" + ), + ), + ) + + setup_flow.apply_setup(spec, {"api_token": "tok-1", "room": "incidents"}) + + assert recorder.saved[0][1]["credentials"]["room"] == "incidents" + + +def test_a_required_field_with_a_default_is_never_missing(recorder: _Recorder) -> None: + spec = dataclasses.replace( + _SPEC, + fields=( + _FIELDS[0], + setup_flow.SetupField( + name="room", + label="Demo room", + env_var="DEMO_ROOM", + default="general", + required=True, + ), + ), + ) + + outcome = setup_flow.apply_setup(spec, {"api_token": "tok-1"}) + + assert outcome.ok is True + + def test_failed_verification_persists_nothing(recorder: _Recorder) -> None: def _rejecting(_source: str, _config: dict[str, str]) -> dict[str, str]: return {"status": "failed", "detail": "Demo rejected the token."} diff --git a/tests/integrations/test_setup_spec_env_round_trip.py b/tests/integrations/test_setup_spec_env_round_trip.py new file mode 100644 index 0000000000..7b3b66cac5 --- /dev/null +++ b/tests/integrations/test_setup_spec_env_round_trip.py @@ -0,0 +1,133 @@ +"""Every ``SetupField.env_var`` must be a name the catalog actually reads back. + +:func:`integrations.setup_flow.apply_setup` writes credentials to ``.env`` and +the keyring; :func:`integrations._catalog_impl.load_env_integrations` is what +reads them again — and the two sides name the same value differently +(``base_url`` is written as ``HONEYCOMB_API_URL``, ``endpoint`` as +``GRAFANA_INSTANCE_URL``). A spec that declares an env var nothing reads still +passes every test in :mod:`tests.integrations.test_setup_flow`, because those +mock the writers: the value lands in ``.env`` and is silently never resolved +again. The failure surfaces only as a deploy preflight calling a fully +configured integration missing. + +So this closes the loop end to end — persist through the real ``.env`` writer, +load the result into the environment, and require the catalog to hand back the +same credentials. +""" + +from __future__ import annotations + +import dataclasses +import functools +from pathlib import Path +from typing import Any + +import pytest + +import integrations.setup_flow as setup_flow +from config.env_file import env_assignment_key, read_env_lines, sync_env_values +from integrations._catalog_impl import load_env_integrations +from integrations.coralogix.setup import CORALOGIX_SETUP +from integrations.datadog.setup import DATADOG_SETUP +from integrations.honeycomb.setup import HONEYCOMB_SETUP +from integrations.telegram.setup import TELEGRAM_SETUP + +# A distinct, recognizable value per field, so two fields of the same +# integration swapping places fails instead of coincidentally matching. Values +# are deliberately non-default (EU hosts, a named dataset) — a default would +# still "round-trip" through a spec that wrote nothing at all. +_SUBMITTED: dict[str, dict[str, str]] = { + "datadog": {"api_key": "dd-api-key", "app_key": "dd-app-key", "site": "datadoghq.eu"}, + "honeycomb": { + "api_key": "hc-api-key", + "dataset": "checkout-prod", + "base_url": "https://api.eu1.honeycomb.io", + }, + "coralogix": { + "api_key": "cx-api-key", + "base_url": "https://api.eu2.coralogix.com", + "application_name": "checkout", + "subsystem_name": "api", + }, + "telegram": {"bot_token": "123456:tg-bot-token", "default_chat_id": "-1001234567890"}, +} + +_SPECS = [DATADOG_SETUP, HONEYCOMB_SETUP, CORALOGIX_SETUP, TELEGRAM_SETUP] + + +@dataclasses.dataclass +class _Persisted: + """Where a run's credentials ended up.""" + + env_path: Path + secrets: dict[str, str] + + +@pytest.fixture +def persisted(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> _Persisted: + """Point the flow's writers at a throwaway ``.env`` and an in-memory keyring. + + The real :func:`config.env_file.sync_env_values` is kept and only its target + moves, so its refusal to write a sensitive key to disk still applies. Only + the keyring backend is replaced — a test must not touch the real one. + """ + written = _Persisted(env_path=tmp_path / ".env", secrets={}) + monkeypatch.setattr( + setup_flow, + "sync_env_values", + functools.partial(sync_env_values, env_path=written.env_path), + ) + monkeypatch.setattr(setup_flow, "sync_env_secret", written.secrets.__setitem__) + monkeypatch.setattr(setup_flow, "upsert_integration", lambda _service, _payload: None) + return written + + +def _restore_environment(written: _Persisted, monkeypatch: pytest.MonkeyPatch) -> None: + """Reproduce the environment a later process would start with. + + Keyring secrets are seeded straight into ``os.environ`` because + ``resolve_env_credential`` checks the environment first — which is what a + deploy, and this assertion, ultimately depend on. + """ + for key, value in written.secrets.items(): + monkeypatch.setenv(key, value) + for line in read_env_lines(written.env_path): + key = env_assignment_key(line) + if key: + monkeypatch.setenv(key, line.split("=", 1)[1].strip()) + + +def _catalog_credentials(service: str) -> dict[str, Any]: + for record in load_env_integrations(): + if record.get("service") == service: + credentials = record.get("credentials") + assert isinstance(credentials, dict) + return credentials + raise AssertionError(f"{service} was not discovered from the environment") + + +@pytest.mark.parametrize("spec", _SPECS, ids=lambda spec: spec.service) +def test_persisted_credentials_are_read_back_by_the_catalog( + spec: setup_flow.IntegrationSetupSpec, persisted: _Persisted, monkeypatch: pytest.MonkeyPatch +) -> None: + submitted = _SUBMITTED[spec.service] + assert {field.name for field in spec.fields} == set(submitted), ( + f"{spec.service} spec fields changed; update this test's submitted values" + ) + + # Verification and reference resolution are each integration's own concern + # and covered per vendor. Dropping them here keeps the test on one question: + # do the values come back out under the names the spec wrote them? + outcome = setup_flow.apply_setup( + dataclasses.replace(spec, verify=None, resolve=None), submitted + ) + assert outcome.ok, outcome.detail + + _restore_environment(persisted, monkeypatch) + + resolved = _catalog_credentials(spec.service) + for field in spec.fields: + assert resolved.get(field.name) == submitted[field.name], ( + f"{spec.service}.{field.name} was persisted as {field.env_var!r}, " + "which the catalog does not read back into that credential" + )