feat!: Allow configuring port numbers and validate config properties (#43)

* feat!: Allow configuring port numbers and validate config properties

* ix tests

* changelog

* typo

* finally...

Author: sbernauer

CHANGELOG.md

@@ -4,11 +4,17 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+### Added
+
+- Added a configuration to specify the port numbers for `http`, `https` and `metrics` ([#43]).
+
 ### Changed
 
+- BREAKING: Ensure no unknown config properties have been set. This is to make the user aware that what he tried to configure is not a valid configuration. You may need to adapt your configuration and remove any unknown properties ([#43]).
 - Bump dependencies, such as `opentelemetry` 0.23 -> 0.24, `kube` 0.92 -> 0.93 and `redis` 0.25 -> 0.26 ([#41]).
 
 [#41]: https://github.com/stackabletech/trino-lb/pull/41
+[#43]: https://github.com/stackabletech/trino-lb/pull/43
 
 ## [0.2.3] - 2024-07-01
 

deploy/helm/trino-lb/configs/trino-lb-config.yaml

@@ -1,7 +1,3 @@
-_: &common-credentials
-  username: admin
-  password: adminadmin
-
 trinoLb:
   externalAddress: https://5.250.179.64:8443
   tls:
@@ -39,7 +35,9 @@ trinoClusterGroups:
     trinoClusters:
       - name: trino-s-1
         endpoint: https://trino-s-1-coordinator-default.default.svc.cluster.local:8443
-        credentials: *common-credentials
+        credentials: &common-credentials
+          username: admin
+          password: adminadmin
   m:
     maxRunningQueries: 2
     autoscaling:
@@ -69,7 +67,7 @@ routers:
   # This list will be walked top to bottom and the first router making a decision will choose the target trinoClusterGroup.
   # If no router makes a decision the query will fall through to the "routingFallback".
   - trinoRoutingGroupHeader:
-      header: X-Trino-Routing-Group
+      headerName: X-Trino-Routing-Group
   - pythonScript:
       script: |
         # Tested using Python 3.11

docs/routing/TrinoRoutingGroupHeaderRouter.md

@@ -17,5 +17,5 @@ Additionally you can configure the name of the HTTP header in case it differs fr
 ```yaml
 routers:
   - trinoRoutingGroupHeader:
-      header: X-My-Custom-Routing-Header # optional, defaults to X-Trino-Routing-Group
+      headerName: X-My-Custom-Routing-Header # optional, defaults to X-Trino-Routing-Group
 ```

example-configs/dev-local-with-tracing.yaml

@@ -1,7 +1,3 @@
-_: &common-credentials
-  username: admin
-  password: adminadmin
-
 trinoLb:
   externalAddress: https://127.0.0.1:8443
   tls:
@@ -23,14 +19,21 @@ trinoLb:
     OTEL_EXPORTER_OTLP_PROTOCOL: Grpc
     # In case endpoint and protocol are not set here, they will still be read from the env vars
     # OTEL_EXPORTER_OTLP_ENDPOINT and OTEL_EXPORTER_OTLP_PROTOCOL
+  ports:
+    # Default values
+    http: 8080
+    https: 8443
+    metrics: 9090
 
 trinoClusterGroups:
   s:
     maxRunningQueries: 1
     trinoClusters:
       - name: trino-s-1
         endpoint: https://5.250.181.98:8443
-        credentials: *common-credentials
+        credentials: &common-credentials
+          username: admin
+          password: adminadmin
   m:
     maxRunningQueries: 1
     trinoClusters:
@@ -43,7 +46,7 @@ routers:
   # This list will be walked top to bottom and the first router making a decision will choose the target trinoClusterGroup.
   # If no router makes a decision the query will fall through to the "routingFallback".
   - trinoRoutingGroupHeader:
-      header: X-Trino-Routing-Group
+      headerName: X-Trino-Routing-Group
   - pythonScript:
       script: |
         # Tested using Python 3.11

example-configs/ha-redis-autoscaling-stackable.yaml

@@ -1,7 +1,3 @@
-_: &common-credentials
-  username: admin
-  password: adminadmin
-
 trinoLb:
   externalAddress: https://5.250.179.64:8443
   tls:
@@ -39,7 +35,9 @@ trinoClusterGroups:
     trinoClusters:
       - name: trino-s-1
         endpoint: https://trino-s-1-coordinator-default.default.svc.cluster.local:8443
-        credentials: *common-credentials
+        credentials: &common-credentials
+          username: admin
+          password: adminadmin
   m:
     maxRunningQueries: 2
     autoscaling:
@@ -69,7 +67,7 @@ routers:
   # This list will be walked top to bottom and the first router making a decision will choose the target trinoClusterGroup.
   # If no router makes a decision the query will fall through to the "routingFallback".
   - trinoRoutingGroupHeader:
-      header: X-Trino-Routing-Group
+      headerName: X-Trino-Routing-Group
   - pythonScript:
       script: |
         # Tested using Python 3.11

trino-lb-core/src/config.rs

@@ -28,19 +28,26 @@ pub enum Error {
 }
 
 #[derive(Clone, Debug, Deserialize)]
-#[serde(rename_all = "camelCase")]
+// We want to fail on unknown config properties (as Trino is doing as well) to make the user aware that what he tried to
+// configure is not a valid configuration.
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct Config {
     pub trino_lb: TrinoLbConfig,
+
     pub trino_cluster_groups: HashMap<String, TrinoClusterGroupConfig>,
+
     #[serde(default)]
     pub trino_cluster_groups_ignore_cert: bool,
+
     pub routers: Vec<RoutingConfig>,
+
     pub routing_fallback: String,
+
     pub cluster_autoscaler: Option<ScalerConfig>,
 }
 
 #[derive(Clone, Debug, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct TrinoLbConfig {
     pub external_address: Url,
 
@@ -56,14 +63,17 @@ pub struct TrinoLbConfig {
     pub refresh_query_counter_interval: Duration,
 
     pub tracing: Option<TrinoLbTracingConfig>,
+
+    #[serde(default)]
+    pub ports: TrinoLbPortsConfig,
 }
 
 fn default_refresh_query_counter_interval() -> Duration {
     Duration::from_secs(60)
 }
 
 #[derive(Clone, Debug, Default, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct TrinoLbTlsConfig {
     #[serde(default)]
     pub enabled: bool,
@@ -73,7 +83,7 @@ pub struct TrinoLbTlsConfig {
 }
 
 #[derive(Clone, Debug, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct TrinoLbTracingConfig {
     #[serde(default)]
     pub enabled: bool,
@@ -86,15 +96,54 @@ pub struct TrinoLbTracingConfig {
 }
 
 #[derive(Clone, Debug, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
+pub struct TrinoLbPortsConfig {
+    #[serde(default = "TrinoLbPortsConfig::default_http_port")]
+    pub http: u16,
+
+    #[serde(default = "TrinoLbPortsConfig::default_https_port")]
+    pub https: u16,
+
+    #[serde(default = "TrinoLbPortsConfig::default_metrics_port")]
+    pub metrics: u16,
+}
+
+impl TrinoLbPortsConfig {
+    /// Same port Trino is using by default
+    fn default_http_port() -> u16 {
+        8080
+    }
+
+    /// Same port Trino is using by default
+    fn default_https_port() -> u16 {
+        8443
+    }
+
+    fn default_metrics_port() -> u16 {
+        9090
+    }
+}
+
+impl Default for TrinoLbPortsConfig {
+    fn default() -> Self {
+        Self {
+            http: Self::default_http_port(),
+            https: Self::default_https_port(),
+            metrics: Self::default_metrics_port(),
+        }
+    }
+}
+
+#[derive(Clone, Debug, Deserialize)]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub enum PersistenceConfig {
     InMemory {},
     Redis(RedisConfig),
     Postgres(PostgresConfig),
 }
 
 #[derive(Clone, Debug, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct RedisConfig {
     pub endpoint: Url,
 
@@ -103,7 +152,7 @@ pub struct RedisConfig {
 }
 
 #[derive(Clone, Debug, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct PostgresConfig {
     pub url: Url,
 
@@ -116,30 +165,30 @@ fn default_max_connections() -> u32 {
 }
 
 #[derive(Clone, Debug, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct TrinoClusterGroupConfig {
     pub max_running_queries: u64,
     pub autoscaling: Option<TrinoClusterGroupAutoscalingConfig>,
     pub trino_clusters: Vec<TrinoClusterConfig>,
 }
 
 #[derive(Clone, Debug, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct TrinoClusterConfig {
     pub name: String,
     pub endpoint: Url,
     pub credentials: TrinoClusterCredentialsConfig,
 }
 
 #[derive(Clone, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct TrinoClusterCredentialsConfig {
     pub username: String,
     pub password: String,
 }
 
 #[derive(Clone, Debug, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct TrinoClusterGroupAutoscalingConfig {
     pub upscale_queued_queries_threshold: u64,
     pub downscale_running_queries_percentage_threshold: u64,
@@ -149,7 +198,7 @@ pub struct TrinoClusterGroupAutoscalingConfig {
 }
 
 #[derive(Clone, Debug, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct MinClustersConfig {
     pub time_utc: String,
     pub weekdays: String,
@@ -166,7 +215,7 @@ impl Debug for TrinoClusterCredentialsConfig {
 }
 
 #[derive(Clone, Debug, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub enum RoutingConfig {
     ExplainCosts(ExplainCostsRouterConfig),
     TrinoRoutingGroupHeader(TrinoRoutingGroupHeaderRouterConfig),
@@ -175,15 +224,15 @@ pub enum RoutingConfig {
 }
 
 #[derive(Clone, Debug, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct ExplainCostsRouterConfig {
     pub trino_cluster_to_run_explain_query: TrinoClientConfig,
 
     pub targets: Vec<ExplainCostTargetConfig>,
 }
 
 #[derive(Clone, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct TrinoClientConfig {
     pub endpoint: Url,
     #[serde(default)]
@@ -204,6 +253,8 @@ impl Debug for TrinoClientConfig {
 }
 
 #[derive(Clone, Debug, Deserialize)]
+// #[serde(flatten)] is not supported in combination with structs that use deny_unknown_fields. Neither the outer nor
+// inner flattened struct should use that attribute.
 #[serde(rename_all = "camelCase")]
 pub struct ExplainCostTargetConfig {
     #[serde(flatten)]
@@ -212,7 +263,7 @@ pub struct ExplainCostTargetConfig {
 }
 
 #[derive(Clone, Debug, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct TrinoRoutingGroupHeaderRouterConfig {
     #[serde(default = "default_trino_routing_group_header")]
     pub header_name: String,
@@ -223,12 +274,14 @@ fn default_trino_routing_group_header() -> String {
 }
 
 #[derive(Clone, Debug, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct PythonScriptRouterConfig {
     pub script: String,
 }
 
 #[derive(Clone, Debug, Deserialize)]
+// #[serde(flatten)] is not supported in combination with structs that use deny_unknown_fields. Neither the outer nor
+// inner flattened struct should use that attribute.
 #[serde(rename_all = "camelCase")]
 pub struct ClientTagsRouterConfig {
     #[serde(flatten)]
@@ -244,19 +297,19 @@ pub enum TagMatchingStrategy {
 }
 
 #[derive(Clone, Debug, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub enum ScalerConfig {
     Stackable(StackableScalerConfig),
 }
 
 #[derive(Clone, Debug, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct StackableScalerConfig {
     pub clusters: HashMap<TrinoClusterName, StackableCluster>,
 }
 
 #[derive(Clone, Debug, Deserialize)]
-#[serde(rename_all = "camelCase")]
+#[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct StackableCluster {
     pub name: String,
     pub namespace: String,