Skip to content

Commit f9113fc

Browse files
committed
chore: Add unwrap and panic lints
1 parent d74c0ce commit f9113fc

File tree

24 files changed

+147
-55
lines changed

24 files changed

+147
-55
lines changed

Cargo.toml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,11 @@ url = { version = "2.5.2", features = ["serde"] }
8484
x509-cert = { version = "0.2.5", features = ["builder"] }
8585
zeroize = "1.8.1"
8686

87+
[workspace.lints.clippy]
88+
unwrap_in_result = "deny"
89+
unwrap_used = "deny"
90+
panic = "deny"
91+
8792
# Use O3 in tests to improve the RSA key generation speed in the stackable-certs crate
8893
[profile.test.package]
8994
stackable-certs.opt-level = 3

clippy.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
allow-unwrap-in-tests = true
2+
allow-panic-in-tests = true

crates/k8s-version/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,6 @@ quote.workspace = true
2323
proc-macro2.workspace = true
2424
serde_yaml.workspace = true
2525
syn.workspace = true
26+
27+
[lints]
28+
workspace = true

crates/k8s-version/src/level/mod.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,14 @@ pub enum Level {
4747
impl FromStr for Level {
4848
type Err = ParseLevelError;
4949

50+
// SAFETY: We purposefully allow the `clippy::unwrap_in_result` lint below in this function.
51+
// We can use expect here, because the correct match labels must be used.
52+
//
53+
// FIXME (@Techassi): This attribute can be used on individual unwrap and expect calls since
54+
// Rust 1.91.0. We should move this attribute to not contaminate an unnecessarily large scope
55+
// once we bump the toolchain to 1.91.0.
56+
// See https://github.com/rust-lang/rust-clippy/pull/15445
57+
#[allow(clippy::unwrap_in_result)]
5058
fn from_str(input: &str) -> Result<Self, Self::Err> {
5159
let captures = LEVEL_REGEX.captures(input).context(InvalidFormatSnafu)?;
5260

crates/k8s-version/src/version/mod.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,14 @@ pub struct Version {
5353
impl FromStr for Version {
5454
type Err = ParseVersionError;
5555

56+
// SAFETY: We purposefully allow the `clippy::unwrap_in_result` lint below in this function.
57+
// We can use expect here, because the correct match label must be used.
58+
//
59+
// FIXME (@Techassi): This attribute can be used on individual unwrap and expect calls since
60+
// Rust 1.91.0. We should move this attribute to not contaminate an unnecessarily large scope
61+
// once we bump the toolchain to 1.91.0.
62+
// See https://github.com/rust-lang/rust-clippy/pull/15445
63+
#[allow(clippy::unwrap_in_result)]
5664
fn from_str(input: &str) -> Result<Self, Self::Err> {
5765
let captures = VERSION_REGEX.captures(input).context(InvalidFormatSnafu)?;
5866

@@ -141,6 +149,7 @@ mod test {
141149
#[case("v1gamma12", ParseVersionError::ParseLevel { source: ParseLevelError::UnknownIdentifier })]
142150
#[case("v1betä1", ParseVersionError::InvalidFormat)]
143151
#[case("1beta1", ParseVersionError::InvalidFormat)]
152+
#[case("v", ParseVersionError::InvalidFormat)]
144153
#[case("", ParseVersionError::InvalidFormat)]
145154
fn invalid_version(#[case] input: &str, #[case] error: ParseVersionError) {
146155
let err = Version::from_str(input).expect_err("invalid Kubernetes version");

crates/stackable-certs/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,6 @@ tokio.workspace = true
2929
tracing.workspace = true
3030
x509-cert.workspace = true
3131
zeroize.workspace = true
32+
33+
[lints]
34+
workspace = true

crates/stackable-certs/src/ca/mod.rs

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ impl PartialEq for Error {
110110
x509_cert::builder::Error::Signature(_),
111111
x509_cert::builder::Error::Signature(_),
112112
) => panic!(
113-
"it is impossible to compare the opaque Error contained witin signature::error::Error"
113+
"it is impossible to compare the opaque Error contained within signature::error::Error"
114114
),
115115
_ => false,
116116
},
@@ -205,6 +205,16 @@ where
205205
/// validity, this function offers complete control over these parameters.
206206
/// If this level of control is not needed, use [`CertificateAuthority::new`]
207207
/// instead.
208+
//
209+
// SAFETY: We purposefully allow the `clippy::unwrap_in_result` lint below in this function.
210+
// We can use expect here, because the subject name is defined as a constant which must be able
211+
// to be parsed.
212+
//
213+
// FIXME (@Techassi): This attribute can be used on individual unwrap and expect calls since
214+
// Rust 1.91.0. We should move this attribute to not contaminate an unnecessarily large scope
215+
// once we bump the toolchain to 1.91.0.
216+
// See https://github.com/rust-lang/rust-clippy/pull/15445
217+
#[allow(clippy::unwrap_in_result)]
208218
#[instrument(name = "create_certificate_authority_with", skip(signing_key_pair))]
209219
pub fn new_with(signing_key_pair: S, serial_number: u64, validity: Duration) -> Result<Self> {
210220
let serial_number = SerialNumber::from(serial_number);
@@ -214,7 +224,7 @@ where
214224
// created by us should contain the same subject consisting a common set
215225
// of distinguished names (DNs).
216226
let subject = Name::from_str(SDP_ROOT_CA_SUBJECT)
217-
.expect("the SDP_ROOT_CA_SUBJECT must be a valid subject");
227+
.expect("the constant SDP_ROOT_CA_SUBJECT must be a valid subject");
218228

219229
let spki_pem = signing_key_pair
220230
.verifying_key()
@@ -511,7 +521,7 @@ mod tests {
511521

512522
#[tokio::test]
513523
async fn rsa_key_generation() {
514-
let mut ca = CertificateAuthority::new_rsa().unwrap();
524+
let mut ca = CertificateAuthority::new_rsa().expect("must be able to create RSA-based CA");
515525
let cert = ca
516526
.generate_rsa_leaf_certificate("Product", "pod", [TEST_SAN], TEST_CERT_LIFETIME)
517527
.expect(
@@ -523,7 +533,9 @@ mod tests {
523533

524534
#[tokio::test]
525535
async fn ecdsa_key_generation() {
526-
let mut ca = CertificateAuthority::new_ecdsa().unwrap();
536+
let mut ca =
537+
CertificateAuthority::new_ecdsa().expect("must be able to create ECDSA-based CA");
538+
527539
let cert = ca
528540
.generate_ecdsa_leaf_certificate("Product", "pod", [TEST_SAN], TEST_CERT_LIFETIME)
529541
.expect(
@@ -535,11 +547,11 @@ mod tests {
535547

536548
fn assert_cert_attributes(cert: &Certificate) {
537549
let cert = &cert.tbs_certificate;
550+
let expected_subject = Name::from_str("CN=Product Certificate for pod")
551+
.expect("constant subject must be valid");
552+
538553
// Test subject
539-
assert_eq!(
540-
cert.subject,
541-
Name::from_str("CN=Product Certificate for pod").unwrap()
542-
);
554+
assert_eq!(cert.subject, expected_subject);
543555

544556
// Test SAN extension is present
545557
let extensions = cert.extensions.as_ref().expect("cert must have extensions");

crates/stackable-operator-derive/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,6 @@ syn.workspace = true
2121

2222
[dev-dependencies]
2323
stackable-operator = { path = "../stackable-operator" }
24+
25+
[lints]
26+
workspace = true

crates/stackable-operator/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,6 @@ url.workspace = true
5757
[dev-dependencies]
5858
rstest.workspace = true
5959
tempfile.workspace = true
60+
61+
[lints]
62+
workspace = true

crates/stackable-operator/src/logging/k8s_events.rs

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! Utilities for publishing Kubernetes events
22
3-
use std::error::Error;
3+
use std::{error::Error, fmt::Write};
44

55
use kube::runtime::{
66
controller,
@@ -12,25 +12,20 @@ use super::controller::ReconcilerError;
1212
/// Converts an [`Error`] into a publishable Kubernetes [`Event`]
1313
fn error_to_event<E: ReconcilerError>(err: &E) -> Event {
1414
// Walk the whole error chain, so that we get all the full reason for the error
15-
let mut full_msg = {
16-
use std::fmt::Write;
17-
let mut buf = err.to_string();
18-
let mut err: &dyn Error = err;
19-
loop {
20-
err = match err.source() {
21-
Some(err) => {
22-
write!(buf, ": {err}").unwrap();
23-
err
24-
}
25-
None => break buf,
26-
}
27-
}
28-
};
29-
message::truncate_with_ellipsis(&mut full_msg, 1024);
15+
let mut error = err.to_string();
16+
let mut source = err.source();
17+
18+
while let Some(err) = source {
19+
write!(error, ": {err}").expect("must be able to concat errors");
20+
source = err.source();
21+
}
22+
23+
message::truncate_with_ellipsis(&mut error, 1024);
24+
3025
Event {
3126
type_: EventType::Warning,
3227
reason: err.category().to_string(),
33-
note: Some(full_msg),
28+
note: Some(error),
3429
action: "Reconcile".to_string(),
3530
secondary: err.secondary_object().map(|secondary| secondary.into()),
3631
}
@@ -70,8 +65,7 @@ mod message {
7065
pub fn truncate_with_ellipsis(msg: &mut String, max_len: usize) {
7166
const ELLIPSIS: char = '…';
7267
const ELLIPSIS_LEN: usize = ELLIPSIS.len_utf8();
73-
let len = msg.len();
74-
if len > max_len {
68+
if msg.len() > max_len {
7569
let start_of_trunc_char = find_start_of_char(msg, max_len.saturating_sub(ELLIPSIS_LEN));
7670
msg.truncate(start_of_trunc_char);
7771
if ELLIPSIS_LEN <= max_len {

0 commit comments

Comments
 (0)