Skip to content

Commit 4f57d01

Browse files
tazarovtazarov
authored
Merge pull request #134 from amikos-tech/codex/issue-96-parameter-validation
Validate chart parameters for #96
2 parents 1f1adab + 206f47e commit 4f57d01

4 files changed

Lines changed: 111 additions & 13 deletions

File tree

‎README.md‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,23 +48,23 @@ Example `values.yaml` file:
4848

4949
```yaml
5050
chromadb:
51-
allowReset: "true"
51+
allowReset: true
5252
```
5353
5454
Alternatively you can specify each parameter using the `--set key=value[,key=value]` argument to `helm install`.
5555

5656
```bash
57-
helm install chroma chroma/chromadb --set chromadb.allowReset="true"
57+
helm install chroma chroma/chromadb --set chromadb.allowReset=true
5858
```
5959

6060
## Chart Configuration Values
6161

6262
| Key | Type | Default | Description |
6363
|-----------------------------------------------------|---------|---------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
6464
| `chromadb.apiVersion` | string | `1.5.0` (Chart app version) | The ChromaDB version. Supported version `0.4.3` - `1.x` |
65-
| `chromadb.allowReset` | boolean | `false` | Allows resetting the index (delete all data) |
66-
| `chromadb.isPersistent` | boolean | `true` | `< 1.0.0`: controls PVC plus `IS_PERSISTENT` server mode. `>= 1.0.0`: controls only PVC creation/mounting for `persistDirectory`; the Rust server always writes to disk, so data is ephemeral without a PVC. |
67-
| `chromadb.persistDirectory` | string | `/data` | The location to store the index data. This configure both chromadb and underlying persistent volume |
65+
| `chromadb.allowReset` | boolean | `false` | Allows resetting the index (delete all data). Accepts bool or string `true`/`false` (case-insensitive); rendered value is normalized to lowercase. |
66+
| `chromadb.isPersistent` | boolean | `true` | `< 1.0.0`: controls PVC plus `IS_PERSISTENT` server mode. `>= 1.0.0`: controls only PVC creation/mounting for `persistDirectory`; the Rust server always writes to disk, so data is ephemeral without a PVC. Accepts bool or string `true`/`false` (case-insensitive); rendered value is normalized to lowercase. |
67+
| `chromadb.persistDirectory` | string | `/data` | Absolute path where index data is stored. Used for both Chroma server config and mounted persistent volume path. |
6868
| `chromadb.anonymizedTelemetry` | boolean | `false` | Legacy PostHog telemetry flag for `< 1.0.0`. **Note**: This has no effect in Chroma `>= 1.0.0`; use `chromadb.telemetry.*` for OTEL. |
6969
| `chromadb.corsAllowOrigins` | list | `[]` | List of allowed CORS origins. Wildcard `["*"]` is supported. |
7070
| `chromadb.apiImpl` | string | `- "chromadb.api.segment.SegmentAPI"` | Legacy/removed key kept for historical compatibility in docs. The chart does not read this value in current versions. |

‎charts/chromadb-chart/templates/_helpers.tpl‎

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,59 @@ Get the chroma api version
120120
{{- end }}
121121
{{- end }}
122122

123+
{{/*
124+
Normalize boolean values passed as bool or string.
125+
Returns "true" or "false".
126+
*/}}
127+
{{- define "chromadb.boolValue" -}}
128+
{{- $name := .name -}}
129+
{{- $value := .value -}}
130+
{{- if kindIs "bool" $value -}}
131+
{{- if $value }}true{{ else }}false{{ end -}}
132+
{{- else if kindIs "string" $value -}}
133+
{{- $normalized := lower (trim $value) -}}
134+
{{- if or (eq $normalized "true") (eq $normalized "false") -}}
135+
{{- $normalized -}}
136+
{{- else -}}
137+
{{- fail (printf "%s must be true or false (case-insensitive), got %q" $name $value) -}}
138+
{{- end -}}
139+
{{- else -}}
140+
{{- fail (printf "%s must be a boolean or a string true/false, got type %s" $name (kindOf $value)) -}}
141+
{{- end -}}
142+
{{- end }}
143+
144+
{{/*
145+
Normalize/validate chromadb.allowReset.
146+
*/}}
147+
{{- define "chromadb.allowReset" -}}
148+
{{- include "chromadb.boolValue" (dict "name" "chromadb.allowReset" "value" .Values.chromadb.allowReset) -}}
149+
{{- end }}
150+
151+
{{/*
152+
Normalize/validate chromadb.isPersistent.
153+
*/}}
154+
{{- define "chromadb.isPersistent" -}}
155+
{{- include "chromadb.boolValue" (dict "name" "chromadb.isPersistent" "value" .Values.chromadb.isPersistent) -}}
156+
{{- end }}
157+
158+
{{/*
159+
Validate and normalize the persist directory path.
160+
*/}}
161+
{{- define "chromadb.persistDirectory" -}}
162+
{{- $raw := .Values.chromadb.persistDirectory -}}
163+
{{- if not (kindIs "string" $raw) -}}
164+
{{- fail (printf "chromadb.persistDirectory must be a string absolute path, got type %s" (kindOf $raw)) -}}
165+
{{- end -}}
166+
{{- $path := trim $raw -}}
167+
{{- if eq $path "" -}}
168+
{{- fail "chromadb.persistDirectory must not be empty" -}}
169+
{{- end -}}
170+
{{- if not (hasPrefix "/" $path) -}}
171+
{{- fail (printf "chromadb.persistDirectory must be an absolute path starting with '/': got %q" $raw) -}}
172+
{{- end -}}
173+
{{- $path -}}
174+
{{- end }}
175+
123176
{{/*
124177
Build the server config dict for the v1-config ConfigMap.
125178
*/}}
@@ -133,12 +186,14 @@ Build the server config dict for the v1-config ConfigMap.
133186
{{- fail (printf "chromadb.maxPayloadSizeBytes must be a positive integer, got: %v" .Values.chromadb.maxPayloadSizeBytes) -}}
134187
{{- end -}}
135188
{{- $isV1 := semverCompare ">= 1.0.0" (include "chromadb.apiVersion" .) -}}
189+
{{- $allowReset := eq (include "chromadb.allowReset" .) "true" -}}
190+
{{- $persistDirectory := include "chromadb.persistDirectory" . -}}
136191
{{- $config := dict -}}
137192
{{- $_ := set $config "port" $port -}}
138193
{{- $_ := set $config "listen_address" .Values.chromadb.serverHost -}}
139194
{{- $_ := set $config "max_payload_size_bytes" $maxPayload -}}
140-
{{- $_ := set $config "persist_path" .Values.chromadb.persistDirectory -}}
141-
{{- $_ := set $config "allow_reset" .Values.chromadb.allowReset -}}
195+
{{- $_ := set $config "persist_path" $persistDirectory -}}
196+
{{- $_ := set $config "allow_reset" $allowReset -}}
142197
{{- if .Values.chromadb.corsAllowOrigins -}}
143198
{{- $_ := set $config "cors_allow_origins" .Values.chromadb.corsAllowOrigins -}}
144199
{{- end -}}

‎charts/chromadb-chart/templates/statefulset.yaml‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
{{- $allowReset := include "chromadb.allowReset" . -}}
2+
{{- $isPersistentStr := include "chromadb.isPersistent" . -}}
3+
{{- $isPersistent := eq $isPersistentStr "true" -}}
4+
{{- $persistDirectory := include "chromadb.persistDirectory" . -}}
15
apiVersion: apps/v1
26
kind: StatefulSet
37
metadata:
@@ -87,11 +91,11 @@ spec:
8791
fieldPath: metadata.name
8892
{{- if and (semverCompare "< 1.0.0" (include "chromadb.apiVersion" .)) }}
8993
- name: IS_PERSISTENT
90-
value: "{{ .Values.chromadb.isPersistent }}"
94+
value: "{{ $isPersistentStr }}"
9195
- name: PERSIST_DIRECTORY
92-
value: "{{ .Values.chromadb.persistDirectory }}"
96+
value: "{{ $persistDirectory }}"
9397
- name: ALLOW_RESET
94-
value: "{{ .Values.chromadb.allowReset | default false}}"
98+
value: "{{ $allowReset }}"
9599
- name: ANONYMIZED_TELEMETRY
96100
value: "{{ .Values.chromadb.anonymizedTelemetry | default false }}"
97101
{{- if .Values.chromadb.corsAllowOrigins }}
@@ -165,8 +169,8 @@ spec:
165169
name: log-config-legacy
166170
subPath: log_config.yaml
167171
{{- end }}
168-
{{- if eq .Values.chromadb.isPersistent true }}
169-
- mountPath: "{{.Values.chromadb.persistDirectory}}"
172+
{{- if $isPersistent }}
173+
- mountPath: "{{ $persistDirectory }}"
170174
name: data
171175
{{- end }}
172176
{{- if and (semverCompare "< 1.0.0" (include "chromadb.apiVersion" .)) (semverCompare ">= 0.4.7" (include "chromadb.apiVersion" .)) .Values.chromadb.auth.enabled (eq .Values.chromadb.auth.type "basic") }}
@@ -226,7 +230,7 @@ spec:
226230
name: v1-config
227231
defaultMode: 0644
228232
{{- end }}
229-
{{- if eq .Values.chromadb.isPersistent true }}
233+
{{- if $isPersistent }}
230234
volumeClaimTemplates:
231235
- metadata:
232236
name: data

‎tests/test_v1_config.sh‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,16 @@ get_statefulset_env_value() {
5454
echo "$output" | yq eval ".spec.template.spec.containers[] | select(.name == \"chromadb\") | [.env[]? | select(.name == \"$env_name\") | .value][0] // \"null\"" -
5555
}
5656

57+
get_statefulset_value() {
58+
local expr="$1"; shift
59+
local output
60+
output=$(helm template test "$CHART_DIR" "$@" --show-only templates/statefulset.yaml 2>&1) || {
61+
echo "TEMPLATE_ERROR: $output" >&2
62+
return 1
63+
}
64+
echo "$output" | yq eval "$expr" -
65+
}
66+
5767
assert_equal() {
5868
local desc="$1" actual="$2" expected="$3"
5969
if [ "$actual" = "$expected" ]; then
@@ -288,6 +298,35 @@ echo "28. extraConfig must reject scalar values"
288298
assert_template_fails "extraConfig rejects scalar string values" \
289299
--set 'chromadb.extraConfig=invalid'
290300

301+
echo ""
302+
echo "29. allowReset accepts case-insensitive string booleans"
303+
config=$(get_v1_config --set-string 'chromadb.allowReset=TRUE')
304+
assert_config_key "allow_reset normalizes uppercase TRUE to true" "$config" "allow_reset" "true"
305+
306+
echo ""
307+
echo "30. isPersistent accepts case-insensitive string booleans"
308+
is_persistent_env=$(get_statefulset_env_value "IS_PERSISTENT" --set 'chromadb.apiVersion=0.6.3' --set-string 'chromadb.isPersistent=FALSE')
309+
assert_equal "IS_PERSISTENT normalizes uppercase FALSE to false" "$is_persistent_env" "false"
310+
pvc_templates=$(get_statefulset_value '.spec.volumeClaimTemplates | length // 0' --set-string 'chromadb.isPersistent=FALSE')
311+
assert_equal "volumeClaimTemplates omitted when isPersistent is false string" "$pvc_templates" "0"
312+
313+
echo ""
314+
echo "31. allowReset rejects invalid string values"
315+
assert_template_fails "allowReset rejects non-boolean strings" \
316+
--set-string 'chromadb.allowReset=yes'
317+
318+
echo ""
319+
echo "32. isPersistent rejects invalid string values"
320+
assert_template_fails "isPersistent rejects non-boolean strings" \
321+
--set-string 'chromadb.isPersistent=on'
322+
323+
echo ""
324+
echo "33. persistDirectory must be an absolute path"
325+
assert_template_fails "persistDirectory rejects relative paths" \
326+
--set-string 'chromadb.persistDirectory=data'
327+
assert_template_fails "persistDirectory rejects empty strings" \
328+
--set-string 'chromadb.persistDirectory='
329+
291330
echo ""
292331
echo "--- Results: $PASS passed, $FAIL failed ---"
293332
[ "$FAIL" -eq 0 ] || exit 1

0 commit comments

Comments
 (0)