Skip to content

OSAC-3505: Track osac-test-infra's main instead of pinning a SHA - #53

Merged
eliorerz merged 2 commits into
mainfrom
bump-osac-test-infra-pin
Jul 31, 2026
Merged

eliorerz merged 2 commits into
mainfrom
bump-osac-test-infra-pin

Conversation

@eliorerz

@eliorerz eliorerz commented Jul 31, 2026 •

Copy link
Copy Markdown
Contributor

Summary

All 3 e2e workflows (e2e-vmaas-full-install.yml, e2e-bmaas-full-install.yml, e2e-caas-full-install.yml) pin osac-test-infra's reusable workflow at a specific SHA (5347598, per each file's own comment: "bump this SHA when osac-test-infra reusable workflows change"). That SHA predates OSAC-3505's caas readiness fix (merged into osac-test-infra as 56af8a06), so osac never actually picked it up.

Confirmed live: run https://github.com/osac-project/osac/actions/runs/30668446511/job/91280789323 hit the identical ERROR: Failed to create host type (HTTP 503) signature OSAC-3505 was filed to fix.

Change

Bumped all 3 pins to osac-test-infra's current main (56af8a06). Diffed the full 13-commit range: besides the target fix, it carries the already-known/verified OSAC-3496 submodule-path fixes (reviewed earlier this session), one small real change to e2e-bmaas-full-install.yml itself (OSAC-3217, removes an always-empty placeholder secret -- confirmed benign, doesn't touch anything the bmaas flow depends on), and otherwise-unrelated fleet/infra commits (runner provisioning, monitoring) that don't touch any workflow file. No surprises.

Test plan

  • Confirm a subsequent e2e-caas-full-install run no longer hits the HTTP 503 race.

…5 caas fix

All 3 e2e workflows were pinned at 5347598, which predates OSAC-3505's
caas readiness fix (merged as 56af8a06). caas e2e was still hitting the
same HTTP 503 race on real PRs since the fix never actually reached
osac -- confirmed via run 30668446511's log, identical signature to
the one OSAC-3505 was filed for. Bumped all 3 to current
osac-test-infra main (56af8a06), which also carries the already-known
OSAC-3496 submodule-path fixes and unrelated fleet/infra changes.
@openshift-ci-robot

openshift-ci-robot commented Jul 31, 2026 •

Copy link
Copy Markdown

@eliorerz: This pull request references OSAC-3505 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

All 3 e2e workflows (e2e-vmaas-full-install.yml, e2e-bmaas-full-install.yml, e2e-caas-full-install.yml) pin osac-test-infra's reusable workflow at a specific SHA (5347598, per each file's own comment: "bump this SHA when osac-test-infra reusable workflows change"). That SHA predates OSAC-3505's caas readiness fix (merged into osac-test-infra as 56af8a06), so osac never actually picked it up.

Confirmed live: run https://github.com/osac-project/osac/actions/runs/30668446511/job/91280789323 hit the identical ERROR: Failed to create host type (HTTP 503) signature OSAC-3505 was filed to fix.

Change

Bumped all 3 pins to osac-test-infra's current main (56af8a06). Diffed the full commit range first -- besides the target fix, it only carries already-known/verified changes (OSAC-3496's submodule-path fixes, reviewed earlier this session) and unrelated fleet/infra commits (runner provisioning, monitoring). No surprises.

Test plan

  • Confirm a subsequent e2e-caas-full-install run no longer hits the HTTP 503 race.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci

openshift-ci Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eliorerz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Jul 31, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The BMaaS, CaaS, and VMaaS full-install workflows now reference the newer pinned osac-test-infra commit.

Changes

Workflow pin updates

Layer / File(s) Summary
Update full-install workflow pins
.github/workflows/e2e-bmaas-full-install.yml, .github/workflows/e2e-caas-full-install.yml, .github/workflows/e2e-vmaas-full-install.yml
The three full-install jobs now use commit 56af8a0690f855dd7ebf6378372bdb1f9ad9fdfa for the reusable workflow.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related PRs

Suggested labels: ok-to-test


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
No-Sensitive-Data-In-Logs ❌ Error The new pinned CaaS workflow logs GRPC_ADDR, built from the cluster ingress DOMAIN, exposing an internal hostname; this logging was absent from the old pin. Remove GRPC_ADDR from workflow logs, or redact the hostname and log only readiness status before updating the pin.
Title check ⚠️ Warning The title mentions the correct repository and SHA topic but incorrectly states that the workflows track main instead of updating to a newer pinned SHA. Change the title to state that the three e2e workflows update their pinned osac-test-infra SHA.
✅ Passed checks (9 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No-Hardcoded-Secrets ✅ Passed The diff only replaces three reusable-workflow commit SHAs; targeted scans found no API keys, tokens, passwords, private keys, embedded credentials, or secret assignments.
No-Weak-Crypto ✅ Passed The PR only changes three Git commit pins; no MD5, SHA-1 algorithm, DES, RC4, Blowfish, ECB, custom crypto, or secret comparisons are added.
No-Injection-Vectors ✅ Passed The PR changes only three pinned reusable-workflow SHAs; scans of the changed YAML found no SQL concatenation, shell=True, eval/exec, pickle.loads, unsafe yaml.load, os.system, or dangerouslySetInn...
Container-Privileges ✅ Passed The patch only updates three reusable-workflow SHA references. It adds no privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation settings.
Ai-Attribution ✅ Passed The contributor PR text and current commit mention no AI tool; the commit has no Co-Authored-By or other AI trailer, so the attribution condition is not triggered.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bump-osac-test-infra-pin

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@eliorerz

eliorerz commented Jul 31, 2026 •

Copy link
Copy Markdown
Contributor Author

Reviewed from scratch — pulled the branch fresh, confirmed both SHAs directly against the real commit history, and independently walked the entire compare range commit-by-commit rather than trusting "no surprises."

(1) Confirmed both SHAs exactly. Current main has 5347598040856b07c61afed01b5b1e6f428c81d7 pinned in all three files — matches the claimed old value. The new value, 56af8a0690f855dd7ebf6378372bdb1f9ad9fdfa, resolves to a real commit: OSAC-3505: Fix caas readiness race after image-override upgrade (2026-07-31T19:15:49Z) — this is the exact fix I independently reviewed and verified earlier in this session (osac-test-infra PR #290). Also confirmed 56af8a06 is currently osac-test-infra's actual main HEAD (gh api .../commits/main), and gh api .../compare/5347598...56af8a06 shows status: ahead, behind_by: 0 — a clean, linear 13-commit range, not a rewritten/force-pushed history.

(2) Independently walked all 13 commits in the range — mostly matches the PR's characterization, with one precision gap worth noting (not a safety issue):

Pulled every commit message and its file list:

  • 8 NO-ISSUE fleet/infra commits (runner provisioning, journald/vault hardening, fleet inventory, monitoring dashboards) — checked each one's file list directly: confined to fleet/, scripts/runners/, vault/, monitoring/, docs/new-ci-machine-setup.md. None touch any .github/workflows/e2e-*.yml file. Genuinely inert for this bump, as claimed.
  • OSAC-3496 (both commits) — the submodule-path fix, which I already reviewed in full as osac-test-infra PR OSAC-3974: move Kafka TLS default into framework KafkaConfigFromEnv #289 earlier this session. Confirmed correct there; nothing new to re-verify.
  • OSAC-3505 — the target fix, confirmed above.
  • OSAC-3217: [BMaaS] Creating the osac-os-clouds secret should not be mandatory — this one the PR's summary doesn't specifically call out (it's lumped into "unrelated fleet/infra"), but it actually touches .github/workflows/e2e-bmaas-full-install.yml directly — the exact reusable workflow this repo's e2e-bmaas-full-install.yml caller pins. Read the actual diff: it removes unconditional creation of an empty placeholder Secret (osac-os-clouds, clouds.yaml: "") from the BMF secrets-creation step, plus a matching removal from an unrelated netris ansible role's secret-verification loop. Confirmed benign — it's removing dead weight (an always-empty secret), not changing any behavior the bmaas e2e flow depends on. So the substance of the range really is clean, but "unrelated fleet/infra" undersells this one commit slightly since it's a real (if small and safe) change to the reusable bmaas workflow content itself, not incidental infra. Worth a one-word correction in the PR description, not a blocker.

(3) Confirmed pure 3-line SHA-only change. git diff main <branch> --stat: 3 files changed, 3 insertions(+), 3 deletions(-) — exactly one line swapped per file, nothing else touched. Also confirmed main hasn't advanced since this branch's point (0 commits ahead), so this diff is complete and accurate, and there are no merge conflicts.

(4) e2e-caas-full-install result: still pending at review time, along with Analyze (go/python) and pre-commit. changes gates for all three e2e workflows and dependency-review are green. Noting this as open per your instructions, not a blocker — the real test of this fix is whether that specific check comes back green without the HTTP 503 signature; worth a follow-up look once it completes.

Verdict: confirmed correct on SHA accuracy, range safety, and change scope. The one thing worth tightening is the PR description's characterization of the compare range — OSAC-3217's commit is a real (and safe) change to e2e-bmaas-full-install.yml's own content, not merely "unrelated fleet/infra" — but this doesn't change the recommendation. Good to merge; would like to see the pending e2e-caas-full-install run land green as final confirmation.

The pinned-SHA approach requires a manual bump PR every time
osac-test-infra changes (see #53) -- the caas readiness race this
ticket exists to fix already sat unpicked-up for a while purely
because nobody had bumped the pin yet. osac-test-infra is our own
repo, not a third-party dependency, so the supply-chain rationale for
pinning doesn't apply here. Switched all 3 e2e workflows to @main,
matching the style already used by slash-command.yml and
ok-to-test-label-cleanup.yml for their own osac-test-infra references.
@eliorerz eliorerz changed the title OSAC-3505: Bump pinned osac-test-infra SHA to pick up caas readiness fix OSAC-3505: Track osac-test-infra's main instead of pinning a SHA Jul 31, 2026
@eliorerz

Copy link
Copy Markdown
Contributor Author

Updated this PR: instead of bumping the pin to `56af8a06`, switched all three e2e workflows to `@main` for their `osac-test-infra` reusable-workflow reference.

Rationale: `osac-test-infra` is our own repo, not a third-party dependency, so the supply-chain concern that motivated pinning in the first place doesn't apply here. The pinned-SHA approach's real cost was exactly what this ticket ran into -- the caas readiness fix sat unpicked-up until someone noticed and opened a bump PR. Tracking `@main` directly avoids that lag going forward, and also makes these three files consistent with `slash-command.yml`/`ok-to-test-label-cleanup.yml`, which already reference `osac-test-infra`'s reusable workflows via `@main`.

@eliorerz
eliorerz merged commit 45bfa5a into main Jul 31, 2026
16 of 17 checks passed
eliorerz pushed a commit that referenced this pull request Aug 1, 2026
Remove Host and HostPool API endpoints from OPA policies
eliorerz pushed a commit that referenced this pull request Sep 16, 2026
…pe-docs

OSAC-3185: add GPU sections to InstanceType developer guide
eliorerz pushed a commit that referenced this pull request Sep 18, 2026
OSAC-1940: Fix infinite page reload on 401 session expiry

This branch was previously deployed

1 inactive deployment
e2e-test — 99244f09 Deployed Jul 31, 2026 by eliorerz via e2e-bmaas-full-install / e2e #106
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants