v2 - Change script action from sqlcmd to mssql query (#99)

* Change script action from sqlcmd to mssql query * Update action.yml * Fully qualify Table1 in sql script * Add more debug logging * Clone config before changing db to master * Cleanup * Set TEST_DB name before cleanup * Use runner.temp * Always cleanup * PR comments
Azure · Jun 14, 2022 · f94e710 · f94e710
1 parent eb605f8
commit f94e710
Show file tree

Hide file tree

Showing 8 changed files with 261 additions and 54 deletions.
diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml
@@ -59,9 +59,13 @@ jobs:
         connection-string: '${{ secrets.AZURE_SQL_CONNECTION_STRING_NO_DATABASE }}Initial Catalog=${{ env.TEST_DB }};'
         sql-file: ./__testdata__/testsql.sql
 
+    - name: Set database name for cleanup
+      if: always()
+      run: sed 's/$(DbName)/${{ env.TEST_DB }}/' > ${{ runner.temp }}/cleanup.sql
+
     - name: Cleanup Test Database
+      if: always()
       uses: ./
       with: 
         connection-string: '${{ secrets.AZURE_SQL_CONNECTION_STRING_NO_DATABASE }}Initial Catalog=master;'
-        sql-file: ./__testdata__/cleanup.sql
-        arguments: '-v DbName="${{ env.TEST_DB }}"'
+        sql-file: ${{ runner.temp }}/cleanup.sql
diff --git a/__testdata__/testsql.sql b/__testdata__/testsql.sql
@@ -1,7 +1,7 @@
 -- This script is used by pr-check.yml to test the SQLCMD action
 
 -- This should successfully insert data into the table created in the DACPAC step
-INSERT INTO [Table1] VALUES(1, 'test');
+INSERT INTO [dbo].[Table1] VALUES(1, 'test');
 
 -- This should successfully SELECT from the view created by the sqlproj
-SELECT * FROM [View1];
+SELECT * FROM [dbo].[View1];
diff --git a/__tests__/AzureSqlAction.test.ts b/__tests__/AzureSqlAction.test.ts
@@ -1,9 +1,13 @@
+import * as fs from 'fs';
 import * as path from 'path';
 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 SqlConnectionConfig from '../src/SqlConnectionConfig';
+import SqlUtils from '../src/SqlUtils';
+
+jest.mock('fs');
 
 describe('AzureSqlAction tests', () => {
     afterEach(() => {
@@ -36,29 +40,37 @@ describe('AzureSqlAction tests', () => {
     });
 
     it('executes sql action for SqlAction type', async () => {
-        let inputs = getInputs(ActionType.SqlAction) as ISqlActionInputs;
-        inputs.additionalArguments = '-t 20'
-        let action = new AzureSqlAction(inputs);
-
-        let getSqlCmdPathSpy = jest.spyOn(AzureSqlActionHelper, 'getSqlCmdPath').mockResolvedValue('SqlCmd.exe');
-        let execSpy = jest.spyOn(exec, 'exec').mockResolvedValue(0);
-
+        const inputs = getInputs(ActionType.SqlAction) as ISqlActionInputs;
+        const action = new AzureSqlAction(inputs);
+
+        const fsSpy = jest.spyOn(fs, 'readFileSync').mockReturnValue('select * from table1');
+        const sqlSpy = jest.spyOn(SqlUtils, 'executeSql').mockResolvedValue();
+
         await action.execute();
 
-        expect(getSqlCmdPathSpy).toHaveBeenCalledTimes(1);
-        expect(execSpy).toHaveBeenCalledTimes(1);
-        expect(execSpy).toHaveBeenCalledWith(`"SqlCmd.exe" -S testServer.database.windows.net -d testDB -U "testUser" -P "placeholder" -i "./TestFile.sql" -t 20`);
+        expect(fsSpy).toHaveBeenCalledTimes(1);
+        expect(sqlSpy).toHaveBeenCalledTimes(1);
+        expect(sqlSpy).toHaveBeenCalledWith(inputs.connectionConfig, 'select * from table1');
     });
 
-    it('throws if SqlCmd.exe fails to execute sql', async () => {
-        let inputs = getInputs(ActionType.SqlAction) as ISqlActionInputs;
-        let action = new AzureSqlAction(inputs);
+    it('throws if sql action cannot read file', async () => {
+        const inputs = getInputs(ActionType.SqlAction) as ISqlActionInputs;
+        const action = new AzureSqlAction(inputs);
+        const fsSpy = jest.spyOn(fs, 'readFileSync').mockImplementation(() => {
+            throw new Error('Cannot read file');
+        });
 
-        let getSqlCmdPathSpy = jest.spyOn(AzureSqlActionHelper, 'getSqlCmdPath').mockResolvedValue('SqlCmd.exe');
-        jest.spyOn(exec, 'exec').mockRejectedValue(1);
+        let error: Error | undefined;
+        try {
+            await action.execute();
+        }
+        catch (e) {
+            error = e;
+        }
 
-        expect(await action.execute().catch(() => null)).rejects;
-        expect(getSqlCmdPathSpy).toHaveBeenCalledTimes(1);
+        expect(fsSpy).toHaveBeenCalledTimes(1);
+        expect(error).toBeDefined();
+        expect(error!.message).toMatch(`Cannot read contents of file ./TestFile.sql due to error 'Cannot read file'.`);
     });
 
     it('should build and publish database project', async () => {
@@ -134,8 +146,7 @@ function getInputs(actionType: ActionType) {
                 serverName: config.Config.server,
                 actionType: ActionType.SqlAction,
                 connectionConfig: config,
-                sqlFile: './TestFile.sql',
-                additionalArguments: '-t 20'
+                sqlFile: './TestFile.sql'
             } as ISqlActionInputs;
         }
         case ActionType.BuildAndPublish: {

diff --git a/__tests__/SqlUtils.test.ts b/__tests__/SqlUtils.test.ts
@@ -1,33 +1,168 @@
+import * as core from "@actions/core";
+import AggregateError from 'es-aggregate-error';
 import mssql from 'mssql';
 import SqlUtils from "../src/SqlUtils";
 import SqlConnectionConfig from '../src/SqlConnectionConfig';
 
 describe('SqlUtils tests', () => {
+    afterEach(() => {
+       jest.restoreAllMocks();
+    });
+
     it('detectIPAddress should return ipaddress', async () => {
-        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 mssqlSpy = jest.spyOn(mssql, 'connect').mockImplementation(() => {
+            throw 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'));
+        const ipAddress = await SqlUtils.detectIPAddress(getConnectionConfig());
+
+        expect(mssqlSpy).toHaveBeenCalledTimes(1);
+        expect(ipAddress).toBe('1.2.3.4');
+    });
+
+    it('detectIPAddress should return ipaddress when connection returns AggregateError', async () => {
+        const mssqlSpy = jest.spyOn(mssql, 'connect').mockImplementation(() => {
+            const errors = new AggregateError([
+                new Error(`We don't care about this error.`),
+                new Error(`Client with IP address '1.2.3.4' is not allowed to access the server.`)
+            ])
+            throw new mssql.ConnectionError(errors);
+        });
+        const ipAddress = await SqlUtils.detectIPAddress(getConnectionConfig());
 
         expect(mssqlSpy).toHaveBeenCalledTimes(1);
         expect(ipAddress).toBe('1.2.3.4');
     });
 
     it('detectIPAddress should return empty', async () => {
-        const mssqlSpy = jest.spyOn(mssql.ConnectionPool.prototype, 'connect').mockImplementation((callback) => {
-            // Successful connections calls back with null error
-            callback(null);
+        const mssqlSpy = jest.spyOn(mssql, 'connect').mockImplementation(() => {
+            // Successful connection
+            return new mssql.ConnectionPool('');
         });
-        const ipAddress = await SqlUtils.detectIPAddress(new SqlConnectionConfig('Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=placeholder'));
+        const ipAddress = await SqlUtils.detectIPAddress(getConnectionConfig());
 
         expect(mssqlSpy).toHaveBeenCalledTimes(1);
         expect(ipAddress).toBe('');
     });
 
-    it('detectIPAddress should throw error', () => {
-        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;
+    it('detectIPAddress should throw error', async () => {
+        const mssqlSpy = jest.spyOn(mssql.ConnectionPool.prototype, 'connect').mockImplementation(() => {
+            throw new mssql.ConnectionError(new Error('Failed to connect.'));
+        });
+
+        let error: Error | undefined;
+        try {
+            await SqlUtils.detectIPAddress(getConnectionConfig());
+        }
+        catch (e) {
+            error = e;
+        }
+
+        expect(error).toBeDefined();
+        expect(error!.message).toMatch('Failed to add firewall rule. Unable to detect client IP Address.');
         expect(mssqlSpy).toHaveBeenCalledTimes(1);
     });
 
-});
+    it('should report single MSSQLError', async () => {
+        const errorSpy = jest.spyOn(core, 'error');
+        const error = new mssql.RequestError(new Error('Fake error'));
+
+        await SqlUtils.reportMSSQLError(error);
+
+        expect(errorSpy).toHaveBeenCalledTimes(1);
+        expect(errorSpy).toHaveBeenCalledWith('Fake error');
+    });
+
+    it('should report multiple MSSQLErrors', async () => {
+        const errorSpy = jest.spyOn(core, 'error');
+        const aggErrors = new AggregateError([
+            new Error('Fake error 1'),
+            new Error('Fake error 2'),
+            new Error('Fake error 3')
+        ]);
+        const error = new mssql.ConnectionError(aggErrors);
+
+        await SqlUtils.reportMSSQLError(error);
+
+        expect(errorSpy).toHaveBeenCalledTimes(3);
+        expect(errorSpy).toHaveBeenNthCalledWith(1, 'Fake error 1');
+        expect(errorSpy).toHaveBeenNthCalledWith(2, 'Fake error 2');
+        expect(errorSpy).toHaveBeenNthCalledWith(3, 'Fake error 3');
+    })
+
+    it('should execute sql script', async () => {
+        const connectionConfig = getConnectionConfig();
+        const connectSpy = jest.spyOn(mssql, 'connect').mockImplementation(() => {
+            // Successful connection
+            return new mssql.ConnectionPool(connectionConfig.Config);
+        });
+        const querySpy = jest.spyOn(mssql.ConnectionPool.prototype, 'query').mockImplementation(() => {
+            return {
+                recordsets: [{test: "11"}, {test: "22"}],
+                rowsAffected: [1, 2]
+            };
+        });
+        const consoleSpy = jest.spyOn(console, 'log');
+
+        await SqlUtils.executeSql(connectionConfig, 'select * from Table1');
+
+        expect(connectSpy).toHaveBeenCalledTimes(1);
+        expect(querySpy).toHaveBeenCalledTimes(1);
+        expect(consoleSpy).toHaveBeenCalledTimes(4);
+        expect(consoleSpy).toHaveBeenNthCalledWith(1, 'Rows affected: 1');
+        expect(consoleSpy).toHaveBeenNthCalledWith(2, 'Result: {"test":"11"}');
+        expect(consoleSpy).toHaveBeenNthCalledWith(3, 'Rows affected: 2');
+        expect(consoleSpy).toHaveBeenNthCalledWith(4, 'Result: {"test":"22"}');
+    });
+
+    it('should fail to execute sql due to connection error', async () => {
+        const connectSpy = jest.spyOn(mssql, 'connect').mockImplementation(() => {
+            throw new mssql.ConnectionError(new Error('Failed to connect'));
+        });
+        const errorSpy = jest.spyOn(core, 'error');
+
+        let error: Error | undefined;
+        try {
+            await SqlUtils.executeSql(getConnectionConfig(), 'select * from Table1');
+        }
+        catch (e) {
+            error = e;
+        }
+
+        expect(connectSpy).toHaveBeenCalledTimes(1);
+        expect(error).toBeDefined();
+        expect(error!.message).toMatch('Failed to execute query.');
+        expect(errorSpy).toHaveBeenCalledTimes(1);
+        expect(errorSpy).toHaveBeenCalledWith('Failed to connect');
+    });
+
+    it('should fail to execute sql due to request error', async () => {
+        const connectSpy = jest.spyOn(mssql, 'connect').mockImplementation(() => {
+            // Successful connection
+            return new mssql.ConnectionPool('');
+        });
+        const querySpy = jest.spyOn(mssql.ConnectionPool.prototype, 'query').mockImplementation(() => {
+            throw new mssql.RequestError(new Error('Failed to query'));
+        })
+        const errorSpy = jest.spyOn(core, 'error');
+
+        let error: Error | undefined;
+        try {
+            await SqlUtils.executeSql(getConnectionConfig(), 'select * from Table1');
+        }
+        catch (e) {
+            error = e;
+        }
+
+        expect(connectSpy).toHaveBeenCalledTimes(1);
+        expect(querySpy).toHaveBeenCalledTimes(1);
+        expect(error).toBeDefined();
+        expect(error!.message).toMatch('Failed to execute query.');
+        expect(errorSpy).toHaveBeenCalledTimes(1);
+        expect(errorSpy).toHaveBeenCalledWith('Failed to query');
+    });
+
+});
+
+function getConnectionConfig(): SqlConnectionConfig {
+    return new SqlConnectionConfig('Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=placeholder');
+}
diff --git a/action.yml b/action.yml
@@ -17,7 +17,7 @@ inputs:
     description: 'Path to the SQL database project file to deploy'
     required: false
   arguments:
-    description: 'In case DACPAC option is selected, additional SqlPackage arguments that will be applied. When SQL query option is selected, additional sqlcmd arguments will be applied.'
+    description: 'In case DACPAC option is selected, additional SqlPackage arguments that will be applied.'
     required: false
   build-arguments:
     description: 'In case Build and Publish option is selected, additional arguments that will be applied to dotnet build when building the database project.'

diff --git a/lib/main.js b/lib/main.js
diff --git a/src/AzureSqlAction.ts b/src/AzureSqlAction.ts
@@ -1,3 +1,4 @@
+import * as fs from 'fs';
 import * as path from 'path';
 import * as core from '@actions/core';
 import * as exec from '@actions/exec';
@@ -6,6 +7,7 @@ import AzureSqlActionHelper from './AzureSqlActionHelper';
 import DotnetUtils from './DotnetUtils';
 import Constants from './Constants';
 import SqlConnectionConfig from './SqlConnectionConfig';
+import SqlUtils from './SqlUtils';
 
 export enum ActionType {
     DacpacAction,
@@ -77,7 +79,7 @@ export default class AzureSqlAction {
     }
 
     private async _executeDacpacAction(inputs: IDacpacActionInputs) {
-        core.debug('Begin executing action')
+        core.debug('Begin executing sqlpackage');
         let sqlPackagePath = await AzureSqlActionHelper.getSqlPackagePath();
         let sqlPackageArgs = this._getSqlPackageArguments(inputs);
 
@@ -87,13 +89,22 @@ export default class AzureSqlAction {
     }
 
     private async _executeSqlFile(inputs: ISqlActionInputs) {
-        let sqlCmdPath = await AzureSqlActionHelper.getSqlCmdPath();
-        await exec.exec(`"${sqlCmdPath}" -S ${inputs.serverName} -d ${inputs.connectionConfig.Config.database} -U "${inputs.connectionConfig.Config.user}" -P "${inputs.connectionConfig.Config.password}" -i "${inputs.sqlFile}" ${inputs.additionalArguments}`);
-
-        console.log(`Successfully executed Sql file on target database.`);
+        core.debug('Begin executing sql script');
+        let scriptContents: string;
+        try {
+            scriptContents = fs.readFileSync(inputs.sqlFile, "utf8");
+        }
+        catch (e) {
+            throw new Error(`Cannot read contents of file ${inputs.sqlFile} due to error '${e.message}'.`);
+        }
+
+        await SqlUtils.executeSql(inputs.connectionConfig, scriptContents);
+
+        console.log(`Successfully executed SQL file on target database.`);
     }
 
     private async _executeBuildProject(inputs: IBuildAndPublishInputs): Promise<string> {
+        core.debug('Begin building project');
         const projectName = path.basename(inputs.projectFile, Constants.sqlprojExtension);
         const additionalBuildArguments = inputs.buildArguments ?? '';
         const parsedArgs = await DotnetUtils.parseCommandArguments(additionalBuildArguments);