feat: manage a per-scaleset OpenStack security group

.github/workflows/garm_e2e.yaml

@@ -531,14 +531,32 @@ jobs:
           ORPHANS=$(openstack server list --long --image "$E2E_RUNNER_IMAGE_NAME" -f json \
             | python3 -c "import json,sys; print('\n'.join(s['ID'] for s in json.load(sys.stdin) if 'garm-controller-id' in (s.get('Properties') or {})))")
 
-          if [ -z "$ORPHANS" ]; then
+          if [ -n "$ORPHANS" ]; then
+            echo "$ORPHANS" | while read -r ID; do
+              echo "Deleting orphaned runner ${ID}"
+              openstack server delete --wait "$ID" || echo "::warning::Could not delete ${ID}"
+            done
+          else
             echo "No runners left on the tenant."
+          fi
+
+          # Each run's configurator manages its own garm-<label> security group and, by
+          # design, never deletes it. With the runners gone it has no ports, so sweep it
+          # here: the tenant belongs to this suite and runs are serialised, so the prefix
+          # cannot match another run's group.
+          # Matched on the scale-set label pattern (garm-e2e-<digest>, from _e2e_label)
+          # so the relay's own garm-e2e-relay-<run> group is left to the step below.
+          MANAGED=$(openstack security group list -f json \
+            | python3 -c "import json,re,sys; print('\n'.join(g['Name'] for g in json.load(sys.stdin) if re.fullmatch(r'garm-e2e-[0-9a-f]{6}', g.get('Name') or '')))")
+
+          if [ -z "$MANAGED" ]; then
+            echo "No managed security groups left on the tenant."
             exit 0
           fi
 
-          echo "$ORPHANS" | while read -r ID; do
-            echo "Deleting orphaned runner ${ID}"
-            openstack server delete --wait "$ID" || echo "::warning::Could not delete ${ID}"
+          echo "$MANAGED" | while read -r NAME; do
+            echo "Deleting managed security group ${NAME}"
+            openstack security group delete "$NAME" || echo "::warning::Could not delete ${NAME}"
           done
 
       - name: Delete the callback relay

CONTRIBUTING.md

@@ -258,12 +258,17 @@ through spread. The suite deploys GARM with PostgreSQL and a traefik ingress —
 load-balancer address is what makes GARM's callback and metadata URLs reachable from
 the tenant — waits for a runner VM to register, and dispatches
 `.github/workflows/garm_e2e_test_run.yaml` at the scale set's label; the test passes
-only if that job concludes successfully. The host admits no inbound traffic from the
+only if that job concludes successfully. Before dispatching, it also checks the
+configurator's security group in the tenant directly: that `garm-<label>` admits no
+ingress, and that the runner's port carries that group and no other — the one assertion
+that fails if the group id stops reaching the provider while the rest of the suite stays
+green. The host admits no inbound traffic from the
 tenant, so the workflow boots a relay VM there that the host feeds over a reverse SSH
 tunnel, and runner VMs send their GARM traffic to it; the relay and its security group
 are deleted in an always-run cleanup step. Runners
 left on the tenant are deleted whatever the outcome, matched on the
-`garm-controller-id` GARM stamps on every server it creates.
+`garm-controller-id` GARM stamps on every server it creates, and each run's
+`garm-<label>` security group is swept with them.
 
 It is triggered manually and is **not** a merge gate:
 

charms/garm-configurator/charmcraft.yaml

@@ -133,6 +133,24 @@ config:
       description: |
         A Python dict-like string containing key-value pairs of script name
         to bash script to be run prior to runner installation.
