Merge pull request #586 from liangchenye/annotation

add annotation and prop tests

Author: Zhou Hao

cmd/oci-runtime-tool/validate.go

@@ -44,7 +44,8 @@ var bundleValidateCommand = cli.Command{
 			for _, e := range levelErrors.Warnings {
 				logrus.Warn(e)
 			}
-			return levelErrors.Error
+
+			return levelErrors.Error.ErrorOrNil()
 		}
 		fmt.Println("Bundle validation succeeded.")
 		return nil

validate/validate.go

@@ -114,6 +114,7 @@ func (v *Validator) CheckAll() error {
 	errs = multierror.Append(errs, v.CheckMounts())
 	errs = multierror.Append(errs, v.CheckProcess())
 	errs = multierror.Append(errs, v.CheckLinux())
+	errs = multierror.Append(errs, v.CheckAnnotations())
 	if v.platform == "linux" || v.platform == "solaris" {
 		errs = multierror.Append(errs, v.CheckHooks())
 	}
@@ -655,6 +656,32 @@ func (v *Validator) CheckLinuxResources() (errs error) {
 	return
 }
 
+// CheckAnnotations checks v.spec.Annotations
+func (v *Validator) CheckAnnotations() (errs error) {
+	logrus.Debugf("check annotations")
+
+	reversedDomain := regexp.MustCompile(`^[A-Za-z]{2,6}(\.[A-Za-z0-9-]{1,63})+$`)
+	for key := range v.spec.Annotations {
+		if strings.HasPrefix(key, "org.opencontainers") {
+			errs = multierror.Append(errs,
+				specerror.NewError(
+					specerror.AnnotationsKeyReservedNS,
+					fmt.Errorf("key %q is reserved", key),
+					rspec.Version))
+		}
+
+		if !reversedDomain.MatchString(key) {
+			errs = multierror.Append(errs,
+				specerror.NewError(
+					specerror.AnnotationsKeyReversedDomain,
+					fmt.Errorf("key %q SHOULD be named using a reverse domain notation", key),
+					rspec.Version))
+		}
+	}
+
+	return
+}
+
 // CapValid checks whether a capability is valid
 func CapValid(c string, hostSpecific bool) error {
 	isValid := false

validate/validate_test.go

@@ -759,3 +759,47 @@ func TestCheckMandatoryFields(t *testing.T) {
 		})
 	}
 }
+
+func TestCheckAnnotations(t *testing.T) {
+	cases := []struct {
+		val      rspec.Spec
+		expected specerror.Code
+	}{
+		{
+			val:      rspec.Spec{},
+			expected: specerror.NonError,
+		},
+		{
+			val: rspec.Spec{
+				Annotations: map[string]string{},
+			},
+			expected: specerror.NonError,
+		},
+		{
+			val: rspec.Spec{
+				Annotations: map[string]string{"invalid": ""},
+			},
+			expected: specerror.AnnotationsKeyReversedDomain,
+		},
+		{
+			val: rspec.Spec{
+				Annotations: map[string]string{"org.opencontainers.oci": ""},
+			},
+			expected: specerror.AnnotationsKeyReservedNS,
+		},
+		{
+			val: rspec.Spec{
+				Annotations: map[string]string{"com.example": ""},
+			},
+			expected: specerror.NonError,
+		},
+	}
+	for _, c := range cases {
+		v, err := NewValidator(&c.val, ".", false, "")
+		if err != nil {
+			t.Errorf("unexpected NewValidator error: %+v", err)
+		}
+		err = v.CheckAnnotations()
+		assert.Equal(t, c.expected, specerror.FindError(err, c.expected), fmt.Sprintf("failed CheckAnnotations: %v %d", err, c.expected))
+	}
+}

validation/misc_props.go

@@ -0,0 +1,79 @@
+package main
+
+import (
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"time"
+
+	"github.com/mndrix/tap-go"
+	rspecs "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/opencontainers/runtime-tools/specerror"
+	"github.com/opencontainers/runtime-tools/validation/util"
+	uuid "github.com/satori/go.uuid"
+)
+
+func saveConfig(path string, v interface{}) error {
+	data, err := json.Marshal(v)
+	if err != nil {
+		return err
+	}
+
+	return ioutil.WriteFile(path, data, 0644)
+}
+
+func main() {
+	t := tap.New()
+	t.Header(0)
+	bundleDir, err := util.PrepareBundle()
+	if err != nil {
+		util.Fatal(err)
+	}
+	defer os.RemoveAll(bundleDir)
+	configFile := filepath.Join(bundleDir, "config.json")
+
+	type extendedSpec struct {
+		rspecs.Spec
+		Unknown string `json:"unknown,omitempty"`
+	}
+
+	containerID := uuid.NewV4().String()
+	basicConfig := util.GetDefaultGenerator()
+	basicConfig.SetProcessArgs([]string{"true"})
+	annotationConfig := basicConfig
+	annotationConfig.AddAnnotation(fmt.Sprintf("org.%s", containerID), "")
+	invalidConfig := basicConfig
+	invalidConfig.SetVersion("invalid")
+
+	cases := []struct {
+		eSpec       extendedSpec
+		action      util.LifecycleAction
+		errExpected bool
+		err         error
+	}{
+		{extendedSpec{Spec: *annotationConfig.Spec()}, util.LifecycleActionCreate | util.LifecycleActionStart | util.LifecycleActionDelete, true, specerror.NewError(specerror.AnnotationsKeyIgnoreUnknown, fmt.Errorf("implementations that are reading/processing this configuration file MUST NOT generate an error if they encounter an unknown annotation key"), rspecs.Version)},
+		{extendedSpec{Spec: *basicConfig.Spec(), Unknown: "unknown"}, util.LifecycleActionCreate | util.LifecycleActionStart | util.LifecycleActionDelete, true, specerror.NewError(specerror.ExtensibilityIgnoreUnknownProp, fmt.Errorf("runtimes that are reading or processing this configuration file MUST NOT generate an error if they encounter an unknown property"), rspecs.Version)},
+		{extendedSpec{Spec: *invalidConfig.Spec()}, util.LifecycleActionCreate | util.LifecycleActionStart | util.LifecycleActionDelete, false, specerror.NewError(specerror.ValidValues, fmt.Errorf("runtimes that are reading or processing this configuration file MUST generate an error when invalid or unsupported values are encountered"), rspecs.Version)},
+	}
+
+	for _, c := range cases {
+		config := util.LifecycleConfig{
+			BundleDir: bundleDir,
+			Actions:   c.action,
+			PreCreate: func(r *util.Runtime) error {
+				r.SetID(containerID)
+				return saveConfig(configFile, c.eSpec)
+			},
+			PreDelete: func(r *util.Runtime) error {
+				util.WaitingForStatus(*r, util.LifecycleStatusCreated|util.LifecycleStatusStopped, time.Second*10, time.Second*1)
+				return nil
+			},
+		}
+		err := util.RuntimeLifecycleValidate(config)
+		util.SpecErrorOK(t, (err == nil) == c.errExpected, c.err, err)
+	}
+
+	t.AutoPlan()
+}