Opened 3 years ago
Closed 3 years ago
#33009 closed Bug (invalid)
./manage.py test --keepdb fails when DB permissions are not sufficient
Reported by: | Jed Laundry | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 3.2 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Background:
- I'm using Django==3.1.13, and mssql-django==1.0
- I have created Azure SQL Server, using Service Principal authentication, with two databases: [db] and [test_db]
- the Service Principal has been configured with db_owner on both databases (i.e.,
CREATE USER [app] FROM EXTERNAL PROVIDER; EXEC sp_addrolemember 'db_owner', [app];
) - I have updated DATABASES in settings.py with
Authentication=ActiveDirectoryServicePrincipal
I can use Django with no issues.
However, when I run ./manage.py test --keepdb
, I get the following error:
pyodbc.InterfaceError: ('28000', "[28000] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Login failed for user '<token-identified principal>'. (18456) (SQLDriverConnect)")
This is because the test runner first connects to [dbo].[master], verifies [test_db] exists, and then reconnects to [test_db]. However, by default Service Principals do not have permission to access [dbo].[master] (and ideally shouldn't, when using the same server for multiple applications).
This appears to be an intentional design decision to try connecting without the DB name even when using keepdb, as per django/db/backends/base/creation.py:50-55:
# We could skip this call if keepdb is True, but we instead # give it the keepdb param. This is to handle the case # where the test DB doesn't exist, in which case we need to # create it, then just not destroy it. If we instead skip # this, we will get an exception. self._create_test_db(verbosity, autoclobber, keepdb)
If I comment out this line, it works without issue.
I'm happy to create a patch, but would like guidance on which is more acceptable fix:
- create a new flag, along the lines of "keepdb_dont_create", to skip _create_test_db
- reuse the keepdb flag, and only call _create_test_db if it is False
- put _create_test_db into a try: and except: pass, to avoid DB permission issues entirely
Thanks for the report, however you using a 3rd-party database backend that can override
_create_test_db()
, if needed. I see you've already reported this on their issue tracker.