Skip to content
This repository has been archived by the owner on Mar 9, 2022. It is now read-only.

[WIP] adds option for setting default oci hooks #1248

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,12 @@ version = 2
# when using containerd with Kubernetes <=1.11.
disable_proc_mount = false

# default_oci_hooks is the filepath to an OCI runtime spec Hooks struct in json.
# see details and example:
# https://github.com/opencontainers/runtime-spec/blob/master/config.md#posix-platform-hooks
# ** Note: Overridden if set by CRI
default_oci_hooks = ""

# 'plugins."io.containerd.grpc.v1.cri".containerd' contains config related to containerd
[plugins."io.containerd.grpc.v1.cri".containerd]

Expand Down
4 changes: 4 additions & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,10 @@ type PluginConfig struct {
// DisableProcMount disables Kubernetes ProcMount support. This MUST be set to `true`
// when using containerd with Kubernetes <=1.11.
DisableProcMount bool `toml:"disable_proc_mount" json:"disableProcMount"`
// DefaultOCIHooks (optional) is a path to a json file that specifies a
// default OCI spec Hooks struct. ** Note: The any hooks set by default can be
// overridden if/when the hooks are set via the CRI.
DefaultOCIHooks string `toml:"default_oci_hooks" json:"defaultOCIHooks,omitempty"`
}

// X509KeyPairStreaming contains the x509 configuration for streaming
Expand Down
8 changes: 8 additions & 0 deletions pkg/containerd/opts/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,14 @@ func (m orderedMounts) parts(i int) int {
return strings.Count(filepath.Clean(m[i].ContainerPath), string(os.PathSeparator))
}

// WithHooks sets or replaces the runtimespec.Hooks with the provided hooks
func WithHooks(hooks *runtimespec.Hooks) oci.SpecOpts {
return func(ctx context.Context, client oci.Client, c *containers.Container, s *runtimespec.Spec) error {
s.Hooks = hooks
return nil
}
}

// WithAnnotation sets the provided annotation
func WithAnnotation(k, v string) oci.SpecOpts {
return func(ctx context.Context, client oci.Client, c *containers.Container, s *runtimespec.Spec) error {
Expand Down
18 changes: 18 additions & 0 deletions pkg/server/container_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package server

import (
"encoding/json"
"io/ioutil"
"path/filepath"
"time"

Expand Down Expand Up @@ -293,3 +295,19 @@ func runtimeSpec(id string, opts ...oci.SpecOpts) (*runtimespec.Spec, error) {
}
return spec, nil
}

// generateOCIHooks generates a runtimespec.Hooks struct from json in the passed in filepath.
func (c *criService) generateOCIHooks(profile string) (*runtimespec.Hooks, error) {
if profile == "" {
return nil, nil
}
hooks := &runtimespec.Hooks{}
f, err := ioutil.ReadFile(profile)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could set a reasonable file size limit to avoid memory exhaustion, especially with user input.

I thought there was a previous Kubernetes security recommendation about this code pattern (ReadFile + Unmarshal) but I can't find it anymore.

Copy link
Member Author

@mikebrow mikebrow Sep 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const reasonable_filesize = initial_reasonable_filesize^(current_year - 1970) // must build me each year

if err != nil {
return nil, err
}
if err := json.Unmarshal(f, hooks); err != nil {
return nil, err
}
return hooks, nil
}
9 changes: 9 additions & 0 deletions pkg/server/container_create_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,15 @@ func (c *criService) containerSpec(id string, sandboxID string, sandboxPid uint3
specOpts = append(specOpts, oci.WithReadonlyPaths(securityContext.GetReadonlyPaths()))
}

ociHooksProfile := c.config.DefaultOCIHooks
hooks, err := c.generateOCIHooks(ociHooksProfile)
if err != nil { // TODO (mikebrow): clean up prototype
return nil, errors.Wrapf(err, "failed to generate OCI hooks %+v", ociHooksProfile)
}
if hooks != nil {
specOpts = append(specOpts, customopts.WithHooks(hooks))
}

if securityContext.GetPrivileged() {
if !sandboxConfig.GetLinux().GetSecurityContext().GetPrivileged() {
return nil, errors.New("no privileged container allowed in sandbox")
Expand Down
10 changes: 10 additions & 0 deletions pkg/server/container_create_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/containerd/cri/pkg/annotations"
"github.com/containerd/cri/pkg/config"
customopts "github.com/containerd/cri/pkg/containerd/opts"
"github.com/pkg/errors"
)

// No container mounts for windows.
Expand Down Expand Up @@ -86,6 +87,15 @@ func (c *criService) containerSpec(id string, sandboxID string, sandboxPid uint3
specOpts = append(specOpts, customopts.WithAnnotation(pKey, pValue))
}

ociHooksProfile := c.config.DefaultOCIHooks
hooks, err := c.generateOCIHooks(ociHooksProfile)
if err != nil { // TODO (mikebrow): clean up prototype
return nil, errors.Wrapf(err, "failed to generate OCI hooks %+v", ociHooksProfile)
}
if hooks != nil {
specOpts = append(specOpts, customopts.WithHooks(hooks))
}

specOpts = append(specOpts,
customopts.WithAnnotation(annotations.ContainerType, annotations.ContainerTypeContainer),
customopts.WithAnnotation(annotations.SandboxID, sandboxID),
Expand Down