fix: harden extraConfig validation and enforce CI/integration coverage#132
fix: harden extraConfig validation and enforce CI/integration coverage#132
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds type validation for the chromadb.extraConfig Helm value (introduced to address issue #127) and strengthens CI/integration test coverage to verify that extraConfig values are correctly injected into the v1-config ConfigMap. The PR hardens test scripts with better error handling and adds comprehensive test coverage for scalar/list rejection scenarios.
Changes:
- Added map/object type validation for
chromadb.extraConfigin the Helm template helper to prevent invalid configurations - Enhanced test script to validate v1-config existence and reject invalid extraConfig types (scalars/lists)
- Added CI workflow steps to install yq via Go and run v1 config template tests during linting
- Extended integration tests to validate extraConfig keys in live v1-config for ChromaDB 1.5.0
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| charts/chromadb-chart/templates/_helpers.tpl | Added type validation to ensure extraConfig is a map/object before merging |
| tests/test_v1_config.sh | Added v1-config existence check and test cases for extraConfig scalar/list rejection |
| tests/ci_smoke.sh | New smoke test script that runs v1 config tests and validates extraConfig rendering |
| Makefile | Added ci-smoke target for running smoke tests |
| .github/workflows/lint.yml | Added yq installation and v1 config template test execution to lint workflow |
| .github/workflows/integration-test.yml | Added extraConfig injection and validation for 1.5.0, hardened shell flags |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Install yq | ||
| run: | | ||
| set -euo pipefail | ||
| YQ_VERSION="v4.44.6" | ||
| GOBIN_DIR="$(mktemp -d)" | ||
| cleanup() { | ||
| rm -rf "${GOBIN_DIR}" | ||
| } | ||
| trap cleanup EXIT | ||
| GOBIN="${GOBIN_DIR}" go install "github.com/mikefarah/yq/v4@${YQ_VERSION}" | ||
| sudo install -m 0755 "${GOBIN_DIR}/yq" /usr/local/bin/yq | ||
| yq --version |
There was a problem hiding this comment.
The PR description mentions "checksum verification" for the yq installation, but no checksum verification is present in the actual implementation. Consider adding checksum verification for enhanced security, or update the PR description to accurately reflect what was implemented.
1 participant
Summary
Validation
Closes #127