+    security-group-rules:
+      type: string
+      default: '[{"direction": "egress", "remote_ip_prefix": "0.0.0.0/0"}, {"direction": "egress", "remote_ip_prefix": "::/0"}]'
+      description: |
+        JSON list of rules for the OpenStack security group this charm manages for the
+        scaleset's runners. The charm keeps the group's rules equal to this list, so a
+        rule added by hand is removed. A rule takes: "direction" (required, "ingress" or
+        "egress"), "protocol" ("tcp", "udp", "icmp", "icmpv6", or "any", default "any"),
+        "port_min"/"port_max" (tcp and udp only), and "remote_ip_prefix" (a CIDR; without
+        one the rule covers every address of its ethertype, IPv4 unless the CIDR is IPv6),
+        for example:
+        [{"direction": "ingress", "protocol": "tcp", "port_min": 22, "remote_ip_prefix": "10.0.0.0/8"}]
+        The default allows all egress and no ingress, which is what a runner needs to reach
+        GitHub. OpenStack attaches this group instead of the project's default group, so a
+        list without an egress rule is rejected. Set an empty list to manage no group, which
+        deletes the group this charm created unless a port still attaches it; removing the
+        application does the same. Requires OpenStack credentials that may create and delete
+        security groups and their rules.
     dockerhub-mirror:
       type: string
       description: |

charms/garm-configurator/src/charm.py

@@ -38,16 +38,19 @@ def __init__(self, *args: typing.Any) -> None:
             self.on[IMAGE_RELATION_NAME].relation_joined,
             self.on[IMAGE_RELATION_NAME].relation_changed,
             self.on[IMAGE_RELATION_NAME].relation_broken,
