Repository navigation
fix(config): use api.{realm} CRUD host and add O11y API URL override (SAO-18181) #304
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,12 +3,13 @@ | |
| import os | ||
| from dataclasses import dataclass | ||
| from enum import StrEnum | ||
| from urllib.parse import urlsplit | ||
|
|
||
| from pydantic import SecretStr | ||
|
|
||
| from splunk_ao.shared.exceptions import AmbiguousConfigurationError, MissingConfigurationError | ||
| from splunk_ao.shared.exceptions import AmbiguousConfigurationError, ConfigurationError, MissingConfigurationError | ||
|
|
||
| _O11Y_ENV_VARS = ("SPLUNK_AO_REALM", "SPLUNK_AO_O11Y_TOKEN", "SPLUNK_AO_O11Y_API_TOKEN") | ||
| _O11Y_ENV_VARS = ("SPLUNK_AO_REALM", "SPLUNK_AO_O11Y_TOKEN", "SPLUNK_AO_O11Y_API_TOKEN", "SPLUNK_AO_O11Y_API_URL") | ||
| _STANDALONE_ENV_VARS = ("SPLUNK_AO_API_KEY", "SPLUNK_AO_CONSOLE_URL", "SPLUNK_AO_API_URL") | ||
|
|
||
|
|
||
|
|
@@ -46,13 +47,34 @@ def resolve_deployment() -> DeploymentMode: | |
| ) | ||
|
|
||
|
|
||
| def _normalize_api_origin(value: str) -> str: | ||
| """Validate an API origin override and return it as ``https://host[:port]``.""" | ||
| try: | ||
| parts = urlsplit(value.strip()) | ||
| port = parts.port | ||
| except ValueError: | ||
| raise ConfigurationError("SPLUNK_AO_O11Y_API_URL is not a valid URL.") from None | ||
| if parts.scheme != "https" or not parts.hostname: | ||
| raise ConfigurationError("SPLUNK_AO_O11Y_API_URL must be an https:// URL.") | ||
| if "@" in parts.netloc or parts.query or parts.fragment or parts.path.strip("/"): | ||
| raise ConfigurationError( | ||
| "SPLUNK_AO_O11Y_API_URL must be an origin only (scheme://host[:port]) without credentials, path, " | ||
| "query, or fragment." | ||
| ) | ||
| if any(ch.isspace() for ch in parts.netloc): | ||
| raise ConfigurationError("SPLUNK_AO_O11Y_API_URL has an invalid host.") | ||
| host = f"[{parts.hostname}]" if ":" in parts.hostname else parts.hostname | ||
| return f"https://{host}:{port}" if port is not None else f"https://{host}" | ||
|
|
||
|
|
||
| @dataclass | ||
| class O11yConfig: | ||
| """Configuration for a Splunk Observability Cloud deployment.""" | ||
|
|
||
| realm: str | ||
| o11y_token: SecretStr | None = None | ||
| o11y_api_token: SecretStr | None = None | ||
| api_url: str | None = None | ||
|
|
||
| def __post_init__(self) -> None: | ||
| missing = [] | ||
|
|
@@ -63,6 +85,11 @@ def __post_init__(self) -> None: | |
| if missing: | ||
| raise MissingConfigurationError(f"O11y deployment requires {' and '.join(missing)} to be set.") | ||
|
|
||
| if self.api_url: | ||
| self.api_url = _normalize_api_origin(self.api_url) | ||
|
Comment on lines
+88
to
+89
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 minor (design): An invalid override is validated eagerly in
Scenario: a telemetry-only user sets Options:
Option 1 is simpler and catches typos early. It needs the doc fix and the test. 🤖 Generated by the Astra agent |
||
| else: | ||
| self.api_url = None | ||
|
|
||
| if self.o11y_token is not None and not isinstance(self.o11y_token, SecretStr): | ||
| self.o11y_token = SecretStr(self.o11y_token) | ||
| if self.o11y_api_token is not None and not isinstance(self.o11y_api_token, SecretStr): | ||
|
|
@@ -78,6 +105,7 @@ def from_env(cls) -> "O11yConfig": | |
| realm=realm or "", | ||
| o11y_token=SecretStr(o11y_token) if o11y_token else None, | ||
| o11y_api_token=SecretStr(o11y_api_token) if o11y_api_token else None, | ||
| api_url=_env("SPLUNK_AO_O11Y_API_URL"), | ||
| ) | ||
|
|
||
| @property | ||
|
|
@@ -107,11 +135,13 @@ def require_ingest_token(self) -> SecretStr: | |
|
|
||
| @property | ||
| def api_root(self) -> str: | ||
| """Return the realm-derived application origin used by the AO API.""" | ||
| return f"https://app.{self.realm}.observability.splunkcloud.com" | ||
| """Return the explicit API origin override, or the realm-derived origin used by the AO API.""" | ||
| if self.api_url: | ||
| return self.api_url | ||
| return f"https://api.{self.realm}.observability.splunkcloud.com" | ||
|
|
||
| def require_api_url(self) -> str: | ||
| """Return the realm-derived AO CRUD API URL.""" | ||
| """Return the AO CRUD API URL from the override origin or the realm-derived origin.""" | ||
| return f"{self.api_root}/ao/api/" | ||
|
|
||
| def require_console_url(self) -> str: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 minor (bug): The host check rejects only whitespace.
urlsplitaccepts and keeps other invalid host characters. I checked these values with stdliburlsplit:https://h.com;xgives hostnameh.com;x.https://h.com\evilgives hostnameh.com\evil.https://h%2e.comgives hostnameh%2e.com.https://h:0gives port0. The SDK accepts it ashttps://h:0.urlsplitalso silently strips embedded\nand\t, sohttps://api.exa\nmple.combecomesapi.example.com.These values pass validation and then fail later at request time with an unclear transport error. That defeats the purpose of the clear error here.
Fix: check
parts.hostnameagainst an allowlist pattern (LDH labels, or an IPv6 literal). Reject port0. If the user enters a Unicode IDN, encode it withidnafirst, or reject it with a message.🤖 Generated by the Astra agent