Skip to content

Commit

Permalink
v2 - Use tedious mssql module instead of sqlcmd (#96)
Browse files Browse the repository at this point in the history
* Use tedious mssql library instead of sqlcmd

* Fix mssql connection

* Fix SqlUtils tests

* Use config instead of connection string

* Replace conn string builder with mssql config

* Connect to master db

* Restore connection string validation regex

* PR comments, fix error handling

* Update main.js

* Use try catch for error handling

* Fix typo
  • Loading branch information
zijchen authored Jun 7, 2022
1 parent 7e69fdc commit eb605f8
Show file tree
Hide file tree
Showing 15 changed files with 2,813 additions and 427 deletions.
31 changes: 10 additions & 21 deletions __tests__/AzureSqlAction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,7 @@ import * as exec from '@actions/exec';
import AzureSqlAction, { IBuildAndPublishInputs, IDacpacActionInputs, ISqlActionInputs, ActionType, SqlPackageAction } from "../src/AzureSqlAction";
import AzureSqlActionHelper from "../src/AzureSqlActionHelper";
import DotnetUtils from '../src/DotnetUtils';
import SqlConnectionStringBuilder from '../src/SqlConnectionStringBuilder';

let sqlConnectionStringBuilderMock = jest.mock('../src/SqlConnectionStringBuilder', () => {
return ((connectionString) => {
return {
connectionString: connectionString,
userId: 'testUser',
password: 'placeholder',
database: 'testDB'
}
})
})
import SqlConnectionConfig from '../src/SqlConnectionConfig';

describe('AzureSqlAction tests', () => {
afterEach(() => {
Expand All @@ -32,7 +21,7 @@ describe('AzureSqlAction tests', () => {

expect(getSqlPackagePathSpy).toHaveBeenCalledTimes(1);
expect(execSpy).toHaveBeenCalledTimes(1);
expect(execSpy).toHaveBeenCalledWith(`"SqlPackage.exe" /Action:Publish /TargetConnectionString:"${inputs.connectionString.connectionString}" /SourceFile:"${inputs.dacpacPackage}" /TargetTimeout:20`);
expect(execSpy).toHaveBeenCalledWith(`"SqlPackage.exe" /Action:Publish /TargetConnectionString:"${inputs.connectionConfig.ConnectionString}" /SourceFile:"${inputs.dacpacPackage}" /TargetTimeout:20`);
});

it('throws if SqlPackage.exe fails to publish dacpac', async () => {
Expand Down Expand Up @@ -89,7 +78,7 @@ describe('AzureSqlAction tests', () => {
expect(getSqlPackagePathSpy).toHaveBeenCalledTimes(1);
expect(execSpy).toHaveBeenCalledTimes(2);
expect(execSpy).toHaveBeenNthCalledWith(1, `dotnet build "./TestProject.sqlproj" -p:NetCoreBuild=true --verbose --test "test value"`);
expect(execSpy).toHaveBeenNthCalledWith(2, `"SqlPackage.exe" /Action:Publish /TargetConnectionString:"${inputs.connectionString.connectionString}" /SourceFile:"${expectedDacpac}"`);
expect(execSpy).toHaveBeenNthCalledWith(2, `"SqlPackage.exe" /Action:Publish /TargetConnectionString:"${inputs.connectionConfig.ConnectionString}" /SourceFile:"${expectedDacpac}"`);
});

it('throws if dotnet fails to build sqlproj', async () => {
Expand Down Expand Up @@ -129,22 +118,22 @@ function getInputs(actionType: ActionType) {

switch(actionType) {
case ActionType.DacpacAction: {
const connectionString = new SqlConnectionStringBuilder('Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=placeholder');
const config = new SqlConnectionConfig('Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=placeholder');
return {
serverName: connectionString.server,
serverName: config.Config.server,
actionType: ActionType.DacpacAction,
connectionString: connectionString,
connectionConfig: config,
dacpacPackage: './TestPackage.dacpac',
sqlpackageAction: SqlPackageAction.Publish,
additionalArguments: '/TargetTimeout:20'
} as IDacpacActionInputs;
}
case ActionType.SqlAction: {
const connectionString = new SqlConnectionStringBuilder('Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=placeholder');
const config = new SqlConnectionConfig('Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=placeholder');
return {
serverName: connectionString.server,
serverName: config.Config.server,
actionType: ActionType.SqlAction,
connectionString: connectionString,
connectionConfig: config,
sqlFile: './TestFile.sql',
additionalArguments: '-t 20'
} as ISqlActionInputs;
Expand All @@ -153,7 +142,7 @@ function getInputs(actionType: ActionType) {
return {
serverName: 'testServer.database.windows.net',
actionType: ActionType.BuildAndPublish,
connectionString: new SqlConnectionStringBuilder('Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=placeholder'),
connectionConfig: new SqlConnectionConfig('Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=placeholder'),
projectFile: './TestProject.sqlproj',
buildArguments: '--verbose --test "test value"'
} as IBuildAndPublishInputs
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import * as core from '@actions/core';
import SqlConnectionStringBuilder from "../src/SqlConnectionStringBuilder";
import { ConnectionPool } from 'mssql';
import SqlConnectionConfig from '../src/SqlConnectionConfig';

jest.mock('@actions/core');

describe('SqlConnectionStringBuilder tests', () => {
describe('SqlConnectionConfig tests', () => {

describe('validate correct connection strings', () => {
let validConnectionStrings = [
const validConnectionStrings = [
[`Server=test1.database.windows.net;User Id=user;Password="ab'=abcdf''c;123";Initial catalog=testdb`, 'validates values enclosed with double quotes ', `ab'=abcdf''c;123`],
[`Server=test1.database.windows.net;User Id=user;Password='abc;1""2"adf=33';Initial catalog=testdb`, 'validates values enclosed with single quotes ', `abc;1""2"adf=33`],
[`Server=test1.database.windows.net;User Id=user;Password="abc;1""2""adf(012j^72''asj;')'=33";Initial catalog=testdb`, 'validates values beginning with double quotes and also contains escaped double quotes', `abc;1"2"adf(012j^72''asj;')'=33`],
Expand All @@ -16,18 +17,18 @@ describe('SqlConnectionStringBuilder tests', () => {
];

it.each(validConnectionStrings)('Input `%s` %s', (connectionStringInput, testDescription, passwordOutput) => {
let connectionString = new SqlConnectionStringBuilder(connectionStringInput);
const connectionString = new SqlConnectionConfig(connectionStringInput);

expect(connectionString.connectionString).toMatch(connectionStringInput);
expect(connectionString.password).toMatch(passwordOutput);
expect(connectionString.userId).toMatch(`user`);
expect(connectionString.database).toMatch('testdb');
if(!!connectionString.server) expect(connectionString.server).toMatch('test1.database.windows.net');
expect(connectionString.ConnectionString).toMatch(connectionStringInput);
expect(connectionString.Config.password).toMatch(passwordOutput);
expect(connectionString.Config.user).toMatch(`user`);
expect(connectionString.Config.database).toMatch('testdb');
if(!!connectionString.Config.server) expect(connectionString.Config.server).toMatch('test1.database.windows.net');
});
})

describe('throw for invalid connection strings', () => {
let invalidConnectionStrings = [
const invalidConnectionStrings = [
[`Server=test1.database.windows.net;User Id=user;Password="ab'=abcdf''c;123;Initial catalog=testdb`, 'validates values beginning with double quotes but not ending with double quotes'],
[`Server=test1.database.windows.net;User Id=user;Password='abc;1""2"adf=33;Initial catalog=testdb`, 'validates values beginning with single quote but not ending with single quote'],
[`Server=test1.database.windows.net;User Id=user;Password="abc;1""2"adf(012j^72''asj;')'=33";Initial catalog=testdb`, 'validates values enclosed in double quotes but does not escape double quotes in between'],
Expand All @@ -39,14 +40,19 @@ describe('SqlConnectionStringBuilder tests', () => {
];

it.each(invalidConnectionStrings)('Input `%s` %s', (connectionString) => {
expect(() => new SqlConnectionStringBuilder(connectionString)).toThrow();
expect(() => new SqlConnectionConfig(connectionString)).toThrow();
})
})

it('should mask connection string password', () => {
let setSecretSpy = jest.spyOn(core, 'setSecret');
new SqlConnectionStringBuilder('User Id=user;Password=1234;Server=test1.database.windows.net;Initial Catalog=testDB');

const setSecretSpy = jest.spyOn(core, 'setSecret');
new SqlConnectionConfig('User Id=user;Password=1234;Server=test1.database.windows.net;Initial Catalog=testDB');
expect(setSecretSpy).toHaveBeenCalled();
});
});

it('should call into mssql module to parse connection string', () => {
const parseConnectionStringSpy = jest.spyOn(ConnectionPool, 'parseConnectionString');
new SqlConnectionConfig('User Id=user;Password=1234;Server=test1.database.windows.net;Initial Catalog=testDB');
expect(parseConnectionStringSpy).toHaveBeenCalled();
});
})
37 changes: 16 additions & 21 deletions __tests__/SqlUtils.test.ts
Original file line number Diff line number Diff line change
@@ -1,38 +1,33 @@
import * as exec from '@actions/exec';
import mssql from 'mssql';
import SqlUtils from "../src/SqlUtils";
import AzureSqlActionHelper from "../src/AzureSqlActionHelper";
import SqlConnectionStringBuilder from '../src/SqlConnectionStringBuilder';
import SqlConnectionConfig from '../src/SqlConnectionConfig';

describe('SqlUtils tests', () => {
it('detectIPAddress should return ipaddress', async () => {
let getSqlCmdPathSpy = jest.spyOn(AzureSqlActionHelper, 'getSqlCmdPath').mockResolvedValue('SqlCmd.exe');
let execSpy = jest.spyOn(exec, 'exec').mockImplementation((_commandLine, _args, options) => {
let sqlClientError = `Client with IP address '1.2.3.4' is not allowed to access the server.`;
options!.listeners!.stderr!(Buffer.from(sqlClientError));
return Promise.reject(1);
});
let ipAddress = await SqlUtils.detectIPAddress('serverName', new SqlConnectionStringBuilder('Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=placeholder'));
const mssqlSpy = jest.spyOn(mssql.ConnectionPool.prototype, 'connect').mockImplementation((callback) => {
callback(new mssql.ConnectionError(new Error(`Client with IP address '1.2.3.4' is not allowed to access the server.`)));
});
const ipAddress = await SqlUtils.detectIPAddress(new SqlConnectionConfig('Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=placeholder'));

expect(getSqlCmdPathSpy).toHaveBeenCalledTimes(1);
expect(execSpy).toHaveBeenCalledTimes(1);
expect(mssqlSpy).toHaveBeenCalledTimes(1);
expect(ipAddress).toBe('1.2.3.4');
});

it('detectIPAddress should return empty', async () => {
let getSqlCmdSpy = jest.spyOn(AzureSqlActionHelper, 'getSqlCmdPath').mockResolvedValue('SqlCmd.exe');
let execSpy = jest.spyOn(exec, 'exec').mockResolvedValue(0);
let ipAddress = await SqlUtils.detectIPAddress('serverName', new SqlConnectionStringBuilder('Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=placeholder'));
const mssqlSpy = jest.spyOn(mssql.ConnectionPool.prototype, 'connect').mockImplementation((callback) => {
// Successful connections calls back with null error
callback(null);
});
const ipAddress = await SqlUtils.detectIPAddress(new SqlConnectionConfig('Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=placeholder'));

expect(getSqlCmdSpy).toHaveBeenCalledTimes(1);
expect(execSpy).toHaveBeenCalledTimes(1);
expect(mssqlSpy).toHaveBeenCalledTimes(1);
expect(ipAddress).toBe('');
});

it('detectIPAddress should throw error', () => {
let getSqlCmdSpy = jest.spyOn(AzureSqlActionHelper, 'getSqlCmdPath').mockResolvedValue('SqlCmd.exe')

expect(SqlUtils.detectIPAddress('serverName', new SqlConnectionStringBuilder('Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=placeholder'))).rejects;
expect(getSqlCmdSpy).toHaveBeenCalledTimes(1);
const mssqlSpy = jest.spyOn(mssql.ConnectionPool.prototype, 'connect');
expect(SqlUtils.detectIPAddress(new SqlConnectionConfig('Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=placeholder'))).rejects;
expect(mssqlSpy).toHaveBeenCalledTimes(1);
});

});
17 changes: 6 additions & 11 deletions __tests__/main.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,13 @@ import run from "../src/main";
import AzureSqlAction from "../src/AzureSqlAction";
import FirewallManager from "../src/FirewallManager";
import AzureSqlActionHelper from '../src/AzureSqlActionHelper';
import SqlConnectionStringBuilder from '../src/SqlConnectionStringBuilder';
import SqlUtils from '../src/SqlUtils';

jest.mock('@actions/core');
jest.mock('azure-actions-webclient/AuthorizerFactory');
jest.mock('../src/AzureSqlAction');
jest.mock('../src/FirewallManager');
jest.mock('../src/AzureSqlResourceManager');
jest.mock('../src/SqlConnectionStringBuilder');

describe('main.ts tests', () => {
afterEach(() => {
Expand Down Expand Up @@ -46,7 +44,6 @@ describe('main.ts tests', () => {
expect(detectIPAddressSpy).toHaveBeenCalled();
expect(getAuthorizerSpy).not.toHaveBeenCalled();
expect(getInputSpy).toHaveBeenCalledTimes(7);
expect(SqlConnectionStringBuilder).toHaveBeenCalled();
expect(resolveFilePathSpy).toHaveBeenCalled();
expect(addFirewallRuleSpy).not.toHaveBeenCalled();
expect(actionExecuteSpy).toHaveBeenCalled();
Expand Down Expand Up @@ -81,12 +78,11 @@ describe('main.ts tests', () => {
expect(detectIPAddressSpy).toHaveBeenCalled();
expect(getAuthorizerSpy).not.toHaveBeenCalled();
expect(getInputSpy).toHaveBeenCalledTimes(4);
expect(SqlConnectionStringBuilder).toHaveBeenCalled();
expect(resolveFilePathSpy).toHaveBeenCalled();
expect(addFirewallRuleSpy).not.toHaveBeenCalled();
expect(actionExecuteSpy).toHaveBeenCalled();
expect(removeFirewallRuleSpy).not.toHaveBeenCalled();
expect(setFailedSpy).not.toHaveBeenCalled();
expect(actionExecuteSpy).toHaveBeenCalled();
expect(removeFirewallRuleSpy).not.toHaveBeenCalled();
expect(setFailedSpy).not.toHaveBeenCalled();
})

it('gets inputs and executes sql action', async () => {
Expand Down Expand Up @@ -115,12 +111,11 @@ describe('main.ts tests', () => {
expect(detectIPAddressSpy).toHaveBeenCalled();
expect(getAuthorizerSpy).not.toHaveBeenCalled();
expect(getInputSpy).toHaveBeenCalledTimes(5);
expect(SqlConnectionStringBuilder).toHaveBeenCalled();
expect(resolveFilePathSpy).toHaveBeenCalled();
expect(addFirewallRuleSpy).not.toHaveBeenCalled();
expect(actionExecuteSpy).toHaveBeenCalled();
expect(removeFirewallRuleSpy).not.toHaveBeenCalled();
expect(setFailedSpy).not.toHaveBeenCalled();
expect(actionExecuteSpy).toHaveBeenCalled();
expect(removeFirewallRuleSpy).not.toHaveBeenCalled();
expect(setFailedSpy).not.toHaveBeenCalled();
})

it('throws if input file is not found', async() => {
Expand Down
3 changes: 2 additions & 1 deletion lib/main.js

Large diffs are not rendered by default.

50 changes: 50 additions & 0 deletions lib/main.js.LICENSE.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*!
* mime-db
* Copyright(c) 2014 Jonathan Ong
* MIT Licensed
*/

/*!
* mime-types
* Copyright(c) 2014 Jonathan Ong
* Copyright(c) 2015 Douglas Christopher Wilson
* MIT Licensed
*/

/*! *****************************************************************************
Copyright (c) Microsoft Corporation.

Permission to use, copy, modify, and/or distribute this software for any
purpose with or without fee is hereby granted.

THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH
REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT,
INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR
OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
PERFORMANCE OF THIS SOFTWARE.
***************************************************************************** */

/*! @azure/msal-common v4.5.1 2021-08-02 */

/*! @azure/msal-common v6.3.0 2022-05-02 */

/**
* @copyright (c) 2016, Philipp Thürwächter & Pattrick Hüper
* @copyright (c) 2007-present, Stephen Colebourne & Michael Nascimento Santos
* @license BSD-3-Clause (see LICENSE in the root directory of this source tree)
*/

/**
* @copyright (c) 2016, Philipp Thürwächter & Pattrick Hüper
* @license BSD-3-Clause (see LICENSE in the root directory of this source tree)
*/

//! @copyright (c) 2007-present, Stephen Colebourne & Michael Nascimento Santos

//! @copyright (c) 2015-present, Philipp Thürwächter, Pattrick Hüper & js-joda contributors

//! @license BSD-3-Clause (see LICENSE in the root directory of this source tree)

//! @version @js-joda/core - 4.3.1
Loading

0 comments on commit eb605f8

Please sign in to comment.