+            # So a blocked unit retries when the OpenStack API comes back, rather than
+            # waiting for an unrelated event.
+            self.on.update_status,
         ]:
             self.framework.observe(event, self._reconcile)
 
     def _reconcile(self, event: ops.EventBase) -> None:
         """Handle all charm events by reconciling configuration and relation data.
 
         Reads the current charm state, forwards OpenStack credentials to the
-        image-builder relation, publishes scaleset configuration (and secrets
-        when the image reference is available) to all garm-configurator relations,
-        then sets unit status to reflect readiness.
+        image-builder relation, reconciles the scaleset's security group, publishes
+        scaleset configuration (and secrets when the image reference is available) to
+        all garm-configurator relations, then sets unit status to reflect readiness.
 
         Args:
             event: The triggering event.
@@ -62,7 +65,7 @@ def _reconcile(self, event: ops.EventBase) -> None:
             return
 
         self._update_image_relation(state)
-        self._configure_garm_relation(state)
+        self._configure_garm_relation(state=state)
 
         if state.image_overrides_builder:
             self.unit.status = ops.ActiveStatus("Ready; configured image overrides image builder")
@@ -134,7 +137,8 @@ def _configure_garm_relation(self, state: CharmState) -> None:
 
         Writes non-secret scaleset fields (name, provider, credentials, image,
         image visibility, flavor, arch, runner counts, labels, runner group,
-        remote-shell toggle, and pre-install scripts) to the relation. The ``image_id`` wire field
+        remote-shell toggle, pre-install scripts, and the security group rules the GARM
+        charm applies for this scale set) to the relation. The ``image_id`` wire field
         carries either an OpenStack image name or ID for compatibility.
         The optional ``org`` and ``repo`` fields are included only when set.
 
@@ -156,6 +160,7 @@ def _configure_garm_relation(self, state: CharmState) -> None:
 
         pre_install = state.scaleset_config.pre_install_scripts
         rc = state.runner_config
+        rules = state.scaleset_config.security_group_rules
         basic_data: dict[str, str] = {
             "name": state.scaleset_config.name,
             "provider_name": state.provider_config.provider_name,
@@ -177,6 +182,9 @@ def _configure_garm_relation(self, state: CharmState) -> None:
             "aproxy_redirect_ports": rc.aproxy_redirect_ports or "",
             "otel_collector_endpoint": rc.otel_collector_endpoint or "",
             "pre_job_script": rc.pre_job_script or "",
+            # An empty list publishes "[]": the GARM charm then attaches no group and removes
+            # the one it created, which is the opt-out.
+            "security_group_rules": json.dumps([rule.model_dump() for rule in rules]),
         }
         if state.scaleset_config.org:
             basic_data["org"] = state.scaleset_config.org

charms/garm-configurator/src/charm_state.py

@@ -4,10 +4,13 @@
 """State of the GARM configurator charm."""
 
 import ipaddress
+import json
+from typing import Literal
 
 import ops
 from pydantic import (
     BaseModel,
+    ConfigDict,
     HttpUrl,
     TypeAdapter,
     ValidationError,
@@ -42,6 +45,7 @@
 SCALESET_RUNNER_GROUP_CONFIG_NAME = "runner-group"
 SCALESET_ENABLE_SHELL_CONFIG_NAME = "enable-shell"
 SCALESET_PRE_INSTALL_SCRIPTS_CONFIG_NAME = "pre-install-scripts"
+SCALESET_SECURITY_GROUP_RULES_CONFIG_NAME = "security-group-rules"
 
 DOCKERHUB_MIRROR_CONFIG_NAME = "dockerhub-mirror"
 RUNNER_HTTP_PROXY_CONFIG_NAME = "runner-http-proxy"
@@ -218,6 +222,74 @@ def from_charm(cls, charm: ops.CharmBase) -> "GithubAppConfig":
         )
 
 
+class SecurityGroupRule(BaseModel):
+    """One rule of the OpenStack security group the charm manages for a scaleset.
+
+    Attributes:
+        direction: Whether the rule admits ingress or egress traffic.
+        protocol: Transport protocol the rule applies to, or "any" for every protocol.
+        port_min: First port of the range, or None to cover every port.
+        port_max: Last port of the range, or None to cover only port_min.
+        remote_ip_prefix: CIDR the rule applies to, or None to cover every address.
+    """
+
+    # An unknown key is rejected rather than dropped: a misspelt ``remote_ip_prefix``
+    # would otherwise silently widen the rule to every address.
+    model_config = ConfigDict(extra="forbid")
+
+    direction: Literal["ingress", "egress"]
+    protocol: Literal["tcp", "udp", "icmp", "icmpv6", "any"] = "any"
+    port_min: int | None = None
+    port_max: int | None = None
+    remote_ip_prefix: str | None = None
+
+    @field_validator("remote_ip_prefix")
+    @classmethod
+    def _validate_cidr(cls, value: str | None) -> str | None:
+        """Require a CIDR, normalised to the network address Neutron expects."""
+        if value is None:
+            return None
+        try:
+            # strict, because Neutron rejects a prefix with host bits set; a bare
+            # address is read as a single-host network.
+            return str(ipaddress.ip_network(value.strip(), strict=True))
+        except ValueError as exc:
+            raise ValueError(
+                f"must be a CIDR such as 10.0.0.0/8, 192.0.2.1/32, or ::/0; got {value!r}"
+            ) from exc
+
+    @model_validator(mode="after")
+    def _validate_icmp_ethertype(self) -> "SecurityGroupRule":
+        """Reject the protocol and ethertype pairings Neutron refuses."""
+        # A rule with no prefix is IPv4, because only an IPv6 prefix names its ethertype.
+        ipv6 = bool(self.remote_ip_prefix and ":" in self.remote_ip_prefix)
+        if self.protocol == "icmpv6" and not ipv6:
+            raise ValueError("protocol icmpv6 requires an IPv6 remote_ip_prefix")
+        if self.protocol == "icmp" and ipv6:
+            raise ValueError("protocol icmp requires an IPv4 remote_ip_prefix")
+        return self
+
+    @model_validator(mode="after")
+    def _validate_ports(self) -> "SecurityGroupRule":
+        """Reject a port range the protocol cannot carry, and fill in a single port."""
+        # A range needs a lower bound; Neutron has no "everything up to" form.
+        if self.port_max is not None and self.port_min is None:
+            raise ValueError("port_max requires port_min")
+        if self.port_min is None:
+            return self
+        if self.protocol not in ("tcp", "udp"):
+            raise ValueError(
+                f"port_min and port_max require protocol tcp or udp, not {self.protocol}"
+            )
+        port_max = self.port_min if self.port_max is None else self.port_max
+        if not 1 <= self.port_min <= 65535 or not 1 <= port_max <= 65535:
+            raise ValueError("port_min and port_max must be in 1..65535")
+        if self.port_min > port_max:
+            raise ValueError("port_min must not be greater than port_max")
+        self.port_max = port_max
+        return self
+
+
 class ScalesetConfig(BaseModel):
     """Scaleset configuration.
 
