feat: support Azure AD auth in e2e tests #659
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The Azure DevOps pipeline runs tests twice: 1. SQL2022: Uses SQL auth (sa user) - tests will run 2. SQLDB: Uses Azure AD auth (service principal) - tests should skip The live connection e2e tests require SQL auth credentials (SQLCMDUSER/SQLCMDPASSWORD) which aren't available in the Azure AD auth scenario. Added skipIfNoSQLAuth() to properly skip these tests instead of failing with 'Login failed for user ''.
There was a problem hiding this comment.
Pull request overview
This PR fixes pipeline failures from PR #642 by adding proper test skipping logic for end-to-end tests that require SQL authentication credentials. The SQLDB pipeline test runs use Azure AD authentication without SQLCMDUSER/SQLCMDPASSWORD environment variables, causing these tests to fail inappropriately.
Changes:
- Added
hasSQLAuthCredentials()helper function to check for SQL authentication credentials (SQLCMDUSER and SQLCMDPASSWORD) - Added
skipIfNoSQLAuth()helper function that skips tests when SQL authentication is not available - Updated three live connection tests to use the new skip function instead of just checking for SQLCMDSERVER
Collaborator
|
why can't these tests look for the same auth settings as the other tests? |
Instead of skipping tests when SQL auth credentials are unavailable, detect Azure AD authentication and use appropriate --authentication-method flag, matching the approach used in pkg/sqlcmd/sqlcmd_test.go. Tests now run on both SQL2022 (SQL auth) and SQLDB (Azure AD) pipelines.
dlevy-msft-sql
changed the title
Contributor
Author
It turns out there's no good reason. Fixed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
PR #642 added e2e tests that originally only worked with SQL authentication credentials (SQLCMDUSER/SQLCMDPASSWORD). The ADO pipeline runs tests twice with different authentication methods:
Solution
Updated the e2e tests to detect and use Azure AD authentication when appropriate, matching the approach used in
pkg/sqlcmd/sqlcmd_test.go:canTestAzureAuth()- detects Azure SQL endpoint + no SQL credentialsgetAuthArgs()- returns appropriate--authentication-methodflag:ActiveDirectoryAzurePipelineswhen running in Azure PipelinesActiveDirectoryDefaultotherwiseResult
Tests now execute on both pipeline runs with the appropriate authentication method.
Files Changed
cmd/modern/e2e_test.go: Added Azure AD auth detection and--authentication-methodflag support