Jakub's comments

Signed-off-by: Lukas Kral <lukywill16@gmail.com>

Author: im-konge

api/src/main/java/io/strimzi/api/kafka/model/user/KafkaUserTlsClientAuthentication.java

@@ -21,6 +21,16 @@
 @JsonPropertyOrder({"type", "validityDays", "renewalDays"})
 @EqualsAndHashCode(callSuper = true)
 @ToString(callSuper = true)
+@CelValidation(rules = {
+    @CelValidation.CelValidationRule(
+        rule = "has(self.renewalDays) == has(self.validityDays)",
+        message = "Both 'validityDays' and 'renewalDays' must be set together, or both must be unset."
+        ),
+    @CelValidation.CelValidationRule(
+        rule = "!has(self.renewalDays) || !has(self.validityDays) || self.renewalDays < self.validityDays",
+        message = "'renewalDays' must be less than 'validityDays'."
+        )
+})
 public class KafkaUserTlsClientAuthentication extends KafkaUserAuthentication {
     public static final String TYPE_TLS = "tls";
 
@@ -42,9 +52,7 @@ public String getType() {
     })
     @Description(
         "Number of days for which the user certificate should be valid. " +
-        "If not configured, default User Operator value is used. " +
-        "If new validity policy would make the current certificate expired or current certificate's validity period would exceed new policy, " +
-        "the certificate is immediately renewed, without waiting for maintenance window. "
+        "If not configured, Clients CA configuration is used."
     )
     @JsonInclude(value = JsonInclude.Include.NON_NULL)
     public Integer getValidityDays() {
@@ -63,7 +71,7 @@ public void setValidityDays(Integer validityDays) {
     })
     @Description(
         "Configures how many days before the certificate expiration should be the user certificate renewed. " +
-        "If not configured, default User Operator value is used."
+        "If not configured, Clients CA configuration is used."
     )
     @JsonInclude(value = JsonInclude.Include.NON_NULL)
     public Integer getRenewalDays() {

development-docs/systemtests/io.strimzi.systemtest.operators.user.UserST.md

@@ -97,7 +97,7 @@
 * [user-operator](labels/user-operator.md)
 
 
-## testTlsValidityDays
+## testTlsValidityDaysWithForceRenewal
 
 **Description:** Verifies functionality of the mTLS `validityDays` and `renewalDays` configured inside each KafkaUser.
 
@@ -110,7 +110,7 @@
 | 3. | Obtain the `KafkaUser`'s `Secret` and check validity period of the user certificate. | Validity period should be default - 200 days. |
 | 4. | Do message transmission to verify, that we are able to connect to Kafka cluster with the TLS `KafkaUser`. | Messages are successfully sent and received. |
 | 5. | Change the `validityDays` and `renewalDays` in the `KafkaUser` `.spec.authentication` to 60 and 10. | The `validityDays` and `renewalDays` should be changed in the `KafkaUser`. |
-| 6. | Because the current certificate would exceed the new validity period, `KafkaUser`'s `Secret` and user certificate should be renewed - we are waiting for the certificate change. | The user certificate was changed. |
+| 6. | Because we changed the `validityDays` and `renewalDays`, we need to force renew the certificate using the `strimzi.io/force-renew=true` annotation | The user certificate was renewed. |
 | 7. | Obtain the `KafkaUser`'s `Secret` again and check the validity period of the user certificate. | Validity period should be 60 days. |
 | 8. | Do message transmission again to verify, that we are able to connect to Kafka cluster with the new user's certificate. | Messages are successfully sent and received using new certificate. |
 

development-docs/systemtests/labels/user-operator.md

@@ -15,7 +15,7 @@ They verify user authentication mechanisms (TLS, SCRAM-SHA-512, external TLS), a
 - [testTlsExternalUser](../io.strimzi.systemtest.operators.user.UserST.md)
 - [testTlsExternalUserWithQuotas](../io.strimzi.systemtest.operators.user.UserST.md)
 - [testTlsUserWithQuotas](../io.strimzi.systemtest.operators.user.UserST.md)
-- [testTlsValidityDays](../io.strimzi.systemtest.operators.user.UserST.md)
+- [testTlsValidityDaysWithForceRenewal](../io.strimzi.systemtest.operators.user.UserST.md)
 - [testUpdateUser](../io.strimzi.systemtest.operators.user.UserST.md)
 - [testUserWithNameMoreThan64Chars](../io.strimzi.systemtest.operators.user.UserST.md)
 - [testUserWithQuotas](../io.strimzi.systemtest.operators.user.UserST.md)

documentation/modules/appendix_crds.adoc

@@ -2680,10 +2680,10 @@ It must have the value `tls` for the type `KafkaUserTlsClientAuthentication`.
 |Must be `tls`.
 |validityDays
 |integer
-|Number of days for which the user certificate should be valid. If not configured, default User Operator value is used. If new validity policy would make the current certificate expired or current certificate's validity period would exceed new policy, the certificate is immediately renewed, without waiting for maintenance window. 
+|Number of days for which the user certificate should be valid. If not configured, Clients CA configuration is used.
 |renewalDays
 |integer
-|Configures how many days before the certificate expiration should be the user certificate renewed. If not configured, default User Operator value is used.
+|Configures how many days before the certificate expiration should be the user certificate renewed. If not configured, Clients CA configuration is used.
 |====
 
 [id='type-KafkaUserTlsExternalClientAuthentication-{context}']

operator-common/src/main/java/io/strimzi/operator/common/model/Ca.java

@@ -536,41 +536,6 @@ public boolean isExpiring(Secret secret, String certKey)  {
         return certNeedsRenewal(currentCert);
     }
 
-    /**
-     * Checks if the validityDays configuration is different from previous state.
-     * If yes, the method checks if the new configuration of validityDays would make the certificate expired
-     * or if the current Certificate's validity period exceeds the new policy.
-     *
-     * @param secret    Secret with the certificate
-     * @param certKey   Key under which is the certificate stored
-     *
-     * @return  True if the certificate should be renewed due to new validity period. False otherwise.
-     */
-    public boolean requiresImmediateRenewalDueToValidityChange(Secret secret, String certKey) {
-        X509Certificate currentCert = cert(secret, certKey);
-
-        Instant notBefore = currentCert.getNotBefore().toInstant();
-        Instant notAfter = currentCert.getNotAfter().toInstant();
-        int currentValidityDays = (int) ChronoUnit.DAYS.between(notBefore, notAfter);
-
-        if (currentValidityDays != validityDays) {
-            Instant wouldExpireUnderNewPolicy = notBefore.plus(validityDays, ChronoUnit.DAYS);
-
-            if (!this.clock.instant().isBefore(wouldExpireUnderNewPolicy)) {
-                LOGGER.infoCr(reconciliation, "Certificate of Secret {}/{} would be expired under new validity policy, it will be renewed",
-                    secret.getMetadata().getNamespace(), secret.getMetadata().getName());
-                return true;
-            }
-            if (currentValidityDays > validityDays) {
-                LOGGER.infoCr(reconciliation, "Certificate's current validity period of Secret {}/{} exceeds new policy, it will be renewed",
-                    secret.getMetadata().getNamespace(), secret.getMetadata().getName());
-                return true;
-            }
-        }
-
-        return false;
-    }
-
     /**
      * Returns whether the certificate is expiring or not
      *

operator-common/src/test/java/io/strimzi/operator/common/operator/MockCertManager.java

@@ -5,9 +5,7 @@
 package io.strimzi.operator.common.operator;
 
 import io.strimzi.certs.CertManager;
-import io.strimzi.certs.OpenSslCertManager;
 import io.strimzi.certs.Subject;
-import io.strimzi.operator.common.Util;
 
 import java.io.ByteArrayOutputStream;
 import java.io.File;
@@ -78,9 +76,59 @@ public class MockCertManager implements CertManager {
             D0z+vgrfionoRhyWUDh7POlWwdUOWiBDBOFrkgeKNphSC0glYFN+2IW7
             -----END CERTIFICATE-----
             """;
-
-    private static final String CLIENTS_KEY;
-    private static final String CLIENTS_CERT;
+    private static final String CLIENTS_KEY = """
+            -----BEGIN PRIVATE KEY-----
+            MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCWws5FEJOpfZ/s
+            FJWYbYdJVxmZB+5PjCwA2TUZxF/3P4w/5g2KZaXNy89AfBC5vRDRgyDyj/RwcDg8
+            0kDGKobcGhTx5YkWoNvR/2WuTN6KC8DM78bfEREDHDxiXfAXrMIi7Ux2FvUX13l7
+            6Sp9kiG3ETLjFom3n/qhg1ITJqPJSJi3tey0o2Pd5Arv0MIhQyep++URtZfND5fg
+            F5x7hgnSf9Q1P1dJnVadu+ohUmmG7g+zX4rTqjN2jmHcf9V4lLKdPGWwLQEGnP9y
+            Dqlm8x12M/BcIJasRgcciVsKYFuXe09NEYBvUjW8L6gaQ6U9wcYZ2MlKW/8LMGkS
+            FfO4quAJAgMBAAECggEAQ1NdsEQV3UQHrfMHV1naZ6so+EktaILNh9d4OjiTLqRH
+            aqW++EYqhDv3IvIEuh2vrBCmHwygebHzu12dpaGKNjLDlb8OuHc/k4k9jFgxrW5Q
+            PHT719QUR9JNORSASuJQlC5qzfW0oGAOlYJsAkXHHqzkj7sZ51HfKE+v0HOaAyHj
+            8gOeBNk1Mtb3Sj5mXpWFQGpXXuG01Vsjj7Nj/91a4KtWAWOqeagc2Bk+C0aZ7d1p
+            SQcLVWjJYwoejgCc2elZxzbfmDtVSAgFtdTPxwf9uflMducTfp/RyaQbzuYSrSmz
+            rnZq/59i9lYl314rjjkCusDaDSPdK5QziN54tQ+BcQKBgQDE9ulPecHtZhOsI9zT
+            J+xTJtZq1w8kFV5jMqXnL3jAFBXsC3s02KLq36ppvf8kVzUHrHE+DiWnHKEIiy/U
+            luMnPvJb/6qqdQNDpcrF+CE2JevvoPl5hrKdyzAI4TNu96aU+9qVrO2rB7bWBvlA
+            dVwIZ8zkk3pwbdEj9rYpMA1VVQKBgQDD8rJAEd9fLtX53NQh8XWEJ1dEfncmg/ib
+            0vyoYlqSDjPTot85sCunVZNHwUoKUsukzi+Tc9hxaXCjEB6ICVeXqWc4PYnbK79H
+            N+2X6YaO/rKAzbxM1F/Km3IzzvoXFJnPG4hxvBmpdApKgBGOVixnjD7PzNz4jh9u
+            1qhDocdf5QKBgQCDsLqporTgr0Ez9P5uR+Egb3UpFgVPkOH83R5Dhl/rvQIzQjHs
+            UXQMKeNcs+XlPFF+gfNtFDRkmSWp+rXOI9xYnyOYE0belUHLdwwudQpvk8c9/pkO
+            gdrm2bWSGlAzP22nawTo0ihOE+hRDXSVfmI8VHqP0XMpvKL6srd0rmYbyQKBgAYD
+            PXr/0WXfTwuSviOogB2lA2WDp+5ToF5PtBcKpZLTwr1cwxLHGB/TXWiXQslcTwlo
+            lkclB+A7BwzJ4tXzy29I8HTmVoOWLRFnYvAFZ26d3CZdqciFv8a8zF1QnZX1uN6F
+            DsPGrNbpS6OLmH5QoJ4wzICd3a321noVNiaVIUQNAoGAYu4RrGcBKRuy75lfKARD
+            gNxxVlvuI33ieK/3A9nUWc3LXl5D/yiSePCUs4giOwi2gFrGjcmIqLXZE5XUYGEu
+            zXWWQCGbMqyX15/A2/eTuj658F292nkSyU/5U2999WjCm79sfnGJB1zavfv2fzGK
+            g4trXCUkjAVG3Toaq05saGM=
+            -----END PRIVATE KEY-----
+            """;
+    private static final String CLIENTS_CERT = """
+            -----BEGIN CERTIFICATE-----
+            MIIDhjCCAm6gAwIBAgIJAOKzFJgrn+rZMA0GCSqGSIb3DQEBCwUAMFcxCzAJBgNV
+            BAYTAlhYMRUwEwYDVQQHDAxEZWZhdWx0IENpdHkxHDAaBgNVBAoME0RlZmF1bHQg
+            Q29tcGFueSBMdGQxEzARBgNVBAMMCmNsaWVudHMtY2EwIBcNMTgwODIzMTYyMTI1
+            WhgPMjExODA3MzAxNjIxMjVaMFcxCzAJBgNVBAYTAlhYMRUwEwYDVQQHDAxEZWZh
+            dWx0IENpdHkxHDAaBgNVBAoME0RlZmF1bHQgQ29tcGFueSBMdGQxEzARBgNVBAMM
+            CmNsaWVudHMtY2EwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCWws5F
+            EJOpfZ/sFJWYbYdJVxmZB+5PjCwA2TUZxF/3P4w/5g2KZaXNy89AfBC5vRDRgyDy
+            j/RwcDg80kDGKobcGhTx5YkWoNvR/2WuTN6KC8DM78bfEREDHDxiXfAXrMIi7Ux2
+            FvUX13l76Sp9kiG3ETLjFom3n/qhg1ITJqPJSJi3tey0o2Pd5Arv0MIhQyep++UR
+            tZfND5fgF5x7hgnSf9Q1P1dJnVadu+ohUmmG7g+zX4rTqjN2jmHcf9V4lLKdPGWw
+            LQEGnP9yDqlm8x12M/BcIJasRgcciVsKYFuXe09NEYBvUjW8L6gaQ6U9wcYZ2MlK
+            W/8LMGkSFfO4quAJAgMBAAGjUzBRMB0GA1UdDgQWBBQUwNmfsNj+PM240pVPxYx9
+            Q9eQhDAfBgNVHSMEGDAWgBQUwNmfsNj+PM240pVPxYx9Q9eQhDAPBgNVHRMBAf8E
+            BTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQAnhbNKwMmnayHsT6kKgyyDV6RUUYs6
+            nYf3nx+GIQWSw4c5TOHDcTWdKpOxVnLNXYKQoSkb1RBoSMLBdQwidZ5K2DB5eXaG
+            rcfEbKNBc5ZCFgFEAyy35pitJOmU/KzCdKyvx+TR5hIgGoKajYX5JZxj+1rTPGKO
+            ePT9iFp1ZbzHjgw6vFeJ+D2ov6HfW6C/KuK9Y6xUpvRQLVjMJYCyzxkxQAxZvu/0
+            0HVYYH6UJ7kuWywFMWoBdZ8US/vuUSBYyCGNL9p6ol+h9rsz3cIWBVBjx8C3qKki
+            QtlIdmFljGSaGGY6aJjUvUdgoPp1yQPa5oS+afr5g9gaEp4lxP6mc+Li
+            -----END CERTIFICATE-----
+            """;
 
     private static final String ALTERNATE_CLIENTS_CERT = """
             -----BEGIN CERTIFICATE-----
@@ -221,24 +269,6 @@ public class MockCertManager implements CertManager {
         CLUSTER_CERT_STORE = loadResource(is);
         is = MockCertManager.class.getClassLoader().getResourceAsStream("CLIENTS_CERT.str");
         CLIENTS_CERT_STORE = loadResource(is);
-        // generate clients certificate and key, to satisfy the validity days check mainly in KafkaUser tests
-        OpenSslCertManager openSslCertManager = new OpenSslCertManager();
-        try {
-            File keyFile = Files.createTempFile("tls", "key").toFile();
-            File csrFile = Files.createTempFile("tls", "csr").toFile();
-            File certFile = Files.createTempFile("tls", "cert").toFile();
-
-            Subject.Builder subject = new Subject.Builder();
-            subject.withCommonName("strimzi-client");
-
-            openSslCertManager.generateCsr(keyFile, csrFile, subject.build());
-            openSslCertManager.generateCert(csrFile, Util.decodeBytesFromBase64(clusterCaKey()), Util.decodeBytesFromBase64(clusterCaCert()), certFile, subject.build(), 365);
-
-            CLIENTS_CERT = Base64.getEncoder().encodeToString(Files.readAllBytes(certFile.toPath()));
-            CLIENTS_KEY = Base64.getEncoder().encodeToString(Files.readAllBytes(keyFile.toPath()));
-        } catch (Exception e) {
-            throw new RuntimeException("Failed to create tmp files");
-        }
     }
 
     public static String clusterCaCert() {
@@ -250,21 +280,29 @@ public static String clusterCaKey() {
     }
 
     public static String clientsCaCert() {
-        return CLIENTS_CERT;
+        return Base64.getEncoder().encodeToString(CLIENTS_CERT.getBytes(Charset.defaultCharset()));
     }
 
     public static String alternateClientsCaCert() {
         return Base64.getEncoder().encodeToString(ALTERNATE_CLIENTS_CERT.getBytes(Charset.defaultCharset()));
     }
 
     public static String clientsCaKey() {
-        return CLIENTS_KEY;
+        return Base64.getEncoder().encodeToString(CLIENTS_KEY.getBytes(Charset.defaultCharset()));
     }
 
     public static String clusterCaCertStore() {
         return Base64.getEncoder().encodeToString(CLUSTER_CERT_STORE);
     }
 
+    public static String clientsCaCertStore() {
+        return Base64.getEncoder().encodeToString(CLIENTS_CERT_STORE);
+    }
+
+    public static String certStorePassword() {
+        return Base64.getEncoder().encodeToString(CERT_STORE_PASSWORD.getBytes(Charset.defaultCharset()));
+    }
+
     private void write(File keyFile, String str) throws IOException {
         try (FileWriter writer = new FileWriter(keyFile)) {
             writer.write(str);

packaging/helm-charts/helm3/strimzi-kafka-operator/crds/044-Crd-kafkauser.yaml

@@ -83,7 +83,7 @@ spec:
                       description: "Specify the password for the user. If not set, a new password is generated by the User Operator."
                     renewalDays:
                       type: integer
-                      description: "Configures how many days before the certificate expiration should be the user certificate renewed. If not configured, default User Operator value is used."
+                      description: "Configures how many days before the certificate expiration should be the user certificate renewed. If not configured, Clients CA configuration is used."
                       x-kubernetes-validations:
                         - rule: self > 0
                           message: '''renewalDays'' has to be higher than 0.'
@@ -96,7 +96,7 @@ spec:
                       description: Authentication type.
                     validityDays:
                       type: integer
-                      description: "Number of days for which the user certificate should be valid. If not configured, default User Operator value is used. If new validity policy would make the current certificate expired or current certificate's validity period would exceed new policy, the certificate is immediately renewed, without waiting for maintenance window. "
+                      description: "Number of days for which the user certificate should be valid. If not configured, Clients CA configuration is used."
                       x-kubernetes-validations:
                         - rule: self > 0
                           message: '''validityDays'' has to be higher than 0.'

packaging/install/cluster-operator/044-Crd-kafkauser.yaml

@@ -82,7 +82,7 @@ spec:
                     description: "Specify the password for the user. If not set, a new password is generated by the User Operator."
                   renewalDays:
                     type: integer
-                    description: "Configures how many days before the certificate expiration should be the user certificate renewed. If not configured, default User Operator value is used."
+                    description: "Configures how many days before the certificate expiration should be the user certificate renewed. If not configured, Clients CA configuration is used."
                     x-kubernetes-validations:
                     - rule: self > 0
                       message: '''renewalDays'' has to be higher than 0.'
@@ -95,7 +95,7 @@ spec:
                     description: Authentication type.
                   validityDays:
                     type: integer
-                    description: "Number of days for which the user certificate should be valid. If not configured, default User Operator value is used. If new validity policy would make the current certificate expired or current certificate's validity period would exceed new policy, the certificate is immediately renewed, without waiting for maintenance window. "
+                    description: "Number of days for which the user certificate should be valid. If not configured, Clients CA configuration is used."
                     x-kubernetes-validations:
                     - rule: self > 0
                       message: '''validityDays'' has to be higher than 0.'

packaging/install/user-operator/04-Crd-kafkauser.yaml

@@ -82,7 +82,7 @@ spec:
                     description: "Specify the password for the user. If not set, a new password is generated by the User Operator."
                   renewalDays:
                     type: integer
-                    description: "Configures how many days before the certificate expiration should be the user certificate renewed. If not configured, default User Operator value is used."
+                    description: "Configures how many days before the certificate expiration should be the user certificate renewed. If not configured, Clients CA configuration is used."
                     x-kubernetes-validations:
                     - rule: self > 0
                       message: '''renewalDays'' has to be higher than 0.'
@@ -95,7 +95,7 @@ spec:
                     description: Authentication type.
                   validityDays:
                     type: integer
-                    description: "Number of days for which the user certificate should be valid. If not configured, default User Operator value is used. If new validity policy would make the current certificate expired or current certificate's validity period would exceed new policy, the certificate is immediately renewed, without waiting for maintenance window. "
+                    description: "Number of days for which the user certificate should be valid. If not configured, Clients CA configuration is used."
                     x-kubernetes-validations:
                     - rule: self > 0
                       message: '''validityDays'' has to be higher than 0.'