19980: Recognize the arv: prefix for all system properties
authorBrett Smith <brett.smith@curii.com>
Mon, 20 Feb 2023 15:53:06 +0000 (10:53 -0500)
committerBrett Smith <brett.smith@curii.com>
Mon, 20 Feb 2023 19:55:27 +0000 (14:55 -0500)
arvados-cwl-runner currently uses this prefix to record Git information,
and we want to use it for all future system properties.

Arvados-DCO-1.1-Signed-off-by: Brett Smith <brett.smith@curii.com>

sdk/go/arvados/vocabulary.go
sdk/go/arvados/vocabulary_test.go

index 16f80db07d29741c0ffe825a5849e3bf7981436a..caab6e8fbee0a5b45690735da0505bea105886d7 100644 (file)
@@ -10,10 +10,13 @@ import (
        "errors"
        "fmt"
        "reflect"
+       "regexp"
        "strconv"
        "strings"
 )
 
+const systemKeyPattern = `^arv:[a-zA-Z]`
+
 type Vocabulary struct {
        reservedTagKeys map[string]bool          `json:"-"`
        StrictTags      bool                     `json:"strict_tags"`
@@ -26,7 +29,10 @@ type VocabularyTag struct {
        Values map[string]VocabularyTagValue `json:"values"`
 }
 
-// Cannot have a constant map in Go, so we have to use a function
+// Cannot have a constant map in Go, so we have to use a function.
+// If you are adding a new system property, it SHOULD match `systemKeyPattern`
+// above, and Check will allow it. This map is for historical exceptions that
+// predate standardizing on this prefix.
 func (v *Vocabulary) systemTagKeys() map[string]bool {
        return map[string]bool{
                // Collection keys - set by arvados-cwl-runner
@@ -265,9 +271,13 @@ func (v *Vocabulary) Check(data map[string]interface{}) error {
        if v == nil {
                return nil
        }
+       systemKeyRegexp, err := regexp.Compile(systemKeyPattern)
+       if err != nil {
+               return err
+       }
        for key, val := range data {
                // Checks for key validity
-               if v.reservedTagKeys[key] {
+               if systemKeyRegexp.MatchString(key) || v.reservedTagKeys[key] {
                        // Allow reserved keys to be used even if they are not defined in
                        // the vocabulary no matter its strictness.
                        continue
index 2025f97b337749eba8a3df038b666cd85b85bb5a..c06efda3e0a3de0fa91b38bf9831935b1ffb8bba 100644 (file)
@@ -6,6 +6,7 @@ package arvados
 
 import (
        "encoding/json"
+       "fmt"
        "regexp"
        "strings"
 
@@ -301,6 +302,44 @@ func (s *VocabularySuite) TestNewVocabulary(c *check.C) {
        }
 }
 
+func (s *VocabularySuite) TestValidSystemProperties(c *check.C) {
+       s.testVoc.StrictTags = true
+       properties := map[string]interface{}{
+               "arv:gitBranch": "main",
+               "arv:OK":        true,
+               "arv:cost":      123,
+       }
+       c.Check(s.testVoc.Check(properties), check.IsNil)
+}
+
+func (s *VocabularySuite) TestSystemPropertiesFirstCharacterAlphabetic(c *check.C) {
+       s.testVoc.StrictTags = true
+       properties := map[string]interface{}{"arv:": "value"}
+       c.Check(s.testVoc.Check(properties), check.NotNil)
+       // If we expand the list of allowed characters in the future, these lists
+       // may need adjustment to match.
+       for _, prefix := range []string{" ", ".", "_", "-", "1"} {
+               for _, suffix := range []string{"", "invalid"} {
+                       key := fmt.Sprintf("arv:%s%s", prefix, suffix)
+                       properties := map[string]interface{}{key: "value"}
+                       c.Check(s.testVoc.Check(properties), check.NotNil)
+               }
+       }
+}
+
+func (s *VocabularySuite) TestSystemPropertiesPrefixTypo(c *check.C) {
+       s.testVoc.StrictTags = true
+       for _, key := range []string{
+               "arv :foo",
+               "arvados",
+               "arvados:foo",
+               "Arv:foo",
+       } {
+               properties := map[string]interface{}{key: "value"}
+               c.Check(s.testVoc.Check(properties), check.NotNil)
+       }
+}
+
 func (s *VocabularySuite) TestValidationErrors(c *check.C) {
        tests := []struct {
                name       string