Repository: apache/solr-operator

Issue:
It is impossible to setup TLS between Solr and Zookeeper
Tried setting up solr-operator with TLS between Solr and Zookeeper. (Zookeeper is setup with TLS, and Solr must connect to Zookeeper's TLS port.) It doesn't work or appear to have ever been tested.

Summary of problems I found before giving up:

* Solr cannot connect to Zookeepers using TLSv1.3 (will fail with `alert: protocol_version` until you downgrade).
* Contrary to the documentation, Solr's initContainers ignore `spec.solrZkOpts` completely... spec.solrZkOpts sets SOLR_OPTS, but the `solr zk` and `zkCli.sh` commands ignore SOLR_OPTS.
  * `solr zk` uses `SOLR_TOOL_OPTS` not `SOLR_OPTS` (and this is specific to Solr 9... Solr 8 uses a different env var) https://github.com/apache/solr/blob/main/solr/bin/solr#L601-L610
  * `zkcli.sh` uses `ZKCLI_JVM_FLAGS` not `SOLR_OPTS`
* The Solr TLS keystores/truststores don't even get mounted into the initContainers (and there is no way to do so), so even if you were able to specify the right combination of flags to `solr zk` and `zkcli.sh` in the initContainers, you can't add the certificates to setup TLS to Zookeeper anyways.
* The environment variables for the location of the truststore/keystore/passwords/etc. are not mounted in the initContainer
* There's no documentation on how to setup TLS between Solr and Zookeeper

To reproduce the issue:

Setup Zookeeper with TLS using the [Bitnami Helm chart](https://github.com/bitnami/charts/tree/main/bitnami/zookeeper) `helm install zookeeper bitnami/zookeeper -f zookeeper.yml`. Example `zookeeper.yml` (uses TLS for encryption, but not for authentication):

```yml
# note: solr flat-out cannot connect to zookeepers that use TLSv1.3,
# so we downgrade to TLSv1.2 here
jvmFlags: -Dzookeeper.ssl.protocol=TLSv1.2

# disabled for simplicity here
auth:
  client:
    enabled: false
  quorum:
    enabled: false

networkPolicy:
  extraIngress:
    - ports:
        - port: 3181
service:
  # zookeeper healthchecks and stuff are broken without this
  disableBaseClientPort: true
  headless:
    publishNotReadyAddresses: false
tls:
  client:
    # clients encrypt connections with TLS,
    # but do not use mTLS authentication
    enabled: true
    autoGenerated: true
    auth: "none"
  quorum:
    # use full mTLS auth+encryption between servers
    enabled: true
    autoGenerated: true
    auth: "need"
```

Then install solr-operator, and create a SolrCloud CRD. In this case I've generated some Certificate resources via cert-manger that the SolrCloud is using... 

```yml
apiVersion: solr.apache.org/v1beta1
kind: SolrCloud
metadata:
  name: solr
spec:
  replicas: 3
  solrImage:
    tag: 9.6.1
  dataStorage:
    persistent:
      reclaimPolicy: Delete
      pvcTemplate:
        spec:
          resources:
            requests:
              storage: "20Gi"
  solrTLS:
    # can't supply just a trustStore on its own, which is annoying
    # (maybe users want to get Solr -> Zookeepr TLS setup first before doing TLS for Solr itself?)
    pkcs12Secret:
      name: solr-tls
      key: keystore.p12
    keyStorePasswordSecret:
      name: solr-tls-password
      key: password
    # doesn't even get mounted into the initcontainer
    trustStoreSecret:
      name: solr-tls
      key: truststore.p12
    trustStorePasswordSecret:
      name: solr-tls-password
      key: password
  solrZkOpts: >
    -Dzookeeper.client.secure=true
    -Dzookeeper.clientCnxnSocket=org.apache.zookeeper.ClientCnxnSocketNetty
    -Dzookeeper.ssl.hostnameVerification=false
    -Dzookeeper.ssl.trustStore.location=/var/solr/tls/truststore.p12
    -Dzookeeper.ssl.trustStore.password=some-password
  zookeeperRef:
    connectionInfo:
      externalConnectionString: "zookeeper-0.zookeeper-headless.solr:3181,zookeeper-1.zookeeper-headless.solr:3181,zookeeper-2.zookeeper-headless.solr:3181"
      chroot: /solr
```

Discussion:
@jstaf: I got this to work with a custom solr image with a custom version of `solr.in.sh` and `zkcli.sh`. Still needs to be fixed in solr-operator though.

solr.in.sh (this change has `solr zk` auto-trust the zk tls certificate and obey settings from `$SOLR_OPTS` set by `solrZkOpts`)
```bash
#!/bin/bash
set -eo pipefail

# import zk certificate into default system truststore if not present
if [ ! -f /tmp/cacerts.jks ]; then
    # this step is extremely race-y for some reason
    sleep $((RANDOM % 10))
    echo |\
        openssl s_client -connect $(echo $ZK_SERVER | sed 's/,.*//g') -showcerts |\
        sed -ne '/-BEGIN CERTIFICATE-/,/-END CERTIFICATE-/p' > /tmp/ca.crt
    # we create a copy of the system truststore in /tmp since the root
    # filesystem is not writeable >:(
    cp /opt/java/openjdk/lib/security/cacerts /tmp/cacerts.jks
    keytool -import -noprompt -trustcacerts -alias solr-ca -file /tmp/ca.crt -keystore /tmp/cacerts.jks -storepass changeit
fi

# have "solr zk" and zkcli.sh use $SOLR_OPTS
SOLR_TOOL_OPTS="$SOLR_OPTS -Dzookeeper.ssl.trustStore.location=/tmp/cacerts.jks -Dzookeeper.ssl.trustStore.password=changeit"
ZKCLI_JVM_FLAGS="$SOLR_TOOL_OPTS"
```

zkcli.sh (make this script actually obey `$SOLR_OPTS` by sourcing our custom `solr.in.sh`)
```bash
#!/usr/bin/env bash

# You can override pass the following parameters to this script:
#

JVM="java"

# Find location of this script

sdir="`dirname \"$0\"`"

log4j_config="file:$sdir/../../resources/log4j2-console.xml"

solr_home="$sdir/../../solr"

# Settings for ZK ACL
#SOLR_ZK_CREDS_AND_ACLS="-DzkACLProvider=org.apache.solr.common.cloud.DigestZkACLProvider \
#  -DzkCredentialsProvider=org.apache.solr.common.cloud.DigestZkCredentialsProvider \
#  -DzkCredentialsInjector=org.apache.solr.common.cloud.VMParamsZkCredentialsInjector \
#  -DzkDigestUsername=admin-user -DzkDigestPassword=CHANGEME-ADMIN-PASSWORD \
#  -DzkDigestReadonlyUsername=readonly-user -DzkDigestReadonlyPassword=CHANGEME-READONLY-PASSWORD"
# optionally, you can use using a a Java properties file 'zkDigestCredentialsFile'
#...
#   -DzkDigestCredentialsFile=/path/to/zkDigestCredentialsFile.properties
#...

source /etc/default/solr.in.sh  # this is the only change
PATH=$JAVA_HOME/bin:$PATH $JVM $SOLR_ZK_CREDS_AND_ACLS $ZKCLI_JVM_FLAGS -Dlog4j.configurationFile=$log4j_config -Dsolr.home=$solr_home \
-classpath "$sdir/../../solr-webapp/webapp/WEB-INF/lib/*:$sdir/../../lib/ext/*:$sdir/../../lib/*" org.apache.solr.cloud.ZkCLI ${1+"$@"}
```@gerlowskija: Thanks for the detailed writeup @jstaf!  I noticed there are some other planned changes in how the operator interacts with ZK [here](https://github.com/apache/solr-operator/issues/731) , probably makes sense to tackle this once that's wrapped up?

It seems like you understand the mechanisms here pretty well - is this something you'd be willing to put together a PR for?@gerlowskija: FWIW @jstaf 's comment above that "Solr's initContainers ignore spec.solrZkOpts completely" is still true even with #731 wrapped up.

At a minimum we need to be setting `SOLR_TOOL_OPTS` in our initContainers.@gerlowskija: I've recently merged PR #804, which addresses this issue by setting SOLR_TOOL_OPTS in initContainers.  The value of this env-var will contain/obey any system props, etc. configured in `.spec.solrZkOpts`.

Solution patch:
diff --git a/controllers/util/solr_util.go b/controllers/util/solr_util.go
index 50fdbc4..2cce36a 100644
--- a/controllers/util/solr_util.go
+++ b/controllers/util/solr_util.go
@@ -19,6 +19,10 @@ package util
 
 import (
 	"fmt"
+	"sort"
+	"strconv"
+	"strings"
+
 	solr "github.com/apache/solr-operator/api/v1beta1"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -28,9 +32,6 @@ import (
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/utils/pointer"
 	"k8s.io/utils/ptr"
-	"sort"
-	"strconv"
-	"strings"
 )
 
 const (
@@ -1236,6 +1237,9 @@ func generateZKInteractionInitContainer(solrCloud *solr.SolrCloud, solrCloudStat
 		envVars = append(envVars, corev1.EnvVar{
 			Name:  "SOLR_OPTS",
 			Value: strings.Join(allSolrOpts, " "),
+		}, corev1.EnvVar{
+			Name:  "SOLR_TOOL_OPTS",
+			Value: strings.Join(allSolrOpts, " "),
 		})
 	}
 
@@ -1251,9 +1255,6 @@ func generateZKInteractionInitContainer(solrCloud *solr.SolrCloud, solrCloudStat
 
 	if security != nil && security.SecurityJson != "" {
 		envVars = append(envVars, corev1.EnvVar{Name: "SECURITY_JSON", ValueFrom: security.SecurityJsonSrc})
-		if solrCloud.Spec.SolrZkOpts != "" {
-			envVars = append(envVars, corev1.EnvVar{Name: "ZKCLI_JVM_FLAGS", Value: solrCloud.Spec.SolrZkOpts})
-		}
 		cmd += cmdToPutSecurityJsonInZk()
 	}
 
diff --git a/helm/solr-operator/Chart.yaml b/helm/solr-operator/Chart.yaml
index fb16b8e..ddbd812 100644
--- a/helm/solr-operator/Chart.yaml
+++ b/helm/solr-operator/Chart.yaml
@@ -72,6 +72,13 @@ annotations:
           url: https://github.com/apache/solr-operator/issues/778
         - name: Github PR
           url: https://github.com/apache/solr-operator/pull/779
+    - kind: fixed
+      description: SOLR_TOOL_OPTS is now set, allowing 'setup-zk' initContainer to obey ZK connection settings
+      links:
+        - name: Github Issue
+          url: https://github.com/apache/solr-operator/issues/717
+        - name: Github PR
+          url: https://github.com/apache/solr-operator/pull/804
   artifacthub.io/images: |
     - name: solr-operator
       image: apache/solr-operator:v0.10.0-prerelease



Test patch:
diff --git a/controllers/solrcloud_controller_basic_auth_test.go b/controllers/solrcloud_controller_basic_auth_test.go
index 54b309e..f01fe91 100644
--- a/controllers/solrcloud_controller_basic_auth_test.go
+++ b/controllers/solrcloud_controller_basic_auth_test.go
@@ -324,10 +324,11 @@ func expectBasicAuthConfigOnPodTemplateWithGomega(g Gomega, solrCloud *solrv1bet
 			// if the zookeeperRef has ACLs set, verify the env vars were set correctly for this initContainer
 			allACL, _ := solrCloud.Spec.ZookeeperRef.GetACLs()
 			if allACL != nil {
-				g.Expect(expInitContainer.Env).To(HaveLen(10), "Wrong number of env vars using ACLs and Basic Auth")
-				g.Expect(expInitContainer.Env[len(expInitContainer.Env)-2].Name).To(Equal("SOLR_OPTS"), "Env var SOLR_OPTS is misplaced the Solr Pod env vars")
-				g.Expect(expInitContainer.Env[len(expInitContainer.Env)-1].Name).To(Equal("SECURITY_JSON"), "Env var SECURITY_JSON is misplaced the Solr Pod env vars")
-				testACLEnvVarsWithGomega(g, expInitContainer.Env[3:len(expInitContainer.Env)-2], true)
+				g.Expect(expInitContainer.Env).To(HaveLen(11), "Wrong number of env vars using ACLs and Basic Auth")
+				g.Expect(expInitContainer.Env[len(expInitContainer.Env)-3].Name).To(Equal("SOLR_OPTS"), "Env var SOLR_OPTS is misplaced among the initContainer env vars")
+				g.Expect(expInitContainer.Env[len(expInitContainer.Env)-2].Name).To(Equal("SOLR_TOOL_OPTS"), "Env var SOLR_TOOL_OPTS is misplaced among the initContainer env vars")
+				g.Expect(expInitContainer.Env[len(expInitContainer.Env)-1].Name).To(Equal("SECURITY_JSON"), "Env var SECURITY_JSON is misplaced among the initContainer env vars")
+				testACLEnvVarsWithGomega(g, expInitContainer.Env[3:len(expInitContainer.Env)-3], true)
 			} // else this ref not using ACLs
 
 			expectPutSecurityJsonInZkCmd(g, expInitContainer)
diff --git a/controllers/solrcloud_controller_zk_test.go b/controllers/solrcloud_controller_zk_test.go
index 78d070d..7193420 100644
--- a/controllers/solrcloud_controller_zk_test.go
+++ b/controllers/solrcloud_controller_zk_test.go
@@ -583,9 +583,9 @@ var _ = FDescribe("SolrCloud controller - Zookeeper", func() {
 			testPodEnvVariables(expectedEnvVars, statefulSet.Spec.Template.Spec.Containers[0].Env)
 
 			expectedInitContainerEnvVars := map[string]string{
-				"SOLR_ZK_OPTS":    testSolrZKOpts,
-				"SOLR_OPTS":       "$(SOLR_ZK_OPTS) " + testSolrOpts,
-				"ZKCLI_JVM_FLAGS": "-Dsolr.zk.opts=this",
+				"SOLR_ZK_OPTS":   testSolrZKOpts,
+				"SOLR_OPTS":      "$(SOLR_ZK_OPTS) " + testSolrOpts,
+				"SOLR_TOOL_OPTS": "$(SOLR_ZK_OPTS) " + testSolrOpts,
 			}
 			testPodEnvVariables(expectedInitContainerEnvVars, statefulSet.Spec.Template.Spec.InitContainers[1].Env)
 		})