Skip to content

fix(ambient-api-server): run as non-root and add OIDC secret placeholders#1547

Open
javierpena wants to merge 3 commits into
ambient-code:mainfrom
javierpena:main
Open

fix(ambient-api-server): run as non-root and add OIDC secret placeholders#1547
javierpena wants to merge 3 commits into
ambient-code:mainfrom
javierpena:main

Conversation

@javierpena
Copy link
Copy Markdown

@javierpena javierpena commented May 11, 2026

Add USER 1001 to the Dockerfile to satisfy restricted SecurityContext requirements.

Add empty clientId/clientSecret keys to the base ambient-api-server Secret so the ambient-control-plane pod can start in Kind where OIDC is not configured (token auth is used instead).

Summary by CodeRabbit

  • Chores
    • Improved runtime security by running the API server with reduced privileges in production to lower the attack surface.
    • Added client ID and client secret configuration fields (currently empty) to the API server secrets to prepare for future credential-based authentication; no other runtime or build behavior was changed.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 46f87c00-c8e5-421b-b261-8d4d2558cf34

📥 Commits

Reviewing files that changed from the base of the PR and between 7d03343 and 45e8a6a.

📒 Files selected for processing (1)
  • components/ambient-api-server/Dockerfile

📝 Walkthrough

Walkthrough

Runtime image now runs as non-root (USER 1001) and the Kubernetes Secret for ambient-api-server declares empty clientId and clientSecret stringData fields.

Changes

Ambient API Server: runtime user & secret fields

Layer / File(s) Summary
Secret: declare client credentials
components/manifests/base/platform/ambient-api-server-secrets.yml
Added stringData.clientId: "" and stringData.clientSecret: "" to the ambient-api-server Secret.
Runtime image: non-root user
components/ambient-api-server/Dockerfile
Runtime stage now sets USER 1001 (non-root) after EXPOSE 8000 and before ENTRYPOINT/LABEL.

Possibly related PRs

  • ambient-code/platform#1445: Adds clientId/clientSecret keys to ambient-api-server Secret and references them for OIDC_CLIENT_ID/OIDC_CLIENT_SECRET via secretKeyRef.

Suggested labels

queued


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Security And Secret Handling ❌ Error Kubernetes Secrets lack OwnerReferences: ambient-api-server-db (lines 1-15) and ambient-api-server (lines 18-29) in ambient-api-server-secrets.yml. Pre-existing issue. Add ownerReferences to both Secrets in ambient-api-server-secrets.yml to enable proper garbage collection and ownership tracking.
✅ Passed checks (7 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title follows Conventional Commits format (fix scope: description) and accurately describes both changes: adding non-root user and OIDC secret placeholders.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Performance And Algorithmic Complexity ✅ Passed PR contains only configuration changes: Dockerfile USER directive and Secret key additions. No algorithmic, loop-based, N+1, unbounded growth, or pagination concerns detected.
Kubernetes Resource Safety ✅ Passed PR modifies Dockerfile and Kubernetes Secret with empty keys. Resources properly namespace-scoped; pods have security contexts and resource limits. No violations found.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

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/platform/ambient-api-server-secrets.yml (1)

4-25: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add ownerReferences to both Secret resources (ambient-api-server-db, ambient-api-server).

Both Secrets are missing metadata.ownerReferences, which violates manifest ownership/lifecycle policy for child resources.

As per coding guidelines "All child resources (Jobs, Secrets, PVCs) must have OwnerReferences set with controller owner refs".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/manifests/base/platform/ambient-api-server-secrets.yml` around
lines 4 - 25, Both Secret manifests (metadata.name: ambient-api-server-db and
metadata.name: ambient-api-server) are missing metadata.ownerReferences; add an
ownerReferences array on each Secret pointing to the owning controller (set
apiVersion, kind, name and uid of the parent/controller and set controller: true
and blockOwnerDeletion: true) so they are properly garbage-collected and comply
with the "child resources must have OwnerReferences" guideline; update the
Secret resources with ownerReferences referencing the appropriate parent
Deployment/CustomResource by its name/uid.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@components/manifests/base/platform/ambient-api-server-secrets.yml`:
- Around line 4-25: Both Secret manifests (metadata.name: ambient-api-server-db
and metadata.name: ambient-api-server) are missing metadata.ownerReferences; add
an ownerReferences array on each Secret pointing to the owning controller (set
apiVersion, kind, name and uid of the parent/controller and set controller: true
and blockOwnerDeletion: true) so they are properly garbage-collected and comply
with the "child resources must have OwnerReferences" guideline; update the
Secret resources with ownerReferences referencing the appropriate parent
Deployment/CustomResource by its name/uid.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4ec727de-951a-4e47-8703-454ab9b06165

📥 Commits

Reviewing files that changed from the base of the PR and between 136db29 and 3646bfa.

📒 Files selected for processing (2)
  • components/ambient-api-server/Dockerfile
  • components/manifests/base/platform/ambient-api-server-secrets.yml

@netlify
Copy link
Copy Markdown

netlify Bot commented May 11, 2026

Deploy Preview for cheerful-kitten-f556a0 canceled.

Name Link
🔨 Latest commit 56f1226
🔍 Latest deploy log https://app.netlify.com/projects/cheerful-kitten-f556a0/deploys/6a0e859cfadaa60007927cff

…ders

Add USER 1001 to the Dockerfile to satisfy restricted SecurityContext
requirements.

Add empty clientId/clientSecret keys to the base ambient-api-server
Secret so the ambient-control-plane pod can start in Kind where OIDC
is not configured (token auth is used instead).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant