[ARVADOS] updated: 2.1.0-1507-gc89fe7089
Git user
git at public.arvados.org
Thu Oct 28 20:51:13 UTC 2021
Summary of changes:
lib/config/load.go | 6 +++-
lib/controller/localdb/collection_test.go | 2 +-
lib/controller/localdb/container_request_test.go | 2 +-
lib/controller/localdb/group_test.go | 2 +-
sdk/go/arvados/vocabulary.go | 38 +++++++++++++++++++++---
sdk/go/arvados/vocabulary_test.go | 26 +++++++++++++++-
6 files changed, 67 insertions(+), 9 deletions(-)
via c89fe7089e41c75de4e4d6339855e5768e2ad01c (commit)
from e16cea2d0f84c5e87e3b8b596267efad93df307c (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 c89fe7089e41c75de4e4d6339855e5768e2ad01c
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date: Thu Oct 28 17:47:48 2021 -0300
17944: Adds reserved keys support on vocabulary.
Managed properties & other system-level property keys are specially treated
so that they'll be accepted on strict vocabularies.
Also, don't accept a vocabulary that attempts to define one of these keys to
avoid surprising the users that any value is allowed.
onArvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>
diff --git a/lib/config/load.go b/lib/config/load.go
index a5b626a06..ee1000d14 100644
--- a/lib/config/load.go
+++ b/lib/config/load.go
@@ -405,7 +405,11 @@ func (ldr *Loader) loadVocabulary(cfg *arvados.Config) error {
if err != nil {
return fmt.Errorf("couldn't read vocabulary file %q: %v", cc.API.VocabularyPath, err)
}
- voc, err := arvados.NewVocabulary(vf)
+ mk := make([]string, 0, len(cc.Collections.ManagedProperties))
+ for k := range cc.Collections.ManagedProperties {
+ mk = append(mk, k)
+ }
+ voc, err := arvados.NewVocabulary(vf, mk)
if err != nil {
return fmt.Errorf("while loading vocabulary file %q: %s", cc.API.VocabularyPath, err)
}
diff --git a/lib/controller/localdb/collection_test.go b/lib/controller/localdb/collection_test.go
index d7395d750..09a8dfbe1 100644
--- a/lib/controller/localdb/collection_test.go
+++ b/lib/controller/localdb/collection_test.go
@@ -65,7 +65,7 @@ func (s *CollectionSuite) setUpVocabulary(c *check.C, testVocabulary string) {
}
}`
}
- voc, err := arvados.NewVocabulary([]byte(testVocabulary))
+ voc, err := arvados.NewVocabulary([]byte(testVocabulary), []string{})
c.Assert(err, check.IsNil)
c.Assert(voc.Validate(), check.IsNil)
s.cluster.API.Vocabulary = voc
diff --git a/lib/controller/localdb/container_request_test.go b/lib/controller/localdb/container_request_test.go
index ce55d650b..ab9d5b093 100644
--- a/lib/controller/localdb/container_request_test.go
+++ b/lib/controller/localdb/container_request_test.go
@@ -62,7 +62,7 @@ func (s *ContainerRequestSuite) setUpVocabulary(c *check.C, testVocabulary strin
}
}`
}
- voc, err := arvados.NewVocabulary([]byte(testVocabulary))
+ voc, err := arvados.NewVocabulary([]byte(testVocabulary), []string{})
c.Assert(err, check.IsNil)
c.Assert(voc.Validate(), check.IsNil)
s.cluster.API.Vocabulary = voc
diff --git a/lib/controller/localdb/group_test.go b/lib/controller/localdb/group_test.go
index d6b739c41..37bddfbd0 100644
--- a/lib/controller/localdb/group_test.go
+++ b/lib/controller/localdb/group_test.go
@@ -62,7 +62,7 @@ func (s *GroupSuite) setUpVocabulary(c *check.C, testVocabulary string) {
}
}`
}
- voc, err := arvados.NewVocabulary([]byte(testVocabulary))
+ voc, err := arvados.NewVocabulary([]byte(testVocabulary), []string{})
c.Assert(err, check.IsNil)
c.Assert(voc.Validate(), check.IsNil)
s.cluster.API.Vocabulary = voc
diff --git a/sdk/go/arvados/vocabulary.go b/sdk/go/arvados/vocabulary.go
index 336927282..cb1106e9b 100644
--- a/sdk/go/arvados/vocabulary.go
+++ b/sdk/go/arvados/vocabulary.go
@@ -13,8 +13,9 @@ import (
)
type Vocabulary struct {
- StrictTags bool `json:"strict_tags"`
- Tags map[string]VocabularyTag `json:"tags"`
+ reservedTagKeys map[string]bool `json:"-"`
+ StrictTags bool `json:"strict_tags"`
+ Tags map[string]VocabularyTag `json:"tags"`
}
type VocabularyTag struct {
@@ -23,6 +24,20 @@ type VocabularyTag struct {
Values map[string]VocabularyTagValue `json:"values"`
}
+// Cannot have a constant map in Go, so we have to use a function
+func (v *Vocabulary) systemTagKeys() map[string]bool {
+ return map[string]bool{
+ "type": true,
+ "template_uuid": true,
+ "groups": true,
+ "username": true,
+ "image_timestamp": true,
+ "docker-image-repo-tag": true,
+ "filters": true,
+ "container_request": true,
+ }
+}
+
type VocabularyLabel struct {
Label string `json:"label"`
}
@@ -31,7 +46,7 @@ type VocabularyTagValue struct {
Labels []VocabularyLabel `json:"labels"`
}
-func NewVocabulary(data []byte) (voc *Vocabulary, err error) {
+func NewVocabulary(data []byte, managedTagKeys []string) (voc *Vocabulary, err error) {
if r := bytes.Compare(data, []byte("")); r == 0 {
return &Vocabulary{}, nil
}
@@ -42,6 +57,13 @@ func NewVocabulary(data []byte) (voc *Vocabulary, err error) {
if reflect.DeepEqual(voc, &Vocabulary{}) {
return nil, fmt.Errorf("JSON data provided doesn't match Vocabulary format: %q", data)
}
+ voc.reservedTagKeys = make(map[string]bool)
+ for _, managedKey := range managedTagKeys {
+ voc.reservedTagKeys[managedKey] = true
+ }
+ for systemKey := range voc.systemTagKeys() {
+ voc.reservedTagKeys[systemKey] = true
+ }
err = voc.Validate()
if err != nil {
return nil, err
@@ -60,6 +82,9 @@ func (v *Vocabulary) Validate() error {
}
// Checks for duplicate tag keys
for key := range v.Tags {
+ if v.reservedTagKeys[key] {
+ return fmt.Errorf("tag key %q is reserved", key)
+ }
if tagKeys[key] {
return fmt.Errorf("duplicate tag key %q", key)
}
@@ -144,6 +169,11 @@ func (v *Vocabulary) Check(data map[string]interface{}) error {
}
for key, val := range data {
// Checks for key validity
+ if v.reservedTagKeys[key] {
+ // Allow reserved keys to be used even if they are not defined in
+ // the vocabulary no matter its strictness.
+ continue
+ }
if _, ok := v.Tags[key]; !ok {
lcKey := strings.ToLower(key)
alias, ok := v.getLabelsToKeys()[lcKey]
@@ -153,7 +183,7 @@ func (v *Vocabulary) Check(data map[string]interface{}) error {
return fmt.Errorf("tag key %q is not defined", key)
}
// If the key is not defined, we don't need to check the value
- return nil
+ continue
}
// Checks for value validity -- key is defined
switch val := val.(type) {
diff --git a/sdk/go/arvados/vocabulary_test.go b/sdk/go/arvados/vocabulary_test.go
index 043c37139..b2748c7be 100644
--- a/sdk/go/arvados/vocabulary_test.go
+++ b/sdk/go/arvados/vocabulary_test.go
@@ -18,6 +18,9 @@ var _ = check.Suite(&VocabularySuite{})
func (s *VocabularySuite) SetUpTest(c *check.C) {
s.testVoc = &Vocabulary{
+ reservedTagKeys: map[string]bool{
+ "reservedKey": true,
+ },
StrictTags: false,
Tags: map[string]VocabularyTag{
"IDTAGANIMALS": {
@@ -68,6 +71,7 @@ func (s *VocabularySuite) TestCheck(c *check.C) {
{"Known key, known value", false, `{"IDTAGANIMALS":"IDVALANIMAL1"}`, 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},
+ {"Undefined but reserved key on strict vocabulary", true, `{"reservedKey":"bar"}`, 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
@@ -120,6 +124,16 @@ func (s *VocabularySuite) TestNewVocabulary(c *check.C) {
}}`,
true, "",
&Vocabulary{
+ reservedTagKeys: map[string]bool{
+ "type": true,
+ "template_uuid": true,
+ "groups": true,
+ "username": true,
+ "image_timestamp": true,
+ "docker-image-repo-tag": true,
+ "filters": true,
+ "container_request": true,
+ },
StrictTags: false,
Tags: map[string]VocabularyTag{
"IDTAGANIMALS": {
@@ -137,11 +151,21 @@ func (s *VocabularySuite) TestNewVocabulary(c *check.C) {
},
},
},
+ {
+ "Valid data, but uses reserved key",
+ `{"tags":{
+ "type":{
+ "strict": false,
+ "labels": [{"label": "Type"}]
+ }
+ }}`,
+ false, "tag key.*is reserved", nil,
+ },
}
for _, tt := range tests {
c.Log(c.TestName()+" ", tt.name)
- voc, err := NewVocabulary([]byte(tt.data))
+ voc, err := NewVocabulary([]byte(tt.data), []string{})
if tt.isValid {
c.Assert(err, check.IsNil)
} else {
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list