diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 87e0546..9ae25dc 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -1,7 +1,11 @@ name: Build and publish to PyPi on: - workflow_run: + # This trigger fires when any workflow with this name completes, whatever its event or head + # repository. The if: on deploy_to_pypi publishes only after a successful release-triggered + # run whose head is in this repository. The zizmor ignore covers every trigger under on:, so + # tests/unit/test_workflows.py fails when another trigger is added here. + workflow_run: # zizmor: ignore[dangerous-triggers] workflows: ["Run Integration Tests on Release"] types: - completed @@ -12,12 +16,19 @@ jobs: name: Publish to PyPI permissions: contents: read - runs-on: mdb-dev - if: github.actor != 'mindsdbadmin' + runs-on: ubuntu-latest + if: >- + github.actor != 'mindsdbadmin' && + github.event.workflow_run.conclusion == 'success' && + github.event.workflow_run.event == 'release' && + github.event.workflow_run.head_repository.full_name == github.repository steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + with: + ref: ${{ github.event.workflow_run.head_sha }} + persist-credentials: false - name: Set up Python - uses: actions/setup-python@v5.6.0 + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 with: python-version: ${{ vars.CI_PYTHON_VERSION }} - name: Install dependencies @@ -25,12 +36,13 @@ jobs: pip install -r requirements.txt pip install setuptools wheel twine - name: Clean previous builds - run: rm -rf dist/ build/ *.egg-info + run: rm -rf dist/ build/ ./*.egg-info - name: Build and publish env: TWINE_USERNAME: __token__ TWINE_PASSWORD: ${{ secrets.PYPI_PASSWORD }} run: | - # This uses the version string from __about__.py, which we checked matches the git tag above + # Builds the version in minds/__about__.py at the tested commit. Nothing compares that + # version with the release tag. python setup.py sdist twine upload dist/* diff --git a/.github/workflows/test_on_deploy.yml b/.github/workflows/test_on_deploy.yml index d010cde..9ae4438 100644 --- a/.github/workflows/test_on_deploy.yml +++ b/.github/workflows/test_on_deploy.yml @@ -6,7 +6,7 @@ on: jobs: test: - runs-on: mdb-dev + runs-on: ubuntu-latest permissions: contents: read strategy: @@ -14,9 +14,11 @@ jobs: python-version: ['3.10'] steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + with: + persist-credentials: false - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v5.6.0 + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 with: python-version: ${{ matrix.python-version }} - name: Install dependencies @@ -29,5 +31,5 @@ jobs: pytest tests/integration --disable-warnings env: PYTHONPATH: ./ - MINDS_API_KEY: ${{ secrets.MINDS_API_KEY }} - BASE_URL: 'https://mdb.ai' + MINDS_API_TOKEN: ${{ secrets.MINDS_API_KEY }} + MINDS_API_BASE_URL: 'https://mdb.ai' diff --git a/requirements_test.txt b/requirements_test.txt index 4fa21a1..e596ecb 100644 --- a/requirements_test.txt +++ b/requirements_test.txt @@ -1,3 +1,4 @@ pytest python-dotenv psycopg2-binary==2.9.10 +pyyaml diff --git a/tests/unit/test_workflows.py b/tests/unit/test_workflows.py new file mode 100644 index 0000000..43e4ba4 --- /dev/null +++ b/tests/unit/test_workflows.py @@ -0,0 +1,105 @@ +import re +from pathlib import Path + +import pytest +import yaml + +WORKFLOWS_DIR = Path(__file__).resolve().parents[2] / '.github' / 'workflows' +WORKFLOW_FILES = sorted([*WORKFLOWS_DIR.glob('*.yml'), *WORKFLOWS_DIR.glob('*.yaml')]) +RELEASE_WORKFLOWS = ['deploy.yml', 'test_on_deploy.yml'] +# GitHub matches runner labels case-insensitively. +SELF_HOSTED_RUNNER = re.compile(r'\b(mdb-dev|mdb-prod|self-hosted)\b', re.IGNORECASE) +PINNED_ACTION = re.compile(r'uses:\s+[^@\s]+@[0-9a-f]{40}\s+#\s*v\d') + + +def _load(name): + return yaml.safe_load((WORKFLOWS_DIR / name).read_text()) + + +def _integration_test_step(job): + return next(step for step in job['steps'] if 'pytest tests/integration' in step.get('run', '')) + + +@pytest.mark.parametrize('path', WORKFLOW_FILES, ids=lambda path: path.name) +def test_workflow_names_no_self_hosted_runner(path): + """GitHub recommends GitHub-hosted runners for public repositories such as this one. + + The check reads the raw file, so it also catches a label passed as a matrix value or as an + input to a reusable workflow. + """ + match = SELF_HOSTED_RUNNER.search(path.read_text()) + assert match is None, f'{path.name} names the self-hosted runner {match.group(0)!r}' + + +def test_release_tests_get_the_env_vars_the_integration_suite_reads(): + """tests/integration/config.py reads MINDS_API_BASE_URL and MINDS_API_TOKEN. + + Without MINDS_API_BASE_URL the suite fails at import, so the release run ends before any test + runs. + """ + pytest_step = _integration_test_step(_load('test_on_deploy.yml')['jobs']['test']) + assert {'MINDS_API_BASE_URL', 'MINDS_API_TOKEN'} <= set(pytest_step['env']) + + +def test_a_failing_release_test_fails_the_release_run(): + """deploy.yml publishes only after a successful release run. + + continue-on-error on the job or the step, or '|| true' after pytest, would make every release run + succeed, so a release whose tests failed would still publish. + """ + job = _load('test_on_deploy.yml')['jobs']['test'] + pytest_step = _integration_test_step(job) + assert 'continue-on-error' not in job + assert 'continue-on-error' not in pytest_step + assert '||' not in pytest_step['run'] + + +def test_publish_runs_only_after_a_successful_release_run_from_this_repository(): + """deploy.yml's workflow_run trigger fires when any workflow named "Run Integration Tests on + Release" completes, whatever its event or head repository. + + These conditions let only a successful release run from this repository publish to PyPI. + """ + condition = ' '.join(_load('deploy.yml')['jobs']['deploy_to_pypi']['if'].split()) + assert '||' not in condition + assert set(condition.split(' && ')) >= { + "github.event.workflow_run.conclusion == 'success'", + "github.event.workflow_run.event == 'release'", + "github.event.workflow_run.head_repository.full_name == github.repository", + } + + +def test_publish_has_no_trigger_besides_workflow_run(): + """The zizmor ignore on deploy.yml's workflow_run line silences dangerous-triggers for every + trigger under on:, so zizmor would stay quiet about a second one such as pull_request_target. + """ + # PyYAML reads the bare key on as the boolean True. + assert set(_load('deploy.yml')[True]) == {'workflow_run'} + + +def test_publish_builds_the_commit_the_release_tests_ran_on(): + """Without a ref, checkout under workflow_run takes the default branch's head, which can be newer + than the release.""" + steps = _load('deploy.yml')['jobs']['deploy_to_pypi']['steps'] + checkout = next(step for step in steps if step.get('uses', '').startswith('actions/checkout@')) + assert checkout.get('with', {}).get('ref') == '${{ github.event.workflow_run.head_sha }}' + + +@pytest.mark.parametrize('name', RELEASE_WORKFLOWS) +def test_release_workflows_pin_every_action_to_a_commit(name): + """These workflows run with PYPI_PASSWORD or MINDS_API_KEY. Whoever moves an action's tag decides + what that action runs, so each one is pinned to a commit. + + The check reads the raw file, because the parsed YAML drops the version comment. + """ + lines = [line.strip() for line in (WORKFLOWS_DIR / name).read_text().splitlines() if 'uses:' in line] + assert [line for line in lines if not PINNED_ACTION.search(line)] == [] + + +@pytest.mark.parametrize('name', RELEASE_WORKFLOWS) +def test_release_checkouts_keep_the_token_out_of_git_config(name): + """Without persist-credentials: false, checkout writes the job's GITHUB_TOKEN into .git/config, + where every later step, including setup.py and the integration tests, can read it.""" + steps = [step for job in _load(name)['jobs'].values() for step in job['steps']] + checkouts = [step for step in steps if step.get('uses', '').startswith('actions/checkout@')] + assert all(step.get('with', {}).get('persist-credentials') is False for step in checkouts)