Improve TLS handling in etcd helm chart #620
Conversation
dosubot
bot
added
the
size:M
WalkthroughThis pull request updates the etcd chart template and related configuration files. The Changes
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/extra/etcd/values.schema.json (1)
19-40: Accurate JSON Schema Enhancements for TLS Configurability
The schema now includes "caDuration" and a "subject" object with detailed sub-properties, which aligns well with the new templatable certificate configuration. As a potential refinement, consider specifying the array item types (e.g., adding "items": {"type": "string"} for properties like "organizations" and "countries") to enforce that only string values are provided. This refinement isn’t critical but could further improve schema validation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/extra/etcd/templates/etcd-cluster.yaml(7 hunks)packages/extra/etcd/values.schema.json(1 hunks)packages/extra/etcd/values.yaml(1 hunks)
🔇 Additional comments (7)
packages/extra/etcd/values.yaml (2)
10-30: Thorough Documentation for the New X509 Subject Parameters
The added block documents the new "subject" object very clearly with separate entries for organizations, organizational units, countries, localities, provinces, postal codes, street addresses, and serial number. The inline comments (and the use of commented-out fields) appropriately guide users on how they may customize these values if needed.
31-32: Clear CA Certificate Duration Setting
Defining "caDuration" as 87600h (representing a 10-year period) is consistent with the PR objectives. This provides a good default while still leaving room for user customization if necessary.packages/extra/etcd/templates/etcd-cluster.yaml (5)
75-77: Dynamic Configuration in etcd-peer-ca Certificate
The changes for the etcd-peer-ca certificate now include a dynamic "duration" field (using .Values.caDuration) and a templated "subject" (via .Values.subject | toYaml | nindent 4). This meets the PR objectives by removing hardcoded values and enhancing flexibility.
97-99: Enhanced etcd-ca Certificate Settings
Similar to the etcd-peer-ca, the etcd-ca certificate now dynamically incorporates the "duration" and "subject" fields. This consistent approach simplifies configuration management for CA certificates.
130-147: Comprehensive Updates to the etcd-server Certificate
The etcd-server certificate now specifies a "commonName", a dynamic "subject", and moves the "127.0.0.1" into its own "ipAddresses" field. Additionally, including "client auth" in its usages directly addresses the self-querying behavior described in the PR objectives. Please verify the indentation produced by "toYaml | nindent 4" to ensure it renders correctly in the final manifest.
160-177: Consistent TLS Customization for the etcd-peer Certificate
The etcd-peer certificate now has a set "commonName", a dynamic "subject" field, and defines "ipAddresses" appropriately. This ensures that peer certificates leverage the new templating features and enhanced identification setup.
190-192: Updated etcd-client Certificate with Templated Subject
Adding the dynamic "subject" field to the etcd-client certificate extends the flexible TLS configuration to the client side. This change helps maintain consistency across all TLS certificates defined in this chart.
lllamnyp
force-pushed
the
chore/improve-etcd-tls
branch
from
df97bf5 to
14d93d2
Compare
dosubot
bot
added
size:S
1. Add a `commonName` to every certificate. 2. Move 127.0.0.1 from DNS names to IP Addresses in the certificate spec. 3. Add **client** auth usage to the etcd-**server** certificate (yes, that's necessary), because etcd queries itself using its [server cert as a client cert](etcd-io/etcd#9785 (comment)). 4. Default all CA certificates' durations to 10 years. 5. Set subject org to release namespace and OU to name so that subjects are unique
lllamnyp
force-pushed
the
chore/improve-etcd-tls
branch
from
14d93d2 to
0f87c73
Compare
dosubot
bot
added
size:M
commonNameto every certificate.spec.
that's necessary), because etcd queries itself using its
server cert as a client cert.
are unique.
Summary by CodeRabbit
New Features
Chores