feat: Add support for running UI deployment in restrictive environments#722
feat: Add support for running UI deployment in restrictive environments#722qasmi wants to merge 9 commits intokagent-dev:mainfrom
Conversation
|
Please see the comment on the issue |
68dce45 to
f394db4
Compare
66b6489 to
7db948c
Compare
Signed-off-by: Sara Qasmi <saraqasmi@Saras-MacBook-Pro.local>
Signed-off-by: Sara Qasmi <saraqasmi@Saras-MacBook-Pro.local>
* feat: Add support for nodeSelector and tolerations Signed-off-by: Sara Qasmi <saraqasmi@Saras-MacBook-Pro.local> * feat: Add support for nodeSelector and tolerations in tools Signed-off-by: Sara Qasmi <saraqasmi@Saras-MacBook-Pro.local> --------- Signed-off-by: Sara Qasmi <saraqasmi@Saras-MacBook-Pro.local> Co-authored-by: Sara Qasmi <saraqasmi@Saras-MacBook-Pro.local> Signed-off-by: Sara Qasmi <saraqasmi@Saras-MacBook-Pro.local>
Signed-off-by: Sara Qasmi <saraqasmi@Saras-MacBook-Pro.local>
Signed-off-by: Sara Qasmi <saraqasmi@Saras-MacBook-Pro.local>
…r to avoid permission error (kagent-dev#743) Signed-off-by: Paul Yu <paul.d.yu@gmail.com> Signed-off-by: Sara Qasmi <saraqasmi@Saras-MacBook-Pro.local>
Signed-off-by: Sara Qasmi <saraqasmi@Saras-MacBook-Pro.local>
|
@EItanya I updated the PR following your feedback |
|
Sounds good! I'll take a look today :) |
|
Hey there, I got this when running locally: |
|
hey there, are you still interested in this? |
Signed-off-by: sara <qasmisara@gmail.com>
|
Still needed |
There was a problem hiding this comment.
Pull request overview
This PR adds support for running the UI deployment in restrictive Kubernetes environments with strict pod security contexts. The changes address issue #718 where nginx fails to start due to missing writable directories when using restrictive securityContext settings.
Changes:
- Added emptyDir volume mounts for writable directories (
/tmp,/var/lib/nginx,/run/nginx) to support read-only root filesystems and restrictive security contexts - Added configuration options for init containers, custom volumes, and volume mounts to allow users to prepare requirements before the main container starts
- Added UI-specific pod and container security contexts with secure defaults (non-root user, no privilege escalation, dropped capabilities)
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| ui/Dockerfile | Updated directory creation to include nginx runtime directories and set proper ownership for the nextjs user (uid 1001) |
| helm/kagent/values.yaml | Added UI-specific security contexts, init containers, volumes, and volumeMounts configuration options |
| helm/kagent/templates/ui-deployment.yaml | Implemented emptyDir volumes for writable paths, integrated init containers support, and applied UI-specific security contexts |
| helm/kagent/tests/ui-deployment_test.yaml | Added test coverage for init containers and custom volumes/volumeMounts features |
Comments suppressed due to low confidence (1)
helm/kagent/templates/ui-deployment.yaml:54
- The container securityContext is using the global .Values.securityContext instead of the UI-specific .Values.ui.securityContext that was added in this PR. This is inconsistent with the controller deployment pattern which uses .Values.controller.securityContext. Change this line to use (.Values.ui.securityContext | default .Values.securityContext) to align with the established pattern and allow UI-specific security context configuration.
securityContext:
{{- toYaml .Values.securityContext | nindent 12 }}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - mountPath: /tmp | ||
| name: tmp |
There was a problem hiding this comment.
The test is using /tmp as an example for volumeMounts, but /tmp is already mounted by default in the deployment template (lines 81-82 of ui-deployment.yaml). This test would create a duplicate mount path which is invalid in Kubernetes. Use a different path like /custom-mount in the test example to avoid confusion and demonstrate that additional volumeMounts work alongside the default ones.
| @@ -76,12 +76,16 @@ RUN mkdir -p $BUN_INSTALL \ | |||
| && curl -fsSL https://bun.sh/install | bash -s "bun-v$TOOLS_BUN_VERSION" \ | |||
There was a problem hiding this comment.
The Docker build uses curl -fsSL https://bun.sh/install | bash -s "bun-v$TOOLS_BUN_VERSION" to download and execute a remote installer script without any checksum or signature verification. If bun.sh or its TLS channel is compromised, an attacker could execute arbitrary code during the image build and embed a backdoor or otherwise tamper with the resulting image. Consider switching to a distribution/package-manager-based install or at minimum downloading the installer and verifying it via a pinned checksum or signature before execution.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
|
This pull request has been marked as stale because of no activity in the last 15 days. It will be closed in the next 5 days unless it is tagged "no stalebot" or other activity occurs. |
#718