[GitHub] [lucene-solr-operator] HoustonPutman commented on a change in pull request #151: Integrate with cert-manager to issue TLS certs for Solr

[GitBox <gi...@apache.org>]

HoustonPutman commented on a change in pull request #151:
URL: https://github.com/apache/lucene-solr-operator/pull/151#discussion_r568020961



##########
File path: api/v1beta1/solrcloud_types.go
##########
@@ -758,8 +770,12 @@ func (sc *SolrCloud) CommonServiceName() string {
 }
 
 // InternalURLForCloud returns the name of the common service for the cloud
-func InternalURLForCloud(cloudName string, namespace string) string {
-	return fmt.Sprintf("http://%s-solrcloud-common.%s", cloudName, namespace)
+func InternalURLForCloud(sc *SolrCloud) string {
+	urlScheme := "http"

Review comment:
       this should probably use `sc.urlScheme()`

##########
File path: controllers/solrcloud_controller.go
##########
@@ -261,12 +268,77 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 		blockReconciliationOfStatefulSet = true
 	}
 
+	tlsCertMd5 := ""
+	needsPkcs12InitContainer := false // flag if the StatefulSet needs an additional initCont to create PKCS12 keystore
+	// don't start reconciling TLS until we have ZK connectivity, avoids TLS code having to check for ZK
+	if !blockReconciliationOfStatefulSet && instance.Spec.SolrTLS != nil {
+		ctx := context.TODO()
+		// Create the autogenerated TLS Cert and wait for it to be issued
+		if instance.Spec.SolrTLS.AutoCreate != nil {
+			tlsReady, err := r.reconcileAutoCreateTLS(ctx, instance)
+			// don't create the StatefulSet until we have a cert, which can take a while for a Let's Encrypt Issuer
+			if !tlsReady || err != nil {
+				if err != nil {
+					r.Log.Error(err, "Reconcile TLS Certificate failed")
+				} else {
+					wait := 30 * time.Second
+					if instance.Spec.SolrTLS.AutoCreate.IssuerRef == nil {
+						// this is a self-signed cert, so no need to wait very long for it to issue
+						wait = 2 * time.Second
+					}
+					requeueOrNot.RequeueAfter = wait
+				}
+				return requeueOrNot, err

Review comment:
       Instead of returning here, you can use `blockReconciliationOfStatefulSet=true`, to make sure that only the creation of the statefulSet is blocked until the Certs are issued.

##########
File path: main.go
##########
@@ -65,6 +69,7 @@ func init() {
 
 	_ = solrv1beta1.AddToScheme(scheme)
 	_ = zkv1beta1.AddToScheme(scheme)
+	_ = certv1.AddToScheme(scheme)
 
 	// +kubebuilder:scaffold:scheme
 	flag.BoolVar(&useZookeeperCRD, "zk-operator", true, "The operator will not use the zk operator & crd when this flag is set to false.")

Review comment:
       Do we need a flag for the Cert Manager CRDs? Or are we going to assume that all users have these installed in their cluster?

##########
File path: controllers/solrcloud_controller.go
##########
@@ -422,9 +494,9 @@ func reconcileCloudStatus(r *SolrCloudReconciler, solrCloud *solr.SolrCloud, new
 		nodeStatus := solr.SolrNodeStatus{}
 		nodeStatus.Name = p.Name
 		nodeStatus.NodeName = p.Spec.NodeName
-		nodeStatus.InternalAddress = "http://" + solrCloud.InternalNodeUrl(nodeStatus.Name, true)

Review comment:
       maybe these methods should just include an option to `includeScheme`, just like `includePort`. But we could do this separately.

##########
File path: controllers/solrcloud_controller.go
##########
@@ -716,7 +785,10 @@ func (r *SolrCloudReconciler) SetupWithManagerAndReconciler(mgr ctrl.Manager, re
 		Owns(&corev1.ConfigMap{}).
 		Owns(&appsv1.StatefulSet{}).
 		Owns(&corev1.Service{}).
-		Owns(&extv1.Ingress{})
+		Owns(&extv1.Ingress{}).
+		Owns(&appsv1.Deployment{}).

Review comment:
       Did the deployment get left over from a merge with master?
   
   Also do any of the Cert manager things need to be here?

##########
File path: controllers/solrcloud_controller.go
##########
@@ -772,3 +848,188 @@ func (r *SolrCloudReconciler) indexAndWatchForProvidedConfigMaps(mgr ctrl.Manage
 		},
 		builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})), nil
 }
+
+// Reconciles the TLS cert, returns either a bool to indicate if the cert is ready or an error
+func (r *SolrCloudReconciler) reconcileAutoCreateTLS(ctx context.Context, instance *solr.SolrCloud) (bool, error) {
+
+	// short circuit this method with a quick check if the cert exists and is ready
+	// this is useful b/c it may take many minutes for a cert to be issued, so we avoid
+	// all the other checking that happens below while we're waiting for the cert
+	foundCert := &certv1.Certificate{}
+	if err := r.Get(ctx, types.NamespacedName{Name: instance.Spec.SolrTLS.AutoCreate.Name, Namespace: instance.Namespace}, foundCert); err == nil {
+		// cert exists, but is it ready? need to wait until we see the TLS secret
+		if foundTLSSecret := r.isCertificateReady(ctx, foundCert, instance.Spec.SolrTLS); foundTLSSecret != nil {
+			cert := util.GenerateCertificate(instance)
+			return r.afterCertificateReady(ctx, instance, &cert, foundCert, foundTLSSecret)
+		}
+	}
+
+	r.Log.Info("Reconciling TLS config", "tls", instance.Spec.SolrTLS)
+
+	// cert not found, do full reconcile for TLS ...
+	var err error
+	var tlsReady bool
+
+	// First, create the keystore password secret if needed
+	keystoreSecret := util.GenerateKeystoreSecret(instance)
+	foundSecret := &corev1.Secret{}
+	err = r.Get(ctx, types.NamespacedName{Name: keystoreSecret.Name, Namespace: keystoreSecret.Namespace}, foundSecret)
+	if err != nil && errors.IsNotFound(err) {
+		r.Log.Info("Creating keystore secret", "namespace", keystoreSecret.Namespace, "name", keystoreSecret.Name)
+		if err := controllerutil.SetControllerReference(instance, &keystoreSecret, r.scheme); err != nil {
+			return false, err
+		}
+		err = r.Create(ctx, &keystoreSecret)
+	}
+	if err != nil {
+		return false, err
+	}
+
+	// Create a self-signed cert issuer if no issuerRef provided
+	if instance.Spec.SolrTLS.AutoCreate.IssuerRef == nil {
+		issuerName := fmt.Sprintf("%s-selfsigned-issuer", instance.Name)
+		foundIssuer := &certv1.Issuer{}
+		err = r.Get(ctx, types.NamespacedName{Name: issuerName, Namespace: instance.Namespace}, foundIssuer)
+		if err != nil && errors.IsNotFound(err) {
+			// specified Issuer not found, let's go create a self-signed for this
+			issuer := util.GenerateSelfSignedIssuer(instance, issuerName)
+			if err := controllerutil.SetControllerReference(instance, &issuer, r.scheme); err != nil {
+				return false, err
+			}
+			r.Log.Info("Creating Self-signed Certificate Issuer", "issuer", issuer)
+			err = r.Create(ctx, &issuer)
+		} else if err == nil {
+			r.Log.Info("Found Self-signed Certificate Issuer", "issuer", issuerName)
+		}
+		if err != nil {
+			return false, err
+		}
+	} else {
+		// real problems arise if we create the Certificate and the Issuer doesn't exist so make we have a good config here
+		if instance.Spec.SolrTLS.AutoCreate.IssuerRef.Kind == "Issuer" {
+			foundIssuer := &certv1.Issuer{}
+			err = r.Get(ctx, types.NamespacedName{Name: instance.Spec.SolrTLS.AutoCreate.IssuerRef.Name, Namespace: instance.Namespace}, foundIssuer)
+			if err != nil {
+				if errors.IsNotFound(err) {
+					r.Log.Info("cert-manager Issuer not found in namespace, cannot create a TLS certificate without an Issuer",
+						"issuer", instance.Spec.SolrTLS.AutoCreate.IssuerRef.Name, "ns", instance.Namespace)
+				}
+				return false, err
+			}
+		} // else assume ClusterIssuer and good luck
+	}
+
+	// Reconcile the Certificate to use for TLS ... A Certificate is a request to Issue the cert, the
+	// actual cert lives in a TLS secret created by the Issuer
+	cert := util.GenerateCertificate(instance)
+	err = r.Get(ctx, types.NamespacedName{Name: cert.Name, Namespace: cert.Namespace}, foundCert)
+	if err != nil && errors.IsNotFound(err) {
+		r.Log.Info("Creating Certificate", "cert", cert)
+		// Set the operator as the owner of the cert
+		if err := controllerutil.SetControllerReference(instance, &cert, r.scheme); err != nil {
+			return false, err
+		}
+		// Create the cert
+		err = r.Create(ctx, &cert)
+		if err != nil {
+			return false, err
+		}
+	} else if err == nil {
+		r.Log.Info("Found Certificate, checking if it is ready", "cert", foundCert.Name)
+		if foundTLSSecret := r.isCertificateReady(ctx, foundCert, instance.Spec.SolrTLS); foundTLSSecret != nil {
+			tlsReady, err = r.afterCertificateReady(ctx, instance, &cert, foundCert, foundTLSSecret)
+			if tlsReady {
+				r.Log.Info("TLS Certificate reconciled.", "cert", foundCert.Name)
+			}
+		} else {
+			r.Log.Info("Certificate not ready, current status", "status", foundCert.Status)
+		}
+	}
+
+	if err != nil {
+		return false, err
+	}
+
+	return tlsReady, nil
+}
+
+func (r *SolrCloudReconciler) isCertificateReady(ctx context.Context, cert *certv1.Certificate, tlsOpts *solr.SolrTLSOptions) *corev1.Secret {
+	// Cert is ready, lookup the secret holding the keystore
+	foundTLSSecret := &corev1.Secret{}
+	err := r.Get(ctx, types.NamespacedName{Name: cert.Spec.SecretName, Namespace: cert.Namespace}, foundTLSSecret)
+	if err != nil {
+		if errors.IsNotFound(err) {
+			r.Log.Info("TLS secret not found", "name", cert.Spec.SecretName)
+		} else {
+			r.Log.Error(err, "TLS secret lookup failed", "name", cert.Spec.SecretName)
+		}
+		foundTLSSecret = nil
+	}
+
+	if foundTLSSecret == nil {
+		if cert.Status.Conditions != nil {
+			for _, cond := range cert.Status.Conditions {
+				if cond.Type == certv1.CertificateConditionIssuing {
+					r.Log.Info("Certificate is still issuing", "name", cert.Name, "status", cond.Status)
+					break
+				}
+			}
+		}
+	}
+
+	// Make sure the secret containing the keystore password exists as well
+	if foundTLSSecret != nil {
+		keyStorePasswordSecret := &corev1.Secret{}
+		err := r.Get(ctx, types.NamespacedName{Name: tlsOpts.KeyStorePasswordSecret.Name, Namespace: foundTLSSecret.Namespace}, keyStorePasswordSecret)
+		if err != nil {
+			r.Log.Error(err, "Keystore password secret not found", "name", tlsOpts.KeyStorePasswordSecret.Name)

Review comment:
       Is this also an unrecoverable error? Might be good to add some comments around failure scenarios because this isn't really clear to people who aren't very good at the cert stuff (very much me).

##########
File path: controllers/solrcloud_controller.go
##########
@@ -182,6 +188,7 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 		}
 	}
 
+	// Generate ConfigMap unless the user supplied a custom ConfigMap for solr.xml

Review comment:
       Duplicate?

##########
File path: controllers/solrcloud_controller.go
##########
@@ -261,12 +268,77 @@ func (r *SolrCloudReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 		blockReconciliationOfStatefulSet = true
 	}
 
+	tlsCertMd5 := ""
+	needsPkcs12InitContainer := false // flag if the StatefulSet needs an additional initCont to create PKCS12 keystore
+	// don't start reconciling TLS until we have ZK connectivity, avoids TLS code having to check for ZK
+	if !blockReconciliationOfStatefulSet && instance.Spec.SolrTLS != nil {
+		ctx := context.TODO()
+		// Create the autogenerated TLS Cert and wait for it to be issued
+		if instance.Spec.SolrTLS.AutoCreate != nil {
+			tlsReady, err := r.reconcileAutoCreateTLS(ctx, instance)
+			// don't create the StatefulSet until we have a cert, which can take a while for a Let's Encrypt Issuer
+			if !tlsReady || err != nil {
+				if err != nil {
+					r.Log.Error(err, "Reconcile TLS Certificate failed")
+				} else {
+					wait := 30 * time.Second
+					if instance.Spec.SolrTLS.AutoCreate.IssuerRef == nil {
+						// this is a self-signed cert, so no need to wait very long for it to issue
+						wait = 2 * time.Second
+					}
+					requeueOrNot.RequeueAfter = wait
+				}
+				return requeueOrNot, err
+			}
+		}
+
+		// go get the current version of the TLS secret so changes get picked up by our STS spec (via env vars)
+		foundTLSSecret := &corev1.Secret{}
+		lookupErr := r.Get(ctx, types.NamespacedName{Name: instance.Spec.SolrTLS.PKCS12Secret.Name, Namespace: instance.Namespace}, foundTLSSecret)
+		if lookupErr != nil {
+			r.Log.Info("TLS secret not found.", "secret", instance.Spec.SolrTLS.PKCS12Secret.Name)

Review comment:
       This should log an error, right? If it's blocking our reconciliation.
   
   Or is this going to happen as we are waiting for things to be created? If so, the log line should probably say something like that.

##########
File path: controllers/util/common.go
##########
@@ -248,6 +248,11 @@ func CopyIngressFields(from, to *extv1.Ingress, logger logr.Logger) bool {
 		}
 	}
 
+	if !requireUpdate && !DeepEqualWithNils(to.Spec.TLS, from.Spec.TLS) {

Review comment:
       why `!requireUpdate &&`? Shouldn't this always be updated, even if other things need to be updated too?

##########
File path: controllers/solrcloud_controller.go
##########
@@ -772,3 +848,188 @@ func (r *SolrCloudReconciler) indexAndWatchForProvidedConfigMaps(mgr ctrl.Manage
 		},
 		builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})), nil
 }
+
+// Reconciles the TLS cert, returns either a bool to indicate if the cert is ready or an error
+func (r *SolrCloudReconciler) reconcileAutoCreateTLS(ctx context.Context, instance *solr.SolrCloud) (bool, error) {
+
+	// short circuit this method with a quick check if the cert exists and is ready
+	// this is useful b/c it may take many minutes for a cert to be issued, so we avoid
+	// all the other checking that happens below while we're waiting for the cert
+	foundCert := &certv1.Certificate{}
+	if err := r.Get(ctx, types.NamespacedName{Name: instance.Spec.SolrTLS.AutoCreate.Name, Namespace: instance.Namespace}, foundCert); err == nil {
+		// cert exists, but is it ready? need to wait until we see the TLS secret
+		if foundTLSSecret := r.isCertificateReady(ctx, foundCert, instance.Spec.SolrTLS); foundTLSSecret != nil {
+			cert := util.GenerateCertificate(instance)
+			return r.afterCertificateReady(ctx, instance, &cert, foundCert, foundTLSSecret)
+		}
+	}
+
+	r.Log.Info("Reconciling TLS config", "tls", instance.Spec.SolrTLS)
+
+	// cert not found, do full reconcile for TLS ...
+	var err error
+	var tlsReady bool
+
+	// First, create the keystore password secret if needed
+	keystoreSecret := util.GenerateKeystoreSecret(instance)
+	foundSecret := &corev1.Secret{}
+	err = r.Get(ctx, types.NamespacedName{Name: keystoreSecret.Name, Namespace: keystoreSecret.Namespace}, foundSecret)
+	if err != nil && errors.IsNotFound(err) {
+		r.Log.Info("Creating keystore secret", "namespace", keystoreSecret.Namespace, "name", keystoreSecret.Name)
+		if err := controllerutil.SetControllerReference(instance, &keystoreSecret, r.scheme); err != nil {
+			return false, err
+		}
+		err = r.Create(ctx, &keystoreSecret)
+	}
+	if err != nil {
+		return false, err
+	}
+
+	// Create a self-signed cert issuer if no issuerRef provided
+	if instance.Spec.SolrTLS.AutoCreate.IssuerRef == nil {
+		issuerName := fmt.Sprintf("%s-selfsigned-issuer", instance.Name)
+		foundIssuer := &certv1.Issuer{}
+		err = r.Get(ctx, types.NamespacedName{Name: issuerName, Namespace: instance.Namespace}, foundIssuer)
+		if err != nil && errors.IsNotFound(err) {
+			// specified Issuer not found, let's go create a self-signed for this
+			issuer := util.GenerateSelfSignedIssuer(instance, issuerName)
+			if err := controllerutil.SetControllerReference(instance, &issuer, r.scheme); err != nil {
+				return false, err
+			}
+			r.Log.Info("Creating Self-signed Certificate Issuer", "issuer", issuer)
+			err = r.Create(ctx, &issuer)
+		} else if err == nil {
+			r.Log.Info("Found Self-signed Certificate Issuer", "issuer", issuerName)
+		}
+		if err != nil {
+			return false, err
+		}
+	} else {
+		// real problems arise if we create the Certificate and the Issuer doesn't exist so make we have a good config here
+		if instance.Spec.SolrTLS.AutoCreate.IssuerRef.Kind == "Issuer" {
+			foundIssuer := &certv1.Issuer{}
+			err = r.Get(ctx, types.NamespacedName{Name: instance.Spec.SolrTLS.AutoCreate.IssuerRef.Name, Namespace: instance.Namespace}, foundIssuer)
+			if err != nil {
+				if errors.IsNotFound(err) {
+					r.Log.Info("cert-manager Issuer not found in namespace, cannot create a TLS certificate without an Issuer",
+						"issuer", instance.Spec.SolrTLS.AutoCreate.IssuerRef.Name, "ns", instance.Namespace)
+				}
+				return false, err
+			}
+		} // else assume ClusterIssuer and good luck
+	}
+
+	// Reconcile the Certificate to use for TLS ... A Certificate is a request to Issue the cert, the
+	// actual cert lives in a TLS secret created by the Issuer
+	cert := util.GenerateCertificate(instance)
+	err = r.Get(ctx, types.NamespacedName{Name: cert.Name, Namespace: cert.Namespace}, foundCert)
+	if err != nil && errors.IsNotFound(err) {
+		r.Log.Info("Creating Certificate", "cert", cert)
+		// Set the operator as the owner of the cert
+		if err := controllerutil.SetControllerReference(instance, &cert, r.scheme); err != nil {
+			return false, err
+		}
+		// Create the cert
+		err = r.Create(ctx, &cert)
+		if err != nil {
+			return false, err
+		}
+	} else if err == nil {
+		r.Log.Info("Found Certificate, checking if it is ready", "cert", foundCert.Name)
+		if foundTLSSecret := r.isCertificateReady(ctx, foundCert, instance.Spec.SolrTLS); foundTLSSecret != nil {
+			tlsReady, err = r.afterCertificateReady(ctx, instance, &cert, foundCert, foundTLSSecret)
+			if tlsReady {
+				r.Log.Info("TLS Certificate reconciled.", "cert", foundCert.Name)
+			}
+		} else {
+			r.Log.Info("Certificate not ready, current status", "status", foundCert.Status)
+		}
+	}
+
+	if err != nil {
+		return false, err
+	}
+
+	return tlsReady, nil
+}
+
+func (r *SolrCloudReconciler) isCertificateReady(ctx context.Context, cert *certv1.Certificate, tlsOpts *solr.SolrTLSOptions) *corev1.Secret {
+	// Cert is ready, lookup the secret holding the keystore
+	foundTLSSecret := &corev1.Secret{}
+	err := r.Get(ctx, types.NamespacedName{Name: cert.Spec.SecretName, Namespace: cert.Namespace}, foundTLSSecret)
+	if err != nil {
+		if errors.IsNotFound(err) {
+			r.Log.Info("TLS secret not found", "name", cert.Spec.SecretName)
+		} else {
+			r.Log.Error(err, "TLS secret lookup failed", "name", cert.Spec.SecretName)
+		}
+		foundTLSSecret = nil
+	}
+
+	if foundTLSSecret == nil {
+		if cert.Status.Conditions != nil {
+			for _, cond := range cert.Status.Conditions {
+				if cond.Type == certv1.CertificateConditionIssuing {
+					r.Log.Info("Certificate is still issuing", "name", cert.Name, "status", cond.Status)

Review comment:
       Should we do something if it's not issuing? That seems like an unrecoverable error state.

##########
File path: controllers/solrcloud_controller.go
##########
@@ -772,3 +848,188 @@ func (r *SolrCloudReconciler) indexAndWatchForProvidedConfigMaps(mgr ctrl.Manage
 		},
 		builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})), nil
 }
+
+// Reconciles the TLS cert, returns either a bool to indicate if the cert is ready or an error
+func (r *SolrCloudReconciler) reconcileAutoCreateTLS(ctx context.Context, instance *solr.SolrCloud) (bool, error) {
+
+	// short circuit this method with a quick check if the cert exists and is ready
+	// this is useful b/c it may take many minutes for a cert to be issued, so we avoid
+	// all the other checking that happens below while we're waiting for the cert
+	foundCert := &certv1.Certificate{}
+	if err := r.Get(ctx, types.NamespacedName{Name: instance.Spec.SolrTLS.AutoCreate.Name, Namespace: instance.Namespace}, foundCert); err == nil {
+		// cert exists, but is it ready? need to wait until we see the TLS secret
+		if foundTLSSecret := r.isCertificateReady(ctx, foundCert, instance.Spec.SolrTLS); foundTLSSecret != nil {
+			cert := util.GenerateCertificate(instance)
+			return r.afterCertificateReady(ctx, instance, &cert, foundCert, foundTLSSecret)
+		}
+	}
+
+	r.Log.Info("Reconciling TLS config", "tls", instance.Spec.SolrTLS)
+
+	// cert not found, do full reconcile for TLS ...
+	var err error
+	var tlsReady bool
+
+	// First, create the keystore password secret if needed
+	keystoreSecret := util.GenerateKeystoreSecret(instance)
+	foundSecret := &corev1.Secret{}
+	err = r.Get(ctx, types.NamespacedName{Name: keystoreSecret.Name, Namespace: keystoreSecret.Namespace}, foundSecret)
+	if err != nil && errors.IsNotFound(err) {
+		r.Log.Info("Creating keystore secret", "namespace", keystoreSecret.Namespace, "name", keystoreSecret.Name)
+		if err := controllerutil.SetControllerReference(instance, &keystoreSecret, r.scheme); err != nil {
+			return false, err
+		}
+		err = r.Create(ctx, &keystoreSecret)
+	}
+	if err != nil {
+		return false, err
+	}
+
+	// Create a self-signed cert issuer if no issuerRef provided
+	if instance.Spec.SolrTLS.AutoCreate.IssuerRef == nil {
+		issuerName := fmt.Sprintf("%s-selfsigned-issuer", instance.Name)
+		foundIssuer := &certv1.Issuer{}
+		err = r.Get(ctx, types.NamespacedName{Name: issuerName, Namespace: instance.Namespace}, foundIssuer)
+		if err != nil && errors.IsNotFound(err) {
+			// specified Issuer not found, let's go create a self-signed for this
+			issuer := util.GenerateSelfSignedIssuer(instance, issuerName)
+			if err := controllerutil.SetControllerReference(instance, &issuer, r.scheme); err != nil {
+				return false, err
+			}
+			r.Log.Info("Creating Self-signed Certificate Issuer", "issuer", issuer)
+			err = r.Create(ctx, &issuer)
+		} else if err == nil {
+			r.Log.Info("Found Self-signed Certificate Issuer", "issuer", issuerName)
+		}
+		if err != nil {
+			return false, err
+		}
+	} else {
+		// real problems arise if we create the Certificate and the Issuer doesn't exist so make we have a good config here
+		if instance.Spec.SolrTLS.AutoCreate.IssuerRef.Kind == "Issuer" {
+			foundIssuer := &certv1.Issuer{}
+			err = r.Get(ctx, types.NamespacedName{Name: instance.Spec.SolrTLS.AutoCreate.IssuerRef.Name, Namespace: instance.Namespace}, foundIssuer)
+			if err != nil {
+				if errors.IsNotFound(err) {
+					r.Log.Info("cert-manager Issuer not found in namespace, cannot create a TLS certificate without an Issuer",
+						"issuer", instance.Spec.SolrTLS.AutoCreate.IssuerRef.Name, "ns", instance.Namespace)
+				}
+				return false, err
+			}
+		} // else assume ClusterIssuer and good luck
+	}
+
+	// Reconcile the Certificate to use for TLS ... A Certificate is a request to Issue the cert, the
+	// actual cert lives in a TLS secret created by the Issuer
+	cert := util.GenerateCertificate(instance)
+	err = r.Get(ctx, types.NamespacedName{Name: cert.Name, Namespace: cert.Namespace}, foundCert)
+	if err != nil && errors.IsNotFound(err) {
+		r.Log.Info("Creating Certificate", "cert", cert)
+		// Set the operator as the owner of the cert
+		if err := controllerutil.SetControllerReference(instance, &cert, r.scheme); err != nil {
+			return false, err
+		}
+		// Create the cert
+		err = r.Create(ctx, &cert)
+		if err != nil {
+			return false, err
+		}
+	} else if err == nil {
+		r.Log.Info("Found Certificate, checking if it is ready", "cert", foundCert.Name)
+		if foundTLSSecret := r.isCertificateReady(ctx, foundCert, instance.Spec.SolrTLS); foundTLSSecret != nil {
+			tlsReady, err = r.afterCertificateReady(ctx, instance, &cert, foundCert, foundTLSSecret)
+			if tlsReady {
+				r.Log.Info("TLS Certificate reconciled.", "cert", foundCert.Name)
+			}
+		} else {
+			r.Log.Info("Certificate not ready, current status", "status", foundCert.Status)
+		}
+	}
+
+	if err != nil {
+		return false, err
+	}
+
+	return tlsReady, nil
+}
+
+func (r *SolrCloudReconciler) isCertificateReady(ctx context.Context, cert *certv1.Certificate, tlsOpts *solr.SolrTLSOptions) *corev1.Secret {
+	// Cert is ready, lookup the secret holding the keystore
+	foundTLSSecret := &corev1.Secret{}
+	err := r.Get(ctx, types.NamespacedName{Name: cert.Spec.SecretName, Namespace: cert.Namespace}, foundTLSSecret)
+	if err != nil {
+		if errors.IsNotFound(err) {
+			r.Log.Info("TLS secret not found", "name", cert.Spec.SecretName)
+		} else {
+			r.Log.Error(err, "TLS secret lookup failed", "name", cert.Spec.SecretName)
+		}
+		foundTLSSecret = nil
+	}
+
+	if foundTLSSecret == nil {
+		if cert.Status.Conditions != nil {
+			for _, cond := range cert.Status.Conditions {
+				if cond.Type == certv1.CertificateConditionIssuing {
+					r.Log.Info("Certificate is still issuing", "name", cert.Name, "status", cond.Status)
+					break
+				}
+			}
+		}
+	}
+
+	// Make sure the secret containing the keystore password exists as well
+	if foundTLSSecret != nil {
+		keyStorePasswordSecret := &corev1.Secret{}
+		err := r.Get(ctx, types.NamespacedName{Name: tlsOpts.KeyStorePasswordSecret.Name, Namespace: foundTLSSecret.Namespace}, keyStorePasswordSecret)
+		if err != nil {
+			r.Log.Error(err, "Keystore password secret not found", "name", tlsOpts.KeyStorePasswordSecret.Name)
+			return nil
+		}
+	}
+
+	return foundTLSSecret
+}
+
+// Once the cert is ready, apply any changes if needed, otherwise, stash the TLSSecretVersion
+func (r *SolrCloudReconciler) afterCertificateReady(ctx context.Context, instance *solr.SolrCloud, cert *certv1.Certificate, foundCert *certv1.Certificate, foundTLSSecret *corev1.Secret) (bool, error) {
+	// see if the create config changed, thus requiring a change to the underlying Certificate object
+	var err error
+	if util.CopyCreateCertificateFields(cert, foundCert) {
+		r.Log.Info("Certificate fields changed, updating", "cert", foundCert)
+		if instance.Spec.SolrTLS.RestartOnTLSSecretUpdate {
+			// tricky ~ we have to delete the TLS secret or the cert won't get re-issued
+			// but we only do this if we track the version
+			r.Log.Info("Deleting TLS secret because the Certificate changed", "cert", foundCert.Name)
+			err = r.Delete(ctx, foundTLSSecret)
+			if err != nil {
+				return false, err
+			}
+			r.Log.Info("Deleted TLS secret so it gets re-created after cert update", "secret", foundTLSSecret.Name)
+		}
+
+		// now update the existing Certificate to trigger the cert-manager to re-issue it
+		err = r.Update(ctx, foundCert)
+		if err != nil {

Review comment:
       This err check isn't needed, the following return statement should be the same.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org