Unable to delete domain with users in it

While the error message is ugly (should not 500 the server) the behavior is intentional: we put a referential constraint on there to keep from accidentally deleting a load of data if the user accidentally deletes the domain: you have to disable the domain first, and then delete it for the same reason.

Although not explicitly in the SQL model definition, the FK is user.domain_id -> project.id is added in a migration: https://github.com/openstack/keystone/blob/2bd88d3/keystone/common/sql/expand_repo/versions/014_expand_add_domain_id_to_user_table.py#L140-L141

@Adam, but the error occurs when trying to delete a domain that is disabled. Notice the steps disable the domain before deleting it.

Fix proposed to branch: stable/pike
Review: https://review.openstack.org/511061

Fix proposed to branch: stable/ocata
Review: https://review.openstack.org/511062

Fix proposed to branch: stable/newton
Review: https://review.openstack.org/511063

Change abandoned by Tony Breeds (<email address hidden>) on branch: stable/newton
Review: https://review.openstack.org/511063
Reason: This branch (stable/newton) is at End Of Life

@samuel Are you still working on the proposed fix https://review.openstack.org/#/c/511061 ? It failed Zuul and status since October 2017 has not been changed.

Fix proposed to branch: master
Review: https://review.openstack.org/539347

"Unable to delete domain with users in it Edit" is the desired behavior. Please restate the bug. I think the problem is that if you try you get the database into an unstable state?

Deleting a domain should not be a casual thing. Once you do, all of the resources in the remote services are unmanagable. Removing the local user records, while annoying, ensures you do not accidentally destroy your entire user database by accidentally deleting a domain.

APIs have no feelings. Nor can it read the user's mind. All it care is performing the function it is designed to do. We've already required a domain to be disabled before being deleted. I know the Hawaii missile alert freaks people out. But come-on. :-) How many safe-guards do we need?

Heh. Before accidentally orphaning all of the users ain a domain? At least one more, I would say.

Adam - deleting all the users and groups was the original behavior: http://git.openstack.org/cgit/openstack/keystone/tree/keystone/identity/core.py#n491

The fact that it doesn't do that any more and emits a 500 error is a regression caused by the addition of a foreign key in the user table.

We require the domain to be disabled before deleting to prevent footgunning.

We could "fix" the behavior to issue a 4XX instead of exploding but this would not be in line with the current intended behavior.

The original behavior was bad, too. Would have been better to fix it deliberately than as a side effect.

The user side actually worries me less than the project side: deleting a project without deleting the resources in the remote services leave them unmanageable and require a system level admin to clean them up with some global scope of responsibility.

On the user side, disabling a domain prior to delete is necessary, as that keeps people from adding new users while someone else is trying frantically to delete them all before domain deletion.

Do we really want to do this?

> The user side actually worries me less than the project side: deleting a project without deleting the resources in the remote services leave them unmanageable and require a system level admin to clean them up with some global scope of responsibility.

That's not really our problem. We have to support deleting projects, and yes if you delete a project that still has a nova server in it then you're going to need to clean that up. That's already the case today and not the focus of this bug.

> Do we really want to do this?

Letting keystone emit a 500 is not an option, we either need to fix the old behavior or change the behavior to emit a validation error. We might be in a gray area now that we've already broken it but in general switching the response from a 2XX to a 4XX is not okay.

Reviewed: https://review.openstack.org/539347
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=62ee18b359cbb2e6a9469bdaac9057ef19de1bdf
Submitter: Zuul
Branch: master

commit 62ee18b359cbb2e6a9469bdaac9057ef19de1bdf
Author: Colleen Murphy <email address hidden>
Date: Tue Jan 30 23:23:15 2018 +0100

    Delete SQL users before deleting domain

    Since the users table has a foreign key to the projects table[1], users
    must be deleted before the domain can be deleted. However, the
    notification emitted from the domain deletion comes too late, and
    keystone runs into a foreign key reference error before it can delete
    the users. This patch addresses the problem by adding a new internal
    notification to alert the identity manager that users should be deleted.
    This uses a new notification rather than the existing notification
    because the existing one is used to alert listeners that the domain
    deletion has been fully completed, whereas this one must happen in the
    middle of the domain delete process.

    The callback must also only try to delete SQL users. The LDAP driver
    doesn't support deleting users, and we can't assume other drivers
    support it either. Moreover, the foreign key reference is only a problem
    for SQL users anyway.

    Because our backend unit tests run with SQLite and foreign keys do not
    work properly, we can't properly expose this bug in our unit tests, but
    there is an accompanying tempest test[2][3] to validate this fix.

    [1] https://github.com/openstack/keystone/blob/2bd88d3/keystone/common/sql/expand_repo/versions/014_expand_add_domain_id_to_user_table.py#L140-L141
    [2] https://review.openstack.org/#/c/509610
    [3] https://review.openstack.org/#/c/509947

    Change-Id: If5bdb6f5eef80b50b000aed5188ce7da4dfd1083
    Closes-bug: #1718747

