Skip to content

Commit 5cf02b5

Browse files
maltesanderTechassiNickLarsenNZsbernauer
authored
Prepare shared code: Support running on non default dns settings (#893)
* wip * rework - wip * docs & clippy fix * fix tests * adapted changelog * fix changelog entry * add missing log message * improve logging messages * mark changelog entry as breaking * fix whitespaces * fix tests * refactor: Adjust cluster domain parsing code There is still some bits and pieces we want to change, mostly regarding the error type / error handling. This commit also moves the input fixtures for testing the resolv parser into dedicated files and utilizies rstest's #[files] attribute to generate tests based on these files. Co-authored-by: Nick Larsen <nick.larsen@stackable.tech> * chore: Adjust error variants * chore: Update dev comment * chore: Adjust doc comment * chore: Adjust changelog * chore: Add no_run to example * Update crates/stackable-operator/src/utils/cluster_domain.rs Co-authored-by: Techassi <git@techassi.dev> * improve tracing messages * add note * replace expect with error handling * clippy * refactor: Adjust tracing * Apply suggestions from code review Co-authored-by: Sebastian Bernauer <sebastian.bernauer@stackable.de> * Remove re-export * fix docs * chore: Apply suggestions from code review Co-authored-by: Malte Sander <malte.sander.it@gmail.com> Co-authored-by: Techassi <sascha.lautenschlaeger@stackable.tech> * chore: Apply suggestions from code review Co-authored-by: Malte Sander <malte.sander.it@gmail.com> Co-authored-by: Techassi <sascha.lautenschlaeger@stackable.tech> --------- Co-authored-by: Techassi <sascha.lautenschlaeger@stackable.tech> Co-authored-by: Nick Larsen <nick.larsen@stackable.tech> Co-authored-by: Techassi <git@techassi.dev> Co-authored-by: Sebastian Bernauer <sebastian.bernauer@stackable.tech> Co-authored-by: Sebastian Bernauer <sebastian.bernauer@stackable.de>
1 parent 82a0ef6 commit 5cf02b5

File tree

10 files changed

+233
-7
lines changed

10 files changed

+233
-7
lines changed

.pre-commit-config.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,5 +51,5 @@ repos:
5151
name: .scripts/verify-crate-versions
5252
language: system
5353
entry: .scripts/verify_crate_versions.sh
54-
stages: [commit, merge-commit, manual]
54+
stages: [pre-commit, pre-merge-commit, manual]
5555
pass_filenames: false

crates/stackable-operator/CHANGELOG.md

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,26 @@ All notable changes to this project will be documented in this file.
88

99
- Re-export the `YamlSchema` trait and the `stackable-shared` crate as the `shared` module ([#883]).
1010
- BREAKING: Added `preferredAddressType` field to ListenerClass CRD ([#885]).
11+
- BREAKING: The cluster domain (default: `cluster.local`) can now be configured in the individual
12+
operators via the ENV variable `KUBERNETES_CLUSTER_DOMAIN` or resolved automatically by parsing
13+
the `/etc/resolve.conf` file. This requires using `initialize_operator` instead of `create_client`
14+
in the `main.rs` of the individual operators ([#893]).
1115

1216
### Changed
1317

1418
- BREAKING: The `CustomResourceExt` trait is now re-exported from the `stackable-shared` crate. The
1519
trait functions use the same parameters but return a different error type ([#883]).
16-
- BREAKING: `KeyValuePairs` (as well as `Labels`/`Annotations` via it) is now backed by a `BTreeMap` rather than a `BTreeSet` ([#888]).
20+
- BREAKING: `KeyValuePairs` (as well as `Labels`/`Annotations` via it) is now backed by a `BTreeMap`
21+
rather than a `BTreeSet` ([#888]).
1722
- The `Deref` impl now returns a `BTreeMap` instead.
1823
- `iter()` now clones the values.
1924

2025
### Fixed
2126

22-
- BREAKING: `KeyValuePairs::insert` (as well as `Labels::`/`Annotations::` via it) now overwrites the old value if the key already exists ([#888]).
23-
- Previously, `iter()` would return *both* values in lexicographical order (causing further conversions like `Into<BTreeMap>` to prefer the maximum value).
27+
- BREAKING: `KeyValuePairs::insert` (as well as `Labels::`/`Annotations::` via it) now overwrites
28+
the old value if the key already exists. Previously, `iter()` would return *both* values in
29+
lexicographical order (causing further conversions like `Into<BTreeMap>` to prefer the maximum
30+
value) ([#888]).
2431

2532
### Removed
2633

@@ -31,6 +38,7 @@ All notable changes to this project will be documented in this file.
3138
[#883]: https://github.com/stackabletech/operator-rs/pull/883
3239
[#885]: https://github.com/stackabletech/operator-rs/pull/885
3340
[#888]: https://github.com/stackabletech/operator-rs/pull/888
41+
[#893]: https://github.com/stackabletech/operator-rs/pull/893
3442

3543
## [0.78.0] - 2024-09-30
3644

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
nameserver 10.243.21.53
2+
options ndots:5
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
search baz svc.foo.bar foo.bar
2+
search sble-operators.svc.cluster.local svc.cluster.local cluster.local
3+
nameserver 10.243.21.53
4+
options ndots:5
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
search sble-operators.svc.cluster.local svc.cluster.local cluster.local
2+
nameserver 10.243.21.53
3+
options ndots:5
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
search openshift-service-ca-operator.svc.cluster.local svc.cluster.local cluster.local cmx.repl-openshift.build
2+
nameserver 172.30.0.10
3+
options ndots:5

crates/stackable-operator/src/client.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use crate::kvp::LabelSelectorExt;
2+
use crate::utils::cluster_domain::{self, retrieve_cluster_domain, KUBERNETES_CLUSTER_DOMAIN};
23

34
use either::Either;
45
use futures::StreamExt;
@@ -77,6 +78,9 @@ pub enum Error {
7778

7879
#[snafu(display("unable to create kubernetes client"))]
7980
CreateKubeClient { source: kube::Error },
81+
82+
#[snafu(display("unable to to resolve kubernetes cluster domain"))]
83+
ResolveKubernetesClusterDomain { source: cluster_domain::Error },
8084
}
8185

8286
/// This `Client` can be used to access Kubernetes.
@@ -510,12 +514,12 @@ impl Client {
510514
/// use tokio::time::error::Elapsed;
511515
/// use kube::runtime::watcher;
512516
/// use k8s_openapi::api::core::v1::Pod;
513-
/// use stackable_operator::client::{Client, create_client};
517+
/// use stackable_operator::client::{Client, initialize_operator};
514518
///
515519
/// #[tokio::main]
516520
/// async fn main(){
517521
///
518-
/// let client: Client = create_client(None).await.expect("Unable to construct client.");
522+
/// let client: Client = initialize_operator(None).await.expect("Unable to construct client.");
519523
/// let watcher_config: watcher::Config =
520524
/// watcher::Config::default().fields(&format!("metadata.name=nonexistent-pod"));
521525
///
@@ -622,7 +626,13 @@ where
622626
}
623627
}
624628

625-
pub async fn create_client(field_manager: Option<String>) -> Result<Client> {
629+
pub async fn initialize_operator(field_manager: Option<String>) -> Result<Client> {
630+
let _ = KUBERNETES_CLUSTER_DOMAIN
631+
.set(retrieve_cluster_domain().context(ResolveKubernetesClusterDomainSnafu)?);
632+
create_client(field_manager).await
633+
}
634+
635+
async fn create_client(field_manager: Option<String>) -> Result<Client> {
626636
let kubeconfig: Config = kube::Config::infer()
627637
.await
628638
.map_err(kube::Error::InferConfig)

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@ impl TryFrom<String> for DomainName {
3030
}
3131
}
3232

33+
impl TryFrom<&str> for DomainName {
34+
type Error = validation::Errors;
35+
36+
fn try_from(value: &str) -> Result<Self, Self::Error> {
37+
value.parse()
38+
}
39+
}
40+
3341
impl From<DomainName> for String {
3442
fn from(value: DomainName) -> Self {
3543
value.0
Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
use std::{env, path::PathBuf, str::FromStr, sync::OnceLock};
2+
3+
use snafu::{ResultExt, Snafu};
4+
use tracing::instrument;
5+
6+
use crate::commons::networking::DomainName;
7+
8+
const KUBERNETES_CLUSTER_DOMAIN_ENV: &str = "KUBERNETES_CLUSTER_DOMAIN";
9+
const KUBERNETES_SERVICE_HOST_ENV: &str = "KUBERNETES_SERVICE_HOST";
10+
11+
const KUBERNETES_CLUSTER_DOMAIN_DEFAULT: &str = "cluster.local";
12+
const RESOLVE_CONF_FILE_PATH: &str = "/etc/resolv.conf";
13+
14+
#[derive(Debug, Snafu)]
15+
pub enum Error {
16+
#[snafu(display("failed to read resolv config from {RESOLVE_CONF_FILE_PATH:?}"))]
17+
ReadResolvConfFile { source: std::io::Error },
18+
19+
#[snafu(display("failed to parse {cluster_domain:?} as domain name"))]
20+
ParseDomainName {
21+
source: crate::validation::Errors,
22+
cluster_domain: String,
23+
},
24+
25+
#[snafu(display(r#"unable to find "search" entry"#))]
26+
NoSearchEntry,
27+
28+
#[snafu(display(r#"unable to find unambiguous domain in "search" entry"#))]
29+
AmbiguousDomainEntries,
30+
}
31+
32+
/// Tries to retrieve the Kubernetes cluster domain.
33+
///
34+
/// 1. Return `KUBERNETES_CLUSTER_DOMAIN` if set, otherwise
35+
/// 2. Return the cluster domain parsed from the `/etc/resolv.conf` file if `KUBERNETES_SERVICE_HOST`
36+
/// is set, otherwise fall back to `cluster.local`.
37+
///
38+
/// This variable is initialized in [`crate::client::initialize_operator`], which is called in the
39+
/// main function. It can be used as suggested below.
40+
///
41+
/// ## Usage
42+
///
43+
/// ```no_run
44+
/// use stackable_operator::utils::cluster_domain::KUBERNETES_CLUSTER_DOMAIN;
45+
///
46+
/// let kubernetes_cluster_domain = KUBERNETES_CLUSTER_DOMAIN.get()
47+
/// .expect("KUBERNETES_CLUSTER_DOMAIN must first be set by calling initialize_operator");
48+
///
49+
/// tracing::info!(%kubernetes_cluster_domain, "Found cluster domain");
50+
/// ```
51+
///
52+
/// ## See
53+
///
54+
/// - <https://github.com/stackabletech/issues/issues/436>
55+
/// - <https://kubernetes.io/docs/concepts/services-networking/dns-pod-service/>
56+
pub static KUBERNETES_CLUSTER_DOMAIN: OnceLock<DomainName> = OnceLock::new();
57+
58+
#[instrument]
59+
pub(crate) fn retrieve_cluster_domain() -> Result<DomainName, Error> {
60+
// 1. Read KUBERNETES_CLUSTER_DOMAIN env var
61+
tracing::debug!("Trying to determine the Kubernetes cluster domain...");
62+
63+
match env::var(KUBERNETES_CLUSTER_DOMAIN_ENV) {
64+
Ok(cluster_domain) if !cluster_domain.is_empty() => {
65+
let cluster_domain = DomainName::from_str(&cluster_domain)
66+
.context(ParseDomainNameSnafu { cluster_domain })?;
67+
tracing::info!(
68+
%cluster_domain,
69+
"Using Kubernetes cluster domain from {KUBERNETES_CLUSTER_DOMAIN_ENV:?} environment variable"
70+
);
71+
return Ok(cluster_domain);
72+
}
73+
_ => {}
74+
};
75+
76+
// 2. If no env var is set, check if we run in a clustered (Kubernetes/Openshift) environment
77+
// by checking if KUBERNETES_SERVICE_HOST is set: If not default to 'cluster.local'.
78+
tracing::debug!(
79+
"Trying to determine the operator runtime environment as environment variable \
80+
{KUBERNETES_CLUSTER_DOMAIN_ENV:?} is not set"
81+
);
82+
83+
match env::var(KUBERNETES_SERVICE_HOST_ENV) {
84+
Ok(_) => {
85+
let cluster_domain = retrieve_cluster_domain_from_resolv_conf(RESOLVE_CONF_FILE_PATH)?;
86+
let cluster_domain = DomainName::from_str(&cluster_domain)
87+
.context(ParseDomainNameSnafu { cluster_domain })?;
88+
89+
tracing::info!(
90+
%cluster_domain,
91+
"Using Kubernetes cluster domain from {RESOLVE_CONF_FILE_PATH:?} file"
92+
);
93+
94+
Ok(cluster_domain)
95+
}
96+
Err(_) => {
97+
let cluster_domain = DomainName::from_str(KUBERNETES_CLUSTER_DOMAIN_DEFAULT)
98+
.expect("KUBERNETES_CLUSTER_DOMAIN_DEFAULT constant must a valid domain");
99+
100+
tracing::info!(
101+
%cluster_domain,
102+
"Could not determine Kubernetes cluster domain as the operator is not running within Kubernetes, assuming default Kubernetes cluster domain"
103+
);
104+
105+
Ok(cluster_domain)
106+
}
107+
}
108+
}
109+
110+
#[instrument]
111+
fn retrieve_cluster_domain_from_resolv_conf(
112+
path: impl Into<PathBuf> + std::fmt::Debug,
113+
) -> Result<String, Error> {
114+
let path = path.into();
115+
let content = std::fs::read_to_string(&path)
116+
.inspect_err(|error| {
117+
tracing::error!(%error, path = %path.display(), "Cannot read resolv conf");
118+
})
119+
.context(ReadResolvConfFileSnafu)?;
120+
121+
// If there are multiple search directives, only the search
122+
// man 5 resolv.conf
123+
let Some(last_search_entry) = content
124+
.lines()
125+
.rev()
126+
.map(|l| l.trim())
127+
.find(|&l| l.starts_with("search"))
128+
.map(|l| l.trim_start_matches("search").trim())
129+
else {
130+
return NoSearchEntrySnafu.fail();
131+
};
132+
133+
let Some(shortest_entry) = last_search_entry
134+
.split_ascii_whitespace()
135+
.min_by_key(|item| item.len())
136+
else {
137+
return AmbiguousDomainEntriesSnafu.fail();
138+
};
139+
140+
// NOTE (@Techassi): This is really sad and bothers me more than I would like to admit. This
141+
// clone could be removed by using the code directly in the calling function. But that would
142+
// remove the possibility to easily test the parsing.
143+
Ok(shortest_entry.to_owned())
144+
}
145+
146+
#[cfg(test)]
147+
mod tests {
148+
use std::path::PathBuf;
149+
150+
use super::*;
151+
use rstest::rstest;
152+
153+
#[test]
154+
fn use_different_kubernetes_cluster_domain_value() {
155+
let cluster_domain = "my-cluster.local".to_string();
156+
157+
// set different domain via env var
158+
unsafe {
159+
env::set_var(KUBERNETES_CLUSTER_DOMAIN_ENV, &cluster_domain);
160+
}
161+
162+
// initialize the lock
163+
let _ = KUBERNETES_CLUSTER_DOMAIN.set(retrieve_cluster_domain().unwrap());
164+
165+
assert_eq!(
166+
cluster_domain,
167+
KUBERNETES_CLUSTER_DOMAIN.get().unwrap().to_string()
168+
);
169+
}
170+
171+
#[rstest]
172+
fn parse_resolv_conf_pass(
173+
#[files("fixtures/cluster_domain/pass/*.resolv.conf")] path: PathBuf,
174+
) {
175+
assert_eq!(
176+
retrieve_cluster_domain_from_resolv_conf(path).unwrap(),
177+
KUBERNETES_CLUSTER_DOMAIN_DEFAULT
178+
);
179+
}
180+
181+
#[rstest]
182+
fn parse_resolv_conf_fail(
183+
#[files("fixtures/cluster_domain/fail/*.resolv.conf")] path: PathBuf,
184+
) {
185+
assert!(retrieve_cluster_domain_from_resolv_conf(path).is_err());
186+
}
187+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
pub mod bash;
2+
pub mod cluster_domain;
23
pub mod crds;
34
pub mod logging;
45
mod option;

0 commit comments

Comments
 (0)