diff --git a/charms/garm-configurator/charmcraft.yaml b/charms/garm-configurator/charmcraft.yaml index 02732d97..3dbfa241 100644 --- a/charms/garm-configurator/charmcraft.yaml +++ b/charms/garm-configurator/charmcraft.yaml @@ -109,6 +109,30 @@ config: description: | A Python dict-like string containing key-value pairs of script name to bash script to be run prior to runner installation. + dockerhub-mirror: + type: string + description: | + Optional Docker registry mirror URL for runners. Must be a valid http(s) address. + runner-http-proxy: + type: string + description: | + Optional HTTP proxy that aproxy forwards to. Must be a valid http(s) URL. + aproxy-exclude-addresses: + type: string + description: | + Optional comma-separated IPv4 addresses/CIDRs to exclude from aproxy forwarding. + aproxy-redirect-ports: + type: string + description: | + Optional comma-separated ports or port ranges (e.g. 80,443,8000-9000) to forward to aproxy. + otel-collector-endpoint: + type: string + description: | + Optional OTEL exporter address for the otel-collector to connect to. Must be a valid http(s) URL. + pre-job-script: + type: string + description: | + Optional bash script appended to the end of the runner pre-job script. requires: image: diff --git a/charms/garm-configurator/src/charm.py b/charms/garm-configurator/src/charm.py index bf65ef85..eb0f8b2d 100755 --- a/charms/garm-configurator/src/charm.py +++ b/charms/garm-configurator/src/charm.py @@ -146,6 +146,7 @@ def _configure_garm_relation(self, state: CharmState) -> None: return pre_install = state.scaleset_config.pre_install_scripts + rc = state.runner_config basic_data: dict[str, str] = { "name": state.scaleset_config.name, "provider_name": state.provider_config.provider_name, @@ -159,6 +160,12 @@ def _configure_garm_relation(self, state: CharmState) -> None: "pre_install_scripts": json.dumps({"pre_install.sh": pre_install}) if pre_install else "", + "dockerhub_mirror": rc.dockerhub_mirror or "", + "runner_http_proxy": rc.runner_http_proxy or "", + "aproxy_exclude_addresses": rc.aproxy_exclude_addresses or "", + "aproxy_redirect_ports": rc.aproxy_redirect_ports or "", + "otel_collector_endpoint": rc.otel_collector_endpoint or "", + "pre_job_script": rc.pre_job_script or "", } if state.scaleset_config.org: basic_data["org"] = state.scaleset_config.org diff --git a/charms/garm-configurator/src/charm_state.py b/charms/garm-configurator/src/charm_state.py index 59e86306..1d988710 100644 --- a/charms/garm-configurator/src/charm_state.py +++ b/charms/garm-configurator/src/charm_state.py @@ -4,7 +4,16 @@ """State of the GARM configurator charm.""" import ops -from pydantic import BaseModel +from pydantic import ( + BaseModel, + HttpUrl, + IPvAnyNetwork, + TypeAdapter, + ValidationError, + ValidationInfo, + field_validator, + model_validator, +) OPENSTACK_AUTH_URL_CONFIG_NAME = "openstack-auth-url" OPENSTACK_USERNAME_CONFIG_NAME = "openstack-username" @@ -30,6 +39,16 @@ SCALESET_RUNNER_GROUP_CONFIG_NAME = "runner-group" SCALESET_PRE_INSTALL_SCRIPTS_CONFIG_NAME = "pre-install-scripts" +DOCKERHUB_MIRROR_CONFIG_NAME = "dockerhub-mirror" +RUNNER_HTTP_PROXY_CONFIG_NAME = "runner-http-proxy" +APROXY_EXCLUDE_ADDRESSES_CONFIG_NAME = "aproxy-exclude-addresses" +APROXY_REDIRECT_PORTS_CONFIG_NAME = "aproxy-redirect-ports" +OTEL_COLLECTOR_ENDPOINT_CONFIG_NAME = "otel-collector-endpoint" +PRE_JOB_SCRIPT_CONFIG_NAME = "pre-job-script" + +_HTTP_URL_ADAPTER = TypeAdapter(HttpUrl) +_IP_NETWORK_ADAPTER: TypeAdapter[IPvAnyNetwork] = TypeAdapter(IPvAnyNetwork) + IMAGE_RELATION_NAME = "image" GARM_RELATION_NAME = "garm-configurator" @@ -296,6 +315,153 @@ def from_charm(cls, charm: ops.CharmBase) -> "ScalesetConfig": ) +class RunnerConfig(BaseModel): + """Optional runner-level configuration forwarded to the GARM scaleset. + + Attributes: + dockerhub_mirror: Optional Docker registry mirror URL. + runner_http_proxy: HTTP proxy address for aproxy to forward to. + aproxy_exclude_addresses: Comma-separated IPs/CIDRs excluded from aproxy forwarding. + aproxy_redirect_ports: Comma-separated ports or N-M ranges forwarded to aproxy. + otel_collector_endpoint: OTEL exporter address for the otel-collector. + pre_job_script: Bash snippet appended to the runner pre-job script. + """ + + dockerhub_mirror: str | None = None + runner_http_proxy: str | None = None + aproxy_exclude_addresses: str | None = None + aproxy_redirect_ports: str | None = None + otel_collector_endpoint: str | None = None + pre_job_script: str | None = None + + @model_validator(mode="before") + @classmethod + def _strip_to_none(cls, data: object) -> object: + """Strip whitespace and treat empty/whitespace-only values as unset.""" + if not isinstance(data, dict): + return data + return { + key: (str(value).strip() or None) if value is not None else None + for key, value in data.items() + } + + @field_validator("dockerhub_mirror", "runner_http_proxy", "otel_collector_endpoint") + @classmethod + def _validate_http_url(cls, value: str | None, info: ValidationInfo) -> str | None: + """Require a set option to be a well-formed http(s) URL.""" + if value is None: + return None + # Field names are the snake_case form of the kebab-case config option. + msg = f"{(info.field_name or '').replace('_', '-')} must be a valid http(s) URL" + # Reject whitespace: the value is rendered into scripts and env files, + # where a newline could inject extra lines. + if any(char.isspace() for char in value): + raise ValueError(msg) + try: + parsed = _HTTP_URL_ADAPTER.validate_python(value) + except ValidationError as exc: + raise ValueError(msg) from exc + if parsed.port == 0: + raise ValueError(msg) + return value + + @field_validator("aproxy_exclude_addresses") + @classmethod + def _validate_ipv4_list(cls, value: str | None) -> str | None: + """Require a comma-separated list of IPv4 addresses/CIDRs; normalise spacing.""" + if value is None: + return None + tokens = [token.strip() for token in value.split(",")] + for token in tokens: + # The values are rendered into an nft IPv4 (table ip) ruleset, so a + # hostname or IPv6 address would validate here but fail at runtime. + try: + network = _IP_NETWORK_ADAPTER.validate_python(token) + except ValidationError as exc: + raise ValueError( + f"{APROXY_EXCLUDE_ADDRESSES_CONFIG_NAME} must be a comma-separated list " + f"of IPv4 addresses or CIDRs; got invalid token: {token!r}" + ) from exc + if network.version != 4: + raise ValueError( + f"{APROXY_EXCLUDE_ADDRESSES_CONFIG_NAME} only supports IPv4 addresses or " + f"CIDRs (the aproxy nft ruleset is IPv4-only); got: {token!r}" + ) + return ",".join(tokens) + + @field_validator("aproxy_redirect_ports") + @classmethod + def _validate_port_list(cls, value: str | None) -> str | None: + """Require a comma-separated list of ports or N-M ranges in 1..65535.""" + if value is None: + return None + msg = ( + f"{APROXY_REDIRECT_PORTS_CONFIG_NAME} must be a comma-separated list of " + "ports or N-M ranges in 1..65535" + ) + tokens = [token.strip() for token in value.split(",")] + for token in tokens: + bounds = token.split("-") + if len(bounds) > 2: + raise ValueError(msg) + try: + ports = [int(part) for part in bounds] + except ValueError as exc: + raise ValueError(msg) from exc + if not all(1 <= port <= 65535 for port in ports) or ports != sorted(ports): + raise ValueError(msg) + return ",".join(tokens) + + @model_validator(mode="after") + def _aproxy_requires_proxy(self) -> "RunnerConfig": + """The aproxy options only take effect alongside a proxy. + + The runner template renders the aproxy block solely when runner-http-proxy + is set, so reject these options on their own rather than letting them no-op. + """ + if (self.aproxy_exclude_addresses or self.aproxy_redirect_ports) and ( + not self.runner_http_proxy + ): + raise ValueError( + f"{APROXY_EXCLUDE_ADDRESSES_CONFIG_NAME} and {APROXY_REDIRECT_PORTS_CONFIG_NAME} " + f"require {RUNNER_HTTP_PROXY_CONFIG_NAME} to be set" + ) + return self + + @classmethod + def from_charm(cls, charm: ops.CharmBase) -> "RunnerConfig": + """Build and validate the runner config from charm configuration. + + All options are optional; unset, empty, or whitespace-only values become None. + + Args: + charm: The charm instance. + + Raises: + CharmConfigInvalidError: If a set option fails validation. + + Returns: + The parsed runner configuration. + """ + + def config_str(name: str) -> str | None: + """Return a string config value, or None when unset.""" + value = charm.config.get(name) + return None if value is None else str(value) + + try: + return cls( + dockerhub_mirror=config_str(DOCKERHUB_MIRROR_CONFIG_NAME), + runner_http_proxy=config_str(RUNNER_HTTP_PROXY_CONFIG_NAME), + aproxy_exclude_addresses=config_str(APROXY_EXCLUDE_ADDRESSES_CONFIG_NAME), + aproxy_redirect_ports=config_str(APROXY_REDIRECT_PORTS_CONFIG_NAME), + otel_collector_endpoint=config_str(OTEL_COLLECTOR_ENDPOINT_CONFIG_NAME), + pre_job_script=config_str(PRE_JOB_SCRIPT_CONFIG_NAME), + ) + except ValidationError as exc: + raise CharmConfigInvalidError(str(exc)) from exc + + class CharmState: """The charm state. @@ -303,6 +469,7 @@ class CharmState: provider_config: OpenStack provider configuration. github_app_config: GitHub App configuration. scaleset_config: Scaleset configuration. + runner_config: Optional runner-level configuration. image_id: OpenStack image UUID received from the image builder relation, or None. """ @@ -312,6 +479,7 @@ def __init__( provider_config: ProviderConfig, github_app_config: GithubAppConfig, scaleset_config: ScalesetConfig, + runner_config: RunnerConfig, image_id: str | None, ) -> None: """Initialize the charm state. @@ -320,11 +488,13 @@ def __init__( provider_config: The OpenStack provider configuration. github_app_config: The GitHub App configuration. scaleset_config: The scaleset configuration. + runner_config: The optional runner configuration. image_id: The OpenStack image UUID from the image builder relation. """ self.provider_config = provider_config self.github_app_config = github_app_config self.scaleset_config = scaleset_config + self.runner_config = runner_config self.image_id = image_id @classmethod @@ -343,11 +513,13 @@ def from_charm(cls, charm: ops.CharmBase) -> "CharmState": provider_config = ProviderConfig.from_charm(charm) github_app_config = GithubAppConfig.from_charm(charm) scaleset_config = ScalesetConfig.from_charm(charm) + runner_config = RunnerConfig.from_charm(charm) image_id = _get_image_id_from_relation(charm) return cls( provider_config=provider_config, github_app_config=github_app_config, scaleset_config=scaleset_config, + runner_config=runner_config, image_id=image_id, ) diff --git a/charms/garm-configurator/tests/unit/test_charm.py b/charms/garm-configurator/tests/unit/test_charm.py index 06cff97f..566d2bfe 100644 --- a/charms/garm-configurator/tests/unit/test_charm.py +++ b/charms/garm-configurator/tests/unit/test_charm.py @@ -636,3 +636,167 @@ def test_reconcile_writes_optional_scaleset_fields_to_garm_relation(): {"pre_install.sh": '{"setup.sh": "#!/bin/bash\\necho hello"}'} ) assert "repo" not in garm_out.local_unit_data + +@pytest.mark.parametrize( + "config_key, bad_value, expected_fragment", + [ + pytest.param( + "dockerhub-mirror", + "ftp://registry.example.com", + "dockerhub-mirror must be a valid http(s) URL", + id="dockerhub-mirror-bad-scheme", + ), + pytest.param( + "runner-http-proxy", + "not-a-url", + "runner-http-proxy must be a valid http(s) URL", + id="runner-http-proxy-no-scheme", + ), + pytest.param( + "otel-collector-endpoint", + "://missing-host", + "otel-collector-endpoint must be a valid http(s) URL", + id="otel-collector-endpoint-missing-host", + ), + pytest.param( + "aproxy-redirect-ports", + "80,not-a-port", + "aproxy-redirect-ports must be a comma-separated list of ports or N-M ranges in 1..65535", + id="aproxy-redirect-ports-non-numeric", + ), + pytest.param( + "aproxy-redirect-ports", + "0", + "aproxy-redirect-ports must be a comma-separated list of ports or N-M ranges in 1..65535", + id="aproxy-redirect-ports-out-of-range", + ), + pytest.param( + "aproxy-redirect-ports", + "443-80", + "aproxy-redirect-ports must be a comma-separated list of ports or N-M ranges in 1..65535", + id="aproxy-redirect-ports-inverted-range", + ), + pytest.param( + "aproxy-exclude-addresses", + "10.0.0.1,not-an-ip", + "aproxy-exclude-addresses must be a comma-separated list of IPv4 addresses or CIDRs", + id="aproxy-exclude-addresses-non-ip", + ), + pytest.param( + "aproxy-exclude-addresses", + "2001:db8::1", + "aproxy-exclude-addresses only supports IPv4", + id="aproxy-exclude-addresses-ipv6", + ), + pytest.param( + "runner-http-proxy", + "http://pro xy.example.com", + "runner-http-proxy must be a valid http(s) URL", + id="runner-http-proxy-embedded-whitespace", + ), + pytest.param( + "runner-http-proxy", + "http://proxy.example.com:bad", + "runner-http-proxy must be a valid http(s) URL", + id="runner-http-proxy-invalid-port", + ), + ], +) +def test_charm_blocked_invalid_runner_config( + config_key: str, bad_value: str, expected_fragment: str +): + """ + arrange: A single runner config option is set to an invalid value. + act: Run config-changed. + assert: Unit status is Blocked with a message matching the expected fragment. + """ + ctx = Context(GarmConfiguratorCharm) + secret = _make_secret() + pk_secret = _make_private_key_secret() + config = _valid_config(secret, pk_secret) + config[config_key] = bad_value + state = State(config=config, secrets=[secret, pk_secret]) + out = ctx.run(ctx.on.config_changed(), state) + assert isinstance(out.unit_status, ops.BlockedStatus) + assert expected_fragment in out.unit_status.message + + +def test_runner_config_aproxy_options_require_proxy(): + """ + arrange: aproxy options set without runner-http-proxy. + act: Run config-changed. + assert: Unit is Blocked — the aproxy options would otherwise silently no-op. + """ + ctx = Context(GarmConfiguratorCharm) + secret = _make_secret() + pk_secret = _make_private_key_secret() + config = _valid_config(secret, pk_secret) + config["aproxy-redirect-ports"] = "80,443" + state = State(config=config, secrets=[secret, pk_secret]) + out = ctx.run(ctx.on.config_changed(), state) + assert isinstance(out.unit_status, ops.BlockedStatus) + assert "require runner-http-proxy to be set" in out.unit_status.message + + +def test_runner_config_fields_written_to_garm_configurator_relation(): + """ + arrange: Valid config with all six runner config options set. + act: Run config-changed with a garm-configurator relation present. + assert: All six runner config keys appear in the relation databag with correct values. + """ + ctx = Context(GarmConfiguratorCharm) + secret = _make_secret() + pk_secret = _make_private_key_secret() + config = _valid_config(secret, pk_secret) + config["dockerhub-mirror"] = "https://mirror.example.com" + config["runner-http-proxy"] = "http://proxy.example.com:3128" + config["aproxy-exclude-addresses"] = "10.0.0.1,192.168.0.0/16" + config["aproxy-redirect-ports"] = "80,443,8000-9000" + config["otel-collector-endpoint"] = "http://otel.example.com:4317" + config["pre-job-script"] = " echo start " + garm_relation = _make_garm_configurator_relation() + state = State( + config=config, + secrets=[secret, pk_secret], + relations=[garm_relation], + ) + + out = ctx.run(ctx.on.config_changed(), state) + + rel_out = out.get_relation(garm_relation.id) + assert rel_out.local_unit_data["dockerhub_mirror"] == "https://mirror.example.com" + assert rel_out.local_unit_data["runner_http_proxy"] == "http://proxy.example.com:3128" + assert rel_out.local_unit_data["aproxy_exclude_addresses"] == "10.0.0.1,192.168.0.0/16" + assert rel_out.local_unit_data["aproxy_redirect_ports"] == "80,443,8000-9000" + assert rel_out.local_unit_data["otel_collector_endpoint"] == "http://otel.example.com:4317" + assert rel_out.local_unit_data["pre_job_script"] == "echo start" + + +def test_runner_config_fields_absent_when_unset(): + """ + arrange: Valid config with no runner config options set. + act: Run config-changed with a garm-configurator relation present. + assert: The six runner config keys are absent from the databag. + """ + ctx = Context(GarmConfiguratorCharm) + secret = _make_secret() + pk_secret = _make_private_key_secret() + garm_relation = _make_garm_configurator_relation() + state = State( + config=_valid_config(secret, pk_secret), + secrets=[secret, pk_secret], + relations=[garm_relation], + ) + + out = ctx.run(ctx.on.config_changed(), state) + + rel_out = out.get_relation(garm_relation.id) + for key in ( + "dockerhub_mirror", + "runner_http_proxy", + "aproxy_exclude_addresses", + "aproxy_redirect_ports", + "otel_collector_endpoint", + "pre_job_script", + ): + assert rel_out.local_unit_data.get(key, "") == "" diff --git a/charms/garm/AGENTS.md b/charms/garm/AGENTS.md index 1f5d9f9b..14a64118 100644 --- a/charms/garm/AGENTS.md +++ b/charms/garm/AGENTS.md @@ -9,12 +9,18 @@ charm conventions; this file lists only what's specific to `garm`. - **Secrets (owner).** `garm` owns two labelled juju secrets — `GARM_SECRETS_LABEL` and `GARM_ADMIN_CREDENTIALS_LABEL` — created leader-only in `_ensure_secrets`. - **DO** read them with plain `get_content()` (`_get_secrets`, `_get_admin_credentials`). - **DON'T** pass `refresh=True` — that's an observer concept (see root `AGENTS.md`). -- **Domain logic is factored out of `charm.py`**: `src/garm_api.py` and `src/garm_client/` - **Domain logic is factored out of `charm.py`**: `src/garm_api.py` and `src/garm_client/` (the GARM HTTP client), the per-resource reconcilers (`src/github_reconciler.py`, `src/entity_reconciler.py`, `src/scaleset_reconciler.py`), and relation-derived desired state (`src/charm_state.py` — `CharmState.from_charm`). Extend these rather than growing the charm class. TOML rendering: `render_garm_toml()` in `src/charm.py`. + - **Relation-/config-derived value objects belong in `src/charm_state.py`** (per + [ISD014](https://discourse.charmhub.io/t/specification-isd014-managing-charm-complexity/11619)): + keep desired-state types parsed from relation or config data (`CharmState`, `SSHDebugInfo`, + `RunnerConfig`) there. **DON'T** define them in rendering/helper modules — e.g. + `runner_template.py` only *renders* a `RunnerConfig`, it doesn't own it. Rendering and + reconciler modules import these types; **DO** keep `charm_state.py` free of + rendering/reconciler imports so it can't form an import cycle. - **`src/garm_client/` is generated** by `scripts/generate_client.sh` (openapi-generator) — **DON'T** hand-edit it. The script **patches the GARM swagger spec** before generating so the template `data` field (Go `[]byte`, base64 string on the wire) becomes `StrictStr` diff --git a/charms/garm/requirements.txt b/charms/garm/requirements.txt index e4a148ae..bfc20646 100644 --- a/charms/garm/requirements.txt +++ b/charms/garm/requirements.txt @@ -1,5 +1,6 @@ ops==3.8.0 paas-charm==1.11.3 +Jinja2==3.1.6 tomli-w==1.2.0 PyYAML==6.0.3 urllib3==2.7.0 diff --git a/charms/garm/src/charm.py b/charms/garm/src/charm.py index ac5aa352..bb1f9c64 100755 --- a/charms/garm/src/charm.py +++ b/charms/garm/src/charm.py @@ -24,6 +24,7 @@ DEBUG_SSH_INTEGRATION_NAME, GARM_CONFIGURATOR_RELATION_NAME, CharmState, + RunnerConfig, credential_name, ) from entity_reconciler import EntityReconciler @@ -826,6 +827,7 @@ def _build_desired_scalesets(self, template_id: int | None) -> list[ScalesetSpec data.get("pre_install_scripts", "") ), template_id=template_id, + runner_config=RunnerConfig.from_databag(data), ) ) return specs diff --git a/charms/garm/src/charm_state.py b/charms/garm/src/charm_state.py index d777e702..30f44c74 100644 --- a/charms/garm/src/charm_state.py +++ b/charms/garm/src/charm_state.py @@ -11,8 +11,11 @@ """ import dataclasses +import ipaddress import logging +import re import typing +from collections.abc import Mapping import ops @@ -20,6 +23,8 @@ logger = logging.getLogger(__name__) +_PORT_TOKEN_RE = re.compile(r"^\d{1,5}(-\d{1,5})?$") + DEBUG_SSH_INTEGRATION_NAME: typing.Final[str] = "debug-ssh" GARM_CONFIGURATOR_RELATION_NAME: typing.Final[str] = "garm-configurator" @@ -41,6 +46,123 @@ class SSHDebugInfo: ed25519_fingerprint: str +# Databag keys written by the garm-configurator charm — the single source of +# truth for the configurator→garm relation contract for runner options. +RUNNER_OPTION_DATABAG_KEYS: typing.Final = ( + "dockerhub_mirror", + "runner_http_proxy", + "aproxy_exclude_addresses", + "aproxy_redirect_ports", + "otel_collector_endpoint", + "pre_job_script", +) + + +@dataclasses.dataclass(frozen=True) +class RunnerConfig: + """Runner-behaviour options sourced from the garm-configurator relation. + + Every field is a plain string; an empty string means "unset". + :meth:`from_databag` sanitises the security-sensitive fields (ports, + addresses, env-file values) at parse time. + + Attributes: + dockerhub_mirror: Docker registry mirror URL, or "". + runner_http_proxy: Upstream HTTP proxy that aproxy forwards to, or "". + aproxy_exclude_addresses: Comma-separated addresses/CIDRs to bypass, or "". + aproxy_redirect_ports: Comma-separated ports / N-M ranges to redirect, or "". + otel_collector_endpoint: OTEL exporter endpoint, or "". + pre_job_script: Operator bash appended to the pre-job hook, or "". + """ + + dockerhub_mirror: str = "" + runner_http_proxy: str = "" + aproxy_exclude_addresses: str = "" + aproxy_redirect_ports: str = "" + otel_collector_endpoint: str = "" + pre_job_script: str = "" + + @classmethod + def from_databag(cls, data: Mapping[str, str]) -> "RunnerConfig": + """Build a config from a relation databag, ignoring missing keys. + + Args: + data: The relation unit databag (or any mapping). + + Returns: + A RunnerConfig with each field taken from its databag key, "" if absent, and the + security-sensitive fields sanitised (see class docstring). + """ + values = {key: (data.get(key) or "").strip() for key in RUNNER_OPTION_DATABAG_KEYS} + values["aproxy_redirect_ports"] = ",".join( + _valid_port_tokens(values["aproxy_redirect_ports"]) + ) + values["aproxy_exclude_addresses"] = ",".join( + _valid_ipv4_tokens(values["aproxy_exclude_addresses"]) + ) + values["otel_collector_endpoint"] = _strip_newlines(values["otel_collector_endpoint"]) + values["dockerhub_mirror"] = _strip_newlines(values["dockerhub_mirror"]) + return cls(**values) + + def has_config(self) -> bool: + """Whether any runner option is set (i.e. a custom template is needed). + + Returns: + True if at least one field is non-empty. + """ + return any(getattr(self, key) for key in RUNNER_OPTION_DATABAG_KEYS) + + +def _strip_newlines(value: str) -> str: + """Flatten a value so it is safe as a single-line env-file entry.""" + return value.replace("\r", "").replace("\n", "") + + +def _valid_port_tokens(spec: str) -> list[str]: + """Return only the in-range port / N-M range tokens from a comma list. + + A token is kept only if it is ``N`` or ``N-M`` with every port in 1..65535 + and ``N <= M`` — out-of-range or inverted tokens are dropped so they can't + break the rendered nft ruleset. + + Args: + spec: A comma-separated ports string (possibly empty or untrusted). + + Returns: + The subset of well-formed, in-range tokens. + """ + valid: list[str] = [] + for raw in spec.split(","): + token = raw.strip() + if not _PORT_TOKEN_RE.match(token): + continue + ports = [int(part) for part in token.split("-")] + if all(1 <= port <= 65535 for port in ports) and ports == sorted(ports): + valid.append(token) + return valid + + +def _valid_ipv4_tokens(spec: str) -> list[str]: + """Return only the valid IPv4 address/CIDR tokens from a comma list. + + Args: + spec: A comma-separated address string (possibly empty or untrusted). + + Returns: + The subset of tokens that parse as IPv4 networks. + """ + valid: list[str] = [] + for token in spec.split(","): + token = token.strip() + try: + network = ipaddress.ip_network(token, strict=False) + except ValueError: + continue + if network.version == 4: + valid.append(token) + return valid + + def credential_name(app_id: int, installation_id: int) -> str: """Return the GARM credential name for a GitHub App installation. diff --git a/charms/garm/src/garm_api.py b/charms/garm/src/garm_api.py index 49cbcf26..9370e802 100644 --- a/charms/garm/src/garm_api.py +++ b/charms/garm/src/garm_api.py @@ -246,8 +246,16 @@ def _api_client(self) -> ApiClient: header_value=f"Bearer {self._token}", ) - def list_templates(self) -> list[Template]: - """List all runner install templates known to GARM. + def list_templates( + self, + partial_name: str | None = None, + os_type: str | None = None, + ) -> list[Template]: + """List runner install templates known to GARM, optionally filtered. + + Args: + partial_name: Optional partial or full template name to filter by. + os_type: Optional OS type filter (e.g. ``"linux"``). Returns: List of Template objects. @@ -258,7 +266,14 @@ def list_templates(self) -> list[Template]: with self._api_client() as client: api = TemplatesApi(api_client=client) try: - return api.list_templates(_request_timeout=_REQUEST_TIMEOUT) or [] + return ( + api.list_templates( + partial_name=partial_name, + os_type=os_type, + _request_timeout=_REQUEST_TIMEOUT, + ) + or [] + ) except ApiException as exc: raise GarmApiError( f"GARM list_templates failed ({exc.status}): {exc.body}" @@ -292,19 +307,19 @@ def get_template(self, template_id: int) -> Template: def create_template( self, name: str, - description: str, - forge_type: str, - os_type: str, data: bytes, + description: str = "", + forge_type: str = "github", + os_type: str = "linux", ) -> Template: """Create a new runner install template. Args: name: Template name. + data: Raw shell script bytes for the template body. description: Human-readable description. forge_type: Source forge type, e.g. ``"github"``. os_type: OS type, e.g. ``"linux"``. - data: Raw shell script bytes for the template body. Returns: The created Template. diff --git a/charms/garm/src/runner_template.py b/charms/garm/src/runner_template.py new file mode 100644 index 00000000..6da1a531 --- /dev/null +++ b/charms/garm/src/runner_template.py @@ -0,0 +1,266 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Render runner-behaviour options into a GARM runner-install template. + +GARM stores the runner-install script as a server-side template that a +scaleset references by ``template_id``. The six operator-facing runner options +(docker mirror, proxy/aproxy, telemetry, pre-job hook) are delivered by copying +GARM's system ``github_linux`` template and injecting two blocks right after the +shebang — before the base template's ``set -e`` — so a best-effort step can never +abort the whole bootstrap: + + * a *pre-install* block that runs as root before the runner is installed + (aproxy, docker registry mirror, static host prep), and + * a *runner job hooks* block that writes the GitHub job-start hook and the + runner ``env`` file under ``/home/runner/actions-runner`` (GARM hardcodes the + ``runner`` user and that path). + +The reconciler creates/updates the per-scaleset template with the bytes returned +by :func:`build_template_data` and only does so when :meth:`RunnerConfig.has_config` +is true. + +Blocks are rendered from Jinja2 templates in ``src/templates/*.j2``, loaded via a +``FileSystemLoader`` rooted next to this module; ``src/`` is packed verbatim into +the rock, so the template files ship alongside the charm code. ``autoescape`` is +off throughout: the output is shell/YAML, not HTML. +""" + +import json +import pathlib +import shlex + +import jinja2 + +from charm_state import RunnerConfig + +# GARM hardcodes the runner username and actions-runner directory; the env file +# read by the runner service therefore lives at the path below. +RUNNER_USER = "runner" +RUNNER_HOME = "/home/runner/actions-runner" +PRE_JOB_HOOK_PATH = f"{RUNNER_HOME}/pre-job.sh" +RUNNER_ENV_PATH = f"{RUNNER_HOME}/env" + +_TEMPLATES_DIR = pathlib.Path(__file__).parent / "templates" +_JINJA_ENV = jinja2.Environment( + loader=jinja2.FileSystemLoader(str(_TEMPLATES_DIR)), + autoescape=False, + trim_blocks=True, + lstrip_blocks=True, + keep_trailing_newline=True, + undefined=jinja2.StrictUndefined, +) + +# OpenTelemetry collector config: hostmetrics/otlp/loki pipelines exporting to +# the configured endpoint. `$GITHUB_*`, `$RUNNER_NAME` and `$(uname -m)` are left +# as literal shell syntax — they sit inside the *inner*, unquoted `tee < bytes: + """Inject the runner-option blocks into a base runner-install template. + + The blocks are inserted immediately after the shebang line (before the base + template's ``set -e``), mirroring GARM's documented "prepend after the + shebang" approach. + + Args: + base: The system ``github_linux`` template bytes to copy from. + config: The runner options to render. + + Returns: + The new template bytes, with the pre-install and job-hook blocks injected. + """ + text = base.decode("utf-8") + injection = render_pre_install(config) + render_pre_job_hooks(config) + if "\n" in text: + shebang, rest = text.split("\n", 1) + return f"{shebang}\n{injection}{rest}".encode("utf-8") + return f"{text}\n{injection}".encode("utf-8") + + +def render_pre_install(config: RunnerConfig) -> str: + """Render the root pre-install block (runs before the runner is installed). + + Args: + config: The runner options to render. + + Returns: + A bash snippet, terminated by a newline. + """ + aproxy = _render_aproxy(config) if config.runner_http_proxy else "" + dockerhub = ( + _render_dockerhub_mirror(config.dockerhub_mirror) if config.dockerhub_mirror else "" + ) + return _PRE_INSTALL_TEMPLATE.render( + aproxy=aproxy, dockerhub=dockerhub, static_prep=_render_static_host_prep() + ) + + +def render_pre_job_hooks(config: RunnerConfig) -> str: + """Render the runner ``env`` file and GitHub job-start hook. + + The hook file sets up the OpenTelemetry collector, when configured, and runs + the operator's ``pre-job-script``. The docker mirror and OTEL endpoint are + exported via the runner ``env`` file, read once per job by the runner service. + + Args: + config: The runner options to render. + + Returns: + A bash snippet, terminated by a newline. + """ + otel_endpoint = config.otel_collector_endpoint + dockerhub_mirror = config.dockerhub_mirror + + hook_body = _render_pre_job_hook_body(config, otel_endpoint) + + env_entries = [] + if dockerhub_mirror: + env_entries.append(f"DOCKERHUB_MIRROR={dockerhub_mirror}") + env_entries.append(f"CONTAINER_REGISTRY_URL={dockerhub_mirror}") + env_entries.append(f"ACTIONS_RUNNER_HOOK_JOB_STARTED={PRE_JOB_HOOK_PATH}") + if otel_endpoint: + env_entries.append(f"ACTION_OTEL_EXPORTER_OTLP_ENDPOINT={otel_endpoint}") + + # Pick delimiters that don't collide with the (operator-controlled) content, + # so a pre-job-script containing the literal delimiter can't terminate the + # heredoc early. Deterministic for a given content, keeping the rendered + # template stable across reconciles. Computed in Python (two-phase render) + # since the delimiter depends on the already-rendered hook body. + prejob_delim = _heredoc_delimiter(hook_body, "GARM_CHARM_PREJOB") + env_delim = _heredoc_delimiter("\n".join(env_entries), "GARM_CHARM_ENV") + return _PRE_JOB_HOOKS_TEMPLATE.render( + runner_home=RUNNER_HOME, + hook_path=PRE_JOB_HOOK_PATH, + env_path=RUNNER_ENV_PATH, + runner_user=RUNNER_USER, + hook_body=hook_body, + env_entries=env_entries, + prejob_delim=prejob_delim, + env_delim=env_delim, + ) + + +def _render_pre_job_hook_body(config: RunnerConfig, otel_endpoint: str) -> str: + """Render the contents of the pre-job hook file (the GitHub job-start hook). + + Args: + config: The runner options to render. + otel_endpoint: The sanitised OTEL endpoint, or "" if unset. + + Returns: + The full contents of the hook script (no trailing newline). + """ + otel = _OTEL_COLLECTOR_SETUP_TEMPLATE.render(endpoint=otel_endpoint) if otel_endpoint else "" + custom_script = ( + _render_custom_pre_job_script(config.pre_job_script) if config.pre_job_script else "" + ) + return _HOOK_BODY_TEMPLATE.render(otel=otel, custom_script=custom_script) + + +def _heredoc_delimiter(content: str, base: str) -> str: + """Return a heredoc delimiter that does not appear as a line in *content*. + + Args: + content: The heredoc body the delimiter must not collide with. + base: The preferred delimiter; extended only if it collides. + + Returns: + ``base``, suffixed with underscores until no line of *content* matches it. + """ + lines = content.splitlines() + delimiter = base + while delimiter in lines: + delimiter += "_" + return delimiter + + +def _render_custom_pre_job_script(script: str) -> str: + """Render the custom pre-job script block. + + Writes the operator script to a temp file, runs it, and removes it — a + failure is logged but never aborts the job. + + Args: + script: The operator-provided pre-job-script content, inserted verbatim + (mirrors the template's ``| safe`` filter). + + Returns: + A bash snippet. + """ + delim = _heredoc_delimiter(script, "GARM_CHARM_CUSTOM_PREJOB") + return _CUSTOM_PRE_JOB_SCRIPT_TEMPLATE.render(delim=delim, script=script) + + +def _render_aproxy(config: RunnerConfig) -> str: + """Render aproxy install + nftables redirect rules. + + aproxy listens on ``:54969`` and an nftables DNAT ruleset (written to + ``/etc/nftables.conf``) redirects egress traffic to it, both for + locally-initiated connections (``output`` chain) and forwarded ones + (``prerouting`` chain). + + Args: + config: The runner options (uses proxy, exclude addresses, redirect ports). + + Returns: + A bash snippet configuring aproxy as a transparent forward proxy. + """ + # Values are already validated at parse time (RunnerConfig.from_databag); only + # shell-safe embedding (quoting, the empty-string split guard below) happens here. + ports = [t for t in config.aproxy_redirect_ports.split(",") if t] or ["80", "443"] + nft_ports = ", ".join(ports) + excludes = [t for t in config.aproxy_exclude_addresses.split(",") if t] + exclude_elements = ", ".join(["127.0.0.0/8", *excludes]) + # `\$default-ipv4` stays escaped so the literal two characters `$default-ipv4` + # land in the file, which nft itself resolves against the `define` above when + # it loads the file. + dnat_rule = ( + f"ip daddr != @exclude tcp dport {{ {nft_ports} }} counter dnat to \\$default-ipv4:54969" + ) + + return _APROXY_TEMPLATE.render( + proxy=shlex.quote(config.runner_http_proxy), + exclude_elements=exclude_elements, + dnat_rule=dnat_rule, + ) + + +def _render_dockerhub_mirror(mirror: str) -> str: + """Render Docker daemon config pointing at the registry mirror. + + Args: + mirror: The registry mirror URL. + + Returns: + A bash snippet writing /etc/docker/daemon.json and restarting docker. + """ + daemon_json = json.dumps({"registry-mirrors": [mirror]}) + return _DOCKERHUB_MIRROR_TEMPLATE.render(daemon_json=daemon_json) + + +def _render_static_host_prep() -> str: + """Render the always-on host-preparation steps. + + Adds the runner account to the ``lxd`` and ``adm`` groups. + + Returns: + A bash snippet. Each command is guarded by ``|| true`` so it is a no-op + if the runner account doesn't exist yet. + """ + return _STATIC_HOST_PREP_TEMPLATE.render(runner_user=RUNNER_USER) diff --git a/charms/garm/src/scaleset_reconciler.py b/charms/garm/src/scaleset_reconciler.py index 0d246640..58734e7d 100644 --- a/charms/garm/src/scaleset_reconciler.py +++ b/charms/garm/src/scaleset_reconciler.py @@ -4,16 +4,24 @@ """Scaleset reconciler: diffs desired vs observed GARM scalesets and applies changes.""" +import base64 import logging from dataclasses import dataclass, field +from charm_state import RunnerConfig from garm_api import GarmApiError, GarmAuthenticatedClient from garm_client.models.create_scale_set_params import CreateScaleSetParams from garm_client.models.scale_set import ScaleSet +from garm_client.models.template import Template from garm_client.models.update_scale_set_params import UpdateScaleSetParams +from runner_template import build_template_data logger = logging.getLogger(__name__) +# GARM seeds a non-editable system template per forge/OS; we copy this one to +# build per-scaleset runner templates carrying the operator's runner options. +SYSTEM_TEMPLATE_NAME = "github_linux" + @dataclass class ScalesetSpec: @@ -33,6 +41,7 @@ class ScalesetSpec: runner_group: str = "Default" pre_install_scripts: dict[str, str] = field(default_factory=dict) template_id: int | None = None + runner_config: RunnerConfig = field(default_factory=RunnerConfig) class ScalesetReconciler: @@ -49,52 +58,111 @@ def __init__(self, client: GarmAuthenticatedClient) -> None: def reconcile(self, desired: list[ScalesetSpec]) -> None: """Sync GARM scalesets to match *desired*. - Performs the minimum set of CREATE / UPDATE / DELETE operations. - If a referenced provider is missing or the target entity (org/repo) - is not registered in GARM, that spec is skipped silently (deferred - creation) — no error state is set. + Performs the minimum set of CREATE / UPDATE / DELETE operations, and + maintains a per-scaleset runner-install template carrying the runner + options. If a referenced provider is missing or the target entity + (org/repo) is not registered in GARM, that spec is skipped silently + (deferred creation) — no error state is set. Args: desired: The full desired set of scalesets. """ providers = {provider.name for provider in self._client.list_providers()} - observed = {scaleset.name: scaleset for scaleset in self._client.list_scalesets()} + observed: dict[str, ScaleSet] = {} + for scaleset in self._client.list_scalesets(): + if not scaleset.name: + logger.warning("Skipping observed scaleset with missing name (id=%s)", scaleset.id) + continue + observed[scaleset.name] = scaleset + templates = self._load_templates(desired, observed) all_desired_names: set[str] = {spec.name for spec in desired} for spec in desired: - try: - create_params = self._to_create_params(spec) - except Exception as exc: - logger.warning("Skipping scaleset %s: spec validation failed: %s", spec.name, exc) - continue - - if spec.provider_name not in providers: - logger.warning( - "Skipping scaleset %s: provider %s not registered yet", - spec.name, - spec.provider_name, - ) - continue - - entity_id = self._resolve_entity_id(spec) - if entity_id is None: - logger.warning( - "Skipping scaleset %s: %s '%s' not registered in GARM yet", - spec.name, - spec.entity_type, - spec.entity_name, - ) - continue - - if spec.name in observed: - self._maybe_update(observed[spec.name], spec) - else: - self._create(spec, entity_id, create_params) + self._reconcile_one(spec, providers, observed, templates) for name, scaleset in observed.items(): if name not in all_desired_names: self._delete_orphaned(scaleset) + self._delete_custom_template(name, templates) + + def _load_templates( + self, desired: list[ScalesetSpec], observed: dict[str, ScaleSet] + ) -> dict[str, Template]: + """Fetch observed templates keyed by name, only when a reconcile pass needs them. + + Args: + desired: The full desired set of scalesets. + observed: Observed scalesets keyed by name. + + Returns: + Observed templates keyed by name, or an empty dict when none are needed. + """ + # Templates are only needed when a spec carries runner options or an + # existing scaleset already references a custom template (to update or + # detach it); skip the API call entirely otherwise. + templates: dict[str, Template] = {} + if any(spec.runner_config.has_config() for spec in desired) or any( + scaleset.template_id for scaleset in observed.values() + ): + templates = { + (template.name or ""): template + for template in self._client.list_templates() + if template.name + } + return templates + + def _reconcile_one( + self, + spec: ScalesetSpec, + providers: set[str | None], + observed: dict[str, ScaleSet], + templates: dict[str, Template], + ) -> None: + """Reconcile a single desired scaleset: validate, create or update, and sync its template. + + Args: + spec: The desired scaleset. + providers: Names of providers currently registered in GARM. + observed: Observed scalesets keyed by name. + templates: Observed templates keyed by name. + """ + try: + create_params = self._to_create_params(spec) + except Exception as exc: + logger.warning("Skipping scaleset %s: spec validation failed: %s", spec.name, exc) + return + + if spec.provider_name not in providers: + logger.warning( + "Skipping scaleset %s: provider %s not registered yet", + spec.name, + spec.provider_name, + ) + return + + entity_id = self._resolve_entity_id(spec) + if entity_id is None: + logger.warning( + "Skipping scaleset %s: %s '%s' not registered in GARM yet", + spec.name, + spec.entity_type, + spec.entity_name, + ) + return + + template_id = self._ensure_template(spec, templates) + + if spec.name in observed: + self._maybe_update(observed[spec.name], spec, template_id) + else: + self._create(spec, entity_id, create_params, template_id) + + if not spec.runner_config.has_config(): + # Runner options were cleared (or the system template is + # unavailable): the scaleset has been reverted to the default + # template above, so drop any now-unreferenced custom template. + self._delete_custom_template(spec.name, templates) def _delete_orphaned(self, scaleset: ScaleSet) -> None: """Disable then delete a scaleset that is no longer in the desired set.""" @@ -132,6 +200,138 @@ def _resolve_entity_id(self, spec: ScalesetSpec) -> str | None: logger.warning("Unknown entity_type %r for scaleset %s", spec.entity_type, spec.name) return None + def _ensure_template(self, spec: ScalesetSpec, templates: dict[str, Template]) -> int: + """Ensure the scaleset's runner template reflects its runner options. + + Copies the system ``github_linux`` template, injects the runner options, + and creates or updates the per-scaleset template. The template content is + refreshed in place (same id) on every reconcile, so an option change is + applied without touching the scaleset itself. + + Args: + spec: The desired scaleset. + templates: Observed templates keyed by name. + + Returns: + The custom template id to reference from the scaleset, or ``0`` to use + GARM's default template (no runner options set, or the system template + is unavailable and no custom template already exists). Returning ``0`` + for a scaleset that previously had a custom template detaches it. + """ + custom_name = f"{SYSTEM_TEMPLATE_NAME}-{spec.name}" + existing = templates.get(custom_name) + + if not spec.runner_config.has_config(): + return spec.template_id or 0 + + base = self._template_by_id(templates, spec.template_id) or templates.get( + SYSTEM_TEMPLATE_NAME + ) + if base is None: + # The system template is not listed (transient/compat). Don't destroy + # an existing custom template over it — keep the last-rendered one + # rather than detaching and losing the runner config; only fall back + # to the default when there is nothing to keep. + if existing is not None: + logger.warning( + "System template %s not found; keeping existing custom template for %s", + SYSTEM_TEMPLATE_NAME, + spec.name, + ) + return existing.id or 0 + logger.warning( + "System template %s not found; scaleset %s will use the default template", + SYSTEM_TEMPLATE_NAME, + spec.name, + ) + return 0 + + new_data = build_template_data(self._template_bytes(base), spec.runner_config) + if existing is not None: + if existing.id is None: + logger.warning( + "Runner template %s has no id; scaleset %s will use the default template", + custom_name, + spec.name, + ) + return 0 + if self._template_bytes(existing) != new_data: + logger.info("Updating runner template %s", custom_name) + self._client.update_template(existing.id, data=new_data) + return existing.id + + logger.info("Creating runner template %s", custom_name) + created = self._client.create_template( + name=custom_name, + data=new_data, + description=f"Runner template for scaleset {spec.name}", + ) + return created.id or 0 + + def _template_bytes(self, template: Template) -> bytes: + """Return a template's raw bytes, fetching the full object if needed. + + Args: + template: A template, possibly without its ``data`` field populated + (the list endpoint omits the body). + + Returns: + The decoded template bytes. + """ + # data is Any at runtime: the list endpoint omits it, and it can come back + # as a base64 str or (in tests) the raw list-of-ints byte representation. + data = getattr(template, "data", None) + if not data: + if template.id is None: + return b"" + fetched = self._client.get_template(template.id) + data = getattr(fetched, "data", None) + # Cache it back so repeated lookups don't re-fetch, but only when + # the generated model can accept the assigned value. + if isinstance(data, str): + setattr(template, "data", data) + if not data: + return b"" + if isinstance(data, bytes): + return data + if isinstance(data, str): + return base64.b64decode(data) + return bytes(data) + + def _template_by_id( + self, templates: dict[str, Template], template_id: int | None + ) -> Template | None: + """Return a listed template by id, or None when absent.""" + if template_id is None: + return None + return next( + (template for template in templates.values() if (template.id or 0) == template_id), + None, + ) + + def _delete_custom_template(self, scaleset_name: str, templates: dict[str, Template]) -> None: + """Delete a scaleset's custom runner template if one exists. + + Args: + scaleset_name: The name of the scaleset being removed. + templates: Observed templates keyed by name. + """ + custom_name = f"{SYSTEM_TEMPLATE_NAME}-{scaleset_name}" + custom = templates.get(custom_name) + if custom is not None: + if custom.id is None: + logger.warning("Skipping delete for runner template %s: missing id", custom_name) + return + logger.info("Deleting orphaned runner template %s", custom.name or custom_name) + try: + self._client.delete_template(custom.id) + except GarmApiError as exc: + logger.warning( + "Could not delete runner template %s (will retry on next reconcile): %s", + custom.name or custom_name, + exc, + ) + @staticmethod def _to_create_params(spec: ScalesetSpec) -> CreateScaleSetParams: """Build and validate CreateScaleSetParams from a ScalesetSpec. @@ -158,6 +358,7 @@ def _to_create_params(spec: ScalesetSpec) -> CreateScaleSetParams: "os_type": spec.os_type, "min_idle_runners": spec.min_idle_runners, "max_runners": spec.max_runners, + "enabled": True, "labels": sorted(spec.labels), "github_runner_group": spec.runner_group or None, "extra_specs": extra_specs or None, @@ -165,14 +366,22 @@ def _to_create_params(spec: ScalesetSpec) -> CreateScaleSetParams: } ) - def _create(self, spec: ScalesetSpec, entity_id: str, params: CreateScaleSetParams) -> None: + def _create( + self, + spec: ScalesetSpec, + entity_id: str, + params: CreateScaleSetParams, + template_id: int, + ) -> None: + if template_id: + params.template_id = template_id logger.info("Creating scaleset %s under %s %s", spec.name, spec.entity_type, entity_id) if spec.entity_type == "organization": self._client.create_org_scaleset(entity_id, params) else: self._client.create_repo_scaleset(entity_id, params) - def _maybe_update(self, observed, spec: ScalesetSpec) -> None: + def _maybe_update(self, observed: ScaleSet, spec: ScalesetSpec, template_id: int) -> None: observed_labels = sorted(t.name for t in (observed.tags or []) if t.name) if observed_labels != sorted(spec.labels): # UpdateScaleSetParams has no labels field; label changes require @@ -187,7 +396,11 @@ def _maybe_update(self, observed, spec: ScalesetSpec) -> None: sorted(spec.labels), ) - if not self._needs_update(observed, spec): + observed_template_id = observed.template_id or 0 + + # _needs_update already covers the template id (its last clause), so an + # id change alone forces an update here. + if not self._needs_update(observed, spec, template_id): logger.debug("Scaleset %s is up to date", spec.name) return @@ -200,10 +413,16 @@ def _maybe_update(self, observed, spec: ScalesetSpec) -> None: flavor=spec.flavor, min_idle_runners=spec.min_idle_runners, max_runners=spec.max_runners, + enabled=True, runner_group=spec.runner_group or None, extra_specs=extra_specs or None, template_id=spec.template_id, ) + # Send template_id when the scaleset has, or had, a custom template — a 0 + # value detaches it (reverts to the default); omit it otherwise so an + # unrelated update never spuriously sets the field. + if template_id or observed_template_id: + params.template_id = template_id logger.info("Updating scaleset %s (id=%s)", spec.name, observed.id) if observed.id is None: logger.warning("Scaleset %s has no id; skipping update", spec.name) @@ -211,14 +430,15 @@ def _maybe_update(self, observed, spec: ScalesetSpec) -> None: self._client.update_scaleset(observed.id, params) @staticmethod - def _needs_update(observed, spec: ScalesetSpec) -> bool: + def _needs_update(observed: ScaleSet, spec: ScalesetSpec, template_id: int) -> bool: observed_scripts = (observed.extra_specs or {}).get("pre_install_scripts", {}) return ( observed.image != spec.image or observed.flavor != spec.flavor or observed.max_runners != spec.max_runners or observed.min_idle_runners != spec.min_idle_runners + or observed.enabled is not True or observed.github_runner_group != (spec.runner_group or None) or observed_scripts != spec.pre_install_scripts - or (spec.template_id is not None and observed.template_id != spec.template_id) + or (observed.template_id or 0) != template_id ) diff --git a/charms/garm/src/templates/aproxy.j2 b/charms/garm/src/templates/aproxy.j2 new file mode 100644 index 00000000..f1a79e20 --- /dev/null +++ b/charms/garm/src/templates/aproxy.j2 @@ -0,0 +1,24 @@ +# Transparently forward runner egress through the configured HTTP proxy. +snap install aproxy --edge +snap set aproxy proxy={{ proxy }} listen=:54969 +nft delete table ip aproxy 2>/dev/null || true +cat << EOF > /etc/nftables.conf +define default-ipv4 = $(ip route get $(ip route show 0.0.0.0/0 | grep -oP 'via \K\S+') | grep -oP 'src \K\S+') +table ip aproxy { + set exclude { + type ipv4_addr; + flags interval; auto-merge; + elements = { {{ exclude_elements }} } + } + chain prerouting { + type nat hook prerouting priority dstnat; policy accept; + {{ dnat_rule }} + } + chain output { + type nat hook output priority -100; policy accept; + {{ dnat_rule }} + } +} +EOF +systemctl enable nftables.service +nft -f /etc/nftables.conf \ No newline at end of file diff --git a/charms/garm/src/templates/custom_pre_job_script.j2 b/charms/garm/src/templates/custom_pre_job_script.j2 new file mode 100644 index 00000000..c9a182f5 --- /dev/null +++ b/charms/garm/src/templates/custom_pre_job_script.j2 @@ -0,0 +1,7 @@ +cat > /tmp/custom_pre_job_script <<'{{ delim }}' +{{ script }} +{{ delim }} +chmod +x /tmp/custom_pre_job_script +logger -s "Running custom pre-job script" +/tmp/custom_pre_job_script || logger -s "Custom pre-job script failed, continuing with the job" +rm /tmp/custom_pre_job_script || logger -s "Failed to remove custom pre-job script" \ No newline at end of file diff --git a/charms/garm/src/templates/dockerhub_mirror.j2 b/charms/garm/src/templates/dockerhub_mirror.j2 new file mode 100644 index 00000000..6d31f932 --- /dev/null +++ b/charms/garm/src/templates/dockerhub_mirror.j2 @@ -0,0 +1,7 @@ +# Point Docker at the configured registry mirror. +mkdir -p /etc/docker +cat > /etc/docker/daemon.json <<'GARM_CHARM_DOCKER' +{{ daemon_json }} +GARM_CHARM_DOCKER +systemctl daemon-reload +systemctl restart docker \ No newline at end of file diff --git a/charms/garm/src/templates/hook_body.j2 b/charms/garm/src/templates/hook_body.j2 new file mode 100644 index 00000000..6f53370c --- /dev/null +++ b/charms/garm/src/templates/hook_body.j2 @@ -0,0 +1,12 @@ +#!/usr/bin/env bash +set +e +{%- if otel %} + + +{{ otel }} +{%- endif %} +{%- if custom_script %} + + +{{ custom_script }} +{%- endif %} \ No newline at end of file diff --git a/charms/garm/src/templates/otel_collector_setup.j2 b/charms/garm/src/templates/otel_collector_setup.j2 new file mode 100644 index 00000000..6c888254 --- /dev/null +++ b/charms/garm/src/templates/otel_collector_setup.j2 @@ -0,0 +1,155 @@ +/usr/bin/logger -s "OpenTelemetry collector is enabled." +/usr/bin/logger -s "Additional OpenTelemetry collector configuration can be added." +/usr/bin/logger -s "The exporter endpoint is at the environment variable ACTION_OTEL_EXPORTER_OTLP_ENDPOINT." +/usr/bin/sudo /usr/bin/mkdir -p /etc/otelcol/config.d +/usr/bin/sudo /usr/bin/touch /etc/otelcol/config.d/github.yaml +/usr/bin/sudo /usr/bin/tee /etc/otelcol/config.d/github.yaml < + set(resource.attributes["loki.resource.labels"], + Concat([resource.attributes["loki.resource.labels"], + ", service.name, github.repository, github.runner, github.workflow, github.job, github.run.id, github.run.attempt, host.arch"], "")) + where resource.attributes["loki.resource.labels"] != nil + - > + set(resource.attributes["loki.resource.labels"], + "service.name, github.repository, github.runner, github.workflow, github.job, github.run.id, github.run.attempt, host.arch") + where resource.attributes["loki.resource.labels"] == nil + batch: +exporters: + otlp/self_hosted_runner: + endpoint: {{ endpoint }} + tls: + insecure: true +service: + pipelines: + metrics: + receivers: [hostmetrics] + processors: [attributes/self_hosted_runner_github_labels, batch] + exporters: [otlp/self_hosted_runner] + metrics/otlp: + receivers: [otlp] + processors: [attributes/self_hosted_runner_github_labels, batch] + exporters: [otlp/self_hosted_runner] + logs/otlp: + receivers: [otlp] + processors: + - resource/self_hosted_runner_github_labels + - transform/self_hosted_runner_loki_labels + - batch + exporters: [otlp/self_hosted_runner] + logs/loki: + receivers: [loki] + processors: + - resource/self_hosted_runner_github_labels + - transform/self_hosted_runner_loki_labels + - batch + exporters: [otlp/self_hosted_runner] + logs/aproxy: + receivers: [filelog/aproxy] + processors: + - resource/self_hosted_runner_github_labels + - resource/self_hosted_runner_github_aproxy_labels + - transform/self_hosted_runner_loki_labels + - batch + exporters: [otlp/self_hosted_runner] + traces: + receivers: [otlp] + processors: [resource/self_hosted_runner_github_labels, batch] + exporters: [otlp/self_hosted_runner] +EOF + +/usr/bin/sudo /usr/bin/snap enable opentelemetry-collector +/usr/bin/sudo /usr/bin/snap start opentelemetry-collector \ No newline at end of file diff --git a/charms/garm/src/templates/pre_install.j2 b/charms/garm/src/templates/pre_install.j2 new file mode 100644 index 00000000..c2ed5f62 --- /dev/null +++ b/charms/garm/src/templates/pre_install.j2 @@ -0,0 +1,10 @@ + +# ===== charm-injected pre-install setup (runs as root) ===== +{% if aproxy %} +{{ aproxy }} +{% endif %} +{% if dockerhub %} +{{ dockerhub }} +{% endif %} +{{ static_prep }} +# ===== end charm-injected pre-install setup ===== diff --git a/charms/garm/src/templates/pre_job_hooks.j2 b/charms/garm/src/templates/pre_job_hooks.j2 new file mode 100644 index 00000000..a325c956 --- /dev/null +++ b/charms/garm/src/templates/pre_job_hooks.j2 @@ -0,0 +1,14 @@ + +# ===== charm-injected runner job hooks ===== +mkdir -p {{ runner_home }} +cat > {{ hook_path }} <<'{{ prejob_delim }}' +{{ hook_body }} +{{ prejob_delim }} +chmod 0755 {{ hook_path }} +cat >> {{ env_path }} <<'{{ env_delim }}' +{% for entry in env_entries %} +{{ entry }} +{% endfor %} +{{ env_delim }} +chown -R {{ runner_user }}:{{ runner_user }} {{ runner_home }} 2>/dev/null || true +# ===== end charm-injected runner job hooks ===== diff --git a/charms/garm/src/templates/static_host_prep.j2 b/charms/garm/src/templates/static_host_prep.j2 new file mode 100644 index 00000000..545923e1 --- /dev/null +++ b/charms/garm/src/templates/static_host_prep.j2 @@ -0,0 +1,3 @@ +# Static runner host preparation. +adduser {{ runner_user }} lxd || true +adduser {{ runner_user }} adm || true \ No newline at end of file diff --git a/charms/garm/tests/unit/test_charm_state.py b/charms/garm/tests/unit/test_charm_state.py index 5e2e60af..7b3ccfeb 100644 --- a/charms/garm/tests/unit/test_charm_state.py +++ b/charms/garm/tests/unit/test_charm_state.py @@ -7,7 +7,7 @@ import pytest -from charm_state import CharmState, SSHDebugInfo, _get_ssh_debug_connections +from charm_state import CharmState, RunnerConfig, SSHDebugInfo, _get_ssh_debug_connections _COMPLETE = { "host": "10.0.0.1", @@ -257,3 +257,72 @@ def test_desired_entities_skip_incomplete_unit(unit_data): assert: No entity is built. """ assert _entities([unit_data]) == [] + + +def test_runner_config_from_databag_and_has_config(): + """ + arrange: Empty and populated runner-config databags. + act: Build RunnerConfig instances from the databags and inspect has_config. + assert: Known keys are mapped, unknown keys are ignored, and has_config reflects content. + """ + empty = RunnerConfig.from_databag({}) + assert empty == RunnerConfig() + assert not empty.has_config() + + populated = RunnerConfig.from_databag( + {"dockerhub_mirror": " https://m.test ", "irrelevant": "x"} + ) + assert populated.dockerhub_mirror == "https://m.test" + assert populated.has_config() + + +def test_runner_config_from_databag_drops_malformed_port_and_address_tokens(): + """ + arrange: A databag with well-formed, malformed, out-of-range and IPv6 tokens. + act: Build a RunnerConfig from the databag. + assert: Only the well-formed, in-range IPv4 tokens survive. + """ + config = RunnerConfig.from_databag( + { + "aproxy_redirect_ports": "80,not-a-port,8000-9000,99 rm,99999,443-80", + "aproxy_exclude_addresses": "10.0.0.0/8,evil;,2001:db8::1", + } + ) + + assert config.aproxy_redirect_ports == "80,8000-9000" + assert config.aproxy_exclude_addresses == "10.0.0.0/8" + + +def test_runner_config_from_databag_strips_newlines_from_otel_endpoint(): + """ + arrange: A databag whose OTEL endpoint contains a newline and extra assignment text. + act: Build a RunnerConfig from the databag. + assert: The endpoint is flattened into a single line, preventing env-file line injection. + """ + config = RunnerConfig.from_databag( + {"otel_collector_endpoint": "http://o.test:4318\nMALICIOUS=1"} + ) + + assert config.otel_collector_endpoint == "http://o.test:4318MALICIOUS=1" + + +def test_runner_config_from_databag_strips_newlines_from_dockerhub_mirror(): + """ + arrange: A databag whose Docker mirror URL contains a newline and extra assignment text. + act: Build a RunnerConfig from the databag. + assert: The mirror value is flattened, preventing env-file line injection. + """ + config = RunnerConfig.from_databag({"dockerhub_mirror": "https://m.test\nMALICIOUS=1"}) + + assert config.dockerhub_mirror == "https://m.testMALICIOUS=1" + + +def test_runner_config_from_databag_preserves_multiline_pre_job_script(): + """ + arrange: A databag whose pre_job_script is legitimately multi-line. + act: Build a RunnerConfig from the databag. + assert: Newlines in pre_job_script are preserved (unlike the single-line env fields). + """ + config = RunnerConfig.from_databag({"pre_job_script": "echo one\necho two"}) + + assert config.pre_job_script == "echo one\necho two" diff --git a/charms/garm/tests/unit/test_runner_template.py b/charms/garm/tests/unit/test_runner_template.py new file mode 100644 index 00000000..feaddc8a --- /dev/null +++ b/charms/garm/tests/unit/test_runner_template.py @@ -0,0 +1,191 @@ +# Copyright 2026 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Unit tests for the runner-install template rendering.""" + +import pytest + +from charm_state import RunnerConfig +from runner_template import RUNNER_ENV_PATH, build_template_data + +SAMPLE_BASE = b"#!/bin/bash\nset -e\necho original-bootstrap\n" + + +def _full_config() -> RunnerConfig: + return RunnerConfig( + dockerhub_mirror="https://mirror.example.com", + runner_http_proxy="http://proxy.example.com:3128", + aproxy_exclude_addresses="10.0.0.0/8,192.168.1.5", + aproxy_redirect_ports="80,443,8000-9000", + otel_collector_endpoint="http://otel.example.com:4318", + pre_job_script="echo hello-from-operator", + ) + + +def test_build_template_data_injects_all_options(): + """ + arrange: A bootstrap script and a runner config with every optional field set. + act: Build the runner template data. + assert: The rendered template preserves the bootstrap and injects every block. + """ + result = build_template_data(SAMPLE_BASE, _full_config()).decode() + lines = result.splitlines() + + assert lines[0] == "#!/bin/bash" + assert "echo original-bootstrap" in result + # Injection sits between the shebang and the base body's `set -e`. + assert result.index("charm-injected") < result.index("set -e") + + assert "mirror.example.com" in result + assert "registry-mirrors" in result + assert "proxy.example.com:3128" in result + assert "snap install aproxy" in result + assert "10.0.0.0/8" in result and "192.168.1.5" in result + assert "8000-9000" in result + assert "ACTION_OTEL_EXPORTER_OTLP_ENDPOINT=http://otel.example.com:4318" in result + assert "echo hello-from-operator" in result + assert "ACTIONS_RUNNER_HOOK_JOB_STARTED=" in result + assert RUNNER_ENV_PATH in result + + # aproxy: correct listen port, nftables file (not a piped `nft -f -`), and + # both the prerouting and output DNAT chains. + assert "listen=:54969" in result + assert "default-ipv4" in result + assert "/etc/nftables.conf" in result + assert "table ip aproxy\nflush table ip aproxy" not in result + assert "nft delete table ip aproxy 2>/dev/null || true" in result + assert "prerouting" in result + assert "127.0.0.0/8" in result + assert "counter dnat to \\$default-ipv4:54969" in result + + # docker mirror: both env vars and a full daemon-reload + restart. + assert "DOCKERHUB_MIRROR=https://mirror.example.com" in result + assert "CONTAINER_REGISTRY_URL=https://mirror.example.com" in result + assert "systemctl daemon-reload" in result + + # otel collector config is written to disk and enabled. + assert "/etc/otelcol/config.d/github.yaml" in result + assert "snap enable opentelemetry-collector" in result + + # custom pre-job script: temp-file / run / cleanup pattern. + assert "cat > /tmp/custom_pre_job_script <<" in result + assert "chmod +x /tmp/custom_pre_job_script" in result + assert "rm /tmp/custom_pre_job_script" in result + + +def test_build_template_data_empty_config_omits_optional_blocks(): + """ + arrange: A bootstrap script and an empty runner config. + act: Build the runner template data. + assert: The static setup remains and all optional blocks are omitted. + """ + result = build_template_data(SAMPLE_BASE, RunnerConfig()).decode() + + assert "echo original-bootstrap" in result + assert "ACTIONS_RUNNER_HOOK_JOB_STARTED=" in result + assert "adduser runner lxd || true" in result + assert "adduser runner adm || true" in result + + assert "snap install aproxy" not in result + assert "registry-mirrors" not in result + assert "ACTION_OTEL_EXPORTER_OTLP_ENDPOINT" not in result + + +@pytest.mark.parametrize( + "config, present, absent", + [ + pytest.param( + RunnerConfig(dockerhub_mirror="https://m.test"), + "https://m.test", + "snap install aproxy", + id="dockerhub-mirror-only", + ), + pytest.param( + RunnerConfig(runner_http_proxy="http://p.test:8080"), + "http://p.test:8080", + "registry-mirrors", + id="proxy-only", + ), + pytest.param( + RunnerConfig(otel_collector_endpoint="http://o.test:4318"), + "ACTION_OTEL_EXPORTER_OTLP_ENDPOINT=http://o.test:4318", + "snap install aproxy", + id="otel-only", + ), + pytest.param( + RunnerConfig(pre_job_script="run-my-thing --flag"), + "run-my-thing --flag", + "registry-mirrors", + id="pre-job-script-only", + ), + ], +) +def test_build_template_data_per_option(config, present, absent): + """ + arrange: A bootstrap script and a runner config with exactly one optional field set. + act: Build the runner template data. + assert: The selected block is present and an unrelated optional block is absent. + """ + result = build_template_data(SAMPLE_BASE, config).decode() + assert present in result + assert absent not in result + + +def test_aproxy_render_uses_configured_ports_and_excludes(): + """ + arrange: A runner config with a proxy plus well-formed redirect ports and excludes. + act: Build the runner template data. + assert: The nft ruleset embeds exactly those ports and addresses. + """ + config = RunnerConfig( + runner_http_proxy="http://p.test:3128", + aproxy_redirect_ports="80,8000-9000", + aproxy_exclude_addresses="10.0.0.0/8", + ) + result = build_template_data(SAMPLE_BASE, config).decode() + + assert "tcp dport { 80, 8000-9000 }" in result + assert "10.0.0.0/8" in result + + +def test_heredoc_delimiter_avoids_collision_with_pre_job_script(): + """ + arrange: A pre-job script containing the default heredoc delimiter token. + act: Build the runner template data. + assert: The renderer chooses a non-colliding heredoc delimiter. + """ + config = RunnerConfig(pre_job_script="echo hi\nGARM_CHARM_PREJOB\necho bye") + result = build_template_data(SAMPLE_BASE, config).decode() + + assert "<<'GARM_CHARM_PREJOB_'" in result + assert "echo bye" in result + + +def test_build_template_data_from_untrusted_databag_drops_malformed_values(): + """ + arrange: A raw relation databag with malformed ports/addresses and newline-bearing values. + act: Parse it via RunnerConfig.from_databag and build the runner template data. + assert: The malformed/injected content never reaches the rendered script. + """ + config = RunnerConfig.from_databag( + { + "runner_http_proxy": "http://p.test:3128", + "aproxy_redirect_ports": "80,not-a-port,8000-9000,99999,443-80", + "aproxy_exclude_addresses": "10.0.0.0/8,evil;,2001:db8::1", + "otel_collector_endpoint": "http://o.test:4318\nMALICIOUS=1", + "dockerhub_mirror": "https://m.test\nMALICIOUS=1", + } + ) + result = build_template_data(SAMPLE_BASE, config).decode() + + assert "tcp dport { 80, 8000-9000 }" in result + assert "not-a-port" not in result + assert "99999" not in result # out of range + assert "443-80" not in result # inverted range + assert "10.0.0.0/8" in result + assert "evil" not in result + assert "2001:db8::1" not in result # IPv6 dropped: the nft table is IPv4-only + assert "ACTION_OTEL_EXPORTER_OTLP_ENDPOINT=http://o.test:4318MALICIOUS=1" in result + assert "DOCKERHUB_MIRROR=https://m.testMALICIOUS=1" in result + assert "CONTAINER_REGISTRY_URL=https://m.testMALICIOUS=1" in result + assert "\nMALICIOUS=1" not in result diff --git a/charms/garm/tests/unit/test_scaleset_reconciler.py b/charms/garm/tests/unit/test_scaleset_reconciler.py index 7cdc755e..34614830 100644 --- a/charms/garm/tests/unit/test_scaleset_reconciler.py +++ b/charms/garm/tests/unit/test_scaleset_reconciler.py @@ -3,8 +3,11 @@ """Unit tests for the scaleset reconciler.""" +import base64 + import pytest +from garm_client.models.template import Template from scaleset_reconciler import ScalesetReconciler, ScalesetSpec @@ -31,6 +34,7 @@ def __init__( extra_specs=None, tags=None, template_id=None, + enabled=True, ): self.name = name self.id = sid @@ -42,6 +46,7 @@ def __init__( self.extra_specs = extra_specs or {} self.tags = [_FakeTag(t) for t in (tags or [])] self.template_id = template_id + self.enabled = enabled class FakeGarmClient: @@ -65,6 +70,7 @@ def __init__(self, providers=None, scalesets=None, org_id="org-uuid", repo_id=No extra_specs=ss.get("extra_specs", {}), tags=ss.get("tags", []), template_id=ss.get("template_id", None), + enabled=ss.get("enabled", True), ) for ss in (scalesets or []) ] @@ -98,6 +104,23 @@ def update_scaleset(self, scaleset_id, params): def delete_scaleset(self, scaleset_id): self.deleted.append(scaleset_id) + # Template stubs: return empty results so the reconciler's template path + # is a no-op when no runner config is set. + def list_templates(self, partial_name=None, os_type=None): + return [] + + def get_template(self, template_id): + return None + + def create_template(self, name, data, description="", os_type="linux") -> object: + return None + + def update_template(self, template_id, data): + return None + + def delete_template(self, template_id): + pass + def _spec( name="my-scaleset", @@ -113,7 +136,10 @@ def _spec( runner_group="", pre_install_scripts=None, template_id=None, + runner_config=None, ): + from charm_state import RunnerConfig + return ScalesetSpec( name=name, provider_name=provider_name, @@ -128,6 +154,7 @@ def _spec( runner_group=runner_group, pre_install_scripts=pre_install_scripts or {}, template_id=template_id, + runner_config=runner_config or RunnerConfig(), ) @@ -164,6 +191,7 @@ def test_create_scaleset(entity_type, entity_name, create_key, expected_entity_i assert params.name == "my-scaleset" assert params.image == "ubuntu-22.04" assert params.flavor == "m1.small" + assert params.enabled is True assert client.updated == [] assert client.deleted == [] @@ -201,6 +229,7 @@ def _existing_scaleset(**overrides): github_runner_group=None, extra_specs={}, tags=[], + enabled=True, ) base.update(overrides) return base @@ -259,6 +288,23 @@ def test_no_update_when_scaleset_unchanged(): assert client.deleted == [] +def test_update_when_scaleset_disabled(): + """ + arrange: FakeGarmClient with one existing disabled scaleset that otherwise matches. + act: Reconcile the desired spec. + assert: The reconciler enables the scaleset so GARM can spawn runners. + """ + client = FakeGarmClient( + providers=["openstack-demo"], + scalesets=[_existing_scaleset(enabled=False)], + ) + _reconcile(client, [_spec()]) + + assert len(client.updated) == 1 + _, params = client.updated[0] + assert params.enabled is True + + def test_delete_orphaned_scaleset(): """ arrange: FakeGarmClient with an observed scaleset not present in the desired set. @@ -274,6 +320,21 @@ def test_delete_orphaned_scaleset(): assert client.deleted == [42] +def test_unnamed_observed_scaleset_is_skipped(): + """ + arrange: FakeGarmClient with one observed scaleset lacking a name. + act: Reconcile an unrelated desired spec. + assert: The unnamed observed scaleset is ignored rather than deleted under an empty-name key. + """ + client = FakeGarmClient( + providers=["openstack-demo"], + scalesets=[_existing_scaleset(name=None, id=42)], + ) + _reconcile(client, [_spec(name="new-scaleset")]) + + assert client.deleted == [] + + @pytest.mark.parametrize( "providers, scalesets, desired, expected_deleted", [ @@ -307,3 +368,304 @@ def test_pre_install_scripts_passed_in_create(): assert len(client.created) == 1 _, _, params = client.created[0] assert params.extra_specs == {"pre_install_scripts": scripts} + + +class _FakeTemplate: + """Minimal fake for the GARM Template model used in template lifecycle tests.""" + + def __init__(self, name, tid, data=b"", description=""): + self.name = name + self.id = tid + self.data = list(data) if isinstance(data, bytes) and data else data or None + self.description = description + + +class _TemplateTrackingClient(FakeGarmClient): + """FakeGarmClient variant that tracks template create/update/delete operations.""" + + def __init__(self, templates=None, **kwargs): + super().__init__(**kwargs) + self._templates = list(templates or []) + self.created_templates: list[tuple[str, bytes, str]] = [] + self.updated_templates: list[tuple[int, bytes]] = [] + self.deleted_templates: list[int] = [] + + def list_templates(self, partial_name=None, os_type=None): + if partial_name is None: + return list(self._templates) + return [t for t in self._templates if t.name and partial_name in t.name] + + def get_template(self, template_id): + for t in self._templates: + if t.id == template_id: + return t + return None + + def create_template(self, name, data, description="", os_type="linux") -> _FakeTemplate: # type: ignore[override] + tid = max((t.id for t in self._templates), default=0) + 1 + template = _FakeTemplate(name, tid, data, description) + self._templates.append(template) + self.created_templates.append((name, data, description)) + return template + + def update_template(self, template_id, data): + for t in self._templates: + if t.id == template_id: + t.data = list(data) + break + self.updated_templates.append((template_id, data)) + + def delete_template(self, template_id): + self._templates = [t for t in self._templates if t.id != template_id] + self.deleted_templates.append(template_id) + + +class _DeleteFailingTemplateClient(_TemplateTrackingClient): + """TemplateTrackingClient variant whose delete_template raises a GARM API error.""" + + def delete_template(self, template_id): + from garm_api import GarmApiError + + raise GarmApiError(f"delete failed for template {template_id}") + + +_SYSTEM_TEMPLATE = _FakeTemplate("github_linux", tid=1, data=b"#!/bin/bash\nset -e\necho base\n") + + +def _spec_with_runner_config(**rc_kwargs): + """Build a spec with runner_config populated from the given kwargs.""" + from charm_state import RunnerConfig + + return _spec(runner_config=RunnerConfig(**rc_kwargs)) + + +def test_template_created_when_runner_config_set(): + """ + arrange: No existing scalesets or custom templates; system template exists. + act: Reconcile a spec with runner options (dockerhub_mirror). + assert: A custom template is created and the scaleset references it via template_id. + """ + client = _TemplateTrackingClient( + providers=["openstack-demo"], + scalesets=[], + templates=[_SYSTEM_TEMPLATE], + ) + _reconcile( + client, + [_spec_with_runner_config(dockerhub_mirror="https://mirror.example.com")], + ) + + assert len(client.created_templates) == 1 + name, data, _ = client.created_templates[0] + assert name == "github_linux-my-scaleset" + assert b"registry-mirrors" in data + assert len(client.created) == 1 + _, _, params = client.created[0] + # The created template's id is 2 (system=1, first custom=2) + assert params.template_id == 2 + + +def test_template_created_when_garm_returns_template_data_as_string(): + """ + arrange: The system template data is returned as a base64 string by the GARM API. + act: Reconcile a spec with runner options. + assert: A custom template is created from the decoded script data without hook errors. + """ + client = _TemplateTrackingClient( + providers=["openstack-demo"], + scalesets=[], + templates=[ + _FakeTemplate( + "github_linux", + tid=1, + data=base64.b64encode(b"#!/bin/bash\nset -e\necho base\n").decode(), + ) + ], + ) + _reconcile( + client, + [_spec_with_runner_config(dockerhub_mirror="https://mirror.example.com")], + ) + + assert len(client.created_templates) == 1 + _, data, _ = client.created_templates[0] + assert isinstance(data, bytes) + assert b"registry-mirrors" in data + assert b"#!/bin/bash" in data + + +def test_template_created_from_charmed_template_when_scaleset_references_it(): + """ + arrange: A scaleset references the charm-managed template and runner options are set. + act: Reconcile the desired spec. + assert: The custom template is derived from the charmed template rather than the bare system one. + """ + from charm_state import RunnerConfig + + client = _TemplateTrackingClient( + providers=["openstack-demo"], + scalesets=[], + templates=[ + _SYSTEM_TEMPLATE, + _FakeTemplate( + "github_linux_charmed", + tid=7, + data=base64.b64encode(b"#!/bin/bash\nset -e\necho charmed-base\n").decode(), + ), + ], + ) + _reconcile( + client, + [ + _spec( + template_id=7, + runner_config=RunnerConfig(dockerhub_mirror="https://mirror.example.com"), + ) + ], + ) + + assert len(client.created_templates) == 1 + _, data, _ = client.created_templates[0] + assert b"echo charmed-base" in data + assert b"echo base" not in data + + +def test_template_updated_when_runner_config_changes(): + """ + arrange: A scaleset with an existing custom template. + act: Reconcile with a changed runner config. + assert: The custom template is updated (not recreated); the scaleset template_id is unchanged. + """ + from charm_state import RunnerConfig + from runner_template import build_template_data + + old_config = RunnerConfig(dockerhub_mirror="https://old.example.com") + custom_template = _FakeTemplate( + "github_linux-my-scaleset", + tid=2, + data=build_template_data(b"#!/bin/bash\nset -e\necho base\n", old_config), + ) + client = _TemplateTrackingClient( + providers=["openstack-demo"], + scalesets=[_existing_scaleset(template_id=2)], + templates=[_SYSTEM_TEMPLATE, custom_template], + ) + _reconcile( + client, + [_spec_with_runner_config(dockerhub_mirror="https://new.example.com")], + ) + + assert len(client.updated_templates) == 1 + template_id, data = client.updated_templates[0] + assert template_id == 2 + assert b"new.example.com" in data + assert b"old.example.com" not in data + # Scaleset should not be updated just because template content changed. + assert client.updated == [] + + +def test_template_detached_when_runner_config_cleared(): + """ + arrange: A scaleset with an existing custom template. + act: Reconcile with no runner options. + assert: The scaleset is updated with template_id=0 (detach), and the custom template is deleted. + """ + client = _TemplateTrackingClient( + providers=["openstack-demo"], + scalesets=[_existing_scaleset(template_id=2)], + templates=[ + _SYSTEM_TEMPLATE, + _FakeTemplate( + "github_linux-my-scaleset", tid=2, data=b"#!/bin/bash\nset -e\necho x\n" + ), + ], + ) + _reconcile(client, [_spec()]) + + assert len(client.updated) == 1 + _, params = client.updated[0] + assert params.template_id == 0 + assert 2 in client.deleted_templates + + +def test_template_kept_when_system_template_missing(): + """ + arrange: A scaleset with an existing custom template, but the system template is not listed. + act: Reconcile with runner options still set. + assert: The existing custom template is kept (not destroyed); no create/update/delete on templates. + """ + client = _TemplateTrackingClient( + providers=["openstack-demo"], + scalesets=[_existing_scaleset(template_id=2)], + templates=[ + _FakeTemplate( + "github_linux-my-scaleset", tid=2, data=b"#!/bin/bash\nset -e\necho x\n" + ), + ], + ) + _reconcile( + client, + [_spec_with_runner_config(dockerhub_mirror="https://m.example.com")], + ) + + assert client.created_templates == [] + assert client.updated_templates == [] + assert client.deleted_templates == [] + + +def test_template_with_missing_id_is_not_updated(): + """ + arrange: A custom template exists without an id and its rendered content differs. + act: Reconcile a spec with runner options. + assert: The reconciler skips the update and falls back to the default template id. + """ + client = _TemplateTrackingClient( + providers=["openstack-demo"], + scalesets=[_existing_scaleset(template_id=2)], + templates=[ + _SYSTEM_TEMPLATE, + _FakeTemplate("github_linux-my-scaleset", tid=None, data=b"stale"), + ], + ) + _reconcile( + client, + [_spec_with_runner_config(dockerhub_mirror="https://mirror.example.com")], + ) + + assert client.updated_templates == [] + assert len(client.updated) == 1 + _, params = client.updated[0] + assert params.template_id == 0 + + +def test_template_bytes_does_not_cache_invalid_placeholder_on_generated_model(): + """ + arrange: A generated Template model whose list response omits data and whose fetched body is still missing. + act: Read template bytes through the reconciler helper. + assert: Empty bytes are returned and the Template model is not assigned a non-string placeholder. + """ + client = _TemplateTrackingClient() + reconciler = ScalesetReconciler(client) + listed = Template(id=5, name="github_linux") + client._templates = [listed] + + assert reconciler._template_bytes(listed) == b"" + assert listed.data is None + + +def test_delete_custom_template_skips_missing_id_and_api_errors(): + """ + arrange: One custom template has no id and another raises on delete. + act: Reconcile removal of the associated scalesets. + assert: Missing-id templates are skipped and delete errors do not abort reconcile. + """ + missing_id_client = _TemplateTrackingClient( + templates=[_FakeTemplate("github_linux-stale", tid=None, data=b"x")], + ) + _reconcile(missing_id_client, []) + assert missing_id_client.deleted_templates == [] + + failing_client = _DeleteFailingTemplateClient( + templates=[_FakeTemplate("github_linux-stale", tid=7, data=b"x")], + ) + _reconcile(failing_client, []) diff --git a/charms/tests/integration/test_garm.py b/charms/tests/integration/test_garm.py index f60325b2..6a59719c 100644 --- a/charms/tests/integration/test_garm.py +++ b/charms/tests/integration/test_garm.py @@ -631,13 +631,13 @@ def _point_github_endpoint_at_mock(base_url: str, token: str, mock_base_url: str def _detach_synced_credential(base_url: str, token: str) -> None: - """Delete the charm-synced org and credential so github.com can be repointed. - - GARM locks an endpoint's URLs while a credential references it. The charm syncs its credential - onto the built-in github.com endpoint, so it must be removed (along with any org bound to it) - before the endpoint can be pointed at the mock. Deletion tolerates a not-yet-created entity - (404); a real failure surfaces later as the repoint's 400. - """ + """Delete the charm-synced scalesets, org and credential so github.com can be repointed.""" + # GARM locks an endpoint's URLs while a credential references it, refuses to delete a + # credential while an org is bound to it, and refuses to delete an org while a scaleset + # references it. The charm syncs its credential onto the built-in github.com endpoint, so the + # chain must be unwound in dependency order — scalesets, then org, then credential — before + # the endpoint can be repointed at the mock. + _delete_scalesets(base_url, token) for org in _list_orgs(base_url, token): if org.get("name") == "test-org" and org.get("id"): _delete_if_present(f"{base_url}/organizations/{org['id']}", token) @@ -649,8 +649,40 @@ def _detach_synced_credential(base_url: str, token: str) -> None: def _delete_if_present(url: str, token: str) -> None: """DELETE a GARM resource, treating 404 as already-gone.""" resp = requests.delete(url, headers=_garm_auth_headers(token), timeout=30) - if resp.status_code != requests.codes.not_found: + if resp.status_code == requests.codes.not_found: + return + if not resp.ok: + logger.warning("DELETE %s -> %s: %s", url, resp.status_code, resp.text) + resp.raise_for_status() + + +def _delete_scalesets(base_url: str, token: str) -> None: + """Disable and delete every scaleset.""" + for scaleset in _list_scalesets(base_url, token): + scaleset_id = scaleset.get("id") + if scaleset_id is None: + continue + # GARM 400s a scaleset delete while the scaleset is still enabled or draining runners, so + # disable it (idle count to zero) to trigger the drain before deleting. + resp = requests.put( + f"{base_url}/scalesets/{scaleset_id}", + json={"enabled": False, "min_idle_runners": 0}, + headers=_garm_auth_headers(token), + timeout=30, + ) resp.raise_for_status() + _delete_scaleset_drained(base_url, token, scaleset_id) + + +@retry( + retry=retry_if_exception_type(requests.exceptions.RequestException), + wait=wait_exponential(multiplier=1, min=2, max=15), + stop=stop_after_attempt(20), + reraise=True, +) +def _delete_scaleset_drained(base_url: str, token: str, scaleset_id: int) -> None: + """DELETE a scaleset, retrying while GARM drains it (400); 404 means already gone.""" + _delete_if_present(f"{base_url}/scalesets/{scaleset_id}", token) def _restore_system_templates(garm_url: str, token: str) -> None: @@ -779,3 +811,111 @@ def test_github_credentials_synced_from_relation( assert "github.com" in endpoint_names, ( f"Built-in github.com endpoint must be preserved; got {sorted(endpoint_names)}" ) + + +def _get_template_data(base_url: str, token: str, template_id: int) -> str: + """Fetch a GARM template and return its decoded (base64) script content. + + Args: + base_url: GARM API base URL. + token: JWT token for authentication. + template_id: Integer template ID. + + Returns: + The decoded template script as a string. + """ + resp = requests.get( + f"{base_url}/templates/{template_id}", + headers=_garm_auth_headers(token), + timeout=30, + ) + resp.raise_for_status() + data = resp.json().get("data", "") + return base64.b64decode(data).decode() if data else "" + + +@retry( + retry=retry_if_exception_type((AssertionError, requests.exceptions.RequestException)), + wait=wait_exponential(multiplier=1, min=2, max=20), + stop=stop_after_attempt(30), + reraise=True, +) +def _wait_for_scaleset_template_data( + base_url: str, + token: str, + scaleset_name: str, + expected_markers: tuple[str, ...], +) -> str: + """Wait until a scaleset references a template containing the expected markers.""" + scaleset = _wait_for_scaleset(base_url, token, scaleset_name) + template_id = scaleset.get("template_id") + assert template_id, f"Expected scaleset to reference a custom template, got: {scaleset}" + + rendered = _get_template_data(base_url, token, template_id) + missing = [marker for marker in expected_markers if marker not in rendered] + assert not missing, ( + f"Expected markers {missing!r} in rendered template {template_id}, got:\n{rendered}" + ) + return rendered + + +def test_runner_options_render_into_scaleset_template( + juju: jubilant.Juju, + configurator_garm: str, + configurator_with_image: str, + fake_github_api_url: str, +): + """ + arrange: GARM and garm-configurator are integrated, with a credential and org + registered so a scaleset can be created. + act: Set the runner-behaviour config options on the configurator charm. + assert: The scaleset references a custom template whose rendered content + reflects each runner option — proving the options reach GARM via live + reconcile (no upgrade). + """ + address = _get_garm_address(juju, configurator_garm) + base_url = _garm_api_base_url(address) + token = _garm_first_run(juju, address) + _detach_synced_credential(base_url, token) + _point_github_endpoint_at_mock(base_url, token, fake_github_api_url) + _restore_system_templates(base_url, token) + + juju.config( + configurator_with_image, + values={ + "dockerhub-mirror": "https://mirror.example.com", + "runner-http-proxy": "http://proxy.example.com:3128", + "aproxy-exclude-addresses": "192.168.0.0/16", + "aproxy-redirect-ports": "80,443", + "otel-collector-endpoint": "http://otel.example.com:4318", + "pre-job-script": "echo integration-marker", + }, + ) + juju.wait( + lambda status: jubilant.all_active(status, configurator_with_image) + and jubilant.all_agents_idle(status, configurator_garm), + timeout=3 * 60, + delay=10, + ) + + # One marker per config option, proving each reaches GARM via live reconcile: + # dockerhub-mirror (daemon.json + env var), runner-http-proxy + aproxy-* + # (aproxy listener and nftables ruleset), otel-collector-endpoint (env var), + # and pre-job-script (job-start hook). + expected_markers = ( + "registry-mirrors", + "DOCKERHUB_MIRROR=https://mirror.example.com", + "proxy=http://proxy.example.com:3128 listen=:54969", + "192.168.0.0/16", + "tcp dport { 80, 443 }", + "ACTION_OTEL_EXPORTER_OTLP_ENDPOINT=http://otel.example.com:4318", + "echo integration-marker", + ) + # Asserts each expected marker is present in the referenced template, retrying + # until GARM reflects the reconciled config. + _wait_for_scaleset_template_data( + base_url, + token, + _SCALESET_TEST_NAME, + expected_markers, + )