-
Notifications
You must be signed in to change notification settings - Fork 115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Option to merge the JVM truststore with user-supplied truststore #461
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this work with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typically, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, nvm all that So maybe we just punt on this feature for 0.6 and solve it using an init-db script instead? There's already some code in place for mounting a script into the init-db. |
||
|
||
// Password for the Java truststore to merge; defaults to "changeit" | ||
MergeJavaTruststorePass string `json:"mergeJavaTrustStorePass,omitempty"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be a secret reference? How bad is it to have this in plain text? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seemed like overkill to me since it's the truststore pass for the JVM which is most likely "changeit" and isn't used by Solr ... that said, we can pull from a secret too ... doubt many would ever use this option. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ahhh ok, didn't understand it was unusual to change it. Sounds good |
||
} | ||
|
||
// +kubebuilder:validation:Enum=Basic | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,6 +43,7 @@ const ( | |
DefaultPkcs12KeystoreFile = "keystore.p12" | ||
DefaultPkcs12TruststoreFile = "truststore.p12" | ||
DefaultKeystorePasswordFile = "keystore-password" | ||
DefaultJvmTruststore = "$JAVA_HOME/lib/security/cacerts" | ||
) | ||
|
||
// Helper struct for holding server and/or client cert config | ||
|
@@ -72,6 +73,7 @@ type TLSConfig struct { | |
TruststorePath string | ||
VolumePrefix string | ||
Namespace string | ||
EnvVarPrefix string | ||
} | ||
|
||
// Get a TLSCerts struct for reconciling TLS on a SolrCloud | ||
|
@@ -83,6 +85,7 @@ func TLSCertsForSolrCloud(instance *solr.SolrCloud) *TLSCerts { | |
TruststorePath: DefaultTrustStorePath, | ||
CertMd5Annotation: SolrTlsCertMd5Annotation, | ||
Namespace: instance.Namespace, | ||
EnvVarPrefix: "SOLR_SSL", | ||
}, | ||
InitContainerImage: instance.Spec.BusyBoxImage, | ||
} | ||
|
@@ -94,6 +97,7 @@ func TLSCertsForSolrCloud(instance *solr.SolrCloud) *TLSCerts { | |
VolumePrefix: "client-", | ||
CertMd5Annotation: SolrClientTlsCertMd5Annotation, | ||
Namespace: instance.Namespace, | ||
EnvVarPrefix: "SOLR_SSL_CLIENT", | ||
} | ||
} | ||
return tls | ||
|
@@ -117,6 +121,7 @@ func TLSCertsForExporter(prometheusExporter *solr.SolrPrometheusExporter) *TLSCe | |
TruststorePath: DefaultTrustStorePath, | ||
CertMd5Annotation: SolrClientTlsCertMd5Annotation, | ||
Namespace: prometheusExporter.Namespace, | ||
EnvVarPrefix: "SOLR_SSL_CLIENT", | ||
}, | ||
InitContainerImage: bbImage, | ||
} | ||
|
@@ -129,8 +134,9 @@ func TLSCertsForExporter(prometheusExporter *solr.SolrPrometheusExporter) *TLSCe | |
func (tls *TLSCerts) enableTLSOnSolrCloudStatefulSet(stateful *appsv1.StatefulSet) { | ||
serverCert := tls.ServerConfig | ||
|
||
// Add the SOLR_SSL_* vars to the main container's environment | ||
mainContainer := &stateful.Spec.Template.Spec.Containers[0] | ||
|
||
// Add the SOLR_SSL_* vars to the main container's environment | ||
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 { | ||
|
@@ -151,6 +157,13 @@ func (tls *TLSCerts) enableTLSOnSolrCloudStatefulSet(stateful *appsv1.StatefulSe | |
// use an initContainer to create the wrapper script in the initdb | ||
stateful.Spec.Template.Spec.InitContainers = append(stateful.Spec.Template.Spec.InitContainers, tls.generateTLSInitdbScriptInitContainer()) | ||
} | ||
|
||
if serverCert.Options.MergeJavaTruststore != "" { | ||
serverCert.addMergeTruststoreInitContainer(&stateful.Spec.Template) | ||
} | ||
if tls.ClientConfig != nil && tls.ClientConfig.Options.MergeJavaTruststore != "" { | ||
tls.ClientConfig.addMergeTruststoreInitContainer(&stateful.Spec.Template) | ||
} | ||
} | ||
|
||
// Enrich the config for a Prometheus Exporter Deployment to allow the exporter to make requests to TLS enabled Solr pods | ||
|
@@ -172,6 +185,11 @@ func (tls *TLSCerts) enableTLSOnExporterDeployment(deployment *appsv1.Deployment | |
// volumes and mounts for TLS when using the mounted dir option | ||
clientCert.mountTLSWrapperScriptAndInitContainer(deployment, tls.InitContainerImage) | ||
} | ||
|
||
if clientCert.Options.MergeJavaTruststore != "" { | ||
// add an initContainer that merges the truststores together | ||
clientCert.addMergeTruststoreInitContainer(&deployment.Spec.Template) | ||
} | ||
} | ||
|
||
// Configures a pod template (either StatefulSet or Deployment) to mount the TLS files from a secret | ||
|
@@ -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{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
Name: volName, | ||
Image: solrImageName, // we use the Solr image for the initContainer since it has the truststore and keytool | ||
ImagePullPolicy: imagePullPolicy, | ||
TerminationMessagePath: "/dev/termination-log", | ||
TerminationMessagePolicy: "File", | ||
Command: []string{"sh", "-c", cmd}, | ||
VolumeMounts: mounts, | ||
Env: envVars, | ||
}, mergeVol, mergeMount | ||
} | ||
|
||
func (tls *TLSConfig) trustStoreEnvVarName() string { | ||
return tls.EnvVarPrefix + "_TRUST_STORE" | ||
} | ||
|
||
func (tls *TLSConfig) mergedTruststoreEnvVarName() string { | ||
if tls.VolumePrefix == "client-" { | ||
return "MERGED_CLIENT_TRUST_STORE" | ||
} | ||
return "MERGED_TRUST_STORE" | ||
} | ||
|
||
func (tls *TLSConfig) mergedTruststoreMountPath() string { | ||
return fmt.Sprintf("/var/solr/tls-%smerged", tls.VolumePrefix) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add the
// +optional
tag for both of these new options? Just for consistency sake