address 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

@@ -47,6 +47,8 @@ public String getType() {
 
     @Description(
         "Number of days for which the user certificate should be valid. " +
+        "It has to be configured together with `renewalDays`, or none of them should be configured." +
+        "The number should be bigger than 0 and than `renewalDays`." +
         "If not configured, Clients CA configuration is used."
     )
     @Minimum(1)
@@ -61,6 +63,8 @@ public void setValidityDays(Integer validityDays) {
 
     @Description(
         "Number of days before certificate expiration when the user certificate should be renewed. " +
+        "It has to be configured together with `validityDays`, or none of them should be configured." +
+        "The number should be bigger than 0 and smaller than `validityDays`." +
         "If not configured, Clients CA configuration is used."
     )
     @Minimum(1)

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

@@ -97,7 +97,7 @@
 * [user-operator](labels/user-operator.md)
 
 
-## testTlsValidityDaysWithForceRenewal
+## testTlsValidityDays
 
 **Description:** Verifies functionality of the mTLS `validityDays` and `renewalDays` configured inside each KafkaUser.
 
@@ -106,12 +106,12 @@
 | Step | Action | Result |
 | - | - | - |
 | 1. | Create `KafkaTopic` to which we will send (and from which we will receive) messages - created in existing Kafka cluster. | `KafkaTopic` is created. |
-| 2. | Create `KafkaUser` with TLS authentication; together with default `validityDays` (200 days) and `renewalDays` (20 days) - configured in User operator. | `KafkaUser` is created with defaults. |
+| 2. | Create `KafkaUser` with TLS authentication; without configuring the `validityDays` and `renewalDays` - values from User Operator are taken. | `KafkaUser` is created with values from User Operator. |
 | 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 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. |
+| 5. | Change the `validityDays` and `renewalDays` in the `KafkaUser` `.spec.authentication` to 40 and 20. | The `validityDays` and `renewalDays` should be changed in the `KafkaUser`. |
+| 6. | Because of the change of `validityDays` and `renewalDays` (and because of the values inside), the certificate will be renewed | 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 40 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. |
 
 **Labels:**

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)
-- [testTlsValidityDaysWithForceRenewal](../io.strimzi.systemtest.operators.user.UserST.md)
+- [testTlsValidityDays](../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

@@ -2686,10 +2686,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, Clients CA configuration is used.
+|Number of days for which the user certificate should be valid. It has to be configured together with `renewalDays`, or none of them should be configured.The number should be bigger than 0 and than `renewalDays`.If not configured, Clients CA configuration is used.
 |renewalDays
 |integer
-|Number of days before certificate expiration when the user certificate should be renewed. If not configured, Clients CA configuration is used.
+|Number of days before certificate expiration when the user certificate should be renewed. It has to be configured together with `validityDays`, or none of them should be configured.The number should be bigger than 0 and smaller than `validityDays`.If not configured, Clients CA configuration is used.
 |====
 
 [id='type-KafkaUserTlsExternalClientAuthentication-{context}']

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

@@ -84,7 +84,7 @@ spec:
                     renewalDays:
                       type: integer
                       minimum: 1
-                      description: "Number of days before certificate expiration when the user certificate should be renewed. If not configured, Clients CA configuration is used."
+                      description: "Number of days before certificate expiration when the user certificate should be renewed. It has to be configured together with `validityDays`, or none of them should be configured.The number should be bigger than 0 and smaller than `validityDays`.If not configured, Clients CA configuration is used."
                     type:
                       type: string
                       enum:
@@ -95,7 +95,7 @@ spec:
                     validityDays:
                       type: integer
                       minimum: 1
-                      description: "Number of days for which the user certificate should be valid. If not configured, Clients CA configuration is used."
+                      description: "Number of days for which the user certificate should be valid. It has to be configured together with `renewalDays`, or none of them should be configured.The number should be bigger than 0 and than `renewalDays`.If not configured, Clients CA configuration is used."
                   required:
                     - type
                   description: "Authentication mechanism enabled for this Kafka user. The supported authentication mechanisms are `scram-sha-512`, `tls`, and `tls-external`. \n\n* `scram-sha-512` generates a secret with SASL SCRAM-SHA-512 credentials.\n* `tls` generates a secret with user certificate for mutual TLS authentication.\n* `tls-external` does not generate a user certificate.   But prepares the user for using mutual TLS authentication using a user certificate generated outside the User Operator.\n  ACLs and quotas set for this user are configured in the `CN=<username>` format.\n\nAuthentication is optional. If authentication is not configured, no credentials are generated. ACLs and quotas set for the user are configured in the `<username>` format suitable for SASL authentication."

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

@@ -83,7 +83,7 @@ spec:
                   renewalDays:
                     type: integer
                     minimum: 1
-                    description: "Number of days before certificate expiration when the user certificate should be renewed. If not configured, Clients CA configuration is used."
+                    description: "Number of days before certificate expiration when the user certificate should be renewed. It has to be configured together with `validityDays`, or none of them should be configured.The number should be bigger than 0 and smaller than `validityDays`.If not configured, Clients CA configuration is used."
                   type:
                     type: string
                     enum:
@@ -94,7 +94,7 @@ spec:
                   validityDays:
                     type: integer
                     minimum: 1
-                    description: "Number of days for which the user certificate should be valid. If not configured, Clients CA configuration is used."
+                    description: "Number of days for which the user certificate should be valid. It has to be configured together with `renewalDays`, or none of them should be configured.The number should be bigger than 0 and than `renewalDays`.If not configured, Clients CA configuration is used."
                 required:
                 - type
                 description: "Authentication mechanism enabled for this Kafka user. The supported authentication mechanisms are `scram-sha-512`, `tls`, and `tls-external`. \n\n* `scram-sha-512` generates a secret with SASL SCRAM-SHA-512 credentials.\n* `tls` generates a secret with user certificate for mutual TLS authentication.\n* `tls-external` does not generate a user certificate.   But prepares the user for using mutual TLS authentication using a user certificate generated outside the User Operator.\n  ACLs and quotas set for this user are configured in the `CN=<username>` format.\n\nAuthentication is optional. If authentication is not configured, no credentials are generated. ACLs and quotas set for the user are configured in the `<username>` format suitable for SASL authentication."

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

@@ -83,7 +83,7 @@ spec:
                   renewalDays:
                     type: integer
                     minimum: 1
-                    description: "Number of days before certificate expiration when the user certificate should be renewed. If not configured, Clients CA configuration is used."
+                    description: "Number of days before certificate expiration when the user certificate should be renewed. It has to be configured together with `validityDays`, or none of them should be configured.The number should be bigger than 0 and smaller than `validityDays`.If not configured, Clients CA configuration is used."
                   type:
                     type: string
                     enum:
@@ -94,7 +94,7 @@ spec:
                   validityDays:
                     type: integer
                     minimum: 1
-                    description: "Number of days for which the user certificate should be valid. If not configured, Clients CA configuration is used."
+                    description: "Number of days for which the user certificate should be valid. It has to be configured together with `renewalDays`, or none of them should be configured.The number should be bigger than 0 and than `renewalDays`.If not configured, Clients CA configuration is used."
                 required:
                 - type
                 description: "Authentication mechanism enabled for this Kafka user. The supported authentication mechanisms are `scram-sha-512`, `tls`, and `tls-external`. \n\n* `scram-sha-512` generates a secret with SASL SCRAM-SHA-512 credentials.\n* `tls` generates a secret with user certificate for mutual TLS authentication.\n* `tls-external` does not generate a user certificate.   But prepares the user for using mutual TLS authentication using a user certificate generated outside the User Operator.\n  ACLs and quotas set for this user are configured in the `CN=<username>` format.\n\nAuthentication is optional. If authentication is not configured, no credentials are generated. ACLs and quotas set for the user are configured in the `<username>` format suitable for SASL authentication."

systemtest/src/test/java/io/strimzi/systemtest/operators/user/UserST.java

@@ -87,8 +87,8 @@ class UserST extends AbstractST {
 
     private TestStorage sharedTestStorage;
     private String scraperPodName = "";
-    private final int defaultValidityDays = 200;
-    private final int defaultRenewalDays = 20;
+    private final int caValidityDays = 10;
+    private final int caRenewalDays = 5;
 
     @TestDoc(
         description = @Desc("Verifies that Kafka users with names longer than 64 characters are rejected, while users with valid names are accepted."),
@@ -572,24 +572,23 @@ void testTlsExternalUser() {
         description = @Desc("Verifies functionality of the mTLS `validityDays` and `renewalDays` configured inside each KafkaUser."),
         steps = {
             @Step(value = "Create `KafkaTopic` to which we will send (and from which we will receive) messages - created in existing Kafka cluster.", expected = "`KafkaTopic` is created."),
-            @Step(value = "Create `KafkaUser` with TLS authentication; together with default `validityDays` (200 days) and `renewalDays` (20 days) - configured in User operator.", expected = "`KafkaUser` is created with defaults."),
+            @Step(value = "Create `KafkaUser` with TLS authentication; without configuring the `validityDays` and `renewalDays` - values from User Operator are taken.", expected = "`KafkaUser` is created with values from User Operator."),
             @Step(value = "Obtain the `KafkaUser`'s `Secret` and check validity period of the user certificate.", expected = "Validity period should be default - 200 days."),
             @Step(value = "Do message transmission to verify, that we are able to connect to Kafka cluster with the TLS `KafkaUser`.", expected = "Messages are successfully sent and received."),
-            @Step(value = "Change the `validityDays` and `renewalDays` in the `KafkaUser` `.spec.authentication` to 60 and 10.", expected = "The `validityDays` and `renewalDays` should be changed in the `KafkaUser`."),
-            @Step(value = "Because we changed the `validityDays` and `renewalDays`, we need to force renew the certificate using the `strimzi.io/force-renew=true` annotation",
-                expected = "The user certificate was renewed."),
-            @Step(value = "Obtain the `KafkaUser`'s `Secret` again and check the validity period of the user certificate.", expected = "Validity period should be 60 days."),
+            @Step(value = "Change the `validityDays` and `renewalDays` in the `KafkaUser` `.spec.authentication` to 40 and 20.", expected = "The `validityDays` and `renewalDays` should be changed in the `KafkaUser`."),
+            @Step(value = "Because of the change of `validityDays` and `renewalDays` (and because of the values inside), the certificate will be renewed", expected = "The user certificate was renewed."),
+            @Step(value = "Obtain the `KafkaUser`'s `Secret` again and check the validity period of the user certificate.", expected = "Validity period should be 40 days."),
             @Step(value = "Do message transmission again to verify, that we are able to connect to Kafka cluster with the new user's certificate.", expected = "Messages are successfully sent and received using new certificate."),
         },
         labels = {
             @Label(TestDocsLabels.USER_OPERATOR)
         }
     )
     @ParallelTest
-    void testTlsValidityDaysWithForceRenewal() {
+    void testTlsValidityDays() {
         final TestStorage testStorage = new TestStorage(KubeResourceManager.get().getTestContext());
-        final int newValidityDays = 60;
-        final int newRenewalDays = 10;
+        final int newValidityDays = 40;
+        final int newRenewalDays = 20;
 
         KubeResourceManager.get().createResourceWithWait(KafkaTopicTemplates.topic(testStorage.getNamespaceName(), testStorage.getTopicName(), sharedTestStorage.getClusterName()).build());
 
@@ -600,7 +599,7 @@ void testTlsValidityDaysWithForceRenewal() {
         String userCertificate = tlsUserSecret.getData().get("user.crt");
 
         // check that notBefore and notAfter contains really the default value of validityDays
-        assertThat("validity period of the certificate has incorrect value", KafkaUserUtils.getValidityDaysOfCertificate(userCertificate), is(defaultValidityDays));
+        assertThat("validity period of the certificate has incorrect value", KafkaUserUtils.getValidityDaysOfCertificate(userCertificate), is(caValidityDays));
 
         LOGGER.info("Produce and consume messages before changing the validity - in order to see that everything works as expected.");
         final KafkaProducerConsumer kafkaProducerConsumer = new KafkaProducerConsumerBuilder()
@@ -677,8 +676,8 @@ void setup() {
                     )
                 .endKafka()
                 .withNewClientsCa()
-                    .withValidityDays(defaultValidityDays)
-                    .withRenewalDays(defaultRenewalDays)
+                    .withValidityDays(caValidityDays)
+                    .withRenewalDays(caRenewalDays)
                 .endClientsCa()
             .endSpec()
             .build(),

user-operator/src/main/java/io/strimzi/operator/user/model/KafkaUserModel.java

@@ -221,24 +221,24 @@ public Secret generateSecret(boolean generatePkcs12Stores)  {
      * @param clientsCaCertSecret   The clients CA certificate Secret.
      * @param clientsCaKeySecret    The clients CA key Secret.
      * @param userSecret            Secret with the user certificate
-     * @param defaultValidityDays   The default number of days (configured in Clients CA) the certificate should be valid for.
-     * @param defaultRenewalDays    The default renewal days (configured in Clients CA).
+     * @param caValidityDays        The default number of days (configured in Clients CA) the certificate should be valid for.
+     * @param caRenewalDays         The default renewal days (configured in Clients CA).
      * @param maintenanceWindows    List of configured maintenance windows
      * @param clock                 The clock for supplying the reconciler with the time instant of each reconciliation cycle.
      *                              That time is used for checking maintenance windows
      * @param generatePkcs12Stores  Flag indicating whether PKCS12 keystores should be generated for the user certificates
      */
     @SuppressWarnings("checkstyle:BooleanExpressionComplexity")
     public void maybeGenerateCertificates(Reconciliation reconciliation, CertManager certManager, PasswordGenerator passwordGenerator,
-                                          Secret clientsCaCertSecret, Secret clientsCaKeySecret, Secret userSecret, int defaultValidityDays,
-                                          int defaultRenewalDays, List<String> maintenanceWindows, Clock clock, boolean generatePkcs12Stores) {
+                                          Secret clientsCaCertSecret, Secret clientsCaKeySecret, Secret userSecret, int caValidityDays,
+                                          int caRenewalDays, List<String> maintenanceWindows, Clock clock, boolean generatePkcs12Stores) {
         // in case that validityDays and renewalDays are configured inside the authentication part of KafkaUser,
         // use those instead of default Clients CA configuration
         // we are checking it here as we have all needed information about the KafkaUser configuration and also default configuration of Clients CA
         KafkaUserTlsClientAuthentication kafkaUserTlsClientAuthentication = (KafkaUserTlsClientAuthentication) authentication;
 
-        int validityDays = kafkaUserTlsClientAuthentication.getValidityDays() != null ? kafkaUserTlsClientAuthentication.getValidityDays() : defaultValidityDays;
-        int renewalDays = kafkaUserTlsClientAuthentication.getRenewalDays() != null ? kafkaUserTlsClientAuthentication.getRenewalDays() : defaultRenewalDays;
+        int validityDays = kafkaUserTlsClientAuthentication.getValidityDays() != null ? kafkaUserTlsClientAuthentication.getValidityDays() : caValidityDays;
+        int renewalDays = kafkaUserTlsClientAuthentication.getRenewalDays() != null ? kafkaUserTlsClientAuthentication.getRenewalDays() : caRenewalDays;
         validateCACertificates(clientsCaCertSecret, clientsCaKeySecret);
 
         ClientsCa clientsCa = new ClientsCa(