)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e9ed0a25629e4ca9d7ed2b768097afb0d1c33ea7","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Martin Schuppert \u003cmschuppert@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-08-14 16:53:15 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Make pagination work with changes-since and deleted marker"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently, Nova does not paginate correctly if changes-since is"},{"line_number":10,"context_line":"being used and the marker is a deleted server. When trying to"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1a4dcd0f_b04b0b5f","line":7,"updated":"2015-08-14 15:53:15.000000000","message":"I think I would just be a little more general here. Even though you came from the changes-since issue, I think it\u0027s most clear (when looking back) to just say that a marker that is or gets deleted between queries will cause a BadRequest error.","commit_id":"d73ac6e7e479af01b207f5cacd85d0acd94e40cd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e9ed0a25629e4ca9d7ed2b768097afb0d1c33ea7","unresolved":false,"context_lines":[{"line_number":16,"context_line":"Change-Id: I79a1fb79a38ac4006db622590574308aafdcb5b5"},{"line_number":17,"context_line":"Closes-bug: #1481812"},{"line_number":18,"context_line":"Related-Bug: #1398086"},{"line_number":19,"context_line":"Signed-off-by: Martin Schuppert \u003cmschuppert@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1a4dcd0f_f0499361","line":19,"updated":"2015-08-14 15:53:15.000000000","message":"No need for this in openstack right now :)","commit_id":"d73ac6e7e479af01b207f5cacd85d0acd94e40cd"}],"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"b4084995cd0b7fe7bb200fd67dd7477c111a251b","unresolved":false,"context_lines":[{"line_number":2060,"context_line":"    # paginate query"},{"line_number":2061,"context_line":"    if marker is not None:"},{"line_number":2062,"context_line":"        try:"},{"line_number":2063,"context_line":"            if deleted or changes:"},{"line_number":2064,"context_line":"                marker \u003d _instance_get_by_uuid("},{"line_number":2065,"context_line":"                    context.elevated(read_deleted\u003d\u0027yes\u0027), marker,"},{"line_number":2066,"context_line":"                    session\u003dsession)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_6c8202ca","line":2063,"updated":"2015-08-06 17:28:55.000000000","message":"I wonder how we ran into this in the first place (https://bugs.launchpad.net/nova/+bug/1481812/comments/2).\n\nMy current understanding is that we explicitly forbid non-admin users to see deleted entries, but looks like passing the `changes-since` parameter to the compute API allows to circumvent that.\n\nAt the same time, when a user already saw the deleted entry and try to pass its UUID as a marker value, it can\u0027t be found (which is correct, as simply filter out all deleted entries).\n\nIn other words, looks like it\u0027s not the marker problem which should be fixed, but rather `changes-since` behaviour, so that it does not show deleted instances to non-admin users in the first place.","commit_id":"abdf086ef49cec564b7abe028383a43701cb4575"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c11b9fec5827501144eb70060f616e269d6497c1","unresolved":false,"context_lines":[{"line_number":2060,"context_line":"    # paginate query"},{"line_number":2061,"context_line":"    if marker is not None:"},{"line_number":2062,"context_line":"        try:"},{"line_number":2063,"context_line":"            if deleted or changes:"},{"line_number":2064,"context_line":"                marker \u003d _instance_get_by_uuid("},{"line_number":2065,"context_line":"                    context.elevated(read_deleted\u003d\u0027yes\u0027), marker,"},{"line_number":2066,"context_line":"                    session\u003dsession)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_a5b30f99","line":2063,"in_reply_to":"1a4dcd0f_0e36a502","updated":"2015-08-13 14:21:04.000000000","message":"Yeah, you\u0027re probably right. To be honest, I\u0027m not even sure that we expose the deleted marker anyway. If deleted\u003dFalse above, then we should be filtering deleted items out of the result, which means we\u0027re not exposing any deleted instances anyway, which makes this not an attack vector, AFAICT.","commit_id":"abdf086ef49cec564b7abe028383a43701cb4575"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0e1a5012682c970ddaac1862c4177c909668ebb1","unresolved":false,"context_lines":[{"line_number":2060,"context_line":"    # paginate query"},{"line_number":2061,"context_line":"    if marker is not None:"},{"line_number":2062,"context_line":"        try:"},{"line_number":2063,"context_line":"            if deleted or changes:"},{"line_number":2064,"context_line":"                marker \u003d _instance_get_by_uuid("},{"line_number":2065,"context_line":"                    context.elevated(read_deleted\u003d\u0027yes\u0027), marker,"},{"line_number":2066,"context_line":"                    session\u003dsession)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_6ee969e5","line":2063,"in_reply_to":"1a4dcd0f_6c8202ca","updated":"2015-08-13 13:41:59.000000000","message":"I\u0027m not sure your assertion is correct. IIRC, we show deleted flavors to people when they follow the link from their instance, but filter them out of the list view when they\u0027re looking for all supported flavors.\n\nI believe there is a race here between fetching the initial batch, and fetching the next page. If the marker server is deleted by another user between those two requests, then we would fail if we don\u0027t fetch deleted.\n\nAt a minimum, this doesn\u0027t seem to change what the user is seeing, and only makes the current behavior work properly. For that reason, I think this is probably a good fix. If we decide to change that behavior, we can do that separately.","commit_id":"abdf086ef49cec564b7abe028383a43701cb4575"},{"author":{"_account_id":8648,"name":"Radomir Dopieralski","email":"openstack@dopieralski.pl","username":"thesheep"},"change_message_id":"a3f9e8f42b2ab5e99f8eed07acc9942cdfcc0f1a","unresolved":false,"context_lines":[{"line_number":2060,"context_line":"    # paginate query"},{"line_number":2061,"context_line":"    if marker is not None:"},{"line_number":2062,"context_line":"        try:"},{"line_number":2063,"context_line":"            if deleted or changes:"},{"line_number":2064,"context_line":"                marker \u003d _instance_get_by_uuid("},{"line_number":2065,"context_line":"                    context.elevated(read_deleted\u003d\u0027yes\u0027), marker,"},{"line_number":2066,"context_line":"                    session\u003dsession)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a4dcd0f_0e36a502","line":2063,"in_reply_to":"1a4dcd0f_6ee969e5","updated":"2015-08-13 13:47:37.000000000","message":"I wonder why to add a special case for this, and not allow deleted markers always? That could potentially avoid more such race conditions?","commit_id":"abdf086ef49cec564b7abe028383a43701cb4575"}],"nova/tests/unit/db/test_db_api.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e9ed0a25629e4ca9d7ed2b768097afb0d1c33ea7","unresolved":false,"context_lines":[{"line_number":2149,"context_line":"        result \u003d db.instance_get_all_by_filters(self.ctxt,"},{"line_number":2150,"context_line":"                                                filters)"},{"line_number":2151,"context_line":"        self._assertEqualListsOfInstances([i2], result)"},{"line_number":2152,"context_line":""},{"line_number":2153,"context_line":"    def test_instance_get_all_by_filters_exact_match(self):"},{"line_number":2154,"context_line":"        instance \u003d self.create_instance_with_args(host\u003d\u0027host1\u0027)"},{"line_number":2155,"context_line":"        self.create_instance_with_args(host\u003d\u0027host12\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a4dcd0f_f08e7378","line":2152,"updated":"2015-08-14 15:53:15.000000000","message":"Ideally, we\u0027d have another test that just runs a nearly blank filter query and mocks everything out, just checking that read_deleted\u003dyes on that marker query. If you need help with such a test, ping me.","commit_id":"d73ac6e7e479af01b207f5cacd85d0acd94e40cd"}]}