Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix | Fixing issue for SQL named Instances in Managed SNI using named pipes. #2142

Merged
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
56a19de
Implement support for NamePiped Instance in Managed SNI.
arellegue Aug 31, 2023
730e0c5
Removed path sepator prefix for default pipe name.
arellegue Aug 31, 2023
731048a
Added ReadOnlySpan to access string array.
arellegue Sep 1, 2023
4dbd383
Enhance InstanceNameTest to get NormalizedPipePath using either m_nor…
arellegue Sep 25, 2023
c948a59
Removed nuget package source from Nuget.config.
arellegue Sep 25, 2023
370aa8c
Removed SkipOnTargetFramework from NamedPipeInstanceNormalizedPipePa…
arellegue Sep 27, 2023
6d3e7c4
Added TCPInstanceConnectionString and NPInstanceConnectionString to c…
arellegue Sep 27, 2023
db07788
Include newly added config properties to SqlDbManager class.
arellegue Oct 4, 2023
b3660e4
Add condition to create Northwind database in named instance sql server.
arellegue Oct 4, 2023
6e7e24b
Use more meaningful name in third command line argument.
arellegue Oct 5, 2023
21d8eaf
Remove third command line argument. Control the Northwind creation in…
arellegue Oct 5, 2023
fc4f20e
Add ConditionalFact to test if NP Instance connection string is setup…
arellegue Oct 12, 2023
7a5297d
Revert all changes for newly added NamedInstance unit test. There is …
arellegue Oct 13, 2023
0d5a1e2
Revert config.default.json.
arellegue Oct 13, 2023
56903a9
Added 500ms delay after creating a user in SqlCredentialTest.
arellegue Oct 20, 2023
3bee9cf
Fixed Login exception error in OldCredentialsShouldFail unit test.
arellegue Oct 21, 2023
ee1d035
Add 5 seconds delay in createTestUser.
arellegue Oct 23, 2023
3724a46
Revert OldCredentialsShouldFail.
arellegue Oct 23, 2023
102e5b2
Removed unecessary comment.
arellegue Oct 23, 2023
b3a91cc
Removed blank line.
arellegue Oct 23, 2023
81f5f73
Removed blank line in SqlCredentialTest.
arellegue Oct 24, 2023
b5a581f
Removed using System.Data from InstanceNameTest class.
arellegue Oct 25, 2023
a96a5b9
Delete commented out InferLocalServerName();
arellegue Oct 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public SNINpHandle(string serverName, string pipeName, TimeoutTimer timeout, boo
{
int timeoutMilliseconds = timeout.MillisecondsRemainingInt;
SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNINpHandle), EventType.INFO,
"Connection Id {0}, Setting server name = {1}, pipe name = {2}. Connecting within the {3} sepecified milliseconds.",
"Connection Id {0}, Setting server name = {1}, pipe name = {2}. Connecting within the {3} specified milliseconds.",
args0: _connectionId,
args1: serverName,
args2: pipeName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,8 @@ internal class DataSource
private const string LocalDbHost_NP = @"np:\\.\pipe\LOCALDB#";
private const string NamedPipeInstanceNameHeader = "mssql$";
private const string DefaultPipeName = "sql\\query";
private const string InstancePrefix = "MSSQL$";
private const string PathSeparator = "\\";

internal enum Protocol { TCP, NP, None, Admin };

Expand Down Expand Up @@ -676,12 +678,35 @@ private bool InferNamedPipesInformation()
// If we have a datasource beginning with a pipe or we have already determined that the protocol is Named Pipe
if (_dataSourceAfterTrimmingProtocol.StartsWith(PipeBeginning, StringComparison.Ordinal) || _connectionProtocol == Protocol.NP)
{
// If the data source is "np:servername"
// If the data source starts with "np:servername"
if (!_dataSourceAfterTrimmingProtocol.Contains(PipeBeginning))
{
PipeHostName = ServerName = _dataSourceAfterTrimmingProtocol;
InferLocalServerName();
PipeName = SNINpHandle.DefaultPipePath;
// Assuming that user did not change default NamedPipe name, if the datasource is in the format servername\instance,
// separate servername and instance and prepend instance with MSSQL$ and append default pipe path
// https://learn.microsoft.com/en-us/sql/tools/configuration-manager/named-pipes-properties?view=sql-server-ver16
if (_dataSourceAfterTrimmingProtocol.Contains(PathSeparator) && _connectionProtocol == Protocol.NP)
{
string[] tokensByBackSlash = _dataSourceAfterTrimmingProtocol.Split(BackSlashCharacter);
if (tokensByBackSlash.Length == 2)
{
// NamedPipeClientStream object will create the network path using PipeHostName and PipeName
// and can be seen in its _normalizedPipePath variable in the format \\servername\pipe\MSSQL$<instancename>\sql\query
PipeHostName = ServerName = tokensByBackSlash[0];
InferLocalServerName();
PipeName = $"{InstancePrefix}{tokensByBackSlash[1].ToUpper()}{PathSeparator}{DefaultPipeName}";
}
else
{
ReportSNIError(SNIProviders.NP_PROV);
return false;
}
}
else
{
PipeHostName = ServerName = _dataSourceAfterTrimmingProtocol;
InferLocalServerName();
PipeName = SNINpHandle.DefaultPipePath;
}
arellegue marked this conversation as resolved.
Show resolved Hide resolved
David-Engel marked this conversation as resolved.
Show resolved Hide resolved
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,15 @@ internal static object LocalMachineRegistryValue(string subkey, string queryvalu
#if !NET6_0_OR_GREATER
(new RegistryPermission(RegistryPermissionAccess.Read, "HKEY_LOCAL_MACHINE\\" + subkey)).Assert(); // MDAC 62028
#endif

// Uncomment this block if debugging in WSL
//#if DEBUG
// // Non-Windows OS do not have registry
// if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
// {
// return null;
// }
//#endif
try
{
using (RegistryKey key = Registry.LocalMachine.OpenSubKey(subkey, false))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ public static class DataTestUtility
{
public static readonly string NPConnectionString = null;
public static readonly string TCPConnectionString = null;
public static readonly string NPInstanceConnectionString = null;
public static readonly string TCPInstanceConnectionString = null;
public static readonly string TCPConnectionStringHGSVBS = null;
public static readonly string TCPConnectionStringAASVBS = null;
public static readonly string TCPConnectionStringNoneVBS = null;
Expand Down Expand Up @@ -90,6 +92,8 @@ static DataTestUtility()
Config c = Config.Load();
NPConnectionString = c.NPConnectionString;
TCPConnectionString = c.TCPConnectionString;
NPInstanceConnectionString = c.NPInstanceConnectionString;
TCPInstanceConnectionString = c.TCPInstanceConnectionString;
TCPConnectionStringHGSVBS = c.TCPConnectionStringHGSVBS;
TCPConnectionStringAASVBS = c.TCPConnectionStringAASVBS;
TCPConnectionStringNoneVBS = c.TCPConnectionStringNoneVBS;
Expand Down Expand Up @@ -311,6 +315,11 @@ public static bool IsTCPConnStringSetup()
return !string.IsNullOrEmpty(TCPConnectionString);
}

public static bool IsNPInstanceConnStringsSetup()
{
return !string.IsNullOrEmpty(NPInstanceConnectionString);
}

// Synapse: Always Encrypted is not supported with Azure Synapse.
// Ref: https://feedback.azure.com/forums/307516-azure-synapse-analytics/suggestions/17858869-support-always-encrypted-in-sql-data-warehouse
public static bool AreConnStringSetupForAE()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Data;
David-Engel marked this conversation as resolved.
Show resolved Hide resolved
using System.Net;
using System.Net.Sockets;
using System.Text;
Expand All @@ -13,6 +14,7 @@ namespace Microsoft.Data.SqlClient.ManualTesting.Tests
{
public static class InstanceNameTest
{

[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsNotAzureServer), nameof(DataTestUtility.IsNotAzureSynapse), nameof(DataTestUtility.AreConnStringsSetup))]
public static void ConnectToSQLWithInstanceNameTest()
{
Expand Down Expand Up @@ -76,13 +78,33 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove
if (!IsValidInstance(hostname, instanceName))
{
builder.DataSource = hostname + "\\" + instanceName;

using SqlConnection connection = new(builder.ConnectionString);
SqlException ex = Assert.Throws<SqlException>(() => connection.Open());
Assert.Contains("Error Locating Server/Instance Specified", ex.Message);
}
}

[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsNPInstanceConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))]
[PlatformSpecific(TestPlatforms.Windows)] // Named pipes with the given input strings are not supported on Unix
public async static void NamedPipeSecondaryInstanceTest()
{
AppContext.SetSwitch("Switch.Microsoft.Data.SqlClient.UseManagedNetworkingOnWindows", true);

SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(DataTestUtility.NPInstanceConnectionString);
builder.ConnectTimeout = 5;
await OpenGoodConnection(builder.ConnectionString);
}

private async static Task OpenGoodConnection(string connectionString)
{
using (SqlConnection conn = new SqlConnection(connectionString))
{
await conn.OpenAsync();
DataTestUtility.AssertEqualsWithDescription( ConnectionState.Open, conn.State, "FAILED: Connection should be in open state");
}
}

private static bool IsBrowserAlive(string browserHostname)
{
const byte ClntUcastEx = 0x03;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public static class SqlDbManager

private const string TCPConnectionString = "TCPConnectionString";
private const string NPConnectionString = "NPConnectionString";
private const string TCPInstanceConnectionString = "TCPInstanceConnectionString";
private const string NPInstanceConnectionString = "NPInstanceConnectionString";
private const string TCPConnectionStringAASSGX = "TCPConnectionStringAASSGX";
private const string TCPConnectionStringAASVBS = "TCPConnectionStringAASVBS";
private const string TCPConnectionStringHGSVBS = "TCPConnectionStringHGSVBS";
Expand Down Expand Up @@ -63,7 +65,7 @@ public static void Run(string[] args)
{
// We do not create database for HGS-VBS since SQL Server for AASVBS and HGSVBS connection strings is same.
// Do not create database for NP connection string, since server is always same as TCP
if (activeConnString.Key != TCPConnectionStringHGSVBS && activeConnString.Key != NPConnectionString)
if (activeConnString.Key != TCPConnectionStringHGSVBS && activeConnString.Key != NPConnectionString && activeConnString.Key != NPInstanceConnectionString)
{
//Create a new database
CreateDatabase(dbName, context);
Expand All @@ -77,7 +79,7 @@ public static void Run(string[] args)
{
// We do not drop database for HGS-VBS since SQL Server for AASVBS and HGSVBS connection strings is same.
// Do not drop database for NP connection string, since server is always same as TCP
if (activeConnString.Key != TCPConnectionStringHGSVBS && activeConnString.Key != NPConnectionString)
if (activeConnString.Key != TCPConnectionStringHGSVBS && activeConnString.Key != NPConnectionString && activeConnString.Key != NPInstanceConnectionString)
{
// Drop Northwind for test run.
DropIfExistsDatabase(dbName, context);
Expand Down Expand Up @@ -118,6 +120,16 @@ private static void LoadActiveConnectionStrings()
{
s_activeConnectionStrings.Add(NPConnectionString, s_configJson.NPConnectionString);
}
if (!string.IsNullOrEmpty(s_configJson.TCPInstanceConnectionString))
{
Console.WriteLine($"Loading {s_configJson.TCPInstanceConnectionString}");
s_activeConnectionStrings.Add(TCPInstanceConnectionString, s_configJson.TCPInstanceConnectionString);
}
if (!string.IsNullOrEmpty(s_configJson.NPInstanceConnectionString))
{
Console.WriteLine($"Loading {s_configJson.NPInstanceConnectionString}");
s_activeConnectionStrings.Add(NPInstanceConnectionString, s_configJson.NPInstanceConnectionString);
}
if (s_configJson.EnclaveEnabled)
{
if (!string.IsNullOrEmpty(s_configJson.TCPConnectionStringAASSGX))
Expand Down Expand Up @@ -145,6 +157,12 @@ private static void UpdateConfig(string key, SqlConnectionStringBuilder builder)
case NPConnectionString:
s_configJson.NPConnectionString = builder.ConnectionString;
break;
case TCPInstanceConnectionString:
s_configJson.TCPInstanceConnectionString = builder.ConnectionString;
break;
case NPInstanceConnectionString:
s_configJson.NPInstanceConnectionString = builder.ConnectionString;
break;
case TCPConnectionStringAASSGX:
s_configJson.TCPConnectionStringAASSGX = builder.ConnectionString;
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ public class Config
{
public string TCPConnectionString = null;
public string NPConnectionString = null;
public string TCPInstanceConnectionString = null;
public string NPInstanceConnectionString = null;
public string TCPConnectionStringHGSVBS = null;
public string TCPConnectionStringAASVBS = null;
public string TCPConnectionStringNoneVBS = null;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
{
"TCPConnectionString": "Data Source=tcp:localhost;Database=Northwind;Integrated Security=true;Encrypt=false;",
"NPConnectionString": "Data Source=np:localhost;Database=Northwind;Integrated Security=true;Encrypt=false;",
"TCPInstanceConnectionString": "",
"NPInstanceConnectionString": "",
"TCPConnectionStringHGSVBS": "",
"TCPConnectionStringAASVBS": "",
"TCPConnectionStringNoneVBS": "",
Expand Down