From 9f5f49e956f57c3ae71531fb2f77075338e873c0 Mon Sep 17 00:00:00 2001 From: AkshayMata Date: Fri, 3 Feb 2023 06:43:39 -0800 Subject: [PATCH] Fix connecting to MI login bug (#21821) This PR fixes a regression for migration login for MI instances that was introduced in https://github.com/microsoft/azuredatastudio/pull/21776/files#diff-93c1a62583fa32d99f775b71ad27922cf31f660d10717ecc6966784306de1b6f. After that change, support for MI would fail as MI server types were going into the Sql VM path in sqlutils because the underlying logic for isSqlServerVM() was returning wrong results. The new approach uses the targetType set in StateMachine to extract the correct serverName for connection string based on the targetType. Testing: - Tested SQL VM login migration end to end - Tested SQL MI login migration end to end This change also bumps the sql-migration version to 1.3.0 --- extensions/sql-migration/package.json | 2 +- extensions/sql-migration/src/api/azure.ts | 8 ----- extensions/sql-migration/src/api/sqlUtils.ts | 36 ++++--------------- .../sql-migration/src/models/stateMachine.ts | 27 +++++++++++++- .../loginMigrationTargetSelectionPage.ts | 20 ++++------- .../src/wizard/loginSelectorPage.ts | 4 +-- 6 files changed, 42 insertions(+), 55 deletions(-) diff --git a/extensions/sql-migration/package.json b/extensions/sql-migration/package.json index 689b946fb5..3d0f0eb0e9 100644 --- a/extensions/sql-migration/package.json +++ b/extensions/sql-migration/package.json @@ -2,7 +2,7 @@ "name": "sql-migration", "displayName": "%displayName%", "description": "%description%", - "version": "1.2.6", + "version": "1.3.0", "publisher": "Microsoft", "preview": false, "license": "https://raw.githubusercontent.com/Microsoft/azuredatastudio/main/LICENSE.txt", diff --git a/extensions/sql-migration/src/api/azure.ts b/extensions/sql-migration/src/api/azure.ts index c59774fd59..17eaee0abc 100644 --- a/extensions/sql-migration/src/api/azure.ts +++ b/extensions/sql-migration/src/api/azure.ts @@ -194,10 +194,6 @@ export interface AzureSqlDatabaseServer { }, } -export function isAzureSqlDatabaseServer(instance: any): instance is AzureSqlDatabaseServer { - return (instance as AzureSqlDatabaseServer) !== undefined; -} - export type SqlVMServer = { properties: { virtualMachineResourceId: string, @@ -215,10 +211,6 @@ export type SqlVMServer = { networkInterfaces: Map, }; -export function isSqlVMServer(instance: any): instance is SqlVMServer { - return (instance as SqlVMServer) !== undefined; -} - export type VirtualMachineInstanceView = { computerName: string, osName: string, diff --git a/extensions/sql-migration/src/api/sqlUtils.ts b/extensions/sql-migration/src/api/sqlUtils.ts index 739224d7d5..390c1bfc03 100644 --- a/extensions/sql-migration/src/api/sqlUtils.ts +++ b/extensions/sql-migration/src/api/sqlUtils.ts @@ -5,11 +5,10 @@ import * as azdata from 'azdata'; import { azureResource } from 'azurecore'; -import { AzureSqlDatabase, AzureSqlDatabaseServer, isAzureSqlDatabaseServer, isSqlManagedInstance, isSqlVMServer, SqlManagedInstance, SqlVMServer } from './azure'; +import { AzureSqlDatabase, AzureSqlDatabaseServer } from './azure'; import { generateGuid } from './utils'; import * as utils from '../api/utils'; import { TelemetryAction, TelemetryViews, logError } from '../telemetry'; -import { NetworkInterfaceModel } from './dataModels/azure/networkInterfaceModel'; const query_database_tables_sql = ` SELECT @@ -167,14 +166,13 @@ function getSqlDbConnectionProfile( } export function getConnectionProfile( - server: string | SqlManagedInstance | SqlVMServer | AzureSqlDatabaseServer, + serverName: string, azureResourceId: string, userName: string, password: string, trustServerCert: boolean = false): azdata.IConnectionProfile { const connectId = generateGuid(); - const serverName = extractNameFromServer(server); return { serverName: serverName, id: connectId, @@ -205,29 +203,6 @@ export function getConnectionProfile( }; } -function extractNameFromServer( - server: string | SqlManagedInstance | SqlVMServer | AzureSqlDatabaseServer): string { - - // No need to extract name if the server is a string - if (typeof server === 'string') { - return server - } - - if (isSqlVMServer(server)) { - // For sqlvm, we need to use ip address from the network interface to connect to the server - const sqlVm = server as SqlVMServer; - const networkInterfaces = Array.from(sqlVm.networkInterfaces.values()); - return NetworkInterfaceModel.getIpAddress(networkInterfaces); - } - - // check if the target server is a managed instance or a VM - if (isSqlManagedInstance(server) || isAzureSqlDatabaseServer(server)) { - return server.properties.fullyQualifiedDomainName; - } - - return ""; -} - export async function collectSourceDatabaseTableInfo(sourceConnectionId: string, sourceDatabase: string): Promise { const ownerUri = await azdata.connection.getUriForConnection(sourceConnectionId); const connectionProvider = azdata.dataprotocol.getProvider( @@ -413,14 +388,15 @@ export async function collectSourceLogins( } export async function collectTargetLogins( - targetServer: SqlManagedInstance | SqlVMServer | AzureSqlDatabaseServer, + serverName: string, + azureResourceId: string, userName: string, password: string, includeWindowsAuth: boolean = true): Promise { const connectionProfile = getConnectionProfile( - targetServer, - targetServer.id, + serverName, + azureResourceId, userName, password, true /* trustServerCertificate */); diff --git a/extensions/sql-migration/src/models/stateMachine.ts b/extensions/sql-migration/src/models/stateMachine.ts index 2b886e6456..b4bd65aa98 100644 --- a/extensions/sql-migration/src/models/stateMachine.ts +++ b/extensions/sql-migration/src/models/stateMachine.ts @@ -17,6 +17,7 @@ import { SKURecommendationPage } from '../wizard/skuRecommendationPage'; import { excludeDatabases, getConnectionProfile, LoginTableInfo, SourceDatabaseInfo, TargetDatabaseInfo } from '../api/sqlUtils'; import { LoginMigrationModel, LoginMigrationStep } from './loginMigrationModel'; import { TdeMigrationDbResult, TdeMigrationModel } from './tdeModels'; +import { NetworkInterfaceModel } from '../api/dataModels/azure/networkInterfaceModel'; const localize = nls.loadMessageBundle(); export enum ValidateIrState { @@ -528,6 +529,30 @@ export class MigrationStateModel implements Model, vscode.Disposable { return this._skuRecommendationResults; } + public setTargetServerName(): void { + switch (this._targetType) { + case MigrationTargetType.SQLMI: + const sqlMi = this._targetServerInstance as SqlManagedInstance; + this._targetServerName = sqlMi.properties.fullyQualifiedDomainName; + case MigrationTargetType.SQLDB: + const sqlDb = this._targetServerInstance as AzureSqlDatabaseServer; + this._targetServerName = sqlDb.properties.fullyQualifiedDomainName; + case MigrationTargetType.SQLVM: + // For sqlvm, we need to use ip address from the network interface to connect to the server + const sqlVm = this._targetServerInstance as SqlVMServer; + const networkInterfaces = Array.from(sqlVm.networkInterfaces.values()); + this._targetServerName = NetworkInterfaceModel.getIpAddress(networkInterfaces); + } + } + + public get targetServerName(): string { + // If the target server name is not already set, return it + if (!this._targetServerName) { + this.setTargetServerName(); + } + + return this._targetServerName; + } public async getSourceConnectionString(): Promise { return await azdata.connection.getConnectionString(this._sourceConnectionId, true); @@ -535,7 +560,7 @@ export class MigrationStateModel implements Model, vscode.Disposable { public async getTargetConnectionString(): Promise { const connectionProfile = getConnectionProfile( - this._targetServerInstance, + this.targetServerName, this._targetServerInstance.id, this._targetUserName, this._targetPassword, diff --git a/extensions/sql-migration/src/wizard/loginMigrationTargetSelectionPage.ts b/extensions/sql-migration/src/wizard/loginMigrationTargetSelectionPage.ts index 807368c620..df6924ad11 100644 --- a/extensions/sql-migration/src/wizard/loginMigrationTargetSelectionPage.ts +++ b/extensions/sql-migration/src/wizard/loginMigrationTargetSelectionPage.ts @@ -152,9 +152,6 @@ export class LoginMigrationTargetSelectionPage extends MigrationWizardPage { break; } - const isSqlDbTarget = this.migrationStateModel._targetType === MigrationTargetType.SQLDB; - console.log(isSqlDbTarget); - if (this._targetUserNameInputBox) { await this._targetUserNameInputBox.updateProperty('required', true); } @@ -174,6 +171,7 @@ export class LoginMigrationTargetSelectionPage extends MigrationWizardPage { this._targetPasswordInputBox.value = ''; this.migrationStateModel._sqlMigrationServices = undefined!; this.migrationStateModel._targetServerInstance = undefined!; + this.migrationStateModel._targetServerName = undefined!; this.migrationStateModel._resourceGroup = undefined!; this.migrationStateModel._location = undefined!; await this.populateLocationDropdown(); @@ -337,7 +335,6 @@ export class LoginMigrationTargetSelectionPage extends MigrationWizardPage { this.migrationStateModel.isWindowsAuthMigrationSupported)); this.migrationStateModel._loginMigrationModel.collectedSourceLogins = true; this.migrationStateModel._loginMigrationModel.loginsOnSource = sourceLogins; - console.log(this.migrationStateModel._targetType); })); const flexContainer = this._view.modelBuilder.flexContainer() @@ -616,7 +613,8 @@ export class LoginMigrationTargetSelectionPage extends MigrationWizardPage { this.wizard.nextButton.enabled = false; loginsOnTarget.push( ...await collectTargetLogins( - targetDatabaseServer, + this.migrationStateModel.targetServerName, + targetDatabaseServer.id, userName, password, this.migrationStateModel.isWindowsAuthMigrationSupported)); @@ -750,6 +748,7 @@ export class LoginMigrationTargetSelectionPage extends MigrationWizardPage { this.migrationStateModel._azureAccount, this.migrationStateModel._targetSubscription, this.migrationStateModel._targetServerInstance); + this.migrationStateModel.setTargetServerName(); this.wizard.message = { text: '' }; @@ -779,6 +778,7 @@ export class LoginMigrationTargetSelectionPage extends MigrationWizardPage { if (selectedMi) { this.migrationStateModel._targetServerInstance = utils.deepClone(selectedMi)! as azureResource.AzureSqlManagedInstance; + this.migrationStateModel.setTargetServerName(); this.wizard.message = { text: '' }; if (this.migrationStateModel._targetServerInstance.properties.state !== 'Ready') { @@ -797,6 +797,7 @@ export class LoginMigrationTargetSelectionPage extends MigrationWizardPage { if (sqlDatabaseServer) { this.migrationStateModel._targetServerInstance = utils.deepClone(sqlDatabaseServer)! as AzureSqlDatabaseServer; + this.migrationStateModel.setTargetServerName(); this.wizard.message = { text: '' }; if (this.migrationStateModel._targetServerInstance.properties.state === 'Ready') { this._targetUserNameInputBox.value = this.migrationStateModel._targetServerInstance.properties.administratorLogin; @@ -813,6 +814,7 @@ export class LoginMigrationTargetSelectionPage extends MigrationWizardPage { } } else { this.migrationStateModel._targetServerInstance = undefined!; + this.migrationStateModel._targetServerName = undefined!; if (isSqlDbTarget) { this._targetUserNameInputBox.value = ''; } @@ -990,14 +992,6 @@ export class LoginMigrationTargetSelectionPage extends MigrationWizardPage { this.migrationStateModel._resourceGroup?.name, constants.NO_VIRTUAL_MACHINE_FOUND); - let response = await utils.getVirtualMachinesDropdownValues( - this.migrationStateModel._targetSqlVirtualMachines, - this.migrationStateModel._location, - this.migrationStateModel._resourceGroup, - this.migrationStateModel._azureAccount, - this.migrationStateModel._targetSubscription); - console.log(response); - break; case MigrationTargetType.SQLDB: this._azureResourceDropdown.values = utils.getAzureResourceDropdownValues( diff --git a/extensions/sql-migration/src/wizard/loginSelectorPage.ts b/extensions/sql-migration/src/wizard/loginSelectorPage.ts index 8ee4c3a025..b7002a9412 100644 --- a/extensions/sql-migration/src/wizard/loginSelectorPage.ts +++ b/extensions/sql-migration/src/wizard/loginSelectorPage.ts @@ -11,7 +11,6 @@ import * as constants from '../constants/strings'; import { debounce, getLoginStatusImage, getLoginStatusMessage } from '../api/utils'; import * as styles from '../constants/styles'; import { collectSourceLogins, collectTargetLogins, LoginTableInfo } from '../api/sqlUtils'; -import { AzureSqlDatabaseServer } from '../api/azure'; import { IconPathHelper } from '../constants/iconPathHelper'; import * as utils from '../api/utils'; import { LoginType } from '../models/loginMigrationModel'; @@ -377,7 +376,8 @@ export class LoginSelectorPage extends MigrationWizardPage { try { if (this.isTargetInstanceSet()) { targetLogins.push(...await collectTargetLogins( - stateMachine._targetServerInstance as AzureSqlDatabaseServer, + stateMachine.targetServerName, + stateMachine._targetServerInstance.id, stateMachine._targetUserName, stateMachine._targetPassword, stateMachine.isWindowsAuthMigrationSupported));