fix(k8s-infra): add ca.pem mount - #760
Conversation
WalkthroughAdds conditional mounting of a CA certificate (ca.pem) to OpenTelemetry agent and deployment templates when .Values.otelTlsSecrets.ca is set, alongside existing cert.pem and key.pem handling. Also bumps the Helm chart version from 0.14.1 to 0.14.2. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User Values.yaml
participant H as Helm
participant T1 as otel-agent DaemonSet tpl
participant T2 as otel Deployment tpl
participant K as Kubernetes API
U->>H: Provide .Values (otelTlsSecrets.enabled, .ca, .cert, .key)
H->>T1: Render daemonset.yaml with values
alt TLS secrets enabled
alt CA provided (.Values.otelTlsSecrets.ca)
T1-->>H: items include cert.pem, key.pem, ca.pem
else No CA
T1-->>H: items include cert.pem, key.pem
end
else TLS disabled
T1-->>H: No TLS secret volume/items
end
H->>T2: Render deployment.yaml with values
alt TLS secrets enabled
alt CA provided
T2-->>H: items include cert.pem, key.pem, ca.pem
else No CA
T2-->>H: items include cert.pem, key.pem
end
else TLS disabled
T2-->>H: No TLS secret volume/items
end
H->>K: Apply rendered manifests
K-->>H: Resources created/updated
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2024-09-17T09:57:12.685ZApplied to files:
📚 Learning: 2024-10-16T19:01:15.514ZApplied to files:
🔇 Additional comments (3)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
👋 Can confirm this bug is blocking real-world usage. Reproducing the issue: otelInsecure: false
insecureSkipVerify: false
otelTlsSecrets:
enabled: true
certificate: ""
key: ""
ca: |
<CA_CERTIFICATE>Error encountered: followed by: Impact: Current workaround:
This PR would fix the issue properly. Could this be prioritized for merge? Happy to test the fix. |
Summary
Added missing
ca.pemmount to both otel-agent and otel-deploymentFixes
CA certificate support was added to values and exporter config, but
ca.pemmounts were missing, rendering it unusable.Related PR: #252
Related issue: #250
Summary by CodeRabbit