-
Notifications
You must be signed in to change notification settings - Fork 106
Delegate urls flag #1591
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Delegate urls flag #1591
Changes from all commits
6031ec7
606ce90
6c55b40
f0c83d1
27ca356
ecc111f
423fe26
5a526ef
05d3dad
2cf0e26
c608bd0
813c4e6
9d1e6c0
4c0ee4b
85b6476
9b27e17
76e9181
5fb4711
12d027e
e252262
b673993
07c771f
3b12dbc
cd29d3e
19cae2a
eea6dbf
d8ca236
aca8627
da9e091
59db0de
f7c264f
fc506ef
88d2738
ab195e8
bc7a893
81be018
8409458
4a337c6
f946eb2
58123c5
3731512
04290ab
8e365a1
fabbc95
5d31cec
d292964
336a759
f190ae5
2af8216
f0cafaf
a96106f
9a63f96
93927f7
6e294e7
622f62f
db6bdd3
0d7e8c0
a3ede83
1b05e80
0a4d259
2bf0dd5
ae273be
393378a
5c6a9d3
3e2ebcf
b5c3b0b
259ca05
f180dbe
a584836
a4ced8e
4896c26
61b41a7
f120bf9
1920c92
0082943
21ce5df
4459daf
a9ac6d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| # Patch for production frontend deployment | ||
| # - Adds OAuth proxy sidecar for authentication | ||
| # - Adds OAuth proxy sidecar for authentication using OpenShift OAuth | ||
| # - Uses service account token for cookie secret (no vault secret needed) | ||
| # - Overrides resource limits to prevent OOMKills (sawtooth memory pattern) | ||
| apiVersion: apps/v1 | ||
| kind: Deployment | ||
|
|
@@ -20,20 +21,23 @@ spec: | |
| cpu: 1000m | ||
| # OAuth proxy sidecar | ||
| - name: oauth-proxy | ||
| image: quay.io/openshift/origin-oauth-proxy:4.14 | ||
| image: registry.redhat.io/openshift4/ose-oauth-proxy-rhel9:v4.18.0-202506230505.p0.gcbd44ad.assembly.stream.el9 | ||
| imagePullPolicy: IfNotPresent | ||
| args: | ||
| - --http-address=:8443 | ||
| - --https-address= | ||
| - --provider=openshift | ||
| - --client-id=ambient-code | ||
| - --client-secret-file=/etc/oauth-client/client_secret | ||
| - --upstream=http://localhost:3000 | ||
| - --client-id=ambient-frontend | ||
| - --client-secret-file=/etc/oauth/config/client-secret | ||
| - --cookie-secret-file=/etc/oauth/config/cookie_secret | ||
| - --cookie-expire=23h0m0s | ||
| - --tls-cert=/etc/tls/private/tls.crt | ||
| - --tls-key=/etc/tls/private/tls.key | ||
| - --cookie-secret-file=/etc/oauth-cookie/cookie_secret | ||
|
Comment on lines
+31
to
+35
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Locate the base frontend Deployment to confirm oauth-client/oauth-cookie volumes & securityContext exist.
fd -t f -e yaml -e yml | xargs rg -lP '^\s*name:\s*frontend\s*$' | xargs -I{} rg -nP -C5 '(oauth-client|oauth-cookie|securityContext|volumes:|volumeMounts:)' {}
# Also check the kustomize component for any companion volumes definition.
fd -t f . components/manifests/components/oauth-proxyRepository: ambient-code/platform Length of output: 6354 🏁 Script executed: fd -t f -e yaml -e yml -path "*/components/manifests/*" | head -20Repository: ambient-code/platform Length of output: 234 🏁 Script executed: cat -n components/manifests/components/oauth-proxy/kustomization.yamlRepository: ambient-code/platform Length of output: 412 🏁 Script executed: rg -n "name.*frontend" components/manifests -A2 -B2 -t yaml | grep -E "(kind:|name:|Deployment)" | head -20Repository: ambient-code/platform Length of output: 1800 🏁 Script executed: sed -n '620,800p' components/manifests/templates/template-services.yaml | cat -nRepository: ambient-code/platform Length of output: 6474 🏁 Script executed: rg -n "kind: Deployment" components/manifests/templates/template-services.yaml -A100 | grep -A100 "name: frontend$" | head -120Repository: ambient-code/platform Length of output: 3901 🏁 Script executed: cat -n components/manifests/components/oauth-proxy/frontend-oauth-deployment-patch.yamlRepository: ambient-code/platform Length of output: 3013 🏁 Script executed: rg -n "securityContext" components/manifests/components/oauth-proxy/ -B2 -A5Repository: ambient-code/platform Length of output: 47 🏁 Script executed: rg -n "containers:" components/manifests/templates/template-services.yaml -A50 | grep -A40 "name: oauth-proxy" | grep -i "securitycontext"Repository: ambient-code/platform Length of output: 47 🏁 Script executed: sed -n '678,750p' components/manifests/templates/template-services.yaml | cat -nRepository: ambient-code/platform Length of output: 2949 🏁 Script executed: rg -n "oauth-proxy" components/manifests/templates/template-services.yaml -B5 -A30 | grep -A30 "name: oauth-proxy" | head -40Repository: ambient-code/platform Length of output: 1112 Strategic merge patch will lose oauth-client and oauth-cookie secret mounts, causing oauth-proxy startup failure. The patch replaces the entire Either:
Additionally, securityContext:
runAsNonRoot: true
allowPrivilegeEscalation: false
capabilities:
drop:
- ALL
readOnlyRootFilesystem: true🤖 Prompt for AI Agents |
||
| - --cookie-expire=24h | ||
| - --cookie-refresh=1h | ||
| - --pass-access-token | ||
| - --scope=user:full | ||
| - --openshift-delegate-urls={"/":{"resource":"projects","verb":"list"}} | ||
| - --upstream-timeout=5m | ||
| - --skip-auth-regex=^/metrics | ||
| ports: | ||
| - containerPort: 8443 | ||
|
|
@@ -42,38 +46,33 @@ spec: | |
| httpGet: | ||
| path: /oauth/healthz | ||
| port: dashboard-ui | ||
| scheme: HTTP | ||
| initialDelaySeconds: 30 | ||
| scheme: HTTPS | ||
| initialDelaySeconds: 10 | ||
| timeoutSeconds: 1 | ||
| periodSeconds: 5 | ||
| periodSeconds: 10 | ||
| successThreshold: 1 | ||
| failureThreshold: 3 | ||
| readinessProbe: | ||
| httpGet: | ||
| path: /oauth/healthz | ||
| port: dashboard-ui | ||
| scheme: HTTP | ||
| initialDelaySeconds: 5 | ||
| scheme: HTTPS | ||
| initialDelaySeconds: 10 | ||
| timeoutSeconds: 1 | ||
| periodSeconds: 5 | ||
| periodSeconds: 10 | ||
| successThreshold: 1 | ||
| failureThreshold: 3 | ||
| resources: | ||
| requests: | ||
| memory: 256Mi | ||
| cpu: 50m | ||
| memory: 50Mi | ||
| cpu: 10m | ||
| limits: | ||
| memory: 512Mi | ||
| memory: 200Mi | ||
| cpu: 200m | ||
| volumeMounts: | ||
| - mountPath: /etc/oauth/config | ||
| name: oauth-config | ||
| - mountPath: /etc/tls/private | ||
| name: proxy-tls | ||
| name: frontend-proxy-tls | ||
| volumes: | ||
| - name: oauth-config | ||
| secret: | ||
| secretName: frontend-oauth-config | ||
| - name: proxy-tls | ||
| - name: frontend-proxy-tls | ||
| secret: | ||
| secretName: dashboard-proxy-tls | ||
| secretName: frontend-proxy-tls | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,12 @@ | ||
| # Patch to add OAuth port to frontend service | ||
| # - Adds HTTPS port for OAuth proxy sidecar | ||
| # - Uses service.alpha annotation for auto-generated TLS cert | ||
| apiVersion: v1 | ||
| kind: Service | ||
| metadata: | ||
| name: frontend-service | ||
| annotations: | ||
| service.beta.openshift.io/serving-cert-secret-name: dashboard-proxy-tls | ||
| service.alpha.openshift.io/serving-cert-secret-name: frontend-proxy-tls | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Compare existing usage across the repo to confirm beta is the prevailing convention.
rg -nP 'service\.(alpha|beta)\.openshift\.io/serving-cert-secret-name'Repository: ambient-code/platform Length of output: 2213 Use The file uses 🤖 Prompt for AI Agents |
||
| spec: | ||
| ports: | ||
| - port: 8443 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| --- | ||
| apiVersion: v1 | ||
| kind: Secret | ||
| metadata: | ||
| name: ambient-code-rds | ||
| labels: | ||
| app: ambient-api-server | ||
| component: database | ||
| annotations: | ||
| # External RDS connection managed via Vault secrets from app-interface Phase 2 | ||
| # These values will be injected by vault-secret-manager from Vault path: | ||
| # app-interface/data/ambient-code-platform/stage/rds-credentials | ||
| qontract.recycle: "true" | ||
| type: Opaque | ||
|
Comment on lines
+4
to
+14
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add This new Secret is missing As per coding guidelines 🤖 Prompt for AI Agents |
||
| stringData: | ||
| # Placeholders - actual values injected from Vault at runtime | ||
| db.host: "VAULT_INJECTED" | ||
| db.port: "5432" | ||
| db.name: "ambient_code" | ||
| db.user: "VAULT_INJECTED" | ||
| db.password: "VAULT_INJECTED" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| # App-interface: set environment to stage | ||
| apiVersion: apps/v1 | ||
| kind: Deployment | ||
| metadata: | ||
| name: ambient-api-server | ||
| spec: | ||
| template: | ||
| spec: | ||
| containers: | ||
| - name: api-server | ||
| env: | ||
| - name: AMBIENT_ENV | ||
| value: stage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale Node.js 20 references in comments.
The image tags are now
nodejs-24/nodejs-24-minimalbut the surrounding comments still say "Node.js 20" / "nodejs-20". Quick cleanup to keep the docs honest.📝 Proposed fix
Also applies to: 13-15
🤖 Prompt for AI Agents