Skip to content

Commit 557845f

Browse files
committed
refactor!: Add Host type. Fix S3 structs to have mandatory fields
1 parent 04f3b11 commit 557845f

File tree

6 files changed

+181
-68
lines changed

6 files changed

+181
-68
lines changed

crates/stackable-operator/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file.
77
### Added
88

99
- Add `Hostname` and `KerberosRealmName` types extracted from secret-operator ([#851]).
10+
- BREAKING: Add `Host` type and use it within LDAP and OIDC AuthenticationClass as well as S3Connection ([#XXX]).
1011
- Add support for listener volume scopes to `SecretOperatorVolumeSourceBuilder` ([#858]).
1112

1213
### Changed
@@ -15,6 +16,7 @@ All notable changes to this project will be documented in this file.
1516

1617
### Fixed
1718

19+
- BREAKING: The fields on `S3BucketSpec`, `InlinedS3BucketSpec` and `S3ConnectionSpec` are now mandatory (as they should be) ([#XXX]).
1820
- Fix the CRD description of `ClientAuthenticationDetails` to not contain internal Rust doc, but a public CRD description ([#846]).
1921
- `StackableAffinity` fields are no longer erroneously marked as required ([#855]).
2022

crates/stackable-operator/src/commons/authentication/ldap.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use crate::{
1111
tls::{TlsClientDetails, TlsClientDetailsError},
1212
SECRET_BASE_PATH,
1313
},
14+
networking::Host,
1415
secret_class::{SecretClassVolume, SecretClassVolumeError},
1516
},
1617
};
@@ -36,8 +37,8 @@ pub enum Error {
3637
)]
3738
#[serde(rename_all = "camelCase")]
3839
pub struct AuthenticationProvider {
39-
/// Hostname of the LDAP server, for example: `my.ldap.server`.
40-
pub hostname: String,
40+
/// Host of the LDAP server, for example: `my.ldap.server`.
41+
pub hostname: Host,
4142

4243
/// Port of the LDAP server. If TLS is used defaults to 636 otherwise to 389.
4344
port: Option<u16>,

crates/stackable-operator/src/commons/authentication/oidc.rs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ use url::{ParseError, Url};
1111

1212
#[cfg(doc)]
1313
use crate::commons::authentication::AuthenticationClass;
14-
use crate::commons::authentication::{tls::TlsClientDetails, SECRET_BASE_PATH};
14+
use crate::commons::{
15+
authentication::{tls::TlsClientDetails, SECRET_BASE_PATH},
16+
networking::Host,
17+
};
1518

1619
pub type Result<T, E = Error> = std::result::Result<T, E>;
1720

@@ -40,8 +43,8 @@ pub enum Error {
4043
)]
4144
#[serde(rename_all = "camelCase")]
4245
pub struct AuthenticationProvider {
43-
/// Hostname of the identity provider, e.g. `my.keycloak.corp`.
44-
hostname: String,
46+
/// Host of the identity provider, e.g. `my.keycloak.corp`.
47+
hostname: Host,
4548

4649
/// Port of the identity provider. If TLS is used defaults to 443,
4750
/// otherwise to 80.
@@ -90,7 +93,7 @@ fn default_root_path() -> String {
9093

9194
impl AuthenticationProvider {
9295
pub fn new(
93-
hostname: String,
96+
hostname: Host,
9497
port: Option<u16>,
9598
root_path: String,
9699
tls: TlsClientDetails,
@@ -113,8 +116,12 @@ impl AuthenticationProvider {
113116
/// configuration path, use `url.join()`. This module provides the default
114117
/// path at [`DEFAULT_OIDC_WELLKNOWN_PATH`].
115118
pub fn endpoint_url(&self) -> Result<Url> {
116-
let mut url = Url::parse(&format!("http://{}:{}", self.hostname, self.port()))
117-
.context(ParseOidcEndpointUrlSnafu)?;
119+
let mut url = Url::parse(&format!(
120+
"http://{}:{}",
121+
self.hostname.as_url_host(),
122+
self.port()
123+
))
124+
.context(ParseOidcEndpointUrlSnafu)?;
118125

119126
if self.tls.uses_tls() {
120127
url.set_scheme("https").map_err(|_| {
@@ -317,7 +324,7 @@ mod test {
317324
fn test_oidc_ipv6_endpoint_url() {
318325
let oidc = serde_yaml::from_str::<AuthenticationProvider>(
319326
"
320-
hostname: '[2606:2800:220:1:248:1893:25c8:1946]'
327+
hostname: 2606:2800:220:1:248:1893:25c8:1946
321328
rootPath: my-root-path
322329
port: 12345
323330
scopes: [openid]

crates/stackable-operator/src/commons/networking.rs

Lines changed: 122 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,31 @@
1-
use std::{fmt::Display, ops::Deref};
1+
use std::{fmt::Display, net::IpAddr, ops::Deref, str::FromStr};
22

33
use schemars::JsonSchema;
44
use serde::{Deserialize, Serialize};
55

66
use crate::validation;
77

8-
/// A validated hostname type, for use in CRDs.
9-
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
8+
/// A validated hostname type conforming to RFC 1123, e.g. not an IPv6 address.
9+
#[derive(
10+
Serialize, Deserialize, Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd, JsonSchema,
11+
)]
1012
#[serde(try_from = "String", into = "String")]
1113
pub struct Hostname(#[validate(regex(path = "validation::RFC_1123_SUBDOMAIN_REGEX"))] String);
1214

15+
impl FromStr for Hostname {
16+
type Err = validation::Errors;
17+
18+
fn from_str(value: &str) -> Result<Self, Self::Err> {
19+
validation::is_rfc_1123_subdomain(&value)?;
20+
Ok(Hostname(value.to_owned()))
21+
}
22+
}
23+
1324
impl TryFrom<String> for Hostname {
1425
type Error = validation::Errors;
1526

1627
fn try_from(value: String) -> Result<Self, Self::Error> {
17-
validation::is_rfc_1123_subdomain(&value)?;
18-
Ok(Hostname(value))
28+
value.parse()
1929
}
2030
}
2131

@@ -39,6 +49,68 @@ impl Deref for Hostname {
3949
}
4050
}
4151

52+
/// A validated host (either a [`Hostname`] or IP address) type.
53+
#[derive(
54+
Serialize, Deserialize, Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd, JsonSchema,
55+
)]
56+
#[serde(try_from = "String", into = "String")]
57+
pub enum Host {
58+
IpAddress(IpAddr),
59+
Hostname(Hostname),
60+
}
61+
62+
impl FromStr for Host {
63+
type Err = validation::Error;
64+
65+
fn from_str(value: &str) -> Result<Self, Self::Err> {
66+
if let Ok(ip) = value.parse::<IpAddr>() {
67+
return Ok(Host::IpAddress(ip));
68+
}
69+
70+
if let Ok(hostname) = value.parse() {
71+
return Ok(Host::Hostname(hostname));
72+
};
73+
74+
Err(validation::Error::NotAHost {})
75+
}
76+
}
77+
78+
impl TryFrom<String> for Host {
79+
type Error = validation::Error;
80+
81+
fn try_from(value: String) -> Result<Self, Self::Error> {
82+
value.parse()
83+
}
84+
}
85+
86+
impl From<Host> for String {
87+
fn from(value: Host) -> Self {
88+
format!("{value}")
89+
}
90+
}
91+
92+
impl Display for Host {
93+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
94+
match self {
95+
Host::IpAddress(ip) => write!(f, "{ip}"),
96+
Host::Hostname(hostname) => write!(f, "{hostname}"),
97+
}
98+
}
99+
}
100+
101+
impl Host {
102+
/// Formats the host in such a way that it can be used in URLs.
103+
pub fn as_url_host(&self) -> String {
104+
match self {
105+
Host::IpAddress(ip) => match ip {
106+
IpAddr::V4(ip) => ip.to_string(),
107+
IpAddr::V6(ip) => format!("[{ip}]"),
108+
},
109+
Host::Hostname(hostname) => hostname.to_string(),
110+
}
111+
}
112+
}
113+
42114
/// A validated kerberos realm name type, for use in CRDs.
43115
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
44116
#[serde(try_from = "String", into = "String")]
@@ -74,3 +146,48 @@ impl Deref for KerberosRealmName {
74146
&self.0
75147
}
76148
}
149+
150+
#[cfg(test)]
151+
mod tests {
152+
use super::*;
153+
154+
use rstest::rstest;
155+
156+
#[rstest]
157+
#[case("foo")]
158+
#[case("foo.bar")]
159+
// Well this is also a valid hostname I guess
160+
#[case("1.2.3.4")]
161+
fn test_host_and_hostname_parsing_success(#[case] hostname: String) {
162+
let parsed_hostname: Hostname = hostname.parse().expect("hostname can not be parsed");
163+
// Every host is also a valid hostname
164+
let parsed_host: Host = hostname.parse().expect("host can not be parsed");
165+
166+
// Also test the round-trip
167+
assert_eq!(parsed_hostname.to_string(), hostname);
168+
assert_eq!(parsed_host.to_string(), hostname);
169+
}
170+
171+
#[rstest]
172+
#[case("")]
173+
#[case("foo.bar:1234")]
174+
#[case("fe80::1")]
175+
fn test_hostname_parsing_invalid_input(#[case] hostname: &str) {
176+
assert!(hostname.parse::<Hostname>().is_err());
177+
}
178+
179+
#[rstest]
180+
#[case("foo", "foo")]
181+
#[case("foo.bar", "foo.bar")]
182+
#[case("1.2.3.4", "1.2.3.4")]
183+
#[case("fe80::1", "[fe80::1]")]
184+
fn test_host_parsing_success(#[case] host: &str, #[case] expected_url_host: &str) {
185+
// Every host is also a valid hostname
186+
let parsed_host: Host = host.parse().expect("host can not be parsed");
187+
188+
// Also test the round-trip
189+
assert_eq!(parsed_host.to_string(), host);
190+
191+
assert_eq!(parsed_host.as_url_host(), expected_url_host);
192+
}
193+
}

0 commit comments

Comments
 (0)