Running migrations on SQL Server fail - database is attempted to create though exists already

[eruponen]

Describe the bug
When attempting to run migration on Azure SQL Server - using Access Token and there's no explicit admin connection string defined - the migrations attempt to create a new database though the database already exists.

Version affected is 1.6.0. For 1.5.4 everything works as expected.

To Reproduce

1. Use Azure SQL Server.
2. Use Access Token authentication.
3. Don't define explicit admin connection string.
4. Attempt to run migrations against an existing database.

Expected behavior
The existing database is found as expected and it's not attempted to be created. Migrations are run successfully against it.

Screenshots

Desktop (please complete the following information):

• OS: Windows, Azure self-hosted pipeline agent

Additional context
Version affected 1.6.0.

[kimjamia]

I think this is related to Azure SQL Database and does not affect SQL Server. I did a rough test locally (with a server-level SQL LOGIN authentication instead of access token which might be significant when accessing the master database) and it worked without specifying the admin connection string. Then I tested against an Azure SQL Database with access token authentication with a database-contained user and the error appeared:
Unhandled exception: Microsoft.Data.SqlClient.SqlException (0x80131904): CREATE DATABASE permission denied in database 'master'.

I think this bug was introduced by #405 (SqlServerDatabase.cs) where there's a new override for DatabaseExists() which now seems to expect the admin connection string to be set.

[hoangthanh28]

It can be the case when grate is trying to create target db without admin permission, can you go with option do not create the database by adding --createdatabase=false into command argument. By default, grate always attempts creating the target db.
If the issue still happen, can you go down the log level to debug by using -v=Debug in grate command and share with us.

[eruponen]

@hoangthanh28 - with version 1.5.4 no such issue occurs so it suggests that the logic has changed recently in a breaking manner. Changing that flag sounds like a thing which could work, but why it's all of a sudden needed?

[hoangthanh28]

@eruponen yes, agreed with you, wouldn't introduce any breaking change in the new version. The PR above can cause this issue since I'm not aware this scenario, will check later this week when back from holiday then.

[erikbra]

This is probably a regression in 1.6.0, then, sorry. If not trying to create the database, we should not need the admin connection string. I did a lot of refactoring in 1.6.0 release, so there might be some regressions there.

Could you please give me an example (anonymized/generalized) connection string you use for Azure?
You are definitely sure the database does exist?

[erikbra]

I see the problem, definitely. You are correct, @kimjamia.

You had such a large PR that I didn't thoroughly read this one, @hoangthanh28, sorry. I'll create a rollback of this optimization. We cannot assume that all users have access to the master database.

[erikbra]

I think #442 should solve this. Would anyone care to take a look at the PR? Do we agree that would solve it?

[eruponen]

@erikbra - at least to me the code change makes sense. Thanks for the quick delivery!
Having a published preview version of 1.6.1 would allow us to test it easily as well.

[erikbra]

@eruponen - could you please confirm if/that grate 1.6.1 works for you?

[kimjamia]

I can confirm that it works with 1.6.1 now. Thanks for the quick reaction!

[eruponen]

@erikbra - As @kimjamia mentioned above the issue is resolved and with 1.6.1 our migration pipelines now work as expected. Many thanks!

[erikbra]

Perfect, guys! Thanks for the feedback, @eruponen and @kimjamia !