Introducing sql-action v2

[zijchen]

Merging v2 changes to master branch.

Main changes in v2:

• Adds support for ActiveDirectoryPassword, ActiveDirectoryServicePrincipal, and ActiveDirectoryDefault connection strings.
• Adds support for Script, DeployReport, and DritReport sqlpackage actions (see Add support for Script, DeployReport, and DriftReport #106)
• Uses go-sqlcmd instead of sqlcmd for script action. This makes script action possible with the different AAD authentication types. go-sqlcmd will be automatically downloaded and cached on the runner VM if it is not found.
• [Backend changes] Uses mssql for connection string parsing and the initial firewall check, also makes AAD possible.
• Various bug fixes

Breaking changes from v1:

For complete documentation for v2, visit the README on v2 branch

• Changes to the yaml inputs:
  • The project-file, dacpac-package, sql-file parameters are consolidated into a single path parameter. sql-action will determine the appropriate action based on the file extension.
  • New action parameter used to specify the sqlpackage action (when .sqlproj or .dacpac files are used)
  • Additional arguments and build-arguments for optional arguments
• go-sqlcmd has some breaking changes from sqlcmd, view the complete list here: https://docs.microsoft.com/sql/tools/go-sqlcmd-utility#breaking-changes-from-sqlcmd

[vermegi]

@zijchen I am in the middle of testing this out with a Service Principal logging, but v2 keeps on failing with:
Error: Failed to add firewall rule. Unable to detect client IP Address. sqlcmd: error: unexpected argument Directory.

Action config looks like:

database-updates:
runs-on: ubuntu-latest
needs: deploy-infra
steps:
- uses: actions/checkout@v2
- name: Azure SQL Allow MI
uses: Azure/sql-action@v2
with:
# Name of the Azure SQL Server name, like Fabrikam.database.windows.net.
# server-name: ${{ steps.deploy.outputs.sqlServerFullyQualifiedDomainName }}
# The connection string, including authentication information, for the Azure SQL Server database.
connection-string: ${{ format('Server={0};Initial Catalog={1};Authentication=Active Directory Service Principal;User Id={2};Password={3}', needs.deploy-infra.outputs.sqlServerFullyQualifiedDomainName, needs.deploy-infra.outputs.databaseName, secrets.SQL_USERNAME, secrets.SQL_PASSWORD) }}
# Path to SQL script file to deploy
path: ./deploy/mi.sql

Where SQL_USERNAME contains SP clientID and SQL_PASSWORD contains clientSecret of the SP.

I do have a previous job that has an azure login. But the login is not present in the database-updates job, so I would guess this then defaults to not setting temp FW rules and uses current FW rules. SQL Server in Azure has been setup with allow azure services access.
SP has contributor on the RG like indicated in the new README file on the v2 branch.

[vermegi]

So the issue in my case is in the connection string:
sqlcmd: error: unexpected argument Directory.

changed connection string to:

connection-string: ${{ format('Server={0};Initial Catalog={1};Authentication="Active Directory Service Principal";User Id={2};Password={3}', needs.deploy-infra.outputs.sqlServerFullyQualifiedDomainName, needs.deploy-infra.outputs.databaseName, secrets.SQL_USERNAME, secrets.SQL_PASSWORD) }}

Might want to change that error message to 'Failed to connect to database' instead of 'failed to add FW rule', since that is misleading.

Also, might want to add the format of the connectionstring in the README file. Currently I had to look for it in the test cases in the code.

Still struggling a bit with the rest of the connection string ... will let you know once I get this working.

[zijchen]

'Server={0};Initial Catalog={1};Authentication=Active Directory Service Principal;User Id={2};Password={3}'

@vermegi Thank you for trying v2. Your connection string should have worked, I have submitted #137 to fix the issue of requiring quotation marks around the authentication option. Could you tell us more about the issue you're still facing after adding the quotation marks? Feel free to share your debug logs with us too.

[dzsquared]

@vermegi @zijchen I'll have a PR into v2 with updates to the readme with connection string info

done - #138

[vermegi]

Still getting an error:
mssql: login error: Login failed for user ''.

This is the connection string:
Run Azure/sql-action@v2
with:
connection-string: Server=appsvcnetworkingdemoxprjxhuoxapcy.database.windows.net;Initial Catalog=sampledb;Authentication="Active Directory Service Principal";User Id=;
path: ./deploy/mi.sql
env:
AZURE_RESOURCEGROUP_NAME: appsvcnetworkingdemo

I tried both clientId and clientId@tenant of the service principal as value for the User Id. The service principal has both contributor and SQL Security Manager permissions on the resource group.

Any hints on how to properly make the login work would be appreciated.

[vermegi]

Btw, the actual connection string is this:
connection-string: ${{ format('Server={0};Initial Catalog={1};Authentication="Active Directory Service Principal";User Id={2};Password={3}', needs.deploy-infra.outputs.sqlServerFullyQualifiedDomainName , needs.deploy-infra.outputs.sqlDatabaseName, secrets.SQL_USERNAME, secrets.SQL_PASSWORD) }}

Full workflow file: https://github.com/vermegi/app-service-networking-samples/blob/givermei-workflow-update/.github/workflows/infradeploy.yml

It's just in the action workflow output that 'Password' seems to get fully obfuscated.

Error:

[zijchen]

@vermegi I tried something exactly like your yaml in my own repo and it seemed to work. I have some ideas you can try:

1. See if you can call go-sqlcmd directly from your machine. If you also get the "Login failed for user <token-identified principal>" error then it's likely a SQL issue.

SET SQLCMDPASSWORD=<service_principal_client_secret>
sqlcmd -S <server_name> -d <database_name> --authentication-method=ActiveDirectoryServicePrincipal -U <client_id> -i <sql_file>

2. Check to make sure the service principal has a corresponding user in the server and it has the appropriate permissions. You should see a row from the query below with the name of your service principal user.

SELECT * FROM sys.database_principals WHERE type = 'E'

3. If the service principal used for sql-action the same as the one you're using for the azure/login step? If so, in your database-updates job, could you try adding the azure/login, then changing the connection string to "Active Directory Default" authentication, no need for user ID/password. sql-action should be able to connect using the credentials from the azure/login step.

[vermegi]

I probably have a chicken or egg problem there, then, since trying to automate a full setup ... and this is not an issue of this specific action btw, but of SQL Server on Azure in general.

[vermegi]

Login succeeds and works like a charm when I first add my SP as a user in the database.
For my deployment script however the chicken or egg problem remains, but again, that's not an issue on this action, it's a SQL issue.

[zijchen]

Login succeeds and works like a charm when I first add my SP as a user in the database. For my deployment script however the chicken or egg problem remains, but again, that's not an issue on this action, it's a SQL issue.

That's good to hear. Maybe you can first create an AAD user and run this action with AAD Password auth to create your SP user?

[vermegi]

Well, thing is I don't have the permissions to do that on the tenant I'm using. So will unfortunately not be able to perform that test.