NO-ISSUE: Restore e2e-caas-netris-full-install as on-demand only - #169
NO-ISSUE: Restore e2e-caas-netris-full-install as on-demand only#169eliorerz wants to merge 2 commits into
Conversation
|
@eliorerz: This pull request explicitly references no jira issue. DetailsIn response to this:
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. |
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: osac-project/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdded a GitHub Actions workflow for the CaaS Netris full-install E2E suite. It defines triggers, change-based gating, reusable E2E execution with component image builds, and final failure reporting. ChangesCaaS Netris full-install E2E
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/test e2e-caas-netris-full-install |
|
@eliorerz: No presubmit jobs available for osac-project/osac@main DetailsIn response to this:
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 kubernetes-sigs/prow repository. |
|
Triggered: E2E CaaS Netris Full Install |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/e2e-caas-netris-full-install.yml:
- Around line 66-75: Update the workflow’s reusable-workflow invocation to pass
fork-pr-author-association, fork-pr-author, and pr-number from the pull request
event, then add or use the reusable workflow’s authorization guard to reject
unauthorized fork PRs before provision executes. Keep repository and ref
selection unchanged for authorized runs.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d0a51b8d-051b-43ca-983b-d77e1584b74d
📒 Files selected for processing (1)
.github/workflows/e2e-caas-netris-full-install.yml
| osac-repo: ${{ github.event.pull_request.head.repo.clone_url || format('https://github.com/{0}.git', github.repository) }} | ||
| osac-branch: ${{ github.event.pull_request.head.ref || github.ref_name }} | ||
| test-infra-repository: osac-project/osac-test-infra | ||
| components: >- | ||
| [ | ||
| {"repo":"${{ github.event.pull_request.head.repo.full_name || github.repository }}","ref":"${{ github.event.pull_request.head.ref || github.ref_name }}","containerfile":"fulfillment-service/Containerfile","imageKey":"service.images.service"}, | ||
| {"repo":"${{ github.event.pull_request.head.repo.full_name || github.repository }}","ref":"${{ github.event.pull_request.head.ref || github.ref_name }}","containerfile":"osac-operator/Containerfile","imageKey":"operator.image.repository"}, | ||
| {"repo":"${{ github.event.pull_request.head.repo.full_name || github.repository }}","ref":"${{ github.event.pull_request.head.ref || github.ref_name }}","containerfile":"osac-aap/execution-environment/execution-environment.yaml","imageKey":"aap.bootstrap.image","buildType":"ansible-builder"}, | ||
| {"repo":"${{ github.event.pull_request.head.repo.full_name || github.repository }}","ref":"${{ github.event.pull_request.head.ref || github.ref_name }}","containerfile":"osac-aap/execution-environment/execution-environment.yaml","imageKey":"aap.configAsCode.eeImage","buildType":"ansible-builder"} | ||
| ] |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
workflow="$(mktemp)"
gh api 'repos/osac-project/osac-test-infra/contents/.github/workflows/e2e-caas-netris-full-install.yml?ref=main' \
--jq '.content' | tr -d '\n' | base64 --decode > "$workflow"
rg -n -C 5 'workflow_call|fork-pr-author-association|fork-pr-author|pr-number|author_association|authorization' "$workflow"
rm -f "$workflow"Repository: osac-project/osac
Length of output: 603
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
gh api 'repos/osac-project/osac-test-infra/contents/.github/workflows/e2e-caas-netris-full-install.yml?ref=main' \
--jq '.content' | tr -d '\n' | base64 --decode > "$tmp/workflow.yml"
printf '%s\n' '--- workflow_call inputs ---'
sed -n '14,150p' "$tmp/workflow.yml"
printf '%s\n' '--- authorization and runner-provisioning references ---'
rg -n -C 8 'fork-pr|author_association|pr-number|pull_request|workflow_dispatch|ec2|runner|provision|uses:' "$tmp/workflow.yml" || trueRepository: osac-project/osac
Length of output: 19166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
for path in \
'.github/workflows/ec2-runner-provision.yml' \
'.github/workflows/ec2-runner-teardown.yml'
do
out="$tmp/$(basename "$path")"
gh api "repos/osac-project/osac-test-infra/contents/${path}?ref=main" \
--jq '.content' | tr -d '\n' | base64 --decode > "$out"
printf '\n--- %s ---\n' "$path"
rg -n -C 10 'workflow_call|fork-pr|author_association|pr-number|pull_request|aws|secrets|github.token|runner|ec2|provision' "$out" || true
doneRepository: osac-project/osac
Length of output: 12228
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='.github/workflows/e2e-caas-netris-full-install.yml'
printf '%s\n' '--- caller trigger and jobs ---'
sed -n '1,115p' "$file"
printf '%s\n' '--- references to the netris workflow ---'
rg -n -C 6 'e2e-caas-netris-full-install|pull_request|workflow_call|uses:' .github/workflowsRepository: osac-project/osac
Length of output: 50374
Authorization Bypass (CWE-862): Missing Authorization
Reachability: External · Exploitability: Moderate
Block unauthorized fork PRs before provision.
This workflow can run on an explicit PR rerun and passes fork-controlled repository and ref values to the reusable workflow. Neither reusable workflow defines fork authorization inputs or checks before provisioning EC2 and obtaining runner credentials. Pass fork-pr-author-association, fork-pr-author, and pr-number, then reject unauthorized fork PRs before provision starts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/e2e-caas-netris-full-install.yml around lines 66 - 75,
Update the workflow’s reusable-workflow invocation to pass
fork-pr-author-association, fork-pr-author, and pr-number from the pull request
event, then add or use the reusable workflow’s authorization guard to reject
unauthorized fork PRs before provision executes. Keep repository and ref
selection unchanged for authorized runs.
PR osac-project#110 (OSAC-2801) added this suite auto-triggered on every PR. It started failing on every PR (including unrelated ones) with 'gh: Resource not accessible by personal access token (HTTP 403)' when registering the ephemeral EC2 instance as a JIT GitHub Actions runner -- a separate, still-open token/permissions problem, not addressed here. PR osac-project#166 reverted it entirely rather than leave every PR red. Restores the original file (git show 4416ff2) with one substantive change: the changes job's should-run output now also requires github.run_attempt != '1' on pull_request events, so the suite is skipped (not run) on a PR's automatic first check run. schedule/ workflow_dispatch/merge_group are untouched -- always real runs. github.run_attempt is 1 for a workflow's original run and 2+ for each rerun. osac-test-infra's existing shared slash-command-handler.yml (unchanged) already finds an existing PR-triggered run for a given workflow at the PR's head SHA and reruns it on /test <workflow> -- that's exactly attempt 2+. Keeping on: pull_request means there's always a run to find and rerun. No new mechanism, no shared-code changes, no effect on e2e-bmaas/e2e-vmaas/e2e-caas-full-install's own /test commands.
7b049ab to
c24631e
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/test e2e-caas-netris-full-install |
|
@eliorerz: No presubmit jobs available for osac-project/osac@main DetailsIn response to this:
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 kubernetes-sigs/prow repository. |
|
Triggered: E2E CaaS Netris Full Install |
…nstall Mirrors e2e-caas-full-install.yml's own caller: passes fork-pr-author-association/-author/pr-number so the reusable workflow can gate fork PR execution behind the same authorize-fork-pr check every sibling e2e suite already uses. Depends on osac-test-infra#324 (adds the corresponding inputs and gate to the reusable workflow) merging first -- this workflow is on-demand only (gated on github.run_attempt != '1'), so there's no risk to automatic CI in the meantime; it would only affect someone running /test e2e-caas-netris-full-install before that companion PR lands.
|
Fixed in 7f058e7 — this workflow had zero fork-PR authorization (unlike every other e2e suite here), confirmed by checking the reusable workflow in osac-test-infra directly: no fork-pr-author-association/-author/pr-number inputs, no authorize-fork-pr call at all. Real fix needed a companion change since the reusable workflow itself had nothing to hook into: opened osac-test-infra#324 to add the same inputs + the same Authorize fork PR gate (mirroring e2e-bmaas/vmaas/caas-full-install exactly, same position -- right after setup, before any Vault secret fetch). This PR's commit passes those inputs from the caller side. Ordering note: this depends on osac-test-infra#324 merging first. No risk to automatic CI in the meantime since this workflow only runs on-demand (github.run_attempt != '1' gate) -- it would only matter if someone ran /test e2e-caas-netris-full-install before #324 lands, in which case it'd fail clearly on an invalid-input error rather than silently skipping the auth check. |
|
/test e2e-caas-netris-full-install |
|
@eliorerz: No presubmit jobs available for osac-project/osac@main DetailsIn response to this:
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 kubernetes-sigs/prow repository. |
2 participants
Summary
PR #110 (OSAC-2801) added e2e-caas-netris-full-install.yml (provisions an ephemeral EC2 c5n.metal box, runs a full Netris Spectrum-X simulated lab + OCP SNO + OSAC install, then the CaaS suite), auto-triggered on every PR. It started failing on every PR -- including totally unrelated ones -- with
gh: Resource not accessible by personal access token (HTTP 403)when registering the ephemeral EC2 instance as a JIT GitHub Actions runner. That's a separate, still-open token/permissions problem, not addressed by this PR. #166 reverted the whole workflow rather than leave every PR red.This restores the original file (
git show 4416ff26:.github/workflows/e2e-caas-netris-full-install.yml) verbatim, with exactly one substantive change: thechangesjob'sshould-runoutput now also requiresgithub.run_attempt != '1'onpull_requestevents, so the suite is skipped (not run) on a PR's automatic first check run.schedule/workflow_dispatch/merge_grouptriggers are untouched -- those always run for real. Also adds a log step (pull_request + attempt 1 only) explaining how to actually trigger it.Why this works with zero shared-code changes:
github.run_attemptis1for a workflow's original run and2+for eachgh run rerun.osac-test-infra's existing sharedslash-command-handler.yml(untouched) already finds an existing PR-triggered run for a given workflow at the PR's current head SHA and reruns it on/test <workflow>-- that's exactly attempt 2+. Keepingon: pull_requestmeans there's always a run for the handler to find and rerun; no new trigger mechanism, and zero effect on e2e-bmaas/e2e-vmaas/e2e-caas-full-install's own/testcommands (their own workflows and the shared handler are both unmodified).Test plan
yamllint --strict,actionlint, fullpre-commit runall passE2E CaaS Netris Full Installcompleting quickly as skipped on the initial push (not running, not red) -- proves the JIT-runner-403 path is no longer hit automatically/test e2e-caas-netris-full-installon this PR and confirm the existing slash-command handler finds and reruns the same check (not a new one), and that attempt 2 actually starts the real job (reaches EC2 provisioning) -- expected to still hit the separate JIT-runner-403 issue at that point, which is out of scope here; this only proves it's on-demand now, not that the suite passes end-to-endSummary by CodeRabbit