Skip to content

ci: run integration tests on CodeBuild-hosted runners - #654

Draft
ThirdEyeSqueegee wants to merge 1 commit into
mainfrom
codebuild-runner
Draft

ci: run integration tests on CodeBuild-hosted runners#654
ThirdEyeSqueegee wants to merge 1 commit into
mainfrom
codebuild-runner

Conversation

@ThirdEyeSqueegee

Copy link
Copy Markdown
Member

Runs the integration tests on CodeBuild-hosted GitHub Actions runners instead of GitHub-hosted runners.

On a CodeBuild-hosted runner, AWS credentials come from the IAM role attached to the runner itself, so the workflow no
longer assumes a role through the GitHub OIDC provider and no longer needs id-token: write. Runners stay ephemeral —
one job per runner — and arm64 jobs run on Graviton rather than emulated hardware.

Changes:

  • runs-on uses the CodeBuild label form. The matrix dimension runner becomes image: linux-5.0 for x86_64 and
    arm-3.0 for Graviton.
  • Dropped the Configure AWS credentials step and id-token: write, here and in the integ.yml call from
    build-test-image.yml. AWS_REGION is now set on the job.
  • harden-runner runs only on GitHub-hosted runners; on self-hosted runners it needs a StepSecurity agent that
    CodeBuild runners do not have.
  • The eksctl install no longer assumes sudo is available, since the CodeBuild image runs as root.

Merge this only once the ascp-runner CodeBuild project exists in the test account, otherwise the job waits for a
runner that never registers. Contributors running these tests from their own fork will not get a runner, since the
project is scoped to this repository — that path needs a CodeBuild project of their own. The safe-to-test label gate
on build-test-image.yml is unchanged.

Reference: https://docs.aws.amazon.com/codebuild/latest/userguide/action-runner.html

@secrets-manager-ai-code-review

secrets-manager-ai-code-review Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ThirdEyeSqueegee's task in 44m 54s —— View job


Code Review: CodeBuild-hosted integration test runners

Reviewed 4afbd37 against origin/main. The migration is coherent and the mechanics are mostly right — the runs-on label syntax is valid, the runnerimage matrix rename preserves semantics, and dropping id-token: write follows correctly from deleting the OIDC step. What concerns me is a cluster of implicit guarantees that moved from GitHub's runners to a CodeBuild project this repo doesn't describe, and one step that is now dead code rather than conditionally skipped.

I left 8 inline comments with specifics and suggested diffs. Summary below.

Worth resolving before merge

1. integ.yml:66 — the Harden Runner guard can never be true. runner.environment returns self-hosted for CodeBuild runners, so if: runner.environment == 'github-hosted' is false on 4/4 legs, permanently. The comment's reasoning is right, but the effect isn't "skip on self-hosted" — egress auditing is gone from this job entirely. That lands hardest here: it's the only job holding AWS credentials and creating real EKS clusters, it runs several mutable third-party tags, and it's reachable from build-test-image.yml's pull_request_target + allow-unsafe-pr-checkout: true path. Every other workflow in the repo still hardens unconditionally. Deleting the step would at least make the gap visible in the diff.

2. integ.yml:55 — no timeout-minutes, and CodeBuild's own timeout can skip cleanup. Two new failure modes:

  • If the project name, webhook subscription (WORKFLOW_JOB_QUEUED) or repo scoping is off, no runner registers and the job sits Queued up to 24h. With concurrency: {group: integration-tests, cancel-in-progress: false}, that wedges every later integration run, including the release path in release-chart.yml.
  • The effective ceiling is now min(360, project timeoutInMinutes), and CodeBuild's project default is 60 minutes. When CodeBuild kills the container the runner dies with it, so the if: always() cleanup at line 131 never executes — leaking the cluster, its VPC, NAT gateway and ENIs. That is precisely the network-interface quota exhaustion the concurrency comment on line 25 exists to prevent, and the next run then hits AlreadyExistsException. The role-duration-seconds: 14400 this PR removes suggests a 4-hour budget was expected.

3. integ.yml:43envsubst and jq are ubuntu-latest freebies, not Amazon Linux ones. The workflow installs kubectl/Helm/eksctl/Python/bats explicitly but not these two, and tests/integration.bats.template:173 (envsubst), :153 and tests/helpers.bash:73 (jq) all depend on them. A missing envsubst applies the SecretProviderClass unexpanded ~20 min in, on all four legs, and reads like a test bug. sudo dnf install -y gettext jq closes it.

