mk-sql: use cluster name with odbc

When a SQL Server instance belongs to a Windows Server Failover Cluster
(WSFC), ODBC connections must use the cluster's Virtual Network Name
(VNN) as the SERVER parameter instead of the physical hostname.

Send downstream cluster_name through `create_odbc_client` and `make_connection_string`
so that the VNN takes priority over hostname when present. Affects both
SqlInstance::connect (established instances) and get_custom_instance_builder.

CMK-32976

Change-Id: Ie907123939966f36a4a274bf462fd5c5bd55ccd6

Author: s-kipnis

packages/mk-sql/src/ms_sql/instance.rs

@@ -481,7 +481,13 @@ impl SqlInstance {
         if self.tcp {
             create_tcp_client(endpoint, database, self.port()).await
         } else {
-            create_odbc_client(endpoint.conn(), &self.name, database).await
+            create_odbc_client(
+                endpoint.conn(),
+                self.cluster_name.as_ref(),
+                &self.name,
+                database,
+            )
+            .await
         }
     }
 
@@ -1383,21 +1389,24 @@ pub async fn create_tcp_client(
 
 pub async fn create_odbc_client(
     connection: &Connection,
+    cluster_name: Option<&ClusterName>,
     instance_name: &InstanceName,
     database: Option<String>,
 ) -> Result<UniClient> {
     let hostname = connection.hostname();
     #[cfg(unix)]
     anyhow::bail!(
-        "ODBC Not supported `{}` `{}` db:`{:?}`",
+        "ODBC Not supported `{}` `{}` cluster:`{:?}` db:`{:?}`",
         hostname,
         instance_name,
+        cluster_name,
         database
     );
     #[cfg(windows)]
     {
         let connection_string = odbc::make_connection_string(
             Some(&hostname),
+            cluster_name,
             instance_name,
             database.as_deref(),
             None,
@@ -2197,8 +2206,11 @@ async fn get_custom_instance_builder(
     let auth = endpoint.auth();
     let conn = endpoint.conn();
     if is_local_endpoint(auth, conn) && !is_use_tcp(instance_name, auth, conn) {
-        if let Ok(mut client) = create_odbc_client(conn, instance_name, None).await {
-            log::debug!("Trying to connect to `{instance_name}` using ODBC");
+        log::debug!("Trying to connect to `{instance_name}` using ODBC");
+        if let Ok(mut client) =
+            create_odbc_client(conn, builder.cluster_name.as_ref(), instance_name, None).await
+        {
+            log::debug!("Connected to `{instance_name}` using ODBC");
             let b = obtain_properties(&mut client, instance_name)
                 .await
                 .map(|p| to_instance_builder(builder.to_owned(), endpoint, &p));
@@ -2280,7 +2292,7 @@ async fn find_custom_instance(
                 .map(|p| {
                     // Fresh builder: this function has no access to the original customized
                     // builder, so customizations (cluster, alias, piggyback) are not carried
-                    // through here.
+                    // through here. Accepted limitation for this port-scan fallback path.
                     to_instance_builder(SqlInstanceBuilder::new(), endpoint, &p).port(detected_port)
                 })
         } else {

packages/mk-sql/src/platform.rs

@@ -268,6 +268,7 @@ mod tests {
 #[cfg(windows)]
 pub mod odbc {
     use super::Block;
+    use crate::types::ClusterName;
     use anyhow::Result;
     use odbc_api::{
         buffers::{ColumnarBuffer, TextColumn, TextRowSet},
@@ -310,15 +311,22 @@ pub mod odbc {
     /// always SSPI = Trusted connection, but TrustServerCertificate depends
     pub fn make_connection_string(
         hostname: Option<&HostName>,
+        cluster_name: Option<&ClusterName>,
         instance: &InstanceName,
         database: Option<&str>,
         driver: Option<&str>,
         trust_server_certificate: bool,
     ) -> String {
+        let server = match cluster_name {
+            Some(c) => c.to_string(),
+            None => hostname
+                .map(|h| h.to_string())
+                .unwrap_or_else(|| "(local)".to_string()),
+        };
         format!(
             "Driver={{{}}};SERVER={}{};Database={};Integrated Security=SSPI;Trusted_Connection=yes;Encrypt=yes;TrustServerCertificate={};",
             driver.unwrap_or(&ODBC_DRIVER.clone()),
-            hostname.map(|h| h.to_string()).unwrap_or("(local)".to_string()),
+            server,
             if instance.to_string().to_uppercase() == *"MSSQLSERVER" {
                 "".to_string()
             } else {
@@ -413,11 +421,12 @@ pub mod odbc {
     #[cfg(test)]
     mod tests {
         use crate::platform::odbc::{self, ODBC_DRIVER};
-        use crate::types::{HostName, InstanceName};
+        use crate::types::{ClusterName, HostName, InstanceName};
 
         #[test]
         fn test_make_connection_string() {
             assert_eq!( odbc::make_connection_string(
+                None,
                 None,
                 &InstanceName::from("SQLEXPRESS_NAME"),
                 None,
@@ -426,15 +435,28 @@ pub mod odbc {
             assert_eq!(
                 odbc::make_connection_string(
                     Some(&HostName::from("host".to_string())),
+                    None,
                     &InstanceName::from("Instance"),
                     Some("db"),
                     Some("driver"),
                     true
                 ),
                 "Driver={driver};SERVER=host\\Instance;Database=db;Integrated Security=SSPI;Trusted_Connection=yes;Encrypt=yes;TrustServerCertificate=yes;"
             );
+            assert_eq!(
+                odbc::make_connection_string(
+                    Some(&HostName::from("host".to_string())),
+                    Some(&ClusterName::from("cluster".to_string())),
+                    &InstanceName::from("Instance"),
+                    Some("db"),
+                    Some("driver"),
+                    true
+                ),
+                "Driver={driver};SERVER=cluster\\Instance;Database=db;Integrated Security=SSPI;Trusted_Connection=yes;Encrypt=yes;TrustServerCertificate=yes;"
+            );
             assert_eq!( odbc::make_connection_string(
                 Some(&HostName::from("host".to_string())),
+                None,
                 &InstanceName::from("mssqlserver"),
                     None,
                     None, false),

packages/mk-sql/tests/test_ms_sql.rs

@@ -1623,6 +1623,7 @@ fn test_odbc() {
 
     let s = odbc::make_connection_string(
         Some(&HostName::from("127.0.0.1".to_string())),
+        None,
         &InstanceName::from("SQLEXPRESS_NAME"),
         Some("master"),
         None,
@@ -1653,6 +1654,7 @@ fn test_odbc() {
 #[test]
 fn test_odbc_timeout() {
     let s = odbc::make_connection_string(
+        None,
         None,
         &InstanceName::from("SQLEXPRESS_XX"),
         Some("master"),
@@ -1689,7 +1691,9 @@ async fn test_odbc_high_level() {
         )
         .unwrap()
         .unwrap();
-        create_odbc_client(&c, &instance_name, None).await.unwrap()
+        create_odbc_client(&c, None, &instance_name, None)
+            .await
+            .unwrap()
     }
 
     async fn get_props(name: &str, trust: bool) -> Option<SqlInstanceProperties> {