feat(charts): add br-sta chart - #1710
Open
guimoreirar wants to merge 11 commits into
Open
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:
📝 WalkthroughWalkthroughThe pull request adds the Changesbr-sta Helm chart
Sequence Diagram(s)sequenceDiagram
participant HelmRelease
participant BootstrapPostgresJob
participant PostgreSQL
participant Kubernetes
participant Manager
participant Worker
HelmRelease->>BootstrapPostgresJob: Run external PostgreSQL bootstrap hook
BootstrapPostgresJob->>PostgreSQL: Create role, database, and grants
HelmRelease->>Kubernetes: Deploy manager and worker resources
Kubernetes->>PostgreSQL: Wait for infrastructure readiness
Kubernetes->>Manager: Start with ConfigMap and Secret configuration
Manager->>PostgreSQL: Connect using resolved credentials
opt when worker enabled
Kubernetes->>Worker: Start with shared and worker configuration
Worker->>PostgreSQL: Connect using resolved credentials
end
Possibly related PRs
✨ Finishing Touches✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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-sta/README.md`:
- Line 31: Update the worker Deployment description in the README to identify
its binary as /service, matching the worker command configured in values.yaml;
leave the manager description unchanged.
In `@charts/br-sta/templates/bootstrap-postgres.yaml`:
- Around line 19-93: Harden the bootstrap Job template by adding pod-level and
container-level securityContext settings to the pod spec, covering both
wait-for-dependencies and psql. Match the existing security baseline from
deployment.yaml, including non-root execution, disabled privilege escalation,
dropped capabilities, and read-only root filesystems where compatible with the
images.
In `@charts/br-sta/templates/migrations.yaml`:
- Around line 41-49: The migration template must fail during rendering when
PostgreSQL is external and POSTGRES_HOST is unset. Update the host resolution
logic around $pgHost, $configmap, and $internalPg to validate that an external
configuration provides a non-empty host, while preserving the existing internal
PostgreSQL hostname generation for standalone and replication architectures.
- Around line 96-98: Update the migration Job fields backoffLimit,
activeDeadlineSeconds, and ttlSecondsAfterFinished to use hasKey-based fallback
logic instead of default, preserving explicitly configured zero values while
applying 3 or 600 only when the corresponding key is absent.
In `@charts/br-sta/templates/NOTES.txt`:
- Around line 31-35: The PostgreSQL endpoint in NOTES.txt always appends
“-primary” and must match the architecture-aware POSTGRES_HOST logic from
configmap.yaml. Update the PostgreSQL host rendering under the
postgresql.enabled condition to append “-primary” only when
postgresql.architecture is “replication”; use the plain dependency fullname for
standalone mode.
In `@charts/br-sta/templates/worker/deployment.yaml`:
- Around line 24-32: Update the worker Deployment template around the
replicaCount expression to fail rendering when worker.replicaCount is greater
than 1, while retaining the default of 1 when it is unset. Ensure the validation
produces a clear configuration error and preserves the existing single-replica
behavior.
- Around line 62-92: Add a securityContext to the wait-for-infra init container
matching the existing $secCtx applied to the main container, reusing the same
pod security settings rather than leaving the init container on defaults.
In `@charts/br-sta/values-template.yaml`:
- Around line 10-11: Replace the image.tag default in the production values
template with a non-deployable placeholder that requires users to select a
published immutable tag, while preserving the existing pinning guidance.
In `@charts/br-sta/values.yaml`:
- Line 666: Remove the trailing blank line at the end of values.yaml, ensuring
the file ends immediately after its final content line so YAML linting passes.
- Around line 273-275: Swap the values assigned to REDIS_MIN_RETRY_BACKOFF and
REDIS_MAX_RETRY_BACKOFF in the Redis configuration so the minimum is the smaller
backoff value and the maximum is the larger value, while leaving
REDIS_MAX_RETRIES unchanged.
🪄 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: a7c2652e-088a-4d77-8c46-8040a3376084
⛔ Files ignored due to path filters (1)
charts/br-sta/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
charts/br-sta/Chart.yamlcharts/br-sta/README.mdcharts/br-sta/templates/NOTES.txtcharts/br-sta/templates/_helpers.tplcharts/br-sta/templates/bootstrap-postgres.yamlcharts/br-sta/templates/configmap.yamlcharts/br-sta/templates/deployment.yamlcharts/br-sta/templates/hpa.yamlcharts/br-sta/templates/ingress.yamlcharts/br-sta/templates/migrations.yamlcharts/br-sta/templates/pdb.yamlcharts/br-sta/templates/secrets.yamlcharts/br-sta/templates/service.yamlcharts/br-sta/templates/serviceaccount.yamlcharts/br-sta/templates/worker/configmap.yamlcharts/br-sta/templates/worker/deployment.yamlcharts/br-sta/templates/worker/serviceaccount.yamlcharts/br-sta/values-template.yamlcharts/br-sta/values.schema.jsoncharts/br-sta/values.yaml
Comment on lines
+19
to
+93
| template: | ||
| spec: | ||
| restartPolicy: Never | ||
| initContainers: | ||
| - name: wait-for-dependencies | ||
| image: busybox:1.37 | ||
| env: | ||
| - name: DB_HOST | ||
| value: {{ .Values.global.externalPostgresDefinitions.connection.host | quote }} | ||
| - name: DB_PORT | ||
| value: {{ .Values.global.externalPostgresDefinitions.connection.port | quote }} | ||
| command: | ||
| - /bin/sh | ||
| - -c | ||
| - > | ||
| TIMEOUT=300; | ||
| ELAPSED=0; | ||
| echo "Checking $DB_HOST:$DB_PORT..."; | ||
| while ! nc -z "$DB_HOST" "$DB_PORT"; do | ||
| if [ $ELAPSED -ge $TIMEOUT ]; then | ||
| echo "Timeout waiting for $DB_HOST:$DB_PORT after ${TIMEOUT}s"; | ||
| exit 1; | ||
| fi; | ||
| echo "$DB_HOST:$DB_PORT not ready, waiting (${ELAPSED}s/${TIMEOUT}s)"; | ||
| sleep 5; | ||
| ELAPSED=$((ELAPSED + 5)); | ||
| done; | ||
| echo "$DB_HOST:$DB_PORT is ready!"; | ||
| containers: | ||
| - name: psql | ||
| image: postgres:17 | ||
| env: | ||
| - name: DB_HOST | ||
| value: {{ .Values.global.externalPostgresDefinitions.connection.host | quote }} | ||
| - name: DB_PORT | ||
| value: {{ .Values.global.externalPostgresDefinitions.connection.port | quote }} | ||
| - name: DB_USER_ADMIN | ||
| {{- if .Values.global.externalPostgresDefinitions.postgresAdminLogin.useExistingSecret.name }} | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: {{ .Values.global.externalPostgresDefinitions.postgresAdminLogin.useExistingSecret.name | quote }} | ||
| key: DB_USER_ADMIN | ||
| {{- else }} | ||
| value: {{ .Values.global.externalPostgresDefinitions.postgresAdminLogin.username | quote }} | ||
| {{- end }} | ||
| - name: DB_ADMIN_PASSWORD | ||
| {{- if .Values.global.externalPostgresDefinitions.postgresAdminLogin.useExistingSecret.name }} | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: {{ .Values.global.externalPostgresDefinitions.postgresAdminLogin.useExistingSecret.name | quote }} | ||
| key: DB_ADMIN_PASSWORD | ||
| {{- else }} | ||
| value: {{ .Values.global.externalPostgresDefinitions.postgresAdminLogin.password | quote }} | ||
| {{- end }} | ||
| - name: DB_PASSWORD_BR_STA | ||
| {{- if .Values.global.externalPostgresDefinitions.appCredentials.useExistingSecret.name }} | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: {{ .Values.global.externalPostgresDefinitions.appCredentials.useExistingSecret.name | quote }} | ||
| key: DB_PASSWORD_BR_STA | ||
| {{- else }} | ||
| value: {{ .Values.global.externalPostgresDefinitions.appCredentials.password | quote }} | ||
| {{- end }} | ||
| - name: DB_DATABASE | ||
| value: postgres | ||
| - name: APP_DB | ||
| value: br_sta | ||
| - name: APP_ROLE | ||
| value: br_sta | ||
| command: | ||
| - /bin/sh | ||
| - -c | ||
| - | | ||
| set -euo pipefail | ||
| echo "=== br-sta PostgreSQL Bootstrap ===" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Add a securityContext to the bootstrap Job.
Neither the pod nor its wait-for-dependencies/psql containers declare a securityContext, so this Job (which carries Postgres admin credentials in env vars) runs with default (root-capable) privileges. deployment.yaml sets both podSecurityContext and container-level securityContext for the manager — this Job should follow the same hardening baseline (e.g. runAsNonRoot, allowPrivilegeEscalation: false, dropped capabilities, readOnlyRootFilesystem where the image allows it).
🔒 Example hardening
template:
spec:
restartPolicy: Never
+ securityContext:
+ runAsNonRoot: true
+ seccompProfile:
+ type: RuntimeDefault
initContainers:
- name: wait-for-dependencies
image: busybox:1.37
+ securityContext:
+ allowPrivilegeEscalation: false
+ capabilities:
+ drop: ["ALL"]🤖 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-sta/templates/bootstrap-postgres.yaml` around lines 19 - 93, Harden
the bootstrap Job template by adding pod-level and container-level
securityContext settings to the pod spec, covering both wait-for-dependencies
and psql. Match the existing security baseline from deployment.yaml, including
non-root execution, disabled privilege escalation, dropped capabilities, and
read-only root filesystems where compatible with the images.
Comment on lines
+10
to
+11
| image: | ||
| tag: "latest" # pin to a published immutable tag |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Production overlay defaults tag: "latest", contradicting the inline "pin" guidance.
This is a production starter; leaving latest here invites mutable-tag deploys. Use a placeholder that forces a decision.
🔧 Suggested change
image:
- tag: "latest" # pin to a published immutable tag
+ tag: "" # REQUIRED: pin to a published immutable tag (e.g. "1.0.0-beta.32")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| image: | |
| tag: "latest" # pin to a published immutable tag | |
| image: | |
| tag: "" # REQUIRED: pin to a published immutable tag (e.g. "1.0.0-beta.32") |
🤖 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-sta/values-template.yaml` around lines 10 - 11, Replace the
image.tag default in the production values template with a non-deployable
placeholder that requires users to select a published immutable tag, while
preserving the existing pinning guidance.
Adopt lerian-common 1.3.4 in the same model as br-ccs / midaz #1741: - MULTI_TENANT_* -> multiTenant.env (toggle via br-sta.multiTenant.enabled; URL + Redis infra from global.multiTenant). MT infra now renders only when enabled (was always emitted empty via the passthrough range) — env-var absent == empty for the app. - OTEL -> otel.env (ENABLE_TELEMETRY / OTLP endpoint / deployment-env from global.observability) + otel.envFlat (identity; SERVICE_VERSION tracks the image tag). - POSTGRES/REDIS/RABBITMQ host + primary conn fields -> datastore.value mask (configmap > br-sta.datastores > global.datastores > default); subchart DNS derivation preserved when the mask yields no host. - Productized keys excluded from the generic configmap/secret passthrough range ( set) and removed from the shipped configmap defaults so grouped / global / mask actually drive them (native key had masked them). - Fail-fast secrets: MULTI_TENANT_SERVICE_API_KEY (MT on) via multiTenant.secret; RABBITMQ_DEFAULT_PASS (rmq on, no RABBITMQ_URL). - Add render-gate fixture exercising the enabled paths. Worker inherits the manager ConfigMap/Secret via envFrom — unchanged. Default render is byte-identical except the MT-disabled infra keys (now gated, as in every other productized chart). helm lint + render-gate + strict standard pass.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
charts/br-sta/templates/configmap.yaml (1)
133-136: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExtra env vars aren't coerced to strings like the rest of the ConfigMap.
Every other entry in this ConfigMap uses
| quoteto guarantee a string value, since Kubernetes requiresConfigMap.datavalues to be strings. TheextraEnvVarspassthrough uses rawtoYamlinstead. If a user supplies an unquoted numeric or boolean value inbr-sta.extraEnvVars, the rendered manifest can fail Kubernetes API validation.🐛 Proposed fix
{{- with $component.extraEnvVars }} - {{- toYaml . | nindent 2 }} + {{- range $key, $value := . }} + {{ $key }}: {{ $value | toString | quote }} + {{- end }} {{- end }}🤖 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-sta/templates/configmap.yaml` around lines 133 - 136, Update the extraEnvVars rendering near $component.extraEnvVars in the ConfigMap template so each variable value is explicitly quoted/coerced to a string before serialization, matching the existing ConfigMap.data entries. Preserve the current passthrough structure and indentation while ensuring numeric and boolean inputs render as valid string values.
🤖 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-sta/templates/configmap.yaml`:
- Around line 95-102: Update the replica configuration block guarded by
$replicaHost to resolve port, user, and SSL mode through
lerian-common.datastore.value using the canonical fields port, user, and ssl.
Keep POSTGRES_REPLICA_NAME sourced from $cm.POSTGRES_REPLICA_NAME, defaulting to
POSTGRES_NAME, while preserving the existing replica host resolution and output
keys.
In `@charts/br-sta/values.yaml`:
- Line 57: Update the commented example entries in the values configuration,
including the lines corresponding to MULTI_TENANT_URL and the other referenced
setting, so their comments are positioned above the inline empty-map values or
aligned with the associated keys. Preserve each existing {} value as a YAML map
and resolve the comments-indentation lint errors.
---
Outside diff comments:
In `@charts/br-sta/templates/configmap.yaml`:
- Around line 133-136: Update the extraEnvVars rendering near
$component.extraEnvVars in the ConfigMap template so each variable value is
explicitly quoted/coerced to a string before serialization, matching the
existing ConfigMap.data entries. Preserve the current passthrough structure and
indentation while ensuring numeric and boolean inputs render as valid string
values.
🪄 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: b904d51c-51d6-429f-852e-4d10c8a39ee9
⛔ Files ignored due to path filters (1)
charts/br-sta/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.github/configs/helm-render-values/br-sta.yamlcharts/br-sta/Chart.yamlcharts/br-sta/templates/configmap.yamlcharts/br-sta/templates/secrets.yamlcharts/br-sta/values.yaml
…e + grouped params Replace the generic configmap passthrough range with per-key lerian-common.cfgValue resolution (configmap.<KEY> escape-hatch > br-sta.<group>.<field> > chart default) and empty br-sta.configmap (defaults now live in the template). Adds grouped param blocks (app/server/cors/multiTenant/postgres/redis/broker/outbox/sta/auth/swagger/ rateLimit/pagination/idempotency). Postgres/Redis/RabbitMQ host+conn fields still via the datastore mask; MT/OTEL via the global.* helpers; worker sub-component unchanged. No raw env vars shipped in values.yaml anymore; operators use the typed grouped API (e.g. br-sta.redis.poolSize) with configmap.<KEY> still winning as override. Default render byte-identical to the prior productized head. helm lint + render-gate + strict pass.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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-sta/templates/configmap.yaml`:
- Line 69: Update the POSTGRES_NAME mapping to use the documented grouped
PostgreSQL field by changing the cfgValue field selector from postgresName to
name, preserving the existing default and other parameters.
In `@charts/br-sta/values.yaml`:
- Line 265: Remove the duplicate br-sta.multiTenant key near the end of the
values file, retain a single mapping at the existing multiTenant definition, and
move any needed grouped fields into that mapping. Preserve the existing enabled:
false setting.
🪄 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: af3f6c2b-eeae-49e5-9e9e-5d08df6814e8
📒 Files selected for processing (2)
charts/br-sta/templates/configmap.yamlcharts/br-sta/values.yaml
…er/trust-store) The .env-coverage check surfaced 46 vars the app declares but the chart never emitted (the byte-identical rework preserved the prior chart's incompleteness). Close them: - Wire serviceDiscovery.env + global.serviceDiscovery (toggle br-sta.serviceDiscovery.enabled; SD_TOKEN is a Secret) and streaming.env + global.streaming + streaming.secret (toggle br-sta.streaming.enabled; STREAMING_SASL_PASSWORD fail-fast). - Add emitAllowInsecure to multiTenant.env; emit MULTI_TENANT_POOL_MAX_CONNS/IDLE via cfgValue. - Add ~22 config keys via cfgValue in new/existing groups: audit, transfer, truststore, masterKey (provider/version), postgres (maxConnections/sharedBuffers), broker (scheme), server (trustedProxies), app (bacenEnvironment/credentialsRecoveryOnBoot), m2m (awsAccessKeyId). - Add MASTER_KEYS / AWS_SECRET_ACCESS_KEY / SD_TOKEN / STREAMING_SASL_PASSWORD to secrets. Pure additive — no existing key changed. 46 gaps → 8 (all conditional: emit-when-set secrets, subchart-sourced passwords, optional SD advertise keys). lint + render-gate + strict pass.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
charts/br-sta/values.yaml (1)
271-271: 🎯 Functional Correctness | 🟠 MajorRemove the duplicate
br-sta.multiTenantkey.
br-sta.multiTenantis already defined at Lines 94-96. The second definition at Line 271 creates duplicate YAML keys. Helm lint can fail, and a last-key-wins parser can discardenabled: false. Keep one mapping and move thedefaultTenantIddocumentation into the first mapping.Proposed fix
multiTenant: enabled: false + # defaultTenantId: "" ... - multiTenant: {} # defaultTenantId ...🤖 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-sta/values.yaml` at line 271, Remove the duplicate multiTenant mapping near the later values section, and move its defaultTenantId documentation into the existing br-sta.multiTenant mapping defined near the top. Preserve the existing enabled setting and keep only one multiTenant YAML key.
🤖 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 @.github/configs/helm-render-values/br-sta.yaml:
- Line 15: Update the top-level br-sta configuration in this fixture to
explicitly choose the intended broker mode: set rabbitmq.enabled to true when
testing the bundled RabbitMQ subchart, or configure the external broker with
RABBITMQ_HOST and RABBITMQ_DEFAULT_USER or RABBITMQ_URL. Keep RABBITMQ_ENABLED
consistent with that selection.
In `@charts/br-sta/templates/secrets.yaml`:
- Around line 7-10: The variable $rmqEnabled is currently only reading from the
ConfigMap and does not respect the shared precedence rule. Update $rmqEnabled to
use the lerian-common.cfgValue helper function following the same pattern as
$streamingEnabled, incorporating the configmap dictionary, nativeKey
"RABBITMQ_ENABLED", the params set to $component.broker with a default empty
dictionary, the field "enabled", and a default value of "false" to properly
resolve RabbitMQ enablement.
---
Duplicate comments:
In `@charts/br-sta/values.yaml`:
- Line 271: Remove the duplicate multiTenant mapping near the later values
section, and move its defaultTenantId documentation into the existing
br-sta.multiTenant mapping defined near the top. Preserve the existing enabled
setting and keep only one multiTenant YAML key.
🪄 Autofix
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: bef664b3-0e69-4663-8a73-ab9ea05e532e
📒 Files selected for processing (4)
.github/configs/helm-render-values/br-sta.yamlcharts/br-sta/templates/configmap.yamlcharts/br-sta/templates/secrets.yamlcharts/br-sta/values.yaml
| streaming: | ||
| enabled: true | ||
| configmap: | ||
| RABBITMQ_ENABLED: "true" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 \
'condition: rabbitmq\.enabled|RABBITMQ_ENABLED|broker\.host|br-sta\.yaml' \
charts/br-sta/Chart.yaml \
charts/br-sta/values.yaml \
charts/br-sta/templates \
.githubRepository: LerianStudio/helm
Length of output: 12412
🏁 Script executed:
cat -n .github/configs/helm-render-values/br-sta.yamlRepository: LerianStudio/helm
Length of output: 1602
🏁 Script executed:
# Check the default state of rabbitmq in the main values.yaml
sed -n '558,580p' charts/br-sta/values.yamlRepository: LerianStudio/helm
Length of output: 994
Set rabbitmq.enabled: true to test the bundled RabbitMQ subchart, or explicitly configure an external broker endpoint.
Line 15 sets RABBITMQ_ENABLED: "true" in the application configmap. charts/br-sta/Chart.yaml line 53 gates the bundled RabbitMQ subchart with condition: rabbitmq.enabled. The fixture does not set rabbitmq.enabled: true at the top level, so the subchart is not included. The render gate passes because the fixture provides RABBITMQ_DEFAULT_PASS (line 24), which satisfies the template's fail-fast check in secrets.yaml line 53.
If this fixture must exercise the bundled RabbitMQ subchart, add rabbitmq.enabled: true under the top-level br-sta configuration. If the fixture must test external broker configuration, add RABBITMQ_HOST and RABBITMQ_DEFAULT_USER or RABBITMQ_URL to make the external endpoint explicit.
🤖 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 @.github/configs/helm-render-values/br-sta.yaml at line 15, Update the
top-level br-sta configuration in this fixture to explicitly choose the intended
broker mode: set rabbitmq.enabled to true when testing the bundled RabbitMQ
subchart, or configure the external broker with RABBITMQ_HOST and
RABBITMQ_DEFAULT_USER or RABBITMQ_URL. Keep RABBITMQ_ENABLED consistent with
that selection.
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 br-sta.redis.zTYPO at helm install; permissive on root/subcharts/global/open maps) and an auto-generated parameter table from the '# --' annotations.
…henate) Adopt the standard values structure: global: (shared) + <componentName>: (camelCase service name, no hyphen) + subcharts at root. Rename the component key br-sta: -> brSta: so templates use .Values.brSta instead of index .Values "br-sta" (21 sites), and value-path references (fail messages, precedence docs, valuesPrefix) follow. Helper names (br-sta.fullname/labels) and k8s identifiers (nameOverride, image repo, br_sta DB user) are unchanged. Regenerated values.schema.json (brSta.<group> now strict) + README. Breaking values API for a not-yet- released chart. lint + render + typo-rejection pass.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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-sta/README.params.md`:
- Around line 10-21: Update the parameter-table generator’s structured-value
extraction so object and array schemas retain their actual types and defaults,
including `{}` values, rather than falling back to strings or source comments.
Ensure recursive extraction includes nested fields for all grouped parameters
such as server, cors, postgres, redis, broker, probes, and resources, then
regenerate README.params.md so the table reflects the corrected schema.
In `@charts/br-sta/values.schema.json`:
- Around line 31-42: Add an explicit JSON Schema type to every property in
values.schema.json that currently defines only a default, using the type
matching its intended value (including all grouped properties through the
referenced blocks). Ensure fields such as defaultTenantId, enabled,
poolMaxConns, poolMaxIdleConns, and app.logLevel reject non-matching JSON values
while preserving their existing defaults and descriptions.
🪄 Autofix
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: 7d3ceb92-5ea7-42c7-bb12-f3b5f76d6868
📒 Files selected for processing (2)
charts/br-sta/README.params.mdcharts/br-sta/values.schema.json
Comment on lines
+10
to
+21
| | `global.externalPostgresDefinitions` | string | `{}` | Bootstrap job for external PostgreSQL: creates database and role | | ||
| | `global.externalPostgresDefinitions.enabled` | bool | `false` | Enable or disable the PostgreSQL bootstrap job | | ||
| | `global.externalPostgresDefinitions.connection` | string | `{}` | PostgreSQL connection settings (used by the bootstrap job only) | | ||
| | `global.externalPostgresDefinitions.connection.host` | string | `"br-sta-postgresql-primary"` | PostgreSQL host | | ||
| | `global.externalPostgresDefinitions.connection.port` | string | `"5432"` | PostgreSQL port | | ||
| | `global.externalPostgresDefinitions.postgresAdminLogin` | string | `{}` | Admin (superuser) credentials used to create the application DB and role | | ||
| | `global.externalPostgresDefinitions.postgresAdminLogin.useExistingSecret.name` | string | `""` | Name of existing secret containing DB_USER_ADMIN and DB_ADMIN_PASSWORD keys | | ||
| | `global.externalPostgresDefinitions.postgresAdminLogin.username` | string | `"postgres"` | Admin username (ignored if useExistingSecret.name is set) | | ||
| | `global.externalPostgresDefinitions.postgresAdminLogin.password` | string | `""` | Admin password (ignored if useExistingSecret.name is set) | | ||
| | `global.externalPostgresDefinitions.appCredentials` | string | `{}` | Credentials for the br_sta role created by the job | | ||
| | `global.externalPostgresDefinitions.appCredentials.useExistingSecret.name` | string | `""` | Name of existing secret containing DB_PASSWORD_BR_STA key | | ||
| | `global.externalPostgresDefinitions.appCredentials.password` | string | `""` | Password for br_sta role (ignored if useExistingSecret.name is set) | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Fix structured value extraction in the parameter-table generator.
The table emits incorrect types and defaults for structured values. Examples include:
global.externalPostgresDefinitionsis an object, not a string.br-sta.configmaphas{}as its default, nottemplates/configmap.yaml.br-sta.apphas{}as its default, not an inline source comment.worker.commandis an array in the schema, not a string.- Nested fields for
server,cors,postgres,redis,broker, probes, resources, and other groups are absent.
Fix the generator and regenerate this file. Values written from the current table can fail schema validation.
Also applies to: 63-80, 90-106
🤖 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-sta/README.params.md` around lines 10 - 21, Update the
parameter-table generator’s structured-value extraction so object and array
schemas retain their actual types and defaults, including `{}` values, rather
than falling back to strings or source comments. Ensure recursive extraction
includes nested fields for all grouped parameters such as server, cors,
postgres, redis, broker, probes, and resources, then regenerate README.params.md
so the table reflects the corrected schema.
Comment on lines
+31
to
+42
| "defaultTenantId": { | ||
| "default": "11111111-1111-1111-1111-111111111111" | ||
| }, | ||
| "enabled": { | ||
| "default": "false", | ||
| "description": "Enable multi-tenancy (MULTI_TENANT_ENABLED)" | ||
| }, | ||
| "poolMaxConns": { | ||
| "default": "20" | ||
| }, | ||
| "poolMaxIdleConns": { | ||
| "default": "5" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# List defaulted properties that do not define a validation type.
python - <<'PY'
import json
path = "charts/br-sta/values.schema.json"
with open(path, encoding="utf-8") as file:
schema = json.load(file)
def walk(node, path=()):
if not isinstance(node, dict):
return
if "default" in node and "type" not in node:
print(".".join(path))
for name, child in node.get("properties", {}).items():
walk(child, path + (name,))
walk(schema)
PYRepository: LerianStudio/helm
Length of output: 3412
🏁 Script executed:
# Inspect the actual schema file at the cited lines
cat -n charts/br-sta/values.schema.json | sed -n '25,50p'Repository: LerianStudio/helm
Length of output: 1151
🏁 Script executed:
# Get a count of total properties in the schema to understand scope
wc -l charts/br-sta/values.schema.jsonRepository: LerianStudio/helm
Length of output: 193
🏁 Script executed:
# Verify the schema structure: check if properties without type really accept invalid types
python3 - <<'PY'
import json
path = "charts/br-sta/values.schema.json"
with open(path, encoding="utf-8") as file:
schema = json.load(file)
# Check specific examples from lines 31-42
multiTenant = schema.get("properties", {}).get("br-sta", {}).get("properties", {}).get("multiTenant", {}).get("properties", {})
print("multiTenant.enabled:", multiTenant.get("enabled", {}))
print("multiTenant.defaultTenantId:", multiTenant.get("defaultTenantId", {}))
print()
# Verify a property without type accepts any value type
app = schema.get("properties", {}).get("br-sta", {}).get("properties", {}).get("app", {}).get("properties", {})
print("app.logLevel:", app.get("logLevel", {}))
PYRepository: LerianStudio/helm
Length of output: 375
🏁 Script executed:
# Check what lines 527-935 and 548-552 correspond to
echo "=== Lines 527-552 ==="
sed -n '527,552p' charts/br-sta/values.schema.json | head -30Repository: LerianStudio/helm
Length of output: 910
Add type constraints to properties with only default.
JSON Schema does not infer type from the default keyword. Properties without type accept any JSON value type. For example, br-sta.multiTenant.enabled and br-sta.app.logLevel currently validate as valid even when supplied as objects, arrays, numbers, or booleans.
The property block additionalProperties: false rejects unknown keys but does not enforce value types. Add "type": "string" or the applicable type for each property. This affects 127 properties across lines 31–1256, including the grouped blocks at lines 527–935.
🤖 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-sta/values.schema.json` around lines 31 - 42, Add an explicit JSON
Schema type to every property in values.schema.json that currently defines only
a default, using the type matching its intended value (including all grouped
properties through the referenced blocks). Ensure fields such as
defaultTenantId, enabled, poolMaxConns, poolMaxIdleConns, and app.logLevel
reject non-matching JSON values while preserving their existing defaults and
descriptions.
The generated schema closed operational/structural blocks (probes, autoscaling, pdb, resources, service, ingress, ...) with additionalProperties:false and type-locked their scalar leaves, rejecting valid operator input like pdb.maxUnavailable (int-or-string) or resources.limits. Strictness now applies only to the finite cfgValue config groups (key-typo protection preserved there); k8s passthrough stays open. Regenerated with the corrected generator.
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.
- postgres group: rename cfgValue fields to bare names (name/maxOpenConns/...) matching values.yaml/README docs so the grouped-param path actually wires - remove duplicate brSta.multiTenant key (YAML-lint failure; last-wins clobbered enabled) - resolve POSTGRES_REPLICA_* through datastore.value mask - fail-fast render when migrations target external Postgres with no POSTGRES_HOST - honor explicit 0 for migration Job knobs (hasKey, not default) - bootstrap-postgres + worker init: securityContext hardening; fail if worker replicaCount>1 - secrets.yaml: resolve rmqEnabled with the ConfigMap's precedence - README worker binary /worker->/service; NOTES.txt postgres host arch-aware - regenerate values.schema.json + README.params.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat(charts): add br-sta chart
New chart promoted from develop as its own PR to main (no changes to other charts).