4. integ.yml:114 — the fail-fast credential check is gone. configure-aws-credentials used to fail loudly on a bad role. Now the first AWS call is eksctl get cluster ... 2>/dev/null inside an if, which treats any non-zero exit as "no stale cluster" — so a permissions or throttling problem surfaces ~15 min later inside bats setup_file, and a genuinely leaked cluster gets masked. Also flagged there: the caller account is now the CodeBuild project's, while secrets.POD_IDENTITY_ROLE_ARN stays a hardcoded ARN — if those accounts differ, both pod-identity legs fail and both irsa legs pass, a 2-of-4 split that looks like flakiness.

5. integ.yml:91install -m 0755 /tmp/eksctl /usr/local/bin with a bare destination. install only installs into DEST if it already exists as a directory; otherwise it writes the binary to that path and exits 0. The step goes green and line 93 fails with eksctl: command not found. install -D ... /usr/local/bin/eksctl fixes it. (To be explicit: the [ ... ] && SUDO=sudo probe does not trip bash -e — bash exempts a failing non-final command in an && list. Not a bug, just fragile to reordering.)

Smaller items

  • integ.yml:59 — the deleted action set both AWS_REGION and AWS_DEFAULT_REGION; this sets only the former, while CodeBuild pre-sets both to the project's region. Nothing breaks today (eksctl and CLI v2 prefer AWS_REGION; every boto3 client passes region_name=), but botocore honours only AWS_DEFAULT_REGION, so it's a latent cross-region trap. Set both. us-west-2 now appears 5× in this file.
  • build-test-image.yml:85 — the OIDC removal is half done: still granted at build-test-image.yml:10 (workflow level, on a pull_request_target + secrets: inherit workflow) and at release-chart.yml:8 and :36. secrets.ROLE_ARN now has no consumer anywhere — worth deleting the secret. More importantly, nothing in the repo documents the ascp-runner project or the permissions its service role needs; with the OIDC role gone that's the only thing between a maintainer and a working run.
  • integ.yml:45-52- os: linux is dead (matrix.os is unused in this file), and image/arch/arch-short are three spellings of one binary choice joined by an include lookup.
  • integ.yml:55-large is hardcoded for a job that mostly waits on EKS control-plane provisioning, now billed per-minute × 4 legs × every run. And the label omits auth_type, so two legs render byte-identical runs-on strings — supported, but it makes CodeBuild-build ↔ GitHub-job correlation ambiguous when debugging.

Checked and clear

Ruling these out so they don't get re-litigated: the codebuild-<project>-<run_id>-<run_attempt>-<image>-<size> label form is valid and linux-5.0/arm-3.0/large are real identifiers; the &&-list set -e concern above is a non-issue; the runnerimage matrix rename is semantically equivalent; duplicate runs-on labels across matrix expansions are documented as safe; GNU parallel is only reached for multi-target invocations, which this workflow never makes; and no CLAUDE.md exists in the repo, so there are no project conventions in play here.

Pre-existing, out of scope

Not caused by this PR (tests/ is untouched), but it explains flaky integration runs and is worth a follow-up: tests/run-tests.sh:44 calls generate-test-files.py cleanup-secrets without forwarding $2, and generate-test-files.py:289 falls back to list(CONFIGS.keys()) — all four configs, in both regions, via ForceDeleteWithoutRecovery=True. With fail-fast: false and four concurrent legs of differing duration, the first leg to finish destroys the other three legs' Secrets Manager secrets and SSM parameters mid-run. Relatedly, delete_cluster() at run-tests.sh:38 passes no --region, which is what makes the AWS_REGION item above matter.


Note: I can't push changes to .github/workflows (app permission), so the inline comments carry suggested diffs rather than commits. I also can't submit a formal PR review or approve — this comment is advisory only.

@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.62%. Comparing base (3843ceb) to head (4afbd37).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #654   +/-   ##
=======================================
  Coverage   61.62%   61.62%           
=======================================
  Files          11       11           
  Lines         740      740           
=======================================
  Hits          456      456           
  Misses        265      265           
  Partials       19       19           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.


