Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support marking cluster domains as FQDNs, and change the default to FQDN #939

Merged
merged 11 commits into from
Jan 15, 2025
Merged
5 changes: 4 additions & 1 deletion crates/stackable-operator/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ All notable changes to this project will be documented in this file.

- BREAKING: Bump Rust dependencies to enable Kubernetes 1.32 (via `kube` 0.98.0 and `k8s-openapi`
0.23.0) ([#867]).
- BREAKING: Append a dot to the default cluster domain and allow a trailing dot in `DomainName` ([#939]).
dervoeti marked this conversation as resolved.
Show resolved Hide resolved

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

## [0.83.0] - 2024-12-03

Expand Down Expand Up @@ -316,7 +319,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::FQDN_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_fqdn(value)?;
Ok(DomainName(value.to_owned()))
}
}
Expand Down
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 does not end with a dot (\".\"). We recommend to add a trailing dot to reduce DNS requests, see https://github.com/stackabletech/issues/issues/656 for details");
dervoeti marked this conversation as resolved.
Show resolved Hide resolved
}

cluster_domain.clone()
}
Expand Down
44 changes: 39 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 FQDN_MAX_LENGTH: usize = RFC_1123_SUBDOMAIN_MAX_LENGTH;
/// Same as [`RFC_1123_SUBDOMAIN_FMT`], but allows a trailing dot
const FQDN_FMT: &str = concatcp!(RFC_1123_SUBDOMAIN_FMT, "\\.?");
dervoeti marked this conversation as resolved.
Show resolved Hide resolved
const FQDN_ERROR_MSG: &str = "a FQDN 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,9 @@ const KERBEROS_REALM_NAME_ERROR_MSG: &str =
"Kerberos realm name must only contain alphanumeric characters, '-', and '.'";

// Lazily initialized regular expressions
pub(crate) static FQDN_REGEX: LazyLock<Regex> =
LazyLock::new(|| Regex::new(&format!("^{FQDN_FMT}$")).expect("failed to compile FQDN 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 +186,23 @@ fn validate_all(validations: impl IntoIterator<Item = Result<(), Error>>) -> Res
}
}

pub fn is_fqdn(value: &str) -> Result {
validate_all([
validate_str_length(value, FQDN_MAX_LENGTH),
validate_str_regex(
value,
&FQDN_REGEX,
FQDN_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 +419,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 FQDN
assert!(is_fqdn(value).is_ok());
}

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

#[test]
Expand Down
Loading