thelabdude commented on a change in pull request #312:
URL: https://github.com/apache/solr-operator/pull/312#discussion_r699691075



##########
File path: controllers/util/solr_tls_util.go
##########
@@ -27,171 +27,217 @@ import (
 )
 
 const (
-       SolrTlsCertMd5Annotation    = "solr.apache.org/tlsCertMd5"
-       DefaultKeyStorePath         = "/var/solr/tls"
-       DefaultWritableKeyStorePath = "/var/solr/tls/pkcs12"
-       TLSCertKey                  = "tls.crt"
-       DefaultTrustStorePath       = "/var/solr/tls-truststore"
-       InitdbPath                  = "/docker-entrypoint-initdb.d"
-       DefaultPkcs12KeystoreFile   = "keystore.p12"
-       DefaultPkcs12TruststoreFile = "truststore.p12"
-       DefaultKeystorePasswordFile = "keystore-password"
+       SolrTlsCertMd5Annotation       = "solr.apache.org/tlsCertMd5"
+       SolrClientTlsCertMd5Annotation = "solr.apache.org/tlsClientCertMd5"
+       DefaultKeyStorePath            = "/var/solr/tls"
+       DefaultClientKeyStorePath      = "/var/solr/client-tls"
+       DefaultWritableKeyStorePath    = "/var/solr/tls/pkcs12"
+       TLSCertKey                     = "tls.crt"
+       DefaultTrustStorePath          = "/var/solr/tls-truststore"
+       DefaultClientTrustStorePath    = "/var/solr/client-tls-truststore"
+       InitdbPath                     = "/docker-entrypoint-initdb.d"
+       DefaultPkcs12KeystoreFile      = "keystore.p12"
+       DefaultPkcs12TruststoreFile    = "truststore.p12"
+       DefaultKeystorePasswordFile    = "keystore-password"
 )
 
-// Holds Options from the user config as well as other config properties 
determined during reconciliation
+// Helper struct for holding server and/or client cert config
+// This struct is intended for internal use only and is only exposed outside 
the package so that the controllers can access
+type TLSCerts struct {
+       // Server cert config
+       ServerConfig *TLSConfig
+       // Client cert config
+       ClientConfig *TLSConfig
+       // Image used for initContainers that help configure the TLS settings
+       InitContainerImage *solr.ContainerImage
+}
+
+// Holds TLS options from the user config as well as other config properties 
determined during reconciliation
 // This struct is intended for internal use only and is only exposed outside 
the package so that the controllers can access
 type TLSConfig struct {
        // TLS options provided by the user in the CRD definition
        Options *solr.SolrTLSOptions
        // Flag to indicate if we need to convert the provided keystore into 
the p12 format needed by Java
        NeedsPkcs12InitContainer bool
-       // The MD5 hash of the TLS cert, used for restarting Solr pods after 
the cert updates if so desired
+       // The MD5 hash of the cert, used for restarting Solr pods after the 
cert updates if so desired
        CertMd5 string
-       // Image used for initContainers that help configure the TLS settings
-       InitContainerImage *solr.ContainerImage
+       // The annotation varies based on the cert type (client or server)
+       CertMd5Annotation string
+       // The paths vary based on whether this config is for a client or 
server cert
+       KeystorePath   string
+       TruststorePath string
+}
+
+// Get a TLSCerts struct for reconciling TLS on a SolrCloud
+func TLSCertsForSolrCloud(instance *solr.SolrCloud) *TLSCerts {
+       tls := &TLSCerts{
+               ServerConfig: &TLSConfig{
+                       Options:        instance.Spec.SolrTLS.DeepCopy(),
+                       KeystorePath:   DefaultKeyStorePath,
+                       TruststorePath: DefaultTrustStorePath,
+               },
+               InitContainerImage: instance.Spec.BusyBoxImage,
+       }
+       if instance.Spec.SolrClientTLS != nil {
+               tls.ClientConfig = &TLSConfig{
+                       Options:        instance.Spec.SolrClientTLS.DeepCopy(),
+                       KeystorePath:   DefaultClientKeyStorePath,
+                       TruststorePath: DefaultClientTrustStorePath,
+               }
+       }
+       return tls
+}
+
+// Get a TLSCerts struct for reconciling TLS on an Exporter
+func TLSCertsForExporter(prometheusExporter *solr.SolrPrometheusExporter) 
*TLSCerts {
+       // when using mounted dir option, we need a busy box image for our 
initContainers
+       bbImage := prometheusExporter.Spec.BusyBoxImage
+       if bbImage == nil {
+               bbImage = &solr.ContainerImage{
+                       Repository: solr.DefaultBusyBoxImageRepo,
+                       Tag:        solr.DefaultBusyBoxImageVersion,
+                       PullPolicy: solr.DefaultPullPolicy,
+               }
+       }
+       return &TLSCerts{
+               ClientConfig: &TLSConfig{
+                       Options:        
prometheusExporter.Spec.SolrReference.SolrTLS.DeepCopy(),
+                       KeystorePath:   DefaultKeyStorePath,
+                       TruststorePath: DefaultTrustStorePath,
+               },
+               InitContainerImage: bbImage,
+       }
 }
 
 // Enrich the config for a SolrCloud StatefulSet to enable TLS, either loaded 
