Refactor cert handling

cluster-operator/src/main/java/io/strimzi/operator/cluster/model/CertUtils.java

@@ -4,17 +4,69 @@
  */
 package io.strimzi.operator.cluster.model;
 
+import io.fabric8.kubernetes.api.model.OwnerReference;
 import io.fabric8.kubernetes.api.model.Secret;
+import io.strimzi.certs.CertAndKey;
+import io.strimzi.operator.common.Reconciliation;
+import io.strimzi.operator.common.ReconciliationLogger;
 import io.strimzi.operator.common.Util;
 import io.strimzi.operator.common.model.Ca;
+import io.strimzi.operator.common.model.Labels;
 
+import java.io.IOException;
 import java.math.BigInteger;
+import java.nio.charset.StandardCharsets;
 import java.security.cert.CertificateEncodingException;
+import java.util.ArrayList;
+import java.util.Base64;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static java.util.Collections.emptyMap;
 
 /**
  * Certificate utility methods
  */
 public class CertUtils {
+    protected static final ReconciliationLogger LOGGER = ReconciliationLogger.create(CertUtils.class.getName());
+
+    /**
+     * A certificate entry in a Kubernetes Secret. Used to construct the keys in the Secret data where certificates are stored.
+     */
+    public enum SecretEntry {
+        /**
+         * A 64-bit encoded X509 Certificate
+         */
+        CRT(".crt"),
+        /**
+         * Entity private key
+         */
+        KEY(".key"),
+        /**
+         * Entity certificate and key as a P12 keystore
+         */
+        P12_KEYSTORE(".p12"),
+        /**
+         * P12 keystore password
+         */
+        P12_KEYSTORE_PASSWORD(".password");
+
+        final String suffix;
+
+        SecretEntry(String suffix) {
+            this.suffix = suffix;
+        }
+
+        /**
+         * @return The suffix of the key in the Secret
+         */
+        public String getSuffix() {
+            return suffix;
+        }
+
+    }
+
     /**
      * Generates a short SHA1-hash (a hash stub) of the certificate which is used to track when the certificate changes and rolling update needs to be triggered.
      *
@@ -42,4 +94,147 @@ public static String getCertificateThumbprint(Secret certSecret, String key) {
             throw new RuntimeException("Failed to get certificate thumbprint of " + key + " from Secret " + certSecret.getMetadata().getName(), e);
         }
     }
+
+    /**
+     * Builds a clusterCa certificate secret for the different Strimzi components (TO, UO, KE, ...)
+     *
+     * @param reconciliation                        Reconciliation marker
+     * @param clusterCa                             The Cluster CA
+     * @param secret                                Existing Kubernetes certificate Secret containing the certificate to use if present and does not need renewing
+     * @param namespace                             Namespace
+     * @param secretName                            Name of the Kubernetes secret
+     * @param commonName                            Common Name of the certificate
+     * @param keyCertName                           Key under which the certificate will be stored in the new Secret
+     * @param labels                                Labels
+     * @param ownerReference                        Owner reference
+     * @param isMaintenanceTimeWindowsSatisfied     Flag whether we are inside a maintenance window or not
+     *
+     * @return  Newly built Secret
+     */
+    public static Secret buildTrustedCertificateSecret(Reconciliation reconciliation, ClusterCa clusterCa, Secret secret, String namespace,
+                                                       String secretName, String commonName, String keyCertName,
+                                                       Labels labels, OwnerReference ownerReference, boolean isMaintenanceTimeWindowsSatisfied) {
+        boolean shouldBeRegenerated = false;
+        List<String> reasons = new ArrayList<>(2);
+
+        if (secret == null) {
+            reasons.add("certificate doesn't exist yet");
+            shouldBeRegenerated = true;
+        } else {
+            if (clusterCa.keyCreated()
+                    || clusterCa.certRenewed()
+                    || (isMaintenanceTimeWindowsSatisfied && clusterCa.isExpiring(secret, keyCertName + SecretEntry.CRT.getSuffix()))
+                    || clusterCa.hasCaCertGenerationChanged(secret)) {
+                reasons.add("certificate needs to be renewed");
+                shouldBeRegenerated = true;
+            }
+        }
+
+        CertAndKey certAndKey = null;
+        if (shouldBeRegenerated) {
+            LOGGER.debugCr(reconciliation, "Certificate for pod {} need to be regenerated because: {}", keyCertName, String.join(", ", reasons));
+
+            try {
+                certAndKey = clusterCa.generateSignedCert(commonName, Ca.IO_STRIMZI);
+            } catch (IOException e) {
+                LOGGER.warnCr(reconciliation, "Error while generating certificates", e);
+            }
+
+            LOGGER.debugCr(reconciliation, "End generating certificates");
+        } else {
+            CertAndKey keyStoreCertAndKey = keyStoreCertAndKey(secret, keyCertName);
+            if (keyStoreCertAndKey.keyStore().length != 0
+                    && keyStoreCertAndKey.storePassword() != null) {
+                certAndKey = keyStoreCertAndKey;
+            } else {
+                try {
+                    // coming from an older operator version, the secret exists but without keystore and password
+                    certAndKey = clusterCa.addKeyAndCertToKeyStore(commonName,
+                            keyStoreCertAndKey.key(),
+                            keyStoreCertAndKey.cert());
+                } catch (IOException e) {
+                    LOGGER.errorCr(reconciliation, "Error generating the keystore for {}", keyCertName, e);
+                }
+            }
+        }
+
+        Map<String, String> secretData = certAndKey == null ? Map.of() : buildSecretData(Map.of(keyCertName, certAndKey));
+
+        return ModelUtils.createSecret(secretName, namespace, labels, ownerReference, secretData, Map.ofEntries(clusterCa.caCertGenerationFullAnnotation()), emptyMap());
+    }
+
+    /**
+     * Constructs a Map containing the provided certificates to be stored in a Kubernetes Secret.
+     *
+     * @param certificates to store
+     * @return Map of certificate identifier to base64 encoded certificate or key
+     */
+    public static Map<String, String> buildSecretData(Map<String, CertAndKey> certificates) {
+        Map<String, String> data = new HashMap<>(certificates.size() * 4);
+        certificates.forEach((keyCertName, certAndKey) -> {
+            data.put(keyCertName + SecretEntry.KEY.getSuffix(), certAndKey.keyAsBase64String());
+            data.put(keyCertName + SecretEntry.CRT.getSuffix(), certAndKey.certAsBase64String());
+            data.put(keyCertName + SecretEntry.P12_KEYSTORE.getSuffix(), certAndKey.keyStoreAsBase64String());
+            data.put(keyCertName + SecretEntry.P12_KEYSTORE_PASSWORD.getSuffix(), certAndKey.storePasswordAsBase64String());
+        });
+        return data;
+    }
+
+    private static byte[] decodeFromSecret(Secret secret, String key) {
+        if (secret.getData().get(key) != null && !secret.getData().get(key).isEmpty()) {
+            return Base64.getDecoder().decode(secret.getData().get(key));
+        } else {
+            return new byte[]{};
+        }
+    }
+
+    /**
+     * Extracts the KeyStore from the Kubernetes Secret as a CertAndKey
+     * @param secret to extract certificate and key from
+     * @param keyCertName name of the KeyStore
+     * @return the KeyStore as a CertAndKey. Returned object has empty truststore and
+     * may have empty key, cert or keystore and null store password.
+     */
+    public static CertAndKey keyStoreCertAndKey(Secret secret, String keyCertName) {
+        byte[] passwordBytes = decodeFromSecret(secret, keyCertName + SecretEntry.P12_KEYSTORE_PASSWORD.getSuffix());
+        String password = passwordBytes.length == 0 ? null : new String(passwordBytes, StandardCharsets.US_ASCII);
+        return new CertAndKey(
+                decodeFromSecret(secret, keyCertName + SecretEntry.KEY.getSuffix()),
+                decodeFromSecret(secret, keyCertName + SecretEntry.CRT.getSuffix()),
+                new byte[]{},
+                decodeFromSecret(secret, keyCertName + SecretEntry.P12_KEYSTORE.getSuffix()),
+                password
+        );
+    }
+
+    /**
+     * Compares two Kubernetes Secrets with certificates and checks whether any value for a key which exists in both Secrets
+     * changed. This method is used to evaluate whether rolling update of existing brokers is needed when secrets with
+     * certificates change. It separates changes for existing certificates with other changes to the Secret such as
+     * added or removed certificates (scale-up or scale-down).
+     *
+     * @param current   Existing secret
+     * @param desired   Desired secret
+     *
+     * @return  True if there is a key which exists in the data sections of both secrets and which changed.
+     */
+    public static boolean doExistingCertificatesDiffer(Secret current, Secret desired) {
+        Map<String, String> currentData = current.getData();
+        Map<String, String> desiredData = desired.getData();
+
+        if (currentData == null) {
+            return true;
+        } else {
+            for (Map.Entry<String, String> entry : currentData.entrySet()) {
+                String desiredValue = desiredData.get(entry.getKey());
+                if (entry.getValue() != null
+                        && desiredValue != null
+                        && !entry.getValue().equals(desiredValue)) {
+                    return true;
+                }
+            }
+        }
+
+        return false;
+    }
 }

