Skip to content

feat(chart): typed tls.* knob mounts mTLS Secret (#301)#411

Merged
trilamsr merged 1 commit into
mainfrom
feat/chart-tls-and-netpol-301
Jun 1, 2026
Merged

feat(chart): typed tls.* knob mounts mTLS Secret (#301)#411
trilamsr merged 1 commit into
mainfrom
feat/chart-tls-and-netpol-301

Conversation

@trilamsr

@trilamsr trilamsr commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #301 by adding a first-class tls.* chart surface so operators
wire cert-manager-issued mTLS material without a custom DaemonSet
patch overlay. The NetworkPolicy half of the issue (opt-in
networkPolicy.enabled) landed earlier in #338; this PR closes the
remaining gap.

  • tls.enabled (bool, default false), tls.certificateRef
    (kubernetes.io/tls Secret name; required when enabled — helm-template
    render fails closed with a clear error otherwise), tls.mountPath
    (absolute dir, schema-validated ^/, default /etc/tracecore/tls).
  • DaemonSet projects the Secret read-only (defaultMode: 0400); the
    chart does NOT inject tls: clauses into the rendered config —
    operators wire cert_file / key_file / ca_file /
    client_ca_file via the free-form config: block referencing the
    projected file literals.
  • docs/integrations/cert-manager-mtls.md loses the "requires a patch
    overlay" workaround and gains an aggregation-side example showing
    client_ca_file placement (the falsifier for silent
    one-way-TLS downgrade).

Root cause

Issue #301 lists tls.enabled and tls.certificateRef as required
values knobs. The chart never shipped them — the cert-manager mtls
recipe instead carried prose telling operators to "patch overlay" the
DaemonSet template, which is precisely the kind of friction the
chart-surface knob exists to eliminate. This PR fixes the root cause
(no typed knob) rather than refreshing the workaround prose.

Test plan

  • helm lint install/kubernetes/tracecore — clean.
  • helm lint install/kubernetes/tracecore -f values-production.yaml — clean.
  • helm template default render — zero tls volumes.
  • helm template --set tls.enabled=true — fails closed with
    operator-visible error naming tls.certificateRef.
  • helm template --set tls.enabled=true --set tls.certificateRef=foo
    — projects tls Secret volume + volumeMount at default
    /etc/tracecore/tls, readOnly true, mode 0400.
  • helm template --set tls.mountPath=not-absolute — schema rejects
    with Does not match pattern '^/'.
  • CI .github/workflows/chart.yml render-job has a five-step
    falsifier suite covering all of the above.
  • Pre-commit gates: make lint, make vet, go mod verify,
    attribute-namespace-check, hit-line-format-stable, and
    no-autoupdate-check all green at commit time.
- chart: typed `tls.{enabled,certificateRef,mountPath}` knob mounts a
  cert-manager-issued mTLS Secret into the DaemonSet read-only. Default
  off; required Secret reference is enforced at helm-template time so
  misconfiguration fails closed rather than silently disabling mTLS.

Closes A13 in horizon roadmap by adding a first-class `tls.*` chart
surface so operators wire cert-manager-issued client/server material
without a custom DaemonSet patch overlay.

Knobs (additive; default OFF):
  - tls.enabled         — bool, default false
  - tls.certificateRef  — kubernetes.io/tls Secret name; required when
                          enabled (helm-template render fails closed
                          with a clear error otherwise)
  - tls.mountPath       — absolute dir; schema-validated `^/`;
                          default /etc/tracecore/tls

DaemonSet projects the Secret read-only (defaultMode 0400). The chart
does NOT inject `tls:` clauses into the rendered config — operators
wire `cert_file` / `key_file` / `ca_file` / `client_ca_file` via the
free-form `config:` block referencing the projected file literals.

cert-manager mTLS recipe (docs/integrations/cert-manager-mtls.md)
loses the "requires a patch overlay" workaround and gains an
aggregation-side example showing `client_ca_file` placement.

CI gates (.github/workflows/chart.yml): five-step falsifier suite
covering default-OFF render, fail-closed without ref, mount path
projection, custom mountPath, and schema rejection of relative paths.

NetworkPolicy template + opt-in `networkPolicy.enabled` (also #301)
shipped earlier in #338 and stays unchanged here.

Signed-off-by: Tri Lam <tri@maydow.com>
@trilamsr trilamsr enabled auto-merge (squash) June 1, 2026 22:57
@trilamsr trilamsr disabled auto-merge June 1, 2026 23:02
@trilamsr

trilamsr commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Independent Review: #411 — feat(chart): typed tls.* knob mounts mTLS Secret

Verdict: SHIP — No blockers found. Fail-closed gates work correctly, test coverage is thorough, and docs include the security-critical client_ca_file falsifier.


Key Findings

  1. Fail-closed enforcement is correct: The Helm {{- fail }} guard inside {{- if .Values.tls.enabled }} stops template rendering before injecting the volumeMount and volume when tls.certificateRef is missing. Both volume and volumeMount are guarded by the same conditional, ensuring they're injected together or not at all.

  2. Schema design follows conditional-validation pattern: Only enabled is marked required in values.schema.json; certificateRef is enforced at template render-time via fail(), not schema. Correct approach for conditional fields in Helm — schema validates structure, fail() enforces runtime business logic.

  3. Secret mount mode 0400 is appropriate: Read-only for owner only; container will be configured to read it (implicit Kubernetes behavior for mounted Secrets).

  4. CI test coverage is thorough: The 5-step falsifier suite covers all paths: default (no tls), enabled-without-ref (fails with clear error), enabled-with-ref, custom mountPath, and non-absolute mountPath rejection. All tests inspect both exit code AND error message text.

  5. Doc examples are load-bearing: The cert-manager-mtls.md updates include the critical client_ca_file detail on the receiver side — the false-positive falsifier that prevents silent one-way-TLS downgrade. Both exporter and receiver examples are present and match the default mountPath.

  6. Issue A13: opt-in default-deny NetworkPolicy + cert-manager mTLS reference #301 scope closure verified: The NetworkPolicy half was completed in PR feat(v1-rc1): 2 detectors + 8 pattern specs + chart NetPol + rc1 audits #338 (confirmed in its commit message). This PR closes the mTLS half cleanly.


Non-blocking observations

  • The tls.mountPath schema regex ^/ adds defensive validation; justified trade-off to prevent operator runtime surprises when config file paths don't match the mount point.

  • If tls.enabled=false but tls.certificateRef is set, the chart ignores the ref without warning. Harmless, but documenting the "enabled is the gate" behavior in the README would clarify intent.

@trilamsr trilamsr merged commit 80e0241 into main Jun 1, 2026
19 checks passed
@trilamsr trilamsr deleted the feat/chart-tls-and-netpol-301 branch June 1, 2026 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A13: opt-in default-deny NetworkPolicy + cert-manager mTLS reference

1 participant