fix(api-server): complete TLS enablement and upgrade to rh-trex-ai v0.0.19#843
fix(api-server): complete TLS enablement and upgrade to rh-trex-ai v0.0.19#843
Conversation
markturansky
force-pushed
the
fix/database_pod_security_context
branch
from
55a5d48 to
930e412
Compare
Claude Code ReviewSummaryThis PR enables end-to-end TLS for the ambient-api-server, upgrades rh-trex-ai from v0.0.14 to v0.0.19, fixes OpenShift SCC compliance, and scopes JWKS keys per environment. The TLS enablement and security posture improvements are meaningful and well-executed. However, there is a fragile positional JSON patch targeting a hard-coded command array index that poses a silent misconfiguration risk in production. Issues by SeverityBlocker IssuesFragile positional JSON patch for TLS arguments Suggested fix: Use a strategic merge patch or a named-container command replacement instead of positional JSON patches: # strategic merge patch approach
spec:
template:
spec:
containers:
- name: ambient-api-server
command:
- /usr/local/bin/ambient-api-server
- --tls-cert-file=/etc/tls/tls.crt
- --tls-key-file=/etc/tls/tls.key
- --jwk-cert-url=$(JWK_CERT_URL)Critical IssuesNone Major IssuesMissing TLS secret documentation/validation Suggested fix: Add a comment in the kustomization or a companion rh-trex-ai bump spans 5 minor versions with no changelog reference Suggested fix: Add a comment citing the changelog or key changes, e.g.: # rh-trex-ai v0.0.19: fixes JWT audience validation bug (see upstream CHANGELOG)
newTag: v0.0.19Minor IssuesRepeated patch blocks could use anchors Positive Highlights
Recommendations
|
markturansky
force-pushed
the
fix/database_pod_security_context
branch
2 times, most recently
from
c4154ae to
cb4b501
Compare
|
TEST |
|
Claude Code Review - full review posted below |
|
Claude Code Review Summary PR #843 implements end-to-end TLS for the ambient-api-server by enabling HTTPS REST, gRPC TLS, and upgrading from OpenShift edge to reencrypt TLS termination. It also upgrades rh-trex-ai to v0.0.19, fixes OpenShift SCC compliance for the database pod, and introduces Kustomize overlays for production JWT configuration. The core TLS implementation is sound, but several configuration choices in the production overlay warrant attention before merge. Issues by Severity Blocker Issues: None Critical Issues 1. Authorization explicitly disabled in production overlay
Major Issues 1.
2. Database connections unencrypted in production
3. No TLS certificate provisioning for Kind environment
4.
Minor Issues 1. Missing trailing newline in
2. Hardcoded RSA public key committed to Kind overlay
Positive Highlights
Recommendations
Review performed by Claude Code using repository standards from |
…ix OpenShift SCC - Upgrade rh-trex-ai from v0.0.17 to v0.0.19 - Enable end-to-end HTTPS and gRPC TLS in base manifest - Add OpenShift serving-cert annotation for TLS secret provisioning - Fix OpenShift SCC: drop ALL capabilities on init and main containers - Fix EnableJWT breaking change introduced in v0.0.19 - Use quay.io images (revert from local registry) - Update probe schemes HTTP → HTTPS 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…rlays - Dev: load JWKS from local file (secrets/kind-jwks.json), disable gRPC JWT - Production: use Red Hat SSO URL via manifest JSON patch - Cap log verbosity at -v=8 to prevent Authorization header logging at V(10) - Base ConfigMap: empty JWKS (no cluster-specific keys in shared base) - Kind overlay: ConfigMap patch with Kind cluster RSA public key - Production overlay: JSON patch replaces --jwk-cert-file with --jwk-cert-url 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
markturansky
force-pushed
the
fix/database_pod_security_context
branch
3 times, most recently
from
ab654ff to
ad4af82
Compare
|
test |
|
Claude Code Review - PR 843 findings: MAJOR: (1) Duplicate JWK URL in e_production.go lines 21-23 - Server.JwkCertURL and Auth.JwkCertURL both set to same SSO URL; future update to one but not the other will silently misconfigure one code path. Consolidate or document why both must be kept in sync. (2) Wildcard CORS in production overlay ambient-api-server-jwt-args-patch.yaml line 37 --cors-allowed-origins=* allows any origin to make cross-site requests with a valid token; scope to the frontend hostname instead. MINOR: (1) fsGroup: 999 removed from PostgreSQL pod spec without explaining how volume permissions are maintained. (2) PR description says v0.0.17 to v0.0.19 but go.mod shows v0.0.14 to v0.0.19. (3) go.mod inline version comment may be stripped by go mod tidy. POSITIVE: Critical bug fixed - enable-mock: true was in ProductionEnvImpl.Flags() and is now removed. Edge-to-reencrypt TLS termination is an excellent security improvement. DB SSL enforced with db-sslmode=require. Clean Kustomize overlay separation for dev vs production config. Verbosity reduced from -v=10 to -v=2 for production. |
|
Deleting the test comment above - see my review below. |
Refactor base manifests to be a minimal no-TLS default that works for Kind without overrides. TLS, service-ca, RHEL DB image, and init-db configuration are now scoped to overlay-specific patches for local-dev and production. Remove the now-unnecessary Kind no-tls-patch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
maskarb
force-pushed
the
fix/database_pod_security_context
branch
from
ad4af82 to
338064f
Compare
WalkthroughChanges reconfigure authentication and TLS handling across development, production, and local-dev environments. Middleware type references shift from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
|
Claude Code Review Summary PR 843 implements end-to-end TLS for the ambient-api-server component, upgrades rh-trex-ai to v0.0.19, enables JWT validation, and fixes OpenShift SCC compliance for the database pod. The structural approach of moving TLS and JWT config into overlay patches is sound. However, the SCC fix went too far by stripping all container-level security constraints from the base manifest, and the production deployment has a CORS wildcard that requires attention before merge. BLOCKER 1: Container SecurityContext stripped from database pod base manifest File: components/manifests/base/ambient-api-server-db.yml The PR removes the entire container-level securityContext block (allowPrivilegeEscalation: false, capabilities.drop: ALL) from the PostgreSQL container. The PR body says the motivation was removing fsGroup: 999 (blocked by restricted-v2 SCC), but fsGroup is in the pod-level securityContext, not the container-level block. On Kind and vanilla K8s, the database container now has no privilege escalation prevention. The RHEL image patches replace image/env/probes but do not restore securityContext. Standard violated: security-standards.md - Always Set SecurityContext: AllowPrivilegeEscalation: false, Capabilities.Drop: ALL Fix: Remove only fsGroup: 999 from the pod-level spec. Retain the container-level securityContext block in the base manifest. BLOCKER 2: Init containers added without any SecurityContext Files: overlays/local-dev/ambient-api-server-init-db-patch.yaml, overlays/production/ambient-api-server-init-db-patch.yaml The init-db init container has no securityContext at all. On Kind and vanilla K8s it runs unconstrained with no privilege escalation prevention. Standard violated: security-standards.md - Container SecurityContext: AllowPrivilegeEscalation: false, Drop: ALL Fix: Add securityContext to the init-db container in both patches: allowPrivilegeEscalation: false, runAsNonRoot: true, capabilities.drop: [ALL]. CRITICAL 3: CORS wildcard in production deployment Files: overlays/production/ambient-api-server-jwt-args-patch.yaml, overlays/local-dev/ambient-api-server-tls-patch.yaml Both production and local-dev TLS patches pass --cors-allowed-origins=*. Even with JWT auth enabled, a wildcard CORS policy permits any malicious website to use a victim's stored token to make cross-origin API calls from the browser. Standard violated: security-standards.md - Input validation and scope-restriction; OWASP CORS misconfiguration. Fix: Pin CORS to known frontend origins. If the server is truly internal-only and never browser-accessible, document this explicitly. CRITICAL 4: Authorization disabled in production with no documented compensating network control Files: overlays/production/ambient-api-server-jwt-args-patch.yaml, base/ambient-api-server-service.yml --enable-authz=false disables the server authorization middleware in production. The comment says auth is handled by the backend proxy with a TODO. No NetworkPolicy restricts access to the api-server port, so any pod in the cluster can bypass authorization by calling port 8000 directly. Standard violated: security-standards.md - RBAC check performed before resource access; defense-in-depth. Fix: Add a NetworkPolicy restricting api-server ingress to only backend proxy pods, or document the compensating control and file a tracking issue for the authz TODO. MAJOR 5: Identical patch files duplicated across overlays overlays/local-dev/ambient-api-server-init-db-patch.yaml and overlays/production/ambient-api-server-init-db-patch.yaml are byte-for-byte identical. Same for both ambient-api-server-db-json-patch.yaml files. Any update must be made in two places and will silently diverge. Fix: Move shared patches to manifests/base/ or a shared directory. MAJOR 6: Duplicate JwkCertURL config in e_production.go File: components/ambient-api-server/cmd/ambient-api-server/environments/e_production.go The same JWKS URL is assigned to both c.Server.JwkCertURL and c.Auth.JwkCertURL. If the SSO endpoint changes, only one field might get updated, silently breaking auth in one code path. Fix: Use a single constant assigned to both fields. MINOR 7: secrets/db.port missing end-of-file newline The end-of-file-fixer pre-commit hook will fire on the next commit touching this file. MINOR 8: Local-dev and production TLS patches are near-identical overlays/local-dev/ambient-api-server-tls-patch.yaml and overlays/production/ambient-api-server-jwt-args-patch.yaml are 79-line files that differ only in the JWKS source flag. Consider a shared base patch. Positive Highlights
Recommendations
Generated by Claude Code |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/manifests/base/ambient-api-server-service.yml (1)
70-88:⚠️ Potential issue | 🟠 MajorAllow
Authorizationin CORS preflights.With
--enable-jwt=true, browser clients will send anAuthorizationheader, but the allowlist still only includesX-Ambient-Project. Cross-origin authenticated requests will fail at the preflight step.Suggested patch
- - --cors-allowed-headers=X-Ambient-Project + - --cors-allowed-headers=Authorization,X-Ambient-Project🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/manifests/base/ambient-api-server-service.yml` around lines 70 - 88, The CORS allowed headers flag (--cors-allowed-headers) currently only lists X-Ambient-Project which blocks browser preflight requests that include Authorization when --enable-jwt=true; update the manifest to include Authorization in the --cors-allowed-headers value (e.g. add ",Authorization" or include both headers explicitly) so preflight requests with the Authorization header are permitted by the api-server.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/ambient-api-server/go.mod`:
- Line 13: Check whether rh-trex-ai v0.0.19 still defines and uses the EnableJWT
field (used in e_development.go) and adjust usage accordingly: open
e_development.go and locate references to EnableJWT and any JWT-related config,
confirm the field and behavior exist in the v0.0.19 API (or replace with the new
config key/constructor), update code to remove or rename the field if it was
deprecated, and recompile/run tests to ensure plugin registration via
registry.RegisterService still works with the upgraded library; run go mod tidy
to ensure dependencies are consistent.
In `@components/ambient-api-server/plugins/users/plugin.go`:
- Around line 51-52: The plugin template generate-plugin.txt is still emitting
the old callback type auth.JWTMiddleware which no longer matches the
RegisterRoutes signature; update the template so the generated RegisterRoutes
callback uses environments.JWTMiddleware instead of auth.JWTMiddleware (so
generated plugins like users/project/sessions/projectSettings will declare
func(apiV1Router *mux.Router, services pkgserver.ServicesInterface,
authMiddleware environments.JWTMiddleware, authzMiddleware
auth.AuthorizationMiddleware) and compile against the current RegisterRoutes).
In `@components/manifests/base/ambient-api-server-db.yml`:
- Around line 84-101: The readinessProbe and livenessProbe use pg_isready
without specifying a database, causing checks to default to the user name;
update the exec commands for readinessProbe and livenessProbe to include the
database flag -d "$POSTGRES_DB" alongside -U "$POSTGRES_USER" (i.e., pg_isready
-U "$POSTGRES_USER" -d "$POSTGRES_DB"); apply this same change in the local-dev
and production overlay patches and in the postgresql-deployment.yaml and its
overlays so all probes consistently pass the POSTGRES_DB variable.
In
`@components/manifests/overlays/local-dev/ambient-api-server-init-db-patch.yaml`:
- Around line 21-37: The script is using PGDATABASE (the target DB to be
created) for health checks and psql commands, so on a fresh install those
commands try to connect to a non‑existent DB; change the admin connections to
use an existing admin DB (e.g., "postgres") by passing -d postgres (or
temporarily setting PGDATABASE=postgres) to pg_isready/psql calls used for the
readiness check, the existence test (psql -h "$PGHOST" -U "$PGUSER" -d postgres
-lqt | ...), and the CREATE DATABASE invocation (keep CREATE DATABASE
$PGDATABASE; executed against the admin connection) so the script can probe and
create the target DB reliably.
- Around line 10-12: The init container "init-db" lacks a container-level
securityContext which can cause admission failures on restricted SCC; add a
securityContext to the initContainers -> name: init-db entry that specifies
allowPrivilegeEscalation: false, privileged: false, runAsNonRoot: true (and a
non-root runAsUser uid if appropriate), readOnlyRootFilesystem: true, and drop
all capabilities (capabilities: drop: ["ALL"]), and optionally set
seccompProfile: type: RuntimeDefault to match the pod-level hardening.
In `@components/manifests/overlays/local-dev/ambient-api-server-tls-patch.yaml`:
- Around line 21-23: The overlay currently disables internal authz via the flag
--enable-authz=false while still exposing the service through
ambient-api-server-route.yaml in the local-dev kustomization, creating a direct
Route that bypasses proxy-enforced authorization; fix this by either re-enabling
internal authorization (remove or set --enable-authz=false to true) on the
ambient API server invocation or remove ambient-api-server-route.yaml from the
local-dev kustomization so the service is only reachable via the backend proxy;
update whichever of the two you choose (the flag in
ambient-api-server-tls-patch.yaml or the inclusion of
ambient-api-server-route.yaml in the kustomization) to ensure the Route cannot
be reached without proxy authz.
In `@components/manifests/overlays/local-dev/kustomization.yaml`:
- Around line 40-41: The DB credentials patch
ambient-api-server-db-credentials-patch.yaml is missing an explicit target entry
in the kustomization (unlike the other ambient-api-server patches); update the
kustomization.yaml to add a matching target selector for
ambient-api-server-db-credentials-patch.yaml (same style as the existing
ambient-api-server patches) so it explicitly targets the ambient-api-server
resource(s) (use the same kind/name/namespace or label selector pattern used by
the other patches) to prevent accidental metadata drift.
In
`@components/manifests/overlays/production/ambient-api-server-init-db-patch.yaml`:
- Around line 10-12: The initContainers entry for the init-db container lacks a
Pod/Container securityContext to satisfy restricted-v2 SCC; update the init
container named "init-db" to include a securityContext that sets
allowPrivilegeEscalation: false, drops all Linux capabilities (e.g.,
capabilities.drop: ["ALL"]), and sets readOnlyRootFilesystem: true (and
runAsNonRoot/runAsUser as appropriate) so the init container adheres to SCC
hardening and restricted policies.
- Around line 21-37: The readiness and existence checks (pg_isready and psql
invocations) currently use PGDATABASE and may try to connect to the target DB
before it exists; change these commands to target the maintenance DB (e.g.,
"postgres") instead of "$PGDATABASE" for readiness, listing databases, and
running the CREATE DATABASE statement (use pg_isready -d postgres and psql -d
postgres or supply -c with a connection to postgres) while still creating
"$PGDATABASE" with a CREATE DATABASE "$PGDATABASE" executed against the
maintenance DB; reference the pg_isready and psql calls and the PGDATABASE
variable when making this change.
In
`@components/manifests/overlays/production/ambient-api-server-jwt-args-patch.yaml`:
- Line 44: The CORS flag currently uses a wildcard (--cors-allowed-origins=*)
which is too permissive for production; change the --cors-allowed-origins
argument to a restricted, comma-separated list of trusted origins (e.g.,
https://app.example.com,https://admin.example.com) or reference a deployment
environment variable (e.g., --cors-allowed-origins=${CORS_ALLOWED_ORIGINS}) so
allowed domains can be managed securely; update the manifest entry that sets the
--cors-allowed-origins flag accordingly and ensure the runtime/env var contains
only the approved origins.
---
Outside diff comments:
In `@components/manifests/base/ambient-api-server-service.yml`:
- Around line 70-88: The CORS allowed headers flag (--cors-allowed-headers)
currently only lists X-Ambient-Project which blocks browser preflight requests
that include Authorization when --enable-jwt=true; update the manifest to
include Authorization in the --cors-allowed-headers value (e.g. add
",Authorization" or include both headers explicitly) so preflight requests with
the Authorization header are permitted by the api-server.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 15e16366-d4ce-45d5-99c7-eb1005795ab9
⛔ Files ignored due to path filters (1)
components/ambient-api-server/go.sumis excluded by!**/*.sum
📒 Files selected for processing (28)
components/ambient-api-server/cmd/ambient-api-server/environments/e_development.gocomponents/ambient-api-server/cmd/ambient-api-server/environments/e_production.gocomponents/ambient-api-server/go.modcomponents/ambient-api-server/plugins/projectSettings/plugin.gocomponents/ambient-api-server/plugins/projects/plugin.gocomponents/ambient-api-server/plugins/sessions/plugin.gocomponents/ambient-api-server/plugins/users/plugin.gocomponents/ambient-api-server/secrets/db.portcomponents/manifests/base/ambient-api-server-db.ymlcomponents/manifests/base/ambient-api-server-route.ymlcomponents/manifests/base/ambient-api-server-secrets.ymlcomponents/manifests/base/ambient-api-server-service.ymlcomponents/manifests/overlays/kind/ambient-api-server-jwks-patch.yamlcomponents/manifests/overlays/kind/kustomization.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/local-dev/ambient-api-server-route.yamlcomponents/manifests/overlays/local-dev/ambient-api-server-service-ca-patch.yamlcomponents/manifests/overlays/local-dev/ambient-api-server-tls-patch.yamlcomponents/manifests/overlays/local-dev/kustomization.yamlcomponents/manifests/overlays/production/ambient-api-server-db-json-patch.yamlcomponents/manifests/overlays/production/ambient-api-server-env-patch.yamlcomponents/manifests/overlays/production/ambient-api-server-init-db-patch.yamlcomponents/manifests/overlays/production/ambient-api-server-jwt-args-patch.yamlcomponents/manifests/overlays/production/ambient-api-server-route.yamlcomponents/manifests/overlays/production/ambient-api-server-service-ca-patch.yamlcomponents/manifests/overlays/production/kustomization.yaml
💤 Files with no reviewable changes (1)
- components/manifests/base/ambient-api-server-route.yml
| github.com/onsi/gomega v1.27.1 | ||
| github.com/openshift-online/ocm-sdk-go v0.1.334 | ||
| github.com/openshift-online/rh-trex-ai v0.0.14 | ||
| github.com/openshift-online/rh-trex-ai v0.0.19 // v0.0.14→v0.0.19: gRPC TLS support, JWT audience validation fix, OpenAPI generator improvements |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for any remaining usage of EnableJWT field that may have been removed
rg -n "EnableJWT" --type go components/ambient-api-server/
# Search for auth.JWTMiddleware which should now be environments.JWTMiddleware per AI summary
rg -n "auth\.JWTMiddleware" --type go components/ambient-api-server/Repository: ambient-code/platform
Length of output: 47
🏁 Script executed:
#!/bin/bash
# List Go files in the directory
echo "=== Go files in components/ambient-api-server/ ==="
find components/ambient-api-server/ -name "*.go" -type f | head -20
# Search for any imports of rh-trex-ai
echo "=== Imports of rh-trex-ai ==="
rg -n "rh-trex-ai\|rh_trex_ai" --type go components/ambient-api-server/
# Search for any usage of trex or rh-trex related types/packages
echo "=== Potential trex package usage ==="
rg -n "trex|\"rhtrex" --type go components/ambient-api-server/ | head -20Repository: ambient-code/platform
Length of output: 3816
🏁 Script executed:
#!/bin/bash
# Check main.go for initialization
echo "=== main.go ==="
cat -n components/ambient-api-server/cmd/ambient-api-server/main.go
echo -e "\n=== environments.go ==="
cat -n components/ambient-api-server/cmd/ambient-api-server/environments/environments.go | head -100
echo -e "\n=== environments/types.go ==="
cat -n components/ambient-api-server/cmd/ambient-api-server/environments/types.goRepository: ambient-code/platform
Length of output: 4067
🏁 Script executed:
#!/bin/bash
# Check middleware
echo "=== middleware/forwarded_token.go ==="
cat -n components/ambient-api-server/pkg/middleware/forwarded_token.go
echo -e "\n=== Check for JWT or security middleware ==="
find components/ambient-api-server/pkg/middleware -name "*.go" -type f
echo -e "\n=== Check environment implementations for JWT ==="
grep -n -i "jwt\|middleware\|security" components/ambient-api-server/cmd/ambient-api-server/environments/e_*.go
echo -e "\n=== Check for plugin registration patterns ==="
grep -n "Register\|Factory" components/ambient-api-server/plugins/users/factory.go 2>/dev/null | head -20Repository: ambient-code/platform
Length of output: 1366
🏁 Script executed:
#!/bin/bash
# Check all environment implementations
echo "=== e_development.go ==="
cat -n components/ambient-api-server/cmd/ambient-api-server/environments/e_development.go
echo -e "\n=== e_production.go ==="
cat -n components/ambient-api-server/cmd/ambient-api-server/environments/e_production.go
echo -e "\n=== e_integration_testing.go ==="
cat -n components/ambient-api-server/cmd/ambient-api-server/environments/e_integration_testing.go
echo -e "\n=== e_unit_testing.go ==="
cat -n components/ambient-api-server/cmd/ambient-api-server/environments/e_unit_testing.goRepository: ambient-code/platform
Length of output: 7480
🏁 Script executed:
#!/bin/bash
# Read the go.mod file
echo "=== go.mod ==="
cat -n components/ambient-api-server/go.mod
echo -e "\n=== Search for rh-trex-ai require ==="
grep -n "rh-trex-ai" components/ambient-api-server/go.modRepository: ambient-code/platform
Length of output: 7149
🏁 Script executed:
#!/bin/bash
# Check for build output or test results
echo "=== Look for any go.sum entries for rh-trex-ai ==="
grep -n "rh-trex-ai" components/ambient-api-server/go.sum | head -5
# Search for any TODO or FIXME comments related to JWT or EnableJWT
echo -e "\n=== TODO/FIXME comments ==="
rg -n "TODO|FIXME" --type go components/ambient-api-server/ | grep -i "jwt\|enable\|breaking"
# Look for any recent git history on e_development.go
echo -e "\n=== Check git status of environment files ==="
git -C components/ambient-api-server/ log --oneline -10 -- cmd/ambient-api-server/environments/e_development.go 2>/dev/null || echo "Git history not available"
# Check if there's a CHANGELOG or release notes
echo -e "\n=== Look for changelog/docs ==="
find components/ambient-api-server -maxdepth 2 -iname "*change*" -o -iname "*release*" -o -iname "*breaking*"Repository: ambient-code/platform
Length of output: 510
🏁 Script executed:
#!/bin/bash
# Check plugin files for registration patterns
echo "=== Plugin factory files ==="
find components/ambient-api-server/plugins -name "factory.go" -o -name "plugin.go" | xargs -I {} sh -c 'echo "File: {}"; head -50 {}' 2>/dev/null
# Search for any plugin registration functions
echo -e "\n=== Plugin registration patterns ==="
rg -n "RegisterPlugin\|Register\|init\(\)" --type go components/ambient-api-server/plugins/ -A 3 | head -40
# Check if tests pass to verify compatibility
echo -e "\n=== Look for test files that would validate v0.0.19 compatibility ==="
find components/ambient-api-server -name "*_test.go" | wc -lRepository: ambient-code/platform
Length of output: 6306
🏁 Script executed:
#!/bin/bash
# Check for gRPC TLS configuration
echo "=== gRPC TLS references ==="
rg -n "TLS\|tls\|grpc" --type go components/ambient-api-server/ | grep -i "tls\|cipher\|cert" | head -15
# Check for JWT audience validation
echo -e "\n=== JWT audience references ==="
rg -n "audience\|Audience" --type go components/ambient-api-server/
# Check for any config related to JWT
echo -e "\n=== JWT configuration ==="
rg -n "JWT\|Jwt" --type go components/ambient-api-server/cmd/ambient-api-server/environments/
# Check ApplicationConfig struct to understand EnableJWT field
echo -e "\n=== Check if ApplicationConfig is used elsewhere ==="
rg -n "ApplicationConfig" --type go components/ambient-api-server/ | head -15Repository: ambient-code/platform
Length of output: 206
Verify rh-trex-ai v0.0.19 compatibility.
The upgrade from v0.0.14 to v0.0.19 includes framework-level improvements (gRPC TLS support, JWT audience validation). The codebase reference to EnableJWT field appears only in e_development.go (line 24), and plugin registration via registry.RegisterService is consistent across all plugins. Ensure the EnableJWT field is still supported in v0.0.19 or remove if deprecated.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/ambient-api-server/go.mod` at line 13, Check whether rh-trex-ai
v0.0.19 still defines and uses the EnableJWT field (used in e_development.go)
and adjust usage accordingly: open e_development.go and locate references to
EnableJWT and any JWT-related config, confirm the field and behavior exist in
the v0.0.19 API (or replace with the new config key/constructor), update code to
remove or rename the field if it was deprecated, and recompile/run tests to
ensure plugin registration via registry.RegisterService still works with the
upgraded library; run go mod tidy to ensure dependencies are consistent.
| pkgserver.RegisterRoutes("users", func(apiV1Router *mux.Router, services pkgserver.ServicesInterface, authMiddleware environments.JWTMiddleware, authzMiddleware auth.AuthorizationMiddleware) { | ||
| envServices := services.(*environments.Services) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the template file
find . -name "generate-plugin.txt" -type fRepository: ambient-code/platform
Length of output: 126
🏁 Script executed:
# Also search for RegisterRoutes pattern to understand the signature change
rg "RegisterRoutes" --type go -A 2 -B 2Repository: ambient-code/platform
Length of output: 2605
🏁 Script executed:
# Examine the template file content
cat -n ./components/ambient-api-server/templates/generate-plugin.txtRepository: ambient-code/platform
Length of output: 3720
🏁 Script executed:
# Check if there are any other plugin files that might not have been updated
find ./components/ambient-api-server/plugins -name "plugin.go" -type fRepository: ambient-code/platform
Length of output: 304
Update the plugin generator to match the new RegisterRoutes signature.
Line 47 of components/ambient-api-server/templates/generate-plugin.txt still emits auth.JWTMiddleware, causing template drift. All checked-in plugins (users, projects, sessions, projectSettings) have been updated to use environments.JWTMiddleware, and the next generated plugin will reintroduce the old callback shape without this fix.
🛠️ Template update
- pkgserver.RegisterRoutes("{{.KindLowerPlural}}", func(apiV1Router *mux.Router, services pkgserver.ServicesInterface, authMiddleware auth.JWTMiddleware, authzMiddleware auth.AuthorizationMiddleware) {
+ pkgserver.RegisterRoutes("{{.KindLowerPlural}}", func(apiV1Router *mux.Router, services pkgserver.ServicesInterface, authMiddleware environments.JWTMiddleware, authzMiddleware auth.AuthorizationMiddleware) {📝 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.
| pkgserver.RegisterRoutes("users", func(apiV1Router *mux.Router, services pkgserver.ServicesInterface, authMiddleware environments.JWTMiddleware, authzMiddleware auth.AuthorizationMiddleware) { | |
| envServices := services.(*environments.Services) | |
| pkgserver.RegisterRoutes("{{.KindLowerPlural}}", func(apiV1Router *mux.Router, services pkgserver.ServicesInterface, authMiddleware environments.JWTMiddleware, authzMiddleware auth.AuthorizationMiddleware) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/ambient-api-server/plugins/users/plugin.go` around lines 51 - 52,
The plugin template generate-plugin.txt is still emitting the old callback type
auth.JWTMiddleware which no longer matches the RegisterRoutes signature; update
the template so the generated RegisterRoutes callback uses
environments.JWTMiddleware instead of auth.JWTMiddleware (so generated plugins
like users/project/sessions/projectSettings will declare func(apiV1Router
*mux.Router, services pkgserver.ServicesInterface, authMiddleware
environments.JWTMiddleware, authzMiddleware auth.AuthorizationMiddleware) and
compile against the current RegisterRoutes).
| readinessProbe: | ||
| exec: | ||
| command: | ||
| - /bin/sh | ||
| - -c | ||
| - exec /usr/bin/pg_isready -U $POSTGRES_USER -d $POSTGRES_DB -h localhost -p 5432 | ||
| initialDelaySeconds: 5 | ||
| timeoutSeconds: 1 | ||
| volumeMounts: | ||
| - mountPath: /var/lib/pgsql/data | ||
| name: ambient-api-server-db-data | ||
| securityContext: | ||
| allowPrivilegeEscalation: false | ||
| readOnlyRootFilesystem: false | ||
| runAsNonRoot: true | ||
| capabilities: | ||
| drop: | ||
| - ALL | ||
| - pg_isready -U "$POSTGRES_USER" | ||
| initialDelaySeconds: 10 | ||
| periodSeconds: 10 | ||
| timeoutSeconds: 5 | ||
| livenessProbe: | ||
| exec: | ||
| command: | ||
| - /bin/sh | ||
| - -c | ||
| - pg_isready -U "$POSTGRES_USER" | ||
| initialDelaySeconds: 30 | ||
| periodSeconds: 30 | ||
| timeoutSeconds: 5 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Locate the manifest files mentioned
find . -type f -name "ambient-api-server-db.yml" -o -name "ambient-api-server-secrets.yml" -o -name "*ambient-api-server-db-json-patch.yaml"Repository: ambient-code/platform
Length of output: 338
🏁 Script executed:
# Check the file structure
git ls-files | grep -E "(manifests|ambient)" | head -20Repository: ambient-code/platform
Length of output: 1267
🏁 Script executed:
# Read the base DB manifest to verify the probe configuration
cat -n ./components/manifests/base/ambient-api-server-db.yml | sed -n '70,115p'Repository: ambient-code/platform
Length of output: 1475
🏁 Script executed:
# Check the secrets file for db.user and db.name definitions
cat -n ./components/manifests/base/ambient-api-server-secrets.ymlRepository: ambient-code/platform
Length of output: 1071
🏁 Script executed:
# Check the local-dev overlay patch
cat -n ./components/manifests/overlays/local-dev/ambient-api-server-db-json-patch.yamlRepository: ambient-code/platform
Length of output: 1621
🏁 Script executed:
# Check the production overlay patch
cat -n ./components/manifests/overlays/production/ambient-api-server-db-json-patch.yamlRepository: ambient-code/platform
Length of output: 1621
🏁 Script executed:
# Verify PostgreSQL pg_isready behavior and documentation
pip install psycopg2-binary 2>/dev/null || true
# Search for any pg_isready usage documentation or examples in the codebase
rg "pg_isready" --type yaml -A 2 -B 2Repository: ambient-code/platform
Length of output: 8071
🌐 Web query:
PostgreSQL pg_isready default database behavior documentation
💡 Result:
pg_isready uses libpq’s normal connection defaults when you don’t pass -d/--dbname (or a connection string). (postgresql.org)
So the “default database” resolution is:
- If
PGDATABASEis set, that value is used asdbname. (postgresql.org) - Otherwise, libpq’s built-in default for
dbnameis the same as the user name. (postgresql.org)- And the default
useris the operating system user name (unless overridden, e.g., viaPGUSER/-U). (postgresql.org)
- And the default
Also, per the pg_isready docs, the database/user/password values don’t need to be correct to determine server readiness; wrong values mainly just cause a failed connection attempt to be logged. (postgresql.org)
Citations:
- 1: https://www.postgresql.org/docs/15/app-pg-isready.html
- 2: https://www.postgresql.org/docs/15/libpq-envars.html
- 3: https://www.postgresql.org/docs/15/libpq-connect.html
- 4: https://www.postgresql.org/docs/15/libpq-connect.html
- 5: https://www.postgresql.org/docs/15/app-pg-isready.html
Add database name to pg_isready probes to avoid spurious log warnings.
The probes omit the -d flag, causing pg_isready to default to checking database ambient (the username) instead of ambient_api_server. PostgreSQL logs a failed connection attempt for each probe cycle. Add -d "$POSTGRES_DB" to both readinessProbe and livenessProbe commands. The same fix is also needed in the local-dev and production overlay patches. Additionally, base/postgresql-deployment.yaml and its corresponding overlay patches have the same issue.
🩺 Probe fix
- - pg_isready -U "$POSTGRES_USER"
+ - pg_isready -U "$POSTGRES_USER" -d "$POSTGRES_DB"📝 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.
| readinessProbe: | |
| exec: | |
| command: | |
| - /bin/sh | |
| - -c | |
| - exec /usr/bin/pg_isready -U $POSTGRES_USER -d $POSTGRES_DB -h localhost -p 5432 | |
| initialDelaySeconds: 5 | |
| timeoutSeconds: 1 | |
| volumeMounts: | |
| - mountPath: /var/lib/pgsql/data | |
| name: ambient-api-server-db-data | |
| securityContext: | |
| allowPrivilegeEscalation: false | |
| readOnlyRootFilesystem: false | |
| runAsNonRoot: true | |
| capabilities: | |
| drop: | |
| - ALL | |
| - pg_isready -U "$POSTGRES_USER" | |
| initialDelaySeconds: 10 | |
| periodSeconds: 10 | |
| timeoutSeconds: 5 | |
| livenessProbe: | |
| exec: | |
| command: | |
| - /bin/sh | |
| - -c | |
| - pg_isready -U "$POSTGRES_USER" | |
| initialDelaySeconds: 30 | |
| periodSeconds: 30 | |
| timeoutSeconds: 5 | |
| readinessProbe: | |
| exec: | |
| command: | |
| - /bin/sh | |
| - -c | |
| - pg_isready -U "$POSTGRES_USER" -d "$POSTGRES_DB" | |
| initialDelaySeconds: 10 | |
| periodSeconds: 10 | |
| timeoutSeconds: 5 | |
| livenessProbe: | |
| exec: | |
| command: | |
| - /bin/sh | |
| - -c | |
| - pg_isready -U "$POSTGRES_USER" -d "$POSTGRES_DB" | |
| initialDelaySeconds: 30 | |
| periodSeconds: 30 | |
| timeoutSeconds: 5 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/manifests/base/ambient-api-server-db.yml` around lines 84 - 101,
The readinessProbe and livenessProbe use pg_isready without specifying a
database, causing checks to default to the user name; update the exec commands
for readinessProbe and livenessProbe to include the database flag -d
"$POSTGRES_DB" alongside -U "$POSTGRES_USER" (i.e., pg_isready -U
"$POSTGRES_USER" -d "$POSTGRES_DB"); apply this same change in the local-dev and
production overlay patches and in the postgresql-deployment.yaml and its
overlays so all probes consistently pass the POSTGRES_DB variable.
| initContainers: | ||
| - name: init-db | ||
| image: registry.redhat.io/rhel10/postgresql-16:10.1 |
There was a problem hiding this comment.
Harden the new init container for restricted SCC.
init-db is introduced without a container-level securityContext, so it misses the capability drop / privilege-escalation controls this PR is otherwise adding for SCC compliance. That can still fail admission on restricted OpenShift clusters.
Suggested fix
- name: init-db
image: registry.redhat.io/rhel10/postgresql-16:10.1
+ securityContext:
+ allowPrivilegeEscalation: false
+ capabilities:
+ drop:
+ - ALL
+ seccompProfile:
+ type: RuntimeDefault
command:🤖 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" lacks a container-level
securityContext which can cause admission failures on restricted SCC; add a
securityContext to the initContainers -> name: init-db entry that specifies
allowPrivilegeEscalation: false, privileged: false, runAsNonRoot: true (and a
non-root runAsUser uid if appropriate), readOnlyRootFilesystem: true, and drop
all capabilities (capabilities: drop: ["ALL"]), and optionally set
seccompProfile: type: RuntimeDefault to match the pod-level hardening.
There was a problem hiding this comment.
Openshift will create default security context.
| until pg_isready -h "$PGHOST" -U "$PGUSER"; do | ||
| RETRY_COUNT=$((RETRY_COUNT + 1)) | ||
| if [ $RETRY_COUNT -ge $MAX_RETRIES ]; then | ||
| echo "ERROR: PostgreSQL not ready after 10 minutes, giving up" | ||
| exit 1 | ||
| fi | ||
| echo "PostgreSQL not ready, waiting... (attempt $RETRY_COUNT/$MAX_RETRIES)" | ||
| sleep 2 | ||
| done | ||
| echo "PostgreSQL is ready" | ||
|
|
||
| echo "Checking if database '$PGDATABASE' exists..." | ||
| if psql -h "$PGHOST" -U "$PGUSER" -lqt | cut -d \| -f 1 | grep -qw "$PGDATABASE"; then | ||
| echo "Database '$PGDATABASE' already exists" | ||
| else | ||
| echo "Creating database '$PGDATABASE'..." | ||
| psql -h "$PGHOST" -U "$PGUSER" -c "CREATE DATABASE $PGDATABASE;" |
There was a problem hiding this comment.
Connect admin commands to an existing database, not $PGDATABASE.
This script exports PGDATABASE as the database being created, then runs pg_isready/psql without -d. On a fresh install those commands can try to connect to the non-existent target DB, so the existence check and CREATE DATABASE path fail exactly when bootstrapping is needed.
Suggested fix
- until pg_isready -h "$PGHOST" -U "$PGUSER"; do
+ until pg_isready -h "$PGHOST" -U "$PGUSER" -d postgres; do
RETRY_COUNT=$((RETRY_COUNT + 1))
if [ $RETRY_COUNT -ge $MAX_RETRIES ]; then
echo "ERROR: PostgreSQL not ready after 10 minutes, giving up"
exit 1
fi
echo "PostgreSQL not ready, waiting... (attempt $RETRY_COUNT/$MAX_RETRIES)"
sleep 2
done
echo "PostgreSQL is ready"
echo "Checking if database '$PGDATABASE' exists..."
- if psql -h "$PGHOST" -U "$PGUSER" -lqt | cut -d \| -f 1 | grep -qw "$PGDATABASE"; then
+ if psql -h "$PGHOST" -U "$PGUSER" -d postgres \
+ --set=db_name="$PGDATABASE" \
+ -Atqc "SELECT 1 FROM pg_database WHERE datname = :'db_name'" | grep -q 1; then
echo "Database '$PGDATABASE' already exists"
else
echo "Creating database '$PGDATABASE'..."
- psql -h "$PGHOST" -U "$PGUSER" -c "CREATE DATABASE $PGDATABASE;"
+ psql -h "$PGHOST" -U "$PGUSER" -d postgres \
+ --set=ON_ERROR_STOP=1 \
+ --set=db_name="$PGDATABASE" \
+ -c 'CREATE DATABASE :"db_name"'
echo "Database '$PGDATABASE' created successfully"
fi📝 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.
| until pg_isready -h "$PGHOST" -U "$PGUSER"; do | |
| RETRY_COUNT=$((RETRY_COUNT + 1)) | |
| if [ $RETRY_COUNT -ge $MAX_RETRIES ]; then | |
| echo "ERROR: PostgreSQL not ready after 10 minutes, giving up" | |
| exit 1 | |
| fi | |
| echo "PostgreSQL not ready, waiting... (attempt $RETRY_COUNT/$MAX_RETRIES)" | |
| sleep 2 | |
| done | |
| echo "PostgreSQL is ready" | |
| echo "Checking if database '$PGDATABASE' exists..." | |
| if psql -h "$PGHOST" -U "$PGUSER" -lqt | cut -d \| -f 1 | grep -qw "$PGDATABASE"; then | |
| echo "Database '$PGDATABASE' already exists" | |
| else | |
| echo "Creating database '$PGDATABASE'..." | |
| psql -h "$PGHOST" -U "$PGUSER" -c "CREATE DATABASE $PGDATABASE;" | |
| until pg_isready -h "$PGHOST" -U "$PGUSER" -d postgres; do | |
| RETRY_COUNT=$((RETRY_COUNT + 1)) | |
| if [ $RETRY_COUNT -ge $MAX_RETRIES ]; then | |
| echo "ERROR: PostgreSQL not ready after 10 minutes, giving up" | |
| exit 1 | |
| fi | |
| echo "PostgreSQL not ready, waiting... (attempt $RETRY_COUNT/$MAX_RETRIES)" | |
| sleep 2 | |
| done | |
| echo "PostgreSQL is ready" | |
| echo "Checking if database '$PGDATABASE' exists..." | |
| if psql -h "$PGHOST" -U "$PGUSER" -d postgres \ | |
| --set=db_name="$PGDATABASE" \ | |
| -Atqc "SELECT 1 FROM pg_database WHERE datname = :'db_name'" | grep -q 1; then | |
| echo "Database '$PGDATABASE' already exists" | |
| else | |
| echo "Creating database '$PGDATABASE'..." | |
| psql -h "$PGHOST" -U "$PGUSER" -d postgres \ | |
| --set=ON_ERROR_STOP=1 \ | |
| --set=db_name="$PGDATABASE" \ | |
| -c 'CREATE DATABASE :"db_name"' | |
| echo "Database '$PGDATABASE' created successfully" | |
| fi |
🤖 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 21 - 37, The script is using PGDATABASE (the target DB to be
created) for health checks and psql commands, so on a fresh install those
commands try to connect to a non‑existent DB; change the admin connections to
use an existing admin DB (e.g., "postgres") by passing -d postgres (or
temporarily setting PGDATABASE=postgres) to pg_isready/psql calls used for the
readiness check, the existence test (psql -h "$PGHOST" -U "$PGUSER" -d postgres
-lqt | ...), and the CREATE DATABASE invocation (keep CREATE DATABASE
$PGDATABASE; executed against the admin connection) so the script can probe and
create the target DB reliably.
| # Authz disabled: api-server has no internal RBAC middleware yet. | ||
| # Auth is handled by the backend proxy. TODO: enable when authz is implemented. | ||
| - --enable-authz=false |
There was a problem hiding this comment.
Don't rely on the backend proxy while exposing a direct Route.
components/manifests/overlays/local-dev/kustomization.yaml Line 9 still includes ambient-api-server-route.yaml, so clients can reach this service without going through the proxy mentioned in the comment. With --enable-authz=false, that route becomes a JWT-only bypass of any proxy-enforced authorization.
🤖 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-tls-patch.yaml`
around lines 21 - 23, The overlay currently disables internal authz via the flag
--enable-authz=false while still exposing the service through
ambient-api-server-route.yaml in the local-dev kustomization, creating a direct
Route that bypasses proxy-enforced authorization; fix this by either re-enabling
internal authorization (remove or set --enable-authz=false to true) on the
ambient API server invocation or remove ambient-api-server-route.yaml from the
local-dev kustomization so the service is only reachable via the backend proxy;
update whichever of the two you choose (the flag in
ambient-api-server-tls-patch.yaml or the inclusion of
ambient-api-server-route.yaml in the kustomization) to ensure the Route cannot
be reached without proxy authz.
| # Ambient API Server DB: fix db.host for vteam- namePrefix | ||
| - path: ambient-api-server-db-credentials-patch.yaml |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider targeting the DB credentials patch explicitly too.
ambient-api-server-db-credentials-patch.yaml is now the odd one out in this block. Giving it the same explicit target treatment as the other ambient-api-server patches would make the overlay easier to audit and less sensitive to metadata drift inside the patch file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/manifests/overlays/local-dev/kustomization.yaml` around lines 40 -
41, The DB credentials patch ambient-api-server-db-credentials-patch.yaml is
missing an explicit target entry in the kustomization (unlike the other
ambient-api-server patches); update the kustomization.yaml to add a matching
target selector for ambient-api-server-db-credentials-patch.yaml (same style as
the existing ambient-api-server patches) so it explicitly targets the
ambient-api-server resource(s) (use the same kind/name/namespace or label
selector pattern used by the other patches) to prevent accidental metadata
drift.
| initContainers: | ||
| - name: init-db | ||
| image: registry.redhat.io/rhel10/postgresql-16:10.1 |
There was a problem hiding this comment.
The production init container still needs SCC hardening.
This new init-db container does not set allowPrivilegeEscalation: false or drop capabilities, so it can still violate the restricted-v2 constraints the rest of the PR is targeting.
Suggested fix
- name: init-db
image: registry.redhat.io/rhel10/postgresql-16:10.1
+ securityContext:
+ allowPrivilegeEscalation: false
+ capabilities:
+ drop:
+ - ALL
+ seccompProfile:
+ type: RuntimeDefault
command:📝 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.
| initContainers: | |
| - name: init-db | |
| image: registry.redhat.io/rhel10/postgresql-16:10.1 | |
| initContainers: | |
| - name: init-db | |
| image: registry.redhat.io/rhel10/postgresql-16:10.1 | |
| securityContext: | |
| allowPrivilegeEscalation: false | |
| capabilities: | |
| drop: | |
| - ALL | |
| seccompProfile: | |
| type: RuntimeDefault |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@components/manifests/overlays/production/ambient-api-server-init-db-patch.yaml`
around lines 10 - 12, The initContainers entry for the init-db container lacks a
Pod/Container securityContext to satisfy restricted-v2 SCC; update the init
container named "init-db" to include a securityContext that sets
allowPrivilegeEscalation: false, drops all Linux capabilities (e.g.,
capabilities.drop: ["ALL"]), and sets readOnlyRootFilesystem: true (and
runAsNonRoot/runAsUser as appropriate) so the init container adheres to SCC
hardening and restricted policies.
| until pg_isready -h "$PGHOST" -U "$PGUSER"; do | ||
| RETRY_COUNT=$((RETRY_COUNT + 1)) | ||
| if [ $RETRY_COUNT -ge $MAX_RETRIES ]; then | ||
| echo "ERROR: PostgreSQL not ready after 10 minutes, giving up" | ||
| exit 1 | ||
| fi | ||
| echo "PostgreSQL not ready, waiting... (attempt $RETRY_COUNT/$MAX_RETRIES)" | ||
| sleep 2 | ||
| done | ||
| echo "PostgreSQL is ready" | ||
|
|
||
| echo "Checking if database '$PGDATABASE' exists..." | ||
| if psql -h "$PGHOST" -U "$PGUSER" -lqt | cut -d \| -f 1 | grep -qw "$PGDATABASE"; then | ||
| echo "Database '$PGDATABASE' already exists" | ||
| else | ||
| echo "Creating database '$PGDATABASE'..." | ||
| psql -h "$PGHOST" -U "$PGUSER" -c "CREATE DATABASE $PGDATABASE;" |
There was a problem hiding this comment.
This bootstrap path can fail before the database exists.
Because PGDATABASE is set to the target database name, these pg_isready and psql calls can inherit that value and attempt to connect to the DB before it has been created. The production init container should connect to a maintenance DB such as postgres for readiness checks, existence checks, and the CREATE DATABASE statement.
Suggested fix
- until pg_isready -h "$PGHOST" -U "$PGUSER"; do
+ until pg_isready -h "$PGHOST" -U "$PGUSER" -d postgres; do
RETRY_COUNT=$((RETRY_COUNT + 1))
if [ $RETRY_COUNT -ge $MAX_RETRIES ]; then
echo "ERROR: PostgreSQL not ready after 10 minutes, giving up"
exit 1
fi
echo "PostgreSQL not ready, waiting... (attempt $RETRY_COUNT/$MAX_RETRIES)"
sleep 2
done
echo "PostgreSQL is ready"
echo "Checking if database '$PGDATABASE' exists..."
- if psql -h "$PGHOST" -U "$PGUSER" -lqt | cut -d \| -f 1 | grep -qw "$PGDATABASE"; then
+ if psql -h "$PGHOST" -U "$PGUSER" -d postgres \
+ --set=db_name="$PGDATABASE" \
+ -Atqc "SELECT 1 FROM pg_database WHERE datname = :'db_name'" | grep -q 1; then
echo "Database '$PGDATABASE' already exists"
else
echo "Creating database '$PGDATABASE'..."
- psql -h "$PGHOST" -U "$PGUSER" -c "CREATE DATABASE $PGDATABASE;"
+ psql -h "$PGHOST" -U "$PGUSER" -d postgres \
+ --set=ON_ERROR_STOP=1 \
+ --set=db_name="$PGDATABASE" \
+ -c 'CREATE DATABASE :"db_name"'
echo "Database '$PGDATABASE' created successfully"
fi📝 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.
| until pg_isready -h "$PGHOST" -U "$PGUSER"; do | |
| RETRY_COUNT=$((RETRY_COUNT + 1)) | |
| if [ $RETRY_COUNT -ge $MAX_RETRIES ]; then | |
| echo "ERROR: PostgreSQL not ready after 10 minutes, giving up" | |
| exit 1 | |
| fi | |
| echo "PostgreSQL not ready, waiting... (attempt $RETRY_COUNT/$MAX_RETRIES)" | |
| sleep 2 | |
| done | |
| echo "PostgreSQL is ready" | |
| echo "Checking if database '$PGDATABASE' exists..." | |
| if psql -h "$PGHOST" -U "$PGUSER" -lqt | cut -d \| -f 1 | grep -qw "$PGDATABASE"; then | |
| echo "Database '$PGDATABASE' already exists" | |
| else | |
| echo "Creating database '$PGDATABASE'..." | |
| psql -h "$PGHOST" -U "$PGUSER" -c "CREATE DATABASE $PGDATABASE;" | |
| until pg_isready -h "$PGHOST" -U "$PGUSER" -d postgres; do | |
| RETRY_COUNT=$((RETRY_COUNT + 1)) | |
| if [ $RETRY_COUNT -ge $MAX_RETRIES ]; then | |
| echo "ERROR: PostgreSQL not ready after 10 minutes, giving up" | |
| exit 1 | |
| fi | |
| echo "PostgreSQL not ready, waiting... (attempt $RETRY_COUNT/$MAX_RETRIES)" | |
| sleep 2 | |
| done | |
| echo "PostgreSQL is ready" | |
| echo "Checking if database '$PGDATABASE' exists..." | |
| if psql -h "$PGHOST" -U "$PGUSER" -d postgres \ | |
| --set=db_name="$PGDATABASE" \ | |
| -Atqc "SELECT 1 FROM pg_database WHERE datname = :'db_name'" | grep -q 1; then | |
| echo "Database '$PGDATABASE' already exists" | |
| else | |
| echo "Creating database '$PGDATABASE'..." | |
| psql -h "$PGHOST" -U "$PGUSER" -d postgres \ | |
| --set=ON_ERROR_STOP=1 \ | |
| --set=db_name="$PGDATABASE" \ | |
| -c 'CREATE DATABASE :"db_name"' | |
| echo "Database '$PGDATABASE' created successfully" | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@components/manifests/overlays/production/ambient-api-server-init-db-patch.yaml`
around lines 21 - 37, The readiness and existence checks (pg_isready and psql
invocations) currently use PGDATABASE and may try to connect to the target DB
before it exists; change these commands to target the maintenance DB (e.g.,
"postgres") instead of "$PGDATABASE" for readiness, listing databases, and
running the CREATE DATABASE statement (use pg_isready -d postgres and psql -d
postgres or supply -c with a connection to postgres) while still creating
"$PGDATABASE" with a CREATE DATABASE "$PGDATABASE" executed against the
maintenance DB; reference the pg_isready and psql calls and the PGDATABASE
variable when making this change.
| - --enable-metrics-https=false | ||
| - --http-read-timeout=5s | ||
| - --http-write-timeout=30s | ||
| - --cors-allowed-origins=* |
There was a problem hiding this comment.
Reconsider wildcard CORS for production.
--cors-allowed-origins=* allows any origin to make cross-origin requests. For a production API server, this is typically too permissive. Consider restricting to specific trusted domains.
🔒 Suggested fix
- - --cors-allowed-origins=*
+ - --cors-allowed-origins=https://console.redhat.com,https://your-frontend.example.com📝 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.
| - --cors-allowed-origins=* | |
| - --cors-allowed-origins=https://console.redhat.com,https://your-frontend.example.com |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@components/manifests/overlays/production/ambient-api-server-jwt-args-patch.yaml`
at line 44, The CORS flag currently uses a wildcard (--cors-allowed-origins=*)
which is too permissive for production; change the --cors-allowed-origins
argument to a restricted, comma-separated list of trusted origins (e.g.,
https://app.example.com,https://admin.example.com) or reference a deployment
environment variable (e.g., --cors-allowed-origins=${CORS_ALLOWED_ORIGINS}) so
allowed domains can be managed securely; update the manifest entry that sets the
--cors-allowed-origins flag accordingly and ensure the runtime/env var contains
only the approved origins.
Route moved to overlays (local-dev, production) in #843. Base reference was dangling — file deleted from base but kustomization not updated. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Route moved to overlays (local-dev, production) in #843. Base reference was dangling — file deleted from base but kustomization not updated. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
2 participants
Summary
Changes Made
TLS Implementation
EnableHTTPS=falseoverride from development environmentFramework Upgrade
rh-trex-ai v0.0.17 → v0.0.19for enhanced TLS configurationEnableJWTfield (moved to flag-based auth)OpenShift Compliance
fsGroup: 999from database pod securityContext (blocked by restricted-v2 SCC)Verification
Test Plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores