From 2c94bc18cbc3e8461db9b0b84c6de694f7352ff1 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Tue, 28 Oct 2025 05:42:34 +0000 Subject: [PATCH 01/82] mark external repo-policy-compliance configurations as deprecated --- config.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/config.yaml b/config.yaml index 113005dddf..4a455ae540 100644 --- a/config.yaml +++ b/config.yaml @@ -88,15 +88,18 @@ options: repo-policy-compliance-token: type: string description: >- + DEPRECATED, please use repo-fork-check configuration option instead. The token to authenticate with the repository-policy-compliance service in order to generate one-time-tokens. This option requires the repo-policy-compliance-url to be set. If not set, the repository-policy-compliance service will not be used. repo-policy-compliance-url: type: string description: >- + DEPRECATED, please use repo-fork-check configuration option instead. The URL to the repository-policy-compliance service. This option requires the repo-policy-compliance-token to be set. If not set, the repository-policy-compliance service will not be used. This option is only supported when using OpenStack Cloud. + repo-policy- test-mode: type: string description: >- From 7ba3881a8cfce401210b0833427c06aea7fee97b Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Tue, 28 Oct 2025 06:15:02 +0000 Subject: [PATCH 02/82] add allow-forked-repository charm configuration option --- config.yaml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/config.yaml b/config.yaml index 4a455ae540..f1e2b98f3f 100644 --- a/config.yaml +++ b/config.yaml @@ -99,7 +99,13 @@ options: The URL to the repository-policy-compliance service. This option requires the repo-policy-compliance-token to be set. If not set, the repository-policy-compliance service will not be used. This option is only supported when using OpenStack Cloud. - repo-policy- + allow-forked-repository: + type: boolean + description: >- + EXPERIMENTAL + Whether to allow runs from forked repository contributions. Enabling this option will disable + all runs from forked repositories except contribution authors being in COLLABORATOR, MEMBER + or OWNER status. See https://docs.github.com/en/graphql/reference/enums#commentauthorassociation. test-mode: type: string description: >- From f19bbe748e77e520221c22c646a40754438767e7 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Tue, 28 Oct 2025 06:22:03 +0000 Subject: [PATCH 03/82] add deprecated configuration option warning --- src/charm_state.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/charm_state.py b/src/charm_state.py index 6f21b575dc..7c8d704190 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -247,6 +247,13 @@ def from_charm(cls, charm: CharmBase) -> "RepoPolicyComplianceConfig": f"Missing {REPO_POLICY_COMPLIANCE_URL_CONFIG_NAME} configuration" ) + logger.warning( + "%s, %s configuration option is marked for deprecation." + "Consider using %s configuration option instead.", + REPO_POLICY_COMPLIANCE_URL_CONFIG_NAME, + REPO_POLICY_COMPLIANCE_TOKEN_CONFIG_NAME, + ALLOW_FORKED_REPOSITORY_CONFIG_NAME, + ) # pydantic allows string to be passed as AnyHttpUrl, mypy complains about it return cls(url=url, token=token) # type: ignore From 8134a511d0437442ec94d6d8f0a1db8f3144797b Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Tue, 28 Oct 2025 07:20:07 +0000 Subject: [PATCH 04/82] use type casting for bool charm configs --- src/charm_state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm_state.py b/src/charm_state.py index 7c8d704190..6f3826c0be 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -561,7 +561,7 @@ def from_charm(cls, charm: CharmBase) -> "CharmConfig": manager_proxy_command = ( cast(str, charm.config.get(MANAGER_SSH_PROXY_COMMAND_CONFIG_NAME, "")) or None ) - use_aproxy = bool(charm.config.get(USE_APROXY_CONFIG_NAME)) + use_aproxy = cast(bool, charm.config.get(USE_APROXY_CONFIG_NAME)) custom_pre_job_script = ( cast(str, charm.config.get(CUSTOM_PRE_JOB_SCRIPT_CONFIG_NAME, "")) or None From ef9b73cee878e302095e7015392792fc6467ccf3 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Tue, 28 Oct 2025 07:20:27 +0000 Subject: [PATCH 05/82] change config option name to allow-external-contributor --- config.yaml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/config.yaml b/config.yaml index f1e2b98f3f..e071976e2a 100644 --- a/config.yaml +++ b/config.yaml @@ -99,13 +99,15 @@ options: The URL to the repository-policy-compliance service. This option requires the repo-policy-compliance-token to be set. If not set, the repository-policy-compliance service will not be used. This option is only supported when using OpenStack Cloud. - allow-forked-repository: + allow-external-contributor: type: boolean + default: False description: >- EXPERIMENTAL - Whether to allow runs from forked repository contributions. Enabling this option will disable - all runs from forked repositories except contribution authors being in COLLABORATOR, MEMBER - or OWNER status. See https://docs.github.com/en/graphql/reference/enums#commentauthorassociation. + Whether to allow runs from forked repository from an external contributor. Enabling this + option will enable all runs from forked repositories. By default, runs from contribution + authors being in COLLABORATOR, MEMBER or OWNER status is allowed. See + https://docs.github.com/en/graphql/reference/enums#commentauthorassociation. test-mode: type: string description: >- From d753dc7a4ce059c943529d8da382c4e84c77445d Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Tue, 28 Oct 2025 07:21:04 +0000 Subject: [PATCH 06/82] parse allow-external-contributor charm config option --- src/charm_state.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/charm_state.py b/src/charm_state.py index 6f3826c0be..de5c3526ec 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -36,6 +36,7 @@ CHARM_STATE_PATH = Path("charm_state.json") +ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME = "allow-external-contributor" BASE_VIRTUAL_MACHINES_CONFIG_NAME = "base-virtual-machines" DOCKERHUB_MIRROR_CONFIG_NAME = "dockerhub-mirror" FLAVOR_LABEL_COMBINATIONS_CONFIG_NAME = "flavor-label-combinations" @@ -252,7 +253,7 @@ def from_charm(cls, charm: CharmBase) -> "RepoPolicyComplianceConfig": "Consider using %s configuration option instead.", REPO_POLICY_COMPLIANCE_URL_CONFIG_NAME, REPO_POLICY_COMPLIANCE_TOKEN_CONFIG_NAME, - ALLOW_FORKED_REPOSITORY_CONFIG_NAME, + ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME, ) # pydantic allows string to be passed as AnyHttpUrl, mypy complains about it return cls(url=url, token=token) # type: ignore @@ -264,6 +265,9 @@ class CharmConfig(BaseModel): Some charm configurations are grouped into other configuration models. Attributes: + allow_external_contributor: Whether to allow runs from forked repositories with from + an external contributor with author association status less than COLLABORATOR. See + https://docs.github.com/en/graphql/reference/enums#commentauthorassociation. dockerhub_mirror: Private docker registry as dockerhub mirror for the runners to use. labels: Additional runner labels to append to default (i.e. os, flavor, architecture). openstack_clouds_yaml: The openstack clouds.yaml configuration. @@ -282,6 +286,7 @@ class CharmConfig(BaseModel): runner_manager_log_level: The log level of the runner manager application. """ + allow_external_contributor: bool dockerhub_mirror: AnyHttpsUrl | None labels: tuple[str, ...] openstack_clouds_yaml: OpenStackCloudsYAML @@ -572,6 +577,9 @@ def from_charm(cls, charm: CharmBase) -> "CharmConfig": ) # pydantic allows to pass str as AnyHttpUrl, mypy complains about it return cls( + allow_external_contributor=cast( + bool, charm.config.get(ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME, False) + ), dockerhub_mirror=dockerhub_mirror, # type: ignore labels=labels, openstack_clouds_yaml=openstack_clouds_yaml, From 89f2badf515b00c5551df5173cf3a99291f6c870 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Tue, 28 Oct 2025 07:27:45 +0000 Subject: [PATCH 07/82] test: configuration parsing unit tests --- src/charm_state.py | 2 +- tests/unit/conftest.py | 1 + tests/unit/test_charm_state.py | 3 +++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/charm_state.py b/src/charm_state.py index de5c3526ec..04d7e7ed47 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -266,7 +266,7 @@ class CharmConfig(BaseModel): Attributes: allow_external_contributor: Whether to allow runs from forked repositories with from - an external contributor with author association status less than COLLABORATOR. See + an external contributor with author association status less than COLLABORATOR. See \ https://docs.github.com/en/graphql/reference/enums#commentauthorassociation. dockerhub_mirror: Private docker registry as dockerhub mirror for the runners to use. labels: Additional runner labels to append to default (i.e. os, flavor, architecture). diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index afcef11444..2e9abd7875 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -130,6 +130,7 @@ def complete_charm_state_fixture(): no_proxy="10.0.0.1", ), charm_config=charm_state.CharmConfig( + allow_external_contributor=False, dockerhub_mirror="https://docker.example.com", labels=("label1", "label2"), openstack_clouds_yaml=OpenStackCloudsYAML( diff --git a/tests/unit/test_charm_state.py b/tests/unit/test_charm_state.py index f0011133db..b5f7d7fc4b 100644 --- a/tests/unit/test_charm_state.py +++ b/tests/unit/test_charm_state.py @@ -15,6 +15,7 @@ import charm_state from charm_state import ( + ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME, APROXY_EXCLUDE_ADDRESSES_CONFIG_NAME, APROXY_REDIRECT_PORTS_CONFIG_NAME, BASE_VIRTUAL_MACHINES_CONFIG_NAME, @@ -410,6 +411,7 @@ def test_charm_config_from_charm_valid(): """ mock_charm = MockGithubRunnerCharmFactory() mock_charm.config = { + ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME: "False", PATH_CONFIG_NAME: "owner/repo", RECONCILE_INTERVAL_CONFIG_NAME: "5", DOCKERHUB_MIRROR_CONFIG_NAME: "https://example.com", @@ -461,6 +463,7 @@ def test_charm_config_from_charm_valid(): assert result.token == "abc123" assert "openssl s_client" in result.manager_proxy_command assert result.custom_pre_job_script == custom_pre_job_script + assert not result.allow_external_contributor def test_openstack_image_from_charm_no_connections(): From 4ffd599e7f777bdfa8857755a22fec08f78646ec Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Tue, 28 Oct 2025 07:28:10 +0000 Subject: [PATCH 08/82] fix: add use_aproxy default value --- src/charm_state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm_state.py b/src/charm_state.py index 04d7e7ed47..bb236570f0 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -566,7 +566,7 @@ def from_charm(cls, charm: CharmBase) -> "CharmConfig": manager_proxy_command = ( cast(str, charm.config.get(MANAGER_SSH_PROXY_COMMAND_CONFIG_NAME, "")) or None ) - use_aproxy = cast(bool, charm.config.get(USE_APROXY_CONFIG_NAME)) + use_aproxy = cast(bool, charm.config.get(USE_APROXY_CONFIG_NAME, False)) custom_pre_job_script = ( cast(str, charm.config.get(CUSTOM_PRE_JOB_SCRIPT_CONFIG_NAME, "")) or None From 4ecc739948e4c92519de805aaa2a6b5390c27c60 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Tue, 28 Oct 2025 08:54:20 +0000 Subject: [PATCH 09/82] add allow_external_contributor configuration to application config --- .../src/github_runner_manager/configuration/base.py | 6 ++++++ src/factories.py | 1 + tests/unit/test_factories.py | 1 + 3 files changed, 8 insertions(+) diff --git a/github-runner-manager/src/github_runner_manager/configuration/base.py b/github-runner-manager/src/github_runner_manager/configuration/base.py index 978cfb4b8e..9219827ba4 100644 --- a/github-runner-manager/src/github_runner_manager/configuration/base.py +++ b/github-runner-manager/src/github_runner_manager/configuration/base.py @@ -38,6 +38,11 @@ class ApplicationConfiguration(BaseModel): """Main entry point for the Application Configuration. Attributes: + allow_external_contributor: Whether to allow runs from forked repository from an external + contributor. Enabling this option will enable all runs from forked repositories. By + default, runs from contribution authors being in COLLABORATOR, MEMBER or OWNER status + is allowed. See \ + https://docs.github.com/en/graphql/reference/enums#commentauthorassociation. name: Name to identify the manager. Used for metrics. extra_labels: Extra labels to add to the runner. jobmanager_config: Configuration for the jobmanager platform. @@ -49,6 +54,7 @@ class ApplicationConfiguration(BaseModel): reconcile_interval: Seconds to wait between reconciliation. """ + allow_external_contributor: bool name: str extra_labels: list[str] jobmanager_config: jobmanager.JobManagerConfiguration | None diff --git a/src/factories.py b/src/factories.py index a925854341..64e2889afa 100644 --- a/src/factories.py +++ b/src/factories.py @@ -96,6 +96,7 @@ def create_application_configuration( reactive_configuration = _get_reactive_configuration(state, app_name) openstack_configuration = create_openstack_configuration(state, unit_name) return ApplicationConfiguration( + allow_external_contributor=state.charm_config.allow_external_contributor, name=app_name, extra_labels=extra_labels, github_config=github_configuration, diff --git a/tests/unit/test_factories.py b/tests/unit/test_factories.py index e13e34b67d..111e3e3c47 100644 --- a/tests/unit/test_factories.py +++ b/tests/unit/test_factories.py @@ -71,6 +71,7 @@ def test_create_application_configuration( app_configuration = factories.create_application_configuration(state, "app_name", "unit_name") assert app_configuration == ApplicationConfiguration( + allow_external_contributor=False, name="app_name", extra_labels=["label1", "label2"], github_config=expected_github_config, From 24981691b91b5fe0e6ece03f44f134ef315546bd Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Tue, 28 Oct 2025 08:56:41 +0000 Subject: [PATCH 10/82] add deprecation warning for repo-policy-compliance-client --- .../github_runner_manager/repo_policy_compliance_client.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/github-runner-manager/src/github_runner_manager/repo_policy_compliance_client.py b/github-runner-manager/src/github_runner_manager/repo_policy_compliance_client.py index db9f5eecc7..17c35c45c9 100644 --- a/github-runner-manager/src/github_runner_manager/repo_policy_compliance_client.py +++ b/github-runner-manager/src/github_runner_manager/repo_policy_compliance_client.py @@ -30,6 +30,10 @@ def __init__(self, url: str, charm_token: str) -> None: url: Base URL to the repo policy compliance service. charm_token: Charm token configured for the repo policy compliance service. """ + logger.warning( + "Repo policy compliance service is marked for deprecation." + "Consier using `allow_external_contributor` option instead." + ) self._session = self._create_session() self.base_url = url self.token = charm_token From dc3f02222aaf6029c15c70959f71b94754b83cc6 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Tue, 28 Oct 2025 10:14:10 +0000 Subject: [PATCH 11/82] add allow_external_contributor option to application --- .../github_runner_manager/configuration/base.py | 2 +- .../github_runner_manager/manager/runner_scaler.py | 1 + .../openstack_cloud/models.py | 3 +++ .../openstack_cloud/openstack_runner_manager.py | 4 ++++ .../repo_policy_compliance_client.py | 2 +- .../src/github_runner_manager/templates/pre-job.j2 | 14 ++++++++++++++ .../test_openstack_runner_manager.py | 1 + .../tests/unit/test_runner_scaler.py | 2 ++ 8 files changed, 27 insertions(+), 2 deletions(-) diff --git a/github-runner-manager/src/github_runner_manager/configuration/base.py b/github-runner-manager/src/github_runner_manager/configuration/base.py index 9219827ba4..0c01e0e5aa 100644 --- a/github-runner-manager/src/github_runner_manager/configuration/base.py +++ b/github-runner-manager/src/github_runner_manager/configuration/base.py @@ -54,7 +54,7 @@ class ApplicationConfiguration(BaseModel): reconcile_interval: Seconds to wait between reconciliation. """ - allow_external_contributor: bool + allow_external_contributor: bool = False name: str extra_labels: list[str] jobmanager_config: jobmanager.JobManagerConfiguration | None diff --git a/github-runner-manager/src/github_runner_manager/manager/runner_scaler.py b/github-runner-manager/src/github_runner_manager/manager/runner_scaler.py index 456b63e219..d5ad0d09f3 100644 --- a/github-runner-manager/src/github_runner_manager/manager/runner_scaler.py +++ b/github-runner-manager/src/github_runner_manager/manager/runner_scaler.py @@ -136,6 +136,7 @@ def build( # pylint: disable-msg=too-many-locals base_quantity = combination.base_virtual_machines openstack_runner_manager_config = OpenStackRunnerManagerConfig( + allow_external_contributor=application_configuration.allow_external_contributor, prefix=application_configuration.openstack_configuration.vm_prefix, credentials=application_configuration.openstack_configuration.credentials, server_config=server_config, diff --git a/github-runner-manager/src/github_runner_manager/openstack_cloud/models.py b/github-runner-manager/src/github_runner_manager/openstack_cloud/models.py index 5b0460168a..f43a915e14 100644 --- a/github-runner-manager/src/github_runner_manager/openstack_cloud/models.py +++ b/github-runner-manager/src/github_runner_manager/openstack_cloud/models.py @@ -29,12 +29,15 @@ class OpenStackRunnerManagerConfig: """Configuration for OpenStack runner manager. Attributes: + allow_external_contributor: Whether to allow runs from forked repository from an external + contributor. prefix: The prefix of the runner names. credentials: The OpenStack authorization information. server_config: The configuration for OpenStack server. service_config: The configuration for supporting services. """ + allow_external_contributor: bool prefix: str credentials: OpenStackCredentials server_config: OpenStackServerConfig | None diff --git a/github-runner-manager/src/github_runner_manager/openstack_cloud/openstack_runner_manager.py b/github-runner-manager/src/github_runner_manager/openstack_cloud/openstack_runner_manager.py index dc07372ad7..1d9761a71e 100644 --- a/github-runner-manager/src/github_runner_manager/openstack_cloud/openstack_runner_manager.py +++ b/github-runner-manager/src/github_runner_manager/openstack_cloud/openstack_runner_manager.py @@ -193,6 +193,10 @@ def _generate_cloud_init(self, runner_context: RunnerContext) -> str: } ) + pre_job_contents_dict.update( + {"allow_external_contributor": self._config.allow_external_contributor} + ) + pre_job_contents = jinja.get_template("pre-job.j2").render(pre_job_contents_dict) use_aproxy = service_config.use_aproxy diff --git a/github-runner-manager/src/github_runner_manager/repo_policy_compliance_client.py b/github-runner-manager/src/github_runner_manager/repo_policy_compliance_client.py index 17c35c45c9..aed0c58896 100644 --- a/github-runner-manager/src/github_runner_manager/repo_policy_compliance_client.py +++ b/github-runner-manager/src/github_runner_manager/repo_policy_compliance_client.py @@ -32,7 +32,7 @@ def __init__(self, url: str, charm_token: str) -> None: """ logger.warning( "Repo policy compliance service is marked for deprecation." - "Consier using `allow_external_contributor` option instead." + "Consider using `allow_external_contributor` option instead." ) self._session = self._create_session() self.base_url = url diff --git a/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 b/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 index 90889ce82f..2c96204218 100644 --- a/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 +++ b/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 @@ -22,6 +22,8 @@ jq -n \ {% endif %} {% if do_repo_policy_check %} + logger -s "Repo policy check is marked for deprecation. Consider using \ + `allow_external_contributor` instead" # Log common env variables. logger -s "GITHUB_EVENT_NAME: ${GITHUB_EVENT_NAME}, \ @@ -132,6 +134,18 @@ jq -n \ logger -s "The repository setup check has passed, proceeding to execute jobs" {% endif %} +{% if not allow_external_contributor %} + AUTHOR_ASSOCIATION=$(jq -r '.pull_request.author_association' "$GITHUB_EVENT_PATH") + logger -s "Author association: $AUTHOR_ASSOCIATION" + + if [[ "$AUTHOR_ASSOCIATION" != "OWNER" && "$AUTHOR_ASSOCIATION" != "MEMBER" && "$AUTHOR_ASSOCIATION" != "COLLABORATOR" ]]; then + logger -p user.error -s "User with author_association '$AUTHOR_ASSOCIATION' is not authorized. Only OWNER, MEMBER, or COLLABORATOR may run workflows." + exit 1 + fi + + logger -s "The contributor check has passed, proceeding to execute jobs" +{% endif %} + if [[ -n "$DOCKERHUB_MIRROR" ]]; then logger -s "A private docker registry is setup as a dockerhub mirror for this self-hosted runner." logger -s "The docker daemon on this self-hosted runner is configured to use the dockerhub mirror." diff --git a/github-runner-manager/tests/unit/openstack_cloud/test_openstack_runner_manager.py b/github-runner-manager/tests/unit/openstack_cloud/test_openstack_runner_manager.py index c66a5eea77..d1b6472937 100644 --- a/github-runner-manager/tests/unit/openstack_cloud/test_openstack_runner_manager.py +++ b/github-runner-manager/tests/unit/openstack_cloud/test_openstack_runner_manager.py @@ -45,6 +45,7 @@ def openstack_runner_manager_fixture( service_config_mock.ssh_debug_connections = [] service_config_mock.repo_policy_compliance = None config = OpenStackRunnerManagerConfig( + allow_external_contributor=False, prefix="test", credentials=MagicMock(), server_config=MagicMock(), diff --git a/github-runner-manager/tests/unit/test_runner_scaler.py b/github-runner-manager/tests/unit/test_runner_scaler.py index a56e53b17f..8198980cb1 100644 --- a/github-runner-manager/tests/unit/test_runner_scaler.py +++ b/github-runner-manager/tests/unit/test_runner_scaler.py @@ -300,6 +300,7 @@ def test_build_runner_scaler( assert runner_scaler._manager.manager_name == "app_name" assert runner_scaler._manager._labels == ["label1", "label2", "arm64", "noble", "flavorlabel"] assert runner_scaler._manager._cloud._config == OpenStackRunnerManagerConfig( + allow_external_contributor=False, prefix="unit_name", credentials=OpenStackCredentials( auth_url="auth_url", @@ -345,6 +346,7 @@ def test_build_runner_scaler( token="githubtoken", path=GitHubOrg(org="canonical", group="group") ), cloud_runner_manager=OpenStackRunnerManagerConfig( + allow_external_contributor=False, prefix="unit_name", credentials=OpenStackCredentials( auth_url="auth_url", From 1e10fff9b703cbe0c6ecd49e97eab805ced7507b Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 30 Oct 2025 03:45:06 +0000 Subject: [PATCH 12/82] test: initial forked repo testing --- .../templates/pre-job.j2 | 2 +- tests/integration/test_charm_fork_repo.py | 55 +++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 b/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 index 2c96204218..31ddcadf8b 100644 --- a/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 +++ b/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 @@ -139,7 +139,7 @@ jq -n \ logger -s "Author association: $AUTHOR_ASSOCIATION" if [[ "$AUTHOR_ASSOCIATION" != "OWNER" && "$AUTHOR_ASSOCIATION" != "MEMBER" && "$AUTHOR_ASSOCIATION" != "COLLABORATOR" ]]; then - logger -p user.error -s "User with author_association '$AUTHOR_ASSOCIATION' is not authorized. Only OWNER, MEMBER, or COLLABORATOR may run workflows." + logger -p user.error -s "Insufficient user authorization (author_association). Only OWNER, MEMBER, or COLLABORATOR may run workflows." exit 1 fi diff --git a/tests/integration/test_charm_fork_repo.py b/tests/integration/test_charm_fork_repo.py index 538312c114..2569f91e54 100644 --- a/tests/integration/test_charm_fork_repo.py +++ b/tests/integration/test_charm_fork_repo.py @@ -7,6 +7,7 @@ """ from datetime import datetime, timezone +from pathlib import Path import pytest import requests @@ -17,6 +18,7 @@ from tests.integration.helpers.common import ( DISPATCH_FAILURE_TEST_WORKFLOW_FILENAME, + DISPATCH_TEST_WORKFLOW_FILENAME, dispatch_workflow, ) from tests.integration.helpers.openstack import OpenStackInstanceHelper, setup_repo_policy @@ -80,3 +82,56 @@ async def test_dispatch_workflow_failure( ) assert "Endpoint designed for testing that always fails" in logs assert "Should not echo if pre-job script failed" not in logs + + +@pytest.mark.openstack +@pytest.mark.asyncio +@pytest.mark.abort_on_fail +async def test_forked_repository_pre_job_failure( + model: Model, + app_with_forked_repo: Application, + forked_github_repository: Repository, + forked_github_branch: Branch, + instance_helper: OpenStackInstanceHelper, + token: str, + https_proxy: str, +) -> None: + """ + arrange: \ + 1. A forked repository from a user not any of OWNER/MEMBER/COLLABORATOR. \ + 2. A forked repository from a user who is a COLLABORATOR. + act: \ + 1. Trigger a workflow dispatch from a forked repository. \ + 2. Trigger a workflow dispatch from a forked repository. + assert: \ + 1. The workflow that was dispatched failed and the reason is logged. \ + 2. The workflow that was dispatched succeeds. + """ + Path("token").write_text(data=token, encoding="utf-8") + raise ValueError() + # start_time = datetime.now(timezone.utc) + + # workflow = forked_github_repository.get_workflow( + # id_or_file_name=DISPATCH_TEST_WORKFLOW_FILENAME + # ) + + # await dispatch_workflow( + # app=app_with_forked_repo, + # workflow_id_or_name=DISPATCH_TEST_WORKFLOW_FILENAME, + # branch=forked_github_branch, + # github_repository=forked_github_repository, + # conclusion="failure", + # ) + + # # Unable to find the run id of the workflow that was dispatched. + # # Therefore, all runs after this test start should pass the conditions. + # for run in workflow.get_runs(created=f">={start_time.isoformat()}"): + # if start_time > run.created_at: + # continue + + # logs_url = run.jobs()[0].logs_url() + # logs = requests.get(logs_url).content.decode("utf-8") + + # if f"Job is about to start running on the runner: {app_with_forked_repo.name}-" in logs: + # assert run.jobs()[0].conclusion == "failure" + # assert "Insufficient user authorization (author_association)" in logs From d1d4c52a1274509148bf062b885ec2fb3c6f7e41 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 30 Oct 2025 03:45:16 +0000 Subject: [PATCH 13/82] debug --- .github/workflows/integration_test.yaml | 32 +++++++++++++------------ 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 23b995f8e4..3ab5d52149 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -20,26 +20,28 @@ jobs: juju-channel: 3.6/stable provider: lxd test-tox-env: integration-juju3.6 - modules: '["test_charm_metrics_failure", "test_charm_metrics_success", "test_charm_fork_repo", "test_charm_fork_path_change", "test_charm_no_runner", "test_charm_runner", "test_debug_ssh", "test_charm_upgrade", "test_reactive", "test_jobmanager_prespawned", "test_jobmanager_reactive"]' - extra-arguments: '-m openstack --log-format="%(asctime)s %(levelname)s %(message)s"' - self-hosted-runner: true - self-hosted-runner-label: pfe-ci - openstack-integration-tests-cross-controller-private-endpoint: - name: Cross controller integration test using private-endpoint - uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main - secrets: inherit - with: - juju-channel: 3.6/stable - pre-run-script: tests/integration/setup-integration-tests.sh - provider: lxd - test-tox-env: integration-juju3.6 - modules: '["test_prometheus_metrics"]' + modules: '["test_charm_fork_repo"]' # "test_charm_metrics_failure", "test_charm_metrics_success", "test_charm_fork_repo", "test_charm_fork_path_change", "test_charm_no_runner", "test_charm_runner", "test_debug_ssh", "test_charm_upgrade", "test_reactive", "test_jobmanager_prespawned", "test_jobmanager_reactive" extra-arguments: '-m openstack --log-format="%(asctime)s %(levelname)s %(message)s"' self-hosted-runner: true self-hosted-runner-label: pfe-ci + tmate-debug: true + tmate-timeout: 90 + # openstack-integration-tests-cross-controller-private-endpoint: + # name: Cross controller integration test using private-endpoint + # uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main + # secrets: inherit + # with: + # juju-channel: 3.6/stable + # pre-run-script: tests/integration/setup-integration-tests.sh + # provider: lxd + # test-tox-env: integration-juju3.6 + # modules: '["test_prometheus_metrics"]' + # extra-arguments: '-m openstack --log-format="%(asctime)s %(levelname)s %(message)s"' + # self-hosted-runner: true + # self-hosted-runner-label: pfe-ci allure-report: if: ${{ (success() || failure()) && github.event_name == 'schedule' }} needs: - openstack-integration-tests-private-endpoint - - openstack-integration-tests-cross-controller-private-endpoint + # - openstack-integration-tests-cross-controller-private-endpoint uses: canonical/operator-workflows/.github/workflows/allure_report.yaml@main From 953fcb5e41415cebf3a0b64d96eed3a7e8bc4e60 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 30 Oct 2025 04:30:42 +0000 Subject: [PATCH 14/82] debug: ping me when test done --- .github/workflows/integration_test.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 3ab5d52149..bd17037a42 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -14,7 +14,7 @@ concurrency: jobs: openstack-integration-tests-private-endpoint: name: Integration test using private-endpoint - uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main + uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@feat/mattermost-ping secrets: inherit with: juju-channel: 3.6/stable @@ -26,6 +26,7 @@ jobs: self-hosted-runner-label: pfe-ci tmate-debug: true tmate-timeout: 90 + mattermost-message: "Test result available @charlie4284!" # openstack-integration-tests-cross-controller-private-endpoint: # name: Cross controller integration test using private-endpoint # uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main From f2c73785d5c674d66ae3175dc7ae851264a798a8 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Wed, 19 Nov 2025 12:08:10 +0800 Subject: [PATCH 15/82] feat: enhance integration tests for trusted and untrusted forked repositories --- tests/integration/conftest.py | 15 -- tests/integration/test_charm_fork_repo.py | 262 ++++++++++++++++++---- 2 files changed, 221 insertions(+), 56 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index bf9f9a01a7..98a14aacf6 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -35,7 +35,6 @@ OPENSTACK_CLOUDS_YAML_CONFIG_NAME, OPENSTACK_FLAVOR_CONFIG_NAME, OPENSTACK_NETWORK_CONFIG_NAME, - PATH_CONFIG_NAME, USE_APROXY_CONFIG_NAME, ) from tests.integration.helpers.common import ( @@ -682,20 +681,6 @@ def forked_github_branch( branch_ref.delete() -@pytest_asyncio.fixture(scope="module") -async def app_with_forked_repo( - model: Model, basic_app: Application, forked_github_repository: Repository -) -> Application: - """Application with no runner on a forked repo. - - Test should ensure it returns with the application in a good state and has - one runner. - """ - await basic_app.set_config({PATH_CONFIG_NAME: forked_github_repository.full_name}) - - return basic_app - - @pytest_asyncio.fixture(scope="module", name="test_github_branch") async def test_github_branch_fixture(github_repository: Repository) -> AsyncIterator[Branch]: """Create a new branch for testing, from latest commit in current branch.""" diff --git a/tests/integration/test_charm_fork_repo.py b/tests/integration/test_charm_fork_repo.py index 2569f91e54..efecada369 100644 --- a/tests/integration/test_charm_fork_repo.py +++ b/tests/integration/test_charm_fork_repo.py @@ -6,16 +6,22 @@ The forked repo is configured to fail the repo-policy-compliance check. """ +import secrets from datetime import datetime, timezone -from pathlib import Path +from time import sleep +from typing import Iterator import pytest +import pytest_asyncio import requests +from github import Github from github.Branch import Branch +from github.GithubException import GithubException from github.Repository import Repository from juju.application import Application from juju.model import Model +from charm_state import ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME, PATH_CONFIG_NAME from tests.integration.helpers.common import ( DISPATCH_FAILURE_TEST_WORKFLOW_FILENAME, DISPATCH_TEST_WORKFLOW_FILENAME, @@ -24,6 +30,129 @@ from tests.integration.helpers.openstack import OpenStackInstanceHelper, setup_repo_policy +@pytest_asyncio.fixture(scope="module") +async def app_with_forked_repo( + model: Model, basic_app: Application, forked_github_repository: Repository +) -> Application: + """Application with no runner on a forked repo. + + Test should ensure it returns with the application in a good state and has + one runner. + """ + await basic_app.set_config({PATH_CONFIG_NAME: forked_github_repository.full_name}) + + return basic_app + + +@pytest.fixture(scope="module") +def trusted_forked_github_repository( + github_repository: Repository, + github_client: Github, + token_alt: str, +) -> Iterator[Repository]: + """Create a fork from an alternate user account and add that user as collaborator. + + This fixture creates a fork using token_alt (different GitHub user) and then + adds that user as a collaborator with push permissions to the original repository. + This simulates a trusted fork scenario where the fork owner is a COLLABORATOR. + + Note: token_alt must represent a different GitHub user account than token. + """ + # Create GitHub client with alternate token + alt_github_client = Github(token_alt) + alt_user = alt_github_client.get_user() + primary_user = github_client.get_user() + + # Verify tokens represent different users + assert ( + alt_user.login != primary_user.login + ), f"token_alt must be from a different user. Both tokens have user: {alt_user.login}" + + # Get the repository using alternate token to create fork from that account + alt_repo_ref = alt_github_client.get_repo(github_repository.full_name) + trusted_fork = alt_repo_ref.create_fork(name=f"test-trusted-{github_repository.name}") + + # Wait for fork to be ready + for _ in range(10): + try: + sleep(10) + trusted_fork.get_branches() + break + except GithubException: + pass + else: + assert False, "timed out whilst waiting for trusted fork creation" + + # Add alternate user as collaborator to original repository + github_repository.add_to_collaborators(alt_user.login, permission="push") + + # Wait for collaborator permissions to propagate + for _ in range(10): + try: + if github_repository.has_in_collaborators(alt_user.login): + break + sleep(5) + except GithubException: + sleep(5) + else: + assert False, "timed out whilst waiting for collaborator permissions to propagate" + + yield trusted_fork + + # Cleanup: Remove collaborator + try: + github_repository.remove_from_collaborators(alt_user.login) + except GithubException: + pass # Best effort cleanup + + # Note: Fork is not deleted to support parallel test runs + + +@pytest.fixture(scope="module") +def trusted_forked_github_branch( + github_repository: Repository, trusted_forked_github_repository: Repository +) -> Iterator[Branch]: + """Create a new branch in the trusted forked repository for testing.""" + branch_name = f"test/{secrets.token_hex(4)}" + + main_branch = trusted_forked_github_repository.get_branch(github_repository.default_branch) + branch_ref = trusted_forked_github_repository.create_git_ref( + ref=f"refs/heads/{branch_name}", sha=main_branch.commit.sha + ) + + for _ in range(10): + try: + branch = trusted_forked_github_repository.get_branch(branch_name) + break + except GithubException as err: + if err.status == 404: + sleep(5) + continue + raise + else: + assert ( + False + ), "Failed to get created branch in trusted fork repo, issue with GitHub or network." + + yield branch + + branch_ref.delete() + + +@pytest_asyncio.fixture(scope="module") +async def app_with_trusted_forked_repo( + model: Model, basic_app: Application, trusted_forked_github_repository: Repository +) -> Application: + """Application with no runner on a trusted forked repo. + + Test should ensure it returns with the application in a good state and has + one runner. + """ + await basic_app.set_config({PATH_CONFIG_NAME: trusted_forked_github_repository.full_name}) + + return basic_app + + @pytest.mark.openstack @pytest.mark.asyncio @pytest.mark.abort_on_fail @@ -87,51 +216,102 @@ async def test_dispatch_workflow_failure( @pytest.mark.openstack @pytest.mark.asyncio @pytest.mark.abort_on_fail -async def test_forked_repository_pre_job_failure( +async def test_untrusted_fork_owner_workflow_blocked( model: Model, app_with_forked_repo: Application, forked_github_repository: Repository, forked_github_branch: Branch, instance_helper: OpenStackInstanceHelper, - token: str, - https_proxy: str, ) -> None: """ - arrange: \ - 1. A forked repository from a user not any of OWNER/MEMBER/COLLABORATOR. \ - 2. A forked repository from a user who is a COLLABORATOR. - act: \ - 1. Trigger a workflow dispatch from a forked repository. \ - 2. Trigger a workflow dispatch from a forked repository. - assert: \ - 1. The workflow that was dispatched failed and the reason is logged. \ - 2. The workflow that was dispatched succeeds. + arrange: A forked repository where the owner is NOT OWNER/MEMBER/COLLABORATOR \ + and allow-external-contributor is set to False. + act: Trigger a workflow dispatch from the untrusted forked repository. + assert: The workflow execution fails and insufficient authorization is logged. """ - Path("token").write_text(data=token, encoding="utf-8") - raise ValueError() - # start_time = datetime.now(timezone.utc) - - # workflow = forked_github_repository.get_workflow( - # id_or_file_name=DISPATCH_TEST_WORKFLOW_FILENAME - # ) - - # await dispatch_workflow( - # app=app_with_forked_repo, - # workflow_id_or_name=DISPATCH_TEST_WORKFLOW_FILENAME, - # branch=forked_github_branch, - # github_repository=forked_github_repository, - # conclusion="failure", - # ) - - # # Unable to find the run id of the workflow that was dispatched. - # # Therefore, all runs after this test start should pass the conditions. - # for run in workflow.get_runs(created=f">={start_time.isoformat()}"): - # if start_time > run.created_at: - # continue - - # logs_url = run.jobs()[0].logs_url() - # logs = requests.get(logs_url).content.decode("utf-8") - - # if f"Job is about to start running on the runner: {app_with_forked_repo.name}-" in logs: - # assert run.jobs()[0].conclusion == "failure" - # assert "Insufficient user authorization (author_association)" in logs + start_time = datetime.now(timezone.utc) + + # Set allow-external-contributor to False to block untrusted forks + await app_with_forked_repo.set_config({ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME: "false"}) + await instance_helper.ensure_charm_has_runner(app_with_forked_repo) + + workflow = forked_github_repository.get_workflow( + id_or_file_name=DISPATCH_TEST_WORKFLOW_FILENAME + ) + + await dispatch_workflow( + app=app_with_forked_repo, + workflow_id_or_name=DISPATCH_TEST_WORKFLOW_FILENAME, + branch=forked_github_branch, + github_repository=forked_github_repository, + conclusion="failure", + ) + + # Unable to find the run id of the workflow that was dispatched. + # Therefore, all runs after this test start should pass the conditions. + for run in workflow.get_runs(created=f">={start_time.isoformat()}"): + if start_time > run.created_at: + continue + + logs_url = run.jobs()[0].logs_url() + logs = requests.get(logs_url).content.decode("utf-8") + + if f"Job is about to start running on the runner: {app_with_forked_repo.name}-" in logs: + assert run.jobs()[0].conclusion == "failure" + assert "Insufficient user authorization (author_association)" in logs + assert "Only OWNER, MEMBER, or COLLABORATOR may run workflows" in logs + + +@pytest.mark.openstack +@pytest.mark.asyncio +@pytest.mark.abort_on_fail +async def test_trusted_fork_owner_workflow_succeeds( + model: Model, + app_with_trusted_forked_repo: Application, + trusted_forked_github_repository: Repository, + trusted_forked_github_branch: Branch, + instance_helper: OpenStackInstanceHelper, +) -> None: + """ + arrange: A forked repository where the owner IS a COLLABORATOR \ + and allow-external-contributor is set to False. + act: Trigger a workflow dispatch from the trusted forked repository. + assert: The workflow completes successfully and contributor check passes. + """ + start_time = datetime.now(timezone.utc) + + # Set allow-external-contributor to False, but this should still succeed + # because the fork owner is a COLLABORATOR + await app_with_trusted_forked_repo.set_config( + {ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME: "false"} + ) + await instance_helper.ensure_charm_has_runner(app_with_trusted_forked_repo) + + workflow = trusted_forked_github_repository.get_workflow( + id_or_file_name=DISPATCH_TEST_WORKFLOW_FILENAME + ) + + await dispatch_workflow( + app=app_with_trusted_forked_repo, + workflow_id_or_name=DISPATCH_TEST_WORKFLOW_FILENAME, + branch=trusted_forked_github_branch, + github_repository=trusted_forked_github_repository, + conclusion="success", + ) + + # Unable to find the run id of the workflow that was dispatched. + # Therefore, all runs after this test start should pass the conditions. + for run in workflow.get_runs(created=f">={start_time.isoformat()}"): + if start_time > run.created_at: + continue + + logs_url = run.jobs()[0].logs_url() + logs = requests.get(logs_url).content.decode("utf-8") + + if ( + f"Job is about to start running on the runner: {app_with_trusted_forked_repo.name}-" + in logs + ): + assert run.jobs()[0].conclusion == "success" + assert "The contributor check has passed, proceeding to execute jobs" in logs + assert "Insufficient user authorization (author_association)" not in logs From e2350d075501e916780748f9fba458d87ee24fcd Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Wed, 19 Nov 2025 14:40:29 +0800 Subject: [PATCH 16/82] feat: update GitHub client initialization to use Token authentication --- tests/integration/conftest.py | 3 ++- tests/integration/test_charm_fork_repo.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 98a14aacf6..fcde8f8393 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -19,6 +19,7 @@ import yaml from git import Repo from github import Github, GithubException +from github.Auth import Token from github.Branch import Branch from github.Repository import Repository from github_runner_manager.github_client import GithubClient @@ -615,7 +616,7 @@ async def tmate_ssh_server_unit_ip_fixture( @pytest.fixture(scope="module") def github_client(token: str) -> Github: """Returns the github client.""" - gh = Github(token) + gh = Github(auth=Token(token=token)) rate_limit = gh.get_rate_limit() logging.info("GitHub token rate limit: %s", rate_limit.rate) return gh diff --git a/tests/integration/test_charm_fork_repo.py b/tests/integration/test_charm_fork_repo.py index efecada369..c74d0211e2 100644 --- a/tests/integration/test_charm_fork_repo.py +++ b/tests/integration/test_charm_fork_repo.py @@ -15,6 +15,7 @@ import pytest_asyncio import requests from github import Github +from github.Auth import Token from github.Branch import Branch from github.GithubException import GithubException from github.Repository import Repository @@ -59,7 +60,7 @@ def trusted_forked_github_repository( Note: token_alt must represent a different GitHub user account than token. """ # Create GitHub client with alternate token - alt_github_client = Github(token_alt) + alt_github_client = Github(auth=Token(token=token_alt)) alt_user = alt_github_client.get_user() primary_user = github_client.get_user() From 32f98184a748da77be0f66481aa1e0c0cc111020 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 20 Nov 2025 13:57:27 +0800 Subject: [PATCH 17/82] chore: revert charm integration tests --- tests/integration/test_charm_fork_repo.py | 222 +--------------------- 1 file changed, 1 insertion(+), 221 deletions(-) diff --git a/tests/integration/test_charm_fork_repo.py b/tests/integration/test_charm_fork_repo.py index c74d0211e2..2c5f78c66b 100644 --- a/tests/integration/test_charm_fork_repo.py +++ b/tests/integration/test_charm_fork_repo.py @@ -6,26 +6,19 @@ The forked repo is configured to fail the repo-policy-compliance check. """ -import secrets from datetime import datetime, timezone -from time import sleep -from typing import Iterator import pytest import pytest_asyncio import requests -from github import Github -from github.Auth import Token from github.Branch import Branch -from github.GithubException import GithubException from github.Repository import Repository from juju.application import Application from juju.model import Model -from charm_state import ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME, PATH_CONFIG_NAME +from charm_state import PATH_CONFIG_NAME from tests.integration.helpers.common import ( DISPATCH_FAILURE_TEST_WORKFLOW_FILENAME, - DISPATCH_TEST_WORKFLOW_FILENAME, dispatch_workflow, ) from tests.integration.helpers.openstack import OpenStackInstanceHelper, setup_repo_policy @@ -45,115 +38,6 @@ async def app_with_forked_repo( return basic_app -@pytest.fixture(scope="module") -def trusted_forked_github_repository( - github_repository: Repository, - github_client: Github, - token_alt: str, -) -> Iterator[Repository]: - """Create a fork from an alternate user account and add that user as collaborator. - - This fixture creates a fork using token_alt (different GitHub user) and then - adds that user as a collaborator with push permissions to the original repository. - This simulates a trusted fork scenario where the fork owner is a COLLABORATOR. - - Note: token_alt must represent a different GitHub user account than token. - """ - # Create GitHub client with alternate token - alt_github_client = Github(auth=Token(token=token_alt)) - alt_user = alt_github_client.get_user() - primary_user = github_client.get_user() - - # Verify tokens represent different users - assert ( - alt_user.login != primary_user.login - ), f"token_alt must be from a different user. Both tokens have user: {alt_user.login}" - - # Get the repository using alternate token to create fork from that account - alt_repo_ref = alt_github_client.get_repo(github_repository.full_name) - trusted_fork = alt_repo_ref.create_fork(name=f"test-trusted-{github_repository.name}") - - # Wait for fork to be ready - for _ in range(10): - try: - sleep(10) - trusted_fork.get_branches() - break - except GithubException: - pass - else: - assert False, "timed out whilst waiting for trusted fork creation" - - # Add alternate user as collaborator to original repository - github_repository.add_to_collaborators(alt_user.login, permission="push") - - # Wait for collaborator permissions to propagate - for _ in range(10): - try: - if github_repository.has_in_collaborators(alt_user.login): - break - sleep(5) - except GithubException: - sleep(5) - else: - assert False, "timed out whilst waiting for collaborator permissions to propagate" - - yield trusted_fork - - # Cleanup: Remove collaborator - try: - github_repository.remove_from_collaborators(alt_user.login) - except GithubException: - pass # Best effort cleanup - - # Note: Fork is not deleted to support parallel test runs - - -@pytest.fixture(scope="module") -def trusted_forked_github_branch( - github_repository: Repository, trusted_forked_github_repository: Repository -) -> Iterator[Branch]: - """Create a new branch in the trusted forked repository for testing.""" - branch_name = f"test/{secrets.token_hex(4)}" - - main_branch = trusted_forked_github_repository.get_branch(github_repository.default_branch) - branch_ref = trusted_forked_github_repository.create_git_ref( - ref=f"refs/heads/{branch_name}", sha=main_branch.commit.sha - ) - - for _ in range(10): - try: - branch = trusted_forked_github_repository.get_branch(branch_name) - break - except GithubException as err: - if err.status == 404: - sleep(5) - continue - raise - else: - assert ( - False - ), "Failed to get created branch in trusted fork repo, issue with GitHub or network." - - yield branch - - branch_ref.delete() - - -@pytest_asyncio.fixture(scope="module") -async def app_with_trusted_forked_repo( - model: Model, basic_app: Application, trusted_forked_github_repository: Repository -) -> Application: - """Application with no runner on a trusted forked repo. - - Test should ensure it returns with the application in a good state and has - one runner. - """ - await basic_app.set_config({PATH_CONFIG_NAME: trusted_forked_github_repository.full_name}) - - return basic_app - - @pytest.mark.openstack @pytest.mark.asyncio @pytest.mark.abort_on_fail @@ -212,107 +96,3 @@ async def test_dispatch_workflow_failure( ) assert "Endpoint designed for testing that always fails" in logs assert "Should not echo if pre-job script failed" not in logs - - -@pytest.mark.openstack -@pytest.mark.asyncio -@pytest.mark.abort_on_fail -async def test_untrusted_fork_owner_workflow_blocked( - model: Model, - app_with_forked_repo: Application, - forked_github_repository: Repository, - forked_github_branch: Branch, - instance_helper: OpenStackInstanceHelper, -) -> None: - """ - arrange: A forked repository where the owner is NOT OWNER/MEMBER/COLLABORATOR \ - and allow-external-contributor is set to False. - act: Trigger a workflow dispatch from the untrusted forked repository. - assert: The workflow execution fails and insufficient authorization is logged. - """ - start_time = datetime.now(timezone.utc) - - # Set allow-external-contributor to False to block untrusted forks - await app_with_forked_repo.set_config({ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME: "false"}) - await instance_helper.ensure_charm_has_runner(app_with_forked_repo) - - workflow = forked_github_repository.get_workflow( - id_or_file_name=DISPATCH_TEST_WORKFLOW_FILENAME - ) - - await dispatch_workflow( - app=app_with_forked_repo, - workflow_id_or_name=DISPATCH_TEST_WORKFLOW_FILENAME, - branch=forked_github_branch, - github_repository=forked_github_repository, - conclusion="failure", - ) - - # Unable to find the run id of the workflow that was dispatched. - # Therefore, all runs after this test start should pass the conditions. - for run in workflow.get_runs(created=f">={start_time.isoformat()}"): - if start_time > run.created_at: - continue - - logs_url = run.jobs()[0].logs_url() - logs = requests.get(logs_url).content.decode("utf-8") - - if f"Job is about to start running on the runner: {app_with_forked_repo.name}-" in logs: - assert run.jobs()[0].conclusion == "failure" - assert "Insufficient user authorization (author_association)" in logs - assert "Only OWNER, MEMBER, or COLLABORATOR may run workflows" in logs - - -@pytest.mark.openstack -@pytest.mark.asyncio -@pytest.mark.abort_on_fail -async def test_trusted_fork_owner_workflow_succeeds( - model: Model, - app_with_trusted_forked_repo: Application, - trusted_forked_github_repository: Repository, - trusted_forked_github_branch: Branch, - instance_helper: OpenStackInstanceHelper, -) -> None: - """ - arrange: A forked repository where the owner IS a COLLABORATOR \ - and allow-external-contributor is set to False. - act: Trigger a workflow dispatch from the trusted forked repository. - assert: The workflow completes successfully and contributor check passes. - """ - start_time = datetime.now(timezone.utc) - - # Set allow-external-contributor to False, but this should still succeed - # because the fork owner is a COLLABORATOR - await app_with_trusted_forked_repo.set_config( - {ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME: "false"} - ) - await instance_helper.ensure_charm_has_runner(app_with_trusted_forked_repo) - - workflow = trusted_forked_github_repository.get_workflow( - id_or_file_name=DISPATCH_TEST_WORKFLOW_FILENAME - ) - - await dispatch_workflow( - app=app_with_trusted_forked_repo, - workflow_id_or_name=DISPATCH_TEST_WORKFLOW_FILENAME, - branch=trusted_forked_github_branch, - github_repository=trusted_forked_github_repository, - conclusion="success", - ) - - # Unable to find the run id of the workflow that was dispatched. - # Therefore, all runs after this test start should pass the conditions. - for run in workflow.get_runs(created=f">={start_time.isoformat()}"): - if start_time > run.created_at: - continue - - logs_url = run.jobs()[0].logs_url() - logs = requests.get(logs_url).content.decode("utf-8") - - if ( - f"Job is about to start running on the runner: {app_with_trusted_forked_repo.name}-" - in logs - ): - assert run.jobs()[0].conclusion == "success" - assert "The contributor check has passed, proceeding to execute jobs" in logs - assert "Insufficient user authorization (author_association)" not in logs From 1eea0f70dea3abb6463f79fd9222649b33c7d00b Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 20 Nov 2025 13:59:41 +0800 Subject: [PATCH 18/82] chore: revert charm integration tests --- tests/integration/conftest.py | 14 ++++++++++++++ tests/integration/test_charm_fork_repo.py | 16 ---------------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index fcde8f8393..9080844f1d 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -36,6 +36,7 @@ OPENSTACK_CLOUDS_YAML_CONFIG_NAME, OPENSTACK_FLAVOR_CONFIG_NAME, OPENSTACK_NETWORK_CONFIG_NAME, + PATH_CONFIG_NAME, USE_APROXY_CONFIG_NAME, ) from tests.integration.helpers.common import ( @@ -682,6 +683,19 @@ def forked_github_branch( branch_ref.delete() +@pytest_asyncio.fixture(scope="module") +async def app_with_forked_repo( + model: Model, basic_app: Application, forked_github_repository: Repository +) -> Application: + """Application with no runner on a forked repo. + Test should ensure it returns with the application in a good state and has + one runner. + """ + await basic_app.set_config({PATH_CONFIG_NAME: forked_github_repository.full_name}) + + return basic_app + + @pytest_asyncio.fixture(scope="module", name="test_github_branch") async def test_github_branch_fixture(github_repository: Repository) -> AsyncIterator[Branch]: """Create a new branch for testing, from latest commit in current branch.""" diff --git a/tests/integration/test_charm_fork_repo.py b/tests/integration/test_charm_fork_repo.py index 2c5f78c66b..538312c114 100644 --- a/tests/integration/test_charm_fork_repo.py +++ b/tests/integration/test_charm_fork_repo.py @@ -9,14 +9,12 @@ from datetime import datetime, timezone import pytest -import pytest_asyncio import requests from github.Branch import Branch from github.Repository import Repository from juju.application import Application from juju.model import Model -from charm_state import PATH_CONFIG_NAME from tests.integration.helpers.common import ( DISPATCH_FAILURE_TEST_WORKFLOW_FILENAME, dispatch_workflow, @@ -24,20 +22,6 @@ from tests.integration.helpers.openstack import OpenStackInstanceHelper, setup_repo_policy -@pytest_asyncio.fixture(scope="module") -async def app_with_forked_repo( - model: Model, basic_app: Application, forked_github_repository: Repository -) -> Application: - """Application with no runner on a forked repo. - - Test should ensure it returns with the application in a good state and has - one runner. - """ - await basic_app.set_config({PATH_CONFIG_NAME: forked_github_repository.full_name}) - - return basic_app - - @pytest.mark.openstack @pytest.mark.asyncio @pytest.mark.abort_on_fail From 56a182ef6ae97440b961343d19d8d921c2900cd3 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 20 Nov 2025 14:00:13 +0800 Subject: [PATCH 19/82] chore: merge pre_job_contents_dict generation --- .../openstack_cloud/openstack_runner_manager.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/github-runner-manager/src/github_runner_manager/openstack_cloud/openstack_runner_manager.py b/github-runner-manager/src/github_runner_manager/openstack_cloud/openstack_runner_manager.py index 1d9761a71e..4219e107e8 100644 --- a/github-runner-manager/src/github_runner_manager/openstack_cloud/openstack_runner_manager.py +++ b/github-runner-manager/src/github_runner_manager/openstack_cloud/openstack_runner_manager.py @@ -182,6 +182,7 @@ def _generate_cloud_init(self, runner_context: RunnerContext) -> str: "metrics_exchange_path": str(METRICS_EXCHANGE_PATH), "do_repo_policy_check": False, "custom_pre_job_script": service_config.custom_pre_job_script, + "allow_external_contributor": self._config.allow_external_contributor, } repo_policy = self._get_repo_policy_compliance_client() if repo_policy is not None: @@ -193,10 +194,6 @@ def _generate_cloud_init(self, runner_context: RunnerContext) -> str: } ) - pre_job_contents_dict.update( - {"allow_external_contributor": self._config.allow_external_contributor} - ) - pre_job_contents = jinja.get_template("pre-job.j2").render(pre_job_contents_dict) use_aproxy = service_config.use_aproxy From 596a559091011c757c76eb433dda2394c2987eb0 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 20 Nov 2025 14:22:50 +0800 Subject: [PATCH 20/82] feat: add GitHub and OpenStack configuration options for integration tests --- github-runner-manager/tests/conftest.py | 45 +++ .../tests/integration/application.py | 181 +++++++++ .../tests/integration/conftest.py | 76 ++++ .../tests/integration/factories.py | 163 ++++++++ .../tests/integration/github_helpers.py | 142 +++++++ .../test_external_contributor_security.py | 362 ++++++++++++++++++ .../tests/unit/test_pre_job_script.py | 328 ++++++++++++++++ 7 files changed, 1297 insertions(+) create mode 100644 github-runner-manager/tests/integration/application.py create mode 100644 github-runner-manager/tests/integration/conftest.py create mode 100644 github-runner-manager/tests/integration/factories.py create mode 100644 github-runner-manager/tests/integration/github_helpers.py create mode 100644 github-runner-manager/tests/integration/test_external_contributor_security.py create mode 100644 github-runner-manager/tests/unit/test_pre_job_script.py diff --git a/github-runner-manager/tests/conftest.py b/github-runner-manager/tests/conftest.py index 09a84dda80..98aa6d2e81 100644 --- a/github-runner-manager/tests/conftest.py +++ b/github-runner-manager/tests/conftest.py @@ -11,3 +11,48 @@ def pytest_addoption(parser): parser: Pytest parser. """ parser.addoption("--charm-file", action="store") + parser.addoption( + "--github-token", + action="store", + help="GitHub personal access token for integration tests.", + ) + parser.addoption( + "--github-repository", + action="store", + help="The GitHub repository in / format for integration tests.", + ) + parser.addoption( + "--github-token-alt", + action="store", + help="Alternate GitHub token from a different user for fork testing.", + ) + parser.addoption( + "--openstack-auth-url", + action="store", + help="OpenStack authentication URL for integration tests.", + ) + parser.addoption( + "--openstack-project", + action="store", + help="OpenStack project name for integration tests.", + ) + parser.addoption( + "--openstack-username", + action="store", + help="OpenStack username for integration tests.", + ) + parser.addoption( + "--openstack-password", + action="store", + help="OpenStack password for integration tests.", + ) + parser.addoption( + "--openstack-network", + action="store", + help="OpenStack network name for integration tests.", + ) + parser.addoption( + "--openstack-region", + action="store", + help="OpenStack region name for integration tests.", + ) diff --git a/github-runner-manager/tests/integration/application.py b/github-runner-manager/tests/integration/application.py new file mode 100644 index 0000000000..e40359ab2e --- /dev/null +++ b/github-runner-manager/tests/integration/application.py @@ -0,0 +1,181 @@ +# Copyright 2025 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Test infrastructure for managing running application instances.""" + +import multiprocessing +import time +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +import requests +import yaml +from click.testing import CliRunner + +from src.github_runner_manager.cli import main + + +def wait_for_server(host: str, port: int, timeout: float = 10.0) -> bool: + """Wait for a server to become responsive. + + Args: + host: The host to connect to. + port: The port to connect to. + timeout: Maximum time to wait in seconds. + + Returns: + True if the server became responsive, False otherwise. + """ + start_time = time.time() + while time.time() - start_time < timeout: + try: + response = requests.get( + f"http://{host}:{port}/health", timeout=1, allow_redirects=False + ) + if response.status_code in (200, 404): # Server is responding + return True + except (requests.ConnectionError, requests.Timeout): + time.sleep(0.5) + return False + + +def _start_cli_server(config_file_path: Path, port: int, host: str = "127.0.0.1") -> None: + """Start the CLI server in a separate process. + + Args: + config_file_path: Path to the configuration file. + port: Port to listen on. + host: Host to listen on. + """ + runner = CliRunner() + result = runner.invoke( + main, + [ + "--config-file", + str(config_file_path), + "--host", + host, + "--port", + str(port), + "--log-level", + "DEBUG", + ], + catch_exceptions=False, + ) + if result.exit_code != 0: + print(f"CLI exited with code {result.exit_code}") + print(f"Output: {result.output}") + + +@dataclass +class RunningApplication: + """Information about a running application instance. + + This class manages the lifecycle of a github-runner-manager application + instance running in a separate process for integration testing. + + Attributes: + process: The multiprocessing.Process running the application. + port: The port the application is listening on. + host: The host the application is listening on. + config_file_path: Path to the configuration file used. + """ + + process: multiprocessing.Process + port: int + host: str + config_file_path: Path + + @property + def base_url(self) -> str: + """Get the base URL of the application. + + Returns: + The base URL (e.g., http://127.0.0.1:8080). + """ + return f"http://{self.host}:{self.port}" + + @property + def config(self) -> dict[str, Any]: + """Get the configuration used to start the application. + + Returns: + The configuration dictionary. + """ + with open(self.config_file_path, "r", encoding="utf-8") as f: + return yaml.safe_load(f) + + def get(self, path: str, **kwargs: Any) -> requests.Response: + """Make a GET request to the application. + + Args: + path: The path to request (e.g., /health). + **kwargs: Additional arguments to pass to requests.get. + + Returns: + The response object. + """ + url = f"{self.base_url}{path}" + return requests.get(url, **kwargs) + + def is_alive(self) -> bool: + """Check if the application process is still alive. + + Returns: + True if the process is alive, False otherwise. + """ + return self.process.is_alive() + + @classmethod + def create( + cls, config_file_path: Path, host: str = "127.0.0.1", port: int = 8080 + ) -> "RunningApplication": + """Create and start a new application instance. + + Args: + config_file_path: Path to the configuration file. + host: Host to listen on. Defaults to 127.0.0.1. + port: Port to listen on. If None, an available port is automatically selected. + + Returns: + A RunningApplication instance with the application running. + + Raises: + RuntimeError: If the application fails to start. + """ + # Start the server process + process = multiprocessing.Process( + target=_start_cli_server, + args=(config_file_path, port, host), + ) + process.start() + + # Wait for the server to become ready + server_ready = wait_for_server(host, port, timeout=10.0) + + if not server_ready: + process.terminate() + process.join(timeout=5) + if process.is_alive(): + process.kill() + process.join() + raise RuntimeError( + f"Application failed to start within the expected time on {host}:{port}" + ) + + return cls( + process=process, + port=port, + host=host, + config_file_path=config_file_path, + ) + + def stop(self) -> None: + """Stop the application process.""" + if self.process.is_alive(): + self.process.terminate() + self.process.join(timeout=5) + if self.process.is_alive(): + self.process.kill() + self.process.join() diff --git a/github-runner-manager/tests/integration/conftest.py b/github-runner-manager/tests/integration/conftest.py new file mode 100644 index 0000000000..e7127bbfeb --- /dev/null +++ b/github-runner-manager/tests/integration/conftest.py @@ -0,0 +1,76 @@ +# Copyright 2025 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Fixtures for github-runner-manager integration tests.""" + +import os + +import pytest + +from .factories import GitHubConfig, OpenStackConfig, TestConfig + + +@pytest.fixture +def test_config() -> TestConfig: + """Create a unique test configuration for parallel test execution. + + Returns: + Test configuration with unique identifiers. + """ + return TestConfig() + + +@pytest.fixture(scope="module") +def github_config(pytestconfig: pytest.Config) -> GitHubConfig: + """Get GitHub configuration from pytest options or environment. + + Args: + pytestconfig: Pytest configuration object. + + Returns: + GitHub configuration object. + + Raises: + pytest.skip: If neither --github-token option nor GITHUB_TOKEN + environment variable is set. + """ + token = pytestconfig.getoption("--github-token") or os.getenv("GITHUB_TOKEN") + path = pytestconfig.getoption("--github-repository") or os.getenv("GITHUB_REPOSITORY") + + if not token or not path: + pytest.skip( + "GitHub configuration not provided. Use --github-token and --github-repository options or " + "set GITHUB_TOKEN and GITHUB_REPOSITORY environment variables." + ) + + return GitHubConfig(token=token, path=path) + + +@pytest.fixture(scope="module") +def openstack_config(pytestconfig: pytest.Config) -> OpenStackConfig | None: + """Get OpenStack configuration from pytest options or environment. + + Args: + pytestconfig: Pytest configuration object. + + Returns: + OpenStack configuration object, or None if not all parameters are provided. + """ + auth_url = pytestconfig.getoption("--openstack-auth-url") or os.getenv("OS_AUTH_URL") + project = pytestconfig.getoption("--openstack-project") or os.getenv("OS_PROJECT_NAME") + username = pytestconfig.getoption("--openstack-username") or os.getenv("OS_USERNAME") + password = pytestconfig.getoption("--openstack-password") or os.getenv("OS_PASSWORD") + network = pytestconfig.getoption("--openstack-network") or os.getenv("OS_NETWORK") + region_name = pytestconfig.getoption("--openstack-region") or os.getenv("OS_REGION_NAME") + + # Only return config if all required parameters are provided + if all([auth_url, project, username, password, network, region_name]): + return OpenStackConfig( + auth_url=auth_url, + project=project, + username=username, + password=password, + network=network, + region_name=region_name, + ) + return None diff --git a/github-runner-manager/tests/integration/factories.py b/github-runner-manager/tests/integration/factories.py new file mode 100644 index 0000000000..9b20c7726e --- /dev/null +++ b/github-runner-manager/tests/integration/factories.py @@ -0,0 +1,163 @@ +# Copyright 2025 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Factory functions for creating test data and configurations.""" + +import secrets +import string +from dataclasses import dataclass, field +from typing import Any + + +def _generate_test_id() -> str: + """Generate a unique test identifier. + + Returns: + A random 8-character alphanumeric string. + """ + return "".join(secrets.choice(string.ascii_lowercase + string.digits) for _ in range(8)) + + +@dataclass +class GitHubConfig: + """GitHub configuration for tests. + + Attributes: + token: GitHub personal access token. + path: GitHub path in / or format. + """ + + token: str + path: str + + +@dataclass +class OpenStackConfig: + """OpenStack configuration for tests. + + Attributes: + auth_url: OpenStack authentication URL. + project: OpenStack project name. + username: OpenStack username. + password: OpenStack password. + network: OpenStack network name. + region_name: OpenStack region name. + """ + + auth_url: str + project: str + username: str + password: str + network: str + region_name: str = "RegionOne" + + +@dataclass +class TestConfig: + """Test-specific configuration for parallel test execution. + + Attributes: + test_id: Unique identifier for this test run. + runner_name: Name prefix for the runner manager. + labels: Extra labels to identify runners from this test. + vm_prefix: Prefix for VM names to avoid conflicts. + """ + + test_id: str = field(default_factory=_generate_test_id) + runner_name: str = field(init=False) + labels: list[str] = field(init=False) + vm_prefix: str = field(init=False) + + def __post_init__(self) -> None: + """Initialize derived fields based on test_id.""" + self.runner_name = f"test-manager-{self.test_id}" + self.labels = [f"test-{self.test_id}"] + self.vm_prefix = f"test-runner-{self.test_id}" + + +def create_default_config( + allow_external_contributor: bool = False, + github_config: GitHubConfig | None = None, + openstack_config: OpenStackConfig | None = None, + test_config: TestConfig | None = None, +) -> dict[str, Any]: + """Create a default test configuration dictionary. + + Args: + allow_external_contributor: Whether to allow external contributors. + github_config: GitHub configuration. Defaults to test values. + openstack_config: OpenStack configuration. Defaults to test values. + test_config: Test-specific configuration for parallel execution. + Defaults to new unique values. + + Returns: + Configuration dictionary for the application. + """ + # Use defaults if not provided + if github_config is None: + github_config = GitHubConfig( + token="ghp_test_token_1234567890abcdef", + path="test-org", + ) + + if openstack_config is None: + openstack_config = OpenStackConfig( + auth_url="http://openstack.example.com:5000/v3", + project="test-project", + username="test-user", + password="test-password", + network="test-network", + region_name="RegionOne", + ) + + if test_config is None: + test_config = TestConfig() + + # Parse GitHub path + if "/" in github_config.path: + owner, repo = github_config.path.split("/", 1) + path_config = {"owner": owner, "repo": repo} + else: + path_config = {"org": github_config.path, "group": "default"} + + return { + "name": test_config.runner_name, + "allow_external_contributor": allow_external_contributor, + "extra_labels": test_config.labels, + "github_config": { + "token": github_config.token, + "path": path_config, + }, + "service_config": { + "use_aproxy": False, + "dockerhub_mirror": None, + "ssh_debug_connections": [], + "repo_policy_compliance": None, + "proxy_config": None, + "runner_proxy_config": None, + }, + "non_reactive_configuration": { + "combinations": [ + { + "image": {"name": "noble", "labels": ["noble", "x64"]}, + "flavor": {"name": "small", "labels": ["small"]}, + "base_virtual_machines": 1, + } + ] + }, + "reactive_configuration": None, + "openstack_configuration": { + "vm_prefix": test_config.vm_prefix, + "network": openstack_config.network, + "credentials": { + "auth_url": openstack_config.auth_url, + "project_name": openstack_config.project, + "username": openstack_config.username, + "password": openstack_config.password, + "user_domain_name": "Default", + "project_domain_name": "Default", + "region_name": openstack_config.region_name, + }, + }, + "reconcile_interval": 60, + } diff --git a/github-runner-manager/tests/integration/github_helpers.py b/github-runner-manager/tests/integration/github_helpers.py new file mode 100644 index 0000000000..45d28b5108 --- /dev/null +++ b/github-runner-manager/tests/integration/github_helpers.py @@ -0,0 +1,142 @@ +# Copyright 2025 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Helper functions for GitHub integration testing.""" + +from datetime import datetime, timezone +from time import sleep + +import requests +from github.GithubException import GithubException +from github.PullRequest import PullRequest +from github.Repository import Repository +from github.WorkflowRun import WorkflowRun + + +def create_fork_and_pr( + github_repository: Repository, + forked_github_repository: Repository, + test_id: str, +) -> PullRequest: + """Create a PR from a forked repository. + + Args: + github_repository: The original repository. + forked_github_repository: The forked repository. + test_id: Unique test identifier. + + Returns: + The created pull request. + """ + # Create a unique branch in the fork + branch_name = f"test/{test_id}" + main_branch = forked_github_repository.get_branch(github_repository.default_branch) + forked_github_repository.create_git_ref( + ref=f"refs/heads/{branch_name}", sha=main_branch.commit.sha + ) + + # Create a test file to ensure there's a change + test_file_path = f"test-{test_id}.txt" + forked_github_repository.create_file( + path=test_file_path, + message=f"Test change from fork {test_id}", + content=f"Test content {test_id}", + branch=branch_name, + ) + + # Create PR from fork to original repository + pr = github_repository.create_pull( + title=f"Test PR from fork {test_id}", + body=f"This is a test PR from a forked repository for testing external contributor security. Test ID: {test_id}", + head=f"{forked_github_repository.owner.login}:{branch_name}", + base=github_repository.default_branch, + ) + + return pr + + +def dispatch_workflow_and_get_run( + repository: Repository, + workflow_filename: str, + runner_label: str, +) -> WorkflowRun | None: + """Dispatch a workflow and return the workflow run. + + Args: + repository: Repository to dispatch workflow on. + workflow_filename: Workflow file to dispatch. + runner_label: Runner label to target. + + Returns: + The workflow run object, or None if not found. + """ + start_time = datetime.now(timezone.utc) + workflow = repository.get_workflow(id_or_file_name=workflow_filename) + + # Dispatch the workflow + success = workflow.create_dispatch( + ref=repository.default_branch, + inputs={"runner": runner_label}, + ) + + if not success: + return None + + # Wait for the workflow run to appear (up to 2 minutes) + for _ in range(24): # 24 * 5 = 120 seconds + sleep(5) + runs = workflow.get_runs(created=f">={start_time.isoformat(timespec='seconds')}") + if runs.totalCount > 0: + return runs[0] + + return None + + +def wait_for_workflow_completion(run: WorkflowRun, timeout: int = 600) -> bool: + """Wait for a workflow run to complete. + + Args: + run: The workflow run to wait for. + timeout: Maximum time to wait in seconds. + + Returns: + True if workflow completed, False if timeout. + """ + iterations = timeout // 10 + for _ in range(iterations): + run.update() + if run.status == "completed": + return True + sleep(10) + return False + + +def get_job_logs(run: WorkflowRun) -> str: + """Get the logs from a workflow run's first job. + + Args: + run: The workflow run. + + Returns: + The job logs as a string. + """ + jobs = run.jobs() + if jobs.totalCount == 0: + return "" + + job = jobs[0] + logs_url = job.logs_url() + response = requests.get(logs_url) + return response.content.decode("utf-8") + + +def close_pull_request(pr: PullRequest) -> None: + """Close a pull request (best effort). + + Args: + pr: The pull request to close. + """ + try: + pr.edit(state="closed") + except GithubException: + pass # Best effort cleanup diff --git a/github-runner-manager/tests/integration/test_external_contributor_security.py b/github-runner-manager/tests/integration/test_external_contributor_security.py new file mode 100644 index 0000000000..5224134605 --- /dev/null +++ b/github-runner-manager/tests/integration/test_external_contributor_security.py @@ -0,0 +1,362 @@ +# Copyright 2025 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Integration tests for external contributor security configuration.""" + +import logging +import os +import secrets +from pathlib import Path +from time import sleep +from typing import TYPE_CHECKING, Iterator + +import pytest +import yaml +from github import Github +from github.Auth import Token +from github.GithubException import GithubException +from github.PullRequest import PullRequest +from github.Repository import Repository + +from .application import RunningApplication +from .factories import GitHubConfig, OpenStackConfig, create_default_config +from .github_helpers import ( + close_pull_request, + create_fork_and_pr, + dispatch_workflow_and_get_run, + get_job_logs, + wait_for_workflow_completion, +) + +if TYPE_CHECKING: + from .factories import TestConfig + +logger = logging.getLogger(__name__) + +# Test workflow filename +DISPATCH_TEST_WORKFLOW_FILENAME = "workflow_dispatch_test.yaml" + + +@pytest.fixture(scope="module") +def github_token(github_config: GitHubConfig) -> str: + """Get GitHub token from github_config. + + Args: + github_config: GitHub configuration object. + + Returns: + GitHub personal access token. + """ + return github_config.token + + +@pytest.fixture(scope="module") +def github_repository(github_config: GitHubConfig) -> Repository: + """Get GitHub repository for testing. + + Args: + github_config: GitHub configuration object. + + Returns: + GitHub repository object. + """ + auth = Token(github_config.token) + github = Github(auth=auth) + return github.get_repo(github_config.path) + + +@pytest.fixture(scope="module") +def forked_github_repository( + github_repository: Repository, pytestconfig: pytest.Config +) -> Iterator[Repository]: + """Create a fork of the test repository to simulate external contributor. + + This fixture uses an alternate GitHub token (if provided) to create a fork + from a different user account, simulating an external contributor scenario. + + Args: + github_repository: The original repository. + pytestconfig: Pytest configuration object. + + Yields: + The forked repository. + """ + # Try to get alternate token for creating fork + # This simulates a different user (external contributor) + alt_token = pytestconfig.getoption("--github-token-alt", None) or os.getenv("GITHUB_TOKEN_ALT") + + if not alt_token: + logger.warning( + "Alternate GitHub token not provided. Skipping test requiring fork creation." + ) + pytest.fail( + "Alternate GitHub token not provided. Use --github-token-alt option or " + "set GITHUB_TOKEN_ALT environment variable to test with actual fork." + ) + + logger.info( + "Creating fork of repository %s using alternate token", github_repository.full_name + ) + # Create fork using alternate token + alt_github = Github(auth=Token(token=alt_token)) + + # Get the repository using alternate token to create fork + alt_repo_ref = alt_github.get_repo(github_repository.full_name) + fork = alt_repo_ref.create_fork(name=f"test-fork-{secrets.token_hex(4)}") + + logger.info("Waiting for fork to be ready (up to 5 attempts)...") + # Wait for fork to be ready + for attempt in range(5): + try: + logger.debug("Checking fork readiness (attempt %d/5)", attempt + 1) + sleep(15) + fork.get_branches() + logger.info("Fork is ready: %s", fork.full_name) + break + except GithubException: + pass + else: + logger.error("Timed out waiting for fork creation after 5 attempts") + pytest.fail("Timed out waiting for fork creation") + + yield fork + + # Cleanup: Delete fork (best effort) + logger.info("Cleaning up: Attempting to delete fork %s", fork.full_name) + try: + fork.delete() + logger.info("Fork deleted successfully") + except GithubException as e: + logger.warning( + "Failed to delete fork %s. Manual cleanup may be required. Error: %s", + fork.full_name, + str(e), + ) + + +@pytest.fixture +def application_with_external_contributor_disabled( + tmp_path: Path, + github_config: GitHubConfig, + openstack_config: OpenStackConfig | None, + test_config: TestConfig, +) -> Iterator[RunningApplication]: + """Start application with external contributor checks enabled (disabled access). + + Args: + tmp_path: Pytest fixture providing temporary directory. + github_config: GitHub configuration object. + openstack_config: OpenStack configuration object or None. + test_config: Test-specific configuration for unique identification. + + Yields: + A running application instance. + """ + config = create_default_config( + allow_external_contributor=False, + github_config=github_config, + openstack_config=openstack_config, + test_config=test_config, + ) + config_path = tmp_path / "config.yaml" + config_path.write_text(yaml.dump(config), encoding="utf-8") + + logger.info( + "Starting application with external contributor disabled (test_id: %s)", + test_config.test_id, + ) + app = RunningApplication.create(config_path) + try: + # Verify application is running and healthy before yielding + logger.info("Verifying application is alive...") + assert app.is_alive(), "Application should be running" + logger.info("Checking application health endpoint...") + response = app.get("/health") + assert response.status_code == 200, "Health endpoint should return 200 OK" + logger.info("Application is running and healthy") + + yield app + + finally: + logger.info("Stopping application") + app.stop() + + +@pytest.fixture +def application_with_external_contributor_enabled( + tmp_path: Path, + github_config: GitHubConfig, + openstack_config: OpenStackConfig | None, + test_config: TestConfig, +) -> Iterator[RunningApplication]: + """Start application with external contributor checks disabled (permissive mode). + + Args: + tmp_path: Pytest fixture providing temporary directory. + github_config: GitHub configuration object. + openstack_config: OpenStack configuration object or None. + test_config: Test-specific configuration for unique identification. + + Yields: + A running application instance. + """ + config = create_default_config( + allow_external_contributor=True, + github_config=github_config, + openstack_config=openstack_config, + test_config=test_config, + ) + config_path = tmp_path / "config.yaml" + config_path.write_text(yaml.dump(config), encoding="utf-8") + + logger.info( + "Starting application with external contributor enabled (test_id: %s)", + test_config.test_id, + ) + app = RunningApplication.create(config_path) + try: + # Verify application is running and healthy before yielding + logger.info("Verifying application is alive...") + assert app.is_alive(), "Application should be running" + logger.info("Checking application health endpoint...") + response = app.get("/health") + assert response.status_code == 200, "Health endpoint should return 200 OK" + logger.info("Application is running and healthy") + + yield app + + finally: + logger.info("Stopping application") + app.stop() + + +@pytest.fixture +def external_contributor_pull_request( + github_repository: Repository, + forked_github_repository: Repository, + test_config: "TestConfig", +) -> Iterator[PullRequest]: + """Create a pull request from forked repository simulating external contributor. + + Args: + github_repository: The original GitHub repository. + forked_github_repository: The forked repository (different user). + test_config: Test configuration with unique identifiers. + + Yields: + The created pull request. + """ + logger.info( + "Creating pull request from fork to simulate external contributor (test_id: %s)", + test_config.test_id, + ) + pr = create_fork_and_pr(github_repository, forked_github_repository, test_config.test_id) + logger.info("Pull request created: #%d - %s", pr.number, pr.title) + + yield pr + + # Cleanup: Close the PR + logger.info("Cleaning up: Closing pull request #%d", pr.number) + close_pull_request(pr) + logger.info("Pull request closed successfully") + + +@pytest.mark.usefixtures( + "application_with_external_contributor_disabled", "external_contributor_pull_request" +) +def test_external_contributor_disabled_default_security( + github_repository: Repository, + test_config: "TestConfig", +): + """ + arrange: Application running with allow_external_contributor=False, forked repository \ + from external contributor. + act: Create PR from fork and dispatch workflow. + assert: Workflow fails with "Insufficient user authorization" error in logs. + """ + logger.info("Test started: test_external_contributor_disabled_default_security") + # Dispatch a workflow targeting this runner + # Use the unique test label to ensure this runner picks up the job + runner_label = test_config.labels[0] + logger.info("Dispatching workflow with runner label: %s", runner_label) + run = dispatch_workflow_and_get_run( + repository=github_repository, + workflow_filename=DISPATCH_TEST_WORKFLOW_FILENAME, + runner_label=runner_label, + ) + assert run is not None, "Workflow run should be created" + logger.info("Workflow run created: %s (run_id: %d)", run.html_url, run.id) + + # Wait for the workflow to complete + logger.info("Waiting for workflow to complete (timeout: 600 seconds)...") + completed = wait_for_workflow_completion(run, timeout=600) + assert completed, "Workflow should complete within timeout" + logger.info("Workflow completed") + + # Verify the workflow failed due to security check + run.update() + logger.info("Workflow conclusion: %s", run.conclusion) + assert run.conclusion == "failure", ( + f"Workflow should fail when external contributor is blocked. " + f"Got conclusion: {run.conclusion}" + ) + + # Get the job logs and verify the failure reason + logger.info("Retrieving job logs to verify failure reason...") + logs = get_job_logs(run) + assert "Insufficient user authorization" in logs or "author_association" in logs, ( + "Logs should indicate external contributor was blocked. " f"Actual logs: {logs[:500]}" + ) + + +@pytest.mark.usefixtures( + "application_with_external_contributor_enabled", "external_contributor_pull_request" +) +def test_external_contributor_enabled_permissive_mode( + github_repository: Repository, + test_config: "TestConfig", +): + """ + arrange: Application running with allow_external_contributor=True, forked repository \ + from external contributor. + act: Create PR from fork and dispatch workflow. + assert: Workflow succeeds with no authorization errors in logs. + """ + logger.info("Test started: test_external_contributor_enabled_permissive_mode") + # Dispatch a workflow targeting this runner + # Use the unique test label to ensure this runner picks up the job + runner_label = test_config.labels[0] + logger.info("Dispatching workflow with runner label: %s", runner_label) + run = dispatch_workflow_and_get_run( + repository=github_repository, + workflow_filename=DISPATCH_TEST_WORKFLOW_FILENAME, + runner_label=runner_label, + ) + assert run is not None, "Workflow run should be created" + logger.info("Workflow run created: %s (run_id: %d)", run.html_url, run.id) + + # Wait for the workflow to complete + logger.info("Waiting for workflow to complete (timeout: 600 seconds)...") + completed = wait_for_workflow_completion(run, timeout=600) + assert completed, "Workflow should complete within timeout" + logger.info("Workflow completed") + + # Verify the workflow succeeded (external contributor allowed) + run.update() + logger.info("Workflow conclusion: %s", run.conclusion) + assert run.conclusion == "success", ( + f"Workflow should succeed when external contributor is allowed. " + f"Got conclusion: {run.conclusion}" + ) + + # Get the job logs and verify it ran successfully + logger.info("Retrieving job logs to verify success...") + logs = get_job_logs(run) + assert "Should not echo if pre-job script failed" in logs, ( + "Logs should contain the expected success message. " f"Actual logs: {logs[:500]}" + ) + + # Ensure no security errors in logs + assert ( + "Insufficient user authorization" not in logs + ), "Logs should not contain security errors when external contributor is allowed" diff --git a/github-runner-manager/tests/unit/test_pre_job_script.py b/github-runner-manager/tests/unit/test_pre_job_script.py new file mode 100644 index 0000000000..74cfb13840 --- /dev/null +++ b/github-runner-manager/tests/unit/test_pre_job_script.py @@ -0,0 +1,328 @@ +# Copyright 2025 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Unit tests for pre-job script template and execution.""" + +import json +import os +import subprocess +from pathlib import Path +from typing import Dict + +import pytest +from jinja2 import Template + +# GitHub Actions default environment variables that are always present +# Reference: +# https://docs.github.com/en/actions/reference/workflows-and-actions/variables +GITHUB_DEFAULT_ENV_VARS = { + "CI": "true", + "GITHUB_ACTION": "__run", + "GITHUB_ACTIONS": "true", + "GITHUB_ACTOR": "test-user", + "GITHUB_API_URL": "https://api.github.com", + "GITHUB_BASE_REF": "", + "GITHUB_ENV": "/tmp/github_env", + "GITHUB_EVENT_NAME": "pull_request", + "GITHUB_EVENT_PATH": "", # Will be set dynamically in tests + "GITHUB_GRAPHQL_URL": "https://api.github.com/graphql", + "GITHUB_HEAD_REF": "feature-branch", + "GITHUB_JOB": "test-job", + "GITHUB_PATH": "/tmp/github_path", + "GITHUB_REF": "refs/pull/123/merge", + "GITHUB_REF_NAME": "123/merge", + "GITHUB_REF_TYPE": "branch", + "GITHUB_REPOSITORY": "canonical/github-runner-operator", + "GITHUB_REPOSITORY_OWNER": "canonical", + "GITHUB_RUN_ATTEMPT": "1", + "GITHUB_RUN_ID": "1234567890", + "GITHUB_RUN_NUMBER": "42", + "GITHUB_SERVER_URL": "https://github.com", + "GITHUB_SHA": "abc123def456", + "GITHUB_WORKFLOW": "Test Workflow", + "GITHUB_WORKSPACE": "/home/runner/work/repo/repo", + "RUNNER_ARCH": "X64", + "RUNNER_NAME": "test-runner", + "RUNNER_OS": "Linux", + "RUNNER_TEMP": "/tmp", + "RUNNER_TOOL_CACHE": "/opt/hostedtoolcache", +} + + +@pytest.fixture +def pre_job_template() -> Template: + """Load the pre-job.j2 template.""" + template_path = ( + Path(__file__).parent.parent.parent / "src/github_runner_manager/templates/pre-job.j2" + ) + return Template(template_path.read_text()) + + +@pytest.fixture +def github_env_vars(tmp_path: Path) -> Dict[str, str]: + """Provide GitHub Actions environment variables with temporary paths. + + This fixture can be used as a base for parameterized tests requiring + different GitHub event contexts. + """ + env_vars = GITHUB_DEFAULT_ENV_VARS.copy() + # Create temporary files for GitHub Actions file-based environment variables + env_vars["GITHUB_ENV"] = str(tmp_path / "github_env") + env_vars["GITHUB_PATH"] = str(tmp_path / "github_path") + # GITHUB_EVENT_PATH will be set per test + env_vars["GITHUB_EVENT_PATH"] = str(tmp_path / "event.json") + + # Ensure PATH is set for subprocess execution + env_vars["PATH"] = os.environ.get("PATH", "/usr/local/bin:/usr/bin:/bin") + + return env_vars + + +@pytest.fixture +def default_template_vars() -> Dict: + """Provide default template variables for pre-job script rendering. + + This fixture returns the most common configuration with allow_external_contributor + disabled. Tests can override specific values as needed. + """ + return { + "allow_external_contributor": False, + "issue_metrics": False, + "do_repo_policy_check": False, + "dockerhub_mirror": "", + "custom_pre_job_script": "", + } + + +def _create_github_event_payload( + author_association: str, + event_type: str = "pull_request", +) -> Dict: + """Create a GitHub event payload for testing. + + Args: + author_association: The author's association with the repository + (OWNER, MEMBER, COLLABORATOR, CONTRIBUTOR, FIRST_TIME_CONTRIBUTOR, NONE, etc.) + event_type: The type of GitHub event (pull_request, push, etc.) + + Returns: + A dictionary representing the GitHub event payload + """ + payload = { + "action": event_type, + "number": 123, + "pull_request": { + "author_association": author_association, + "number": 123, + "title": "Test PR", + "user": { + "login": "test-user", + "type": "User", + }, + "head": { + "ref": "feature-branch", + "sha": "abc123", + }, + "base": { + "ref": "main", + "sha": "def456", + }, + }, + "repository": { + "name": "github-runner-operator", + "full_name": "canonical/github-runner-operator", + "owner": { + "login": "canonical", + }, + }, + "sender": { + "login": "test-user", + }, + } + return payload + + +def render_and_execute_script( + template: Template, + template_vars: Dict, + env_vars: Dict[str, str], + github_event: Dict, + tmp_path: Path, +) -> subprocess.CompletedProcess: + """Render the pre-job template and execute it with given environment. + + Args: + template: The Jinja2 template to render + template_vars: Variables to pass to the template + env_vars: Environment variables for script execution + github_event: GitHub event payload to write to GITHUB_EVENT_PATH + tmp_path: Temporary directory for files + + Returns: + CompletedProcess with the result of script execution + """ + # Render the template + script_content = template.render(**template_vars) + script_path = tmp_path / "pre-job.sh" + script_path.write_text(script_content) + script_path.chmod(0o755) + + # Write GitHub event payload + event_path = tmp_path / "event.json" + event_path.write_text(json.dumps(github_event)) + env_vars["GITHUB_EVENT_PATH"] = str(event_path) + + # Execute the script + return subprocess.run( + [str(script_path)], + env=env_vars, + capture_output=True, + text=True, + timeout=10, + ) + + +@pytest.mark.parametrize( + "author_association", + ["OWNER", "MEMBER", "COLLABORATOR"], +) +def test_allow_external_contributor_disabled_allows_trusted_roles( + pre_job_template: Template, + github_env_vars: Dict[str, str], + tmp_path: Path, + author_association: str, + default_template_vars: Dict, +): + """Test that OWNER/MEMBER/COLLABORATOR are all allowed.""" + github_event = _create_github_event_payload(author_association=author_association) + + result = render_and_execute_script( + template=pre_job_template, + template_vars=default_template_vars, + env_vars=github_env_vars, + github_event=github_event, + tmp_path=tmp_path, + ) + + assert result.returncode == 0, ( + f"Expected exit code 0 for {author_association}, got {result.returncode}\n" + f"stderr: {result.stderr}" + ) + assert f"Author association: {author_association}" in result.stderr + assert "The contributor check has passed, proceeding to execute jobs" in result.stderr + assert "Insufficient user authorization" not in result.stderr + + +@pytest.mark.parametrize( + "author_association", + ["CONTRIBUTOR", "FIRST_TIME_CONTRIBUTOR", "FIRST_TIMER", "NONE"], +) +def test_allow_external_contributor_disabled_blocks_untrusted_roles( + pre_job_template: Template, + github_env_vars: Dict[str, str], + tmp_path: Path, + author_association: str, + default_template_vars: Dict, +): + """Test that untrusted author associations are blocked.""" + github_event = _create_github_event_payload(author_association=author_association) + + result = render_and_execute_script( + template=pre_job_template, + template_vars=default_template_vars, + env_vars=github_env_vars, + github_event=github_event, + tmp_path=tmp_path, + ) + + assert result.returncode == 1, ( + f"Expected exit code 1 for {author_association}, got {result.returncode}\n" + f"stderr: {result.stderr}" + ) + assert f"Author association: {author_association}" in result.stderr + assert "Insufficient user authorization (author_association)" in result.stderr + assert "Only OWNER, MEMBER, or COLLABORATOR may run workflows" in result.stderr + + +def test_allow_external_contributor_enabled_skips_check( + pre_job_template: Template, + github_env_vars: Dict[str, str], + tmp_path: Path, + default_template_vars: Dict, +): + """Test that check is skipped when allow_external_contributor=True.""" + template_vars = {**default_template_vars, "allow_external_contributor": True} + + # Even with CONTRIBUTOR, script should succeed + github_event = _create_github_event_payload(author_association="CONTRIBUTOR") + + result = render_and_execute_script( + template=pre_job_template, + template_vars=template_vars, + env_vars=github_env_vars, + github_event=github_event, + tmp_path=tmp_path, + ) + + assert ( + result.returncode == 0 + ), f"Expected exit code 0, got {result.returncode}\nstderr: {result.stderr}" + # The check wasn't performed, so these messages shouldn't appear + assert "AUTHOR_ASSOCIATION" not in result.stderr + assert "Insufficient user authorization" not in result.stderr + assert "The contributor check has passed" not in result.stderr + + +@pytest.mark.parametrize( + "github_event,description", + [ + ( + { + "ref": "refs/heads/main", + "repository": { + "name": "github-runner-operator", + "full_name": "canonical/github-runner-operator", + }, + }, + "missing pull_request field (push event)", + ), + ( + { + "pull_request": { + "number": 123, + "title": "Test PR", + }, + "repository": { + "name": "github-runner-operator", + }, + }, + "missing author_association field", + ), + ], + ids=["no_pull_request", "no_author_association"], +) +def test_allow_external_contributor_null_or_missing_fields_blocked( + pre_job_template: Template, + github_env_vars: Dict[str, str], + tmp_path: Path, + github_event: Dict, + description: str, + default_template_vars: Dict, +): + """Test that null/missing fields are blocked when check is enabled.""" + result = render_and_execute_script( + template=pre_job_template, + template_vars=default_template_vars, + env_vars=github_env_vars, + github_event=github_event, + tmp_path=tmp_path, + ) + + # When pull_request or author_association is missing, jq returns "null" + # The check should fail as "null" != "OWNER|MEMBER|COLLABORATOR" + assert result.returncode == 1, ( + f"Expected exit code 1 for {description}, got {result.returncode}\n" + f"stderr: {result.stderr}" + ) + assert "Author association: null" in result.stderr + assert "Insufficient user authorization" in result.stderr From 1e796ff287e974e3d4e5396b85e944196198989c Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 20 Nov 2025 14:33:20 +0800 Subject: [PATCH 21/82] refactor: simplify configuration file reading in RunningApplication class --- github-runner-manager/tests/integration/application.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/github-runner-manager/tests/integration/application.py b/github-runner-manager/tests/integration/application.py index e40359ab2e..54065fd160 100644 --- a/github-runner-manager/tests/integration/application.py +++ b/github-runner-manager/tests/integration/application.py @@ -103,8 +103,7 @@ def config(self) -> dict[str, Any]: Returns: The configuration dictionary. """ - with open(self.config_file_path, "r", encoding="utf-8") as f: - return yaml.safe_load(f) + return yaml.safe_load(self.config_file_path.read_text(encoding="utf-8")) def get(self, path: str, **kwargs: Any) -> requests.Response: """Make a GET request to the application. From 9200a9b15d01f93c1f3dca0198f67b1be126f2d9 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 20 Nov 2025 14:33:31 +0800 Subject: [PATCH 22/82] chore: remove deprecated OpenStack ARM64 options from pytest configuration --- tests/conftest.py | 46 ---------------------------------------------- 1 file changed, 46 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 9968b1d664..8d6821d34a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -127,52 +127,6 @@ def pytest_addoption(parser: Parser): action="store", help="The Openstack region to authenticate to.", ) - # Private endpoint options ARM64 - parser.addoption( - "--openstack-network-name-arm64", - action="store", - help="The Openstack network to create testing instances under.", - ) - parser.addoption( - "--openstack-flavor-name-arm64", - action="store", - help="The Openstack flavor to create testing instances with.", - ) - parser.addoption( - "--openstack-auth-url-arm64", - action="store", - help="The URL to Openstack authentication service, i.e. keystone.", - ) - parser.addoption( - "--openstack-password-arm64", - action="store", - help="The password to authenticate to Openstack service.", - ) - parser.addoption( - "--openstack-project-domain-name-arm64", - action="store", - help="The Openstack project domain name to use.", - ) - parser.addoption( - "--openstack-project-name-arm64", - action="store", - help="The Openstack project name to use.", - ) - parser.addoption( - "--openstack-user-domain-name-arm64", - action="store", - help="The Openstack user domain name to use.", - ) - parser.addoption( - "--openstack-username-arm64", - action="store", - help="The Openstack user to authenticate as.", - ) - parser.addoption( - "--openstack-region-name-arm64", - action="store", - help="The Openstack region to authenticate to.", - ) # Interface testing args parser.addoption( "--openstack-test-image", From d4a3453e5b6da17febcf8246125f1af766b8701f Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 20 Nov 2025 17:26:50 +0800 Subject: [PATCH 23/82] feat: add integration tests configuration to GitHub Actions and tox --- .../workflows/test_github_runner_manager.yaml | 25 +++++++++++++- github-runner-manager/tox.ini | 34 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test_github_runner_manager.yaml b/.github/workflows/test_github_runner_manager.yaml index 3f7eff3758..09168d1bcf 100644 --- a/.github/workflows/test_github_runner_manager.yaml +++ b/.github/workflows/test_github_runner_manager.yaml @@ -4,10 +4,33 @@ on: pull_request: jobs: - unit-tests-github-runner-manager: + application-unit-tests: + name: Unit tests for github-runner-manager uses: canonical/operator-workflows/.github/workflows/test.yaml@main secrets: inherit with: self-hosted-runner: true self-hosted-runner-label: edge working-directory: ./github-runner-manager/ + application-integration-tests: + name: Integration tests for github-runner-manager + runs-on: [self-hosted, pfe-ci] + steps: + - name: Checkout code + uses: actions/checkout@v5.0.1 + - name: Setup Astral UV + uses: astral-sh/setup-uv@v7 + - name: Install tox with UV + run: uv tool install tox --with tox-uv + - name: Run integration tests + run: uv run tox -e integration -- ./github-runner-manager/tests/integration/ + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GITHUB_TOKEN_ALT: ${{ secrets.GITHUB_TOKEN_ALT }} + GITHUB_REPOSITORY: canonical/github-runner-operator + OS_AUTH_URL: ${{ secrets.OS_AUTH_URL }} + OS_PROJECT_NAME: ${{ secrets.OS_PROJECT_NAME }} + OS_USERNAME: ${{ secrets.OS_USERNAME }} + OS_PASSWORD: ${{ secrets.OS_PASSWORD }} + OS_NETWORK: ${{ secrets.OS_NETWORK }} + OS_REGION_NAME: ${{ secrets.OS_REGION_NAME }} diff --git a/github-runner-manager/tox.ini b/github-runner-manager/tox.ini index 83811fac8f..e10e8318b5 100644 --- a/github-runner-manager/tox.ini +++ b/github-runner-manager/tox.ini @@ -98,3 +98,37 @@ deps = -r{toxinidir}/requirements.txt commands = bandit -c {toxinidir}/pyproject.toml -r {[vars]src_path} + +[testenv:integration] +description = Run integration tests +deps = + pytest + pytest-asyncio + pyyaml + requests + pygithub + -r{toxinidir}/requirements.txt + -r{toxinidir}/tests/integration/requirements.txt +passenv = + PYTHONPATH + GITHUB_TOKEN + GITHUB_TOKEN_ALT + GITHUB_REPOSITORY + OS_AUTH_URL + OS_PROJECT_NAME + OS_USERNAME + OS_PASSWORD + OS_NETWORK + OS_REGION_NAME +commands = + pytest {[vars]tst_path}integration \ + --github-token={env:GITHUB_TOKEN:} \ + --github-token-alt={env:GITHUB_TOKEN_ALT:} \ + --github-repository={env:GITHUB_REPOSITORY:} \ + --openstack-auth-url={env:OS_AUTH_URL:} \ + --openstack-project={env:OS_PROJECT_NAME:} \ + --openstack-username={env:OS_USERNAME:} \ + --openstack-password={env:OS_PASSWORD:} \ + --openstack-network={env:OS_NETWORK:} \ + --openstack-region={env:OS_REGION_NAME:} \ + -v --tb native -s {posargs} From 7134402a7b0dcb74df3a252d50652a79b1c65f21 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 20 Nov 2025 12:20:35 +0000 Subject: [PATCH 24/82] chore: lint fixes --- .../tests/integration/application.py | 4 +++- .../tests/integration/conftest.py | 16 ++++++++-------- .../tests/integration/github_helpers.py | 5 ++++- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/github-runner-manager/tests/integration/application.py b/github-runner-manager/tests/integration/application.py index 54065fd160..5a5452514d 100644 --- a/github-runner-manager/tests/integration/application.py +++ b/github-runner-manager/tests/integration/application.py @@ -76,6 +76,8 @@ class RunningApplication: instance running in a separate process for integration testing. Attributes: + base_url: The base URL (e.g., http://127.0.0.1:8080). + config: The configuration dictionary. process: The multiprocessing.Process running the application. port: The port the application is listening on. host: The host the application is listening on. @@ -110,7 +112,7 @@ def get(self, path: str, **kwargs: Any) -> requests.Response: Args: path: The path to request (e.g., /health). - **kwargs: Additional arguments to pass to requests.get. + kwargs: Additional arguments to pass to requests.get. Returns: The response object. diff --git a/github-runner-manager/tests/integration/conftest.py b/github-runner-manager/tests/integration/conftest.py index e7127bbfeb..361ea9fbc8 100644 --- a/github-runner-manager/tests/integration/conftest.py +++ b/github-runner-manager/tests/integration/conftest.py @@ -39,8 +39,8 @@ def github_config(pytestconfig: pytest.Config) -> GitHubConfig: if not token or not path: pytest.skip( - "GitHub configuration not provided. Use --github-token and --github-repository options or " - "set GITHUB_TOKEN and GITHUB_REPOSITORY environment variables." + "GitHub configuration not provided. Use --github-token and --github-repository " + "options or set GITHUB_TOKEN and GITHUB_REPOSITORY environment variables." ) return GitHubConfig(token=token, path=path) @@ -56,12 +56,12 @@ def openstack_config(pytestconfig: pytest.Config) -> OpenStackConfig | None: Returns: OpenStack configuration object, or None if not all parameters are provided. """ - auth_url = pytestconfig.getoption("--openstack-auth-url") or os.getenv("OS_AUTH_URL") - project = pytestconfig.getoption("--openstack-project") or os.getenv("OS_PROJECT_NAME") - username = pytestconfig.getoption("--openstack-username") or os.getenv("OS_USERNAME") - password = pytestconfig.getoption("--openstack-password") or os.getenv("OS_PASSWORD") - network = pytestconfig.getoption("--openstack-network") or os.getenv("OS_NETWORK") - region_name = pytestconfig.getoption("--openstack-region") or os.getenv("OS_REGION_NAME") + auth_url = pytestconfig.getoption("--openstack-auth-url") or os.getenv("OS_AUTH_URL", "") + project = pytestconfig.getoption("--openstack-project") or os.getenv("OS_PROJECT_NAME", "") + username = pytestconfig.getoption("--openstack-username") or os.getenv("OS_USERNAME", "") + password = pytestconfig.getoption("--openstack-password") or os.getenv("OS_PASSWORD", "") + network = pytestconfig.getoption("--openstack-network") or os.getenv("OS_NETWORK", "") + region_name = pytestconfig.getoption("--openstack-region") or os.getenv("OS_REGION_NAME", "") # Only return config if all required parameters are provided if all([auth_url, project, username, password, network, region_name]): diff --git a/github-runner-manager/tests/integration/github_helpers.py b/github-runner-manager/tests/integration/github_helpers.py index 45d28b5108..b39c4f746e 100644 --- a/github-runner-manager/tests/integration/github_helpers.py +++ b/github-runner-manager/tests/integration/github_helpers.py @@ -47,7 +47,10 @@ def create_fork_and_pr( # Create PR from fork to original repository pr = github_repository.create_pull( title=f"Test PR from fork {test_id}", - body=f"This is a test PR from a forked repository for testing external contributor security. Test ID: {test_id}", + body=( + f"This is a test PR from a forked repository for testing " + f"external contributor security. Test ID: {test_id}" + ), head=f"{forked_github_repository.owner.login}:{branch_name}", base=github_repository.default_branch, ) From a850b007d3da240d67f13134a122134101760562 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 21 Nov 2025 03:00:15 +0000 Subject: [PATCH 25/82] refactor: streamline OpenStack options and remove deprecated token_alt fixture --- tests/conftest.py | 72 +++++++++-------------------------- tests/integration/conftest.py | 40 +++++++------------ 2 files changed, 31 insertions(+), 81 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 9968b1d664..fed9d64b89 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,6 +3,7 @@ """Fixtures for github runner charm.""" +import os from pytest import Parser @@ -83,95 +84,56 @@ def pytest_addoption(parser: Parser): ) # Private endpoint options AMD64 parser.addoption( - "--openstack-network-name-amd64", + "--openstack-network-name", action="store", help="The Openstack network to create testing instances under.", ) parser.addoption( - "--openstack-flavor-name-amd64", + "--openstack-flavor-name", action="store", help="The Openstack flavor to create testing instances with.", ) parser.addoption( - "--openstack-auth-url-amd64", + "--openstack-auth-url", action="store", help="The URL to Openstack authentication service, i.e. keystone.", + default=os.environ.get("OS_AUTH_URL"), ) parser.addoption( - "--openstack-password-amd64", + "--openstack-password", action="store", help="The password to authenticate to Openstack service.", + default=os.environ.get("OS_PASSWORD"), ) parser.addoption( - "--openstack-project-domain-name-amd64", + "--openstack-project-domain-name", action="store", help="The Openstack project domain name to use.", + default=os.environ.get("OS_PROJECT_DOMAIN_NAME"), ) parser.addoption( - "--openstack-project-name-amd64", + "--openstack-project-name", action="store", help="The Openstack project name to use.", + default=os.environ.get("OS_PROJECT_NAME"), ) parser.addoption( - "--openstack-user-domain-name-amd64", + "--openstack-user-domain-name", action="store", help="The Openstack user domain name to use.", + default=os.environ.get("OS_USER_DOMAIN_NAME"), ) parser.addoption( - "--openstack-username-amd64", + "--openstack-username", action="store", help="The Openstack user to authenticate as.", + default=os.environ.get("OS_USERNAME"), ) parser.addoption( - "--openstack-region-name-amd64", - action="store", - help="The Openstack region to authenticate to.", - ) - # Private endpoint options ARM64 - parser.addoption( - "--openstack-network-name-arm64", - action="store", - help="The Openstack network to create testing instances under.", - ) - parser.addoption( - "--openstack-flavor-name-arm64", - action="store", - help="The Openstack flavor to create testing instances with.", - ) - parser.addoption( - "--openstack-auth-url-arm64", - action="store", - help="The URL to Openstack authentication service, i.e. keystone.", - ) - parser.addoption( - "--openstack-password-arm64", - action="store", - help="The password to authenticate to Openstack service.", - ) - parser.addoption( - "--openstack-project-domain-name-arm64", - action="store", - help="The Openstack project domain name to use.", - ) - parser.addoption( - "--openstack-project-name-arm64", - action="store", - help="The Openstack project name to use.", - ) - parser.addoption( - "--openstack-user-domain-name-arm64", - action="store", - help="The Openstack user domain name to use.", - ) - parser.addoption( - "--openstack-username-arm64", - action="store", - help="The Openstack user to authenticate as.", - ) - parser.addoption( - "--openstack-region-name-arm64", + "--openstack-region-name", action="store", help="The Openstack region to authenticate to.", + default=os.environ.get("OS_REGION_NAME"), ) # Interface testing args parser.addoption( diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index bf9f9a01a7..e48c85fec3 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -132,18 +132,6 @@ def token(pytestconfig: pytest.Config) -> str: return random_token -@pytest.fixture(scope="module") -def token_alt(pytestconfig: pytest.Config, token: str) -> str: - """Configured token_alt setting.""" - token_alt = pytestconfig.getoption("--token-alt") or os.environ.get("INTEGRATION_TOKEN_ALT") - assert token_alt, ( - "Please specify the --token-alt command line option with GitHub Personal " - "Access Token value." - ) - assert token_alt != token, "Please specify a different token for --token-alt" - return token_alt - - @pytest.fixture(scope="module") def http_proxy(pytestconfig: pytest.Config) -> str: """Configured http_proxy setting.""" @@ -189,17 +177,17 @@ def openstack_no_proxy_fixture(pytestconfig: pytest.Config) -> str: @pytest.fixture(scope="module", name="private_endpoint_config") def private_endpoint_config_fixture(pytestconfig: pytest.Config) -> PrivateEndpointConfigs | None: """The private endpoint configuration values.""" - auth_url = pytestconfig.getoption("--openstack-auth-url-amd64") - password = pytestconfig.getoption("--openstack-password-amd64") - password = password or os.environ.get("INTEGRATION_OPENSTACK_PASSWORD_AMD64") - assert ( - password - ), "Please specify the --openstack-password-amd64 option or INTEGRATION_OPENSTACK_PASSWORD_AMD64 environment variable" - project_domain_name = pytestconfig.getoption("--openstack-project-domain-name-amd64") - project_name = pytestconfig.getoption("--openstack-project-name-amd64") - user_domain_name = pytestconfig.getoption("--openstack-user-domain-name-amd64") - user_name = pytestconfig.getoption("--openstack-username-amd64") - region_name = pytestconfig.getoption("--openstack-region-name-amd64") + auth_url = pytestconfig.getoption("--openstack-auth-url") + password = pytestconfig.getoption("--openstack-password") + assert password, ( + "Please specify the --openstack-password option or " + "INTEGRATION_OPENSTACK_PASSWORD or OS_PASSWORD environment variable" + ) + project_domain_name = pytestconfig.getoption("--openstack-project-domain-name") + project_name = pytestconfig.getoption("--openstack-project-name") + user_domain_name = pytestconfig.getoption("--openstack-user-domain-name") + user_name = pytestconfig.getoption("--openstack-username") + region_name = pytestconfig.getoption("--openstack-region-name") if any( not val for val in ( @@ -264,15 +252,15 @@ def clouds_yaml_contents_fixture( @pytest.fixture(scope="module", name="network_name") def network_name_fixture(pytestconfig: pytest.Config) -> str: """Network to use to spawn test instances under.""" - network_name = pytestconfig.getoption("--openstack-network-name-amd64") - assert network_name, "Please specify the --openstack-network-name-amd64 command line option" + network_name = pytestconfig.getoption("--openstack-network-name") + assert network_name, "Please specify the --openstack-network-name command line option" return network_name @pytest.fixture(scope="module", name="flavor_name") def flavor_name_fixture(pytestconfig: pytest.Config) -> str: """Flavor to create testing instances with.""" - flavor_name = pytestconfig.getoption("--openstack-flavor-name-amd64") + flavor_name = pytestconfig.getoption("--openstack-flavor-name") assert flavor_name, "Please specify the --openstack-flavor-name command line option" return flavor_name From 3ef26d0baa87850f6573fd77874fb270af252847 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 21 Nov 2025 03:02:09 +0000 Subject: [PATCH 26/82] chore: update integration test environment variables for OpenStack --- tox.ini | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index a09c975544..c691f5f7bc 100644 --- a/tox.ini +++ b/tox.ini @@ -114,9 +114,15 @@ commands = description = Run integration tests pass_env = PYTEST_ADDOPTS - INTEGRATION_OPENSTACK_PASSWORD_AMD64 INTEGRATION_TOKEN INTEGRATION_TOKEN_ALT + OS_AUTH_URL + OS_PASSWORD + OS_PROJECT_DOMAIN_NAME + OS_PROJECT_NAME + OS_REGION_NAME + OS_USER_DOMAIN_NAME + OS_USERNAME deps = juju3.1: juju==3.1.* juju3.6: juju==3.6.* From f2953418345272c1db62e69c326c511dd30dec09 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 21 Nov 2025 03:16:21 +0000 Subject: [PATCH 27/82] chore: undo charm CI testing changes --- .github/workflows/integration_test.yaml | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index fb9d48c15b..1384a602ed 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -14,32 +14,16 @@ concurrency: jobs: openstack-integration-tests-private-endpoint: name: Integration test using private-endpoint - uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@feat/mattermost-ping - secrets: inherit - with: - juju-channel: 3.6/stable - provider: lxd - test-tox-env: integration-juju3.6 - modules: '["test_charm_metrics_failure", "test_charm_metrics_success", "test_charm_fork_repo", "test_charm_fork_path_change", "test_charm_no_runner", "test_charm_runner", "test_debug_ssh", "test_charm_upgrade", "test_reactive"]' - extra-arguments: '-m openstack --log-format="%(asctime)s %(levelname)s %(message)s"' - self-hosted-runner: true - self-hosted-runner-label: pfe-ci - openstack-integration-tests-cross-controller-private-endpoint: - name: Cross controller integration test using private-endpoint uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main secrets: inherit with: juju-channel: 3.6/stable - pre-run-script: tests/integration/setup-integration-tests.sh provider: lxd test-tox-env: integration-juju3.6 - modules: '["test_prometheus_metrics"]' + modules: '["test_charm_metrics_failure", "test_charm_metrics_success", "test_charm_fork_repo", "test_charm_fork_path_change", "test_charm_no_runner", "test_charm_runner", "test_debug_ssh", "test_charm_upgrade", "test_reactive"]' extra-arguments: '-m openstack --log-format="%(asctime)s %(levelname)s %(message)s"' self-hosted-runner: true self-hosted-runner-label: pfe-ci - tmate-debug: true - tmate-timeout: 90 - mattermost-message: "Test result available @charlie4284!" openstack-integration-tests-cross-controller-private-endpoint: name: Cross controller integration test using private-endpoint uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main From 97e063f799bf9ad721a513d3bf93b16c92985139 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 21 Nov 2025 03:19:01 +0000 Subject: [PATCH 28/82] chore: refactor testing args parsing to upper level --- github-runner-manager/tests/conftest.py | 9 ++++++++- .../tests/integration/conftest.py | 18 ++++++++---------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/github-runner-manager/tests/conftest.py b/github-runner-manager/tests/conftest.py index 98aa6d2e81..288196fec1 100644 --- a/github-runner-manager/tests/conftest.py +++ b/github-runner-manager/tests/conftest.py @@ -3,6 +3,8 @@ """Fixtures for charm tests.""" +import os + def pytest_addoption(parser): """Parse additional pytest options. @@ -10,7 +12,6 @@ def pytest_addoption(parser): Args: parser: Pytest parser. """ - parser.addoption("--charm-file", action="store") parser.addoption( "--github-token", action="store", @@ -30,29 +31,35 @@ def pytest_addoption(parser): "--openstack-auth-url", action="store", help="OpenStack authentication URL for integration tests.", + default=os.getenv("OS_AUTH_URL"), ) parser.addoption( "--openstack-project", action="store", help="OpenStack project name for integration tests.", + default=os.getenv("OS_PROJECT_NAME"), ) parser.addoption( "--openstack-username", action="store", help="OpenStack username for integration tests.", + default=os.getenv("OS_USERNAME"), ) parser.addoption( "--openstack-password", action="store", help="OpenStack password for integration tests.", + default=os.getenv("OS_PASSWORD"), ) parser.addoption( "--openstack-network", action="store", help="OpenStack network name for integration tests.", + default=os.getenv("OS_NETWORK"), ) parser.addoption( "--openstack-region", action="store", help="OpenStack region name for integration tests.", + default=os.getenv("OS_REGION_NAME"), ) diff --git a/github-runner-manager/tests/integration/conftest.py b/github-runner-manager/tests/integration/conftest.py index 361ea9fbc8..05e46ae7e7 100644 --- a/github-runner-manager/tests/integration/conftest.py +++ b/github-runner-manager/tests/integration/conftest.py @@ -3,8 +3,6 @@ """Fixtures for github-runner-manager integration tests.""" -import os - import pytest from .factories import GitHubConfig, OpenStackConfig, TestConfig @@ -34,8 +32,8 @@ def github_config(pytestconfig: pytest.Config) -> GitHubConfig: pytest.skip: If neither --github-token option nor GITHUB_TOKEN environment variable is set. """ - token = pytestconfig.getoption("--github-token") or os.getenv("GITHUB_TOKEN") - path = pytestconfig.getoption("--github-repository") or os.getenv("GITHUB_REPOSITORY") + token = pytestconfig.getoption("--github-token") + path = pytestconfig.getoption("--github-repository") if not token or not path: pytest.skip( @@ -56,12 +54,12 @@ def openstack_config(pytestconfig: pytest.Config) -> OpenStackConfig | None: Returns: OpenStack configuration object, or None if not all parameters are provided. """ - auth_url = pytestconfig.getoption("--openstack-auth-url") or os.getenv("OS_AUTH_URL", "") - project = pytestconfig.getoption("--openstack-project") or os.getenv("OS_PROJECT_NAME", "") - username = pytestconfig.getoption("--openstack-username") or os.getenv("OS_USERNAME", "") - password = pytestconfig.getoption("--openstack-password") or os.getenv("OS_PASSWORD", "") - network = pytestconfig.getoption("--openstack-network") or os.getenv("OS_NETWORK", "") - region_name = pytestconfig.getoption("--openstack-region") or os.getenv("OS_REGION_NAME", "") + auth_url = pytestconfig.getoption("--openstack-auth-url") + project = pytestconfig.getoption("--openstack-project") + username = pytestconfig.getoption("--openstack-username") + password = pytestconfig.getoption("--openstack-password") + network = pytestconfig.getoption("--openstack-network") + region_name = pytestconfig.getoption("--openstack-region") # Only return config if all required parameters are provided if all([auth_url, project, username, password, network, region_name]): From f528f4505ac181b7a15b3cdde34a9327161b1032 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 21 Nov 2025 03:22:30 +0000 Subject: [PATCH 29/82] refactor: remove environment variable dependency for alternate GitHub token in tests --- .../integration/test_external_contributor_security.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/github-runner-manager/tests/integration/test_external_contributor_security.py b/github-runner-manager/tests/integration/test_external_contributor_security.py index 5224134605..43d449d478 100644 --- a/github-runner-manager/tests/integration/test_external_contributor_security.py +++ b/github-runner-manager/tests/integration/test_external_contributor_security.py @@ -4,7 +4,6 @@ """Integration tests for external contributor security configuration.""" import logging -import os import secrets from pathlib import Path from time import sleep @@ -83,15 +82,15 @@ def forked_github_repository( """ # Try to get alternate token for creating fork # This simulates a different user (external contributor) - alt_token = pytestconfig.getoption("--github-token-alt", None) or os.getenv("GITHUB_TOKEN_ALT") + alt_token = pytestconfig.getoption("--github-token-alt", None) if not alt_token: logger.warning( "Alternate GitHub token not provided. Skipping test requiring fork creation." ) pytest.fail( - "Alternate GitHub token not provided. Use --github-token-alt option or " - "set GITHUB_TOKEN_ALT environment variable to test with actual fork." + "Alternate GitHub token not provided. Use --github-token-alt option to test with " + "actual fork." ) logger.info( From 2ffb626bd9b65f2acaf0a4c9a80792b7688f1995 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 21 Nov 2025 03:23:24 +0000 Subject: [PATCH 30/82] chore: apply pflake8 rules to all tests --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index e5121fb41b..4721d14afd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,7 +54,7 @@ select = ["E", "W", "F", "C", "N", "R", "D", "H"] ignore = ["W503", "E501", "D107", "E203"] # D100, D101, D102, D103, D104: Ignore docstring style issues in tests # temporary disable E402 for the fix in charm.py for lp:2058335 -per-file-ignores = ["src/charm.py:E402", "tests/*:D100,D101,D102,D103,D104,D205,D212"] +per-file-ignores = ["src/charm.py:E402", "**/tests/*:D100,D101,D102,D103,D104,D205,D212"] docstring-convention = "google" # Check for properly formatted copyright header in each file copyright-check = "True" From 0e7133d96575ec15d2ca76ebcb38298a122b64e9 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 21 Nov 2025 03:25:26 +0000 Subject: [PATCH 31/82] chore: remove reference to unused env var --- tests/integration/conftest.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index e48c85fec3..859e8bdf16 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -179,10 +179,9 @@ def private_endpoint_config_fixture(pytestconfig: pytest.Config) -> PrivateEndpo """The private endpoint configuration values.""" auth_url = pytestconfig.getoption("--openstack-auth-url") password = pytestconfig.getoption("--openstack-password") - assert password, ( - "Please specify the --openstack-password option or " - "INTEGRATION_OPENSTACK_PASSWORD or OS_PASSWORD environment variable" - ) + assert ( + password + ), "Please specify the --openstack-password option or OS_PASSWORD environment variable" project_domain_name = pytestconfig.getoption("--openstack-project-domain-name") project_name = pytestconfig.getoption("--openstack-project-name") user_domain_name = pytestconfig.getoption("--openstack-user-domain-name") From 03102bb195394d0604ff7fc3a0e9034d33a1bdca Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 01:49:53 +0000 Subject: [PATCH 32/82] test: adapt tests to latest GH repository secrets --- .../workflows/test_github_runner_manager.yaml | 22 ++++++++++--------- github-runner-manager/tests/conftest.py | 2 ++ github-runner-manager/tox.ini | 4 ++-- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/.github/workflows/test_github_runner_manager.yaml b/.github/workflows/test_github_runner_manager.yaml index 09168d1bcf..dec48e0371 100644 --- a/.github/workflows/test_github_runner_manager.yaml +++ b/.github/workflows/test_github_runner_manager.yaml @@ -23,14 +23,16 @@ jobs: - name: Install tox with UV run: uv tool install tox --with tox-uv - name: Run integration tests - run: uv run tox -e integration -- ./github-runner-manager/tests/integration/ + working-directory: ./github-runner-manager/ env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - GITHUB_TOKEN_ALT: ${{ secrets.GITHUB_TOKEN_ALT }} - GITHUB_REPOSITORY: canonical/github-runner-operator - OS_AUTH_URL: ${{ secrets.OS_AUTH_URL }} - OS_PROJECT_NAME: ${{ secrets.OS_PROJECT_NAME }} - OS_USERNAME: ${{ secrets.OS_USERNAME }} - OS_PASSWORD: ${{ secrets.OS_PASSWORD }} - OS_NETWORK: ${{ secrets.OS_NETWORK }} - OS_REGION_NAME: ${{ secrets.OS_REGION_NAME }} + # GitHub configuration + INTEGRATION_TOKEN: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_2 }} + INTEGRATION_TOKEN_ALT: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_3 }} + # OpenStack configuration + OS_AUTH_URL: ${{ vars.INTEGRATION_TEST_OPENSTACK_AUTH_URL }} + OS_PROJECT_NAME: ${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_NAME }} + OS_USERNAME: ${{ vars.INTEGRATION_TEST_OPENSTACK_USERNAME }} + OS_PASSWORD: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_1 }} + OS_NETWORK: ${{ vars.INTEGRATION_TEST_OPENSTACK_NETWORK_NAME }} + OS_REGION_NAME: ${{ vars.INTEGRATION_TEST_OPENSTACK_REGION_NAME }} + run: tox -e integration -- -v --tb=native -s --github-repository=${{ vars.INTEGRATION_TEST_PATH }} diff --git a/github-runner-manager/tests/conftest.py b/github-runner-manager/tests/conftest.py index 288196fec1..b18521ff5c 100644 --- a/github-runner-manager/tests/conftest.py +++ b/github-runner-manager/tests/conftest.py @@ -16,6 +16,7 @@ def pytest_addoption(parser): "--github-token", action="store", help="GitHub personal access token for integration tests.", + default=os.getenv("INTEGRATION_TOKEN"), ) parser.addoption( "--github-repository", @@ -26,6 +27,7 @@ def pytest_addoption(parser): "--github-token-alt", action="store", help="Alternate GitHub token from a different user for fork testing.", + default=os.getenv("INTEGRATION_TOKEN_ALT"), ) parser.addoption( "--openstack-auth-url", diff --git a/github-runner-manager/tox.ini b/github-runner-manager/tox.ini index e10e8318b5..6cc4d17fa2 100644 --- a/github-runner-manager/tox.ini +++ b/github-runner-manager/tox.ini @@ -111,9 +111,9 @@ deps = -r{toxinidir}/tests/integration/requirements.txt passenv = PYTHONPATH - GITHUB_TOKEN - GITHUB_TOKEN_ALT GITHUB_REPOSITORY + INTEGRATION_TOKEN + INTEGRATION_TOKEN_ALT OS_AUTH_URL OS_PROJECT_NAME OS_USERNAME From ebcffe619dff9f6ef61b80da2b4f24a5cb79cf12 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 01:54:21 +0000 Subject: [PATCH 33/82] chore: remove integration test requirements.txt --- github-runner-manager/tox.ini | 1 - 1 file changed, 1 deletion(-) diff --git a/github-runner-manager/tox.ini b/github-runner-manager/tox.ini index 6cc4d17fa2..62cac38e86 100644 --- a/github-runner-manager/tox.ini +++ b/github-runner-manager/tox.ini @@ -108,7 +108,6 @@ deps = requests pygithub -r{toxinidir}/requirements.txt - -r{toxinidir}/tests/integration/requirements.txt passenv = PYTHONPATH GITHUB_REPOSITORY From 9e44423b42ddd9f5aa8ae6b059ea113732cc242c Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 02:03:17 +0000 Subject: [PATCH 34/82] chore: tidy integration test & fail if required params not supplied --- .../tests/integration/__init__.py | 2 ++ .../tests/integration/conftest.py | 24 ++++++++++--------- .../test_external_contributor_security.py | 4 ++-- 3 files changed, 17 insertions(+), 13 deletions(-) create mode 100644 github-runner-manager/tests/integration/__init__.py diff --git a/github-runner-manager/tests/integration/__init__.py b/github-runner-manager/tests/integration/__init__.py new file mode 100644 index 0000000000..97bca57b0b --- /dev/null +++ b/github-runner-manager/tests/integration/__init__.py @@ -0,0 +1,2 @@ +# Copyright 2025 Canonical Ltd. +# See LICENSE file for licensing details. diff --git a/github-runner-manager/tests/integration/conftest.py b/github-runner-manager/tests/integration/conftest.py index 05e46ae7e7..996e1dfb97 100644 --- a/github-runner-manager/tests/integration/conftest.py +++ b/github-runner-manager/tests/integration/conftest.py @@ -36,7 +36,7 @@ def github_config(pytestconfig: pytest.Config) -> GitHubConfig: path = pytestconfig.getoption("--github-repository") if not token or not path: - pytest.skip( + pytest.fail( "GitHub configuration not provided. Use --github-token and --github-repository " "options or set GITHUB_TOKEN and GITHUB_REPOSITORY environment variables." ) @@ -62,13 +62,15 @@ def openstack_config(pytestconfig: pytest.Config) -> OpenStackConfig | None: region_name = pytestconfig.getoption("--openstack-region") # Only return config if all required parameters are provided - if all([auth_url, project, username, password, network, region_name]): - return OpenStackConfig( - auth_url=auth_url, - project=project, - username=username, - password=password, - network=network, - region_name=region_name, - ) - return None + assert all( + [auth_url, project, username, password, network, region_name] + ), "All OpenStack configurations must be supplied" + + return OpenStackConfig( + auth_url=auth_url, + project=project, + username=username, + password=password, + network=network, + region_name=region_name, + ) diff --git a/github-runner-manager/tests/integration/test_external_contributor_security.py b/github-runner-manager/tests/integration/test_external_contributor_security.py index 43d449d478..5a42170bd1 100644 --- a/github-runner-manager/tests/integration/test_external_contributor_security.py +++ b/github-runner-manager/tests/integration/test_external_contributor_security.py @@ -138,7 +138,7 @@ def application_with_external_contributor_disabled( tmp_path: Path, github_config: GitHubConfig, openstack_config: OpenStackConfig | None, - test_config: TestConfig, + test_config: "TestConfig", ) -> Iterator[RunningApplication]: """Start application with external contributor checks enabled (disabled access). @@ -186,7 +186,7 @@ def application_with_external_contributor_enabled( tmp_path: Path, github_config: GitHubConfig, openstack_config: OpenStackConfig | None, - test_config: TestConfig, + test_config: "TestConfig", ) -> Iterator[RunningApplication]: """Start application with external contributor checks disabled (permissive mode). From 3784ddb82d59d626f618f6c7a98b3970aa14bc49 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 02:03:47 +0000 Subject: [PATCH 35/82] test: update docstring --- github-runner-manager/tests/integration/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/github-runner-manager/tests/integration/conftest.py b/github-runner-manager/tests/integration/conftest.py index 996e1dfb97..49fa4645a2 100644 --- a/github-runner-manager/tests/integration/conftest.py +++ b/github-runner-manager/tests/integration/conftest.py @@ -29,7 +29,7 @@ def github_config(pytestconfig: pytest.Config) -> GitHubConfig: GitHub configuration object. Raises: - pytest.skip: If neither --github-token option nor GITHUB_TOKEN + pytest.fail: If neither --github-token option nor INTEGRATION_TOKEN environment variable is set. """ token = pytestconfig.getoption("--github-token") @@ -38,7 +38,7 @@ def github_config(pytestconfig: pytest.Config) -> GitHubConfig: if not token or not path: pytest.fail( "GitHub configuration not provided. Use --github-token and --github-repository " - "options or set GITHUB_TOKEN and GITHUB_REPOSITORY environment variables." + "options or set INTEGRATION_TOKEN and GITHUB_REPOSITORY environment variables." ) return GitHubConfig(token=token, path=path) From 4a7030c161df64b7024aaf773f732723c00ad490 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 02:18:16 +0000 Subject: [PATCH 36/82] ci: install application package before testing --- .github/workflows/test_github_runner_manager.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/test_github_runner_manager.yaml b/.github/workflows/test_github_runner_manager.yaml index dec48e0371..7b38a674ed 100644 --- a/.github/workflows/test_github_runner_manager.yaml +++ b/.github/workflows/test_github_runner_manager.yaml @@ -22,6 +22,9 @@ jobs: uses: astral-sh/setup-uv@v7 - name: Install tox with UV run: uv tool install tox --with tox-uv + - name: Install the github-runner-manager application package + working-directory: ./github-runner-manager/ + run: uv pip install -e . - name: Run integration tests working-directory: ./github-runner-manager/ env: From 2f8b0cd26f9234474750b1eaea3995a1fe164345 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 02:29:27 +0000 Subject: [PATCH 37/82] ci: test --- .github/workflows/test_github_runner_manager.yaml | 3 --- github-runner-manager/tox.ini | 10 +--------- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/.github/workflows/test_github_runner_manager.yaml b/.github/workflows/test_github_runner_manager.yaml index 7b38a674ed..dec48e0371 100644 --- a/.github/workflows/test_github_runner_manager.yaml +++ b/.github/workflows/test_github_runner_manager.yaml @@ -22,9 +22,6 @@ jobs: uses: astral-sh/setup-uv@v7 - name: Install tox with UV run: uv tool install tox --with tox-uv - - name: Install the github-runner-manager application package - working-directory: ./github-runner-manager/ - run: uv pip install -e . - name: Run integration tests working-directory: ./github-runner-manager/ env: diff --git a/github-runner-manager/tox.ini b/github-runner-manager/tox.ini index 62cac38e86..567fd1c3c9 100644 --- a/github-runner-manager/tox.ini +++ b/github-runner-manager/tox.ini @@ -102,6 +102,7 @@ commands = [testenv:integration] description = Run integration tests deps = + -e . pytest pytest-asyncio pyyaml @@ -121,13 +122,4 @@ passenv = OS_REGION_NAME commands = pytest {[vars]tst_path}integration \ - --github-token={env:GITHUB_TOKEN:} \ - --github-token-alt={env:GITHUB_TOKEN_ALT:} \ - --github-repository={env:GITHUB_REPOSITORY:} \ - --openstack-auth-url={env:OS_AUTH_URL:} \ - --openstack-project={env:OS_PROJECT_NAME:} \ - --openstack-username={env:OS_USERNAME:} \ - --openstack-password={env:OS_PASSWORD:} \ - --openstack-network={env:OS_NETWORK:} \ - --openstack-region={env:OS_REGION_NAME:} \ -v --tb native -s {posargs} From ee2edeebd96a7f74267c8bb72fdfd4c3578c0c63 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 03:03:57 +0000 Subject: [PATCH 38/82] ci: fix secrets mapping --- .github/workflows/test_github_runner_manager.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test_github_runner_manager.yaml b/.github/workflows/test_github_runner_manager.yaml index dec48e0371..1a46c63385 100644 --- a/.github/workflows/test_github_runner_manager.yaml +++ b/.github/workflows/test_github_runner_manager.yaml @@ -26,13 +26,13 @@ jobs: working-directory: ./github-runner-manager/ env: # GitHub configuration - INTEGRATION_TOKEN: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_2 }} - INTEGRATION_TOKEN_ALT: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_3 }} + INTEGRATION_TOKEN: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_1 }} + INTEGRATION_TOKEN_ALT: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_2 }} # OpenStack configuration OS_AUTH_URL: ${{ vars.INTEGRATION_TEST_OPENSTACK_AUTH_URL }} OS_PROJECT_NAME: ${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_NAME }} OS_USERNAME: ${{ vars.INTEGRATION_TEST_OPENSTACK_USERNAME }} - OS_PASSWORD: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_1 }} + OS_PASSWORD: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_3 }} OS_NETWORK: ${{ vars.INTEGRATION_TEST_OPENSTACK_NETWORK_NAME }} OS_REGION_NAME: ${{ vars.INTEGRATION_TEST_OPENSTACK_REGION_NAME }} run: tox -e integration -- -v --tb=native -s --github-repository=${{ vars.INTEGRATION_TEST_PATH }} From 8ed2ac2c4307a2b368e0e1082072bf8514af8f8b Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 03:15:59 +0000 Subject: [PATCH 39/82] ci: skip heavy testing for debug --- .github/workflows/integration_test.yaml | 148 ++++++++++++------------ 1 file changed, 77 insertions(+), 71 deletions(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 617e038d69..0fb63f5888 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -12,74 +12,80 @@ concurrency: cancel-in-progress: true jobs: - openstack-integration-tests-private-endpoint: - name: Integration test using private-endpoint - uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main - secrets: inherit - with: - juju-channel: 3.6/stable - provider: lxd - test-tox-env: integration-juju3.6 - modules: '["test_charm_metrics_failure", "test_charm_metrics_success", "test_charm_fork_repo", "test_charm_fork_path_change", "test_charm_no_runner", "test_charm_runner", "test_debug_ssh", "test_charm_upgrade", "test_reactive"]' - # INTEGRATION_TOKEN, OS_PASSWORD are passed through INTEGRATION_TEST_SECRET_ENV_VALUE_ - # mapping. See CONTRIBUTING.md for more details. - extra-arguments: | - -m=openstack \ - --log-format="%(asctime)s %(levelname)s %(message)s" \ - --path="${{ vars.INTEGRATION_TEST_PATH }}" \ - --openstack-auth-url="${{ vars.INTEGRATION_TEST_OPENSTACK_AUTH_URL }}" \ - --openstack-project-domain-name="${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_DOMAIN_NAME }}" \ - --openstack-project-name="${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_NAME }}" \ - --openstack-user-domain-name="${{ vars.INTEGRATION_TEST_OPENSTACK_USER_DOMAIN_NAME }}" \ - --openstack-username="${{ vars.INTEGRATION_TEST_OPENSTACK_USERNAME }}" \ - --openstack-region-name="${{ vars.INTEGRATION_TEST_OPENSTACK_REGION_NAME }}" \ - --openstack-network-name="${{ vars.INTEGRATION_TEST_OPENSTACK_NETWORK_NAME }}" \ - --openstack-flavor-name="${{ vars.INTEGRATION_TEST_OPENSTACK_FLAVOR_NAME }}" \ - --https-proxy="${{ vars.INTEGRATION_TEST_HTTPS_PROXY }}" \ - --http-proxy="${{ vars.INTEGRATION_TEST_HTTP_PROXY }}" \ - --no-proxy="${{ vars.INTEGRATION_TEST_NO_PROXY }}" \ - --openstack-https-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTPS_PROXY }}" \ - --openstack-http-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTP_PROXY }}" \ - --openstack-no-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_NO_PROXY }}" \ - --test-image-id="${{ vars.INTEGRATION_TEST_IMAGE_ID }}" - self-hosted-runner: true - self-hosted-runner-label: pfe-ci - openstack-integration-tests-cross-controller-private-endpoint: - name: Cross controller integration test using private-endpoint - uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main - secrets: inherit - with: - juju-channel: 3.6/stable - pre-run-script: tests/integration/setup-integration-tests.sh - provider: lxd - test-tox-env: integration-juju3.6 - modules: '["test_prometheus_metrics"]' - # INTEGRATION_TOKEN, OS_PASSWORD are passed through INTEGRATION_TEST_SECRET_ENV_VALUE_ - # mapping. See CONTRIBUTING.md for more details. - extra-arguments: | - -m=openstack \ - --log-format="%(asctime)s %(levelname)s %(message)s" \ - --path="${{ vars.INTEGRATION_TEST_PATH }}" \ - --openstack-auth-url="${{ vars.INTEGRATION_TEST_OPENSTACK_AUTH_URL }}" \ - --openstack-project-domain-name="${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_DOMAIN_NAME }}" \ - --openstack-project-name="${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_NAME }}" \ - --openstack-user-domain-name="${{ vars.INTEGRATION_TEST_OPENSTACK_USER_DOMAIN_NAME }}" \ - --openstack-username="${{ vars.INTEGRATION_TEST_OPENSTACK_USERNAME }}" \ - --openstack-region-name="${{ vars.INTEGRATION_TEST_OPENSTACK_REGION_NAME }}" \ - --openstack-network-name="${{ vars.INTEGRATION_TEST_OPENSTACK_NETWORK_NAME }}" \ - --openstack-flavor-name="${{ vars.INTEGRATION_TEST_OPENSTACK_FLAVOR_NAME }}" \ - --https-proxy="${{ vars.INTEGRATION_TEST_HTTPS_PROXY }}" \ - --http-proxy="${{ vars.INTEGRATION_TEST_HTTP_PROXY }}" \ - --no-proxy="${{ vars.INTEGRATION_TEST_NO_PROXY }}" \ - --openstack-https-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTPS_PROXY }}" \ - --openstack-http-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTP_PROXY }}" \ - --openstack-no-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_NO_PROXY }}" \ - --test-image-id="${{ vars.INTEGRATION_TEST_IMAGE_ID }}" - self-hosted-runner: true - self-hosted-runner-label: pfe-ci - allure-report: - if: ${{ (success() || failure()) && github.event_name == 'schedule' }} - needs: - - openstack-integration-tests-private-endpoint - - openstack-integration-tests-cross-controller-private-endpoint - uses: canonical/operator-workflows/.github/workflows/allure_report.yaml@main + skip: + name: Skip + if: false + runs-on: ubuntu-latest + steps: + - run: echo "Skipping integration tests" + # openstack-integration-tests-private-endpoint: + # name: Integration test using private-endpoint + # uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main + # secrets: inherit + # with: + # juju-channel: 3.6/stable + # provider: lxd + # test-tox-env: integration-juju3.6 + # modules: '["test_charm_metrics_failure", "test_charm_metrics_success", "test_charm_fork_repo", "test_charm_fork_path_change", "test_charm_no_runner", "test_charm_runner", "test_debug_ssh", "test_charm_upgrade", "test_reactive"]' + # # INTEGRATION_TOKEN, OS_PASSWORD are passed through INTEGRATION_TEST_SECRET_ENV_VALUE_ + # # mapping. See CONTRIBUTING.md for more details. + # extra-arguments: | + # -m=openstack \ + # --log-format="%(asctime)s %(levelname)s %(message)s" \ + # --path="${{ vars.INTEGRATION_TEST_PATH }}" \ + # --openstack-auth-url="${{ vars.INTEGRATION_TEST_OPENSTACK_AUTH_URL }}" \ + # --openstack-project-domain-name="${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_DOMAIN_NAME }}" \ + # --openstack-project-name="${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_NAME }}" \ + # --openstack-user-domain-name="${{ vars.INTEGRATION_TEST_OPENSTACK_USER_DOMAIN_NAME }}" \ + # --openstack-username="${{ vars.INTEGRATION_TEST_OPENSTACK_USERNAME }}" \ + # --openstack-region-name="${{ vars.INTEGRATION_TEST_OPENSTACK_REGION_NAME }}" \ + # --openstack-network-name="${{ vars.INTEGRATION_TEST_OPENSTACK_NETWORK_NAME }}" \ + # --openstack-flavor-name="${{ vars.INTEGRATION_TEST_OPENSTACK_FLAVOR_NAME }}" \ + # --https-proxy="${{ vars.INTEGRATION_TEST_HTTPS_PROXY }}" \ + # --http-proxy="${{ vars.INTEGRATION_TEST_HTTP_PROXY }}" \ + # --no-proxy="${{ vars.INTEGRATION_TEST_NO_PROXY }}" \ + # --openstack-https-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTPS_PROXY }}" \ + # --openstack-http-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTP_PROXY }}" \ + # --openstack-no-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_NO_PROXY }}" \ + # --test-image-id="${{ vars.INTEGRATION_TEST_IMAGE_ID }}" + # self-hosted-runner: true + # self-hosted-runner-label: pfe-ci + # openstack-integration-tests-cross-controller-private-endpoint: + # name: Cross controller integration test using private-endpoint + # uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main + # secrets: inherit + # with: + # juju-channel: 3.6/stable + # pre-run-script: tests/integration/setup-integration-tests.sh + # provider: lxd + # test-tox-env: integration-juju3.6 + # modules: '["test_prometheus_metrics"]' + # # INTEGRATION_TOKEN, OS_PASSWORD are passed through INTEGRATION_TEST_SECRET_ENV_VALUE_ + # # mapping. See CONTRIBUTING.md for more details. + # extra-arguments: | + # -m=openstack \ + # --log-format="%(asctime)s %(levelname)s %(message)s" \ + # --path="${{ vars.INTEGRATION_TEST_PATH }}" \ + # --openstack-auth-url="${{ vars.INTEGRATION_TEST_OPENSTACK_AUTH_URL }}" \ + # --openstack-project-domain-name="${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_DOMAIN_NAME }}" \ + # --openstack-project-name="${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_NAME }}" \ + # --openstack-user-domain-name="${{ vars.INTEGRATION_TEST_OPENSTACK_USER_DOMAIN_NAME }}" \ + # --openstack-username="${{ vars.INTEGRATION_TEST_OPENSTACK_USERNAME }}" \ + # --openstack-region-name="${{ vars.INTEGRATION_TEST_OPENSTACK_REGION_NAME }}" \ + # --openstack-network-name="${{ vars.INTEGRATION_TEST_OPENSTACK_NETWORK_NAME }}" \ + # --openstack-flavor-name="${{ vars.INTEGRATION_TEST_OPENSTACK_FLAVOR_NAME }}" \ + # --https-proxy="${{ vars.INTEGRATION_TEST_HTTPS_PROXY }}" \ + # --http-proxy="${{ vars.INTEGRATION_TEST_HTTP_PROXY }}" \ + # --no-proxy="${{ vars.INTEGRATION_TEST_NO_PROXY }}" \ + # --openstack-https-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTPS_PROXY }}" \ + # --openstack-http-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTP_PROXY }}" \ + # --openstack-no-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_NO_PROXY }}" \ + # --test-image-id="${{ vars.INTEGRATION_TEST_IMAGE_ID }}" + # self-hosted-runner: true + # self-hosted-runner-label: pfe-ci + # allure-report: + # if: ${{ (success() || failure()) && github.event_name == 'schedule' }} + # needs: + # - openstack-integration-tests-private-endpoint + # - openstack-integration-tests-cross-controller-private-endpoint + # uses: canonical/operator-workflows/.github/workflows/allure_report.yaml@main From 54caba5f3c9bb26b81f7b6aa6023f50b95e438fa Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 03:17:15 +0000 Subject: [PATCH 40/82] ci: debug --- .github/workflows/test_github_runner_manager.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/test_github_runner_manager.yaml b/.github/workflows/test_github_runner_manager.yaml index 1a46c63385..17b76ea439 100644 --- a/.github/workflows/test_github_runner_manager.yaml +++ b/.github/workflows/test_github_runner_manager.yaml @@ -16,6 +16,11 @@ jobs: name: Integration tests for github-runner-manager runs-on: [self-hosted, pfe-ci] steps: + - name: Setup tmate session + uses: canonical/action-tmate@main + with: + detached: true + - name: Checkout code uses: actions/checkout@v5.0.1 - name: Setup Astral UV From 2cf25d91a49ea34370c0ae44d78c2a479f370c82 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 03:26:38 +0000 Subject: [PATCH 41/82] fix: proxy configuration check --- .../openstack_cloud/openstack_runner_manager.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/github-runner-manager/src/github_runner_manager/openstack_cloud/openstack_runner_manager.py b/github-runner-manager/src/github_runner_manager/openstack_cloud/openstack_runner_manager.py index 4219e107e8..6b26e3c177 100644 --- a/github-runner-manager/src/github_runner_manager/openstack_cloud/openstack_runner_manager.py +++ b/github-runner-manager/src/github_runner_manager/openstack_cloud/openstack_runner_manager.py @@ -197,7 +197,10 @@ def _generate_cloud_init(self, runner_context: RunnerContext) -> str: pre_job_contents = jinja.get_template("pre-job.j2").render(pre_job_contents_dict) use_aproxy = service_config.use_aproxy - if not service_config.runner_proxy_config.proxy_address: + if ( + not service_config.runner_proxy_config + or not service_config.runner_proxy_config.proxy_address + ): use_aproxy = False aproxy_redirect_ports = service_config.aproxy_redirect_ports if not aproxy_redirect_ports: From 6c938ad3a7951086758c6688625eb0d838b43703 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 03:36:33 +0000 Subject: [PATCH 42/82] test: use proxies --- .../workflows/test_github_runner_manager.yaml | 10 ++++- github-runner-manager/tests/conftest.py | 36 ++++++++++++++++ .../tests/integration/conftest.py | 33 +++++++++++++- .../tests/integration/factories.py | 43 ++++++++++++++++++- 4 files changed, 118 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test_github_runner_manager.yaml b/.github/workflows/test_github_runner_manager.yaml index 17b76ea439..cf5781d6a9 100644 --- a/.github/workflows/test_github_runner_manager.yaml +++ b/.github/workflows/test_github_runner_manager.yaml @@ -40,4 +40,12 @@ jobs: OS_PASSWORD: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_3 }} OS_NETWORK: ${{ vars.INTEGRATION_TEST_OPENSTACK_NETWORK_NAME }} OS_REGION_NAME: ${{ vars.INTEGRATION_TEST_OPENSTACK_REGION_NAME }} - run: tox -e integration -- -v --tb=native -s --github-repository=${{ vars.INTEGRATION_TEST_PATH }} + run: | + tox -e integration -- -v --tb=native -s \ + --github-repository=${{ vars.INTEGRATION_TEST_PATH }} \ + --https-proxy="${{ vars.INTEGRATION_TEST_HTTPS_PROXY }}" \ + --http-proxy="${{ vars.INTEGRATION_TEST_HTTP_PROXY }}" \ + --no-proxy="${{ vars.INTEGRATION_TEST_NO_PROXY }}" \ + --openstack-https-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTPS_PROXY }}" \ + --openstack-http-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTP_PROXY }}" \ + --openstack-no-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_NO_PROXY }}" diff --git a/github-runner-manager/tests/conftest.py b/github-runner-manager/tests/conftest.py index b18521ff5c..fb65f6ac6f 100644 --- a/github-runner-manager/tests/conftest.py +++ b/github-runner-manager/tests/conftest.py @@ -65,3 +65,39 @@ def pytest_addoption(parser): help="OpenStack region name for integration tests.", default=os.getenv("OS_REGION_NAME"), ) + parser.addoption( + "--https-proxy", + action="store", + help="HTTPS proxy for runner configuration.", + default=os.getenv("HTTPS_PROXY"), + ) + parser.addoption( + "--http-proxy", + action="store", + help="HTTP proxy for runner configuration.", + default=os.getenv("HTTP_PROXY"), + ) + parser.addoption( + "--no-proxy", + action="store", + help="No proxy configuration for runner.", + default=os.getenv("NO_PROXY"), + ) + parser.addoption( + "--openstack-https-proxy", + action="store", + help="HTTPS proxy for OpenStack runner operations.", + default=os.getenv("OPENSTACK_HTTPS_PROXY"), + ) + parser.addoption( + "--openstack-http-proxy", + action="store", + help="HTTP proxy for OpenStack runner operations.", + default=os.getenv("OPENSTACK_HTTP_PROXY"), + ) + parser.addoption( + "--openstack-no-proxy", + action="store", + help="No proxy configuration for OpenStack runner operations.", + default=os.getenv("OPENSTACK_NO_PROXY"), + ) diff --git a/github-runner-manager/tests/integration/conftest.py b/github-runner-manager/tests/integration/conftest.py index 49fa4645a2..fed7bc707f 100644 --- a/github-runner-manager/tests/integration/conftest.py +++ b/github-runner-manager/tests/integration/conftest.py @@ -5,7 +5,7 @@ import pytest -from .factories import GitHubConfig, OpenStackConfig, TestConfig +from .factories import GitHubConfig, OpenStackConfig, ProxyConfig, TestConfig @pytest.fixture @@ -74,3 +74,34 @@ def openstack_config(pytestconfig: pytest.Config) -> OpenStackConfig | None: network=network, region_name=region_name, ) + + +@pytest.fixture(scope="module") +def proxy_config(pytestconfig: pytest.Config) -> ProxyConfig | None: + """Get proxy configuration from pytest options or environment. + + Args: + pytestconfig: Pytest configuration object. + + Returns: + Proxy configuration object, or None if no proxy is configured. + """ + http_proxy = pytestconfig.getoption("--http-proxy") + https_proxy = pytestconfig.getoption("--https-proxy") + no_proxy = pytestconfig.getoption("--no-proxy") + openstack_http_proxy = pytestconfig.getoption("--openstack-http-proxy") + openstack_https_proxy = pytestconfig.getoption("--openstack-https-proxy") + openstack_no_proxy = pytestconfig.getoption("--openstack-no-proxy") + + # Return None if no proxy is configured + if not any([http_proxy, https_proxy, openstack_http_proxy, openstack_https_proxy]): + return None + + return ProxyConfig( + http_proxy=http_proxy, + https_proxy=https_proxy, + no_proxy=no_proxy, + openstack_http_proxy=openstack_http_proxy, + openstack_https_proxy=openstack_https_proxy, + openstack_no_proxy=openstack_no_proxy, + ) diff --git a/github-runner-manager/tests/integration/factories.py b/github-runner-manager/tests/integration/factories.py index 9b20c7726e..9f1bce9189 100644 --- a/github-runner-manager/tests/integration/factories.py +++ b/github-runner-manager/tests/integration/factories.py @@ -52,6 +52,27 @@ class OpenStackConfig: region_name: str = "RegionOne" +@dataclass +class ProxyConfig: + """Proxy configuration for tests. + + Attributes: + http_proxy: HTTP proxy URL. + https_proxy: HTTPS proxy URL. + no_proxy: Comma-separated list of hosts to exclude from proxy. + openstack_http_proxy: HTTP proxy for OpenStack operations. + openstack_https_proxy: HTTPS proxy for OpenStack operations. + openstack_no_proxy: No proxy configuration for OpenStack operations. + """ + + http_proxy: str | None = None + https_proxy: str | None = None + no_proxy: str | None = None + openstack_http_proxy: str | None = None + openstack_https_proxy: str | None = None + openstack_no_proxy: str | None = None + + @dataclass class TestConfig: """Test-specific configuration for parallel test execution. @@ -80,6 +101,7 @@ def create_default_config( github_config: GitHubConfig | None = None, openstack_config: OpenStackConfig | None = None, test_config: TestConfig | None = None, + proxy_config: ProxyConfig | None = None, ) -> dict[str, Any]: """Create a default test configuration dictionary. @@ -89,6 +111,7 @@ def create_default_config( openstack_config: OpenStack configuration. Defaults to test values. test_config: Test-specific configuration for parallel execution. Defaults to new unique values. + proxy_config: Proxy configuration. Defaults to no proxy. Returns: Configuration dictionary for the application. @@ -120,6 +143,22 @@ def create_default_config( else: path_config = {"org": github_config.path, "group": "default"} + # Build proxy configuration + runner_proxy = None + if proxy_config and (proxy_config.http_proxy or proxy_config.https_proxy): + runner_proxy = { + "proxy_address": proxy_config.https_proxy or proxy_config.http_proxy, + "no_proxy": proxy_config.no_proxy, + } + + openstack_proxy = None + if proxy_config and (proxy_config.openstack_http_proxy or proxy_config.openstack_https_proxy): + openstack_proxy = { + "http": proxy_config.openstack_http_proxy, + "https": proxy_config.openstack_https_proxy, + "no_proxy": proxy_config.openstack_no_proxy, + } + return { "name": test_config.runner_name, "allow_external_contributor": allow_external_contributor, @@ -133,8 +172,8 @@ def create_default_config( "dockerhub_mirror": None, "ssh_debug_connections": [], "repo_policy_compliance": None, - "proxy_config": None, - "runner_proxy_config": None, + "proxy_config": openstack_proxy, + "runner_proxy_config": runner_proxy, }, "non_reactive_configuration": { "combinations": [ From 23d07135311bcb6a870a2d2df824e91ee38151c5 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 04:19:04 +0000 Subject: [PATCH 43/82] test: use proxies in fixtures --- .../integration/test_external_contributor_security.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/github-runner-manager/tests/integration/test_external_contributor_security.py b/github-runner-manager/tests/integration/test_external_contributor_security.py index 5a42170bd1..438888dcf2 100644 --- a/github-runner-manager/tests/integration/test_external_contributor_security.py +++ b/github-runner-manager/tests/integration/test_external_contributor_security.py @@ -18,7 +18,7 @@ from github.Repository import Repository from .application import RunningApplication -from .factories import GitHubConfig, OpenStackConfig, create_default_config +from .factories import GitHubConfig, OpenStackConfig, ProxyConfig, create_default_config from .github_helpers import ( close_pull_request, create_fork_and_pr, @@ -139,6 +139,7 @@ def application_with_external_contributor_disabled( github_config: GitHubConfig, openstack_config: OpenStackConfig | None, test_config: "TestConfig", + proxy_config: ProxyConfig | None, ) -> Iterator[RunningApplication]: """Start application with external contributor checks enabled (disabled access). @@ -147,6 +148,7 @@ def application_with_external_contributor_disabled( github_config: GitHub configuration object. openstack_config: OpenStack configuration object or None. test_config: Test-specific configuration for unique identification. + proxy_config: Proxy configuration object or None. Yields: A running application instance. @@ -156,6 +158,7 @@ def application_with_external_contributor_disabled( github_config=github_config, openstack_config=openstack_config, test_config=test_config, + proxy_config=proxy_config, ) config_path = tmp_path / "config.yaml" config_path.write_text(yaml.dump(config), encoding="utf-8") @@ -187,6 +190,7 @@ def application_with_external_contributor_enabled( github_config: GitHubConfig, openstack_config: OpenStackConfig | None, test_config: "TestConfig", + proxy_config: ProxyConfig | None, ) -> Iterator[RunningApplication]: """Start application with external contributor checks disabled (permissive mode). @@ -195,6 +199,7 @@ def application_with_external_contributor_enabled( github_config: GitHub configuration object. openstack_config: OpenStack configuration object or None. test_config: Test-specific configuration for unique identification. + proxy_config: Proxy configuration object or None. Yields: A running application instance. @@ -204,6 +209,7 @@ def application_with_external_contributor_enabled( github_config=github_config, openstack_config=openstack_config, test_config=test_config, + proxy_config=proxy_config, ) config_path = tmp_path / "config.yaml" config_path.write_text(yaml.dump(config), encoding="utf-8") From f40df9587d44dc5eed948f9d7bb004df29bc34c4 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 05:09:53 +0000 Subject: [PATCH 44/82] test: use test configured flavor/image --- .github/workflows/test_github_runner_manager.yaml | 4 +++- github-runner-manager/tests/conftest.py | 12 ++++++++++++ github-runner-manager/tests/integration/conftest.py | 4 ++++ .../tests/integration/factories.py | 13 +++++++++++-- 4 files changed, 30 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test_github_runner_manager.yaml b/.github/workflows/test_github_runner_manager.yaml index cf5781d6a9..de1b5f26e6 100644 --- a/.github/workflows/test_github_runner_manager.yaml +++ b/.github/workflows/test_github_runner_manager.yaml @@ -48,4 +48,6 @@ jobs: --no-proxy="${{ vars.INTEGRATION_TEST_NO_PROXY }}" \ --openstack-https-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTPS_PROXY }}" \ --openstack-http-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTP_PROXY }}" \ - --openstack-no-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_NO_PROXY }}" + --openstack-no-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_NO_PROXY }}" \ + --openstack-flavor="${{ vars.INTEGRATION_TEST_OPENSTACK_FLAVOR_NAME }}" \ + --openstack-image-id="${{ vars.INTEGRATION_TEST_IMAGE_ID }}" diff --git a/github-runner-manager/tests/conftest.py b/github-runner-manager/tests/conftest.py index fb65f6ac6f..f033519798 100644 --- a/github-runner-manager/tests/conftest.py +++ b/github-runner-manager/tests/conftest.py @@ -101,3 +101,15 @@ def pytest_addoption(parser): help="No proxy configuration for OpenStack runner operations.", default=os.getenv("OPENSTACK_NO_PROXY"), ) + parser.addoption( + "--openstack-flavor", + action="store", + help="OpenStack flavor name for runner instances.", + default=os.getenv("OPENSTACK_FLAVOR"), + ) + parser.addoption( + "--openstack-image-id", + action="store", + help="OpenStack image ID for runner instances.", + default=os.getenv("OPENSTACK_IMAGE_ID"), + ) diff --git a/github-runner-manager/tests/integration/conftest.py b/github-runner-manager/tests/integration/conftest.py index fed7bc707f..45890b4bc6 100644 --- a/github-runner-manager/tests/integration/conftest.py +++ b/github-runner-manager/tests/integration/conftest.py @@ -60,6 +60,8 @@ def openstack_config(pytestconfig: pytest.Config) -> OpenStackConfig | None: password = pytestconfig.getoption("--openstack-password") network = pytestconfig.getoption("--openstack-network") region_name = pytestconfig.getoption("--openstack-region") + flavor = pytestconfig.getoption("--openstack-flavor") + image_id = pytestconfig.getoption("--openstack-image-id") # Only return config if all required parameters are provided assert all( @@ -73,6 +75,8 @@ def openstack_config(pytestconfig: pytest.Config) -> OpenStackConfig | None: password=password, network=network, region_name=region_name, + flavor=flavor, + image_id=image_id, ) diff --git a/github-runner-manager/tests/integration/factories.py b/github-runner-manager/tests/integration/factories.py index 9f1bce9189..0963a7acbf 100644 --- a/github-runner-manager/tests/integration/factories.py +++ b/github-runner-manager/tests/integration/factories.py @@ -42,6 +42,8 @@ class OpenStackConfig: password: OpenStack password. network: OpenStack network name. region_name: OpenStack region name. + flavor: OpenStack flavor name for runner instances. + image_id: OpenStack image ID for runner instances. """ auth_url: str @@ -50,6 +52,8 @@ class OpenStackConfig: password: str network: str region_name: str = "RegionOne" + flavor: str | None = None + image_id: str | None = None @dataclass @@ -131,6 +135,8 @@ def create_default_config( password="test-password", network="test-network", region_name="RegionOne", + flavor="small", + image_id=None, ) if test_config is None: @@ -178,8 +184,11 @@ def create_default_config( "non_reactive_configuration": { "combinations": [ { - "image": {"name": "noble", "labels": ["noble", "x64"]}, - "flavor": {"name": "small", "labels": ["small"]}, + "image": { + "name": openstack_config.image_id or "noble", + "labels": ["noble", "x64"], + }, + "flavor": {"name": openstack_config.flavor or "small", "labels": ["small"]}, "base_virtual_machines": 1, } ] From ee73b5965a558ff4363c73076aef2584ae24e893 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 05:10:25 +0000 Subject: [PATCH 45/82] test: application accept 204 --- github-runner-manager/tests/integration/application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github-runner-manager/tests/integration/application.py b/github-runner-manager/tests/integration/application.py index 5a5452514d..169a399871 100644 --- a/github-runner-manager/tests/integration/application.py +++ b/github-runner-manager/tests/integration/application.py @@ -33,7 +33,7 @@ def wait_for_server(host: str, port: int, timeout: float = 10.0) -> bool: response = requests.get( f"http://{host}:{port}/health", timeout=1, allow_redirects=False ) - if response.status_code in (200, 404): # Server is responding + if response.status_code in (200, 204, 404): # Server is responding return True except (requests.ConnectionError, requests.Timeout): time.sleep(0.5) From aa7e39de53f686d0c786cea142dd6103d3b97344 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 05:11:02 +0000 Subject: [PATCH 46/82] test: use configurable metrics log path --- .../src/github_runner_manager/metrics/events.py | 3 ++- .../tests/integration/application.py | 13 ++++++++++++- .../test_external_contributor_security.py | 6 ++++-- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/github-runner-manager/src/github_runner_manager/metrics/events.py b/github-runner-manager/src/github_runner_manager/metrics/events.py index f09e9afb0e..91b22e6c6b 100644 --- a/github-runner-manager/src/github_runner_manager/metrics/events.py +++ b/github-runner-manager/src/github_runner_manager/metrics/events.py @@ -3,6 +3,7 @@ """Models and functions for the metric events.""" import logging +import os from pathlib import Path from typing import Any, Optional @@ -11,7 +12,7 @@ from github_runner_manager.errors import IssueMetricEventError from github_runner_manager.manager.vm_manager import CodeInformation -METRICS_LOG_PATH = Path("/var/log/github-runner-metrics.log") +METRICS_LOG_PATH = Path(os.getenv("METRICS_LOG_PATH", "/var/log/github-runner-metrics.log")) logger = logging.getLogger(__name__) diff --git a/github-runner-manager/tests/integration/application.py b/github-runner-manager/tests/integration/application.py index 169a399871..da13b47dbb 100644 --- a/github-runner-manager/tests/integration/application.py +++ b/github-runner-manager/tests/integration/application.py @@ -4,6 +4,7 @@ """Test infrastructure for managing running application instances.""" import multiprocessing +import os import time from dataclasses import dataclass from pathlib import Path @@ -130,7 +131,11 @@ def is_alive(self) -> bool: @classmethod def create( - cls, config_file_path: Path, host: str = "127.0.0.1", port: int = 8080 + cls, + config_file_path: Path, + host: str = "127.0.0.1", + port: int = 8080, + metrics_log_path: Path | None = None, ) -> "RunningApplication": """Create and start a new application instance. @@ -138,6 +143,8 @@ def create( config_file_path: Path to the configuration file. host: Host to listen on. Defaults to 127.0.0.1. port: Port to listen on. If None, an available port is automatically selected. + metrics_log_path: Path to the metrics log file. If provided, sets METRICS_LOG_PATH + environment variable for the application process. Returns: A RunningApplication instance with the application running. @@ -145,6 +152,10 @@ def create( Raises: RuntimeError: If the application fails to start. """ + # Set metrics log path if provided + if metrics_log_path: + os.environ["METRICS_LOG_PATH"] = str(metrics_log_path) + # Start the server process process = multiprocessing.Process( target=_start_cli_server, diff --git a/github-runner-manager/tests/integration/test_external_contributor_security.py b/github-runner-manager/tests/integration/test_external_contributor_security.py index 438888dcf2..0d5aa6fe5e 100644 --- a/github-runner-manager/tests/integration/test_external_contributor_security.py +++ b/github-runner-manager/tests/integration/test_external_contributor_security.py @@ -167,7 +167,8 @@ def application_with_external_contributor_disabled( "Starting application with external contributor disabled (test_id: %s)", test_config.test_id, ) - app = RunningApplication.create(config_path) + metrics_log_path = tmp_path / "github-runner-metrics.log" + app = RunningApplication.create(config_path, metrics_log_path=metrics_log_path) try: # Verify application is running and healthy before yielding logger.info("Verifying application is alive...") @@ -218,7 +219,8 @@ def application_with_external_contributor_enabled( "Starting application with external contributor enabled (test_id: %s)", test_config.test_id, ) - app = RunningApplication.create(config_path) + metrics_log_path = tmp_path / "github-runner-metrics.log" + app = RunningApplication.create(config_path, metrics_log_path=metrics_log_path) try: # Verify application is running and healthy before yielding logger.info("Verifying application is alive...") From 4114abe0bf2a48aaaeba0df791c74934ffacbf92 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 05:11:16 +0000 Subject: [PATCH 47/82] test: prefer reactive configuration by default for tests --- github-runner-manager/tests/integration/factories.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/github-runner-manager/tests/integration/factories.py b/github-runner-manager/tests/integration/factories.py index 0963a7acbf..550ced6af2 100644 --- a/github-runner-manager/tests/integration/factories.py +++ b/github-runner-manager/tests/integration/factories.py @@ -181,7 +181,8 @@ def create_default_config( "proxy_config": openstack_proxy, "runner_proxy_config": runner_proxy, }, - "non_reactive_configuration": { + "non_reactive_configuration": None, + "reactive_configuration": { "combinations": [ { "image": { @@ -189,11 +190,9 @@ def create_default_config( "labels": ["noble", "x64"], }, "flavor": {"name": openstack_config.flavor or "small", "labels": ["small"]}, - "base_virtual_machines": 1, } ] }, - "reactive_configuration": None, "openstack_configuration": { "vm_prefix": test_config.vm_prefix, "network": openstack_config.network, From 68f8de6df34c6b40c24d2924379a4fd192ff608a Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 05:13:26 +0000 Subject: [PATCH 48/82] test: prefer non reactive configuration by default to save deploy components --- github-runner-manager/tests/integration/factories.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/github-runner-manager/tests/integration/factories.py b/github-runner-manager/tests/integration/factories.py index 550ced6af2..0963a7acbf 100644 --- a/github-runner-manager/tests/integration/factories.py +++ b/github-runner-manager/tests/integration/factories.py @@ -181,8 +181,7 @@ def create_default_config( "proxy_config": openstack_proxy, "runner_proxy_config": runner_proxy, }, - "non_reactive_configuration": None, - "reactive_configuration": { + "non_reactive_configuration": { "combinations": [ { "image": { @@ -190,9 +189,11 @@ def create_default_config( "labels": ["noble", "x64"], }, "flavor": {"name": openstack_config.flavor or "small", "labels": ["small"]}, + "base_virtual_machines": 1, } ] }, + "reactive_configuration": None, "openstack_configuration": { "vm_prefix": test_config.vm_prefix, "network": openstack_config.network, From cf571e2bdd5927e973bd7877abf0072c130a3ee7 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 05:28:45 +0000 Subject: [PATCH 49/82] test: remove duplicate health checks (create app health check enabled) --- .../test_external_contributor_security.py | 34 +++++-------------- 1 file changed, 8 insertions(+), 26 deletions(-) diff --git a/github-runner-manager/tests/integration/test_external_contributor_security.py b/github-runner-manager/tests/integration/test_external_contributor_security.py index 0d5aa6fe5e..b9cadae9dc 100644 --- a/github-runner-manager/tests/integration/test_external_contributor_security.py +++ b/github-runner-manager/tests/integration/test_external_contributor_security.py @@ -169,20 +169,11 @@ def application_with_external_contributor_disabled( ) metrics_log_path = tmp_path / "github-runner-metrics.log" app = RunningApplication.create(config_path, metrics_log_path=metrics_log_path) - try: - # Verify application is running and healthy before yielding - logger.info("Verifying application is alive...") - assert app.is_alive(), "Application should be running" - logger.info("Checking application health endpoint...") - response = app.get("/health") - assert response.status_code == 200, "Health endpoint should return 200 OK" - logger.info("Application is running and healthy") - yield app + yield app - finally: - logger.info("Stopping application") - app.stop() + logger.info("Stopping application") + app.stop() @pytest.fixture @@ -221,20 +212,11 @@ def application_with_external_contributor_enabled( ) metrics_log_path = tmp_path / "github-runner-metrics.log" app = RunningApplication.create(config_path, metrics_log_path=metrics_log_path) - try: - # Verify application is running and healthy before yielding - logger.info("Verifying application is alive...") - assert app.is_alive(), "Application should be running" - logger.info("Checking application health endpoint...") - response = app.get("/health") - assert response.status_code == 200, "Health endpoint should return 200 OK" - logger.info("Application is running and healthy") - - yield app - - finally: - logger.info("Stopping application") - app.stop() + + yield app + + logger.info("Stopping application") + app.stop() @pytest.fixture From f5cb83a399e93614d96fd3bad5537d5f917330a6 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 05:44:46 +0000 Subject: [PATCH 50/82] test: fix create PR from fork --- github-runner-manager/tests/integration/github_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github-runner-manager/tests/integration/github_helpers.py b/github-runner-manager/tests/integration/github_helpers.py index b39c4f746e..c12781f272 100644 --- a/github-runner-manager/tests/integration/github_helpers.py +++ b/github-runner-manager/tests/integration/github_helpers.py @@ -45,7 +45,7 @@ def create_fork_and_pr( ) # Create PR from fork to original repository - pr = github_repository.create_pull( + pr = forked_github_repository.create_pull( title=f"Test PR from fork {test_id}", body=( f"This is a test PR from a forked repository for testing " From ff22f45567b5e3a19be97519040e2d3bcca9bced Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 06:28:47 +0000 Subject: [PATCH 51/82] test: fix PR base --- github-runner-manager/tests/integration/github_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github-runner-manager/tests/integration/github_helpers.py b/github-runner-manager/tests/integration/github_helpers.py index c12781f272..4cd647d279 100644 --- a/github-runner-manager/tests/integration/github_helpers.py +++ b/github-runner-manager/tests/integration/github_helpers.py @@ -52,7 +52,7 @@ def create_fork_and_pr( f"external contributor security. Test ID: {test_id}" ), head=f"{forked_github_repository.owner.login}:{branch_name}", - base=github_repository.default_branch, + base=f"{github_repository.full_name}:{github_repository.default_branch}", ) return pr From d89fd730c32b6fb808b092fa662a965bb1118f50 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 08:22:11 +0000 Subject: [PATCH 52/82] test: stepped PR deployment --- .github/workflows/test_github_runner_manager.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test_github_runner_manager.yaml b/.github/workflows/test_github_runner_manager.yaml index de1b5f26e6..e955d73c47 100644 --- a/.github/workflows/test_github_runner_manager.yaml +++ b/.github/workflows/test_github_runner_manager.yaml @@ -42,7 +42,7 @@ jobs: OS_REGION_NAME: ${{ vars.INTEGRATION_TEST_OPENSTACK_REGION_NAME }} run: | tox -e integration -- -v --tb=native -s \ - --github-repository=${{ vars.INTEGRATION_TEST_PATH }} \ + --github-repository=${{ vars.INTEGRATION_TEST_PATH_2 }} \ --https-proxy="${{ vars.INTEGRATION_TEST_HTTPS_PROXY }}" \ --http-proxy="${{ vars.INTEGRATION_TEST_HTTP_PROXY }}" \ --no-proxy="${{ vars.INTEGRATION_TEST_NO_PROXY }}" \ From 3f245034a3af950937a0af09c72c2d5d1fbe8d90 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 08:28:09 +0000 Subject: [PATCH 53/82] refactor: remove unused dispatch workflow function and update tests to use get_pr_workflow_runs --- .../tests/integration/github_helpers.py | 83 ++++++++++--------- .../test_external_contributor_security.py | 48 ++++++----- 2 files changed, 68 insertions(+), 63 deletions(-) diff --git a/github-runner-manager/tests/integration/github_helpers.py b/github-runner-manager/tests/integration/github_helpers.py index 4cd647d279..19f8abcd18 100644 --- a/github-runner-manager/tests/integration/github_helpers.py +++ b/github-runner-manager/tests/integration/github_helpers.py @@ -3,8 +3,7 @@ """Helper functions for GitHub integration testing.""" -from datetime import datetime, timezone -from time import sleep +from time import sleep, time import requests from github.GithubException import GithubException @@ -58,43 +57,6 @@ def create_fork_and_pr( return pr -def dispatch_workflow_and_get_run( - repository: Repository, - workflow_filename: str, - runner_label: str, -) -> WorkflowRun | None: - """Dispatch a workflow and return the workflow run. - - Args: - repository: Repository to dispatch workflow on. - workflow_filename: Workflow file to dispatch. - runner_label: Runner label to target. - - Returns: - The workflow run object, or None if not found. - """ - start_time = datetime.now(timezone.utc) - workflow = repository.get_workflow(id_or_file_name=workflow_filename) - - # Dispatch the workflow - success = workflow.create_dispatch( - ref=repository.default_branch, - inputs={"runner": runner_label}, - ) - - if not success: - return None - - # Wait for the workflow run to appear (up to 2 minutes) - for _ in range(24): # 24 * 5 = 120 seconds - sleep(5) - runs = workflow.get_runs(created=f">={start_time.isoformat(timespec='seconds')}") - if runs.totalCount > 0: - return runs[0] - - return None - - def wait_for_workflow_completion(run: WorkflowRun, timeout: int = 600) -> bool: """Wait for a workflow run to complete. @@ -105,8 +67,8 @@ def wait_for_workflow_completion(run: WorkflowRun, timeout: int = 600) -> bool: Returns: True if workflow completed, False if timeout. """ - iterations = timeout // 10 - for _ in range(iterations): + start_time = time() + while time() - start_time < timeout: run.update() if run.status == "completed": return True @@ -143,3 +105,42 @@ def close_pull_request(pr: PullRequest) -> None: pr.edit(state="closed") except GithubException: pass # Best effort cleanup + + +def get_pr_workflow_runs( + repository: Repository, + pr: PullRequest, + workflow_name: str, + timeout: int = 120, +) -> list[WorkflowRun]: + """Get workflow runs triggered by a pull request. + + Args: + repository: The repository containing the workflow. + pr: The pull request that triggered the workflow. + workflow_name: The name of the workflow to look for. + timeout: Maximum time to wait for workflow runs to appear in seconds. + + Returns: + List of workflow runs matching the criteria, sorted by created_at descending + (most recent first). + """ + start_time = time() + while time() - start_time < timeout: + # Get all runs for this PR + all_runs = repository.get_workflow_runs( + event="pull_request", + head_sha=pr.head.sha, + ) + + # Filter by workflow name + matching_runs = [run for run in all_runs if run.name == workflow_name] + + if matching_runs: + # Sort by created_at descending to get most recent first + matching_runs.sort(key=lambda r: r.created_at, reverse=True) + return matching_runs + + sleep(5) + + return [] diff --git a/github-runner-manager/tests/integration/test_external_contributor_security.py b/github-runner-manager/tests/integration/test_external_contributor_security.py index b9cadae9dc..1ae13c5719 100644 --- a/github-runner-manager/tests/integration/test_external_contributor_security.py +++ b/github-runner-manager/tests/integration/test_external_contributor_security.py @@ -22,8 +22,8 @@ from .github_helpers import ( close_pull_request, create_fork_and_pr, - dispatch_workflow_and_get_run, get_job_logs, + get_pr_workflow_runs, wait_for_workflow_completion, ) @@ -33,7 +33,7 @@ logger = logging.getLogger(__name__) # Test workflow filename -DISPATCH_TEST_WORKFLOW_FILENAME = "workflow_dispatch_test.yaml" +PULL_REQUEST_WORKFLOW_NAME = "Pull request test" @pytest.fixture(scope="module") @@ -255,26 +255,28 @@ def external_contributor_pull_request( ) def test_external_contributor_disabled_default_security( github_repository: Repository, + external_contributor_pull_request: PullRequest, test_config: "TestConfig", ): """ arrange: Application running with allow_external_contributor=False, forked repository \ from external contributor. - act: Create PR from fork and dispatch workflow. + act: Create PR from fork and wait for pull_request_test workflow. assert: Workflow fails with "Insufficient user authorization" error in logs. """ logger.info("Test started: test_external_contributor_disabled_default_security") - # Dispatch a workflow targeting this runner - # Use the unique test label to ensure this runner picks up the job - runner_label = test_config.labels[0] - logger.info("Dispatching workflow with runner label: %s", runner_label) - run = dispatch_workflow_and_get_run( + + # Wait for the pull request workflow to be triggered + logger.info("Waiting for pull request workflow to be triggered...") + runs = get_pr_workflow_runs( repository=github_repository, - workflow_filename=DISPATCH_TEST_WORKFLOW_FILENAME, - runner_label=runner_label, + pr=external_contributor_pull_request, + workflow_name=PULL_REQUEST_WORKFLOW_NAME, + timeout=120, ) - assert run is not None, "Workflow run should be created" - logger.info("Workflow run created: %s (run_id: %d)", run.html_url, run.id) + assert runs, "Pull request workflow should be triggered" + run = runs[0] + logger.info("Workflow run found: %s (run_id: %d)", run.html_url, run.id) # Wait for the workflow to complete logger.info("Waiting for workflow to complete (timeout: 600 seconds)...") @@ -303,26 +305,28 @@ def test_external_contributor_disabled_default_security( ) def test_external_contributor_enabled_permissive_mode( github_repository: Repository, + external_contributor_pull_request: PullRequest, test_config: "TestConfig", ): """ arrange: Application running with allow_external_contributor=True, forked repository \ from external contributor. - act: Create PR from fork and dispatch workflow. + act: Create PR from fork and wait for pull_request_test workflow. assert: Workflow succeeds with no authorization errors in logs. """ logger.info("Test started: test_external_contributor_enabled_permissive_mode") - # Dispatch a workflow targeting this runner - # Use the unique test label to ensure this runner picks up the job - runner_label = test_config.labels[0] - logger.info("Dispatching workflow with runner label: %s", runner_label) - run = dispatch_workflow_and_get_run( + + # Wait for the pull request workflow to be triggered + logger.info("Waiting for pull request workflow to be triggered...") + runs = get_pr_workflow_runs( repository=github_repository, - workflow_filename=DISPATCH_TEST_WORKFLOW_FILENAME, - runner_label=runner_label, + pr=external_contributor_pull_request, + workflow_name=PULL_REQUEST_WORKFLOW_NAME, + timeout=120, ) - assert run is not None, "Workflow run should be created" - logger.info("Workflow run created: %s (run_id: %d)", run.html_url, run.id) + assert runs, "Pull request workflow should be triggered" + run = runs[0] + logger.info("Workflow run found: %s (run_id: %d)", run.html_url, run.id) # Wait for the workflow to complete logger.info("Waiting for workflow to complete (timeout: 600 seconds)...") From 185dc80624d73af8b0fa68a3c2ceabb680faa2a5 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 08:35:37 +0000 Subject: [PATCH 54/82] ci: trigger From 92202881643cc9e613eb9dc285f9371b015c7e81 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 09:13:44 +0000 Subject: [PATCH 55/82] fix: PR base --- github-runner-manager/tests/integration/github_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github-runner-manager/tests/integration/github_helpers.py b/github-runner-manager/tests/integration/github_helpers.py index 19f8abcd18..f1a09aa71d 100644 --- a/github-runner-manager/tests/integration/github_helpers.py +++ b/github-runner-manager/tests/integration/github_helpers.py @@ -51,7 +51,7 @@ def create_fork_and_pr( f"external contributor security. Test ID: {test_id}" ), head=f"{forked_github_repository.owner.login}:{branch_name}", - base=f"{github_repository.full_name}:{github_repository.default_branch}", + base=f"{github_repository.owner.login}:{github_repository.default_branch}", ) return pr From 376cf5f13cf6d39543d46974a8f65bac63885e85 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 10:15:01 +0000 Subject: [PATCH 56/82] test: origina/upstream/fork repository differentiation --- .../tests/integration/conftest.py | 10 ++-- .../tests/integration/factories.py | 2 + .../tests/integration/github_helpers.py | 8 +-- .../test_external_contributor_security.py | 55 ++++++++----------- 4 files changed, 36 insertions(+), 39 deletions(-) diff --git a/github-runner-manager/tests/integration/conftest.py b/github-runner-manager/tests/integration/conftest.py index 45890b4bc6..49701c7f7b 100644 --- a/github-runner-manager/tests/integration/conftest.py +++ b/github-runner-manager/tests/integration/conftest.py @@ -33,15 +33,17 @@ def github_config(pytestconfig: pytest.Config) -> GitHubConfig: environment variable is set. """ token = pytestconfig.getoption("--github-token") + alt_token = pytestconfig.getoption("--github-token-alt", None) path = pytestconfig.getoption("--github-repository") - if not token or not path: + if not token or not alt_token or not path: pytest.fail( - "GitHub configuration not provided. Use --github-token and --github-repository " - "options or set INTEGRATION_TOKEN and GITHUB_REPOSITORY environment variables." + "GitHub configuration not provided. Use --github-token, --github-token-alt, and " + "--github-repository options or set INTEGRATION_TOKEN, INTEGRATION_TOKEN_ALT, and " + "GITHUB_REPOSITORY environment variables." ) - return GitHubConfig(token=token, path=path) + return GitHubConfig(token=token, alt_token=alt_token, path=path) @pytest.fixture(scope="module") diff --git a/github-runner-manager/tests/integration/factories.py b/github-runner-manager/tests/integration/factories.py index 0963a7acbf..2afbb0bbca 100644 --- a/github-runner-manager/tests/integration/factories.py +++ b/github-runner-manager/tests/integration/factories.py @@ -24,10 +24,12 @@ class GitHubConfig: Attributes: token: GitHub personal access token. + alt_token: Alternate GitHub personal access token for external contributor. path: GitHub path in / or format. """ token: str + alt_token: str path: str diff --git a/github-runner-manager/tests/integration/github_helpers.py b/github-runner-manager/tests/integration/github_helpers.py index f1a09aa71d..3551632505 100644 --- a/github-runner-manager/tests/integration/github_helpers.py +++ b/github-runner-manager/tests/integration/github_helpers.py @@ -13,7 +13,7 @@ def create_fork_and_pr( - github_repository: Repository, + upstream_repository: Repository, forked_github_repository: Repository, test_id: str, ) -> PullRequest: @@ -29,7 +29,7 @@ def create_fork_and_pr( """ # Create a unique branch in the fork branch_name = f"test/{test_id}" - main_branch = forked_github_repository.get_branch(github_repository.default_branch) + main_branch = forked_github_repository.get_branch(upstream_repository.default_branch) forked_github_repository.create_git_ref( ref=f"refs/heads/{branch_name}", sha=main_branch.commit.sha ) @@ -44,14 +44,14 @@ def create_fork_and_pr( ) # Create PR from fork to original repository - pr = forked_github_repository.create_pull( + pr = upstream_repository.create_pull( title=f"Test PR from fork {test_id}", body=( f"This is a test PR from a forked repository for testing " f"external contributor security. Test ID: {test_id}" ), + base=upstream_repository.default_branch, head=f"{forked_github_repository.owner.login}:{branch_name}", - base=f"{github_repository.owner.login}:{github_repository.default_branch}", ) return pr diff --git a/github-runner-manager/tests/integration/test_external_contributor_security.py b/github-runner-manager/tests/integration/test_external_contributor_security.py index 1ae13c5719..ef1e101d50 100644 --- a/github-runner-manager/tests/integration/test_external_contributor_security.py +++ b/github-runner-manager/tests/integration/test_external_contributor_security.py @@ -65,43 +65,40 @@ def github_repository(github_config: GitHubConfig) -> Repository: @pytest.fixture(scope="module") -def forked_github_repository( - github_repository: Repository, pytestconfig: pytest.Config -) -> Iterator[Repository]: +def upstream_repository(github_config: GitHubConfig) -> Repository: + """Get GitHub repository for testing. + + Args: + github_config: GitHub configuration object. + + Returns: + GitHub repository object. + """ + auth = Token(github_config.alt_token) + github = Github(auth=auth) + return github.get_repo(github_config.path) + + +@pytest.fixture(scope="module") +def forked_github_repository(upstream_repository: Repository) -> Iterator[Repository]: """Create a fork of the test repository to simulate external contributor. This fixture uses an alternate GitHub token (if provided) to create a fork from a different user account, simulating an external contributor scenario. Args: - github_repository: The original repository. - pytestconfig: Pytest configuration object. + upstream_repository: The upstream repository using alt token. Yields: The forked repository. """ # Try to get alternate token for creating fork # This simulates a different user (external contributor) - alt_token = pytestconfig.getoption("--github-token-alt", None) - - if not alt_token: - logger.warning( - "Alternate GitHub token not provided. Skipping test requiring fork creation." - ) - pytest.fail( - "Alternate GitHub token not provided. Use --github-token-alt option to test with " - "actual fork." - ) logger.info( - "Creating fork of repository %s using alternate token", github_repository.full_name + "Creating fork of repository %s using alternate token", upstream_repository.full_name ) - # Create fork using alternate token - alt_github = Github(auth=Token(token=alt_token)) - - # Get the repository using alternate token to create fork - alt_repo_ref = alt_github.get_repo(github_repository.full_name) - fork = alt_repo_ref.create_fork(name=f"test-fork-{secrets.token_hex(4)}") + fork = upstream_repository.create_fork(name=f"test-fork-{secrets.token_hex(4)}") logger.info("Waiting for fork to be ready (up to 5 attempts)...") # Wait for fork to be ready @@ -221,14 +218,14 @@ def application_with_external_contributor_enabled( @pytest.fixture def external_contributor_pull_request( - github_repository: Repository, + upstream_repository: Repository, forked_github_repository: Repository, test_config: "TestConfig", ) -> Iterator[PullRequest]: """Create a pull request from forked repository simulating external contributor. Args: - github_repository: The original GitHub repository. + upstream_repository: The original GitHub repository. forked_github_repository: The forked repository (different user). test_config: Test configuration with unique identifiers. @@ -239,7 +236,7 @@ def external_contributor_pull_request( "Creating pull request from fork to simulate external contributor (test_id: %s)", test_config.test_id, ) - pr = create_fork_and_pr(github_repository, forked_github_repository, test_config.test_id) + pr = create_fork_and_pr(upstream_repository, forked_github_repository, test_config.test_id) logger.info("Pull request created: #%d - %s", pr.number, pr.title) yield pr @@ -254,9 +251,7 @@ def external_contributor_pull_request( "application_with_external_contributor_disabled", "external_contributor_pull_request" ) def test_external_contributor_disabled_default_security( - github_repository: Repository, - external_contributor_pull_request: PullRequest, - test_config: "TestConfig", + github_repository: Repository, external_contributor_pull_request: PullRequest ): """ arrange: Application running with allow_external_contributor=False, forked repository \ @@ -304,9 +299,7 @@ def test_external_contributor_disabled_default_security( "application_with_external_contributor_enabled", "external_contributor_pull_request" ) def test_external_contributor_enabled_permissive_mode( - github_repository: Repository, - external_contributor_pull_request: PullRequest, - test_config: "TestConfig", + github_repository: Repository, external_contributor_pull_request: PullRequest ): """ arrange: Application running with allow_external_contributor=True, forked repository \ From 16a5a0cf14c4a1f59122f0e7e869152b0c17466b Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 11:42:43 +0000 Subject: [PATCH 57/82] ci: trigger From 3f3d3e5fc8bc7b67e957315e11bbd49ebd967453 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 13:08:48 +0000 Subject: [PATCH 58/82] test: fix use aproxy configuration by default --- .../tests/integration/factories.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/github-runner-manager/tests/integration/factories.py b/github-runner-manager/tests/integration/factories.py index 2afbb0bbca..2ffc633d0a 100644 --- a/github-runner-manager/tests/integration/factories.py +++ b/github-runner-manager/tests/integration/factories.py @@ -155,7 +155,8 @@ def create_default_config( runner_proxy = None if proxy_config and (proxy_config.http_proxy or proxy_config.https_proxy): runner_proxy = { - "proxy_address": proxy_config.https_proxy or proxy_config.http_proxy, + "http": proxy_config.http_proxy, + "https": proxy_config.https_proxy, "no_proxy": proxy_config.no_proxy, } @@ -176,12 +177,16 @@ def create_default_config( "path": path_config, }, "service_config": { - "use_aproxy": False, + "manager_proxy_command": None, + "proxy_config": runner_proxy, + "runner_proxy_config": openstack_proxy, + "use_aproxy": True, + "aproxy_exclude_addresses": [], + "aproxy_redirect_ports": ["1-3127", "3129-65535"], "dockerhub_mirror": None, "ssh_debug_connections": [], "repo_policy_compliance": None, - "proxy_config": openstack_proxy, - "runner_proxy_config": runner_proxy, + "custom_pre_job_script": None, }, "non_reactive_configuration": { "combinations": [ From 4a472977f35f9d813f12d6f615870b458345e3d0 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 13:30:58 +0000 Subject: [PATCH 59/82] test: get all logs from a run --- .../tests/integration/github_helpers.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/github-runner-manager/tests/integration/github_helpers.py b/github-runner-manager/tests/integration/github_helpers.py index 3551632505..4d20be3265 100644 --- a/github-runner-manager/tests/integration/github_helpers.py +++ b/github-runner-manager/tests/integration/github_helpers.py @@ -89,10 +89,13 @@ def get_job_logs(run: WorkflowRun) -> str: if jobs.totalCount == 0: return "" - job = jobs[0] - logs_url = job.logs_url() - response = requests.get(logs_url) - return response.content.decode("utf-8") + logs = "" + for job in jobs: + logs_url = job.logs_url() + response = requests.get(logs_url) + logs += response.content.decode("utf-8") + + return logs def close_pull_request(pr: PullRequest) -> None: From 39c1a3f595bb012d393328d7c897d6542758b08b Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 14:07:28 +0000 Subject: [PATCH 60/82] test: log cli output --- github-runner-manager/tests/integration/application.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/github-runner-manager/tests/integration/application.py b/github-runner-manager/tests/integration/application.py index da13b47dbb..defa10f2a2 100644 --- a/github-runner-manager/tests/integration/application.py +++ b/github-runner-manager/tests/integration/application.py @@ -3,6 +3,7 @@ """Test infrastructure for managing running application instances.""" +import logging import multiprocessing import os import time @@ -16,6 +17,8 @@ from src.github_runner_manager.cli import main +logger = logging.getLogger(__name__) + def wait_for_server(host: str, port: int, timeout: float = 10.0) -> bool: """Wait for a server to become responsive. @@ -65,8 +68,10 @@ def _start_cli_server(config_file_path: Path, port: int, host: str = "127.0.0.1" catch_exceptions=False, ) if result.exit_code != 0: - print(f"CLI exited with code {result.exit_code}") - print(f"Output: {result.output}") + logger.error("CLI exited with code %d", result.exit_code) + logger.error("Output: %s", result.output) + else: + logger.info("CLI output: %s", result.output) @dataclass From 3f76829e1c20a518ff07948d71c4c113a552765b Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 15:27:52 +0000 Subject: [PATCH 61/82] test: fix log assertion --- .../tests/integration/test_external_contributor_security.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/github-runner-manager/tests/integration/test_external_contributor_security.py b/github-runner-manager/tests/integration/test_external_contributor_security.py index ef1e101d50..c98bfffd94 100644 --- a/github-runner-manager/tests/integration/test_external_contributor_security.py +++ b/github-runner-manager/tests/integration/test_external_contributor_security.py @@ -338,9 +338,6 @@ def test_external_contributor_enabled_permissive_mode( # Get the job logs and verify it ran successfully logger.info("Retrieving job logs to verify success...") logs = get_job_logs(run) - assert "Should not echo if pre-job script failed" in logs, ( - "Logs should contain the expected success message. " f"Actual logs: {logs[:500]}" - ) # Ensure no security errors in logs assert ( From 2f8bbad35d8085146956cbba3919713565b8f70a Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 16:19:22 +0000 Subject: [PATCH 62/82] ci: remove debug --- .github/workflows/test_github_runner_manager.yaml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.github/workflows/test_github_runner_manager.yaml b/.github/workflows/test_github_runner_manager.yaml index e955d73c47..d3e79179b7 100644 --- a/.github/workflows/test_github_runner_manager.yaml +++ b/.github/workflows/test_github_runner_manager.yaml @@ -16,11 +16,6 @@ jobs: name: Integration tests for github-runner-manager runs-on: [self-hosted, pfe-ci] steps: - - name: Setup tmate session - uses: canonical/action-tmate@main - with: - detached: true - - name: Checkout code uses: actions/checkout@v5.0.1 - name: Setup Astral UV From f2c46a4ed8313b0bf0d31bd160648ae27cc5f527 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 16:20:06 +0000 Subject: [PATCH 63/82] ci: renable charm tests --- .github/workflows/integration_test.yaml | 142 ++++++++++++------------ 1 file changed, 71 insertions(+), 71 deletions(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 0fb63f5888..55a5197482 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -18,74 +18,74 @@ jobs: runs-on: ubuntu-latest steps: - run: echo "Skipping integration tests" - # openstack-integration-tests-private-endpoint: - # name: Integration test using private-endpoint - # uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main - # secrets: inherit - # with: - # juju-channel: 3.6/stable - # provider: lxd - # test-tox-env: integration-juju3.6 - # modules: '["test_charm_metrics_failure", "test_charm_metrics_success", "test_charm_fork_repo", "test_charm_fork_path_change", "test_charm_no_runner", "test_charm_runner", "test_debug_ssh", "test_charm_upgrade", "test_reactive"]' - # # INTEGRATION_TOKEN, OS_PASSWORD are passed through INTEGRATION_TEST_SECRET_ENV_VALUE_ - # # mapping. See CONTRIBUTING.md for more details. - # extra-arguments: | - # -m=openstack \ - # --log-format="%(asctime)s %(levelname)s %(message)s" \ - # --path="${{ vars.INTEGRATION_TEST_PATH }}" \ - # --openstack-auth-url="${{ vars.INTEGRATION_TEST_OPENSTACK_AUTH_URL }}" \ - # --openstack-project-domain-name="${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_DOMAIN_NAME }}" \ - # --openstack-project-name="${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_NAME }}" \ - # --openstack-user-domain-name="${{ vars.INTEGRATION_TEST_OPENSTACK_USER_DOMAIN_NAME }}" \ - # --openstack-username="${{ vars.INTEGRATION_TEST_OPENSTACK_USERNAME }}" \ - # --openstack-region-name="${{ vars.INTEGRATION_TEST_OPENSTACK_REGION_NAME }}" \ - # --openstack-network-name="${{ vars.INTEGRATION_TEST_OPENSTACK_NETWORK_NAME }}" \ - # --openstack-flavor-name="${{ vars.INTEGRATION_TEST_OPENSTACK_FLAVOR_NAME }}" \ - # --https-proxy="${{ vars.INTEGRATION_TEST_HTTPS_PROXY }}" \ - # --http-proxy="${{ vars.INTEGRATION_TEST_HTTP_PROXY }}" \ - # --no-proxy="${{ vars.INTEGRATION_TEST_NO_PROXY }}" \ - # --openstack-https-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTPS_PROXY }}" \ - # --openstack-http-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTP_PROXY }}" \ - # --openstack-no-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_NO_PROXY }}" \ - # --test-image-id="${{ vars.INTEGRATION_TEST_IMAGE_ID }}" - # self-hosted-runner: true - # self-hosted-runner-label: pfe-ci - # openstack-integration-tests-cross-controller-private-endpoint: - # name: Cross controller integration test using private-endpoint - # uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main - # secrets: inherit - # with: - # juju-channel: 3.6/stable - # pre-run-script: tests/integration/setup-integration-tests.sh - # provider: lxd - # test-tox-env: integration-juju3.6 - # modules: '["test_prometheus_metrics"]' - # # INTEGRATION_TOKEN, OS_PASSWORD are passed through INTEGRATION_TEST_SECRET_ENV_VALUE_ - # # mapping. See CONTRIBUTING.md for more details. - # extra-arguments: | - # -m=openstack \ - # --log-format="%(asctime)s %(levelname)s %(message)s" \ - # --path="${{ vars.INTEGRATION_TEST_PATH }}" \ - # --openstack-auth-url="${{ vars.INTEGRATION_TEST_OPENSTACK_AUTH_URL }}" \ - # --openstack-project-domain-name="${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_DOMAIN_NAME }}" \ - # --openstack-project-name="${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_NAME }}" \ - # --openstack-user-domain-name="${{ vars.INTEGRATION_TEST_OPENSTACK_USER_DOMAIN_NAME }}" \ - # --openstack-username="${{ vars.INTEGRATION_TEST_OPENSTACK_USERNAME }}" \ - # --openstack-region-name="${{ vars.INTEGRATION_TEST_OPENSTACK_REGION_NAME }}" \ - # --openstack-network-name="${{ vars.INTEGRATION_TEST_OPENSTACK_NETWORK_NAME }}" \ - # --openstack-flavor-name="${{ vars.INTEGRATION_TEST_OPENSTACK_FLAVOR_NAME }}" \ - # --https-proxy="${{ vars.INTEGRATION_TEST_HTTPS_PROXY }}" \ - # --http-proxy="${{ vars.INTEGRATION_TEST_HTTP_PROXY }}" \ - # --no-proxy="${{ vars.INTEGRATION_TEST_NO_PROXY }}" \ - # --openstack-https-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTPS_PROXY }}" \ - # --openstack-http-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTP_PROXY }}" \ - # --openstack-no-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_NO_PROXY }}" \ - # --test-image-id="${{ vars.INTEGRATION_TEST_IMAGE_ID }}" - # self-hosted-runner: true - # self-hosted-runner-label: pfe-ci - # allure-report: - # if: ${{ (success() || failure()) && github.event_name == 'schedule' }} - # needs: - # - openstack-integration-tests-private-endpoint - # - openstack-integration-tests-cross-controller-private-endpoint - # uses: canonical/operator-workflows/.github/workflows/allure_report.yaml@main + openstack-integration-tests-private-endpoint: + name: Integration test using private-endpoint + uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main + secrets: inherit + with: + juju-channel: 3.6/stable + provider: lxd + test-tox-env: integration-juju3.6 + modules: '["test_charm_metrics_failure", "test_charm_metrics_success", "test_charm_fork_repo", "test_charm_fork_path_change", "test_charm_no_runner", "test_charm_runner", "test_debug_ssh", "test_charm_upgrade", "test_reactive"]' + # INTEGRATION_TOKEN, OS_PASSWORD are passed through INTEGRATION_TEST_SECRET_ENV_VALUE_ + # mapping. See CONTRIBUTING.md for more details. + extra-arguments: | + -m=openstack \ + --log-format="%(asctime)s %(levelname)s %(message)s" \ + --path="${{ vars.INTEGRATION_TEST_PATH }}" \ + --openstack-auth-url="${{ vars.INTEGRATION_TEST_OPENSTACK_AUTH_URL }}" \ + --openstack-project-domain-name="${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_DOMAIN_NAME }}" \ + --openstack-project-name="${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_NAME }}" \ + --openstack-user-domain-name="${{ vars.INTEGRATION_TEST_OPENSTACK_USER_DOMAIN_NAME }}" \ + --openstack-username="${{ vars.INTEGRATION_TEST_OPENSTACK_USERNAME }}" \ + --openstack-region-name="${{ vars.INTEGRATION_TEST_OPENSTACK_REGION_NAME }}" \ + --openstack-network-name="${{ vars.INTEGRATION_TEST_OPENSTACK_NETWORK_NAME }}" \ + --openstack-flavor-name="${{ vars.INTEGRATION_TEST_OPENSTACK_FLAVOR_NAME }}" \ + --https-proxy="${{ vars.INTEGRATION_TEST_HTTPS_PROXY }}" \ + --http-proxy="${{ vars.INTEGRATION_TEST_HTTP_PROXY }}" \ + --no-proxy="${{ vars.INTEGRATION_TEST_NO_PROXY }}" \ + --openstack-https-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTPS_PROXY }}" \ + --openstack-http-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTP_PROXY }}" \ + --openstack-no-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_NO_PROXY }}" \ + --test-image-id="${{ vars.INTEGRATION_TEST_IMAGE_ID }}" + self-hosted-runner: true + self-hosted-runner-label: pfe-ci + openstack-integration-tests-cross-controller-private-endpoint: + name: Cross controller integration test using private-endpoint + uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main + secrets: inherit + with: + juju-channel: 3.6/stable + pre-run-script: tests/integration/setup-integration-tests.sh + provider: lxd + test-tox-env: integration-juju3.6 + modules: '["test_prometheus_metrics"]' + # INTEGRATION_TOKEN, OS_PASSWORD are passed through INTEGRATION_TEST_SECRET_ENV_VALUE_ + # mapping. See CONTRIBUTING.md for more details. + extra-arguments: | + -m=openstack \ + --log-format="%(asctime)s %(levelname)s %(message)s" \ + --path="${{ vars.INTEGRATION_TEST_PATH }}" \ + --openstack-auth-url="${{ vars.INTEGRATION_TEST_OPENSTACK_AUTH_URL }}" \ + --openstack-project-domain-name="${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_DOMAIN_NAME }}" \ + --openstack-project-name="${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_NAME }}" \ + --openstack-user-domain-name="${{ vars.INTEGRATION_TEST_OPENSTACK_USER_DOMAIN_NAME }}" \ + --openstack-username="${{ vars.INTEGRATION_TEST_OPENSTACK_USERNAME }}" \ + --openstack-region-name="${{ vars.INTEGRATION_TEST_OPENSTACK_REGION_NAME }}" \ + --openstack-network-name="${{ vars.INTEGRATION_TEST_OPENSTACK_NETWORK_NAME }}" \ + --openstack-flavor-name="${{ vars.INTEGRATION_TEST_OPENSTACK_FLAVOR_NAME }}" \ + --https-proxy="${{ vars.INTEGRATION_TEST_HTTPS_PROXY }}" \ + --http-proxy="${{ vars.INTEGRATION_TEST_HTTP_PROXY }}" \ + --no-proxy="${{ vars.INTEGRATION_TEST_NO_PROXY }}" \ + --openstack-https-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTPS_PROXY }}" \ + --openstack-http-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_HTTP_PROXY }}" \ + --openstack-no-proxy="${{ vars.INTEGRATION_TEST_OPENSTACK_NO_PROXY }}" \ + --test-image-id="${{ vars.INTEGRATION_TEST_IMAGE_ID }}" + self-hosted-runner: true + self-hosted-runner-label: pfe-ci + allure-report: + if: ${{ (success() || failure()) && github.event_name == 'schedule' }} + needs: + - openstack-integration-tests-private-endpoint + - openstack-integration-tests-cross-controller-private-endpoint + uses: canonical/operator-workflows/.github/workflows/allure_report.yaml@main From 2db2abe6b01d843def025ba0f7225757c36ee867 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Thu, 27 Nov 2025 16:22:18 +0000 Subject: [PATCH 64/82] fix: lint fixes --- github-runner-manager/tests/integration/factories.py | 1 + github-runner-manager/tests/integration/github_helpers.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/github-runner-manager/tests/integration/factories.py b/github-runner-manager/tests/integration/factories.py index 2ffc633d0a..ba27a8ecc6 100644 --- a/github-runner-manager/tests/integration/factories.py +++ b/github-runner-manager/tests/integration/factories.py @@ -126,6 +126,7 @@ def create_default_config( if github_config is None: github_config = GitHubConfig( token="ghp_test_token_1234567890abcdef", + alt_token="ghp_test_alt_token_1234567890abcdef", path="test-org", ) diff --git a/github-runner-manager/tests/integration/github_helpers.py b/github-runner-manager/tests/integration/github_helpers.py index 4d20be3265..f7a20d4167 100644 --- a/github-runner-manager/tests/integration/github_helpers.py +++ b/github-runner-manager/tests/integration/github_helpers.py @@ -20,7 +20,7 @@ def create_fork_and_pr( """Create a PR from a forked repository. Args: - github_repository: The original repository. + upstream_repository: The original repository. forked_github_repository: The forked repository. test_id: Unique test identifier. From 459b1e8cfeacf61ca6616d489c0c338fab5ac23e Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 28 Nov 2025 02:08:02 +0000 Subject: [PATCH 65/82] docs: update allow-external-contributor configuration name --- config.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/config.yaml b/config.yaml index e071976e2a..cf8935e61a 100644 --- a/config.yaml +++ b/config.yaml @@ -88,20 +88,20 @@ options: repo-policy-compliance-token: type: string description: >- - DEPRECATED, please use repo-fork-check configuration option instead. + DEPRECATED, please use allow-external-contributor configuration option instead. The token to authenticate with the repository-policy-compliance service in order to generate one-time-tokens. This option requires the repo-policy-compliance-url to be set. If not set, the repository-policy-compliance service will not be used. repo-policy-compliance-url: type: string description: >- - DEPRECATED, please use repo-fork-check configuration option instead. + DEPRECATED, please use allow-external-contributor configuration option instead The URL to the repository-policy-compliance service. This option requires the repo-policy-compliance-token to be set. If not set, the repository-policy-compliance service will not be used. This option is only supported when using OpenStack Cloud. allow-external-contributor: type: boolean - default: False + default: True description: >- EXPERIMENTAL Whether to allow runs from forked repository from an external contributor. Enabling this From 00817b2d7ccda3f3b92b4e5816b63d30a1408eb3 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 28 Nov 2025 02:08:13 +0000 Subject: [PATCH 66/82] ci: revert debug test --- .github/workflows/integration_test.yaml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 55a5197482..617e038d69 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -12,12 +12,6 @@ concurrency: cancel-in-progress: true jobs: - skip: - name: Skip - if: false - runs-on: ubuntu-latest - steps: - - run: echo "Skipping integration tests" openstack-integration-tests-private-endpoint: name: Integration test using private-endpoint uses: canonical/operator-workflows/.github/workflows/integration_test.yaml@main From be8ab20d294bbc3f8cf68b105d2da83055ef2660 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 28 Nov 2025 02:13:47 +0000 Subject: [PATCH 67/82] test: revert subdir tests per-file-ignores settings --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 4721d14afd..e5121fb41b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,7 +54,7 @@ select = ["E", "W", "F", "C", "N", "R", "D", "H"] ignore = ["W503", "E501", "D107", "E203"] # D100, D101, D102, D103, D104: Ignore docstring style issues in tests # temporary disable E402 for the fix in charm.py for lp:2058335 -per-file-ignores = ["src/charm.py:E402", "**/tests/*:D100,D101,D102,D103,D104,D205,D212"] +per-file-ignores = ["src/charm.py:E402", "tests/*:D100,D101,D102,D103,D104,D205,D212"] docstring-convention = "google" # Check for properly formatted copyright header in each file copyright-check = "True" From b5b39a9ad832316342727063308f1a2651d354e8 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 28 Nov 2025 02:21:30 +0000 Subject: [PATCH 68/82] test: fix syntax err --- tests/integration/conftest.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index cbcd04f4bb..47b9395a83 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -182,9 +182,8 @@ def private_endpoint_config_fixture(pytestconfig: pytest.Config) -> PrivateEndpo user_domain_name = pytestconfig.getoption("--openstack-user-domain-name") user_name = pytestconfig.getoption("--openstack-username") region_name = pytestconfig.getoption("--openstack-region-name") - if any( - not val - for val in ( + assert all( + [ auth_url, password, project_domain_name, @@ -192,7 +191,7 @@ def private_endpoint_config_fixture(pytestconfig: pytest.Config) -> PrivateEndpo user_domain_name, user_name, region_name, - ) + ] ), "Specify all OpenStack private endpoint options." return { "auth_url": auth_url, From 9ea9786baf8f4332fb14e0d17f7d28b5f7033eae Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 28 Nov 2025 03:05:19 +0000 Subject: [PATCH 69/82] test: add openstack user/project domain name --- .github/workflows/test_github_runner_manager.yaml | 2 ++ github-runner-manager/tests/conftest.py | 12 ++++++++++++ github-runner-manager/tox.ini | 2 ++ 3 files changed, 16 insertions(+) diff --git a/.github/workflows/test_github_runner_manager.yaml b/.github/workflows/test_github_runner_manager.yaml index d3e79179b7..ae67a2c7b6 100644 --- a/.github/workflows/test_github_runner_manager.yaml +++ b/.github/workflows/test_github_runner_manager.yaml @@ -30,7 +30,9 @@ jobs: INTEGRATION_TOKEN_ALT: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_2 }} # OpenStack configuration OS_AUTH_URL: ${{ vars.INTEGRATION_TEST_OPENSTACK_AUTH_URL }} + OS_PROJECT_DOMAIN_NAME: ${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_DOMAIN_NAME }} OS_PROJECT_NAME: ${{ vars.INTEGRATION_TEST_OPENSTACK_PROJECT_NAME }} + OS_USER_DOMAIN_NAME: ${{ vars.INTEGRATION_TEST_OPENSTACK_USER_DOMAIN_NAME }} OS_USERNAME: ${{ vars.INTEGRATION_TEST_OPENSTACK_USERNAME }} OS_PASSWORD: ${{ secrets.INTEGRATION_TEST_SECRET_ENV_VALUE_3 }} OS_NETWORK: ${{ vars.INTEGRATION_TEST_OPENSTACK_NETWORK_NAME }} diff --git a/github-runner-manager/tests/conftest.py b/github-runner-manager/tests/conftest.py index f033519798..c9d7ea641f 100644 --- a/github-runner-manager/tests/conftest.py +++ b/github-runner-manager/tests/conftest.py @@ -35,12 +35,24 @@ def pytest_addoption(parser): help="OpenStack authentication URL for integration tests.", default=os.getenv("OS_AUTH_URL"), ) + parser.addoption( + "--openstack-project-domain-name", + action="store", + help="OpenStack project domain name for integration tests.", + default=os.getenv("OS_PROJECT_DOMAIN_NAME"), + ) parser.addoption( "--openstack-project", action="store", help="OpenStack project name for integration tests.", default=os.getenv("OS_PROJECT_NAME"), ) + parser.addoption( + "--openstack-user-domain-name", + action="store", + help="OpenStack user domain name for integration tests.", + default=os.getenv("OS_USER_DOMAIN_NAME"), + ) parser.addoption( "--openstack-username", action="store", diff --git a/github-runner-manager/tox.ini b/github-runner-manager/tox.ini index 567fd1c3c9..b3ef624757 100644 --- a/github-runner-manager/tox.ini +++ b/github-runner-manager/tox.ini @@ -115,7 +115,9 @@ passenv = INTEGRATION_TOKEN INTEGRATION_TOKEN_ALT OS_AUTH_URL + OS_PROJECT_DOMAIN_NAME OS_PROJECT_NAME + OS_USER_DOMAIN_NAME OS_USERNAME OS_PASSWORD OS_NETWORK From 85574bb7a7bb6277cdfd044bc66f6b9748248e67 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 28 Nov 2025 03:15:33 +0000 Subject: [PATCH 70/82] test: add openstack user/project domain name --- github-runner-manager/tests/integration/factories.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/github-runner-manager/tests/integration/factories.py b/github-runner-manager/tests/integration/factories.py index ba27a8ecc6..a710f95ec8 100644 --- a/github-runner-manager/tests/integration/factories.py +++ b/github-runner-manager/tests/integration/factories.py @@ -44,6 +44,8 @@ class OpenStackConfig: password: OpenStack password. network: OpenStack network name. region_name: OpenStack region name. + user_domain_name: OpenStack user domain name. + project_domain_name: OpenStack project domain name. flavor: OpenStack flavor name for runner instances. image_id: OpenStack image ID for runner instances. """ @@ -54,6 +56,8 @@ class OpenStackConfig: password: str network: str region_name: str = "RegionOne" + user_domain_name: str = "Default" + project_domain_name: str = "Default" flavor: str | None = None image_id: str | None = None @@ -210,8 +214,8 @@ def create_default_config( "project_name": openstack_config.project, "username": openstack_config.username, "password": openstack_config.password, - "user_domain_name": "Default", - "project_domain_name": "Default", + "user_domain_name": openstack_config.user_domain_name, + "project_domain_name": openstack_config.project_domain_name, "region_name": openstack_config.region_name, }, }, From fbeb9d6e53cacb34639e217481fde96ba3268be5 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 28 Nov 2025 03:15:58 +0000 Subject: [PATCH 71/82] test: add openstack user/project domain name --- .../tests/integration/conftest.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/github-runner-manager/tests/integration/conftest.py b/github-runner-manager/tests/integration/conftest.py index 49701c7f7b..83875d6580 100644 --- a/github-runner-manager/tests/integration/conftest.py +++ b/github-runner-manager/tests/integration/conftest.py @@ -62,12 +62,23 @@ def openstack_config(pytestconfig: pytest.Config) -> OpenStackConfig | None: password = pytestconfig.getoption("--openstack-password") network = pytestconfig.getoption("--openstack-network") region_name = pytestconfig.getoption("--openstack-region") + user_domain_name = pytestconfig.getoption("--openstack-user-domain-name") + project_domain_name = pytestconfig.getoption("--openstack-project-domain-name") flavor = pytestconfig.getoption("--openstack-flavor") image_id = pytestconfig.getoption("--openstack-image-id") # Only return config if all required parameters are provided assert all( - [auth_url, project, username, password, network, region_name] + [ + auth_url, + project, + project_domain_name, + username, + user_domain_name, + password, + network, + region_name, + ] ), "All OpenStack configurations must be supplied" return OpenStackConfig( @@ -77,6 +88,8 @@ def openstack_config(pytestconfig: pytest.Config) -> OpenStackConfig | None: password=password, network=network, region_name=region_name, + user_domain_name=user_domain_name, + project_domain_name=project_domain_name, flavor=flavor, image_id=image_id, ) From 6d97fae4b411999d3cc6348bdfd53d9b27457f16 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 28 Nov 2025 03:28:24 +0000 Subject: [PATCH 72/82] test: add openstack cleanup fixture --- .../tests/integration/conftest.py | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/github-runner-manager/tests/integration/conftest.py b/github-runner-manager/tests/integration/conftest.py index 83875d6580..ddded58098 100644 --- a/github-runner-manager/tests/integration/conftest.py +++ b/github-runner-manager/tests/integration/conftest.py @@ -3,10 +3,32 @@ """Fixtures for github-runner-manager integration tests.""" +import logging +from typing import Generator + +import openstack import pytest from .factories import GitHubConfig, OpenStackConfig, ProxyConfig, TestConfig +logger = logging.getLogger(__name__) + + +@pytest.hookimpl(tryfirst=True, hookwrapper=True) +def pytest_runtest_makereport(item, call): + """Hook to capture test outcome for fixtures. + + Args: + item: Test item. + call: Test call information. + + Yields: + Test report. + """ + outcome = yield + rep = outcome.get_result() + setattr(item, f"rep_{rep.when}", rep) + @pytest.fixture def test_config() -> TestConfig: @@ -124,3 +146,80 @@ def proxy_config(pytestconfig: pytest.Config) -> ProxyConfig | None: openstack_https_proxy=openstack_https_proxy, openstack_no_proxy=openstack_no_proxy, ) + + +@pytest.fixture(autouse=True) +def openstack_cleanup( + openstack_config: OpenStackConfig, test_config: TestConfig, request: pytest.FixtureRequest +) -> Generator[None, None, None]: + """Clean up OpenStack resources after test execution. + + Args: + openstack_config: OpenStack configuration for connection. + test_config: Test configuration with unique VM prefix. + request: Pytest request fixture for accessing test outcome. + + Yields: + None + """ + yield + + if not openstack_config: + return + + logger.info("Cleaning up OpenStack resources with prefix: %s", test_config.vm_prefix) + + try: + with openstack.connect( + auth_url=openstack_config.auth_url, + project_name=openstack_config.project, + username=openstack_config.username, + password=openstack_config.password, + user_domain_name=openstack_config.user_domain_name, + project_domain_name=openstack_config.project_domain_name, + region_name=openstack_config.region_name, + ) as conn: + # Clean up servers + servers = [ + server + for server in conn.list_servers() + if server.name.startswith(test_config.vm_prefix) + ] + for server in servers: + logger.info("Fetching console log for server: %s", server.name) + try: + console_log = conn.get_server_console(server.id) + if console_log: + logger.info("Console log for server %s:\n%s", server.name, console_log) + except Exception as log_error: + logger.warning( + "Failed to fetch console log for server %s: %s", + server.name, + log_error, + ) + + logger.info("Deleting server: %s", server.name) + conn.delete_server(server.id, wait=True) + + # Clean up keypairs + keypairs = [ + keypair + for keypair in conn.list_keypairs() + if keypair.name.startswith(test_config.vm_prefix) + ] + for keypair in keypairs: + logger.info("Deleting keypair: %s", keypair.name) + conn.delete_keypair(keypair.name) + + # Clean up security groups (if named with prefix) + security_groups = [ + sg + for sg in conn.list_security_groups() + if sg.name.startswith(f"{test_config.vm_prefix}-") + ] + for sg in security_groups: + logger.info("Deleting security group: %s", sg.name) + conn.delete_security_group(sg.id) + + except Exception as e: + logger.error("Failed to clean up OpenStack resources: %s", e) From 1e764f5dc4020ade61140fd4d78a76bfc7d88e0a Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Fri, 28 Nov 2025 03:37:34 +0000 Subject: [PATCH 73/82] test: remove runtime hook --- .../tests/integration/conftest.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/github-runner-manager/tests/integration/conftest.py b/github-runner-manager/tests/integration/conftest.py index ddded58098..f73c1610c5 100644 --- a/github-runner-manager/tests/integration/conftest.py +++ b/github-runner-manager/tests/integration/conftest.py @@ -14,22 +14,6 @@ logger = logging.getLogger(__name__) -@pytest.hookimpl(tryfirst=True, hookwrapper=True) -def pytest_runtest_makereport(item, call): - """Hook to capture test outcome for fixtures. - - Args: - item: Test item. - call: Test call information. - - Yields: - Test report. - """ - outcome = yield - rep = outcome.get_result() - setattr(item, f"rep_{rep.when}", rep) - - @pytest.fixture def test_config() -> TestConfig: """Create a unique test configuration for parallel test execution. From 461744db110b481228da15b69f38843fc5ab45e4 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Sun, 30 Nov 2025 23:16:16 +0800 Subject: [PATCH 74/82] feat: flush runners on external contributor config change --- src/charm.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/charm.py b/src/charm.py index 64ebdaff78..370884a038 100755 --- a/src/charm.py +++ b/src/charm.py @@ -46,6 +46,7 @@ import logrotate import manager_service from charm_state import ( + ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME, DEBUG_SSH_INTEGRATION_NAME, IMAGE_INTEGRATION_NAME, LABELS_CONFIG_NAME, @@ -338,6 +339,14 @@ def _on_config_changed(self, _: ConfigChangedEvent) -> None: if self.config[LABELS_CONFIG_NAME] != self._stored.labels: self._stored.labels = self.config[LABELS_CONFIG_NAME] flush_runners = True + if ( + self.config[ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME] + != state.charm_config.allow_external_contributor + ): + self._stored.allow_external_contributor = self.config[ + ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME + ] + flush_runners = True self._check_image_ready() From 62ea95ad2d246f4f0293e0cc6b1d5fd09bcd5ebd Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Sun, 30 Nov 2025 23:17:18 +0800 Subject: [PATCH 75/82] feat: add allow_external_contributor to stored state --- src/charm.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/charm.py b/src/charm.py index 370884a038..c673b3486c 100755 --- a/src/charm.py +++ b/src/charm.py @@ -198,6 +198,9 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: path=self.config[PATH_CONFIG_NAME], # for detecting changes token=self.config[TOKEN_CONFIG_NAME], # for detecting changes labels=self.config[LABELS_CONFIG_NAME], # for detecting changes + allow_external_contributor=self.config[ + ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME + ], # for detecting changes ) self.on.define_event("reconcile_runners", ReconcileRunnersEvent) From 33d49e60f49a98040fdc69b110637e7ba359f704 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Sun, 30 Nov 2025 23:19:57 +0800 Subject: [PATCH 76/82] chore: increment app version & docs: update changelog --- docs/changelog.md | 5 +++++ github-runner-manager/pyproject.toml | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/changelog.md b/docs/changelog.md index 988f57aca2..463dd4bc86 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,6 +2,11 @@ This changelog documents user-relevant changes to the GitHub runner charm. +## 2025-11-30 + +- Added a new configuration option `allow-external-contributor` as an non-external replacement for + `repo-policy-compliance` service. + ## 2025-11-21 - Updated broken links in `docs/how-to/index.md`. diff --git a/github-runner-manager/pyproject.toml b/github-runner-manager/pyproject.toml index 36e9476832..6da56b2281 100644 --- a/github-runner-manager/pyproject.toml +++ b/github-runner-manager/pyproject.toml @@ -3,7 +3,7 @@ [project] name = "github-runner-manager" -version = "0.7.0" +version = "0.8.0" authors = [ { name = "Canonical IS DevOps", email = "is-devops-team@canonical.com" }, ] From c323b96741707576bc1b49f995174a48a48b2a23 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 1 Dec 2025 01:37:58 +0000 Subject: [PATCH 77/82] fix: use stored state --- src/charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index c673b3486c..640bb475ff 100755 --- a/src/charm.py +++ b/src/charm.py @@ -344,7 +344,7 @@ def _on_config_changed(self, _: ConfigChangedEvent) -> None: flush_runners = True if ( self.config[ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME] - != state.charm_config.allow_external_contributor + != self._stored.allow_external_contributor ): self._stored.allow_external_contributor = self.config[ ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME From 30dc913d7907aca3df6bdb27f8c999f90751575f Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 1 Dec 2025 06:52:12 +0000 Subject: [PATCH 78/82] fix: only check user association for pull_request events --- .../templates/pre-job.j2 | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 b/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 index 31ddcadf8b..d312137ca2 100644 --- a/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 +++ b/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 @@ -22,8 +22,7 @@ jq -n \ {% endif %} {% if do_repo_policy_check %} - logger -s "Repo policy check is marked for deprecation. Consider using \ - `allow_external_contributor` instead" + logger -s "Repo policy check is marked for deprecation. Consider using `allow_external_contributor` instead" # Log common env variables. logger -s "GITHUB_EVENT_NAME: ${GITHUB_EVENT_NAME}, \ @@ -135,15 +134,20 @@ jq -n \ {% endif %} {% if not allow_external_contributor %} - AUTHOR_ASSOCIATION=$(jq -r '.pull_request.author_association' "$GITHUB_EVENT_PATH") - logger -s "Author association: $AUTHOR_ASSOCIATION" + # Only check author association for pull request events + if [[ "${GITHUB_EVENT_NAME}" == "pull_request" ]]; then + AUTHOR_ASSOCIATION=$(jq -r '.pull_request.author_association' "$GITHUB_EVENT_PATH") + logger -s "Author association: $AUTHOR_ASSOCIATION" - if [[ "$AUTHOR_ASSOCIATION" != "OWNER" && "$AUTHOR_ASSOCIATION" != "MEMBER" && "$AUTHOR_ASSOCIATION" != "COLLABORATOR" ]]; then - logger -p user.error -s "Insufficient user authorization (author_association). Only OWNER, MEMBER, or COLLABORATOR may run workflows." - exit 1 - fi + if [[ "$AUTHOR_ASSOCIATION" != "OWNER" && "$AUTHOR_ASSOCIATION" != "MEMBER" && "$AUTHOR_ASSOCIATION" != "COLLABORATOR" ]]; then + logger -p user.error -s "Insufficient user authorization (author_association). Only OWNER, MEMBER, or COLLABORATOR may run workflows." + exit 1 + fi - logger -s "The contributor check has passed, proceeding to execute jobs" + logger -s "The contributor check has passed, proceeding to execute jobs" + else + logger -s "Skipping contributor check for non-pull request event: ${GITHUB_EVENT_NAME}" + fi {% endif %} if [[ -n "$DOCKERHUB_MIRROR" ]]; then From 1a1a037e72de5c3ea25a8085576e5123e6025bd8 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 1 Dec 2025 06:52:28 +0000 Subject: [PATCH 79/82] chore: whitespace for concatenated string --- .../src/github_runner_manager/repo_policy_compliance_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github-runner-manager/src/github_runner_manager/repo_policy_compliance_client.py b/github-runner-manager/src/github_runner_manager/repo_policy_compliance_client.py index aed0c58896..d289223230 100644 --- a/github-runner-manager/src/github_runner_manager/repo_policy_compliance_client.py +++ b/github-runner-manager/src/github_runner_manager/repo_policy_compliance_client.py @@ -31,7 +31,7 @@ def __init__(self, url: str, charm_token: str) -> None: charm_token: Charm token configured for the repo policy compliance service. """ logger.warning( - "Repo policy compliance service is marked for deprecation." + "Repo policy compliance service is marked for deprecation. " "Consider using `allow_external_contributor` option instead." ) self._session = self._create_session() From dadbf4133e817e59e2bd3e76070ded1d940ee5f8 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 1 Dec 2025 06:58:02 +0000 Subject: [PATCH 80/82] revert removed whitespace from test fixture docstring --- tests/integration/conftest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 43350905c5..0f0e2ecd77 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -733,6 +733,7 @@ async def app_with_forked_repo( model: Model, basic_app: Application, forked_github_repository: Repository ) -> Application: """Application with no runner on a forked repo. + Test should ensure it returns with the application in a good state and has one runner. """ From f51a1f415bf4e58a75a0bdcb26d6b0260095ea98 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Mon, 1 Dec 2025 07:34:36 +0000 Subject: [PATCH 81/82] feat: check all events that can be triggered by external contributors --- .../src/github_runner_manager/templates/pre-job.j2 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 b/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 index d312137ca2..f9b6d595ad 100644 --- a/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 +++ b/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 @@ -134,9 +134,9 @@ jq -n \ {% endif %} {% if not allow_external_contributor %} - # Only check author association for pull request events - if [[ "${GITHUB_EVENT_NAME}" == "pull_request" ]]; then - AUTHOR_ASSOCIATION=$(jq -r '.pull_request.author_association' "$GITHUB_EVENT_PATH") + # Check author association for events that can be triggered by external contributors + if [[ "${GITHUB_EVENT_NAME}" == "pull_request" || "${GITHUB_EVENT_NAME}" == "pull_request_target" || "${GITHUB_EVENT_NAME}" == "pull_request_review" || "${GITHUB_EVENT_NAME}" == "pull_request_review_comment" || "${GITHUB_EVENT_NAME}" == "issue_comment" ]]; then + AUTHOR_ASSOCIATION=$(jq -r '.pull_request.author_association // .issue.author_association // .review.author_association // .comment.author_association // empty' "$GITHUB_EVENT_PATH") logger -s "Author association: $AUTHOR_ASSOCIATION" if [[ "$AUTHOR_ASSOCIATION" != "OWNER" && "$AUTHOR_ASSOCIATION" != "MEMBER" && "$AUTHOR_ASSOCIATION" != "COLLABORATOR" ]]; then @@ -146,7 +146,7 @@ jq -n \ logger -s "The contributor check has passed, proceeding to execute jobs" else - logger -s "Skipping contributor check for non-pull request event: ${GITHUB_EVENT_NAME}" + logger -s "Skipping contributor check for event: ${GITHUB_EVENT_NAME}" fi {% endif %} From f3ecd9fc8d49b6b59e179089437d08f440b14bb8 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Wed, 3 Dec 2025 10:59:11 +0000 Subject: [PATCH 82/82] test: cover different event types --- .../templates/pre-job.j2 | 2 + .../tests/unit/test_pre_job_script.py | 321 ++++++++++++++++-- 2 files changed, 298 insertions(+), 25 deletions(-) diff --git a/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 b/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 index f9b6d595ad..fda5704aa5 100644 --- a/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 +++ b/github-runner-manager/src/github_runner_manager/templates/pre-job.j2 @@ -135,6 +135,8 @@ jq -n \ {% if not allow_external_contributor %} # Check author association for events that can be triggered by external contributors + # https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows + # This does not cover all possible events, only the most common ones that can be triggered by external contributors. if [[ "${GITHUB_EVENT_NAME}" == "pull_request" || "${GITHUB_EVENT_NAME}" == "pull_request_target" || "${GITHUB_EVENT_NAME}" == "pull_request_review" || "${GITHUB_EVENT_NAME}" == "pull_request_review_comment" || "${GITHUB_EVENT_NAME}" == "issue_comment" ]]; then AUTHOR_ASSOCIATION=$(jq -r '.pull_request.author_association // .issue.author_association // .review.author_association // .comment.author_association // empty' "$GITHUB_EVENT_PATH") logger -s "Author association: $AUTHOR_ASSOCIATION" diff --git a/github-runner-manager/tests/unit/test_pre_job_script.py b/github-runner-manager/tests/unit/test_pre_job_script.py index 74cfb13840..efbee18753 100644 --- a/github-runner-manager/tests/unit/test_pre_job_script.py +++ b/github-runner-manager/tests/unit/test_pre_job_script.py @@ -103,31 +103,13 @@ def _create_github_event_payload( Args: author_association: The author's association with the repository (OWNER, MEMBER, COLLABORATOR, CONTRIBUTOR, FIRST_TIME_CONTRIBUTOR, NONE, etc.) - event_type: The type of GitHub event (pull_request, push, etc.) + event_type: The type of GitHub event (pull_request, push, issue_comment, etc.) Returns: A dictionary representing the GitHub event payload """ - payload = { - "action": event_type, - "number": 123, - "pull_request": { - "author_association": author_association, - "number": 123, - "title": "Test PR", - "user": { - "login": "test-user", - "type": "User", - }, - "head": { - "ref": "feature-branch", - "sha": "abc123", - }, - "base": { - "ref": "main", - "sha": "def456", - }, - }, + # Base payload common to all events + base_payload = { "repository": { "name": "github-runner-operator", "full_name": "canonical/github-runner-operator", @@ -139,7 +121,155 @@ def _create_github_event_payload( "login": "test-user", }, } - return payload + + # Event-specific payloads + if event_type == "pull_request": + return { + **base_payload, + "action": "opened", + "number": 123, + "pull_request": { + "author_association": author_association, + "number": 123, + "title": "Test PR", + "user": { + "login": "test-user", + "type": "User", + }, + "head": { + "ref": "feature-branch", + "sha": "abc123", + "repo": { + "full_name": "canonical/github-runner-operator", + }, + }, + "base": { + "ref": "main", + "sha": "def456", + }, + }, + } + + elif event_type == "pull_request_target": + return { + **base_payload, + "action": "opened", + "number": 123, + "pull_request": { + "author_association": author_association, + "number": 123, + "title": "Test PR Target", + "user": { + "login": "test-user", + "type": "User", + }, + "head": { + "ref": "feature-branch", + "sha": "abc123", + "repo": { + "full_name": "canonical/github-runner-operator", + }, + }, + "base": { + "ref": "main", + "sha": "def456", + }, + }, + } + + elif event_type == "pull_request_review": + return { + **base_payload, + "action": "submitted", + "pull_request": { + "number": 123, + "title": "Test PR for Review", + }, + "review": { + "author_association": author_association, + "user": { + "login": "test-user", + }, + "state": "approved", + "body": "LGTM", + }, + } + + elif event_type == "pull_request_review_comment": + return { + **base_payload, + "action": "created", + "pull_request": { + "number": 123, + "title": "Test PR for Review Comment", + }, + "comment": { + "author_association": author_association, + "user": { + "login": "test-user", + }, + "body": "Good catch!", + }, + } + + elif event_type == "issue_comment": + return { + **base_payload, + "action": "created", + "issue": { + "author_association": author_association, + "number": 456, + "title": "Test Issue", + "user": { + "login": "test-user", + }, + }, + "comment": { + "author_association": author_association, + "user": { + "login": "test-user", + }, + "body": "This is a comment", + }, + } + + elif event_type == "push": + return { + **base_payload, + "ref": "refs/heads/main", + "before": "def456", + "after": "abc123", + "commits": [ + { + "id": "abc123", + "message": "Test commit", + "author": { + "name": "test-user", + "email": "test@example.com", + }, + } + ], + } + + elif event_type == "workflow_dispatch": + return { + **base_payload, + "ref": "refs/heads/main", + "inputs": {}, + } + + elif event_type == "schedule": + return { + **base_payload, + "schedule": "0 0 * * *", + } + + else: + # Default fallback for unknown event types + return { + **base_payload, + "action": "unknown", + } def render_and_execute_script( @@ -318,11 +448,152 @@ def test_allow_external_contributor_null_or_missing_fields_blocked( tmp_path=tmp_path, ) - # When pull_request or author_association is missing, jq returns "null" - # The check should fail as "null" != "OWNER|MEMBER|COLLABORATOR" + # When pull_request or author_association is missing, jq returns empty string + # The check should fail as "" != "OWNER|MEMBER|COLLABORATOR" assert result.returncode == 1, ( f"Expected exit code 1 for {description}, got {result.returncode}\n" f"stderr: {result.stderr}" ) - assert "Author association: null" in result.stderr + assert "Author association: " in result.stderr assert "Insufficient user authorization" in result.stderr + + +@pytest.mark.parametrize( + "event_type,author_association,expected_exit_code", + [ + # Events that trigger author association checks + ("pull_request", "OWNER", 0), + ("pull_request", "MEMBER", 0), + ("pull_request", "COLLABORATOR", 0), + ("pull_request", "CONTRIBUTOR", 1), + ("pull_request_target", "OWNER", 0), + ("pull_request_target", "CONTRIBUTOR", 1), + ("pull_request_review", "MEMBER", 0), + ("pull_request_review", "FIRST_TIME_CONTRIBUTOR", 1), + ("pull_request_review_comment", "COLLABORATOR", 0), + ("pull_request_review_comment", "NONE", 1), + ("issue_comment", "OWNER", 0), + ("issue_comment", "CONTRIBUTOR", 1), + # Events that skip author association checks + ("push", "CONTRIBUTOR", 0), # Should be skipped + ("workflow_dispatch", "CONTRIBUTOR", 0), # Should be skipped + ("schedule", "CONTRIBUTOR", 0), # Should be skipped + ], + ids=[ + "pr_owner_allowed", + "pr_member_allowed", + "pr_collaborator_allowed", + "pr_contributor_blocked", + "pr_target_owner_allowed", + "pr_target_contributor_blocked", + "pr_review_member_allowed", + "pr_review_first_time_blocked", + "pr_review_comment_collaborator_allowed", + "pr_review_comment_none_blocked", + "issue_comment_owner_allowed", + "issue_comment_contributor_blocked", + "push_skipped", + "workflow_dispatch_skipped", + "schedule_skipped", + ], +) +def test_author_association_check_by_event_type( + pre_job_template: Template, + github_env_vars: Dict[str, str], + tmp_path: Path, + event_type: str, + author_association: str, + expected_exit_code: int, + default_template_vars: Dict, +): + """Test author association checks for different GitHub event types.""" + # Update environment to match the event type + env_vars = github_env_vars.copy() + env_vars["GITHUB_EVENT_NAME"] = event_type + + github_event = _create_github_event_payload( + author_association=author_association, + event_type=event_type, + ) + + result = render_and_execute_script( + template=pre_job_template, + template_vars=default_template_vars, + env_vars=env_vars, + github_event=github_event, + tmp_path=tmp_path, + ) + + assert result.returncode == expected_exit_code, ( + f"Expected exit code {expected_exit_code} for {event_type} with {author_association}, " + f"got {result.returncode}\nstderr: {result.stderr}" + ) + + # Check for appropriate log messages + if event_type in [ + "pull_request", + "pull_request_target", + "pull_request_review", + "pull_request_review_comment", + "issue_comment", + ]: + # These events should trigger the author association check + assert f"Author association: {author_association}" in result.stderr + + if expected_exit_code == 0: + assert "The contributor check has passed, proceeding to execute jobs" in result.stderr + assert "Insufficient user authorization" not in result.stderr + else: + assert "Insufficient user authorization (author_association)" in result.stderr + assert "Only OWNER, MEMBER, or COLLABORATOR may run workflows" in result.stderr + else: + # These events should skip the author association check + assert f"Skipping contributor check for event: {event_type}" in result.stderr + assert "Author association:" not in result.stderr + assert "Insufficient user authorization" not in result.stderr + + +@pytest.mark.parametrize( + "event_type", + [ + "push", + "workflow_dispatch", + "schedule", + "release", + "deployment", + "unknown_event", + ], +) +def test_events_skip_author_association_check( + pre_job_template: Template, + github_env_vars: Dict[str, str], + tmp_path: Path, + event_type: str, + default_template_vars: Dict, +): + """Test that non-contributor events skip the author association check.""" + env_vars = github_env_vars.copy() + env_vars["GITHUB_EVENT_NAME"] = event_type + + github_event = _create_github_event_payload( + author_association="CONTRIBUTOR", # This would normally be blocked + event_type=event_type, + ) + + result = render_and_execute_script( + template=pre_job_template, + template_vars=default_template_vars, + env_vars=env_vars, + github_event=github_event, + tmp_path=tmp_path, + ) + + # All these events should succeed regardless of author association + assert ( + result.returncode == 0 + ), f"Expected exit code 0 for {event_type}, got {result.returncode}\nstderr: {result.stderr}" + + # Should skip the contributor check + assert f"Skipping contributor check for event: {event_type}" in result.stderr + assert "Author association:" not in result.stderr + assert "Insufficient user authorization" not in result.stderr