diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index dc6fe6016..93293d80d 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -20,7 +20,7 @@ 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"]' + modules: '["test_charm_metrics_failure", "test_charm_metrics_success", "test_charm_fork_path_change", "test_charm_no_runner", "test_charm_runner", "test_debug_ssh", "test_charm_upgrade", "test_reactive"]' # INTEGRATION_TOKEN, INTEGRATION_TOKEN_ALT, OS_* are passed through INTEGRATION_TEST_SECRET_ENV_VALUE_ # mapping. See CONTRIBUTING.md for more details. extra-arguments: | diff --git a/config.yaml b/config.yaml index cf8935e61..59fe1475f 100644 --- a/config.yaml +++ b/config.yaml @@ -85,13 +85,6 @@ options: Minutes between each reconciliation of the current runners state and their targeted state. On reconciliation, the charm polls the state of runners and see if actions are needed. The value should be kept low, unless Github API rate limiting is encountered. - 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: >- diff --git a/docs/changelog.md b/docs/changelog.md index 6a2b6c1fd..f7d98fabf 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,13 +2,17 @@ This changelog documents user-relevant changes to the GitHub runner charm. +## 2025-12-09 + +- Deprecate `repo-policy-compliance` service. + ## 2025-12-05 - Modified pre-job script to distinguish between internal PRs and fork PRs when applying author association checks. Internal PRs (where head and base repositories match) now skip the author association check, allowing team members to run workflows on their internal branches. Fork PRs continue to enforce OWNER/MEMBER/COLLABORATOR requirements for security. ## 2025-12-01 -- Added timeouts to API calls (Openstack,GitHub,Repo policy compliance) to fix a hanging GitHub runner manager application. +- Added timeouts to API calls (Openstack,GitHub,Repo policy compliance) to fix a hanging GitHub runner manager application. ## 2025-11-30 @@ -39,7 +43,6 @@ This changelog documents user-relevant changes to the GitHub runner charm. - Fix issue with scaling down overshooting in deleting runners after cleanup - ## 2025-08-20 - Document relevant log files. @@ -72,7 +75,7 @@ This changelog documents user-relevant changes to the GitHub runner charm. ## 2025-07-09 - Specify max supported nova compute API to be 2.91. This fixes an issue where the charm could fail - due to a bug on the OpenStack side: https://bugs.launchpad.net/nova/+bug/2095364 + due to a bug on the OpenStack side: https://bugs.launchpad.net/nova/+bug/2095364 ### 2025-06-30 @@ -92,8 +95,7 @@ This changelog documents user-relevant changes to the GitHub runner charm. ### 2025-06-16 - Revert `copytruncate logrotate` method for reactive processes, as `copytruncate` keeps log files on disks and does not remove them, and each process is writing to a new file leading to a huge and increasing amount -of zero sized files in the reactive log directory. This is a temporary fix until a better solution is implemented, as it has the downside that long lived reactive processes may write to deleted log files. - + of zero sized files in the reactive log directory. This is a temporary fix until a better solution is implemented, as it has the downside that long lived reactive processes may write to deleted log files. ### 2025-06-12 @@ -105,9 +107,9 @@ of zero sized files in the reactive log directory. This is a temporary fix until ### 2025-06-04 -- Reduce the reconcile-interval configuration from 10 minutes to 5 minutes. This is the interval -between reconciling the current and intended number of runners. The value should be kept low, -unless GitHub API rate limiting is encountered. +- Reduce the reconcile-interval configuration from 10 minutes to 5 minutes. This is the interval + between reconciling the current and intended number of runners. The value should be kept low, + unless GitHub API rate limiting is encountered. - Removed the reconcile-runners Juju action. ### 2025-06-03 @@ -116,9 +118,8 @@ unless GitHub API rate limiting is encountered. ### 2025-05-22 -- Add possibility to run a script in the pre-job phase of a runner. This can be useful to setup -network/infrastructure specific things. - +- Add possibility to run a script in the pre-job phase of a runner. This can be useful to setup + network/infrastructure specific things. ### 2025-05-09 @@ -127,8 +128,8 @@ network/infrastructure specific things. ### 2025-05-06 - The ssh health checks are removed and GitHub is used instead to get the runners health -information. This implies many changes in both the structure of the project and its functionality. Potentially, many race conditions should -disappear. + information. This implies many changes in both the structure of the project and its functionality. Potentially, many race conditions should + disappear. ### 2025-04-28 @@ -162,7 +163,7 @@ disappear. ### 2025-03-24 - New terraform product module. This module is composed of one github-runner-image-builder application and the related -github-runner applications. + github-runner applications. ### 2024-12-13 @@ -208,14 +209,14 @@ github-runner applications. ### 2024-10-17 - Use in-memory authentication instead of clouds.yaml on disk for OpenStack. This prevents -the multi-processing fighting over the file handle for the clouds.yaml file in the `github-runner-manager`. + the multi-processing fighting over the file handle for the clouds.yaml file in the `github-runner-manager`. - Fixed a bug where metrics storage for unmatched runners could not get cleaned up. ### 2024-10-11 - Added support for COS integration with reactive runners. -- The charm now creates a dedicated user which is used for running the reactive process and +- The charm now creates a dedicated user which is used for running the reactive process and storing metrics and ssh keys (also for non-reactive mode). ### 2024-10-07 diff --git a/docs/explanation/charm-architecture.md b/docs/explanation/charm-architecture.md index 0579e774f..29a76aa33 100644 --- a/docs/explanation/charm-architecture.md +++ b/docs/explanation/charm-architecture.md @@ -8,7 +8,6 @@ Conceptually, the charm can be divided into the following: - Management of the virtual machine image - Management of the network - GitHub API usage -- Management of [Python web service for checking GitHub repository settings](https://github.com/canonical/repo-policy-compliance) - Management of dependencies # Description of the charm's main components @@ -112,18 +111,28 @@ The charm requires a GitHub personal access token for the [`token` configuration - Requesting a list of self-hosted runners configured in an organization or repository - Deleting self-hosted runners -The token is also passed to [repo-policy-compliance](https://github.com/canonical/repo-policy-compliance) to access GitHub API for the service. - Note that the GitHub API uses a [rate-limiting mechanism](https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28). When this is reached, the charm may not be able to perform the necessary operations and may go into BlockedStatus. The charm will automatically recover from this state once the rate limit is reset, but using a different token with a higher rate limit may be a better solution depending on your deployment requirements. -## GitHub repository setting check +## External contributor access control -The [repo-policy-compliance](https://github.com/canonical/repo-policy-compliance) is a [Flask application](https://flask.palletsprojects.com/) hosted on [Gunicorn](https://gunicorn.org/) that provides a RESTful HTTP API to check the settings of GitHub repositories. This ensures the GitHub repository settings do not allow the execution of code not reviewed by maintainers on the self-hosted runners. - -Using the [pre-job script](https://docs.github.com/en/actions/hosting-your-own-runners/managing-self-hosted-runners/running-scripts-before-or-after-a-job#about-pre--and-post-job-scripts), the self-hosted runners call the Python web service to check if the GitHub repository settings for the job are compliant. If not compliant, it will output an error message and force stop the runner to prevent code from being executed. +The charm provides security controls for managing external contributor access through the `allow-external-contributor` configuration option. +External contributors are defined as users not in COLLABORATOR, MEMBER, or OWNER status. For +example, pull requests from fork repositories from users not in COLLABORATOR, MEMBER or OWNER +status would be considered requests from an external contributor. Internal requests such as +an internal PR from a renovate bot account would not be considered an event from an external +contributor. +When set to `false`, the charm restricts workflow execution to external contributors for the following GitHub events: + +- `pull_request` - Pull requests from external contributors +- `pull_request_target` - Pull request targeting (designed for handling PRs from forks) +- `pull_request_review` - Pull request reviews from external contributors +- `pull_request_review_comment` - Comments on pull request diffs from external contributors +- `issue_comment` - Comments on issues or pull requests from external contributors + +Using the [pre-job script](https://docs.github.com/en/actions/hosting-your-own-runners/managing-self-hosted-runners/running-scripts-before-or-after-a-job#about-pre--and-post-job-scripts), the self-hosted runners check the author association of the user triggering the workflow. If the user does not have sufficient permissions, the runner will output an error message and stop execution to prevent unauthorized code from running. ## COS integration Upon integration through the `cos-agent`, the charm initiates the logging of specific metric events diff --git a/docs/explanation/security.md b/docs/explanation/security.md index 3462b0dca..e740d4ad6 100644 --- a/docs/explanation/security.md +++ b/docs/explanation/security.md @@ -6,12 +6,13 @@ This document describes the security design of the GitHub runner charm. The char The charm manages GitHub self-hosted runners to run GitHub Actions jobs. This allows users on GitHub to execute code on the servers hosting the runners, which poses a remote code execution risk if the code is not trusted. Therefore, the charm should only spawn runners to trusted organizations or repositories. -For third-party contributions, the charm can integrate with the [Repo Policy Compliance charm](https://charmhub.io/repo-policy-compliance) to manage the repository policy. With this integration, the self-hosted runner will not execute the GitHub jobs if the policy is not met. See [working with outside collaborators](https://charmhub.io/github-runner/docs/how-to-comply-security#working-with-outside-collaborators) for the recommended settings to ensure the code is reviewed by a trusted user prior to execution in the runner. +For external contributor security, see [How to manage external contributors securely](../how-to/manage-external-contributors.md) for configuration options and recommended practices. ### Good practices - Only register the GitHub self-hosted runners to a trusted organization or repository so that only workflows from trusted users are able to run on the runners. -- For outside collaborators: Use the [`repo-policy-compliance` charm](https://charmhub.io/repo-policy-compliance) with [policy for outside collaborators](https://charmhub.io/github-runner/docs/how-to-comply-security#working-with-outside-collaborators) to ensure the code executed in runners are reviewed by a trusted user. +- For outside collaborators: Use the `allow-external-contributor` configuration option (set to `false`) to restrict workflow execution to users with COLLABORATOR, MEMBER, or OWNER status. This prevents unauthorized code execution from untrusted external contributors. +- Configure appropriate repository settings and protection rules to ensure the code executed in runners are reviewed by a trusted user. ## Permission for GitHub app or personal access token diff --git a/docs/how-to/comply-security.md b/docs/how-to/comply-security.md index 0750ddf75..00667f0cf 100644 --- a/docs/how-to/comply-security.md +++ b/docs/how-to/comply-security.md @@ -2,9 +2,29 @@ According to GitHub, running code inside the GitHub self-hosted runner [poses a significant security risk of arbitrary code execution](https://docs.github.com/en/actions/hosting-your-own-runners/managing-self-hosted-runners/about-self-hosted-runners#self-hosted-runner-security). The self-hosted runners managed by the charm are isolated in its own single-use virtual machine instance. In addition, the charm enforces some repository settings to ensure all code running on the self-hosted runners is reviewed by someone trusted. -The charm can be integrated with the [Repo Policy Compliance charm](https://charmhub.io/repo-policy-compliance) to enforce a set of good practices around GitHub repository settings. Self-hosted runners managed by the charm will not run jobs on repositories unless they are compliant with the practices. +The charm provides the `allow-external-contributor` configuration option to control whether workflows triggered by external contributors can execute on the self-hosted runners. When set to `false`, only users with COLLABORATOR, MEMBER, or OWNER status can trigger workflows from pull requests, reviews, and comments. -In this guide, a recommended set of policies will be presented. +In this guide, a recommended set of policies and security practices will be presented. + +## External contributor access control + +Configure the charm to restrict external contributor access: + +```bash +juju config github-runner allow-external-contributor=false +``` + +With this setting, workflows will only run for users with the following GitHub author associations: +- `OWNER` - Repository or organization owners +- `MEMBER` - Organization members +- `COLLABORATOR` - Users with explicit collaborator access + +The charm checks author associations for these events: +- `pull_request` - Pull requests from external contributors +- `pull_request_target` - Pull request targeting (designed for handling PRs from forks) +- `pull_request_review` - Pull request reviews from external contributors +- `pull_request_review_comment` - Comments on pull request diffs from external contributors +- `issue_comment` - Comments on issues or pull requests from external contributors ## Recommended policy @@ -20,4 +40,12 @@ With these settings, the common workflow of creating branches with pull requests ### Working with outside collaborators -Generally, outside collaborators are not completely trusted, but still would need to contribute in some manner. As such, this charm requires pull requests by outside collaborators to be reviewed by someone with `write` permission or above. Once the review is completed, the reviewer should add a comment including the following string: `/canonical/self-hosted-runners/run-workflows `, where `` is the commit SHA of the approved commit. Once posted, the self-hosted runners will run the workflow for this commit. \ No newline at end of file +When `allow-external-contributor` is set to `false`, outside collaborators can still contribute through the following secure workflow: + +1. External contributors create pull requests as usual +2. A repository maintainer with COLLABORATOR, MEMBER, or OWNER status reviews the code +3. If the code is safe, the maintainer can: + - Approve and merge the pull request to another branch (workflows will run with the maintainer's permissions) + - Manually trigger workflow runs if needed (using workflow dispatch on the target branch) + +This approach ensures that all code from external contributors is reviewed by trusted users before execution on self-hosted runners, eliminating the need for manual comment-based approval workflows. diff --git a/docs/how-to/manage-external-contributors.md b/docs/how-to/manage-external-contributors.md new file mode 100644 index 000000000..67b3d848f --- /dev/null +++ b/docs/how-to/manage-external-contributors.md @@ -0,0 +1,92 @@ +# How to manage external contributors securely + +According to GitHub, running code inside the GitHub self-hosted runner [poses a significant security risk of arbitrary code execution](https://docs.github.com/en/actions/hosting-your-own-runners/managing-self-hosted-runners/about-self-hosted-runners#self-hosted-runner-security). The self-hosted runners managed by the charm are isolated in their own single-use virtual machine instances. In addition, the charm provides security controls to ensure that code from external contributors is only executed when authorized. + +The charm provides the `allow-external-contributor` configuration option to control whether workflows triggered by external contributors can execute on the self-hosted runners. When set to `false`, only users with COLLABORATOR, MEMBER, or OWNER status can trigger workflows. + +In this guide, we'll explain how to configure external contributor access and recommended security practices. + +## External contributor access control + +The charm checks the GitHub author association for the following events that can be triggered by external contributors: + +- `pull_request` - Pull requests from external contributors +- `pull_request_target` - Pull request targeting (designed for handling PRs from forks) +- `pull_request_review` - Pull request reviews from external contributors +- `pull_request_review_comment` - Comments on pull request diffs from external contributors +- `issue_comment` - Comments on issues or pull requests from external contributors + +### Disabling external contributor access + +To prevent external contributors from triggering workflows: + +```bash +juju config github-runner allow-external-contributor=false +``` + +With this setting, only users with the following GitHub author associations can trigger workflows: + +- `OWNER` - Repository or organization owners +- `MEMBER` - Organization members +- `COLLABORATOR` - Users with explicit collaborator access + +### Enabling external contributor access + +To allow all external contributors to trigger workflows: + +```bash +juju config github-runner allow-external-contributor=true +``` + +**Warning**: This setting allows any external contributor to trigger workflow execution on your self-hosted runners. Only use this in trusted environments or when you have other security controls in place. + +## Recommended security practices + +When working with external contributors, consider the following security practices: + +### Repository configuration + +- For outside collaborators, set permissions to read-only. See [here](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/managing-repository-settings/managing-teams-and-people-with-access-to-your-repository#changing-permissions-for-a-team-or-person) for instructions to change collaborator permissions. + +### Branch protection rules + +Create the following branch protection rules using the instructions [here](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/managing-a-branch-protection-rule#creating-a-branch-protection-rule): + +- Branch name pattern `**` with `Require signed commits` enabled. +- Branch name pattern matching only the default branch of the repository, such as `main`, with the following enabled: + - `Dismiss stale pull request approvals when new commits are pushed` + - `Required signed commits` + - `Do not allow bypassing the above settings` + +### Working with external contributors + +When `allow-external-contributor` is set to `false`, external contributors can still contribute through the following workflow: + +1. External contributors create pull requests as usual +2. A repository maintainer with COLLABORATOR, MEMBER, or OWNER status reviews the code +3. If the code is safe, the maintainer can: + +- Approve and merge the pull request to another branch (workflows will run with the maintainer's permissions) +- Manually trigger workflow runs if needed (using workflow dispatch on the target branch) + +This approach ensures that all code from external contributors is reviewed by trusted users before execution on self-hosted runners. + +## Troubleshooting + +This section covers troubleshooting for common issues that users may encounter. + +### Workflows not running for external contributors + +If workflows are not running for external contributors when `allow-external-contributor=false`: + +1. Check the runner logs for authorization errors +2. Verify the user's author association in the GitHub repository + +### Workflows running for untrusted users + +If you notice workflows running for users who shouldn't have access: + +1. Set `allow-external-contributor=false` +2. Review repository collaborator permissions +3. Check for any bypass rules in branch protection settings +4. Audit recent workflow runs in the GitHub Actions tab diff --git a/docs/index.md b/docs/index.md index c124c4763..7e47c983f 100644 --- a/docs/index.md +++ b/docs/index.md @@ -85,7 +85,7 @@ Thinking about using the GitHub runner charm for your next project? [Get in touc 1. [Debug with SSH](local-lxd/how-to/debug-with-ssh.md) 1. [Deploy on ARM64](local-lxd/how-to/deploy-on-arm64.md) 1. [Integrate with COS](local-lxd/how-to/integrate-with-cos.md) - 1. [Comply with repository policies](local-lxd/how-to/repo-policy.md) + 1. [Manage external contributors](local-lxd/how-to/manage-external-contributors.md) 1. [Run on LXD cloud](local-lxd/how-to/run-on-lxd.md) 1. [Set base image](local-lxd/how-to/set-base-image.md) 1. [Reference](local-lxd/reference) diff --git a/docs/local-lxd/how-to/comply-security.md b/docs/local-lxd/how-to/comply-security.md index 6ce195b83..9368115d7 100644 --- a/docs/local-lxd/how-to/comply-security.md +++ b/docs/local-lxd/how-to/comply-security.md @@ -20,4 +20,4 @@ With these settings, the common workflow of creating branches with pull requests ### Working with outside collaborators -Generally, outside collaborators are not completely trusted, but still would need to contribute in some manner. As such, this charm requires pull requests by outside collaborators to be reviewed by someone with `write` permission or above. Once the review is completed, the reviewer should add a comment including the following string: `/canonical/self-hosted-runners/run-workflows `, where `` is the commit SHA of the approved commit. Once posted, the self-hosted runners will run the workflow for this commit. \ No newline at end of file +Generally, outside collaborators are not completely trusted, but still would need to contribute in some manner. As such, this charm requires pull requests by outside collaborators to be reviewed by someone with `write` permission or above. Once the review is completed, the reviewer should add a comment including the following string: `/canonical/self-hosted-runners/run-workflows `, where `` is the commit SHA of the approved commit. Once posted, the self-hosted runners will run the workflow for this commit. diff --git a/docs/local-lxd/how-to/repo-policy.md b/docs/local-lxd/how-to/repo-policy.md index 242b14861..6c076e788 100644 --- a/docs/local-lxd/how-to/repo-policy.md +++ b/docs/local-lxd/how-to/repo-policy.md @@ -18,4 +18,4 @@ With these settings, the common workflow of creating branches with pull requests ### Working with outside collaborators -Contributions from outside collaborators (in the case where a repository is public) need to be handled slightly differently. As such, this charm requires pull requests by outside collaborators to be reviewed by someone with `write` permission or above. Once the review is completed, the reviewer should add a comment including the following string: `/canonical/self-hosted-runners/run-workflows `, where `` is the commit SHA of the approved commit. Once posted, the self-hosted runners will run the workflow for this commit. \ No newline at end of file +Contributions from outside collaborators (in the case where a repository is public) need to be handled slightly differently. As such, this charm requires pull requests by outside collaborators to be reviewed by someone with `write` permission or above. Once the review is completed, the reviewer should add a comment including the following string: `/canonical/self-hosted-runners/run-workflows `, where `` is the commit SHA of the approved commit. Once posted, the self-hosted runners will run the workflow for this commit. diff --git a/docs/reference/cryptographic-overview.md b/docs/reference/cryptographic-overview.md index 773e64a94..029b600e2 100644 --- a/docs/reference/cryptographic-overview.md +++ b/docs/reference/cryptographic-overview.md @@ -28,7 +28,6 @@ The following cryptographic technologies are used internally by our product: ### TLS - Communication with GitHub is done via TLS v1.3 -- [The `repo-policy-compliance` tool](https://github.com/canonical/repo-policy-compliance) communicates with the [GitHub API](https://docs.github.com/en/rest?apiVersion=2022-11-28) using TLS v1.3. - Communication with the OpenStack API uses TLS v1.3. ### Signature verification diff --git a/github-runner-manager/pyproject.toml b/github-runner-manager/pyproject.toml index 232256c5c..c3b70e953 100644 --- a/github-runner-manager/pyproject.toml +++ b/github-runner-manager/pyproject.toml @@ -3,7 +3,7 @@ [project] name = "github-runner-manager" -version = "0.8.1" +version = "0.9.0" authors = [ { name = "Canonical IS DevOps", email = "is-devops-team@canonical.com" }, ] @@ -38,11 +38,7 @@ skips = ["*/*test.py", "*/test_*.py", "*tests/*.py"] # Testing tools configuration [tool.coverage.run] branch = true -omit = [ - # Contains interface for calling repo policy compliance service. Tested in integration test - # and end to end tests. - "src/github_runner_manager/repo_policy_compliance_client.py", -] +omit = [] [tool.coverage.report] fail_under = 73 diff --git a/github-runner-manager/src/github_runner_manager/configuration/__init__.py b/github-runner-manager/src/github_runner_manager/configuration/__init__.py index 9695cf30a..a5a01e561 100644 --- a/github-runner-manager/src/github_runner_manager/configuration/__init__.py +++ b/github-runner-manager/src/github_runner_manager/configuration/__init__.py @@ -12,7 +12,6 @@ ProxyConfig, QueueConfig, ReactiveConfiguration, - RepoPolicyComplianceConfig, SSHDebugConnection, SupportServiceConfig, UserInfo, 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 4aaffaa78..6349ff7ad 100644 --- a/github-runner-manager/src/github_runner_manager/configuration/base.py +++ b/github-runner-manager/src/github_runner_manager/configuration/base.py @@ -89,7 +89,6 @@ class SupportServiceConfig(BaseModel): aproxy_redirect_ports: A list of ports to redirect to the aproxy proxy. dockerhub_mirror: The dockerhub mirror to use for runners. ssh_debug_connections: The information on the ssh debug services. - repo_policy_compliance: The configuration of the repo policy compliance service. custom_pre_job_script: The custom pre-job script to run before the job. """ @@ -101,7 +100,6 @@ class SupportServiceConfig(BaseModel): aproxy_redirect_ports: list[str] = [] dockerhub_mirror: str | None ssh_debug_connections: "list[SSHDebugConnection]" - repo_policy_compliance: "RepoPolicyComplianceConfig | None" custom_pre_job_script: str | None @root_validator(pre=False, skip_on_failure=True) @@ -195,18 +193,6 @@ class SSHDebugConnection(BaseModel): local_proxy_port: int = 3129 -class RepoPolicyComplianceConfig(BaseModel): - """Configuration for the repo policy compliance service. - - Attributes: - token: Token for the repo policy compliance service. - url: URL of the repo policy compliance service. - """ - - token: str - url: AnyHttpUrl - - class NonReactiveConfiguration(BaseModel): """Configuration for non-reactive mode. diff --git a/github-runner-manager/src/github_runner_manager/manager/vm_manager.py b/github-runner-manager/src/github_runner_manager/manager/vm_manager.py index 22b8db57f..4a2ff27e5 100644 --- a/github-runner-manager/src/github_runner_manager/manager/vm_manager.py +++ b/github-runner-manager/src/github_runner_manager/manager/vm_manager.py @@ -147,12 +147,12 @@ class PostJobStatus(str, Enum): Attributes: NORMAL: Represents a normal post-job. ABNORMAL: Represents an error with post-job. - REPO_POLICY_CHECK_FAILURE: Represents an error with repo-policy-compliance check. + EXTERNAL_CONTRIBUTOR_CHECK_FAILURE: Represents a failure in external contributor check. """ NORMAL = "normal" ABNORMAL = "abnormal" - REPO_POLICY_CHECK_FAILURE = "repo-policy-check-failure" + EXTERNAL_CONTRIBUTOR_CHECK_FAILURE = "external-contributor-check-failure" class CodeInformation(BaseModel): 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 6b26e3c17..8c5ca7656 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 @@ -25,7 +25,6 @@ ) from github_runner_manager.openstack_cloud.models import OpenStackRunnerManagerConfig from github_runner_manager.openstack_cloud.openstack_cloud import OpenstackCloud, OpenstackInstance -from github_runner_manager.repo_policy_compliance_client import RepoPolicyComplianceClient from github_runner_manager.utilities import set_env_var logger = logging.getLogger(__name__) @@ -184,15 +183,6 @@ def _generate_cloud_init(self, runner_context: RunnerContext) -> str: "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: - pre_job_contents_dict.update( - { - "repo_policy_base_url": repo_policy.base_url, - "repo_policy_one_time_token": repo_policy.get_one_time_token(), - "do_repo_policy_check": True, - } - ) pre_job_contents = jinja.get_template("pre-job.j2").render(pre_job_contents_dict) @@ -222,19 +212,6 @@ def _generate_cloud_init(self, runner_context: RunnerContext) -> str: runner_proxy_config=service_config.runner_proxy_config, ) - def _get_repo_policy_compliance_client(self) -> RepoPolicyComplianceClient | None: - """Get repo policy compliance client. - - Returns: - The repo policy compliance client. - """ - if (service_config := self._config.service_config).repo_policy_compliance is not None: - return RepoPolicyComplianceClient( - service_config.repo_policy_compliance.url, - service_config.repo_policy_compliance.token, - ) - return None - def delete_vms( self, instance_ids: Sequence[InstanceID], wait: bool = False, timeout: int = 60 * 10 ) -> list[InstanceID]: diff --git a/github-runner-manager/src/github_runner_manager/platform/platform_provider.py b/github-runner-manager/src/github_runner_manager/platform/platform_provider.py index a7192a108..439299206 100644 --- a/github-runner-manager/src/github_runner_manager/platform/platform_provider.py +++ b/github-runner-manager/src/github_runner_manager/platform/platform_provider.py @@ -191,8 +191,7 @@ class PlatformRunnerState(str, Enum): Attributes: BUSY: Runner is working on a job assigned. - IDLE: Runner is waiting to take a job or is running pre-job tasks (i.e. - repo-policy-compliance check). + IDLE: Runner is waiting to take a job or is running pre-job tasks. OFFLINE: Runner is not connected. """ 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 deleted file mode 100644 index 71fe7d1ec..000000000 --- a/github-runner-manager/src/github_runner_manager/repo_policy_compliance_client.py +++ /dev/null @@ -1,86 +0,0 @@ -# Copyright 2025 Canonical Ltd. -# See LICENSE file for licensing details. - -"""Client for requesting repo policy compliance service.""" - -import logging -from urllib.parse import urljoin - -import requests -import urllib3 - -from github_runner_manager.errors import RunnerCreateError - -logger = logging.getLogger(__name__) - -# Timeout for repo policy compliance service API calls in seconds -REPO_POLICY_COMPLIANCE_TIMEOUT = 5 * 60 - - -# Disable pylint public method number check as this class can be extended in the future. -class RepoPolicyComplianceClient: # pylint: disable=too-few-public-methods - """Client for repo policy compliance service. - - Attributes: - base_url: Base url to the repo policy compliance service. - token: Charm token configured for the repo policy compliance service. - """ - - def __init__(self, url: str, charm_token: str) -> None: - """Construct the RepoPolicyComplianceClient. - - Args: - 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 - - def get_one_time_token(self) -> str: - """Get a single-use token for repo policy compliance check. - - Raises: - RunnerCreateError: If there was an error getting one-time token from \ - repo-policy-compliance service. - - Returns: - The one-time token to be used in a single request of repo policy compliance check. - """ - url = urljoin(self.base_url, "one-time-token") - try: - response = self._session.get( - url, - headers={"Authorization": f"Bearer {self.token}"}, - timeout=REPO_POLICY_COMPLIANCE_TIMEOUT, - ) - response.raise_for_status() - return response.content.decode("utf-8") - except requests.RequestException as exc: - logger.exception("Unable to get one time token from repo policy compliance service.") - raise RunnerCreateError from exc - - def _create_session(self) -> requests.Session: - """Create a new requests session. - - Returns: - A new requests session with retries and no proxy settings. - """ - # The repo policy compliance service might be on localhost and should not have any proxies - # setting configured. This can be changed in the future when we also rely on an - # external service for LXD cloud. - adapter = requests.adapters.HTTPAdapter( - max_retries=urllib3.Retry( - total=3, backoff_factor=0.3, status_forcelist=[500, 502, 503, 504] - ) - ) - - session = requests.Session() - session.mount("http://", adapter) - session.mount("https://", adapter) - session.trust_env = False - return session 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 d2a7e6edb..81772a41c 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 @@ -21,118 +21,6 @@ jq -n \ }' > "{{ metrics_exchange_path }}/pre-job-metrics.json" || true {% 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}, \ - GITHUB_REPOSITORY: ${GITHUB_REPOSITORY}, \ - GITHUB_SHA: ${GITHUB_SHA}" - - # Prepare curl arguments - CURL_ARGS=( - --silent - --show-error - --max-time 60 - --noproxy '*' - --fail-with-body - -o repo_check_output.txt - --stderr repo_check_error.txt - --write-out "%{http_code}" - -H 'Authorization: Bearer {{repo_policy_one_time_token}}' - -H 'Content-Type: application/json' - ) - - # Set REPO_CHECK to a failure code as a safe guard. - REPO_CHECK=1 - - # Special Workflow dispatch repo-policy-compliance service check designed to fail: - if [[ "${GITHUB_WORKFLOW}" == "Workflow Dispatch Failure Tests 2a34f8b1-41e4-4bcb-9bbf-7a74e6c482f7" ]]; then - logger -s "Running the test workflow for integration tests, this test is configured to fail" - - REPO_CHECK_HTTP_CODE=$(curl "${CURL_ARGS[@]}" \ - -X POST \ - {{repo_policy_base_url}}/always-fail/check-run) - REPO_CHECK=$? - - # Pull request - Request repo-policy-compliance service check: - elif [[ "${GITHUB_EVENT_NAME}" == "pull_request" ]]; then - - GITHUB_SOURCE_REPOSITORY=$(cat "${GITHUB_EVENT_PATH}" | jq -r '.pull_request.head.repo.full_name') - COMMIT_SHA=$(cat "${GITHUB_EVENT_PATH}" | jq -r '.pull_request.head.sha') - - logger -s " \ - GITHUB_SOURCE_REPOSITORY: ${GITHUB_SOURCE_REPOSITORY} \ - GITHUB_BASE_REF: ${GITHUB_BASE_REF}, \ - GITHUB_HEAD_REF: ${GITHUB_HEAD_REF}, \ - COMMIT_SHA: ${COMMIT_SHA}" - - REPO_CHECK_HTTP_CODE=$(curl "${CURL_ARGS[@]}" \ - -d "{\"repository_name\": \"${GITHUB_REPOSITORY}\", \"source_repository_name\": \"${GITHUB_SOURCE_REPOSITORY}\", \"target_branch_name\": \"${GITHUB_BASE_REF}\", \"source_branch_name\": \"${GITHUB_HEAD_REF}\", \"commit_sha\": \"${COMMIT_SHA}\"}" \ - {{repo_policy_base_url}}/pull_request/check-run) - REPO_CHECK=$? - - else - # Workflow dispatch, Push and Schedule use their respective endpoints, all other events use default by default. - CHECK_NAME="default" - if [[ "${GITHUB_EVENT_NAME}" == "workflow_dispatch" ]] || [[ "${GITHUB_EVENT_NAME}" == "push" ]] || [[ "${GITHUB_EVENT_NAME}" == "schedule" ]]; then - CHECK_NAME="${GITHUB_EVENT_NAME}" - fi - - logger -s "GITHUB_REF_NAME: ${GITHUB_REF_NAME}" - - REPO_CHECK_HTTP_CODE=$(curl "${CURL_ARGS[@]}" \ - -d "{\"repository_name\": \"${GITHUB_REPOSITORY}\"}" \ - {{repo_policy_base_url}}/${CHECK_NAME}/check-run) - REPO_CHECK=$? - - fi - - if [[ $REPO_CHECK -ne 0 ]]; then - if [[ -s repo_check_output.txt ]]; then - logger -p user.error -s -f repo_check_output.txt - fi - if [[ -s repo_check_error.txt ]]; then - logger -p user.error -s -f repo_check_error.txt - fi - if [[ $REPO_CHECK_HTTP_CODE -ge 500 ]] && [[ $REPO_CHECK_HTTP_CODE -lt 600 ]]; then - logger -p user.error -s "The repository setup check failed with HTTP code ${REPO_CHECK_HTTP_CODE}." - logger -p user.error -s "This runner will be stopped or lost, please contact the repo-policy-compliance server operators or try again later." - else - logger -p user.error -s "Stopping execution of jobs due to repository setup is not compliant with policies." - logger -p user.error -s "This runner will be stopped or lost, please fix the setup of the repository, then rerun this job." - fi - - # Killing the runner.Listener process to stop the runner application. This will prevent jobs from being executed. - pkill -2 Runner.Listener - - - {% if issue_metrics %} - # Write Post Job metrics with status "repo-policy-check-failure" . - # We write it here, rather than relying on the post-job script, - # as it may not run due to the poweroff command below. - post_job_timestamp=$(date +%s) - - jq -n \ - --argjson timestamp "$post_job_timestamp" \ - --argjson http_code "$REPO_CHECK_HTTP_CODE" \ - '{ - "timestamp": $timestamp, - "status": "repo-policy-check-failure", - "status_info": {code: $http_code} - }' > "{{ metrics_exchange_path }}/post-job-metrics.json" || true - {% endif %} - - # Shutdown the instance as a safe guard. The time delay is needed for the runner application to upload the logs. - bash -c "sleep 10; sudo systemctl poweroff -i" & - - exit 1 - - fi - - 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 @@ -173,7 +61,7 @@ jq -n \ if [[ "$is_allowed" == "false" ]]; then logger -p user.error -s "Insufficient user authorization - only OWNER, MEMBER, or COLLABORATOR may run workflows" - exit 1 + return 1 fi return 0 fi @@ -211,7 +99,7 @@ jq -n \ if [[ "$is_allowed" == "false" ]]; then logger -p user.error -s "Insufficient user authorization - only OWNER, MEMBER, or COLLABORATOR may run workflows" - exit 1 + return 1 fi return 0 @@ -219,6 +107,16 @@ jq -n \ # Perform the author association check check_author_association "$GITHUB_EVENT_NAME" "$GITHUB_EVENT_PATH" + if [[ $? -ne 0 ]]; then + logger -s "Contributor check failed - aborting job execution" + jq -n \ + --argjson timestamp "$timestamp" \ + '{ + "timestamp": $timestamp, + "status": "external-contributor-check-failure" + }' > "{{ metrics_exchange_path }}/post-job-metrics.json" || true + exit 1 + fi logger -s "Contributor check passed - proceeding to execute jobs" {% endif %} 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 d1b647293..185b510fd 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 @@ -43,7 +43,6 @@ def openstack_runner_manager_fixture( service_config_mock.runner_proxy_config = None service_config_mock.use_aproxy = False service_config_mock.ssh_debug_connections = [] - service_config_mock.repo_policy_compliance = None config = OpenStackRunnerManagerConfig( allow_external_contributor=False, prefix="test", diff --git a/github-runner-manager/tests/unit/test_config.py b/github-runner-manager/tests/unit/test_config.py index 595724f01..e4b6b48e0 100644 --- a/github-runner-manager/tests/unit/test_config.py +++ b/github-runner-manager/tests/unit/test_config.py @@ -23,7 +23,6 @@ ProxyConfig, QueueConfig, ReactiveConfiguration, - RepoPolicyComplianceConfig, SSHDebugConnection, SupportServiceConfig, ) @@ -79,9 +78,6 @@ https: http://httpsrunnerproxy.example.com:3128 no_proxy: 127.0.0.1 use_aproxy: false - repo_policy_compliance: - token: token - url: https://compliance.example.com ssh_debug_connections: - ed25519_fingerprint: SHA256:ed25519 host: 10.10.10.10 @@ -132,10 +128,6 @@ def app_config_fixture() -> ApplicationConfiguration: ed25519_fingerprint="SHA256:ed25519", ) ], - repo_policy_compliance=RepoPolicyComplianceConfig( - token="token", - url="https://compliance.example.com", # type: ignore - ), ), non_reactive_configuration=NonReactiveConfiguration( combinations=[ diff --git a/github-runner-manager/tests/unit/test_runner_scaler.py b/github-runner-manager/tests/unit/test_runner_scaler.py index 8198980cb..fac0b773c 100644 --- a/github-runner-manager/tests/unit/test_runner_scaler.py +++ b/github-runner-manager/tests/unit/test_runner_scaler.py @@ -19,7 +19,6 @@ ProxyConfig, QueueConfig, ReactiveConfiguration, - RepoPolicyComplianceConfig, SSHDebugConnection, SupportServiceConfig, UserInfo, @@ -147,10 +146,6 @@ def application_configuration_fixture() -> ApplicationConfiguration: ed25519_fingerprint="SHA256:ed25519", ) ], - repo_policy_compliance=RepoPolicyComplianceConfig( - token="token", - url="https://compliance.example.com", - ), ), non_reactive_configuration=NonReactiveConfiguration( combinations=[ @@ -328,10 +323,6 @@ def test_build_runner_scaler( ed25519_fingerprint="SHA256:ed25519", ) ], - repo_policy_compliance=RepoPolicyComplianceConfig( - token="token", - url="https://compliance.example.com", - ), ), ) reactive_process_config = runner_scaler._reactive_config @@ -376,10 +367,6 @@ def test_build_runner_scaler( ed25519_fingerprint="SHA256:ed25519", ) ], - repo_policy_compliance=RepoPolicyComplianceConfig( - token="token", - url="https://compliance.example.com", - ), ), ), github_token="githubtoken", diff --git a/pyproject.toml b/pyproject.toml index e5121fb41..afa95a440 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,11 +9,7 @@ skips = ["*/*test.py", "*/test_*.py", "*tests/*.py"] # Testing tools configuration [tool.coverage.run] branch = true -omit = [ - # Contains interface for calling repo policy compliance service. Tested in integration test - # and end to end tests. - "src/repo_policy_compliance_client.py", -] +omit = [] [tool.coverage.report] fail_under = 83 diff --git a/src/charm_state.py b/src/charm_state.py index ede55392a..23b48642f 100644 --- a/src/charm_state.py +++ b/src/charm_state.py @@ -16,14 +16,7 @@ from github_runner_manager.configuration import ProxyConfig, SSHDebugConnection from github_runner_manager.configuration.github import GitHubPath, parse_github_path from ops import CharmBase -from pydantic import ( - AnyHttpUrl, - BaseModel, - MongoDsn, - ValidationError, - create_model_from_typeddict, - validator, -) +from pydantic import BaseModel, MongoDsn, ValidationError, create_model_from_typeddict, validator from errors import MissingMongoDBError from models import AnyHttpsUrl, FlavorLabel, OpenStackCloudsYAML @@ -49,9 +42,6 @@ OPENSTACK_FLAVOR_CONFIG_NAME = "openstack-flavor" PATH_CONFIG_NAME = "path" RECONCILE_INTERVAL_CONFIG_NAME = "reconcile-interval" -# bandit thinks this is a hardcoded password -REPO_POLICY_COMPLIANCE_TOKEN_CONFIG_NAME = "repo-policy-compliance-token" # nosec -REPO_POLICY_COMPLIANCE_URL_CONFIG_NAME = "repo-policy-compliance-url" RUNNER_HTTP_PROXY_CONFIG_NAME = "runner-http-proxy" SENSITIVE_PLACEHOLDER = "*****" TEST_MODE_CONFIG_NAME = "test-mode" @@ -166,52 +156,6 @@ def _parse_labels(labels: str) -> tuple[str, ...]: return tuple(valid_labels) -class RepoPolicyComplianceConfig(BaseModel): - """Configuration for the repo policy compliance service. - - Attributes: - token: Token for the repo policy compliance service. - url: URL of the repo policy compliance service. - """ - - token: str - url: AnyHttpUrl - - @classmethod - def from_charm(cls, charm: CharmBase) -> "RepoPolicyComplianceConfig": - """Initialize the config from charm. - - Args: - charm: The charm instance. - - Raises: - CharmConfigInvalidError: If an invalid configuration was set. - - Returns: - Current repo-policy-compliance config. - """ - token = charm.config.get(REPO_POLICY_COMPLIANCE_TOKEN_CONFIG_NAME) - if not token: - raise CharmConfigInvalidError( - f"Missing {REPO_POLICY_COMPLIANCE_TOKEN_CONFIG_NAME} configuration" - ) - url = charm.config.get(REPO_POLICY_COMPLIANCE_URL_CONFIG_NAME) - if not url: - raise CharmConfigInvalidError( - 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 - - class CharmConfig(BaseModel): """General charm configuration. @@ -227,7 +171,6 @@ class CharmConfig(BaseModel): path: GitHub repository path in the format '/', or the GitHub organization name. reconcile_interval: Time between each reconciliation of runners in minutes. - repo_policy_compliance: Configuration for the repo policy compliance service. token: GitHub personal access token for GitHub API. manager_proxy_command: ProxyCommand for the SSH connection from the manager to the runner. use_aproxy: Whether to use aproxy in the runner. @@ -243,7 +186,6 @@ class CharmConfig(BaseModel): openstack_clouds_yaml: OpenStackCloudsYAML path: GitHubPath | None reconcile_interval: int - repo_policy_compliance: RepoPolicyComplianceConfig | None token: str | None manager_proxy_command: str | None use_aproxy: bool @@ -500,16 +442,6 @@ def from_charm(cls, charm: CharmBase) -> "CharmConfig": except ValueError as exc: raise CharmConfigInvalidError(f"Invalid {LABELS_CONFIG_NAME} config: {exc}") from exc - repo_policy_compliance = None - if charm.config.get(REPO_POLICY_COMPLIANCE_TOKEN_CONFIG_NAME) or charm.config.get( - REPO_POLICY_COMPLIANCE_URL_CONFIG_NAME - ): - if not openstack_clouds_yaml: - raise CharmConfigInvalidError( - "Cannot use repo-policy-compliance config without using OpenStack." - ) - repo_policy_compliance = RepoPolicyComplianceConfig.from_charm(charm) - manager_proxy_command = ( cast(str, charm.config.get(MANAGER_SSH_PROXY_COMMAND_CONFIG_NAME, "")) or None ) @@ -532,7 +464,6 @@ def from_charm(cls, charm: CharmBase) -> "CharmConfig": openstack_clouds_yaml=openstack_clouds_yaml, path=github_config.path if github_config else None, reconcile_interval=reconcile_interval, - repo_policy_compliance=repo_policy_compliance, token=github_config.token if github_config else None, manager_proxy_command=manager_proxy_command, use_aproxy=use_aproxy, diff --git a/src/factories.py b/src/factories.py index 0a14c4fc4..996904cb8 100644 --- a/src/factories.py +++ b/src/factories.py @@ -78,7 +78,6 @@ def create_application_configuration( runner_proxy_config=state.runner_proxy_config, dockerhub_mirror=state.charm_config.dockerhub_mirror, ssh_debug_connections=state.ssh_debug_connections, - repo_policy_compliance=state.charm_config.repo_policy_compliance, use_aproxy=state.charm_config.use_aproxy, aproxy_exclude_addresses=state.charm_config.aproxy_exclude_addresses, aproxy_redirect_ports=state.charm_config.aproxy_redirect_ports, diff --git a/tests/integration/helpers/charm_metrics.py b/tests/integration/helpers/charm_metrics.py index 574c6cfd8..1109bc134 100644 --- a/tests/integration/helpers/charm_metrics.py +++ b/tests/integration/helpers/charm_metrics.py @@ -186,9 +186,6 @@ async def assert_events_after_reconciliation( None, JobConclusion.CANCELLED, ] - elif post_job_status == PostJobStatus.REPO_POLICY_CHECK_FAILURE: - assert metric_log.get("status_info", {}).get("code", 0) == 403 - assert metric_log.get("job_conclusion") == JobConclusion.FAILURE else: assert "status_info" not in metric_log assert metric_log.get("job_conclusion") == JobConclusion.SUCCESS diff --git a/tests/integration/helpers/openstack.py b/tests/integration/helpers/openstack.py index a4be0073a..10b97fdd5 100644 --- a/tests/integration/helpers/openstack.py +++ b/tests/integration/helpers/openstack.py @@ -1,9 +1,8 @@ # Copyright 2025 Canonical Ltd. # See LICENSE file for licensing details. import logging -import secrets from asyncio import sleep -from typing import Optional, TypedDict +from typing import TypedDict import openstack.connection from github_runner_manager import constants @@ -12,7 +11,7 @@ from openstack.compute.v2.server import Server from charm_state import BASE_VIRTUAL_MACHINES_CONFIG_NAME -from tests.integration.helpers.common import run_in_unit, wait_for, wait_for_runner_ready +from tests.integration.helpers.common import run_in_unit, wait_for_runner_ready logger = logging.getLogger(__name__) @@ -215,141 +214,6 @@ def get_single_runner(self, unit: Unit) -> Server: return runners[0] -async def setup_repo_policy( - app: Application, - openstack_connection: openstack.connection.Connection, - token: str, - https_proxy: Optional[str], -) -> None: - """Setup the repo policy compliance service for one runner. - - Does also setup a runner if one is not present. - - Args: - app: The GitHub Runner Charm app to create the runner for. - openstack_connection: OpenStack connection object. - token: GitHub token. - https_proxy: HTTPS proxy url to use. - """ - unit = app.units[0] - charm_token = secrets.token_hex(16) - await _install_repo_policy( - unit=unit, github_token=token, charm_token=charm_token, https_proxy=https_proxy - ) - instance_helper = OpenStackInstanceHelper(openstack_connection) - - await app.expose() - unit_name_without_slash = unit.name.replace("/", "-") - await run_in_unit( - unit=unit, - command=f"/var/lib/juju/tools/unit-{unit_name_without_slash}/open-port 8080", - assert_on_failure=True, - assert_msg="Failed to open port 8080", - ) - await app.set_config( - { - "repo-policy-compliance-token": charm_token, - "repo-policy-compliance-url": "http://localhost:8080", - } - ) - - await instance_helper.ensure_charm_has_runner(app=app) - await instance_helper.expose_to_instance(unit, 8080) - # This tests the connection to the repo policy compliance, not a health check of service. - await instance_helper.run_in_instance( - unit=unit, - command="curl http://localhost:8080", - assert_on_failure=True, - assert_msg="Unable to reach the repo policy compliance server setup", - ) - - -async def _install_repo_policy( - unit: Unit, github_token: str, charm_token: str, https_proxy: Optional[str] -): - """Start the repo policy compliance service. - - Args: - unit: Unit instance to check for the profile. - github_token: GitHub token to use in the repo-policy service. - charm_token: Charm token to use in the repo-policy service. - https_proxy: HTTPS proxy url to use. - """ - await run_in_unit( - unit, - "apt install -y python3-pip", - assert_on_failure=True, - assert_msg="Failed to install python3-pip", - ) - await run_in_unit( - unit, - "rm -rf /home/ubuntu/repo_policy_compliance", - assert_on_failure=True, - assert_msg="Failed to remove repo-policy-compliance", - ) - await run_in_unit( - unit, - f'sudo -u ubuntu HTTPS_PROXY={https_proxy if https_proxy else ""} git clone https://github.com/canonical/repo-policy-compliance.git /home/ubuntu/repo_policy_compliance', - assert_on_failure=True, - assert_msg="Failed to clone repo-policy-compliance", - ) - await run_in_unit( - unit, - f'sudo -u ubuntu HTTPS_PROXY={https_proxy if https_proxy else ""} pip install {f"--proxy {https_proxy}" if https_proxy else ""} -r /home/ubuntu/repo_policy_compliance/requirements.txt', - assert_on_failure=True, - assert_msg="Failed to install repo-policy-compliance requirements", - ) - await run_in_unit( - unit=unit, - command=f"HTTPS_PROXY={https_proxy if https_proxy else ''} python3 -m pip install gunicorn", - assert_on_failure=True, - assert_msg="Failed to install gunicorn", - ) - await run_in_unit( - unit, - f"""cat < /etc/systemd/system/repo-policy-compliance.service -[Unit] -Description=Simple HTTP server for testing -After=network.target - -[Service] -User=ubuntu -Group=www-data -Environment="GITHUB_TOKEN={github_token}" -Environment="CHARM_TOKEN={charm_token}" -Environment="HTTPS_PROXY={https_proxy if https_proxy else ""}" -Environment="https_proxy={https_proxy if https_proxy else ""}" -WorkingDirectory=/home/ubuntu/repo_policy_compliance -ExecStart=/usr/local/bin/gunicorn --bind 0.0.0.0:8080 --timeout 60 app:app -EOT""", - assert_on_failure=True, - assert_msg="Failed to create service file", - ) - await run_in_unit( - unit, - "/usr/bin/systemctl daemon-reload", - assert_on_failure=True, - assert_msg="Failed to reload systemd", - ) - await run_in_unit( - unit, - "/usr/bin/systemctl restart repo-policy-compliance", - assert_on_failure=True, - assert_msg="Failed to restart service", - ) - - async def server_is_ready() -> bool: - """Check if the server is ready. - - Returns: - Whether the server is ready. - """ - return_code, stdout, _ = await run_in_unit(unit, "curl http://localhost:8080") - return return_code == 0 and bool(stdout) - - await wait_for(server_is_ready, timeout=30, check_interval=3) - - class PrivateEndpointConfigs(TypedDict): """The Private endpoint configuration values. diff --git a/tests/integration/test_charm_fork_repo.py b/tests/integration/test_charm_fork_repo.py deleted file mode 100644 index ececde782..000000000 --- a/tests/integration/test_charm_fork_repo.py +++ /dev/null @@ -1,83 +0,0 @@ -# Copyright 2025 Canonical Ltd. -# See LICENSE file for licensing details. - -"""Integration tests for github-runner charm with a fork repo. - -The forked repo is configured to fail the repo-policy-compliance check. -""" - -from datetime import datetime, timezone - -import pytest -import requests -from github.Branch import Branch -from github.Repository import Repository -from juju.application import Application -from juju.model import Model - -from tests.integration.conftest import GitHubConfig, ProxyConfig -from tests.integration.helpers.common import ( - DISPATCH_FAILURE_TEST_WORKFLOW_FILENAME, - dispatch_workflow, -) -from tests.integration.helpers.openstack import OpenStackInstanceHelper, setup_repo_policy - - -@pytest.mark.openstack -@pytest.mark.asyncio -@pytest.mark.abort_on_fail -async def test_dispatch_workflow_failure( - model: Model, - app_with_forked_repo: Application, - forked_github_repository: Repository, - forked_github_branch: Branch, - instance_helper: OpenStackInstanceHelper, - github_config: GitHubConfig, - proxy_config: ProxyConfig, -) -> None: - """ - arrange: \ - 1. A forked repository. \ - 2. A working application using repo-policy checks with one runner on the forked repository. - act: Trigger a workflow dispatch that fails the repo policy check on a branch - in the forked repository. - assert: The workflow that was dispatched failed and the reason is logged. - """ - start_time = datetime.now(timezone.utc) - - await setup_repo_policy( - app=app_with_forked_repo, - openstack_connection=instance_helper.openstack_connection, - token=github_config.token, - https_proxy=proxy_config.https_proxy, - ) - - workflow = forked_github_repository.get_workflow( - id_or_file_name=DISPATCH_FAILURE_TEST_WORKFLOW_FILENAME - ) - - await dispatch_workflow( - app=app_with_forked_repo, - workflow_id_or_name=DISPATCH_FAILURE_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 ( - "Stopping execution of jobs due to repository setup is not compliant with policies" - in logs - ) - assert "Endpoint designed for testing that always fails" in logs - assert "Should not echo if pre-job script failed" not in logs diff --git a/tests/integration/test_charm_metrics_failure.py b/tests/integration/test_charm_metrics_failure.py index a020a7e60..a0ab0c59a 100644 --- a/tests/integration/test_charm_metrics_failure.py +++ b/tests/integration/test_charm_metrics_failure.py @@ -14,7 +14,6 @@ from juju.application import Application from charm_state import BASE_VIRTUAL_MACHINES_CONFIG_NAME, PATH_CONFIG_NAME -from tests.integration.conftest import GitHubConfig, ProxyConfig from tests.integration.helpers.charm_metrics import ( assert_events_after_reconciliation, cancel_workflow_run, @@ -24,11 +23,9 @@ ) from tests.integration.helpers.common import ( DISPATCH_CRASH_TEST_WORKFLOW_FILENAME, - DISPATCH_FAILURE_TEST_WORKFLOW_FILENAME, - dispatch_workflow, wait_for_reconcile, ) -from tests.integration.helpers.openstack import OpenStackInstanceHelper, setup_repo_policy +from tests.integration.helpers.openstack import OpenStackInstanceHelper @pytest_asyncio.fixture(scope="function", name="app") @@ -42,8 +39,6 @@ async def app_fixture(app_for_metric: Application) -> AsyncIterator[Application] await app_for_metric.set_config( { BASE_VIRTUAL_MACHINES_CONFIG_NAME: "0", - "repo-policy-compliance-token": "", - "repo-policy-compliance-url": "", } ) await wait_for_reconcile(app=app_for_metric) @@ -51,58 +46,6 @@ async def app_fixture(app_for_metric: Application) -> AsyncIterator[Application] yield app_for_metric -@pytest.mark.openstack -@pytest.mark.asyncio -@pytest.mark.abort_on_fail -async def test_charm_issues_metrics_for_failed_repo_policy( - app: Application, - forked_github_repository: Repository, - forked_github_branch: Branch, - github_config: GitHubConfig, - proxy_config: ProxyConfig, - instance_helper: OpenStackInstanceHelper, -): - """ - arrange: A properly integrated charm with a runner registered on the fork repo. - act: Dispatch a test workflow that fails the repo-policy check. After completion, reconcile. - assert: The RunnerStart, RunnerStop and Reconciliation metric is logged. - The Reconciliation metric has the post job status set to failure. - """ - await app.set_config({PATH_CONFIG_NAME: forked_github_repository.full_name}) - - await setup_repo_policy( - app=app, - openstack_connection=instance_helper.openstack_connection, - token=github_config.token, - https_proxy=proxy_config.https_proxy, - ) - - # Clear metrics log to make reconciliation event more predictable - unit = app.units[0] - await clear_metrics_log(unit) - await dispatch_workflow( - app=app, - branch=forked_github_branch, - github_repository=forked_github_repository, - conclusion="failure", - workflow_id_or_name=DISPATCH_FAILURE_TEST_WORKFLOW_FILENAME, - ) - - # Set the number of virtual machines to 0 to speedup reconciliation - await app.set_config( - { - BASE_VIRTUAL_MACHINES_CONFIG_NAME: "0", - } - ) - await wait_for_reconcile(app=app) - - await assert_events_after_reconciliation( - app=app, - github_repository=forked_github_repository, - post_job_status=PostJobStatus.REPO_POLICY_CHECK_FAILURE, - ) - - @pytest.mark.openstack @pytest.mark.asyncio @pytest.mark.abort_on_fail diff --git a/tests/integration/test_charm_no_runner.py b/tests/integration/test_charm_no_runner.py index 614b01f8b..72cf5cec3 100644 --- a/tests/integration/test_charm_no_runner.py +++ b/tests/integration/test_charm_no_runner.py @@ -24,11 +24,6 @@ logger = logging.getLogger(__name__) -REPO_POLICY_COMPLIANCE_VER_0_2_GIT_SOURCE = ( - "git+https://github.com/canonical/" - "repo-policy-compliance@48b36c130b207278d20c3847ce651ac13fb9e9d7" -) - pytestmark = pytest.mark.openstack diff --git a/tests/integration/test_charm_runner.py b/tests/integration/test_charm_runner.py index 4a178f4fb..06bdf106e 100644 --- a/tests/integration/test_charm_runner.py +++ b/tests/integration/test_charm_runner.py @@ -12,7 +12,6 @@ from juju.application import Application from charm_state import BASE_VIRTUAL_MACHINES_CONFIG_NAME, CUSTOM_PRE_JOB_SCRIPT_CONFIG_NAME -from tests.integration.conftest import GitHubConfig, ProxyConfig from tests.integration.helpers.common import ( DISPATCH_TEST_WORKFLOW_FILENAME, DISPATCH_WAIT_TEST_WORKFLOW_FILENAME, @@ -22,7 +21,7 @@ wait_for_reconcile, wait_for_runner_ready, ) -from tests.integration.helpers.openstack import OpenStackInstanceHelper, setup_repo_policy +from tests.integration.helpers.openstack import OpenStackInstanceHelper @pytest_asyncio.fixture(scope="function", name="app") @@ -173,37 +172,3 @@ async def test_custom_pre_job_script( logs = get_job_logs(workflow_run.jobs("latest")[0]) assert "SSH config" in logs assert "proxycommand socat - PROXY:squid.internal:%h:%p,proxyport=3128" in logs - - -# WARNING: the test below sets up repo policy which affects all tests coming after it. It should -# be the last test in the file. -@pytest.mark.openstack -@pytest.mark.asyncio -@pytest.mark.abort_on_fail -async def test_repo_policy_enabled( - app: Application, - github_repository: Repository, - test_github_branch: Branch, - github_config: GitHubConfig, - proxy_config: ProxyConfig, - instance_helper: OpenStackInstanceHelper, -) -> None: - """ - arrange: A working application with one runner with repo policy enabled. - act: Dispatch a workflow. - assert: Workflow run successfully passed. - """ - await setup_repo_policy( - app=app, - openstack_connection=instance_helper.openstack_connection, - token=github_config.token, - https_proxy=proxy_config.https_proxy, - ) - - await dispatch_workflow( - app=app, - branch=test_github_branch, - github_repository=github_repository, - conclusion="success", - workflow_id_or_name=DISPATCH_TEST_WORKFLOW_FILENAME, - ) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 2e9abd787..6243dc0d6 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -150,10 +150,6 @@ def complete_charm_state_fixture(): ), path=GitHubOrg(org="canonical", group="group"), reconcile_interval=5, - repo_policy_compliance=charm_state.RepoPolicyComplianceConfig( - token="token", - url="https://compliance.example.com", - ), token="githubtoken", manager_proxy_command="ssh -W %h:%p example.com", use_aproxy=True, diff --git a/tests/unit/mock.py b/tests/unit/mock.py index f8658d935..5a2269b91 100644 --- a/tests/unit/mock.py +++ b/tests/unit/mock.py @@ -203,25 +203,3 @@ def delete_self_hosted_runner_from_org(self, org: str, runner_id: str): runner_id: Placeholder for runner id. """ pass - - -class MockRepoPolicyComplianceClient: - """Mock for RepoPolicyComplianceClient.""" - - def __init__(self, session=None, url=None, charm_token=None): - """Placeholder method for initialization. - - Args: - session: Placeholder for requests session. - url: Placeholder for repo policy compliance url. - charm_token: Placeholder for charm token. - """ - pass - - def get_one_time_token(self) -> str: - """Generate a test token value. - - Returns: - A test token value. - """ - return "MOCK_TOKEN_" + secrets.token_hex(8) diff --git a/tests/unit/test_factories.py b/tests/unit/test_factories.py index 6e3d4d53e..ebea31df9 100644 --- a/tests/unit/test_factories.py +++ b/tests/unit/test_factories.py @@ -1,6 +1,6 @@ # Copyright 2025 Canonical Ltd. # See LICENSE file for licensing details. -from github_runner_manager.configuration import ( +from github_runner_manager.configuration.base import ( ApplicationConfiguration, Flavor, Image, @@ -9,7 +9,6 @@ ProxyConfig, QueueConfig, ReactiveConfiguration, - RepoPolicyComplianceConfig, SSHDebugConnection, SupportServiceConfig, ) @@ -69,10 +68,6 @@ def test_create_application_configuration( local_proxy_port=3129, ) ], - repo_policy_compliance=RepoPolicyComplianceConfig( - token="token", - url="https://compliance.example.com", - ), ), non_reactive_configuration=NonReactiveConfiguration( combinations=[