[GitHub] [lucene-solr-operator] thelabdude commented on a change in pull request #151: Integrate with cert-manager to issue TLS certs for Solr
thelabdude commented on a change in pull request #151:
URL:
https://github.com/apache/lucene-solr-operator/pull/151#discussion_r568152014
##
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:
From a reconcile perspective, we really only care about the TLS secret
that cert-manager creates once the Certificate is issued. The "watching" of the
Certificate to come online is really for status reporting while the cert is
issuing as it can take several minutes for the cert to be issued. Notice the
`isCertificateReady` is mostly about checking for the TLS secret.
The operator does create a Certificate for `autoCreate` mode but in that
case, the cert definition should come from the SolrCloud CRD and we don't want
to let users edit the Certificate externally; this is similar to the default
`solr.xml` ConfigMap and any direct edits to that cm are lost, same with
`autoCreate` certs.
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:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
[GitHub] [lucene-solr-operator] thelabdude commented on a change in pull request #151: Integrate with cert-manager to issue TLS certs for Solr
thelabdude commented on a change in pull request #151:
URL:
https://github.com/apache/lucene-solr-operator/pull/151#discussion_r568141856
##
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:
Ok I see, I was confused because you put the comment on the
`AddToScheme` line so thought the problem was about that line of code.
I don't think the Solr operator needs to own `Certificate` objects ... all
it cares about is the TLS secret that gets created by the cert-manager in
response to a change to the `Certificate`. It seems like the secret changing
does trigger a reconcile in my testing but maybe we need to add a specific
watch for that secret changing like you did for user-provided ConfigMaps?
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:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
[GitHub] [lucene-solr-operator] thelabdude commented on a change in pull request #151: Integrate with cert-manager to issue TLS certs for Solr
thelabdude commented on a change in pull request #151:
URL:
https://github.com/apache/lucene-solr-operator/pull/151#discussion_r568133031
##
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.N
[GitHub] [lucene-solr-operator] thelabdude commented on a change in pull request #151: Integrate with cert-manager to issue TLS certs for Solr
thelabdude commented on a change in pull request #151:
URL:
https://github.com/apache/lucene-solr-operator/pull/151#discussion_r568132449
##
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.N
[GitHub] [lucene-solr-operator] thelabdude commented on a change in pull request #151: Integrate with cert-manager to issue TLS certs for Solr
thelabdude commented on a change in pull request #151:
URL:
https://github.com/apache/lucene-solr-operator/pull/151#discussion_r568131545
##
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:
just a mistake
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:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
[GitHub] [lucene-solr-operator] thelabdude commented on a change in pull request #151: Integrate with cert-manager to issue TLS certs for Solr
thelabdude commented on a change in pull request #151:
URL:
https://github.com/apache/lucene-solr-operator/pull/151#discussion_r568127921
##
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:
As far as I've seen, calling `AddToScheme` works even if the
Cert-manager CRDs are not installed so not sure a flag is technically needed
but doesn't hurt to add one.
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:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
[GitHub] [lucene-solr-operator] thelabdude commented on a change in pull request #151: Integrate with cert-manager to issue TLS certs for Solr
thelabdude commented on a change in pull request #151:
URL:
https://github.com/apache/lucene-solr-operator/pull/151#discussion_r568114059
##
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:
Certs can take several minutes to issue, so I think we want to return
here with the extended wait period otherwise you get a ton of noise in the logs
until the cert issues ...
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:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
