wait for client cert to become ready before getting etcd client - #269

examples/manifests/etcdcluster-with-external-certificates.yaml (6)

47-47: Approved: RSA key size reduction for ca-certificate-peer

The change from 4096 to 2048 bits for the RSA private key is acceptable. While this slightly reduces the theoretical security strength, 2048-bit RSA keys are still considered secure for most applications as of 2024. This change may improve performance, especially in resource-constrained environments.

Consider documenting this change and the rationale behind it in your project's security policy or documentation.

---

99-99: Approved: RSA key size reduction for ca-certificate-client

The change from 4096 to 2048 bits for the RSA private key is consistent with the previous certificate changes and is acceptable.

Given that this change affects all CA certificates (peer, server, and client), it's recommended to conduct a security review to ensure this change aligns with your organization's security policies and risk tolerance.

---

165-165: Approved: RSA key size reduction for server-certificate

The change from 4096 to 2048 bits for the RSA private key is consistent with the previous changes and is acceptable.

Note that this certificate has a rotation policy set to "Always". With the reduced key size, consider reviewing and potentially adjusting the rotation frequency to maintain an appropriate balance between security and performance.

---

200-200: Approved: RSA key size reduction for peer-certificate

The change from 4096 to 2048 bits for the RSA private key is consistent with the previous changes and is acceptable.

As this certificate is used for both server and client authentication in peer communication, monitor the performance impact of this change, especially during certificate rotation operations. Consider setting up metrics to track the duration of TLS handshakes and overall cluster communication latency.

---

219-219: Approved with caution: RSA key size reduction for client-certificate

The change from 4096 to 2048 bits for the RSA private key is consistent with the previous changes and is acceptable from a technical standpoint.

However, given that this client certificate has the common name "root", which implies high privileges, extra caution is warranted:

1. Ensure that the reduced key size doesn't conflict with any specific security requirements for root-level access.
2. Review the access controls and policies associated with this certificate to ensure they align with the principle of least privilege.
3. Consider implementing additional monitoring and alerting for the usage of this certificate, given its potentially elevated privileges.

---

Line range hint 1-221: Summary: Consistent RSA key size reduction across all certificates

The changes in this PR consistently reduce the RSA private key size from 4096 to 2048 bits across all certificate resources in the EtcdCluster configuration. This change aligns with the PR objectives of potentially improving performance, especially in resource-constrained environments.

Key points:

1. The change is consistently applied to all CA, server, peer, and client certificates.
2. 2048-bit RSA keys are still considered secure for most applications as of 2024.
3. This change may improve performance, particularly during TLS handshakes and certificate operations.

Recommendations:

1. Document this change in your project's security policy and architecture documentation.
2. Conduct a thorough security review to ensure alignment with your organization's security policies and risk tolerance.
3. Implement monitoring to track the performance impact of this change, especially during certificate rotation operations.
4. Review and potentially adjust certificate rotation frequencies in light of the reduced key sizes.
5. Pay special attention to the "root" client certificate and ensure its usage aligns with the principle of least privilege.

Consider creating a follow-up task to evaluate the real-world performance improvements gained from this change and to reassess the security implications after a defined period of operation with the new key sizes.

test/e2e/e2e_test.go (1)

225-235: LGTM! Consider enhancing error handling and success message.

The addition of this Eventually block effectively addresses the PR objective by ensuring the client certificate is ready before proceeding with the etcd cluster readiness check. This should improve the reliability of the test.

Some minor suggestions for improvement:

1. Consider adding more detailed error handling to aid in debugging if the wait fails. For example:

Eventually(func() error {
    cmd := exec.Command("kubectl", "wait",
        "certificate/client-certificate",
        "--for", "condition=Ready",
        "--namespace", namespace,
        "--timeout", "5m",
    )
    output, err := utils.Run(cmd)
    if err != nil {
        return fmt.Errorf("failed to wait for client certificate: %v, output: %s", err, output)
    }
    return nil
}, time.Second*20, time.Second*2).Should(Succeed(), "client certificate should become ready within 20 seconds")

2. Update the success message to be more descriptive, e.g., "wait for client certificate to become ready".

Configuration used: CodeRabbit UI
Review profile: CHILL