ci: unblock release pipeline — runner label, license fatal, webpack stamp word-split#64
Merged
Conversation
Five release/mirror workflows reference oracle-16cpu-64gb-x86-64 but the currently-online self-hosted runners use oracle-vm-16cpu-64gb-x86-64. Confirmed by perf_clickhouse, perf_soc_attack, and build_and_test which all run cleanly on the -vm- label. release/cloud/v0.0.10-pre-v0.0 queued for 1h+ under the legacy label before being retried after the fix. Patches: cloud_release.yaml, vizier_release.yaml, operator_release.yaml, cli_release.yaml, mirror_deps.yaml.
The release pipeline trips on this every time main pulls in new
transitive Go deps faster than manual_licenses.json is curated.
manual_licenses.json has 37 entries; v0.0.10-pre-v0.0 hit 38 newly
missing modules, blocking the release on an unrelated thing.
Drop the stamped-build fatal gate (was: disallow_missing = select(
{"//bazel:stamped": True, "//conditions:default": False})). Missing
licenses are still recorded in {go,deps}_licenses_missing.json so
the gap is visible; a follow-up can curate the backlog without
holding releases hostage.
ROOT CAUSE of release/cloud/v0.0.10-pre-v0.0 failure:
The webpack actions eval workspace_status_command output via
\$(sed -E "s/^([A-Za-z_]+)\\s*(.*)/export \\1=\\2/g" "stable-status.txt")
to import stamp vars into the action env. workspace_status_command
emits FORMATTED_DATE whose value is space-separated
("2026 Jun 18 22 06 02 Thu"). \$(...) command substitution
word-splits the sed output BEFORE single quotes can protect the
value, so bash sees `export FORMATTED_DATE=2026 Jun 18 ...` and
tries to also `export 18`, `export 22`, `export 02`, ... — all
failing with "not a valid identifier" on line 1. The action aborts
before any cp / tar / yarn runs.
Every prior "yarn failed silently" symptom was this. The file's own
comment called it out:
"Hopefully, no special characters/spaces/quotes in the results ..."
Filter the sed with -n + /p to emit only the two vars
webpack.config.js' EnvironmentPlugin actually reads
(STABLE_BUILD_TAG = version string, BUILD_TIMESTAMP = unix
timestamp). Both space-free, so no quoting gymnastics needed.
Also belt-and-braces: invoke yarn by absolute path
(/opt/px_dev/tools/node/bin/yarn). bazel's
--incompatible_strict_action_env strips host PATH from actions
even with use_default_shell_env=True, so a bare `yarn` doesn't
reliably resolve. The dev image's chef recipe
(tools/chef/cookbooks/px_dev/recipes/nodejs.rb:32) installs node
there, verified by `which yarn` inside the dev image. Keep the
PATH export so children (webpack → node) can find each other.
Replaces the capture-into-\$output + unquoted-echo pattern with a
direct `yarn build_prod` so any future failure surfaces yarn's
real stderr instead of an empty string.
Verified: release/cloud/v0.0.10-pre-v0.0 with all of these patches
shipped all 15 cloud-* images to ghcr.io/k8sstormcenter and the
bundle layer correctly contains the new dx_evidence_graph script.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughFive GitHub Actions workflow files have their Oracle runner label renamed from ChangesGitHub Actions runner label rename
Bazel UI build strict-env fixes
Kubernetes and Terraform configuration updates
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@bazel/ui.bzl`:
- Line 46: The yarn install command is redirecting its output to build.log
instead of streaming directly, which prevents error messages from appearing in
CI logs when the installation fails. Remove the `&> build.log` redirect from the
yarn install command so its output streams directly to the console, consistent
with how the yarn build_prod command on line 108 is handled. This ensures CI
logs capture actual error messages if yarn install fails.
- Around line 83-93: Add quotes around the captured value in both sed
replacement patterns to ensure the exported environment variables are properly
quoted. In both sed command patterns (one for ctx.info_file.path and one for
ctx.version_file.path), modify the replacement string from export \\1=\\2 to
export \\1="\\2" to defensively quote the value portion against potential spaces
or special characters in future format changes.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b12b9dd9-d2ad-429e-8323-63f787285ca5
📒 Files selected for processing (7)
.github/workflows/cli_release.yaml.github/workflows/cloud_release.yaml.github/workflows/mirror_deps.yaml.github/workflows/operator_release.yaml.github/workflows/vizier_release.yamlbazel/ui.bzltools/licenses/BUILD.bazel
Reviewer caught a dumb belt-and-braces I added when chasing the wrong diagnosis earlier today. The real fix for "yarn: command not found" was a) the export PATH=/opt/px_dev/tools/node/bin:... + use_default_ shell_env=True at the action level (which IS the right fix and stays), and b) the whitelist sed for STABLE_BUILD_TAG / BUILD_TIMESTAMP that unblocked everything. The /opt/px_dev/tools/node/bin/yarn absolute- path commands were redundant once those landed, and they hardcode an install path nobody outside this dev image has. Revert the four absolute-path invocations (yarn install / yarn build_prod / yarn license_check / yarn pnpify) back to bare `yarn`. The export PATH at the top of ui_shared_cmds_start makes them resolve fine on the dev image and on any host that has yarn on PATH. Refresh the stale comment on the yarn build_prod line so it explains the PATH export instead of pretending we still need the absolute path.
entlein
requested changes
Jun 19, 2026
entlein
left a comment
entlein
left a comment
There was a problem hiding this comment.
WHY are you using whitelist again, how many times a day must i tell you to use the word "allowlist"
|
|
||
| cmd = ui_shared_cmds_start + cp_cmds + [ | ||
| 'pushd "$TMPPATH/src/ui" &> /dev/null', | ||
| "yarn install --immutable &> build.log", |
| 'tar -xzf "$BASE_PATH/{}"'.format(ctx.file.deps.path), | ||
| "yarn license_check --excludePrivatePackages --production --json --out $LIC_TMPPATH/checker.json", | ||
| 'yarn pnpify node ./tools/licenses/yarn_license_extractor.js --input=$LIC_TMPPATH/checker.json --output="$BASE_PATH/{}"'.format(out.path), | ||
| "/opt/px_dev/tools/node/bin/yarn license_check --excludePrivatePackages --production --json --out $LIC_TMPPATH/checker.json", |
| "//bazel:stamped": True, | ||
| "//conditions:default": False, | ||
| }), | ||
| disallow_missing = False, |
Drop the verbose explanatory comments added in this branch (set -x, use_default_shell_env, stamp allowlist sed, license drift, yarn PATH). Renamed the one remaining 'whitelist' wording to 'allowlist' per styleguide/inclusive_naming_guide.md before stripping. Lint fixes: - private/cockpit/cloud_ingress.yaml: wrap long external-dns hostname annotation onto two lines (line-length). - terraform/kubernetes/auth0/auth0_import.tf: drop trailing newline.
Signed-off-by: Duck <70207455+entlein@users.noreply.github.com>
Signed-off-by: Duck <70207455+entlein@users.noreply.github.com>
entlein
pushed a commit
that referenced
this pull request
Jun 21, 2026
Consolidates PR #62 (entlein/dx-evidence-graph-viz, 28 commits) onto current main. Squashed because ~13 of its commits were UI-build CI fixes (use_default_shell_env, yarn-by-abs-path, set -x, license/runner-label) that main already got via #64 — a linear rebase collided on bazel/ui.bzl repeatedly; a 3-way squash-merge reconciles them cleanly. Net deliverable: dx_evidence_graph PxL script + vis.json + manifest, vispb/vis.proto graph fields, and the GraphWidget (graph.tsx/graph-utils.ts) edge-label/pin/popup rendering wired to the forensic ClickHouse DSN — the dx→pixie attack-graph viz.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three independent CI fixes that were proven out on the v0.0.10 release tag. Without these on `main`, the next `release/cloud/v0.0.11-pre-v0.0` tag will hit the same silent failures that took us an afternoon to chase last time.
What's in
Test plan
Non-goals
These commits are deliberately CI-only — no schema/code changes. The dx_evidence_graph viz scaffold sitting in PR #62 stays separate so it can iterate on CodeRabbit feedback without holding the CI fixes hostage. Once this lands, PR #62 rebases onto a fixed main.
Type of change
/kind bug