Skip to content

Conversation

@thelabdude
Copy link
Contributor

@thelabdude thelabdude commented Aug 25, 2021

Fixes #300

Solr supports using a different TLS certificate (keystore & truststore) for making client requests (to other Solr pods) than the server certificate. This PR enhances the TLS configuration options to support this additional client certificate.

I also cleaned up the Prometheus exporter TLS code to allow only using a truststore as the exporter doesn't need a keystore unless mTLS is enabled since the exporter only exposes HTTP endpoints. In other words, the exporter config should support users only providing a truststore and not require a keystore (for flexibility).

I also renamed mountedServerTLSDir to mountedTLSDir since I introduced a spec.SolrClientTLS to the SolrCloudSpec struct; this is a new struct added for v0.4.0, so no back-compat concerns with this rename. When I originally created mountedServerTLSDir, I was thinking I would have a mountedClientTLSDir as well for this feature but I think having an additional top-level SolrClientTLS on the SolrCloudSpec is better since we don't want to introduce ClientKeystore + ClientKeystorePassword + ClientTruststore ... for the non-mounted dir option. Also, mountedTLSDir works better for the Prometheus exporter CRD since it's only a TLS client and doesn't serve HTTPs traffic.

It probably doesn't make sense to review the "diff" for controllers/util/solr_tls_util.go and instead just go over the entire file since I had to make so many changes there to accommodate these changes.

Here's an example SolrCloud config to put these changes into context:

  solrTLS:
    clientAuth: Need
    checkPeerName: true
    verifyClientHostname: true
    restartOnTLSSecretUpdate: true
    keyStorePasswordSecret:
      name: pkcs12-password-secret
      key: password-key
    pkcs12Secret:
      name: dev-selfsigned-cert-tls
      key: keystore.p12
    trustStoreSecret:
      name: dev-selfsigned-cert-tls
      key: truststore.p12

  solrClientTLS:
    restartOnTLSSecretUpdate: true
    keyStorePasswordSecret:
      name: pkcs12-password-secret
      key: password-key
    pkcs12Secret:
      name: dev-selfsigned-client-cert-tls
      key: keystore.p12
    trustStoreSecret:
      name: dev-selfsigned-client-cert-tls
      key: truststore.p12

@thelabdude thelabdude marked this pull request as draft August 25, 2021 20:26
@thelabdude thelabdude force-pushed the issue-300-mtls-client-cert branch from 2237ccf to 1adf882 Compare August 25, 2021 21:22
@thelabdude thelabdude marked this pull request as ready for review August 30, 2021 21:30
@thelabdude thelabdude force-pushed the issue-300-mtls-client-cert branch from b8c1aba to 537b425 Compare August 31, 2021 15:24
Copy link
Contributor

@HoustonPutman HoustonPutman left a comment

Choose a reason for hiding this comment

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

Looks really good! Don't have many comments to make other than small things.

@thelabdude thelabdude merged commit f2894be into apache:main Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Config should allow using a separate client cert for mTLS

2 participants