feat(midaz): add unified ledger config (CRM/Fees/KMS/tracer) and tracer templates - #1838
Conversation
The midaz v4 ledger binary serves onboarding, transaction, CRM and fees in one process, so it opens the CRM and Fees Mongo databases itself and fails fast at boot without MONGO_CRM_*. The chart still only configured the onboarding and transaction modules, so a v4 image could not start. Adds to the ledger ConfigMap: MONGO_CRM_*, MONGO_FEES_*, KMS_VENDOR and its Vault fields, the TRACER_* reservation seam, STREAMING_*, DEFAULT_CURRENCY, CASDOOR_JWK_ADDRESS and MULTI_TENANT_CACHE_TTL_SEC. Credentials stay out of the ConfigMap: MONGO_CRM_PASSWORD, MONGO_FEES_PASSWORD, LCRYPTO_HASH_SECRET_KEY, LCRYPTO_ENCRYPT_SECRET_KEY and KMS_VAULT_SECRET_ID render into the ledger Secret instead. The LCRYPTO keys protect CRM holder PII and the SecretID is the Vault AppRole credential, so neither belongs in a non-secret object. Claude-Session: https://claude.ai/code/session_01ABpSaU8BhcJ514v8AW8y9n
Tracer ships from the midaz monorepo (components/tracer) in v4, so it belongs in the umbrella chart rather than a separate one. Gated behind tracer.enabled, which defaults to false: existing releases render exactly as before. Secrets (DB_PASSWORD, API_KEY, multi-tenant material) render into the tracer Secret, never the ConfigMap. Claude-Session: https://claude.ai/code/session_01ABpSaU8BhcJ514v8AW8y9n
|
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 chart adds a disabled-by-default Tracer component with configuration, secrets, workloads, migrations, exposure, and validation. It also adds Ledger migration controls, Tracer database bootstrapping, CRM and fees MongoDB settings, KMS settings, and related secret wiring. ChangesTracer Helm chart
Sequence Diagram(s)sequenceDiagram
participant HelmValues
participant TracerConfigMap
participant TracerSecret
participant TracerDeployment
participant TracerService
participant TracerIngress
HelmValues->>TracerConfigMap: render Tracer configuration
HelmValues->>TracerSecret: render conditional secrets
TracerConfigMap->>TracerDeployment: provide environment values
TracerSecret->>TracerDeployment: provide secret values
TracerDeployment->>TracerService: expose HTTP port
TracerIngress->>TracerService: route configured traffic
✨ 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: 5
🤖 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/midaz/templates/_helpers.tpl`:
- Around line 172-174: Update the midaz-tracer.fullname helper to check
.Values.tracer.fullnameOverride first, then .Values.tracer.nameOverride, and
only fall back to .Values.tracer.name when neither override is set. Preserve the
existing truncation and trailing-hyphen removal for the selected name.
In `@charts/midaz/templates/ledger/secrets.yaml`:
- Around line 46-50: Add internal-Mongo secret mappings in the Ledger
Deployment’s `midaz.infraSecretRef` configuration for `MONGO_CRM_PASSWORD` and
`MONGO_FEES_PASSWORD`, using the same `mongodb-root-password` source as the
existing onboarding and transaction mappings. Keep the external-Mongo secret
generation paths unchanged.
In `@charts/midaz/templates/tracer/pdb.yaml`:
- Around line 15-18: Update charts/midaz/templates/tracer/pdb.yaml lines 15-18
to use hasKey when selecting maxUnavailable, preserving an explicitly configured
maxUnavailable: 0; otherwise render minAvailable. Update
charts/midaz/values.yaml lines 603-609 to remove maxUnavailable and set
minAvailable to 1.
In `@charts/midaz/values.yaml`:
- Around line 276-281: Update
global.externalMongoDefinitions.midazCredentials.roles to grant the midaz user
readWrite access to the fees database, matching the MONGO_FEES_USER and
MONGO_FEES_NAME defaults used by the MongoDB Fees module.
🪄 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: 4e10acb8-19ab-4fc0-a728-38d8ffa0fbc4
📒 Files selected for processing (11)
charts/midaz/templates/_helpers.tplcharts/midaz/templates/ledger/configmap.yamlcharts/midaz/templates/ledger/secrets.yamlcharts/midaz/templates/tracer/configmap.yamlcharts/midaz/templates/tracer/deployment.yamlcharts/midaz/templates/tracer/hpa.yamlcharts/midaz/templates/tracer/ingress.yamlcharts/midaz/templates/tracer/pdb.yamlcharts/midaz/templates/tracer/secrets.yamlcharts/midaz/templates/tracer/service.yamlcharts/midaz/values.yaml
- inject MONGO_CRM_PASSWORD and MONGO_FEES_PASSWORD into the ledger Deployment for both internal (mongodb-root-password secretKeyRef) and external Mongo branches - grant the midaz user readWrite on the fees database in externalMongoDefinitions bootstrap roles - fail rendering when KMS_VENDOR=hashicorp-vault and KMS_VAULT_ADDR, KMS_VAULT_ROLE_ID, KMS_VAULT_AUTH_METHOD or KMS_VAULT_SECRET_ID are missing (SecretID check skipped with ledger.useExistingSecret) - honor tracer.fullnameOverride and tracer.nameOverride in midaz-tracer.fullname - protect the single-replica tracer by defaulting the PDB to minAvailable: 1 and selecting maxUnavailable via hasKey so an explicit 0 stays valid Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
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/midaz/templates/ledger/configmap.yaml (1)
135-156: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winCheck CRM and Fees MongoDB before Ledger startup.
If either CRM or Fees uses a different host or port, the
wait-for-dependenciesloop does not probe it. Add both endpoints to the loop, or ensure Ledger retries these connections before serving traffic.🤖 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/midaz/templates/ledger/configmap.yaml` around lines 135 - 156, Update the Ledger dependency-readiness flow, using the CRM and Fees MongoDB configuration symbols (MONGO_CRM_HOST/MONGO_CRM_PORT and MONGO_FEES_HOST/MONGO_FEES_PORT), so wait-for-dependencies probes both endpoints before Ledger starts serving traffic. Preserve the existing dependency checks and ensure differing hosts or ports are retried rather than omitted.
🤖 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/midaz/templates/ledger/deployment.yaml`:
- Around line 127-128: Update the wait-for-dependencies endpoint loop in the
ledger deployment template to include both CRM and Fees MongoDB endpoint pairs
alongside onboarding and transaction endpoints. Use the existing endpoint
configuration symbols and preserve the current retry behavior so independently
configured CRM and Fees connections are checked before startup.
In `@charts/midaz/templates/tracer/pdb.yaml`:
- Around line 15-18: Update the PDB template’s minAvailable branch to check
.Values.tracer.pdb with hasKey before falling back to 1, preserving an
explicitly configured minAvailable value of 0. Keep maxUnavailable precedence
unchanged and only apply the default when minAvailable is absent.
---
Outside diff comments:
In `@charts/midaz/templates/ledger/configmap.yaml`:
- Around line 135-156: Update the Ledger dependency-readiness flow, using the
CRM and Fees MongoDB configuration symbols (MONGO_CRM_HOST/MONGO_CRM_PORT and
MONGO_FEES_HOST/MONGO_FEES_PORT), so wait-for-dependencies probes both endpoints
before Ledger starts serving traffic. Preserve the existing dependency checks
and ensure differing hosts or ports are retried rather than omitted.
🪄 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: 79574d9d-aa65-4a3d-aa9c-f6d65321b1ef
📒 Files selected for processing (6)
charts/midaz/templates/_helpers.tplcharts/midaz/templates/ledger/configmap.yamlcharts/midaz/templates/ledger/deployment.yamlcharts/midaz/templates/ledger/secrets.yamlcharts/midaz/templates/tracer/pdb.yamlcharts/midaz/values.yaml
…cit tracer PDB minAvailable: 0 Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
gandalf-at-lerian
left a comment
gandalf-at-lerian
left a comment
There was a problem hiding this comment.
Requesting changes: the chart still cannot run the Midaz v4 artifacts it claims to support. I verified the rendered manifests against the v4.0.0-beta.24 runtime contract: the bundled PostgreSQL path cannot authenticate/provision Tracer, neither Ledger nor Tracer gets its required migration runner, and the Tracer defaults target the old standalone artifact. The auth and memory defaults add further boot/OOM failure modes.
The strict chart validator, render gate, helm lint, and helm template all pass; the gap is that they validate YAML shape, not the v4 application bootstrap contract. I also confirmed CodeRabbit's unresolved LCRYPTO finding: KMS_VENDOR=none renders with both keys empty, then Ledger's legacy cipher initialization fails with an AES key-size error.
| revisionHistoryLimit: 10 | ||
| image: | ||
| # -- Repository for the Tracer service container image | ||
| repository: lerianstudio/tracer |
There was a problem hiding this comment.
[P1] Use the Midaz v4 Tracer artifact here. This PR says Tracer now ships from midaz/components/tracer, but the default renders lerianstudio/tracer:1.0.0; that image's OCI metadata points to the separate LerianStudio/tracer repository. The v4.0.0-beta.24 pipeline publishes lerianstudio/midaz-tracer. As written, tracer.enabled=true runs the old standalone binary rather than the configuration contract introduced by this PR.
| DB_HOST: "midaz-postgresql" | ||
| DB_PORT: "5432" | ||
| DB_NAME: "tracer" | ||
| DB_USER: "tracer" |
There was a problem hiding this comment.
[P1] Provision a database/user that matches the rendered credentials. With bundled PostgreSQL, this renders DB_USER=tracer/DB_NAME=tracer, while the Deployment reads key password from the subchart Secret, which belongs to postgresql.auth.username=midaz. files/midaz/init.sql creates only onboarding and transaction, with no tracer database or role. The default Tracer pod therefore cannot connect. Either use the midaz role and create/grant the tracer DB in both internal/external bootstrap paths, or provision a dedicated tracer role and Secret consistently.
| DB_NAME: {{ .Values.tracer.configmap.DB_NAME | default "tracer" | quote }} | ||
| DB_USER: {{ .Values.tracer.configmap.DB_USER | default "tracer" | quote }} | ||
| DB_SSL_MODE: {{ .Values.tracer.configmap.DB_SSL_MODE | default "disable" | quote }} | ||
| MIGRATIONS_PATH: {{ .Values.tracer.configmap.MIGRATIONS_PATH | default "./migrations" | quote }} |
There was a problem hiding this comment.
[P1] Render the dedicated migration runner instead of passing MIGRATIONS_PATH to the app. Midaz v4 explicitly no longer consumes this variable: the service boots against an already-migrated schema, and the release publishes midaz-tracer-migrations. This chart renders no migration Job, so even a pre-created Tracer database remains schema-empty. Add the migrations image values and a pre-sync/pre-install Job before the Deployment. The same lifecycle gap needs to be closed for the v4 Ledger migration image.
| # -- CPU and memory limits for pods | ||
| limits: | ||
| cpu: 500m | ||
| memory: 512Mi |
There was a problem hiding this comment.
[P1] Align GOMEMLIMIT with this cgroup limit. Both the standalone 1.0.0 image and the Midaz v4 Dockerfile set GOMEMLIMIT=1800MiB; the v4 Dockerfile explicitly says Helm must override it to about 90% of the container memory limit. This chart caps memory at 512Mi and injects no override, so the Go runtime is allowed to retain far more heap than the cgroup budget before the kernel OOM-kills the pod. Set roughly 460MiB here via env, or raise the memory limit to the 2GiB assumed by the image.
| LOG_LEVEL: {{ .Values.tracer.configmap.LOG_LEVEL | default "info" | quote }} | ||
|
|
||
| # AUTHENTICATION | ||
| API_KEY_ENABLED: {{ .Values.tracer.configmap.API_KEY_ENABLED | default "false" | quote }} |
There was a problem hiding this comment.
[P1] Mirror the v4 API-key validation at render time. ValidateAuthConfig rejects API_KEY_ENABLED=true when API_KEY is missing and also rejects it with CORS_ALLOWED_ORIGINS="*". This chart accepts both, and wildcard CORS is the default; I reproduced a successful Helm render with API-key auth enabled that the v4 process deterministically rejects at boot. Require the inline key (or an existing Secret) and fail/override wildcard CORS when API-key auth is enabled.
|
|
||
| # MULTI-TENANT | ||
| MULTI_TENANT_ENABLED: {{ .Values.tracer.configmap.MULTI_TENANT_ENABLED | default "false" | quote }} | ||
| {{- if eq (.Values.tracer.configmap.MULTI_TENANT_ENABLED | default "false" | toString) "true" }} |
There was a problem hiding this comment.
[P1] Complete the multi-tenant fail-fast validation. The v4 bootstrap requires PLUGIN_AUTH_ENABLED=true whenever MULTI_TENANT_ENABLED=true because API-key-only mode cannot verify tenant JWT signatures. The chart currently validates URL, Redis host, and service key but renders successfully with plugin auth disabled, after which the process exits during bootstrap. Enforce the plugin-auth invariant here as well (and reject API_KEY_ENABLED_ONLY_VALIDATION=true if exposed through extraEnvVars).
| name: {{ include "midaz-tracer.fullname" . }} | ||
| {{- else }} | ||
| - secretRef: | ||
| name: {{ .Values.tracer.existingSecretName }} |
There was a problem hiding this comment.
[P1] Require tracer.existingSecretName when tracer.useExistingSecret=true. This branch currently renders envFrom.secretRef.name: ""; helm template and helm lint accept it, but I reproduced a Kubernetes server-side dry-run rejection: spec.template.spec.containers[0].envFrom[0].secretRef.name: Required value. Fail the Helm render with required (and apply the same guard anywhere $secretName is reused) instead of producing an invalid Deployment.
|
@BlackSmith -- comments do gandalf. |
|
Got it, I'm reviewing the autofix changes on PR 1838. |
| # TRACER_BASE_URL empty disables the integration (nil reserver). | ||
| # ============================================================================= | ||
| TRACER_BASE_URL: {{ .Values.ledger.configmap.TRACER_BASE_URL | default "" | quote }} | ||
| TRACER_TRANSPORT: {{ .Values.ledger.configmap.TRACER_TRANSPORT | default "grpc" | quote }} |
There was a problem hiding this comment.
[P1] Do not default this integration to a transport that the bundled Tracer does not expose. With tracer.enabled=true and TRACER_BASE_URL set, the rendered Ledger selects grpc, but the Tracer ConfigMap does not set TRACER_GRPC_PORT (v4 treats empty as gRPC disabled) and its Service exposes only HTTP 4020. The in-chart Ledger→Tracer path therefore cannot connect with the documented defaults. Either configure/expose the Tracer gRPC port end-to-end, or default this seam to rest while only the HTTP port is available.
…edger Points the tracer at the v4 artifact, provisions its database, adds the dedicated migration-runner Jobs, aligns GOMEMLIMIT with the cgroup limit, and mirrors the tracer boot validators at render time. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
All seven of gandalf's findings are addressed in
Default renders stay at 49 objects and byte-identical to One finding of my own, pre-existing on |
![View with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg)
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/midaz/files/midaz/init.sql`:
- Around line 13-18: Update the bundled PostgreSQL initialization around CREATE
DATABASE so it stays consistent with tracer.configmap.DB_NAME: render the
configured database name instead of hardcoding tracer, or validate and reject
any non-tracer value when bundled PostgreSQL is enabled. Preserve unconditional
initialization behavior for the tracer database.
In `@charts/midaz/README.md`:
- Line 303: Update the Migrations documentation to state that the tracer
migration Job renders only when both tracer.enabled and
tracer.migrations.enabled are true, replacing the claim that it is always on
with tracer. Explicitly note that disabling tracer.migrations.enabled requires
applying the schema migration out of band.
In `@charts/midaz/templates/_helpers.tpl`:
- Around line 274-278: Update the midaz.tagIsV4 helper’s regexMatch validation
to require a complete valid SemVer string, including optional pre-release and
build metadata, before calling semverCompare. Preserve the existing v-prefix
trimming and >=4.0.0 comparison while rejecting extra numeric segments such as
4.0.0.1.
In `@charts/midaz/templates/ledger/migrations-job.yaml`:
- Around line 24-37: Ensure both migration Jobs block their corresponding
Deployments until completion: update
charts/midaz/templates/ledger/migrations-job.yaml lines 24-37 and
charts/midaz/templates/tracer/migrations-job.yaml lines 23-36 with a completion
gate or deployment-blocking hook, preserving the existing Job settings and
applying the gate when migrations are enabled.
🪄 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: ef410c54-d885-438f-a3b6-6420dc8a843d
📒 Files selected for processing (11)
charts/midaz/README.mdcharts/midaz/files/midaz/init.sqlcharts/midaz/templates/_helpers.tplcharts/midaz/templates/bootstrap-postgres.yamlcharts/midaz/templates/ledger/migrations-job.yamlcharts/midaz/templates/ledger/secrets.yamlcharts/midaz/templates/tracer/configmap.yamlcharts/midaz/templates/tracer/deployment.yamlcharts/midaz/templates/tracer/migrations-job.yamlcharts/midaz/templates/tracer/secrets.yamlcharts/midaz/values.yaml
…tighten semver guard Address review findings on the unified ledger and tracer work: - midaz.tagIsV4 now requires a complete SemVer string (optional pre-release and build metadata) before calling semverCompare, so a malformed tag such as 4.0.0.1 resolves to "not v4" instead of aborting the render. - The bundled init.sql creates the tracer database from tracer.configmap.DB_NAME (rendered through tpl) so the internal cluster, the external bootstrap Job, the tracer Deployment and its migration Job all agree on the name. - While a migration Job renders, its Deployment carries argocd.argoproj.io/sync-wave: "1" (<component>.migrations.deploymentSyncWave, "" opts out) so Argo CD applies the service only once the Job is Complete. The Jobs stay in the default wave because the bundled PostgreSQL Secret they read is created during Sync. - README documents the tracer Job's tracer.migrations.enabled condition and the out-of-band migration requirement when it is disabled. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
charts/midaz/files/midaz/init.sql (1)
13-20: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftCreate the Tracer database on upgrades.
init.sqlruns only during the first PostgreSQL initialization. Existing releases have already passed this phase before this script is added. If an operator upgrades such a release and enables Tracer, the migration Job and Deployment target a database that does not exist.Add an idempotent bootstrap Job for bundled PostgreSQL upgrades. Create and grant the configured Tracer database in that Job. Do not rely only on
init.sql.🤖 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/midaz/files/midaz/init.sql` around lines 13 - 20, Add an idempotent bootstrap Job for bundled PostgreSQL upgrades that creates the configured Tracer database and grants the required permissions, reusing the database name from tracer.configmap.DB_NAME and the existing PostgreSQL credentials/configuration. Ensure the Job runs when Tracer is enabled, supports existing releases where init.sql has already executed, and does not replace the unconditional init.sql creation path.charts/midaz/README.md (1)
6-6: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDocument the full LCRYPTO key requirement.
The README limits the requirement to
KMS_VENDOR=none. Incharts/midaz/templates/ledger/secrets.yaml, Line [57-75], v4 images require both LCRYPTO keys wheneverKMS_VENDORis nothashicorp-vault.Update this sentence to include every non-
hashicorp-vaultvalue, including the defaultnone.Proposed wording
- when KMS_VENDOR=none + when KMS_VENDOR is not hashicorp-vault, including the default none🤖 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/midaz/README.md` at line 6, Update the required-secrets sentence in the README to state that 4.x ledger images require both LCRYPTO keys whenever KMS_VENDOR is not hashicorp-vault, covering the default none and all other non-vault values; preserve the existing CRM and external-backend credential guidance.
🤖 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/midaz/README.md`:
- Line 302: Update the manual SQL guidance in the README paragraph to create the
database using the configured tracer.configmap.DB_NAME value, rather than
hardcoding tracer, while preserving the existing context for pre-existing
internal clusters.
---
Outside diff comments:
In `@charts/midaz/files/midaz/init.sql`:
- Around line 13-20: Add an idempotent bootstrap Job for bundled PostgreSQL
upgrades that creates the configured Tracer database and grants the required
permissions, reusing the database name from tracer.configmap.DB_NAME and the
existing PostgreSQL credentials/configuration. Ensure the Job runs when Tracer
is enabled, supports existing releases where init.sql has already executed, and
does not replace the unconditional init.sql creation path.
In `@charts/midaz/README.md`:
- Line 6: Update the required-secrets sentence in the README to state that 4.x
ledger images require both LCRYPTO keys whenever KMS_VENDOR is not
hashicorp-vault, covering the default none and all other non-vault values;
preserve the existing CRM and external-backend credential guidance.
🪄 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: dc2b64ea-5e06-4991-a45e-e35aef669a55
📒 Files selected for processing (7)
charts/midaz/README.mdcharts/midaz/files/midaz/init.sqlcharts/midaz/templates/_helpers.tplcharts/midaz/templates/configmap-postgres-midaz.yamlcharts/midaz/templates/ledger/deployment.yamlcharts/midaz/templates/tracer/deployment.yamlcharts/midaz/values.yaml
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
gandalf-at-lerian
left a comment
gandalf-at-lerian
left a comment
There was a problem hiding this comment.
Second pass on cd62b43. The database provisioning, migration-runner resources, GOMEMLIMIT, and empty existingSecretName fixes move this forward, but the chart is still not deployable across its advertised matrix. Five additional blockers are inline. The previously reported Ledger gRPC default vs Tracer HTTP-only Service mismatch also remains unresolved.
| # external bootstrap Job, and is owned by the same `midaz` role whose | ||
| # password the subchart Secret holds. A dedicated `tracer` role would need | ||
| # its own Secret, which nothing in this chart provisions. | ||
| DB_HOST: "midaz-postgresql-primary" |
There was a problem hiding this comment.
[P1] Derive the bundled PostgreSQL hostname from the Helm release instead of hard-coding midaz. Rendering this chart as release review creates the Service review-postgresql-primary, while both the Tracer ConfigMap and migration Job use midaz-postgresql-primary; neither pod can resolve its database. This also contradicts the PR's alternate-release validation claim. Use the dependency fullname helper plus the -primary suffix for the internal topology, while preserving an explicit DB_HOST for external PostgreSQL.
| */}} | ||
| {{- define "midaz-tracer.migrationsFullname" -}} | ||
| {{- $tag := include "midaz-tracer.migrationsTag" . -}} | ||
| {{- printf "%s-migrations-%s" (include "midaz-tracer.fullname" .) (regexReplaceAll "[^a-z0-9.]+" (lower $tag) "-") | trunc 63 | trimSuffix "-" | trimSuffix "." -}} |
There was a problem hiding this comment.
[P1] Preserve the tag discriminator after truncation. With a 55-character fullnameOverride, both 4.0.0-beta.24 and 4.0.0-beta.25 render the identical Job name aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-migrati; the 63-character truncation removes the entire tag. The next upgrade therefore hits the immutable Job spec problem this helper is intended to solve. Truncate the base name before appending a fixed-length tag hash/suffix. The Ledger helper below has the same collision.
| {{- if and $tracer.useExistingSecret (not $tracer.existingSecretName) -}} | ||
| {{- fail "tracer.useExistingSecret=true requires tracer.existingSecretName (an empty secretRef.name is rejected by the API server)" -}} | ||
| {{- end -}} | ||
| {{- if eq ($cm.API_KEY_ENABLED | default "false" | toString) "true" -}} |
There was a problem hiding this comment.
[P1] Mirror the runtime's boolean parser instead of comparing only the lowercase string true. Midaz loads these fields through strconv.ParseBool, so TRUE, True, 1, t, and T are all enabled at runtime. API_KEY_ENABLED=TRUE currently renders successfully with no API key and wildcard CORS, then fails at boot; conversely, valid MULTI_TENANT_ENABLED=true plus PLUGIN_AUTH_ENABLED=TRUE is rejected by Helm. Normalize using exactly the tokens accepted by strconv.ParseBool and reuse that helper for every boolean validation here and in tracer/configmap.yaml.
| and never read them, so the requirement is gated on the image tag. Kept as a | ||
| template comment so the rendered Secret (and its checksum) is unchanged for | ||
| 3.x releases. */ -}} | ||
| {{- $requireLcrypto := and (include "midaz.tagIsV4" (.Values.ledger.image.tag | default .Chart.AppVersion)) (ne (.Values.ledger.configmap.KMS_VENDOR | default "none") "hashicorp-vault") }} |
There was a problem hiding this comment.
[P1] Validate the encryption key contract, not just presence. LCRYPTO_ENCRYPT_SECRET_KEY is hex-decoded and passed to AES; it must be a hex string encoding 16, 24, or 32 bytes (32, 48, or 64 hex characters). The chart accepts x or 0011, but the v4 bootstrap fails respectively with encoding/hex: invalid byte and crypto/aes: invalid key size 2. When the key comes from values rather than an existing Secret, reject non-hex values and invalid lengths during render.
| enabled: | ||
| image: | ||
| # -- Repository for the ledger migration-runner image | ||
| repository: lerianstudio/midaz-ledger-migrations |
There was a problem hiding this comment.
[P1] Point the defaults at artifacts a default installation can pull. Anonymous manifest resolution for lerianstudio/midaz-ledger-migrations:4.0.0-beta.24, lerianstudio/midaz-tracer:4.0.0-beta.24, and lerianstudio/midaz-tracer-migrations:4.0.0-beta.24 currently returns denied/unauthorized, while both components default imagePullSecrets to []. Because this Job auto-enables for a v4 Ledger tag, the advertised upgrade stalls at ImagePullBackOff before migrations run. Make the packages public, use the public artifact coordinates, or make registry credentials an explicit validated prerequisite.
|
@BlackSmith -- comments do gandalf. faca a triagem e fix. |
…arden validation Defaults the ledger-to-tracer seam to the transport the bundled tracer actually exposes (with opt-in gRPC wiring), derives the tracer PostgreSQL host from the release name, keeps migration Job names unique past the 63 character limit, matches booleans the way strconv.ParseBool does, validates the LCRYPTO AES key format, and warns when the private v4 images cannot be pulled. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Triagem das seis observações novas do gandalf (a de 06:06 mais as cinco de 07:02): todas verificadas contra o código v4 e todas procedentes. Corrigidas em Transporte ledger→tracer ( Host do PostgreSQL por release ( Colisão de nome de Job ( Parser booleano ( Contrato da chave AES ( Imagens não públicas ( Validação: Continua de fora, e re-sinalizo: |
|
@BlackSmith -- lembrando que todas as imagens desse chart precisam ser publicas |
…t workaround Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Feito. Estado atual do PR #1838 (branch O que já foi feito1ª rodada de review (
Autofixes ( 2ª rodada ( Último commit ( Auditoria de imagensRodei manifest anônimo no Docker Hub em tudo que um render completo pode produzir:
Não existe substituto público: Validações passando: Falta só: publicar os três pacotes no Docker Hub — fora do escopo deste chart. |
|
@BlackSmith -- boa, precisamos abrir um PR no midaz corrigindo essas imagens para serem |
PR aberto no midaz com o seu usuário: LerianStudio/midaz#2303 Causa raiz: o Docker Hub cria o repositório no primeiro push usando a visibilidade padrão da organização (privada), e o O que o PR faz:
O workflow também é Um ponto para confirmar antes do merge: o Validei com |
@BlackSmith -- dei merge |
O merge quebrou o release em O login e as leituras funcionaram (ele viu corretamente Abri o follow-up (com o seu usuário): LerianStudio/midaz#2304 Ele torna o gate um O que só você (ou alguém owner na org) pode fazer — nenhum credential no CI consegue hoje:
Estado atual das imagens: |
|
@guimoreirar -- aqui e no repo do midaz. |
PR de follow-up no midaz: LerianStudio/midaz#2304 Ele destrava o release em @guimoreirar: o que falta e não dá para fazer por CI é criar um organization access token do Docker Hub com escopo |
|
@BlackSmith / @guimoreirar -- ja dei merge no PR do midaz. pendente virar as imagens pra public. |
Merge confirmado e o release em As três imagens continuam 401 no pull anônimo: @guimoreirar, para fechar isso são três passos, e nenhum deles pode ser feito pelo CI com o token atual:
Enquanto o passo 1 não acontecer, o install default do chart no helm#1838 continua sem funcionar de forma anônima. |
|
@CodeRabbit -- approve |
|
✅ Action performedComments resolved and changes approved. |
The unified-ledger (#1838) config requires the midaz v4 images, but the chart still defaulted ledger to 3.8.0 (a 3.x image that lacks the collapsed CRM/Fees/tracer contract). Bump ledger and tracer image tags to 4.0.0-beta.26 (matches benedita dev-st, which runs the v4 unified ledger). Migration images track the app tag (empty), so they follow automatically. This activates the productized v4 crypto guard: a 4.x ledger with KMS_VENDOR=none now requires hex-encoded LCRYPTO_HASH/ENCRYPT_SECRET_KEY (the CRM cipher fails to boot without them) — benedita provides these via Vault. Updated the render-gate fixture to supply valid hex sample keys under ledger.secrets so the gate exercises the v4 path.
2 participants
Problem
The midaz v4 ledger binary is unified: one process serves onboarding,
transaction, CRM and fees. It opens the CRM and Fees Mongo databases itself
and fails fast at boot without
MONGO_CRM_*:The chart only configured onboarding and transaction, so no v4 image can
start under it. Tracer has the same gap: it ships from the midaz monorepo
(
components/tracer) in v4 but had no place in the umbrella chart.What this does
Ledger ConfigMap gains the config the unified binary reads:
MONGO_CRM_*,MONGO_FEES_*,KMS_VENDORand its Vault fields, theTRACER_*reservation seam,STREAMING_*,DEFAULT_CURRENCY,CASDOOR_JWK_ADDRESS,MULTI_TENANT_CACHE_TTL_SEC.Tracer templates join the umbrella chart, gated behind
tracer.enabled(default
false).The v4 runtime contract is now actually satisfied, not just the YAML shape:
lerianstudio/midaz-tracer(the artifact the v4 pipelinepublishes and whose tag the gitops automation bumps through
.tracer.image.tag), not the standalonelerianstudio/tracer1.x image.midazrole that owns the otherdatabases, and its
tracerdatabase is created byinit.sql(internal) andby the external bootstrap Job.
ledger.migrationsandtracer.migrationsrender the dedicatedmidaz-*-migrationsrunner Jobs.The ledger Job is version-gated: on for 4.x tags, off for 3.x, which still
self-migrates.
MIGRATIONS_PATHis gone; v4 tracer never reads it.GOMEMLIMITis pinned to ~90% ofresources.limits.memory; the image ships1800MiB, sized for a 2Gi container, which would OOM-kill a 512Mi pod.Same for
API_KEY_ENABLED=truewithout a key or with wildcard CORS,API_KEY_ENABLED_ONLY_VALIDATION=trueunder multi-tenant,useExistingSecretwithout a name, and the 4.x ledger LCRYPTO pair underKMS_VENDOR=none(the CRM cipher fails with an AES key-size error otherwise).Security — what changed vs #1738
#1738 was closed with review findings that were correct: it rendered
credentials into the ConfigMap. This PR keeps them in the Secret:
MONGO_CRM_PASSWORD,MONGO_FEES_PASSWORDLCRYPTO_HASH_SECRET_KEY,LCRYPTO_ENCRYPT_SECRET_KEYKMS_VAULT_SECRET_IDKMS_VAULT_ROLE_ID,KMS_VAULT_ADDR,KMS_VAULT_AUTH_METHODDB_PASSWORD,API_KEY, multi-tenant materialThe LCRYPTO keys protect CRM holder PII and the SecretID is the Vault
AppRole credential, so neither belongs in an object readable by anyone with
ConfigMap access.
Backward compatibility
No breaking change for 3.x releases. A default render still produces 49
objects and is byte-identical to
developapart from one added line ininit.sql(CREATE DATABASE tracer;, which only runs on a fresh cluster).Every new requirement is gated on
tracer.enabledor a 4.x image tag, so anexisting
helm upgradesees no new failure and no pod restart.Validation
helm lintclean; chart-standard validator and render gate passhelm templateacross the matrix: defaults,tracer.enabled=true, 4.xledger tag, non-semver tag, external PostgreSQL, alternate release name,
and each fail-fast path asserted to fail with its intended message
kubeconform -stricton the rendered outputdevelopto confirm nothing is droppedSecond-pass fixes
TRACER_TRANSPORTnow defaults torest, theonly transport the bundled tracer exposes. gRPC is opt-in end to end: setting
tracer.configmap.TRACER_GRPC_PORTstarts the seam and publishestracer.service.grpcPorton the container and Service, and selectinggrpcwithout it fails the render.
tracer.configmap.DB_HOSTdefaults to empty andresolves to this release's PostgreSQL primary Service, so a release named
reviewno longer points the tracer and its migration Job atmidaz-postgresql-primary.<name>-migrations-<tag>form while it fits in 63 characters and fall back toa truncated base plus a tag hash beyond it, so a long
fullnameOverridecan nolonger make two tags share one immutable Job.
strconv.ParseBool, soTRUE,True,1,tandTenable a feature atrender time exactly as they do at runtime.
LCRYPTO_ENCRYPT_SECRET_KEYis checked for hex and for a16/24/32-byte length when a 4.x ledger would read it, instead of only presence.
imagePullSecretsare required. Three v4 packages do not satisfy that yet(see below); the chart keeps the pipeline coordinates rather than designing
around private registries.
Notes, out of scope
crm/configmap.yamldefaultsKMS_VENDORtohashicorp-vaultpointing atmidaz-hc-vault. Where that subchart is not deployed, the CRM readinessprobe never passes (its KMS health check times out) and the pod stays
0/1 Runningforever.ledger/configmap.yamlemitsSWAGGER_VERSIONtwice (lines 36 and 352), sothe rendered ConfigMap has a duplicate key and
kubeconform -strictrejectsit. Pre-existing on
develop; the last occurrence wins today, so removingline 36 would be behaviour-preserving.
Every image a full render of this chart can produce is anonymously pullable
except three, which must be made public on Docker Hub before a v4 install works
without credentials:
lerianstudio/midaz-ledger:4.0.0-beta.24lerianstudio/midaz-crm,postgres:16,busybox:1.37, bundled Bitnami/RabbitMQ imageslerianstudio/midaz-tracer:4.0.0-beta.24denied/unauthorizedlerianstudio/midaz-tracer-migrations:4.0.0-beta.24denied/unauthorizedlerianstudio/midaz-ledger-migrations:4.0.0-beta.24denied/unauthorizedThere is no public substitute:
lerianstudio/tracer2.x is the standalonepre-v4 artifact, and its one v4-tagged image (
4.0.0-beta.11) is actually themigrations runner (
ENTRYPOINT /migrate-entrypoint.sh) pushed to thatrepository by mistake. Publishing the three packages is a midaz-side change.