fix: allow CORS wildcard for Rust server and refactor v1-config to helper template by tazarov · Pull Request #128 · amikos-tech/chromadb-chart

tazarov · 2026-02-17T11:37:12Z

Summary

Removes the incorrect CORS wildcard ["*"] validation for Chroma >= 1.0.0 — the Rust server supports it
Refactors inline v1-config ConfigMap rendering into a chromadb.serverConfig helper template in _helpers.tpl
Adds chromadb.extraConfig escape hatch for merging arbitrary config keys into the v1 server config
Adds template rendering test suite (tests/test_v1_config.sh) with 21 assertions

Test plan

helm lint passes
helm template with wildcard CORS on >= 1.0.0 renders correctly
helm template with multiple CORS origins renders correctly
helm template with OTEL enabled renders correctly
helm template with custom server settings renders correctly
helm template with extraConfig merge renders correctly
helm template with extraConfig override of chart-managed keys works
CI integration tests pass

Closes #123
Partially addresses #127

…lper template The CORS wildcard validation incorrectly rejected ["*"] for Chroma >= 1.0.0, but the Rust server supports it. Refactored v1-config generation into a chromadb.serverConfig helper template that builds a config dict from values and supports extraConfig merge for advanced users. Closes #123

Copilot

Pull request overview

This PR updates the Helm chart’s Chroma v1 (>= 1.0.0) configuration rendering to support wildcard CORS origins for the Rust server, while refactoring the v1 ConfigMap generation into a reusable helper and adding an extraConfig merge escape hatch.

Changes:

Removes the v1-config template validation that rejected cors_allow_origins: ["*"].
Refactors v1 server config rendering into a chromadb.serverConfig helper and adds .Values.chromadb.extraConfig merge support.
Adds a shell-based template rendering test script for v1-config and updates documentation/values accordingly.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`tests/test_v1_config.sh`	Adds a Helm+`yq`-based test suite validating v1-config rendering scenarios (CORS, OTEL, extraConfig).
`charts/chromadb-chart/values.yaml`	Updates CORS comment and introduces `chromadb.extraConfig` with examples.
`charts/chromadb-chart/templates/config.yaml`	Replaces inline v1-config YAML rendering with a helper include.
`charts/chromadb-chart/templates/_helpers.tpl`	Introduces `chromadb.serverConfig` helper that builds the v1 config dict and merge-overwrites `extraConfig`.
`README.md`	Updates supported versions, documents wildcard CORS behavior, and adds `extraConfig` documentation.
`ISSUE_CONTEXT.md`	Adds captured issue context for #123.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…port overrides

- Add {{ fail }} guards for extraConfig overriding port/listen_address - Add validation for port <= 0 and maxPayloadSizeBytes <= 0 - Add validation for telemetry.enabled without endpoint - Fix CORS wildcard documentation in README, values.yaml, and CLAUDE.md - Remove auto-generated ISSUE_CONTEXT.md and add to .gitignore - Add error handling to get_v1_config and assert_template_fails helper - Add negative test cases for all new validations (24 tests passing)

Copilot

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-17T13:04:03Z

+  {{- $config = mergeOverwrite $config . -}}
+{{- end -}}
+{{- if ne (get $config "port" | int) ($port) -}}
+  {{- fail (printf "extraConfig.port (%v) conflicts with chromadb.serverHttpPort (%v) — update serverHttpPort instead" (get $config "port") $.Values.chromadb.serverHttpPort) -}}


The validation logic for extraConfig.port has a type conversion issue. Line 154 converts the config value to int, but if a user provides a string value in extraConfig (e.g., "9999"), the comparison ne (get $config "port" | int) ($port) may behave unexpectedly. The left side converts the potentially overridden value to int, while $port is already an int. This works, but the error message on line 155 uses (get $config "port") without the int conversion, which could display a string in the error message while comparing against an int. For consistency, either use (get $config "port" | int) in the error message or ensure type consistency throughout.

Suggested change

{{- fail (printf "extraConfig.port (%v) conflicts with chromadb.serverHttpPort (%v) — update serverHttpPort instead" (get $config "port") $.Values.chromadb.serverHttpPort) -}}

{{- fail (printf "extraConfig.port (%v) conflicts with chromadb.serverHttpPort (%v) — update serverHttpPort instead" (get $config "port" | int) $.Values.chromadb.serverHttpPort) -}}

Copilot · 2026-02-17T13:04:03Z

+assert_template_fails "telemetry.enabled without endpoint rejected" \
+  --set 'chromadb.telemetry.enabled=true'
+
+echo ""


Test 10 validates that CORS wildcard renders in v1-config for apiVersion 0.6.3 (pre-1.0), but this test is not meaningful because v1-config is only mounted and used for versions >= 1.0.0 (see statefulset.yaml lines 175-179 and 220-225). While the ConfigMap will render for all versions, it's effectively unused for < 1.0.0. This test gives false confidence that wildcard CORS works for pre-1.0 versions when in reality those versions use a different configuration mechanism. Consider either removing this test or adding a comment explaining that it only validates template rendering, not actual functionality for pre-1.0 versions.

Suggested change

echo ""

echo ""

# NOTE: v1-config is only mounted and used for apiVersion >= 1.0.0.

# For apiVersion < 1.0.0 (e.g. 0.6.3), this test only validates that the

# Helm template renders the expected CORS wildcard in the ConfigMap; it

# does NOT verify actual runtime CORS behavior for pre-1.0.0, which uses

# a different configuration mechanism.

Copilot · 2026-02-17T13:04:03Z

+> Keys in `extraConfig` override chart-managed keys of the same name. When overriding `port` or
+> `listen_address`, you must also update `chromadb.serverHttpPort` and `chromadb.serverHost` to match,
+> otherwise the Service, container port, and health probes will be out of sync.


The documentation states "Keys in extraConfig override chart-managed keys of the same name" and warns users about needing to update serverHttpPort and serverHost when overriding port or listen_address. However, the actual implementation in _helpers.tpl (lines 154-159) PREVENTS these overrides by failing with an error message. The warning should be updated to clarify that overriding port or listen_address via extraConfig is not allowed and will cause template rendering to fail. The current wording suggests it's possible but problematic, when in fact it's explicitly blocked.

Suggested change

> Keys in `extraConfig` override chart-managed keys of the same name. When overriding `port` or

> `listen_address`, you must also update `chromadb.serverHttpPort` and `chromadb.serverHost` to match,

> otherwise the Service, container port, and health probes will be out of sync.

> Keys in `extraConfig` override chart-managed keys of the same name. Overriding `port` or

> `listen_address` via `extraConfig` is **not allowed** and will cause Helm template rendering to fail.

> To change the server port or host, use `chromadb.serverHttpPort` and `chromadb.serverHost` instead so

> that the Service, container port, and health probes remain in sync.

Copilot · 2026-02-17T13:04:04Z

+  {{- $otel := dict "service_name" .Values.chromadb.telemetry.serviceName "endpoint" .Values.chromadb.telemetry.endpoint -}}
+  {{- $_ := set $config "open_telemetry" $otel -}}
+{{- end -}}
+{{- with .Values.chromadb.extraConfig -}}


The use of mergeOverwrite allows users to inject arbitrary YAML keys into the server configuration through extraConfig. While this is intentional and documented, it could potentially allow users to override security-sensitive configuration options (beyond just port and listen_address which are explicitly blocked). Consider documenting any other keys that should not be overridden for security reasons, or implement additional validation to prevent overriding other sensitive keys like authentication-related settings if they exist in the v1 config format.

Suggested change

{{- with .Values.chromadb.extraConfig -}}

{{- with .Values.chromadb.extraConfig -}}

{{- $disallowedKeys := list "auth" "authentication" "auth_token" "jwt" "security" -}}

{{- range $k := $disallowedKeys -}}

{{- if hasKey . $k -}}

{{- fail (printf "chromadb.extraConfig cannot set '%s' because it is security-sensitive; use chromadb.auth.* Helm values instead" $k) -}}

{{- end -}}

{{- end -}}

Copilot · 2026-02-17T13:04:04Z

+{{- if le $port 0 -}}
+  {{- fail (printf "chromadb.serverHttpPort must be a positive integer, got: %v" .Values.chromadb.serverHttpPort) -}}
+{{- end -}}
+{{- $maxPayload := .Values.chromadb.maxPayloadSizeBytes | int64 -}}


The validation for maxPayloadSizeBytes at line 132 checks if the value is <= 0, but the conversion to int64 happens before the check. If the user provides a non-numeric value, the int64 function may return 0, which would then fail the validation with a potentially confusing error message saying "got: ". Consider adding explicit type checking or updating the error message to clarify that the value must be numeric.

Suggested change

{{- $maxPayload := .Values.chromadb.maxPayloadSizeBytes | int64 -}}

{{- $rawMaxPayload := .Values.chromadb.maxPayloadSizeBytes -}}

{{- if not (regexMatch "^[0-9]+$" (toString $rawMaxPayload)) -}}

{{- fail (printf "chromadb.maxPayloadSizeBytes must be a positive integer (numeric), got non-numeric value: %v" $rawMaxPayload) -}}

{{- end -}}

{{- $maxPayload := $rawMaxPayload | int64 -}}

- README WARNING now states port/listen_address overrides are blocked - Add clarifying comment to test 10 about v1-config mount scope

github-actions Bot added the chart label Feb 17, 2026

docs: update README for CORS wildcard support and extraConfig

e5548aa

tazarov requested a review from Copilot February 17, 2026 11:57

Copilot started reviewing on behalf of tazarov February 17, 2026 11:58 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

Comment thread tests/test_v1_config.sh Outdated

Comment thread README.md Outdated

Comment thread tests/test_v1_config.sh Outdated

tazarov added 2 commits February 17, 2026 14:08

fix: address PR feedback - remove dead code, show stderr, warn about …

6c10f3f

…port overrides

tazarov requested a review from Copilot February 17, 2026 12:56

Copilot started reviewing on behalf of tazarov February 17, 2026 12:56 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

fix: correct README WARNING and clarify pre-1.0 test scope

bed9102

- README WARNING now states port/listen_address overrides are blocked - Add clarifying comment to test 10 about v1-config mount scope

tazarov merged commit 2d7608d into main Feb 17, 2026
8 checks passed

tazarov deleted the issue-123-bug-cors-wildcard-validation branch February 17, 2026 14:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: allow CORS wildcard for Rust server and refactor v1-config to helper template#128

fix: allow CORS wildcard for Rust server and refactor v1-config to helper template#128
tazarov merged 5 commits into
mainfrom
issue-123-bug-cors-wildcard-validation

tazarov commented Feb 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Copilot AI Feb 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	{{- fail (printf "extraConfig.port (%v) conflicts with chromadb.serverHttpPort (%v) — update serverHttpPort instead" (get $config "port") $.Values.chromadb.serverHttpPort) -}}
	{{- fail (printf "extraConfig.port (%v) conflicts with chromadb.serverHttpPort (%v) — update serverHttpPort instead" (get $config "port" \| int) $.Values.chromadb.serverHttpPort) -}}

-echo ""
+echo ""
+# NOTE: v1-config is only mounted and used for apiVersion >= 1.0.0.
+# For apiVersion < 1.0.0 (e.g. 0.6.3), this test only validates that the
+# Helm template renders the expected CORS wildcard in the ConfigMap; it
+# does NOT verify actual runtime CORS behavior for pre-1.0.0, which uses
+# a different configuration mechanism.

-> Keys in `extraConfig` override chart-managed keys of the same name. When overriding `port` or
-> `listen_address`, you must also update `chromadb.serverHttpPort` and `chromadb.serverHost` to match,
-> otherwise the Service, container port, and health probes will be out of sync.
+> Keys in `extraConfig` override chart-managed keys of the same name. Overriding `port` or
+> `listen_address` via `extraConfig` is **not allowed** and will cause Helm template rendering to fail.
+> To change the server port or host, use `chromadb.serverHttpPort` and `chromadb.serverHost` instead so
+> that the Service, container port, and health probes remain in sync.

-{{- $maxPayload := .Values.chromadb.maxPayloadSizeBytes | int64 -}}
+{{- $rawMaxPayload := .Values.chromadb.maxPayloadSizeBytes -}}
+{{- if not (regexMatch "^[0-9]+$" (toString $rawMaxPayload)) -}}
+  {{- fail (printf "chromadb.maxPayloadSizeBytes must be a positive integer (numeric), got non-numeric value: %v" $rawMaxPayload) -}}
+{{- end -}}
+{{- $maxPayload := $rawMaxPayload | int64 -}}

Conversation

tazarov commented Feb 17, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants