[GitHub] [solr-operator] HoustonPutman commented on a diff in pull request #461: Option to merge the JVM truststore with user-supplied truststore

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

HoustonPutman commented on code in PR #461:
URL: https://github.com/apache/solr-operator/pull/461#discussion_r934737694


##########
api/v1beta1/solrcloud_types.go:
##########
@@ -1523,6 +1523,14 @@ type SolrTLSOptions struct {
 	// This option is typically used with `spec.updateStrategy.restartSchedule` to restart Solr pods before the mounted TLS cert expires.
 	// +optional
 	MountedTLSDir *MountedTLSDirectory `json:"mountedTLSDir,omitempty"`
+
+	// Path on the Solr image to your JVM's truststore to merge with an external truststore.
+	// If supplied, Solr will be configured to use the merged truststore.
+	// The truststore for the JVM in the default Solr image is: $JAVA_HOME/lib/security/cacerts
+	MergeJavaTruststore string `json:"mergeJavaTrustStore,omitempty"`
+
+	// Password for the Java truststore to merge; defaults to "changeit"
+	MergeJavaTruststorePass string `json:"mergeJavaTrustStorePass,omitempty"`

Review Comment:
   Should this be a secret reference? How bad is it to have this in plain text?



##########
controllers/util/solr_tls_util.go:
##########
@@ -807,3 +825,87 @@ func verifyTLSSecretConfig(client *client.Client, secretName string, secretNames
 
 	return foundTLSSecret, nil
 }
+
+// Adds an initContainer that merges the JVM's truststore with the user-supplied truststore
+func (tls *TLSConfig) addMergeTruststoreInitContainer(template *corev1.PodTemplateSpec) {
+	mainContainer := &template.Spec.Containers[0]
+
+	// supports either client or server truststore env var names
+	envVar := tls.trustStoreEnvVarName()
+
+	// build an initContainer that merges the truststores together
+	initContainer, mergeVol, mergeMount :=
+		tls.buildMergeTruststoreInitContainer(mainContainer.Image, mainContainer.ImagePullPolicy, mainContainer.Env)
+	template.Spec.InitContainers = append(template.Spec.InitContainers, *initContainer)
+	template.Spec.Volumes = append(template.Spec.Volumes, *mergeVol)
+	mainContainer.VolumeMounts = append(mainContainer.VolumeMounts, *mergeMount)
+	// point the truststore to the merged
+	updateEnvVars := []corev1.EnvVar{}
+	for _, n := range mainContainer.Env {
+		// copy over all but the one we're swapping out ...
+		if n.Name != envVar {
+			updateEnvVars = append(updateEnvVars, n)
+		}
+	}
+	mainContainer.Env = append(updateEnvVars, corev1.EnvVar{Name: envVar, Value: mergeMount.MountPath + "/truststore.p12"})
+}
+
+func (tls *TLSConfig) buildMergeTruststoreInitContainer(solrImageName string, imagePullPolicy corev1.PullPolicy, serverEnvVars []corev1.EnvVar) (*corev1.Container, *corev1.Volume, *corev1.VolumeMount) {
+	// StatefulSet might have a client and server SSL config, so need to vary the initContainer and vol mount names
+	envVar := tls.trustStoreEnvVarName()
+	passEnvVar := envVar + "_PASSWORD"
+	volName := fmt.Sprintf("merge-%struststore", tls.VolumePrefix)
+	mergedEnvVar := tls.mergedTruststoreEnvVarName()
+	mountPath := tls.mergedTruststoreMountPath()
+	envVars := []corev1.EnvVar{
+		{
+			Name:  mergedEnvVar,
+			Value: mountPath + "/truststore.p12",
+		},
+	}
+	envVars = append(envVars, serverEnvVars...)
+
+	// the default truststore pass for most JVM is "changeit"
+	javaTruststorePass := tls.Options.MergeJavaTruststorePass
+	if javaTruststorePass == "" {
+		javaTruststorePass = "changeit"
+	}
+
+	// Use Java's keytool to merge the JVM's truststore with the user-supplied truststore
+	cmd := fmt.Sprintf(`keytool -importkeystore -srckeystore %s -srcstorepass %s -destkeystore $%s -deststoretype pkcs12 -deststorepass $%s;
+keytool -importkeystore -srckeystore $%s -srcstorepass $%s -destkeystore $%s -deststoretype pkcs12 -deststorepass $%s`,
+		tls.Options.MergeJavaTruststore, javaTruststorePass, mergedEnvVar, passEnvVar, envVar, passEnvVar, mergedEnvVar, passEnvVar)
+
+	// Need volume mounts from mainContainer (to get access to the user-supplied truststore)
+	_, mounts := tls.volumesAndMounts()
+	// Mount a shared dir between the initContainer and mainContainer to store the merged truststore
+	mergeVol := &corev1.Volume{Name: volName, VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}}
+	mergeMount := &corev1.VolumeMount{Name: volName, ReadOnly: false, MountPath: mountPath}
+	mounts = append(mounts, *mergeMount)
+
+	return &corev1.Container{

Review Comment:
   Could we possibly use the default initContainer resources?
   
   You can see how it was done in this PR: https://github.com/apache/solr-operator/pull/451/files#diff-653faf42eabedf3285e433f247c993282f035ee70781d151f8c8d68fee2621a3R618-R649



##########
api/v1beta1/solrcloud_types.go:
##########
@@ -1523,6 +1523,14 @@ type SolrTLSOptions struct {
 	// This option is typically used with `spec.updateStrategy.restartSchedule` to restart Solr pods before the mounted TLS cert expires.
 	// +optional
 	MountedTLSDir *MountedTLSDirectory `json:"mountedTLSDir,omitempty"`
+
+	// Path on the Solr image to your JVM's truststore to merge with an external truststore.
+	// If supplied, Solr will be configured to use the merged truststore.
+	// The truststore for the JVM in the default Solr image is: $JAVA_HOME/lib/security/cacerts
+	MergeJavaTruststore string `json:"mergeJavaTrustStore,omitempty"`

Review Comment:
   Can we add the `// +optional` tag for both of these new options? Just for consistency sake



-- 
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-unsubscribe@solr.apache.org

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


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