Open
Conversation
Signed-off-by: red-hat-konflux <konflux@no-reply.konflux-ci.dev>
Signed-off-by: red-hat-konflux <konflux@no-reply.konflux-ci.dev>
Signed-off-by: red-hat-konflux <konflux@no-reply.konflux-ci.dev>
Signed-off-by: red-hat-konflux <konflux@no-reply.konflux-ci.dev>
removing label vendor value
Signed-off-by: red-hat-konflux <konflux@no-reply.konflux-ci.dev>
…d-main Red Hat Konflux update ambient-code-backend-main
…nd-main Red Hat Konflux update ambient-code-frontend-main
…or-main Red Hat Konflux update ambient-code-operator-main
…-api-main Red Hat Konflux update ambient-code-public-api-main
…t-api-server-main Red Hat Konflux update ambient-code-ambient-api-server-main
Signed-off-by: red-hat-konflux <konflux@no-reply.konflux-ci.dev>
…nt-runner-main Red Hat Konflux update ambient-code-ambient-runner-main
Creates kustomize overlay for deploying to hcmais01ue1 via app-interface: - Uses Konflux images from redhat-services-prod/hcm-eng-prod-tenant - Scales down in-cluster databases (using external RDS from app-interface Phase 2) - Scales down MinIO (using external S3 from app-interface Phase 2) - Includes CRDs, RBAC, routes, and all application components - Patches operator to use Konflux runner image Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Creates kustomize overlay for deploying to hcmais01ue1 via app-interface: - Uses Konflux images from redhat-services-prod/hcm-eng-prod-tenant - Scales down in-cluster databases (using external RDS from app-interface Phase 2) - Scales down MinIO (using external S3 from app-interface Phase 2) - Includes CRDs, RBAC, routes, and all application components - Patches operator to use Konflux runner image Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…-overlay Add app-interface overlay for AppSRE platform deployment
Convert kustomize overlay to OpenShift Template format for app-interface SaaS deployment. Split into two templates: 1. template-operator.yaml (CRDs, ClusterRoles, operator deployment) - Operator and ambient-runner images - Cluster-scoped resources (CRDs, RBAC) - Operator deployment and its ConfigMaps 2. template-services.yaml (Application services) - Backend, frontend, public-api, ambient-api-server images - All deployments, services, routes, configmaps - Scales in-cluster services to 0 (minio, postgresql, unleash) Both templates use IMAGE_TAG parameter (auto-generated from git commit SHA) and support Konflux image gating through app-interface. This allows app-interface to use provider: openshift-template with proper parameter substitution instead of the directory provider which doesn't run kustomize build.
Add openshift template
The objects field must be a YAML array with proper list indicators. Previous version was missing the '-' prefix on array items, causing: 'unable to decode STDIN: json: cannot unmarshal object into Go struct field Template.objects of type []runtime.RawExtension' Changes: - Rebuild templates using Python yaml library for correct formatting - Objects now properly formatted as YAML array with '- apiVersion:' - Add validate.sh script for testing with oc process - Both templates validated successfully Generated from kustomize overlay output with proper YAML structure.
fix: correct OpenShift Template objects array format
Remove minio, postgresql, unleash, ambient-api-server-db. Using external RDS and S3 from app-interface. Removed 12 resources (4 Deployments, 4 Services, 3 PVCs, 1 Secret) Remaining: ambient-api-server, backend-api, frontend, public-api
refactor: remove in-cluster services from template
updating postgresql db name
enabling SSL mode for RDS
Disables OTEL metrics export by commenting out OTEL_EXPORTER_OTLP_ENDPOINT environment variable in operator deployment manifests. The operator was configured to send metrics to otel-collector.ambient-code.svc:4317, but this service does not exist in the cluster, causing repeated gRPC connection failures every 30 seconds with error: "failed to upload metrics: context deadline exceeded: rpc error: code = Unavailable desc = name resolver error: produced zero addresses" With OTEL_EXPORTER_OTLP_ENDPOINT unset, InitMetrics() will skip metrics export and log "metrics export disabled" instead of throwing connection errors. Changes: - Comment out OTEL_EXPORTER_OTLP_ENDPOINT in base operator deployment - Comment out OTEL_EXPORTER_OTLP_ENDPOINT in OpenShift template - Add clarifying comment about re-enabling when collector is deployed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Switch from custom vault secrets to OpenShift service account-based OAuth: - Use Red Hat's official ose-oauth-proxy-rhel9 image - Use service account token for cookie secret (no vault needed) - Enable HTTPS on OAuth proxy with OpenShift service-ca auto-generated certs - Add system:auth-delegator ClusterRoleBinding for OAuth delegation - Add OAuth redirect reference annotation to frontend ServiceAccount - Fix service account reference from 'nginx' to 'frontend' - Add missing NAMESPACE and UPSTREAM_TIMEOUT parameters Benefits: - No manual vault secret management - Automatic TLS cert rotation via service-ca - Standard OpenShift OAuth integration pattern - Follows app-interface team recommendations Files changed: - frontend-rbac.yaml: Added OAuth annotations and auth-delegator binding - oauth-proxy component patches: Updated to new configuration - Templates: Regenerated with OAuth fixes (27 operator, 21 service resources) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Fix OAuth proxy configuration to use OpenShift service account auth
The RDS credentials secret should not be in the OpenShift template - it's
provided by the external resource provider (terraform) in app-interface.
The namespace's externalResources section already defines:
- provider: rds
output_resource_name: ambient-code-rds
This automatically creates the secret with the correct RDS credentials.
Including the secret in the template with VAULT_INJECTED placeholders
caused deployment failures.
Changes:
- Excluded ambient-code-rds secret from template generation
- Template now has 20 service resources (down from 21)
- Deployment still references the secret via volumeMount (correct)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Exclude ambient-code-rds secret from services template
Signed-off-by: Chris Mitchell <cmitchel@redhat.com>
…ination fix: fix frontent route termination
Signed-off-by: Chris Mitchell <cmitchel@redhat.com>
fix: revert https changes for oauth pods
Changes GCP service account configuration to align with app-interface deployment where credentials are provided via Vault. Changes: - template-services.yaml: Update backend vertex-credentials secret name from 'ambient-vertex' to 'stage-gcp-creds' (matches Vault secret) - template-operator.yaml: Update GOOGLE_APPLICATION_CREDENTIALS path to match Vault secret key name 'itpc-gcp-hcm-pe-eng.json' The secret is provided by app-interface via: path: engineering-productivity/ambient-code/stage-gcp-creds This allows the backend and operator to use Vertex AI for Claude and Gemini API calls with the service account configured with roles/aiplatform.user permissions. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Chris Mitchell <cmitchel@redhat.com>
Update Vertex AI credentials to use app-interface Vault secret
Configure OAuth proxy sidecar to inject authentication token into forwarded requests, fixing 401 errors on /api/projects endpoints. Changes: - Add --pass-access-token=true flag to inject X-Forwarded-Access-Token header - Change upstream from frontend-service:3000 to localhost:3000 (correct sidecar pattern) - Remove --request-logging to reduce log noise Backend logs showed: tokenSource=none hasAuthHeader=false hasFwdToken=false The backend expects the X-Forwarded-Access-Token header, which is now injected by the OAuth proxy for all authenticated requests. Flow: 1. User authenticates via OpenShift OAuth ✓ 2. OAuth proxy injects token header ✓ (new) 3. Frontend forwards token to backend API ✓ (fixed) This resolves the 401 authentication errors while maintaining the working OpenShift OAuth integration. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Fix OAuth proxy to pass access token to backend API
Removed the '--set-authorization-header=true' option from the configuration.
updating ambient env to production
Changes: - Use proper 32-byte cookie secret from Vault instead of service account token - Add --pass-access-token to forward user's OAuth token to upstream - Add --scope=user:full to request full user permissions - Mount stage-cookie-secret at /etc/oauth-cookie Problem: OAuth proxy was authenticating users but not forwarding tokens to the Next.js frontend. When frontend made backend API calls, it had no token to forward, resulting in 401 errors. Root cause: The service account token (1618 bytes) is too large for AES cipher when --pass-access-token is enabled, which requires 16/24/32 byte secrets. Solution: Use a proper 32-byte cookie secret from Vault (matching UAT config), enabling --pass-access-token to forward the authenticated user's token through the chain: OAuth proxy → Next.js → Backend. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Chris Mitchell <cmitchel@redhat.com>
Fix OAuth proxy to forward user tokens to frontend/backend
❌ Deploy Preview for cheerful-kitten-f556a0 failed.
|
Contributor
📝 WalkthroughWalkthroughAdds 10 Tekton PipelineRun manifests for CI/CD automation across six components on PRs and pushes to main, renames database secrets from Changes
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
components/manifests/overlays/local-dev/ambient-api-server-init-db-patch.yaml (1)
10-12:⚠️ Potential issue | 🟠 MajorAdd resource requests/limits to the
init-dbcontainer.This patch injects a full init container but leaves it unbounded. Please add conservative defaults and tune per environment; apply the same check to other touched DB containers.
Example resource bounds
initContainers: - name: init-db image: registry.redhat.io/rhel10/postgresql-16:10.1 + resources: + requests: + cpu: 50m + memory: 128Mi + limits: + cpu: 250m + memory: 256Mi command:As per coding guidelines,
components/manifests/**/*.yaml: “RBAC must follow least-privilege. Resource limits/requests required on containers.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/manifests/overlays/local-dev/ambient-api-server-init-db-patch.yaml` around lines 10 - 12, The init container "init-db" is missing resource requests/limits; add conservative CPU and memory requests and limits to its container spec (under the initContainers entry for name "init-db") and apply the same pattern to any other database-related containers touched by this patch; ensure you add both resources.requests (e.g., cpu, memory) and resources.limits (e.g., cpu, memory) fields using conservative defaults and keep tunable values commented or documented for environment-specific overrides.components/manifests/components/ambient-api-server-db/ambient-api-server-init-db-patch.yaml (1)
10-60:⚠️ Potential issue | 🟠 MajorAdd resource requests and limits to
init-db.This init container has no resource guardrails, which may fail policy admission or run unbounded during DB startup/migration.
Suggested patch
- | set -e echo "Waiting for PostgreSQL to be ready (timeout: 10 minutes)..." @@ psql -h "$PGHOST" -U "$PGUSER" -c "CREATE DATABASE $PGDATABASE;" echo "Database '$PGDATABASE' created successfully" fi + resources: + requests: + cpu: 10m + memory: 64Mi + limits: + cpu: 100m + memory: 128Mi env: - name: PGHOSTAs per coding guidelines,
components/manifests/**/*.yaml: "RBAC must follow least-privilege. Resource limits/requests required on containers."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/manifests/components/ambient-api-server-db/ambient-api-server-init-db-patch.yaml` around lines 10 - 60, The init container "init-db" in the initContainers spec lacks resource requests/limits; add a resources block under the init-db container (identify by name: init-db and image: registry.redhat.io/rhel10/postgresql-16:10.1) with appropriate requests and limits (cpu and memory) to satisfy admission policy and enforce least-privilege resource usage; ensure the resources block uses both requests and limits keys and is placed at the same indentation level as env and command in the init-db container spec.components/ambient-api-server/templates/db-template.yml (1)
13-17:⚠️ Potential issue | 🟠 MajorSplit database service name from secret name.
Line 17 sets
DATABASE_SERVICE_NAMEtoambient-code-rds, but this parameter is used for the Service, Deployment, PVC, Secret, anddb.hostvalue. This conflates the in-cluster database hostname with the credentials Secret; the in-cluster Service should beambient-api-server-dband only the Secret should beambient-code-rds. Settingdb.hostto the RDS name breaks DNS resolution for pods connecting to the in-cluster database.Introduce a separate
DATABASE_SECRET_NAMEparameter and update secretKeyRef entries to reference it instead ofDATABASE_SERVICE_NAME.Proposed split between service name and secret name
- name: DATABASE_SERVICE_NAME description: The name of the OpenShift Service exposed for the database. displayName: Database Service Name required: true - value: ambient-code-rds + value: ambient-api-server-db + + - name: DATABASE_SECRET_NAME + description: The name of the Secret containing PostgreSQL connection values. + displayName: Database Secret Name + required: true + value: ambient-code-rds @@ key: db.user - name: ${DATABASE_SERVICE_NAME} + name: ${DATABASE_SECRET_NAME} @@ key: db.password - name: ${DATABASE_SERVICE_NAME} + name: ${DATABASE_SECRET_NAME} @@ key: db.name - name: ${DATABASE_SERVICE_NAME} + name: ${DATABASE_SECRET_NAME} @@ kind: Secret metadata: - name: ${DATABASE_SERVICE_NAME} + name: ${DATABASE_SECRET_NAME} stringData: db.host: ${DATABASE_SERVICE_NAME}Also applies to: 51-67, 82-99, 102-117, 151-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ambient-api-server/templates/db-template.yml` around lines 13 - 17, The template currently conflates DATABASE_SERVICE_NAME (used for Service, Deployment, PVC, Secret and db.host) with the credentials Secret, causing db.host to point to the RDS secret name; introduce a new parameter DATABASE_SECRET_NAME and set its default to ambient-code-rds, change DATABASE_SERVICE_NAME default to ambient-api-server-db, then update all secretKeyRef entries (and any places using the secret name) to reference DATABASE_SECRET_NAME while keeping db.host and Service/Deployment/PVC references using DATABASE_SERVICE_NAME so pods resolve the in-cluster Service hostname correctly.
🧹 Nitpick comments (2)
components/manifests/overlays/app-interface/kustomization.yaml (1)
30-62: Consider deleting unused workloads rather than scaling to 0.Scaling
postgresql,ambient-api-server-db,minio, andunleashto 0 replicas leaves their Deployments, Services, PVCs, ConfigMaps, and Secrets applied to the stage cluster, which still consumes storage (PVCs), clutters the namespace, and risks accidental re-scaling. For a deployment that has truly moved to external RDS/S3, prefer removing these resources via kustomizepatcheswith$patch: delete(or a dedicated component that excludes them), so they're not reconciled at all in app-interface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/manifests/overlays/app-interface/kustomization.yaml` around lines 30 - 62, Replace the "scale to 0" JSON6902 patches in kustomization.yaml for the Deployments named postgresql, ambient-api-server-db, minio, and unleash with kustomize delete patches so the resources are removed entirely (or remove those resources via a dedicated component). Specifically, locate the entries that target kind: Deployment with name: postgresql, ambient-api-server-db, minio, and unleash and change the patch strategy to use $patch: delete (or remove the resource from the base) so ConfigMaps, Secrets, PVCs and Services are not left applied; ensure each patch references the correct name (postgresql, ambient-api-server-db, minio, unleash) to delete the corresponding deployment..tekton/ambient-code-public-api-main-push.yaml (1)
1-583: Standard Konflux push pipeline — LGTM.Matches the established pattern used across the other five push pipelines in this PR. Heads-up: the 10 new Konflux manifests are ~95% identical boilerplate. If Konflux ever supports shared pipeline definitions (via
pipelineRefbundle), consolidating would remove a lot of copy-paste maintenance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.tekton/ambient-code-public-api-main-push.yaml around lines 1 - 583, The PR is approved but duplicates ~95% of pipelineSpec across 10 Konflux manifests; consolidate to avoid boilerplate. Create a single shared Tekton pipeline bundle (or pipelineRef bundle) containing the common pipelineSpec (parameters, tasks, results, workspaces, taskRunTemplate) and then update this PipelineRun (metadata.name ambient-code-public-api-main-on-push) and the other nine manifests to reference that shared pipeline via pipelineRef/bundle while passing only manifest-specific params (e.g., git-url, revision, output-image, path-context, dockerfile) and workspace secrets; ensure any unique per-run annotations/labels remain on each PipelineRun and validate the resulting runs still set taskRunTemplate.serviceAccountName and workspaces as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.tekton/ambient-code-backend-main-pull-request.yaml:
- Line 5: The build annotation build.appstudio.openshift.io/repo is pointing to
the wrong GitHub repo
(https://github.com/RedHatInsights/ambient-code-platform?rev={{revision}});
update that annotation value to the correct repository (for this PR) — e.g.
replace the value with https://github.com/ambient-code/platform?rev={{revision}}
— so the build metadata and AppStudio/Konflux traceability reference the correct
source; edit the line containing build.appstudio.openshift.io/repo to use the
new URL.
- Around line 11-13: Update the CEL path filters to remove the leading "./" so
glob patterns match Git's reported paths: replace occurrences like
"./components/backend/***".pathChanged() with
"components/backend/***".pathChanged() (and similarly remove "./" from
".tekton/ambient-code-backend-main-pull-request.yaml".pathChanged() and
"Dockerfile".pathChanged() patterns) inside the
pipelinesascode.tekton.dev/on-cel-expression entry; apply the same removal of
"./" for component patterns across all .tekton/ files (backend, frontend,
public-api, operator, ambient-runner, ambient-api-server) so pathChanged() uses
standard globs that match Git paths.
In @.tekton/ambient-code-frontend-main-pull-request.yaml:
- Around line 11-13: Remove the redundant root Dockerfile path check from the
pipelinesascode CEL expression: delete the `"Dockerfile".pathChanged()` clause
inside the pipelinesascode.tekton.dev/on-cel-expression so the condition relies
on `"./components/frontend/***".pathChanged()` (and other component path checks)
only; also search other manifests in .tekton/ and remove the same
`"Dockerfile".pathChanged()` dead clause wherever it appears to keep expressions
consistent.
In @.tekton/ambient-code-operator-main-pull-request.yaml:
- Line 5: Update the incorrect repository annotation value for
build.appstudio.openshift.io/repo in all Tekton manifests listed (e.g., the
entry shown in .tekton/ambient-code-operator-main-pull-request.yaml) from
"https://github.com/RedHatInsights/ambient-code-platform?rev={{revision}}" to
"https://github.com/ambient-code/platform?rev={{revision}}"; ensure you change
the same annotation key (build.appstudio.openshift.io/repo) in each of the 12
files referenced (.tekton/*-main-*.yaml) so Konflux receives the correct
org/repo for commit-status links and provenance tracking.
In `@components/manifests/base/core/operator-deployment.yaml`:
- Around line 125-128: The operator deployment currently has the
OTEL_EXPORTER_OTLP_ENDPOINT environment variable commented out, which disables
metrics export by default; update the operator deployment manifest to either
uncomment and set OTEL_EXPORTER_OTLP_ENDPOINT to the observability collector
address ("otel-collector.ambient-code.svc:4317") in the core operator Deployment
spec (the env entry named OTEL_EXPORTER_OTLP_ENDPOINT) or move this env var into
a production overlay so the base stays neutral but overlays enable
observability; ensure the env name OTEL_EXPORTER_OTLP_ENDPOINT is present in the
container spec (containers[].env) so the operator can export metrics to the OTel
collector.
In `@components/manifests/base/rbac/frontend-rbac.yaml`:
- Around line 6-7: The OAuth redirect reference annotation
serviceaccounts.openshift.io/oauth-redirectreference.frontend currently
references reference.name "frontend" which doesn't match the actual Route;
update the annotation's reference.name value from "frontend" to "frontend-route"
so it matches the Route resource (ensure the annotation JSON string inside
frontend-rbac.yaml uses "frontend-route" for reference.name).
In
`@components/manifests/components/oauth-proxy/frontend-oauth-deployment-patch.yaml`:
- Line 33: The deployment currently sets --cookie-secret-file to the service
account token path which is invalid for openshift/oauth-proxy (it requires a
16/24/32 byte secret); replace this by mounting a dedicated cookie secret and
point --cookie-secret-file at the mounted secret file instead of
/var/run/secrets/kubernetes.io/serviceaccount/token, add the corresponding
volume and volumeMount that reference a K8s Secret containing a 16/24/32-byte
base64 value (generate via openssl rand -base64 32 or via the deploy script),
and ensure the oauth-proxy container uses that mounted path for the cookie
secret (look for the --cookie-secret-file flag and the container's volumeMount
block to modify).
In
`@components/manifests/components/oauth-proxy/frontend-oauth-service-patch.yaml`:
- Around line 2-9: Update the Service metadata annotation on the Service named
"frontend-service": replace the annotation key
"service.alpha.openshift.io/serving-cert-secret-name" with
"service.beta.openshift.io/serving-cert-secret-name" so the Service CA operator
will create the "frontend-proxy-tls" secret; locate this in the Service manifest
metadata -> annotations block and change the annotation key accordingly.
In
`@components/manifests/overlays/app-interface/ambient-api-server-env-patch.yaml`:
- Around line 1-13: The AMBIENT_ENV is set to an unregistered value "stage" for
the Deployment named ambient-api-server; change the env var AMBIENT_ENV under
the container named api-server to "production" so it matches the registered
EnvironmentImpls and picks up production overrides (CORS, SSO JWKS URL, DB
session setup); alternatively, if you intend a true "stage" environment, add a
corresponding EnvironmentImpl registration and implementation to the environment
map instead of using the unregistered value.
In
`@components/manifests/overlays/app-interface/operator-runner-image-patch.yaml`:
- Line 13: The image value currently uses the floating tag
"ambient-code-ambient-runner-main:latest"; replace it with an immutable
reference instead — either a digest form
(quay.io/.../ambient-code-ambient-runner-main@sha256:<hash>) or an explicit,
parameterized tag using the Template variable IMAGE_TAG (e.g.
quay.io/.../ambient-code-ambient-runner-main:${IMAGE_TAG}) so restarts are
reproducible and rollbacks reliable; update the same `value` entry in
operator-runner-image-patch.yaml and ensure IMAGE_TAG is wired into the Template
where images are rendered.
In `@components/manifests/overlays/app-interface/route.yaml`:
- Line 4: The Route in this overlay is named "frontend-route" but the base
ServiceAccount annotation in frontend-rbac.yaml references "Route/frontend",
causing a redirect mismatch; to fix, either rename the Route resource in
route.yaml from "frontend-route" to "frontend" or (preferably, since
template-operator.yaml already expects "frontend-route") update the
oauth-redirectreference in components/manifests/base/rbac/frontend-rbac.yaml to
reference "Route/frontend-route" so the ServiceAccount annotation and the Route
name match.
In `@components/manifests/templates/template-operator.yaml`:
- Around line 842-869: The RBAC rule in the ambient-project-edit role currently
allows creating tokens for any ServiceAccount via the "serviceaccounts/token:
create" permission; tighten this by either removing that verb from the rule or
limiting it with a resourceNames list to only the dedicated runner
ServiceAccount(s) (e.g., the project runner SA name(s) your operator uses).
Locate the ambient-project-edit role rule block in template-operator.yaml (the
rule that lists "serviceaccounts/token" and "create") and change it to omit
token creation or add explicit resourceNames entries for the approved SA(s), and
ensure any RoleBinding or Role creation logic that expects broader token
creation is updated accordingly. Ensure the final role follows least-privilege
(no wildcard token creation) and update tests/docs referencing
ambient-project-edit if needed.
- Around line 1343-1348: The operator manifest currently hardcodes S3_ENDPOINT
and S3_BUCKET (S3_ENDPOINT: http://minio.ambient-code.svc:9000, S3_BUCKET:
ambient-sessions) which conflicts with removing in-cluster MinIO; change these
to be templated or sourced from operator config the same way
IMAGE_TAG/IMAGE_OPERATOR are handled: replace the hardcoded S3_ENDPOINT and
S3_BUCKET env values with template variables (e.g., {{ .Values.s3.endpoint }} /
{{ .Values.s3.bucket }}) or switch the env entries to pull from the operator
ConfigMap (operator-config) via envFrom or valueFrom so the app-interface
overlay can supply the real external S3/RDS values at deploy time, and ensure
any runner/state-sync code reading S3_* uses those env names unchanged.
In `@components/manifests/templates/template-services.yaml`:
- Around line 25-26: The UPSTREAM_TIMEOUT template parameter is declared as
UPSTREAM_TIMEOUT (default "5m") but not used; update the oauth-proxy container
args (the line that currently hardcodes "--upstream-timeout=5m") to reference
the template parameter instead (e.g., substitute the hardcoded value with the
UPSTREAM_TIMEOUT template variable), or remove the UPSTREAM_TIMEOUT parameter if
you prefer to keep the hardcoded value; make the change where the oauth-proxy
args are constructed so the arg uses the template variable name
UPSTREAM_TIMEOUT.
- Around line 242-243: The manifest currently sets --cors-allowed-origins=*
which is too permissive; change the flag in the ambient-api-server container
args to use a templated frontend host variable (e.g. FRONTEND_HOST) instead of
*, e.g. set --cors-allowed-origins to the FRONTEND_HOST template value and
ensure FRONTEND_HOST is declared/parameterized in the template and populated at
deploy time; keep the existing --cors-allowed-headers (X-Ambient-Project)
intact.
- Around line 180-194: The template still creates an orphaned
PersistentVolumeClaim named "postgresql-data" (PersistentVolumeClaim block in
template-services.yaml) even though in-cluster Postgres was removed; delete this
PVC resource from the template (or wrap it behind a values flag such as
enablePostgres or usePostgresPVC and gate its creation) and remove any
references to "postgresql-data" so app-interface stops provisioning the 10Gi
claim on each deploy; update the template that defines the PersistentVolumeClaim
and any Helm/values logic that would otherwise render it.
---
Outside diff comments:
In `@components/ambient-api-server/templates/db-template.yml`:
- Around line 13-17: The template currently conflates DATABASE_SERVICE_NAME
(used for Service, Deployment, PVC, Secret and db.host) with the credentials
Secret, causing db.host to point to the RDS secret name; introduce a new
parameter DATABASE_SECRET_NAME and set its default to ambient-code-rds, change
DATABASE_SERVICE_NAME default to ambient-api-server-db, then update all
secretKeyRef entries (and any places using the secret name) to reference
DATABASE_SECRET_NAME while keeping db.host and Service/Deployment/PVC references
using DATABASE_SERVICE_NAME so pods resolve the in-cluster Service hostname
correctly.
In
`@components/manifests/components/ambient-api-server-db/ambient-api-server-init-db-patch.yaml`:
- Around line 10-60: The init container "init-db" in the initContainers spec
lacks resource requests/limits; add a resources block under the init-db
container (identify by name: init-db and image:
registry.redhat.io/rhel10/postgresql-16:10.1) with appropriate requests and
limits (cpu and memory) to satisfy admission policy and enforce least-privilege
resource usage; ensure the resources block uses both requests and limits keys
and is placed at the same indentation level as env and command in the init-db
container spec.
In
`@components/manifests/overlays/local-dev/ambient-api-server-init-db-patch.yaml`:
- Around line 10-12: The init container "init-db" is missing resource
requests/limits; add conservative CPU and memory requests and limits to its
container spec (under the initContainers entry for name "init-db") and apply the
same pattern to any other database-related containers touched by this patch;
ensure you add both resources.requests (e.g., cpu, memory) and resources.limits
(e.g., cpu, memory) fields using conservative defaults and keep tunable values
commented or documented for environment-specific overrides.
---
Nitpick comments:
In @.tekton/ambient-code-public-api-main-push.yaml:
- Around line 1-583: The PR is approved but duplicates ~95% of pipelineSpec
across 10 Konflux manifests; consolidate to avoid boilerplate. Create a single
shared Tekton pipeline bundle (or pipelineRef bundle) containing the common
pipelineSpec (parameters, tasks, results, workspaces, taskRunTemplate) and then
update this PipelineRun (metadata.name ambient-code-public-api-main-on-push) and
the other nine manifests to reference that shared pipeline via
pipelineRef/bundle while passing only manifest-specific params (e.g., git-url,
revision, output-image, path-context, dockerfile) and workspace secrets; ensure
any unique per-run annotations/labels remain on each PipelineRun and validate
the resulting runs still set taskRunTemplate.serviceAccountName and workspaces
as before.
In `@components/manifests/overlays/app-interface/kustomization.yaml`:
- Around line 30-62: Replace the "scale to 0" JSON6902 patches in
kustomization.yaml for the Deployments named postgresql, ambient-api-server-db,
minio, and unleash with kustomize delete patches so the resources are removed
entirely (or remove those resources via a dedicated component). Specifically,
locate the entries that target kind: Deployment with name: postgresql,
ambient-api-server-db, minio, and unleash and change the patch strategy to use
$patch: delete (or remove the resource from the base) so ConfigMaps, Secrets,
PVCs and Services are not left applied; ensure each patch references the correct
name (postgresql, ambient-api-server-db, minio, unleash) to delete the
corresponding deployment.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8dd06016-65e3-427e-9a84-9c61ef2b056c
📒 Files selected for processing (51)
.tekton/ambient-code-ambient-api-server-main-pull-request.yaml.tekton/ambient-code-ambient-api-server-main-push.yaml.tekton/ambient-code-ambient-runner-main-pull-request.yaml.tekton/ambient-code-ambient-runner-main-push.yaml.tekton/ambient-code-backend-main-pull-request.yaml.tekton/ambient-code-backend-main-push.yaml.tekton/ambient-code-frontend-main-pull-request.yaml.tekton/ambient-code-frontend-main-push.yaml.tekton/ambient-code-operator-main-pull-request.yaml.tekton/ambient-code-operator-main-push.yaml.tekton/ambient-code-public-api-main-pull-request.yaml.tekton/ambient-code-public-api-main-push.yamlcomponents/ambient-api-server/Dockerfilecomponents/ambient-api-server/templates/db-template.ymlcomponents/manifests/README.mdcomponents/manifests/base/core/ambient-api-server-service.ymlcomponents/manifests/base/core/operator-deployment.yamlcomponents/manifests/base/platform/ambient-api-server-db.ymlcomponents/manifests/base/platform/ambient-api-server-secrets.ymlcomponents/manifests/base/rbac/frontend-rbac.yamlcomponents/manifests/components/ambient-api-server-db/ambient-api-server-db-json-patch.yamlcomponents/manifests/components/ambient-api-server-db/ambient-api-server-init-db-patch.yamlcomponents/manifests/components/ambient-api-server-db/kustomization.yamlcomponents/manifests/components/oauth-proxy/frontend-oauth-deployment-patch.yamlcomponents/manifests/components/oauth-proxy/frontend-oauth-service-patch.yamlcomponents/manifests/overlays/app-interface/ambient-api-server-db-secret-patch.yamlcomponents/manifests/overlays/app-interface/ambient-api-server-env-patch.yamlcomponents/manifests/overlays/app-interface/ambient-api-server-route.yamlcomponents/manifests/overlays/app-interface/ambient-api-server-service-ca-patch.yamlcomponents/manifests/overlays/app-interface/ambient-api-server-ssl-patch.yamlcomponents/manifests/overlays/app-interface/backend-route.yamlcomponents/manifests/overlays/app-interface/kustomization.yamlcomponents/manifests/overlays/app-interface/namespace-patch.yamlcomponents/manifests/overlays/app-interface/namespace.yamlcomponents/manifests/overlays/app-interface/operator-config-openshift.yamlcomponents/manifests/overlays/app-interface/operator-runner-image-patch.yamlcomponents/manifests/overlays/app-interface/public-api-route.yamlcomponents/manifests/overlays/app-interface/route.yamlcomponents/manifests/overlays/kind/api-server-db-security-patch.yamlcomponents/manifests/overlays/kind/api-server-no-jwt-patch.yamlcomponents/manifests/overlays/local-dev/ambient-api-server-db-credentials-patch.yamlcomponents/manifests/overlays/local-dev/ambient-api-server-db-json-patch.yamlcomponents/manifests/overlays/local-dev/ambient-api-server-init-db-patch.yamlcomponents/manifests/overlays/production/ambient-api-server-jwt-args-patch.yamlcomponents/manifests/overlays/production/ambient-api-server-migration-ssl-patch.yamlcomponents/manifests/overlays/production/frontend-oauth-patch.yamlcomponents/manifests/overlays/production/kustomization.yamlcomponents/manifests/templates/template-operator.yamlcomponents/manifests/templates/template-services.yamlcomponents/manifests/templates/validate.shcomponents/runners/ambient-runner/Dockerfile
💤 Files with no reviewable changes (2)
- components/manifests/overlays/production/frontend-oauth-patch.yaml
- components/ambient-api-server/Dockerfile
| kind: PipelineRun | ||
| metadata: | ||
| annotations: | ||
| build.appstudio.openshift.io/repo: https://github.com/RedHatInsights/ambient-code-platform?rev={{revision}} |
Contributor
There was a problem hiding this comment.
Fix the stale repository annotation.
This points build metadata at RedHatInsights/ambient-code-platform, while this PR is for ambient-code/platform. That can make AppStudio/Konflux traceability and image metadata link to the wrong source repository.
Suggested fix
- build.appstudio.openshift.io/repo: https://github.com/RedHatInsights/ambient-code-platform?rev={{revision}}
+ build.appstudio.openshift.io/repo: https://github.com/ambient-code/platform?rev={{revision}}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| build.appstudio.openshift.io/repo: https://github.com/RedHatInsights/ambient-code-platform?rev={{revision}} | |
| build.appstudio.openshift.io/repo: https://github.com/ambient-code/platform?rev={{revision}} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.tekton/ambient-code-backend-main-pull-request.yaml at line 5, The build
annotation build.appstudio.openshift.io/repo is pointing to the wrong GitHub
repo (https://github.com/RedHatInsights/ambient-code-platform?rev={{revision}});
update that annotation value to the correct repository (for this PR) — e.g.
replace the value with https://github.com/ambient-code/platform?rev={{revision}}
— so the build metadata and AppStudio/Konflux traceability reference the correct
source; edit the line containing build.appstudio.openshift.io/repo to use the
new URL.
Comment on lines
+11
to
+13
| pipelinesascode.tekton.dev/on-cel-expression: event == "pull_request" && target_branch | ||
| == "main" && ( "./components/backend/***".pathChanged() || ".tekton/ambient-code-backend-main-pull-request.yaml".pathChanged() | ||
| || "Dockerfile".pathChanged() ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Changed backend paths reported by git:"
base="$(git merge-base HEAD origin/main 2>/dev/null || true)"
if [ -n "$base" ]; then
git diff --name-only "$base"...HEAD | grep -E '^components/backend/' | head -20 || true
else
git diff --name-only HEAD~1..HEAD | grep -E '^components/backend/' | head -20 || true
fi
echo
echo "Local pathChanged expressions:"
rg -n -C2 'pathChanged\(\)' .tektonRepository: ambient-code/platform
Length of output: 8508
🏁 Script executed:
# Verify the specific file mentioned in the review
cat -n .tekton/ambient-code-backend-main-pull-request.yaml | sed -n '8,15p'Repository: ambient-code/platform
Length of output: 592
🌐 Web query:
Pipelines as Code pathChanged() leading dot behavior documentation
💡 Result:
In Pipelines as Code, pathChanged is a CEL suffix function used in on-cel-expression annotations to check if files matching a glob pattern have changed in a Git event (supported for GitHub and GitLab). Example: event == "pull_request" && "docs/.md".pathChanged. It uses standard glob patterns (not regex), where patterns like /.md match files ending in .md, including those with leading dots if explicitly specified (e.g., "..md" for .gitignore.md, or ".tekton/" for .tekton directory). No official documentation explicitly states special "leading dot behavior" different from standard glob semantics. Globs follow the "glob library" syntax (linked in docs), which typically requires explicit matching for dotfiles (e.g., . or . for hidden files/directories), as in Unix glob where * does not match files starting with .. The tkn pac info globbing command can test patterns locally against files. For path-based triggering without CEL, use annotations pipelinesascode.tekton.dev/on-path-change: "pattern" or on-path-change-ignore, which also use the same glob patterns.
Citations:
- 1: https://pipelinesascode.com/docs/guides/event-matching/path-matching/
- 2: https://pipelinesascode.com/docs/guide/matchingevents/
- 3: https://pipelinesascode.com/docs/guides/event-matching/cel-expressions/
- 4: https://www.mintlify.com/openshift-pipelines/pipelines-as-code/cli/info
- 5: https://pipelinesascode.com/docs/guide/cli/
🌐 Web query:
Tekton Pipelines as Code CEL path matching behavior
💡 Result:
Tekton Pipelines uses CEL (Common Expression Language) in WhenExpressions for conditional task execution. The .matches method provides regex-based string matching, including path matching. Key behavior: - Regex pattern matching: "'$(params.branch)'.matches('release/.')" matches branches like "release/v1.0". - Variables must be wrapped in single quotes for substitution: "'$(params.path)'.matches('/api/.')". - Supports standard CEL regex syntax per cel-spec: https://github.com/google/cel-spec. - Alpha feature: Requires enable-cel-in-whenexpression feature flag enabled. - All when expressions use AND logic (all must evaluate true). - Validated at admission webhook. Examples from official docs: when: - cel: "'$(params.branch)'.matches('release/.*')" when: - cel: "'$(tasks.unit-test.results.test-coverage)' > 0.9" No special "path matcher" beyond standard CEL .matches(regex). For Triggers CEL (separate), additional extensions like header.match exist, but Pipelines uses core CEL + Tekton vars. As of 2026 releases (v1.10+), no changes to this behavior; still uses cel-go implementation.
Citations:
- 1: https://tekton.dev/docs/pipelines/pipelines/
- 2: https://tekton.dev/vault/pipelines-main/pipelines
- 3: https://tekton.dev/vault/pipelines-v1.0.x-lts/pipelines
- 4: https://tekton.dev/docs/triggers/cel_expressions/
- 5: https://github.com/tektoncd/community/blob/main/teps/0145-cel-in-whenexpression.md
- 6: https://tekton.dev/docs/triggers/cel_expressions
- 7: https://github.com/tektoncd/pipeline/releases/tag/v1.6.1
- 8: https://github.com/tektoncd/pipeline/releases/tag/v1.7.0
- 9: https://github.com/tektoncd/pipeline/releases/tag/v1.10.0
Remove the leading ./ from all component path filters.
PaC's pathChanged() uses standard glob patterns without normalization. Git reports changed paths as components/backend/... (no leading ./), but the current pattern ./components/backend/*** expects literal ./ prefix. This mismatch breaks triggering for all component PR/push pipelines.
Fix required across all .tekton/ files (backend, frontend, public-api, operator, ambient-runner, ambient-api-server):
Example fix for this file
- == "main" && ( "./components/backend/***".pathChanged() || ".tekton/ambient-code-backend-main-pull-request.yaml".pathChanged()
+ == "main" && ( "components/backend/***".pathChanged() || ".tekton/ambient-code-backend-main-pull-request.yaml".pathChanged()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pipelinesascode.tekton.dev/on-cel-expression: event == "pull_request" && target_branch | |
| == "main" && ( "./components/backend/***".pathChanged() || ".tekton/ambient-code-backend-main-pull-request.yaml".pathChanged() | |
| || "Dockerfile".pathChanged() ) | |
| pipelinesascode.tekton.dev/on-cel-expression: event == "pull_request" && target_branch | |
| == "main" && ( "components/backend/***".pathChanged() || ".tekton/ambient-code-backend-main-pull-request.yaml".pathChanged() | |
| || "Dockerfile".pathChanged() ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.tekton/ambient-code-backend-main-pull-request.yaml around lines 11 - 13,
Update the CEL path filters to remove the leading "./" so glob patterns match
Git's reported paths: replace occurrences like
"./components/backend/***".pathChanged() with
"components/backend/***".pathChanged() (and similarly remove "./" from
".tekton/ambient-code-backend-main-pull-request.yaml".pathChanged() and
"Dockerfile".pathChanged() patterns) inside the
pipelinesascode.tekton.dev/on-cel-expression entry; apply the same removal of
"./" for component patterns across all .tekton/ files (backend, frontend,
public-api, operator, ambient-runner, ambient-api-server) so pathChanged() uses
standard globs that match Git paths.
Comment on lines
+11
to
+13
| pipelinesascode.tekton.dev/on-cel-expression: event == "pull_request" && target_branch | ||
| == "main" && ( "./components/frontend/***".pathChanged() || ".tekton/ambient-code-frontend-main-pull-request.yaml".pathChanged() | ||
| || "Dockerfile".pathChanged() ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how other Tekton PR manifests in this PR form their CEL path globs
fd -e yaml . .tekton | xargs rg -nP 'on-cel-expression|pathChanged' -C1Repository: ambient-code/platform
Length of output: 7449
🏁 Script executed:
# Check if a root-level Dockerfile exists
ls -la | grep -i dockerfile
# Also check component Dockerfiles to understand structure
fd -name Dockerfile | head -20Repository: ambient-code/platform
Length of output: 295
🌐 Web query:
Konflux PaC CEL expression path globbing syntax documentation leading dot slash
💡 Result:
Konflux uses Pipelines as Code (PaC) with Tekton for CI/CD pipelines. The path globbing syntax in CEL expressions uses the .pathChanged suffix function appended to a glob pattern string. Syntax: "glob-pattern".pathChanged This returns true if any changed file matches the glob pattern. Globs use standard Unix-style patterns: - * matches any sequence of non-separator characters - matches any number of directories (recursive) - ? matches a single character - [abc] matches any character in brackets - ! for negation (in some contexts) Examples from Konflux docs: "tasks/echo/0.1/".pathChanged Triggers if files in tasks/echo/0.1/ or subdirs changed. Note * likely means recursive any files. "docs/.md".pathChanged Triggers on .md files in docs/. For leading ./ or .: - Paths in git events are typically relative without leading ./. - Use "./somepath/*".pathChanged if paths start with ./; test with tkn pac info globbing "[PATTERN]". - Flux issues note leading . paths need special handling, but PaC .pathChanged uses git changed files as-is. Full annotation: pipelinesascode.tekton.dev/on-cel-expression: | event == "pull_request" && target_branch == "main" && "./path/to/glob.pathChanged Alternative: files.all.exists(x, x.matches("glob-pattern")) for regex or file type filtering. Official docs: Pipelines as Code CEL guide (pipelinesascode.com), Konflux Tekton tasks guide (konflux-ci.dev/docs/end-to-end/building-tekton-tasks/).
Citations:
- 1: https://pipelinesascode.com/docs/guides/event-matching/cel-expressions/
- 2: https://pipelinesascode.com/docs/guides/event-matching/path-matching/
- 3: https://konflux-ci.dev/docs/end-to-end/building-tekton-tasks/
- 4: https://github.com/konflux-ci/olm-operator-konflux-sample/blob/main/docs/konflux-onboarding.md
- 5: GitRepository source with "." assumes an incorrect path. fluxcd/flux2#2438
- 6: Flux create & sync the wrong path on bootstrap using a path starting with a "." fluxcd/flux2#1669
🏁 Script executed:
# Check if root Dockerfile exists and list all Dockerfiles
ls -la Dockerfile 2>/dev/null || echo "No root Dockerfile found"
echo "---"
echo "All Dockerfiles in repo:"
fd DockerfileRepository: ambient-code/platform
Length of output: 488
🏁 Script executed:
# Check what the "Dockerfile" parameter references in the component configs
# Look at how dockerfile param is used in each component
rg -l "dockerfile:" .tekton --type yaml | head -3 | xargs cat | grep -A2 -B2 "dockerfile:"Repository: ambient-code/platform
Length of output: 47
🏁 Script executed:
# Check if the Dockerfile parameter in task steps actually references the component Dockerfile
# Look at one component's task definition to see how dockerfile param is used
fd -e yaml .tekton | xargs rg "dockerfile:" -C2 | head -30Repository: ambient-code/platform
Length of output: 47
Remove redundant "Dockerfile".pathChanged() check—no root Dockerfile exists.
The CEL expression checks "Dockerfile".pathChanged() at the repo root, but no root Dockerfile exists. All component Dockerfiles are nested under components/<name>/Dockerfile and are already covered by "./components/frontend/***".pathChanged(). This clause is dead code.
The leading ./ prefix in "./components/frontend/***" is valid in Konflux PaC CEL expressions and consistent across all component manifests in this PR—no change needed there.
Suggested fix
pipelinesascode.tekton.dev/on-cel-expression: event == "pull_request" && target_branch
== "main" && ( "./components/frontend/***".pathChanged() || ".tekton/ambient-code-frontend-main-pull-request.yaml".pathChanged()
- || "Dockerfile".pathChanged() )
+ )Note: This pattern (including the dead Dockerfile check) is repeated across all 10+ component manifests in .tekton/. Consider fixing them all.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.tekton/ambient-code-frontend-main-pull-request.yaml around lines 11 - 13,
Remove the redundant root Dockerfile path check from the pipelinesascode CEL
expression: delete the `"Dockerfile".pathChanged()` clause inside the
pipelinesascode.tekton.dev/on-cel-expression so the condition relies on
`"./components/frontend/***".pathChanged()` (and other component path checks)
only; also search other manifests in .tekton/ and remove the same
`"Dockerfile".pathChanged()` dead clause wherever it appears to keep expressions
consistent.
| kind: PipelineRun | ||
| metadata: | ||
| annotations: | ||
| build.appstudio.openshift.io/repo: https://github.com/RedHatInsights/ambient-code-platform?rev={{revision}} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the mismatch across all new Tekton pipelines.
rg -n 'build.appstudio.openshift.io/repo' .tekton/Repository: ambient-code/platform
Length of output: 2054
All 12 Tekton manifests point to wrong repository org.
The annotation build.appstudio.openshift.io/repo uses https://github.com/RedHatInsights/ambient-code-platform but this PR is in ambient-code/platform. Update all files to use https://github.com/ambient-code/platform?rev={{revision}}. Konflux relies on this annotation for commit-status links and provenance tracking — the incorrect org will break status reporting.
Affected files:
.tekton/ambient-code-public-api-main-push.yaml.tekton/ambient-code-public-api-main-pull-request.yaml.tekton/ambient-code-operator-main-push.yaml.tekton/ambient-code-operator-main-pull-request.yaml.tekton/ambient-code-frontend-main-push.yaml.tekton/ambient-code-frontend-main-pull-request.yaml.tekton/ambient-code-backend-main-push.yaml.tekton/ambient-code-backend-main-pull-request.yaml.tekton/ambient-code-ambient-runner-main-push.yaml.tekton/ambient-code-ambient-runner-main-pull-request.yaml.tekton/ambient-code-ambient-api-server-main-push.yaml.tekton/ambient-code-ambient-api-server-main-pull-request.yaml
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.tekton/ambient-code-operator-main-pull-request.yaml at line 5, Update the
incorrect repository annotation value for build.appstudio.openshift.io/repo in
all Tekton manifests listed (e.g., the entry shown in
.tekton/ambient-code-operator-main-pull-request.yaml) from
"https://github.com/RedHatInsights/ambient-code-platform?rev={{revision}}" to
"https://github.com/ambient-code/platform?rev={{revision}}"; ensure you change
the same annotation key (build.appstudio.openshift.io/repo) in each of the 12
files referenced (.tekton/*-main-*.yaml) so Konflux receives the correct
org/repo for commit-status links and provenance tracking.
Comment on lines
125
to
+128
| # OpenTelemetry configuration | ||
| - name: OTEL_EXPORTER_OTLP_ENDPOINT | ||
| value: "otel-collector.ambient-code.svc:4317" # Deploy OTel collector separately | ||
| # Disabled: OTel collector not deployed. Uncomment when collector is available. | ||
| # - name: OTEL_EXPORTER_OTLP_ENDPOINT | ||
| # value: "otel-collector.ambient-code.svc:4317" # Deploy OTel collector separately |
Contributor
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
#!/bin/bash
# Expectation: production overlays that need metrics should define OTEL_EXPORTER_OTLP_ENDPOINT.
rg -n -C3 'OTEL_EXPORTER_OTLP_ENDPOINT|otel-collector' components/manifests components/operatorRepository: ambient-code/platform
Repository: ambient-code/platform
Exit code: 0
stdout:
components/operator/internal/controller/otel_metrics.go-46-)
components/operator/internal/controller/otel_metrics.go-47-
components/operator/internal/controller/otel_metrics.go-48-// InitMetrics initializes OpenTelemetry metrics.
components/operator/internal/controller/otel_metrics.go:49:// Set OTEL_EXPORTER_OTLP_ENDPOINT to configure the collector address.
components/operator/internal/controller/otel_metrics.go-50-// Leave unset or empty to disable metrics export (no-op).
components/operator/internal/controller/otel_metrics.go-51-func InitMetrics(ctx context.Context) (func(), error) {
components/operator/internal/controller/otel_metrics.go-52- // Get OTLP endpoint from environment; skip if not configured
components/operator/internal/controller/otel_metrics.go:53: endpoint := os.Getenv("OTEL_EXPORTER_OTLP_ENDPOINT")
components/operator/internal/controller/otel_metrics.go-54- if endpoint == "" {
components/operator/internal/controller/otel_metrics.go:55: log.Println("OTEL_EXPORTER_OTLP_ENDPOINT not set, metrics export disabled")
components/operator/internal/controller/otel_metrics.go-56- return func() {}, nil
components/operator/internal/controller/otel_metrics.go-57- }
components/operator/internal/controller/otel_metrics.go-58-
--
components/manifests/observability/base/servicemonitor.yaml-1-apiVersion: monitoring.coreos.com/v1
components/manifests/observability/base/servicemonitor.yaml-2-kind: ServiceMonitor
components/manifests/observability/base/servicemonitor.yaml-3-metadata:
components/manifests/observability/base/servicemonitor.yaml:4: name: ambient-otel-collector
components/manifests/observability/base/servicemonitor.yaml-5- namespace: ambient-code
components/manifests/observability/base/servicemonitor.yaml-6- labels:
components/manifests/observability/base/servicemonitor.yaml:7: app: otel-collector
components/manifests/observability/base/servicemonitor.yaml-8- # Required for OpenShift User Workload Monitoring to discover this ServiceMonitor
components/manifests/observability/base/servicemonitor.yaml-9- openshift.io/cluster-monitoring: "true"
components/manifests/observability/base/servicemonitor.yaml-10-spec:
components/manifests/observability/base/servicemonitor.yaml-11- selector:
components/manifests/observability/base/servicemonitor.yaml-12- matchLabels:
components/manifests/observability/base/servicemonitor.yaml:13: app: otel-collector
components/manifests/observability/base/servicemonitor.yaml-14- endpoints:
components/manifests/observability/base/servicemonitor.yaml-15- - port: prometheus
components/manifests/observability/base/servicemonitor.yaml-16- interval: 30s
--
components/manifests/observability/base/kustomization.yaml-4-namespace: ambient-code
components/manifests/observability/base/kustomization.yaml-5-
components/manifests/observability/base/kustomization.yaml-6-resources:
components/manifests/observability/base/kustomization.yaml:7: - otel-collector.yaml
components/manifests/observability/base/kustomization.yaml-8- - servicemonitor.yaml
--
components/manifests/observability/README.md-109-### OTel Collector Logs
components/manifests/observability/README.md-110-
components/manifests/observability/README.md-111-```bash
components/manifests/observability/README.md:112:kubectl logs -n ambient-code -l app=otel-collector -f
components/manifests/observability/README.md-113-```
components/manifests/observability/README.md-114-
components/manifests/observability/README.md-115-## Production Setup
--
components/manifests/observability/README.md-147-
components/manifests/observability/README.md-148-2. **Check ServiceMonitor is discovered**:
components/manifests/observability/README.md-149- ```bash
components/manifests/observability/README.md:150: oc get servicemonitor ambient-otel-collector -n ambient-code
components/manifests/observability/README.md:151: oc describe servicemonitor ambient-otel-collector -n ambient-code
components/manifests/observability/README.md-152- ```
components/manifests/observability/README.md-153-
components/manifests/observability/README.md-154-3. **Check OTel Collector is receiving metrics**:
components/manifests/observability/README.md-155- ```bash
components/manifests/observability/README.md:156: kubectl logs -n ambient-code -l app=otel-collector | grep -i "metric"
components/manifests/observability/README.md-157- ```
components/manifests/observability/README.md-158-
components/manifests/observability/README.md-159-4. **Check operator is sending metrics**:
--
components/manifests/observability/README.md-163-
components/manifests/observability/README.md-164-5. **Test direct query to OTel Collector**:
components/manifests/observability/README.md-165- ```bash
components/manifests/observability/README.md:166: kubectl port-forward svc/otel-collector 8889:8889 -n ambient-code
components/manifests/observability/README.md-167- curl http://localhost:8889/metrics | grep ambient
components/manifests/observability/README.md-168- ```
components/manifests/observability/README.md-169-
--
components/manifests/observability/base/otel-collector.yaml-1-apiVersion: v1
components/manifests/observability/base/otel-collector.yaml-2-kind: ConfigMap
components/manifests/observability/base/otel-collector.yaml-3-metadata:
components/manifests/observability/base/otel-collector.yaml:4: name: otel-collector-config
components/manifests/observability/base/otel-collector.yaml-5-data:
components/manifests/observability/base/otel-collector.yaml:6: otel-collector-config.yaml: |
components/manifests/observability/base/otel-collector.yaml-7- receivers:
components/manifests/observability/base/otel-collector.yaml-8- otlp:
components/manifests/observability/base/otel-collector.yaml-9- protocols:
--
components/manifests/observability/base/otel-collector.yaml-49-apiVersion: apps/v1
components/manifests/observability/base/otel-collector.yaml-50-kind: Deployment
components/manifests/observability/base/otel-collector.yaml-51-metadata:
components/manifests/observability/base/otel-collector.yaml:52: name: otel-collector
components/manifests/observability/base/otel-collector.yaml-53- labels:
components/manifests/observability/base/otel-collector.yaml:54: app: otel-collector
components/manifests/observability/base/otel-collector.yaml-55-spec:
components/manifests/observability/base/otel-collector.yaml-56- replicas: 1
components/manifests/observability/base/otel-collector.yaml-57- selector:
components/manifests/observability/base/otel-collector.yaml-58- matchLabels:
components/manifests/observability/base/otel-collector.yaml:59: app: otel-collector
components/manifests/observability/base/otel-collector.yaml-60- template:
components/manifests/observability/base/otel-collector.yaml-61- metadata:
components/manifests/observability/base/otel-collector.yaml-62- labels:
components/manifests/observability/base/otel-collector.yaml:63: app: otel-collector
components/manifests/observability/base/otel-collector.yaml-64- spec:
components/manifests/observability/base/otel-collector.yaml-65- containers:
components/manifests/observability/base/otel-collector.yaml:66: - name: otel-collector
components/manifests/observability/base/otel-collector.yaml-67- image: otel/opentelemetry-collector-contrib:0.94.0
components/manifests/observability/base/otel-collector.yaml-68- args:
components/manifests/observability/base/otel-collector.yaml:69: - "--config=/conf/otel-collector-config.yaml"
components/manifests/observability/base/otel-collector.yaml-70- ports:
components/manifests/observability/base/otel-collector.yaml-71- - containerPort: 4317
components/manifests/observability/base/otel-collector.yaml-72- name: otlp-grpc
--
components/manifests/observability/base/otel-collector.yaml-87- volumes:
components/manifests/observability/base/otel-collector.yaml-88- - name: config
components/manifests/observability/base/otel-collector.yaml-89- configMap:
components/manifests/observability/base/otel-collector.yaml:90: name: otel-collector-config
components/manifests/observability/base/otel-collector.yaml-91-
components/manifests/observability/base/otel-collector.yaml-92----
components/manifests/observability/base/otel-collector.yaml-93-apiVersion: v1
components/manifests/observability/base/otel-collector.yaml-94-kind: Service
components/manifests/observability/base/otel-collector.yaml-95-metadata:
components/manifests/observability/base/otel-collector.yaml:96: name: otel-collector
components/manifests/observability/base/otel-collector.yaml-97- labels:
components/manifests/observability/base/otel-collector.yaml:98: app: otel-collector
components/manifests/observability/base/otel-collector.yaml-99-spec:
components/manifests/observability/base/otel-collector.yaml-100- selector:
components/manifests/observability/base/otel-collector.yaml:101: app: otel-collector
components/manifests/observability/base/otel-collector.yaml-102- ports:
components/manifests/observability/base/otel-collector.yaml-103- - port: 4317
components/manifests/observability/base/otel-collector.yaml-104- name: otlp-grpc
--
components/manifests/base/core/operator-deployment.yaml-124- # value: "86400" # Default inactivity timeout in seconds (24h). Set to 0 to disable.
components/manifests/base/core/operator-deployment.yaml-125- # OpenTelemetry configuration
components/manifests/base/core/operator-deployment.yaml-126- # Disabled: OTel collector not deployed. Uncomment when collector is available.
components/manifests/base/core/operator-deployment.yaml:127: # - name: OTEL_EXPORTER_OTLP_ENDPOINT
components/manifests/base/core/operator-deployment.yaml:128: # value: "otel-collector.ambient-code.svc:4317" # Deploy OTel collector separately
components/manifests/base/core/operator-deployment.yaml-129- - name: DEPLOYMENT_ENV
components/manifests/base/core/operator-deployment.yaml-130- value: "production"
components/manifests/base/core/operator-deployment.yaml-131- - name: VERSION
--
components/manifests/base/core/public-api-deployment.yaml-37- - name: RATE_LIMIT_BURST
components/manifests/base/core/public-api-deployment.yaml-38- value: "200"
components/manifests/base/core/public-api-deployment.yaml-39- # OpenTelemetry configuration (optional)
components/manifests/base/core/public-api-deployment.yaml:40: # - name: OTEL_EXPORTER_OTLP_ENDPOINT
components/manifests/base/core/public-api-deployment.yaml:41: # value: "http://otel-collector:4318"
components/manifests/base/core/public-api-deployment.yaml-42- # - name: OTEL_ENABLED
components/manifests/base/core/public-api-deployment.yaml-43- # value: "true"
components/manifests/base/core/public-api-deployment.yaml-44- # CORS configuration (optional, defaults to common dev origins)
Metrics disabled by default in the production base deployment.
OTEL_EXPORTER_OTLP_ENDPOINT is commented out, so the operator disables metrics export unless an overlay restores it. The OTel collector infrastructure exists in components/manifests/observability/base/, but the core operator deployment won't use it without the environment variable set. Either uncomment the endpoint or make it overlay-provided so production deployments don't lose observability silently.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/manifests/base/core/operator-deployment.yaml` around lines 125 -
128, The operator deployment currently has the OTEL_EXPORTER_OTLP_ENDPOINT
environment variable commented out, which disables metrics export by default;
update the operator deployment manifest to either uncomment and set
OTEL_EXPORTER_OTLP_ENDPOINT to the observability collector address
("otel-collector.ambient-code.svc:4317") in the core operator Deployment spec
(the env entry named OTEL_EXPORTER_OTLP_ENDPOINT) or move this env var into a
production overlay so the base stays neutral but overlays enable observability;
ensure the env name OTEL_EXPORTER_OTLP_ENDPOINT is present in the container spec
(containers[].env) so the operator can export metrics to the OTel collector.
Comment on lines
+842
to
+869
| - apiGroups: | ||
| - '' | ||
| resources: | ||
| - serviceaccounts | ||
| verbs: | ||
| - get | ||
| - list | ||
| - create | ||
| - update | ||
| - patch | ||
| - apiGroups: | ||
| - rbac.authorization.k8s.io | ||
| resources: | ||
| - roles | ||
| - rolebindings | ||
| verbs: | ||
| - get | ||
| - list | ||
| - create | ||
| - update | ||
| - patch | ||
| - delete | ||
| - apiGroups: | ||
| - '' | ||
| resources: | ||
| - serviceaccounts/token | ||
| verbs: | ||
| - create |
Contributor
There was a problem hiding this comment.
RBAC: ambient-project-edit can mint tokens for any ServiceAccount in the namespace.
serviceaccounts/token: create with no resourceNames restriction lets an edit-level user impersonate any SA in the project — including SAs bound to elevated cluster roles (e.g., runner SAs the backend creates). Combined with rolebindings: create/update/delete on roles, this is effectively an admin path via the edit role. Since this PR moves auth to project-based perms, please tighten the edit role to either omit token creation or restrict it via resourceNames to a specific runner SA.
As per coding guidelines: "RBAC must follow least-privilege."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/manifests/templates/template-operator.yaml` around lines 842 -
869, The RBAC rule in the ambient-project-edit role currently allows creating
tokens for any ServiceAccount via the "serviceaccounts/token: create"
permission; tighten this by either removing that verb from the rule or limiting
it with a resourceNames list to only the dedicated runner ServiceAccount(s)
(e.g., the project runner SA name(s) your operator uses). Locate the
ambient-project-edit role rule block in template-operator.yaml (the rule that
lists "serviceaccounts/token" and "create") and change it to omit token creation
or add explicit resourceNames entries for the approved SA(s), and ensure any
RoleBinding or Role creation logic that expects broader token creation is
updated accordingly. Ensure the final role follows least-privilege (no wildcard
token creation) and update tests/docs referencing ambient-project-edit if
needed.
Comment on lines
+1343
to
+1348
| - name: STATE_SYNC_IMAGE | ||
| value: quay.io/ambient_code/vteam_state_sync:latest | ||
| - name: S3_ENDPOINT | ||
| value: http://minio.ambient-code.svc:9000 | ||
| - name: S3_BUCKET | ||
| value: ambient-sessions |
Contributor
There was a problem hiding this comment.
S3_ENDPOINT hardcoded to in-cluster MinIO contradicts PR intent.
The PR summary says in-cluster MinIO is being removed in favor of external S3, but the operator deployment hardcodes S3_ENDPOINT: http://minio.ambient-code.svc:9000 (and S3_BUCKET: ambient-sessions). Runner pods spawned by the operator will attempt to hit a Service that won't exist in the app-interface deployment, breaking state-sync/persistence. Parameterize these (as you did for IMAGE_TAG/IMAGE_OPERATOR) or source them from operator-config so the app-interface overlay can set real RDS/S3 values.
🛠️ Suggested fix
+ - name: S3_ENDPOINT
+ valueFrom:
+ configMapKeyRef:
+ key: S3_ENDPOINT
+ name: operator-config
+ optional: false
+ - name: S3_BUCKET
+ valueFrom:
+ configMapKeyRef:
+ key: S3_BUCKET
+ name: operator-config
+ optional: false
- - name: S3_ENDPOINT
- value: http://minio.ambient-code.svc:9000
- - name: S3_BUCKET
- value: ambient-sessions📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: STATE_SYNC_IMAGE | |
| value: quay.io/ambient_code/vteam_state_sync:latest | |
| - name: S3_ENDPOINT | |
| value: http://minio.ambient-code.svc:9000 | |
| - name: S3_BUCKET | |
| value: ambient-sessions | |
| - name: STATE_SYNC_IMAGE | |
| value: quay.io/ambient_code/vteam_state_sync:latest | |
| - name: S3_ENDPOINT | |
| valueFrom: | |
| configMapKeyRef: | |
| key: S3_ENDPOINT | |
| name: operator-config | |
| optional: false | |
| - name: S3_BUCKET | |
| valueFrom: | |
| configMapKeyRef: | |
| key: S3_BUCKET | |
| name: operator-config | |
| optional: false |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/manifests/templates/template-operator.yaml` around lines 1343 -
1348, The operator manifest currently hardcodes S3_ENDPOINT and S3_BUCKET
(S3_ENDPOINT: http://minio.ambient-code.svc:9000, S3_BUCKET: ambient-sessions)
which conflicts with removing in-cluster MinIO; change these to be templated or
sourced from operator config the same way IMAGE_TAG/IMAGE_OPERATOR are handled:
replace the hardcoded S3_ENDPOINT and S3_BUCKET env values with template
variables (e.g., {{ .Values.s3.endpoint }} / {{ .Values.s3.bucket }}) or switch
the env entries to pull from the operator ConfigMap (operator-config) via
envFrom or valueFrom so the app-interface overlay can supply the real external
S3/RDS values at deploy time, and ensure any runner/state-sync code reading S3_*
uses those env names unchanged.
Comment on lines
+25
to
+26
| - name: UPSTREAM_TIMEOUT | ||
| value: 5m |
Contributor
There was a problem hiding this comment.
UPSTREAM_TIMEOUT parameter is declared but never referenced.
Template parameter UPSTREAM_TIMEOUT (line 25-26, default 5m) is declared but the oauth-proxy args hardcode --upstream-timeout=5m (line 689). The parameter is dead weight — either wire it in or drop it. Given long-running SSE/streaming endpoints the frontend likely needs, making this tunable is probably the intent.
🛠️ Suggested fix
- - --upstream-timeout=5m
+ - --upstream-timeout=${UPSTREAM_TIMEOUT}Also applies to: 689-689
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/manifests/templates/template-services.yaml` around lines 25 - 26,
The UPSTREAM_TIMEOUT template parameter is declared as UPSTREAM_TIMEOUT (default
"5m") but not used; update the oauth-proxy container args (the line that
currently hardcodes "--upstream-timeout=5m") to reference the template parameter
instead (e.g., substitute the hardcoded value with the UPSTREAM_TIMEOUT template
variable), or remove the UPSTREAM_TIMEOUT parameter if you prefer to keep the
hardcoded value; make the change where the oauth-proxy args are constructed so
the arg uses the template variable name UPSTREAM_TIMEOUT.
Comment on lines
+180
to
+194
| - apiVersion: v1 | ||
| kind: PersistentVolumeClaim | ||
| metadata: | ||
| labels: | ||
| app: postgresql | ||
| app.kubernetes.io/component: database | ||
| app.kubernetes.io/name: postgresql | ||
| name: postgresql-data | ||
| namespace: ambient-code | ||
| spec: | ||
| accessModes: | ||
| - ReadWriteOnce | ||
| resources: | ||
| requests: | ||
| storage: 10Gi |
Contributor
There was a problem hiding this comment.
Leftover postgresql-data PVC after RDS migration.
PR summary states in-cluster Postgres was removed in favor of RDS, and ambient-api-server now consumes ambient-code-rds via the renamed Secret — but this 10Gi postgresql-data PVC is still instantiated by the template with no workload mounting it. It's orphaned storage that app-interface will provision on every deploy.
-- apiVersion: v1
- kind: PersistentVolumeClaim
- metadata:
- labels:
- app: postgresql
- app.kubernetes.io/component: database
- app.kubernetes.io/name: postgresql
- name: postgresql-data
- namespace: ambient-code
- spec:
- accessModes:
- - ReadWriteOnce
- resources:
- requests:
- storage: 10Gi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - apiVersion: v1 | |
| kind: PersistentVolumeClaim | |
| metadata: | |
| labels: | |
| app: postgresql | |
| app.kubernetes.io/component: database | |
| app.kubernetes.io/name: postgresql | |
| name: postgresql-data | |
| namespace: ambient-code | |
| spec: | |
| accessModes: | |
| - ReadWriteOnce | |
| resources: | |
| requests: | |
| storage: 10Gi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/manifests/templates/template-services.yaml` around lines 180 -
194, The template still creates an orphaned PersistentVolumeClaim named
"postgresql-data" (PersistentVolumeClaim block in template-services.yaml) even
though in-cluster Postgres was removed; delete this PVC resource from the
template (or wrap it behind a values flag such as enablePostgres or
usePostgresPVC and gate its creation) and remove any references to
"postgresql-data" so app-interface stops provisioning the 10Gi claim on each
deploy; update the template that defines the PersistentVolumeClaim and any
Helm/values logic that would otherwise render it.
Comment on lines
+242
to
+243
| - --cors-allowed-origins=* | ||
| - --cors-allowed-headers=X-Ambient-Project |
Contributor
There was a problem hiding this comment.
--cors-allowed-origins=* is too permissive for a production API behind OAuth.
With the frontend now behind an OAuth proxy issuing cookies, a wildcard CORS allowlist on ambient-api-server invites cross-origin requests from any origin. Lock this down to the actual frontend route host (parameterizable via the template, e.g. FRONTEND_HOST).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/manifests/templates/template-services.yaml` around lines 242 -
243, The manifest currently sets --cors-allowed-origins=* which is too
permissive; change the flag in the ambient-api-server container args to use a
templated frontend host variable (e.g. FRONTEND_HOST) instead of *, e.g. set
--cors-allowed-origins to the FRONTEND_HOST template value and ensure
FRONTEND_HOST is declared/parameterized in the template and populated at deploy
time; keep the existing --cors-allowed-headers (X-Ambient-Project) intact.
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.
Authorization is handled through the backend. Attempting to remove
--openshift-delegate-urlstemporarily.Summary by CodeRabbit