Let secret op create pkcs12 stores (#470)

* wip - keystores from secret op working * reduced visibility of import methods, added comments * wip - adapted tests * use generated uuid for truststore alias * adapted changelog * try to fix yaml lints
stackabletech · Sep 26, 2023 · 5affb01 · 5affb01
1 parent 2599608
commit 5affb01
Show file tree

Hide file tree

Showing 7 changed files with 95 additions and 46 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,8 +16,9 @@ All notable changes to this project will be documented in this file.
 
 ### Changed
 
-- `operator-rs` `0.44.0` -> `0.45.1` ([#441], [#453]).
+- `operator-rs` `0.44.0` -> `0.48.0` ([#441], [#453], [#470]).
 - `vector` `0.26.0` -> `0.31.0` ([#453]).
+- Let secret-operator handle certificate conversion ([#470]).
 
 [#441]: https://github.com/stackabletech/trino-operator/pull/441
 [#449]: https://github.com/stackabletech/trino-operator/pull/449
@@ -26,6 +27,7 @@ All notable changes to this project will be documented in this file.
 [#461]: https://github.com/stackabletech/trino-operator/pull/461
 [#463]: https://github.com/stackabletech/trino-operator/pull/463
 [#466]: https://github.com/stackabletech/trino-operator/pull/466
+[#470]: https://github.com/stackabletech/trino-operator/pull/470
 [#474]: https://github.com/stackabletech/trino-operator/pull/474
 
 ## [23.7.0] - 2023-07-14

diff --git a/rust/operator-binary/src/catalog/commons.rs b/rust/operator-binary/src/catalog/commons.rs
@@ -144,7 +144,7 @@ impl ExtendCatalogConfig for S3ConnectionDef {
 
                     // Copy the ca.crt from the ca-cert secretclass into truststore for external services
                     catalog_config.init_container_extra_start_commands.extend(
-                        command::add_cert_to_stackable_truststore(
+                        command::add_cert_to_truststore(
                             format!("{volume_mount_path}/ca.crt").as_str(),
                             STACKABLE_CLIENT_TLS_DIR,
                             &volume_name,

diff --git a/rust/operator-binary/src/command.rs b/rust/operator-binary/src/command.rs
@@ -11,10 +11,6 @@ use stackable_trino_crd::{
     STACKABLE_TLS_STORE_PASSWORD, SYSTEM_TRUST_STORE, SYSTEM_TRUST_STORE_PASSWORD,
 };
 
-pub const STACKABLE_CLIENT_CA_CERT: &str = "stackable-client-ca-cert";
-pub const STACKABLE_SERVER_CA_CERT: &str = "stackable-server-ca-cert";
-pub const STACKABLE_INTERNAL_CA_CERT: &str = "stackable-internal-ca-cert";
-
 pub fn container_prepare_args(
     trino: &TrinoCluster,
     catalogs: &[CatalogConfig],
@@ -39,34 +35,36 @@ pub fn container_prepare_args(
     }
 
     if trino.tls_enabled() {
-        args.extend(create_key_and_trust_store(
+        args.extend(import_truststore(
+            STACKABLE_MOUNT_SERVER_TLS_DIR,
+            STACKABLE_SERVER_TLS_DIR,
+        ));
+        args.extend(import_keystore(
             STACKABLE_MOUNT_SERVER_TLS_DIR,
             STACKABLE_SERVER_TLS_DIR,
-            STACKABLE_SERVER_CA_CERT,
         ));
     }
 
     if trino.get_internal_tls().is_some() {
-        args.extend(create_key_and_trust_store(
+        args.extend(import_truststore(
+            STACKABLE_MOUNT_INTERNAL_TLS_DIR,
+            STACKABLE_INTERNAL_TLS_DIR,
+        ));
+        args.extend(import_keystore(
             STACKABLE_MOUNT_INTERNAL_TLS_DIR,
             STACKABLE_INTERNAL_TLS_DIR,
-            STACKABLE_INTERNAL_CA_CERT,
         ));
-        // Add cert to internal truststore
         if trino.tls_enabled() {
-            args.extend(add_cert_to_stackable_truststore(
-                format!("{STACKABLE_MOUNT_SERVER_TLS_DIR}/ca.crt").as_str(),
+            args.extend(import_truststore(
+                STACKABLE_MOUNT_SERVER_TLS_DIR,
                 STACKABLE_INTERNAL_TLS_DIR,
-                STACKABLE_CLIENT_CA_CERT,
-            ));
+            ))
         }
     }
 
     // Create truststore that will be used when talking to external tools like S3
     // It will be populated from the system truststore so that connections against public services like AWS S3 are still possible
-    args.extend(create_truststore_from_system_truststore(
-        STACKABLE_CLIENT_TLS_DIR,
-    ));
+    args.extend(import_system_truststore(STACKABLE_CLIENT_TLS_DIR));
 
     // Add the commands that are needed to set up the catalogs
     catalogs.iter().for_each(|catalog| {
@@ -114,39 +112,67 @@ pub fn container_trino_args(
     vec![args.join(" && ")]
 }
 
-/// Generates the shell script to create key and truststores from the certificates provided
-/// by the secret operator.
-pub fn create_key_and_trust_store(
-    cert_directory: &str,
-    stackable_cert_directory: &str,
+/// Adds a CA file from `cert_file` into a truststore named `truststore.p12` in `destination_directory`
+/// under the alias `alias_name`.
+pub fn add_cert_to_truststore(
+    cert_file: &str,
+    destination_directory: &str,
     alias_name: &str,
 ) -> Vec<String> {
     vec![
-        format!("echo [{stackable_cert_directory}] Cleaning up truststore - just in case"),
-        format!("rm -f {stackable_cert_directory}/truststore.p12"),
-        format!("echo [{stackable_cert_directory}] Creating truststore"),
-        format!("keytool -importcert -file {cert_directory}/ca.crt -keystore {stackable_cert_directory}/truststore.p12 -storetype pkcs12 -noprompt -alias {alias_name} -storepass {STACKABLE_TLS_STORE_PASSWORD}"),
-        format!("echo [{stackable_cert_directory}] Creating certificate chain"),
-        format!("cat {cert_directory}/ca.crt {cert_directory}/tls.crt > {stackable_cert_directory}/chain.crt"),
-        format!("echo [{stackable_cert_directory}] Creating keystore"),
-        format!("openssl pkcs12 -export -in {stackable_cert_directory}/chain.crt -inkey {cert_directory}/tls.key -out {stackable_cert_directory}/keystore.p12 --passout pass:{STACKABLE_TLS_STORE_PASSWORD}")
+        format!("echo Adding cert from {cert_file} to truststore {destination_directory}/truststore.p12"),
+        format!("keytool -importcert -file {cert_file} -keystore {destination_directory}/truststore.p12 -storetype pkcs12 -noprompt -alias {alias_name} -storepass {STACKABLE_TLS_STORE_PASSWORD}"),
     ]
 }
 
-pub fn create_truststore_from_system_truststore(truststore_directory: &str) -> Vec<String> {
+/// Generates the shell script to import a secret operator provided keystore without password
+/// into a new keystore with password in a writeable empty dir
+///
+/// # Arguments
+/// - `source_directory`      - The directory of the source keystore.
+///                             Should usually be a secret operator volume mount.
+/// - `destination_directory` - The directory of the destination keystore.
+///                             Should usually be an empty dir.
+fn import_keystore(source_directory: &str, destination_directory: &str) -> Vec<String> {
     vec![
-        format!("echo [{truststore_directory}] Creating truststore {truststore_directory}/truststore.p12 from system truststore {SYSTEM_TRUST_STORE}"),
-        format!("keytool -importkeystore -srckeystore {SYSTEM_TRUST_STORE} -srcstoretype jks -srcstorepass {SYSTEM_TRUST_STORE_PASSWORD} -destkeystore {truststore_directory}/truststore.p12 -deststoretype pkcs12 -deststorepass {STACKABLE_TLS_STORE_PASSWORD} -noprompt"),
+        // The source directory is a secret-op mount and we do not want to write / add anything in there
+        // Therefore we import all the contents to a keystore in "writeable" empty dirs.
+        // Keytool is only barking if a password is not set for the destination keystore (which we set)
+        // and do provide an empty password for the source keystore coming from the secret-operator.
+        // Using no password will result in a warning.
+        format!("echo Importing {source_directory}/keystore.p12 to {destination_directory}/keystore.p12"),
+        format!("keytool -importkeystore -srckeystore {source_directory}/keystore.p12 -srcstoretype PKCS12 -srcstorepass \"\" -destkeystore {destination_directory}/keystore.p12 -deststoretype PKCS12 -deststorepass {STACKABLE_TLS_STORE_PASSWORD} -noprompt"),
     ]
 }
 
-pub fn add_cert_to_stackable_truststore(
-    cert_file: &str,
-    truststore_directory: &str,
-    alias_name: &str,
-) -> Vec<String> {
+/// Generates the shell script to import a secret operator provided truststore without password
+/// into a new truststore with password in a writeable empty dir
+///
+/// # Arguments
+/// - `source_directory`      - The directory of the source truststore.
+///                             Should usually be a secret operator volume mount.
+/// - `destination_directory` - The directory of the destination truststore.
+///                             Should usually be an empty dir.
+fn import_truststore(source_directory: &str, destination_directory: &str) -> Vec<String> {
+    vec![
+        // The source directory is a secret-op mount and we do not want to write / add anything in there
+        // Therefore we import all the contents to a truststore in "writeable" empty dirs.
+        // Keytool is only barking if a password is not set for the destination truststore (which we set)
+        // and do provide an empty password for the source truststore coming from the secret-operator.
+        // Using no password will result in a warning.
+        // All secret-op generated truststores have one entry with alias "1". We generate a UUID for 
+        // the destination truststore to avoid conflicts when importing multiple secret-op generated 
+        // truststores. We do not use the UUID rust crate since this will continuously change the STS... and
+        // leads to never-ending reconciles.
+        format!("echo Importing {source_directory}/truststore.p12 to {destination_directory}/truststore.p12"),
+        format!("keytool -importkeystore -srckeystore {source_directory}/truststore.p12 -srcstoretype PKCS12 -srcstorepass \"\" -srcalias 1 -destkeystore {destination_directory}/truststore.p12 -deststoretype PKCS12 -deststorepass {STACKABLE_TLS_STORE_PASSWORD} -destalias $(cat /proc/sys/kernel/random/uuid) -noprompt"),
+    ]
+}
+
+/// Import the system truststore to a truststore named `truststore.p12` in `destination_directory`.
+fn import_system_truststore(destination_directory: &str) -> Vec<String> {
     vec![
-        format!("echo [{truststore_directory}] Adding cert from {cert_file} to truststore {truststore_directory}/truststore.p12"),
-        format!("keytool -importcert -file {cert_file} -keystore {truststore_directory}/truststore.p12 -storetype pkcs12 -noprompt -alias {alias_name} -storepass {STACKABLE_TLS_STORE_PASSWORD}"),
+        format!("echo Importing {SYSTEM_TRUST_STORE} to {destination_directory}/truststore.p12"),
+        format!("keytool -importkeystore -srckeystore {SYSTEM_TRUST_STORE} -srcstoretype jks -srcstorepass {SYSTEM_TRUST_STORE_PASSWORD} -destkeystore {destination_directory}/truststore.p12 -deststoretype pkcs12 -deststorepass {STACKABLE_TLS_STORE_PASSWORD} -noprompt"),
     ]
 }
diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs
@@ -9,6 +9,7 @@ use crate::{
 
 use indoc::formatdoc;
 use snafu::{OptionExt, ResultExt, Snafu};
+use stackable_operator::builder::SecretFormat;
 use stackable_operator::{
     builder::{
         resources::ResourceRequirementsBuilder, ConfigMapBuilder, ContainerBuilder,
@@ -1392,6 +1393,7 @@ fn create_tls_volume(volume_name: &str, tls_secret_class: &str) -> Volume {
             SecretOperatorVolumeSourceBuilder::new(tls_secret_class)
                 .with_pod_scope()
                 .with_node_scope()
+                .with_format(SecretFormat::TlsPkcs12)
                 .build(),
         )
         .build()

diff --git a/tests/templates/kuttl/tls/20-install-check.yaml b/tests/templates/kuttl/tls/20-install-check.yaml
@@ -19,3 +19,24 @@ spec:
         - name: trino-test-helper
           image: docker.stackable.tech/stackable/testing-tools:0.2.0-stackable0.0.0-dev
           command: ["sleep", "infinity"]
+          volumeMounts:
+            - mountPath: "/stackable/trusted"
+              name: server-tls-mount
+      securityContext:
+        runAsUser: 1000
+        runAsGroup: 1000
+        fsGroup: 1000
+      volumes:
+        - name: server-tls-mount
+          ephemeral:
+            volumeClaimTemplate:
+              metadata:
+                annotations:
+                  secrets.stackable.tech/class: trino-tls
+                  secrets.stackable.tech/scope: pod,node
+              spec:
+                accessModes: ["ReadWriteOnce"]
+                resources:
+                  requests:
+                    storage: "1"
+                storageClassName: secrets.stackable.tech
diff --git a/tests/templates/kuttl/tls/21-install-requirements.yaml.j2 b/tests/templates/kuttl/tls/21-install-requirements.yaml.j2
@@ -5,7 +5,5 @@ commands:
   - script: kubectl cp -n $NAMESPACE ./check-tls.py       trino-test-helper-0:/tmp || true
   - script: kubectl cp -n $NAMESPACE ./test-config.json   trino-test-helper-0:/tmp
 {% if test_scenario['values']['use-authentication'] == 'true' or test_scenario['values']['use-tls'] == 'true' %}
-  - script: kubectl cp -n $NAMESPACE trino-coordinator-default-0:/stackable/mount_server_tls/ca.crt /tmp/ca.crt || true
-  - script: kubectl cp -n $NAMESPACE /tmp/ca.crt          trino-test-helper-0:/tmp/ca.crt || true
-  - script: kubectl cp -n $NAMESPACE ./untrusted-cert.crt trino-test-helper-0:/tmp/untrusted-cert.crt || true
+  - script: kubectl cp -n $NAMESPACE ./untrusted-cert.crt trino-test-helper-0:/stackable/untrusted-cert.crt || true
 {% endif %}
diff --git a/tests/templates/kuttl/tls/check-tls.py b/tests/templates/kuttl/tls/check-tls.py
@@ -66,8 +66,8 @@ def read_json(config_path):
 
     conf = read_json("/tmp/test-config.json")  # config file to indicate our test script if auth / tls is used or not
     coordinator_host = 'trino-coordinator-default-0.trino-coordinator-default.' + namespace + '.svc.cluster.local'
-    trusted_ca = "/tmp/ca.crt"  # will be copied via kubectl from the coordinator pod
-    untrusted_ca = "/tmp/untrusted-cert.crt"  # some random CA
+    trusted_ca = "/stackable/trusted/ca.crt"  # will be mounted from secret op
+    untrusted_ca = "/stackable/untrusted-cert.crt"  # some random CA
     query = "SHOW CATALOGS"
 
     # We expect these to work