-
Notifications
You must be signed in to change notification settings - Fork 94
Use project based perms #1402
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?
Use project based perms #1402
Changes from all commits
b0ef0d1
78f5c9d
70ba199
ac9edd5
2d9a3ca
c5b4bae
b44e31e
d9c2e14
c93ad06
a38898d
37fb2ab
03a447b
f91ae6e
083ded9
f49b567
f519b91
8639118
c631f34
5162154
2f79829
c5b41a3
3471461
1257427
c040bb7
3b3f0fc
2f251bb
21b201d
7ecd3c7
9c2b0f6
6e36009
8e9ca5e
71683a5
4dd36fe
97ebefa
6e7ee36
79ed385
1b026f4
a81ec1f
2ea1cc2
2d400d7
f923806
67d4d13
97feae7
f89514d
dbfea85
5ce42d3
6ca642c
2ae85d0
f15ada3
22229ce
88d0604
5a01b54
c8c52ad
d8ec1ae
f7a43e7
4e5efe3
26825fb
0dabcaa
868e653
ef594bf
5f59d1b
82631b0
130db34
f047289
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.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,8 @@ kind: ServiceAccount | |
| metadata: | ||
| name: frontend | ||
| namespace: ambient-code | ||
| annotations: | ||
| serviceaccounts.openshift.io/oauth-redirectreference.frontend: '{"kind":"OAuthRedirectReference","apiVersion":"v1","reference":{"kind":"Route","name":"frontend"}}' | ||
|
Comment on lines
+6
to
+7
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. OAuth redirect reference target doesn't exist.
🔧 Proposed fix- serviceaccounts.openshift.io/oauth-redirectreference.frontend: '{"kind":"OAuthRedirectReference","apiVersion":"v1","reference":{"kind":"Route","name":"frontend"}}'
+ serviceaccounts.openshift.io/oauth-redirectreference.frontend: '{"kind":"OAuthRedirectReference","apiVersion":"v1","reference":{"kind":"Route","name":"frontend-route"}}'🤖 Prompt for AI Agents |
||
| --- | ||
| apiVersion: rbac.authorization.k8s.io/v1 | ||
| kind: ClusterRole | ||
|
|
@@ -28,3 +30,16 @@ subjects: | |
| - kind: ServiceAccount | ||
| name: frontend | ||
| namespace: ambient-code | ||
| --- | ||
| apiVersion: rbac.authorization.k8s.io/v1 | ||
| kind: ClusterRoleBinding | ||
| metadata: | ||
| name: ambient-frontend-oauth-delegator | ||
| roleRef: | ||
| apiGroup: rbac.authorization.k8s.io | ||
| kind: ClusterRole | ||
| name: system:auth-delegator | ||
| subjects: | ||
| - kind: ServiceAccount | ||
| name: frontend | ||
| namespace: ambient-code | ||
| 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,19 +21,17 @@ 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= | ||
| - --https-address=:8443 | ||
| - --provider=openshift | ||
| - --openshift-service-account=frontend | ||
| - --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 | ||
| - --pass-access-token | ||
| - --scope=user:full | ||
| - --openshift-delegate-urls={"/":{"resource":"projects","verb":"list"}} | ||
| - --tls-cert=/etc/tls/private/tls.crt | ||
| - --tls-key=/etc/tls/private/tls.key | ||
| - --cookie-secret-file=/var/run/secrets/kubernetes.io/serviceaccount/token | ||
|
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
# Confirm how openshift/oauth-proxy validates cookie-secret length in the version used here.
rg -nP --no-messages 'cookie[_-]secret' -C2
# Inspect template-services for the sibling oauth-proxy sidecar to see how cookie secret is wired elsewhere
fd -t f 'template-services.yaml' | xargs -I{} sh -c 'echo "=== {} ==="; rg -n "cookie|oauth-proxy" {} -C2'Repository: ambient-code/platform Length of output: 8358 🏁 Script executed: # Check if this patch is referenced in kustomization or deployment configs
fd -t f 'kustomization.yaml' | xargs grep -l "frontend-oauth-deployment-patch" 2>/dev/null
# Check the full context of the patch file
cat -n components/manifests/components/oauth-proxy/frontend-oauth-deployment-patch.yaml
# See what volumes are mounted in this patch
rg -A20 "volumes:" components/manifests/components/oauth-proxy/frontend-oauth-deployment-patch.yamlRepository: ambient-code/platform Length of output: 2963 Cookie secret using service account token will fail at startup. Line 33 points The patch comment claims "no vault secret needed" by using the SA token, but a dedicated cookie secret must be mounted. Add a volumeMount for a proper cookie secret (see 🤖 Prompt for AI Agents |
||
| - --upstream-timeout=5m | ||
| - --skip-auth-regex=^/metrics | ||
| ports: | ||
| - containerPort: 8443 | ||
|
|
@@ -41,38 +40,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 | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+2
to
+9
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
set -euo pipefail
# Verify service-ca annotation consistency and the mounted TLS secret name.
# Expected: serving-cert annotations use the supported key for the target OpenShift version,
# and the generated secret name matches frontend-proxy-tls.
rg -n -C2 'service\.(alpha|beta)\.openshift\.io/serving-cert-secret-name|frontend-proxy-tls' components/manifestsRepository: ambient-code/platform Length of output: 6125 Use This patch uses Proposed fix-# - Uses service.alpha annotation for auto-generated TLS cert
+# - Uses service.beta annotation for auto-generated TLS cert
@@
- service.alpha.openshift.io/serving-cert-secret-name: frontend-proxy-tls
+ service.beta.openshift.io/serving-cert-secret-name: frontend-proxy-tls📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
| spec: | ||||||||||||||||||||||||||||||||||||
| ports: | ||||||||||||||||||||||||||||||||||||
| - port: 8443 | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| --- | ||
| apiVersion: v1 | ||
| kind: Secret | ||
| metadata: | ||
| name: ambient-code-rds | ||
| labels: | ||
| app: ambient-api-server | ||
| component: database | ||
| annotations: | ||
| # External RDS credentials injected from Vault at runtime | ||
| qontract.recycle: "true" | ||
| type: Opaque | ||
| 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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1
to
+13
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
set -euo pipefail
# Verify whether AMBIENT_ENV=stage is registered by the ambient-api-server environment package.
# Expected: either a stage EnvironmentImpl exists, or this patch should use production.
rg -n -C3 'AMBIENT_ENV|EnvironmentImpls|Stage|stage|ProductionEnv' \
components/ambient-api-server/cmd/ambient-api-server/environments \
components/manifests/overlays/app-interface/ambient-api-server-env-patch.yamlRepository: ambient-code/platform Length of output: 8474 Environment The Fix to use production environment - name: AMBIENT_ENV
- value: stage
+ value: production📝 Committable suggestion
Suggested change
🧰 Tools🪛 Checkov (3.2.519)[medium] 2-13: Containers should not run with allowPrivilegeEscalation (CKV_K8S_20) [medium] 2-13: Minimize the admission of root containers (CKV_K8S_23) 🪛 Trivy (0.69.3)[error] 10-13: Root file system is not read-only Container 'api-server' of Deployment 'ambient-api-server' should set 'securityContext.readOnlyRootFilesystem' to true Rule: KSV-0014 (IaC/Kubernetes) [error] 10-13: Default security context configured container ambient-api-server in default namespace is using the default security context Rule: KSV-0118 (IaC/Kubernetes) [error] 8-13: Default security context configured deployment ambient-api-server in default namespace is using the default security context, which allows root privileges Rule: KSV-0118 (IaC/Kubernetes) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| --- | ||
| apiVersion: route.openshift.io/v1 | ||
| kind: Route | ||
| metadata: | ||
| name: ambient-api-server | ||
| labels: | ||
| app: ambient-api-server | ||
| component: api | ||
| spec: | ||
| to: | ||
| kind: Service | ||
| name: ambient-api-server | ||
| port: | ||
| targetPort: api | ||
| tls: | ||
| termination: reencrypt | ||
| insecureEdgeTerminationPolicy: Redirect | ||
| --- | ||
| apiVersion: route.openshift.io/v1 | ||
| kind: Route | ||
| metadata: | ||
| name: ambient-api-server-grpc | ||
| labels: | ||
| app: ambient-api-server | ||
| component: grpc | ||
| spec: | ||
| to: | ||
| kind: Service | ||
| name: ambient-api-server | ||
| port: | ||
| targetPort: grpc | ||
| tls: | ||
| termination: reencrypt | ||
| insecureEdgeTerminationPolicy: Redirect |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| # OpenShift service-ca: auto-provision and rotate TLS certs for ambient-api-server | ||
| apiVersion: v1 | ||
| kind: Service | ||
| metadata: | ||
| name: ambient-api-server | ||
| annotations: | ||
| service.beta.openshift.io/serving-cert-secret-name: ambient-api-server-tls |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| # App-interface (stage): enable SSL for external RDS connection | ||
| apiVersion: apps/v1 | ||
| kind: Deployment | ||
| metadata: | ||
| name: ambient-api-server | ||
| spec: | ||
| template: | ||
| spec: | ||
| # Migration init container: add SSL mode | ||
| initContainers: | ||
| - name: migration | ||
| command: | ||
| - /usr/local/bin/ambient-api-server | ||
| - migrate | ||
| - --db-host-file=/secrets/db/db.host | ||
| - --db-port-file=/secrets/db/db.port | ||
| - --db-user-file=/secrets/db/db.user | ||
| - --db-password-file=/secrets/db/db.password | ||
| - --db-name-file=/secrets/db/db.name | ||
| - --db-sslmode=require | ||
| - --alsologtostderr | ||
| - -v=4 | ||
| # API server container: add SSL mode | ||
| containers: | ||
| - name: api-server | ||
| command: | ||
| - /usr/local/bin/ambient-api-server | ||
| - serve | ||
| - --db-host-file=/secrets/db/db.host | ||
| - --db-port-file=/secrets/db/db.port | ||
| - --db-user-file=/secrets/db/db.user | ||
| - --db-password-file=/secrets/db/db.password | ||
| - --db-name-file=/secrets/db/db.name | ||
| - --enable-jwt=true | ||
| - --enable-authz=false | ||
| - --jwk-cert-file=/configs/authentication/jwks.json | ||
| - --enable-https=false | ||
| - --api-server-bindaddress=:8000 | ||
| - --metrics-server-bindaddress=:4433 | ||
| - --health-check-server-bindaddress=:4434 | ||
| - --db-sslmode=require | ||
| - --db-max-open-connections=50 | ||
| - --enable-db-debug=false | ||
| - --enable-metrics-https=false | ||
| - --http-read-timeout=5s | ||
| - --http-write-timeout=30s | ||
| - --cors-allowed-origins=* | ||
| - --cors-allowed-headers=X-Ambient-Project | ||
| - --enable-grpc=true | ||
| - --grpc-server-bindaddress=:9000 | ||
| - --alsologtostderr | ||
| - -v=4 |
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.
❓ Verification inconclusive
Script executed:
Repository: ambient-code/platform
Repository: ambient-code/platform
Exit code: 0
stdout:
Metrics disabled by default in the production base deployment.
OTEL_EXPORTER_OTLP_ENDPOINTis commented out, so the operator disables metrics export unless an overlay restores it. The OTel collector infrastructure exists incomponents/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