Skip to content

fix(helm): quote custom labels and fix default securityContext #862

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.

Sign up for GitHub

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

Merged
slin1237 merged 3 commits into from
Mar 22, 2026
Merged

fix(helm): quote custom labels and fix default securityContext #862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eb6f122
fix(helm): correct CLI flag names and defaults in chart templates
slin1237 Mar 21, 2026
b379ea1
fix(helm): remove consistent_hashing and bucket from policy schema
slin1237 Mar 21, 2026
a7701c4
fix(helm): quote custom labels and fix default securityContext
slin1237 Mar 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion deploy/helm/smg/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: smg
description: "Shepherd Model Gateway - high-performance inference router for LLM deployments"
type: application
version: 0.1.0
appVersion: "0.7.0"
appVersion: "1.3.3"
Comment on lines 5 to +6

@coderabbitai coderabbitai Bot Mar 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider bumping chart version alongside appVersion.

The appVersion bump to 1.3.3 reflects CLI flag renames (breaking changes for users with custom values). While version: 0.1.0 can remain if chart structure is unchanged, SemVer best practice for Helm charts is to bump the chart version when default values or template behavior changes, as this signals to users that an upgrade may require values file updates.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@deploy/helm/smg/Chart.yaml` around lines 5 - 6, Bump the Chart.yaml chart
version to reflect the appVersion change: update the version field (currently
"0.1.0") to a higher SemVer (e.g., "0.1.1" or appropriate minor/patch bump) so
users are signaled about the breaking/default value changes; modify the version
key in Chart.yaml adjacent to the existing appVersion key to a new SemVer value.

kubeVersion: ">=1.26.0-0"
home: https://github.com/lightseekorg/smg
sources:
Expand Down
10 changes: 5 additions & 5 deletions deploy/helm/smg/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ Called from the router Deployment template.
- {{ .Values.router.balanceAbsThreshold | quote }}
- "--balance-rel-threshold"
- {{ .Values.router.balanceRelThreshold | quote }}
- "--eviction-interval-secs"
- "--eviction-interval"
- {{ .Values.router.evictionIntervalSecs | quote }}
- "--max-tree-size"
- {{ int .Values.router.maxTreeSize | quote }}
Expand Down Expand Up @@ -198,26 +198,26 @@ Called from the router Deployment template.
- "--postgres-db-url"
- {{ .Values.history.postgres.url | quote }}
{{- end }}
- "--postgres-pool-max"
- "--postgres-pool-max-size"
- {{ .Values.history.postgres.poolMax | quote }}
{{- end }}
{{- if eq .Values.history.backend "redis" }}
{{- if .Values.history.redis.url }}
- "--redis-url"
- {{ .Values.history.redis.url | quote }}
{{- end }}
- "--redis-pool-max"
- "--redis-pool-max-size"
- {{ .Values.history.redis.poolMax | quote }}
{{- end }}
{{- if eq .Values.history.backend "oracle" }}
{{- if .Values.history.oracle.dsn }}
- "--oracle-connect-descriptor"
- "--oracle-dsn"
- {{ .Values.history.oracle.dsn | quote }}
{{- end }}
- "--oracle-pool-max"
- {{ .Values.history.oracle.poolMax | quote }}
{{- if .Values.history.oracle.user }}
- "--oracle-username"
- "--oracle-user"
- {{ .Values.history.oracle.user | quote }}
{{- end }}
{{- if .Values.history.oracle.password }}
Expand Down
4 changes: 2 additions & 2 deletions deploy/helm/smg/templates/grafana-dashboard.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ metadata:
name: {{ include "smg.fullname" . }}-grafana-dashboard
labels:
{{- include "smg.labels" . | nindent 4 }}
{{- with .Values.grafana.dashboard.labels }}
{{- toYaml . | nindent 4 }}
{{- range $key, $value := .Values.grafana.dashboard.labels }}
{{ $key }}: {{ $value | quote }}
{{- end }}
data:
smg-dashboard.json: |
Expand Down
4 changes: 2 additions & 2 deletions deploy/helm/smg/templates/servicemonitor.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ metadata:
name: {{ include "smg.fullname" . }}-router
labels:
{{- include "smg.labels" . | nindent 4 }}
{{- with .Values.router.metrics.serviceMonitor.labels }}
{{- toYaml . | nindent 4 }}
{{- range $key, $value := .Values.router.metrics.serviceMonitor.labels }}
{{ $key }}: {{ $value | quote }}
{{- end }}
spec:
selector:
Expand Down
2 changes: 1 addition & 1 deletion deploy/helm/smg/values.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"properties": {
"policy": {
"type": "string",
"enum": ["cache_aware", "round_robin", "power_of_two", "manual", "random"]
"enum": ["cache_aware", "round_robin", "power_of_two", "manual", "random", "prefix_hash"]
},
"port": { "type": "integer", "minimum": 1, "maximum": 65535 },
"replicas": { "type": "integer", "minimum": 1 },
Expand Down
6 changes: 3 additions & 3 deletions deploy/helm/smg/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ global:
router:
replicas: 1

# -- Routing policy: cache_aware, round_robin, power_of_two, manual, random
# -- Routing policy: cache_aware, round_robin, power_of_two, manual, random, prefix_hash
policy: cache_aware

# -- Explicit worker URLs
Expand All @@ -43,7 +43,7 @@ router:
cacheThreshold: 0.3
balanceAbsThreshold: 64
balanceRelThreshold: 1.5
evictionIntervalSecs: 60
evictionIntervalSecs: 120
maxTreeSize: 67108864
blockSize: 16

Expand Down Expand Up @@ -154,7 +154,7 @@ router:
# -- Security context for router container
securityContext:
readOnlyRootFilesystem: true
runAsNonRoot: true
runAsNonRoot: false
allowPrivilegeEscalation: false
Comment on lines 155 to 158

@coderabbitai coderabbitai Bot Mar 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Document the rationale for runAsNonRoot: false.

The change fixes the CreateContainerConfigError since lightseekorg/smg runs as root. However, this weakens the default security posture. Consider adding a comment explaining why root is required, so future maintainers understand the constraint:

   # -- Security context for router container
+  # NOTE: runAsNonRoot is false because the lightseekorg/smg image currently
+  # runs as root. Set to true if using a non-root image variant.
   securityContext:
     readOnlyRootFilesystem: true
     runAsNonRoot: false
     allowPrivilegeEscalation: false
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@deploy/helm/smg/values.yaml` around lines 155 - 158, Add an inline comment
next to the securityContext configuration explaining why runAsNonRoot is set to
false: document that the container image "lightseekorg/smg" currently requires
root to start (it triggers CreateContainerConfigError when enforced), note any
known limitations or plans to upstream a non-root build, and reference any
tickets or TODOs for converting the image to non-root in the future; update the
comment near the securityContext/runAsNonRoot setting so future maintainers
understand the constraint and mitigation plan.


# -- Liveness probe tuning
Expand Down