Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into feat/jvm-arguments
Browse files Browse the repository at this point in the history
  • Loading branch information
sbernauer committed Jan 16, 2025
2 parents 5769f5c + 6f1ef43 commit 9a967fa
Show file tree
Hide file tree
Showing 7 changed files with 230 additions and 14 deletions.
7 changes: 4 additions & 3 deletions crates/stackable-operator/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@ All notable changes to this project will be documented in this file.

### Changed

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

[#931]: https://github.com/stackabletech/operator-rs/pull/931
[#938]: https://github.com/stackabletech/operator-rs/pull/938
[#939]: https://github.com/stackabletech/operator-rs/pull/939

## [0.83.0] - 2024-12-03

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

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

## [0.69.1] 2024-06-10
## [0.69.1] - 2024-06-10

### Added

Expand Down
4 changes: 2 additions & 2 deletions crates/stackable-operator/src/commons/networking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ use crate::validation;
Serialize, Deserialize, Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd, JsonSchema,
)]
#[serde(try_from = "String", into = "String")]
pub struct DomainName(#[validate(regex(path = "validation::RFC_1123_SUBDOMAIN_REGEX"))] String);
pub struct DomainName(#[validate(regex(path = "validation::DOMAIN_REGEX"))] String);

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

fn from_str(value: &str) -> Result<Self, Self::Err> {
validation::is_rfc_1123_subdomain(value)?;
validation::is_domain(value)?;
Ok(DomainName(value.to_owned()))
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/stackable-operator/src/config/fragment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ pub trait FromFragment: Sized {
/// 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
/// [`Fragment`].
type Fragment;
/// A variant of [`Self::Fragment`] that is used when the container already provides a to indicate that a value is optional.
/// A variant of [`Self::Fragment`] that is used when the container already provides a way to indicate that a value is optional.
///
/// For example, there's no use marking a value as [`Option`]al again if the value is already contained in an `Option`.
///
Expand Down
2 changes: 2 additions & 0 deletions crates/stackable-operator/src/config/merge.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//! Automatically merges objects *deeply*, especially fragments.
use k8s_openapi::{
api::core::v1::{NodeAffinity, PodAffinity, PodAntiAffinity, PodTemplateSpec},
apimachinery::pkg::{api::resource::Quantity, apis::meta::v1::LabelSelector},
Expand Down
171 changes: 171 additions & 0 deletions crates/stackable-operator/src/config/mod.rs
Original file line number Diff line number Diff line change
@@ -1,2 +1,173 @@
//! The Stacklet Configuration System™©®️️️ (SCS).
//!
//! # But oh god why is this monstrosity a thing?
//!
//! Products are complicated. They need to be supplied many kinds of configuration.
//! Some of it applies to the whole installation (Stacklet). Some of it applies only to one [role](`Role`).
//! Some of it applies only to a subset of the instances of that role (we call this a [`RoleGroup`]).
//!
//! We (usually) don't know at what level it makes sense to apply a given piece of configuration, but we also
//! don't want to force users to repeat themselves constantly! Instead, we model the configuration as a tree:
//!
//! ```yaml
//! stacklet1:
//! role1:
//! group1:
//! group2:
//! role2:
//! group3:
//! group4:
//! stacklet2:
//! role3:
//! group5:
//! ```
//!
//! Where only the leaves (*groups*) are actually realized into running products, but every level inherits
//! the configuration of its parents. So `group1` would inherit any keys from `role1` (and, transitively, `stacklet1`),
//! unless it overrides them.
//!
//! We also want to *validate* that the configuration actually makes sense, but only once we have the fully realized
//! configuration for a given rolegroup.
//!
//! However, in practice, living in a fully typed land like Rust makes this slightly awkward. We end up having to choose from
//! a few awkward options:
//!
//! 1. Give up on type safety until we're done merging - Type safety is nice, and we still need to produce a schema for
//! Kubernetes to validate against.
//! 2. Give on distinguishing between pre- and post-validation types - Type safety is nice, and it gets error-prone having to memorize
//! which [`Option::unwrap`]s are completely benign, and which are going to bring down the whole cluster. And, uh, good luck trying
//! to *change* that in either direction.
//! 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
//! even counting the validation ([parsing]) and inheritance routines! That's not really stuff you want to get wrong!
//!
//! So far, none of those options look particularly great. 3 would probably be the least unworkable path, but...
//! But then again, uh, we have a compiler. What if we could just make it do the hard work?
//!
//! # Okay, but how does it work?
//!
//! The SCS™©®️️️ is split into two subsystems: [`fragment`] and [`merge`].
//!
//! ## Uhhhh, fragments?
//!
//! The [`Fragment`] macro implements option 3 from above for you. You define the final validated type,
//! and it generates a "Fragment mirror type", where all fields are replaced by [`Option`]al counterparts.
//!
//! For example,
//!
//! ```
//! # use stackable_operator::config::fragment::Fragment;
//! #[derive(Fragment)]
//! struct Foo {
//! bar: String,
//! baz: u8,
//! }
//! ```
//!
//! generates this:
//!
//! ```
//! struct FooFragment {
//! bar: Option<String>,
//! baz: Option<u8>,
//! }
//! ```
//!
//! Additionally, it provides the [`validate`] function, which lets you turn your `FooFragment` back into a `Foo`
//! (while also making sure that the contents actually make sense).
//!
//! Fragments can also be *nested*, as long as the whole hierarchy has fragments. In this case, the fragment of the substruct will be used,
//! instead of wrapping it in an Option. For example, this:
//!
//! ```
//! # use stackable_operator::config::fragment::Fragment;
//! #[derive(Fragment)]
//! struct Foo {
//! bar: Bar,
//! }
//!
//! #[derive(Fragment)]
//! struct Bar {
//! baz: String,
//! }
//! ```
//!
//! generates this:
//!
//! ```
//! struct FooFragment {
//! bar: BarFragment,
//! }
//!
//! struct BarFragment {
//! baz: Option<String>,
//! }
//! ```
//!
//! rather than wrapping `Bar` as an option, like this:
//!
//! ```
//! struct FooFragment {
//! bar: Option<Bar>,
//! }
//!
//! struct Bar {
//! baz: String,
//! }
//! // BarFragment would be irrelevant here
//! ```
//!
//! ### How does it actually know whether to use a subfragment or an [`Option`]?
//!
//! That's (kind of) a trick question! [`Fragment`] actually has no idea about what an [`Option`] even is!
//! It always uses [`FromFragment::Fragment`]. A type can opt into the [`Option`] treatment by implementing
//! [`Atomic`], which is a marker trait for leaf types that cannot be merged any further.
//!
//! ### And what about defaults? That seems like a pretty big oversight.
//!
//! The Fragment system doesn't natively support default values! Instead, this comes "for free" with the merge system (below).
//! One benefit of this is that the same `Fragment` type can support different default values in different contexts
//! (for example: different defaults in different rolegroups).
//!
//! ### Can I customize my `Fragment` types?
//!
//! Attributes can be applied to the generated types using the `#[fragment_attrs]` attribute. For example,
//! `#[fragment_attrs(derive(Default))]` applies `#[derive(Default)]` to the `Fragment` type.
//!
//! ## And what about merging? So far, those fragments seem pretty useless...
//!
//! This is where the [`Merge`] macro (and trait) comes in! It is designed to be applied to the `Fragment` types (see above),
//! and merges their contents field-by-field, deeply (as in: [`merge`] will recurse into substructs, and merge *their* keys in turn).
//!
//! Just like for `Fragment`s, types can opt out of being merged using the [`Atomic`] trait. This is useful both for "primitive" values
//! (like [`String`], the recursion needs to end *somewhere*, after all), and for values that don't really make sense to merge
//! (like a set of search query parameters).
//!
//! # Fine, how do I actually use it, then?
//!
//! For declarations (in CRDs):
//! - Apply `#[derive(Fragment)] #[fragment_attrs(derive(Merge))]` for your product configuration (and any of its nested types).
//! - DON'T: `#[derive(Fragment, Merge)]`
//! - Pretty much always derive deserialization and defaulting on the `Fragment`, not the validated type:
//! - DO: `#[fragment_attrs(derive(Serialize, Deserialize, Default, JsonSchema))]`
//! - DON'T: `#[derive(Fragment, Serialize, Deserialize, Default, JsonSchema)]`
//! - Refer to the `Fragment` type in CRDs, not the validated type.
//! - Implementing [`Atomic`] if something doesn't make sense to merge.
//! - Define the "validated form" of your configuration: only make fields [`Option`]al if [`None`] is actually a legal value.
//!
//! For runtime code:
//! - Validate and merge with [`RoleGroup::validate_config`] for CRDs, otherwise [`merge`] manually and then validate with [`validate`].
//! - Validate as soon as possible, user code should never read the contents of `Fragment`s.
//! - Defaults are just another layer to be [`merge`]d.
//!
//! [parsing]: https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-validate/
//! [`merge`]: Merge::merge
pub mod fragment;
pub mod merge;

#[cfg(doc)]
use crate::role_utils::{Role, RoleGroup};
#[cfg(doc)]
use fragment::{validate, Fragment, FromFragment};
#[cfg(doc)]
use merge::{Atomic, Merge};
13 changes: 10 additions & 3 deletions crates/stackable-operator/src/utils/cluster_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,22 @@ use std::str::FromStr;

use crate::commons::networking::DomainName;

const KUBERNETES_CLUSTER_DOMAIN_DEFAULT: &str = "cluster.local";
const KUBERNETES_CLUSTER_DOMAIN_DEFAULT: &str = "cluster.local.";

/// Some information that we know about the Kubernetes cluster.
#[derive(Debug, Clone)]
pub struct KubernetesClusterInfo {
/// The Kubernetes cluster domain, typically `cluster.local`.
/// The Kubernetes cluster domain, typically `cluster.local.`.
pub cluster_domain: DomainName,
}

#[derive(clap::Parser, Debug, Default, PartialEq, Eq)]
pub struct KubernetesClusterInfoOpts {
/// Kubernetes cluster domain, usually this is `cluster.local`.
/// Kubernetes cluster domain, usually this is `cluster.local.`.
///
/// Please note that we recommend adding a trailing dot (".") to reduce DNS requests, see
/// <https://github.com/stackabletech/issues/issues/656> for details.
//
// We are not using a default value here, as operators will probably do an more advanced
// auto-detection of the cluster domain in case it is not specified in the future.
#[arg(long, env)]
Expand All @@ -25,6 +29,9 @@ impl KubernetesClusterInfo {
let cluster_domain = match &cluster_info_opts.kubernetes_cluster_domain {
Some(cluster_domain) => {
tracing::info!(%cluster_domain, "Using configured Kubernetes cluster domain");
if !cluster_domain.ends_with('.') {
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");
}

cluster_domain.clone()
}
Expand Down
45 changes: 40 additions & 5 deletions crates/stackable-operator/src/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,23 @@ use const_format::concatcp;
use regex::Regex;
use snafu::Snafu;

/// Minimal length required by RFC 1123 is 63. Up to 255 allowed, unsupported by k8s.
const RFC_1123_LABEL_MAX_LENGTH: usize = 63;
// FIXME: According to https://www.rfc-editor.org/rfc/rfc1035#section-2.3.1 domain names must start with a letter
// (and not a number).
const RFC_1123_LABEL_FMT: &str = "[a-z0-9]([-a-z0-9]*[a-z0-9])?";
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";

/// This is a subdomain's max length in DNS (RFC 1123)
const RFC_1123_SUBDOMAIN_MAX_LENGTH: usize = 253;
const RFC_1123_SUBDOMAIN_FMT: &str =
concatcp!(RFC_1123_LABEL_FMT, "(\\.", RFC_1123_LABEL_FMT, ")*");
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";
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";

// This is a subdomain's max length in DNS (RFC 1123)
const RFC_1123_SUBDOMAIN_MAX_LENGTH: usize = 253;
// Minimal length required by RFC 1123 is 63. Up to 255 allowed, unsupported by k8s.
const RFC_1123_LABEL_MAX_LENGTH: usize = 63;
const DOMAIN_MAX_LENGTH: usize = RFC_1123_SUBDOMAIN_MAX_LENGTH;
/// Same as [`RFC_1123_SUBDOMAIN_FMT`], but allows a trailing dot
const DOMAIN_FMT: &str = concatcp!(RFC_1123_SUBDOMAIN_FMT, "\\.?");
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 '.'";

const RFC_1035_LABEL_FMT: &str = "[a-z]([-a-z0-9]*[a-z0-9])?";
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";
Expand All @@ -46,6 +51,10 @@ const KERBEROS_REALM_NAME_ERROR_MSG: &str =
"Kerberos realm name must only contain alphanumeric characters, '-', and '.'";

// Lazily initialized regular expressions
pub(crate) static DOMAIN_REGEX: LazyLock<Regex> = LazyLock::new(|| {
Regex::new(&format!("^{DOMAIN_FMT}$")).expect("failed to compile domain regex")
});

pub(crate) static RFC_1123_SUBDOMAIN_REGEX: LazyLock<Regex> = LazyLock::new(|| {
Regex::new(&format!("^{RFC_1123_SUBDOMAIN_FMT}$"))
.expect("failed to compile RFC 1123 subdomain regex")
Expand Down Expand Up @@ -178,6 +187,23 @@ fn validate_all(validations: impl IntoIterator<Item = Result<(), Error>>) -> Res
}
}

pub fn is_domain(value: &str) -> Result {
validate_all([
validate_str_length(value, DOMAIN_MAX_LENGTH),
validate_str_regex(
value,
&DOMAIN_REGEX,
DOMAIN_ERROR_MSG,
&[
"example.com",
"example.com.",
"cluster.local",
"cluster.local.",
],
),
])
}

/// Tests for a string that conforms to the definition of a subdomain in DNS (RFC 1123).
pub fn is_rfc_1123_subdomain(value: &str) -> Result {
validate_all([
Expand Down Expand Up @@ -394,6 +420,15 @@ mod tests {
#[case(&"a".repeat(253))]
fn is_rfc_1123_subdomain_pass(#[case] value: &str) {
assert!(is_rfc_1123_subdomain(value).is_ok());
// Every valid RFC1123 is also a valid domain
assert!(is_domain(value).is_ok());
}

#[rstest]
#[case("cluster.local")]
#[case("cluster.local.")]
fn is_domain_pass(#[case] value: &str) {
assert!(is_domain(value).is_ok());
}

#[test]
Expand Down

0 comments on commit 9a967fa

Please sign in to comment.