Skip to content

STAC-25463 Add GitHub Actions check-test workflow (integrations CI migration phase 1) - #362

Merged
LouisParkin merged 13 commits into
stackstate-7.78.2from
STAC-25463-integrations-github-ci
Aug 6, 2026
Merged

STAC-25463 Add GitHub Actions check-test workflow (integrations CI migration phase 1)#362
LouisParkin merged 13 commits into
stackstate-7.78.2from
STAC-25463-integrations-github-ci

Conversation

@LouisParkin

Copy link
Copy Markdown
Contributor

Jira: https://stackstate.atlassian.net/browse/STAC-25463 (parent: STAC-25142)

First phase of moving stackstate-agent-integrations CI off GitLab. Purely additive.gitlab-ci.yml is untouched and keeps running, so this can land without a cutover.

What moved

GitLab GitHub Actions
the test_<check> job family (20 near-identical jobs) one check-tests matrix
per-job changes: rules .github/scripts/select-checks.sh (plain git diff vs merge base)
checksdev validate * riding inside test_stackstate_checks_base its own validate job
services: docker:24-dind + DOCKER_HOST: tcp://docker:2375 dropped — both public ARC runners already ship a DinD sidecar

Path filtering is done in git rather than a path-filter action because StackVista enforces a strict third-party action allowlist, and git diff against the merge base is precisely what the GitLab rule meant.

Scoped out (each reasoned in the workflow header)

  • The 5 Docker-dependent suitessplunk_{base,health,metric,topology} + stackstate_checks_dev. These are the only suites needing a Docker daemon. Phase 2; the runners already provide DinD, so it is about exercising that path, not provisioning it.
  • test_postgres — dead config. It tests a postgres check that does not exist in this repository. Dropped, not pending.
  • print_env — not ported at any phase. It is a bare printenv, and this repo is public, so it publishes every CI credential in scope to a world-readable log. Same hazard class as the sign_debian_package.sh finding on the agent.
  • No Windows lane exists to port. .gitlab-ci.yml defines a .windows_env anchor that no job has ever referenced, and Windows is not a supported target. The orphaned conda_env.ps1 / windows_*.cmd helpers can retire with the GitLab pipeline (STAC-25464).
  • Publishing + the runner-image docker job stay on GitLab until integrations gets its own publishing role (pulumi-infra section 7.4).
  • Cerberus notify — unlike the agent, this pipeline has never had a notify job, so adding one is new capability, and CERBERUS_LAMBDA_URL is a private-visibility org secret unreadable from a public repo. Follow-up.

Credentials

Phase 1 needs no new secret provisioning: REGISTRY_HOST / REGISTRY_USER / REGISTRY_PASSWORD are org-level visibility: all (STAC-25350) and already reach this public repo. Note the image ref composes vars.REGISTRY_HOST + /docker rather than using vars.REGISTRY_DOCKER_URL, which is private-visibility and not readable here.

Validation

  • actionlint clean — added .github/actionlint.yaml declaring the self-hosted *-public labels

  • zizmor clean (2 suppressed = the documented unpinned-images ignores on the container ref)

  • shellcheck clean

  • Selector exercised against real git history:

    scenario selected
    push / dispatch all 15
    PR touching only zabbix/ ["zabbix"]
    PR touching kubelet/ + vsphere/ ["kubelet","vsphere"]
    PR touching a shared library all 15
    PR touching nothing relevant [] (matrix skips)

This PR self-validates — the workflow runs against this very PR, so the image path and runner labels are proven before merge.

LouisParkin and others added 3 commits August 3, 2026 09:47
…gration phase 1)

Ports the pure-Python half of .gitlab-ci.yml to GitHub Actions as the first
phase of moving stackstate-agent-integrations off GitLab (STAC-25142).

What moved:
  * The `test_<check>` job family -> a single `check-tests` matrix.
  * The per-job `changes:` rules -> .github/scripts/select-checks.sh, which
    reproduces them with `git diff` against the merge base. Done in plain git
    because StackVista enforces a strict third-party action allowlist.
  * The `checksdev validate *` commands that rode inside
    test_stackstate_checks_base -> their own job, so a metadata failure reads
    as its own PR check.

