Skip to content

feat: add GitOps deployment via OpenShift GitOps (ArgoCD) #314

Open
luis5tb wants to merge 8 commits into
RHEcosystemAppEng:mainfrom
luis5tb:gitops-argocd-deployment
Open

feat: add GitOps deployment via OpenShift GitOps (ArgoCD) #314
luis5tb wants to merge 8 commits into
RHEcosystemAppEng:mainfrom
luis5tb:gitops-argocd-deployment

Conversation

@luis5tb

@luis5tb luis5tb commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add deploy/gitops/ with ArgoCD-managed deployment configs for two targets:
    OpenShift (syncs existing Helm chart) and Google Cloud (triggers Cloud Build for Cloud Run)
  • PR-based workflow: change image tags or config → merge → ArgoCD auto-syncs → redeployment

OpenShift target (deploy/gitops/openshift/)

  • ArgoCD Application CR (multi-source) pointing to the existing deploy/openshift/ Helm chart
  • Environment-specific values-override.yaml for image tags, deployment mode, provider URL

Google Cloud target (deploy/gitops/google-cloud/)

  • Helm chart that ArgoCD manages on OpenShift to trigger GCP deployments
  • ConfigMap maps all 26 Cloud Build substitutions from values.yaml to cloudbuild.yaml variables
  • PostSync hook Job authenticates to GCP and runs gcloud builds submit
  • Jobs hardened: runAsNonRoot, seccompProfile, resource limits, optional git token for private repos

Test plan

  • Verify ArgoCD Application CRs apply cleanly: oc apply -f deploy/gitops/openshift/application.yaml
  • Verify Helm chart templates render: helm template test deploy/gitops/google-cloud/ --set project.id=test
  • Verify deploy Job is triggered as a PostSync hook after ArgoCD sync
  • End-to-end: change image tag in values-override.yaml → merge → confirm Cloud Build triggers

@luis5tb luis5tb force-pushed the gitops-argocd-deployment branch from fe3a1e7 to 5f519bc Compare July 2, 2026 11:13

@r2dedios r2dedios left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just a few observations, but looks great :)

Comment thread deploy/gitops/google-cloud/templates/external-secret.yaml Outdated
Comment thread deploy/gitops/google-cloud/templates/deploy-job.yaml
Comment thread deploy/gitops/google-cloud/templates/deploy-job.yaml Outdated
Comment thread deploy/gitops/google-cloud/templates/external-secret.yaml Outdated
Comment thread deploy/gitops/google-cloud/templates/secret-store.yaml Outdated
Comment thread deploy/gitops/google-cloud/templates/secret-store.yaml Outdated
Comment thread deploy/gitops/google-cloud/application.yaml Outdated
Comment thread deploy/gitops/google-cloud/application.yaml Outdated
@luis5tb luis5tb force-pushed the gitops-argocd-deployment branch 13 times, most recently from fd4573b to 2caf03e Compare July 8, 2026 15:37
@luis5tb luis5tb changed the title [WIP] feat: add GitOps deployment via OpenShift GitOps (ArgoCD) feat: add GitOps deployment via OpenShift GitOps (ArgoCD) Jul 8, 2026
r2dedios
r2dedios previously approved these changes Jul 10, 2026

@r2dedios r2dedios left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great Work! 🚀

@r2dedios r2dedios left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just a couple of questions

Comment thread deploy/cloudrun/service.yaml
Comment thread deploy/gitops/google-cloud/values.yaml
luis5tb and others added 7 commits July 10, 2026 15:16
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add Helm chart and setup scripts for ArgoCD-managed deployment of the
Lightspeed Agent on Google Cloud (Cloud Run). ArgoCD manages intermediate
K8s resources on OpenShift; a PostSync hook Job triggers Cloud Build to
deploy Cloud Run services.

Key design decisions:
- Uses --no-source with an injected git clone build step to avoid VPC
  Service Controls blocking the default Cloud Build bucket
- Cloud Build logging set to CLOUD_LOGGING_ONLY (no GCS log bucket needed)
- _SCAN_SEVERITY patched via sed (commas conflict with --substitutions)
- ESO CRD defaults added explicitly to prevent ArgoCD perpetual diffs

Includes:
- deploy/gitops/google-cloud/ — Helm chart (templates, values, helpers)
- deploy/gitops/setup/ — prerequisite scripts (GitOps operator, ESO, GCP SA)
- deploy/gitops/README.md — architecture, setup, secrets management docs

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Store the service account key in GCP Secret Manager so ESO can pull it
automatically when externalSecrets.enabled=true. If the secret already
exists, a new version is added.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The bootstrap K8s secret (gcp-sa-bootstrap) already contains the GCP SA
key needed by the deploy Job. ESO was just pulling a redundant copy of
the same key from GCP Secret Manager.

Removed:
- external-secret.yaml, secret-store.yaml templates
- install-eso-operator.sh setup script
- externalSecrets values block and secretmanager.secretAccessor role
- GCP Secret Manager upload step from setup-gcp-sa.sh

