Skip to content

Commit 0e23203

Browse files
slonopotamusthaJeztah
slonopotamus
authored and
thaJeztah
committed
Uncopypaste parsing of OCI Bundle spec file
Signed-off-by: Marat Radchenko <[email protected]> (cherry picked from commit 9e34b8b) Signed-off-by: Sebastiaan van Stijn <[email protected]>
1 parent 8c08766 commit 0e23203

6 files changed

Lines changed: 48 additions & 34 deletions

File tree

‎cmd/containerd/command/oci-hook.go‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ import (
2525
"syscall"
2626
"text/template"
2727

28-
specs "github.com/opencontainers/runtime-spec/specs-go"
28+
"github.com/containerd/containerd/oci"
29+
"github.com/opencontainers/runtime-spec/specs-go"
2930
"github.com/urfave/cli"
3031
)
3132

@@ -37,7 +38,8 @@ var ociHook = cli.Command{
3738
if err != nil {
3839
return err
3940
}
40-
spec, err := loadSpec(state.Bundle)
41+
specFile := filepath.Join(state.Bundle, oci.ConfigFilename)
42+
spec, err := loadSpec(specFile)
4143
if err != nil {
4244
return err
4345
}
@@ -56,14 +58,16 @@ var ociHook = cli.Command{
5658
},
5759
}
5860

61+
// hookSpec is a shallow version of [oci.Spec] containing only the
62+
// fields we need for the hook. We use a shallow struct to reduce
63+
// the overhead of unmarshaling.
5964
type hookSpec struct {
60-
Root struct {
61-
Path string `json:"path"`
62-
} `json:"root"`
65+
// Root configures the container's root filesystem.
66+
Root *specs.Root `json:"root,omitempty"`
6367
}
6468

65-
func loadSpec(bundle string) (*hookSpec, error) {
66-
f, err := os.Open(filepath.Join(bundle, "config.json"))
69+
func loadSpec(path string) (*hookSpec, error) {
70+
f, err := os.Open(path)
6771
if err != nil {
6872
return nil, err
6973
}

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

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package main
1818

1919
import (
2020
"context"
21-
"encoding/json"
2221
"fmt"
2322
"os"
2423
"path/filepath"
@@ -35,8 +34,6 @@ import (
3534
)
3635

3736
const (
38-
ociConfigFilename = "config.json"
39-
4037
failpointPrefixKey = "io.containerd.runtime.v2.shim.failpoint."
4138
)
4239

@@ -113,15 +110,10 @@ func newFailpointFromOCIAnnotation() (map[string]*failpoint.Failpoint, error) {
113110
return nil, fmt.Errorf("failed to get current working dir: %w", err)
114111
}
115112

116-
configPath := filepath.Join(cwd, ociConfigFilename)
117-
data, err := os.ReadFile(configPath)
113+
configPath := filepath.Join(cwd, oci.ConfigFilename)
114+
spec, err := oci.ReadSpec(configPath)
118115
if err != nil {
119-
return nil, fmt.Errorf("failed to read %v: %w", configPath, err)
120-
}
121-
122-
var spec oci.Spec
123-
if err := json.Unmarshal(data, &spec); err != nil {
124-
return nil, fmt.Errorf("failed to parse oci.Spec(%v): %w", string(data), err)
116+
return nil, err
125117
}
126118

127119
res := make(map[string]*failpoint.Failpoint)

‎oci/spec.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ package oci
1818

1919
import (
2020
"context"
21+
"encoding/json"
22+
"os"
2123
"path/filepath"
2224
"runtime"
2325

@@ -43,6 +45,22 @@ var (
4345
// to be created without the "issues" with go vendoring and package imports
4446
type Spec = specs.Spec
4547

48+
const ConfigFilename = "config.json"
49+
50+
// ReadSpec deserializes JSON into an OCI runtime Spec from a given path.
51+
func ReadSpec(path string) (*Spec, error) {
52+
f, err := os.Open(path)
53+
if err != nil {
54+
return nil, err
55+
}
56+
defer f.Close()
57+
var s Spec
58+
if err := json.NewDecoder(f).Decode(&s); err != nil {
59+
return nil, err
60+
}
61+
return &s, nil
62+
}
63+
4664
// GenerateSpec will generate a default spec from the provided image
4765
// for use as a containerd container
4866
func GenerateSpec(ctx context.Context, client Client, c *containers.Container, opts ...SpecOpts) (*Spec, error) {

‎runtime/v2/bundle.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,11 @@ import (
2626
"github.com/containerd/containerd/identifiers"
2727
"github.com/containerd/containerd/mount"
2828
"github.com/containerd/containerd/namespaces"
29+
"github.com/containerd/containerd/oci"
2930
"github.com/containerd/typeurl/v2"
3031
"github.com/opencontainers/runtime-spec/specs-go"
3132
)
3233

33-
const configFilename = "config.json"
34-
3534
// LoadBundle loads an existing bundle from disk
3635
func LoadBundle(ctx context.Context, root, id string) (*Bundle, error) {
3736
ns, err := namespaces.NamespaceRequired(ctx)
@@ -107,9 +106,10 @@ func NewBundle(ctx context.Context, root, state, id string, spec typeurl.Any) (b
107106
}
108107
if spec := spec.GetValue(); spec != nil {
109108
// write the spec to the bundle
110-
err = os.WriteFile(filepath.Join(b.Path, configFilename), spec, 0666)
109+
specPath := filepath.Join(b.Path, oci.ConfigFilename)
110+
err = os.WriteFile(specPath, spec, 0666)
111111
if err != nil {
112-
return nil, fmt.Errorf("failed to write %s", configFilename)
112+
return nil, fmt.Errorf("failed to write bundle spec: %w", err)
113113
}
114114
}
115115
return b, nil

‎runtime/v2/runc/manager/manager_linux.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
"github.com/containerd/containerd/log"
3333
"github.com/containerd/containerd/mount"
3434
"github.com/containerd/containerd/namespaces"
35+
"github.com/containerd/containerd/oci"
3536
"github.com/containerd/containerd/pkg/process"
3637
"github.com/containerd/containerd/pkg/schedcore"
3738
"github.com/containerd/containerd/runtime/v2/runc"
@@ -58,7 +59,11 @@ var groupLabels = []string{
5859
"io.kubernetes.cri.sandbox-id",
5960
}
6061

62+
// spec is a shallow version of [oci.Spec] containing only the
63+
// fields we need for the hook. We use a shallow struct to reduce
64+
// the overhead of unmarshaling.
6165
type spec struct {
66+
// Annotations contains arbitrary metadata for the container.
6267
Annotations map[string]string `json:"annotations,omitempty"`
6368
}
6469

@@ -97,7 +102,7 @@ func newCommand(ctx context.Context, id, containerdAddress, containerdTTRPCAddre
97102
}
98103

99104
func readSpec() (*spec, error) {
100-
f, err := os.Open("config.json")
105+
f, err := os.Open(oci.ConfigFilename)
101106
if err != nil {
102107
return nil, err
103108
}

‎runtime/v2/runc/util.go‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,13 @@ package runc
2020

2121
import (
2222
"context"
23-
"encoding/json"
24-
"os"
2523
"path/filepath"
2624

2725
"github.com/containerd/containerd/api/events"
2826
"github.com/containerd/containerd/log"
27+
"github.com/containerd/containerd/oci"
2928
"github.com/containerd/containerd/runtime"
30-
specs "github.com/opencontainers/runtime-spec/specs-go"
29+
"github.com/opencontainers/runtime-spec/specs-go"
3130
"github.com/sirupsen/logrus"
3231
)
3332

@@ -64,18 +63,14 @@ func GetTopic(e interface{}) string {
6463
// ShouldKillAllOnExit reads the bundle's OCI spec and returns true if
6564
// there is an error reading the spec or if the container has a private PID namespace
6665
func ShouldKillAllOnExit(ctx context.Context, bundlePath string) bool {
67-
var bundleSpec specs.Spec
68-
bundleConfigContents, err := os.ReadFile(filepath.Join(bundlePath, "config.json"))
66+
spec, err := oci.ReadSpec(filepath.Join(bundlePath, oci.ConfigFilename))
6967
if err != nil {
7068
log.G(ctx).WithError(err).Error("shouldKillAllOnExit: failed to read config.json")
7169
return true
7270
}
73-
if err := json.Unmarshal(bundleConfigContents, &bundleSpec); err != nil {
74-
log.G(ctx).WithError(err).Error("shouldKillAllOnExit: failed to unmarshal bundle json")
75-
return true
76-
}
77-
if bundleSpec.Linux != nil {
78-
for _, ns := range bundleSpec.Linux.Namespaces {
71+
72+
if spec.Linux != nil {
73+
for _, ns := range spec.Linux.Namespaces {
7974
if ns.Type == specs.PIDNamespace && ns.Path == "" {
8075
return false
8176
}

0 commit comments

Comments
 (0)