Repository: appc/spec
Pull request #657 (2016-09-21 01:39:13)

CRI: Change CRILabels type to map[string]string.
Follow https://github.com/appc/spec/pull/656#discussion_r79746750

Here I see two solutions:
1, use `CRIAnnotations` for those labels I mentioned in the above comments.
2, Merge this PR to allow arbitrary string in labels.

I am fine with both.

/cc @squeed @lucab @s-urbaniak 




Patch:
diff --git a/schema/types/app.go b/schema/types/app.go
index 88435f2..df383a2 100644
--- a/schema/types/app.go
+++ b/schema/types/app.go
@@ -91,8 +91,5 @@ func (a *App) assertValid() error {
 	if err := a.Isolators.assertValid(); err != nil {
 		return err
 	}
-	if err := a.CRILabels.assertValid(); err != nil {
-		return err
-	}
 	return nil
 }
diff --git a/schema/types/cri_labels.go b/schema/types/cri_labels.go
index 7253b73..8a87cd0 100644
--- a/schema/types/cri_labels.go
+++ b/schema/types/cri_labels.go
@@ -14,18 +14,5 @@
 
 package types
 
-import "fmt"
-
-type CRILabels map[ACIdentifier]string
-
-func (l CRILabels) assertValid() error {
-	for k, _ := range l {
-		if err := k.assertValid(); err != nil {
-			return err
-		}
-		if len(k) > 63 {
-			return fmt.Errorf(`label %q too long`, k)
-		}
-	}
-	return nil
-}
+// CRILabels are arbitrary key-value pairs
+type CRILabels map[string]string


Test patch:
diff --git a/schema/types/app_test.go b/schema/types/app_test.go
index 14bdbed..91f1cba 100644
--- a/schema/types/app_test.go
+++ b/schema/types/app_test.go
@@ -212,7 +212,7 @@ func TestAppUnmarshal(t *testing.T) {
 			false,
 		},
 		{
-			`{"Exec":["/a"],"User":"0","Group":"0","CRIAnnotations":{"weird!":"normal?"},"CRILabels":{"one":"two"}}`,
+			`{"Exec":["/a"],"User":"0","Group":"0","CRIAnnotations":{"weird!":"normal?"},"CRILabels":{"one!":"two?"}}`,
 			&App{
 				Exec: Exec{
 					"/a",
@@ -224,16 +224,11 @@ func TestAppUnmarshal(t *testing.T) {
 					"weird!": "normal?",
 				},
 				CRILabels: CRILabels{
-					"one": "two",
+					"one!": "two?",
 				},
 			},
 			false,
 		},
-		{
-			`{"Exec":["/a"],"User":"0","Group":"0","CRIAnnotations":{"weird!":"normal?"},"CRILabels":{"!one":"two"}}`,
-			&App{},
-			true,
-		},
 	}
 	for i, tt := range tests {
 		a := &App{}
diff --git a/schema/types/cri_labels_test.go b/schema/types/cri_labels_test.go
deleted file mode 100644
index fdaf789..0000000
--- a/schema/types/cri_labels_test.go
+++ /dev/null
@@ -1,52 +0,0 @@
-// Copyright 2015 The appc Authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package types
-
-import "testing"
-
-func TestCRILabelsAssertValid(t *testing.T) {
-	tests := []struct {
-		in   CRILabels
-		werr bool
-	}{
-		// empty is OK
-		{
-			CRILabels{},
-			false,
-		},
-		{
-			CRILabels{"a": "b"},
-			false,
-		},
-		{
-			CRILabels{"a!": "b"},
-			true,
-		},
-		{
-			CRILabels{"/a": "b"},
-			true,
-		},
-		{
-			CRILabels{"a/a": "b"},
-			false,
-		},
-	}
-	for i, tt := range tests {
-		err := tt.in.assertValid()
-		if gerr := (err != nil); gerr != tt.werr {
-			t.Errorf("#%d: gerr=%t, want %t (err=%v)", i, gerr, tt.werr, err)
-		}
-	}
-}