Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Option to merge the JVM truststore with user-supplied truststore #461

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Option to merge the JVM truststore with user-supplied truststore #461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dcb59ee
Option to merge the JVM truststore with user-supplied truststore
thelabdude Jul 28, 2022
0420970
Add some bulk to the tests
HoustonPutman Aug 1, 2022
f99abc0
Add resource requests for the merge initContainer
thelabdude Aug 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions api/v1beta1/solrcloud_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1523,6 +1523,16 @@ 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
// +optional
MergeJavaTruststore string `json:"mergeJavaTrustStore,omitempty"`
Copy link
Contributor

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work with mountedTLSDir?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


// Password for the Java truststore to merge; defaults to "changeit"
// +optional
MergeJavaTruststorePass string `json:"mergeJavaTrustStorePass,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

The 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
Expand Down
12 changes: 12 additions & 0 deletions config/crd/bases/solr.apache.org_solrclouds.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4926,6 +4926,12 @@ spec:
required:
- key
type: object
mergeJavaTrustStore:
description: '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'
type: string
mergeJavaTrustStorePass:
description: Password for the Java truststore to merge; defaults to "changeit"
type: string
mountedTLSDir:
description: Used to specify a path where the keystore, truststore, and password files for the TLS certificate are mounted by an external agent or CSI driver. This option is typically used with `spec.updateStrategy.restartSchedule` to restart Solr pods before the mounted TLS cert expires.
properties:
Expand Down Expand Up @@ -5087,6 +5093,12 @@ spec:
required:
- key
type: object
mergeJavaTrustStore:
description: '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'
type: string
mergeJavaTrustStorePass:
description: Password for the Java truststore to merge; defaults to "changeit"
type: string
mountedTLSDir:
description: Used to specify a path where the keystore, truststore, and password files for the TLS certificate are mounted by an external agent or CSI driver. This option is typically used with `spec.updateStrategy.restartSchedule` to restart Solr pods before the mounted TLS cert expires.
properties:
Expand Down
6 changes: 6 additions & 0 deletions config/crd/bases/solr.apache.org_solrprometheusexporters.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3688,6 +3688,12 @@ spec:
required:
- key
type: object
mergeJavaTrustStore:
description: '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'
type: string
mergeJavaTrustStorePass:
description: Password for the Java truststore to merge; defaults to "changeit"
type: string
mountedTLSDir:
description: Used to specify a path where the keystore, truststore, and password files for the TLS certificate are mounted by an external agent or CSI driver. This option is typically used with `spec.updateStrategy.restartSchedule` to restart Solr pods before the mounted TLS cert expires.
properties:
Expand Down
180 changes: 173 additions & 7 deletions controllers/solrcloud_controller_tls_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,64 @@ var _ = FDescribe("SolrCloud controller - TLS", func() {
})
})

FContext("Secret TLS - Merge Truststore", func() {
tlsSecretName := "tls-cert-secret-from-user"
keystorePassKey := "some-password-key-thingy"
trustStoreSecretName := "custom-truststore-secret"
trustStoreFile := "truststore.p12"
BeforeEach(func() {
solrCloud.Spec.SolrSecurity = &solrv1beta1.SolrSecurityOptions{AuthenticationType: solrv1beta1.Basic}
solrCloud.Spec.SolrTLS = createTLSOptions(tlsSecretName, keystorePassKey, false)
solrCloud.Spec.SolrTLS.TrustStoreSecret = &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{Name: trustStoreSecretName},
Key: trustStoreFile,
}
solrCloud.Spec.SolrTLS.TrustStorePasswordSecret = &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{Name: trustStoreSecretName},
Key: "truststore-pass",
}
solrCloud.Spec.SolrTLS.ClientAuth = solrv1beta1.Need // require client auth too (mTLS between the pods)
solrCloud.Spec.SolrTLS.MergeJavaTruststore = util.DefaultJvmTruststore
})
FIt("has the correct resources", func() {
verifyReconcileUserSuppliedTLS(ctx, solrCloud, false, false)
})
})

FContext("Secret TLS - Merge Truststores for Server & Client", func() {
serverCertSecret := "tls-server-cert"
clientCertSecret := "tls-client-cert"
keystorePassKey := "some-password-key-thingy"
BeforeEach(func() {
solrCloud.Spec.SolrTLS = createTLSOptions(serverCertSecret, keystorePassKey, false)
solrCloud.Spec.SolrTLS.MergeJavaTruststore = util.DefaultJvmTruststore

solrCloud.Spec.SolrTLS.ClientAuth = solrv1beta1.Need

// Additional client cert
solrCloud.Spec.SolrClientTLS = &solrv1beta1.SolrTLSOptions{
KeyStorePasswordSecret: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{Name: clientCertSecret},
Key: keystorePassKey,
},
PKCS12Secret: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{Name: clientCertSecret},
Key: util.DefaultPkcs12KeystoreFile,
},
TrustStoreSecret: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{Name: clientCertSecret},
Key: util.DefaultPkcs12TruststoreFile,
},
RestartOnTLSSecretUpdate: false,
MergeJavaTruststore: util.DefaultJvmTruststore,
}
})
FIt("has the correct resources", func() {
By("checking that the User supplied TLS Config is correct in the generated StatefulSet")
verifyReconcileUserSuppliedTLS(ctx, solrCloud, false, false)
})
})

FContext("Secret TLS - Pkcs12 Conversion", func() {
tlsSecretName := "tls-cert-secret-from-user-no-pkcs12"
keystorePassKey := "some-password-key-thingy"
Expand Down Expand Up @@ -466,6 +524,11 @@ func expectTLSConfigOnPodTemplateWithGomega(g Gomega, tls *solrv1beta1.SolrTLSOp
expectedTrustStorePath = util.DefaultTrustStorePath + "/" + tls.TrustStoreSecret.Key
}

// did we merge the Java truststore with the user-supplied?
if tls.MergeJavaTruststore != "" {
expectedTrustStorePath = "/var/solr/tls-merged/truststore.p12"
}

if tls.PKCS12Secret != nil {
expectTLSEnvVarsWithGomega(g, mainContainer.Env, tls.KeyStorePasswordSecret.Name, tls.KeyStorePasswordSecret.Key, needsPkcs12InitContainer, expectedTrustStorePath, clientOnly, clientTLS)
} else if tls.TrustStoreSecret != nil {
Expand All @@ -477,7 +540,11 @@ func expectTLSConfigOnPodTemplateWithGomega(g Gomega, tls *solrv1beta1.SolrTLSOp
g.Expect(len(envVars)).To(Equal(3), "Wrong number of SSL env vars for Pod")
for _, envVar := range envVars {
if envVar.Name == "SOLR_SSL_CLIENT_TRUST_STORE" {
g.Expect(envVar.Value).To(Equal(expectedTrustStorePath), "Wrong envVar value for %s", envVar.Name)
expectedPath := expectedTrustStorePath
if clientTLS != nil && clientTLS.MergeJavaTruststore != "" {
expectedPath = "/var/solr/tls-client-merged/truststore.p12"
}
g.Expect(envVar.Value).To(Equal(expectedPath), "Wrong envVar value for %s", envVar.Name)
}
if envVar.Name == "SOLR_SSL_CLIENT_TRUST_STORE_PASSWORD" {
g.Expect(envVar.Value).To(BeEmpty(), "EnvVar %s should not use an explicit Value, since it is populated from a secret", envVar.Name)
Expand Down Expand Up @@ -531,6 +598,106 @@ func expectTLSConfigOnPodTemplateWithGomega(g Gomega, tls *solrv1beta1.SolrTLSOp
g.Expect(expInitContainer.Command[2]).To(Equal(expCmd), "Wrong TLS initContainer command")
}

if tls.MergeJavaTruststore != "" {
// verify the merge-truststore initContainer was created as well
g.Expect(podTemplate.Spec.InitContainers).To(Not(BeEmpty()), "An init container should exist to merge truststores")
var expInitContainer *corev1.Container = nil
for _, cnt := range podTemplate.Spec.InitContainers {
if cnt.Name == "merge-truststore" {
expInitContainer = &cnt
break
}
}
g.Expect(expInitContainer).To(Not(BeNil()), "Didn't find the merge-truststore InitContainer in the sts!")
g.Expect(expInitContainer.Command[2]).To(ContainSubstring("keytool"), "Wrong merge-truststore initContainer command")
providedTruststorePath := util.DefaultKeyStorePath
trustStoreMount := 0
if tls.TrustStoreSecret != nil && (tls.PKCS12Secret == nil || tls.PKCS12Secret.Name != tls.TrustStoreSecret.Name) {
providedTruststorePath = util.DefaultTrustStorePath
if tls.PKCS12Secret != nil {
trustStoreMount = 1
}
}
g.Expect(expInitContainer.VolumeMounts).To(Not(BeEmpty()), "There are no volume mounts for the merge-truststore InitContainer!")
g.Expect(expInitContainer.VolumeMounts[trustStoreMount].MountPath).To(Equal(providedTruststorePath), "Wrong mountPath for the provided truststore in the merge-truststore InitContainer!")
g.Expect(expInitContainer.VolumeMounts[trustStoreMount+1].MountPath).To(Equal("/var/solr/tls-merged"), "Wrong mountPath for the merge-truststore InitContainer!")
sslTrustStoreVar := "SOLR_SSL_TRUST_STORE"
if clientOnly {
sslTrustStoreVar = "SOLR_SSL_CLIENT_TRUST_STORE"
}
g.Expect(expInitContainer.Command[2]).To(
And(
ContainSubstring("-srckeystore "+tls.MergeJavaTruststore),
ContainSubstring("-srckeystore $"+sslTrustStoreVar),
), "Wrong paths in the merge-truststore InitContainer command!")

// verify the mountpath in the Solr container
var mergedTruststoreVolumeMount *corev1.VolumeMount = nil
for _, vm := range podTemplate.Spec.Containers[0].VolumeMounts {
if vm.Name == "merge-truststore" {
mergedTruststoreVolumeMount = &vm
break
}
}
g.Expect(mergedTruststoreVolumeMount).To(Not(BeNil()), "Didn't find the merge-truststore VolumeMount in the solr container!")
g.Expect(mergedTruststoreVolumeMount.MountPath).To(Equal("/var/solr/tls-merged"), "Wrong merge-truststore VolumeMount in the solr container")

for _, envVar := range expInitContainer.Env {
if envVar.Name == "MERGED_TRUST_STORE" {
g.Expect(envVar.Value).To(Equal("/var/solr/tls-merged/truststore.p12"), "EnvVar %s has the wrong string value", envVar.Name)
g.Expect(envVar.ValueFrom).To(BeNil(), "EnvVar %s must not have a ValueFrom", envVar.Name)
}
}
}

if clientTLS != nil && clientTLS.MergeJavaTruststore != "" {
// verify the merge-truststore initContainer was created as well
g.Expect(podTemplate.Spec.InitContainers).To(Not(BeEmpty()), "An init container should exist to merge client truststores")
var expInitContainer *corev1.Container = nil
for _, cnt := range podTemplate.Spec.InitContainers {
if cnt.Name == "merge-client-truststore" {
expInitContainer = &cnt
break
}
}
g.Expect(expInitContainer).To(Not(BeNil()), "Didn't find the merge-client-truststore InitContainer in the sts!")
g.Expect(expInitContainer.Command[2]).To(ContainSubstring("keytool"), "Wrong merge-client-truststore initContainer command")
g.Expect(expInitContainer.VolumeMounts).To(Not(BeEmpty()), "There are no volume mounts for the merge-client-truststore InitContainer!")
providedTruststorePath := util.DefaultClientKeyStorePath
trustStoreMount := 0
if clientTLS.TrustStoreSecret != nil && (clientTLS.PKCS12Secret == nil || clientTLS.PKCS12Secret.Name != clientTLS.TrustStoreSecret.Name) {
providedTruststorePath = util.DefaultClientTrustStorePath
if clientTLS.PKCS12Secret != nil && clientTLS.PKCS12Secret.Name != clientTLS.TrustStoreSecret.Name {
trustStoreMount = 1
}
}
g.Expect(expInitContainer.VolumeMounts[trustStoreMount].MountPath).To(Equal(providedTruststorePath), "Wrong mountPath for the provided truststore in the merge-client-truststore InitContainer!")
g.Expect(expInitContainer.VolumeMounts[trustStoreMount+1].MountPath).To(Equal("/var/solr/tls-client-merged"), "Wrong mountPath for the merge-client-truststore InitContainer!")
g.Expect(expInitContainer.Command[2]).To(
And(
ContainSubstring("-srckeystore "+clientTLS.MergeJavaTruststore),
ContainSubstring("-srckeystore $SOLR_SSL_CLIENT_TRUST_STORE"),
), "Wrong paths in the merge-client-truststore InitContainer command!")

// verify the mountpath in the Solr container
var mergedTruststoreVolumeMount *corev1.VolumeMount = nil
for _, vm := range podTemplate.Spec.Containers[0].VolumeMounts {
if vm.Name == "merge-client-truststore" {
mergedTruststoreVolumeMount = &vm
break
}
}
g.Expect(mergedTruststoreVolumeMount).To(Not(BeNil()), "Didn't find the merge-client-truststore VolumeMount in the solr container!")
g.Expect(mergedTruststoreVolumeMount.MountPath).To(Equal("/var/solr/tls-client-merged"), "Wrong merge-client-truststore VolumeMount in the solr container")

for _, envVar := range expInitContainer.Env {
if envVar.Name == "MERGED_TRUST_STORE" {
g.Expect(envVar.Value).To(Equal("/var/solr/tls-client-merged/truststore.p12"), "EnvVar %s has the wrong string value", envVar.Name)
g.Expect(envVar.ValueFrom).To(BeNil(), "EnvVar %s must not have a ValueFrom", envVar.Name)
}
}
}

if tls.ClientAuth == solrv1beta1.Need {
// verify the probes use a command with SSL opts
tlsProps := ""
Expand Down Expand Up @@ -611,11 +778,6 @@ func expectMountedTLSDirEnvVars(envVars []corev1.EnvVar, solrCloud *solrv1beta1.
}
}

// ensure the TLS related env vars are set for the Solr pod
func expectTLSEnvVars(envVars []corev1.EnvVar, expectedKeystorePasswordSecretName string, expectedKeystorePasswordSecretKey string, needsPkcs12InitContainer bool, expectedTruststorePath string, clientOnly bool, clientTLS *solrv1beta1.SolrTLSOptions) {
expectTLSEnvVarsWithGomega(Default, envVars, expectedKeystorePasswordSecretName, expectedKeystorePasswordSecretKey, needsPkcs12InitContainer, expectedTruststorePath, clientOnly, clientTLS)
}

// ensure the TLS related env vars are set for the Solr pod
func expectTLSEnvVarsWithGomega(g Gomega, envVars []corev1.EnvVar, expectedKeystorePasswordSecretName string, expectedKeystorePasswordSecretKey string, needsPkcs12InitContainer bool, expectedTruststorePath string, clientOnly bool, clientTLS *solrv1beta1.SolrTLSOptions) {
g.Expect(envVars).To(Not(BeNil()), "Env Vars should not be nil")
Expand Down Expand Up @@ -678,7 +840,11 @@ func expectTLSEnvVarsWithGomega(g Gomega, envVars []corev1.EnvVar, expectedKeyst
}

if envVar.Name == "SOLR_SSL_CLIENT_TRUST_STORE" {
g.Expect(envVar.Value).To(Equal("/var/solr/client-tls/truststore.p12"), "Wrong envVar value for %s", envVar.Name)
expectedPath := "/var/solr/client-tls/truststore.p12"
if clientTLS.MergeJavaTruststore != "" {
expectedPath = "/var/solr/tls-client-merged/truststore.p12"
}
g.Expect(envVar.Value).To(Equal(expectedPath), "Wrong envVar value for %s", envVar.Name)
}

if envVar.Name == "SOLR_SSL_CLIENT_KEY_STORE_PASSWORD" || envVar.Name == "SOLR_SSL_CLIENT_TRUST_STORE_PASSWORD" {
Expand Down
32 changes: 32 additions & 0 deletions controllers/solrprometheusexporter_controller_tls_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,38 @@ var _ = FDescribe("SolrPrometheusExporter controller - TLS", func() {
})
})

FContext("TLS Secret - Merge TrustStore Only", func() {
tlsSecretName := "tls-cert-secret-from-user"
BeforeEach(func() {
solrPrometheusExporter.Spec = solrv1beta1.SolrPrometheusExporterSpec{
SolrReference: solrv1beta1.SolrReference{
Cloud: &solrv1beta1.SolrCloudReference{
ZookeeperConnectionInfo: &solrv1beta1.ZookeeperConnectionInfo{
InternalConnectionString: "host:2181",
ChRoot: "/this/path",
},
},
SolrTLS: &solrv1beta1.SolrTLSOptions{
TrustStorePasswordSecret: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{Name: tlsSecretName},
Key: "keystore-passwords-are-important",
},
TrustStoreSecret: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{Name: tlsSecretName},
Key: util.DefaultPkcs12TruststoreFile,
},
RestartOnTLSSecretUpdate: false,
MergeJavaTruststore: util.DefaultJvmTruststore,
},
},
}
})
FIt("has the correct resources", func() {
By("testing the SolrPrometheusExporter Deployment")
testReconcileWithTruststoreOnly(ctx, solrPrometheusExporter, tlsSecretName)
})
})

FContext("TLS Secret - TrustStore Only - Restart on Secret Update", func() {
tlsSecretName := "tls-cert-secret-from-user"
BeforeEach(func() {
Expand Down
Loading