From e57742ef54f35c447ba2c96b3d161f7ad8342e39 Mon Sep 17 00:00:00 2001 From: Kate Stanley <11195226+katheris@users.noreply.github.com> Date: Fri, 22 Dec 2023 09:50:51 +0000 Subject: [PATCH] Refactor cert handling (#9463) Signed-off-by: Katherine Stanley <11195226+katheris@users.noreply.github.com> --- .../operator/cluster/model/CertUtils.java | 159 ++++++++++++++++++ .../operator/cluster/model/ClusterCa.java | 24 +-- .../operator/cluster/model/CruiseControl.java | 20 +-- .../cluster/model/EntityTopicOperator.java | 2 +- .../cluster/model/EntityUserOperator.java | 2 +- .../operator/cluster/model/KafkaCluster.java | 24 +-- .../operator/cluster/model/KafkaExporter.java | 2 +- .../operator/cluster/model/ModelUtils.java | 149 +--------------- .../cluster/model/ZookeeperCluster.java | 20 +-- .../operator/assembly/CaReconciler.java | 3 +- .../assembly/CruiseControlReconciler.java | 8 +- .../assembly/EntityOperatorReconciler.java | 10 +- .../assembly/KafkaExporterReconciler.java | 8 +- .../operator/assembly/KafkaReconciler.java | 9 +- .../operator/assembly/ReconcilerUtils.java | 3 +- .../assembly/ZooKeeperReconciler.java | 7 +- .../operator/cluster/model/CertUtilsTest.java | 109 ++++++++++++ .../cluster/model/ModelUtilsTest.java | 97 ----------- .../assembly/CertificateRenewalTest.java | 16 +- .../KafkaAssemblyOperatorMockTest.java | 5 +- ...afkaAssemblyOperatorWithPoolsMockTest.java | 7 +- .../PartialRollingUpdateMockTest.java | 9 +- .../io/strimzi/operator/common/model/Ca.java | 72 +++++--- 23 files changed, 386 insertions(+), 379 deletions(-) create mode 100644 cluster-operator/src/test/java/io/strimzi/operator/cluster/model/CertUtilsTest.java diff --git a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/CertUtils.java b/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/CertUtils.java index c7ea982f008..25f07a50f3d 100644 --- a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/CertUtils.java +++ b/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/CertUtils.java @@ -4,17 +4,33 @@ */ 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()); + /** * 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 +58,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 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, Ca.SecretEntry.CRT.asKey(keyCertName))) + || 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 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 buildSecretData(Map certificates) { + Map data = new HashMap<>(certificates.size() * 4); + certificates.forEach((keyCertName, certAndKey) -> { + data.put(Ca.SecretEntry.KEY.asKey(keyCertName), certAndKey.keyAsBase64String()); + data.put(Ca.SecretEntry.CRT.asKey(keyCertName), certAndKey.certAsBase64String()); + data.put(Ca.SecretEntry.P12_KEYSTORE.asKey(keyCertName), certAndKey.keyStoreAsBase64String()); + data.put(Ca.SecretEntry.P12_KEYSTORE_PASSWORD.asKey(keyCertName), 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, Ca.SecretEntry.P12_KEYSTORE_PASSWORD.asKey(keyCertName)); + String password = passwordBytes.length == 0 ? null : new String(passwordBytes, StandardCharsets.US_ASCII); + return new CertAndKey( + decodeFromSecret(secret, Ca.SecretEntry.KEY.asKey(keyCertName)), + decodeFromSecret(secret, Ca.SecretEntry.CRT.asKey(keyCertName)), + new byte[]{}, + decodeFromSecret(secret, Ca.SecretEntry.P12_KEYSTORE.asKey(keyCertName)), + 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 currentData = current.getData(); + Map desiredData = desired.getData(); + + if (currentData == null) { + return true; + } else { + for (Map.Entry entry : currentData.entrySet()) { + String desiredValue = desiredData.get(entry.getKey()); + if (entry.getValue() != null + && desiredValue != null + && !entry.getValue().equals(desiredValue)) { + return true; + } + } + } + + return false; + } } diff --git a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/ClusterCa.java b/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/ClusterCa.java index 49623948244..953f0ef9019 100644 --- a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/ClusterCa.java +++ b/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/ClusterCa.java @@ -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, SecretEntry.KEY.asKey(podName), + SecretEntry.CRT.asKey(podName), + SecretEntry.P12_KEYSTORE.asKey(podName), + SecretEntry.P12_KEYSTORE_PASSWORD.asKey(podName)); } /** @@ -466,7 +466,7 @@ private List 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) { - return secret.getData().containsKey(secretEntryNameForPod(podName, entry)); + return secret.getData().containsKey(entry.asKey(podName)); } /** @@ -479,19 +479,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) { - return secret.getData().get(secretEntryNameForPod(podName, entry)); - } - - /** - * Get the name of secret entry of given SecretEntry type for podName - * - * @param podName Name of the pod which secret entry is looked for - * @param entry The SecretEntry type - * - * @return The name of the secret entry - */ - public static String secretEntryNameForPod(String podName, SecretEntry entry) { - return podName + entry.getSuffix(); + return secret.getData().get(entry.asKey(podName)); } /** diff --git a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/CruiseControl.java b/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/CruiseControl.java index 65b85ea12e4..5e30e31f079 100644 --- a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/CruiseControl.java +++ b/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 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()); } /** diff --git a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/EntityTopicOperator.java b/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/EntityTopicOperator.java index 62782ac3757..aa3d7dafa03 100644 --- a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/EntityTopicOperator.java +++ b/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); } diff --git a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/EntityUserOperator.java b/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/EntityUserOperator.java index da107e8cbc3..3646d870f91 100644 --- a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/EntityUserOperator.java +++ b/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); } diff --git a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/KafkaCluster.java b/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/KafkaCluster.java index bf56a073b0b..fb691867787 100644 --- a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/KafkaCluster.java +++ b/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 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()); } diff --git a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/KafkaExporter.java b/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/KafkaExporter.java index 0c60edb6451..48f40a1a074 100644 --- a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/KafkaExporter.java +++ b/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/KafkaExporter.java @@ -267,7 +267,7 @@ private List 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); } diff --git a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/ModelUtils.java b/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/ModelUtils.java index 76e72478b54..643cc421943 100644 --- a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/ModelUtils.java +++ b/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/ModelUtils.java @@ -24,28 +24,19 @@ import io.strimzi.api.kafka.model.TlsSidecar; import io.strimzi.api.kafka.model.TlsSidecarLogLevel; import io.strimzi.api.kafka.model.storage.Storage; -import io.strimzi.certs.CertAndKey; -import io.strimzi.operator.common.Annotations; -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.InvalidResourceException; import io.strimzi.operator.common.model.Labels; import java.io.IOException; import java.math.BigDecimal; -import java.nio.charset.StandardCharsets; import java.util.ArrayList; -import java.util.Base64; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; -import static java.util.Collections.emptyMap; - /** * ModelUtils is a utility class that holds generic static helper functions * These are generally to be used within the classes that extend the AbstractModel class @@ -58,6 +49,8 @@ private ModelUtils() {} protected static final String TLS_SIDECAR_LOG_LEVEL = "TLS_SIDECAR_LOG_LEVEL"; /** + * Extract certificate validity days from cluster CA configuration + * * @param certificateAuthority The CA configuration. * @return The cert validity. */ @@ -71,6 +64,8 @@ public static int getCertificateValidity(CertificateAuthority certificateAuthori } /** + * Extract certificate renewal days from cluster CA configuration + * * @param certificateAuthority The CA configuration. * @return The renewal days. */ @@ -102,86 +97,6 @@ static EnvVar tlsSidecarLogEnvVar(TlsSidecar tlsSidecar) { tlsSidecar.getLogLevel() : TlsSidecarLogLevel.NOTICE).toValue()); } - /** - * Builds a certificate secret for the different Strimzi components (TO, UO, KE, ...) - * - * @param reconciliation Reconciliation marker - * @param clusterCa The Cluster CA - * @param secret Kubernetes Secret - * @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 buildSecret(Reconciliation reconciliation, ClusterCa clusterCa, Secret secret, String namespace, String secretName, - String commonName, String keyCertName, Labels labels, OwnerReference ownerReference, boolean isMaintenanceTimeWindowsSatisfied) { - Map data = new HashMap<>(4); - CertAndKey certAndKey = null; - boolean shouldBeRegenerated = false; - List 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 + ".crt")) || - clusterCa.hasCaCertGenerationChanged(secret)) { - reasons.add("certificate needs to be renewed"); - shouldBeRegenerated = true; - } - } - - 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 { - if (secret.getData().get(keyCertName + ".p12") != null && - !secret.getData().get(keyCertName + ".p12").isEmpty() && - secret.getData().get(keyCertName + ".password") != null && - !secret.getData().get(keyCertName + ".password").isEmpty()) { - certAndKey = new CertAndKey( - decodeFromSecret(secret, keyCertName + ".key"), - decodeFromSecret(secret, keyCertName + ".crt"), - null, - decodeFromSecret(secret, keyCertName + ".p12"), - new String(decodeFromSecret(secret, keyCertName + ".password"), StandardCharsets.US_ASCII) - ); - } else { - try { - // coming from an older operator version, the secret exists but without keystore and password - certAndKey = clusterCa.addKeyAndCertToKeyStore(commonName, - decodeFromSecret(secret, keyCertName + ".key"), - decodeFromSecret(secret, keyCertName + ".crt")); - } catch (IOException e) { - LOGGER.errorCr(reconciliation, "Error generating the keystore for {}", keyCertName, e); - } - } - } - - if (certAndKey != null) { - data.put(keyCertName + ".key", certAndKey.keyAsBase64String()); - data.put(keyCertName + ".crt", certAndKey.certAsBase64String()); - data.put(keyCertName + ".p12", certAndKey.keyStoreAsBase64String()); - data.put(keyCertName + ".password", certAndKey.storePasswordAsBase64String()); - } - - return createSecret(secretName, namespace, labels, ownerReference, data, - Collections.singletonMap(clusterCa.caCertGenerationAnnotation(), String.valueOf(clusterCa.certGeneration())), emptyMap()); - } - /** * Creates Secret * @@ -264,41 +179,6 @@ public static Map getCustomLabelsOrAnnotations(String envVarName return Util.parseMap(System.getenv().get(envVarName)); } - private static byte[] decodeFromSecret(Secret secret, String key) { - return Base64.getDecoder().decode(secret.getData().get(key)); - } - - /** - * Compares two 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 currentData = current.getData(); - Map desiredData = desired.getData(); - - if (currentData == null) { - return true; - } else { - for (Map.Entry entry : currentData.entrySet()) { - String desiredValue = desiredData.get(entry.getKey()); - if (entry.getValue() != null - && desiredValue != null - && !entry.getValue().equals(desiredValue)) { - return true; - } - } - } - - return false; - } - /** * Checks for the list passed to this method if it is null or not. And either returns the same list, or empty list * if it is null. @@ -424,27 +304,6 @@ public static boolean hasOwnerReference(HasMetadata resource, OwnerReference own } } - /** - * Extracts the CA generation from the CA - * - * @param ca CA from which the generation should be extracted - * - * @return CA generation or the initial generation if no generation is set - */ - public static int caCertGeneration(Ca ca) { - return Annotations.intAnnotation(ca.caCertSecret(), Ca.ANNO_STRIMZI_IO_CA_CERT_GENERATION, Ca.INIT_GENERATION); - } - - /** - * Extract the CA key generation from the CA - * - * @param ca CA from which the generation should be extracted - * @return CA key generation or the initial generation if no generation is set - */ - public static int caKeyGeneration(Ca ca) { - return Annotations.intAnnotation(ca.caKeySecret(), Ca.ANNO_STRIMZI_IO_CA_KEY_GENERATION, Ca.INIT_GENERATION); - } - /** * Generates all possible DNS names for a Kubernetes service: * - service-name diff --git a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/ZookeeperCluster.java b/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/ZookeeperCluster.java index dbd02986d8e..bbf0e6c0edf 100644 --- a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/ZookeeperCluster.java +++ b/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/ZookeeperCluster.java @@ -442,7 +442,6 @@ public StrimziPodSet generatePodSet(int replicas, * @return The generated Secret with the ZooKeeper node certificates */ public Secret generateCertificatesSecret(ClusterCa clusterCa, boolean isMaintenanceTimeWindowsSatisfied) { - Map secretData = new HashMap<>(replicas * 4); Map certs; try { @@ -452,23 +451,8 @@ public Secret generateCertificatesSecret(ClusterCa clusterCa, boolean isMaintena throw new RuntimeException("Failed to prepare ZooKeeper certificates", e); } - for (int i = 0; i < replicas; i++) { - CertAndKey cert = certs.get(KafkaResources.zookeeperPodName(cluster, i)); - secretData.put(KafkaResources.zookeeperPodName(cluster, i) + ".key", cert.keyAsBase64String()); - secretData.put(KafkaResources.zookeeperPodName(cluster, i) + ".crt", cert.certAsBase64String()); - secretData.put(KafkaResources.zookeeperPodName(cluster, i) + ".p12", cert.keyStoreAsBase64String()); - secretData.put(KafkaResources.zookeeperPodName(cluster, i) + ".password", cert.storePasswordAsBase64String()); - } - - return ModelUtils.createSecret( - KafkaResources.zookeeperSecretName(cluster), - namespace, - labels, - ownerReference, - secretData, - Map.of(clusterCa.caCertGenerationAnnotation(), String.valueOf(clusterCa.certGeneration())), - emptyMap() - ); + return ModelUtils.createSecret(KafkaResources.zookeeperSecretName(cluster), namespace, labels, ownerReference, + CertUtils.buildSecretData(certs), Map.ofEntries(clusterCa.caCertGenerationFullAnnotation()), emptyMap()); } /* test */ Container createContainer(ImagePullPolicy imagePullPolicy) { diff --git a/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/CaReconciler.java b/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/CaReconciler.java index 2c7ff81e38a..ffb9fe7f920 100644 --- a/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/CaReconciler.java +++ b/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/CaReconciler.java @@ -17,6 +17,7 @@ import io.strimzi.certs.CertManager; import io.strimzi.operator.cluster.ClusterOperatorConfig; import io.strimzi.operator.cluster.model.AbstractModel; +import io.strimzi.operator.cluster.model.CertUtils; import io.strimzi.operator.common.model.Ca; import io.strimzi.operator.common.model.ClientsCa; import io.strimzi.operator.cluster.model.ClusterCa; @@ -329,7 +330,7 @@ Future clusterOperatorSecret(Clock clock) { LOGGER.warnCr(reconciliation, "Cluster CA needs to be fully trusted across the cluster, keeping current CO secret and certs"); return Future.succeededFuture(); } - Secret secret = ModelUtils.buildSecret( + Secret secret = CertUtils.buildTrustedCertificateSecret( reconciliation, clusterCa, clusterCa.clusterOperatorSecret(), diff --git a/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/CruiseControlReconciler.java b/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/CruiseControlReconciler.java index 107782dde2b..1a7d5fc93d8 100644 --- a/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/CruiseControlReconciler.java +++ b/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/CruiseControlReconciler.java @@ -13,12 +13,12 @@ import io.strimzi.api.kafka.model.Kafka; import io.strimzi.api.kafka.model.storage.Storage; import io.strimzi.operator.cluster.ClusterOperatorConfig; +import io.strimzi.operator.cluster.model.CertUtils; import io.strimzi.operator.common.model.Ca; import io.strimzi.operator.cluster.model.ClusterCa; import io.strimzi.operator.cluster.model.CruiseControl; import io.strimzi.operator.cluster.model.ImagePullPolicy; import io.strimzi.operator.cluster.model.KafkaVersion; -import io.strimzi.operator.cluster.model.ModelUtils; import io.strimzi.operator.cluster.model.NodeRef; import io.strimzi.operator.cluster.operator.resource.ResourceOperatorSupplier; import io.strimzi.operator.common.Annotations; @@ -207,7 +207,7 @@ protected Future certificatesSecret(Clock clock) { .compose(patchResult -> { if (patchResult instanceof ReconcileResult.Patched) { // The secret is patched and some changes to the existing certificates actually occurred - existingCertsChanged = ModelUtils.doExistingCertificatesDiffer(oldSecret, patchResult.resource()); + existingCertsChanged = CertUtils.doExistingCertificatesDiffer(oldSecret, patchResult.resource()); } else { existingCertsChanged = false; } @@ -272,10 +272,10 @@ protected Future deployment(boolean isOpenShift, ImagePullPolicy imagePull if (cruiseControl != null) { Deployment deployment = cruiseControl.generateDeployment(isOpenShift, imagePullPolicy, imagePullSecrets); - int caCertGeneration = ModelUtils.caCertGeneration(clusterCa); + int caCertGeneration = clusterCa.caCertGeneration(); Annotations.annotations(deployment.getSpec().getTemplate()).put( Ca.ANNO_STRIMZI_IO_CLUSTER_CA_CERT_GENERATION, String.valueOf(caCertGeneration)); - int caKeyGeneration = ModelUtils.caKeyGeneration(clusterCa); + int caKeyGeneration = clusterCa.caKeyGeneration(); Annotations.annotations(deployment.getSpec().getTemplate()).put( Ca.ANNO_STRIMZI_IO_CLUSTER_CA_KEY_GENERATION, String.valueOf(caKeyGeneration)); diff --git a/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/EntityOperatorReconciler.java b/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/EntityOperatorReconciler.java index 8b2c8d6474d..f50d4cb755f 100644 --- a/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/EntityOperatorReconciler.java +++ b/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/EntityOperatorReconciler.java @@ -10,12 +10,12 @@ import io.strimzi.api.kafka.model.Kafka; import io.strimzi.api.kafka.model.KafkaResources; import io.strimzi.operator.cluster.ClusterOperatorConfig; +import io.strimzi.operator.cluster.model.CertUtils; import io.strimzi.operator.common.model.Ca; import io.strimzi.operator.cluster.model.ClusterCa; import io.strimzi.operator.cluster.model.EntityOperator; import io.strimzi.operator.cluster.model.ImagePullPolicy; import io.strimzi.operator.cluster.model.KafkaVersion; -import io.strimzi.operator.cluster.model.ModelUtils; import io.strimzi.operator.cluster.operator.resource.ResourceOperatorSupplier; import io.strimzi.operator.common.Annotations; import io.strimzi.operator.common.Reconciliation; @@ -349,7 +349,7 @@ protected Future topicOperatorSecret(Clock clock) { .compose(patchResult -> { if (patchResult instanceof ReconcileResult.Patched) { // The secret is patched and some changes to the existing certificates actually occurred - existingEntityTopicOperatorCertsChanged = ModelUtils.doExistingCertificatesDiffer(oldSecret, patchResult.resource()); + existingEntityTopicOperatorCertsChanged = CertUtils.doExistingCertificatesDiffer(oldSecret, patchResult.resource()); } else { existingEntityTopicOperatorCertsChanged = false; } @@ -382,7 +382,7 @@ protected Future userOperatorSecret(Clock clock) { .compose(patchResult -> { if (patchResult instanceof ReconcileResult.Patched) { // The secret is patched and some changes to the existing certificates actually occurred - existingEntityUserOperatorCertsChanged = ModelUtils.doExistingCertificatesDiffer(oldSecret, patchResult.resource()); + existingEntityUserOperatorCertsChanged = CertUtils.doExistingCertificatesDiffer(oldSecret, patchResult.resource()); } else { existingEntityUserOperatorCertsChanged = false; } @@ -423,9 +423,9 @@ protected Future networkPolicy() { protected Future deployment(boolean isOpenShift, ImagePullPolicy imagePullPolicy, List imagePullSecrets) { if (entityOperator != null) { Deployment deployment = entityOperator.generateDeployment(isOpenShift, imagePullPolicy, imagePullSecrets); - int caCertGeneration = ModelUtils.caCertGeneration(clusterCa); + int caCertGeneration = clusterCa.caCertGeneration(); Annotations.annotations(deployment.getSpec().getTemplate()).put(Ca.ANNO_STRIMZI_IO_CLUSTER_CA_CERT_GENERATION, String.valueOf(caCertGeneration)); - int caKeyGeneration = ModelUtils.caKeyGeneration(clusterCa); + int caKeyGeneration = clusterCa.caKeyGeneration(); Annotations.annotations(deployment.getSpec().getTemplate()).put(Ca.ANNO_STRIMZI_IO_CLUSTER_CA_KEY_GENERATION, String.valueOf(caKeyGeneration)); return deploymentOperator diff --git a/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/KafkaExporterReconciler.java b/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/KafkaExporterReconciler.java index 5a78b07a78c..4380430b15a 100644 --- a/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/KafkaExporterReconciler.java +++ b/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/KafkaExporterReconciler.java @@ -9,12 +9,12 @@ import io.strimzi.api.kafka.model.Kafka; import io.strimzi.api.kafka.model.KafkaExporterResources; import io.strimzi.operator.cluster.ClusterOperatorConfig; +import io.strimzi.operator.cluster.model.CertUtils; import io.strimzi.operator.common.model.Ca; import io.strimzi.operator.cluster.model.ClusterCa; import io.strimzi.operator.cluster.model.ImagePullPolicy; import io.strimzi.operator.cluster.model.KafkaExporter; import io.strimzi.operator.cluster.model.KafkaVersion; -import io.strimzi.operator.cluster.model.ModelUtils; import io.strimzi.operator.cluster.operator.resource.ResourceOperatorSupplier; import io.strimzi.operator.common.Annotations; import io.strimzi.operator.common.Reconciliation; @@ -134,7 +134,7 @@ private Future certificatesSecret(Clock clock) { .compose(patchResult -> { if (patchResult instanceof ReconcileResult.Patched) { // The secret is patched and some changes to the existing certificates actually occurred - existingKafkaExporterCertsChanged = ModelUtils.doExistingCertificatesDiffer(oldSecret, patchResult.resource()); + existingKafkaExporterCertsChanged = CertUtils.doExistingCertificatesDiffer(oldSecret, patchResult.resource()); } else { existingKafkaExporterCertsChanged = false; } @@ -180,10 +180,10 @@ protected Future networkPolicy() { private Future deployment(boolean isOpenShift, ImagePullPolicy imagePullPolicy, List imagePullSecrets) { if (kafkaExporter != null) { Deployment deployment = kafkaExporter.generateDeployment(isOpenShift, imagePullPolicy, imagePullSecrets); - int caCertGeneration = ModelUtils.caCertGeneration(this.clusterCa); + int caCertGeneration = clusterCa.caCertGeneration(); Annotations.annotations(deployment.getSpec().getTemplate()).put( Ca.ANNO_STRIMZI_IO_CLUSTER_CA_CERT_GENERATION, String.valueOf(caCertGeneration)); - int caKeyGeneration = ModelUtils.caKeyGeneration(clusterCa); + int caKeyGeneration = clusterCa.caKeyGeneration(); Annotations.annotations(deployment.getSpec().getTemplate()).put( Ca.ANNO_STRIMZI_IO_CLUSTER_CA_KEY_GENERATION, String.valueOf(caKeyGeneration)); diff --git a/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/KafkaReconciler.java b/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/KafkaReconciler.java index 1abec84f9e4..27d11ace9fc 100644 --- a/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/KafkaReconciler.java +++ b/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/KafkaReconciler.java @@ -42,7 +42,6 @@ import io.strimzi.operator.cluster.model.KafkaPool; import io.strimzi.operator.cluster.model.KafkaVersionChange; import io.strimzi.operator.cluster.model.ListenersUtils; -import io.strimzi.operator.cluster.model.ModelUtils; import io.strimzi.operator.cluster.model.NodeRef; import io.strimzi.operator.common.model.NodeUtils; import io.strimzi.operator.cluster.model.PodSetUtils; @@ -722,7 +721,7 @@ protected Future certificateSecret(Clock clock) { kafkaServerCertificateHash.put( node.nodeId(), CertUtils.getCertificateThumbprint(patchResult.resource(), - ClusterCa.secretEntryNameForPod(node.podName(), Ca.SecretEntry.CRT) + Ca.SecretEntry.CRT.asKey(node.podName()) )); } } @@ -762,9 +761,9 @@ protected Future podDisruptionBudget() { */ private Map podSetPodAnnotations(int nodeId) { Map podAnnotations = new LinkedHashMap<>(9); - podAnnotations.put(Ca.ANNO_STRIMZI_IO_CLUSTER_CA_CERT_GENERATION, String.valueOf(ModelUtils.caCertGeneration(this.clusterCa))); - podAnnotations.put(Ca.ANNO_STRIMZI_IO_CLUSTER_CA_KEY_GENERATION, String.valueOf(ModelUtils.caKeyGeneration(this.clusterCa))); - podAnnotations.put(Ca.ANNO_STRIMZI_IO_CLIENTS_CA_CERT_GENERATION, String.valueOf(ModelUtils.caCertGeneration(this.clientsCa))); + podAnnotations.put(Ca.ANNO_STRIMZI_IO_CLUSTER_CA_CERT_GENERATION, String.valueOf(this.clusterCa.caCertGeneration())); + podAnnotations.put(Ca.ANNO_STRIMZI_IO_CLUSTER_CA_KEY_GENERATION, String.valueOf(this.clusterCa.caKeyGeneration())); + podAnnotations.put(Ca.ANNO_STRIMZI_IO_CLIENTS_CA_CERT_GENERATION, String.valueOf(this.clientsCa.caCertGeneration())); podAnnotations.put(Annotations.ANNO_STRIMZI_LOGGING_APPENDERS_HASH, loggingHash); podAnnotations.put(KafkaCluster.ANNO_STRIMZI_BROKER_CONFIGURATION_HASH, brokerConfigurationHash.get(nodeId)); podAnnotations.put(ANNO_STRIMZI_IO_KAFKA_VERSION, kafka.getKafkaVersion().version()); diff --git a/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/ReconcilerUtils.java b/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/ReconcilerUtils.java index 0dda34a281d..67e743df8da 100644 --- a/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/ReconcilerUtils.java +++ b/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/ReconcilerUtils.java @@ -15,7 +15,6 @@ import io.strimzi.operator.common.model.Ca; import io.strimzi.operator.common.model.ClientsCa; import io.strimzi.operator.cluster.model.KafkaCluster; -import io.strimzi.operator.cluster.model.ModelUtils; import io.strimzi.operator.cluster.model.NodeRef; import io.strimzi.operator.cluster.model.PodSetUtils; import io.strimzi.operator.cluster.model.RestartReason; @@ -193,7 +192,7 @@ public static RestartReasons reasonsToRestartPod(Reconciliation reconciliation, * @return True when the generations match, false otherwise */ private static boolean isPodCaCertUpToDate(Pod pod, Ca ca) { - return ModelUtils.caCertGeneration(ca) == Annotations.intAnnotation(pod, getCaCertAnnotation(ca), Ca.INIT_GENERATION); + return ca.caCertGeneration() == Annotations.intAnnotation(pod, getCaCertAnnotation(ca), Ca.INIT_GENERATION); } /** diff --git a/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/ZooKeeperReconciler.java b/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/ZooKeeperReconciler.java index 76d1fe4399a..b59e62ed9b1 100644 --- a/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/ZooKeeperReconciler.java +++ b/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/ZooKeeperReconciler.java @@ -22,7 +22,6 @@ import io.strimzi.operator.cluster.model.DnsNameGenerator; import io.strimzi.operator.cluster.model.ImagePullPolicy; import io.strimzi.operator.cluster.model.KafkaVersionChange; -import io.strimzi.operator.cluster.model.ModelUtils; import io.strimzi.operator.cluster.model.ZookeeperCluster; import io.strimzi.operator.cluster.operator.resource.ResourceOperatorSupplier; import io.strimzi.operator.cluster.operator.resource.StatefulSetOperator; @@ -421,7 +420,7 @@ protected Future certificateSecret(Clock clock) { zkCertificateHash.put( podNum, CertUtils.getCertificateThumbprint(patchResult.resource(), - ClusterCa.secretEntryNameForPod(podName, Ca.SecretEntry.CRT) + Ca.SecretEntry.CRT.asKey(podName) )); } } @@ -514,8 +513,8 @@ private Future podSet(int replicas) { */ public Map zkPodSetPodAnnotations(int podNum) { Map podAnnotations = new LinkedHashMap<>((int) Math.ceil(podNum / 0.75)); - podAnnotations.put(Ca.ANNO_STRIMZI_IO_CLUSTER_CA_CERT_GENERATION, String.valueOf(ModelUtils.caCertGeneration(this.clusterCa))); - podAnnotations.put(Ca.ANNO_STRIMZI_IO_CLUSTER_CA_KEY_GENERATION, String.valueOf(ModelUtils.caKeyGeneration(this.clusterCa))); + podAnnotations.put(Ca.ANNO_STRIMZI_IO_CLUSTER_CA_CERT_GENERATION, String.valueOf(this.clusterCa.caCertGeneration())); + podAnnotations.put(Ca.ANNO_STRIMZI_IO_CLUSTER_CA_KEY_GENERATION, String.valueOf(this.clusterCa.caKeyGeneration())); podAnnotations.put(Annotations.ANNO_STRIMZI_LOGGING_HASH, loggingHash); podAnnotations.put(ANNO_STRIMZI_SERVER_CERT_HASH, zkCertificateHash.get(podNum)); return podAnnotations; diff --git a/cluster-operator/src/test/java/io/strimzi/operator/cluster/model/CertUtilsTest.java b/cluster-operator/src/test/java/io/strimzi/operator/cluster/model/CertUtilsTest.java new file mode 100644 index 00000000000..0c9ffcb62f2 --- /dev/null +++ b/cluster-operator/src/test/java/io/strimzi/operator/cluster/model/CertUtilsTest.java @@ -0,0 +1,109 @@ +/* + * Copyright Strimzi authors. + * License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html). + */ +package io.strimzi.operator.cluster.model; + +import io.fabric8.kubernetes.api.model.Secret; +import io.fabric8.kubernetes.api.model.SecretBuilder; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; + +public class CertUtilsTest { + @Test + public void testExistingCertificatesDiffer() { + Secret defaultSecret = new SecretBuilder() + .withNewMetadata() + .withName("my-secret") + .endMetadata() + .addToData("my-cluster-kafka-0.crt", "Certificate0") + .addToData("my-cluster-kafka-0.key", "Key0") + .addToData("my-cluster-kafka-1.crt", "Certificate1") + .addToData("my-cluster-kafka-1.key", "Key1") + .addToData("my-cluster-kafka-2.crt", "Certificate2") + .addToData("my-cluster-kafka-2.key", "Key2") + .build(); + + Secret sameAsDefaultSecret = new SecretBuilder() + .withNewMetadata() + .withName("my-secret") + .endMetadata() + .addToData("my-cluster-kafka-0.crt", "Certificate0") + .addToData("my-cluster-kafka-0.key", "Key0") + .addToData("my-cluster-kafka-1.crt", "Certificate1") + .addToData("my-cluster-kafka-1.key", "Key1") + .addToData("my-cluster-kafka-2.crt", "Certificate2") + .addToData("my-cluster-kafka-2.key", "Key2") + .build(); + + Secret scaleDownSecret = new SecretBuilder() + .withNewMetadata() + .withName("my-secret") + .endMetadata() + .addToData("my-cluster-kafka-0.crt", "Certificate0") + .addToData("my-cluster-kafka-0.key", "Key0") + .build(); + + Secret scaleUpSecret = new SecretBuilder() + .withNewMetadata() + .withName("my-secret") + .endMetadata() + .addToData("my-cluster-kafka-0.crt", "Certificate0") + .addToData("my-cluster-kafka-0.key", "Key0") + .addToData("my-cluster-kafka-1.crt", "Certificate1") + .addToData("my-cluster-kafka-1.key", "Key1") + .addToData("my-cluster-kafka-2.crt", "Certificate2") + .addToData("my-cluster-kafka-2.key", "Key2") + .addToData("my-cluster-kafka-3.crt", "Certificate3") + .addToData("my-cluster-kafka-3.key", "Key3") + .addToData("my-cluster-kafka-4.crt", "Certificate4") + .addToData("my-cluster-kafka-4.key", "Key4") + .build(); + + Secret changedSecret = new SecretBuilder() + .withNewMetadata() + .withName("my-secret") + .endMetadata() + .addToData("my-cluster-kafka-0.crt", "Certificate0") + .addToData("my-cluster-kafka-0.key", "Key0") + .addToData("my-cluster-kafka-1.crt", "Certificate1") + .addToData("my-cluster-kafka-1.key", "NewKey1") + .addToData("my-cluster-kafka-2.crt", "Certificate2") + .addToData("my-cluster-kafka-2.key", "Key2") + .build(); + + Secret changedScaleUpSecret = new SecretBuilder() + .withNewMetadata() + .withName("my-secret") + .endMetadata() + .addToData("my-cluster-kafka-0.crt", "Certificate0") + .addToData("my-cluster-kafka-0.key", "Key0") + .addToData("my-cluster-kafka-1.crt", "Certificate1") + .addToData("my-cluster-kafka-1.key", "Key1") + .addToData("my-cluster-kafka-2.crt", "NewCertificate2") + .addToData("my-cluster-kafka-2.key", "Key2") + .addToData("my-cluster-kafka-3.crt", "Certificate3") + .addToData("my-cluster-kafka-3.key", "Key3") + .addToData("my-cluster-kafka-4.crt", "Certificate4") + .addToData("my-cluster-kafka-4.key", "Key4") + .build(); + + Secret changedScaleDownSecret = new SecretBuilder() + .withNewMetadata() + .withName("my-secret") + .endMetadata() + .addToData("my-cluster-kafka-0.crt", "NewCertificate0") + .addToData("my-cluster-kafka-0.key", "NewKey0") + .build(); + + assertThat(CertUtils.doExistingCertificatesDiffer(defaultSecret, defaultSecret), is(false)); + assertThat(CertUtils.doExistingCertificatesDiffer(defaultSecret, sameAsDefaultSecret), is(false)); + assertThat(CertUtils.doExistingCertificatesDiffer(defaultSecret, scaleDownSecret), is(false)); + assertThat(CertUtils.doExistingCertificatesDiffer(defaultSecret, scaleUpSecret), is(false)); + assertThat(CertUtils.doExistingCertificatesDiffer(defaultSecret, changedSecret), is(true)); + assertThat(CertUtils.doExistingCertificatesDiffer(defaultSecret, changedScaleUpSecret), is(true)); + assertThat(CertUtils.doExistingCertificatesDiffer(defaultSecret, changedScaleDownSecret), is(true)); + } +} diff --git a/cluster-operator/src/test/java/io/strimzi/operator/cluster/model/ModelUtilsTest.java b/cluster-operator/src/test/java/io/strimzi/operator/cluster/model/ModelUtilsTest.java index af4b0735780..09d95452ad0 100644 --- a/cluster-operator/src/test/java/io/strimzi/operator/cluster/model/ModelUtilsTest.java +++ b/cluster-operator/src/test/java/io/strimzi/operator/cluster/model/ModelUtilsTest.java @@ -9,8 +9,6 @@ import io.fabric8.kubernetes.api.model.OwnerReferenceBuilder; import io.fabric8.kubernetes.api.model.Pod; import io.fabric8.kubernetes.api.model.PodBuilder; -import io.fabric8.kubernetes.api.model.Secret; -import io.fabric8.kubernetes.api.model.SecretBuilder; import io.strimzi.api.kafka.model.Kafka; import io.strimzi.api.kafka.model.KafkaBuilder; import io.strimzi.api.kafka.model.StrimziPodSet; @@ -88,101 +86,6 @@ public void testStorageSerializationAndDeserialization() { assertThat(ModelUtils.decodeStorageFromJson(ModelUtils.encodeStorageToJson(persistent)), is(persistent)); } - @ParallelTest - public void testExistingCertificatesDiffer() { - Secret defaultSecret = new SecretBuilder() - .withNewMetadata() - .withName("my-secret") - .endMetadata() - .addToData("my-cluster-kafka-0.crt", "Certificate0") - .addToData("my-cluster-kafka-0.key", "Key0") - .addToData("my-cluster-kafka-1.crt", "Certificate1") - .addToData("my-cluster-kafka-1.key", "Key1") - .addToData("my-cluster-kafka-2.crt", "Certificate2") - .addToData("my-cluster-kafka-2.key", "Key2") - .build(); - - Secret sameAsDefaultSecret = new SecretBuilder() - .withNewMetadata() - .withName("my-secret") - .endMetadata() - .addToData("my-cluster-kafka-0.crt", "Certificate0") - .addToData("my-cluster-kafka-0.key", "Key0") - .addToData("my-cluster-kafka-1.crt", "Certificate1") - .addToData("my-cluster-kafka-1.key", "Key1") - .addToData("my-cluster-kafka-2.crt", "Certificate2") - .addToData("my-cluster-kafka-2.key", "Key2") - .build(); - - Secret scaleDownSecret = new SecretBuilder() - .withNewMetadata() - .withName("my-secret") - .endMetadata() - .addToData("my-cluster-kafka-0.crt", "Certificate0") - .addToData("my-cluster-kafka-0.key", "Key0") - .build(); - - Secret scaleUpSecret = new SecretBuilder() - .withNewMetadata() - .withName("my-secret") - .endMetadata() - .addToData("my-cluster-kafka-0.crt", "Certificate0") - .addToData("my-cluster-kafka-0.key", "Key0") - .addToData("my-cluster-kafka-1.crt", "Certificate1") - .addToData("my-cluster-kafka-1.key", "Key1") - .addToData("my-cluster-kafka-2.crt", "Certificate2") - .addToData("my-cluster-kafka-2.key", "Key2") - .addToData("my-cluster-kafka-3.crt", "Certificate3") - .addToData("my-cluster-kafka-3.key", "Key3") - .addToData("my-cluster-kafka-4.crt", "Certificate4") - .addToData("my-cluster-kafka-4.key", "Key4") - .build(); - - Secret changedSecret = new SecretBuilder() - .withNewMetadata() - .withName("my-secret") - .endMetadata() - .addToData("my-cluster-kafka-0.crt", "Certificate0") - .addToData("my-cluster-kafka-0.key", "Key0") - .addToData("my-cluster-kafka-1.crt", "Certificate1") - .addToData("my-cluster-kafka-1.key", "NewKey1") - .addToData("my-cluster-kafka-2.crt", "Certificate2") - .addToData("my-cluster-kafka-2.key", "Key2") - .build(); - - Secret changedScaleUpSecret = new SecretBuilder() - .withNewMetadata() - .withName("my-secret") - .endMetadata() - .addToData("my-cluster-kafka-0.crt", "Certificate0") - .addToData("my-cluster-kafka-0.key", "Key0") - .addToData("my-cluster-kafka-1.crt", "Certificate1") - .addToData("my-cluster-kafka-1.key", "Key1") - .addToData("my-cluster-kafka-2.crt", "NewCertificate2") - .addToData("my-cluster-kafka-2.key", "Key2") - .addToData("my-cluster-kafka-3.crt", "Certificate3") - .addToData("my-cluster-kafka-3.key", "Key3") - .addToData("my-cluster-kafka-4.crt", "Certificate4") - .addToData("my-cluster-kafka-4.key", "Key4") - .build(); - - Secret changedScaleDownSecret = new SecretBuilder() - .withNewMetadata() - .withName("my-secret") - .endMetadata() - .addToData("my-cluster-kafka-0.crt", "NewCertificate0") - .addToData("my-cluster-kafka-0.key", "NewKey0") - .build(); - - assertThat(ModelUtils.doExistingCertificatesDiffer(defaultSecret, defaultSecret), is(false)); - assertThat(ModelUtils.doExistingCertificatesDiffer(defaultSecret, sameAsDefaultSecret), is(false)); - assertThat(ModelUtils.doExistingCertificatesDiffer(defaultSecret, scaleDownSecret), is(false)); - assertThat(ModelUtils.doExistingCertificatesDiffer(defaultSecret, scaleUpSecret), is(false)); - assertThat(ModelUtils.doExistingCertificatesDiffer(defaultSecret, changedSecret), is(true)); - assertThat(ModelUtils.doExistingCertificatesDiffer(defaultSecret, changedScaleUpSecret), is(true)); - assertThat(ModelUtils.doExistingCertificatesDiffer(defaultSecret, changedScaleDownSecret), is(true)); - } - @ParallelTest public void testCreateOwnerReference() { Kafka owner = new KafkaBuilder() diff --git a/cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/CertificateRenewalTest.java b/cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/CertificateRenewalTest.java index 03d7b8c906c..922a02eb327 100644 --- a/cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/CertificateRenewalTest.java +++ b/cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/CertificateRenewalTest.java @@ -8,8 +8,8 @@ import io.fabric8.kubernetes.api.model.Secret; import io.fabric8.kubernetes.api.model.SecretBuilder; import io.strimzi.certs.CertAndKey; +import io.strimzi.operator.cluster.model.CertUtils; import io.strimzi.operator.cluster.model.ClusterCa; -import io.strimzi.operator.cluster.model.ModelUtils; import io.strimzi.operator.common.Reconciliation; import io.strimzi.operator.common.model.Labels; import io.vertx.junit5.VertxExtension; @@ -18,7 +18,9 @@ import java.io.IOException; import java.util.Base64; +import java.util.Map; +import static io.strimzi.operator.common.model.Ca.ANNO_STRIMZI_IO_CLUSTER_CA_CERT_GENERATION; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasEntry; import static org.mockito.ArgumentMatchers.any; @@ -33,6 +35,7 @@ public void testRenewalOfDeploymentCertificatesWithNullSecret() throws IOExcepti CertAndKey newCertAndKey = new CertAndKey("new-key".getBytes(), "new-cert".getBytes(), "new-truststore".getBytes(), "new-keystore".getBytes(), "new-password"); ClusterCa clusterCaMock = mock(ClusterCa.class); when(clusterCaMock.generateSignedCert(anyString(), anyString())).thenReturn(newCertAndKey); + when(clusterCaMock.caCertGenerationFullAnnotation()).thenReturn(Map.entry(ANNO_STRIMZI_IO_CLUSTER_CA_CERT_GENERATION, "1")); String namespace = "my-namespace"; String secretName = "my-secret"; String commonName = "deployment"; @@ -40,7 +43,7 @@ public void testRenewalOfDeploymentCertificatesWithNullSecret() throws IOExcepti Labels labels = Labels.forStrimziCluster("my-cluster"); OwnerReference ownerReference = new OwnerReference(); - Secret newSecret = ModelUtils.buildSecret(Reconciliation.DUMMY_RECONCILIATION, clusterCaMock, null, namespace, secretName, commonName, + Secret newSecret = CertUtils.buildTrustedCertificateSecret(Reconciliation.DUMMY_RECONCILIATION, clusterCaMock, null, namespace, secretName, commonName, keyCertName, labels, ownerReference, true); assertThat(newSecret.getData(), hasEntry("deployment.crt", newCertAndKey.certAsBase64String())); @@ -66,6 +69,7 @@ public void testRenewalOfDeploymentCertificatesWithRenewingCa() throws IOExcepti when(clusterCaMock.certRenewed()).thenReturn(true); when(clusterCaMock.isExpiring(any(), any())).thenReturn(false); when(clusterCaMock.generateSignedCert(anyString(), anyString())).thenReturn(newCertAndKey); + when(clusterCaMock.caCertGenerationFullAnnotation()).thenReturn(Map.entry(ANNO_STRIMZI_IO_CLUSTER_CA_CERT_GENERATION, "1")); String namespace = "my-namespace"; String secretName = "my-secret"; String commonName = "deployment"; @@ -73,7 +77,7 @@ public void testRenewalOfDeploymentCertificatesWithRenewingCa() throws IOExcepti Labels labels = Labels.forStrimziCluster("my-cluster"); OwnerReference ownerReference = new OwnerReference(); - Secret newSecret = ModelUtils.buildSecret(Reconciliation.DUMMY_RECONCILIATION, clusterCaMock, initialSecret, namespace, secretName, commonName, + Secret newSecret = CertUtils.buildTrustedCertificateSecret(Reconciliation.DUMMY_RECONCILIATION, clusterCaMock, initialSecret, namespace, secretName, commonName, keyCertName, labels, ownerReference, true); assertThat(newSecret.getData(), hasEntry("deployment.crt", newCertAndKey.certAsBase64String())); @@ -99,6 +103,7 @@ public void testRenewalOfDeploymentCertificatesDelayedRenewal() throws IOExcepti when(clusterCaMock.certRenewed()).thenReturn(false); when(clusterCaMock.isExpiring(any(), any())).thenReturn(true); when(clusterCaMock.generateSignedCert(anyString(), anyString())).thenReturn(newCertAndKey); + when(clusterCaMock.caCertGenerationFullAnnotation()).thenReturn(Map.entry(ANNO_STRIMZI_IO_CLUSTER_CA_CERT_GENERATION, "1")); String namespace = "my-namespace"; String secretName = "my-secret"; String commonName = "deployment"; @@ -106,7 +111,7 @@ public void testRenewalOfDeploymentCertificatesDelayedRenewal() throws IOExcepti Labels labels = Labels.forStrimziCluster("my-cluster"); OwnerReference ownerReference = new OwnerReference(); - Secret newSecret = ModelUtils.buildSecret(Reconciliation.DUMMY_RECONCILIATION, clusterCaMock, initialSecret, namespace, secretName, commonName, + Secret newSecret = CertUtils.buildTrustedCertificateSecret(Reconciliation.DUMMY_RECONCILIATION, clusterCaMock, initialSecret, namespace, secretName, commonName, keyCertName, labels, ownerReference, true); assertThat(newSecret.getData(), hasEntry("deployment.crt", newCertAndKey.certAsBase64String())); @@ -132,6 +137,7 @@ public void testRenewalOfDeploymentCertificatesDelayedRenewalOutsideOfMaintenanc when(clusterCaMock.certRenewed()).thenReturn(false); when(clusterCaMock.isExpiring(any(), any())).thenReturn(true); when(clusterCaMock.generateSignedCert(anyString(), anyString())).thenReturn(newCertAndKey); + when(clusterCaMock.caCertGenerationFullAnnotation()).thenReturn(Map.entry(ANNO_STRIMZI_IO_CLUSTER_CA_CERT_GENERATION, "1")); String namespace = "my-namespace"; String secretName = "my-secret"; String commonName = "deployment"; @@ -139,7 +145,7 @@ public void testRenewalOfDeploymentCertificatesDelayedRenewalOutsideOfMaintenanc Labels labels = Labels.forStrimziCluster("my-cluster"); OwnerReference ownerReference = new OwnerReference(); - Secret newSecret = ModelUtils.buildSecret(Reconciliation.DUMMY_RECONCILIATION, clusterCaMock, initialSecret, namespace, secretName, commonName, + Secret newSecret = CertUtils.buildTrustedCertificateSecret(Reconciliation.DUMMY_RECONCILIATION, clusterCaMock, initialSecret, namespace, secretName, commonName, keyCertName, labels, ownerReference, false); assertThat(newSecret.getData(), hasEntry("deployment.crt", Base64.getEncoder().encodeToString("old-cert".getBytes()))); diff --git a/cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/KafkaAssemblyOperatorMockTest.java b/cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/KafkaAssemblyOperatorMockTest.java index e1c229ad473..2c4ec4270b6 100644 --- a/cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/KafkaAssemblyOperatorMockTest.java +++ b/cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/KafkaAssemblyOperatorMockTest.java @@ -28,7 +28,6 @@ import io.strimzi.operator.cluster.ResourceUtils; import io.strimzi.operator.common.model.Ca; import io.strimzi.operator.cluster.model.CertUtils; -import io.strimzi.operator.cluster.model.ClusterCa; import io.strimzi.operator.cluster.model.KafkaVersion; import io.strimzi.operator.cluster.model.PodSetUtils; import io.strimzi.operator.cluster.operator.resource.ResourceOperatorSupplier; @@ -197,7 +196,7 @@ private Future initialReconcile(VertxTestContext context) { assertThat(pod.getMetadata().getAnnotations(), hasEntry(Ca.ANNO_STRIMZI_IO_CLUSTER_CA_KEY_GENERATION, "0")); var brokersSecret = client.secrets().inNamespace(NAMESPACE).withName(KafkaResources.kafkaSecretName(CLUSTER_NAME)).get(); assertThat(pod.getMetadata().getAnnotations(), hasEntry(Annotations.ANNO_STRIMZI_SERVER_CERT_HASH, - CertUtils.getCertificateThumbprint(brokersSecret, ClusterCa.secretEntryNameForPod(pod.getMetadata().getName(), Ca.SecretEntry.CRT)) + CertUtils.getCertificateThumbprint(brokersSecret, Ca.SecretEntry.CRT.asKey(pod.getMetadata().getName())) )); }); @@ -207,7 +206,7 @@ private Future initialReconcile(VertxTestContext context) { assertThat(pod.getMetadata().getAnnotations(), hasEntry(Ca.ANNO_STRIMZI_IO_CLUSTER_CA_KEY_GENERATION, "0")); var zooKeeperSecret = client.secrets().inNamespace(NAMESPACE).withName(KafkaResources.zookeeperSecretName(CLUSTER_NAME)).get(); assertThat(pod.getMetadata().getAnnotations(), hasEntry(Annotations.ANNO_STRIMZI_SERVER_CERT_HASH, - CertUtils.getCertificateThumbprint(zooKeeperSecret, ClusterCa.secretEntryNameForPod(pod.getMetadata().getName(), Ca.SecretEntry.CRT)) + CertUtils.getCertificateThumbprint(zooKeeperSecret, Ca.SecretEntry.CRT.asKey(pod.getMetadata().getName())) )); }); assertThat(client.configMaps().inNamespace(NAMESPACE).withName(KafkaResources.zookeeperMetricsAndLogConfigMapName(CLUSTER_NAME)).get(), is(notNullValue())); diff --git a/cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/KafkaAssemblyOperatorWithPoolsMockTest.java b/cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/KafkaAssemblyOperatorWithPoolsMockTest.java index 3ef0beb9e52..e47abe50060 100644 --- a/cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/KafkaAssemblyOperatorWithPoolsMockTest.java +++ b/cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/KafkaAssemblyOperatorWithPoolsMockTest.java @@ -28,7 +28,6 @@ import io.strimzi.operator.cluster.ResourceUtils; import io.strimzi.operator.common.model.Ca; import io.strimzi.operator.cluster.model.CertUtils; -import io.strimzi.operator.cluster.model.ClusterCa; import io.strimzi.operator.cluster.model.KafkaVersion; import io.strimzi.operator.cluster.model.PodSetUtils; import io.strimzi.operator.cluster.operator.resource.ResourceOperatorSupplier; @@ -201,7 +200,7 @@ private Future initialReconcile(VertxTestContext context) { assertThat(pod.getMetadata().getAnnotations(), hasEntry(Ca.ANNO_STRIMZI_IO_CLUSTER_CA_CERT_GENERATION, "0")); var brokersSecret = client.secrets().inNamespace(NAMESPACE).withName(KafkaResources.kafkaSecretName(CLUSTER_NAME)).get(); assertThat(pod.getMetadata().getAnnotations(), hasEntry(Annotations.ANNO_STRIMZI_SERVER_CERT_HASH, - CertUtils.getCertificateThumbprint(brokersSecret, ClusterCa.secretEntryNameForPod(pod.getMetadata().getName(), Ca.SecretEntry.CRT)) + CertUtils.getCertificateThumbprint(brokersSecret, Ca.SecretEntry.CRT.asKey(pod.getMetadata().getName())) )); }); @@ -213,7 +212,7 @@ private Future initialReconcile(VertxTestContext context) { assertThat(pod.getMetadata().getAnnotations(), hasEntry(Ca.ANNO_STRIMZI_IO_CLUSTER_CA_CERT_GENERATION, "0")); var brokersSecret = client.secrets().inNamespace(NAMESPACE).withName(KafkaResources.kafkaSecretName(CLUSTER_NAME)).get(); assertThat(pod.getMetadata().getAnnotations(), hasEntry(Annotations.ANNO_STRIMZI_SERVER_CERT_HASH, - CertUtils.getCertificateThumbprint(brokersSecret, ClusterCa.secretEntryNameForPod(pod.getMetadata().getName(), Ca.SecretEntry.CRT)) + CertUtils.getCertificateThumbprint(brokersSecret, Ca.SecretEntry.CRT.asKey(pod.getMetadata().getName())) )); }); @@ -222,7 +221,7 @@ private Future initialReconcile(VertxTestContext context) { assertThat(pod.getMetadata().getAnnotations(), hasEntry(Ca.ANNO_STRIMZI_IO_CLUSTER_CA_CERT_GENERATION, "0")); var zooKeeperSecret = client.secrets().inNamespace(NAMESPACE).withName(KafkaResources.zookeeperSecretName(CLUSTER_NAME)).get(); assertThat(pod.getMetadata().getAnnotations(), hasEntry(Annotations.ANNO_STRIMZI_SERVER_CERT_HASH, - CertUtils.getCertificateThumbprint(zooKeeperSecret, ClusterCa.secretEntryNameForPod(pod.getMetadata().getName(), Ca.SecretEntry.CRT)) + CertUtils.getCertificateThumbprint(zooKeeperSecret, Ca.SecretEntry.CRT.asKey(pod.getMetadata().getName())) )); }); assertThat(client.configMaps().inNamespace(NAMESPACE).withName(KafkaResources.zookeeperMetricsAndLogConfigMapName(CLUSTER_NAME)).get(), is(notNullValue())); diff --git a/cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/PartialRollingUpdateMockTest.java b/cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/PartialRollingUpdateMockTest.java index 1241a9de50c..0bfad901f47 100644 --- a/cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/PartialRollingUpdateMockTest.java +++ b/cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/PartialRollingUpdateMockTest.java @@ -21,7 +21,6 @@ import io.strimzi.operator.cluster.ResourceUtils; import io.strimzi.operator.common.model.Ca; import io.strimzi.operator.cluster.model.CertUtils; -import io.strimzi.operator.cluster.model.ClusterCa; import io.strimzi.operator.cluster.model.KafkaVersion; import io.strimzi.operator.cluster.model.PodSetUtils; import io.strimzi.operator.cluster.model.PodRevision; @@ -228,7 +227,7 @@ public void testReconcileOfPartiallyRolledKafkaClusterForServerCertificates(Vert for (int brokerId = 0; brokerId < cluster.getSpec().getKafka().getReplicas(); brokerId++) { var pod = client.pods().inNamespace(NAMESPACE).withName(KafkaResources.kafkaPodName(CLUSTER_NAME, brokerId)).get(); var podCertHash = pod.getMetadata().getAnnotations().get(Annotations.ANNO_STRIMZI_SERVER_CERT_HASH); - var expectedCertHash = CertUtils.getCertificateThumbprint(brokersSecret, ClusterCa.secretEntryNameForPod(pod.getMetadata().getName(), Ca.SecretEntry.CRT)); + var expectedCertHash = CertUtils.getCertificateThumbprint(brokersSecret, Ca.SecretEntry.CRT.asKey(pod.getMetadata().getName())); assertThat("Pod " + brokerId + " had unexpected revision", podCertHash, is(expectedCertHash)); } @@ -244,7 +243,7 @@ public void testReconcileOfPartiallyRolledKafkaClusterForServerCertificates(Vert final var finalBrokerId = brokerId; var pod = client.pods().inNamespace(NAMESPACE).withName(KafkaResources.kafkaPodName(CLUSTER_NAME, brokerId)).get(); var podCertHash = pod.getMetadata().getAnnotations().get(Annotations.ANNO_STRIMZI_SERVER_CERT_HASH); - var expectedCertHash = CertUtils.getCertificateThumbprint(brokersSecret, ClusterCa.secretEntryNameForPod(pod.getMetadata().getName(), Ca.SecretEntry.CRT)); + var expectedCertHash = CertUtils.getCertificateThumbprint(brokersSecret, Ca.SecretEntry.CRT.asKey(pod.getMetadata().getName())); context.verify(() -> assertThat("Pod " + finalBrokerId + " had unexpected revision", podCertHash, is(expectedCertHash))); } @@ -293,7 +292,7 @@ public void testReconcileOfPartiallyRolledZookeeperClusterForServerCertificates( for (int zkIndex = 0; zkIndex < cluster.getSpec().getZookeeper().getReplicas(); zkIndex++) { var pod = client.pods().inNamespace(NAMESPACE).withName(KafkaResources.zookeeperPodName(CLUSTER_NAME, zkIndex)).get(); var podCertHash = pod.getMetadata().getAnnotations().get(Annotations.ANNO_STRIMZI_SERVER_CERT_HASH); - var expectedCertHash = CertUtils.getCertificateThumbprint(zkSecret, ClusterCa.secretEntryNameForPod(pod.getMetadata().getName(), Ca.SecretEntry.CRT)); + var expectedCertHash = CertUtils.getCertificateThumbprint(zkSecret, Ca.SecretEntry.CRT.asKey(pod.getMetadata().getName())); assertThat("Pod " + zkIndex + " had unexpected revision", podCertHash, is(expectedCertHash)); } @@ -309,7 +308,7 @@ public void testReconcileOfPartiallyRolledZookeeperClusterForServerCertificates( final var finalZkIndex = zkIndex; var pod = client.pods().inNamespace(NAMESPACE).withName(KafkaResources.zookeeperPodName(CLUSTER_NAME, zkIndex)).get(); var podCertHash = pod.getMetadata().getAnnotations().get(Annotations.ANNO_STRIMZI_SERVER_CERT_HASH); - var expectedCertHash = CertUtils.getCertificateThumbprint(zkSecret, ClusterCa.secretEntryNameForPod(pod.getMetadata().getName(), Ca.SecretEntry.CRT)); + var expectedCertHash = CertUtils.getCertificateThumbprint(zkSecret, Ca.SecretEntry.CRT.asKey(pod.getMetadata().getName())); context.verify(() -> assertThat("Pod " + finalZkIndex + " had unexpected revision", podCertHash, is(expectedCertHash))); } diff --git a/operator-common/src/main/java/io/strimzi/operator/common/model/Ca.java b/operator-common/src/main/java/io/strimzi/operator/common/model/Ca.java index 8a6ab20471b..c8c69c449f1 100644 --- a/operator-common/src/main/java/io/strimzi/operator/common/model/Ca.java +++ b/operator-common/src/main/java/io/strimzi/operator/common/model/Ca.java @@ -62,7 +62,7 @@ public abstract class Ca { /** - * A certificate entry in a ConfigMap. Each entry contains an entry name and data. + * A certificate entry in a Kubernetes Secret. Used to construct the keys in the Secret data where certificates are stored. */ public enum SecretEntry { /** @@ -89,10 +89,22 @@ public enum SecretEntry { } /** - * @return The suffix of the entry name in the Secret + * Build the Kubernetes Secret key to use with this type of SecretEntry. + * + * @param prefix to use for the certificate Secret key + * @return a certificate Secret key with the provided prefix and the suffix of this type of SecretEntry + */ + public String asKey(String prefix) { + return prefix + suffix; + } + + /** + * + * @param key to check the type of + * @return whether this key has the correct suffix for the entry */ - public String getSuffix() { - return suffix; + private boolean matchesType(String key) { + return key.endsWith(suffix); } } @@ -118,36 +130,33 @@ public String getSuffix() { .appendOffsetId() .toFormatter().withChronology(IsoChronology.INSTANCE); + private static final String CA_SECRET_PREFIX = "ca"; + /** * Key for storing the CA private key in a Kubernetes Secret */ - public static final String CA_KEY = "ca.key"; + public static final String CA_KEY = SecretEntry.KEY.asKey(CA_SECRET_PREFIX); /** * Key for storing the CA public key in a Kubernetes Secret */ - public static final String CA_CRT = "ca.crt"; + public static final String CA_CRT = SecretEntry.CRT.asKey(CA_SECRET_PREFIX); /** * Key for storing the CA PKCS21 store in a Kubernetes Secret */ - public static final String CA_STORE = "ca.p12"; + public static final String CA_STORE = SecretEntry.P12_KEYSTORE.asKey(CA_SECRET_PREFIX); /** * Key for storing the PKCS12 store password in a Kubernetes Secret */ - public static final String CA_STORE_PASSWORD = "ca.password"; + public static final String CA_STORE_PASSWORD = SecretEntry.P12_KEYSTORE_PASSWORD.asKey(CA_SECRET_PREFIX); /** * Organization used in the generated CAs */ public static final String IO_STRIMZI = "io.strimzi"; - /** - * Annotation for requesting a renewal of the CA and rolling it out - */ - public static final String ANNO_STRIMZI_IO_FORCE_RENEW = Annotations.STRIMZI_DOMAIN + "force-renew"; - /** * Annotation for tracking the CA key generation used by Kubernetes resources */ @@ -335,6 +344,24 @@ public void setClock(Clock clock) { this.clock = clock; } + /** + * Extracts the CA generation from the CA + * + * @return CA generation or the initial generation if no generation is set + */ + public int caCertGeneration() { + return Annotations.intAnnotation(caCertSecret(), ANNO_STRIMZI_IO_CA_CERT_GENERATION, INIT_GENERATION); + } + + /** + * Extracts the CA key generation from the CA + * + * @return CA key generation or the initial generation if no generation is set + */ + public int caKeyGeneration() { + return Annotations.intAnnotation(caKeySecret(), ANNO_STRIMZI_IO_CA_KEY_GENERATION, INIT_GENERATION); + } + protected static void delete(Reconciliation reconciliation, File file) { if (!file.delete()) { LOGGER.warnCr(reconciliation, "{} cannot be deleted", file.getName()); @@ -533,8 +560,8 @@ public void createRenewOrReplace(String namespace, String clusterName, Map(caCertSecret.getData()); if (certData.containsKey(CA_CRT)) { String notAfterDate = DATE_TIME_FORMATTER.format(currentCert.getNotAfter().toInstant().atZone(ZoneId.of("Z"))); - addCertCaToTrustStore("ca-" + notAfterDate + ".crt", certData); - certData.put("ca-" + notAfterDate + ".crt", certData.remove(CA_CRT)); + addCertCaToTrustStore("ca-" + notAfterDate + SecretEntry.CRT.suffix, certData); + certData.put("ca-" + notAfterDate + SecretEntry.CRT.suffix, certData.remove(CA_CRT)); } ++caCertGeneration; generateCaKeyAndCert(nextCaSubject(++caKeyGeneration), keyData, certData); @@ -773,11 +800,18 @@ public int certGeneration() { LOGGER.warnOp("Secret {}/{} is missing generation annotation {}", caCertSecret.getMetadata().getNamespace(), caCertSecret.getMetadata().getName(), ANNO_STRIMZI_IO_CA_CERT_GENERATION); } - return Annotations.intAnnotation(caCertSecret, ANNO_STRIMZI_IO_CA_CERT_GENERATION, INIT_GENERATION); + return caCertGeneration(); } return INIT_GENERATION; } + /** + * @return the generation of the current CA certificate as an annotation + */ + public Map.Entry caCertGenerationFullAnnotation() { + return Map.entry(caCertGenerationAnnotation(), String.valueOf(certGeneration())); + } + /** * @return the generation of the current CA key */ @@ -787,7 +821,7 @@ public int keyGeneration() { LOGGER.warnOp("Secret {}/{} is missing generation annotation {}", caKeySecret.getMetadata().getNamespace(), caKeySecret.getMetadata().getName(), ANNO_STRIMZI_IO_CA_KEY_GENERATION); } - return Annotations.intAnnotation(caKeySecret, ANNO_STRIMZI_IO_CA_KEY_GENERATION, INIT_GENERATION); + return caKeyGeneration(); } return INIT_GENERATION; } @@ -812,7 +846,7 @@ private boolean removeExpiredCert(Map.Entry entry) { } } catch (CertificateException e) { // doesn't remove stores and related password - if (!certName.endsWith(".p12") && !certName.endsWith(".password")) { + if (!SecretEntry.P12_KEYSTORE.matchesType(certName) && !SecretEntry.P12_KEYSTORE_PASSWORD.matchesType(certName)) { remove = true; LOGGER.debugCr(reconciliation, "The certificate (data.{}) in Secret is not an X.509 certificate; removing it", certName.replace(".", "\\.")); @@ -908,7 +942,7 @@ public static Set certs(Secret secret) { .getData() .entrySet() .stream() - .filter(record -> record.getKey().endsWith(".crt")) + .filter(record -> SecretEntry.CRT.matchesType(record.getKey())) .map(record -> { byte[] bytes = decoder.decode(record.getValue()); try {