Skip to content

Commit 760d477

Browse files
authored
chore: Consolidate error enums and use expect where possible (#803)
* use expect where possible and remove unecessary enums/Results * changelog * improve expect messages * add panic docs to helper functions * remove panics doc whaere it make no sense * move listener pvc name test to the correct place * revert expects where they are dependent on user/calculated input or builder internals
1 parent 3fb1138 commit 760d477

4 files changed

Lines changed: 36 additions & 30 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
the new labels ([#799]).
2020
- Environment variable overrides (`envOverrides`) are now applied after all environment
2121
variables set by the operator ([#799]).
22+
- Make operations infallible where dependent on static inputs ([#803]).
2223

2324
### Fixed
2425

@@ -41,6 +42,7 @@
4142
[#795]: https://github.com/stackabletech/hbase-operator/pull/795
4243
[#797]: https://github.com/stackabletech/hbase-operator/pull/797
4344
[#799]: https://github.com/stackabletech/hbase-operator/pull/799
45+
[#803]: https://github.com/stackabletech/hbase-operator/pull/803
4446

4547
## [26.7.0] - 2026-07-21
4648

rust/operator-binary/src/controller/build/kerberos.rs

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -39,23 +39,18 @@ const PROTECTION_PRIVACY: &str = "privacy";
3939

4040
#[derive(Snafu, Debug)]
4141
pub enum Error {
42-
#[snafu(display("failed to build Kerberos secret volume"))]
43-
BuildKerberosSecretVolume {
42+
#[snafu(display("failed to build Kerberos secret volume source"))]
43+
BuildKerberosSecretVolumeSource {
4444
source: stackable_operator::builder::pod::volume::SecretOperatorVolumeSourceBuilderError,
4545
},
4646

47-
#[snafu(display("failed to build TLS secret volume"))]
48-
BuildTlsSecretVolume {
47+
#[snafu(display("failed to build TLS secret volume source"))]
48+
BuildTlsSecretVolumeSource {
4949
source: stackable_operator::builder::pod::volume::SecretOperatorVolumeSourceBuilderError,
5050
},
5151

5252
#[snafu(display("failed to add needed volume"))]
5353
AddVolume { source: builder::pod::Error },
54-
55-
#[snafu(display("failed to add needed volumeMount"))]
56-
AddVolumeMount {
57-
source: builder::pod::container::Error,
58-
},
5954
}
6055

6156
/// The `hbase-site.xml` Kerberos properties for `cluster`, gated on Kerberos being enabled
@@ -229,6 +224,13 @@ pub fn kerberos_ssl_client_settings() -> BTreeMap<String, String> {
229224
truststore_settings("client")
230225
}
231226

227+
/// Adds the Kerberos keytab and TLS keystore volumes to the [`PodBuilder`] and their mounts to the
228+
/// [`ContainerBuilder`], for whichever of the two secret classes are configured.
229+
///
230+
/// # Panics
231+
///
232+
/// Panics if the volume mounts cannot be added to the container builder. Only call this on a
233+
/// container builder whose mount paths are still distinct from the ones added here.
232234
pub fn add_kerberos_pod_config(
233235
cluster: &ValidatedCluster,
234236
metrics_service_name: &str,
@@ -247,15 +249,15 @@ pub fn add_kerberos_pod_config(
247249
.with_kerberos_service_name(kerberos_service_name())
248250
.with_kerberos_service_name("HTTP")
249251
.build()
250-
.context(BuildKerberosSecretVolumeSnafu)?;
252+
.context(BuildKerberosSecretVolumeSourceSnafu)?;
251253
pb.add_volume(
252254
VolumeBuilder::new(&*KERBEROS_VOLUME_NAME)
253255
.ephemeral(kerberos_secret_operator_volume)
254256
.build(),
255257
)
256258
.context(AddVolumeSnafu)?;
257259
cb.add_volume_mount(&*KERBEROS_VOLUME_NAME, STACKABLE_KERBEROS_DIR)
258-
.context(AddVolumeMountSnafu)?;
260+
.expect("The mount paths are statically defined and there should be no duplicates.");
259261
}
260262

261263
if let Some(https_secret_class) = &cluster.cluster_config.https_secret_class {
@@ -277,13 +279,13 @@ pub fn add_kerberos_pod_config(
277279
.with_tls_pkcs12_password(TLS_STORE_PASSWORD)
278280
.with_auto_tls_cert_lifetime(requested_secret_lifetime)
279281
.build()
280-
.context(BuildTlsSecretVolumeSnafu)?,
282+
.context(BuildTlsSecretVolumeSourceSnafu)?,
281283
)
282284
.build(),
283285
)
284286
.context(AddVolumeSnafu)?;
285287
cb.add_volume_mount(&*TLS_STORE_VOLUME_NAME, TLS_STORE_DIR)
286-
.context(AddVolumeMountSnafu)?;
288+
.expect("The mount paths are statically defined and there should be no duplicates.");
287289
}
288290
Ok(())
289291
}

rust/operator-binary/src/controller/build/resource/listener.rs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
//! Build the listener `Volume`/`PersistentVolumeClaim` exposing a rolegroup.
22
3-
use std::{str::FromStr, sync::LazyLock};
3+
use std::str::FromStr;
44

55
use snafu::{ResultExt, Snafu};
66
use stackable_operator::{
77
builder::pod::volume::{
88
ListenerOperatorVolumeSourceBuilder, ListenerOperatorVolumeSourceBuilderError,
99
ListenerReference, VolumeBuilder,
1010
},
11+
constant,
1112
k8s_openapi::api::core::v1::{PersistentVolumeClaim, Volume},
1213
kvp::Labels,
1314
v2::{
@@ -21,12 +22,7 @@ use stackable_operator::{
2122

2223
use crate::crd::{AnyServiceConfig, HbaseRole, LISTENER_VOLUME_NAME};
2324

24-
/// The rest servers' listener `PersistentVolumeClaim` reuses the listener volume name
25-
/// ([`LISTENER_VOLUME_NAME`]); the claim and the volume must share a name.
26-
static LISTENER_PVC_NAME: LazyLock<PersistentVolumeClaimName> = LazyLock::new(|| {
27-
PersistentVolumeClaimName::from_str(LISTENER_VOLUME_NAME)
28-
.expect("LISTENER_VOLUME_NAME is a valid PersistentVolumeClaim name")
29-
});
25+
constant!(LISTENER_PVC_NAME: PersistentVolumeClaimName = LISTENER_VOLUME_NAME);
3026

3127
#[derive(Snafu, Debug)]
3228
pub enum Error {
@@ -89,3 +85,14 @@ pub fn build_listener_pvc(
8985
)]),
9086
}
9187
}
88+
89+
#[cfg(test)]
90+
mod tests {
91+
use super::*;
92+
93+
#[test]
94+
fn test_constants() {
95+
// Test that dereferencing the constants does not panic.
96+
let _ = *LISTENER_PVC_NAME;
97+
}
98+
}

rust/operator-binary/src/controller/build/resource/statefulset.rs

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,6 @@ pub enum Error {
8888
#[snafu(display("failed to add needed volume"))]
8989
AddVolume { source: builder::pod::Error },
9090

91-
#[snafu(display("failed to add needed volumeMount"))]
92-
AddVolumeMount {
93-
source: builder::pod::container::Error,
94-
},
95-
9691
#[snafu(display("failed to build listener volume"))]
9792
ListenerVolume { source: super::listener::Error },
9893
}
@@ -192,15 +187,15 @@ pub fn build_rolegroup_statefulset(
192187
}])
193188
.add_env_vars(merged_env)
194189
.add_volume_mount(&*HBASE_CONFIG_VOLUME_NAME, HBASE_CONFIG_TMP_DIR)
195-
.context(AddVolumeMountSnafu)?
190+
.expect("The mount paths are statically defined and there should be no duplicates.")
196191
.add_volume_mount(&*HDFS_DISCOVERY_VOLUME_NAME, HDFS_DISCOVERY_TMP_DIR)
197-
.context(AddVolumeMountSnafu)?
192+
.expect("The mount paths are statically defined and there should be no duplicates.")
198193
.add_volume_mount(&*LOG_CONFIG_VOLUME_NAME, HBASE_LOG_CONFIG_TMP_DIR)
199-
.context(AddVolumeMountSnafu)?
194+
.expect("The mount paths are statically defined and there should be no duplicates.")
200195
.add_volume_mount(&*LOG_VOLUME_NAME, STACKABLE_LOG_DIR)
201-
.context(AddVolumeMountSnafu)?
196+
.expect("The mount paths are statically defined and there should be no duplicates.")
202197
.add_volume_mount(LISTENER_VOLUME_NAME, LISTENER_VOLUME_DIR)
203-
.context(AddVolumeMountSnafu)?
198+
.expect("The mount paths are statically defined and there should be no duplicates.")
204199
.add_container_ports(ports)
205200
.resources(merged_config.resources().clone().into())
206201
.startup_probe(startup_probe)

0 commit comments

Comments
 (0)