POST-as-COPY to DLO will compresses manifest

[Hugo Kou]

### DLO

The POST w/o POST_as_COPY messes up the DLO manifest-file, the `Manifest:` was dropped on the manifest file after the POST.

https://gist.github.com/HugoKuo/c45d394a362bf7a7ceb4

It's a must fixed issue from my point of view.

[Samuel Merritt (torgomatic)]

I see at least two separate bugs here.

1) POST without post-as-copy breaks a DLO manifest file. I think we had a similar bug for SLO manifests which we hacked around somehow.

2) Expiration doesn't remove large-object segments.

The first one is definitely worth fixing. The second one may require more discussion; just to start, what happens if a segment is referenced by two manifests with different expiration times?

If you split this into two bugs, the fix for the first won't get bogged down by the endless navel-gazing^W^Wdiscussion about the second.

[David Goetz (david-goetz)]

+1 to Sam. Pretty sure DLO non-post-as-copy never worked and should be fixed. If a second bug is made for 2) then should be a doc change only and handled by clients. Having any kind of tie between the manifest and their segments like that was never intended and would be basically impossible to manage.

[clayg (clay-gerrard)]

OK, let's make this bug the POST-as-COPY bug and I'll open a second for the "impossible to manage situation" [1]

1. Which will probably *also* get split into two bugs

1) to document how the two api features interact (in that they *don't*)

and

2) to update swiftclient to add expires headers to all the segments it creates when you upload a large object with a "delete-at" option

[Janie Richling (jrichli)]

I think the remaining issue here is a duplicate of: https://bugs.launchpad.net/swift/+bug/1487791

[Janie Richling (jrichli)]

The proposed patch 252791 and the original description seem to imply that the only issue is that the "X-Object-Manifest" header is not preserved on a POST when using post-as-copy. But fixing that alone, and not fixing the manifest "compression", is not acceptable. If the manifest header is preserved, then the manifest file must not be the concatenation of the segments. This part is harder to fix, and is also what the other issue was written to fix: https://bugs.launchpad.net/swift/+bug/1487791

[Janie Richling (jrichli)]

