From 380ff9b7fbecd701791aa169f14b7795b80b8ec5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Wed, 9 Oct 2024 13:27:13 +0200 Subject: [PATCH 01/10] Make preferred NodePort address type configurable --- Cargo.lock | 20 +++++- Cargo.nix | 63 ++++++++++++++++--- Cargo.toml | 6 +- crate-hashes.json | 5 +- deploy/helm/listener-operator/crds/crds.yaml | 10 +++ rust/operator-binary/src/csi_server/node.rs | 25 +++++++- .../src/listener_controller.rs | 4 +- rust/operator-binary/src/utils.rs | 44 ++++++++----- 8 files changed, 145 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f5fd108d..fe641107 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2324,8 +2324,8 @@ dependencies = [ [[package]] name = "stackable-operator" -version = "0.76.0" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.76.0#a7e70f174fb043a1766e0a80de95834cb4f7513d" +version = "0.78.0" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=feature/listenerclass-preferred-address-type#91bb8d423e877062c1cceaf0c0e5523f9ad521db" dependencies = [ "chrono", "clap", @@ -2335,6 +2335,7 @@ dependencies = [ "dockerfile-parser", "either", "futures", + "indexmap 2.5.0", "json-patch", "k8s-openapi", "kube", @@ -2349,6 +2350,7 @@ dependencies = [ "serde_yaml", "snafu 0.8.4", "stackable-operator-derive", + "stackable-shared", "strum", "tokio", "tracing", @@ -2361,7 +2363,7 @@ dependencies = [ [[package]] name = "stackable-operator-derive" version = "0.3.1" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.76.0#a7e70f174fb043a1766e0a80de95834cb4f7513d" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=feature/listenerclass-preferred-address-type#91bb8d423e877062c1cceaf0c0e5523f9ad521db" dependencies = [ "darling", "proc-macro2", @@ -2369,6 +2371,18 @@ dependencies = [ "syn 2.0.77", ] +[[package]] +name = "stackable-shared" +version = "0.0.1" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=feature/listenerclass-preferred-address-type#91bb8d423e877062c1cceaf0c0e5523f9ad521db" +dependencies = [ + "kube", + "semver", + "serde", + "serde_yaml", + "snafu 0.8.4", +] + [[package]] name = "strsim" version = "0.11.1" diff --git a/Cargo.nix b/Cargo.nix index 17344144..2ea15ea5 100644 --- a/Cargo.nix +++ b/Cargo.nix @@ -7360,13 +7360,13 @@ rec { }; "stackable-operator" = rec { crateName = "stackable-operator"; - version = "0.76.0"; + version = "0.78.0"; edition = "2021"; workspace_member = null; src = pkgs.fetchgit { - url = "https://github.com/stackabletech/operator-rs.git"; - rev = "a7e70f174fb043a1766e0a80de95834cb4f7513d"; - sha256 = "1cyyyn6lizd0wdq79fc9fjnksnzx073ipydxmh7llciq5si5dnq6"; + url = "https://github.com/stackabletech//operator-rs.git"; + rev = "91bb8d423e877062c1cceaf0c0e5523f9ad521db"; + sha256 = "1yr1jgcmwf0msdpxh76124gnkhych27ab1z7wjnbmwcchxw1r20m"; }; libName = "stackable_operator"; authors = [ @@ -7407,6 +7407,10 @@ rec { name = "futures"; packageId = "futures"; } + { + name = "indexmap"; + packageId = "indexmap 2.5.0"; + } { name = "json-patch"; packageId = "json-patch"; @@ -7471,6 +7475,10 @@ rec { name = "stackable-operator-derive"; packageId = "stackable-operator-derive"; } + { + name = "stackable-shared"; + packageId = "stackable-shared"; + } { name = "strum"; packageId = "strum"; @@ -7514,9 +7522,9 @@ rec { edition = "2021"; workspace_member = null; src = pkgs.fetchgit { - url = "https://github.com/stackabletech/operator-rs.git"; - rev = "a7e70f174fb043a1766e0a80de95834cb4f7513d"; - sha256 = "1cyyyn6lizd0wdq79fc9fjnksnzx073ipydxmh7llciq5si5dnq6"; + url = "https://github.com/stackabletech//operator-rs.git"; + rev = "91bb8d423e877062c1cceaf0c0e5523f9ad521db"; + sha256 = "1yr1jgcmwf0msdpxh76124gnkhych27ab1z7wjnbmwcchxw1r20m"; }; procMacro = true; libName = "stackable_operator_derive"; @@ -7542,6 +7550,47 @@ rec { } ]; + }; + "stackable-shared" = rec { + crateName = "stackable-shared"; + version = "0.0.1"; + edition = "2021"; + workspace_member = null; + src = pkgs.fetchgit { + url = "https://github.com/stackabletech//operator-rs.git"; + rev = "91bb8d423e877062c1cceaf0c0e5523f9ad521db"; + sha256 = "1yr1jgcmwf0msdpxh76124gnkhych27ab1z7wjnbmwcchxw1r20m"; + }; + libName = "stackable_shared"; + authors = [ + "Stackable GmbH " + ]; + dependencies = [ + { + name = "kube"; + packageId = "kube"; + usesDefaultFeatures = false; + features = [ "client" "jsonpatch" "runtime" "derive" "rustls-tls" ]; + } + { + name = "semver"; + packageId = "semver"; + } + { + name = "serde"; + packageId = "serde"; + features = [ "derive" ]; + } + { + name = "serde_yaml"; + packageId = "serde_yaml"; + } + { + name = "snafu"; + packageId = "snafu 0.8.4"; + } + ]; + }; "strsim" = rec { crateName = "strsim"; diff --git a/Cargo.toml b/Cargo.toml index 29bc2bf6..e064a7d0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,7 +21,7 @@ prost = "0.13" prost-types = "0.13" serde = "1.0" snafu = "0.8" -stackable-operator = { git = "https://github.com/stackabletech/operator-rs.git", tag = "stackable-operator-0.76.0" } +stackable-operator = { git = "https://github.com/stackabletech/operator-rs.git", tag = "stackable-operator-0.78.0" } strum = { version = "0.26", features = ["derive"] } socket2 = { version = "0.5", features = ["all"] } tokio = { version = "1.40", features = ["full"] } @@ -31,5 +31,7 @@ tonic-build = "0.12" tonic-reflection = "0.12" tracing = "0.1.40" -# [patch."https://github.com/stackabletech/operator-rs.git"] +[patch."https://github.com/stackabletech/operator-rs.git"] +# stackable-operator = { path = "../operator-rs/crates/stackable-operator" } # stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" } +stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "feature/listenerclass-preferred-address-type" } diff --git a/crate-hashes.json b/crate-hashes.json index 4ac79c7f..91bbbea6 100644 --- a/crate-hashes.json +++ b/crate-hashes.json @@ -1,5 +1,6 @@ { - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.76.0#stackable-operator-derive@0.3.1": "1cyyyn6lizd0wdq79fc9fjnksnzx073ipydxmh7llciq5si5dnq6", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.76.0#stackable-operator@0.76.0": "1cyyyn6lizd0wdq79fc9fjnksnzx073ipydxmh7llciq5si5dnq6", + "git+https://github.com/stackabletech//operator-rs.git?branch=feature%2Flistenerclass-preferred-address-type#stackable-operator-derive@0.3.1": "1yr1jgcmwf0msdpxh76124gnkhych27ab1z7wjnbmwcchxw1r20m", + "git+https://github.com/stackabletech//operator-rs.git?branch=feature%2Flistenerclass-preferred-address-type#stackable-operator@0.78.0": "1yr1jgcmwf0msdpxh76124gnkhych27ab1z7wjnbmwcchxw1r20m", + "git+https://github.com/stackabletech//operator-rs.git?branch=feature%2Flistenerclass-preferred-address-type#stackable-shared@0.0.1": "1yr1jgcmwf0msdpxh76124gnkhych27ab1z7wjnbmwcchxw1r20m", "git+https://github.com/stackabletech/product-config.git?tag=0.7.0#product-config@0.7.0": "0gjsm80g6r75pm3824dcyiz4ysq1ka4c1if6k1mjm9cnd5ym0gny" } \ No newline at end of file diff --git a/deploy/helm/listener-operator/crds/crds.yaml b/deploy/helm/listener-operator/crds/crds.yaml index c78a9200..7e0c7eac 100644 --- a/deploy/helm/listener-operator/crds/crds.yaml +++ b/deploy/helm/listener-operator/crds/crds.yaml @@ -24,6 +24,16 @@ spec: spec: description: Defines a policy for how [Listeners](https://docs.stackable.tech/home/nightly/listener-operator/listener) should be exposed. Read the [ListenerClass documentation](https://docs.stackable.tech/home/nightly/listener-operator/listenerclass) for more information. properties: + preferredAddressType: + default: Hostname + description: |- + Whether addresses should prefer using the IP address (`IP`) or the hostname (`Hostname`). + + The other type will be used if the preferred type is not available. + enum: + - Hostname + - IP + type: string serviceAnnotations: additionalProperties: type: string diff --git a/rust/operator-binary/src/csi_server/node.rs b/rust/operator-binary/src/csi_server/node.rs index 5167ab27..b67c3be5 100644 --- a/rust/operator-binary/src/csi_server/node.rs +++ b/rust/operator-binary/src/csi_server/node.rs @@ -4,8 +4,8 @@ use snafu::{OptionExt, ResultExt, Snafu}; use stackable_operator::{ builder::meta::OwnerReferenceBuilder, commons::listener::{ - Listener, ListenerIngress, ListenerPort, ListenerSpec, PodListener, PodListenerScope, - PodListeners, PodListenersSpec, + Listener, ListenerClass, ListenerIngress, ListenerPort, ListenerSpec, PodListener, + PodListenerScope, PodListeners, PodListenersSpec, }, k8s_openapi::api::core::v1::{Node, PersistentVolume, PersistentVolumeClaim, Pod, Volume}, kube::{ @@ -72,6 +72,9 @@ enum PublishVolumeError { listener: ObjectRef, }, + #[snafu(display("{listener} has no associated ListenerClass"))] + ListenerHasNoClass { listener: ObjectRef }, + #[snafu(display("{pod} has not been scheduled to a node yet"))] PodHasNoNode { pod: ObjectRef }, @@ -131,6 +134,7 @@ impl From for Status { PublishVolumeError::PodHasNoNode { .. } => Status::unavailable(full_msg), PublishVolumeError::ListenerPvReference { .. } => Status::failed_precondition(full_msg), PublishVolumeError::ListenerPodSelector { .. } => Status::failed_precondition(full_msg), + PublishVolumeError::ListenerHasNoClass { .. } => Status::failed_precondition(full_msg), PublishVolumeError::BuildListenerOwnerRef { .. } => Status::unavailable(full_msg), PublishVolumeError::ApplyListener { .. } => Status::unavailable(full_msg), PublishVolumeError::AddListenerLabelToPv { .. } => Status::unavailable(full_msg), @@ -442,10 +446,25 @@ async fn local_listener_addresses_for_pod( .get::(node_name, &()) .await .with_context(|_| GetObjectSnafu { - obj: { ObjectRef::::new(node_name).erase() }, + obj: ObjectRef::::new(node_name).erase(), + })?; + let listener_class_name = + listener + .spec + .class_name + .as_deref() + .with_context(|| ListenerHasNoClassSnafu { + listener: ObjectRef::from_obj(listener), + })?; + let listener_class = client + .get::(listener_class_name, &()) + .await + .with_context(|_| GetObjectSnafu { + obj: ObjectRef::::new(listener_class_name).erase(), })?; Ok(node_primary_address(&node) + .pick(listener_class.spec.preferred_address_type) .map(|(address, address_type)| ListenerIngress { // nodes: Some(vec![node_name.to_string()]), address: address.to_string(), diff --git a/rust/operator-binary/src/listener_controller.rs b/rust/operator-binary/src/listener_controller.rs index 43b36398..285b985c 100644 --- a/rust/operator-binary/src/listener_controller.rs +++ b/rust/operator-binary/src/listener_controller.rs @@ -286,7 +286,9 @@ pub async fn reconcile(listener: Arc, ctx: Arc) -> Result>(); ports = svc .spec diff --git a/rust/operator-binary/src/utils.rs b/rust/operator-binary/src/utils.rs index fd30c83e..58da2c32 100644 --- a/rust/operator-binary/src/utils.rs +++ b/rust/operator-binary/src/utils.rs @@ -101,26 +101,42 @@ pub fn error_full_message(err: &dyn std::error::Error) -> String { full_msg } +#[derive(Debug, Clone, Copy)] +pub struct AddressCandidates<'a> { + pub ip: Option<&'a str>, + pub hostname: Option<&'a str>, +} + +impl<'a> AddressCandidates<'a> { + pub fn pick(&self, preferred_address_type: AddressType) -> Option<(&'a str, AddressType)> { + let ip = self.ip.zip(Some(AddressType::Ip)); + let hostname = self.hostname.zip(Some(AddressType::Hostname)); + match preferred_address_type { + AddressType::Ip => ip.or(hostname), + AddressType::Hostname => hostname.or(ip), + } + } +} + /// Try to guess the primary address of a Node, which it is expected that external clients should be able to reach it on -pub fn node_primary_address(node: &Node) -> Option<(&str, AddressType)> { +pub fn node_primary_address(node: &Node) -> AddressCandidates { let addrs = node .status .as_ref() .and_then(|s| s.addresses.as_deref()) .unwrap_or_default(); - // IP addresses are currently preferred over hostnames since nodes don't always have resolvable hostnames - addrs - .iter() - .find(|addr| addr.type_ == "ExternalIP") - .or_else(|| addrs.iter().find(|addr| addr.type_ == "InternalIP")) - .zip(Some(AddressType::Ip)) - .or_else(|| { - addrs - .iter() - .find(|addr| addr.type_ == "Hostname") - .zip(Some(AddressType::Hostname)) - }) - .map(|(addr, ty)| (addr.address.as_str(), ty)) + + AddressCandidates { + ip: addrs + .iter() + .find(|addr| addr.type_ == "ExternalIP") + .or_else(|| addrs.iter().find(|addr| addr.type_ == "InternalIP")) + .map(|addr| addr.address.as_str()), + hostname: addrs + .iter() + .find(|addr| addr.type_ == "Hostname") + .map(|addr| addr.address.as_str()), + } } #[cfg(test)] From 63e911b9795e1041b4390e3d20ea96f000f5eaf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Wed, 9 Oct 2024 13:53:55 +0200 Subject: [PATCH 02/10] Respect preferred address type for LoadBalancer and ClusterIP services too --- .../src/listener_controller.rs | 36 ++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/rust/operator-binary/src/listener_controller.rs b/rust/operator-binary/src/listener_controller.rs index 285b985c..93cddf89 100644 --- a/rust/operator-binary/src/listener_controller.rs +++ b/rust/operator-binary/src/listener_controller.rs @@ -28,7 +28,10 @@ use stackable_operator::{ }; use strum::IntoStaticStr; -use crate::{csi_server::node::NODE_TOPOLOGY_LABEL_HOSTNAME, utils::node_primary_address}; +use crate::{ + csi_server::node::NODE_TOPOLOGY_LABEL_HOSTNAME, + utils::{node_primary_address, AddressCandidates}, +}; #[cfg(doc)] use stackable_operator::k8s_openapi::api::core::v1::Pod; @@ -269,6 +272,7 @@ pub async fn reconcile(listener: Arc, ctx: Arc) -> Result; + let kubernetes_service_fqdn: String; let addresses: Vec<(&str, AddressType)>; let ports: BTreeMap; match listener_class.spec.service_type { @@ -306,11 +310,11 @@ pub async fn reconcile(listener: Arc, ctx: Arc) -> Result, ctx: Arc) -> Result { - addresses = svc - .spec - .iter() - .flat_map(|s| &s.cluster_ips) - .flatten() - .map(|addr| (&**addr, AddressType::Ip)) - .collect::>(); + addresses = match listener_class.spec.preferred_address_type { + AddressType::Ip => svc + .spec + .iter() + .flat_map(|s| &s.cluster_ips) + .flatten() + .map(|addr| (&**addr, AddressType::Ip)) + .collect::>(), + AddressType::Hostname => { + kubernetes_service_fqdn = format!("{svc_name}.{ns}.svc.cluster.local"); + vec![(&kubernetes_service_fqdn, AddressType::Hostname)] + } + }; ports = svc .spec .as_ref() From a1ae9a006d5c0a905b2d9a642e15b958c8b3f95d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Wed, 9 Oct 2024 14:07:36 +0200 Subject: [PATCH 03/10] Changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index df9415cf..49f6ca07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,14 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Added + +- Listener.status.addresses now allows users to configure whether they prefer IP addresses or DNS hostnames ([#233]). + ### Changed - Listener.status.addresses for NodePort listeners now includes replicas that are currently unavailable ([#231]). +- Listener.status.addresses now defaults to DNS hostnames for all service types (previously NodePort and ClusterIP would prefer IP addresses, [#233]). ### Fixed @@ -14,6 +19,7 @@ All notable changes to this project will be documented in this file. - Listener controller now listens for ListenerClass updates ([#231]). [#231]: https://github.com/stackabletech/listener-operator/pull/231 +[#233]: https://github.com/stackabletech/listener-operator/pull/233 ## [24.7.0] - 2024-07-24 From af27ec10ed617f4ac48ec0c54239cd62e0859234 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Wed, 9 Oct 2024 14:19:23 +0200 Subject: [PATCH 04/10] Docs --- .../pages/listenerclass.adoc | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/modules/listener-operator/pages/listenerclass.adoc b/docs/modules/listener-operator/pages/listenerclass.adoc index ce149c50..8bf85ff4 100644 --- a/docs/modules/listener-operator/pages/listenerclass.adoc +++ b/docs/modules/listener-operator/pages/listenerclass.adoc @@ -30,6 +30,7 @@ How exactly this is accomplished depends on the cloud provider in question, but include::example$listenerclass-internal-gke.yaml[] ---- +[#servicetype] == Service types The service type is defined by `ListenerClass.spec.serviceType`. @@ -62,6 +63,24 @@ Compared to xref:#servicetype-nodeport[], this service type allows Pods to be mo However, it requires https://kubernetes.io/docs/concepts/services-networking/service/#loadbalancer[a cloud controller manager that supports load balancers]. Additionally, many cloud providers charge for load-balanced traffic. +[#addresstype] +== Address types + +The Stackable Listener Operator supports both IP addresses and DNS hostnames. The preferred address type for a given ListenerClass can be configured using the `ListenerClass.spec.preferredAddressType` field. If no `preferredAddressType` is specified then it defaults to xref:#addresstype-hostname[]. + +NOTE: If the preferred address type is not supported for a given environment then another type will be used. + +[#addresstype-ip] +=== IP + +The IP address of a resource. The addresses will be less predictable (especially for xref:#servicetype-clusterip[] services), +but does not require any special client configuration (beyond what the xref:#servicetype[] requires). + +[#addresstype-hostname] +=== Hostname + +The DNS hostname of a resource. Clients must be able to resolve these addresses in order to connect, which may require special DNS configuration. + == Default ListenerClasses The Stackable Data Platform assumes the existence of a few predefined ListenerClasses, and will use them by default as appropriate: From 0e130b5f416fa2dd99e451a7cdd211605ae7eadd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Wed, 9 Oct 2024 14:28:05 +0200 Subject: [PATCH 05/10] Test both DNS and IP address types --- ...lass.yaml => 05-create-listenerclass.yaml.j2} | 1 + .../kuttl/smoke-nodeport/10-assert.yaml | 8 -------- .../kuttl/smoke-nodeport/10-assert.yaml.j2 | 16 ++++++++++++++++ tests/test-definition.yaml | 5 +++++ 4 files changed, 22 insertions(+), 8 deletions(-) rename tests/templates/kuttl/smoke-nodeport/{05-create-listenerclass.yaml => 05-create-listenerclass.yaml.j2} (69%) delete mode 100644 tests/templates/kuttl/smoke-nodeport/10-assert.yaml create mode 100644 tests/templates/kuttl/smoke-nodeport/10-assert.yaml.j2 diff --git a/tests/templates/kuttl/smoke-nodeport/05-create-listenerclass.yaml b/tests/templates/kuttl/smoke-nodeport/05-create-listenerclass.yaml.j2 similarity index 69% rename from tests/templates/kuttl/smoke-nodeport/05-create-listenerclass.yaml rename to tests/templates/kuttl/smoke-nodeport/05-create-listenerclass.yaml.j2 index d501f211..d62b0701 100644 --- a/tests/templates/kuttl/smoke-nodeport/05-create-listenerclass.yaml +++ b/tests/templates/kuttl/smoke-nodeport/05-create-listenerclass.yaml.j2 @@ -5,3 +5,4 @@ metadata: name: listener-operator-test-smoke-nodeport spec: serviceType: NodePort + preferredAddressType: {{ test_scenario['values']['addressType'] }} diff --git a/tests/templates/kuttl/smoke-nodeport/10-assert.yaml b/tests/templates/kuttl/smoke-nodeport/10-assert.yaml deleted file mode 100644 index d7ebbae5..00000000 --- a/tests/templates/kuttl/smoke-nodeport/10-assert.yaml +++ /dev/null @@ -1,8 +0,0 @@ ---- -apiVersion: apps/v1 -kind: StatefulSet -metadata: - name: nginx-long-name-approaching-k8s-limits -status: - readyReplicas: 2 - replicas: 2 diff --git a/tests/templates/kuttl/smoke-nodeport/10-assert.yaml.j2 b/tests/templates/kuttl/smoke-nodeport/10-assert.yaml.j2 new file mode 100644 index 00000000..d0c2ca2c --- /dev/null +++ b/tests/templates/kuttl/smoke-nodeport/10-assert.yaml.j2 @@ -0,0 +1,16 @@ +--- +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: nginx-long-name-approaching-k8s-limits +status: + readyReplicas: 2 + replicas: 2 +--- +apiVersion: listeners.stackable.tech/v1alpha1 +kind: Listener +metadata: + name: listener-nginx-long-name-approaching-k8s-limits-0 +status: + ingressAddresses: + - addressType: {{ test_scenario['values']['addressType'] }} diff --git a/tests/test-definition.yaml b/tests/test-definition.yaml index e8a6def3..90acd866 100644 --- a/tests/test-definition.yaml +++ b/tests/test-definition.yaml @@ -3,10 +3,15 @@ dimensions: - name: openshift values: - "false" + - name: addressType + values: + - IP + - Hostname tests: - name: smoke-nodeport dimensions: - openshift + - addressType suites: - name: nightly - name: openshift From 8ebd8377ae1957a65fb8ad5e4f27b56f47a00cc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 10 Oct 2024 13:56:52 +0200 Subject: [PATCH 06/10] Add node_primary_address tests, simplified from @sbernauer's --- rust/operator-binary/src/utils.rs | 74 ++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/rust/operator-binary/src/utils.rs b/rust/operator-binary/src/utils.rs index 58da2c32..eeeee0d4 100644 --- a/rust/operator-binary/src/utils.rs +++ b/rust/operator-binary/src/utils.rs @@ -141,7 +141,12 @@ pub fn node_primary_address(node: &Node) -> AddressCandidates { #[cfg(test)] mod tests { - use crate::utils::error_full_message; + use stackable_operator::{ + commons::listener::AddressType, + k8s_openapi::api::core::v1::{Node, NodeAddress, NodeStatus}, + }; + + use crate::utils::{error_full_message, node_primary_address}; #[test] fn error_messages() { @@ -159,4 +164,71 @@ mod tests { "leaf: middleware: root error" ); } + + #[test] + fn node_with_only_ips_primary_address_returns_external_ip() { + let node = node_from_addresses(vec![("InternalIP", "10.1.2.3"), ("ExternalIP", "1.2.3.4")]); + let node_primary_address = node_primary_address(&node); + assert_eq!( + node_primary_address.pick(AddressType::Ip), + Some(("1.2.3.4", AddressType::Ip)) + ); + assert_eq!( + node_primary_address.pick(AddressType::Hostname), + Some(("1.2.3.4", AddressType::Ip)) + ); + } + + #[test] + fn node_with_only_hostname_primary_address_returns_hostname() { + let node = node_from_addresses(vec![ + ("Hostname", "first-hostname"), + ("Hostname", "second-hostname"), + ]); + let node_primary_address = node_primary_address(&node); + assert_eq!( + node_primary_address.pick(AddressType::Ip), + Some(("first-hostname", AddressType::Hostname)) + ); + assert_eq!( + node_primary_address.pick(AddressType::Hostname), + Some(("first-hostname", AddressType::Hostname)) + ); + } + + #[test] + fn node_with_hostname_and_ips_primary_address() { + let node = node_from_addresses(vec![ + ("Hostname", "node-0"), + ("ExternalIP", "1.2.3.4"), + ("InternalIP", "10.1.2.3"), + ]); + let node_primary_address = node_primary_address(&node); + assert_eq!( + node_primary_address.pick(AddressType::Ip), + Some(("1.2.3.4", AddressType::Ip)) + ); + assert_eq!( + node_primary_address.pick(AddressType::Hostname), + Some(("node-0", AddressType::Hostname)) + ); + } + + fn node_from_addresses<'a>(addresses: impl IntoIterator) -> Node { + Node { + status: Some(NodeStatus { + addresses: Some( + addresses + .into_iter() + .map(|(ty, addr)| NodeAddress { + type_: ty.to_string(), + address: addr.to_string(), + }) + .collect(), + ), + ..Default::default() + }), + ..Default::default() + } + } } From 71c152c0db4ac898a2eb6557896cb6b29b744dd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 10 Oct 2024 14:51:33 +0200 Subject: [PATCH 07/10] Split node_primary_addresses into a separate module --- rust/operator-binary/src/csi_server/node.rs | 4 +- .../src/listener_controller.rs | 4 +- rust/operator-binary/src/utils/address.rs | 118 ++++++++++++++++++ .../src/{utils.rs => utils/mod.rs} | 115 +---------------- 4 files changed, 125 insertions(+), 116 deletions(-) create mode 100644 rust/operator-binary/src/utils/address.rs rename rust/operator-binary/src/{utils.rs => utils/mod.rs} (51%) diff --git a/rust/operator-binary/src/csi_server/node.rs b/rust/operator-binary/src/csi_server/node.rs index b67c3be5..db5991d7 100644 --- a/rust/operator-binary/src/csi_server/node.rs +++ b/rust/operator-binary/src/csi_server/node.rs @@ -21,7 +21,7 @@ use crate::{ listener_mounted_pod_label, listener_persistent_volume_label, ListenerMountedPodLabelError, ListenerPersistentVolumeLabelError, }, - utils::{error_full_message, node_primary_address}, + utils::{address::node_primary_addresses, error_full_message}, }; use super::{tonic_unimplemented, ListenerSelector, ListenerVolumeContext}; @@ -463,7 +463,7 @@ async fn local_listener_addresses_for_pod( obj: ObjectRef::::new(listener_class_name).erase(), })?; - Ok(node_primary_address(&node) + Ok(node_primary_addresses(&node) .pick(listener_class.spec.preferred_address_type) .map(|(address, address_type)| ListenerIngress { // nodes: Some(vec![node_name.to_string()]), diff --git a/rust/operator-binary/src/listener_controller.rs b/rust/operator-binary/src/listener_controller.rs index 93cddf89..05c53eff 100644 --- a/rust/operator-binary/src/listener_controller.rs +++ b/rust/operator-binary/src/listener_controller.rs @@ -30,7 +30,7 @@ use strum::IntoStaticStr; use crate::{ csi_server::node::NODE_TOPOLOGY_LABEL_HOSTNAME, - utils::{node_primary_address, AddressCandidates}, + utils::address::{node_primary_addresses, AddressCandidates}, }; #[cfg(doc)] @@ -291,7 +291,7 @@ pub async fn reconcile(listener: Arc, ctx: Arc) -> Result>(); ports = svc diff --git a/rust/operator-binary/src/utils/address.rs b/rust/operator-binary/src/utils/address.rs new file mode 100644 index 00000000..f0446804 --- /dev/null +++ b/rust/operator-binary/src/utils/address.rs @@ -0,0 +1,118 @@ +use stackable_operator::{commons::listener::AddressType, k8s_openapi::api::core::v1::Node}; + +/// The primary addresses of an entity, for each type of address. +#[derive(Debug, Clone, Copy)] +pub struct AddressCandidates<'a> { + pub ip: Option<&'a str>, + pub hostname: Option<&'a str>, +} + +impl<'a> AddressCandidates<'a> { + /// Tries to pick the preferred [`AddressType`], falling back if it is not available. + pub fn pick(&self, preferred_address_type: AddressType) -> Option<(&'a str, AddressType)> { + let ip = self.ip.zip(Some(AddressType::Ip)); + let hostname = self.hostname.zip(Some(AddressType::Hostname)); + match preferred_address_type { + AddressType::Ip => ip.or(hostname), + AddressType::Hostname => hostname.or(ip), + } + } +} + +/// Try to guess the primary addresses of a Node, which it is expected that external clients should be able to reach it on +pub fn node_primary_addresses(node: &Node) -> AddressCandidates { + let addrs = node + .status + .as_ref() + .and_then(|s| s.addresses.as_deref()) + .unwrap_or_default(); + + AddressCandidates { + ip: addrs + .iter() + .find(|addr| addr.type_ == "ExternalIP") + .or_else(|| addrs.iter().find(|addr| addr.type_ == "InternalIP")) + .map(|addr| addr.address.as_str()), + hostname: addrs + .iter() + .find(|addr| addr.type_ == "Hostname") + .map(|addr| addr.address.as_str()), + } +} + +#[cfg(test)] +mod tests { + use stackable_operator::{ + commons::listener::AddressType, + k8s_openapi::api::core::v1::{Node, NodeAddress, NodeStatus}, + }; + + use super::node_primary_addresses; + + #[test] + fn node_with_only_ips_primary_address_returns_external_ip() { + let node = node_from_addresses(vec![("InternalIP", "10.1.2.3"), ("ExternalIP", "1.2.3.4")]); + let node_primary_address = node_primary_addresses(&node); + assert_eq!( + node_primary_address.pick(AddressType::Ip), + Some(("1.2.3.4", AddressType::Ip)) + ); + assert_eq!( + node_primary_address.pick(AddressType::Hostname), + Some(("1.2.3.4", AddressType::Ip)) + ); + } + + #[test] + fn node_with_only_hostname_primary_address_returns_hostname() { + let node = node_from_addresses(vec![ + ("Hostname", "first-hostname"), + ("Hostname", "second-hostname"), + ]); + let node_primary_address = node_primary_addresses(&node); + assert_eq!( + node_primary_address.pick(AddressType::Ip), + Some(("first-hostname", AddressType::Hostname)) + ); + assert_eq!( + node_primary_address.pick(AddressType::Hostname), + Some(("first-hostname", AddressType::Hostname)) + ); + } + + #[test] + fn node_with_hostname_and_ips_primary_address() { + let node = node_from_addresses(vec![ + ("Hostname", "node-0"), + ("ExternalIP", "1.2.3.4"), + ("InternalIP", "10.1.2.3"), + ]); + let node_primary_address = node_primary_addresses(&node); + assert_eq!( + node_primary_address.pick(AddressType::Ip), + Some(("1.2.3.4", AddressType::Ip)) + ); + assert_eq!( + node_primary_address.pick(AddressType::Hostname), + Some(("node-0", AddressType::Hostname)) + ); + } + + fn node_from_addresses<'a>(addresses: impl IntoIterator) -> Node { + Node { + status: Some(NodeStatus { + addresses: Some( + addresses + .into_iter() + .map(|(ty, addr)| NodeAddress { + type_: ty.to_string(), + address: addr.to_string(), + }) + .collect(), + ), + ..Default::default() + }), + ..Default::default() + } + } +} diff --git a/rust/operator-binary/src/utils.rs b/rust/operator-binary/src/utils/mod.rs similarity index 51% rename from rust/operator-binary/src/utils.rs rename to rust/operator-binary/src/utils/mod.rs index eeeee0d4..ab754413 100644 --- a/rust/operator-binary/src/utils.rs +++ b/rust/operator-binary/src/utils/mod.rs @@ -2,13 +2,14 @@ use std::{os::unix::prelude::AsRawFd, path::Path}; use pin_project::pin_project; use socket2::Socket; -use stackable_operator::{commons::listener::AddressType, k8s_openapi::api::core::v1::Node}; use tokio::{ io::{AsyncRead, AsyncWrite}, net::{UnixListener, UnixStream}, }; use tonic::transport::server::Connected; +pub mod address; + /// Adapter for using [`UnixStream`] as a [`tonic`] connection /// Tonic usually communicates via TCP sockets, but the Kubernetes CSI interface expects /// plugins to use Unix sockets instead. @@ -101,52 +102,9 @@ pub fn error_full_message(err: &dyn std::error::Error) -> String { full_msg } -#[derive(Debug, Clone, Copy)] -pub struct AddressCandidates<'a> { - pub ip: Option<&'a str>, - pub hostname: Option<&'a str>, -} - -impl<'a> AddressCandidates<'a> { - pub fn pick(&self, preferred_address_type: AddressType) -> Option<(&'a str, AddressType)> { - let ip = self.ip.zip(Some(AddressType::Ip)); - let hostname = self.hostname.zip(Some(AddressType::Hostname)); - match preferred_address_type { - AddressType::Ip => ip.or(hostname), - AddressType::Hostname => hostname.or(ip), - } - } -} - -/// Try to guess the primary address of a Node, which it is expected that external clients should be able to reach it on -pub fn node_primary_address(node: &Node) -> AddressCandidates { - let addrs = node - .status - .as_ref() - .and_then(|s| s.addresses.as_deref()) - .unwrap_or_default(); - - AddressCandidates { - ip: addrs - .iter() - .find(|addr| addr.type_ == "ExternalIP") - .or_else(|| addrs.iter().find(|addr| addr.type_ == "InternalIP")) - .map(|addr| addr.address.as_str()), - hostname: addrs - .iter() - .find(|addr| addr.type_ == "Hostname") - .map(|addr| addr.address.as_str()), - } -} - #[cfg(test)] mod tests { - use stackable_operator::{ - commons::listener::AddressType, - k8s_openapi::api::core::v1::{Node, NodeAddress, NodeStatus}, - }; - - use crate::utils::{error_full_message, node_primary_address}; + use crate::utils::error_full_message; #[test] fn error_messages() { @@ -164,71 +122,4 @@ mod tests { "leaf: middleware: root error" ); } - - #[test] - fn node_with_only_ips_primary_address_returns_external_ip() { - let node = node_from_addresses(vec![("InternalIP", "10.1.2.3"), ("ExternalIP", "1.2.3.4")]); - let node_primary_address = node_primary_address(&node); - assert_eq!( - node_primary_address.pick(AddressType::Ip), - Some(("1.2.3.4", AddressType::Ip)) - ); - assert_eq!( - node_primary_address.pick(AddressType::Hostname), - Some(("1.2.3.4", AddressType::Ip)) - ); - } - - #[test] - fn node_with_only_hostname_primary_address_returns_hostname() { - let node = node_from_addresses(vec![ - ("Hostname", "first-hostname"), - ("Hostname", "second-hostname"), - ]); - let node_primary_address = node_primary_address(&node); - assert_eq!( - node_primary_address.pick(AddressType::Ip), - Some(("first-hostname", AddressType::Hostname)) - ); - assert_eq!( - node_primary_address.pick(AddressType::Hostname), - Some(("first-hostname", AddressType::Hostname)) - ); - } - - #[test] - fn node_with_hostname_and_ips_primary_address() { - let node = node_from_addresses(vec![ - ("Hostname", "node-0"), - ("ExternalIP", "1.2.3.4"), - ("InternalIP", "10.1.2.3"), - ]); - let node_primary_address = node_primary_address(&node); - assert_eq!( - node_primary_address.pick(AddressType::Ip), - Some(("1.2.3.4", AddressType::Ip)) - ); - assert_eq!( - node_primary_address.pick(AddressType::Hostname), - Some(("node-0", AddressType::Hostname)) - ); - } - - fn node_from_addresses<'a>(addresses: impl IntoIterator) -> Node { - Node { - status: Some(NodeStatus { - addresses: Some( - addresses - .into_iter() - .map(|(ty, addr)| NodeAddress { - type_: ty.to_string(), - address: addr.to_string(), - }) - .collect(), - ), - ..Default::default() - }), - ..Default::default() - } - } } From f320c2e336534db975f9af9525988c72214728af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 10 Oct 2024 15:20:27 +0200 Subject: [PATCH 08/10] Split the rest of utils --- .../src/csi_server/controller.rs | 2 +- rust/operator-binary/src/csi_server/node.rs | 2 +- rust/operator-binary/src/main.rs | 2 +- rust/operator-binary/src/utils/error.rs | 35 +++++ rust/operator-binary/src/utils/mod.rs | 126 +----------------- rust/operator-binary/src/utils/unix_stream.rs | 87 ++++++++++++ 6 files changed, 127 insertions(+), 127 deletions(-) create mode 100644 rust/operator-binary/src/utils/error.rs create mode 100644 rust/operator-binary/src/utils/unix_stream.rs diff --git a/rust/operator-binary/src/csi_server/controller.rs b/rust/operator-binary/src/csi_server/controller.rs index 6b501321..c14961e7 100644 --- a/rust/operator-binary/src/csi_server/controller.rs +++ b/rust/operator-binary/src/csi_server/controller.rs @@ -8,7 +8,7 @@ use stackable_operator::{ }; use tonic::{Request, Response, Status}; -use crate::utils::error_full_message; +use crate::utils::error::error_full_message; use super::{tonic_unimplemented, ListenerSelector, ListenerVolumeContext}; diff --git a/rust/operator-binary/src/csi_server/node.rs b/rust/operator-binary/src/csi_server/node.rs index db5991d7..d7c407d5 100644 --- a/rust/operator-binary/src/csi_server/node.rs +++ b/rust/operator-binary/src/csi_server/node.rs @@ -21,7 +21,7 @@ use crate::{ listener_mounted_pod_label, listener_persistent_volume_label, ListenerMountedPodLabelError, ListenerPersistentVolumeLabelError, }, - utils::{address::node_primary_addresses, error_full_message}, + utils::{address::node_primary_addresses, error::error_full_message}, }; use super::{tonic_unimplemented, ListenerSelector, ListenerVolumeContext}; diff --git a/rust/operator-binary/src/main.rs b/rust/operator-binary/src/main.rs index 4101dfbf..4028829f 100644 --- a/rust/operator-binary/src/main.rs +++ b/rust/operator-binary/src/main.rs @@ -17,7 +17,7 @@ use stackable_operator::{ use tokio::signal::unix::{signal, SignalKind}; use tokio_stream::wrappers::UnixListenerStream; use tonic::transport::Server; -use utils::{uds_bind_private, TonicUnixStream}; +use utils::unix_stream::{uds_bind_private, TonicUnixStream}; mod csi_server; mod listener_controller; diff --git a/rust/operator-binary/src/utils/error.rs b/rust/operator-binary/src/utils/error.rs new file mode 100644 index 00000000..2718c85d --- /dev/null +++ b/rust/operator-binary/src/utils/error.rs @@ -0,0 +1,35 @@ +/// Combines the messages of an error and its sources into a [`String`] of the form `"error: source 1: source 2: root error"` +pub fn error_full_message(err: &dyn std::error::Error) -> String { + use std::fmt::Write; + // Build the full hierarchy of error messages by walking up the stack until an error + // without `source` set is encountered and concatenating all encountered error strings. + let mut full_msg = format!("{}", err); + let mut curr_err = err.source(); + while let Some(curr_source) = curr_err { + write!(full_msg, ": {curr_source}").expect("string formatting should be infallible"); + curr_err = curr_source.source(); + } + full_msg +} + +#[cfg(test)] +pub(crate) mod tests { + use super::error_full_message; + + #[test] + fn error_messages() { + assert_eq!( + error_full_message(anyhow::anyhow!("standalone error").as_ref()), + "standalone error" + ); + assert_eq!( + error_full_message( + anyhow::anyhow!("root error") + .context("middleware") + .context("leaf") + .as_ref() + ), + "leaf: middleware: root error" + ); + } +} diff --git a/rust/operator-binary/src/utils/mod.rs b/rust/operator-binary/src/utils/mod.rs index ab754413..39238137 100644 --- a/rust/operator-binary/src/utils/mod.rs +++ b/rust/operator-binary/src/utils/mod.rs @@ -1,125 +1,3 @@ -use std::{os::unix::prelude::AsRawFd, path::Path}; - -use pin_project::pin_project; -use socket2::Socket; -use tokio::{ - io::{AsyncRead, AsyncWrite}, - net::{UnixListener, UnixStream}, -}; -use tonic::transport::server::Connected; - pub mod address; - -/// Adapter for using [`UnixStream`] as a [`tonic`] connection -/// Tonic usually communicates via TCP sockets, but the Kubernetes CSI interface expects -/// plugins to use Unix sockets instead. -/// This provides a wrapper implementation which delegates to tokio's [`UnixStream`] in order -/// to enable tonic to communicate via Unix sockets. -#[pin_project] -pub struct TonicUnixStream(#[pin] pub UnixStream); - -impl AsyncRead for TonicUnixStream { - fn poll_read( - self: std::pin::Pin<&mut Self>, - cx: &mut std::task::Context<'_>, - buf: &mut tokio::io::ReadBuf<'_>, - ) -> std::task::Poll> { - self.project().0.poll_read(cx, buf) - } -} - -impl AsyncWrite for TonicUnixStream { - fn poll_write( - self: std::pin::Pin<&mut Self>, - cx: &mut std::task::Context<'_>, - buf: &[u8], - ) -> std::task::Poll> { - self.project().0.poll_write(cx, buf) - } - - fn poll_flush( - self: std::pin::Pin<&mut Self>, - cx: &mut std::task::Context<'_>, - ) -> std::task::Poll> { - self.project().0.poll_flush(cx) - } - - fn poll_shutdown( - self: std::pin::Pin<&mut Self>, - cx: &mut std::task::Context<'_>, - ) -> std::task::Poll> { - self.project().0.poll_shutdown(cx) - } - - fn poll_write_vectored( - self: std::pin::Pin<&mut Self>, - cx: &mut std::task::Context<'_>, - bufs: &[std::io::IoSlice<'_>], - ) -> std::task::Poll> { - self.project().0.poll_write_vectored(cx, bufs) - } - - fn is_write_vectored(&self) -> bool { - self.0.is_write_vectored() - } -} - -impl Connected for TonicUnixStream { - type ConnectInfo = (); - - fn connect_info(&self) -> Self::ConnectInfo {} -} - -/// Bind a Unix Domain Socket listener that is only accessible to the current user -pub fn uds_bind_private(path: impl AsRef) -> Result { - // Workaround for https://github.com/tokio-rs/tokio/issues/4422 - let socket = Socket::new(socket2::Domain::UNIX, socket2::Type::STREAM, None)?; - unsafe { - // Socket-level chmod is propagated to the file created by Socket::bind. - // We need to chmod /before/ creating the file, because otherwise there is a brief window where - // the file is world-accessible (unless restricted by the global umask). - if libc::fchmod(socket.as_raw_fd(), 0o600) == -1 { - return Err(std::io::Error::last_os_error()); - } - } - socket.bind(&socket2::SockAddr::unix(path)?)?; - socket.listen(1024)?; - socket.set_nonblocking(true)?; - UnixListener::from_std(socket.into()) -} - -/// Combines the messages of an error and its sources into a [`String`] of the form `"error: source 1: source 2: root error"` -pub fn error_full_message(err: &dyn std::error::Error) -> String { - use std::fmt::Write; - // Build the full hierarchy of error messages by walking up the stack until an error - // without `source` set is encountered and concatenating all encountered error strings. - let mut full_msg = format!("{}", err); - let mut curr_err = err.source(); - while let Some(curr_source) = curr_err { - write!(full_msg, ": {curr_source}").expect("string formatting should be infallible"); - curr_err = curr_source.source(); - } - full_msg -} - -#[cfg(test)] -mod tests { - use crate::utils::error_full_message; - - #[test] - fn error_messages() { - assert_eq!( - error_full_message(anyhow::anyhow!("standalone error").as_ref()), - "standalone error" - ); - assert_eq!( - error_full_message( - anyhow::anyhow!("root error") - .context("middleware") - .context("leaf") - .as_ref() - ), - "leaf: middleware: root error" - ); - } -} +pub mod error; +pub mod unix_stream; diff --git a/rust/operator-binary/src/utils/unix_stream.rs b/rust/operator-binary/src/utils/unix_stream.rs new file mode 100644 index 00000000..fb0652d4 --- /dev/null +++ b/rust/operator-binary/src/utils/unix_stream.rs @@ -0,0 +1,87 @@ +use std::{os::unix::prelude::AsRawFd, path::Path}; + +use pin_project::pin_project; +use socket2::Socket; +use tokio::{ + io::{AsyncRead, AsyncWrite}, + net::{UnixListener, UnixStream}, +}; +use tonic::transport::server::Connected; + +/// Adapter for using [`UnixStream`] as a [`tonic`] connection +/// Tonic usually communicates via TCP sockets, but the Kubernetes CSI interface expects +/// plugins to use Unix sockets instead. +/// This provides a wrapper implementation which delegates to tokio's [`UnixStream`] in order +/// to enable tonic to communicate via Unix sockets. +#[pin_project] +pub struct TonicUnixStream(#[pin] pub UnixStream); + +impl AsyncRead for TonicUnixStream { + fn poll_read( + self: std::pin::Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + buf: &mut tokio::io::ReadBuf<'_>, + ) -> std::task::Poll> { + self.project().0.poll_read(cx, buf) + } +} + +impl AsyncWrite for TonicUnixStream { + fn poll_write( + self: std::pin::Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + buf: &[u8], + ) -> std::task::Poll> { + self.project().0.poll_write(cx, buf) + } + + fn poll_flush( + self: std::pin::Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + ) -> std::task::Poll> { + self.project().0.poll_flush(cx) + } + + fn poll_shutdown( + self: std::pin::Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + ) -> std::task::Poll> { + self.project().0.poll_shutdown(cx) + } + + fn poll_write_vectored( + self: std::pin::Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + bufs: &[std::io::IoSlice<'_>], + ) -> std::task::Poll> { + self.project().0.poll_write_vectored(cx, bufs) + } + + fn is_write_vectored(&self) -> bool { + self.0.is_write_vectored() + } +} + +impl Connected for TonicUnixStream { + type ConnectInfo = (); + + fn connect_info(&self) -> Self::ConnectInfo {} +} + +/// Bind a Unix Domain Socket listener that is only accessible to the current user +pub fn uds_bind_private(path: impl AsRef) -> Result { + // Workaround for https://github.com/tokio-rs/tokio/issues/4422 + let socket = Socket::new(socket2::Domain::UNIX, socket2::Type::STREAM, None)?; + unsafe { + // Socket-level chmod is propagated to the file created by Socket::bind. + // We need to chmod /before/ creating the file, because otherwise there is a brief window where + // the file is world-accessible (unless restricted by the global umask). + if libc::fchmod(socket.as_raw_fd(), 0o600) == -1 { + return Err(std::io::Error::last_os_error()); + } + } + socket.bind(&socket2::SockAddr::unix(path)?)?; + socket.listen(1024)?; + socket.set_nonblocking(true)?; + UnixListener::from_std(socket.into()) +} From 9c2ce25586bacc779e0dfedd95d674b704ddf023 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 10 Oct 2024 15:22:33 +0200 Subject: [PATCH 09/10] Reword changelog a bit --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49f6ca07..1b0a5bd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,12 +6,12 @@ All notable changes to this project will be documented in this file. ### Added -- Listener.status.addresses now allows users to configure whether they prefer IP addresses or DNS hostnames ([#233]). +- `Listener.status.addresses` can now be configured to prefer either IP addresses or DNS hostnames ([#233]). ### Changed -- Listener.status.addresses for NodePort listeners now includes replicas that are currently unavailable ([#231]). -- Listener.status.addresses now defaults to DNS hostnames for all service types (previously NodePort and ClusterIP would prefer IP addresses, [#233]). +- `Listener.status.addresses` for NodePort listeners now includes replicas that are currently unavailable ([#231]). +- `Listener.status.addresses` now defaults to DNS hostnames for all service types (previously NodePort and ClusterIP would prefer IP addresses, [#233]). ### Fixed From e52cc1220e0da840987ce556f8e7ced9c2b6c140 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Natalie=20Klestrup=20R=C3=B6ijezon?= Date: Wed, 16 Oct 2024 11:18:38 +0200 Subject: [PATCH 10/10] The op-rs PR was merged --- Cargo.lock | 6 +++--- Cargo.nix | 12 ++++++------ Cargo.toml | 3 +-- crate-hashes.json | 6 +++--- deploy/helm/listener-operator/crds/crds.yaml | 2 +- 5 files changed, 14 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fe641107..9707dceb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2325,7 +2325,7 @@ dependencies = [ [[package]] name = "stackable-operator" version = "0.78.0" -source = "git+https://github.com/stackabletech//operator-rs.git?branch=feature/listenerclass-preferred-address-type#91bb8d423e877062c1cceaf0c0e5523f9ad521db" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=main#d51e8a73e94aa65d5d45338edf4959169b65e00e" dependencies = [ "chrono", "clap", @@ -2363,7 +2363,7 @@ dependencies = [ [[package]] name = "stackable-operator-derive" version = "0.3.1" -source = "git+https://github.com/stackabletech//operator-rs.git?branch=feature/listenerclass-preferred-address-type#91bb8d423e877062c1cceaf0c0e5523f9ad521db" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=main#d51e8a73e94aa65d5d45338edf4959169b65e00e" dependencies = [ "darling", "proc-macro2", @@ -2374,7 +2374,7 @@ dependencies = [ [[package]] name = "stackable-shared" version = "0.0.1" -source = "git+https://github.com/stackabletech//operator-rs.git?branch=feature/listenerclass-preferred-address-type#91bb8d423e877062c1cceaf0c0e5523f9ad521db" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=main#d51e8a73e94aa65d5d45338edf4959169b65e00e" dependencies = [ "kube", "semver", diff --git a/Cargo.nix b/Cargo.nix index 2ea15ea5..d6ec8b03 100644 --- a/Cargo.nix +++ b/Cargo.nix @@ -7365,8 +7365,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech//operator-rs.git"; - rev = "91bb8d423e877062c1cceaf0c0e5523f9ad521db"; - sha256 = "1yr1jgcmwf0msdpxh76124gnkhych27ab1z7wjnbmwcchxw1r20m"; + rev = "d51e8a73e94aa65d5d45338edf4959169b65e00e"; + sha256 = "0cq00sqbwashcsakd0bfmpzpdvvm3xh1xcsjnwaspm6mfi38a9cj"; }; libName = "stackable_operator"; authors = [ @@ -7523,8 +7523,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech//operator-rs.git"; - rev = "91bb8d423e877062c1cceaf0c0e5523f9ad521db"; - sha256 = "1yr1jgcmwf0msdpxh76124gnkhych27ab1z7wjnbmwcchxw1r20m"; + rev = "d51e8a73e94aa65d5d45338edf4959169b65e00e"; + sha256 = "0cq00sqbwashcsakd0bfmpzpdvvm3xh1xcsjnwaspm6mfi38a9cj"; }; procMacro = true; libName = "stackable_operator_derive"; @@ -7558,8 +7558,8 @@ rec { workspace_member = null; src = pkgs.fetchgit { url = "https://github.com/stackabletech//operator-rs.git"; - rev = "91bb8d423e877062c1cceaf0c0e5523f9ad521db"; - sha256 = "1yr1jgcmwf0msdpxh76124gnkhych27ab1z7wjnbmwcchxw1r20m"; + rev = "d51e8a73e94aa65d5d45338edf4959169b65e00e"; + sha256 = "0cq00sqbwashcsakd0bfmpzpdvvm3xh1xcsjnwaspm6mfi38a9cj"; }; libName = "stackable_shared"; authors = [ diff --git a/Cargo.toml b/Cargo.toml index e064a7d0..66999bac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,5 +33,4 @@ tracing = "0.1.40" [patch."https://github.com/stackabletech/operator-rs.git"] # stackable-operator = { path = "../operator-rs/crates/stackable-operator" } -# stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" } -stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "feature/listenerclass-preferred-address-type" } +stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" } diff --git a/crate-hashes.json b/crate-hashes.json index 91bbbea6..258e5db1 100644 --- a/crate-hashes.json +++ b/crate-hashes.json @@ -1,6 +1,6 @@ { - "git+https://github.com/stackabletech//operator-rs.git?branch=feature%2Flistenerclass-preferred-address-type#stackable-operator-derive@0.3.1": "1yr1jgcmwf0msdpxh76124gnkhych27ab1z7wjnbmwcchxw1r20m", - "git+https://github.com/stackabletech//operator-rs.git?branch=feature%2Flistenerclass-preferred-address-type#stackable-operator@0.78.0": "1yr1jgcmwf0msdpxh76124gnkhych27ab1z7wjnbmwcchxw1r20m", - "git+https://github.com/stackabletech//operator-rs.git?branch=feature%2Flistenerclass-preferred-address-type#stackable-shared@0.0.1": "1yr1jgcmwf0msdpxh76124gnkhych27ab1z7wjnbmwcchxw1r20m", + "git+https://github.com/stackabletech//operator-rs.git?branch=main#stackable-operator-derive@0.3.1": "0cq00sqbwashcsakd0bfmpzpdvvm3xh1xcsjnwaspm6mfi38a9cj", + "git+https://github.com/stackabletech//operator-rs.git?branch=main#stackable-operator@0.78.0": "0cq00sqbwashcsakd0bfmpzpdvvm3xh1xcsjnwaspm6mfi38a9cj", + "git+https://github.com/stackabletech//operator-rs.git?branch=main#stackable-shared@0.0.1": "0cq00sqbwashcsakd0bfmpzpdvvm3xh1xcsjnwaspm6mfi38a9cj", "git+https://github.com/stackabletech/product-config.git?tag=0.7.0#product-config@0.7.0": "0gjsm80g6r75pm3824dcyiz4ysq1ka4c1if6k1mjm9cnd5ym0gny" } \ No newline at end of file diff --git a/deploy/helm/listener-operator/crds/crds.yaml b/deploy/helm/listener-operator/crds/crds.yaml index 7e0c7eac..9cc9f087 100644 --- a/deploy/helm/listener-operator/crds/crds.yaml +++ b/deploy/helm/listener-operator/crds/crds.yaml @@ -29,7 +29,7 @@ spec: description: |- Whether addresses should prefer using the IP address (`IP`) or the hostname (`Hostname`). - The other type will be used if the preferred type is not available. + The other type will be used if the preferred type is not available. By default `Hostname` is used. enum: - Hostname - IP