Automatically prepend needed localhost:127 to Derby JDBC connection strings

Author: sbernauer

Cargo.lock

@@ -34,4 +34,4 @@ tracing = "0.1"
 
 [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 = "feat/derby-host-part" }

Cargo.nix

@@ -259,8 +259,7 @@ deepStorage:
     configMapName: druid-hdfs
     directory: /druid
 metadataDatabase:
-  derby:
-    location: "//localhost:1527/var/druid/metadata.db"
+  derby: {}
 zookeeperConfigMapName: zk-config-map
     "#;
 

Cargo.toml

@@ -1,12 +1,11 @@
-use std::ops::Deref;
-
 use serde::{Deserialize, Serialize};
 use stackable_operator::{
     database_connections::{
+        self, TemplatingMechanism,
         databases::{
             derby::DerbyConnection, mysql::MysqlConnection, postgresql::PostgresqlConnection,
         },
-        drivers::jdbc::JdbcDatabaseConnection,
+        drivers::jdbc::{JdbcDatabaseConnection, JdbcDatabaseConnectionDetails},
     },
     schemars::{self, JsonSchema},
 };
@@ -47,14 +46,74 @@ impl MetadataDatabaseConnection {
     }
 }
 
-impl Deref for MetadataDatabaseConnection {
-    type Target = dyn JdbcDatabaseConnection;
-
-    fn deref(&self) -> &Self::Target {
+impl JdbcDatabaseConnection for MetadataDatabaseConnection {
+    /// We do *not* implement [`std::ops::Deref`]` for [`MetadataDatabaseConnection`], as we need
+    /// some special handling for Derby.
+    fn jdbc_connection_details_with_templating(
+        &self,
+        unique_database_name: &str,
+        templating_mechanism: &TemplatingMechanism,
+    ) -> Result<JdbcDatabaseConnectionDetails, database_connections::Error> {
         match self {
-            Self::Postgresql(p) => p,
-            Self::Mysql(m) => m,
-            Self::Derby(d) => d,
+            Self::Postgresql(p) => p.jdbc_connection_details_with_templating(
+                unique_database_name,
+                templating_mechanism,
+            ),
+            Self::Mysql(m) => m.jdbc_connection_details_with_templating(
+                unique_database_name,
+                templating_mechanism,
+            ),
+            Self::Derby(d) => {
+                // According to the [Druid docs](https://druid.apache.org/docs/latest/design/metadata-storage/#derby)
+                // we should configure something like
+                // `jdbc:derby://localhost:1527//opt/var/druid_state/derby;create=true`
+                // instead of the usual `jdbc:derby:/opt/var/druid_state/derby;create=true`.
+                //
+                // It looks like Druid always starts Derby at `localhost:1527`, regardless of what we configure here,
+                // so we can hardcode it here.
+                d.jdbc_connection_details_with_host_part(unique_database_name, "localhost:1527")
+            }
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use rstest::rstest;
+    use stackable_operator::utils::yaml_from_str_singleton_map;
+
+    use super::*;
+
+    #[rstest]
+    #[case::postgres(
+        "postgresql:
+  host: druid-postgresql
+  database: druid
+  credentialsSecretName: druid-credentials",
+        "jdbc:postgresql://druid-postgresql:5432/druid"
+    )]
+    #[case::derby(
+        "derby: {}",
+        "jdbc:derby://localhost:1527//tmp/derby/METADATA/derby.db;create=true"
+    )]
+    #[case::derby_custom_location(
+        "derby:
+  location: /user/provided.db",
+        "jdbc:derby://localhost:1527//user/provided.db;create=true"
+    )]
+    fn test_connection_url(
+        #[case] database_connection_yaml: &str,
+        #[case] expected_connection_url: &str,
+    ) {
+        let database_connection: MetadataDatabaseConnection =
+            yaml_from_str_singleton_map(database_connection_yaml).expect("invalid YAML");
+
+        let jdbc_connection_details = database_connection
+            .jdbc_connection_details("METADATA")
+            .expect("failed to get JDBC connection details");
+        assert_eq!(
+            jdbc_connection_details.connection_url.as_str(),
+            expected_connection_url
+        );
+    }
+}

crate-hashes.json

@@ -796,6 +796,15 @@ impl v1alpha1::DruidCluster {
     }
 }
 
+#[cfg(test)]
+impl stackable_operator::versioned::test_utils::RoundtripTestData for v1alpha1::DruidClusterSpec {
+    fn roundtrip_test_data() -> Vec<Self> {
+        todo!(
+            "Return some test data according to https://stackabletech.github.io/operator-rs/stackable_versioned/test_utils/trait.RoundtripTestData.html"
+        )
+    }
+}
+
 #[derive(
     Clone,
     Debug,

rust/operator-binary/src/crd/authentication.rs

@@ -41,8 +41,7 @@ deepStorage:
     configMapName: druid-hdfs
     directory: /druid
 metadataDatabase:
-  derby:
-    location: "//localhost:1527/var/druid/metadata.db"
+  derby: {}
 zookeeperConfigMapName: zk-config-map
     "#;
 

rust/operator-binary/src/crd/database.rs

@@ -32,6 +32,7 @@ use stackable_operator::{
     },
     constants::RESTART_CONTROLLER_ENABLED_LABEL,
     crd::s3,
+    database_connections::drivers::jdbc::JdbcDatabaseConnection as _,
     k8s_openapi::{
         DeepMerge,
         api::{

rust/operator-binary/src/crd/mod.rs

@@ -28,8 +28,7 @@ spec:
         configMapName: druid-hdfs
         directory: /druid
     metadataDatabase:
-      derby:
-        location: "//localhost:1527/var/druid/metadata.db"
+      derby: {}
 {% if lookup('env', 'VECTOR_AGGREGATOR') %}
     vectorAggregatorConfigMapName: vector-aggregator-discovery
 {% endif %}