From 20056796347977d3c4415657043ab2afaf205f5e Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 18 Feb 2025 17:28:02 +0100 Subject: [PATCH 1/8] add load balancing role service and discovery cm --- .../opa/pages/implementation-notes.adoc | 14 ++++ rust/operator-binary/src/controller.rs | 84 +++++++++++++++---- rust/operator-binary/src/crd/mod.rs | 8 ++ rust/operator-binary/src/discovery.rs | 23 +---- 4 files changed, 93 insertions(+), 36 deletions(-) diff --git a/docs/modules/opa/pages/implementation-notes.adoc b/docs/modules/opa/pages/implementation-notes.adoc index 9401980d..629c73c4 100644 --- a/docs/modules/opa/pages/implementation-notes.adoc +++ b/docs/modules/opa/pages/implementation-notes.adoc @@ -5,11 +5,25 @@ but should not be required reading for regular use. == OPA replica per node +Since OPA is deployed as a DaemonSet and runs on each Node, two entrypoint Services are defined. + +=== Local Traffic Policy + OPA runs on each Node to avoid requiring network round trips for services making policy queries (which are often chained in serial, and block other tasks in the products). Local access is ensured via an https://kubernetes.io/docs/concepts/services-networking/service-traffic-policy/[`InternalTrafficPolicy`]. This means that https://kubernetes.io/docs/concepts/workloads/pods/[Pods] accessing OPA via the service discovery are routed to the OPA Pod on the same https://kubernetes.io/docs/concepts/architecture/nodes/[Node] to reduce request latency and network traffic. +This should be the default entrypoint and has the same name as the defined OPA cluster. If the `metadata.name` is `opa`, this service is called `opa`. + +=== Cluster Traffic Policy (load-balanced / round-robin) + +This service is called as the OPA cluster suffixed with `-lb`. This entrypoint can be used if latency (e.g. no network requests) is less important. +Evaluating complicated rego rules may take some time depending on the provided resources, and can be the limiting factor in e.g. bulk requests. +Therefore, using this service, every Pod in the cluster is utilized to evaluate policies than instead e.g. just one. + +If the `metadata.name` if `opa`, this service is called `opa-lb`. + == OPA Bundle Builder Users can manage policy rules by creating, updating and deleting ConfigMap resources. diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index 1f1d9458..85d56b0e 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -74,7 +74,7 @@ use stackable_operator::{ use strum::{EnumDiscriminants, IntoStaticStr}; use crate::{ - discovery::{self, build_discovery_configmaps}, + discovery::{self, build_discovery_configmap}, operations::graceful_shutdown::add_graceful_shutdown_config, product_logging::{ extend_role_group_config_map, resolve_vector_aggregator_address, BundleBuilderLogLevel, @@ -447,13 +447,22 @@ pub async fn reconcile_opa( .await .context(ResolveVectorAggregatorAddressSnafu)?; - let server_role_service = build_server_role_service(opa, &resolved_product_image)?; + let server_role_service = + build_server_role_service_traffic_local(opa, &resolved_product_image)?; // required for discovery config map later let server_role_service = cluster_resources .add(client, server_role_service) .await .context(ApplyRoleServiceSnafu)?; + let server_role_service_load_balanced = + build_server_role_service_traffic_cluster(opa, &resolved_product_image)?; + // required for discovery config map later + let server_role_service_load_balanced = cluster_resources + .add(client, server_role_service_load_balanced) + .await + .context(ApplyRoleServiceSnafu)?; + let required_labels = cluster_resources .get_required_labels() .context(BuildLabelSnafu)?; @@ -546,20 +555,35 @@ pub async fn reconcile_opa( .context(ApplyPatchRoleGroupDaemonSetSnafu { rolegroup })?; } - for discovery_cm in build_discovery_configmaps( + let discovery_cm_traffic_local = build_discovery_configmap( + &server_role_service.name_any(), opa, opa, &resolved_product_image, &server_role_service, &client.kubernetes_cluster_info, ) - .context(BuildDiscoveryConfigSnafu)? - { - cluster_resources - .add(client, discovery_cm) - .await - .context(ApplyDiscoveryConfigSnafu)?; - } + .context(BuildDiscoveryConfigSnafu)?; + + cluster_resources + .add(client, discovery_cm_traffic_local) + .await + .context(ApplyDiscoveryConfigSnafu)?; + + let discovery_cm_traffic_cluster = build_discovery_configmap( + &server_role_service_load_balanced.name_any(), + opa, + opa, + &resolved_product_image, + &server_role_service_load_balanced, + &client.kubernetes_cluster_info, + ) + .context(BuildDiscoveryConfigSnafu)?; + + cluster_resources + .add(client, discovery_cm_traffic_cluster) + .await + .context(ApplyDiscoveryConfigSnafu)?; let cluster_operation_cond_builder = ClusterOperationsConditionBuilder::new(&opa.spec.cluster_operation); @@ -583,18 +607,48 @@ pub async fn reconcile_opa( /// The server-role service is the primary endpoint that should be used by clients that do not perform internal load balancing, /// including targets outside of the cluster. -pub fn build_server_role_service( +pub fn build_server_role_service_traffic_local( opa: &v1alpha1::OpaCluster, resolved_product_image: &ResolvedProductImage, ) -> Result { - let role_name = v1alpha1::OpaRole::Server.to_string(); - let role_svc_name = opa + let service_name = opa .server_role_service_name() .context(RoleServiceNameNotFoundSnafu)?; + build_server_role_service( + opa, + resolved_product_image, + &service_name, + Some("Local".to_string()), + ) +} + +/// The server-role service is the endpoint that should be used by clients that do perform internal load balancing. +pub fn build_server_role_service_traffic_cluster( + opa: &v1alpha1::OpaCluster, + resolved_product_image: &ResolvedProductImage, +) -> Result { + let service_name = opa + .server_role_service_name_load_balanced() + .context(RoleServiceNameNotFoundSnafu)?; + build_server_role_service( + opa, + resolved_product_image, + &service_name, + Some("Cluster".to_string()), + ) +} + +fn build_server_role_service( + opa: &v1alpha1::OpaCluster, + resolved_product_image: &ResolvedProductImage, + service_name: &str, + internal_traffic_policy: Option, +) -> Result { + let role_name = v1alpha1::OpaRole::Server.to_string(); let metadata = ObjectMetaBuilder::new() .name_and_namespace(opa) - .name(&role_svc_name) + .name(service_name) .ownerreference_from_resource(opa, None, Some(true)) .context(ObjectMissingMetadataForOwnerRefSnafu)? .with_recommended_labels(build_recommended_labels( @@ -618,7 +672,7 @@ pub fn build_server_role_service( ..ServicePort::default() }]), selector: Some(service_selector_labels.into()), - internal_traffic_policy: Some("Local".to_string()), + internal_traffic_policy, ..ServiceSpec::default() }; diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 0c979979..6873279b 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -324,6 +324,14 @@ impl v1alpha1::OpaCluster { self.metadata.name.clone() } + /// The name of the role-level load-balanced Kubernetes `Service` + pub fn server_role_service_name_load_balanced(&self) -> Option { + if let Some(service_name) = self.server_role_service_name() { + return Some(format!("{service_name}-lb")); + } + None + } + /// The fully-qualified domain name of the role-level load-balanced Kubernetes `Service` pub fn server_role_service_fqdn(&self, cluster_info: &KubernetesClusterInfo) -> Option { Some(format!( diff --git a/rust/operator-binary/src/discovery.rs b/rust/operator-binary/src/discovery.rs index 1e7f7af4..b5ed4ef4 100644 --- a/rust/operator-binary/src/discovery.rs +++ b/rust/operator-binary/src/discovery.rs @@ -4,7 +4,7 @@ use stackable_operator::{ builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder}, commons::product_image_selection::ResolvedProductImage, k8s_openapi::api::core::v1::{ConfigMap, Service}, - kube::{runtime::reflector::ObjectRef, Resource, ResourceExt}, + kube::{runtime::reflector::ObjectRef, Resource}, utils::cluster_info::KubernetesClusterInfo, }; @@ -35,27 +35,8 @@ pub enum Error { }, } -/// Builds discovery [`ConfigMap`]s for connecting to a [`v1alpha1::OpaCluster`] for all expected scenarios -pub fn build_discovery_configmaps( - owner: &impl Resource, - opa: &v1alpha1::OpaCluster, - resolved_product_image: &ResolvedProductImage, - svc: &Service, - cluster_info: &KubernetesClusterInfo, -) -> Result, Error> { - let name = owner.name_any(); - Ok(vec![build_discovery_configmap( - &name, - owner, - opa, - resolved_product_image, - svc, - cluster_info, - )?]) -} - /// Build a discovery [`ConfigMap`] containing information about how to connect to a certain [`v1alpha1::OpaCluster`] -fn build_discovery_configmap( +pub fn build_discovery_configmap( name: &str, owner: &impl Resource, opa: &v1alpha1::OpaCluster, From 0f6400fe30c508ea12339d267c1842956277eebb Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 18 Feb 2025 17:37:15 +0100 Subject: [PATCH 2/8] fix typo --- docs/modules/opa/pages/implementation-notes.adoc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/modules/opa/pages/implementation-notes.adoc b/docs/modules/opa/pages/implementation-notes.adoc index 629c73c4..7a7a62b1 100644 --- a/docs/modules/opa/pages/implementation-notes.adoc +++ b/docs/modules/opa/pages/implementation-notes.adoc @@ -14,7 +14,9 @@ OPA runs on each Node to avoid requiring network round trips for services making Local access is ensured via an https://kubernetes.io/docs/concepts/services-networking/service-traffic-policy/[`InternalTrafficPolicy`]. This means that https://kubernetes.io/docs/concepts/workloads/pods/[Pods] accessing OPA via the service discovery are routed to the OPA Pod on the same https://kubernetes.io/docs/concepts/architecture/nodes/[Node] to reduce request latency and network traffic. -This should be the default entrypoint and has the same name as the defined OPA cluster. If the `metadata.name` is `opa`, this service is called `opa`. +This should be the default entrypoint and has the same name as the defined OPA cluster. + +If the `metadata.name` is `opa`, this service is called `opa`. === Cluster Traffic Policy (load-balanced / round-robin) @@ -22,7 +24,7 @@ This service is called as the OPA cluster suffixed with `-lb`. This entrypoint c Evaluating complicated rego rules may take some time depending on the provided resources, and can be the limiting factor in e.g. bulk requests. Therefore, using this service, every Pod in the cluster is utilized to evaluate policies than instead e.g. just one. -If the `metadata.name` if `opa`, this service is called `opa-lb`. +If the `metadata.name` is `opa`, this service is called `opa-lb`. == OPA Bundle Builder From ca4f525a1dd088f9e606ac76ca426896a3b39566 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 18 Feb 2025 17:43:52 +0100 Subject: [PATCH 3/8] linter --- docs/modules/opa/pages/implementation-notes.adoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/modules/opa/pages/implementation-notes.adoc b/docs/modules/opa/pages/implementation-notes.adoc index 7a7a62b1..5674f01d 100644 --- a/docs/modules/opa/pages/implementation-notes.adoc +++ b/docs/modules/opa/pages/implementation-notes.adoc @@ -14,13 +14,13 @@ OPA runs on each Node to avoid requiring network round trips for services making Local access is ensured via an https://kubernetes.io/docs/concepts/services-networking/service-traffic-policy/[`InternalTrafficPolicy`]. This means that https://kubernetes.io/docs/concepts/workloads/pods/[Pods] accessing OPA via the service discovery are routed to the OPA Pod on the same https://kubernetes.io/docs/concepts/architecture/nodes/[Node] to reduce request latency and network traffic. -This should be the default entrypoint and has the same name as the defined OPA cluster. +This should be the default entrypoint and has the same name as the defined OPA cluster. If the `metadata.name` is `opa`, this service is called `opa`. === Cluster Traffic Policy (load-balanced / round-robin) -This service is called as the OPA cluster suffixed with `-lb`. This entrypoint can be used if latency (e.g. no network requests) is less important. +This service is called as the OPA cluster suffixed with `-lb`. This entrypoint can be used if latency (e.g. no network requests) is less important. Evaluating complicated rego rules may take some time depending on the provided resources, and can be the limiting factor in e.g. bulk requests. Therefore, using this service, every Pod in the cluster is utilized to evaluate policies than instead e.g. just one. From 5a3bf1e62a7552343b325aad5403e35ed0a47050 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 18 Feb 2025 17:45:49 +0100 Subject: [PATCH 4/8] added changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72f50b55..23904556 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ All notable changes to this project will be documented in this file. - Run a `containerdebug` process in the background of each OPA container to collect debugging information ([#666]). - Added support for OPA `1.0.x` ([#677]) and ([#687]). - Aggregate emitted Kubernetes events on the CustomResources ([#675]). +- Added role level service and discovery configmap called `-lb` with `internalTrafficPolicy` set to "Cluster" ([#688]). ### Removed @@ -23,6 +24,7 @@ All notable changes to this project will be documented in this file. [#675]: https://github.com/stackabletech/opa-operator/pull/675 [#677]: https://github.com/stackabletech/opa-operator/pull/677 [#687]: https://github.com/stackabletech/opa-operator/pull/687 +[#688]: https://github.com/stackabletech/opa-operator/pull/688 ## [24.11.1] - 2025-01-10 From 85abdc2f38f2afb43599d933811f5113450353db Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Thu, 20 Feb 2025 17:22:58 +0100 Subject: [PATCH 5/8] wip - use 3 services --- rust/operator-binary/src/controller.rs | 249 ++++++------------------- rust/operator-binary/src/crd/mod.rs | 24 ++- rust/operator-binary/src/main.rs | 1 + rust/operator-binary/src/service.rs | 185 ++++++++++++++++++ 4 files changed, 261 insertions(+), 198 deletions(-) create mode 100644 rust/operator-binary/src/service.rs diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index 85d56b0e..36369f2b 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -38,7 +38,7 @@ use stackable_operator::{ apps::v1::{DaemonSet, DaemonSetSpec}, core::v1::{ ConfigMap, EmptyDirVolumeSource, EnvVar, HTTPGetAction, Probe, SecretVolumeSource, - Service, ServiceAccount, ServicePort, ServiceSpec, + ServiceAccount, }, }, apimachinery::pkg::{apis::meta::v1::LabelSelector, util::intstr::IntOrString}, @@ -49,7 +49,7 @@ use stackable_operator::{ runtime::{controller::Action, reflector::ObjectRef}, Resource as KubeResource, ResourceExt, }, - kvp::{Label, LabelError, Labels, ObjectLabels}, + kvp::{LabelError, Labels, ObjectLabels}, logging::controller::ReconcilerError, memory::{BinaryMultiple, MemoryQuantity}, product_config_utils::{transform_all_roles_to_config, validate_all_roles_and_groups_config}, @@ -79,6 +79,7 @@ use crate::{ product_logging::{ extend_role_group_config_map, resolve_vector_aggregator_address, BundleBuilderLogLevel, }, + service::{build_discoverable_services, build_rolegroup_service, ServiceConfig}, }; pub const OPA_CONTROLLER_NAME: &str = "opacluster"; @@ -318,6 +319,9 @@ pub enum Error { AddVolumeMount { source: builder::pod::container::Error, }, + + #[snafu(display("failed to build required services"))] + BuildRequiredServices { source: crate::service::Error }, } type Result = std::result::Result; @@ -447,21 +451,58 @@ pub async fn reconcile_opa( .await .context(ResolveVectorAggregatorAddressSnafu)?; - let server_role_service = - build_server_role_service_traffic_local(opa, &resolved_product_image)?; - // required for discovery config map later - let server_role_service = cluster_resources - .add(client, server_role_service) - .await - .context(ApplyRoleServiceSnafu)?; + let required_services = vec![ + // The server-role service is the primary endpoint that should be used by clients that do + // require local access - Deprecated, kept for downwards compatibility + ServiceConfig { + name: opa + .server_role_service_name_itp_local_deprecated() + .context(RoleServiceNameNotFoundSnafu)?, + internal_traffic_policy: "Local".to_string(), + }, + // The server-role service is the primary endpoint that should be used by clients that do + // require local access + ServiceConfig { + name: opa + .server_role_service_name_itp_local() + .context(RoleServiceNameNotFoundSnafu)?, + internal_traffic_policy: "Local".to_string(), + }, + // The server-role service is the primary endpoint that should be used by clients that do + // perform internal round robin + ServiceConfig { + name: opa + .server_role_service_name_itp_cluster() + .context(RoleServiceNameNotFoundSnafu)?, + internal_traffic_policy: "Cluster".to_string(), + }, + ]; - let server_role_service_load_balanced = - build_server_role_service_traffic_cluster(opa, &resolved_product_image)?; - // required for discovery config map later - let server_role_service_load_balanced = cluster_resources - .add(client, server_role_service_load_balanced) - .await - .context(ApplyRoleServiceSnafu)?; + let services = build_discoverable_services(opa, &resolved_product_image, required_services) + .context(BuildRequiredServicesSnafu)?; + + for svc in services { + // required for discovery config map later + let role_service = cluster_resources + .add(client, svc) + .await + .context(ApplyRoleServiceSnafu)?; + + let discovery_cm = build_discovery_configmap( + &role_service.name_any(), + opa, + opa, + &resolved_product_image, + &role_service, + &client.kubernetes_cluster_info, + ) + .context(BuildDiscoveryConfigSnafu)?; + + cluster_resources + .add(client, discovery_cm) + .await + .context(ApplyDiscoveryConfigSnafu)?; + } let required_labels = cluster_resources .get_required_labels() @@ -499,7 +540,8 @@ pub async fn reconcile_opa( &merged_config, vector_aggregator_address.as_deref(), )?; - let rg_service = build_rolegroup_service(opa, &resolved_product_image, &rolegroup)?; + let rg_service = build_rolegroup_service(opa, &resolved_product_image, &rolegroup) + .context(BuildRequiredServicesSnafu)?; let rg_daemonset = build_server_rolegroup_daemonset( opa, &resolved_product_image, @@ -555,36 +597,6 @@ pub async fn reconcile_opa( .context(ApplyPatchRoleGroupDaemonSetSnafu { rolegroup })?; } - let discovery_cm_traffic_local = build_discovery_configmap( - &server_role_service.name_any(), - opa, - opa, - &resolved_product_image, - &server_role_service, - &client.kubernetes_cluster_info, - ) - .context(BuildDiscoveryConfigSnafu)?; - - cluster_resources - .add(client, discovery_cm_traffic_local) - .await - .context(ApplyDiscoveryConfigSnafu)?; - - let discovery_cm_traffic_cluster = build_discovery_configmap( - &server_role_service_load_balanced.name_any(), - opa, - opa, - &resolved_product_image, - &server_role_service_load_balanced, - &client.kubernetes_cluster_info, - ) - .context(BuildDiscoveryConfigSnafu)?; - - cluster_resources - .add(client, discovery_cm_traffic_cluster) - .await - .context(ApplyDiscoveryConfigSnafu)?; - let cluster_operation_cond_builder = ClusterOperationsConditionBuilder::new(&opa.spec.cluster_operation); @@ -605,131 +617,6 @@ pub async fn reconcile_opa( Ok(Action::await_change()) } -/// The server-role service is the primary endpoint that should be used by clients that do not perform internal load balancing, -/// including targets outside of the cluster. -pub fn build_server_role_service_traffic_local( - opa: &v1alpha1::OpaCluster, - resolved_product_image: &ResolvedProductImage, -) -> Result { - let service_name = opa - .server_role_service_name() - .context(RoleServiceNameNotFoundSnafu)?; - build_server_role_service( - opa, - resolved_product_image, - &service_name, - Some("Local".to_string()), - ) -} - -/// The server-role service is the endpoint that should be used by clients that do perform internal load balancing. -pub fn build_server_role_service_traffic_cluster( - opa: &v1alpha1::OpaCluster, - resolved_product_image: &ResolvedProductImage, -) -> Result { - let service_name = opa - .server_role_service_name_load_balanced() - .context(RoleServiceNameNotFoundSnafu)?; - build_server_role_service( - opa, - resolved_product_image, - &service_name, - Some("Cluster".to_string()), - ) -} - -fn build_server_role_service( - opa: &v1alpha1::OpaCluster, - resolved_product_image: &ResolvedProductImage, - service_name: &str, - internal_traffic_policy: Option, -) -> Result { - let role_name = v1alpha1::OpaRole::Server.to_string(); - - let metadata = ObjectMetaBuilder::new() - .name_and_namespace(opa) - .name(service_name) - .ownerreference_from_resource(opa, None, Some(true)) - .context(ObjectMissingMetadataForOwnerRefSnafu)? - .with_recommended_labels(build_recommended_labels( - opa, - &resolved_product_image.app_version_label, - &role_name, - "global", - )) - .context(ObjectMetaSnafu)? - .build(); - - let service_selector_labels = - Labels::role_selector(opa, APP_NAME, &role_name).context(BuildLabelSnafu)?; - - let service_spec = ServiceSpec { - type_: Some(opa.spec.cluster_config.listener_class.k8s_service_type()), - ports: Some(vec![ServicePort { - name: Some(APP_PORT_NAME.to_string()), - port: APP_PORT.into(), - protocol: Some("TCP".to_string()), - ..ServicePort::default() - }]), - selector: Some(service_selector_labels.into()), - internal_traffic_policy, - ..ServiceSpec::default() - }; - - Ok(Service { - metadata, - spec: Some(service_spec), - status: None, - }) -} - -/// The rolegroup [`Service`] is a headless service that allows direct access to the instances of a certain rolegroup -/// -/// This is mostly useful for internal communication between peers, or for clients that perform client-side load balancing. -fn build_rolegroup_service( - opa: &v1alpha1::OpaCluster, - resolved_product_image: &ResolvedProductImage, - rolegroup: &RoleGroupRef, -) -> Result { - let prometheus_label = - Label::try_from(("prometheus.io/scrape", "true")).context(BuildLabelSnafu)?; - - let metadata = ObjectMetaBuilder::new() - .name_and_namespace(opa) - .name(rolegroup.object_name()) - .ownerreference_from_resource(opa, None, Some(true)) - .context(ObjectMissingMetadataForOwnerRefSnafu)? - .with_recommended_labels(build_recommended_labels( - opa, - &resolved_product_image.app_version_label, - &rolegroup.role, - &rolegroup.role_group, - )) - .context(ObjectMetaSnafu)? - .with_label(prometheus_label) - .build(); - - let service_selector_labels = - Labels::role_group_selector(opa, APP_NAME, &rolegroup.role, &rolegroup.role_group) - .context(BuildLabelSnafu)?; - - let service_spec = ServiceSpec { - // Internal communication does not need to be exposed - type_: Some("ClusterIP".to_string()), - cluster_ip: Some("None".to_string()), - ports: Some(service_ports()), - selector: Some(service_selector_labels.into()), - publish_not_ready_addresses: Some(true), - ..ServiceSpec::default() - }; - - Ok(Service { - metadata, - spec: Some(service_spec), - status: None, - }) -} - /// The rolegroup [`ConfigMap`] configures the rolegroup based on the configuration given by the administrator fn build_server_rolegroup_config_map( opa: &v1alpha1::OpaCluster, @@ -1353,24 +1240,6 @@ fn build_prepare_start_command( prepare_container_args } -fn service_ports() -> Vec { - vec![ - ServicePort { - name: Some(APP_PORT_NAME.to_string()), - port: APP_PORT.into(), - protocol: Some("TCP".to_string()), - ..ServicePort::default() - }, - ServicePort { - name: Some(METRICS_PORT_NAME.to_string()), - port: 9504, // Arbitrary port number, this is never actually used anywhere - protocol: Some("TCP".to_string()), - target_port: Some(IntOrString::String(APP_PORT_NAME.to_string())), - ..ServicePort::default() - }, - ] -} - /// Creates recommended `ObjectLabels` to be used in deployed resources pub fn build_recommended_labels<'a, T>( owner: &'a T, diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 6873279b..2f68a3bb 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -319,24 +319,32 @@ impl v1alpha1::OpaCluster { }) } - /// The name of the role-level load-balanced Kubernetes `Service` - pub fn server_role_service_name(&self) -> Option { + /// DEPRECATED: The name of the role-level traffic policy local Kubernetes `Service` + pub fn server_role_service_name_itp_local_deprecated(&self) -> Option { self.metadata.name.clone() } - /// The name of the role-level load-balanced Kubernetes `Service` - pub fn server_role_service_name_load_balanced(&self) -> Option { - if let Some(service_name) = self.server_role_service_name() { - return Some(format!("{service_name}-lb")); + /// The name of the role-level traffic policy cluster Kubernetes `Service` + pub fn server_role_service_name_itp_local(&self) -> Option { + if let Some(service_name) = &self.metadata.name { + return Some(format!("{service_name}-local")); } None } - /// The fully-qualified domain name of the role-level load-balanced Kubernetes `Service` + /// The name of the role-level traffic policy cluster Kubernetes `Service` + pub fn server_role_service_name_itp_cluster(&self) -> Option { + if let Some(service_name) = &self.metadata.name { + return Some(format!("{service_name}-cluster")); + } + None + } + + /// The fully-qualified domain name of the role-level local Kubernetes `Service` pub fn server_role_service_fqdn(&self, cluster_info: &KubernetesClusterInfo) -> Option { Some(format!( "{role_service_name}.{namespace}.svc.{cluster_domain}", - role_service_name = self.server_role_service_name()?, + role_service_name = self.server_role_service_name_itp_local_deprecated()?, namespace = self.metadata.namespace.as_ref()?, cluster_domain = cluster_info.cluster_domain )) diff --git a/rust/operator-binary/src/main.rs b/rust/operator-binary/src/main.rs index 45b79690..5ad7b972 100644 --- a/rust/operator-binary/src/main.rs +++ b/rust/operator-binary/src/main.rs @@ -31,6 +31,7 @@ mod controller; mod discovery; mod operations; mod product_logging; +mod service; pub mod built_info { include!(concat!(env!("OUT_DIR"), "/built.rs")); diff --git a/rust/operator-binary/src/service.rs b/rust/operator-binary/src/service.rs new file mode 100644 index 00000000..2ea2cf9d --- /dev/null +++ b/rust/operator-binary/src/service.rs @@ -0,0 +1,185 @@ +use snafu::{ResultExt, Snafu}; +use stackable_opa_operator::crd::{v1alpha1, APP_NAME}; +use stackable_operator::{ + builder::meta::ObjectMetaBuilder, + commons::product_image_selection::ResolvedProductImage, + k8s_openapi::{ + api::core::v1::{Service, ServicePort, ServiceSpec}, + apimachinery::pkg::util::intstr::IntOrString, + }, + kube::runtime::reflector::ObjectRef, + kvp::{Label, LabelError, Labels}, + role_utils::RoleGroupRef, +}; + +use crate::controller::{build_recommended_labels, APP_PORT, APP_PORT_NAME, METRICS_PORT_NAME}; + +type Result = std::result::Result; + +#[derive(Snafu, Debug)] +pub enum Error { + #[snafu(display("object {opa} is missing metadata to build owner reference"))] + ObjectMissingMetadataForOwnerRef { + source: stackable_operator::builder::meta::Error, + opa: ObjectRef, + }, + + #[snafu(display("object has no name associated"))] + NoName, + + #[snafu(display("object has no namespace associated"))] + NoNamespace, + + #[snafu(display("failed to build ConfigMap"))] + BuildConfigMap { + source: stackable_operator::builder::configmap::Error, + }, + + #[snafu(display("failed to build object meta data"))] + ObjectMeta { + source: stackable_operator::builder::meta::Error, + }, + + #[snafu(display("failed to build label"))] + BuildLabel { source: LabelError }, +} + +pub struct ServiceConfig { + pub name: String, + pub internal_traffic_policy: String, +} + +pub fn build_discoverable_services( + opa: &v1alpha1::OpaCluster, + resolved_product_image: &ResolvedProductImage, + service_configs: Vec, +) -> Result> { + let mut services = vec![]; + + // discoverable role services + for sc in service_configs { + let service_name = sc.name; + services.push(build_server_role_service( + opa, + resolved_product_image, + &service_name, + Some(sc.internal_traffic_policy.to_string()), + )?); + } + + Ok(services) +} + +fn build_server_role_service( + opa: &v1alpha1::OpaCluster, + resolved_product_image: &ResolvedProductImage, + service_name: &str, + internal_traffic_policy: Option, +) -> Result { + let role_name = v1alpha1::OpaRole::Server.to_string(); + + let metadata = ObjectMetaBuilder::new() + .name_and_namespace(opa) + .name(service_name) + .ownerreference_from_resource(opa, None, Some(true)) + .context(ObjectMissingMetadataForOwnerRefSnafu { + opa: ObjectRef::from_obj(opa), + })? + .with_recommended_labels(build_recommended_labels( + opa, + &resolved_product_image.app_version_label, + &role_name, + "global", + )) + .context(ObjectMetaSnafu)? + .build(); + + let service_selector_labels = + Labels::role_selector(opa, APP_NAME, &role_name).context(BuildLabelSnafu)?; + + let service_spec = ServiceSpec { + type_: Some(opa.spec.cluster_config.listener_class.k8s_service_type()), + ports: Some(vec![ServicePort { + name: Some(APP_PORT_NAME.to_string()), + port: APP_PORT.into(), + protocol: Some("TCP".to_string()), + ..ServicePort::default() + }]), + selector: Some(service_selector_labels.into()), + internal_traffic_policy, + ..ServiceSpec::default() + }; + + Ok(Service { + metadata, + spec: Some(service_spec), + status: None, + }) +} + +/// The rolegroup [`Service`] is a headless service that allows direct access to the instances of a certain rolegroup +/// +/// This is mostly useful for internal communication between peers, or for clients that perform client-side load balancing. +pub fn build_rolegroup_service( + opa: &v1alpha1::OpaCluster, + resolved_product_image: &ResolvedProductImage, + rolegroup: &RoleGroupRef, +) -> Result { + let prometheus_label = + Label::try_from(("prometheus.io/scrape", "true")).context(BuildLabelSnafu)?; + + let metadata = ObjectMetaBuilder::new() + .name_and_namespace(opa) + .name(rolegroup.object_name()) + .ownerreference_from_resource(opa, None, Some(true)) + .context(ObjectMissingMetadataForOwnerRefSnafu { + opa: ObjectRef::from_obj(opa), + })? + .with_recommended_labels(build_recommended_labels( + opa, + &resolved_product_image.app_version_label, + &rolegroup.role, + &rolegroup.role_group, + )) + .context(ObjectMetaSnafu)? + .with_label(prometheus_label) + .build(); + + let service_selector_labels = + Labels::role_group_selector(opa, APP_NAME, &rolegroup.role, &rolegroup.role_group) + .context(BuildLabelSnafu)?; + + let service_spec = ServiceSpec { + // Internal communication does not need to be exposed + type_: Some("ClusterIP".to_string()), + cluster_ip: Some("None".to_string()), + ports: Some(service_ports()), + selector: Some(service_selector_labels.into()), + publish_not_ready_addresses: Some(true), + ..ServiceSpec::default() + }; + + Ok(Service { + metadata, + spec: Some(service_spec), + status: None, + }) +} + +fn service_ports() -> Vec { + vec![ + ServicePort { + name: Some(APP_PORT_NAME.to_string()), + port: APP_PORT.into(), + protocol: Some("TCP".to_string()), + ..ServicePort::default() + }, + ServicePort { + name: Some(METRICS_PORT_NAME.to_string()), + port: 9504, // Arbitrary port number, this is never actually used anywhere + protocol: Some("TCP".to_string()), + target_port: Some(IntOrString::String(APP_PORT_NAME.to_string())), + ..ServicePort::default() + }, + ] +} From b044bdf8ee386f9f0fae6da797542889b916aaf8 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Fri, 21 Feb 2025 10:08:32 +0100 Subject: [PATCH 6/8] cleanup & docs --- CHANGELOG.md | 3 +- .../opa/pages/implementation-notes.adoc | 13 ++--- .../opa/pages/reference/discovery.adoc | 52 +++++++++++++++++-- rust/operator-binary/src/controller.rs | 5 +- rust/operator-binary/src/service.rs | 23 ++------ 5 files changed, 61 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23904556..460d344e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,8 @@ All notable changes to this project will be documented in this file. - Run a `containerdebug` process in the background of each OPA container to collect debugging information ([#666]). - Added support for OPA `1.0.x` ([#677]) and ([#687]). - Aggregate emitted Kubernetes events on the CustomResources ([#675]). -- Added role level service and discovery configmap called `-lb` with `internalTrafficPolicy` set to "Cluster" ([#688]). +- Added role level services and discovery configmaps called `-local` with `internalTrafficPolicy` set to `Local` + and `-cluster` with `internalTrafficPolicy` set to `Cluster` ([#688]). ### Removed diff --git a/docs/modules/opa/pages/implementation-notes.adoc b/docs/modules/opa/pages/implementation-notes.adoc index 5674f01d..e0211286 100644 --- a/docs/modules/opa/pages/implementation-notes.adoc +++ b/docs/modules/opa/pages/implementation-notes.adoc @@ -5,7 +5,7 @@ but should not be required reading for regular use. == OPA replica per node -Since OPA is deployed as a DaemonSet and runs on each Node, two entrypoint Services are defined. +OPA is deployed as a DaemonSet and runs on each Node. two entrypoint Services are defined. === Local Traffic Policy @@ -16,15 +16,16 @@ This means that https://kubernetes.io/docs/concepts/workloads/pods/[Pods] access This should be the default entrypoint and has the same name as the defined OPA cluster. -If the `metadata.name` is `opa`, this service is called `opa`. +If the `metadata.name` is `opa`, this service is called `opa-local`. -=== Cluster Traffic Policy (load-balanced / round-robin) +=== Cluster Traffic Policy (round-robin) -This service is called as the OPA cluster suffixed with `-lb`. This entrypoint can be used if latency (e.g. no network requests) is less important. +This service is called as the OPA cluster suffixed with `-cluster`. This entrypoint can be used if latency (e.g. no network requests) is less important. Evaluating complicated rego rules may take some time depending on the provided resources, and can be the limiting factor in e.g. bulk requests. -Therefore, using this service, every Pod in the cluster is utilized to evaluate policies than instead e.g. just one. +Therefore, using this service, every Pod in the cluster is utilized to evaluate policies (via round robin). This allows better parallelism when +evaluating policies, but results in network roundtrips. -If the `metadata.name` is `opa`, this service is called `opa-lb`. +If the `metadata.name` is `opa`, this service is called `opa-cluster`. == OPA Bundle Builder diff --git a/docs/modules/opa/pages/reference/discovery.adoc b/docs/modules/opa/pages/reference/discovery.adoc index 27342827..5025c570 100644 --- a/docs/modules/opa/pages/reference/discovery.adoc +++ b/docs/modules/opa/pages/reference/discovery.adoc @@ -26,14 +26,15 @@ metadata: spec: [...] ---- -<1> The name of the OPA cluster, which is also the name of the created discovery ConfigMap. -<2> The namespace of the discovery ConfigMap. +<1> The name of the OPA cluster, which is used in the created discovery ConfigMaps. +<2> The namespace of the discovery ConfigMaps. -The resulting discovery ConfigMap is `{namespace}/{clusterName}`. +Currently, three discovery ConfigMaps are provided. -== Contents +=== (DEPRECATED) Internal Traffic Policy `Local` -The `{namespace}/{clusterName}` discovery ConfigMap contains the following fields where `{clusterName}` represents the name and `{namespace}` the namespace of the cluster: +The discovery ConfigMap `{namespace}/{clusterName}` contains the following fields where `{clusterName}` represents the name and `{namespace}` the namespace of the cluster. +This is deprecated and only kept for backwards compatibitliy. Users are adviced to switch to `{namespace}/{clusterName}-local`, which is the identical replacement. `OPA`:: ==== @@ -49,3 +50,44 @@ In order to query policies you have to configure your product and its OPA URL as [subs="attributes"] http://{clusterName}.{namespace}.svc.cluster.local:8081/v1/data/{packageName}/{policyName} ==== + +=== Internal Traffic Policy `Local` + +The discovery ConfigMap `{namespace}/{clusterName}-local` contains the following fields where `{clusterName}-local` represents the name and `{namespace}` the namespace of the cluster. +Using this discovery service, requests from one Node will always reach the OPA Pod on the same Node. This allows for low latency authorization queriers. + +`OPA`:: +==== +A connection string for cluster internal OPA requests. +Provided the cluster example above, the connection string is created as follows: + +[subs="attributes"] + http://{clusterName}-local.{namespace}.svc.cluster.local:8081/ + +This connection string points to the base URL (and web UI) of the OPA cluster. +In order to query policies you have to configure your product and its OPA URL as follows, given the bundle package name `{packageName}` and the policy name `{policyName}`: + +[subs="attributes"] + http://{clusterName}-local.{namespace}.svc.cluster.local:8081/v1/data/{packageName}/{policyName} +==== + +=== Internal Traffic Policy `Cluster` + +The discovery ConfigMap `{namespace}/{clusterName}-cluster` contains the following fields where `{clusterName}-cluster` represents the name and `{namespace}` the namespace of the cluster. +Using this discovery service, requests to OPA are distributed over all available OPA Pods, improving parallelism when evaluating policies but slightly increasing the latency of each single query +to due additional network requests. + +`OPA`:: +==== +A connection string for cluster internal OPA requests. +Provided the cluster example above, the connection string is created as follows: + +[subs="attributes"] + http://{clusterName}-cluster.{namespace}.svc.cluster.local:8081/ + +This connection string points to the base URL (and web UI) of the OPA cluster. +In order to query policies you have to configure your product and its OPA URL as follows, given the bundle package name `{packageName}` and the policy name `{policyName}`: + +[subs="attributes"] + http://{clusterName}-cluster.{namespace}.svc.cluster.local:8081/v1/data/{packageName}/{policyName} +==== diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index 36369f2b..70451cea 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -158,9 +158,6 @@ pub enum Error { source: error_boundary::InvalidObject, }, - #[snafu(display("object does not define meta name"))] - NoName, - #[snafu(display("internal operator failure"))] InternalOperatorFailure { source: stackable_opa_operator::crd::Error, @@ -453,7 +450,7 @@ pub async fn reconcile_opa( let required_services = vec![ // The server-role service is the primary endpoint that should be used by clients that do - // require local access - Deprecated, kept for downwards compatibility + // require local access - deprecated, kept for downwards compatibility ServiceConfig { name: opa .server_role_service_name_itp_local_deprecated() diff --git a/rust/operator-binary/src/service.rs b/rust/operator-binary/src/service.rs index 2ea2cf9d..b065ba38 100644 --- a/rust/operator-binary/src/service.rs +++ b/rust/operator-binary/src/service.rs @@ -27,14 +27,6 @@ pub enum Error { #[snafu(display("object has no name associated"))] NoName, - #[snafu(display("object has no namespace associated"))] - NoNamespace, - - #[snafu(display("failed to build ConfigMap"))] - BuildConfigMap { - source: stackable_operator::builder::configmap::Error, - }, - #[snafu(display("failed to build object meta data"))] ObjectMeta { source: stackable_operator::builder::meta::Error, @@ -58,13 +50,7 @@ pub fn build_discoverable_services( // discoverable role services for sc in service_configs { - let service_name = sc.name; - services.push(build_server_role_service( - opa, - resolved_product_image, - &service_name, - Some(sc.internal_traffic_policy.to_string()), - )?); + services.push(build_server_role_service(opa, resolved_product_image, sc)?); } Ok(services) @@ -73,14 +59,13 @@ pub fn build_discoverable_services( fn build_server_role_service( opa: &v1alpha1::OpaCluster, resolved_product_image: &ResolvedProductImage, - service_name: &str, - internal_traffic_policy: Option, + service_config: ServiceConfig, ) -> Result { let role_name = v1alpha1::OpaRole::Server.to_string(); let metadata = ObjectMetaBuilder::new() .name_and_namespace(opa) - .name(service_name) + .name(service_config.name) .ownerreference_from_resource(opa, None, Some(true)) .context(ObjectMissingMetadataForOwnerRefSnafu { opa: ObjectRef::from_obj(opa), @@ -106,7 +91,7 @@ fn build_server_role_service( ..ServicePort::default() }]), selector: Some(service_selector_labels.into()), - internal_traffic_policy, + internal_traffic_policy: Some(service_config.internal_traffic_policy), ..ServiceSpec::default() }; From 20275727ae911e905748604a70b1943b32532ace Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Fri, 21 Feb 2025 10:15:21 +0100 Subject: [PATCH 7/8] attempt to fix linter --- docs/modules/opa/pages/reference/discovery.adoc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/modules/opa/pages/reference/discovery.adoc b/docs/modules/opa/pages/reference/discovery.adoc index 5025c570..b67c8136 100644 --- a/docs/modules/opa/pages/reference/discovery.adoc +++ b/docs/modules/opa/pages/reference/discovery.adoc @@ -31,7 +31,7 @@ spec: Currently, three discovery ConfigMaps are provided. -=== (DEPRECATED) Internal Traffic Policy `Local` +=== (DEPRECATED) Internal Traffic Policy `Local` The discovery ConfigMap `{namespace}/{clusterName}` contains the following fields where `{clusterName}` represents the name and `{namespace}` the namespace of the cluster. This is deprecated and only kept for backwards compatibitliy. Users are adviced to switch to `{namespace}/{clusterName}-local`, which is the identical replacement. @@ -51,7 +51,7 @@ In order to query policies you have to configure your product and its OPA URL as http://{clusterName}.{namespace}.svc.cluster.local:8081/v1/data/{packageName}/{policyName} ==== -=== Internal Traffic Policy `Local` +=== Internal Traffic Policy `Local` The discovery ConfigMap `{namespace}/{clusterName}-local` contains the following fields where `{clusterName}-local` represents the name and `{namespace}` the namespace of the cluster. Using this discovery service, requests from one Node will always reach the OPA Pod on the same Node. This allows for low latency authorization queriers. @@ -71,7 +71,7 @@ In order to query policies you have to configure your product and its OPA URL as http://{clusterName}-local.{namespace}.svc.cluster.local:8081/v1/data/{packageName}/{policyName} ==== -=== Internal Traffic Policy `Cluster` +=== Internal Traffic Policy `Cluster` The discovery ConfigMap `{namespace}/{clusterName}-cluster` contains the following fields where `{clusterName}-cluster` represents the name and `{namespace}` the namespace of the cluster. Using this discovery service, requests to OPA are distributed over all available OPA Pods, improving parallelism when evaluating policies but slightly increasing the latency of each single query From a35621d9e65a430e6ac6e39e05aeb1359f1cb9fe Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Fri, 21 Feb 2025 10:18:53 +0100 Subject: [PATCH 8/8] fix doc typo --- docs/modules/opa/pages/implementation-notes.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/modules/opa/pages/implementation-notes.adoc b/docs/modules/opa/pages/implementation-notes.adoc index e0211286..5c41bcc5 100644 --- a/docs/modules/opa/pages/implementation-notes.adoc +++ b/docs/modules/opa/pages/implementation-notes.adoc @@ -5,7 +5,7 @@ but should not be required reading for regular use. == OPA replica per node -OPA is deployed as a DaemonSet and runs on each Node. two entrypoint Services are defined. +OPA is deployed as a DaemonSet and runs on each Node. The following entrypoint Services are defined: === Local Traffic Policy