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

* fix: Avllow trailing dots in DomainName

* Add trailing dot to KubernetesClusterInfo::cluster_domain

* Update crates/stackable-operator/src/validation.rs

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

Co-authored-by: Malte Sander <[email protected]>

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

Co-authored-by: Malte Sander <[email protected]>

* chore: update CHANGELOG to reflect breaking changes in cluster domain handling

* test: add additional test cases for FQDN validation

* chore: fixed changelog entry

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

Co-authored-by: Natalie Klestrup Röijezon <[email protected]>

* docs: be more explicit about the change in the CHANGELOG

* fix: renamed some variables and functions, since we validate domains, not just FQDNs

---------

Co-authored-by: Sebastian Bernauer <[email protected]>
Co-authored-by: Sebastian Bernauer <[email protected]>
Co-authored-by: Malte Sander <[email protected]>
Co-authored-by: Natalie Klestrup Röijezon <[email protected]>

Author: 4 people

crates/stackable-operator/CHANGELOG.md

@@ -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 to make it a FQDN and allow FQDNs when validating a `DomainName` ([#939]).
+
+[#939]: https://github.com/stackabletech/operator-rs/pull/939
 
 ## [0.83.0] - 2024-12-03
 
@@ -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
 

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

@@ -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()))
     }
 }

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

@@ -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)]
@@ -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()
             }

crates/stackable-operator/src/validation.rs

@@ -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";
@@ -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")
@@ -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([
@@ -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]