cluster-operator/src/main/java/io/strimzi/operator/cluster/model/ClusterCa.java

@@ -319,8 +319,8 @@ protected boolean hasCaCertGenerationChanged() {
 
             if (!this.certRenewed() // No CA renewal is happening
                     && secret != null && secret.getData() != null // Secret exists and has some data
-                    && secretEntryExists(secret, podName, SecretEntry.CRT) // The secret has the public key for this pod
-                    && secretEntryExists(secret, podName, SecretEntry.KEY) // The secret has the private key for this pod
+                    && secretEntryExists(secret, podName, CertUtils.SecretEntry.CRT) // The secret has the public key for this pod
+                    && secretEntryExists(secret, podName, CertUtils.SecretEntry.KEY) // The secret has the private key for this pod
                     && !hasCaCertGenerationChanged(secret) // The generation on the Secret is the same as the CA has
             )   {
                 // A certificate for this node already exists, so we will try to reuse it
@@ -333,8 +333,8 @@ && secretEntryExists(secret, podName, SecretEntry.KEY) // The secret has the pri
                 } else {
                     // coming from an older operator version, the secret exists but without keystore and password
                     certAndKey = addKeyAndCertToKeyStore(subject.commonName(),
-                            Base64.getDecoder().decode(secretEntryDataForPod(secret, podName, SecretEntry.KEY)),
-                            Base64.getDecoder().decode(secretEntryDataForPod(secret, podName, SecretEntry.CRT)));
+                            Base64.getDecoder().decode(secretEntryDataForPod(secret, podName, CertUtils.SecretEntry.KEY)),
+                            Base64.getDecoder().decode(secretEntryDataForPod(secret, podName, CertUtils.SecretEntry.CRT)));
                 }
 
                 List<String> reasons = new ArrayList<>(2);
