From 6600a30786a7fd89b25eb1b5d4fbfe5213baa6ab Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Fri, 5 Jul 2024 12:29:06 -0700 Subject: [PATCH 1/6] Fix SqlConnectionReliabilityTest and Exception test when run in Azure jobs with Managed Identity Authentication. --- .../SQL/ExceptionTest/ExceptionTest.cs | 25 ++++++++++++++----- .../SQL/RetryLogic/RetryLogicTestHelper.cs | 5 ++-- .../SqlConnectionReliabilityTest.cs | 4 +-- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs index e34b47e262..51c7367498 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs @@ -204,8 +204,18 @@ public static void ExceptionTests() [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] public static void VariousExceptionTests() { - SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString); - + SqlConnectionStringBuilder builder; + if (DataTestUtility.IsNotAzureServer()) + { + builder = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString); + } + else + { + // Strip the password in connection string as Authentication=Active Directory Managed Identity can not be used with Password + string[] removeKeys = { "Password", "PWD" }; + string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.TCPConnectionString, removeKeys); + builder = new SqlConnectionStringBuilder(connStr); + } // Test 1 - A SqlConnectionStringBuilder badBuilder = new SqlConnectionStringBuilder(builder.ConnectionString) { DataSource = badServer, ConnectTimeout = 1 }; @@ -219,11 +229,14 @@ public static void VariousExceptionTests() } // Test 1 - B - badBuilder = new SqlConnectionStringBuilder(builder.ConnectionString) { Password = string.Empty, IntegratedSecurity = false, Authentication = SqlAuthenticationMethod.NotSpecified }; - using (var sqlConnection = new SqlConnection(badBuilder.ConnectionString)) + if (DataTestUtility.IsNotAzureServer()) { - string errorMessage = string.Format(CultureInfo.InvariantCulture, logonFailedErrorMessage, badBuilder.UserID); - VerifyConnectionFailure(() => sqlConnection.Open(), errorMessage, (ex) => VerifyException(ex, 1, 18456, 1, 14)); + badBuilder = new SqlConnectionStringBuilder(builder.ConnectionString) { Password = string.Empty, IntegratedSecurity = false }; + using (var sqlConnection = new SqlConnection(badBuilder.ConnectionString)) + { + string errorMessage = string.Format(CultureInfo.InvariantCulture, logonFailedErrorMessage, badBuilder.UserID); + VerifyConnectionFailure(() => sqlConnection.Open(), errorMessage, (ex) => VerifyException(ex, 1, 18456, 1, 14)); + } } } diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/RetryLogic/RetryLogicTestHelper.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/RetryLogic/RetryLogicTestHelper.cs index ba83e9b22a..a9eb1b52a2 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/RetryLogic/RetryLogicTestHelper.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/RetryLogic/RetryLogicTestHelper.cs @@ -71,7 +71,8 @@ private static readonly HashSet s_defaultTransientErrors 20, 0, -2, // Execution Timeout Expired. The timeout period elapsed prior to completion of the operation or the server is not responding. - 207 // invalid column name + 207, // invalid column name + 18456 // Using managed identity in Azure Sql Server throws 18456 for non-existent database instead of 4060. }; internal static readonly string s_exceedErrMsgPattern = SystemDataResourceManager.Instance.SqlRetryLogic_RetryExceeded; @@ -117,7 +118,7 @@ public static IEnumerable GetConnectionAndRetryStrategy(int numberOfRe public static IEnumerable GetConnectionAndRetryStrategyInvalidCatalog(int numberOfRetries) { - return GetConnectionAndRetryStrategy(numberOfRetries, TimeSpan.FromSeconds(1), FilterSqlStatements.None, null, 250, false); + return GetConnectionAndRetryStrategy(numberOfRetries, TimeSpan.FromSeconds(1), FilterSqlStatements.None, null, 250, true); } public static IEnumerable GetConnectionAndRetryStrategyInvalidCommand(int numberOfRetries) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/RetryLogic/SqlConnectionReliabilityTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/RetryLogic/SqlConnectionReliabilityTest.cs index 285da7f5ac..3096cc78d8 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/RetryLogic/SqlConnectionReliabilityTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/RetryLogic/SqlConnectionReliabilityTest.cs @@ -17,7 +17,7 @@ public class SqlConnectionReliabilityTest #region Sync // Test relies on error 4060 for automatic retry, which is not reliable when using Azure or AAD auth - [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] + [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] [MemberData(nameof(RetryLogicTestHelper.GetConnectionAndRetryStrategyInvalidCatalog), parameters: new object[] { 2 }, MemberType = typeof(RetryLogicTestHelper), DisableDiscoveryEnumeration = true)] public void ConnectionRetryOpenInvalidCatalogFailed(string cnnString, SqlRetryLogicBaseProvider provider) { @@ -35,7 +35,7 @@ public void ConnectionRetryOpenInvalidCatalogFailed(string cnnString, SqlRetryLo } // Test relies on error 4060 for automatic retry, which is not reliable when using Azure or AAD auth - [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] + [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] [MemberData(nameof(RetryLogicTestHelper.GetConnectionAndRetryStrategyInvalidCatalog), parameters: new object[] { 2 }, MemberType = typeof(RetryLogicTestHelper), DisableDiscoveryEnumeration = true)] public void ConnectionCancelRetryOpenInvalidCatalog(string cnnString, SqlRetryLogicBaseProvider provider) { From 82b4398f55ce25f302b8b7c79479f7062e7401c6 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Fri, 5 Jul 2024 14:41:56 -0700 Subject: [PATCH 2/6] Removed IsNotAzureServer condition of VariousExceptionTests and ConnectionRetryOpenAsyncInvalidCatalogFailed unit tests. --- .../tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs | 2 +- .../ManualTests/SQL/RetryLogic/SqlConnectionReliabilityTest.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs index 51c7367498..75f0ad4b9c 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs @@ -201,7 +201,7 @@ public static void ExceptionTests() } // Synapse: 110003;Invalid user or password - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] public static void VariousExceptionTests() { SqlConnectionStringBuilder builder; diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/RetryLogic/SqlConnectionReliabilityTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/RetryLogic/SqlConnectionReliabilityTest.cs index 3096cc78d8..7bdebfcfc1 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/RetryLogic/SqlConnectionReliabilityTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/RetryLogic/SqlConnectionReliabilityTest.cs @@ -158,7 +158,7 @@ public void DefaultOpenWithoutRetry(string connectionString, SqlRetryLogicBasePr #region Async // Test relies on error 4060 for automatic retry, which is not reliable when using Azure or AAD auth - [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] + [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] [MemberData(nameof(RetryLogicTestHelper.GetConnectionAndRetryStrategyInvalidCatalog), parameters: new object[] { 5 }, MemberType = typeof(RetryLogicTestHelper), DisableDiscoveryEnumeration = true)] public async void ConnectionRetryOpenAsyncInvalidCatalogFailed(string cnnString, SqlRetryLogicBaseProvider provider) { From 5909facc7ee1eef5f0e0cd886c22c7e0be0a819c Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Mon, 8 Jul 2024 09:09:27 -0700 Subject: [PATCH 3/6] Fixed the comment to explain why Password is remonved from connection string. --- .../tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs index 75f0ad4b9c..d98cf1afa8 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs @@ -211,7 +211,7 @@ public static void VariousExceptionTests() } else { - // Strip the password in connection string as Authentication=Active Directory Managed Identity can not be used with Password + // Strip the password in connection string as Authentication=Active Directory Managed Identity can not be used with a Password string[] removeKeys = { "Password", "PWD" }; string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.TCPConnectionString, removeKeys); builder = new SqlConnectionStringBuilder(connStr); From e0e5c23d092faa14f24427bddfdf83c6deafa402 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Fri, 26 Jul 2024 12:55:08 -0700 Subject: [PATCH 4/6] In VariousExceptionTests, check first if Authentication is ActiveDirectoryManagedIdentity before stripping the password. --- .../SQL/ExceptionTest/ExceptionTest.cs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs index d98cf1afa8..67bf40f886 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs @@ -204,17 +204,16 @@ public static void ExceptionTests() [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] public static void VariousExceptionTests() { - SqlConnectionStringBuilder builder; - if (DataTestUtility.IsNotAzureServer()) - { - builder = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString); - } - else + SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString); + if (!DataTestUtility.IsNotAzureServer()) { - // Strip the password in connection string as Authentication=Active Directory Managed Identity can not be used with a Password - string[] removeKeys = { "Password", "PWD" }; - string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.TCPConnectionString, removeKeys); - builder = new SqlConnectionStringBuilder(connStr); + // Strip the password in connection string if Authentication=Active Directory Managed Identity as it can not be used with a Password + if (builder.Authentication == SqlAuthenticationMethod.ActiveDirectoryManagedIdentity) + { + string[] removeKeys = { "Password", "PWD" }; + string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.TCPConnectionString, removeKeys); + builder = new SqlConnectionStringBuilder(connStr); + } } // Test 1 - A From 2a4b06ecead8d57c11a15ccb480f82dd8579cb8d Mon Sep 17 00:00:00 2001 From: Aris Rellegue <134557572+arellegue@users.noreply.github.com> Date: Tue, 30 Jul 2024 11:28:05 -0700 Subject: [PATCH 5/6] Update src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs Apply suggested changes. Co-authored-by: DavoudEshtehari <61173489+DavoudEshtehari@users.noreply.github.com> --- .../ManualTests/SQL/ExceptionTest/ExceptionTest.cs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs index 67bf40f886..2ab63fbf0c 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs @@ -205,15 +205,12 @@ public static void ExceptionTests() public static void VariousExceptionTests() { SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString); - if (!DataTestUtility.IsNotAzureServer()) + // Strip the password in connection string if Authentication=Active Directory Managed Identity as it can not be used with a Password + if (builder.Authentication == SqlAuthenticationMethod.ActiveDirectoryManagedIdentity) { - // Strip the password in connection string if Authentication=Active Directory Managed Identity as it can not be used with a Password - if (builder.Authentication == SqlAuthenticationMethod.ActiveDirectoryManagedIdentity) - { - string[] removeKeys = { "Password", "PWD" }; - string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.TCPConnectionString, removeKeys); - builder = new SqlConnectionStringBuilder(connStr); - } + string[] removeKeys = { "Password", "PWD" }; + string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.TCPConnectionString, removeKeys); + builder = new SqlConnectionStringBuilder(connStr); } // Test 1 - A From 8030fb5b0ede750d9b8f74acf904f8655647be79 Mon Sep 17 00:00:00 2001 From: Aris Rellegue <134557572+arellegue@users.noreply.github.com> Date: Tue, 30 Jul 2024 11:28:26 -0700 Subject: [PATCH 6/6] Update src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs Apply suggested changes. Co-authored-by: DavoudEshtehari <61173489+DavoudEshtehari@users.noreply.github.com> --- .../tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs index 2ab63fbf0c..ce751c4d29 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ExceptionTest/ExceptionTest.cs @@ -201,7 +201,7 @@ public static void ExceptionTests() } // Synapse: 110003;Invalid user or password - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))] public static void VariousExceptionTests() { SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString);