ref PR #38 (#64)

dzsquared · yorek · web-flow · commit 61f1675e0d3d · 2022-01-05T10:02:39.000-08:00
* 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 <damauri@microsoft.com>
diff --git 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=<server.database.windows.net>;User ID=<user>;Password=<password>;Initial Catalog=<database>`
+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=<servername>; User ID=<user_id>; Password=<password>; Initial Catalog=<database>`. 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
@@ -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
@@ -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();
-    });
+    });    
 })
diff --git 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'); 
     })
 })
diff --git 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
@@ -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