Fix proposed to branch: stable/pike
Review: https://review.openstack.org/542483

This issue was fixed in the openstack/keystone 13.0.0.0rc1 release candidate.

Fix proposed to branch: stable/ocata
Review: https://review.openstack.org/543379

Reviewed: https://review.openstack.org/542483
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=fb81469404f4c6e8a5f25896d155e02491c3b610
Submitter: Zuul
Branch: stable/pike

commit fb81469404f4c6e8a5f25896d155e02491c3b610
Author: Colleen Murphy <email address hidden>
Date: Tue Jan 30 23:23:15 2018 +0100

    Delete SQL users before deleting domain

    Since the users table has a foreign key to the projects table[1], users
    must be deleted before the domain can be deleted. However, the
    notification emitted from the domain deletion comes too late, and
    keystone runs into a foreign key reference error before it can delete
    the users. This patch addresses the problem by adding a new internal
    notification to alert the identity manager that users should be deleted.
    This uses a new notification rather than the existing notification
    because the existing one is used to alert listeners that the domain
    deletion has been fully completed, whereas this one must happen in the
    middle of the domain delete process.

    The callback must also only try to delete SQL users. The LDAP driver
    doesn't support deleting users, and we can't assume other drivers
    support it either. Moreover, the foreign key reference is only a problem
    for SQL users anyway.

    Because our backend unit tests run with SQLite and foreign keys do not
    work properly, we can't properly expose this bug in our unit tests, but
    there is an accompanying tempest test[2][3] to validate this fix.

    [1] https://github.com/openstack/keystone/blob/2bd88d3/keystone/common/sql/expand_repo/versions/014_expand_add_domain_id_to_user_table.py#L140-L141
    [2] https://review.openstack.org/#/c/509610
    [3] https://review.openstack.org/#/c/509947

    Change-Id: If5bdb6f5eef80b50b000aed5188ce7da4dfd1083
    Closes-bug: #1718747
    (cherry picked from commit 62ee18b359cbb2e6a9469bdaac9057ef19de1bdf)

Change abandoned by Morgan Fainberg (<email address hidden>) on branch: stable/pike
Review: https://review.openstack.org/511061
Reason: Abandoning based upon IRC disucssions. this is not needed.

Reviewed: https://review.openstack.org/543379
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=b6a009254f7975bf806c499874ad87746bb8cbd8
Submitter: Zuul
Branch: stable/ocata

commit b6a009254f7975bf806c499874ad87746bb8cbd8
Author: Colleen Murphy <email address hidden>
Date: Tue Jan 30 23:23:15 2018 +0100

    Delete SQL users before deleting domain

    Since the users table has a foreign key to the projects table[1], users
    must be deleted before the domain can be deleted. However, the
    notification emitted from the domain deletion comes too late, and
    keystone runs into a foreign key reference error before it can delete
    the users. This patch addresses the problem by adding a new internal
    notification to alert the identity manager that users should be deleted.
    This uses a new notification rather than the existing notification
    because the existing one is used to alert listeners that the domain
    deletion has been fully completed, whereas this one must happen in the
    middle of the domain delete process.

    The callback must also only try to delete SQL users. The LDAP driver
    doesn't support deleting users, and we can't assume other drivers
    support it either. Moreover, the foreign key reference is only a problem
    for SQL users anyway.

    Because our backend unit tests run with SQLite and foreign keys do not
    work properly, we can't properly expose this bug in our unit tests, but
    there is an accompanying tempest test[2][3] to validate this fix.

    [1] https://github.com/openstack/keystone/blob/2bd88d3/keystone/common/sql/expand_repo/versions/014_expand_add_domain_id_to_user_table.py#L140-L141
    [2] https://review.openstack.org/#/c/509610
    [3] https://review.openstack.org/#/c/509947

    Change-Id: If5bdb6f5eef80b50b000aed5188ce7da4dfd1083
    Closes-bug: #1718747
    (cherry picked from commit 62ee18b359cbb2e6a9469bdaac9057ef19de1bdf)

Change abandoned by Lance Bragstad (<email address hidden>) on branch: master
Review: https://review.openstack.org/506340

Change abandoned by Lance Bragstad (<email address hidden>) on branch: stable/ocata
Review: https://review.openstack.org/511062

This issue was fixed in the openstack/keystone 11.0.4 release.

This issue was fixed in the openstack/keystone 12.0.1 release.