DB method backup_get_all_by_volume only work for admin

[Rodrigo Barbieri]

While searching for an alternative fix for bug 1869746 and trying out the suggestion by Sofia Enriquez posted in that LP, I get a permission denied error at line [0] which I assumed was because admin privileges are required to perform that operation.

I had previously dismissed this as a dead end, but got back to investigate further, and found out that the error is actually due to [1]. That method is passing the volume_id to be compared with a project_id, which will never be true. Therefore this check is pointless for any user other than admin, which skips the check.

If that wasn't the case, perhaps an elevated context wouldn't need to be used here.

[0] https://github.com/openstack/cinder/blob/49b7e303c03515484833c86b2a435943b48ff52a/cinder/backup/api.py#L246

[1] https://github.com/openstack/cinder/blob/99c09bddceca754b550955db7e6786686316d525/cinder/db/sqlalchemy/api.py#L5273

[Brian Rosmaita (brian-rosmaita)]

Your analysis makes sense. Looks like line 5273 is the result of a careless copy of the previous function. I think we do want to get the project_id from the context in there somewhere, though, so that someone who knows your volume ID but isn't in your project can't get a listing of your backups.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/720833
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=8ebeafcbbafb700052f3abfc1f7ba004269a92e8
Submitter: Zuul
Branch: master

commit 8ebeafcbbafb700052f3abfc1f7ba004269a92e8
Author: Rodrigo Barbieri <email address hidden>
Date: Fri Apr 17 17:54:51 2020 -0300

    Fix cross-project incremental backups

    This patch addresses the scenario where an
    incremental backup can be created having a
    parent backup that was taken in a different
    project. This scenario ultimately leads to
    a silent error when creating/deleting the
    incremental backup and quotas going out of
    sync.

    The proposed fix is to narrow the backup search
    down to the same project. To achieve this, a
    method's signature had to be updated to achieve
    the desired optimized behavior of passing the
    volume's project_id parameter.

    Closes-bug: #1869746
    Closes-bug: #1873518

    Change-Id: Icb621ff5966133f59d9d43ca2dd9f8e1919b1149

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/738943

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/738943
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=5358c996b40bbb0fd749ec182ff3ef67f5d71c16
Submitter: Zuul
Branch: stable/ussuri

commit 5358c996b40bbb0fd749ec182ff3ef67f5d71c16
Author: Rodrigo Barbieri <email address hidden>
Date: Wed Jul 1 18:20:15 2020 -0300

    Fix cross-project incremental backups

    This patch addresses the scenario where an
    incremental backup can be created having a
    parent backup that was taken in a different
    project. This scenario ultimately leads to
    a silent error when creating/deleting the
    incremental backup and quotas going out of
    sync.

    The proposed fix is to narrow the backup search
    down to the same project. To achieve this, a
    method's signature had to be updated to achieve
    the desired optimized behavior of passing the
    volume's project_id parameter.

    Closes-bug: #1869746
    Closes-bug: #1873518

    Change-Id: Icb621ff5966133f59d9d43ca2dd9f8e1919b1149
    (cherry picked from commit 8ebeafcbbafb700052f3abfc1f7ba004269a92e8)

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/train
Review: https://review.opendev.org/739154

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/739154
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=f3cdc27563766b2f49a1d88500f32c8d86838c96
Submitter: Zuul
Branch: stable/train

commit f3cdc27563766b2f49a1d88500f32c8d86838c96
Author: Rodrigo Barbieri <email address hidden>
Date: Wed Jul 1 18:20:15 2020 -0300

    Fix cross-project incremental backups

    This patch addresses the scenario where an
    incremental backup can be created having a
    parent backup that was taken in a different
    project. This scenario ultimately leads to
    a silent error when creating/deleting the
    incremental backup and quotas going out of
    sync.

    The proposed fix is to narrow the backup search
    down to the same project. To achieve this, a
    method's signature had to be updated to achieve
    the desired optimized behavior of passing the
    volume's project_id parameter.

    Closes-bug: #1869746
    Closes-bug: #1873518

    conflicts:
        cinder/tests/unit/backup/test_backup.py

    Test test_create_backup_failed_with_empty_backup_objects
    required a mock for
    test_create_backup_failed_with_empty_backup_objects
    which had been removed in ussuri, as part of commit
    f0211b53b82cde96eb39e9914c3e5fae232db07c. That
    commit removed code and leftover the mock, which I
    had removed in the change this patch was
    cherry-picked from, as it was unnecessary.

    The removed code section of cinder/backup/api.py
    from commit f0211b53b82cde96eb39e9914c3e5fae232db07c
    does not impact the functionality of this change at all.

    Change-Id: Icb621ff5966133f59d9d43ca2dd9f8e1919b1149
    (cherry picked from commit 8ebeafcbbafb700052f3abfc1f7ba004269a92e8)
    (cherry picked from commit 5358c996b40bbb0fd749ec182ff3ef67f5d71c16)

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/stein
Review: https://review.opendev.org/739607

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/739607
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=5983eee1c35ababa1b6e2bad347cf98bd79b226f
Submitter: Zuul
Branch: stable/stein

commit 5983eee1c35ababa1b6e2bad347cf98bd79b226f
Author: Rodrigo Barbieri <email address hidden>
Date: Wed Jul 1 18:20:15 2020 -0300

    Fix cross-project incremental backups

    This patch addresses the scenario where an
    incremental backup can be created having a
    parent backup that was taken in a different
    project. This scenario ultimately leads to
    a silent error when creating/deleting the
    incremental backup and quotas going out of
    sync.

    The proposed fix is to narrow the backup search
    down to the same project. To achieve this, a
    method's signature had to be updated to achieve
    the desired optimized behavior of passing the
    volume's project_id parameter.

    Closes-bug: #1869746
    Closes-bug: #1873518

    Change-Id: Icb621ff5966133f59d9d43ca2dd9f8e1919b1149
    (cherry picked from commit 8ebeafcbbafb700052f3abfc1f7ba004269a92e8)
    (cherry picked from commit 5358c996b40bbb0fd749ec182ff3ef67f5d71c16)
    (cherry picked from commit f3cdc27563766b2f49a1d88500f32c8d86838c96)

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/rocky
Review: https://review.opendev.org/742221

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/742221
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=c4de3a8c838c4053ab60964cee53c6fa5e062d73
Submitter: Zuul
Branch: stable/rocky

commit c4de3a8c838c4053ab60964cee53c6fa5e062d73
Author: Rodrigo Barbieri <email address hidden>
Date: Wed Jul 1 18:20:15 2020 -0300

    Fix cross-project incremental backups

    This patch addresses the scenario where an
    incremental backup can be created having a
    parent backup that was taken in a different
    project. This scenario ultimately leads to
    a silent error when creating/deleting the
    incremental backup and quotas going out of
    sync.

    The proposed fix is to narrow the backup search
    down to the same project. To achieve this, a
    method's signature had to be updated to achieve
    the desired optimized behavior of passing the
    volume's project_id parameter.

    Closes-bug: #1869746
    Closes-bug: #1873518

    Change-Id: Icb621ff5966133f59d9d43ca2dd9f8e1919b1149
    (cherry picked from commit 8ebeafcbbafb700052f3abfc1f7ba004269a92e8)
    (cherry picked from commit 5358c996b40bbb0fd749ec182ff3ef67f5d71c16)
    (cherry picked from commit f3cdc27563766b2f49a1d88500f32c8d86838c96)
    (cherry picked from commit 5983eee1c35ababa1b6e2bad347cf98bd79b226f)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/cinder rocky-eol release.