Merge pull request #8780 from slonopotamus/uncopypaste-read-spec

Uncopypaste parsing of OCI Bundle spec file

cmd/containerd/command/oci-hook.go

@@ -25,7 +25,8 @@ import (
 	"syscall"
 	"text/template"
 
-	specs "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/containerd/containerd/oci"
+	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/urfave/cli"
 )
 
@@ -37,7 +38,8 @@ var ociHook = cli.Command{
 		if err != nil {
 			return err
 		}
-		spec, err := loadSpec(state.Bundle)
+		specFile := filepath.Join(state.Bundle, oci.ConfigFilename)
+		spec, err := loadSpec(specFile)
 		if err != nil {
 			return err
 		}
@@ -56,14 +58,16 @@ var ociHook = cli.Command{
 	},
 }
 
+// hookSpec is a shallow version of [oci.Spec] containing only the
+// fields we need for the hook. We use a shallow struct to reduce
+// the overhead of unmarshaling.
 type hookSpec struct {
-	Root struct {
+	// Root configures the container's root filesystem.
-		Path string `json:"path"`
+	Root *specs.Root `json:"root,omitempty"`
-	} `json:"root"`
 }
 
-func loadSpec(bundle string) (*hookSpec, error) {
+func loadSpec(path string) (*hookSpec, error) {
-	f, err := os.Open(filepath.Join(bundle, "config.json"))
+	f, err := os.Open(path)
 	if err != nil {
 		return nil, err
 	}

integration/failpoint/cmd/containerd-shim-runc-fp-v1/plugin_linux.go

@@ -18,7 +18,6 @@ package main
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -35,8 +34,6 @@ import (
 )
 
 const (
-	ociConfigFilename = "config.json"
-
 	failpointPrefixKey = "io.containerd.runtime.v2.shim.failpoint."
 )
 
@@ -113,15 +110,10 @@ func newFailpointFromOCIAnnotation() (map[string]*failpoint.Failpoint, error) {
 		return nil, fmt.Errorf("failed to get current working dir: %w", err)
 	}
 
-	configPath := filepath.Join(cwd, ociConfigFilename)
+	configPath := filepath.Join(cwd, oci.ConfigFilename)
-	data, err := os.ReadFile(configPath)
+	spec, err := oci.ReadSpec(configPath)
 	if err != nil {
-		return nil, fmt.Errorf("failed to read %v: %w", configPath, err)
+		return nil, err
-	}
-
-	var spec oci.Spec
-	if err := json.Unmarshal(data, &spec); err != nil {
-		return nil, fmt.Errorf("failed to parse oci.Spec(%v): %w", string(data), err)
 	}
 
 	res := make(map[string]*failpoint.Failpoint)

oci/spec.go

@@ -18,6 +18,8 @@ package oci
 
 import (
 	"context"
+	"encoding/json"
+	"os"
 	"path/filepath"
 	"runtime"
 
@@ -46,6 +48,22 @@ var (
 // to be created without the "issues" with go vendoring and package imports
 type Spec = specs.Spec
 
+const ConfigFilename = "config.json"
+
+// ReadSpec deserializes JSON into an OCI runtime Spec from a given path.
+func ReadSpec(path string) (*Spec, error) {
+	f, err := os.Open(path)
+	if err != nil {
+		return nil, err
+	}
+	defer f.Close()
+	var s Spec
+	if err := json.NewDecoder(f).Decode(&s); err != nil {
+		return nil, err
+	}
+	return &s, nil
+}
+
 // GenerateSpec will generate a default spec from the provided image
 // for use as a containerd container
 func GenerateSpec(ctx context.Context, client Client, c *containers.Container, opts ...SpecOpts) (*Spec, error) {

runtime/v2/bundle.go

@@ -26,12 +26,11 @@ import (
 	"github.com/containerd/containerd/identifiers"
 	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/namespaces"
+	"github.com/containerd/containerd/oci"
 	"github.com/containerd/typeurl/v2"
 	"github.com/opencontainers/runtime-spec/specs-go"
 )
 
-const configFilename = "config.json"
-
 // LoadBundle loads an existing bundle from disk
 func LoadBundle(ctx context.Context, root, id string) (*Bundle, error) {
 	ns, err := namespaces.NamespaceRequired(ctx)
@@ -107,9 +106,10 @@ func NewBundle(ctx context.Context, root, state, id string, spec typeurl.Any) (b
 	}
 	if spec := spec.GetValue(); spec != nil {
 		// write the spec to the bundle
-		err = os.WriteFile(filepath.Join(b.Path, configFilename), spec, 0666)
+		specPath := filepath.Join(b.Path, oci.ConfigFilename)
+		err = os.WriteFile(specPath, spec, 0666)
 		if err != nil {
-			return nil, fmt.Errorf("failed to write %s", configFilename)
+			return nil, fmt.Errorf("failed to write bundle spec: %w", err)
 		}
 	}
 	return b, nil

runtime/v2/runc/manager/manager_linux.go

@@ -32,6 +32,7 @@ import (
 	"github.com/containerd/containerd/log"
 	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/namespaces"
+	"github.com/containerd/containerd/oci"
 	"github.com/containerd/containerd/pkg/process"
 	"github.com/containerd/containerd/pkg/schedcore"
 	"github.com/containerd/containerd/runtime/v2/runc"
@@ -58,7 +59,11 @@ var groupLabels = []string{
 	"io.kubernetes.cri.sandbox-id",
 }
 
+// spec is a shallow version of [oci.Spec] containing only the
+// fields we need for the hook. We use a shallow struct to reduce
+// the overhead of unmarshaling.
 type spec struct {
+	// Annotations contains arbitrary metadata for the container.
 	Annotations map[string]string `json:"annotations,omitempty"`
 }
 
@@ -97,7 +102,7 @@ func newCommand(ctx context.Context, id, containerdAddress, containerdTTRPCAddre
 }
 
 func readSpec() (*spec, error) {
-	f, err := os.Open("config.json")
+	f, err := os.Open(oci.ConfigFilename)
 	if err != nil {
 		return nil, err
 	}

runtime/v2/runc/util.go

@@ -20,29 +20,24 @@ package runc
 
 import (
 	"context"
-	"encoding/json"
-	"os"
 	"path/filepath"
 
 	"github.com/containerd/containerd/log"
-	specs "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/containerd/containerd/oci"
+	"github.com/opencontainers/runtime-spec/specs-go"
 )
 
 // ShouldKillAllOnExit reads the bundle's OCI spec and returns true if
 // there is an error reading the spec or if the container has a private PID namespace
 func ShouldKillAllOnExit(ctx context.Context, bundlePath string) bool {
-	var bundleSpec specs.Spec
+	spec, err := oci.ReadSpec(filepath.Join(bundlePath, oci.ConfigFilename))
-	bundleConfigContents, err := os.ReadFile(filepath.Join(bundlePath, "config.json"))
 	if err != nil {
 		log.G(ctx).WithError(err).Error("shouldKillAllOnExit: failed to read config.json")
 		return true
 	}
-	if err := json.Unmarshal(bundleConfigContents, &bundleSpec); err != nil {
+
-		log.G(ctx).WithError(err).Error("shouldKillAllOnExit: failed to unmarshal bundle json")
+	if spec.Linux != nil {
-		return true
+		for _, ns := range spec.Linux.Namespaces {
-	}
-	if bundleSpec.Linux != nil {
-		for _, ns := range bundleSpec.Linux.Namespaces {
 			if ns.Type == specs.PIDNamespace && ns.Path == "" {
 				return false
 			}