Bug #17508
closedArv-keepdocker implicitly tries to delete a collection protected property
Description
A user reports that when running arv-keepdocker
to upload an image, an error is returned like the following:
$ arv-keepdocker some-image:6.0.0 2021-03-30 20:20:56 arvados.arv_put[11053] INFO: Resuming upload from cache file /Users/username/.cache/arvados/arv-put/eb3268c0d9c1d015bfbeae7b33c3df2a 3801M / 3801M 100.0% 2021-03-30 20:20:57 arvados.arv_put[11053] INFO: 2021-03-30 20:20:57 arvados.arv_put[11053] INFO: Collection saved as 'Docker image some-image:6.0.0 sha256:12345 (2021-03-30T18:20:57.430Z)' zzzzz-4zz18-xxxxxxxxxxxxxx Traceback (most recent call last): File "/Users/username/.pyenv/versions/arvados/bin/arv-keepdocker", line 7, in <module> main() File "/Users/username/.pyenv/versions/3.6.8/envs/arvados/lib/python3.6/site-packages/arvados/commands/keepdocker.py", line 508, in main api.collections().update(uuid=coll_uuid, body={"properties": {"docker-image-repo-tag": image_repo_tag}}).execute(num_retries=args.retries) File "/Users/username/.pyenv/versions/3.6.8/envs/arvados/lib/python3.6/site-packages/googleapiclient/_helpers.py", line 130, in positional_wrapper return wrapped(*args, **kwargs) File "/Users/username/.pyenv/versions/3.6.8/envs/arvados/lib/python3.6/site-packages/googleapiclient/http.py", line 840, in execute raise HttpError(resp, content, uri=self.uri) arvados.errors.ApiError: <HttpError 403 when requesting https://example.com/arvados/v1/collections/zzzzz-4zz18-xxxxxxxxxxxxxx?alt=json returned "request failed: http://localhost:8000/arvados/v1/collections/zzzzz-4zz18-xxxxxxxxxxxxxx: 403 Forbidden: Protected property cannot be updated: responsible_person_uuid (req-xxxxxxxxxxxxxxxxxxxx)">
It seems the issue is that arv-keepdocker
tries to add a property to the recently created collection without realizing that it already has some auto-added property, so it doesn't include it on the update call, making the api server return an error because it looks like the client is trying to remove a protected property.
Updated by Lucas Di Pentima over 3 years ago
- Target version changed from 2021-03-31 sprint to 2021-04-14 sprint
Updated by Lucas Di Pentima over 3 years ago
- Status changed from New to In Progress
Updated by Lucas Di Pentima over 3 years ago
Fix at 3b011a2 - branch 17508-arvkeepdocker-fix
Test run: developer-run-tests: #2395
- Makes
arv-keepdocker
request the newly created collection before updating it with a new property, just in case it already has other properties. - Adds unit test.
Updated by Lucas Di Pentima over 3 years ago
Merged master with an unrelated fix at 341a593
Test run: developer-run-tests: #2395
Updated by Tom Clegg over 3 years ago
Just one nit: I think it would be a bit safer to do this
- api().collections().get().execute.return_value = mocked_collection
+ api().collections().get().execute.return_value = copy.deepcopy(mocked_collection)
to ensure the comparison with mocked_collection['properties'] still does what it looks like it does, even though the tested code modifies the values of the dict returned by execute().
Besides that, LGTM, thanks.
Updated by Lucas Di Pentima over 3 years ago
Applied suggested changes at fdfa325
Test run: developer-run-tests: #2398
Updated by Anonymous over 3 years ago
- % Done changed from 0 to 100
- Status changed from In Progress to Resolved
Applied in changeset arvados|c937aa7011849b84cf5a0bb01c18e262a94f95ad.