Skip to content

Commit 1739d44

Browse files
maltesandermaltesander
committed
fix: add s3_tls_truststore_commands & cleanup
1 parent 8c80948 commit 1739d44

5 files changed

Lines changed: 48 additions & 51 deletions

File tree

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -336,9 +336,7 @@ impl TrinoAuthenticationConfig {
336336
pub fn env_vars(&self, role: &TrinoRole, container: &crate::crd::Container) -> Vec<EnvVar> {
337337
self.env_vars
338338
.get(role)
339-
.cloned()
340-
.unwrap_or_default()
341-
.get(container)
339+
.and_then(|by_container| by_container.get(container))
342340
.cloned()
343341
.unwrap_or_default()
344342
}
@@ -347,9 +345,7 @@ impl TrinoAuthenticationConfig {
347345
pub fn commands(&self, role: &TrinoRole, container: &crate::crd::Container) -> Vec<String> {
348346
self.commands
349347
.get(role)
350-
.cloned()
351-
.unwrap_or_default()
352-
.get(container)
348+
.and_then(|by_container| by_container.get(container))
353349
.cloned()
354350
.unwrap_or_default()
355351
}

‎rust/operator-binary/src/authentication/password/ldap.rs‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,12 @@ impl LdapAuthenticator {
5858
/// Return the content of the authenticator config file to register with Trino
5959
pub fn config_file_data(&self) -> Result<BTreeMap<String, String>, Error> {
6060
let mut config_data = BTreeMap::new();
61-
self.ldap.endpoint_url().context(LdapEndpointSnafu)?;
61+
let endpoint_url = self.ldap.endpoint_url().context(LdapEndpointSnafu)?;
6262
config_data.insert(
6363
password::PASSWORD_AUTHENTICATOR_NAME.to_string(),
6464
PASSWORD_AUTHENTICATOR_NAME_LDAP.to_string(),
6565
);
66-
config_data.insert(
67-
LDAP_URL.to_string(),
68-
self.ldap.endpoint_url().context(LdapEndpointSnafu)?.into(),
69-
);
66+
config_data.insert(LDAP_URL.to_string(), endpoint_url.into());
7067

7168
config_data.insert(LDAP_USER_BASE_DN.to_string(), self.ldap.search_base.clone());
7269

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

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use snafu::{OptionExt, ResultExt, ensure};
33
use stackable_operator::{
44
builder::pod::volume::{VolumeBuilder, VolumeMountBuilder},
55
client::Client,
6-
commons::tls_verification::{CaCert, TlsServerVerification, TlsVerification},
76
crd::s3,
87
k8s_openapi::api::core::v1::ConfigMap,
98
v2::types::kubernetes::NamespaceName,
@@ -19,9 +18,9 @@ use super::{
1918
},
2019
};
2120
use crate::{
22-
command,
21+
config,
2322
crd::{
24-
CONFIG_DIR_NAME, STACKABLE_CLIENT_TLS_DIR,
23+
CONFIG_DIR_NAME,
2524
catalog::commons::{HdfsConnection, MetastoreConnection},
2625
},
2726
};
@@ -131,23 +130,10 @@ impl ExtendCatalogConfig for s3::v1alpha1::InlineConnectionOrReference {
131130
469.. => ensure!(s3.tls.uses_tls(), S3TlsRequiredSnafu),
132131
};
133132

134-
if let Some(tls) = s3.tls.tls.as_ref() {
135-
match &tls.verification {
136-
TlsVerification::None {} => return S3TlsNoVerificationNotSupportedSnafu.fail(),
137-
TlsVerification::Server(TlsServerVerification {
138-
ca_cert: CaCert::WebPki {},
139-
}) => {}
140-
TlsVerification::Server(TlsServerVerification {
141-
ca_cert: CaCert::SecretClass(_),
142-
}) => {
143-
if let Some(ca_cert) = s3.tls.tls_ca_cert_mount_path() {
144-
catalog_config.init_container_extra_start_commands.extend(
145-
command::add_cert_to_truststore(&ca_cert, STACKABLE_CLIENT_TLS_DIR),
146-
);
147-
}
148-
}
149-
}
150-
}
133+
catalog_config.init_container_extra_start_commands.extend(
134+
config::s3::s3_tls_truststore_commands(&s3.tls)
135+
.map_err(|_| S3TlsNoVerificationNotSupportedSnafu.build())?,
136+
);
151137

152138
Ok(())
153139
}

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

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::collections::BTreeMap;
33
use snafu::{ResultExt, Snafu};
44
use stackable_operator::{
55
client::Client,
6-
commons::tls_verification::{CaCert, TlsServerVerification, TlsVerification},
6+
commons::tls_verification::{CaCert, TlsClientDetails, TlsServerVerification, TlsVerification},
77
crd::s3,
88
k8s_openapi::api::core::v1::{Volume, VolumeMount},
99
};
@@ -21,6 +21,37 @@ pub enum Error {
2121
S3TlsNoVerificationNotSupported,
2222
}
2323

24+
/// Returned by [`s3_tls_truststore_commands`] when an S3 connection disables TLS verification,
25+
/// which Trino does not support.
26+
#[derive(Debug)]
27+
pub struct S3TlsVerificationDisabled;
28+
29+
/// Build the init-container commands that add an S3 server's CA certificate to the client
30+
/// truststore.
31+
///
32+
/// Returns an empty list when no extra trust setup is needed (TLS disabled, or WebPKI
33+
/// verification), and [`S3TlsVerificationDisabled`] when TLS verification is turned off (which
34+
/// Trino rejects). Shared by the spooling/exchange S3 config and the S3 catalog connection.
35+
pub fn s3_tls_truststore_commands(
36+
tls: &TlsClientDetails,
37+
) -> Result<Vec<String>, S3TlsVerificationDisabled> {
38+
let Some(tls_config) = tls.tls.as_ref() else {
39+
return Ok(Vec::new());
40+
};
41+
match &tls_config.verification {
42+
TlsVerification::None {} => Err(S3TlsVerificationDisabled),
43+
TlsVerification::Server(TlsServerVerification {
44+
ca_cert: CaCert::WebPki {},
45+
}) => Ok(Vec::new()),
46+
TlsVerification::Server(TlsServerVerification {
47+
ca_cert: CaCert::SecretClass(_),
48+
}) => Ok(tls
49+
.tls_ca_cert_mount_path()
50+
.map(|ca_cert| command::add_cert_to_truststore(&ca_cert, STACKABLE_CLIENT_TLS_DIR))
51+
.unwrap_or_default()),
52+
}
53+
}
54+
2455
pub struct ResolvedS3Config {
2556
/// Properties to add to config.properties
2657
pub properties: BTreeMap<String, String>,
@@ -91,23 +122,10 @@ impl ResolvedS3Config {
91122
]);
92123
}
93124

94-
if let Some(tls) = s3_connection.tls.tls.as_ref() {
95-
match &tls.verification {
96-
TlsVerification::None {} => return S3TlsNoVerificationNotSupportedSnafu.fail(),
97-
TlsVerification::Server(TlsServerVerification {
98-
ca_cert: CaCert::WebPki {},
99-
}) => {}
100-
TlsVerification::Server(TlsServerVerification {
101-
ca_cert: CaCert::SecretClass(_),
102-
}) => {
103-
if let Some(ca_cert) = s3_connection.tls.tls_ca_cert_mount_path() {
104-
resolved_config.init_container_extra_start_commands.extend(
105-
command::add_cert_to_truststore(&ca_cert, STACKABLE_CLIENT_TLS_DIR),
106-
);
107-
}
108-
}
109-
}
110-
}
125+
resolved_config.init_container_extra_start_commands.extend(
126+
s3_tls_truststore_commands(&s3_connection.tls)
127+
.map_err(|_| Error::S3TlsNoVerificationNotSupported)?,
128+
);
111129

112130
Ok(resolved_config)
113131
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,8 @@ async fn main() -> anyhow::Result<()> {
186186
config_map_cluster_store
187187
.state()
188188
.into_iter()
189-
.filter(move |druid| references_config_map(druid, &config_map))
190-
.map(|druid| ObjectRef::from_obj(&*druid))
189+
.filter(move |trino| references_config_map(trino, &config_map))
190+
.map(|trino| ObjectRef::from_obj(&*trino))
191191
},
192192
)
193193
.graceful_shutdown_on(sigterm_watcher.handle())

0 commit comments

Comments
 (0)