From 61f1675e0d3dd585718ef30ef1f2c48e164dd02d Mon Sep 17 00:00:00 2001 From: Drew Skwiers-Koballa Date: Wed, 5 Jan 2022 10:02:39 -0800 Subject: [PATCH] ref PR #38 (#64) * proposed improvement #38 * Update main.ts fixed null/empty check * moved check for required server address * added more connection string tests * improved code readability * updated readme * updated action.yml * updated readme * re-added support to server-name to retain backward compatibility * removed unused code? * improved readme * corrected typos * use server-name if specified * added debug message and comment * resolving test failure for connectionString.server Co-authored-by: Davide Mauri --- README.md | 41 ++++++++++---------- __tests__/AzureSqlAction.test.ts | 13 ++++--- __tests__/SqlConnectionStringBuilder.test.ts | 29 ++++++++------ __tests__/main.test.ts | 14 +++---- action.yml | 5 ++- src/SqlConnectionStringBuilder.ts | 4 ++ src/main.ts | 19 ++++++++- 7 files changed, 78 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index 88ad475f..cf533826 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ The definition of this GitHub Action is in [action.yml](https://github.com/Azure If you *can* use the option [Allow Azure Services and resources to access this server](https://docs.microsoft.com/en-us/azure/azure-sql/database/firewall-configure#connections-from-inside-azure), you are all set and you don't need to to anything else to allow GitHub Action to connect to your Azure SQL database. -If you *cannot* use the aformentioned option, additional steps are needed. +If you *cannot* use the aforementioned option, additional steps are needed. - Authenticate using [Azure Login](https://github.com/Azure/login) @@ -35,30 +35,30 @@ Alternatively, if enough permissions are not granted on the service principal or ### Create SQL database and deploy using GitHub Actions -1. Follow the tutorial [Azure SQL Quickstart](https://docs.microsoft.com/en-in/azure/sql-database/sql-database-single-database-get-started?tabs=azure-portal) -2. Copy the [SQL-on-Azure.yml template](https://github.com/Azure/actions-workflow-samples) and paste the contents in `.github/workflows/` in your project repository as `workflow.yml`. -3. Change `server-name` to your Azure SQL Server name. -4. Commit and push your project to GitHub repository, you should see a new GitHub Action initiated in **Actions** tab. +1. Follow the tutorial [Azure SQL Quickstart](https://docs.microsoft.com/azure/sql-database/sql-database-single-database-get-started?tabs=azure-portal) +1. Copy the [SQL-on-Azure.yml template](https://github.com/Azure/actions-workflow-samples) and paste the contents in `.github/workflows/` in your project repository as `workflow.yml`. +1. Update the connection string with your values. Connection string format is: `Server=;User ID=;Password=;Initial Catalog=` +1. Commit and push your project to GitHub repository, you should see a new GitHub Action initiated in **Actions** tab. -### Configure GitHub Secrets +### Configure GitHub Secrets For using any sensitive data/secrets like Azure Service Principal or SQL Connection strings within an Action, add them as [secrets](https://help.github.com/en/github/automating-your-workflow-with-github-actions/virtual-environments-for-github-actions#creating-and-using-secrets-encrypted-variables) in the GitHub repository and then use them in the workflow. Follow the steps to configure the secret: -* Define a new secret under your repository **Settings** > **Secrets** > **Add a new secret** menu -* Paste the contents of the Secret (Example: Connection String) as Value +- Define a new secret under your repository **Settings** > **Secrets** > **Add a new secret** menu +- Paste the contents of the Secret (Example: Connection String) as Value -If you need to configure Azure Credentials to automatically manage firewall rules, you need to create a Service Principal, and store the related credentials into a GitHub Secrect so that it can be used by the Azure Login actions to authenticate and authorize any subsequent request. +If you need to configure Azure Credentials to automatically manage firewall rules, you need to create a Service Principal, and store the related credentials into a GitHub Secret so that it can be used by the Azure Login actions to authenticate and authorize any subsequent request. -Paste the output of the below [az cli](https://docs.microsoft.com/en-us/cli/azure/?view=azure-cli-latest) command as the value of secret variable, for example `AZURE_CREDENTIALS`. +Paste the output of the below [az cli](https://docs.microsoft.com/cli/azure/?view=azure-cli-latest) command as the value of secret variable, for example `AZURE_CREDENTIALS`. ```bash az ad sp create-for-rbac --name "mySQLServer" --role contributor \ --scopes /subscriptions/{subscription-id}/resourceGroups/{resource-group} \ --sdk-auth -# Replace {subscription-id}, {resource-group} and {server-name} with the subscription, resource group and name of the Azure SQL server +# Replace {subscription-id}, {resource-group} with the subscription, resource group and name of the Azure SQL server # The command should output a JSON object similar to this: @@ -70,7 +70,7 @@ az ad sp create-for-rbac --name "mySQLServer" --role contributor \ // ... } ``` - + ### Sample workflow to deploy to an Azure SQL database ```yaml @@ -86,26 +86,27 @@ jobs: with: creds: ${{ secrets.AZURE_CREDENTIALS }} - uses: azure/sql-action@v1 - with: + with: server-name: REPLACE_THIS_WITH_YOUR_SQL_SERVER_NAME connection-string: ${{ secrets.AZURE_SQL_CONNECTION_STRING }} dacpac-package: './Database.dacpac' ``` -**Note:** +**Note:** + +The above means you have to create secrets in GitHub which can be found within your repository within **Settings** and then **Secrets** and also be careful to check the connection string which you copy from Azure SQL as the connection string has this **Password={your_password}** and you will need to supply the correct password for your connection string. -The above means you have to create secrets in GitHub which can be found within your repository within **Settings** and then **Secrets** and also -be careful to check the connection string which you copy from Azure SQL as the connection string has this **Password={your_password}** and you will need to supply -the correct password for your connection string. +The `server-name` is optional and is there only to provide backward compatibility. It is strongly recommended to put the server name in the connection string. The connection string uses this template: `Server=; User ID=; Password=; Initial Catalog=`. In case the server name is put both in the `server-name` and in the `connection-string`, the server name used will be the one specified in the `server-name` YAML key. + +### How to create a .dacpac file from your existing SQL Server Database -### How to create a dacpac file from your existing SQL Server Database - For the above action to work, you will need to create a file called `Database.dacpac` and place it into the root of your GitHub repository. The following link will show you how to go about creating a dacpac file but make sure the file is called `Database.dacpac`. -[Export a Data-tier application](https://docs.microsoft.com/en-us/sql/relational-databases/data-tier-applications/export-a-data-tier-application?view=sql-server-ver15) +[Export a Data-tier application](https://docs.microsoft.com/sql/relational-databases/data-tier-applications/export-a-data-tier-application?view=sql-server-ver15) Azure SQL Action for GitHub is supported for the Azure public cloud as well as Azure government clouds ('AzureUSGovernment' or 'AzureChinaCloud'). Before running this action, login to the respective Azure Cloud using [Azure Login](https://github.com/Azure/login) by setting appropriate value for the `environment` parameter. + ## Contributing This project welcomes contributions and suggestions. Most contributions require you to agree to a diff --git a/__tests__/AzureSqlAction.test.ts b/__tests__/AzureSqlAction.test.ts index 7d47fa8a..4ec0ec49 100644 --- a/__tests__/AzureSqlAction.test.ts +++ b/__tests__/AzureSqlAction.test.ts @@ -72,22 +72,25 @@ describe('AzureSqlAction tests', () => { }); function getInputs(actionType: ActionType) { + switch(actionType) { case ActionType.DacpacAction: { - return{ - serverName: 'testServer.database.windows.net', + const connectionString = new SqlConnectionStringBuilder('Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=testPassword'); + return { + serverName: connectionString.server, actionType: ActionType.DacpacAction, - connectionString: new SqlConnectionStringBuilder('Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=testPassword'), + connectionString: connectionString, 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=testPassword'); return { - serverName: 'testServer.database.windows.net', + serverName: connectionString.server, actionType: ActionType.SqlAction, - connectionString: new SqlConnectionStringBuilder('Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=testPassword'), + connectionString: connectionString, sqlFile: './TestFile.sql', additionalArguments: '-t 20' } as ISqlActionInputs; diff --git a/__tests__/SqlConnectionStringBuilder.test.ts b/__tests__/SqlConnectionStringBuilder.test.ts index 490ed04d..90e83ff7 100644 --- a/__tests__/SqlConnectionStringBuilder.test.ts +++ b/__tests__/SqlConnectionStringBuilder.test.ts @@ -7,11 +7,12 @@ describe('SqlConnectionStringBuilder tests', () => { describe('validate correct connection strings', () => { let validConnectionStrings = [ - [`User Id=user;Password="ab'=abcdf''c;123";Initial catalog=testdb`, 'validates values enclosed with double quotes ', `ab'=abcdf''c;123`], - [`User Id=user;Password='abc;1""2"adf=33';Initial catalog=testdb`, 'validates values enclosed with single quotes ', `abc;1""2"adf=33`], - [`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`], - [`User Id=user;Password='ab""c;1''2''"''adf("0""12j^72''asj;'')''=33';Initial catalog=testdb`, 'validates values beginning with single quotes and also contains escaped single quotes', `ab""c;1'2'"'adf("0""12j^72'asj;')'=33`], - [`User Id=user;Password=JustANormal123@#$password;Initial catalog=testdb`, 'validates values not beginning quotes and not containing quotes or semi-colon', `JustANormal123@#$password`] + [`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`], + [`Server=test1.database.windows.net;User Id=user;Password='ab""c;1''2''"''adf("0""12j^72''asj;'')''=33';Initial catalog=testdb`, 'validates values beginning with single quotes and also contains escaped single quotes', `ab""c;1'2'"'adf("0""12j^72'asj;')'=33`], + [`Server=test1.database.windows.net;User Id=user;Password=JustANormal123@#$password;Initial catalog=testdb`, 'validates values not beginning quotes and not containing quotes or semi-colon', `JustANormal123@#$password`], + [`User Id=user;Password=JustANormal123@#$password;Server=test1.database.windows.net;Initial catalog=testdb`, 'validates connection string without server', `JustANormal123@#$password`] ]; it.each(validConnectionStrings)('Input `%s` %s', (connectionStringInput, testDescription, passwordOutput) => { @@ -21,16 +22,20 @@ describe('SqlConnectionStringBuilder tests', () => { 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'); }); }) describe('throw for invalid connection strings', () => { let invalidConnectionStrings = [ - [`User Id=user;Password="ab'=abcdf''c;123;Initial catalog=testdb`, 'validates values beginning with double quotes but not ending with double quotes'], - [`User Id=user;Password='abc;1""2"adf=33;Initial catalog=testdb`, 'validates values beginning with single quote but not ending with single quote'], - [`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'], - [`User Id=user;Password='ab""c;1'2''"''adf("0""12j^72''asj;'')''=33';Initial catalog=testdb`, 'validates values enclosed in single quotes but does not escape single quotes in between'], - [`User Id=user;Password=NotANormal123@;#$password;Initial catalog=testdb`, 'validates values not enclosed in quotes and containing semi-colon'] + [`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'], + [`Server=test1.database.windows.net;User Id=user;Password='ab""c;1'2''"''adf("0""12j^72''asj;'')''=33';Initial catalog=testdb`, 'validates values enclosed in single quotes but does not escape single quotes in between'], + [`Server=test1.database.windows.net;User Id=user;Password=NotANormal123@;#$password;Initial catalog=testdb`, 'validates values not enclosed in quotes and containing semi-colon'], + [`Server=test1.database.windows.net;Password=password;Initial catalog=testdb`, 'missing user id'], + [`Server=test1.database.windows.net;User Id=user;Initial catalog=testdb`, 'missing password'], + [`Server=test1.database.windows.net;User Id=user;Password=password;`, 'missing initial catalog'] ]; it.each(invalidConnectionStrings)('Input `%s` %s', (connectionString) => { @@ -40,8 +45,8 @@ describe('SqlConnectionStringBuilder tests', () => { it('should mask connection string password', () => { let setSecretSpy = jest.spyOn(core, 'setSecret'); - new SqlConnectionStringBuilder('User Id=user;Password=1234;Initial Catalog=testDB'); + new SqlConnectionStringBuilder('User Id=user;Password=1234;Server=test1.database.windows.net;Initial Catalog=testDB'); expect(setSecretSpy).toHaveBeenCalled(); - }); + }); }) \ No newline at end of file diff --git a/__tests__/main.test.ts b/__tests__/main.test.ts index 91ff12f1..19f30b7b 100644 --- a/__tests__/main.test.ts +++ b/__tests__/main.test.ts @@ -36,7 +36,7 @@ describe('main.ts tests', () => { let addFirewallRuleSpy = jest.spyOn(FirewallManager.prototype, 'addFirewallRule'); let actionExecuteSpy = jest.spyOn(AzureSqlAction.prototype, 'execute'); let removeFirewallRuleSpy = jest.spyOn(FirewallManager.prototype, 'removeFirewallRule'); - let setFaledSpy = jest.spyOn(core, 'setFailed'); + let setFailedSpy = jest.spyOn(core, 'setFailed'); let detectIPAddressSpy = SqlUtils.detectIPAddress = jest.fn().mockImplementationOnce(() => { return ""; }); @@ -52,9 +52,9 @@ describe('main.ts tests', () => { expect(addFirewallRuleSpy).not.toHaveBeenCalled(); expect(actionExecuteSpy).toHaveBeenCalled(); expect(removeFirewallRuleSpy).not.toHaveBeenCalled(); - expect(setFaledSpy).not.toHaveBeenCalled(); + expect(setFailedSpy).not.toHaveBeenCalled(); }) - + it('gets inputs and executes sql action', async () => { let resolveFilePathSpy = jest.spyOn(AzureSqlActionHelper, 'resolveFilePath').mockReturnValue('./TestSqlFile.sql'); let getInputSpy = jest.spyOn(core, 'getInput').mockImplementation((name, options) => { @@ -66,7 +66,7 @@ describe('main.ts tests', () => { } }); - let setFaledSpy = jest.spyOn(core, 'setFailed'); + let setFailedSpy = jest.spyOn(core, 'setFailed'); let getAuthorizerSpy = jest.spyOn(AuthorizerFactory, 'getAuthorizer'); let addFirewallRuleSpy = jest.spyOn(FirewallManager.prototype, 'addFirewallRule'); let actionExecuteSpy = jest.spyOn(AzureSqlAction.prototype, 'execute'); @@ -86,7 +86,7 @@ describe('main.ts tests', () => { expect(addFirewallRuleSpy).not.toHaveBeenCalled(); expect(actionExecuteSpy).toHaveBeenCalled(); expect(removeFirewallRuleSpy).not.toHaveBeenCalled(); - expect(setFaledSpy).not.toHaveBeenCalled(); + expect(setFailedSpy).not.toHaveBeenCalled(); }) it('throws if input file is not found', async() => { @@ -106,11 +106,11 @@ describe('main.ts tests', () => { return ""; }); - let setFaledSpy = jest.spyOn(core, 'setFailed'); + let setFailedSpy = jest.spyOn(core, 'setFailed'); await run(); expect(AzureSqlAction).not.toHaveBeenCalled(); expect(detectIPAddressSpy).not.toHaveBeenCalled(); - expect(setFaledSpy).toHaveBeenCalledWith('Unable to find file at location'); + expect(setFailedSpy).toHaveBeenCalledWith('Unable to find file at location'); }) }) \ No newline at end of file diff --git a/action.yml b/action.yml index 18089156..4936f32f 100644 --- a/action.yml +++ b/action.yml @@ -3,16 +3,19 @@ description: 'Deploy a DACPAC or a SQL script to Azure SQL d inputs: server-name: description: 'Name of the Azure SQL Server name, like Fabrikam.database.windows.net.' - required: true + required: false connection-string: description: 'The connection string, including authentication information, for the Azure SQL Server database.' required: true dacpac-package: description: 'Path to DACPAC file to deploy' + required: false sql-file: description: 'Path to SQL script 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.' + required: false runs: using: 'node12' main: 'lib/main.js' diff --git a/src/SqlConnectionStringBuilder.ts b/src/SqlConnectionStringBuilder.ts index df925579..2e090d51 100644 --- a/src/SqlConnectionStringBuilder.ts +++ b/src/SqlConnectionStringBuilder.ts @@ -57,6 +57,10 @@ export default class SqlConnectionStringBuilder { return this._parsedConnectionString.database; } + public get server(): string { + return this._parsedConnectionString.server; + } + private _validateConnectionString() { if (!connectionStringTester.test(this._connectionString)) { throw new Error('Invalid connection string. A valid connection string is a series of keyword/value pairs separated by semi-colons. If there are any special characters like quotes, semi-colons in the keyword value, enclose the value within quotes. Refer this link for more info on conneciton string https://aka.ms/sqlconnectionstring'); diff --git a/src/main.ts b/src/main.ts index c2cf13c8..baf81416 100644 --- a/src/main.ts +++ b/src/main.ts @@ -48,11 +48,18 @@ export default async function run() { function getInputs(): IActionInputs { core.debug('Get action inputs.'); - let serverName = core.getInput('server-name', { required: true }); + let serverName = core.getInput('server-name', { required: false }); + let connectionString = core.getInput('connection-string', { required: true }); let connectionStringBuilder = new SqlConnectionStringBuilder(connectionString); + if ((!!serverName && !!connectionStringBuilder.server) && (serverName != connectionStringBuilder.server)) + core.debug("'server-name' is conflicting with 'server' property specified in the connection string. 'server-name' will take precedence."); + + // if serverName has not been specified, use the server name from the connection string + if (!serverName) serverName = connectionStringBuilder.server; + let additionalArguments = core.getInput('arguments'); let dacpacPackage = core.getInput('dacpac-package'); @@ -62,6 +69,10 @@ function getInputs(): IActionInputs { throw new Error(`Invalid dacpac file path provided as input ${dacpacPackage}`); } + if (!serverName) { + throw new Error(`Missing server name or address in the configuration.`); + } + return { serverName: serverName, connectionString: connectionStringBuilder, @@ -79,6 +90,10 @@ function getInputs(): IActionInputs { throw new Error(`Invalid sql file path provided as input ${sqlFilePath}`); } + if (!serverName) { + throw new Error(`Missing server name or address in the configuration.`); + } + return { serverName: serverName, connectionString: connectionStringBuilder, @@ -91,4 +106,4 @@ function getInputs(): IActionInputs { throw new Error('Required SQL file or DACPAC package to execute action.'); } -run(); \ No newline at end of file +run();