@@ -386,8 +386,8 @@ && secretEntryExists(secret, podName, SecretEntry.KEY) // The secret has the pri
      * @return  True if this secret was created by a newer version of the operator and false otherwise.
      */
     private boolean isNewVersion(Secret secret, String podName) {
-        String store = secretEntryDataForPod(secret, podName, SecretEntry.P12_KEYSTORE);
-        String password = secretEntryDataForPod(secret, podName, SecretEntry.P12_KEYSTORE_PASSWORD);
+        String store = secretEntryDataForPod(secret, podName, CertUtils.SecretEntry.P12_KEYSTORE);
+        String password = secretEntryDataForPod(secret, podName, CertUtils.SecretEntry.P12_KEYSTORE_PASSWORD);
 
         return store != null && !store.isEmpty() && password != null && !password.isEmpty();
     }
@@ -401,10 +401,10 @@ private boolean isNewVersion(Secret secret, String podName) {
      * @return  CertAndKey instance
      */
     private static CertAndKey asCertAndKey(Secret secret, String podName) {
-        return asCertAndKey(secret, secretEntryNameForPod(podName, SecretEntry.KEY),
-                secretEntryNameForPod(podName, SecretEntry.CRT),
-                secretEntryNameForPod(podName, SecretEntry.P12_KEYSTORE),
-                secretEntryNameForPod(podName, SecretEntry.P12_KEYSTORE_PASSWORD));
+        return asCertAndKey(secret, secretEntryNameForPod(podName, CertUtils.SecretEntry.KEY),
+                secretEntryNameForPod(podName, CertUtils.SecretEntry.CRT),
+                secretEntryNameForPod(podName, CertUtils.SecretEntry.P12_KEYSTORE),
+                secretEntryNameForPod(podName, CertUtils.SecretEntry.P12_KEYSTORE_PASSWORD));
     }
 
     /**
@@ -465,7 +465,7 @@ private List<String> getSubjectAltNames(byte[] certificate) {
      *
      * @return  True if the Secret contains a key based on the pod name and entry type. False otherwise.
      */
-    private static boolean secretEntryExists(Secret secret, String podName, SecretEntry entry) {
+    private static boolean secretEntryExists(Secret secret, String podName, CertUtils.SecretEntry entry) {
         return secret.getData().containsKey(secretEntryNameForPod(podName, entry));
     }
 
@@ -478,7 +478,7 @@ private static boolean secretEntryExists(Secret secret, String podName, SecretEn
      *
      * @return  The data of the secret entry if found or null otherwise
      */
-    private static String secretEntryDataForPod(Secret secret, String podName, SecretEntry entry) {
+    private static String secretEntryDataForPod(Secret secret, String podName, CertUtils.SecretEntry entry) {
         return secret.getData().get(secretEntryNameForPod(podName, entry));
     }
 
@@ -490,7 +490,7 @@ private static String secretEntryDataForPod(Secret secret, String podName, Secre
      *
      * @return  The name of the secret entry
      */
-    public static String secretEntryNameForPod(String podName, SecretEntry entry) {
+    public static String secretEntryNameForPod(String podName, CertUtils.SecretEntry entry) {
         return podName + entry.getSuffix();
     }
 

cluster-operator/src/main/java/io/strimzi/operator/cluster/model/CruiseControl.java

@@ -471,24 +471,8 @@ public Secret generateCertificatesSecret(String namespace, String kafkaName, Clu
         }
         LOGGER.debugCr(reconciliation, "End generating certificates");
 
-        String keyCertName = "cruise-control";
-        Map<String, String> data = new HashMap<>(4);
-
-        CertAndKey cert = ccCerts.get(keyCertName);
-        data.put(keyCertName + ".key", cert.keyAsBase64String());
-        data.put(keyCertName + ".crt", cert.certAsBase64String());
-        data.put(keyCertName + ".p12", cert.keyStoreAsBase64String());
-        data.put(keyCertName + ".password", cert.storePasswordAsBase64String());
-
-        return ModelUtils.createSecret(
-                CruiseControlResources.secretName(cluster),
-                namespace,
-                labels,
-                ownerReference,
-                data,
-                Map.of(clusterCa.caCertGenerationAnnotation(), String.valueOf(clusterCa.certGeneration())),
-                Map.of()
-        );
+        return ModelUtils.createSecret(CruiseControlResources.secretName(cluster), namespace, labels, ownerReference,
+                CertUtils.buildSecretData(ccCerts), Map.ofEntries(clusterCa.caCertGenerationFullAnnotation()), Map.of());
     }
 
     /**

cluster-operator/src/main/java/io/strimzi/operator/cluster/model/EntityTopicOperator.java

@@ -259,7 +259,7 @@ public RoleBinding generateRoleBindingForRole(String namespace, String watchedNa
      */
     public Secret generateSecret(ClusterCa clusterCa, boolean isMaintenanceTimeWindowsSatisfied) {
         Secret secret = clusterCa.entityTopicOperatorSecret();
-        return ModelUtils.buildSecret(reconciliation, clusterCa, secret, namespace, KafkaResources.entityTopicOperatorSecretName(cluster), componentName,
+        return CertUtils.buildTrustedCertificateSecret(reconciliation, clusterCa, secret, namespace, KafkaResources.entityTopicOperatorSecretName(cluster), componentName,
             CERT_SECRET_KEY_NAME, labels, ownerReference, isMaintenanceTimeWindowsSatisfied);
     }
 

cluster-operator/src/main/java/io/strimzi/operator/cluster/model/EntityUserOperator.java

@@ -265,7 +265,7 @@ public RoleBinding generateRoleBindingForRole(String namespace, String watchedNa
      */
     public Secret generateSecret(ClusterCa clusterCa, boolean isMaintenanceTimeWindowsSatisfied) {
         Secret secret = clusterCa.entityUserOperatorSecret();
-        return ModelUtils.buildSecret(reconciliation, clusterCa, secret, namespace, KafkaResources.entityUserOperatorSecretName(cluster), componentName,
+        return CertUtils.buildTrustedCertificateSecret(reconciliation, clusterCa, secret, namespace, KafkaResources.entityUserOperatorSecretName(cluster), componentName,
             CERT_SECRET_KEY_NAME, labels, ownerReference, isMaintenanceTimeWindowsSatisfied);
     }
 

cluster-operator/src/main/java/io/strimzi/operator/cluster/model/KafkaCluster.java

@@ -1117,25 +1117,11 @@ public Secret generateCertificatesSecret(ClusterCa clusterCa, ClientsCa clientsC
             throw new RuntimeException("Failed to prepare Kafka certificates", e);
         }
 
-        Map<String, String> data = new HashMap<>();
-
-        for (NodeRef node : nodes)  {
-            CertAndKey cert = brokerCerts.get(node.podName());
-            data.put(node.podName() + ".key", cert.keyAsBase64String());
-            data.put(node.podName() + ".crt", cert.certAsBase64String());
-            data.put(node.podName() + ".p12", cert.keyStoreAsBase64String());
-            data.put(node.podName() + ".password", cert.storePasswordAsBase64String());
-        }
-
-        return ModelUtils.createSecret(
-                KafkaResources.kafkaSecretName(cluster),
-                namespace,
-                labels,
-                ownerReference,
-                data,
-                Map.of(
-                        clusterCa.caCertGenerationAnnotation(), String.valueOf(clusterCa.certGeneration()),
-                        clientsCa.caCertGenerationAnnotation(), String.valueOf(clientsCa.certGeneration())
+        return ModelUtils.createSecret(KafkaResources.kafkaSecretName(cluster), namespace, labels, ownerReference,
+                CertUtils.buildSecretData(brokerCerts),
+                Map.ofEntries(
+                        clusterCa.caCertGenerationFullAnnotation(),
+                        clientsCa.caCertGenerationFullAnnotation()
                 ),
                 emptyMap());
     }

cluster-operator/src/main/java/io/strimzi/operator/cluster/model/KafkaExporter.java

@@ -267,7 +267,7 @@ private List<Volume> getVolumes(boolean isOpenShift) {
      */
     public Secret generateSecret(ClusterCa clusterCa, boolean isMaintenanceTimeWindowsSatisfied) {
         Secret secret = clusterCa.kafkaExporterSecret();
-        return ModelUtils.buildSecret(reconciliation, clusterCa, secret, namespace, KafkaExporterResources.secretName(cluster), componentName,
+        return CertUtils.buildTrustedCertificateSecret(reconciliation, clusterCa, secret, namespace, KafkaExporterResources.secretName(cluster), componentName,
                 "kafka-exporter", labels, ownerReference, isMaintenanceTimeWindowsSatisfied);
     }