from a secret or
 // a directory on the main pod containing per-pod specific TLS files. In the 
latter case, the "mounted dir"
 // typically comes from an external agent (such as a cert-manager extension) 
or CSI driver that injects the
 // pod-specific TLS files using mutating web hooks
-func (tls *TLSConfig) enableTLSOnSolrCloudStatefulSet(stateful 
*appsv1.StatefulSet) {
+func (tls *TLSCerts) enableTLSOnSolrCloudStatefulSet(stateful 
*appsv1.StatefulSet) {
+       serverCert := tls.ServerConfig
+
        // Add the SOLR_SSL_* vars to the main container's environment
-       tls.enableTLSOnPodTemplate(&stateful.Spec.Template)
+       mainContainer := &stateful.Spec.Template.Spec.Containers[0]
+       mainContainer.Env = append(mainContainer.Env, 
serverCert.serverEnvVars()...)
+       // Was a client cert mounted too? If so, add the client env vars to the 
main container as well
+       if tls.ClientConfig != nil {
+               mainContainer.Env = append(mainContainer.Env, 
tls.ClientConfig.clientEnvVars()...)
+       }
+
+       if serverCert.Options.PKCS12Secret != nil {
+               // Cert comes from a secret, so setup the pod template to mount 
the secret
+               serverCert.mountTLSSecretOnPodTemplate(&stateful.Spec.Template, 
"")
 
-       // volumes and mounts for TLS when using the mounted dir option
-       if tls.Options.MountedServerTLSDir != nil {
+               // mount the client certificate from a different secret (at 
different mount points)
+               if tls.ClientConfig != nil && 
tls.ClientConfig.Options.PKCS12Secret != nil {
+                       
tls.ClientConfig.mountTLSSecretOnPodTemplate(&stateful.Spec.Template, "client-")
+               }
+       } else if serverCert.Options.MountedTLSDir != nil {
                // the TLS files come from some auto-mounted directory on the 
main container
-               tls.mountInitDbIfNeeded(stateful)
+               mountInitDbIfNeeded(stateful)
                // use an initContainer to create the wrapper script in the 
initdb
                stateful.Spec.Template.Spec.InitContainers = 
append(stateful.Spec.Template.Spec.InitContainers, 
tls.generateTLSInitdbScriptInitContainer())
        }
 }
 
 // Enrich the config for a Prometheus Exporter Deployment to allow the 
exporter to make requests to TLS enabled Solr pods
-func (tls *TLSConfig) enableTLSOnExporterDeployment(deployment 
*appsv1.Deployment) {
+func (tls *TLSCerts) enableTLSOnExporterDeployment(deployment 
*appsv1.Deployment) {
+       clientCert := tls.ClientConfig
+
        // Add the SOLR_SSL_* vars to the main container's environment
-       mainContainer := tls.enableTLSOnPodTemplate(&deployment.Spec.Template)
+       mainContainer := &deployment.Spec.Template.Spec.Containers[0]
+       mainContainer.Env = append(mainContainer.Env, 
clientCert.clientEnvVars()...)
+       mainContainer.Env = append(mainContainer.Env, corev1.EnvVar{Name: 
"SOLR_SSL_CHECK_PEER_NAME", Value: 
strconv.FormatBool(clientCert.Options.CheckPeerName)})
 
        // the exporter process doesn't read the SOLR_SSL_* env vars, so we 
need to pass them via JAVA_OPTS
-       tls.appendTLSJavaOptsToEnv(mainContainer)
-
-       // volumes and mounts for TLS when using the mounted dir option
-       if tls.Options.MountedServerTLSDir != nil {
-               tls.mountTLSWrapperScriptAndInitContainer(deployment)
+       appendJavaOptsToEnv(mainContainer, clientCert.clientJavaOpts())
+
+       if clientCert.Options.PKCS12Secret != nil || 
clientCert.Options.TrustStoreSecret != nil {
+               // Cert comes from a secret, so setup the pod template to mount 
the secret
+               
clientCert.mountTLSSecretOnPodTemplate(&deployment.Spec.Template, "")
+       } else if clientCert.Options.MountedTLSDir != nil {
+               // volumes and mounts for TLS when using the mounted dir option
+               clientCert.mountTLSWrapperScriptAndInitContainer(deployment, 
tls.InitContainerImage)
        }
 }
 
-// Configures the TLS env vars, pod annotations, volumes, and pkcs12 
initContainer on a Pod template spec
-// for enabling TLS on either StatefulSet or Deployment
-func (tls *TLSConfig) enableTLSOnPodTemplate(template *corev1.PodTemplateSpec) 
*corev1.Container {
-       // Add the SOLR_SSL_* vars to the main container's environment
+// Configures a pod template (either StatefulSet or Deployment) to mount the 
TLS files from a secret
+func (tls *TLSConfig) mountTLSSecretOnPodTemplate(template 
*corev1.PodTemplateSpec, namePrefix string) *corev1.Container {
        mainContainer := &template.Spec.Containers[0]
-       mainContainer.Env = append(mainContainer.Env, tls.envVars()...)
 
-       if tls.Options.PKCS12Secret != nil {
-               // the TLS files are mounted from a secret, setup the volumes 
and mounts
-               template.Spec.Volumes = append(template.Spec.Volumes, 
tls.volumes()...)
-               mainContainer.VolumeMounts = append(mainContainer.VolumeMounts, 
tls.volumeMounts()...)
-
-               // track the MD5 of the TLS cert (from secret) to trigger 
restarts if the cert changes
-               if tls.Options.RestartOnTLSSecretUpdate && tls.CertMd5 != "" {
-                       if template.Annotations == nil {
-                               template.Annotations = make(map[string]string, 
1)
-                       }
-                       template.Annotations[SolrTlsCertMd5Annotation] = 
tls.CertMd5
-               }
+       // the TLS files are mounted from a secret, setup the volumes and mounts
+       vols, mounts := tls.volumesAndMounts(namePrefix)
+
+       // We need an initContainer to convert a TLS cert into the pkcs12 
format Java wants (using openssl)
+       // but openssl cannot write to the /var/solr/tls directory because of 
the way secret mounts work
+       // so we need to mount an empty directory to write pkcs12 keystore into
+       if tls.NeedsPkcs12InitContainer {
+               vols = append(vols, corev1.Volume{Name: "pkcs12", VolumeSource: 
corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}})
+               mounts = append(mounts, corev1.VolumeMount{Name: "pkcs12", 
ReadOnly: false, MountPath: DefaultWritableKeyStorePath})
+               pkcs12InitContainer := 
tls.generatePkcs12InitContainer(mainContainer.Image, 
mainContainer.ImagePullPolicy, mounts)
+               template.Spec.InitContainers = 
append(template.Spec.InitContainers, pkcs12InitContainer)
+       }
+       template.Spec.Volumes = append(template.Spec.Volumes, vols...)
+       mainContainer.VolumeMounts = append(mainContainer.VolumeMounts, 
mounts...)
 
-               // use an initContainer to convert the keystore to p12 format 
if needed
-               if tls.NeedsPkcs12InitContainer {
-                       pkcs12InitContainer := 
tls.generatePkcs12InitContainer(mainContainer.Image, 
mainContainer.ImagePullPolicy)
-                       template.Spec.InitContainers = 
append(template.Spec.InitContainers, pkcs12InitContainer)
+       // track the MD5 of the TLS cert (from secret) to trigger restarts if 
the cert changes
+       if tls.Options.RestartOnTLSSecretUpdate && tls.CertMd5 != "" {
+               if template.Annotations == nil {
+                       template.Annotations = make(map[string]string, 1)
                }
+               template.Annotations[tls.CertMd5Annotation] = tls.CertMd5
        }
 
        return mainContainer
 }
 
 // Get a list of volumes for the keystore and optionally a truststore loaded 
from a TLS secret
-func (tls *TLSConfig) volumes() []corev1.Volume {
+func (tls *TLSConfig) volumesAndMounts(namePrefix string) ([]corev1.Volume, 
[]corev1.VolumeMount) {
        optional := false
        defaultMode := int32(0664)
-       vols := []corev1.Volume{
-               {
-                       Name: "keystore",
+       vols := []corev1.Volume{}
+       mounts := []corev1.VolumeMount{}
+       keystoreSecretName := ""
+
+       opts := tls.Options
+       if opts.PKCS12Secret != nil {
+               keystoreSecretName = opts.PKCS12Secret.Name
+               volName := namePrefix + "keystore"

Review comment:
       the prefix is either "" or "client-" ... I'll fix it with a helper 
method on `tls.Options`




-- 
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.

To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to