The deploy Job now mounts gcp-sa-bootstrap directly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix GitOps repo URL inconsistency in CLAUDE.md (luis5tb -> RHEcosystemAppEng)
- Remove incorrect ESO mention in CLAUDE.md
- Fix --help sed pattern in both setup scripts to show full help text
- Fix setup/README.md Quick Start paths to clarify GitOps repo origin
- Move bootstrap secret from 'Resources created' to 'Prerequisites' in NOTES.txt
- Add _SCAN_SEVERITY input validation and post-sed verification
- Add GIT_BRANCH character validation
- Add post-sed verification for clone step injection
- Add git token exposure warning for private repos
- Add readOnlyRootFilesystem with emptyDir for /tmp
- Remove unnecessary secrets access from RBAC Role
- Add nameOverride/fullnameOverride to values.yaml
- Add key rotation guidance to setup-gcp-sa.sh

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Document what collides when deploying multiple agent instances to
the same GCP project, what is shared by design, and step-by-step
instructions for setting up staging + prod instances with
independent ArgoCD Applications and per-instance resource names.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Parameterize GEMINI_MODEL and AGENT_LOGGING_DETAIL in Cloud Run
service.yaml so they can be overridden via Cloud Build substitutions
and GitOps values. Make the MCP --debug flag conditional in both
OpenShift (Helm) and Cloud Run (sed-based placeholder) deployments.

Cloud Run / GitOps pipeline:
- service.yaml: replace hardcoded values with substitution variables
- cloudbuild.yaml: add _GEMINI_MODEL, _AGENT_LOGGING_DETAIL, _MCP_DEBUG
- deploy.sh: add matching sed substitutions for manual deploys
- gitops chart: wire new values through values.yaml, ConfigMap, and
  deploy Job SUBSTITUTIONS

OpenShift chart:
- Wrap --debug in conditional (mcp.debug, defaults to true to preserve
  current behavior)
- GEMINI_MODEL and AGENT_LOGGING_DETAIL were already parameterized

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@luis5tb luis5tb force-pushed the gitops-argocd-deployment branch from 25c5592 to 1ce7396 Compare July 10, 2026 13:23
@luis5tb luis5tb requested a review from r2dedios July 10, 2026 13:24
@yuvalk

yuvalk commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

All fixes are implemented, tested, and committed locally on gitops-argocd-deployment (not pushed). Here's the review, formatted to paste into GitHub:


Review of #314 — GitOps deployment via OpenShift GitOps (ArgoCD)

Reviewed by Claude (Anthropic) via automated code review at the requester's direction. Each comment includes a fix commit created on the PR branch — SHAs below, not yet pushed.

🔴 Critical

1. PostSync deploy Job cannot start — runAsNonRoot: true with no runAsUser, but the image runs as root
deploy/gitops/google-cloud/templates/deploy-job.yaml (pod securityContext)
google/cloud-sdk:slim ships no USER directive (verified via skopeo inspectUser field is empty, defaults to UID 0). With runAsNonRoot: true and no runAsUser set, the kubelet refuses to start the container (CreateContainerConfigError: container has runAsNonRoot and image will run as root). The entire mechanism this PR builds — the PostSync hook that triggers Cloud Build — cannot run as shipped, despite the PR description claiming "Jobs hardened: runAsNonRoot..." and checking off an end-to-end test.
Fix: ac6afc4 — pins runAsUser: 1000, runAsGroup: 1000, fsGroup: 1000.

🟠 Important

2. run.admin grant likely has no effect — gcloud builds submit doesn't pin --service-account
deploy/gitops/setup/setup-gcp-sa.sh, deploy/gitops/google-cloud/templates/deploy-job.yaml
Per Google's own docs, Cloud Build executes build steps under whichever service account is passed via --service-account — or the project's default executor SA if omitted — never under the identity that called gcloud builds submit. Since deploy-job.yaml's gcloud builds submit call doesn't pass --service-account, the roles/run.admin/roles/serviceusage.serviceUsageConsumer granted to lightspeed-gitops in setup-gcp-sa.sh don't apply to the identity actually running gcloud run services replace. The pipeline was silently depending on the project's default executor SA already having sufficient permissions from elsewhere.
Fix: c5f5160 — pins --service-account on the build submission and adds the required self-actAs grant, updates docs.