@@ -234,6 +306,8 @@ class ScalesetConfig(BaseModel):
         enable_shell: Whether GARM's remote shell is enabled on the scaleset.
         pre_install_scripts: Script name to bash script pairs for pre-installation.
         image_visibility: Glance visibility the provider searches when resolving an image name.
+        security_group_rules: Rules of the security group the charm manages for the runners,
+            or empty when it manages none.
     """
 
     name: str
@@ -248,6 +322,7 @@ class ScalesetConfig(BaseModel):
     enable_shell: bool = False
     pre_install_scripts: str | None = None
     image_visibility: str | None = None
+    security_group_rules: list[SecurityGroupRule]
 
     @classmethod
     def from_charm(cls, charm: ops.CharmBase) -> "ScalesetConfig":
@@ -329,7 +404,75 @@ def from_charm(cls, charm: ops.CharmBase) -> "ScalesetConfig":
             enable_shell=bool(charm.config.get(SCALESET_ENABLE_SHELL_CONFIG_NAME, False)),
             pre_install_scripts=pre_install_scripts,
             image_visibility=image_visibility,
+            security_group_rules=_parse_security_group_rules(charm),
+        )
+
+
+def _parse_security_group_rules(charm: ops.CharmBase) -> list[SecurityGroupRule]:
+    """Parse the optional security-group-rules JSON document.
+
+    Args:
+        charm: The charm instance.
+
+    Raises:
+        CharmConfigInvalidError: If the value is not a JSON list of valid rules, or lists no
+            egress rule.
+
+    Returns:
+        The parsed rules, empty when the operator opts out of managing a group. The
+        charmcraft default lists all egress, so the group is managed unless it is cleared.
+    """
+    configured = charm.config.get(SCALESET_SECURITY_GROUP_RULES_CONFIG_NAME)
+    if configured is None:
+        # Unset only if the charmcraft default is dropped from the metadata.
+        return []
+    document = str(configured)
+    if not document.strip():
+        # Juju sets an empty string when the option is cleared, which would otherwise read as
+        # the opt-out and silently drop the isolation.
+        raise CharmConfigInvalidError(
+            f"{SCALESET_SECURITY_GROUP_RULES_CONFIG_NAME} cannot be empty; "
+            f"use [] to manage no security group"
         )
+    try:
+        entries = json.loads(document.strip())
+    except json.JSONDecodeError as exc:
+        raise CharmConfigInvalidError(
+            f"{SCALESET_SECURITY_GROUP_RULES_CONFIG_NAME} must be a JSON list of rules"
+        ) from exc
+    if not isinstance(entries, list):
+        raise CharmConfigInvalidError(
+            f"{SCALESET_SECURITY_GROUP_RULES_CONFIG_NAME} must be a JSON list of rules"
+        )
+
+    rules: list[SecurityGroupRule] = []
+    for index, entry in enumerate(entries):
+        try:
+            rules.append(SecurityGroupRule.model_validate(entry))
+        except ValidationError as exc:
+            raise CharmConfigInvalidError(
+                f"{SCALESET_SECURITY_GROUP_RULES_CONFIG_NAME} rule {index}: "
+                f"{_describe_rule_error(exc)}"
+            ) from exc
+    # An empty list is the opt-out: the charm manages no group and the runners keep the
+    # project's default one. A configured group needs egress, though, or it would isolate
+    # the runner from GitHub entirely.
+    if rules and not any(rule.direction == "egress" for rule in rules):
+        raise CharmConfigInvalidError(
+            f"{SCALESET_SECURITY_GROUP_RULES_CONFIG_NAME} needs at least one egress rule: "
+            f"OpenStack attaches this group instead of the project's default group, so a "
+            f"runner with no egress cannot reach GitHub"
+        )
+    return rules
+
+
+def _describe_rule_error(exc: ValidationError) -> str:
+    """Render one rule's first validation error as a single status line."""
+    error = exc.errors()[0]
+    field = ".".join(str(part) for part in error["loc"])
+    # Pydantic prefixes the message of an error raised by a custom validator.
+    message = " ".join(str(error["msg"]).removeprefix("Value error, ").split())
+    return f"{field}: {message}" if field else message
 
 
 def _get_optional_string_config(charm: ops.CharmBase, key: str) -> str | None: