test "kustomize build" for overlays before merge#841
Conversation
ktdreyer
force-pushed
the
kdreyer/kustomize-ci
branch
from
cc9062b to
33f30f1
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
I decided to go with a hard-coded overlay list because we should not be changing those without notice. Several things depend on them (ie make files, other GHA workflows, etc) |
ktdreyer
changed the title
ktdreyer
force-pushed
the
kdreyer/kustomize-ci
branch
3 times, most recently
from
5b354e1 to
d2aceb6
Compare
|
PR 841 adds kustomize-check.yml CI workflow. Review below. BLOCKER: None. CRITICAL: None. MAJOR: None. MINOR 1 - Non-deterministic oc version (.github/workflows/kustomize-check.yml:18): oc: latest picks up new releases silently on each run. A future oc version changing kustomize behavior could break CI without any manifest change. Pin to a specific version (e.g. 4.17) for reproducible builds. MINOR 2 - Floating action tag (.github/workflows/kustomize-check.yml:16): redhat-actions/openshift-tools-installer@v1 is a mutable tag. Pin to an immutable commit SHA for supply-chain hygiene. Standard: security-standards.md. POSITIVE HIGHLIGHTS: RECOMMENDATIONS: (1) Pin oc to a specific version. (2) Pin installer action to a commit SHA. Both minor. PR is ready to merge. -- Review by Claude Code (Amber review system) |
This comment has been minimized.
This comment has been minimized.
|
test body no special chars |
|
Claude Code Review -- PR 841 kustomize-check.yml SUMMARY: PR 841 adds a GitHub Actions workflow to validate kustomize build for all five deployment overlays (production, e2e, kind, kind-local, local-dev). This closes a real CI gap where three overlays had no automated syntax checking. Functionally correct, but several inconsistencies with repo conventions should be addressed. BLOCKER ISSUES: None CRITICAL ISSUES: None MAJOR ISSUES:
MINOR ISSUES:
POSITIVE HIGHLIGHTS:
RECOMMENDATIONS (priority order):
Review performed by Claude Code using repository standards from the Amber memory system. |
Ambient Code PlatformKubernetes-native AI automation platform that orchestrates agentic sessions through containerized microservices. Built with Go (backend, operator), NextJS + Shadcn (frontend), Python (runner), and Kubernetes CRDs.
Structure
Key Files
Session FlowCommandsmake build-all # Build all container images
make deploy # Deploy to cluster
make test # Run tests
make lint # Lint code
make kind-up # Start local Kind cluster
make test-e2e-local # Run E2E tests against KindPer-Component# Backend / Operator (Go)
cd components/backend && gofmt -l . && go vet ./... && golangci-lint run
cd components/operator && gofmt -l . && go vet ./... && golangci-lint run
# Frontend
cd components/frontend && npm run build # Must pass with 0 errors, 0 warnings
# Runner (Python)
cd components/runners/ambient-runner && uv venv && uv pip install -e .
# Docs
cd docs && npm run dev # http://localhost:4321Critical Context
Pre-commit HooksThe project uses the pre-commit framework to run linters locally before every commit. Configuration lives in Installmake setup-hooksWhat RunsOn every
On every
Run Manuallymake lint # All hooks, all files
pre-commit run gofmt-check --all-files # Single hook
pre-commit run --files path/to/file.go # Single fileSkip Hooksgit commit --no-verify # Skip pre-commit hooks
git push --no-verify # Skip pre-push hooksNotes
Testing
More InfoSee BOOKMARKS.md for architecture decisions, development context, code patterns, and component-specific guides. |
|
Deleting test comments - please ignore the previous 3 comments on this PR. |
ktdreyer
force-pushed
the
kdreyer/kustomize-ci
branch
from
af25ec3 to
d748c83
Compare
Claude Code ReviewSummaryThis PR adds a new GitHub Actions workflow to validate Issues by SeverityBlocker IssuesNone Critical IssuesNone Major IssuesKustomize error output is suppressed, making failures undiagnosable in CI
if oc kustomize "components/manifests/overlays/$overlay" > /dev/null; thenRedirecting stdout to Suggested fix: for overlay in production e2e kind kind-local local-dev; do
echo "=== $overlay ==="
if oc kustomize "components/manifests/overlays/$overlay"; then
echo "OK"
else
echo "FAILED"
failed=1
fi
echo ""
doneRemoving the redirect lets kustomize output stream directly to the CI log, which is suppressed on success anyway (GitHub Actions collapses passing step output) and visible when the step fails. Minor Issues
oc: latestUsing Positive Highlights
Recommendations
🔍 View AI decision process (logs available for 90 days) 📋 View memory system files loaded (click to expand)What Amber Loaded for Code ReviewAmber automatically loaded these repository standards from the memory system:
Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines. |
Claude Code ReviewSummaryThis PR adds a GitHub Actions workflow to validate Issues by SeverityBlocker IssuesNone. Critical IssuesNone. Major Issues1. Using Suggested fix: - name: Install oc
uses: redhat-actions/openshift-tools-installer@v1
with:
oc: "4.17" # pin to the version the project currently targets2. Hardcoded overlay list will silently miss future overlays The loop hardcodes Suggested fix — discover overlays dynamically: for overlay in components/manifests/overlays/*/; do
overlay_name=$(basename "$overlay")
echo "=== $overlay_name ==="
if oc kustomize "$overlay" > /dev/null; thenMinor Issues1. No explicit GitHub Actions security best practice is to declare minimal explicit permissions. A read-only kustomize check only needs permissions:
contents: read2. Double-run on PR branches With both push:
branches:
- main
paths:
- components/manifests/**
- .github/workflows/kustomize-check.ymlPositive Highlights
Recommendations
🔍 View AI decision process (logs available for 90 days) 📋 View memory system files loaded (click to expand)What Amber Loaded for Code ReviewAmber automatically loaded these repository standards from the memory system:
Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines. |
|
Addressing the Claude Code review items above:
I did this by design to reduce maintenance.
I did this by design. It's a safety check in case we delete overlays accidentally. I did not want to dynamically discover overlays because I don't anticipate this section of the code changing that much over time.
The default is
This is by design, so that I can test the action while I push to work-in-progress branches. |
Three of our five overlays (kind, kind-local, local-dev) had no CI validation. They can easily break silently if we change something in shared base resources. Add basic syntax checking. Closes #840 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ktdreyer
force-pushed
the
kdreyer/kustomize-ci
branch
from
d748c83 to
d7d3875
Compare
|
(rebased to latest |
WalkthroughA new GitHub Actions workflow is added to validate that all Kustomize overlays (production, e2e, kind, kind-local, local-dev) still build when pull requests modify files under Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 |
🧪 CI InsightsHere's what we observed from your CI run for d7d3875. 🟢 All jobs passed!But CI Insights is watching 👀 |
![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 the current code and only fix it if needed.
Inline comments:
In @.github/workflows/kustomize-check.yml:
- Around line 22-27: Replace the mutable tags actions/checkout@v6 and
redhat-actions/openshift-tools-installer@v1 with full-length commit SHAs to pin
those steps; update the "uses" lines for both the checkout action and
redhat-actions/openshift-tools-installer to use their specific commit SHAs (you
may append a trailing comment like "# vX.Y.Z" for clarity/Dependabot), ensuring
both occurrences are replaced so the workflow is fully pinned and reproducible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 862ccda8-aa7e-44e6-8e86-02ae834754db
📒 Files selected for processing (1)
.github/workflows/kustomize-check.yml
| - uses: actions/checkout@v6 | ||
|
|
||
| - name: Install oc | ||
| uses: redhat-actions/openshift-tools-installer@v1 | ||
| with: | ||
| oc: latest |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/kustomize-check.yml | head -40Repository: ambient-code/platform
Length of output: 1319
🌐 Web query:
GitHub Actions security hardening pinning commit SHA major version tags
💡 Result:
Why pin to a commit SHA (vs @v1, @main, etc.)?
- Tags and branches are mutable. GitHub’s own Actions versioning guidance explicitly describes major-version tags (for example
v1) as pointers that are moved forward to the latest minor/patch release—so@v1can change what code you run without a workflow change. [1] - A full commit SHA is immutable, so your workflow is reproducible and resistant to “tag moved / repo compromised” supply-chain scenarios. GitHub’s security guidance recommends pinning to a full-length commit SHA, and GitHub now supports enforcing this via policy. [2][3]
Recommended hardening pattern
Pin to a full SHA, and keep human context in a comment:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.7Dependabot supports GitHub Actions updates and will update workflows (including updating version comments when present). [4][5]
Enforce it org/enterprise-wide (optional but strong)
If you’re on GitHub Enterprise Cloud, you can set an Actions policy to require actions and reusable workflows be pinned to a full-length commit SHA (workflows that aren’t pinned will fail). [2][3]
Practical compromise
- For internal/private actions, SHA pinning is still best; pair it with Dependabot so updates are deliberate and reviewable. [4]
- For third-party actions, SHA pinning is the default “secure-by-design” choice; major tags are primarily a convenience for auto-updates, not immutability. [1][3]
Sources: GitHub Docs on immutable releases/tags and major tags moving [1]; GitHub Enterprise policy option to require SHA pinning [2]; GitHub Changelog on SHA-pinning enforcement support (Aug 15, 2025) [3]; Dependabot docs for keeping actions up to date [4]; GitHub Changelog on Dependabot updating version comments for SHA-pinned actions [5].
Pin both action references to full commit SHAs.
actions/checkout@v6 and redhat-actions/openshift-tools-installer@v1 use mutable major-version tags. Per GitHub's security hardening guidance, these can be retargeted by upstream maintainers without your workflow changing. Pin to full-length commit SHAs (with optional version comments for clarity with Dependabot) to ensure reproducibility and prevent supply-chain exposure.
Example: uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.7
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/kustomize-check.yml around lines 22 - 27, Replace the
mutable tags actions/checkout@v6 and redhat-actions/openshift-tools-installer@v1
with full-length commit SHAs to pin those steps; update the "uses" lines for
both the checkout action and redhat-actions/openshift-tools-installer to use
their specific commit SHAs (you may append a trailing comment like "# vX.Y.Z"
for clarity/Dependabot), ensuring both occurrences are replaced so the workflow
is fully pinned and reproducible.
Merge Queue Status
This pull request spent 5 seconds in the queue, with no time running CI. Required conditions to merge
|
Review Queue — Blockers Found
|
2 participants
Three of our five overlays (
kind,kind-local,local-dev) had no CI validation. They can easily break silently if we change something in shared base resources.Add basic syntax checking.
Closes #840