Skip to content

Commit

Permalink
Simplify Ca and CaReconciler logic (#10809)
Browse files Browse the repository at this point in the history
Signed-off-by: Katherine Stanley <11195226+katheris@users.noreply.github.com>
katheris authored Nov 21, 2024
1 parent a2ac9ea commit 6ba9cc5
Showing 9 changed files with 47 additions and 88 deletions.
Original file line number Diff line number Diff line change
@@ -93,6 +93,11 @@ public String toString() {
return "cluster-ca";
}

@Override
protected String caName() {
return "Cluster CA";
}

/**
* Prepares the Cruise Control certificate. It either reuses the existing certificate, renews it or generates new
* certificate if needed.
Original file line number Diff line number Diff line change
@@ -45,7 +45,6 @@
import io.strimzi.operator.common.auth.TlsPemIdentity;
import io.strimzi.operator.common.model.Ca;
import io.strimzi.operator.common.model.ClientsCa;
import io.strimzi.operator.common.model.InvalidResourceException;
import io.strimzi.operator.common.model.Labels;
import io.strimzi.operator.common.model.PasswordGenerator;
import io.strimzi.operator.common.operator.resource.ReconcileResult;
@@ -258,24 +257,18 @@ Future<Void> reconcileCas(Clock clock) {
}
}

// When we are not supposed to generate the CA, but it does not exist, we should just throw an error
checkCustomCaSecret(clusterCaConfig, clusterCaCertSecret, clusterCaKeySecret, "Cluster CA");

clusterCa = new ClusterCa(reconciliation, certManager, passwordGenerator, reconciliation.name(), clusterCaCertSecret,
clusterCaKeySecret,
ModelUtils.getCertificateValidity(clusterCaConfig),
ModelUtils.getRenewalDays(clusterCaConfig),
clusterCaConfig == null || clusterCaConfig.isGenerateCertificateAuthority(), clusterCaConfig != null ? clusterCaConfig.getCertificateExpirationPolicy() : null);
clusterCa.createRenewOrReplace(
reconciliation.namespace(), reconciliation.name(), caLabels,
reconciliation.namespace(), caLabels,
clusterCaCertLabels, clusterCaCertAnnotations,
clusterCaConfig != null && !clusterCaConfig.isGenerateSecretOwnerReference() ? null : ownerRef,
clusterCaSecrets,
Util.isMaintenanceTimeWindowsSatisfied(reconciliation, maintenanceWindows, clock.instant()));

// When we are not supposed to generate the CA, but it does not exist, we should just throw an error
checkCustomCaSecret(clientsCaConfig, clientsCaCertSecret, clientsCaKeySecret, "Clients CA");

clientsCa = new ClientsCa(reconciliation, certManager,
passwordGenerator, clientsCaCertName,
clientsCaCertSecret, clientsCaKeyName,
@@ -284,7 +277,7 @@ Future<Void> reconcileCas(Clock clock) {
ModelUtils.getRenewalDays(clientsCaConfig),
clientsCaConfig == null || clientsCaConfig.isGenerateCertificateAuthority(),
clientsCaConfig != null ? clientsCaConfig.getCertificateExpirationPolicy() : null);
clientsCa.createRenewOrReplace(reconciliation.namespace(), reconciliation.name(),
clientsCa.createRenewOrReplace(reconciliation.namespace(),
caLabels, Map.of(), Map.of(),
clientsCaConfig != null && !clientsCaConfig.isGenerateSecretOwnerReference() ? null : ownerRef,
clientsCaSecrets,
@@ -321,22 +314,6 @@ Future<Void> reconcileCas(Clock clock) {
});
}

/**
* Utility method for checking the Secret existence when custom CA is used. The custom CA is configured but the
* secrets do not exist, it will throw InvalidConfigurationException.
*
* @param ca The CA Configuration from the Custom Resource
* @param certSecret Secret with the certificate public key
* @param keySecret Secret with the certificate private key
* @param caDescription The name of the CA for which this check is executed ("Cluster CA" or "Clients CA" - used
* in the exception message)
*/
private void checkCustomCaSecret(CertificateAuthority ca, Secret certSecret, Secret keySecret, String caDescription) {
if (ca != null && !ca.isGenerateCertificateAuthority() && (certSecret == null || keySecret == null)) {
throw new InvalidResourceException(caDescription + " should not be generated, but the secrets were not found.");
}
}

/**
* Asynchronously reconciles the cluster operator Secret used to connect to Kafka and ZooKeeper.
* This only updates the Secret if the latest Cluster CA is fully trusted across the cluster, otherwise if
Original file line number Diff line number Diff line change
@@ -41,7 +41,7 @@ public void testRemoveExpiredCertificate() {

ClusterCa clusterCa = new ClusterCa(Reconciliation.DUMMY_RECONCILIATION, new OpenSslCertManager(clock), new PasswordGenerator(10, "a", "a"), cluster, null, null);
clusterCa.setClock(clock);
clusterCa.createRenewOrReplace(namespace, cluster, emptyMap(), emptyMap(), emptyMap(), null, List.of(), true);
clusterCa.createRenewOrReplace(namespace, emptyMap(), emptyMap(), emptyMap(), null, List.of(), true);
assertThat(clusterCa.caCertSecret().getData().size(), is(3));

// force key replacement so certificate renewal ...
@@ -56,7 +56,7 @@ public void testRemoveExpiredCertificate() {

clusterCa = new ClusterCa(Reconciliation.DUMMY_RECONCILIATION, new OpenSslCertManager(clock), new PasswordGenerator(10, "a", "a"), cluster, clusterCa.caCertSecret(), caKeySecretWithReplaceAnno);
clusterCa.setClock(clock);
clusterCa.createRenewOrReplace(namespace, cluster, emptyMap(), emptyMap(), emptyMap(), null, List.of(), true);
clusterCa.createRenewOrReplace(namespace, emptyMap(), emptyMap(), emptyMap(), null, List.of(), true);
assertThat(clusterCa.caCertSecret().getData().size(), is(4));
assertThat(clusterCa.caCertSecret().getData().containsKey("ca-2023-03-23T09-00-00Z.crt"), is(true));

@@ -66,7 +66,7 @@ public void testRemoveExpiredCertificate() {

clusterCa = new ClusterCa(Reconciliation.DUMMY_RECONCILIATION, new OpenSslCertManager(), new PasswordGenerator(10, "a", "a"), cluster, clusterCa.caCertSecret(), clusterCa.caKeySecret());
clusterCa.setClock(clock);
clusterCa.createRenewOrReplace(namespace, cluster, emptyMap(), emptyMap(), emptyMap(), null, List.of(), true);
clusterCa.createRenewOrReplace(namespace, emptyMap(), emptyMap(), emptyMap(), null, List.of(), true);
assertThat(clusterCa.caCertSecret().getData().size(), is(3));
assertThat(clusterCa.caCertSecret().getData().containsKey("ca-2023-03-23T09-00-00Z.crt"), is(false));
}
@@ -79,7 +79,7 @@ public void testIsExpiringCertificate() {

ClusterCa clusterCa = new ClusterCa(Reconciliation.DUMMY_RECONCILIATION, new OpenSslCertManager(clock), new PasswordGenerator(10, "a", "a"), cluster, null, null);
clusterCa.setClock(clock);
clusterCa.createRenewOrReplace(namespace, cluster, emptyMap(), emptyMap(), emptyMap(), null, List.of(), true);
clusterCa.createRenewOrReplace(namespace, emptyMap(), emptyMap(), emptyMap(), null, List.of(), true);

// check certificate expiration out of the renewal period, certificate is not expiring
instantExpected = "2023-02-15T09:00:00Z";
@@ -102,7 +102,7 @@ public void testRemoveOldCertificate() {

ClusterCa clusterCa = new ClusterCa(Reconciliation.DUMMY_RECONCILIATION, new OpenSslCertManager(clock), new PasswordGenerator(10, "a", "a"), cluster, null, null);
clusterCa.setClock(clock);
clusterCa.createRenewOrReplace(namespace, cluster, emptyMap(), emptyMap(), emptyMap(), null, List.of(), true);
clusterCa.createRenewOrReplace(namespace, emptyMap(), emptyMap(), emptyMap(), null, List.of(), true);
assertThat(clusterCa.caCertSecret().getData().size(), is(3));

// force key replacement so certificate renewal ...
@@ -117,7 +117,7 @@ public void testRemoveOldCertificate() {

clusterCa = new ClusterCa(Reconciliation.DUMMY_RECONCILIATION, new OpenSslCertManager(clock), new PasswordGenerator(10, "a", "a"), cluster, clusterCa.caCertSecret(), caKeySecretWithReplaceAnno);
clusterCa.setClock(clock);
clusterCa.createRenewOrReplace(namespace, cluster, emptyMap(), emptyMap(), emptyMap(), null, List.of(), true);
clusterCa.createRenewOrReplace(namespace, emptyMap(), emptyMap(), emptyMap(), null, List.of(), true);
assertThat(clusterCa.caCertSecret().getData().size(), is(4));
assertThat(clusterCa.caCertSecret().getData().containsKey("ca-2023-03-23T09-00-00Z.crt"), is(true));

Original file line number Diff line number Diff line change
@@ -258,9 +258,9 @@ private void checkHeadlessService(Service headless) {

private Secret generateBrokerSecret(Set<String> externalBootstrapAddress, Map<Integer, Set<String>> externalAddresses) {
ClusterCa clusterCa = new ClusterCa(Reconciliation.DUMMY_RECONCILIATION, new OpenSslCertManager(), new PasswordGenerator(10, "a", "a"), CLUSTER, null, null);
clusterCa.createRenewOrReplace(NAMESPACE, CLUSTER, Map.of(), Map.of(), Map.of(), null, List.of(), true);
clusterCa.createRenewOrReplace(NAMESPACE, Map.of(), Map.of(), Map.of(), null, List.of(), true);
ClientsCa clientsCa = new ClientsCa(Reconciliation.DUMMY_RECONCILIATION, new OpenSslCertManager(), new PasswordGenerator(10, "a", "a"), null, null, null, null, 365, 30, true, CertificateExpirationPolicy.RENEW_CERTIFICATE);
clientsCa.createRenewOrReplace(NAMESPACE, CLUSTER, Map.of(), Map.of(), Map.of(), null, List.of(), true);
clientsCa.createRenewOrReplace(NAMESPACE, Map.of(), Map.of(), Map.of(), null, List.of(), true);

return KC.generateCertificatesSecret(clusterCa, clientsCa, null, externalBootstrapAddress, externalAddresses, true);
}
Original file line number Diff line number Diff line change
@@ -228,9 +228,9 @@ private void checkHeadlessService(Service headless) {

private Secret generateBrokerSecret(Set<String> externalBootstrapAddress, Map<Integer, Set<String>> externalAddresses) {
ClusterCa clusterCa = new ClusterCa(Reconciliation.DUMMY_RECONCILIATION, new OpenSslCertManager(), new PasswordGenerator(10, "a", "a"), CLUSTER, null, null);
clusterCa.createRenewOrReplace(NAMESPACE, CLUSTER, emptyMap(), emptyMap(), emptyMap(), null, List.of(), true);
clusterCa.createRenewOrReplace(NAMESPACE, emptyMap(), emptyMap(), emptyMap(), null, List.of(), true);
ClientsCa clientsCa = new ClientsCa(Reconciliation.DUMMY_RECONCILIATION, new OpenSslCertManager(), new PasswordGenerator(10, "a", "a"), null, null, null, null, 365, 30, true, CertificateExpirationPolicy.RENEW_CERTIFICATE);
clientsCa.createRenewOrReplace(NAMESPACE, CLUSTER, emptyMap(), emptyMap(), emptyMap(), null, List.of(), true);
clientsCa.createRenewOrReplace(NAMESPACE, emptyMap(), emptyMap(), emptyMap(), null, List.of(), true);

return KC.generateCertificatesSecret(clusterCa, clientsCa, null, externalBootstrapAddress, externalAddresses, true);
}
Original file line number Diff line number Diff line change
@@ -149,7 +149,7 @@ private void checkHeadlessService(Service headless) {

private Secret generateCertificatesSecret() {
ClusterCa clusterCa = new ClusterCa(Reconciliation.DUMMY_RECONCILIATION, new OpenSslCertManager(), new PasswordGenerator(10, "a", "a"), CLUSTER, null, null);
clusterCa.createRenewOrReplace(NAMESPACE, CLUSTER, emptyMap(), emptyMap(), emptyMap(), null, List.of(), true);
clusterCa.createRenewOrReplace(NAMESPACE, emptyMap(), emptyMap(), emptyMap(), null, List.of(), true);

return ZC.generateCertificatesSecret(clusterCa, null, true);
}
Loading
Oops, something went wrong.

0 comments on commit 6ba9cc5

Please sign in to comment.