Merge pull request #495 from q384566678/fix-hostspecific

 Add validation when host-specific is set

Author: liangchenye

cmd/oci-runtime-tool/validate.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"fmt"
+	"runtime"
 
 	rfc2119 "github.com/opencontainers/runtime-tools/error"
 	"github.com/opencontainers/runtime-tools/specerror"
@@ -12,7 +13,7 @@ import (
 
 var bundleValidateFlags = []cli.Flag{
 	cli.StringFlag{Name: "path", Value: ".", Usage: "path to a bundle"},
-	cli.StringFlag{Name: "platform", Value: "linux", Usage: "platform of the target bundle (linux, windows, solaris)"},
+	cli.StringFlag{Name: "platform", Value: runtime.GOOS, Usage: "platform of the target bundle (linux, windows, solaris)"},
 }
 
 var bundleValidateCommand = cli.Command{

man/oci-runtime-tool-validate.1.md

@@ -19,7 +19,7 @@ Validate an OCI bundle
   Path to bundle. The default is current working directory.
 
 **--platform**=PLATFORM
-  Platform of the target bundle. (linux, windows, solaris) (default: "linux")
+  Platform of the target bundle. (linux, windows, solaris) The default is host platform.
   It will be overwritten by the host platform if the global option '--host-specific' was set.
 
 # SEE ALSO

validate/validate.go

@@ -66,23 +66,20 @@ type Validator struct {
 }
 
 // NewValidator creates a Validator
-func NewValidator(spec *rspec.Spec, bundlePath string, hostSpecific bool, platform string) Validator {
+func NewValidator(spec *rspec.Spec, bundlePath string, hostSpecific bool, platform string) (Validator, error) {
 	if hostSpecific && platform != runtime.GOOS {
-		platform = runtime.GOOS
+		return Validator{}, fmt.Errorf("When hostSpecific is set, platform must be same as the host platform")
 	}
 	return Validator{
 		spec:         spec,
 		bundlePath:   bundlePath,
 		HostSpecific: hostSpecific,
 		platform:     platform,
-	}
+	}, nil
 }
 
 // NewValidatorFromPath creates a Validator with specified bundle path
 func NewValidatorFromPath(bundlePath string, hostSpecific bool, platform string) (Validator, error) {
-	if hostSpecific && platform != runtime.GOOS {
-		platform = runtime.GOOS
-	}
 	if bundlePath == "" {
 		return Validator{}, fmt.Errorf("bundle path shouldn't be empty")
 	}
@@ -104,7 +101,7 @@ func NewValidatorFromPath(bundlePath string, hostSpecific bool, platform string)
 		return Validator{}, err
 	}
 
-	return NewValidator(&spec, bundlePath, hostSpecific, platform), nil
+	return NewValidator(&spec, bundlePath, hostSpecific, platform)
 }
 
 // CheckAll checks all parts of runtime bundle

validate/validate_test.go

@@ -23,13 +23,16 @@ func TestNewValidator(t *testing.T) {
 		val      Validator
 		expected Validator
 	}{
-		{Validator{testSpec, testBundle, true, testPlatform}, Validator{testSpec, testBundle, true, runtime.GOOS}},
 		{Validator{testSpec, testBundle, true, runtime.GOOS}, Validator{testSpec, testBundle, true, runtime.GOOS}},
 		{Validator{testSpec, testBundle, false, testPlatform}, Validator{testSpec, testBundle, false, testPlatform}},
 	}
 
 	for _, c := range cases {
-		assert.Equal(t, c.expected, NewValidator(c.val.spec, c.val.bundlePath, c.val.HostSpecific, c.val.platform))
+		v, err := NewValidator(c.val.spec, c.val.bundlePath, c.val.HostSpecific, c.val.platform)
+		if err != nil {
+			t.Errorf("unexpected NewValidator error: %+v", err)
+		}
+		assert.Equal(t, c.expected, v)
 	}
 }
 