- name: Harden Runner
# Self-hosted runners need a StepSecurity agent, which CodeBuild runners do not have.
if: runner.environment == 'github-hosted'

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.

This guard can never be true, so Harden Runner is now dead code rather than conditionally skipped.

runs-on (line 55) is unconditionally a CodeBuild label, and runner.environment evaluates to self-hosted for every self-hosted runner — CodeBuild-hosted ones included. The condition is therefore false on 4/4 matrix legs, permanently, with no path that turns it back on.

The comment's reasoning is right (harden-runner does need a StepSecurity agent), but the net effect isn't "skip on self-hosted" — it's egress auditing removed from the integration job entirely, which is easy to miss when the diff reads as a one-line if:. That's the largest loss here because this job:

  • is the only one in the repo that holds AWS credentials and creates/destroys real EKS clusters, secrets and SSM parameters;
  • runs several mutable third-party tags (azure/setup-kubectl@v5, azure/setup-helm@v5, bats-core/bats-action@4.0.0);
  • is reachable from build-test-image.yml, which triggers on pull_request_target and checks out PR-authored code with allow-unsafe-pr-checkout: true.

Every other workflow in the repo still hardens unconditionally (build-test-image.yml:38, go.yml:21, pr_sync.yml:19, release-chart.yml:46), so this job is now the sole exception.

Two options that are both clearer than a dead if::

  1. Delete the step outright, so the diff shows the control being removed and reviewers can weigh it.
  2. Keep it, but gate on something that can actually vary, and pair it with a CodeBuild-side control (VPC egress rules or the project's network configuration) so the capability isn't just dropped.

If you go with (1), a note in the PR description about what replaces egress monitoring on the CodeBuild side would help.

image: arm-3.0
# CodeBuild-hosted runner. AWS credentials come from the runner's own IAM role, so the workflow
# does not assume a role. `image` picks the compute: linux-5.0 is x86_64, arm-3.0 is Graviton.
runs-on: codebuild-ascp-runner-${{ github.run_id }}-${{ github.run_attempt }}-${{ matrix.image }}-large

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.

Moving to a self-hosted label without a timeout-minutes opens two new failure modes, one of which leaks EKS clusters.

The label form itself is correct — codebuild-<project>-<run_id>-<run_attempt>-<image>-<size> is the documented single-label override, and linux-5.0, arm-3.0 and large are all valid identifiers. The concern is what happens around it.

1. The job can queue for up to 24h and wedge every later integration run. ubuntu-latest was always available; a self-hosted label is not. If the project isn't named exactly ascp-runner, or its webhook isn't subscribed to WORKFLOW_JOB_QUEUED, or the org/repo scoping is off, AWS's docs note that CodeBuild won't process the webhook and the workflow may hang. No runner registers, and GitHub holds the job Queued up to its 24-hour self-hosted limit. Because concurrency (lines 24–28) uses group: integration-tests with cancel-in-progress: false, the next push to main — including the release path via release-chart.yml — queues behind the stuck run. This is also the fork/workflow_dispatch story: a contributor triggering this from a fork gets a job that hangs with no signal as to why.

2. CodeBuild's build timeout can kill the runner mid-job, and then if: always() doesn't run. The effective ceiling is now min(GitHub's 360-minute default, the project's build timeout), and the CodeBuild project default is 60 minutes. When CodeBuild terminates the container, the runner process dies with it, so GitHub never gets to execute the if: always() cleanup at line 131 — integ-cluster-<arch>-<auth_type>, its VPC, NAT gateway and ENIs all survive. That's exactly the network-interface quota exhaustion the concurrency comment on line 25 exists to prevent, and it compounds: the next run hits AlreadyExistsException on eksctl create cluster. Under GitHub-hosted runners the cleanup step always got a chance to run.

The role-duration-seconds: 14400 this PR deletes is evidence the maintainers budgeted up to 4 hours for these runs, so a 60-minute project default would be a real regression. Worth confirming the project's timeoutInMinutes (and queuedTimeoutInMinutes/concurrentBuildLimit while you're there), then setting a job timeout comfortably below it so GitHub, not CodeBuild, is the one that stops the job and cleanup still fires:

