[ARVADOS] created: 4f4f6f5fe9367d5cd6c57070fd8a223efc87cb21
Git user
git at public.curoverse.com
Tue Nov 22 13:37:33 EST 2016
at 4f4f6f5fe9367d5cd6c57070fd8a223efc87cb21 (commit)
commit 4f4f6f5fe9367d5cd6c57070fd8a223efc87cb21
Merge: a57f860 9c3cc0f
Author: Tom Clegg <tom at curoverse.com>
Date: Tue Nov 22 13:35:06 2016 -0500
Merge branch 'master' into 9998-unsigned_manifest
Conflicts:
services/api/test/functional/arvados/v1/collections_controller_test.rb
diff --cc services/api/app/controllers/arvados/v1/collections_controller.rb
index cda7d6b,922cf7d..81accd8
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@@ -184,10 -182,10 +184,10 @@@ class Arvados::V1::CollectionsControlle
protected
def load_limit_offset_order_params *args
+ super
if action_name == 'index'
- # Omit manifest_text from index results unless expressly selected.
- @select ||= model_class.selectable_attributes - ["manifest_text"]
+ # Omit manifest_text and unsigned_manifest_text from index results unless expressly selected.
+ @select ||= model_class.selectable_attributes - ["manifest_text", "unsigned_manifest_text"]
end
- super
end
end
diff --cc services/api/test/functional/arvados/v1/collections_controller_test.rb
index c0d63e0,c85cc19..c285f8d
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@@ -61,20 -46,49 +61,63 @@@ class Arvados::V1::CollectionsControlle
end
end
+ test "index with unsigned_manifest_text selected returns only unsigned locators" do
+ authorize_with :active
+ get :index, select: ['unsigned_manifest_text']
+ assert_response :success
+ assert_operator json_response["items"].count, :>, 0
+ locs = 0
+ json_response["items"].each do |coll|
+ assert_equal(coll.keys - ['kind'], ['unsigned_manifest_text'],
+ "Collections index did not respect selected columns")
+ locs += assert_unsigned_manifest coll, coll['uuid']
+ end
+ assert_operator locs, :>, 0, "no locators found in any manifests"
+ end
+
+ test 'index without select returns everything except manifest' do
+ authorize_with :active
+ get :index
+ assert_response :success
+ assert json_response['items'].any?
+ json_response['items'].each do |coll|
+ assert_includes(coll.keys, 'uuid')
+ assert_includes(coll.keys, 'name')
+ assert_includes(coll.keys, 'created_at')
+ refute_includes(coll.keys, 'manifest_text')
+ end
+ end
+
+ ['', nil, false, 'null'].each do |select|
+ test "index with select=#{select.inspect} returns everything except manifest" do
+ authorize_with :active
+ get :index, select: select
+ assert_response :success
+ assert json_response['items'].any?
+ json_response['items'].each do |coll|
+ assert_includes(coll.keys, 'uuid')
+ assert_includes(coll.keys, 'name')
+ assert_includes(coll.keys, 'created_at')
+ refute_includes(coll.keys, 'manifest_text')
+ end
+ end
+ end
+
+ [["uuid"],
+ ["uuid", "manifest_text"],
+ '["uuid"]',
+ '["uuid", "manifest_text"]'].each do |select|
+ test "index with select=#{select.inspect} returns no name" do
+ authorize_with :active
+ get :index, select: select
+ assert_response :success
+ assert json_response['items'].any?
+ json_response['items'].each do |coll|
+ refute_includes(coll.keys, 'name')
+ end
+ end
+ end
+
[0,1,2].each do |limit|
test "get index with limit=#{limit}" do
authorize_with :active
commit a57f8602e4a89e526464587c78e91c2086aae8d8
Author: Tom Clegg <tom at curoverse.com>
Date: Tue Nov 22 13:23:41 2016 -0500
9998: Tidy up, add test.
diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index c7639b2..cda7d6b 100644
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -186,7 +186,7 @@ class Arvados::V1::CollectionsController < ApplicationController
def load_limit_offset_order_params *args
if action_name == 'index'
# Omit manifest_text and unsigned_manifest_text from index results unless expressly selected.
- @select ||= model_class.selectable_attributes - ["manifest_text","unsigned_manifest_text"]
+ @select ||= model_class.selectable_attributes - ["manifest_text", "unsigned_manifest_text"]
end
super
end
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 00330bd..879f029 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -26,7 +26,7 @@ class Collection < ArvadosModel
t.add :properties
t.add :portable_data_hash
t.add :signed_manifest_text, as: :manifest_text
- t.add :unsigned_manifest_text, as: :unsigned_manifest_text
+ t.add :manifest_text, as: :unsigned_manifest_text
t.add :replication_desired
t.add :replication_confirmed
t.add :replication_confirmed_at
@@ -219,12 +219,6 @@ class Collection < ArvadosModel
end
end
- def unsigned_manifest_text
- if has_attribute? :manifest_text
- @unsigned_manifest_text = manifest_text
- end
- end
-
def signed_manifest_text
if has_attribute? :manifest_text
token = current_api_client_authorization.andand.api_token
diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb
index a8583be..c0d63e0 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -2,6 +2,8 @@ require 'test_helper'
class Arvados::V1::CollectionsControllerTest < ActionController::TestCase
+ PERM_TOKEN_RE = /\+A[[:xdigit:]]+@[[:xdigit:]]{8}\b/
+
def permit_unsigned_manifests isok=true
# Set security model for the life of a test.
Rails.configuration.permit_create_collection_with_unsigned_manifest = isok
@@ -10,11 +12,23 @@ class Arvados::V1::CollectionsControllerTest < ActionController::TestCase
def assert_signed_manifest manifest_text, label=''
assert_not_nil manifest_text, "#{label} manifest_text was nil"
manifest_text.scan(/ [[:xdigit:]]{32}\S*/) do |tok|
- assert_match(/\+A[[:xdigit:]]+@[[:xdigit:]]{8}\b/, tok,
+ assert_match(PERM_TOKEN_RE, tok,
"Locator in #{label} manifest_text was not signed")
end
end
+ def assert_unsigned_manifest resp, label=''
+ txt = resp['unsigned_manifest_text']
+ assert_not_nil(txt, "#{label} unsigned_manifest_text was nil")
+ locs = 0
+ txt.scan(/ [[:xdigit:]]{32}\S*/) do |tok|
+ locs += 1
+ refute_match(PERM_TOKEN_RE, tok,
+ "Locator in #{label} unsigned_manifest_text was signed: #{tok}")
+ end
+ return locs
+ end
+
test "should get index" do
authorize_with :active
get :index
@@ -24,12 +38,13 @@ class Arvados::V1::CollectionsControllerTest < ActionController::TestCase
"basic Collections index included manifest_text")
end
- test "collections.get returns signed locators" do
+ test "collections.get returns signed locators, and no unsigned_manifest_text" do
permit_unsigned_manifests
authorize_with :active
get :show, {id: collections(:foo_file).uuid}
assert_response :success
assert_signed_manifest json_response['manifest_text'], 'foo_file'
+ refute_includes json_response, 'unsigned_manifest_text'
end
test "index with manifest_text selected returns signed locators" do
@@ -40,12 +55,26 @@ class Arvados::V1::CollectionsControllerTest < ActionController::TestCase
assert(assigns(:objects).andand.any?,
"no Collections returned for index with columns selected")
json_response["items"].each do |coll|
- assert_equal(columns, columns & coll.keys,
+ assert_equal(coll.keys - ['kind'], columns,
"Collections index did not respect selected columns")
assert_signed_manifest coll['manifest_text'], coll['uuid']
end
end
+ test "index with unsigned_manifest_text selected returns only unsigned locators" do
+ authorize_with :active
+ get :index, select: ['unsigned_manifest_text']
+ assert_response :success
+ assert_operator json_response["items"].count, :>, 0
+ locs = 0
+ json_response["items"].each do |coll|
+ assert_equal(coll.keys - ['kind'], ['unsigned_manifest_text'],
+ "Collections index did not respect selected columns")
+ locs += assert_unsigned_manifest coll, coll['uuid']
+ end
+ assert_operator locs, :>, 0, "no locators found in any manifests"
+ end
+
[0,1,2].each do |limit|
test "get index with limit=#{limit}" do
authorize_with :active
commit 53d04f5af565604b9cf5fd2e84d75549f8bb1c4f
Author: Joshua C. Randall <jcrandall at alum.mit.edu>
Date: Sat Nov 12 02:03:10 2016 +0000
change keep-balance to request unsigned_manifest_text from API server
diff --git a/services/keep-balance/collection.go b/services/keep-balance/collection.go
index f4fc721..147d616 100644
--- a/services/keep-balance/collection.go
+++ b/services/keep-balance/collection.go
@@ -43,7 +43,7 @@ func EachCollection(c *arvados.Client, pageSize int, f func(arvados.Collection)
params := arvados.ResourceListParams{
Limit: &limit,
Order: "modified_at, uuid",
- Select: []string{"uuid", "manifest_text", "modified_at", "portable_data_hash", "replication_desired"},
+ Select: []string{"uuid", "unsigned_manifest_text", "modified_at", "portable_data_hash", "replication_desired"},
}
var last arvados.Collection
var filterTime time.Time
commit 333f65cfbdc3cc4e19fc5029de409522d707b319
Author: Joshua C. Randall <jcrandall at alum.mit.edu>
Date: Sat Nov 12 02:01:36 2016 +0000
add support for unsigned manifests to SizedDigests() in Go SDK
diff --git a/sdk/go/arvados/collection.go b/sdk/go/arvados/collection.go
index 71f5247..295943b 100644
--- a/sdk/go/arvados/collection.go
+++ b/sdk/go/arvados/collection.go
@@ -14,6 +14,7 @@ type Collection struct {
UUID string `json:"uuid,omitempty"`
ExpiresAt *time.Time `json:"expires_at,omitempty"`
ManifestText string `json:"manifest_text,omitempty"`
+ UnsignedManifestText string `json:"unsigned_manifest_text,omitempty"`
CreatedAt *time.Time `json:"created_at,omitempty"`
ModifiedAt *time.Time `json:"modified_at,omitempty"`
PortableDataHash string `json:"portable_data_hash,omitempty"`
@@ -25,13 +26,17 @@ type Collection struct {
// SizedDigests returns the hash+size part of each data block
// referenced by the collection.
func (c *Collection) SizedDigests() ([]SizedDigest, error) {
- if c.ManifestText == "" && c.PortableDataHash != "d41d8cd98f00b204e9800998ecf8427e+0" {
+ manifestText := c.ManifestText
+ if manifestText == "" {
+ manifestText = c.UnsignedManifestText
+ }
+ if manifestText == "" && c.PortableDataHash != "d41d8cd98f00b204e9800998ecf8427e+0" {
// TODO: Check more subtle forms of corruption, too
return nil, fmt.Errorf("manifest is missing")
}
var sds []SizedDigest
- scanner := bufio.NewScanner(strings.NewReader(c.ManifestText))
- scanner.Buffer(make([]byte, 1048576), len(c.ManifestText))
+ scanner := bufio.NewScanner(strings.NewReader(manifestText))
+ scanner.Buffer(make([]byte, 1048576), len(manifestText))
for scanner.Scan() {
line := scanner.Text()
tokens := strings.Split(line, " ")
commit 99992d17f51e59ff80d52d10cca2102389ee2d9b
Author: Joshua C. Randall <jcrandall at alum.mit.edu>
Date: Sat Nov 12 01:15:57 2016 +0000
always omit unsigned_manifest_text when showing collection not in an index
diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index e96fa58..c7639b2 100644
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -31,6 +31,8 @@ class Arvados::V1::CollectionsController < ApplicationController
def show
if @object.is_a? Collection
+ # Omit unsigned_manifest_text
+ @select ||= model_class.selectable_attributes - ["unsigned_manifest_text"]
super
else
send_json @object
commit e2bfcb8bc5f030828754215aff1955f5ab4bd45a
Author: Joshua C. Randall <jcrandall at alum.mit.edu>
Date: Sat Nov 12 00:40:07 2016 +0000
implement unsigned_manifest_text column
diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index 44733cd..e96fa58 100644
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -183,8 +183,8 @@ class Arvados::V1::CollectionsController < ApplicationController
def load_limit_offset_order_params *args
if action_name == 'index'
- # Omit manifest_text from index results unless expressly selected.
- @select ||= model_class.selectable_attributes - ["manifest_text"]
+ # Omit manifest_text and unsigned_manifest_text from index results unless expressly selected.
+ @select ||= model_class.selectable_attributes - ["manifest_text","unsigned_manifest_text"]
end
super
end
diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 8579509..00330bd 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -26,6 +26,7 @@ class Collection < ArvadosModel
t.add :properties
t.add :portable_data_hash
t.add :signed_manifest_text, as: :manifest_text
+ t.add :unsigned_manifest_text, as: :unsigned_manifest_text
t.add :replication_desired
t.add :replication_confirmed
t.add :replication_confirmed_at
@@ -43,6 +44,7 @@ class Collection < ArvadosModel
# We need expires_at to determine the correct
# timestamp in signed_manifest_text.
'manifest_text' => ['manifest_text', 'expires_at'],
+ 'unsigned_manifest_text' => ['manifest_text'],
)
end
@@ -217,6 +219,12 @@ class Collection < ArvadosModel
end
end
+ def unsigned_manifest_text
+ if has_attribute? :manifest_text
+ @unsigned_manifest_text = manifest_text
+ end
+ end
+
def signed_manifest_text
if has_attribute? :manifest_text
token = current_api_client_authorization.andand.api_token
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list