diff --git a/.github/workflows/test_github_runner_manager.yaml b/.github/workflows/test_github_runner_manager.yaml index 3f7eff3758..ae67a2c7b6 100644 --- a/.github/workflows/test_github_runner_manager.yaml +++ b/.github/workflows/test_github_runner_manager.yaml @@ -4,10 +4,47 @@ 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 + working-directory: ./github-runner-manager/ + env: + # GitHub configuration + 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_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 }} + OS_REGION_NAME: ${{ vars.INTEGRATION_TEST_OPENSTACK_REGION_NAME }} + run: | + tox -e integration -- -v --tb=native -s \ + --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 }}" \ + --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-flavor="${{ vars.INTEGRATION_TEST_OPENSTACK_FLAVOR_NAME }}" \ + --openstack-image-id="${{ vars.INTEGRATION_TEST_IMAGE_ID }}" diff --git a/config.yaml b/config.yaml index 113005dddf..cf8935e61a 100644 --- a/config.yaml +++ b/config.yaml @@ -88,15 +88,26 @@ options: repo-policy-compliance-token: type: string description: >- + 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 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: True + description: >- + EXPERIMENTAL + 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: >- diff --git a/docs/changelog.md b/docs/changelog.md index bc9d1f96c8..d7f2f2a56b 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -6,6 +6,11 @@ This changelog documents user-relevant changes to the GitHub runner charm. - Added timeouts to API calls (Openstack,GitHub,Repo policy compliance) to fix a hanging GitHub runner manager application. +## 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" }, ] 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 5183ddb569..4aaffaa78c 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. github_config: GitHub configuration. @@ -48,6 +53,7 @@ class ApplicationConfiguration(BaseModel): reconcile_interval: Seconds to wait between reconciliation. """ + allow_external_contributor: bool = False name: str extra_labels: list[str] github_config: github.GitHubConfiguration | 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 448c0d4265..7ce2207a1f 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/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/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..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 @@ -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: @@ -196,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: 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 a5a0c6dc1e..71fe7d1ecf 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 @@ -33,6 +33,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. " + "Consider using `allow_external_contributor` option instead." + ) self._session = self._create_session() self.base_url = url self.token = charm_token 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..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 @@ -22,6 +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" # Log common env variables. logger -s "GITHUB_EVENT_NAME: ${GITHUB_EVENT_NAME}, \ @@ -132,6 +133,25 @@ jq -n \ logger -s "The repository setup check has passed, proceeding to execute jobs" {% endif %} +{% 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" + + 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" + else + logger -s "Skipping contributor check for event: ${GITHUB_EVENT_NAME}" + fi +{% 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/conftest.py b/github-runner-manager/tests/conftest.py index 09a84dda80..c9d7ea641f 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,4 +12,116 @@ def pytest_addoption(parser): Args: parser: Pytest parser. """ - parser.addoption("--charm-file", action="store") + parser.addoption( + "--github-token", + action="store", + help="GitHub personal access token for integration tests.", + default=os.getenv("INTEGRATION_TOKEN"), + ) + 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.", + default=os.getenv("INTEGRATION_TOKEN_ALT"), + ) + parser.addoption( + "--openstack-auth-url", + action="store", + 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", + 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"), + ) + 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"), + ) + 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/__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/application.py b/github-runner-manager/tests/integration/application.py new file mode 100644 index 0000000000..defa10f2a2 --- /dev/null +++ b/github-runner-manager/tests/integration/application.py @@ -0,0 +1,198 @@ +# Copyright 2025 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Test infrastructure for managing running application instances.""" + +import logging +import multiprocessing +import os +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 + +logger = logging.getLogger(__name__) + + +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, 204, 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: + 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 +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: + 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. + 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. + """ + 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. + + 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, + metrics_log_path: Path | None = None, + ) -> "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. + 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. + + 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, + 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..f73c1610c5 --- /dev/null +++ b/github-runner-manager/tests/integration/conftest.py @@ -0,0 +1,209 @@ +# Copyright 2025 Canonical Ltd. +# See LICENSE file for licensing details. + +"""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.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.fail: If neither --github-token option nor INTEGRATION_TOKEN + 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 alt_token or not path: + pytest.fail( + "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, alt_token=alt_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") + 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") + 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, + project_domain_name, + username, + user_domain_name, + 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, + user_domain_name=user_domain_name, + project_domain_name=project_domain_name, + flavor=flavor, + image_id=image_id, + ) + + +@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, + ) + + +@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) diff --git a/github-runner-manager/tests/integration/factories.py b/github-runner-manager/tests/integration/factories.py new file mode 100644 index 0000000000..a710f95ec8 --- /dev/null +++ b/github-runner-manager/tests/integration/factories.py @@ -0,0 +1,223 @@ +# 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. + alt_token: Alternate GitHub personal access token for external contributor. + path: GitHub path in / or format. + """ + + token: str + alt_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. + 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. + """ + + auth_url: str + project: str + username: str + 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 + + +@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. + + 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, + proxy_config: ProxyConfig | 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. + proxy_config: Proxy configuration. Defaults to no proxy. + + Returns: + Configuration dictionary for the application. + """ + # Use defaults if not provided + if github_config is None: + github_config = GitHubConfig( + token="ghp_test_token_1234567890abcdef", + alt_token="ghp_test_alt_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", + flavor="small", + image_id=None, + ) + + 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"} + + # Build proxy configuration + runner_proxy = None + if proxy_config and (proxy_config.http_proxy or proxy_config.https_proxy): + runner_proxy = { + "http": proxy_config.http_proxy, + "https": proxy_config.https_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, + "extra_labels": test_config.labels, + "github_config": { + "token": github_config.token, + "path": path_config, + }, + "service_config": { + "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, + "custom_pre_job_script": None, + }, + "non_reactive_configuration": { + "combinations": [ + { + "image": { + "name": openstack_config.image_id or "noble", + "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, + "credentials": { + "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, + }, + }, + "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..f7a20d4167 --- /dev/null +++ b/github-runner-manager/tests/integration/github_helpers.py @@ -0,0 +1,149 @@ +# Copyright 2025 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Helper functions for GitHub integration testing.""" + +from time import sleep, time + +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( + upstream_repository: Repository, + forked_github_repository: Repository, + test_id: str, +) -> PullRequest: + """Create a PR from a forked repository. + + Args: + upstream_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(upstream_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 = 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}", + ) + + return pr + + +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. + """ + start_time = time() + while time() - start_time < timeout: + 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 "" + + 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: + """Close a pull request (best effort). + + Args: + pr: The pull request to close. + """ + try: + 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 new file mode 100644 index 0000000000..c98bfffd94 --- /dev/null +++ b/github-runner-manager/tests/integration/test_external_contributor_security.py @@ -0,0 +1,345 @@ +# Copyright 2025 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Integration tests for external contributor security configuration.""" + +import logging +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, ProxyConfig, create_default_config +from .github_helpers import ( + close_pull_request, + create_fork_and_pr, + get_job_logs, + get_pr_workflow_runs, + wait_for_workflow_completion, +) + +if TYPE_CHECKING: + from .factories import TestConfig + +logger = logging.getLogger(__name__) + +# Test workflow filename +PULL_REQUEST_WORKFLOW_NAME = "Pull request test" + + +@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 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: + 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) + + logger.info( + "Creating fork of repository %s using alternate token", upstream_repository.full_name + ) + 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 + 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", + proxy_config: ProxyConfig | None, +) -> 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. + proxy_config: Proxy configuration object or None. + + 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, + proxy_config=proxy_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, + ) + metrics_log_path = tmp_path / "github-runner-metrics.log" + app = RunningApplication.create(config_path, metrics_log_path=metrics_log_path) + + yield app + + 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", + proxy_config: ProxyConfig | None, +) -> 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. + proxy_config: Proxy configuration object or None. + + 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, + proxy_config=proxy_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, + ) + metrics_log_path = tmp_path / "github-runner-metrics.log" + app = RunningApplication.create(config_path, metrics_log_path=metrics_log_path) + + yield app + + logger.info("Stopping application") + app.stop() + + +@pytest.fixture +def external_contributor_pull_request( + upstream_repository: Repository, + forked_github_repository: Repository, + test_config: "TestConfig", +) -> Iterator[PullRequest]: + """Create a pull request from forked repository simulating external contributor. + + Args: + upstream_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(upstream_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, external_contributor_pull_request: PullRequest +): + """ + arrange: Application running with allow_external_contributor=False, forked repository \ + from external contributor. + 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") + + # 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, + pr=external_contributor_pull_request, + workflow_name=PULL_REQUEST_WORKFLOW_NAME, + timeout=120, + ) + 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)...") + 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, external_contributor_pull_request: PullRequest +): + """ + arrange: Application running with allow_external_contributor=True, forked repository \ + from external contributor. + 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") + + # 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, + pr=external_contributor_pull_request, + workflow_name=PULL_REQUEST_WORKFLOW_NAME, + timeout=120, + ) + 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)...") + 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) + + # 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/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_pre_job_script.py b/github-runner-manager/tests/unit/test_pre_job_script.py new file mode 100644 index 0000000000..efbee18753 --- /dev/null +++ b/github-runner-manager/tests/unit/test_pre_job_script.py @@ -0,0 +1,599 @@ +# 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, issue_comment, etc.) + + Returns: + A dictionary representing the GitHub event payload + """ + # Base payload common to all events + base_payload = { + "repository": { + "name": "github-runner-operator", + "full_name": "canonical/github-runner-operator", + "owner": { + "login": "canonical", + }, + }, + "sender": { + "login": "test-user", + }, + } + + # 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( + 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 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: " 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 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", diff --git a/github-runner-manager/tox.ini b/github-runner-manager/tox.ini index 83811fac8f..b3ef624757 100644 --- a/github-runner-manager/tox.ini +++ b/github-runner-manager/tox.ini @@ -98,3 +98,30 @@ deps = -r{toxinidir}/requirements.txt commands = bandit -c {toxinidir}/pyproject.toml -r {[vars]src_path} + +[testenv:integration] +description = Run integration tests +deps = + -e . + pytest + pytest-asyncio + pyyaml + requests + pygithub + -r{toxinidir}/requirements.txt +passenv = + PYTHONPATH + GITHUB_REPOSITORY + 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 + OS_REGION_NAME +commands = + pytest {[vars]tst_path}integration \ + -v --tb native -s {posargs} diff --git a/src/charm.py b/src/charm.py index 64ebdaff78..640bb475ff 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, @@ -197,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) @@ -338,6 +342,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] + != self._stored.allow_external_contributor + ): + self._stored.allow_external_contributor = self.config[ + ALLOW_EXTERNAL_CONTRIBUTOR_CONFIG_NAME + ] + flush_runners = True self._check_image_ready() diff --git a/src/charm_state.py b/src/charm_state.py index d779e78a25..ede55392aa 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" @@ -200,6 +201,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_EXTERNAL_CONTRIBUTOR_CONFIG_NAME, + ) # pydantic allows string to be passed as AnyHttpUrl, mypy complains about it return cls(url=url, token=token) # type: ignore @@ -210,6 +218,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. @@ -226,6 +237,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 @@ -501,7 +513,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, False)) custom_pre_job_script = ( cast(str, charm.config.get(CUSTOM_PRE_JOB_SCRIPT_CONFIG_NAME, "")) or None @@ -512,6 +524,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, diff --git a/src/factories.py b/src/factories.py index 94eb6d100b..0a14c4fc4b 100644 --- a/src/factories.py +++ b/src/factories.py @@ -88,6 +88,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/integration/conftest.py b/tests/integration/conftest.py index 5274a75dc0..0f0e2ecd77 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -21,6 +21,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 @@ -266,8 +267,7 @@ def openstack_config(pytestconfig: pytest.Config) -> OpenStackConfig: region_name = pytestconfig.getoption("--openstack-region-name") assert all( - val - for val in ( + [ auth_url, password, project_domain_name, @@ -275,7 +275,7 @@ def openstack_config(pytestconfig: pytest.Config) -> OpenStackConfig: user_domain_name, username, region_name, - ) + ] ), "Specify all OpenStack private endpoint options." test_image_id = pytestconfig.getoption("--test-image-id") @@ -660,7 +660,7 @@ async def tmate_ssh_server_unit_ip_fixture( @pytest.fixture(scope="module") def github_client(github_config: GitHubConfig) -> Github: """Returns the github client.""" - gh = Github(github_config.token) + gh = Github(auth=Token(token=github_config.token)) rate_limit = gh.get_rate_limit() logging.info("GitHub token rate limit: %s", rate_limit.rate) return gh 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 7a33bfd8bd..aef89f9a8e 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, @@ -341,6 +342,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", @@ -392,6 +394,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(): diff --git a/tests/unit/test_factories.py b/tests/unit/test_factories.py index a32e2a8e0b..6e3d4d53e0 100644 --- a/tests/unit/test_factories.py +++ b/tests/unit/test_factories.py @@ -38,6 +38,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=GitHubConfiguration(