feat(k8s-reporter): add extraVolumes, extraVolumeMounts, extraEnvVars, customCA#777
Conversation
…, customCA Adds two layers of support for customers running the k8s-reporter behind a TLS-inspecting proxy that requires a corporate CA bundle: 1. Generic escape hatch — extraVolumes, extraVolumeMounts, extraEnvVars following Bitnami / prometheus-community naming conventions. 2. customCA convenience wrapper — single config block that mounts the CA via subPath into /etc/ssl/certs/ so Go's stdlib picks it up additively alongside the system bundle (avoids the SSL_CERT_FILE footgun that would otherwise replace the system CAs). The Kosli CLI is Go and uses net/http's default transport; no CLI changes are needed because crypto/x509 reads /etc/ssl/certs/ automatically. Refs #776 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Claude finished @AlexKantor87's task in 3m 16s —— View job PR Review: feat(k8s-reporter): add extraVolumes, extraVolumeMounts, extraEnvVars, customCA
Static AnalysisAll passing — no issues:
(Could not run Review SummaryThis is a well-structured, additive-only Helm chart change. The previous two review rounds caught real issues ( What looks good
Minor observations (non-blocking)
VerdictLGTM — clean, well-documented, and the previous review feedback has been thoroughly addressed. The only remaining nit is the pre-existing whitespace issue in the |
…recation Addresses parallel review feedback on PR #777: - Add explicit defaultMode: 0644 to customCA Secret volume so the permission posture is self-documenting for auditors. - Tighten the docs description of Go's cert-pool loading: it's two independent passes (cert file + dir scan), not one unified scan. - Add the literal "x509: certificate signed by unknown authority" string to the docs section so customers searching the error message find it. - Replace the SSL_CERT_FILE example in the extraEnvVars block with HTTPS_PROXY — SSL_CERT_FILE is the documented footgun and shouldn't be the first thing copy-pasters see. - Shorten the customCA parent comment so the helm-docs values table reads cleanly; defer detail to the README section. - Mark the existing `env:` map as DEPRECATED in favour of extraEnvVars to remove cross-reference confusion. Both still work. Refs #776 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Backing out the env-map deprecation from the previous commit — that was
scope creep beyond the customCA feature, made without auditing who
relies on env: today or whether the team wants to phase it out.
Instead: surface env: in the helm-docs values table (it was previously
invisible because the values.yaml entry was fully commented out) and
describe env: vs extraEnvVars as sibling options. Behaviour unchanged
— env: {} is functionally identical to the previous fully-commented form.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add `required` guard on customCA.key to prevent the silent
empty-subPath footgun (an empty subPath causes K8s to mount the
whole Secret as a directory at the mountPath, which would break Go
trying to read it as a cert file).
- Tighten the env range loop with `{{- end }}` so the rendered env
block doesn't carry a trailing blank line before extraEnvVars.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reverting the editorial framing in the env: comment. The original comment was a directive for single-tenant Kosli instances, not an example. Also dropping the extraEnvVars cross-reference — that was scope creep added without justification. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Picking up safe-to-fix nits regardless of strict scope:
- Quote customCA.secretName and subPath, matching the pattern used for
cronSchedule and image fields. mbevc1 also asked for this.
- Quote $value in the env range so user-supplied values containing
YAML special chars (colons, brackets) render safely.
- Tighten the podLabels range with `{{- end }}` for consistency with
the env-range fix in commit b44df21. Pre-existing whitespace nit,
not introduced by this PR, but cheap to fix while I'm here.
- Collapse the env: helm-docs comment to a single line so it reads
cleanly in the rendered values table (the multi-line YAML example
collapsed badly after helm-docs concatenation).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Same fix as the env value quoting in f2af5bf, applied to podLabels for consistency. Also fixes the double-space typo (`:= .Values.`) in both range loops — harmless cosmetic carry-over from the original template. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Do we need to pull docs changes through to mintlify? |
|
There is a scheduled run that should pick this up |
|
@tooky ignore previous comment, it needed manual prompt. Automation is there only for backed and CLI + TF provider |
Closes #776.
Summary
Adds two layers of support for customers running the k8s-reporter behind a TLS-inspecting proxy that requires a corporate CA bundle:
extraVolumes,extraVolumeMounts,extraEnvVars(Bitnami / prometheus-community naming convention).customCAconvenience wrapper — single config block that mounts the CA viasubPathinto/etc/ssl/certs/so Go's stdlib picks it up additively alongside the system bundle. Deliberately does not setSSL_CERT_FILE— that would replace the system bundle and break trust for any public CAs the customer's bundle does not include.No changes to the Kosli CLI itself: it's Go and uses
net/http's default transport, socrypto/x509reads/etc/ssl/certs/automatically.Files changed
charts/k8s-reporter/values.yaml— new top-level fields with documented examplescharts/k8s-reporter/templates/cronjob.yaml— three injection points (volumes, volumeMounts, env)charts/k8s-reporter/Chart.yaml— bump2.1.0→2.2.0(additive, minor)charts/k8s-reporter/_templates.gotmpl— new "Running behind a TLS-inspecting proxy" docs sectioncharts/k8s-reporter/README.md.gotmpl— wire the new section into the READMEcharts/k8s-reporter/README.md+docs.kosli.com/content/helm/_index.md— regenerated viamake helm-docsVerification
helm lintandhelm templatevalidated against five scenarios:main— no behaviour change for existing userscustomCAonlycustomCA.enabled=truewith nosecretNamecustomCA.secretName is required when customCA.enabled is truemake helm-lintpasses (strict mode too).Test plan
helm templatelocally withcustomCA.enabled=trueand confirms the mount appears at/etc/ssl/certs/kosli-custom-ca.crtwithsubPath: ca.crthelm templatewith the genericextraVolumesexample fromvalues.yamland confirms it appears alongside the chart's ownenvironments-configvolumemake helm-lintpassesmake helm-docsproduces no diff (already regenerated in this PR)Deliberately deferred
tests/directory, CI only runshelm lint). Adding a snapshot/golden-file harness is a meaningful new pattern for this repo and belongs as its own thin slice rather than bundled here. Suggest a follow-up issue if desired.customCA.mountPath/ volume-name override. The wrapper hardcodes/etc/ssl/certs/kosli-custom-ca.crtand volume namecustom-cato keep the UX one-decision-deep. Customers who need different conventions should use the generic fields.Customer context
Reported by a customer whose environment sits behind a TLS-inspecting appliance that re-signs HTTPS traffic with a corporate CA. They confirmed that the dedicated
customCAwrapper would materially help their deployment flow, which is why it ships in the same PR as the generic fields rather than as a follow-up.