diff options
| -rw-r--r-- | keystone/identity/backends/sql.py | 8 | ||||
| -rw-r--r-- | keystone/tests/unit/test_backend_sql.py | 38 | ||||
| -rw-r--r-- | releasenotes/notes/bug-1885753-51df25f3ff1d9ae8.yaml | 6 |
3 files changed, 52 insertions, 0 deletions
diff --git a/keystone/identity/backends/sql.py b/keystone/identity/backends/sql.py index cbe98305f..e3c884551 100644 --- a/keystone/identity/backends/sql.py +++ b/keystone/identity/backends/sql.py @@ -32,6 +32,10 @@ from keystone.identity.backends import sql_model as model CONF = keystone.conf.CONF +def _stale_data_exception_checker(exc): + return isinstance(exc, sqlalchemy.orm.exc.StaleDataError) + + class Identity(base.IdentityDriverBase): # NOTE(henry-nash): Override the __init__() method so as to take a # config parameter to enable sql to be used as a domain-specific driver. @@ -208,6 +212,10 @@ class Identity(base.IdentityDriverBase): return base.filter_user(user_ref.to_dict()) @sql.handle_conflicts(conflict_type='user') + # Explicitly retry on StaleDataErrors, which can happen if two clients + # update the same user's password and the second client has stale password + # information. + @oslo_db_api.wrap_db_retry(exception_checker=_stale_data_exception_checker) def update_user(self, user_id, user): with sql.session_for_write() as session: user_ref = self._get_user(session, user_id) diff --git a/keystone/tests/unit/test_backend_sql.py b/keystone/tests/unit/test_backend_sql.py index 82285fcf8..7e53dce43 100644 --- a/keystone/tests/unit/test_backend_sql.py +++ b/keystone/tests/unit/test_backend_sql.py @@ -16,9 +16,11 @@ import datetime from unittest import mock import uuid +import fixtures import freezegun from oslo_db import exception as db_exception from oslo_db import options +from oslo_log import log import sqlalchemy from sqlalchemy import exc from testtools import matchers @@ -983,6 +985,42 @@ class SqlIdentity(SqlTests, PROVIDERS.resource_api.check_project_depth, 2) + def test_update_user_with_stale_data_forces_retry(self): + # Capture log output so we know oslo.db attempted a retry + log_fixture = self.useFixture(fixtures.FakeLogger(level=log.DEBUG)) + + # Create a new user + user_dict = unit.new_user_ref( + domain_id=CONF.identity.default_domain_id) + new_user_dict = PROVIDERS.identity_api.create_user(user_dict) + + side_effects = [ + # Raise a StaleDataError simulating that another client has + # updated the user's password while this client's request was + # being processed + sqlalchemy.orm.exc.StaleDataError, + # The oslo.db library will retry the request, so the second + # time this method is called let's return a valid session + # object + sql.session_for_write() + ] + with mock.patch('keystone.common.sql.session_for_write') as m: + m.side_effect = side_effects + + # Update a user's attribute, the first attempt will fail but + # oslo.db will handle the exception and retry, the second attempt + # will succeed + new_user_dict['email'] = uuid.uuid4().hex + PROVIDERS.identity_api.update_user( + new_user_dict['id'], new_user_dict) + + # Make sure oslo.db retried the update by checking the log output + expected_log_message = ( + 'Performing DB retry for function keystone.identity.backends.' + 'sql.Identity.update_user' + ) + self.assertIn(expected_log_message, log_fixture.output) + class SqlTrust(SqlTests, trust_tests.TrustTests): diff --git a/releasenotes/notes/bug-1885753-51df25f3ff1d9ae8.yaml b/releasenotes/notes/bug-1885753-51df25f3ff1d9ae8.yaml new file mode 100644 index 000000000..f8f2d7f9c --- /dev/null +++ b/releasenotes/notes/bug-1885753-51df25f3ff1d9ae8.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + [`bug 1885753 <https://bugs.launchpad.net/keystone/+bug/1885753>`_] + Keystone's SQL identity backend now retries update user requests to safely + handle stale data when two clients update a user at the same time. |