From abd3bffbad4f59e09eded27a46ebc843e73cac60 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 25 Jan 2024 14:07:22 +0800 Subject: [PATCH 01/16] feat: support random choice from tmate relations --- src/charm_state.py | 55 ++++++++++++++++--------------- src/runner.py | 11 +++++-- src/runner_manager.py | 4 +-- src/runner_type.py | 4 +-- tests/unit/factories.py | 48 +++++++++++++++++++++++++++ tests/unit/requirements.txt | 1 + tests/unit/test_charm_state.py | 12 +++---- tests/unit/test_runner.py | 44 ++++++++++++++++++++++++- tests/unit/test_runner_manager.py | 2 +- tox.ini | 1 + 10 files changed, 141 insertions(+), 41 deletions(-) create mode 100644 tests/unit/factories.py create mode 100644 tests/unit/requirements.txt diff --git a/src/charm_state.py b/src/charm_state.py index d51a9b71f..0ebaa8136 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -9,7 +9,7 @@ import platform from enum import Enum from pathlib import Path -from typing import Optional +from typing import List, Optional from ops import CharmBase from pydantic import AnyHttpUrl, BaseModel, Field, ValidationError, root_validator @@ -200,31 +200,37 @@ class SSHDebugInfo(BaseModel): ed25519_fingerprint: str = Field(pattern="^SHA256:.*") @classmethod - def from_charm(cls, charm: CharmBase) -> Optional["SSHDebugInfo"]: + def from_charm(cls, charm: CharmBase) -> List["SSHDebugInfo"]: """Initialize the SSHDebugInfo from charm relation data. Args: charm: The charm instance. """ + ssh_debug_connections: list[SSHDebugInfo] = [] relations = charm.model.relations[DEBUG_SSH_INTEGRATION_NAME] if not relations or not (relation := relations[0]).units: - return None - target_unit = next(iter(relation.units)) - relation_data = relation.data[target_unit] - if ( - not (host := relation_data.get("host")) - or not (port := relation_data.get("port")) - or not (rsa_fingerprint := relation_data.get("rsa_fingerprint")) - or not (ed25519_fingerprint := relation_data.get("ed25519_fingerprint")) - ): - logger.warning("%s relation data not yet ready.", DEBUG_SSH_INTEGRATION_NAME) - return None - return SSHDebugInfo( - host=host, - port=port, - rsa_fingerprint=rsa_fingerprint, - ed25519_fingerprint=ed25519_fingerprint, - ) + return ssh_debug_connections + for unit in relation.units: + relation_data = relation.data[unit] + if ( + not (host := relation_data.get("host")) + or not (port := relation_data.get("port")) + or not (rsa_fingerprint := relation_data.get("rsa_fingerprint")) + or not (ed25519_fingerprint := relation_data.get("ed25519_fingerprint")) + ): + logger.warning( + "%s relation data for %s not yet ready.", DEBUG_SSH_INTEGRATION_NAME, unit.name + ) + continue + ssh_debug_connections.append( + SSHDebugInfo( + host=host, + port=port, + rsa_fingerprint=rsa_fingerprint, + ed25519_fingerprint=ed25519_fingerprint, + ) + ) + return ssh_debug_connections @dataclasses.dataclass(frozen=True) @@ -236,14 +242,14 @@ class State: proxy_config: Proxy-related configuration. charm_config: Configuration of the juju charm. arch: The underlying compute architecture, i.e. x86_64, amd64, arm64/aarch64. - ssh_debug_info: The SSH debug connection configuration information. + ssh_debug_infos: SSH debug connections configuration information. """ is_metrics_logging_available: bool proxy_config: ProxyConfig charm_config: CharmConfig arch: ARCH - ssh_debug_info: Optional[SSHDebugInfo] + ssh_debug_infos: List[SSHDebugInfo] @classmethod def from_charm(cls, charm: CharmBase) -> "State": @@ -292,7 +298,7 @@ def from_charm(cls, charm: CharmBase) -> "State": raise CharmConfigInvalidError(f"Unsupported architecture {exc.arch}") from exc try: - ssh_debug_info = SSHDebugInfo.from_charm(charm) + ssh_debug_infos = SSHDebugInfo.from_charm(charm) except ValidationError as exc: logger.error("Invalid SSH debug info: %s.", exc) raise CharmConfigInvalidError("Invalid SSH Debug info") from exc @@ -302,16 +308,13 @@ def from_charm(cls, charm: CharmBase) -> "State": proxy_config=proxy_config, charm_config=charm_config, arch=arch, - ssh_debug_info=ssh_debug_info, + ssh_debug_infos=ssh_debug_infos, ) state_dict = dataclasses.asdict(state) # Convert pydantic object to python object serializable by json module. state_dict["proxy_config"] = json.loads(state_dict["proxy_config"].json()) state_dict["charm_config"] = json.loads(state_dict["charm_config"].json()) - state_dict["ssh_debug_info"] = ( - json.loads(state_dict["ssh_debug_info"].json()) if ssh_debug_info else None - ) json_data = json.dumps(state_dict, ensure_ascii=False) CHARM_STATE_PATH.write_text(json_data, encoding="utf-8") diff --git a/src/runner.py b/src/runner.py index 5fa84deab..2bfbb6e45 100644 --- a/src/runner.py +++ b/src/runner.py @@ -13,6 +13,7 @@ import json import logging import pathlib +import secrets import textwrap import time from dataclasses import dataclass @@ -22,7 +23,7 @@ import yaml import shared_fs -from charm_state import ARCH +from charm_state import ARCH, SSHDebugInfo from errors import ( CreateSharedFilesystemError, LxdError, @@ -671,9 +672,13 @@ def _configure_runner(self) -> None: # As the user already has sudo access, this does not give the user any additional access. self.instance.execute(["/usr/bin/sudo", "chmod", "777", "/usr/local/bin"]) + selected_ssh_connection: SSHDebugInfo | None = ( + secrets.choice(self.config.ssh_debug_infos) if self.config.ssh_debug_infos else None + ) + logger.info("SSH Debug info: %s", selected_ssh_connection) # Load `/etc/environment` file. environment_contents = self._clients.jinja.get_template("environment.j2").render( - proxies=self.config.proxies, ssh_debug_info=self.config.ssh_debug_info + proxies=self.config.proxies, ssh_debug_info=selected_ssh_connection ) self._put_file("/etc/environment", environment_contents) @@ -682,7 +687,7 @@ def _configure_runner(self) -> None: proxies=self.config.proxies, pre_job_script=str(self.pre_job_script), dockerhub_mirror=self.config.dockerhub_mirror, - ssh_debug_info=self.config.ssh_debug_info, + ssh_debug_info=selected_ssh_connection, ) self._put_file(str(self.env_file), env_contents) self.instance.execute(["/usr/bin/chown", "ubuntu:ubuntu", str(self.env_file)]) diff --git a/src/runner_manager.py b/src/runner_manager.py index 187d1590e..d85203d63 100644 --- a/src/runner_manager.py +++ b/src/runner_manager.py @@ -409,7 +409,7 @@ def _spawn_new_runners(self, count: int, resources: VirtualMachineResources): path=self.config.path, proxies=self.proxies, name=self._generate_runner_name(), - ssh_debug_info=self.config.charm_state.ssh_debug_info, + ssh_debug_infos=self.config.charm_state.ssh_debug_infos, ) runner = Runner(self._clients, config, RunnerStatus()) try: @@ -611,7 +611,7 @@ def create_runner_info( name=name, path=self.config.path, proxies=self.proxies, - ssh_debug_info=self.config.charm_state.ssh_debug_info, + ssh_debug_infos=self.config.charm_state.ssh_debug_infos, ) return Runner( self._clients, diff --git a/src/runner_type.py b/src/runner_type.py index 6afed8766..abdb36ae2 100644 --- a/src/runner_type.py +++ b/src/runner_type.py @@ -77,7 +77,7 @@ class RunnerConfig: # pylint: disable=too-many-instance-attributes name. proxies: HTTP(S) proxy settings. dockerhub_mirror: URL of dockerhub mirror to use. - ssh_debug_info: The SSH debug server connection metadata. + ssh_debug_infos: The SSH debug server connections metadata. """ app_name: str @@ -87,7 +87,7 @@ class RunnerConfig: # pylint: disable=too-many-instance-attributes path: GithubPath proxies: ProxySetting dockerhub_mirror: str | None = None - ssh_debug_info: SSHDebugInfo | None = None + ssh_debug_infos: SSHDebugInfo | None = None @dataclass diff --git a/tests/unit/factories.py b/tests/unit/factories.py new file mode 100644 index 000000000..55733a2e3 --- /dev/null +++ b/tests/unit/factories.py @@ -0,0 +1,48 @@ +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Factories for generating test data.""" + +# The factory definitions don't need public methods +# pylint: disable=too-few-public-methods + +import random +from typing import Generic, TypeVar + +import factory +import factory.fuzzy +from pydantic.networks import IPvAnyAddress + +from charm_state import SSHDebugInfo + +T = TypeVar("T") + + +class BaseMetaFactory(Generic[T], factory.base.FactoryMetaClass): + """Used for type hints of factories.""" + + # No need for docstring because it is used for type hints + def __call__(cls, *args, **kwargs) -> T: # noqa: N805 + """Used for type hints of factories.""" # noqa: DCO020 + return super().__call__(*args, **kwargs) # noqa: DCO030 + + +# The attributes of these classes are generators for the attributes of the meta class +# mypy incorrectly believes the factories don't support metaclass +class SSHDebugInfoFactory( + factory.Factory, metaclass=BaseMetaFactory[SSHDebugInfo] # type: ignore +): + # Docstrings have been abbreviated for factories, checking for docstrings on model attributes + # can be skipped. + """Generate PathInfos.""" # noqa: DCO060 + + class Meta: + """Configuration for factory.""" # noqa: DCO060 + + model = SSHDebugInfo + abstract = False + + host: IPvAnyAddress = factory.Faker("ipv4") + port: int = factory.LazyAttribute(lambda n: random.randrange(1024, 65536)) + rsa_fingerprint: str = factory.fuzzy.FuzzyText(prefix="SHA256:") + ed25519_fingerprint: str = factory.fuzzy.FuzzyText(prefix="SHA256:") diff --git a/tests/unit/requirements.txt b/tests/unit/requirements.txt new file mode 100644 index 000000000..c9c3f2669 --- /dev/null +++ b/tests/unit/requirements.txt @@ -0,0 +1 @@ +factory-boy>=3,<4 diff --git a/tests/unit/test_charm_state.py b/tests/unit/test_charm_state.py index c3e147e55..8e1c77ee6 100644 --- a/tests/unit/test_charm_state.py +++ b/tests/unit/test_charm_state.py @@ -132,7 +132,7 @@ def test_ssh_debug_info_from_charm_no_relations(): mock_charm = MagicMock(spec=GithubRunnerCharm) mock_charm.model.relations = {DEBUG_SSH_INTEGRATION_NAME: []} - assert SSHDebugInfo.from_charm(mock_charm) is None + assert not SSHDebugInfo.from_charm(mock_charm) @pytest.mark.parametrize( @@ -220,11 +220,11 @@ def test_from_charm_ssh_debug_info(): mock_charm.app.name = "github-runner-operator" mock_charm.unit.name = "github-runner-operator/0" - ssh_debug_info = State.from_charm(mock_charm).ssh_debug_info - assert str(ssh_debug_info.host) == mock_relation_data["host"] - assert str(ssh_debug_info.port) == mock_relation_data["port"] - assert ssh_debug_info.rsa_fingerprint == mock_relation_data["rsa_fingerprint"] - assert ssh_debug_info.ed25519_fingerprint == mock_relation_data["ed25519_fingerprint"] + ssh_debug_infos = State.from_charm(mock_charm).ssh_debug_infos + assert str(ssh_debug_infos[0].host) == mock_relation_data["host"] + assert str(ssh_debug_infos[0].port) == mock_relation_data["port"] + assert ssh_debug_infos[0].rsa_fingerprint == mock_relation_data["rsa_fingerprint"] + assert ssh_debug_infos[0].ed25519_fingerprint == mock_relation_data["ed25519_fingerprint"] def test_invalid_runner_storage(): diff --git a/tests/unit/test_runner.py b/tests/unit/test_runner.py index 889b8f787..7569090d0 100644 --- a/tests/unit/test_runner.py +++ b/tests/unit/test_runner.py @@ -12,11 +12,13 @@ import pytest from _pytest.monkeypatch import MonkeyPatch +from charm_state import SSHDebugInfo from errors import CreateSharedFilesystemError, RunnerCreateError, RunnerRemoveError from runner import CreateRunnerConfig, Runner, RunnerConfig, RunnerStatus from runner_manager_type import RunnerManagerClients from runner_type import GithubOrg, GithubRepo, VirtualMachineResources from shared_fs import SharedFilesystem +from tests.unit.factories import SSHDebugInfoFactory from tests.unit.mock import ( MockLxdClient, MockRepoPolicyComplianceClient, @@ -89,6 +91,12 @@ def jinja2_environment_fixture() -> MagicMock: return jinja2_mock +@pytest.fixture(scope="function", name="ssh_debug_infos") +def ssh_debug_infos_fixture() -> list[SSHDebugInfo]: + """A list of randomly generated ssh_debug_infos.""" + return SSHDebugInfoFactory.create_batch(size=100) + + @pytest.fixture( scope="function", name="runner", @@ -100,7 +108,13 @@ def jinja2_environment_fixture() -> MagicMock: ), ], ) -def runner_fixture(request, lxd: MockLxdClient, jinja: MagicMock, tmp_path: Path): +def runner_fixture( + request, + lxd: MockLxdClient, + jinja: MagicMock, + tmp_path: Path, + ssh_debug_infos: list[SSHDebugInfo], +): client = RunnerManagerClients( MagicMock(), jinja, @@ -117,6 +131,7 @@ def runner_fixture(request, lxd: MockLxdClient, jinja: MagicMock, tmp_path: Path lxd_storage_path=pool_path, dockerhub_mirror=None, issue_metrics=False, + ssh_debug_infos=ssh_debug_infos, ) status = RunnerStatus() return Runner( @@ -416,3 +431,30 @@ def test_remove_with_delete_error( with pytest.raises(RunnerRemoveError): runner.remove("test_token") + + +def test_random_ssh_connection_choice( + runner: Runner, + vm_resources: VirtualMachineResources, + token: str, + binary_path: Path, +): + """ + arrange: given a mock runner with random batch of ssh debug infos. + act: when runner.configure_runner is called. + assert: selected ssh_debug_info is random. + """ + runner.create( + config=CreateRunnerConfig( + image="test_image", + resources=vm_resources, + binary_path=binary_path, + registration_token=token, + ) + ) + runner._configure_runner() + first_call_args = runner._clients.jinja.get_template("env.j2").render.call_args.kwargs + runner._configure_runner() + second_call_args = runner._clients.jinja.get_template("env.j2").render.call_args.kwargs + + assert first_call_args["ssh_debug_info"] != second_call_args["ssh_debug_info"] diff --git a/tests/unit/test_runner_manager.py b/tests/unit/test_runner_manager.py index 4b7d3a037..2e778c939 100644 --- a/tests/unit/test_runner_manager.py +++ b/tests/unit/test_runner_manager.py @@ -35,7 +35,7 @@ def charm_state_fixture(): mock = MagicMock(spec=State) mock.is_metrics_logging_available = False mock.arch = ARCH.X64 - mock.ssh_debug_info = None + mock.ssh_debug_infos = None return mock diff --git a/tox.ini b/tox.ini index 9dfb8b1ce..a74a585ba 100644 --- a/tox.ini +++ b/tox.ini @@ -77,6 +77,7 @@ deps = requests-mock coverage[toml] -r{toxinidir}/requirements.txt + -r{[vars]tst_path}unit/requirements.txt commands = coverage run --source={[vars]src_path} \ -m pytest --ignore={[vars]tst_path}integration -v --tb native -s {posargs} From 9fdd2bbefdf313242eda456dc29ca3c107c51f58 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 25 Jan 2024 15:58:06 +0800 Subject: [PATCH 02/16] chore: run src-docs --- src-docs/charm_state.py.md | 6 +++--- src-docs/runner.py.md | 6 +++--- src-docs/runner_type.py.md | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src-docs/charm_state.py.md b/src-docs/charm_state.py.md index 2c3119bdd..fa1f893a0 100644 --- a/src-docs/charm_state.py.md +++ b/src-docs/charm_state.py.md @@ -183,7 +183,7 @@ SSH connection information for debug workflow. ### classmethod `from_charm` ```python -from_charm(charm: CharmBase) → Optional[ForwardRef('SSHDebugInfo')] +from_charm(charm: CharmBase) → List[ForwardRef('SSHDebugInfo')] ``` Initialize the SSHDebugInfo from charm relation data. @@ -208,14 +208,14 @@ The charm state. - `proxy_config`: Proxy-related configuration. - `charm_config`: Configuration of the juju charm. - `arch`: The underlying compute architecture, i.e. x86_64, amd64, arm64/aarch64. - - `ssh_debug_info`: The SSH debug connection configuration information. + - `ssh_debug_infos`: SSH debug connections configuration information. --- - + ### classmethod `from_charm` diff --git a/src-docs/runner.py.md b/src-docs/runner.py.md index 3de69178d..9916319b7 100644 --- a/src-docs/runner.py.md +++ b/src-docs/runner.py.md @@ -39,7 +39,7 @@ The configuration values for creating a single runner instance. ## class `Runner` Single instance of GitHub self-hosted runner. - + ### function `__init__` @@ -67,7 +67,7 @@ Construct the runner instance. --- - + ### function `create` @@ -91,7 +91,7 @@ Create the runner instance on LXD and register it on GitHub. --- - + ### function `remove` diff --git a/src-docs/runner_type.py.md b/src-docs/runner_type.py.md index a87aea240..5bf0f4743 100644 --- a/src-docs/runner_type.py.md +++ b/src-docs/runner_type.py.md @@ -83,7 +83,7 @@ Configuration for runner. - `path`: GitHub repository path in the format '/', or the GitHub organization name. - `proxies`: HTTP(S) proxy settings. - `dockerhub_mirror`: URL of dockerhub mirror to use. - - `ssh_debug_info`: The SSH debug server connection metadata. + - `ssh_debug_infos`: The SSH debug server connections metadata. From 0931c0a25ab60d077993471e7627be6fe9391ccb Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 25 Jan 2024 16:02:56 +0800 Subject: [PATCH 03/16] chore: type hint use default list --- src/charm_state.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/charm_state.py b/src/charm_state.py index 0ebaa8136..3178a62b9 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -9,7 +9,7 @@ import platform from enum import Enum from pathlib import Path -from typing import List, Optional +from typing import Optional from ops import CharmBase from pydantic import AnyHttpUrl, BaseModel, Field, ValidationError, root_validator @@ -200,7 +200,7 @@ class SSHDebugInfo(BaseModel): ed25519_fingerprint: str = Field(pattern="^SHA256:.*") @classmethod - def from_charm(cls, charm: CharmBase) -> List["SSHDebugInfo"]: + def from_charm(cls, charm: CharmBase) -> list["SSHDebugInfo"]: """Initialize the SSHDebugInfo from charm relation data. Args: @@ -249,7 +249,7 @@ class State: proxy_config: ProxyConfig charm_config: CharmConfig arch: ARCH - ssh_debug_infos: List[SSHDebugInfo] + ssh_debug_infos: list[SSHDebugInfo] @classmethod def from_charm(cls, charm: CharmBase) -> "State": From 755b7f699f1ec087742f814e3d53f16e1c2c2157 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 25 Jan 2024 16:03:32 +0800 Subject: [PATCH 04/16] test: add failed test description --- src-docs/charm_state.py.md | 2 +- tests/unit/test_runner.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src-docs/charm_state.py.md b/src-docs/charm_state.py.md index fa1f893a0..f4f3c00ee 100644 --- a/src-docs/charm_state.py.md +++ b/src-docs/charm_state.py.md @@ -183,7 +183,7 @@ SSH connection information for debug workflow. ### classmethod `from_charm` ```python -from_charm(charm: CharmBase) → List[ForwardRef('SSHDebugInfo')] +from_charm(charm: CharmBase) → list['SSHDebugInfo'] ``` Initialize the SSHDebugInfo from charm relation data. diff --git a/tests/unit/test_runner.py b/tests/unit/test_runner.py index 7569090d0..3ecfe49de 100644 --- a/tests/unit/test_runner.py +++ b/tests/unit/test_runner.py @@ -457,4 +457,6 @@ def test_random_ssh_connection_choice( runner._configure_runner() second_call_args = runner._clients.jinja.get_template("env.j2").render.call_args.kwargs - assert first_call_args["ssh_debug_info"] != second_call_args["ssh_debug_info"] + assert ( + first_call_args["ssh_debug_info"] != second_call_args["ssh_debug_info"] + ), "Same ssh debug info found, this may have occurred with a very low priority." From 730af138db47044c40018bd74ead05d9b131b3ae Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 25 Jan 2024 20:55:40 +0800 Subject: [PATCH 05/16] fix: json parsing --- src/charm_state.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/charm_state.py b/src/charm_state.py index 3178a62b9..21bcd1475 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -315,6 +315,9 @@ def from_charm(cls, charm: CharmBase) -> "State": # Convert pydantic object to python object serializable by json module. state_dict["proxy_config"] = json.loads(state_dict["proxy_config"].json()) state_dict["charm_config"] = json.loads(state_dict["charm_config"].json()) + state_dict["ssh_debug_infos"] = [ + debug_info.json() for debug_info in state_dict["ssh_debug_infos"] + ] json_data = json.dumps(state_dict, ensure_ascii=False) CHARM_STATE_PATH.write_text(json_data, encoding="utf-8") From 4b47105424349a722690f850cf28858485db246a Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 26 Jan 2024 12:09:01 +0800 Subject: [PATCH 06/16] feat: add tmate-ssh-server ips to allowlist --- src-docs/firewall.py.md | 7 ++- src/charm.py | 6 ++- src/firewall.py | 53 +++++++++++++------- tests/unit/test_charm.py | 61 ++++++++++++++++++++++- tests/unit/test_firewall.py | 97 +++++++++++++++++++++++++++++++++++++ 5 files changed, 203 insertions(+), 21 deletions(-) create mode 100644 tests/unit/test_firewall.py diff --git a/src-docs/firewall.py.md b/src-docs/firewall.py.md index 4d66b1546..2a583ee02 100644 --- a/src-docs/firewall.py.md +++ b/src-docs/firewall.py.md @@ -50,12 +50,15 @@ Get the host IP address for the corresponding LXD network. --- - + ### function `refresh_firewall` ```python -refresh_firewall(denylist: List[FirewallEntry]) +refresh_firewall( + denylist: List[FirewallEntry], + allowlist: Optional[List[FirewallEntry]] = None +) ``` Refresh the firewall configuration. diff --git a/src/charm.py b/src/charm.py index a058a85ac..84c941a3d 100755 --- a/src/charm.py +++ b/src/charm.py @@ -910,8 +910,11 @@ def _refresh_firewall(self): FirewallEntry.decode(entry.strip()) for entry in firewall_denylist_config.split(",") ] + allowlist = [ + FirewallEntry.decode(str(entry.host)) for entry in self._state.ssh_debug_infos + ] firewall = Firewall("lxdbr0") - firewall.refresh_firewall(denylist) + firewall.refresh_firewall(denylist, allowlist) logger.debug( "firewall update, current firewall: %s", execute_command(["/usr/sbin/nft", "list", "ruleset"]), @@ -930,6 +933,7 @@ def _apt_install(self, packages: Sequence[str]) -> None: def _on_debug_ssh_relation_changed(self, _: ops.RelationChangedEvent) -> None: """Handle debug ssh relation changed event.""" + self._refresh_firewall() runner_manager = self._get_runner_manager() runner_manager.flush(flush_busy=False) self._reconcile_runners(runner_manager) diff --git a/src/firewall.py b/src/firewall.py index 7cbf777d6..16bcb6fea 100644 --- a/src/firewall.py +++ b/src/firewall.py @@ -66,7 +66,25 @@ def get_host_ip(self) -> str: ) return str(ipaddress.IPv4Interface(address.strip()).ip) - def refresh_firewall(self, denylist: typing.List[FirewallEntry]): + def _exclude_network( + self, + networks: typing.List[ipaddress.IPv4Network], + excludes: typing.List[ipaddress.IPv4Network], + ) -> typing.Set[ipaddress.IPv4Network]: + total: typing.Set[ipaddress.IPv4Network] = set() + for exclude in excludes: + for network in networks: + try: + total.update(network.address_exclude(exclude)) + except ValueError: + total.update([network]) + return total # this currently doesn't work + + def refresh_firewall( + self, + denylist: typing.List[FirewallEntry], + allowlist: typing.List[FirewallEntry] | None = None, + ): """Refresh the firewall configuration. Args: @@ -128,23 +146,24 @@ def refresh_firewall(self, denylist: typing.List[FirewallEntry]): "state": "enabled", }, ] + host_network = ipaddress.IPv4Network(host_ip) - for entry in denylist: - entry_network = ipaddress.IPv4Network(entry.ip_range) - try: - excluded = list(entry_network.address_exclude(host_network)) - except ValueError: - excluded = [entry_network] - egress_rules.extend( - [ - { - "action": "reject", - "destination": str(ip), - "state": "enabled", - } - for ip in excluded - ] - ) + allowed_ips = [ + host_network, + *[ipaddress.IPv4Network(entry.ip_range) for entry in (allowlist or [])], + ] + ips_to_deny = [ipaddress.IPv4Network(entry.ip_range) for entry in denylist] + denied_ips = self._exclude_network(ips_to_deny, allowed_ips) + egress_rules.extend( + [ + { + "action": "reject", + "destination": str(ip), + "state": "enabled", + } + for ip in denied_ips + ] + ) acl_config["egress"] = egress_rules execute_command( ["lxc", "network", "acl", "edit", self._ACL_RULESET_NAME], diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 31939a64c..d7d2991f3 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -19,6 +19,7 @@ RunnerError, SubprocessError, ) +from firewall import FirewallEntry from github_type import GitHubRunnerStatus from runner_manager import RunnerInfo, RunnerManagerConfig from runner_type import GithubOrg, GithubRepo, VirtualMachineResources @@ -372,8 +373,8 @@ def test_check_runners_action_with_errors(self, run, wt, mkdir, rm): ) @patch("charm.RunnerManager") - @patch("pathlib.Path.mkdir") @patch("pathlib.Path.write_text") + @patch("pathlib.Path.mkdir") @patch("subprocess.run") def test_on_flush_runners_action(self, run, wt, mkdir, rm): mock_event = MagicMock() @@ -391,3 +392,61 @@ def test_on_flush_runners_action(self, run, wt, mkdir, rm): harness.charm._on_flush_runners_action(mock_event) mock_event.set_results.assert_called() mock_event.reset_mock() + + @patch("charm.RunnerManager") + @patch("pathlib.Path.write_text") + @patch("pathlib.Path.mkdir") + @patch("subprocess.run") + @patch("charm.Firewall") + def test__refresh_firewall(self, mock_firewall, *args): + """ + arrange: given multiple tmate-ssh-server units in relation. + act: when refresh_firewall is called. + assert: the unit ip addresses are included in allowlist. + """ + harness = Harness(GithubRunnerCharm) + relation_id = harness.add_relation("debug-ssh", "tmate-ssh-server") + harness.add_relation_unit(relation_id, "tmate-ssh-server/0") + harness.add_relation_unit(relation_id, "tmate-ssh-server/1") + harness.add_relation_unit(relation_id, "tmate-ssh-server/2") + test_unit_ip_addresses = ["127.0.0.1", "127.0.0.2", "127.0.0.3"] + + harness.update_relation_data( + relation_id, + "tmate-ssh-server/0", + { + "host": test_unit_ip_addresses[0], + "port": "10022", + "rsa_fingerprint": "SHA256:abcd", + "ed25519_fingerprint": "abcd", + }, + ) + harness.update_relation_data( + relation_id, + "tmate-ssh-server/1", + { + "host": test_unit_ip_addresses[1], + "port": "10022", + "rsa_fingerprint": "SHA256:abcd", + "ed25519_fingerprint": "abcd", + }, + ) + harness.update_relation_data( + relation_id, + "tmate-ssh-server/2", + { + "host": test_unit_ip_addresses[2], + "port": "10022", + "rsa_fingerprint": "SHA256:abcd", + "ed25519_fingerprint": "abcd", + }, + ) + + harness.begin() + + harness.charm._refresh_firewall() + mocked_firewall_instance = mock_firewall.return_value + allowlist = mocked_firewall_instance.refresh_firewall.call_args_list[0][0][1] + assert all( + FirewallEntry(ip) in allowlist for ip in test_unit_ip_addresses + ), "Expected IP firewall entry not found in allowlist arg." diff --git a/tests/unit/test_firewall.py b/tests/unit/test_firewall.py new file mode 100644 index 000000000..ac0316dfd --- /dev/null +++ b/tests/unit/test_firewall.py @@ -0,0 +1,97 @@ +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Test cases for firewall module.""" + +from ipaddress import IPv4Network + +import pytest + +from firewall import Firewall + + +@pytest.mark.parametrize( + "domain_ranges, excludes_ranges, expected", + [ + pytest.param( + [IPv4Network("10.0.0.0/8")], + [IPv4Network("10.136.12.36/32")], + { + IPv4Network("10.0.0.0/9"), + IPv4Network("10.192.0.0/10"), + IPv4Network("10.160.0.0/11"), + IPv4Network("10.144.0.0/12"), + IPv4Network("10.128.0.0/13"), + IPv4Network("10.140.0.0/14"), + IPv4Network("10.138.0.0/15"), + IPv4Network("10.137.0.0/16"), + IPv4Network("10.136.128.0/17"), + IPv4Network("10.136.64.0/18"), + IPv4Network("10.136.32.0/19"), + IPv4Network("10.136.16.0/20"), + IPv4Network("10.136.0.0/21"), + IPv4Network("10.136.8.0/22"), + IPv4Network("10.136.14.0/23"), + IPv4Network("10.136.13.0/24"), + IPv4Network("10.136.12.128/25"), + IPv4Network("10.136.12.64/26"), + IPv4Network("10.136.12.0/27"), + IPv4Network("10.136.12.48/28"), + IPv4Network("10.136.12.40/29"), + IPv4Network("10.136.12.32/30"), + IPv4Network("10.136.12.38/31"), + IPv4Network("10.136.12.37/32"), + }, + id="exclude a single IP", + ), + pytest.param( + [IPv4Network("10.0.0.0/8"), IPv4Network("192.0.2.0/28")], + [IPv4Network("10.136.12.36/32")], + { + IPv4Network("10.0.0.0/9"), + IPv4Network("10.192.0.0/10"), + IPv4Network("10.160.0.0/11"), + IPv4Network("10.144.0.0/12"), + IPv4Network("10.128.0.0/13"), + IPv4Network("10.140.0.0/14"), + IPv4Network("10.138.0.0/15"), + IPv4Network("10.137.0.0/16"), + IPv4Network("10.136.128.0/17"), + IPv4Network("10.136.64.0/18"), + IPv4Network("10.136.32.0/19"), + IPv4Network("10.136.16.0/20"), + IPv4Network("10.136.0.0/21"), + IPv4Network("10.136.8.0/22"), + IPv4Network("10.136.14.0/23"), + IPv4Network("10.136.13.0/24"), + IPv4Network("10.136.12.128/25"), + IPv4Network("10.136.12.64/26"), + IPv4Network("10.136.12.0/27"), + IPv4Network("10.136.12.48/28"), + IPv4Network("10.136.12.40/29"), + IPv4Network("10.136.12.32/30"), + IPv4Network("10.136.12.38/31"), + IPv4Network("10.136.12.37/32"), + IPv4Network("192.0.2.0/28"), + }, + id="exclude a single IP and one different subnet", + ), + pytest.param( + [IPv4Network("198.18.0.0/15"), IPv4Network("172.16.0.0/12")], + [IPv4Network("10.136.12.36/32")], + {IPv4Network("198.18.0.0/15"), IPv4Network("172.16.0.0/12")}, + id="no matching networks", + ), + ], +) +def test__exclude_network( + domain_ranges: list[IPv4Network], + excludes_ranges: list[IPv4Network], + expected: set[IPv4Network], +): + """ + arrange: given domain networks and some IPs to exclude from the domains. + act: when _exclude_network is called. + assert: new ip networks are returned with excluded target IP ranges. + """ + assert Firewall("test")._exclude_network(domain_ranges, excludes_ranges) == expected From bcc9aa569d5a339803f3198f27c48a244b4100eb Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 26 Jan 2024 21:40:09 +0800 Subject: [PATCH 07/16] test: add integration test --- src-docs/firewall.py.md | 2 +- src/charm.py | 2 +- src/firewall.py | 32 ++++++++++++++++++++--------- tests/integration/conftest.py | 2 +- tests/integration/test_debug_ssh.py | 18 +++++++++++++++- tests/unit/test_charm.py | 2 +- 6 files changed, 43 insertions(+), 15 deletions(-) diff --git a/src-docs/firewall.py.md b/src-docs/firewall.py.md index 2a583ee02..8b0996115 100644 --- a/src-docs/firewall.py.md +++ b/src-docs/firewall.py.md @@ -50,7 +50,7 @@ Get the host IP address for the corresponding LXD network. --- - + ### function `refresh_firewall` diff --git a/src/charm.py b/src/charm.py index 84c941a3d..02ad033cc 100755 --- a/src/charm.py +++ b/src/charm.py @@ -914,7 +914,7 @@ def _refresh_firewall(self): FirewallEntry.decode(str(entry.host)) for entry in self._state.ssh_debug_infos ] firewall = Firewall("lxdbr0") - firewall.refresh_firewall(denylist, allowlist) + firewall.refresh_firewall(denylist=denylist, allowlist=allowlist) logger.debug( "firewall update, current firewall: %s", execute_command(["/usr/sbin/nft", "list", "ruleset"]), diff --git a/src/firewall.py b/src/firewall.py index 16bcb6fea..a4c0459be 100644 --- a/src/firewall.py +++ b/src/firewall.py @@ -71,14 +71,27 @@ def _exclude_network( networks: typing.List[ipaddress.IPv4Network], excludes: typing.List[ipaddress.IPv4Network], ) -> typing.Set[ipaddress.IPv4Network]: - total: typing.Set[ipaddress.IPv4Network] = set() + """Excludes the network segment from a pool of networks. + + Args: + networks: The network pool to apply. + excludes: The networks to exclude from the pool. + + Returns: + The network pool without the network segments in excludes. + """ + total_excluded_networks = set(networks) + for exclude in excludes: - for network in networks: - try: - total.update(network.address_exclude(exclude)) - except ValueError: - total.update([network]) - return total # this currently doesn't work + scoped_excluded_networks: set[ipaddress.IPv4Network] = set() + for net in total_excluded_networks: + if net.overlaps(exclude): + scoped_excluded_networks.update(net.address_exclude(exclude)) + else: + scoped_excluded_networks.add(net) + total_excluded_networks = scoped_excluded_networks + + return total_excluded_networks def refresh_firewall( self, @@ -147,13 +160,12 @@ def refresh_firewall( }, ] - host_network = ipaddress.IPv4Network(host_ip) allowed_ips = [ - host_network, + ipaddress.IPv4Network(host_ip), *[ipaddress.IPv4Network(entry.ip_range) for entry in (allowlist or [])], ] ips_to_deny = [ipaddress.IPv4Network(entry.ip_range) for entry in denylist] - denied_ips = self._exclude_network(ips_to_deny, allowed_ips) + denied_ips = self._exclude_network(networks=ips_to_deny, excludes=allowed_ips) egress_rules.extend( [ { diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 622fe60ff..a49a6da4a 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -294,7 +294,7 @@ async def tmate_ssh_server_app_fixture( """tmate-ssh-server charm application related to GitHub-Runner app charm.""" tmate_app: Application = await model.deploy("tmate-ssh-server", channel="edge") await app_no_wait.relate("debug-ssh", f"{tmate_app.name}:debug-ssh") - await model.wait_for_idle(status=ACTIVE, timeout=60 * 30) + await model.wait_for_idle(apps=[tmate_app.name], status=ACTIVE, timeout=60 * 30) return tmate_app diff --git a/tests/integration/test_debug_ssh.py b/tests/integration/test_debug_ssh.py index cc5ab2178..7ab0a34ae 100644 --- a/tests/integration/test_debug_ssh.py +++ b/tests/integration/test_debug_ssh.py @@ -9,8 +9,10 @@ from github.Repository import Repository from github.WorkflowRun import WorkflowRun from juju.application import Application +from juju.model import Model from tests.integration.helpers import dispatch_workflow, get_job_logs, get_workflow_runs +from tests.status_name import ACTIVE logger = logging.getLogger(__name__) @@ -18,16 +20,30 @@ async def test_ssh_debug( + model: Model, app_no_wait: Application, github_repository: Repository, test_github_branch: Branch, tmate_ssh_server_unit_ip: str, ): """ - arrange: given an integrated GitHub-Runner charm and tmate-ssh-server charm. + arrange: given an integrated GitHub-Runner charm and tmate-ssh-server charm with a denylist + covering ip ranges of tmate-ssh-server. act: when canonical/action-tmate is triggered. assert: the ssh connection info from action-log and tmate-ssh-server matches. """ + await app_no_wait.set_config( + { + "denylist": ( + "0.0.0.0/8,10.0.0.0/8,100.64.0.0/10,169.254.0.0/16," + "172.16.0.0/12,192.0.0.0/24,192.0.2.0/24,192.88.99.0/24,192.168.0.0/16," + "198.18.0.0/15,198.51.100.0/24,203.0.113.0/24,224.0.0.0/4,233.252.0.0/24," + "240.0.0.0/4" + ), + } + ) + await model.wait_for_idle(status=ACTIVE, timeout=60 * 30) + # trigger tmate action logger.info("Dispatching workflow_dispatch_ssh_debug.yaml workflow.") start_time = datetime.now(timezone.utc) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index d7d2991f3..bf29e8aed 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -446,7 +446,7 @@ def test__refresh_firewall(self, mock_firewall, *args): harness.charm._refresh_firewall() mocked_firewall_instance = mock_firewall.return_value - allowlist = mocked_firewall_instance.refresh_firewall.call_args_list[0][0][1] + allowlist = mocked_firewall_instance.refresh_firewall.call_args_list[0][1]["allowlist"] assert all( FirewallEntry(ip) in allowlist for ip in test_unit_ip_addresses ), "Expected IP firewall entry not found in allowlist arg." From 4405ec51879950a5b2aa47bd4e307b40dc2e21a2 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 26 Jan 2024 22:02:36 +0800 Subject: [PATCH 08/16] chore: change typing --- src-docs/firewall.py.md | 8 ++++---- src/firewall.py | 10 ++++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src-docs/firewall.py.md b/src-docs/firewall.py.md index 8b0996115..36a1dee04 100644 --- a/src-docs/firewall.py.md +++ b/src-docs/firewall.py.md @@ -12,7 +12,7 @@ The runner firewall manager. ## class `Firewall` Represent a firewall and provides methods to refresh its configuration. - + ### function `__init__` @@ -33,7 +33,7 @@ Initialize a new Firewall instance. --- - + ### function `get_host_ip` @@ -50,7 +50,7 @@ Get the host IP address for the corresponding LXD network. --- - + ### function `refresh_firewall` @@ -86,7 +86,7 @@ Represent an entry in the firewall. --- - + ### classmethod `decode` diff --git a/src/firewall.py b/src/firewall.py index a4c0459be..970fd8291 100644 --- a/src/firewall.py +++ b/src/firewall.py @@ -11,6 +11,8 @@ from utilities import execute_command +NetworkT = typing.TypeVar("NetworkT", ipaddress.IPv4Network, ipaddress.IPv6Network) + @dataclasses.dataclass class FirewallEntry: @@ -68,9 +70,9 @@ def get_host_ip(self) -> str: def _exclude_network( self, - networks: typing.List[ipaddress.IPv4Network], - excludes: typing.List[ipaddress.IPv4Network], - ) -> typing.Set[ipaddress.IPv4Network]: + networks: typing.List[NetworkT], + excludes: typing.List[NetworkT], + ) -> typing.Set[NetworkT]: """Excludes the network segment from a pool of networks. Args: @@ -83,7 +85,7 @@ def _exclude_network( total_excluded_networks = set(networks) for exclude in excludes: - scoped_excluded_networks: set[ipaddress.IPv4Network] = set() + scoped_excluded_networks: set[NetworkT] = set() for net in total_excluded_networks: if net.overlaps(exclude): scoped_excluded_networks.update(net.address_exclude(exclude)) From ea26b9bf53732198b5b3ad16e834d195e5cc1965 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 26 Jan 2024 22:38:47 +0800 Subject: [PATCH 09/16] chore: rename var --- src/firewall.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/firewall.py b/src/firewall.py index 970fd8291..d99d866c0 100644 --- a/src/firewall.py +++ b/src/firewall.py @@ -82,18 +82,18 @@ def _exclude_network( Returns: The network pool without the network segments in excludes. """ - total_excluded_networks = set(networks) + total_networks_without_excluded = set(networks) for exclude in excludes: scoped_excluded_networks: set[NetworkT] = set() - for net in total_excluded_networks: + for net in total_networks_without_excluded: if net.overlaps(exclude): scoped_excluded_networks.update(net.address_exclude(exclude)) else: scoped_excluded_networks.add(net) - total_excluded_networks = scoped_excluded_networks + total_networks_without_excluded = scoped_excluded_networks - return total_excluded_networks + return total_networks_without_excluded def refresh_firewall( self, From f0a42344cf81a44ab53b59d031b2e18edc309890 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 29 Jan 2024 10:39:15 +0800 Subject: [PATCH 10/16] chore: update type hints --- src-docs/firewall.py.md | 4 ++-- src/firewall.py | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src-docs/firewall.py.md b/src-docs/firewall.py.md index 36a1dee04..fcfd3c7f9 100644 --- a/src-docs/firewall.py.md +++ b/src-docs/firewall.py.md @@ -56,8 +56,8 @@ Get the host IP address for the corresponding LXD network. ```python refresh_firewall( - denylist: List[FirewallEntry], - allowlist: Optional[List[FirewallEntry]] = None + denylist: Iterable[FirewallEntry], + allowlist: Optional[Iterable[FirewallEntry]] = None ) ``` diff --git a/src/firewall.py b/src/firewall.py index d99d866c0..e4cd2f30f 100644 --- a/src/firewall.py +++ b/src/firewall.py @@ -70,9 +70,9 @@ def get_host_ip(self) -> str: def _exclude_network( self, - networks: typing.List[NetworkT], - excludes: typing.List[NetworkT], - ) -> typing.Set[NetworkT]: + networks: typing.Iterable[NetworkT], + excludes: typing.Iterable[NetworkT], + ) -> set[NetworkT]: """Excludes the network segment from a pool of networks. Args: @@ -97,8 +97,8 @@ def _exclude_network( def refresh_firewall( self, - denylist: typing.List[FirewallEntry], - allowlist: typing.List[FirewallEntry] | None = None, + denylist: typing.Iterable[FirewallEntry], + allowlist: typing.Iterable[FirewallEntry] | None = None, ): """Refresh the firewall configuration. From b09b4ec5112f6b7febbb0dc0993ca6d255b401eb Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 29 Jan 2024 10:42:23 +0800 Subject: [PATCH 11/16] chore: update ssh debug info name --- src-docs/charm_state.py.md | 6 +++--- src-docs/runner_type.py.md | 2 +- src/charm.py | 2 +- src/charm_state.py | 20 ++++++++++---------- src/runner.py | 8 +++++--- src/runner_manager.py | 4 ++-- src/runner_type.py | 6 +++--- tests/unit/factories.py | 6 +++--- tests/unit/test_charm_state.py | 16 +++++++++------- tests/unit/test_runner.py | 12 ++++++------ tests/unit/test_runner_manager.py | 2 +- 11 files changed, 44 insertions(+), 40 deletions(-) diff --git a/src-docs/charm_state.py.md b/src-docs/charm_state.py.md index f4f3c00ee..4c9a3e4cb 100644 --- a/src-docs/charm_state.py.md +++ b/src-docs/charm_state.py.md @@ -161,7 +161,7 @@ Supported storage as runner disk. --- -## class `SSHDebugInfo` +## class `SSHDebugConnection` SSH connection information for debug workflow. @@ -183,7 +183,7 @@ SSH connection information for debug workflow. ### classmethod `from_charm` ```python -from_charm(charm: CharmBase) → list['SSHDebugInfo'] +from_charm(charm: CharmBase) → list['SSHDebugConnection'] ``` Initialize the SSHDebugInfo from charm relation data. @@ -208,7 +208,7 @@ The charm state. - `proxy_config`: Proxy-related configuration. - `charm_config`: Configuration of the juju charm. - `arch`: The underlying compute architecture, i.e. x86_64, amd64, arm64/aarch64. - - `ssh_debug_infos`: SSH debug connections configuration information. + - `ssh_debug_connections`: SSH debug connections configuration information. diff --git a/src-docs/runner_type.py.md b/src-docs/runner_type.py.md index 5bf0f4743..21a1f590e 100644 --- a/src-docs/runner_type.py.md +++ b/src-docs/runner_type.py.md @@ -83,7 +83,7 @@ Configuration for runner. - `path`: GitHub repository path in the format '/', or the GitHub organization name. - `proxies`: HTTP(S) proxy settings. - `dockerhub_mirror`: URL of dockerhub mirror to use. - - `ssh_debug_infos`: The SSH debug server connections metadata. + - `ssh_debug_connections`: The SSH debug server connections metadata. diff --git a/src/charm.py b/src/charm.py index 02ad033cc..9f907d9b4 100755 --- a/src/charm.py +++ b/src/charm.py @@ -911,7 +911,7 @@ def _refresh_firewall(self): for entry in firewall_denylist_config.split(",") ] allowlist = [ - FirewallEntry.decode(str(entry.host)) for entry in self._state.ssh_debug_infos + FirewallEntry.decode(str(entry.host)) for entry in self._state.ssh_debug_connections ] firewall = Firewall("lxdbr0") firewall.refresh_firewall(denylist=denylist, allowlist=allowlist) diff --git a/src/charm_state.py b/src/charm_state.py index 21bcd1475..001eea0a5 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -184,7 +184,7 @@ def _get_supported_arch() -> ARCH: raise UnsupportedArchitectureError(arch=arch) -class SSHDebugInfo(BaseModel): +class SSHDebugConnection(BaseModel): """SSH connection information for debug workflow. Attributes: @@ -200,13 +200,13 @@ class SSHDebugInfo(BaseModel): ed25519_fingerprint: str = Field(pattern="^SHA256:.*") @classmethod - def from_charm(cls, charm: CharmBase) -> list["SSHDebugInfo"]: + def from_charm(cls, charm: CharmBase) -> list["SSHDebugConnection"]: """Initialize the SSHDebugInfo from charm relation data. Args: charm: The charm instance. """ - ssh_debug_connections: list[SSHDebugInfo] = [] + ssh_debug_connections: list[SSHDebugConnection] = [] relations = charm.model.relations[DEBUG_SSH_INTEGRATION_NAME] if not relations or not (relation := relations[0]).units: return ssh_debug_connections @@ -223,7 +223,7 @@ def from_charm(cls, charm: CharmBase) -> list["SSHDebugInfo"]: ) continue ssh_debug_connections.append( - SSHDebugInfo( + SSHDebugConnection( host=host, port=port, rsa_fingerprint=rsa_fingerprint, @@ -242,14 +242,14 @@ class State: proxy_config: Proxy-related configuration. charm_config: Configuration of the juju charm. arch: The underlying compute architecture, i.e. x86_64, amd64, arm64/aarch64. - ssh_debug_infos: SSH debug connections configuration information. + ssh_debug_connections: SSH debug connections configuration information. """ is_metrics_logging_available: bool proxy_config: ProxyConfig charm_config: CharmConfig arch: ARCH - ssh_debug_infos: list[SSHDebugInfo] + ssh_debug_connections: list[SSHDebugConnection] @classmethod def from_charm(cls, charm: CharmBase) -> "State": @@ -298,7 +298,7 @@ def from_charm(cls, charm: CharmBase) -> "State": raise CharmConfigInvalidError(f"Unsupported architecture {exc.arch}") from exc try: - ssh_debug_infos = SSHDebugInfo.from_charm(charm) + ssh_debug_connections = SSHDebugConnection.from_charm(charm) except ValidationError as exc: logger.error("Invalid SSH debug info: %s.", exc) raise CharmConfigInvalidError("Invalid SSH Debug info") from exc @@ -308,15 +308,15 @@ def from_charm(cls, charm: CharmBase) -> "State": proxy_config=proxy_config, charm_config=charm_config, arch=arch, - ssh_debug_infos=ssh_debug_infos, + ssh_debug_connections=ssh_debug_connections, ) state_dict = dataclasses.asdict(state) # Convert pydantic object to python object serializable by json module. state_dict["proxy_config"] = json.loads(state_dict["proxy_config"].json()) state_dict["charm_config"] = json.loads(state_dict["charm_config"].json()) - state_dict["ssh_debug_infos"] = [ - debug_info.json() for debug_info in state_dict["ssh_debug_infos"] + state_dict["ssh_debug_connections"] = [ + debug_info.json() for debug_info in state_dict["ssh_debug_connections"] ] json_data = json.dumps(state_dict, ensure_ascii=False) CHARM_STATE_PATH.write_text(json_data, encoding="utf-8") diff --git a/src/runner.py b/src/runner.py index 2bfbb6e45..f5343d363 100644 --- a/src/runner.py +++ b/src/runner.py @@ -23,7 +23,7 @@ import yaml import shared_fs -from charm_state import ARCH, SSHDebugInfo +from charm_state import ARCH, SSHDebugConnection from errors import ( CreateSharedFilesystemError, LxdError, @@ -672,8 +672,10 @@ def _configure_runner(self) -> None: # As the user already has sudo access, this does not give the user any additional access. self.instance.execute(["/usr/bin/sudo", "chmod", "777", "/usr/local/bin"]) - selected_ssh_connection: SSHDebugInfo | None = ( - secrets.choice(self.config.ssh_debug_infos) if self.config.ssh_debug_infos else None + selected_ssh_connection: SSHDebugConnection | None = ( + secrets.choice(self.config.ssh_debug_connections) + if self.config.ssh_debug_connections + else None ) logger.info("SSH Debug info: %s", selected_ssh_connection) # Load `/etc/environment` file. diff --git a/src/runner_manager.py b/src/runner_manager.py index d85203d63..6ea32b0ed 100644 --- a/src/runner_manager.py +++ b/src/runner_manager.py @@ -409,7 +409,7 @@ def _spawn_new_runners(self, count: int, resources: VirtualMachineResources): path=self.config.path, proxies=self.proxies, name=self._generate_runner_name(), - ssh_debug_infos=self.config.charm_state.ssh_debug_infos, + ssh_debug_connections=self.config.charm_state.ssh_debug_connections, ) runner = Runner(self._clients, config, RunnerStatus()) try: @@ -611,7 +611,7 @@ def create_runner_info( name=name, path=self.config.path, proxies=self.proxies, - ssh_debug_infos=self.config.charm_state.ssh_debug_infos, + ssh_debug_connections=self.config.charm_state.ssh_debug_connections, ) return Runner( self._clients, diff --git a/src/runner_type.py b/src/runner_type.py index abdb36ae2..ae3303283 100644 --- a/src/runner_type.py +++ b/src/runner_type.py @@ -8,7 +8,7 @@ from pathlib import Path from typing import NamedTuple, Optional, TypedDict, Union -from charm_state import SSHDebugInfo +from charm_state import SSHDebugConnection @dataclass @@ -77,7 +77,7 @@ class RunnerConfig: # pylint: disable=too-many-instance-attributes name. proxies: HTTP(S) proxy settings. dockerhub_mirror: URL of dockerhub mirror to use. - ssh_debug_infos: The SSH debug server connections metadata. + ssh_debug_connections: The SSH debug server connections metadata. """ app_name: str @@ -87,7 +87,7 @@ class RunnerConfig: # pylint: disable=too-many-instance-attributes path: GithubPath proxies: ProxySetting dockerhub_mirror: str | None = None - ssh_debug_infos: SSHDebugInfo | None = None + ssh_debug_connections: SSHDebugConnection | None = None @dataclass diff --git a/tests/unit/factories.py b/tests/unit/factories.py index 55733a2e3..7c61c21c2 100644 --- a/tests/unit/factories.py +++ b/tests/unit/factories.py @@ -13,7 +13,7 @@ import factory.fuzzy from pydantic.networks import IPvAnyAddress -from charm_state import SSHDebugInfo +from charm_state import SSHDebugConnection T = TypeVar("T") @@ -30,7 +30,7 @@ def __call__(cls, *args, **kwargs) -> T: # noqa: N805 # The attributes of these classes are generators for the attributes of the meta class # mypy incorrectly believes the factories don't support metaclass class SSHDebugInfoFactory( - factory.Factory, metaclass=BaseMetaFactory[SSHDebugInfo] # type: ignore + factory.Factory, metaclass=BaseMetaFactory[SSHDebugConnection] # type: ignore ): # Docstrings have been abbreviated for factories, checking for docstrings on model attributes # can be skipped. @@ -39,7 +39,7 @@ class SSHDebugInfoFactory( class Meta: """Configuration for factory.""" # noqa: DCO060 - model = SSHDebugInfo + model = SSHDebugConnection abstract = False host: IPvAnyAddress = factory.Faker("ipv4") diff --git a/tests/unit/test_charm_state.py b/tests/unit/test_charm_state.py index 8e1c77ee6..b110f491e 100644 --- a/tests/unit/test_charm_state.py +++ b/tests/unit/test_charm_state.py @@ -14,7 +14,7 @@ COS_AGENT_INTEGRATION_NAME, DEBUG_SSH_INTEGRATION_NAME, CharmConfigInvalidError, - SSHDebugInfo, + SSHDebugConnection, State, ) @@ -132,7 +132,7 @@ def test_ssh_debug_info_from_charm_no_relations(): mock_charm = MagicMock(spec=GithubRunnerCharm) mock_charm.model.relations = {DEBUG_SSH_INTEGRATION_NAME: []} - assert not SSHDebugInfo.from_charm(mock_charm) + assert not SSHDebugConnection.from_charm(mock_charm) @pytest.mark.parametrize( @@ -220,11 +220,13 @@ def test_from_charm_ssh_debug_info(): mock_charm.app.name = "github-runner-operator" mock_charm.unit.name = "github-runner-operator/0" - ssh_debug_infos = State.from_charm(mock_charm).ssh_debug_infos - assert str(ssh_debug_infos[0].host) == mock_relation_data["host"] - assert str(ssh_debug_infos[0].port) == mock_relation_data["port"] - assert ssh_debug_infos[0].rsa_fingerprint == mock_relation_data["rsa_fingerprint"] - assert ssh_debug_infos[0].ed25519_fingerprint == mock_relation_data["ed25519_fingerprint"] + ssh_debug_connections = State.from_charm(mock_charm).ssh_debug_connections + assert str(ssh_debug_connections[0].host) == mock_relation_data["host"] + assert str(ssh_debug_connections[0].port) == mock_relation_data["port"] + assert ssh_debug_connections[0].rsa_fingerprint == mock_relation_data["rsa_fingerprint"] + assert ( + ssh_debug_connections[0].ed25519_fingerprint == mock_relation_data["ed25519_fingerprint"] + ) def test_invalid_runner_storage(): diff --git a/tests/unit/test_runner.py b/tests/unit/test_runner.py index 3ecfe49de..e298005bc 100644 --- a/tests/unit/test_runner.py +++ b/tests/unit/test_runner.py @@ -12,7 +12,7 @@ import pytest from _pytest.monkeypatch import MonkeyPatch -from charm_state import SSHDebugInfo +from charm_state import SSHDebugConnection from errors import CreateSharedFilesystemError, RunnerCreateError, RunnerRemoveError from runner import CreateRunnerConfig, Runner, RunnerConfig, RunnerStatus from runner_manager_type import RunnerManagerClients @@ -91,9 +91,9 @@ def jinja2_environment_fixture() -> MagicMock: return jinja2_mock -@pytest.fixture(scope="function", name="ssh_debug_infos") -def ssh_debug_infos_fixture() -> list[SSHDebugInfo]: - """A list of randomly generated ssh_debug_infos.""" +@pytest.fixture(scope="function", name="ssh_debug_connections") +def ssh_debug_connections_fixture() -> list[SSHDebugConnection]: + """A list of randomly generated ssh_debug_connections.""" return SSHDebugInfoFactory.create_batch(size=100) @@ -113,7 +113,7 @@ def runner_fixture( lxd: MockLxdClient, jinja: MagicMock, tmp_path: Path, - ssh_debug_infos: list[SSHDebugInfo], + ssh_debug_connections: list[SSHDebugConnection], ): client = RunnerManagerClients( MagicMock(), @@ -131,7 +131,7 @@ def runner_fixture( lxd_storage_path=pool_path, dockerhub_mirror=None, issue_metrics=False, - ssh_debug_infos=ssh_debug_infos, + ssh_debug_connections=ssh_debug_connections, ) status = RunnerStatus() return Runner( diff --git a/tests/unit/test_runner_manager.py b/tests/unit/test_runner_manager.py index 2e778c939..02af54082 100644 --- a/tests/unit/test_runner_manager.py +++ b/tests/unit/test_runner_manager.py @@ -35,7 +35,7 @@ def charm_state_fixture(): mock = MagicMock(spec=State) mock.is_metrics_logging_available = False mock.arch = ARCH.X64 - mock.ssh_debug_infos = None + mock.ssh_debug_connections = None return mock From 8e19933176162ecf9c2068ce6da89378ab45fc6e Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 29 Jan 2024 10:44:43 +0800 Subject: [PATCH 12/16] chore: update var naem excludes to exclude --- src/firewall.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/firewall.py b/src/firewall.py index e4cd2f30f..21c65dbd1 100644 --- a/src/firewall.py +++ b/src/firewall.py @@ -71,24 +71,24 @@ def get_host_ip(self) -> str: def _exclude_network( self, networks: typing.Iterable[NetworkT], - excludes: typing.Iterable[NetworkT], + exclude: typing.Iterable[NetworkT], ) -> set[NetworkT]: """Excludes the network segment from a pool of networks. Args: networks: The network pool to apply. - excludes: The networks to exclude from the pool. + exclude: The networks to exclude from the pool. Returns: The network pool without the network segments in excludes. """ total_networks_without_excluded = set(networks) - for exclude in excludes: + for exclude_net in exclude: scoped_excluded_networks: set[NetworkT] = set() for net in total_networks_without_excluded: - if net.overlaps(exclude): - scoped_excluded_networks.update(net.address_exclude(exclude)) + if net.overlaps(exclude_net): + scoped_excluded_networks.update(net.address_exclude(exclude_net)) else: scoped_excluded_networks.add(net) total_networks_without_excluded = scoped_excluded_networks @@ -167,7 +167,7 @@ def refresh_firewall( *[ipaddress.IPv4Network(entry.ip_range) for entry in (allowlist or [])], ] ips_to_deny = [ipaddress.IPv4Network(entry.ip_range) for entry in denylist] - denied_ips = self._exclude_network(networks=ips_to_deny, excludes=allowed_ips) + denied_ips = self._exclude_network(networks=ips_to_deny, exclude=allowed_ips) egress_rules.extend( [ { From 5dd54ed208e2acf2ab146f9c163605ad854a5181 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 29 Jan 2024 14:43:49 +0800 Subject: [PATCH 13/16] test: firewall exclude network tests --- src-docs/firewall.py.md | 2 +- src/firewall.py | 16 +-- tests/unit/test_firewall.py | 204 +++++++++++++++++++++++------------- 3 files changed, 142 insertions(+), 80 deletions(-) diff --git a/src-docs/firewall.py.md b/src-docs/firewall.py.md index fcfd3c7f9..b5c30dc37 100644 --- a/src-docs/firewall.py.md +++ b/src-docs/firewall.py.md @@ -50,7 +50,7 @@ Get the host IP address for the corresponding LXD network. --- - + ### function `refresh_firewall` diff --git a/src/firewall.py b/src/firewall.py index 21c65dbd1..e1a1a1425 100644 --- a/src/firewall.py +++ b/src/firewall.py @@ -70,9 +70,9 @@ def get_host_ip(self) -> str: def _exclude_network( self, - networks: typing.Iterable[NetworkT], - exclude: typing.Iterable[NetworkT], - ) -> set[NetworkT]: + networks: list[NetworkT], + exclude: list[NetworkT], + ) -> list[NetworkT]: """Excludes the network segment from a pool of networks. Args: @@ -82,15 +82,17 @@ def _exclude_network( Returns: The network pool without the network segments in excludes. """ - total_networks_without_excluded = set(networks) + total_networks_without_excluded = networks for exclude_net in exclude: - scoped_excluded_networks: set[NetworkT] = set() + scoped_excluded_networks: list[NetworkT] = [] for net in total_networks_without_excluded: + if net == exclude_net or net.subnet_of(exclude_net): + continue if net.overlaps(exclude_net): - scoped_excluded_networks.update(net.address_exclude(exclude_net)) + scoped_excluded_networks.extend(net.address_exclude(exclude_net)) else: - scoped_excluded_networks.add(net) + scoped_excluded_networks.append(net) total_networks_without_excluded = scoped_excluded_networks return total_networks_without_excluded diff --git a/tests/unit/test_firewall.py b/tests/unit/test_firewall.py index ac0316dfd..2b97a0319 100644 --- a/tests/unit/test_firewall.py +++ b/tests/unit/test_firewall.py @@ -3,7 +3,7 @@ """Test cases for firewall module.""" -from ipaddress import IPv4Network +from ipaddress import IPv4Address, IPv4Network import pytest @@ -11,87 +11,147 @@ @pytest.mark.parametrize( - "domain_ranges, excludes_ranges, expected", + "domain_ranges, exclude_ranges, expected", [ + pytest.param([], [], [], id="empty domain[no exclude]"), + pytest.param([], (IPv4Network("127.0.0.1/32")), [], id="empty domain[one ip exclude]"), pytest.param( - [IPv4Network("10.0.0.0/8")], - [IPv4Network("10.136.12.36/32")], - { - IPv4Network("10.0.0.0/9"), - IPv4Network("10.192.0.0/10"), - IPv4Network("10.160.0.0/11"), - IPv4Network("10.144.0.0/12"), - IPv4Network("10.128.0.0/13"), - IPv4Network("10.140.0.0/14"), - IPv4Network("10.138.0.0/15"), - IPv4Network("10.137.0.0/16"), - IPv4Network("10.136.128.0/17"), - IPv4Network("10.136.64.0/18"), - IPv4Network("10.136.32.0/19"), - IPv4Network("10.136.16.0/20"), - IPv4Network("10.136.0.0/21"), - IPv4Network("10.136.8.0/22"), - IPv4Network("10.136.14.0/23"), - IPv4Network("10.136.13.0/24"), - IPv4Network("10.136.12.128/25"), - IPv4Network("10.136.12.64/26"), - IPv4Network("10.136.12.0/27"), - IPv4Network("10.136.12.48/28"), - IPv4Network("10.136.12.40/29"), - IPv4Network("10.136.12.32/30"), - IPv4Network("10.136.12.38/31"), - IPv4Network("10.136.12.37/32"), - }, - id="exclude a single IP", - ), - pytest.param( - [IPv4Network("10.0.0.0/8"), IPv4Network("192.0.2.0/28")], - [IPv4Network("10.136.12.36/32")], - { - IPv4Network("10.0.0.0/9"), - IPv4Network("10.192.0.0/10"), - IPv4Network("10.160.0.0/11"), - IPv4Network("10.144.0.0/12"), - IPv4Network("10.128.0.0/13"), - IPv4Network("10.140.0.0/14"), - IPv4Network("10.138.0.0/15"), - IPv4Network("10.137.0.0/16"), - IPv4Network("10.136.128.0/17"), - IPv4Network("10.136.64.0/18"), - IPv4Network("10.136.32.0/19"), - IPv4Network("10.136.16.0/20"), - IPv4Network("10.136.0.0/21"), - IPv4Network("10.136.8.0/22"), - IPv4Network("10.136.14.0/23"), - IPv4Network("10.136.13.0/24"), - IPv4Network("10.136.12.128/25"), - IPv4Network("10.136.12.64/26"), - IPv4Network("10.136.12.0/27"), - IPv4Network("10.136.12.48/28"), - IPv4Network("10.136.12.40/29"), - IPv4Network("10.136.12.32/30"), - IPv4Network("10.136.12.38/31"), - IPv4Network("10.136.12.37/32"), - IPv4Network("192.0.2.0/28"), - }, - id="exclude a single IP and one different subnet", - ), - pytest.param( - [IPv4Network("198.18.0.0/15"), IPv4Network("172.16.0.0/12")], - [IPv4Network("10.136.12.36/32")], - {IPv4Network("198.18.0.0/15"), IPv4Network("172.16.0.0/12")}, - id="no matching networks", + [], + [IPv4Network("127.0.0.1/32"), IPv4Network("127.0.0.2/32")], + [], + id="empty domain[multiple ips exclude]", + ), + pytest.param( + [IPv4Network("127.0.0.1/32")], + [IPv4Network("127.0.0.2/32")], + [IPv4Network("127.0.0.1/32")], + id="single ip single exclude ip[no overlap]", + ), + pytest.param( + [IPv4Network("127.0.0.1/32")], + [IPv4Network("127.0.0.1/32")], + [], + id="single ip single exclude ip[overlap]", + ), + pytest.param( + [IPv4Network("127.0.0.0/30")], + [IPv4Network("127.0.0.2/32")], + [IPv4Network("127.0.0.0/31"), IPv4Network("127.0.0.3/32")], + id="single domain single exclude ip[overlap single ip]", + ), + pytest.param( + [IPv4Network("127.0.0.0/28")], # 127.0.0.0-14 + [IPv4Network("127.0.0.1/32"), IPv4Network("127.0.1.1/32")], + [ + IPv4Network("127.0.0.0/32"), # 127.0.0.0 + IPv4Network("127.0.0.2/31"), # 127.0.0.2-3 + IPv4Network("127.0.0.4/30"), # 127.0.0.4-7 + IPv4Network("127.0.0.8/29"), # 127.0.0.8-14 + ], + id="single domain multiple exclude ips[overlap partial ips]", + ), + pytest.param( + [IPv4Network("127.0.0.0/30")], # 127.0.0.0-3 + [ + IPv4Network("127.0.0.0/32"), + IPv4Network("127.0.0.1/32"), + IPv4Network("127.0.0.2/32"), + IPv4Network("127.0.0.3/32"), + ], + [], + id="single domain multiple exclude ips[overlap all ips]", + ), + pytest.param( + [IPv4Network("127.0.0.0/30")], + [IPv4Network("127.0.1.0/30")], + [IPv4Network("127.0.0.0/30")], + id="single domain single exclude domain[no overlap]", + ), + pytest.param( + [IPv4Network("127.0.0.0/28")], # 127.0.0.0-15 + [IPv4Network("127.0.0.0/30")], # 127.0.0.0-4 + [ + IPv4Network("127.0.0.8/29"), # 127.0.0.8-15 + IPv4Network("127.0.0.4/30"), # 127.0.0.5-7 + ], + id="single domain single exclude domain[overlap partial range]", + ), + pytest.param( + [IPv4Network("127.0.0.0/30")], + [IPv4Network("127.0.0.0/30")], + [], + id="single domain single exclude domain[overlap full range]", + ), + pytest.param( + [IPv4Network("127.0.0.0/30"), IPv4Network("127.0.1.0/30")], + [IPv4Network("127.0.2.0/30")], + [IPv4Network("127.0.0.0/30"), IPv4Network("127.0.1.0/30")], + id="multiple domain single exclude domain[no overlap]", + ), + pytest.param( + [IPv4Network("127.0.0.0/28"), IPv4Network("127.0.1.0/28")], + [IPv4Network("127.0.0.0/30")], + [ + IPv4Network("127.0.0.8/29"), # 127.0.0.8-15 + IPv4Network("127.0.0.4/30"), # 127.0.0.5-7 + IPv4Network("127.0.1.0/28"), + ], + id="multiple domain single exclude domain[partial overlap]", + ), + pytest.param( + [IPv4Network("127.0.0.0/30"), IPv4Network("127.0.1.0/30")], + [IPv4Network("127.0.1.0/30")], + [IPv4Network("127.0.0.0/30")], + id="multiple domain single exclude domain[full overlap(equivalent network)]", + ), + pytest.param( + [IPv4Network("127.0.0.0/30"), IPv4Network("127.0.1.0/30")], + [IPv4Network("127.0.0.0/8")], + [], + id="multiple domain single exclude domain[full overlap(bigger network)]", + ), + pytest.param( + [IPv4Network("127.0.0.0/30"), IPv4Network("127.0.1.0/30")], + [IPv4Network("127.0.2.0/30"), IPv4Network("127.0.3.0/30")], + [IPv4Network("127.0.0.0/30"), IPv4Network("127.0.1.0/30")], + id="multiple domain multiple exclude domain[no overlaps]", + ), + pytest.param( + [IPv4Network("127.0.0.0/28"), IPv4Network("127.0.1.0/28")], + [IPv4Network("127.0.0.0/30"), IPv4Network("127.0.1.0/30")], + [ + IPv4Network("127.0.0.4/30"), # 127.0.0.5-7 + IPv4Network("127.0.0.8/29"), # 127.0.0.8-15 + IPv4Network("127.0.1.4/30"), # 127.0.1.5-7 + IPv4Network("127.0.1.8/29"), # 127.0.1.8-15 + ], + id="multiple domain multiple exclude domain[multiple partial overlaps]", + ), + pytest.param( + [IPv4Network("127.0.0.0/30"), IPv4Network("127.0.1.0/30")], + [IPv4Network("127.0.0.0/30"), IPv4Network("127.0.1.0/30")], + [], + id="multiple domain multiple exclude domain[multiple full overlaps(equivalent network)]", + ), + pytest.param( + [IPv4Network("127.0.0.0/30"), IPv4Network("127.0.1.0/30")], + [IPv4Network("127.0.0.0/8")], + [], + id="multiple domain multiple exclude domain[multiple full overlaps(bigger network)]", ), ], ) def test__exclude_network( domain_ranges: list[IPv4Network], - excludes_ranges: list[IPv4Network], - expected: set[IPv4Network], + exclude_ranges: list[IPv4Network], + expected: list[IPv4Network], ): """ arrange: given domain networks and some IPs to exclude from the domains. act: when _exclude_network is called. assert: new ip networks are returned with excluded target IP ranges. """ - assert Firewall("test")._exclude_network(domain_ranges, excludes_ranges) == expected + result = Firewall("test")._exclude_network(domain_ranges, exclude_ranges) + assert all(net in result for net in expected) and all( + net in expected for net in result + ), f"Difference in networks found, expected: {expected}, got: {result}." From 391eae011f21a09b5f39f876e4ab55ebfe76f39f Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 29 Jan 2024 14:48:45 +0800 Subject: [PATCH 14/16] docs: firewall rule for ssh-debug --- docs/explanation/ssh-debug.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/explanation/ssh-debug.md b/docs/explanation/ssh-debug.md index 568465cfc..84a525455 100644 --- a/docs/explanation/ssh-debug.md +++ b/docs/explanation/ssh-debug.md @@ -10,3 +10,9 @@ by default on [tmate-ssh-server charm](https://charmhub.io/tmate-ssh-server/). Authorized keys are registered via [action-tmate](https://github.com/canonical/action-tmate/)'s `limit-access-to-actor` feature. This feature uses GitHub users's SSH key to launch an instance of tmate session with `-a` option, which adds the user's SSH key to `~/.ssh/authorized_keys`. + +### Firewall rules + +By default, if there are any overlapping IPs within the `denylist` config option with the IP +assigned to `tmate-ssh-server`, an exception to that IP will be made so that the `debug-ssh` +relation can be setup correctly. From 4f8d630b28d61110e2682f63d9d6f6791f8ea275 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 29 Jan 2024 15:19:19 +0800 Subject: [PATCH 15/16] fix: lint --- tests/unit/test_firewall.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_firewall.py b/tests/unit/test_firewall.py index 2b97a0319..521fa5a2c 100644 --- a/tests/unit/test_firewall.py +++ b/tests/unit/test_firewall.py @@ -131,7 +131,10 @@ [IPv4Network("127.0.0.0/30"), IPv4Network("127.0.1.0/30")], [IPv4Network("127.0.0.0/30"), IPv4Network("127.0.1.0/30")], [], - id="multiple domain multiple exclude domain[multiple full overlaps(equivalent network)]", + id=( + "multiple domain multiple exclude domain[multiple full " + "overlaps(equivalent network)]" + ), ), pytest.param( [IPv4Network("127.0.0.0/30"), IPv4Network("127.0.1.0/30")], From 01e860b960dade637129299784a42573cbe4e789 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 29 Jan 2024 15:34:14 +0800 Subject: [PATCH 16/16] fix: lint --- tests/unit/test_firewall.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_firewall.py b/tests/unit/test_firewall.py index 521fa5a2c..917c0bde4 100644 --- a/tests/unit/test_firewall.py +++ b/tests/unit/test_firewall.py @@ -3,7 +3,7 @@ """Test cases for firewall module.""" -from ipaddress import IPv4Address, IPv4Network +from ipaddress import IPv4Network import pytest