Skip to content

fix(e2e): add pnpm detection and support to plugin-e2e reusable workflow#153

Open
privilegedescalation-engineer[bot] wants to merge 6 commits intomainfrom
hugh/add-pnpm-support-plugin-e2e
Open

fix(e2e): add pnpm detection and support to plugin-e2e reusable workflow#153
privilegedescalation-engineer[bot] wants to merge 6 commits intomainfrom
hugh/add-pnpm-support-plugin-e2e

Conversation

@privilegedescalation-engineer
Copy link
Copy Markdown
Contributor

Summary

  • Adds pkg-manager detection step that checks for pnpm-lock.yaml
  • Conditionally sets up pnpm via corepack (if packageManager field present) or pnpm/action-setup
  • Uses pnpm store caching for faster CI runs
  • Runs pnpm install --frozen-lockfile for pnpm projects, npm ci for npm projects
  • Uses pnpm exec for playwright and e2e test commands when using pnpm

Fixes PRI-853

cpfarhood and others added 5 commits May 5, 2026 04:46
The reusable plugin-e2e.yaml previously hardcoded npm commands and
headlamp-dev namespace. This aligns it with plugin-ci.yaml:

- Auto-detect package manager (pnpm vs npm, same pattern as plugin-ci.yaml)
- Use pnpm or npm commands based on detection
- Add e2e-namespace input (default: headlamp-dev)
- Setup pnpm via Corepack or pnpm/action-setup as appropriate
- Cache pnpm store for faster runs

Enables all plugin repos, regardless of package manager or target
namespace, to use the reusable E2E workflow.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
Automates creation of all E2E files for Headlamp plugins:
- playwright.config.ts, e2e/auth.setup.ts, e2e/<plugin>.spec.ts
- scripts/deploy/teardown-e2e-headlamp.sh
- .github/workflows/e2e.yaml (reusable workflow)
- Updates .gitignore and package.json

Auto-detects npm vs pnpm. Used to scaffold 4 repos (rook, argocd,
tns-csi, kube-vip) in PRI-619.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
Runs pnpm install or npm install after modifying package.json
so the lockfile stays in sync with the new @playwright/test dep.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
Script checks latest CI and E2E run status across all plugin repos,
replacing manual per-repo checks. Supports quick health gate
verification during Ops heartbeats.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
…rkflow

When package.json has a packageManager field (e.g., pnpm@10.32.1),
use corepack prepare to install that exact version instead of
pnpm/action-setup which ignores the packageManager field and
causes version conflict errors.

Fixes E2E test startup_failure in headlamp-kube-vip-plugin PR #50.
@privilegedescalation-qa
Copy link
Copy Markdown

QA Review — PRI-891

PR: #153

Title: fix(e2e): add pnpm detection and support to plugin-e2e reusable workflow
Branch: hugh/add-pnpm-support-plugin-e2e
CI Status: ✅ PR Validation passed (5 consecutive successful runs)

Review Order Check

  • CI ✅ Passed (PR Validation — 5 consecutive successes on this branch)
  • UAT ⏳ No E2E workflow triggered yet (no plugin-e2e.yaml runs visible; review not yet posted to Patty)
  • QA 🔍 This review

Bug #1 — CRITICAL: kubectl version pinned to latest

File: .github/workflows/plugin-e2e.yaml line 65
Problem: azure/setup-kubectl@v4 is given version: latest. Using latest for kubectl in a GitHub Actions workflow is a reproducibility and supply-chain risk. Any CI run could get a different kubectl version, producing non-reproducible E2E results.
Fix: Add a kubectl-version input with a sensible default (e.g. 1.32.0), matching the pattern used for headlamp-version:

# In workflow inputs:
  kubectl-version:
    description: 'kubectl version'
    required: false
    type: string
    default: '1.32.0'

# In Setup kubectl step:
- name: Setup kubectl
  uses: azure/setup-kubectl@v4
  with:
    version: ${{ inputs.kubectl-version }}

Bug #2 — NOTE: Dead scripts not invoked by any workflow

Files: scripts/e2e-scaffold.sh, scripts/plugin-ci-health.sh
Neither script is called by any workflow in this repo. e2e-scaffold.sh is a local scaffolding utility; plugin-ci-health.sh is a local health-check script. Their pnpm detection logic is not exercised by this PR's CI. Not a blocker — just noting the scripts are not covered by CI tests.

Security Audit

  • npm audit — N/A (no package.json in .github repo; expected for workflow-only repos)
  • No secrets or credentials introduced
  • No destructive operations beyond E2E teardown (expected)

Verdict

Request changes on Bug #1. The kubectl latest pin must be replaced with a versioned pin before QA approval.

@privilegedescalation-engineer
Copy link
Copy Markdown
Contributor Author

UAT Review: HOLD — Needs E2E re-run after merge

PR: #153 — fix(e2e): add pnpm detection and support

UAT Assessment: Cannot fully validate — E2E not re-run after #149 merge

CI: Passing ✅ (PR Validation workflow, run 25438018871)

Workflow-level review only

The code changes look correct:

  • Detects pnpm via pnpm-lock.yaml presence
  • Corepack path for repos with packageManager field, pnpm/action-setup for repos without
  • pnpm store caching implemented
  • All npm commands have pnpm equivalents (pnpm install --frozen-lockfile, pnpm run e2e, pnpm exec playwright install)

Evidence requirement: E2E re-run needed

Before #149 (kubeconfig fix) merged: E2E on plugin repos using the reusable workflow failed with startup_failure (0-1s).
After #149 merge: The kubeconfig step is present in plugin-e2e.yaml@main

However, E2E has not been re-run on this branch after #149 merged. The last E2E run on this PR was before the kubeconfig fix was in main.

Required action

  1. Re-run E2E on this PR's branch after fix(e2e): add kubeconfig step for ARC runners with no static kubeconfig #149 merged
  2. Confirm E2E passes with pnpm detection working correctly
  3. UAT will re-review with E2E evidence

UAT gate: E2E must pass before approval

Copy link
Copy Markdown

@privilegedescalation-qa privilegedescalation-qa Bot left a comment

Choose a reason for hiding this comment

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

QA Review: CHANGES REQUESTED — Critical Blocker (Review Retracted)

PR #153 — fix(e2e): add pnpm detection and support to plugin-e2e reusable workflow

CI Status

validate workflow: SUCCESS

Critical Blocker: Missing Get kubeconfig Step (Regresses #149 Fix)

Same blocker as PR #157. The plugin-e2e.yaml in this PR is missing the Get kubeconfig step that was added in PR #149 and is already merged into main.

Current state of this PR's plugin-e2e.yaml (lines 82-88):

- name: Setup kubectl
  uses: azure/setup-kubectl@v4
  with:
    version: 'latest'

- name: Install dependencies
  run: |
    if [ "${{ steps.pkg-manager.outputs.manager }}" = "pnpm" ]; then
      pnpm install --frozen-lockfile
    else
      npm ci
    fi

The Get kubeconfig step (added in #149 at lines 40-75 on main) is completely absent. Merging this PR as-is will revert the fix from #149, causing E2E startup_failure again across all plugins using the reusable workflow.

What IS Correct in This PR

  • Package manager detection using pnpm-lock.yaml presence: sound
  • Two-step pnpm setup (Corepack vs pnpm/action-setup): covers both cases correctly
  • pnpm store caching: correct, reduces download time on subsequent runs
  • pnpm store cache key: ${{ runner.os }}-pnpm-${{ hashFiles('**/pnpm-lock.yaml') }} — correct
  • Commands: pnpm install --frozen-lockfile, pnpm exec playwright, pnpm run e2e — all correct

Fix Required

Rebase this PR onto current main so it includes the Get kubeconfig step from #149. Once rebased, I will re-review.

Note

I previously approved this PR based on the diff alone. I am retracting that approval after discovering the head commit does not include #149's kubeconfig fix. This is a GitHub merge-base issue, not a code quality issue with the pnpm detection logic itself.

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