diff --git a/container_test.go b/container_test.go index dc7dc97d7..52b1ee71b 100644 --- a/container_test.go +++ b/container_test.go @@ -32,7 +32,9 @@ import ( // Register the typeurl "github.com/containerd/containerd/cio" "github.com/containerd/containerd/containers" + "github.com/containerd/containerd/namespaces" "github.com/containerd/containerd/oci" + "github.com/containerd/containerd/platforms" _ "github.com/containerd/containerd/runtime" "github.com/containerd/typeurl" specs "github.com/opencontainers/runtime-spec/specs-go" @@ -1528,3 +1530,59 @@ func TestContainerHook(t *testing.T) { } defer task.Delete(ctx, WithProcessKill) } + +func TestShimSockLength(t *testing.T) { + t.Parallel() + + // Max length of namespace should be 76 + namespace := strings.Repeat("n", 76) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + ctx = namespaces.WithNamespace(ctx, namespace) + + client, err := newClient(t, address) + if err != nil { + t.Fatal(err) + } + defer client.Close() + + image, err := client.Pull(ctx, testImage, + WithPlatformMatcher(platforms.Default()), + WithPullUnpack, + ) + if err != nil { + t.Fatal(err) + } + + id := strings.Repeat("c", 64) + + // We don't have limitation with length of container name, + // but 64 bytes of sha256 is the common case + container, err := client.NewContainer(ctx, id, + WithNewSnapshot(id, image), + WithNewSpec(oci.WithImageConfig(image), withExitStatus(0)), + ) + if err != nil { + t.Fatal(err) + } + defer container.Delete(ctx, WithSnapshotCleanup) + + task, err := container.NewTask(ctx, empty()) + if err != nil { + t.Fatal(err) + } + defer task.Delete(ctx) + + statusC, err := task.Wait(ctx) + if err != nil { + t.Fatal(err) + } + + if err := task.Start(ctx); err != nil { + t.Fatal(err) + } + + <-statusC +} diff --git a/runtime/v1/linux/bundle.go b/runtime/v1/linux/bundle.go index 74a9f2aba..7fcf36a3d 100644 --- a/runtime/v1/linux/bundle.go +++ b/runtime/v1/linux/bundle.go @@ -20,6 +20,7 @@ package linux import ( "context" + "crypto/sha256" "fmt" "io/ioutil" "os" @@ -103,7 +104,7 @@ func ShimLocal(c *Config, exchange *exchange.Exchange) ShimOpt { // ShimConnect is a ShimOpt for connecting to an existing remote shim func ShimConnect(c *Config, onClose func()) ShimOpt { return func(b *bundle, ns string, ropts *runctypes.RuncOptions) (shim.Config, client.Opt) { - return b.shimConfig(ns, c, ropts), client.WithConnect(b.shimAddress(ns), onClose) + return b.shimConfig(ns, c, ropts), client.WithConnect(b.decideShimAddress(ns), onClose) } } @@ -127,10 +128,32 @@ func (b *bundle) Delete() error { return errors.Wrapf(err, "Failed to remove both bundle and workdir locations: %v", err2) } -func (b *bundle) shimAddress(namespace string) string { +func (b *bundle) legacyShimAddress(namespace string) string { return filepath.Join(string(filepath.Separator), "containerd-shim", namespace, b.id, "shim.sock") } +func (b *bundle) shimAddress(namespace string) string { + d := sha256.Sum256([]byte(filepath.Join(namespace, b.id))) + return filepath.Join(string(filepath.Separator), "containerd-shim", fmt.Sprintf("%x.sock", d)) +} + +func (b *bundle) loadAddress() (string, error) { + addressPath := filepath.Join(b.path, "address") + data, err := ioutil.ReadFile(addressPath) + if err != nil { + return "", err + } + return string(data), nil +} + +func (b *bundle) decideShimAddress(namespace string) string { + address, err := b.loadAddress() + if err != nil { + return b.legacyShimAddress(namespace) + } + return address +} + func (b *bundle) shimConfig(namespace string, c *Config, runcOptions *runctypes.RuncOptions) shim.Config { var ( criuPath string diff --git a/runtime/v1/shim/client/client.go b/runtime/v1/shim/client/client.go index ef740308d..e4d28090b 100644 --- a/runtime/v1/shim/client/client.go +++ b/runtime/v1/shim/client/client.go @@ -20,10 +20,12 @@ package client import ( "context" + "fmt" "io" "net" "os" "os/exec" + "path/filepath" "strings" "sync" "syscall" @@ -107,6 +109,10 @@ func WithStart(binary, address, daemonAddress, cgroup string, debug bool, exitHa "address": address, "debug": debug, }).Infof("shim %s started", binary) + + if err := writeAddress(filepath.Join(config.Path, "address"), address); err != nil { + return nil, nil, err + } // set shim in cgroup if it is provided if cgroup != "" { if err := setCgroup(cgroup, cmd); err != nil { @@ -166,6 +172,25 @@ func newCommand(binary, daemonAddress string, debug bool, config shim.Config, so return cmd, nil } +// writeAddress writes a address file atomically +func writeAddress(path, address string) error { + path, err := filepath.Abs(path) + if err != nil { + return err + } + tempPath := filepath.Join(filepath.Dir(path), fmt.Sprintf(".%s", filepath.Base(path))) + f, err := os.OpenFile(tempPath, os.O_RDWR|os.O_CREATE|os.O_EXCL|os.O_SYNC, 0666) + if err != nil { + return err + } + _, err = f.WriteString(address) + f.Close() + if err != nil { + return err + } + return os.Rename(tempPath, path) +} + func newSocket(address string) (*net.UnixListener, error) { if len(address) > 106 { return nil, errors.Errorf("%q: unix socket path too long (> 106)", address) diff --git a/runtime/v2/shim/util_unix.go b/runtime/v2/shim/util_unix.go index b86f8624a..6b9ae4eea 100644 --- a/runtime/v2/shim/util_unix.go +++ b/runtime/v2/shim/util_unix.go @@ -20,6 +20,8 @@ package shim import ( "context" + "crypto/sha256" + "fmt" "net" "path/filepath" "strings" @@ -50,7 +52,8 @@ func SocketAddress(ctx context.Context, id string) (string, error) { if err != nil { return "", err } - return filepath.Join(string(filepath.Separator), "containerd-shim", ns, id, "shim.sock"), nil + d := sha256.Sum256([]byte(filepath.Join(ns, id))) + return filepath.Join(string(filepath.Separator), "containerd-shim", fmt.Sprintf("%x.sock", d)), nil } // AnonDialer returns a dialer for an abstract socket