address feedback

Signed-off-by: Tim Allclair <[email protected]>

Author: tallclair

cri.go

@@ -108,7 +108,7 @@ func initCRIService(ic *plugin.InitContext) (interface{}, error) {
 	return s, nil
 }
 
-// validateConfig validates that the given configuration.
+// validateConfig validates the given configuration.
 func validateConfig(c *criconfig.Config) error {
 	// It is an error to provide both an UntrustedWorkloadRuntime & define an 'untrusted' runtime.
 	if _, ok := c.ContainerdConfig.Runtimes[criconfig.RuntimeUntrusted]; ok {

docs/config.md

@@ -61,6 +61,11 @@ The explanation and default value of each configuration item are as follows:
       runtime_root = ""
 
     # "plugins.cri.containerd.untrusted_workload_runtime" is a runtime to run untrusted workloads on it.
+
+    # DEPRECATED: use plugins.cri.runtimes instead. If provided, this runtime is mapped to the
+    #     runtime handler named 'untrusted'. It is a configuration error to provide both the (now
+    #     deprecated) UntrustedWorkloadRuntime and a handler in the Runtimes handler map (below) for
+    #     'untrusted' workloads at the same time. Please provide one or the other.
     [plugins.cri.containerd.untrusted_workload_runtime]
       # runtime_type is the runtime type to use in containerd e.g. io.containerd.runtime.v1.linux
       runtime_type = ""
@@ -71,6 +76,19 @@ The explanation and default value of each configuration item are as follows:
       # runtime_root is the directory used by containerd for runtime state.
       runtime_root = ""
 
+  	# plugins.cri.containerd.runtimes is a map from CRI RuntimeHandler strings, which specify types
+  	# of runtime configurations, to the matching configurations. In this example,
+  	# 'runtime_handler_name' is the RuntimeHandler string to match.
+    [plugins.cri.containerd.runtimes.runtime_handler_name]
+      # runtime_type is the runtime type to use in containerd e.g. io.containerd.runtime.v1.linux
+      runtime_type = ""
+
+      # runtime_engine is the name of the runtime engine used by containerd.
+      runtime_engine = ""
+
+      # runtime_root is the directory used by containerd for runtime state.
+      runtime_root = ""
+
   # "plugins.cri.cni" contains config related to cni
   [plugins.cri.cni]
     # bin_dir is the directory in which the binaries for the plugin is kept.

pkg/config/config.go

@@ -38,9 +38,12 @@ type ContainerdConfig struct {
 	DefaultRuntime Runtime `toml:"default_runtime" json:"defaultRuntime"`
 	// UntrustedWorkloadRuntime is a runtime to run untrusted workloads on it.
 	// DEPRECATED: use Runtimes instead. If provided, this runtime is mapped to the runtime handler
-	//     named 'untrusted'. It is a configuration error to provide both.
+	//     named 'untrusted'. It is a configuration error to provide both the (now deprecated)
+	//     UntrustedWorkloadRuntime and a handler in the Runtimes handler map (below) for 'untrusted'
+	//     workloads at the same time. Please provide one or the other.
 	UntrustedWorkloadRuntime Runtime `toml:"untrusted_workload_runtime" json:"untrustedWorkloadRuntime"`
-	// Runtimes maps a CRI RuntimeHandler string to a runtime configuration.
+	// Runtimes is a map from CRI RuntimeHandler strings, which specify types of runtime
+	// configurations, to the matching configurations.
 	Runtimes map[string]Runtime `toml:"runtimes" json:"runtimes"`
 	// NoPivot disables pivot-root (linux only), required when running a container in a RamDisk with runc
 	NoPivot bool `toml:"no_pivot" json:"noPivot"`
@@ -191,5 +194,5 @@ func DefaultConfig() PluginConfig {
 
 const (
 	// RuntimeUntrusted is the implicit runtime defined for ContainerdConfig.UntrustedWorkloadRuntime
-	RuntimeUntrusted = "unstrusted"
+	RuntimeUntrusted = "untrusted"
 )

pkg/server/sandbox_run.go

@@ -604,7 +604,7 @@ func hostAccessingSandbox(config *runtime.PodSandboxConfig) bool {
 func (c *criService) getSandboxRuntime(config *runtime.PodSandboxConfig, runtimeHandler string) (criconfig.Runtime, error) {
 	if untrustedWorkload(config) {
 		// If the untrusted annotation is provided, runtimeHandler MUST be empty.
-		if runtimeHandler != "" {
+		if runtimeHandler != "" && runtimeHandler != criconfig.RuntimeUntrusted {
 			return criconfig.Runtime{}, errors.New("untrusted workload with explicit runtime handler is not allowed")
 		}
 

pkg/server/sandbox_run_test.go

@@ -613,6 +613,29 @@ func TestGetSandboxRuntime(t *testing.T) {
 			runtimes:        map[string]criconfig.Runtime{criconfig.RuntimeUntrusted: untrustedWorkloadRuntime},
 			expectedRuntime: untrustedWorkloadRuntime,
 		},
+		"should use 'untrusted' runtime for untrusted workload & handler": {
+			sandboxConfig: &runtime.PodSandboxConfig{
+				Annotations: map[string]string{
+					annotations.UntrustedWorkload: "true",
+				},
+			},
+			runtimeHandler:  "untrusted",
+			defaultRuntime:  defaultRuntime,
+			runtimes:        map[string]criconfig.Runtime{criconfig.RuntimeUntrusted: untrustedWorkloadRuntime},
+			expectedRuntime: untrustedWorkloadRuntime,
+		},
+		"should return an error if untrusted annotation with conflicting handler": {
+			sandboxConfig: &runtime.PodSandboxConfig{
+				Annotations: map[string]string{
+					annotations.UntrustedWorkload: "true",
+				},
+			},
+			runtimeHandler:           "foo",
+			defaultRuntime:           defaultRuntime,
+			untrustedWorkloadRuntime: untrustedWorkloadRuntime,
+			runtimes:                 map[string]criconfig.Runtime{"foo": fooRuntime},
+			expectErr:                true,
+		},
 		"should use correct runtime for a runtime handler": {
 			sandboxConfig:            &runtime.PodSandboxConfig{},
 			runtimeHandler:           "foo",