Fix CC not restarted when API secret changes (strimzi#9616)

Signed-off-by: Federico Valeri <fedevaleri@gmail.com>

Author: fvaleri

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

@@ -415,10 +415,11 @@ protected List<EnvVar> getEnvVars() {
     /**
      * Creates Cruise Control API auth usernames, passwords, and credentials file
      *
+     * @param passwordGenerator The password generator for API users
+     *
      * @return Map containing Cruise Control API auth credentials
      */
-    public static Map<String, String> generateCruiseControlApiCredentials() {
-        PasswordGenerator passwordGenerator = new PasswordGenerator(16);
+    public static Map<String, String> generateCruiseControlApiCredentials(PasswordGenerator passwordGenerator) {
         String apiAdminPassword = passwordGenerator.generate();
         String apiUserPassword = passwordGenerator.generate();
 
@@ -441,10 +442,13 @@ public static Map<String, String> generateCruiseControlApiCredentials() {
     /**
      * Generate the Secret containing the Cruise Control API auth credentials.
      *
+     * @param passwordGenerator The password generator for API users
+     * 
      * @return The generated Secret.
      */
-    public Secret generateApiSecret() {
-        return ModelUtils.createSecret(CruiseControlResources.apiSecretName(cluster), namespace, labels, ownerReference, generateCruiseControlApiCredentials(), Collections.emptyMap(), Collections.emptyMap());
+    public Secret generateApiSecret(PasswordGenerator passwordGenerator) {
+        return ModelUtils.createSecret(CruiseControlResources.apiSecretName(cluster), namespace, labels, ownerReference, 
+            generateCruiseControlApiCredentials(passwordGenerator), Collections.emptyMap(), Collections.emptyMap());
     }
 
     /**

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

@@ -20,11 +20,13 @@
 import io.strimzi.operator.cluster.model.KafkaVersion;
 import io.strimzi.operator.cluster.model.NodeRef;
 import io.strimzi.operator.cluster.operator.resource.ResourceOperatorSupplier;
+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.Labels;
+import io.strimzi.operator.common.model.PasswordGenerator;
 import io.strimzi.operator.common.operator.resource.ConfigMapOperator;
 import io.strimzi.operator.common.operator.resource.DeploymentOperator;
 import io.strimzi.operator.common.operator.resource.NetworkPolicyOperator;
@@ -40,7 +42,6 @@
 import java.util.Map;
 import java.util.Set;
 
-
 /**
  * Class used for reconciliation of Cruise Control. This class contains both the steps of the Cruise Control
  * reconciliation pipeline and is also used to store the state between them.
@@ -63,18 +64,21 @@ public class CruiseControlReconciler {
     private final ServiceOperator serviceOperator;
     private final NetworkPolicyOperator networkPolicyOperator;
     private final ConfigMapOperator configMapOperator;
+    private final PasswordGenerator passwordGenerator;
 
     private boolean existingCertsChanged = false;
 
     private String serverConfigurationHash = "";
     private String capacityConfigurationHash = "";
-
+    private String apiSecretHash = "";
+    
     /**
      * Constructs the Cruise Control reconciler
      *
      * @param reconciliation            Reconciliation marker
      * @param config                    Cluster Operator Configuration
      * @param supplier                  Supplier with Kubernetes Resource Operators
+     * @param passwordGenerator         The password generator for API users
      * @param kafkaAssembly             The Kafka custom resource
      * @param versions                  The supported Kafka versions
      * @param kafkaBrokerNodes          List of the broker nodes which are part of the Kafka cluster
@@ -87,6 +91,7 @@ public CruiseControlReconciler(
             Reconciliation reconciliation,
             ClusterOperatorConfig config,
             ResourceOperatorSupplier supplier,
+            PasswordGenerator passwordGenerator,
             Kafka kafkaAssembly,
             KafkaVersion.Lookup versions,
             Set<NodeRef> kafkaBrokerNodes,
@@ -102,7 +107,8 @@ public CruiseControlReconciler(
         this.operatorNamespace = config.getOperatorNamespace();
         this.operatorNamespaceLabels = config.getOperatorNamespaceLabels();
         this.isNetworkPolicyGeneration = config.isNetworkPolicyGeneration();
-
+        this.passwordGenerator = passwordGenerator;
+        
         this.deploymentOperator = supplier.deploymentOperations;
         this.secretOperator = supplier.secretOperations;
         this.serviceAccountOperator = supplier.serviceAccountOperations;
@@ -240,15 +246,17 @@ protected Future<Void> apiSecret() {
         if (cruiseControl != null) {
             return secretOperator.getAsync(reconciliation.namespace(), CruiseControlResources.apiSecretName(reconciliation.name()))
                     .compose(oldSecret -> {
-                        Secret newSecret = cruiseControl.generateApiSecret();
+                        Secret newSecret = cruiseControl.generateApiSecret(passwordGenerator);
 
                         if (oldSecret != null)  {
                             // The credentials should not change with every release
                             // So if the secret with credentials already exists, we re-use the values
                             // But we use the new secret to update labels etc. if needed
                             newSecret.setData(oldSecret.getData());
                         }
-
+                        
+                        this.apiSecretHash = ReconcilerUtils.hashSecretContent(newSecret);
+                        
                         return secretOperator.reconcile(reconciliation, reconciliation.namespace(), CruiseControlResources.apiSecretName(reconciliation.name()), newSecret)
                                 .map((Void) null);
                     });
@@ -285,7 +293,8 @@ protected Future<Void> deployment(boolean isOpenShift, ImagePullPolicy imagePull
             podAnnotations.put(Ca.ANNO_STRIMZI_IO_CLUSTER_CA_KEY_GENERATION, String.valueOf(clusterCa.caKeyGeneration()));
             podAnnotations.put(CruiseControl.ANNO_STRIMZI_SERVER_CONFIGURATION_HASH, serverConfigurationHash);
             podAnnotations.put(CruiseControl.ANNO_STRIMZI_CAPACITY_CONFIGURATION_HASH, capacityConfigurationHash);
-
+            podAnnotations.put(Annotations.ANNO_STRIMZI_AUTH_HASH, apiSecretHash);
+            
             Deployment deployment = cruiseControl.generateDeployment(podAnnotations, isOpenShift, imagePullPolicy, imagePullSecrets);
 
             return deploymentOperator

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

@@ -699,6 +699,7 @@ CruiseControlReconciler cruiseControlReconciler()   {
                     reconciliation,
                     config,
                     supplier,
+                    passwordGenerator,
                     kafkaAssembly,
                     versions,
                     kafkaBrokerNodes,

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

@@ -381,4 +381,27 @@ public static boolean nodePoolsEnabled(Kafka kafka) {
     public static boolean kraftEnabled(Kafka kafka) {
         return KafkaCluster.ENABLED_VALUE_STRIMZI_IO_KRAFT.equals(Annotations.stringAnnotation(kafka, Annotations.ANNO_STRIMZI_IO_KRAFT, "disabled").toLowerCase(Locale.ENGLISH));
     }
+
+    /**
+     * Creates a hash from Secret's content.
+     * 
+     * @param secret Secret with content.
+     * @return Hash of the secret content.
+     */
+    public static String hashSecretContent(Secret secret) {
+        if (secret == null) {
+            throw new RuntimeException("Secret not found");
+        }
+        
+        if (secret.getData() == null || secret.getData().isEmpty()) {
+            throw new RuntimeException("Empty secret");
+        }
+        
+        StringBuilder sb = new StringBuilder();
+        secret.getData().entrySet().stream()
+            .sorted(Map.Entry.comparingByKey())
+            .forEach(entry -> sb.append(entry.getKey()).append(entry.getValue()));
+        
+        return Util.hashStub(sb.toString());
+    }
 }

cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/CruiseControlReconcilerTest.java

@@ -25,6 +25,7 @@
 import io.strimzi.operator.cluster.model.KafkaVersion;
 import io.strimzi.operator.cluster.model.NodeRef;
 import io.strimzi.operator.cluster.operator.resource.ResourceOperatorSupplier;
+import io.strimzi.operator.common.Annotations;
 import io.strimzi.operator.common.Reconciliation;
 import io.strimzi.operator.common.model.PasswordGenerator;
 import io.strimzi.operator.common.operator.MockCertManager;
@@ -78,7 +79,8 @@ public void reconcileEnabledCruiseControl(VertxTestContext context) {
         ServiceOperator mockServiceOps = supplier.serviceOperations;
         NetworkPolicyOperator mockNetPolicyOps = supplier.networkPolicyOperator;
         ConfigMapOperator mockCmOps = supplier.configMapOperations;
-
+        PasswordGenerator mockPasswordGenerator = new PasswordGenerator(10, "a", "a");
+        
         ArgumentCaptor<ServiceAccount> saCaptor = ArgumentCaptor.forClass(ServiceAccount.class);
         when(mockSaOps.reconcile(any(), eq(NAMESPACE), eq(CruiseControlResources.serviceAccountName(NAME)), saCaptor.capture())).thenReturn(Future.succeededFuture());
 
@@ -119,6 +121,7 @@ public void reconcileEnabledCruiseControl(VertxTestContext context) {
                 Reconciliation.DUMMY_RECONCILIATION,
                 ResourceUtils.dummyClusterOperatorConfig(),
                 supplier,
+                mockPasswordGenerator,
                 kafka,
                 VERSIONS,
                 NODES,
@@ -151,6 +154,7 @@ public void reconcileEnabledCruiseControl(VertxTestContext context) {
                     assertThat(deployCaptor.getValue(), is(notNullValue()));
                     assertThat(deployCaptor.getAllValues().get(0).getSpec().getTemplate().getMetadata().getAnnotations().get(CruiseControl.ANNO_STRIMZI_SERVER_CONFIGURATION_HASH), is("f6dc41c7"));
                     assertThat(deployCaptor.getAllValues().get(0).getSpec().getTemplate().getMetadata().getAnnotations().get(CruiseControl.ANNO_STRIMZI_CAPACITY_CONFIGURATION_HASH), is("1eb49220"));
+                    assertThat(deployCaptor.getAllValues().get(0).getSpec().getTemplate().getMetadata().getAnnotations().get(Annotations.ANNO_STRIMZI_AUTH_HASH), is("27ada64b"));
 
                     async.flag();
                 })));
@@ -202,6 +206,7 @@ public void reconcileDisabledCruiseControl(VertxTestContext context) {
                 Reconciliation.DUMMY_RECONCILIATION,
                 ResourceUtils.dummyClusterOperatorConfig(),
                 supplier,
+                new PasswordGenerator(16),
                 kafka,
                 VERSIONS,
                 NODES,

cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/ReconcilerUtilsTest.java

@@ -290,6 +290,28 @@ public void testEnabledJmxWithAuthWithExistingSecret(VertxTestContext context) {
                     async.flag();
                 })));
     }
+    
+    @Test
+    public void testHashSecretContent() {
+        Secret secret = new SecretBuilder()
+            .addToData(Map.of("username", "foo"))
+            .addToData(Map.of("password", "changeit"))
+            .build();
+        assertThat(ReconcilerUtils.hashSecretContent(secret), is("756937ae"));
+    }
+
+    @Test
+    public void testHashSecretContentWithNoData() {
+        Secret secret = new SecretBuilder().build();
+        RuntimeException ex = assertThrows(RuntimeException.class, () -> ReconcilerUtils.hashSecretContent(secret));
+        assertThat(ex.getMessage(), is("Empty secret"));
+    }
+
+    @Test
+    public void testHashSecretContentWithNoSecret() {
+        RuntimeException ex = assertThrows(RuntimeException.class, () -> ReconcilerUtils.hashSecretContent(null));
+        assertThat(ex.getMessage(), is("Secret not found"));
+    }
 
     static class MockJmxCluster implements SupportsJmx {
         private final JmxModel jmx;

cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/resource/cruisecontrol/MockCruiseControl.java

@@ -12,6 +12,7 @@
 import io.strimzi.operator.cluster.model.CruiseControl;
 import io.strimzi.operator.cluster.model.ModelUtils;
 import io.strimzi.operator.common.model.Labels;
+import io.strimzi.operator.common.model.PasswordGenerator;
 import io.strimzi.operator.common.model.cruisecontrol.CruiseControlEndpoints;
 import io.strimzi.operator.common.model.cruisecontrol.CruiseControlParameters;
 import io.strimzi.operator.common.operator.MockCertManager;
@@ -79,7 +80,7 @@ public class MockCruiseControl {
             .addToData("cruise-control.crt", MockCertManager.clusterCaCert())
             .build();
     public static final Secret CC_API_SECRET = ModelUtils.createSecret(CruiseControlResources.apiSecretName(CLUSTER), NAMESPACE, Labels.EMPTY, null,
-            CruiseControl.generateCruiseControlApiCredentials(), Collections.emptyMap(), Collections.emptyMap());
+            CruiseControl.generateCruiseControlApiCredentials(new PasswordGenerator(16)), Collections.emptyMap(), Collections.emptyMap());
 
     private static final Header AUTH_HEADER = convertToHeader(CruiseControlApiImpl.getAuthHttpHeader(true, CC_API_SECRET));