Suggested change
runs-on: codebuild-ascp-runner-${{ github.run_id }}-${{ github.run_attempt }}-${{ matrix.image }}-large
runs-on: codebuild-ascp-runner-${{ github.run_id }}-${{ github.run_attempt }}-${{ matrix.image }}-large
timeout-minutes: 90

Two smaller things on this line:

  • The label doesn't include auth_type, so the irsa and pod-identity legs for a given image render byte-identical runs-on strings. That's supported, but it makes CodeBuild build ↔ GitHub job correlation ambiguous when you're debugging, and rules out per-leg cost attribution. The positional label format leaves nowhere to add a discriminator.
  • -large is hardcoded for a job that spends most of its wall-clock blocked on EKS control-plane and nodegroup provisioning. Now that compute is billed per-minute across 4 legs on every run, -small/-medium is worth measuring against — this is not a CPU-bound workload.


- name: Clean up stale clusters
run: |
CLUSTER_NAME=integ-cluster-${{ matrix.arch-short }}-${{ matrix.auth_type }}

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.

Deleting Configure AWS credentials removed the only fail-fast credential check, and this step swallows the error that replaces it.

aws-actions/configure-aws-credentials used to fail the job immediately and loudly if the role couldn't be assumed. With it gone, the first AWS call in the job is eksctl get cluster ... 2>/dev/null on the next line — and 2>/dev/null plus if means any non-zero exit is interpreted as "no stale cluster":

  • Credential/permission problems surface ~15 minutes late. If the CodeBuild service role lacks an EKS permission, or the call is throttled, stderr is discarded, the if takes the false branch, and the job carries on as though everything is fine. The failure then appears inside bats setup_file as an opaque error well into the run.
  • A genuinely leaked cluster gets masked. If the describe call fails for any reason other than not-found, the guard this step exists to provide silently doesn't fire, and eksctl create cluster dies with AlreadyExistsException — the exact case this step was added to prevent.

Worth distinguishing "not found" from "call failed" rather than collapsing both to absent, e.g. capture the output and check the exit status / error text explicitly, and drop the blanket 2>/dev/null. A cheap aws sts get-caller-identity early in the job would also restore fail-fast and confirm the runner's identity is what you expect (see the note below about POD_IDENTITY_ROLE_ARN).

Two related consequences of removing that step:

  • CloudTrail attribution is gone. role-session-name: csi-driver-ci-${{ github.run_id }}-${{ matrix.arch }} tagged every CreateCluster/DeleteSecret/DeleteCluster with a run id and arch. All four legs — and anything else sharing the ascp-runner project — now appear as the same service-role identity. Given that the legs create and force-delete similarly-named resources in one account, that's a meaningful loss for incident triage.
  • The account identity changes. Credentials now come from the CodeBuild project's service role, so the caller account is whatever account hosts ascp-runner. secrets.POD_IDENTITY_ROLE_ARN (line 123) is a hardcoded ARN, and generate-test-files.py:244 passes it to eksctl create podidentityassociation. If the CodeBuild project doesn't live in the same account as that role, both pod-identity legs fail while both irsa legs pass — a 2-of-4 split that reads like a flaky test. Confirming this is a same-account move (or noting it in the PR description) would save someone that debugging session.


env:
imageTag: ${{ matrix.arch == 'amd64' && inputs.imageTagAmd64 || inputs.imageTagArm64 }}
AWS_REGION: us-west-2

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.

This replaces an action that set both region variables with one that sets only AWS_REGION.

aws-actions/configure-aws-credentials exported AWS_REGION and AWS_DEFAULT_REGION consistently. CodeBuild also injects both, pointing at the project's own region. Overriding only AWS_REGION means that if ascp-runner isn't in us-west-2, the two disagree for the rest of the job.

Nothing breaks today — I checked: eksctl (aws-sdk-go-v2) and AWS CLI v2 both prefer AWS_REGION, every boto3.client(...) in tests/generate-test-files.py passes region_name= explicitly, and every aws CLI call in the test scripts passes --region. So this is latent, not a live bug. But botocore honours only AWS_DEFAULT_REGION, so the first client added without an explicit region_name would quietly create or delete resources in the project's region instead of us-west-2 — and cross-region resource deletion is not a fun failure to diagnose. Cheap to close now:

Suggested change
AWS_REGION: us-west-2
AWS_REGION: us-west-2
AWS_DEFAULT_REGION: us-west-2

While this block is being touched: us-west-2 is now hardcoded five times in this file (here, lines 115, 117, 128, 135) and us-east-2 twice (129, 136), and tests/integration.bats.template:10 re-exports REGION=us-west-2 unconditionally on top of all of it. Hoisting both regions into this job-level env: (or into workflow_call inputs) and letting the steps inherit would cut that to one place each.

fail-fast: false
matrix:
runner: [ubuntu-latest, ubuntu-24.04-arm]
image: [linux-5.0, arm-3.0]

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.

This line changes the base OS from Ubuntu to Amazon Linux, and two tools the tests depend on come free on ubuntu-latest but aren't guaranteed on the CodeBuild curated images.

The workflow is careful to install kubectl, Helm, eksctl, Python and bats explicitly — but the test scripts also reach for:

  • envsubst (ships in gettext, not a base package) — tests/integration.bats.template:173 runs envsubst < BasicTestMountSPC-<arch>-<auth>.yaml | kubectl apply -f -
  • jqtests/integration.bats.template:153 and tests/helpers.bash:73

Neither is installed here. If envsubst is absent, the SecretProviderClass is applied as an unexpanded template or not at all, roughly 20 minutes into the run, on all 4 legs — and it presents as a test bug rather than a missing host dependency. AWS calls this out directly: "If a dependency provided by GitHub-hosted runners is unavailable in the CodeBuild environment, you can install the dependency using GitHub Actions."

