Skip to content

fix(ci): scope model-discovery PR to only include models.json#912

Closed
maskarb wants to merge 9 commits intomainfrom
fix/model-discovery-creds-leak
Closed

fix(ci): scope model-discovery PR to only include models.json#912
maskarb wants to merge 9 commits intomainfrom
fix/model-discovery-creds-leak

Conversation

@maskarb
Copy link
Copy Markdown
Contributor

@maskarb maskarb commented Mar 13, 2026

Summary

  • Add add-paths to create-pull-request so only models.json is committed (prevents GCP WIF credentials file from leaking into the PR)
  • Scope the diff check to only models.json so transient workspace files don't trigger false positives

See #911 for the PR where the credentials file was accidentally included.

Test plan

  • Trigger model-discovery workflow manually and verify the resulting PR only contains models.json

🤖 Generated with Claude Code

maskarb and others added 9 commits March 10, 2026 14:59
Replace QEMU-emulated cross-compilation with native per-architecture
builders (ubuntu-latest for amd64, ubuntu-24.04-arm for arm64) and a
manifest merge step. This eliminates SIGILL crashes caused by QEMU's
incomplete instruction emulation and builds both arches in parallel
for faster CI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Instead of starting jobs for all components and skipping steps inside,
detect-changes now outputs a JSON matrix of only the components that
need building. Unchanged components don't appear as jobs at all,
making the Actions UI much clearer about what actually built.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add missing permissions block to merge-manifests in prod-release-deploy
- Add fail-fast: false to merge-manifests strategy in both workflows
- Document arch-suffixed tag accumulation as known trade-off

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add sync comment on arch suffixes that must match merge-manifests
- Remove cache-to on PR builds to avoid evicting main-branch cache
- Revert cosmetic backend banner change (CI trigger workaround)
- Fix inconsistent 3-space job indent in prod-release-deploy.yaml

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add comment in merge-manifests step pointing back to the build job's
arch matrix where the -amd64/-arm64 suffixes are defined.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The public-api component still exists in the repo and should continue
to be built. It was removed from the deploy steps intentionally but
should not have been removed from the build/merge-manifests matrix.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix dispatch input description: claude-runner → ambient-runner to
  match the matrix entry name, preventing silent no-ops on dispatch
- Add public-api to deploy-to-openshift: output detection, image tag
  determination, and kustomize set image (was built but never deployed)
- Make deploy-with-dispatch conditional: only update operator env vars
  and agent registry ConfigMap for components that were actually built,
  preventing references to non-existent SHA tags
- Make prod release deploy component-aware: iterate built components
  instead of unconditionally setting all image tags, preventing partial
  releases from referencing non-existent release tags

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The create-pull-request action commits all workspace changes by default,
which caused the GCP WIF credentials file to be included in the PR.
Scope both the diff check and add-paths to only the models manifest.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 13, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f7fc8298-35b5-4dc9-96dc-9ec6c66e55da

📥 Commits

Reviewing files that changed from the base of the PR and between dd0bc67 and e22a92f.

📒 Files selected for processing (3)
  • .github/workflows/components-build-deploy.yml
  • .github/workflows/model-discovery.yml
  • .github/workflows/prod-release-deploy.yaml

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting


Walkthrough

Three GitHub Actions workflow files are restructured to implement component-driven build matrices with multi-architecture support. The workflows introduce dynamic component detection, matrix-based parallel builds for multiple architectures, a new merge-manifests job for multi-arch image handling, and conditional deployment logic based on detected/built components.

Changes

Cohort / File(s) Summary
Multi-arch Component Build Workflows
.github/workflows/components-build-deploy.yml, .github/workflows/prod-release-deploy.yaml
Both workflows replace static build processes with dynamic component matrices (amd64, arm64). Introduce detect-changes job with build-matrix/merge-matrix/has-builds outputs, new merge-manifests job for multi-arch image creation, and conditional deployment steps based on built components. Add per-arch image tagging (suffix-based tags like -amd64/-arm64) and dynamic image tag determination. Update kustomize and deployment steps to conditionally apply changes only for rebuilt components.
Model Discovery Workflow
.github/workflows/model-discovery.yml
Narrows diff check to single file path (components/manifests/base/models.json) and updates PR creation to include that file in add-paths. Restricts workflow triggers to only model manifest changes.

Sequence Diagram

sequenceDiagram
    participant GHA as GitHub Actions
    participant DC as detect-changes Job
    participant BCM as Build Component Matrices
    participant BAP as build-and-push Job<br/>(matrix: arch × component)
    participant MM as merge-manifests Job
    participant DO as deploy-to-openshift Job

    GHA->>DC: Trigger workflow
    DC->>BCM: Compute ALL_COMPONENTS<br/>Filter by selection/force-build
    BCM-->>DC: Output BUILD_MATRIX<br/>MERGE_MATRIX<br/>HAS_BUILDS
    
    DC->>BAP: Dispatch matrix<br/>(arch: amd64, arm64)<br/>per-component
    
    par Multi-arch builds
        BAP->>BAP: Build & push with<br/>arch suffix tags<br/>(-amd64, -arm64)
    end
    
    BAP-->>MM: Build results
    MM->>MM: Create multi-arch<br/>manifests per component<br/>using manifest tool
    MM-->>DO: Manifest outputs
    
    DO->>DO: Detect built components
    DO->>DO: Conditionally update<br/>images & registries<br/>only for built components
    DO->>DO: Apply stage/latest tags
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/model-discovery-creds-leak
📝 Coding Plan
  • Generate coding plan for human review comments

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 and usage tips.

@maskarb maskarb closed this Mar 13, 2026
@maskarb maskarb deleted the fix/model-discovery-creds-leak branch March 13, 2026 14:28
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