Skip to content

Commit 9a967fa

Browse files
committed
Merge remote-tracking branch 'origin/main' into feat/jvm-arguments
2 parents 5769f5c + 6f1ef43 commit 9a967fa

File tree

7 files changed

+230
-14
lines changed

7 files changed

+230
-14
lines changed

crates/stackable-operator/CHANGELOG.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,12 @@ All notable changes to this project will be documented in this file.
2020

2121
### Changed
2222

23-
- BREAKING: Bump Rust dependencies to enable Kubernetes 1.32 (via `kube` 0.98.0 and `k8s-openapi`
24-
0.23.0) ([#938]).
23+
- BREAKING: Bump Rust dependencies to enable Kubernetes 1.32 (via `kube` 0.98.0 and `k8s-openapi` 0.23.0) ([#938]).
24+
- BREAKING: Append a dot to the default cluster domain to make it a FQDN and allow FQDNs when validating a `DomainName` ([#939]).
2525

2626
[#931]: https://github.com/stackabletech/operator-rs/pull/931
2727
[#938]: https://github.com/stackabletech/operator-rs/pull/938
28+
[#939]: https://github.com/stackabletech/operator-rs/pull/939
2829

2930
## [0.83.0] - 2024-12-03
3031

@@ -321,7 +322,7 @@ All notable changes to this project will be documented in this file.
321322

322323
[#808]: https://github.com/stackabletech/operator-rs/pull/808
323324

324-
## [0.69.1] 2024-06-10
325+
## [0.69.1] - 2024-06-10
325326

326327
### Added
327328

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,13 @@ use crate::validation;
1111
Serialize, Deserialize, Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd, JsonSchema,
1212
)]
1313
#[serde(try_from = "String", into = "String")]
14-
pub struct DomainName(#[validate(regex(path = "validation::RFC_1123_SUBDOMAIN_REGEX"))] String);
14+
pub struct DomainName(#[validate(regex(path = "validation::DOMAIN_REGEX"))] String);
1515

1616
impl FromStr for DomainName {
1717
type Err = validation::Errors;
1818

1919
fn from_str(value: &str) -> Result<Self, Self::Err> {
20-
validation::is_rfc_1123_subdomain(value)?;
20+
validation::is_domain(value)?;
2121
Ok(DomainName(value.to_owned()))
2222
}
2323
}

crates/stackable-operator/src/config/fragment.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ pub trait FromFragment: Sized {
109109
/// For complex structs, this should be a variant of `Self` where each field is replaced by its respective `Fragment` type. This can be derived using
110110
/// [`Fragment`].
111111
type Fragment;
112-
/// A variant of [`Self::Fragment`] that is used when the container already provides a to indicate that a value is optional.
112+
/// A variant of [`Self::Fragment`] that is used when the container already provides a way to indicate that a value is optional.
113113
///
114114
/// For example, there's no use marking a value as [`Option`]al again if the value is already contained in an `Option`.
115115
///

crates/stackable-operator/src/config/merge.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
//! Automatically merges objects *deeply*, especially fragments.
2+
13
use k8s_openapi::{
24
api::core::v1::{NodeAffinity, PodAffinity, PodAntiAffinity, PodTemplateSpec},
35
apimachinery::pkg::{api::resource::Quantity, apis::meta::v1::LabelSelector},
Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,173 @@
1+
//! The Stacklet Configuration System™©®️️️ (SCS).
2+
//!
3+
//! # But oh god why is this monstrosity a thing?
4+
//!
5+
//! Products are complicated. They need to be supplied many kinds of configuration.
6+
//! Some of it applies to the whole installation (Stacklet). Some of it applies only to one [role](`Role`).
7+
//! Some of it applies only to a subset of the instances of that role (we call this a [`RoleGroup`]).
8+
//!
9+
//! We (usually) don't know at what level it makes sense to apply a given piece of configuration, but we also
10+
//! don't want to force users to repeat themselves constantly! Instead, we model the configuration as a tree:
11+
//!
12+
//! ```yaml
13+
//! stacklet1:
14+
//! role1:
15+
//! group1:
16+
//! group2:
17+
//! role2:
18+
//! group3:
19+
//! group4:
20+
//! stacklet2:
21+
//! role3:
22+
//! group5:
23+
//! ```
24+
//!
25+
//! Where only the leaves (*groups*) are actually realized into running products, but every level inherits
26+
//! the configuration of its parents. So `group1` would inherit any keys from `role1` (and, transitively, `stacklet1`),
27+
//! unless it overrides them.
28+
//!
29+
//! We also want to *validate* that the configuration actually makes sense, but only once we have the fully realized
30+
//! configuration for a given rolegroup.
31+
//!
32+
//! However, in practice, living in a fully typed land like Rust makes this slightly awkward. We end up having to choose from
33+
//! a few awkward options:
34+
//!
35+
//! 1. Give up on type safety until we're done merging - Type safety is nice, and we still need to produce a schema for
36+
//! Kubernetes to validate against.
37+
//! 2. Give on distinguishing between pre- and post-validation types - Type safety is nice, and it gets error-prone having to memorize
38+
//! which [`Option::unwrap`]s are completely benign, and which are going to bring down the whole cluster. And, uh, good luck trying
39+
//! to *change* that in either direction.
40+
//! 3. Write *separate* types for the pre- and post-validation states - That's a lot of tedious code to have to write twice, and that's not
41+
//! even counting the validation ([parsing]) and inheritance routines! That's not really stuff you want to get wrong!
42+
//!
43+
//! So far, none of those options look particularly great. 3 would probably be the least unworkable path, but...
44+
//! But then again, uh, we have a compiler. What if we could just make it do the hard work?
45+
//!
46+
//! # Okay, but how does it work?
47+
//!
48+
//! The SCS™©®️️️ is split into two subsystems: [`fragment`] and [`merge`].
49+
//!
50+
//! ## Uhhhh, fragments?
51+
//!
52+
//! The [`Fragment`] macro implements option 3 from above for you. You define the final validated type,
53+
//! and it generates a "Fragment mirror type", where all fields are replaced by [`Option`]al counterparts.
54+
//!
55+
//! For example,
56+
//!
57+
//! ```
58+
//! # use stackable_operator::config::fragment::Fragment;
59+
//! #[derive(Fragment)]
60+
//! struct Foo {
61+
//! bar: String,
62+
//! baz: u8,
63+
//! }
64+
//! ```
65+
//!
66+
//! generates this:
67+
//!
68+
//! ```
69+
//! struct FooFragment {
70+
//! bar: Option<String>,
71+
//! baz: Option<u8>,
72+
//! }
73+
//! ```
74+
//!
75+
//! Additionally, it provides the [`validate`] function, which lets you turn your `FooFragment` back into a `Foo`
76+
//! (while also making sure that the contents actually make sense).
77+
//!
78+
//! Fragments can also be *nested*, as long as the whole hierarchy has fragments. In this case, the fragment of the substruct will be used,
79+
//! instead of wrapping it in an Option. For example, this:
80+
//!
81+
//! ```
82+
//! # use stackable_operator::config::fragment::Fragment;
83+
//! #[derive(Fragment)]
84+
//! struct Foo {
85+
//! bar: Bar,
86+
//! }
87+
//!
88+
//! #[derive(Fragment)]
89+
//! struct Bar {
90+
//! baz: String,
91+
//! }
92+
//! ```
93+
//!
94+
//! generates this:
95+
//!
96+
//! ```
97+
//! struct FooFragment {
98+
//! bar: BarFragment,
99+
//! }
100+
//!
101+
//! struct BarFragment {
102+
//! baz: Option<String>,
103+
//! }
104+
//! ```
105+
//!
106+
//! rather than wrapping `Bar` as an option, like this:
107+
//!
108+
//! ```
109+
//! struct FooFragment {
110+
//! bar: Option<Bar>,
111+
//! }
112+
//!
113+
//! struct Bar {
114+
//! baz: String,
115+
//! }
116+
//! // BarFragment would be irrelevant here
117+
//! ```
118+
//!
119+
//! ### How does it actually know whether to use a subfragment or an [`Option`]?
120+
//!
121+
//! That's (kind of) a trick question! [`Fragment`] actually has no idea about what an [`Option`] even is!
122+
//! It always uses [`FromFragment::Fragment`]. A type can opt into the [`Option`] treatment by implementing
123+
//! [`Atomic`], which is a marker trait for leaf types that cannot be merged any further.
124+
//!
125+
//! ### And what about defaults? That seems like a pretty big oversight.
126+
//!
127+
//! The Fragment system doesn't natively support default values! Instead, this comes "for free" with the merge system (below).
128+
//! One benefit of this is that the same `Fragment` type can support different default values in different contexts
129+
//! (for example: different defaults in different rolegroups).
130+
//!
131+
//! ### Can I customize my `Fragment` types?
132+
//!
133+
//! Attributes can be applied to the generated types using the `#[fragment_attrs]` attribute. For example,
134+
//! `#[fragment_attrs(derive(Default))]` applies `#[derive(Default)]` to the `Fragment` type.
135+
//!
136+
//! ## And what about merging? So far, those fragments seem pretty useless...
137+
//!
138+
//! This is where the [`Merge`] macro (and trait) comes in! It is designed to be applied to the `Fragment` types (see above),
139+
//! and merges their contents field-by-field, deeply (as in: [`merge`] will recurse into substructs, and merge *their* keys in turn).
140+
//!
141+
//! Just like for `Fragment`s, types can opt out of being merged using the [`Atomic`] trait. This is useful both for "primitive" values
142+
//! (like [`String`], the recursion needs to end *somewhere*, after all), and for values that don't really make sense to merge
143+
//! (like a set of search query parameters).
144+
//!
145+
//! # Fine, how do I actually use it, then?
146+
//!
147+
//! For declarations (in CRDs):
148+
//! - Apply `#[derive(Fragment)] #[fragment_attrs(derive(Merge))]` for your product configuration (and any of its nested types).
149+
//! - DON'T: `#[derive(Fragment, Merge)]`
150+
//! - Pretty much always derive deserialization and defaulting on the `Fragment`, not the validated type:
151+
//! - DO: `#[fragment_attrs(derive(Serialize, Deserialize, Default, JsonSchema))]`
152+
//! - DON'T: `#[derive(Fragment, Serialize, Deserialize, Default, JsonSchema)]`
153+
//! - Refer to the `Fragment` type in CRDs, not the validated type.
154+
//! - Implementing [`Atomic`] if something doesn't make sense to merge.
155+
//! - Define the "validated form" of your configuration: only make fields [`Option`]al if [`None`] is actually a legal value.
156+
//!
157+
//! For runtime code:
158+
//! - Validate and merge with [`RoleGroup::validate_config`] for CRDs, otherwise [`merge`] manually and then validate with [`validate`].
159+
//! - Validate as soon as possible, user code should never read the contents of `Fragment`s.
160+
//! - Defaults are just another layer to be [`merge`]d.
161+
//!
162+
//! [parsing]: https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-validate/
163+
//! [`merge`]: Merge::merge
164+
1165
pub mod fragment;
2166
pub mod merge;
167+
168+
#[cfg(doc)]
169+
use crate::role_utils::{Role, RoleGroup};
170+
#[cfg(doc)]
171+
use fragment::{validate, Fragment, FromFragment};
172+
#[cfg(doc)]
173+
use merge::{Atomic, Merge};

crates/stackable-operator/src/utils/cluster_info.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,22 @@ use std::str::FromStr;
22

33
use crate::commons::networking::DomainName;
44

5-
const KUBERNETES_CLUSTER_DOMAIN_DEFAULT: &str = "cluster.local";
5+
const KUBERNETES_CLUSTER_DOMAIN_DEFAULT: &str = "cluster.local.";
66

77
/// Some information that we know about the Kubernetes cluster.
88
#[derive(Debug, Clone)]
99
pub struct KubernetesClusterInfo {
10-
/// The Kubernetes cluster domain, typically `cluster.local`.
10+
/// The Kubernetes cluster domain, typically `cluster.local.`.
1111
pub cluster_domain: DomainName,
1212
}
1313

1414
#[derive(clap::Parser, Debug, Default, PartialEq, Eq)]
1515
pub struct KubernetesClusterInfoOpts {
16-
/// Kubernetes cluster domain, usually this is `cluster.local`.
16+
/// Kubernetes cluster domain, usually this is `cluster.local.`.
17+
///
18+
/// Please note that we recommend adding a trailing dot (".") to reduce DNS requests, see
19+
/// <https://github.com/stackabletech/issues/issues/656> for details.
20+
//
1721
// We are not using a default value here, as operators will probably do an more advanced
1822
// auto-detection of the cluster domain in case it is not specified in the future.
1923
#[arg(long, env)]
@@ -25,6 +29,9 @@ impl KubernetesClusterInfo {
2529
let cluster_domain = match &cluster_info_opts.kubernetes_cluster_domain {
2630
Some(cluster_domain) => {
2731
tracing::info!(%cluster_domain, "Using configured Kubernetes cluster domain");
32+
if !cluster_domain.ends_with('.') {
33+
tracing::warn!(%cluster_domain, "Your configured Kubernetes cluster domain is not fully qualified (it does not end with a dot (\".\")). We recommend adding a trailing dot to reduce DNS requests, see https://github.com/stackabletech/issues/issues/656 for details");
34+
}
2835

2936
cluster_domain.clone()
3037
}

crates/stackable-operator/src/validation.rs

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,23 @@ use const_format::concatcp;
1515
use regex::Regex;
1616
use snafu::Snafu;
1717

18+
/// Minimal length required by RFC 1123 is 63. Up to 255 allowed, unsupported by k8s.
19+
const RFC_1123_LABEL_MAX_LENGTH: usize = 63;
1820
// FIXME: According to https://www.rfc-editor.org/rfc/rfc1035#section-2.3.1 domain names must start with a letter
1921
// (and not a number).
2022
const RFC_1123_LABEL_FMT: &str = "[a-z0-9]([-a-z0-9]*[a-z0-9])?";
23+
const RFC_1123_LABEL_ERROR_MSG: &str = "a lowercase RFC 1123 label must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character";
24+
25+
/// This is a subdomain's max length in DNS (RFC 1123)
26+
const RFC_1123_SUBDOMAIN_MAX_LENGTH: usize = 253;
2127
const RFC_1123_SUBDOMAIN_FMT: &str =
2228
concatcp!(RFC_1123_LABEL_FMT, "(\\.", RFC_1123_LABEL_FMT, ")*");
2329
const RFC_1123_SUBDOMAIN_ERROR_MSG: &str = "a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character";
24-
const RFC_1123_LABEL_ERROR_MSG: &str = "a lowercase RFC 1123 label must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character";
2530

26-
// This is a subdomain's max length in DNS (RFC 1123)
27-
const RFC_1123_SUBDOMAIN_MAX_LENGTH: usize = 253;
28-
// Minimal length required by RFC 1123 is 63. Up to 255 allowed, unsupported by k8s.
29-
const RFC_1123_LABEL_MAX_LENGTH: usize = 63;
31+
const DOMAIN_MAX_LENGTH: usize = RFC_1123_SUBDOMAIN_MAX_LENGTH;
32+
/// Same as [`RFC_1123_SUBDOMAIN_FMT`], but allows a trailing dot
33+
const DOMAIN_FMT: &str = concatcp!(RFC_1123_SUBDOMAIN_FMT, "\\.?");
34+
const DOMAIN_ERROR_MSG: &str = "a domain must consist of lower case alphanumeric characters, '-' or '.', and must start with an alphanumeric character and end with an alphanumeric character or '.'";
3035

3136
const RFC_1035_LABEL_FMT: &str = "[a-z]([-a-z0-9]*[a-z0-9])?";
3237
const RFC_1035_LABEL_ERROR_MSG: &str = "a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character";
@@ -46,6 +51,10 @@ const KERBEROS_REALM_NAME_ERROR_MSG: &str =
4651
"Kerberos realm name must only contain alphanumeric characters, '-', and '.'";
4752

4853
// Lazily initialized regular expressions
54+
pub(crate) static DOMAIN_REGEX: LazyLock<Regex> = LazyLock::new(|| {
55+
Regex::new(&format!("^{DOMAIN_FMT}$")).expect("failed to compile domain regex")
56+
});
57+
4958
pub(crate) static RFC_1123_SUBDOMAIN_REGEX: LazyLock<Regex> = LazyLock::new(|| {
5059
Regex::new(&format!("^{RFC_1123_SUBDOMAIN_FMT}$"))
5160
.expect("failed to compile RFC 1123 subdomain regex")
@@ -178,6 +187,23 @@ fn validate_all(validations: impl IntoIterator<Item = Result<(), Error>>) -> Res
178187
}
179188
}
180189

190+
pub fn is_domain(value: &str) -> Result {
191+
validate_all([
192+
validate_str_length(value, DOMAIN_MAX_LENGTH),
193+
validate_str_regex(
194+
value,
195+
&DOMAIN_REGEX,
196+
DOMAIN_ERROR_MSG,
197+
&[
198+
"example.com",
199+
"example.com.",
200+
"cluster.local",
201+
"cluster.local.",
202+
],
203+
),
204+
])
205+
}
206+
181207
/// Tests for a string that conforms to the definition of a subdomain in DNS (RFC 1123).
182208
pub fn is_rfc_1123_subdomain(value: &str) -> Result {
183209
validate_all([
@@ -394,6 +420,15 @@ mod tests {
394420
#[case(&"a".repeat(253))]
395421
fn is_rfc_1123_subdomain_pass(#[case] value: &str) {
396422
assert!(is_rfc_1123_subdomain(value).is_ok());
423+
// Every valid RFC1123 is also a valid domain
424+
assert!(is_domain(value).is_ok());
425+
}
426+
427+
#[rstest]
428+
#[case("cluster.local")]
429+
#[case("cluster.local.")]
430+
fn is_domain_pass(#[case] value: &str) {
431+
assert!(is_domain(value).is_ok());
397432
}
398433

399434
#[test]

0 commit comments

Comments
 (0)