@@ -165,8 +168,11 @@ func TestCheckRoot(t *testing.T) {
 		{rspec.Spec{Root: &rspec.Root{Readonly: true}}, "windows", specerror.RootReadonlyOnWindowsFalse},
 	}
 	for _, c := range cases {
-		v := NewValidator(&c.val, tmpBundle, false, c.platform)
-		err := v.CheckRoot()
+		v, err := NewValidator(&c.val, tmpBundle, false, c.platform)
+		if err != nil {
+			t.Errorf("unexpected NewValidator error: %+v", err)
+		}
+		err = v.CheckRoot()
 		assert.Equal(t, c.expected, specerror.FindError(err, c.expected), fmt.Sprintf("Fail to check Root: %v %d", err, c.expected))
 	}
 }
@@ -183,8 +189,11 @@ func TestCheckSemVer(t *testing.T) {
 	}
 
 	for _, c := range cases {
-		v := NewValidator(&rspec.Spec{Version: c.val}, "", false, "linux")
-		err := v.CheckSemVer()
+		v, err := NewValidator(&rspec.Spec{Version: c.val}, "", false, "linux")
+		if err != nil {
+			t.Errorf("unexpected NewValidator error: %+v", err)
+		}
+		err = v.CheckSemVer()
 		assert.Equal(t, c.expected, specerror.FindError(err, c.expected), "Fail to check SemVer "+c.val)
 	}
 }
@@ -328,8 +337,11 @@ func TestCheckProcess(t *testing.T) {
 		},
 	}
 	for _, c := range cases {
-		v := NewValidator(&c.val, ".", false, c.platform)
-		err := v.CheckProcess()
+		v, err := NewValidator(&c.val, ".", false, c.platform)
+		if err != nil {
+			t.Errorf("unexpected NewValidator error: %+v", err)
+		}
+		err = v.CheckProcess()
 		assert.Equal(t, c.expected, specerror.FindError(err, c.expected), fmt.Sprintf("failed CheckProcess: %v %d", err, c.expected))
 	}
 }
@@ -409,8 +421,11 @@ func TestCheckLinux(t *testing.T) {
 		},
 	}
 	for _, c := range cases {
-		v := NewValidator(&c.val, ".", false, "linux")
-		err := v.CheckLinux()
+		v, err := NewValidator(&c.val, ".", false, "linux")
+		if err != nil {
+			t.Errorf("unexpected NewValidator error: %+v", err)
+		}
+		err = v.CheckLinux()
 		assert.Equal(t, c.expected, specerror.FindError(err, c.expected), fmt.Sprintf("failed CheckLinux: %v %d", err, c.expected))
 	}
 }
@@ -447,8 +462,11 @@ func TestCheckPlatform(t *testing.T) {
 		},
 	}
 	for _, c := range cases {
-		v := NewValidator(&c.val, ".", false, c.platform)
-		err := v.CheckPlatform()
+		v, err := NewValidator(&c.val, ".", false, c.platform)
+		if err != nil {
+			t.Errorf("unexpected NewValidator error: %+v", err)
+		}
+		err = v.CheckPlatform()
 		assert.Equal(t, c.expected, specerror.FindError(err, c.expected), fmt.Sprintf("failed CheckPlatform: %v %d", err, c.expected))
 	}
 }
@@ -486,8 +504,11 @@ func TestCheckHooks(t *testing.T) {
 		},
 	}
 	for _, c := range cases {
-		v := NewValidator(&c.val, ".", false, "linux")
-		err := v.CheckHooks()
+		v, err := NewValidator(&c.val, ".", false, "linux")
+		if err != nil {
+			t.Errorf("unexpected NewValidator error: %+v", err)
+		}
+		err = v.CheckHooks()
 		assert.Equal(t, c.expected, specerror.FindError(err, c.expected), fmt.Sprintf("failed CheckHooks: %v %d", err, c.expected))
 	}
 }