Skip to content

Commit a2aed87

Browse files
sbernaueradwk67
sbernauer
and
adwk67
authored
fix: Prevent missing certificates (#796)
* fix: Certificate handling in case SecretClass has multiple CAs * linter * linter * Update to new CLI args * Delete _WORK folder * Add comment * changelog * changelog * Apply suggestions from code review Co-authored-by: Andrew Kenworthy <1712947+adwk67@users.noreply.github.com> * Update rust/operator-binary/src/command.rs Co-authored-by: Andrew Kenworthy <1712947+adwk67@users.noreply.github.com> --------- Co-authored-by: Andrew Kenworthy <1712947+adwk67@users.noreply.github.com>
1 parent 4174989 commit a2aed87

File tree

7 files changed

+37
-113
lines changed

7 files changed

+37
-113
lines changed

‎CHANGELOG.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,19 @@ All notable changes to this project will be documented in this file.
1010
- Support for the client spooling protocol ([#793]).
1111
- Helm: Allow Pod `priorityClassName` to be configured ([#798]).
1212

13+
### Fixed
14+
15+
- Previously we had a bug that could lead to missing certificates ([#796]).
16+
17+
This could be the case when the Stackable PKI rotated its CA certificate or you specified multiple
18+
CAs in your SecretClass.
19+
Especially the CA rotation could break working clusters at any time.
20+
We now correctly handle multiple certificates for both cases.
21+
See [this GitHub issue](https://github.com/stackabletech/issues/issues/764) for details
22+
1323
[#779]: https://github.com/stackabletech/trino-operator/pull/779
1424
[#793]: https://github.com/stackabletech/trino-operator/pull/793
25+
[#796]: https://github.com/stackabletech/trino-operator/pull/796
1526
[#798]: https://github.com/stackabletech/trino-operator/pull/798
1627

1728
## [25.7.0] - 2025-07-23

‎rust/operator-binary/src/authentication/oidc/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ impl TrinoOidcAuthentication {
184184
oauth2_authentication_config.add_commands(
185185
TrinoRole::Coordinator,
186186
crate::crd::Container::Prepare,
187-
command::add_cert_to_truststore(&path, STACKABLE_CLIENT_TLS_DIR, "oidc-idp"),
187+
command::add_cert_to_truststore(&path, STACKABLE_CLIENT_TLS_DIR),
188188
);
189189
}
190190
}

‎rust/operator-binary/src/catalog/commons.rs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ impl ExtendCatalogConfig for s3::v1alpha1::InlineConnectionOrReference {
7777
async fn extend_catalog_config(
7878
&self,
7979
catalog_config: &mut CatalogConfig,
80-
catalog_name: &str,
80+
_catalog_name: &str,
8181
catalog_namespace: Option<String>,
8282
client: &Client,
8383
trino_version: u16,
@@ -151,11 +151,7 @@ impl ExtendCatalogConfig for s3::v1alpha1::InlineConnectionOrReference {
151151
}) => {
152152
if let Some(ca_cert) = s3.tls.tls_ca_cert_mount_path() {
153153
catalog_config.init_container_extra_start_commands.extend(
154-
command::add_cert_to_truststore(
155-
&ca_cert,
156-
STACKABLE_CLIENT_TLS_DIR,
157-
&format!("{catalog_name}-ca-cert"),
158-
),
154+
command::add_cert_to_truststore(&ca_cert, STACKABLE_CLIENT_TLS_DIR),
159155
);
160156
}
161157
}

‎rust/operator-binary/src/command.rs‎

Lines changed: 19 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use crate::{
1616
RW_CONFIG_DIR_NAME, SPOOLING_MANAGER_PROPERTIES, STACKABLE_CLIENT_TLS_DIR,
1717
STACKABLE_INTERNAL_TLS_DIR, STACKABLE_MOUNT_INTERNAL_TLS_DIR,
1818
STACKABLE_MOUNT_SERVER_TLS_DIR, STACKABLE_SERVER_TLS_DIR, STACKABLE_TLS_STORE_PASSWORD,
19-
SYSTEM_TRUST_STORE, SYSTEM_TRUST_STORE_PASSWORD, TrinoRole, v1alpha1,
19+
TrinoRole, v1alpha1,
2020
},
2121
};
2222

@@ -45,38 +45,25 @@ pub fn container_prepare_args(
4545
));
4646
}
4747

48+
// Create truststore that will be used when talking to external tools like S3
49+
// It will be populated from the system truststore so that connections against public services like AWS S3 are still possible
50+
// FIXME: *Technically* we should only add the system truststore in case any webPki usage is detected, whether that's in
51+
// S3, LDAP, OIDC, FTE or whatnot.
52+
args.push(format!("cert-tools generate-pkcs12-truststore --pem /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem --out {STACKABLE_CLIENT_TLS_DIR}/truststore.p12 --out-password {STACKABLE_TLS_STORE_PASSWORD}"));
53+
4854
if trino.tls_enabled() {
49-
args.extend(import_truststore(
50-
STACKABLE_MOUNT_SERVER_TLS_DIR,
51-
STACKABLE_SERVER_TLS_DIR,
52-
));
53-
args.extend(import_keystore(
54-
STACKABLE_MOUNT_SERVER_TLS_DIR,
55-
STACKABLE_SERVER_TLS_DIR,
56-
));
55+
args.push(format!("cp {STACKABLE_MOUNT_SERVER_TLS_DIR}/truststore.p12 {STACKABLE_SERVER_TLS_DIR}/truststore.p12"));
56+
args.push(format!("cp {STACKABLE_MOUNT_SERVER_TLS_DIR}/keystore.p12 {STACKABLE_SERVER_TLS_DIR}/keystore.p12"));
5757
}
5858

5959
if trino.get_internal_tls().is_some() {
60-
args.extend(import_truststore(
61-
STACKABLE_MOUNT_INTERNAL_TLS_DIR,
62-
STACKABLE_INTERNAL_TLS_DIR,
63-
));
64-
args.extend(import_keystore(
65-
STACKABLE_MOUNT_INTERNAL_TLS_DIR,
66-
STACKABLE_INTERNAL_TLS_DIR,
67-
));
60+
args.push(format!("cp {STACKABLE_MOUNT_INTERNAL_TLS_DIR}/truststore.p12 {STACKABLE_INTERNAL_TLS_DIR}/truststore.p12"));
61+
args.push(format!("cp {STACKABLE_MOUNT_INTERNAL_TLS_DIR}/keystore.p12 {STACKABLE_INTERNAL_TLS_DIR}/keystore.p12"));
6862
if trino.tls_enabled() {
69-
args.extend(import_truststore(
70-
STACKABLE_MOUNT_SERVER_TLS_DIR,
71-
STACKABLE_INTERNAL_TLS_DIR,
72-
))
63+
args.push(format!("cert-tools generate-pkcs12-truststore --pkcs12 {STACKABLE_MOUNT_SERVER_TLS_DIR}/truststore.p12:{STACKABLE_TLS_STORE_PASSWORD} --pkcs12 {STACKABLE_INTERNAL_TLS_DIR}/truststore.p12:{STACKABLE_TLS_STORE_PASSWORD} --out {STACKABLE_INTERNAL_TLS_DIR}/truststore.p12 --out-password {STACKABLE_TLS_STORE_PASSWORD}"));
7364
}
7465
}
7566

76-
// Create truststore that will be used when talking to external tools like S3
77-
// It will be populated from the system truststore so that connections against public services like AWS S3 are still possible
78-
args.extend(import_system_truststore(STACKABLE_CLIENT_TLS_DIR));
79-
8067
// Add the commands that are needed to set up the catalogs
8168
catalogs.iter().for_each(|catalog| {
8269
args.extend_from_slice(&catalog.init_container_extra_start_commands);
@@ -159,77 +146,11 @@ wait_for_termination $!
159146
args
160147
}
161148

162-
/// Adds a CA file from `cert_file` into a truststore named `truststore.p12` in `destination_directory`
163-
/// under the alias `alias_name`.
164-
pub fn add_cert_to_truststore(
165-
cert_file: &str,
166-
destination_directory: &str,
167-
alias_name: &str,
168-
) -> Vec<String> {
169-
vec![
170-
format!(
171-
"echo Adding cert from {cert_file} to truststore {destination_directory}/truststore.p12"
172-
),
173-
format!(
174-
"keytool -importcert -file {cert_file} -keystore {destination_directory}/truststore.p12 -storetype pkcs12 -noprompt -alias {alias_name} -storepass {STACKABLE_TLS_STORE_PASSWORD}"
175-
),
176-
]
177-
}
178-
179-
/// Generates the shell script to import a secret operator provided keystore without password
180-
/// into a new keystore with password in a writeable empty dir
181-
///
182-
/// # Arguments
183-
/// - `source_directory`: The directory of the source keystore. Should usually be a secret operator volume mount.
184-
/// - `destination_directory`: The directory of the destination keystore. Should usually be an empty dir.
185-
fn import_keystore(source_directory: &str, destination_directory: &str) -> Vec<String> {
186-
vec![
187-
// The source directory is a secret-op mount and we do not want to write / add anything in there
188-
// Therefore we import all the contents to a keystore in "writeable" empty dirs.
189-
// Keytool is only barking if a password is not set for the destination keystore (which we set)
190-
// and do provide an empty password for the source keystore coming from the secret-operator.
191-
// Using no password will result in a warning.
192-
format!(
193-
"echo Importing {source_directory}/keystore.p12 to {destination_directory}/keystore.p12"
194-
),
195-
format!(
196-
"keytool -importkeystore -srckeystore {source_directory}/keystore.p12 -srcstoretype PKCS12 -srcstorepass \"\" -destkeystore {destination_directory}/keystore.p12 -deststoretype PKCS12 -deststorepass {STACKABLE_TLS_STORE_PASSWORD} -noprompt"
197-
),
198-
]
199-
}
200-
201-
/// Generates the shell script to import a secret operator provided truststore without password
202-
/// into a new truststore with password in a writeable empty dir
203-
///
204-
/// # Arguments
205-
/// - `source_directory`: The directory of the source truststore. Should usually be a secret operator volume mount.
206-
/// - `destination_directory`: The directory of the destination truststore. Should usually be an empty dir.
207-
fn import_truststore(source_directory: &str, destination_directory: &str) -> Vec<String> {
208-
vec![
209-
// The source directory is a secret-op mount and we do not want to write / add anything in there
210-
// Therefore we import all the contents to a truststore in "writeable" empty dirs.
211-
// Keytool is only barking if a password is not set for the destination truststore (which we set)
212-
// and do provide an empty password for the source truststore coming from the secret-operator.
213-
// Using no password will result in a warning.
214-
// All secret-op generated truststores have one entry with alias "1". We generate a UUID for
215-
// the destination truststore to avoid conflicts when importing multiple secret-op generated
216-
// truststores. We do not use the UUID rust crate since this will continuously change the STS... and
217-
// leads to never-ending reconciles.
218-
format!(
219-
"echo Importing {source_directory}/truststore.p12 to {destination_directory}/truststore.p12"
220-
),
221-
format!(
222-
"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"
223-
),
224-
]
225-
}
226-
227-
/// Import the system truststore to a truststore named `truststore.p12` in `destination_directory`.
228-
fn import_system_truststore(destination_directory: &str) -> Vec<String> {
229-
vec![
230-
format!("echo Importing {SYSTEM_TRUST_STORE} to {destination_directory}/truststore.p12"),
231-
format!(
232-
"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"
233-
),
234-
]
149+
/// Adds a PEM file to configured PKCS12 truststore (using the [`STACKABLE_TLS_STORE_PASSWORD`]
150+
/// password)
151+
pub fn add_cert_to_truststore(cert_file: &str, destination_directory: &str) -> Vec<String> {
152+
let truststore = format!("{destination_directory}/truststore.p12");
153+
vec![format!(
154+
"cert-tools generate-pkcs12-truststore --pkcs12 {truststore}:{STACKABLE_TLS_STORE_PASSWORD} --pem {cert_file} --out {truststore} --out-password {STACKABLE_TLS_STORE_PASSWORD}"
155+
)]
235156
}

‎rust/operator-binary/src/config/s3.rs‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -108,11 +108,7 @@ impl ResolvedS3Config {
108108
}) => {
109109
if let Some(ca_cert) = s3_connection.tls.tls_ca_cert_mount_path() {
110110
resolved_config.init_container_extra_start_commands.extend(
111-
command::add_cert_to_truststore(
112-
&ca_cert,
113-
STACKABLE_CLIENT_TLS_DIR,
114-
"resolved-s3-ca-cert",
115-
),
111+
command::add_cert_to_truststore(&ca_cert, STACKABLE_CLIENT_TLS_DIR),
116112
);
117113
}
118114
}

‎rust/operator-binary/src/controller.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ use crate::{
8787
JVM_SECURITY_PROPERTIES, LOG_PROPERTIES, MAX_TRINO_LOG_FILES_SIZE, METRICS_PORT,
8888
METRICS_PORT_NAME, NODE_PROPERTIES, RW_CONFIG_DIR_NAME, SPOOLING_MANAGER_PROPERTIES,
8989
STACKABLE_CLIENT_TLS_DIR, STACKABLE_INTERNAL_TLS_DIR, STACKABLE_MOUNT_INTERNAL_TLS_DIR,
90-
STACKABLE_MOUNT_SERVER_TLS_DIR, STACKABLE_SERVER_TLS_DIR, TrinoRole,
90+
STACKABLE_MOUNT_SERVER_TLS_DIR, STACKABLE_SERVER_TLS_DIR, STACKABLE_TLS_STORE_PASSWORD,
91+
TrinoRole,
9192
authentication::resolve_authentication_classes,
9293
catalog,
9394
discovery::{TrinoDiscovery, TrinoDiscoveryProtocol, TrinoPodRef},
@@ -1689,6 +1690,7 @@ fn create_tls_volume(
16891690
secret_volume_source_builder
16901691
.with_pod_scope()
16911692
.with_format(SecretFormat::TlsPkcs12)
1693+
.with_tls_pkcs12_password(STACKABLE_TLS_STORE_PASSWORD)
16921694
.with_auto_tls_cert_lifetime(*requested_secret_lifetime);
16931695

16941696
if let Some(listener_scope) = &listener_scope {

‎rust/operator-binary/src/crd/mod.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,8 @@ pub const STACKABLE_INTERNAL_TLS_DIR: &str = "/stackable/internal_tls";
101101
pub const STACKABLE_LOG_DIR: &str = "/stackable/log";
102102
pub const STACKABLE_MOUNT_SERVER_TLS_DIR: &str = "/stackable/mount_server_tls";
103103
pub const STACKABLE_MOUNT_INTERNAL_TLS_DIR: &str = "/stackable/mount_internal_tls";
104-
pub const SYSTEM_TRUST_STORE: &str = "/etc/pki/java/cacerts";
105104
// store pws
106105
pub const STACKABLE_TLS_STORE_PASSWORD: &str = "changeit";
107-
pub const SYSTEM_TRUST_STORE_PASSWORD: &str = "changeit";
108106
// secret vars
109107
pub const ENV_INTERNAL_SECRET: &str = "INTERNAL_SECRET";
110108
pub const ENV_SPOOLING_SECRET: &str = "SPOOLING_SECRET";

0 commit comments

Comments
 (0)