[ARVADOS] updated: 2.1.0-1503-g5257fa183
Git user
git at public.arvados.org
Wed Oct 27 18:08:35 UTC 2021
Summary of changes:
lib/controller/localdb/collection.go | 12 +++-
lib/controller/localdb/collection_test.go | 94 +++++++++++++++++++++++++++++++
lib/controller/localdb/conn.go | 17 +++++-
sdk/go/arvados/vocabulary.go | 36 +++++++++---
sdk/go/arvados/vocabulary_test.go | 14 +++--
5 files changed, 158 insertions(+), 15 deletions(-)
via 5257fa18336a68881038e0cfb75a06d3c8dd3903 (commit)
via 776cd2b9e7eba58efa739345512c374eedd672d3 (commit)
from 3e9284a47aaaac14cf2ebb4104ffcc0d3960fcfe (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 5257fa18336a68881038e0cfb75a06d3c8dd3903
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Wed Oct 27 15:07:58 2021 -0300
17944: Adds vocabulary checking on collection's create & update.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/lib/controller/localdb/collection.go b/lib/controller/localdb/collection.go
index d81dd812b..6f99e57c4 100644
--- a/lib/controller/localdb/collection.go
+++ b/lib/controller/localdb/collection.go
@@ -49,8 +49,12 @@ func (conn *Conn) CollectionList(ctx context.Context, opts arvados.ListOptions)
}
// CollectionCreate defers to railsProxy for everything except blob
-// signatures.
+// signatures and vocabulary checking.
func (conn *Conn) CollectionCreate(ctx context.Context, opts arvados.CreateOptions) (arvados.Collection, error) {
+ err := conn.checkProperties(opts.Attrs["properties"])
+ if err != nil {
+ return arvados.Collection{}, err
+ }
if len(opts.Select) > 0 {
// We need to know IsTrashed and TrashAt to implement
// signing properly, even if the caller doesn't want
@@ -66,8 +70,12 @@ func (conn *Conn) CollectionCreate(ctx context.Context, opts arvados.CreateOptio
}
// CollectionUpdate defers to railsProxy for everything except blob
-// signatures.
+// signatures and vocabulary checking.
func (conn *Conn) CollectionUpdate(ctx context.Context, opts arvados.UpdateOptions) (arvados.Collection, error) {
+ err := conn.checkProperties(opts.Attrs["properties"])
+ if err != nil {
+ return arvados.Collection{}, err
+ }
if len(opts.Select) > 0 {
// We need to know IsTrashed and TrashAt to implement
// signing properly, even if the caller doesn't want
diff --git a/lib/controller/localdb/collection_test.go b/lib/controller/localdb/collection_test.go
index 4a4494964..44584fec0 100644
--- a/lib/controller/localdb/collection_test.go
+++ b/lib/controller/localdb/collection_test.go
@@ -6,6 +6,7 @@ package localdb
import (
"context"
+ "encoding/json"
"regexp"
"strconv"
"time"
@@ -48,6 +49,99 @@ func (s *CollectionSuite) TearDownTest(c *check.C) {
s.railsSpy.Close()
}
+func (s *CollectionSuite) setUpVocabulary(c *check.C, testVocabulary string) {
+ if testVocabulary == "" {
+ testVocabulary = `{
+ "strict_tags": false,
+ "tags": {
+ "IDTAGIMPORTANCES": {
+ "strict": true,
+ "labels": [{"label": "Importance"}, {"label": "Priority"}],
+ "values": {
+ "IDVALIMPORTANCES1": { "labels": [{"label": "Critical"}, {"label": "Urgent"}, {"label": "High"}] },
+ "IDVALIMPORTANCES2": { "labels": [{"label": "Normal"}, {"label": "Moderate"}] },
+ "IDVALIMPORTANCES3": { "labels": [{"label": "Low"}] }
+ }
+ }
+ }
+ }`
+ }
+ voc, err := arvados.NewVocabulary([]byte(testVocabulary))
+ c.Assert(err, check.IsNil)
+ c.Assert(voc.Validate(), check.IsNil)
+ s.cluster.API.Vocabulary = voc
+}
+
+func (s *CollectionSuite) TestCollectionCreateWithProperties(c *check.C) {
+ s.setUpVocabulary(c, "")
+ ctx := auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{arvadostest.ActiveTokenV2}})
+
+ tests := []struct {
+ name string
+ props string
+ success bool
+ }{
+ {"Invalid prop key", `{"Priority":"IDVALIMPORTANCES1"}`, false},
+ {"Invalid prop value", `{"IDTAGIMPORTANCES": "high"}`, false},
+ {"Valid prop key & value", `{"IDTAGIMPORTANCES": "IDVALIMPORTANCES1"}`, true},
+ {"Empty properties", "{}", true},
+ }
+ for _, tt := range tests {
+ c.Log(c.TestName()+" ", tt.name)
+
+ coll, err := s.localdb.CollectionCreate(ctx, arvados.CreateOptions{
+ Select: []string{"uuid", "properties"},
+ Attrs: map[string]interface{}{
+ "properties": tt.props,
+ }})
+ if tt.success {
+ c.Assert(err, check.IsNil)
+ var wantedProps map[string]interface{}
+ err = json.Unmarshal([]byte(tt.props), &wantedProps)
+ c.Assert(err, check.IsNil)
+ c.Assert(coll.Properties, check.DeepEquals, wantedProps)
+ } else {
+ c.Assert(err, check.NotNil)
+ }
+ }
+}
+
+func (s *CollectionSuite) TestCollectionUpdateWithProperties(c *check.C) {
+ s.setUpVocabulary(c, "")
+ ctx := auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{arvadostest.ActiveTokenV2}})
+
+ tests := []struct {
+ name string
+ props string
+ success bool
+ }{
+ {"Invalid prop key", `{"Priority":"IDVALIMPORTANCES1"}`, false},
+ {"Invalid prop value", `{"IDTAGIMPORTANCES": "high"}`, false},
+ {"Valid prop key & value", `{"IDTAGIMPORTANCES": "IDVALIMPORTANCES1"}`, true},
+ {"Empty properties", "{}", true},
+ }
+ for _, tt := range tests {
+ c.Log(c.TestName()+" ", tt.name)
+ coll, err := s.localdb.CollectionCreate(ctx, arvados.CreateOptions{})
+ c.Assert(err, check.IsNil)
+ coll, err = s.localdb.CollectionUpdate(ctx, arvados.UpdateOptions{
+ UUID: coll.UUID,
+ Select: []string{"uuid", "properties"},
+ Attrs: map[string]interface{}{
+ "properties": tt.props,
+ }})
+ if tt.success {
+ c.Assert(err, check.IsNil)
+ var wantedProps map[string]interface{}
+ err = json.Unmarshal([]byte(tt.props), &wantedProps)
+ c.Assert(err, check.IsNil)
+ c.Assert(coll.Properties, check.DeepEquals, wantedProps)
+ } else {
+ c.Assert(err, check.NotNil)
+ }
+ }
+}
+
func (s *CollectionSuite) TestSignatures(c *check.C) {
ctx := auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{arvadostest.ActiveTokenV2}})
diff --git a/lib/controller/localdb/conn.go b/lib/controller/localdb/conn.go
index a90deded5..b6bdf0de6 100644
--- a/lib/controller/localdb/conn.go
+++ b/lib/controller/localdb/conn.go
@@ -6,6 +6,7 @@ package localdb
import (
"context"
+ "encoding/json"
"fmt"
"strings"
@@ -25,8 +26,7 @@ type Conn struct {
func NewConn(cluster *arvados.Cluster) *Conn {
railsProxy := railsproxy.NewConn(cluster)
railsProxy.RedactHostInErrors = true
- var conn Conn
- conn = Conn{
+ conn := Conn{
cluster: cluster,
railsProxy: railsProxy,
}
@@ -34,6 +34,19 @@ func NewConn(cluster *arvados.Cluster) *Conn {
return &conn
}
+func (conn *Conn) checkProperties(properties interface{}) error {
+ if properties == nil {
+ return nil
+ }
+ // Check provided properties against the vocabulary.
+ var props map[string]interface{}
+ err := json.Unmarshal([]byte(properties.(string)), &props)
+ if err != nil {
+ return err
+ }
+ return conn.cluster.API.Vocabulary.Check(props)
+}
+
// Logout handles the logout of conn giving to the appropriate loginController
func (conn *Conn) Logout(ctx context.Context, opts arvados.LogoutOptions) (arvados.LogoutResponse, error) {
return conn.loginController.Logout(ctx, opts)
diff --git a/sdk/go/arvados/vocabulary.go b/sdk/go/arvados/vocabulary.go
index 97d19d78d..cf40eb6ff 100644
--- a/sdk/go/arvados/vocabulary.go
+++ b/sdk/go/arvados/vocabulary.go
@@ -131,7 +131,7 @@ func (v *Vocabulary) checkValue(key, val string) error {
if ok {
return fmt.Errorf("tag value %q for key %q is not defined but is an alias for %q", val, key, alias)
} else if v.Tags[key].Strict {
- return fmt.Errorf("tag value %q is not defined", val)
+ return fmt.Errorf("tag value %q for key %q is not listed as valid", val, key)
}
}
return nil
commit 776cd2b9e7eba58efa739345512c374eedd672d3
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Wed Oct 27 09:15:08 2021 -0300
17944: Adds list of strings support on vocabulary values.
Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/sdk/go/arvados/vocabulary.go b/sdk/go/arvados/vocabulary.go
index e978b3755..97d19d78d 100644
--- a/sdk/go/arvados/vocabulary.go
+++ b/sdk/go/arvados/vocabulary.go
@@ -124,6 +124,19 @@ func (v *Vocabulary) getLabelsToValues(key string) (labels map[string]string) {
return labels
}
+func (v *Vocabulary) checkValue(key, val string) error {
+ if _, ok := v.Tags[key].Values[val]; !ok {
+ lcVal := strings.ToLower(val)
+ alias, ok := v.getLabelsToValues(key)[lcVal]
+ if ok {
+ return fmt.Errorf("tag value %q for key %q is not defined but is an alias for %q", val, key, alias)
+ } else if v.Tags[key].Strict {
+ return fmt.Errorf("tag value %q is not defined", val)
+ }
+ }
+ return nil
+}
+
// Check validates the given data against the vocabulary.
func (v *Vocabulary) Check(data map[string]interface{}) error {
if v == nil {
@@ -143,14 +156,23 @@ func (v *Vocabulary) Check(data map[string]interface{}) error {
return nil
}
// Checks for value validity -- key is defined
- if _, ok := v.Tags[key].Values[val.(string)]; !ok {
- lcVal := strings.ToLower(val.(string))
- alias, ok := v.getLabelsToValues(key)[lcVal]
- if ok {
- return fmt.Errorf("tag value %q for key %q is not defined but is an alias for %q", val, key, alias)
- } else if v.Tags[key].Strict {
- return fmt.Errorf("tag value %q is not defined", val)
+ switch val := val.(type) {
+ case string:
+ return v.checkValue(key, val)
+ case []interface{}:
+ for _, singleVal := range val {
+ switch singleVal := singleVal.(type) {
+ case string:
+ err := v.checkValue(key, singleVal)
+ if err != nil {
+ return err
+ }
+ default:
+ return fmt.Errorf("tag value %q for key %q is not a valid type (%v)", singleVal, key, reflect.TypeOf(singleVal))
+ }
}
+ default:
+ return fmt.Errorf("tag value %q for key %q is not a valid type (%v)", val, key, reflect.TypeOf(val))
}
}
return nil
diff --git a/sdk/go/arvados/vocabulary_test.go b/sdk/go/arvados/vocabulary_test.go
index 3a3e4f1b5..3730e997e 100644
--- a/sdk/go/arvados/vocabulary_test.go
+++ b/sdk/go/arvados/vocabulary_test.go
@@ -64,13 +64,19 @@ func (s *VocabularySuite) TestCheck(c *check.C) {
props string
expectSuccess bool
}{
- {"Unknown key to non-strict vocabulary", false, `{"foo":"bar"}`, true},
- {"Unknown key to strict vocabulary", true, `{"foo":"bar"}`, false},
+ // Check succeeds
{"Known key, known value", false, `{"IDTAGANIMALS":"IDVALANIMAL1"}`, true},
- {"Known non-strict key, unknown value", false, `{"IDTAGANIMALS":"IDVALANIMAL3"}`, true},
+ {"Unknown non-alias key on non-strict vocabulary", false, `{"foo":"bar"}`, true},
+ {"Known non-strict key, unknown non-alias value", false, `{"IDTAGANIMALS":"IDVALANIMAL3"}`, true},
+ {"Known key, list of known values", false, `{"IDTAGANIMALS":["IDVALANIMAL1","IDVALANIMAL2"]}`, true},
+ {"Known non-strict key, list of unknown non-alias values", false, `{"IDTAGCOMMENT":["hello world","lorem ipsum"]}`, true},
+ // Check fails
+ {"Unknown non-alias key on strict vocabulary", true, `{"foo":"bar"}`, false},
{"Known non-strict key, known value alias", false, `{"IDTAGANIMALS":"Loxodonta"}`, false},
- {"Known strict key, unknown value", false, `{"IDTAGIMPORTANCE":"Unimportant"}`, false},
+ {"Known strict key, unknown non-alias value", false, `{"IDTAGIMPORTANCE":"Unimportant"}`, false},
{"Known strict key, known value alias", false, `{"IDTAGIMPORTANCE":"High"}`, false},
+ {"Known strict key, list of known alias values", false, `{"IDTAGIMPORTANCE":["Unimportant","High"]}`, false},
+ {"Known strict key, list of unknown non-alias values", false, `{"IDTAGIMPORTANCE":["foo","bar"]}`, false},
}
for _, tt := range tests {
c.Log(c.TestName()+" ", tt.name)
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list