fix(ambient-api-server): run as non-root and add OIDC secret placeholders#1547
Open
javierpena wants to merge 3 commits into
Open
fix(ambient-api-server): run as non-root and add OIDC secret placeholders#1547javierpena wants to merge 3 commits into
javierpena wants to merge 3 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRuntime image now runs as non-root ( ChangesAmbient API Server: runtime user & secret fields
Possibly related PRs
Suggested labels
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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 winAdd
ownerReferencesto 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
OwnerReferencesset 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
📒 Files selected for processing (2)
components/ambient-api-server/Dockerfilecomponents/manifests/base/platform/ambient-api-server-secrets.yml
✅ Deploy Preview for cheerful-kitten-f556a0 canceled.
|
…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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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