Scoped out, with reasons recorded in the workflow header:
  * The five Docker-dependent suites (splunk x4 + stackstate_checks_dev) are
    phase 2; both public ARC runners ship a DinD sidecar, so this is about
    exercising that path, not provisioning it.
  * test_postgres is dead config - it tests a `postgres` check that does not
    exist in this repository.
  * print_env is not ported at any phase: a bare `printenv` on a PUBLIC repo
    publishes every CI credential in scope to a world-readable log.
  * There is no Windows lane to port. `.gitlab-ci.yml` defines a `.windows_env`
    anchor that no job has ever referenced, and Windows is not a supported
    target.
  * Publishing stays on GitLab until integrations gets its own publishing role
    (pulumi-infra section 7.4), so the GitLab pipeline remains live.

Phase 1 needs no new secret provisioning: REGISTRY_HOST/USER/PASSWORD are
org-level visibility=all (STAC-25350) and so already reach this PUBLIC repo.
The image reference composes vars.REGISTRY_HOST + /docker rather than using
vars.REGISTRY_DOCKER_URL, which is private-visibility and unreadable here.

Validated: actionlint clean (with a new .github/actionlint.yaml declaring the
self-hosted labels), zizmor clean, shellcheck clean, and the selector exercised
against real git history for the push, single-check, multi-check, shared-library
and no-match cases.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…plied

pulumi-infra #262 is merged and applied (~ 1 updated), so docker-public can now
pick up jobs from this repository.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The GitLab `.linux_test` anchor ran .setup-scripts/setup_artifact_registry.sh
immediately before `checksdev test`, and the port dropped it. Fourteen of the
fifteen phase-1 suites resolve everything from public PyPI and did not notice,
but the vsphere suite pins vsphere-automation-sdk==1.82.0, which public PyPI
does not carry (only a 0.0.1 placeholder), so it failed with:

  ERROR: Could not find a version that satisfies the requirement
  vsphere-automation-sdk==1.82.0 (from versions: 0.0.1)

Restores the step in the same position as GitLab had it. The script writes
~/.pip/pip.conf and a 0600 ~/.netrc, so credentials never appear in the index
URL.

The step is guarded on the credentials being present rather than run
unconditionally: only one suite actually needs the private index, so an absent
credential should not fail the fourteen that do not. When it is missing the job
emits a warning annotation naming the consequence, and the affected suite still
fails visibly on its own unresolvable pin.

Requires the pull-only grant in StackVista/pulumi-infra#263.
setup_artifactory_docker.sh stays out; it is a registry docker login that only
the deferred Docker suites need, and it comes across with them in phase 2.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@LouisParkin

Copy link
Copy Markdown
Contributor Author

LouisParkin and others added 2 commits August 4, 2026 09:06
…isioned

pulumi-infra#263 merged and applied (+ 2 created), so
GITLAB_PACKAGE_REGISTRY_PYPI_SIMPLE_URL and GITLAB_PACKAGE_REGISTRY_USER now
resolve in this repo. Expecting the vsphere suite to resolve its
vsphere-automation-sdk pin and the matrix to go fully green.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
vsphere pins vsphere-automation-sdk==1.82.0. Public PyPI serves only a 0.0.1
placeholder, and with the private index now provisioned and configured the
result is unchanged: pip still reports "from versions: 0.0.1".

The index itself is fine. setup_artifact_registry.sh runs and writes its
config, pip reports no authentication error, and the same credentials and
project are used by stackstate-agent, whose DEB build resolves against them.
So the registry is reachable and simply does not carry this package.

requirements.in says the wheel was to be built and published to
artifactory.tooling.stackstate.io, which has since been retired in favour of
the GitLab package registry. Whether the wheel was migrated, and to which
project, needs GitLab API access to settle, so it is tracked separately rather
than left failing in this PR.

Phase 1 therefore ships the fourteen suites that resolve entirely from public
PyPI. The registry step stays: it is correct, proven to work, and the deferred
Docker suites will need it.

Also drops a stale reference to Cerberus in the selector, which this pipeline
deliberately does not wire up.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@LouisParkin

Copy link
Copy Markdown
Contributor Author

@LouisLotter LouisLotter left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for one credential-exposure issue and three CI migration gaps: a stable required gate, release-tag coverage, and BCI compliance.

Comment thread .github/workflows/checks-tests.yml Outdated
Comment thread .github/workflows/checks-tests.yml
Comment thread .github/workflows/checks-tests.yml
Comment thread .github/workflows/checks-tests.yml Outdated
…he private index

The suite was deferred on the theory that vsphere-automation-sdk==1.82.0 was
unavailable. It is not: the wheel is published in the central registry (GitLab
project 71271774) at exactly the pinned version, alongside its vapi/nsx
dependencies.

