Address ppatierno review comments

* Remove unnecessary 'public' in docs
* Add missing null checking
* Throw IllegalArgumentException for unknown CertificateManagerType

Signed-off-by: Kate Stanley <11195226+katheris@users.noreply.github.com>

Author: katheris

CHANGELOG.md

@@ -1,8 +1,11 @@
 # CHANGELOG
 
-## 1.2.0
+## 1.3.0
 
 * Add support for cert-manager. Users can set `spec.clusterCa.type` and/or `spec.clientsCa.type` to `cert-manager.io` to delegate end-entity certificate issuance to a cert-manager `Issuer` or `ClusterIssuer`. This feature is behind the `CertManagerCaType` feature gate, which is disabled by default.
+
+## 1.2.0
+
 * Add support for Apache Kafka 4.3.1
 * Support templated (per-pod) additional volumes for Kafka, Kafka Connect and Kafka MirrorMaker 2 operands
 * Stop auto-mounting Service Account tokens into Pods and mount them through a volume instead

api/src/main/java/io/strimzi/api/kafka/model/common/CertificateManagerType.java

@@ -16,25 +16,18 @@ public enum CertificateManagerType {
 
     @JsonCreator
     public static CertificateManagerType forValue(String value) {
-        switch (value) {
-            case "strimzi.io":
-                return STRIMZI_IO;
-            case "cert-manager.io":
-                return CERT_MANAGER_IO;
-            default:
-                return null;
-        }
+        return switch (value) {
+            case "strimzi.io" -> STRIMZI_IO;
+            case "cert-manager.io" -> CERT_MANAGER_IO;
+            default -> throw new IllegalArgumentException(String.format("Unknown certificate manager type: %s. Must be %s or %s.", value, STRIMZI_IO.toValue(), CERT_MANAGER_IO.toValue()));
+        };
     }
 
     @JsonValue
     public String toValue() {
-        switch (this) {
-            case STRIMZI_IO:
-                return "strimzi.io";
-            case CERT_MANAGER_IO:
-                return "cert-manager.io";
-            default:
-                return null;
-        }
+        return switch (this) {
+            case STRIMZI_IO -> "strimzi.io";
+            case CERT_MANAGER_IO -> "cert-manager.io";
+        };
     }
 }

cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/CertManagerCaProvider.java

@@ -111,7 +111,7 @@ private CompletionStage<String> getCaCertForCertManager() {
                 .thenApply(secret -> {
                     if (secret == null) {
                         throw new InvalidResourceException("CA public certificate Secret " + caCertSecretName + " missing.");
-                    } else if (secret.getData().get(caCertSecretKey) == null) {
+                    } else if (secret.getData() == null || secret.getData().get(caCertSecretKey) == null) {
                         throw new InvalidResourceException("CA public certificate Secret " + caCertSecretName + " missing key " + caCertSecretKey);
                     }
                     CertificateUtils.validateUserCaCertChain(reconciliation, caRole, Map.of(caCertSecretKey, secret.getData().get(caCertSecretKey)));

documentation/modules/security/proc-using-cert-manager.adoc

@@ -9,7 +9,7 @@
 
 [role="_abstract"]
 Configure Strimzi to delegate TLS certificate issuing to https://cert-manager.io/[cert-manager] instead of managing it internally.
-When enabled, Strimzi creates cert-manager `Certificate` resources for each component and reads the CA public certificate from a user-provided `Secret` to establish TLS trust.
+When enabled, Strimzi creates cert-manager `Certificate` resources for each component and reads the CA certificate from a user-provided `Secret` to establish TLS trust.
 cert-manager is responsible for issuing and renewing all end-entity certificates.
 
 This feature requires the `CertManagerCaType` feature gate to be enabled.
@@ -29,7 +29,7 @@ NOTE: Strimzi does not install cert-manager and is not responsible for creating
 [source,shell,subs="+quotes"]
 kubectl apply -f packaging/install/cert-manager/
 
-. Create a `Secret` containing the CA public certificate that Strimzi should trust.
+. Create a `Secret` containing the CA certificate that Strimzi should trust.
 +
 [source,shell,subs="+quotes"]
 ----
@@ -40,13 +40,16 @@ kubectl create secret generic _<ca_cert_secret_name>_ \
 +
 Replace _<ca_cert_secret_name>_ with a name of your choice, _<path_to_ca_cert_pem>_ with the path to the CA certificate PEM file, and _<namespace>_ with the namespace of the Kafka cluster.
 
+NOTE: The `ca.crt` file in the Secret must contain either a single CA certificate or CA certificate chain that can be used to trust certificates issued by cert-manager.
+For this reason the cert-manager `SelfSigned` issuer is not supported.
+
 . Create or update the `Kafka` resource, setting `type: cert-manager.io` on `clusterCa`, `clientsCa`, or both.
 Set `generateCertificateAuthority: false` and provide the `certManager.issuerRef` and `certManager.caCert` configuration.
 +
 .Example `Kafka` resource using cert-manager for both Cluster CA and Clients CA
 [source,yaml,subs="attributes+"]
 ----
-apiVersion: kafka.strimzi.io/v1beta2
+apiVersion: kafka.strimzi.io/v1
 kind: Kafka
 metadata:
   name: my-cluster
@@ -82,7 +85,7 @@ spec:
 <3> Name of the cert-manager `Issuer` or `ClusterIssuer` to use.
 <4> Kind of the issuer. Must be `Issuer` or `ClusterIssuer`.
 <5> API group of the issuer. Defaults to `cert-manager.io`.
-<6> Name of the `Secret` containing the CA public certificate.
+<6> Name of the `Secret` containing the CA certificate.
 <7> Key within the `Secret` that holds the CA certificate PEM data.
 
 . Apply the `Kafka` resource.
@@ -101,7 +104,7 @@ During reconciliation, Strimzi copies the certificates from these Secrets into t
 
 .CA certificate rotation
 
-When the CA public certificate in the user-provided `Secret` changes, Strimzi detects the change on the next reconciliation.
+When the CA certificate in the user-provided `Secret` changes, Strimzi detects the change on the next reconciliation.
 
 * If the new certificate was signed by the same private key (cert renewal), Strimzi rolls the Kafka pods once to update the truststore.
 * If the new certificate was signed by a new private key (key replacement), Strimzi performs a phased rolling restart.

operator-common/src/main/java/io/strimzi/operator/common/ca/CertManagerCa.java

@@ -18,7 +18,6 @@
 import io.strimzi.operator.common.operator.resource.kubernetes.CertManagerCertificateOperator;
 import io.strimzi.operator.common.operator.resource.kubernetes.SecretOperator;
 
-import java.math.BigInteger;
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
 import java.time.Duration;
@@ -111,13 +110,13 @@ public void maybeUpdateCa(String newCaCertAsBase64, String existingCaCertHash, X
                 updatedCertData.put(CA_CRT, newCaCertAsBase64);
             }
             case RENEW_CERT -> {
-                updatedCertData = new HashMap<>();
+                updatedCertData = new HashMap<>(caCertData);
                 updatedCertData.put(CA_CRT, newCaCertAsBase64);
                 ++caCertGeneration;
             }
             case REPLACE_KEY -> {
                 String notAfterDate = DATE_TIME_FORMATTER.format(currentCaCertX509().getNotAfter().toInstant().atZone(ZoneId.of("Z")));
-                updatedCertData = new HashMap<>();
+                updatedCertData = new HashMap<>(caCertData);
                 updatedCertData.put(Ca.SecretEntry.CRT.asKey("ca-" + notAfterDate), caCertData.get(CA_CRT));
                 updatedCertData.put(CA_CRT, newCaCertAsBase64);
                 ++caCertGeneration;
@@ -137,7 +136,7 @@ private RenewalType shouldUpdateCa(String newCaCertAsBase64, String existingCaCe
         String newCaCertHash;
         try {
             x509CaCert = CertificateUtils.x509Certificate(Util.decodeBytesFromBase64(newCaCertAsBase64));
-            newCaCertHash = String.format("%040x", new BigInteger(1, Util.sha1Digest(x509CaCert.getEncoded())));
+            newCaCertHash = CertificateUtils.getCertificateThumbprint(x509CaCert);
         } catch (CertificateException e) {
             throw new RuntimeException(e);
         }
@@ -265,7 +264,7 @@ private Certificate buildCertificateResource(String entityName, Subject subject,
                 .withNewPrivateKey()
                     .withAlgorithm("RSA")
                     .withEncoding("PKCS8")
-                    .withSize(2048)
+                    .withSize(4096)
                 .endPrivateKey()
                 .withDuration(convertToFabric8Duration(validityDays))
                 .withRenewBefore(convertToFabric8Duration(renewalDays))

operator-common/src/main/java/io/strimzi/operator/common/operator/resource/kubernetes/CertManagerCertificateOperator.java

@@ -51,10 +51,14 @@ public CompletionStage<Void> waitForReady(Reconciliation reconciliation, String
 
     private boolean isReady(String namespace, String name) {
         Certificate certificate = operation().inNamespace(namespace).withName(name).get();
-        CertificateStatus status = certificate.getStatus();
+        if (certificate == null) {
+            return false;
+        }
+
         boolean certificateReady = false;
+        CertificateStatus status = certificate.getStatus();
         if (status != null) {
-            List<CertificateCondition> conditions = certificate.getStatus().getConditions();
+            List<CertificateCondition> conditions = status.getConditions();
             Optional<CertificateCondition> readyCondition = conditions.stream().filter(condition -> condition.getType().equals("Ready"))
                     .findFirst();
             if (readyCondition.isPresent() && readyCondition.get().getStatus().equals("True")) {