Worth adding an explicit step (and it's harmless if the tools are already present):

      - name: Install test dependencies
        run: sudo dnf install -y gettext jq

Related, on line 95: actions/setup-python@v6 resolves prebuilt CPython from the actions/python-versions manifest, which is built on Ubuntu images; linux-arm64 coverage there is thinner than linux-x64. Since tests/run-tests.sh calls python3 generate-test-files.py on both the test path (:21) and the clean path (:44), a setup-python failure takes out cleanup as well as the run — leaving a live EKS cluster behind. Pinning an explicit minor version rather than "3", and echoing python3 --version / which pip once on each image, would confirm this works on both legs before you rely on it. (Line 101 uses pip rather than pip3, which works only because setup-python puts it on PATH.)

Comment on lines +88 to +91
SUDO=""
[ "$(id -u)" -ne 0 ] && SUDO=sudo

$SUDO install -m 0755 /tmp/eksctl /usr/local/bin && rm /tmp/eksctl

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.

The real bug on this line is the bare destination, not the sudo probe.

install SRC DEST only installs into DEST when DEST already exists as a directory; otherwise DEST is the target filename. On an image where /usr/local/bin doesn't exist, install writes the eksctl ELF to the path /usr/local/bin, exits 0, rm /tmp/eksctl succeeds, and this step goes green. The failure then shows up one step later at line 93 as a bare eksctl: command not found — and /usr/local/bin is now a regular file, so nothing else can be installed there for the remainder of the job. -D makes it explicit and creates the directory if needed:

Suggested change
SUDO=""
[ "$(id -u)" -ne 0 ] && SUDO=sudo
$SUDO install -m 0755 /tmp/eksctl /usr/local/bin && rm /tmp/eksctl
SUDO=""
if [ "$(id -u)" -ne 0 ]; then SUDO=sudo; fi
$SUDO install -D -m 0755 /tmp/eksctl /usr/local/bin/eksctl && rm /tmp/eksctl

To be explicit about what is not a problem here, since it looks like one: [ "$(id -u)" -ne 0 ] && SUDO=sudo does not abort the step under the default bash -e {0} shell. Bash's errexit exemption covers a failing command that isn't the last in an && list, and this list isn't the script's final command either. So no bug — but the step's exit status does depend on a later line existing, which is why the if form above is worth taking: it survives someone reordering or copy-pasting the idiom to the end of a script.

Two other notes:

  • The non-root branch assumes sudo is installed and the runner user is a sudoer. If neither privilege path is reliable, sidestepping it entirely is more robust than probing — install -D -m 0755 /tmp/eksctl "$HOME/.local/bin/eksctl" plus echo "$HOME/.local/bin" >> "$GITHUB_PATH", which takes effect from the next step onward (line 93).
  • Line 84 fetches eksctl_checksums.txt from the same releases/latest/download/ pointer as the tarball on line 82, so the checksum verifies the download against a file from the same mutable source — it detects a truncated transfer but pins nothing and provides no supply-chain guarantee. Pinning an eksctl version (and reading the checksum from that tag) would make it meaningful. Both curl calls also lack --fail/--retry, and with pipefail off, curl ... | grep | sha256sum --check reports only sha256sum's status — a 404 body yields an empty grep and a confusing downstream error rather than a clean failure.

Comment on lines 45 to +52
include:
- os: linux
- arch: amd64
arch-short: x64
runner: ubuntu-latest
image: linux-5.0
- arch: arm64
arch-short: arm
runner: ubuntu-24.04-arm
runs-on: ${{ matrix.runner }}
image: arm-3.0

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.

Minor: - os: linux is dead, and the rename leaves three parallel encodings of one binary choice.

The runnerimage rename preserves matrix semantics correctly. Two cleanups while this block is open:

- os: linux is an include entry with no key overlapping any matrix dimension, so it's merged into all four combinations — but matrix.os is referenced nowhere in this file (only build-test-image.yml:59). It can go:

Suggested change
include:
- os: linux
- arch: amd64
arch-short: x64
runner: ubuntu-latest
image: linux-5.0
- arch: arm64
arch-short: arm
runner: ubuntu-24.04-arm
runs-on: ${{ matrix.runner }}
image: arm-3.0
include:
- image: linux-5.0
arch: amd64
arch-short: x64
- image: arm-3.0
arch: arm64
arch-short: arm

Beyond that, image (linux-5.0/arm-3.0), arch (amd64/arm64) and arch-short (x64/arm) are three spellings of the same x86-vs-ARM decision, wired together through this lookup table. The footgun is silent: add a value to line 43 without a matching include and you get legs with no arch/arch-short, so ./run-tests.sh -irsa runs against a target name that matches nothing. Dropping the image: list and driving the matrix from a single list of objects removes the mismatch class entirely:

      matrix:
        auth_type: [irsa, pod-identity]
        target:
          - { image: linux-5.0, arch: amd64, arch-short: x64 }
          - { image: arm-3.0,   arch: arm64, arch-short: arm }

(That does mean matrix.target.arch at the reference sites, so it's a judgement call on churn — fine to leave as-is if you'd rather keep the diff tight.)

@@ -83,7 +83,6 @@ jobs:
imageTagAmd64: latest-amd64-${{ needs.build-docker.outputs.privTagSuffix }}
imageTagArm64: latest-arm64-${{ needs.build-docker.outputs.privTagSuffix }}
permissions:

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.

Right change, but the OIDC teardown is only half done.

Dropping id-token: write here is correct — the sole consumer, aws-actions/configure-aws-credentials, is gone from integ.yml, and a called workflow can't use a permission it doesn't declare anyway. Three loose ends elsewhere, though:

  1. build-test-image.yml:10 still grants id-token: write at the workflow level. Top-level permissions are the default for any job that omits its own block, so this remains a standing over-grant on a workflow triggered by pull_request_target with secrets: inherit — and it reads as if OIDC is still in play, contradicting the two places this PR removes it. build-docker authenticates to ghcr.io with secrets.GITHUB_TOKEN, so nothing here needs it.
  2. release-chart.yml:36 is the exact same call site and still grants it, plus line 8 at the workflow level. That file also calls integ.yml, so leaving it untouched means the two callers now disagree about what the reusable workflow needs.
  3. secrets.ROLE_ARN no longer has a consumer anywhere in the repo — I grepped, and the only remaining *ROLE_ARN references are POD_IDENTITY_ROLE_ARN and unrelated docs. Worth deleting the repo/org secret so it isn't mistaken for live configuration later.

More broadly: the IAM identity this workflow now depends on lives entirely outside the repo. Nothing in .github/, tests/README.md or CONTRIBUTING.md describes the ascp-runner CodeBuild project, its service role, or the permissions that role needs (EKS cluster create/delete, IAM OIDC provider, iamserviceaccount, podidentityassociation, iam:PassRole on the pod-identity role, plus Secrets Manager and SSM writes in two regions). With the OIDC role gone, that's the only thing standing between a maintainer and a working integration run — a short section in tests/README.md (or IaC) would keep it discoverable.

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.

1 participant