BTW, if you send a COPY with --multipart-manifest=get on it then it just copies the manifest (that's what i am told - have not tested it)

[pei ran wu (wupeiran)]

Thanks Janie.
I tried the workaround you mentioned for --multipart-manifest=get and the manifest object file will not be modified with a copy.
Now I try to fix it in the middleware layer.

[pei ran wu (wupeiran)]

The dlo fix tested no problem with post_as_copy =true but has issue for the xattr key when post_as_copy=false.
The behavior should be X-Object-Manifest NOT changed by the fast POST.

[pei ran wu (wupeiran)]

Since the metadata whole content is always replaced by key 'user.swift.metadata' based on xattr API. Is it suitable to separate the key of 'X-Object-Manifest' from metadata content to allow X-Object-Manifest NOT changed in the fast POST case?

[Alistair Coles (alistair-coles)]

Are we agreed that this bug is now a duplicate of https://bugs.launchpad.net/swift/+bug/1487791 ?

If so can we mark this one as a duplicate (being that later report) and shift discussion to the other?

[pei ran wu (wupeiran)]

@Alistair I think it's the same root cause since the POST give the wrong copy response for bug 1487791.
btw, I found the fast post case can not retrieve 'x-object-manifest' metadata since it's treated as NOT changed metadata in .data file xattr rather than .meta file.
Can we agree that the 'x-object-manifest' can be modified by POST in the url explicitly but not changed by default and always response the manifest header if no disallow in the conf.
BTW, I updated my patch on https://review.openstack.org/#/c/252791/ for both post_as_copy=true/false cases.

[Alistair Coles (alistair-coles)]

I am marking this a duplicate, lets continue all discussion on bug 1487791

[OpenStack Infra (hudson-openstack)]

Change abandoned by Alistair Coles (<email address hidden>) on branch: master
Review: https://review.openstack.org/252791
Reason: The fix is out of date w.r.t. copy feature being moved to copy middleware [1], at which point I believe the bug was fixed.

Apparently the useful tests here have been included in [2]

Please restore the review if I am incorrect.

[1] https://review.openstack.org/156923
[2] https://review.openstack.org/#/c/347545/

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/347545
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=13747021a8743d358961c1c4820fddfe9c955ea2
Submitter: Jenkins
Branch: master

commit 13747021a8743d358961c1c4820fddfe9c955ea2
Author: Janie Richling <email address hidden>
Date: Tue Jul 26 15:20:52 2016 -0500

    Add test for POST to DLO manifest file

    In the past, a POST to a DLO manifest file when object_post_as_copy
    was true resulted in the manifest file contents being replaced
    by the concatenation of the DLO segments. This no longer
    happens, but tests for this case are missing.

    This patch adds a functional test to assert that the manifest
    file is preserved in a POST request.

    Change-Id: I90546014a7dcc7266f0d0e0ff6339688b7954b96
    Related-bug: #1487791
    Related-bug: #1514317

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: feature/hummingbird
Review: https://review.openstack.org/363111

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/363111
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=1ab2a296f58ae76aeffef9f3f0fb90e15358be27
Submitter: Jenkins
Branch: feature/hummingbird

commit 3b5850836c59c46f2507a7f62aceccf4c37e5d41
Author: gecong1973 <email address hidden>
Date: Tue Aug 30 15:08:49 2016 +0800

    Remove white space between print and ()

    There is a white space between print and ()
    in /tempauth.py, This patch fix it

    Change-Id: Id3493bdef12223aa3a2bffc200db8710f5949101

commit f88e7fc0da2ed6a63e0ea3c3459d80772b3068cd
Author: zheng yin <email address hidden>
Date: Mon Aug 29 20:21:44 2016 +0800

    Clarify test case in common/ring/test_builder

    They use a bare assertRaises(ValueError, ring.RingBuilder, *,*,*), but
    it's not clear which one raises which ValueError(), so I extend them to
    validate the error strings as well.

    Change-Id: I63280a9fc47ff678fe143e635046a0b402fd4506

commit d68b1bd6ddf44c5088e9d02dcb2f1b802c71411b
Author: zhufl <email address hidden>
Date: Mon Aug 29 14:31:27 2016 +0800

    Remove unnecessary tearDown

    This is to remove unnecessary tearDown to keep code clean.

    Change-Id: Ie70e40d6b55f379b0cc9bc372a35705462cade8b

commit d2fc2614575b04fd9cab5ae589880b92eee9b186
Author: Matthew Oliver <email address hidden>
Date: Fri Aug 19 16:17:31 2016 +1000

    Authorise versioned write PUTs before copy

    Currently a versioned write PUT uses a pre-authed request to move
    it into the versioned container before checking whether the
    user is authorised. This can lead to some interesting behaviour
    whereby a user can select a versioned object path that it does not
    have access to, request a put on that versioned object, and this
    request will execute the copy part of the request before it fails
    due to lack of permissions.

    This patch changes the behaviour to be the same as versioned DELETE
    where the request is authorised before anything is moved.

    Change-Id: Ia8b92251718d10b1eb44a456f28d3d2569a30003
    Closes-Bug: #1562175

commit c1ef6539b6ba9d2e4354c9cd2eec8a0195cdb19f
Author: Clay Gerrard <email address hidden>
Date: Thu Aug 25 11:00:49 2016 -0700

    add test for expirer processes == process

    This is a follow up from a change that improved the error message.

    Related-Change: I3d12b79470d122b2114f9ee486b15d381f290f95

    Change-Id: I093801f3516a60b298c13e2aa026c11c68a63792

commit 01477c78c1163822de41484e914a0736e622085b
Author: zheng yin <email address hidden>
Date: Thu Aug 25 15:37:42 2016 +0800

    Fix ValueError information in obj/expirer

    I fix error information in raise ValueError(...)
    For example:
        if a>=b:
            # It should be under below and not 'a must be less than or equal to b'
            raise ValueError('a must be less than b')

    Change-Id: I3d12b79470d122b2114f9ee486b15d381f290f95

commit b81f53b964fdb8f3b50dd369ce2e194ee4dbb0b7
Author: zheng yin <email address hidden>
Date: Tue Aug 23 14:26:47 2016 +0800

    Improve readability in the obj server's unit tests

    This change improves the reada...