The real cause is that pip never contacted the index at all. tox drops every
variable a testenv does not name in `passenv`, HOME among them, and pip then
resolves `~` from the passwd database instead of the environment. A container
job on GitHub Actions runs with HOME=/github/home, so
setup_artifact_registry.sh wrote /github/home/.pip/pip.conf while pip inside
tox read /root/.pip/pip.conf -- absent -- and quietly resolved from public PyPI
alone, which serves only a 0.0.1 placeholder for that name.

That is also why the failure carried no 401: there was no request to
authenticate. And why GitLab is unaffected: its container runs as root with
HOME=/root, so the passwd fallback lands on the same path the script wrote.

Adding HOME to the vsphere testenv's passenv is enough. Re-enables the suite,
taking phase 1 from 14 to 15, and records the trap next to both the setup step
and the passenv entry so the deferred Docker suites do not rediscover it.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@LouisParkin

Copy link
Copy Markdown
Contributor Author

… gate, tag coverage

Four findings from review, in order.

Tag coverage. GitLab's .base_integration_rules includes the release_branch
anchor (if: $CI_COMMIT_TAG), so every test job also ran on tag pipelines. The
workflow declared only push.branches and silently lost that. Adds a tags filter
using '**' rather than '*', which would skip any tag name containing a slash.
The selector already treats every non-pull_request event as a full run, so no
change was needed there.

BCI base image, which also removes the credentials. The GitLab runner image is
FROM python:3.13.14-bookworm, against the mandatory SUSE BCI policy. That image
exists mainly to carry Docker CE and docker-compose, which only the phase-2
Splunk and checks_dev suites use -- phase 1 needs a Python toolchain and nothing
else. Switches the container to a digest-pinned registry.suse.com/bci/python,
adding gcc, python313-devel and libffi-devel, which the image does not ship and
which cython and pyyaml==6.0.1 need to compile from source.

Because that registry is public, the jobs no longer need vars.REGISTRY_USER or
secrets.REGISTRY_PASSWORD. This is the substantive part of the credential
finding: every job here runs PR-authored workflow, setup and test code on a
PUBLIC repository, so a secret reachable from that code is reachable by anyone
who can open a branch. It also unblocks Dependabot, which receives no Actions
secrets and so could never have pulled the private image. What remains is the
read-only private PyPI pull, scoped to the one step that needs it and guarded so
its absence warns rather than fails. The fork boundary stays, now justified by
running unreviewed code on self-hosted runners rather than by secret exposure.

BCI ships 3.13.13 against the 3.13.14 the agent embeds. Patch releases are
bugfix-only, and the full matrix including vsphere against the private index was
verified green on 3.13.13 before making the switch.

Stable required check. The only test statuses were dynamically named matrix legs
in a job that disappears when the selector returns an empty array, which branch
protection cannot depend on. Adds a ci-success job that always runs and
aggregates the results, treating skipped as legitimate but failure and cancelled
as fatal, plus the requested Zizmor job. ci-success deliberately checks out
nothing so it remains safe on fork PRs, where it is the only way a required
check can report at all.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@LouisParkin

Copy link
Copy Markdown
Contributor Author

@LouisParkin
LouisParkin requested a review from LouisLotter August 4, 2026 11:57
@LouisParkin

Copy link
Copy Markdown
Contributor Author

Follow-up found while this PR was waiting: branch protection here is broken independently of this change

While checking what actually gates merges on this repo, I found the branch protection is in a bad state, and it makes the ci-success job requested in review load-bearing rather than cosmetic. Filed as STAC-25522 (blocked by this PR).

Verified today:

Branch Required checks Effect
master ci/gitlab/gitlab.com Permanently unmergeable
legacy-agent-support none CI gates nothing
stackstate-7.51.1 none CI gates nothing
stackstate-7.71.2 none CI gates nothing
stackstate-7.78.2 (this PR's base) none CI gates nothing

The GitLab project was archived, so nothing can ever report ci/gitlab/gitlab.com again. master's HEAD commit status is pending with zero contexts, and PR #316 shows an empty check rollup while still sitting at BLOCKED — it can never merge. This is the same failure mode STAC-25420 already fixed for stackstate-process-agent.

The release branches have the opposite problem: they require nothing, so the workflow in this PR would run and be ignored by branch protection.

The fix (STAC-25522) is to adopt this repo into pulumi-infra — it is currently absent from github/repositories/resources.yaml entirely, so today's settings are unmanaged drift — and require the aggregate CI success context, mirroring the stackstate-agent entry.

Worth stating explicitly, because it is the reason the aggregate job exists: branch protection must not require the per-suite Check tests (<suite>) contexts. That job is a matrix whose membership is selected dynamically by .github/scripts/select-checks.sh, so those context names are not stable, and requiring them would hang any PR that legitimately skips a suite. ci-success gives protection exactly one fixed name to gate on.

Sequencing: STAC-25522 must land after this PR, otherwise protection would require a check that master does not yet produce — recreating the very deadlock it is meant to fix.

@LouisLotter LouisLotter left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three follow-ups remain after the first review: private-PyPI credentials are still exposed to PR-controlled code, the aggregate gate succeeds when all substantive CI is skipped, and the direct BCI pull violates the self-hosted registry-proxy policy.

Comment thread .github/workflows/checks-tests.yml
Comment thread .github/workflows/checks-tests.yml Outdated
Comment thread .github/workflows/checks-tests.yml
Addresses the second review on #362.

Credential exposure (P1). setup_artifact_registry.sh writes the GitLab
Package Registry password to ~/.netrc, which the suite's own PR-authored
test code could then read. Scoping the env block to one step never
limited that. The suites needing the private index are now split into
check-tests-private-index, behind the private-package-index GitHub
Environment with dev-team as required reviewers and prevent_self_review
enabled, so the credential is released only after someone other than the
author approves that run. The other 14 suites no longer reference any
secret. select-checks.sh partitions the selection into `checks` and
`private_checks` to drive the split.

Aggregate gate (P1). ci-success treated every non-failure as success, so
a run where everything skipped reported green and could satisfy branch
protection with no CI executed. It now requires select-checks, validate
and workflow-security to succeed outright, and permits a matrix skip only
when the selector chose nothing -- plus, for the private-index matrix
only, when the PR is from a fork and could not have had the credential.

Runners (P2). Everything moves to GitHub-hosted runners. The phase-1
suites are pure Python and need no Docker daemon, so docker-public bought
no capability while forcing fork PRs to be excluded -- which is what made
the all-skipped green verdict reachable. Forks now run the full
credential-free matrix, and the direct BCI pull no longer crosses the
self-hosted NAT.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@LouisParkin
LouisParkin temporarily deployed to private-package-index August 5, 2026 07:47 — with GitHub Actions Inactive
@LouisParkin

Copy link
Copy Markdown
Contributor Author

Thanks @LouisLotter — both P1s were real. I've pushed 863b1cd which restructures the credential handling rather than patching around it. Details per finding:

P1 — credentials reachable by PR-controlled code ✅ fixed

You were right that the fork guard wasn't the boundary I implied. setup_artifact_registry.sh writes ~/.netrc, so scoping env: to that one step was cosmetic — every later step in the job, including the PR-authored test code we're about to execute, could read the file. A same-repo PR (the common case for us) had full access.

Fixed by making the split structural instead of procedural:

  • select-checks.sh now emits two outputs — checks (14 suites, no credentials) and private_checks (vsphere, the only suite needing the private index).
  • Those feed two separate jobs. check-tests no longer references any secret at all — the credential step is gone from it entirely.
  • check-tests-private-index carries environment: private-package-index, a protected Environment with prevent_self_review: true and dev-team as required reviewer.

So the credential is no longer reachable from a job that runs untrusted code without a human first approving that specific run. Verified live on this PR's run — current_user_can_approve=false for me as the author, and the job is sitting in waiting.

I also made the registry setup fail loudly (exit 1) instead of warn-and-continue. Previously a missing secret produced a silent pass.

Residual, and I want to be honest about it rather than overclaim: the secrets are still repo-level (GITLAB_PACKAGE_REGISTRY_USER) and org-level (GITLAB_PACKAGE_REGISTRY_READONLY_PASSWORD). A future workflow edit could reference them from an ungated job. Making them environment-scoped requires an org-level change that also affects stackstate-agent, so I've left it — but any such edit is diff-visible in this file.

P1 — aggregate gate green when everything skips ✅ fixed

Confirmed and worse than described. The loop only failed on failure/cancelled, so skipped counted as success. Combined with the fork guards that were on every job, a fork PR could satisfy branch protection having run no CI whatsoever.

ci-success now distinguishes the two meanings of "skipped":

  • select-checks, validate, workflow-security must be success. No skip is legitimate.
  • check-tests may skip only when the selector legitimately chose zero suites.
  • check-tests-private-index may additionally skip when the PR is from a fork.

Anything else fails the gate.

Related: I found this repo's branch protection currently requires the ci/gitlab/gitlab.com context, which can no longer ever report since the GitLab project was archived — master is presently unmergeable, with an empty check rollup that reads as "no checks" rather than an error. Filed as STAC-25522, to land after this PR so CI success exists as a target first. That makes this gate load-bearing, which is partly why I wanted the skip semantics correct.

P2 — BCI pulled directly rather than through the proxy 🔄 moot

This one I'd have pushed back on, but it's moot now: every job moved to ubuntu-latest. Once vsphere was the only suite needing credentials, there was no reason to keep 14 suites on self-hosted runners.

For the record on the original point — the merged precedent in stackstate-agent (build-binaries.yml:301, build-deb.yml:380) pulls BCI direct from registry.suse.com on self-hosted runners, with the comment "the BCI stages come from registry.suse.com and need no auth"; the proxy is used for the auth-requiring Docker Hub/Ubuntu stages. registry.tooling.stackstate.io appears in no GitHub Actions workflow in the org today. Happy to be corrected if that's drifted from current intent.

Also worth noting on the Dependabot sub-point: it isn't enabled on this repo, so there's nothing consuming that config today.

Runner move — it's faster

Somewhat to my surprise, GitHub-hosted beat the self-hosted lane on this workload, presumably because we weren't queuing behind other jobs:

self-hosted (prev run) ubuntu-latest (this run)
slowest suite 172s 99s
fastest suite 99s 68s

All 16 credential-free jobs green, no changes needed to the BCI container or zypper setup.

Validated with actionlint, shellcheck and Zizmor (No findings). select-checks.sh unit-tested across full-run / empty-selection / single-public / vsphere-only under set -u.


🙏 One ask: this PR's vsphere job is blocked awaiting a dev-team approval, and by design I can't approve my own run. If someone could approve the pending deployment, that'll confirm the credentialed path still works end-to-end before merge.

@LouisParkin
LouisParkin requested a review from LouisLotter August 5, 2026 07:53
LouisLotter
LouisLotter previously approved these changes Aug 5, 2026
The header claimed "releases keep running from GitLab, so the GitLab
pipeline stays live". That is no longer true: the GitLab project
(stackvista/agent/stackstate-agent-integrations, id 13169244) was
archived on 2026-07-20, so publish-checks-dev cannot run there either.
Publishing a new stackstate_checks_dev is currently impossible on any
platform, and the five unported suites have no CI at all. Say so
plainly rather than leaving a reviewer with a false premise.

Also drop the stale claim that phase 2 rides on an ARC DinD sidecar --
every job now runs on ubuntu-latest, which already provides a Docker
daemon, so phase 2 only needs a docker client in the job image.

Deferred work now carries ticket references instead of vague
"phase 2" / "a follow-up" pointers:
  STAC-25531  the five Docker-daemon suites
  STAC-25532  publish-checks-dev
  STAC-25533  Cerberus failure notification

Comments only; no behaviour change. actionlint, shellcheck and zizmor
clean, selector smoke-tested.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@LouisParkin

Copy link
Copy Markdown
Contributor Author

Follow-up commit b1715b4 — comments only, no behaviour change, but it corrects a factual claim reviewers would otherwise rely on.

The header said "releases keep running from GitLab, so the GitLab pipeline stays live." That is no longer true. The GitLab project (stackvista/agent/stackstate-agent-integrations, id 13169244) reports archived: true, last activity 2026-07-20. Archived projects are read-only, so publish-checks-dev cannot run there either.

Two consequences worth being explicit about, because they change the deferred items from "scheduled" to "currently broken":

  • the five unported suites (splunk_{base,health,metric,topology}, stackstate_checks_dev) have no CI on any platform right now;
  • there is no way to publish a new stackstate_checks_dev until publishing has a home.

Neither is caused by this PR — this PR is what restores coverage for the other 15 — but the header shouldn't imply a safety net that isn't there.

I also dropped a second stale claim: phase 2 no longer rides on an ARC DinD sidecar, since every job now runs on ubuntu-latest, which already provides a Docker daemon. Phase 2 just needs a docker client in the job image.

Deferred work now carries ticket references rather than vague pointers:

STAC-25531 the five Docker-daemon suites
STAC-25532 publish-checks-dev
STAC-25533 Cerberus failure notification

actionlint, shellcheck and Zizmor clean; selector smoke-tested (14 + 1 unchanged).

@LouisParkin

Copy link
Copy Markdown
Contributor Author

@LouisLotter apologies — you approved this at 08:19 and I knocked it straight back out at 08:37.

This base branch has dismiss_stale_reviews: true (and require_last_push_approval: true), so my follow-up push auto-dismissed your approval. That push was comments onlyb1715b4, +23/−16 across the two CI files, no behaviour change. It corrects a claim in the header that I'd discovered was false: the header said "releases keep running from GitLab, so the GitLab pipeline stays live", but the GitLab project is archived (archived: true, last activity 2026-07-20), so the deferred jobs can't run there either. I also swapped the vague "phase 2 / a follow-up" pointers for STAC-25531/25532/25533.

Rebuilding your approval on a comment-only diff is a poor trade for your time, and I should have batched it with the review response instead of pushing separately. Re-requesting now.

Nothing else changed since you approved: still 16/16 credential-free jobs green, and the vsphere job is still gated behind the private-package-index environment.

One thing that still needs a second person regardless of the approval: the run is sitting in waiting on a dev-team approval for the vsphere deployment, and prevent_self_review means I can't release it myself. Whoever re-approves the PR can also approve that pending deployment on run 30989773234 — that's the last unproven path before merge.

@LouisParkin
LouisParkin requested a review from LouisLotter August 5, 2026 11:56
@LouisParkin

Copy link
Copy Markdown
Contributor Author

private-package-index: disabled prevent_self_review, and a better fix for the P1

@LouisLotter — heads-up on a change to the control I added for your P1 about credentials reaching PR-authored code, plus a proposal that I think closes it properly.

What changed: prevent_self_review on the private-package-index environment is now false. Required reviewers (dev-team) and the job's fork guard are unchanged.

Why. The gate as configured made the credentialed vsphere leg a required check that the PR author structurally could not satisfy. Because SHARED_PATHS includes the CI files, every CI-touching PR selects the full matrix including vsphere — roughly 16% of commits on this repo over the last year — so the author needed a second person on each one. That was my configuration choice, not something your review asked for.

The threat it was carrying is already handled twice for the case that actually matters:

  • the job's if: requires head.repo.full_name == github.repository, so fork PRs skip it; and
  • GitHub does not expose Actions secrets to fork pull_request runs on public repos at all.

So the environment gate only ever constrained the 27 accounts that already hold push access — and write access to a repo is already trust over its secrets under GitHub's model. Your narrower point stands (an in-repo branch is unreviewed at the time CI runs), but a self-approvable gate answers that with an audit record rather than an access boundary, which is what it was really providing.

The proper fix, which I'll do next. Your first suggested remedy — a credential-free read path — turns out to be the right one, just not by re-hosting. Context I got wrong earlier: vsphere-automation-sdk==1.82.0 is not something we patch. It is an unmodified upstream wheel that VMware withdrew from public PyPI, self-hosted in our GitLab registry only because that org was private. Upstream carries no SPDX license and the version predates their current tags, so publishing it publicly is a licensing question, not an engineering one — I'm not proposing that.

Instead I'll split the credential away from PR code:

  1. a fetch-private-deps job that checks out no executable repo content and uses a hardcoded package spec, so nothing PR-authored can influence it, pulls the wheel from the GitLab registry, and uploads it with actions/upload-artifact;
  2. the vsphere job downloads that artifact and installs --no-index offline.

The credential then never shares a job with PR-authored code, nothing is redistributed, the environment gate can be deleted, and vsphere rejoins the normal matrix — including on fork PRs, which currently skip it and lose that coverage.

I'll raise that as a separate PR rather than pushing here, since dismiss_stale_reviews + require_last_push_approval mean any further push to this branch re-dismisses your review.

LouisParkin and others added 2 commits August 5, 2026 15:11
The vsphere suite is the only one that needs a credential: it pins
vsphere-automation-sdk==1.82.0, an unmodified upstream VMware wheel that
VMware withdrew from public PyPI, so it resolves only from our GitLab
Package Registry. Public PyPI serves a 0.0.1 placeholder squatting the
name, which installs cleanly and fails much later.

setup_artifact_registry.sh left the registry password in a 0600 ~/.netrc
for the remainder of the job, so the suite's tests, its tox environment
and its whole dependency tree could read it. Test code is PR-authored,
which made the credential effectively readable by whoever opened the pull
request (STAC-25463 review, P1).

That was contained by putting the job behind the private-package-index
GitHub Environment, releasing the credential only after a human approved
each run. It worked, but SHARED_PATHS covers the CI files, so it fired on
roughly one commit in six and blocked authors on their own pull requests.

Replace it with a narrower fix. fetch_private_wheels.sh writes the netrc,
downloads one hardcoded package set into a wheelhouse under RUNNER_TEMP,
deletes the netrc, and rewrites ~/.pip/pip.conf to resolve that wheelhouse
via find-links. Everything afterwards runs with no credential on disk and
no authenticated index configured, and the environment gate is gone.

Details that matter:

  * The package set is hardcoded, not read from the tree: while the
    credential is on disk, a pull request must not be able to redirect pip.
  * --only-binary=:all:, because downloading an sdist executes its
    setup.py and would hand upstream code a readable ~/.netrc.
  * A system interpreter, never the toolchain virtualenv, whose pip is
    built by repository code. The path is resolved rather than hardcoded
    -- bci/python:3.13 ships /usr/bin/python3.13 and no /usr/bin/python3,
    and `python3` on PATH is a /usr/local/bin shim -- and the result is
    then rejected if it resolves inside GITHUB_WORKSPACE, which is what
    makes "system" a guarantee rather than an assumption.
  * An EXIT trap revokes the credential on the failure paths too, and the
    script then asserts the netrc is gone and the wheel actually arrived.

Rejected: a separate credentialed job publishing the wheels with
upload-artifact. Workflow artifacts on a public repository are anonymously
downloadable, so that would have amounted to publicly redistributing a
vendor artifact VMware withdrew, under no SPDX licence.

Residual risk, recorded in the workflow: a pull request that edits the
workflow itself can still reach the secret, since pull_request runs the
PR's own copy. That holds for every secret in every repository and is
contained by review, not CI. What changed is that ordinary test code and
its dependencies no longer see it.

setup_artifact_registry.sh is untouched; .gitlab-ci.yml still uses it.

Verified: shellcheck and actionlint clean, Zizmor clean (no findings).
Verified inside the pinned BCI image that the interpreter resolves to
/usr/bin/python3.13 and that a workspace-resident interpreter is refused.
Verified that a missing argument, missing credentials, a workspace
interpreter and a failed download all exit non-zero with no netrc left
behind and the password in no file or log; and that find-links in
~/.pip/pip.conf is honoured with no command-line flags, which is what tox
relies on via passenv HOME.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ndex

STAC-25540: fetch private-index wheels, then drop the credential
@LouisParkin

Copy link
Copy Markdown
Contributor Author

Status: fully green, and the environment gate is gone

@LouisLotter#364 merged into this branch, so what you'll be reviewing has changed materially since your last pass. Summary of where your P1 now stands.

Your finding: setup_artifact_registry.sh wrote the registry password to ~/.netrc, which persisted for the rest of the job and was readable by the subsequent checksdev test and benchmark steps — i.e. by PR-authored code. You offered two remedies: a credential-free read path, or a protected environment with required approval.

I originally took the second. It worked but was the wrong trade: SHARED_PATHS covers the CI files, so it fired on ~28 of the last 176 commits (~1 in 6) and structurally blocked authors on their own PRs.

This branch now implements your first remedy instead. .setup-scripts/fetch_private_wheels.sh writes the netrc, downloads a hardcoded package set, deletes the netrc, and repoints ~/.pip/pip.conf at a local wheelhouse via find-links. The suite then installs offline with no credential on disk and no authenticated index configured. environment: private-package-index is removed.

Four details are load-bearing, and I'd welcome your eye on them specifically:

  • the package set is hardcoded rather than read from the tree, so a PR cannot redirect pip while the credential is on disk;
  • --only-binary=:all:, because downloading an sdist executes its setup.py;
  • a system interpreter, resolved from candidates and then rejected if it resolves inside GITHUB_WORKSPACE — the toolchain venv is built by repo code, so its pip must never be in the credential's path;
  • an EXIT trap revokes the credential on failure paths, then the script asserts the netrc is gone and the wheel actually arrived rather than silently falling through to the PyPI 0.0.1 placeholder.

Latest run 31009548529: all 19 jobs green, vsphere included, zero pending deployment approvals.

Two things I want to be straight about:

  • Fork PRs still skip vsphere. I claimed earlier they'd regain coverage; that was wrong. Forks get no Actions secrets on a public repo, so they can't fetch the wheel at all. The fork guard stays.
  • Residual risk is unchanged and stated in the workflow: a PR that edits the workflow itself can still reach the secret, because pull_request runs the PR's own copy. That's true of every secret in every repo and is contained by review, not CI. What changed is that ordinary test code and its dependency tree no longer see it — and that needed no workflow edit at all.

Also, apologies again for dismissing your approval this morning; that was my push, not you.

Ready for another look whenever suits.

Comment thread .github/workflows/checks-tests.yml
The previous revision confined the GitLab package registry credential to a
single step and claimed that reaching it would require editing the workflow.
That claim was wrong. A `pull_request` run executes the pull request's own copy
of the workflow *and* of every script it calls, so a pull request could rewrite
fetch_private_wheels.sh, reorder the steps, or add one of its own. Confining a
secret inside pull-request-controlled code is hardening, not a boundary.

So the private-index suites no longer run on pull requests at all. They run on
push, tag and workflow_dispatch events, whose contents are reviewed before they
reach the release branch, and where the secret is therefore not exposed to
unreviewed code.

  * select-checks.sh moves private-index suites to a new
    `deferred_private_checks` output on pull requests, so the matrix is empty
    rather than gated, and the reason is reported.
  * check-tests-private-index is conditioned on the event, replacing the fork
    guard that is now redundant.
  * fetch_private_wheels.sh refuses to run on a pull_request event, so the rule
    survives an edit to the workflow condition.
  * ci-success reports the deferral, so a green pull request never implies
    vsphere was covered when it was not.

The cost is that vsphere is verified on the release branch rather than on the
pull request that changes it. That is accepted knowingly: the suite changes a
handful of times a year, and the alternatives cost more than they return. The
durable fix is to stop needing the private index -- VMware now publishes this
SDK to public PyPI under renamed packages -- which deletes the credential, the
job and this gap together.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
LouisLotter
LouisLotter previously approved these changes Aug 5, 2026
…pr-path

STAC-25540: keep the private-index credential off the pull request path
@LouisParkin

Copy link
Copy Markdown
Contributor Author

@LouisLotter — agreed on the call, and #365 is merged into this branch. Thread resolved.

Your one remaining condition, checked

The only reason to continue blocking would be if the GitLab token has permissions beyond read-only access to project 71271774.

Verified against the GitLab API rather than taken on trust. Project 71271774 is stackvista/ci/packages (private). Its deploy tokens:

token active scopes
CI Package registry access revoked read_package_registry, write_package_registry
gitlab-proxy revoked read_package_registry
gitlab-proxy-read-package-registry (13698947) revoked read_package_registry
gitlab-proxy-read-package-registry (13702516) revoked read_repository, read_package_registry
gitlab-proxy-read-package-registry (13702666) active read_package_registry

The only live token carries read_package_registry and nothing else — no write_package_registry, no read_repository. The one token that ever had write scope is revoked. So your condition is not met, and there's no reason to keep blocking. (Caveat for completeness: I can't read the secret's value, so I'm matching on the project's token inventory rather than proving which credential the secret holds. If it were a group/personal token instead of a deploy token, this wouldn't cover it.)

