You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/07/28 20:36:02 UTC
[GitHub] [solr-operator] thelabdude opened a new pull request, #461: Option to merge the JVM truststore with user-supplied truststore
thelabdude opened a new pull request, #461:
URL: https://github.com/apache/solr-operator/pull/461
Fixes #390 ~ by allowing the JVM cacerts to get merged in with the user-supplied truststore (Let's Encrypt's CA is in the cacerts for modern JVM)
Users can now configure the TLS options to merge the JVM's truststore with the truststore for their certs using:
```
spec:
...
solrTLS:
...
trustStoreSecret:
name: dev-selfsigned-cert-tls
key: truststore.p12
mergeJavaTrustStore: "$JAVA_HOME/lib/security/cacerts"
```
The path given in `mergeJavaTrustStore` option must exist on the Solr docker image! Thus, if user's customize their Solr image, this path may be different.
Behind the scenes, this creates an additional `initContainer` that merges the two truststores together and then points the env var to the "merged" truststore:
For server TLS:
```
- name: SOLR_SSL_TRUST_STORE
value: /var/solr/tls-merged/truststore.p12
``
By pointing `SOLR_SSL_TRUST_STORE` env var at the merged truststore, we're ensured that all the other initContainers and liveness probes continue to work (as they just use the env var to resolve this path).
Added a few simple unit tests and tested manually in my cluster.
For Prom exporter, the config would be:
```
spec:
solrReference:
...
solrTLS:
...
mergeJavaTrustStore: "$JAVA_HOME/lib/security/cacerts"
```
Which results in the exporter container getting configed with env var:
```
- name: SOLR_SSL_CLIENT_TRUST_STORE
value: /var/solr/tls-merged/truststore.p12
```
--
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
[GitHub] [solr-operator] thelabdude commented on a diff in pull request #461: Option to merge the JVM truststore with user-supplied truststore
Posted by GitBox <gi...@apache.org>.
thelabdude commented on code in PR #461:
URL: https://github.com/apache/solr-operator/pull/461#discussion_r934964702
##########
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:
No, nvm all that `volumesAndMounts` doesn't include vols & mounts for the `mountedTLSDir` option. So this option is not going to work with `mountedTLSDir`.
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.
--
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
[GitHub] [solr-operator] HoustonPutman commented on a diff in pull request #461: Option to merge the JVM truststore with user-supplied truststore
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on code in PR #461:
URL: https://github.com/apache/solr-operator/pull/461#discussion_r934797379
##########
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:
Does this work with `mountedTLSDir`?
--
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
[GitHub] [solr-operator] HoustonPutman commented on pull request #461: Option to merge the JVM truststore with user-supplied truststore
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on PR #461:
URL: https://github.com/apache/solr-operator/pull/461#issuecomment-1203155913
Sounds good Tim. Thanks for putting in the effort on this though!
--
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
[GitHub] [solr-operator] HoustonPutman commented on a diff in pull request #461: Option to merge the JVM truststore with user-supplied truststore
Posted by 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
[GitHub] [solr-operator] thelabdude commented on a diff in pull request #461: Option to merge the JVM truststore with user-supplied truststore
Posted by GitBox <gi...@apache.org>.
thelabdude commented on code in PR #461:
URL: https://github.com/apache/solr-operator/pull/461#discussion_r934786764
##########
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:
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.
--
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
[GitHub] [solr-operator] HoustonPutman commented on a diff in pull request #461: Option to merge the JVM truststore with user-supplied truststore
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on code in PR #461:
URL: https://github.com/apache/solr-operator/pull/461#discussion_r934798611
##########
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:
ahhh ok, didn't understand it was unusual to change it. Sounds good
--
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
[GitHub] [solr-operator] thelabdude commented on a diff in pull request #461: Option to merge the JVM truststore with user-supplied truststore
Posted by GitBox <gi...@apache.org>.
thelabdude commented on code in PR #461:
URL: https://github.com/apache/solr-operator/pull/461#discussion_r934944959
##########
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:
Typically, `mountedTLSDir` will have a csi driver volume and corresponding mount on the mainContainer, which would get used by the merge `initContainer`, so the init container would get the truststore file. However, that might cause double creation of the cert for each pod, once for the `initContainer` and once for the main container, so this would likely put a lot of pressure on the Cert issuer. So probably safer to say this feature is not supported with `mountedTLSDir` option for now.
--
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
[GitHub] [solr-operator] thelabdude commented on pull request #461: Option to merge the JVM truststore with user-supplied truststore
Posted by GitBox <gi...@apache.org>.
thelabdude commented on PR #461:
URL: https://github.com/apache/solr-operator/pull/461#issuecomment-1202548384
I think we should hold off on this solution for 0.6 as now I'm thinking the better approach would be to use the init-db script approach built into the Solr image. The problem with the current solution in this PR is it doesn't work with the `mountedTLSDir` option. Originally I was thinking the volumes and mounts would be available to the merge initContainer but that's not the case. I think the init-db approach will actually be fairly easy to implement except for the Solr exporter. Unfortunately, I won't have time to work on this for a bit and I don't want to hold up 0.6
--
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