3. IAM role grants silently swallow failures
deploy/gitops/setup/setup-gcp-sa.sh
Every add-iam-policy-binding call ends in --quiet || true. A failed grant (insufficient caller permissions, API not enabled) produces no warning, and the script's closing summary unconditionally declares "Setup Complete" and lists all roles as granted regardless of what actually succeeded.
Fix: c5f5160 (same commit as #2) — tracks failures explicitly, exits non-zero with a clear summary instead.

4. GEMINI_MODEL default silently changed from gemini-2.5-flash to gemini-3.5-flash, undocumented
cloudbuild.yaml, deploy/cloudrun/deploy.sh, deploy/gitops/google-cloud/values.yaml
Three files introduce gemini-3.5-flash as the new deploy-time default, with no mention in the PR description. src/lightspeed_agent/config/settings.py:75 and the docs (CLAUDE.md, docs/configuration.md) still document gemini-2.5-flash as the default — this leaves docs/code out of sync and is an undocumented behavior change riding along in a "GitOps config" PR.
Fix: d8d1ac2 — reverts the deploy-time default to gemini-2.5-flash, consistent with app config and docs. If a model bump is intended, it should be its own PR that also updates settings.py and the docs.

5. Git PAT exposure is broader than the existing warning acknowledges
deploy/gitops/google-cloud/templates/deploy-job.yaml
The in-container git clone embeds the token directly in the URL, exposing it via process arguments (ps//proc/*/cmdline) to anything else with visibility into the pod. Separately, the injected Cloud Build clone step embeds the token in step args, which persist in the Build resource itself (visible via gcloud builds describe/Console/API to anyone with cloudbuild.builds.viewer) — a broader and longer-lived exposure than the existing comment's "Cloud Logging access" framing suggests.
Fix: 6ea500c — switches the in-container clone to a credential helper (token never on that process's argv), and rewrites the warning to correctly describe the residual Cloud-Build-side exposure with the concrete Secret-Manager-based fix, flagged as follow-up work since it needs new GCP resource provisioning to implement and validate properly.

6. Unused RBAC grant on the deploy Job's ServiceAccount
deploy/gitops/google-cloud/templates/serviceaccount.yaml
The Role grants get/list/watch on all ConfigMaps in the namespace, but the deploy Job's container command never calls the Kubernetes API — its config is injected via envFrom, which the kubelet resolves at pod admission using its own credentials, not the pod's ServiceAccount token. This is a dead permission that gives anything with access to the pod's token read access to every ConfigMap in a shared namespace.
Fix: 194c1a7 — removes the Role/RoleBinding, keeps the bare ServiceAccount.

7. New deploy.sh branching logic has no test coverage
deploy/cloudrun/deploy.sh, tests/shell/
CLAUDE.md explicitly requires test updates for deploy.sh changes. The new MCP_DEBUG if/else (insert vs. delete the --debug placeholder via sed) shipped with zero new tests — existing coverage only calls deploy_agent generically via an unrelated LB test, so CI's function-coverage check didn't catch the gap.
Fix: ba73f65 — adds two bats tests asserting both branches; verified passing locally (25/25 tests green).

🟡 Suggestions

8. Helm Job name has no truncation headroom for its appended suffix
deploy/gitops/google-cloud/templates/_helpers.tpl
chart.fullname truncates to the full 63-char K8s label limit, but deploy-job.yaml and deployment-config.yaml append further suffixes (-deploy-{revision}, -deploy-config) on top of it. For sufficiently long release names — plausible given this PR's own multi-instance/multi-environment guidance — the combined name exceeds 63 chars and breaks pod creation (the auto-generated job-name label). Not triggered by the PR's own documented example names, but a real latent edge case.
Fix: 5ddc8d5 — truncates to 49 chars, verified a deliberately long release name now renders a 58-char Job name.

9. GIT_REPO isn't regex-validated before being spliced into injected YAML
deploy/gitops/google-cloud/templates/deploy-job.yaml
GIT_BRANCH and _SCAN_SEVERITY are validated before being spliced into single-quoted injected YAML, but GIT_REPO (and the token-bearing CLONE_URL derived from it) wasn't — inconsistent with the defensive pattern already used in the same file. Low real-world risk since it's Helm-values-controlled.
Fix: 50898c5 — adds the same validation pattern.

10. PR description's test-plan checklist references a path that doesn't exist in this repo
oc apply -f deploy/gitops/openshift/application.yaml is checked off, but per this PR's own new CLAUDE.md section, Application CRs live only in the separate GitOps repo — deploy/gitops/openshift/ doesn't exist here. Likely just a copy-paste artifact in the description, not a code issue — no fix commit for this one.

✅ Strengths

  • Container hardening on the deploy Job is otherwise well composed: allowPrivilegeEscalation: false, capabilities: drop: [ALL], readOnlyRootFilesystem: true, seccompProfile: RuntimeDefault, and HOME=/tmp + the tmp emptyDir correctly gives gcloud/git a writable home under a read-only rootfs.
  • setup-gcp-sa.sh handles the SA key file carefully: umask 077 + chmod 600, deletes it only after confirming the K8s secret exists, and scopes iam.serviceAccountUser to the specific Cloud Run runtime SA rather than project-wide.
  • _SCAN_SEVERITY and (now) GIT_BRANCH/GIT_REPO injection into the generated cloudbuild.yaml is defensively validated.
  • ${VAR}/$${VAR} Cloud Build escaping is correct end-to-end for the new GEMINI_MODEL/MCP_DEBUG_FLAG substitutions.
  • .gitignore correctly excludes deploy/gitops/google-cloud/secrets.yaml from being committed.

Verdict: Request Changes

The Critical finding (#1) means the deploy Job as shipped cannot run at all — this alone blocks merge. #2#7 are real correctness/security gaps worth fixing before this becomes the standard deployment path. All fixes above are implemented as separate commits on this branch, each verified (helm template/helm lint, shellcheck, bash -n, and the full bats suite — 25/25 passing).


@yuvalk

yuvalk commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

luis5tb#8

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.

3 participants