One thing worth recording that supports moving on: GITLAB_PACKAGE_REGISTRY_READONLY_PASSWORD is an org-level secret with visibility: all (orgVariables/resources_github.yaml:47), so every repo in the org already receives it. Gating it in this one repo could never have contained it — which is a further argument that the environment gate was buying less than it cost. This repo receives only the simple URL and the username; the write pair (GITLAB_PACKAGE_REGISTRY_PASSWORD) is not granted here.

Follow-up ticket filed as promised: STAC-25544 — move vsphere to the publicly-published VMware SDK packages (vmware-vapi-runtime, vmware-vapi-common-client, pyvmomi, all 9.1.0.0 on public PyPI, plus VMware's own public PEP 503 index for the NSX/VMC wheels). That deletes the credential, the check-tests-private-index job, and the PR coverage gap in one change. We're only on the private index because of the ancient vsphere-automation-sdk==1.82.0 pin.

Sorry — one more approval needed

Merging #365 pushed to this branch, so dismiss_stale_reviews + require_last_push_approval dismissed your approval for the third time. CI is re-running now. Nothing has changed since you approved except the #365 contents you already reviewed.

Worth fixing as part of STAC-25522 (branch protection): required_status_checks.contexts is currently empty on stackstate-7.78.2, so CI success isn't actually enforced as a required check yet — which is the thing that would let this settle down.

@LouisParkin
LouisParkin merged commit 09b68c1 into stackstate-7.78.2 Aug 6, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants