feat(charts): add br-ccs chart - #1708
Conversation
New chart promoted from develop as its own PR to main (no changes to other charts).
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughIntroduces the Changesbr-ccs Helm chart
Sequence Diagram(s)sequenceDiagram
participant Helm
participant MigrationJob
participant PostgreSQL
participant RabbitMQBootstrap
participant RabbitMQ
participant Deployment
participant Service
Helm->>MigrationJob: Render migration hook
MigrationJob->>PostgreSQL: Wait for readiness and run migrations
Helm->>RabbitMQBootstrap: Render bootstrap Job when enabled
RabbitMQBootstrap->>RabbitMQ: Apply definitions and update credentials
Helm->>Deployment: Render application Deployment
Deployment->>PostgreSQL: Poll database connectivity
Deployment->>Service: Expose HTTP port 4030
Possibly related PRs
✨ Finishing Touches✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/br-ccs/CHANGELOG.md`:
- Line 8: Update the 0.1.0 heading in the changelog to match the version
declared by the chart's version field, 1.0.0-beta.4, while preserving the
existing release date and changelog structure.
In `@charts/br-ccs/files/rabbitmq/load_definitions.json`:
- Around line 2-7: Add a valid password or password_hash property to the br-ccs
user entry in the RabbitMQ definitions, while preserving its existing name and
tags fields so both external bootstrap and bundled loadDefinition imports
succeed.
In `@charts/br-ccs/templates/bootstrap-postgres.yaml`:
- Around line 17-129: Add matching securityContext blocks to both initContainer
wait-for-dependencies and container psql, aligning with bootstrap-rabbitmq.yaml:
enforce runAsNonRoot, disable privilege escalation, use a read-only root
filesystem, and drop all Linux capabilities.
In `@charts/br-ccs/templates/configmap.yaml`:
- Around line 138-165: Update the RABBITMQ_HOST handling in the ConfigMap
template so external RabbitMQ configurations require an explicitly provided host
and fail rendering when it is missing; only retain the in-cluster service-name
fallback when the RabbitMQ dependency is enabled internally. Align the
conditional behavior with the chart’s existing Postgres/Valkey handling and the
rabbitmq.enabled/rabbitmq.external settings.
In `@charts/br-ccs/templates/deployment.yaml`:
- Around line 141-144: Update the deployment container port in the ports section
to derive from the same configurable SERVER_PORT value used by configmap.yaml,
rather than hardcoding 4030. Preserve the existing http port name so
livenessProbe, readinessProbe, and service.yaml targetPort continue routing to
the configured application port.
- Line 45: Make the init-container image in the deployment template configurable
through chart values instead of hardcoding busybox:1.37. Add or reuse a
dedicated values key for the image repository and tag, and reference it in the
init-container image field while preserving busybox:1.37 as the default.
In `@charts/br-ccs/templates/migration-secret.yaml`:
- Around line 24-29: Update the migration Secret hook configuration in
migration-secret.yaml to add an explicit Helm uninstall cleanup hook, while
preserving the existing pre-install and pre-upgrade behavior. Ensure the Secret
is deleted after release removal, without altering the Job’s ability to consume
it during migration.
In `@charts/br-ccs/values.yaml`:
- Line 716: Remove the leftover “trigger: retry release after README fix”
comment from the chart values content, without changing any surrounding
configuration.
- Around line 710-712: Update the extraSecrets entry in values.yaml to use the
release-scoped RabbitMQ Secret name generated by the br-ccs.fullname helper,
matching the name created in rabbitmq_load_definitions.yaml. Remove the
hardcoded br-ccs prefix while preserving the existing mountPath.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ae820742-a503-4c04-b6c1-3ccaa67d44ed
⛔ Files ignored due to path filters (1)
charts/br-ccs/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
charts/br-ccs/CHANGELOG.mdcharts/br-ccs/Chart.yamlcharts/br-ccs/README.mdcharts/br-ccs/files/rabbitmq/load_definitions.jsoncharts/br-ccs/templates/_helpers.tplcharts/br-ccs/templates/bootstrap-postgres.yamlcharts/br-ccs/templates/bootstrap-rabbitmq.yamlcharts/br-ccs/templates/configmap.yamlcharts/br-ccs/templates/deployment.yamlcharts/br-ccs/templates/hpa.yamlcharts/br-ccs/templates/ingress.yamlcharts/br-ccs/templates/migration-secret.yamlcharts/br-ccs/templates/migrations.yamlcharts/br-ccs/templates/pdb.yamlcharts/br-ccs/templates/rabbitmq_load_definitions.yamlcharts/br-ccs/templates/secrets.yamlcharts/br-ccs/templates/service.yamlcharts/br-ccs/templates/serviceaccount.yamlcharts/br-ccs/values-template.yamlcharts/br-ccs/values.schema.jsoncharts/br-ccs/values.yaml
Add STA_EVENT_HMAC_MASTER_KEYS to the br-ccs chart Secret allowlist so the STA business-event HMAC master-key catalogue can be delivered as key material (via envFrom secretRef), not the plaintext ConfigMap. Mirrors the existing CCS_CRYPTO_MASTER_KEY wiring; documented under brCcs.secrets in values.yaml. Inert until STA_EVENT_HMAC_VERIFY_ENABLED is turned on. X-Lerian-Ref: 0x1 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
charts/br-ccs/values.yaml (2)
323-324: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFix the inverted Redis retry-backoff bounds.
REDIS_MIN_RETRY_BACKOFFis8whileREDIS_MAX_RETRY_BACKOFFis1. This can cause validation failures or collapse the retry range, potentially producing incorrect retry behavior or a retry storm. Swap the values after confirming the configured units.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/br-ccs/values.yaml` around lines 323 - 324, Swap the values assigned to REDIS_MIN_RETRY_BACKOFF and REDIS_MAX_RETRY_BACKOFF in the chart configuration so the minimum is 1 and the maximum is 8, preserving their existing string format and configured units.
294-294: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winBundled PostgreSQL needs a non-TLS client mode
postgresql.enabled: trueuses Bitnami PostgreSQL withtls.enabledoff by default, soPOSTGRES_SSLMODE: "require"breaks the default install. Usedisablefor the bundled database and reserverequirefor TLS-enabled external PostgreSQL, or enable TLS in the subchart.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/br-ccs/values.yaml` at line 294, Update the POSTGRES_SSLMODE value in the chart configuration to use disable for the bundled PostgreSQL default, or conditionally retain require only when external PostgreSQL TLS is enabled; preserve secure TLS behavior for TLS-enabled external databases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@charts/br-ccs/values.yaml`:
- Around line 323-324: Swap the values assigned to REDIS_MIN_RETRY_BACKOFF and
REDIS_MAX_RETRY_BACKOFF in the chart configuration so the minimum is 1 and the
maximum is 8, preserving their existing string format and configured units.
- Line 294: Update the POSTGRES_SSLMODE value in the chart configuration to use
disable for the bundled PostgreSQL default, or conditionally retain require only
when external PostgreSQL TLS is enabled; preserve secure TLS behavior for
TLS-enabled external databases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 63f1e73c-4dab-4809-bb8b-239cc64051ff
📒 Files selected for processing (2)
charts/br-ccs/templates/secrets.yamlcharts/br-ccs/values.yaml
Pilot productization (like #1741). Done: lerian-common 1.3.4 dependency; MULTI_TENANT_* block replaced with lerian-common.multiTenant.env (enable via cfgValue grouped param; requiredUrl/requiredRedisHost preserved). Render-validated equivalent to raw. TODO: OTEL (otel.env + global.observability), datastores (DB/REDIS/RABBITMQ via datastore.value), ENV_NAME/DEPLOYMENT_MODE single-source, global.* values API, fail-fast secrets, render-asserts, backward-compat + lint + render-gate.
…ervability Derive ENABLE_TELEMETRY / OTLP endpoint / deployment-environment from global.observability (env-wide) with a component configmap.<KEY> override; OTEL identity + SERVICE_VERSION via otel.envFlat (version now tracks image.tag | AppVersion instead of a stale hardcoded default). Default render byte-identical to the pre-contract block.
Mask POSTGRES/REDIS/RABBITMQ primary connection fields (host, port, user, ssl, replicaHost) through the datastore mask: brCcs.configmap.<KEY> > brCcs.datastores.<type> (dedicated) > global.datastores.<type> (shared) > default. Bundled-subchart HOST defaults preserved; external HOST now satisfiable via mask or native key (required message updated). Removed the masked keys from configmap defaults so the mask is effective; POSTGRES_NAME and per-service tuning stay native. Default render byte-identical.
…param - MULTI_TENANT_SERVICE_API_KEY via lerian-common.multiTenant.secret: fails fast when multi-tenancy is enabled but the key is empty (was silently skipped). REDIS_PASSWORD optional; REDIS_CA_CERT kept inline. - RABBITMQ_DEFAULT_PASS required when RABBITMQ_ENABLED=true and no full RABBITMQ_URL is supplied. - Comment out MULTI_TENANT_* configmap defaults (incl. MULTI_TENANT_ENABLED) so brCcs.multiTenant.enabled + global.multiTenant actually drive the block; the native keys had masked the grouped API. multiTenant.env / helper defaults are now the single source. Default render byte-identical (verified against the pre-productization chart).
…n paths Fixture turns on multi-tenancy (+ API key), observability (global.otel), and RabbitMQ (+ default pass) so the render gate covers multiTenant.env / multiTenant.secret fail-fast, otel.env global derivation, and the broker datastore mask in their enabled state — not just the all-defaults path.
…e + grouped params Complete the productization to match midaz #1741: route ALL always-on config keys through lerian-common.cfgValue (configmap.<KEY> escape-hatch > brCcs.<group>.<field> > chart default) instead of raw $cm.KEY passthrough, and empty brCcs.configmap (defaults now live in the template). Auth resolves via globalValue (global.auth). Adds clean grouped param blocks: app/server/cors/postgres/redis/broker/outbox/ fetcher/sta/reporter/swagger/rateLimit/pagination/observability/m2m/readiness/ shutdown/objectStorage. The raw configmap map remains as an override escape-hatch and for optional/opt-in keys. No raw env vars shipped in values.yaml anymore; operators use the typed grouped API (e.g. brCcs.redis.poolSize) with configmap.<KEY> still winning as override. Default render byte-identical to the prior productized head (objectStorage bucket defaults carried into the group). helm lint + render-gate + strict standard pass.
… gap) The app's config/.env.example declares SD_* (10 vars) and STREAMING_* that no chart (raw or productized) ever exposed — surfaced by the .env-coverage check. Wire the existing lerian-common helpers, same pattern as MT/OTEL: - serviceDiscovery.env + global.serviceDiscovery, toggle brCcs.serviceDiscovery.enabled; SD_TOKEN (ACL) is a Secret. Full SD_* block derives only when enabled. - streaming.env + global.streaming + streaming.secret, toggle brCcs.streaming.enabled; STREAMING_SASL_PASSWORD fail-fast when SASL is set. Full block derives only when enabled. - Also close 2 fetcher config gaps (CCS_FETCHER_MAX_REDELIVERIES/RETRY_BACKOFF_MS). Render fixture now enables SD + streaming (SASL) so the render gate exercises them. Default render adds only SD_ENABLED/STREAMING_ENABLED (gated, off) — otherwise byte-identical. First run of the productize-chart-env skill. lint + render-gate + strict pass.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
charts/br-ccs/values.yaml (1)
327-329: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument the new supported values.
configmap.yamlsupportsbrCcs.fetcher.maxRedeliveriesandbrCcs.fetcher.retryBackoffMs, but thefetchervalue contract does not list them.secrets.yamlalso readsSD_TOKEN,STREAMING_SASL_PASSWORD, and optionalSTREAMING_TLS_CA_CERT, butbrCcs.secretsdoes not expose them.Add these fields as documented placeholders in
values.yaml. This keepshelm show valuesusable for the new integrations.Also applies to: 358-423
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/br-ccs/values.yaml` around lines 327 - 329, Update the fetcher and secrets value contracts in values.yaml to document placeholders for brCcs.fetcher.maxRedeliveries and brCcs.fetcher.retryBackoffMs, plus SD_TOKEN, STREAMING_SASL_PASSWORD, and optional STREAMING_TLS_CA_CERT under brCcs.secrets. Preserve the existing values structure and comments while exposing all fields consumed by configmap.yaml and secrets.yaml.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@charts/br-ccs/values.yaml`:
- Around line 327-329: Update the fetcher and secrets value contracts in
values.yaml to document placeholders for brCcs.fetcher.maxRedeliveries and
brCcs.fetcher.retryBackoffMs, plus SD_TOKEN, STREAMING_SASL_PASSWORD, and
optional STREAMING_TLS_CA_CERT under brCcs.secrets. Preserve the existing values
structure and comments while exposing all fields consumed by configmap.yaml and
secrets.yaml.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0c67a4c2-cbb4-4183-a4bf-fdf253e9b6f7
📒 Files selected for processing (4)
.github/configs/helm-render-values/br-ccs.yamlcharts/br-ccs/templates/configmap.yamlcharts/br-ccs/templates/secrets.yamlcharts/br-ccs/values.yaml
Generated by productize-chart-env (gen-schema.py + gen-docs.py): strict-where-safe schema (additionalProperties:false on the typed grouped blocks — rejects a mistyped key like brCcs.redis.zTYPO at helm install; permissive on root/subcharts/global/open maps) and an auto-generated parameter table from the '# --' annotations.
The generated schema closed operational/structural blocks (probes, autoscaling, pdb, resources, service, ingress, ...) with additionalProperties:false, rejecting valid operator input like pdb.maxUnavailable or resources.limits. Strictness now applies only to the finite cfgValue config groups (typo protection preserved there); k8s passthrough stays open. Regenerated with the corrected generator.
d5c7ee9 to
f172464
Compare
The pin lagged main (1.3.4 while main shipped 1.4.0), so the merged-PR render could not resolve the file:// dependency (missing-dependency). Align with main and refresh Chart.lock. Merged origin/main to vendor lerian-common 1.4.0.
feat(charts): add br-ccs chart
New chart promoted from develop as its own PR to main (no changes to other charts).