Skip to content

CONTRIBUTING.md

Latest commit

 

History

History
339 lines (251 loc) · 16.5 KB

CONTRIBUTING.md

File metadata and controls

339 lines (251 loc) · 16.5 KB
Raw

Contributing

This document explains the processes and practices recommended for contributing enhancements to the codebase.

Overview

  • Generally, before developing enhancements to this code base, you should consider opening an issue explaining your use case.
  • If you would like to chat with us about your use-cases or proposed implementation, you can reach us at Canonical Charm Development Matrix public channel or Discourse.
  • All enhancements require review before being merged. Code review typically examines
    • code quality
    • test coverage

Code of conduct

When contributing, you must abide by the Ubuntu Code of Conduct.

Changelog

Please ensure that any new feature, fix, or significant change is documented by adding an entry to the CHANGELOG.md file. Use the date of the contribution as the header for new entries.

To learn more about changelog best practices, visit Keep a Changelog.

Submissions

If you want to address an issue or a bug in this project, notify in advance the people involved to avoid confusion; also, reference the issue or bug number when you submit the changes.

  • Fork our GitHub repository and add the changes to your fork, properly structuring your commits, providing detailed commit messages and signing your commits.
  • Make sure the updated project builds and runs without warnings or errors; this includes linting, documentation, code and tests.
  • Submit the changes as a pull request (PR).

Your changes will be reviewed in due time; if approved, they will be eventually merged.

AI

You are free to use any tools you want while preparing your contribution, including AI, provided that you do so lawfully and ethically.

Avoid using AI to complete issues tagged with the "good first issues" label. The purpose of these issues is to provide newcomers with opportunities to contribute to our projects and gain coding skills. Using AI to complete these tasks undermines their purpose.

We have created instructions and tools that you can provide AI while preparing your contribution: copilot-collections

While it isn't necessary to use copilot-collections while preparing your contribution, these files contain details about our quality standards and practices that will help the AI avoid common pitfalls when interacting with our projects. By using these tools, you can avoid longer review times and nitpicks.

If you choose to use AI, please disclose this information to us by indicating AI usage in the PR description (for instance, marking the checklist item about AI usage). You don't need to go into explicit details about how and where you used AI.

Avoid submitting contributions that you don't fully understand. You are responsible for the entire contribution, including the AI-assisted portions. You must be willing to engage in discussion and respond to any questions, comments, or suggestions we may have.

Signing commits

To improve contribution tracking, we use the Canonical contributor license agreement (CLA) as a legal sign-off, and we require all commits to have verified signatures.

Canonical contributor agreement

Canonical welcomes contributions to the GitHub runner Operator. Please check out our contributor agreement if you're interested in contributing to the solution.

The CLA sign-off is simple line at the end of the commit message certifying that you wrote it or have the right to commit it as an open-source contribution.

Verified signatures on commits

All commits in a pull request must have cryptographic (verified) signatures. To add signatures on your commits, follow the GitHub documentation.

Develop

For any problems with these charms, please report bugs here.

The code can be downloaded as follows:

git clone https://github.com/canonical/github-runner-operators.git

The code structure is as follows

  • internal/: Internal Go libraries for the applications
  • cmd/: Entry points for Go applications (planner, waiting-p80-report, webhook-gateway)
  • charms/: Entry point for charms (garm, garm-configurator, planner-operator, webhook-gateway-operator)

Style

The applications written in this repository are written in Go. We like to follow idomatic Go practices and community standards when writing Go code. We have added an instruction file go.instructions.md in .github/instructions.md that is used by GitHub Copilot to help you write code that follows these practices. We have added a Style Guide that you can refer to for more details.

Test

This project uses standard Go testing tools for unit tests and integration tests. You can have a look at the GitHub actions workflows in .github/workflows/ to see how the tests are run in CI.

Run unit tests using:

go test -race -v ./...

Currently, the internal database package is only covered by integration tests. You can run the database integration tests using:

POSTGRESQL_DB_CONNECT_STRING="postgres://postgres:password@localhost:5432/postgres?sslmode=disable" go test -cover -tags=integration -v ./internal/database

It assumes you have access to a Postgres server running reachable at $POSTGRESQL_DB_CONNECT_STRING. You can use docker to run a Postgres server locally:

docker run -d --rm --name postgres -e POSTGRES_PASSWORD=password -p 5432:5432 postgres:16.11

Run webhook-gateway integration tests using:

APP_WEBHOOK_SECRET_VALUE=fake APP_PORT=8080 RABBITMQ_CONNECT_STRING="amqp://guest:guest@localhost:5672/" go test -v -cover -tags=integration -race ./cmd/webhook-gateway/...

It assumes you have access to a RabbitMQ server running reachable at $RABBITMQ_CONNECT_STRING. You can use docker to run a RabbitMQ server locally:

docker run -d  --rm --name rabbitmq -p 5672:5672 -p 15672:15672 rabbitmq:4-management

Run planner integration tests using:

APP_PORT=8080 POSTGRESQL_DB_CONNECT_STRING="postgres://postgres:password@localhost:5432/postgres?sslmode=disable" RABBITMQ_CONNECT_STRING="amqp://guest:guest@localhost:5672/" go test -v -cover -tags=integration -race ./cmd/planner/...

It assumes you can reach postgres and rabbitmq servers as described above.

Test design

We intend to have unit tests for all the logic in the internal packages (located in the internal/ directory). Unit tests should test the logic in isolation using mocks/fakes for external dependencies. They should be fast to execute.

Integration tests should test the integration of various components together. There should be at least an integration test located in the main package of the application they are testing (e.g. in cmd/webhook-gateway/main_test.go for the webhook gateway application).

In addition to application integration tests, we also have charm integration tests located in the charms/tests/integration directory. These tests should test the charm deployment and its integration with other charms. These tests are usually slower than application integration tests, and should not cover application logic tests; those should be covered in the application integration test. Aim to focus the charm integration tests only on operational aspects. E.g.

  • Testing if the charm deploys correctly
  • Testing if the charm config options work as expected
  • Testing if the charm integrates correctly with other charms

Aim to keep all tests (unit, integration, charm integration)

  • Fast to execute
  • Reliable
  • Deterministic and repeatable
  • Easy to set up locally

in order to be able to have fast iterations during development.

Coverage

We require at least 85% code coverage for all internal packages. New code that lowers the current coverage should be avoided and discouraged during code reviews.

Cyclomatic complexity

We recommend keeping cyclomatic complexity of functions/methods below 10. Higher complexity leads to code that is harder to read, understand, test and maintain. There are exceptions where higher complexity is justified (e.g., validation, initialization), but those should require explicit justification using nolint directives.

Cognitive complexity

The garm and garm-configurator charms additionally enforce a cognitive complexity limit of 15 per function (SonarSource's default) over src and tests, via flake8-cognitive-complexity (CCR001) in each charm's complexity tox environment. Cognitive complexity penalises nesting and interruptions to linear flow, so it catches readability problems that the flat branch count of cyclomatic complexity misses. Exceptions require a # noqa: CCR001 comment referencing a tracking issue that lists the function for refactoring in an upcoming PR.

Charm development

The garm, planner-operator and webhook-gateway-operator charms use the 12 factor app pattern; garm-configurator is a plain ops charm. In order to build the charm-name rock, use the build-charm-name-rock.sh script.

The respective charm code is in the charms/charm-name directory.

Integration tests for the charm are in the charms/tests/integration directory.

Have a look at this tutorial for a step-by-step guide to develop a Kubernetes charm using Go.

To run the charm integration test, the charm file and rock has to be provided as input. You would need an LXD and a Kubernetes cloud to run the tests; concierge.yaml provisions Canonical Kubernetes and LXD, and bootstraps a controller on each. Ensure the Kubernetes controller is active in your Juju client before running the tests. An example run command in the root directory is as follows:

Before running webhook-gateway-integration, export the GitHub App credentials required by test_webhook_redelivery.py:

export TEST_GITHUB_APP_ID=<github-app-id>
export TEST_GITHUB_APP_INSTALLATION_ID=<github-app-installation-id>
export TEST_GITHUB_APP_PRIVATE_KEY='<pem-private-key>'
export TEST_GITHUB_PATH=<github-org/github-repo>
tox -e webhook-gateway-integration --  --charm-file ./github-runner-webhook-gateway_amd64.charm --webhook-gateway-image localhost:32000/webhook-gateway:0.1

To add the rock to a local registry at localhost:32000 — MicroK8s ships one as an addon; on Canonical Kubernetes you supply your own — use the following command:

rockcraft.skopeo copy \
  --insecure-policy --dest-tls-verify=false \
  oci-archive:webhook-gateway_0.1_amd64.rock \
  docker://localhost:32000/webhook-gateway:0.1

GARM E2E

The GARM end-to-end test (charms/tests/e2e/) exercises the full chain on an OpenStack deployment: the charm starts, the configurator delivers config, the GARM API becomes reachable, the provider authenticates to OpenStack, a VM is created from the runner image, the runner registers with GitHub, and a dispatched job runs and exits clean.

The workflow first builds the charms and rocks the suite deploys, then runs the suite through spread. The suite deploys GARM with PostgreSQL and a traefik ingress — whose load-balancer address is what makes GARM's callback and metadata URLs reachable from the tenant — waits for a runner VM to register, and dispatches .github/workflows/garm_e2e_test_run.yaml at the scale set's label; the test passes only if that job concludes successfully. The host admits no inbound traffic from the tenant, so the workflow boots a relay VM there that the host feeds over a reverse SSH tunnel, and runner VMs send their GARM traffic to it; the relay and its security group are deleted in an always-run cleanup step. Runners left on the tenant are deleted whatever the outcome, matched on the garm-controller-id GARM stamps on every server it creates.

It is triggered manually and is not a merge gate:

gh workflow run garm_e2e.yaml --ref <feature-branch>

--ref selects which branch's version of both the workflow and the test code runs, so changes to the end-to-end test can be exercised without merging them first.

Only one run happens at a time, repository-wide. The concurrency group is global and does not cancel what is already running, so a second dispatch queues behind the first rather than displacing it — expect to wait out a run in progress, which takes roughly 25 minutes when green. Serialising is deliberate: the suite assumes it owns the private-endpoint host (its Kubernetes cluster, the load-balancer address pinned to the host's own IP) and every GARM-tagged instance on the tenant, none of which is scoped per run. Two runs in parallel would contend for all of it, up to deleting each other's runner VMs.

Required secrets

Infrastructure details are secrets, not variables — endpoints, project and network names included. The runner masks secret values in the log automatically, so the workflow does not register masks for these itself.

Name Description
VAULT_ADDR Vault server address
VAULT_APPROLE_ROLE_ID Vault AppRole role ID
VAULT_APPROLE_SECRET_ID Vault AppRole secret ID
OS_AUTH_URL Keystone endpoint, e.g. https://keystone.example.com:5000/v3
OS_PROJECT_NAME OpenStack project/tenant name
OS_USER_DOMAIN_NAME OpenStack user domain name
OS_PROJECT_DOMAIN_NAME OpenStack project domain name
OS_REGION_NAME OpenStack region name
OS_NETWORK OpenStack network for runner VMs
E2E_RUNNER_IMAGE_NAME Name of the published runner image the VMs boot from; must exist on the tenant
E2E_OPENSTACK_FLAVOR Optional. OpenStack flavor for the runner VMs; the suite falls back to m1.small
E2E_GITHUB_APP_ID GitHub App ID
E2E_GITHUB_APP_INSTALLATION_ID Installation ID of that App on this repository
E2E_GITHUB_APP_PRIVATE_KEY That App's private key. Paste the PEM as issued; base64 is also accepted
E2E_RUNNER_HTTP_PROXY Optional. Proxy the runner VMs' egress is routed through
E2E_APROXY_EXCLUDE_ADDRESSES Optional. Addresses excluded from the aproxy redirect when a proxy is set; the suite falls back to 10.150.0.0/15
E2E_VAULT_KV_PATH Optional. Defaults to kv/data/garm-e2e/prodstack

The OpenStack username and password are not repository secrets. They are read at run time from the Vault KV v2 secret above, which must hold username and password.

The E2E_GITHUB_APP_* trio is a GitHub App of its own, distinct from the TEST_GITHUB_APP_* one the integration suite uses. The end-to-end test registers and tears down a runner scale set on this repository, so its App needs Administration: read & write here, which the integration App has no reason to hold.

Paste the private key into E2E_GITHUB_APP_PRIVATE_KEY exactly as GitHub issues it — the PEM, newlines and all. The workflow reduces it to a single line before it enters the two KEY=value channels that carry it to pytest, neither of which can hold a multi-line value. A value that is already base64-encoded is accepted unchanged, so an existing encoded key does not need re-entering.

Credential hygiene

The test authenticates against a production tenant, so the run logs are part of the contract:

  • Values read from Vault are ::add-mask::ed in the same step that reads them, before any later step runs. Masking registered with the runner scrubs every subsequent log line, including output from code we do not control; a secret read inside the test process gets none of that. Repository secrets are masked by the runner already.
  • They reach later steps through $GITHUB_ENV and pytest through tox's pass_env — never through command line arguments, which would expose them in ps and in pytest's header.
  • Steps handling credentials use set -euo pipefail and never set -x.
  • Assertions report the name of a missing setting, never its value, and use pytest.fail rather than assert so that assertion rewriting cannot introspect os.environ into the failure output.