[ARVADOS] updated: 9d4f6f0284011fbc7a8edd203d1fab93a9a61321
git at public.curoverse.com
git at public.curoverse.com
Thu May 29 17:12:17 EDT 2014
Summary of changes:
sdk/python/bin/arv-put | 16 ++++++++++++++--
sdk/python/test_cmdline.py | 38 +++++++++++++++++++++++++++-----------
sdk/python/test_keep_client.py | 20 ++++++++++++++++----
3 files changed, 57 insertions(+), 17 deletions(-)
via 9d4f6f0284011fbc7a8edd203d1fab93a9a61321 (commit)
from 30b6c5a0f8dc91c76ca30ba4b5263b7eab858bdb (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 9d4f6f0284011fbc7a8edd203d1fab93a9a61321
Author: Tim Pierce <twp at curoverse.com>
Date: Thu May 29 17:08:22 2014 -0400
2755: code review.
* arv-put returns an unsigned manifest UUID.
* The ArvPutSignedManifest test first confirms that the collection is
not present in the API server. Also asserts that the arv-put command
completed successfully.
* Additional tests for KeepPermissionTestCase for each potential
combination of wrong authorization and signature.
diff --git a/sdk/python/bin/arv-put b/sdk/python/bin/arv-put
index 08b3f3d..241ac93 100755
--- a/sdk/python/bin/arv-put
+++ b/sdk/python/bin/arv-put
@@ -203,13 +203,25 @@ elif args.raw:
writer.finish_current_stream()
print string.join(writer.data_locators(), ',')
else:
+ manifest_locator = writer.finish()
+
+ # The manifest locator is also used as its UUID. Remove any
+ # signature Keep may have added to the locator; it should not be
+ # considered part of the UUID and will confuse apiserver if the
+ # signature is passed to arvados.api().collections().get().
+ # apiserver will resolve permissions by checking permission links
+ # anyway.
+ # TODO(twp,tomclegg): re-evaluate the value of storing manifests
+ # in Keep at all.
+ manifest_uuid = re.sub(r'\+A[a-z0-9 at _-]+', '', manifest_locator)
+
# Register the resulting collection in Arvados.
arvados.api().collections().create(
body={
- 'uuid': writer.finish(),
+ 'uuid': manifest_uuid,
'manifest_text': writer.manifest_text(),
},
).execute()
# Print the locator (uuid) of the new collection.
- print writer.finish()
+ print manifest_uuid
diff --git a/sdk/python/test_cmdline.py b/sdk/python/test_cmdline.py
index a7029e2..63b6767 100644
--- a/sdk/python/test_cmdline.py
+++ b/sdk/python/test_cmdline.py
@@ -1,13 +1,26 @@
import os
-import re
import subprocess
import unittest
import tempfile
import yaml
+import apiclient
import arvados
import run_test_server
+# ArvPutTest exercises arv-put behavior on the command line.
+#
+# Existing tests:
+#
+# ArvPutSignedManifest runs "arv-put foo" and then attempts to get
+# the newly created manifest from the API server, testing to confirm
+# that the block locators in the returned manifest are signed.
+#
+# TODO(twp): decide whether this belongs better in test_collections,
+# since it chiefly exercises behavior in arvados.collection.CollectionWriter.
+# Leaving it here for the time being because we may want to add more
+# tests for arv-put command line behavior.
+
class ArvPutTest(unittest.TestCase):
@classmethod
def setUpClass(cls):
@@ -40,26 +53,29 @@ class ArvPutTest(unittest.TestCase):
"ARVADOS_API_TOKEN"]:
os.environ[v] = arvados.config.settings()[v]
+ # Before doing anything, demonstrate that the collection
+ # we're about to create is not present in our test fixture.
+ api = arvados.api('v1', cache=False)
+ manifest_uuid = "00b4e9f40ac4dd432ef89749f1c01e74+47"
+ with self.assertRaises(apiclient.errors.HttpError):
+ notfound = api.collections().get(uuid=manifest_uuid).execute()
+
datadir = tempfile.mkdtemp()
with open(os.path.join(datadir, "foo"), "w") as f:
f.write("The quick brown fox jumped over the lazy dog")
- p = subprocess.Popen(["arv-put", datadir],
+ p = subprocess.Popen(["./bin/arv-put", datadir],
stdout=subprocess.PIPE)
(arvout, arverr) = p.communicate()
+ self.assertEqual(p.returncode, 0)
self.assertEqual(arverr, None)
-
- # The manifest UUID returned by arv-put must be signed.
- manifest_uuid = arvout.strip()
- self.assertRegexpMatches(manifest_uuid, r'\+A[0-9a-f]+@[0-9a-f]{8}')
-
- # The manifest text stored in Keep must contain unsigned locators.
- m = arvados.Keep.get(manifest_uuid)
- self.assertEqual(m, ". 08a008a01d498c404b0c30852b39d3b8+44 0:44:foo\n")
+ self.assertEqual(arvout.strip(), manifest_uuid)
# The manifest text stored in the API server under the same
# manifest UUID must use signed locators.
- api = arvados.api('v1', cache=False)
c = api.collections().get(uuid=manifest_uuid).execute()
self.assertRegexpMatches(
c['manifest_text'],
r'^\. 08a008a01d498c404b0c30852b39d3b8\+44\+A[0-9a-f]+@[0-9a-f]+ 0:44:foo\n')
+
+ os.remove(os.path.join(datadir, "foo"))
+ os.rmdir(datadir)
diff --git a/sdk/python/test_keep_client.py b/sdk/python/test_keep_client.py
index 6d0470a..99c9409 100644
--- a/sdk/python/test_keep_client.py
+++ b/sdk/python/test_keep_client.py
@@ -2,9 +2,10 @@
#
# ARVADOS_API_TOKEN=abc ARVADOS_API_HOST=arvados.local python -m unittest discover
+import os
import unittest
+
import arvados
-import os
import run_test_server
class KeepTestCase(unittest.TestCase):
@@ -105,21 +106,32 @@ class KeepPermissionTestCase(unittest.TestCase):
'foo',
'wrong content from Keep.get(md5("foo"))')
- # With Keep permissions enabled, a GET request without a signature will fail.
+ # GET with an unsigned locator => NotFound
bar_locator = arvados.Keep.put('bar')
+ unsigned_bar_locator = "37b51d194a7513e45b56f6524f2d51f2+3"
self.assertRegexpMatches(
bar_locator,
r'^37b51d194a7513e45b56f6524f2d51f2\+3\+A[a-f0-9]+@[a-f0-9]+$',
'invalid locator from Keep.put("bar"): ' + bar_locator)
self.assertRaises(arvados.errors.NotFoundError,
arvados.Keep.get,
- "37b51d194a7513e45b56f6524f2d51f2")
+ unsigned_bar_locator)
- # A request without an API token will also fail.
+ # GET from a different user => NotFound
+ run_test_server.authorize_with('spectator')
+ self.assertRaises(arvados.errors.NotFoundError,
+ arvados.Keep.get,
+ bar_locator)
+
+ # Unauthenticated GET for a signed locator => NotFound
+ # Unauthenticated GET for a signed locator => NotFound
del arvados.config.settings()["ARVADOS_API_TOKEN"]
self.assertRaises(arvados.errors.NotFoundError,
arvados.Keep.get,
bar_locator)
+ self.assertRaises(arvados.errors.NotFoundError,
+ arvados.Keep.get,
+ unsigned_bar_locator)
# KeepOptionalPermission: starts Keep with --permission-key-file
# but not --enforce-permissions (i.e. generate signatures on PUT
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list