Default to strict decoding of OCI runtime spec

This change defaults to strict decoding of the OCI
runtime spec to avoid dropping unknown fields when making
OCI runtime modifications.

We also add an allow-unknown-oci-spec-fields feature flag
that can be used to opt-out of this behaviour.

Signed-off-by: Evan Lezar <[email protected]>

Author: elezar

internal/config/features.go

@@ -25,6 +25,11 @@ type features struct {
 	// If this feature flag is not set to 'true' only host-rooted config paths
 	// (i.e. paths starting with an '@' are considered valid)
 	AllowLDConfigFromContainer *feature `toml:"allow-ldconfig-from-container,omitempty"`
+	// AllowUnknownOCISpecFields allows the nvidia-container-runtime to ignore
+	// unknown fields when loading the config (OCI spec) associated with a
+	// container.
+	// If this is enabled, these fields are silently dropped.
+	AllowUnknownOCISpecFields *feature `toml:"allow-unknown-oci-spec-fields,omitempty"`
 	// DisableCUDACompatLibHook, when enabled skips the injection of a specific
 	// hook to process CUDA compatibility libraries.
 	//

internal/oci/spec.go

@@ -20,8 +20,6 @@ import (
 	"fmt"
 
 	"github.com/opencontainers/runtime-spec/specs-go"
-
-	"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
 )
 
 // SpecModifier defines an interface for modifying a (raw) OCI spec
@@ -49,17 +47,24 @@ type Spec interface {
 
 // NewSpec creates fileSpec based on the command line arguments passed to the
 // application using the specified logger.
-func NewSpec(logger logger.Interface, args []string) (Spec, error) {
+func NewSpec(args []string, opts ...Option) (Spec, error) {
+	o := &options{
+		allowUnkownFields: false,
+	}
+	for _, opt := range opts {
+		opt(o)
+	}
+
 	bundleDir, err := GetBundleDir(args)
 	if err != nil {
 		return nil, fmt.Errorf("error getting bundle directory: %v", err)
 	}
-	logger.Debugf("Using bundle directory: %v", bundleDir)
+	o.logger.Debugf("Using bundle directory: %v", bundleDir)
 
 	ociSpecPath := GetSpecFilePath(bundleDir)
-	logger.Infof("Using OCI specification file path: %v", ociSpecPath)
+	o.logger.Infof("Using OCI specification file path: %v", ociSpecPath)
 
-	ociSpec := NewFileSpec(ociSpecPath)
+	ociSpec := NewFileSpec(ociSpecPath, !o.allowUnkownFields)
 
 	return ociSpec, nil
 }

internal/oci/spec_file.go

@@ -28,16 +28,28 @@ import (
 type fileSpec struct {
 	memorySpec
 	path string
+	loader
 }
 
+type loader bool
+
+const (
+	strictLoader = loader(true)
+)
+
 var _ Spec = (*fileSpec)(nil)
 
 // NewFileSpec creates an object that encapsulates a file-backed OCI spec.
 // This can be used to read from the file, modify the spec, and write to the
 // same file.
-func NewFileSpec(filepath string) Spec {
+func NewFileSpec(filepath string, isStrict bool) Spec {
+	var loader loader
+	if isStrict {
+		loader = strictLoader
+	}
 	oci := fileSpec{
-		path: filepath,
+		path:   filepath,
+		loader: loader,
 	}
 
 	return &oci
@@ -52,7 +64,7 @@ func (s *fileSpec) Load() (*specs.Spec, error) {
 	}
 	defer specFile.Close()
 
-	spec, err := LoadFrom(specFile)
+	spec, err := s.loadFrom(specFile)
 	if err != nil {
 		return nil, fmt.Errorf("error loading OCI specification from file: %v", err)
 	}
@@ -61,8 +73,11 @@ func (s *fileSpec) Load() (*specs.Spec, error) {
 }
 
 // LoadFrom reads the contents of the OCI spec from the specified io.Reader.
-func LoadFrom(reader io.Reader) (*specs.Spec, error) {
+func (isStrict loader) loadFrom(reader io.Reader) (*specs.Spec, error) {
 	decoder := json.NewDecoder(reader)
+	if isStrict {
+		decoder.DisallowUnknownFields()
+	}
 
 	var spec specs.Spec
 

internal/oci/spec_file_test.go

@@ -44,7 +44,7 @@ func TestLoadFrom(t *testing.T) {
 
 	for i, tc := range testCases {
 		var spec *specs.Spec
-		spec, err := LoadFrom(bytes.NewReader(tc.contents))
+		spec, err := strictLoader.loadFrom(bytes.NewReader(tc.contents))
 
 		if tc.isError {
 			require.Error(t, err, "%d: %v", i, tc)

internal/oci/spec_test.go

@@ -21,7 +21,7 @@ func TestMaintainSpec(t *testing.T) {
 	for _, f := range files {
 		inputSpecPath := filepath.Join(moduleRoot, "tests/input", f)
 
-		spec := NewFileSpec(inputSpecPath).(*fileSpec)
+		spec := NewFileSpec(inputSpecPath, true).(*fileSpec)
 
 		_, err := spec.Load()
 		require.NoError(t, err)

internal/runtime/runtime_factory.go

@@ -42,7 +42,10 @@ func newNVIDIAContainerRuntime(logger logger.Interface, cfg *config.Config, argv
 		return lowLevelRuntime, nil
 	}
 
-	ociSpec, err := oci.NewSpec(logger, argv)
+	ociSpec, err := oci.NewSpec(argv,
+		oci.WithLogger(logger),
+		oci.WithAllowUnknownFields(cfg.Features.AllowUnknownOCISpecFields.IsEnabled()),
+	)
 	if err != nil {
 		return nil, fmt.Errorf("error constructing OCI specification: %v", err)
 	}