feat: check if the nsdelegate mount option

The nsdelegate mount option conflicts with cgroup path config in runc
specs. We check for conflicting configuration and return an error in
that case.

go/action_kit_commons/diskfill/utils.go

@@ -40,7 +40,7 @@ func createBundle(ctx context.Context, r runc.Runc, sidecar SidecarOpts, opts Op
 
  runc.RefreshNamespaces(ctx, sidecar.TargetProcess.Namespaces, specs.PIDNamespace)
 
- if err := bundle.EditSpec(ctx,
+ if err := bundle.EditSpec(
  runc.WithHostname(containerId),
  runc.WithAnnotations(map[string]string{
  "com.steadybit.sidecar": "true",
@@ -56,7 +56,7 @@ func createBundle(ctx context.Context, r runc.Runc, sidecar SidecarOpts, opts Op
  Options: []string{"noexec", "nosuid", "nodev", "rprivate"},
  }),
  ); err != nil {
- return nil, fmt.Errorf("failed to create config.json: %w", err)
+ return nil, err
  }
 
  success = true

go/action_kit_commons/go.mod

@@ -7,15 +7,16 @@ require (
  github.com/opencontainers/runtime-spec v1.2.0
  github.com/rs/zerolog v1.32.0
  github.com/stretchr/testify v1.9.0
+ golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842
 )
 
 require (
  github.com/davecgh/go-spew v1.1.1 // indirect
- github.com/kr/pretty v0.3.0 // indirect
+ github.com/kr/pretty v0.3.1 // indirect
  github.com/mattn/go-colorable v0.1.13 // indirect
  github.com/mattn/go-isatty v0.0.20 // indirect
  github.com/pmezard/go-difflib v1.0.0 // indirect
- github.com/rogpeppe/go-internal v1.11.0 // indirect
+ github.com/rogpeppe/go-internal v1.12.0 // indirect
  github.com/stretchr/objx v0.5.2 // indirect
  golang.org/x/sys v0.20.0 // indirect
  gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect

go/action_kit_commons/go.sum

@@ -5,10 +5,9 @@ github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
 github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
 github.com/kelseyhightower/envconfig v1.4.0 h1:Im6hONhd3pLkfDFsbRgu68RDNkGF1r3dvMUtDTo2cv8=
 github.com/kelseyhightower/envconfig v1.4.0/go.mod h1:cccZRl6mQpaq41TPp5QxidR+Sa3axMbJDNb//FQX6Gg=
-github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
 github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
-github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0=
-github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk=
+github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
+github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
 github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
@@ -21,30 +20,29 @@ github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWE
 github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
 github.com/opencontainers/runtime-spec v1.2.0 h1:z97+pHb3uELt/yiAWD691HNHQIF07bE7dzrbT927iTk=
 github.com/opencontainers/runtime-spec v1.2.0/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0=
+github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
-github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc=
-github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M=
-github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA=
+github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
+github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=
+github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4=
 github.com/rs/xid v1.5.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg=
 github.com/rs/zerolog v1.32.0 h1:keLypqrlIjaFsbmJOBdB/qvyF8KEtCWHwobLp5l/mQ0=
 github.com/rs/zerolog v1.32.0/go.mod h1:/7mN4D5sKwJLZQ2b/znpjC3/GQWY/xaDXUM0kKWRHss=
 github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY=
 github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA=
 github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
 github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
+golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 h1:vr/HnozRka3pE4EsMEg1lgkXJkTFJCVUX+S/ZT6wYzM=
+golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842/go.mod h1:XtvwrStGgqGPLc4cjQfWqZHG1YFdYs6swckp8vpsjnc=
 golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y=
-golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 golang.org/x/sys v0.20.0 h1:Od9JTbYCk261bKm4M/mw7AklTlFYIa0bIp9BgSm1S8Y=
 golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
-gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=
 gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q=
-gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

go/action_kit_commons/network/dig_runner.go

@@ -48,7 +48,6 @@ func (r *RuncDigRunner) Run(ctx context.Context, arg []string, stdin io.Reader)
  runc.RefreshNamespaces(ctx, r.Sidecar.TargetProcess.Namespaces, specs.NetworkNamespace)
 
  if err = bundle.EditSpec(
- ctx,
  runc.WithHostname(fmt.Sprintf("dig-%s", id)),
  runc.WithAnnotations(map[string]string{
  "com.steadybit.sidecar": "true",

go/action_kit_commons/network/list_interfaces.go

@@ -54,15 +54,13 @@ func ListInterfaces(ctx context.Context, r runc.Runc, sidecar SidecarOpts) ([]In
  runc.RefreshNamespaces(ctx, sidecar.TargetProcess.Namespaces, specs.NetworkNamespace)
 
  if err = bundle.EditSpec(
- ctx,
  runc.WithHostname(fmt.Sprintf("ip-link-show-%s", id)),
  runc.WithAnnotations(map[string]string{
  "com.steadybit.sidecar": "true",
  }),
  runc.WithNamespaces(runc.FilterNamespaces(sidecar.TargetProcess.Namespaces, specs.NetworkNamespace)),
  runc.WithCapabilities("CAP_NET_ADMIN"),
- runc.WithProcessArgs("ip", "-json", "link", "show"),
- ); err != nil {
+ runc.WithProcessArgs("ip", "-json", "link", "show")); err != nil {
  return nil, err
  }
 

go/action_kit_commons/network/network.go

@@ -156,7 +156,6 @@ func executeIpCommands(ctx context.Context, r runc.Runc, sidecar SidecarOpts, fa
 
  processArgs := []string{"ip", "-family", string(family), "-force", "-batch", "-"}
  if err = bundle.EditSpec(
- ctx,
  runc.WithHostname(fmt.Sprintf("ip-%s", id)),
  runc.WithAnnotations(map[string]string{"com.steadybit.sidecar": "true"}),
  runc.WithNamespaces(runc.FilterNamespaces(sidecar.TargetProcess.Namespaces, specs.NetworkNamespace)),
@@ -212,7 +211,6 @@ func executeTcCommands(ctx context.Context, r runc.Runc, sidecar SidecarOpts, cm
 
  processArgs := []string{"tc", "-force", "-batch", "-"}
  if err = bundle.EditSpec(
- ctx,
  runc.WithHostname(fmt.Sprintf("tc-%s", id)),
  runc.WithAnnotations(map[string]string{"com.steadybit.sidecar": "true"}),
  runc.WithNamespaces(runc.FilterNamespaces(sidecar.TargetProcess.Namespaces, specs.NetworkNamespace)),

go/action_kit_commons/network/network_test.go

@@ -100,8 +100,8 @@ type MockBundle struct {
  id string
 }
 
-func (m *MockBundle) EditSpec(ctx context.Context, editors ...runc.SpecEditor) error {
- args := m.Called(ctx, editors)
+func (m *MockBundle) EditSpec(editors ...runc.SpecEditor) error {
+ args := m.Called(editors)
  return args.Error(0)
 }
 

go/action_kit_commons/runc/runc.go

@@ -4,6 +4,7 @@
 package runc
 
 import (
+ "bufio"
  "bytes"
  "context"
  "encoding/json"
@@ -12,12 +13,14 @@ import (
  "github.com/kelseyhightower/envconfig"
  "github.com/opencontainers/runtime-spec/specs-go"
  "github.com/rs/zerolog/log"
+ "golang.org/x/exp/slices"
  "io"
  "os"
  "os/exec"
  "path/filepath"
  "runtime/trace"
  "strconv"
+ "strings"
  "syscall"
  "time"
 )
@@ -32,7 +35,7 @@ type Runc interface {
 }
 
 type ContainerBundle interface {
- EditSpec(ctx context.Context, editors ...SpecEditor) error
+ EditSpec(editors ...SpecEditor) error
  MountFromProcess(ctx context.Context, fromPid int, fromPath, mountpoint string) error
  CopyFileFromProcess(ctx context.Context, pid int, fromPath, toPath string) error
  Path() string
@@ -76,6 +79,9 @@ var (
 )
 
 func NewRunc(cfg Config) Runc {
+ if errors.Is(checkForCgroup2Nsdelegate(), ErrCgroup2NsdelegateOptionUsed) {
+ log.Warn().Err(ErrCgroup2NsdelegateOptionUsed).Msg("cgroup2 mount option nsdelegate is set. This may lead to unexpected errors.")
+ }
  return &defaultRunc{cfg: cfg}
 }
 
@@ -233,9 +239,8 @@ func (r *defaultRunc) Kill(ctx context.Context, id string, signal syscall.Signal
 
 type SpecEditor func(spec *specs.Spec)
 
-func (b *containerBundle) EditSpec(ctx context.Context, editors ...SpecEditor) error {
- defer trace.StartRegion(ctx, "runc.EditSpec").End()
- spec, err := readSpec(filepath.Join(b.path, "config.json"))
+func (b *containerBundle) EditSpec(editors ...SpecEditor) error {
+ spec, err := b.readSpec()
  if err != nil {
  return err
  }
@@ -245,23 +250,18 @@ func (b *containerBundle) EditSpec(ctx context.Context, editors ...SpecEditor) e
  for _, fn := range editors {
  fn(spec)
  }
- err = writeSpec(filepath.Join(b.path, "config.json"), spec)
- log.Trace().Str("bundle", b.path).Interface("createSpec", spec).Msg("written runc createSpec")
- return err
-}
 
-func (r *defaultRunc) createSpec(ctx context.Context, bundle string) error {
- defer trace.StartRegion(ctx, "runc.Spec").End()
- log.Trace().Str("bundle", bundle).Msg("creating container createSpec")
- output, err := r.command(ctx, "spec", "--bundle", bundle).CombinedOutput()
- if err != nil {
- return fmt.Errorf("%w: %s", err, output)
+ if err := checkForCgroup2NsdelegateConflict(spec); err != nil {
+ return err
  }
- return nil
+
+ err = b.writeSpec(spec)
+ log.Trace().Str("bundle", b.path).Interface("createSpec", spec).Msg("written runc createSpec")
+ return err
 }
 
-func readSpec(file string) (*specs.Spec, error) {
- content, err := os.ReadFile(file)
+func (b *containerBundle) readSpec() (*specs.Spec, error) {
+ content, err := os.ReadFile(filepath.Join(b.path, "config.json"))
  if err != nil {
  return nil, err
  }
@@ -274,13 +274,22 @@ func readSpec(file string) (*specs.Spec, error) {
 
  return &spec, nil
 }
-
-func writeSpec(file string, spec *specs.Spec) error {
+func (b *containerBundle) writeSpec(spec *specs.Spec) error {
  content, err := json.MarshalIndent(spec, "", "\t")
  if err != nil {
  return err
  }
- return os.WriteFile(file, content, 0644)
+ return os.WriteFile(filepath.Join(b.path, "config.json"), content, 0644)
+}
+
+func (r *defaultRunc) createSpec(ctx context.Context, bundle string) error {
+ defer trace.StartRegion(ctx, "runc.Spec").End()
+ log.Trace().Str("bundle", bundle).Msg("creating container createSpec")
+ output, err := r.command(ctx, "spec", "--bundle", bundle).CombinedOutput()
+ if err != nil {
+ return fmt.Errorf("%w: %s", err, output)
+ }
+ return nil
 }
 
 func (r *defaultRunc) command(ctx context.Context, args ...string) *exec.Cmd {
@@ -428,3 +437,53 @@ func WithNamespace(ns LinuxNamespace) SpecEditor {
  spec.Linux.Namespaces = append(spec.Linux.Namespaces, ns)
  }
 }
+
+var getMountOptions = defaultMountOptions
+
+var ErrCgroup2NsdelegateOptionUsed = errors.New("cgroup2 nsdelegate mount option conflicts with cgroup path in spec")
+
+func checkForCgroup2NsdelegateConflict(spec *specs.Spec) error {
+ if spec == nil || spec.Linux == nil || spec.Linux.CgroupsPath == "" {
+ return nil
+ }
+
+ return checkForCgroup2Nsdelegate()
+}
+
+func checkForCgroup2Nsdelegate() error {
+ opts, err := getMountOptions("/sys/fs/cgroup", "cgroup2")
+ if err != nil {
+ return err
+ }
+
+ if slices.Contains(opts, "nsdelegate") {
+ return ErrCgroup2NsdelegateOptionUsed
+ }
+ return nil
+}
+
+func defaultMountOptions(file, vfstype string) ([]string, error) {
+ f, err := os.Open("/proc/mounts")
+ if err != nil {
+ return nil, err
+ }
+ defer func() { _ = f.Close() }()
+
+ return getMountOptionsFromReader(f, file, vfstype), nil
+}
+
+func getMountOptionsFromReader(r io.Reader, file string, vfstype string) []string {
+ scanner := bufio.NewScanner(r)
+ for scanner.Scan() {
+ fields := strings.Fields(scanner.Text())
+ if len(fields) >= 4 {
+ fsFile := fields[1]
+ fsVfstype := fields[2]
+ fsMntops := fields[3]
+ if fsFile == file && fsVfstype == vfstype {
+ return strings.Split(fsMntops, ",")
+ }
+ }
+ }
+ return nil
+}

go/action_kit_commons/runc/runc_test.go

@@ -1,7 +1,11 @@
 package runc
 
 import (
+ "fmt"
+ "github.com/opencontainers/runtime-spec/specs-go"
+ "github.com/stretchr/testify/assert"
  "reflect"
+ "strings"
  "testing"
  "time"
 )
@@ -68,3 +72,70 @@ func Test_unmarshalGuarded(t *testing.T) {
  })
  }
 }
+
+func Test_getMountOptionsFromReader(t *testing.T) {
+ r := strings.NewReader(`
+/dev/vda1 /etc/hosts ext4 rw,relatime 0 0
+cgroup2 /sys/fs/cgroup cgroup2 rw,nosuid,nodev,noexec,relatime 0 0
+tmpfs /run/docker.sock tmpfs rw,nosuid,nodev,size=762488k,nr_inodes=819200,mode=755 0 0
+`)
+ opts := getMountOptionsFromReader(r, "/sys/fs/cgroup", "cgroup2")
+ assert.Equal(t, []string{"rw", "nosuid", "nodev", "noexec", "relatime"}, opts)
+
+ opts = getMountOptionsFromReader(r, "/not/in/there", "cgroup2")
+ assert.Empty(t, opts)
+}
+
+func TestCheckForCgroup2Nsdelegate(t *testing.T) {
+ specWithCgroupPath := specs.Spec{
+ Linux: &specs.Linux{
+ CgroupsPath: "/some",
+ },
+ }
+ specDefault := specs.Spec{}
+ mountOptsDefault := []string{"rw", "nosuid", "nodev", "noexec", "relatime"}
+ mountOptsNsDelegate := []string{"rw", "nosuid", "nodev", "noexec", "relatime", "nsdelegate"}
+
+ tests := []struct {
+ name string
+ spec specs.Spec
+ mountOpts []string
+ wantErr assert.ErrorAssertionFunc
+ }{
+ {
+ name: "should not error when no cgroup2 is used",
+ spec: specWithCgroupPath,
+ mountOpts: nil,
+ wantErr: assert.NoError,
+ },
+ {
+ name: "should not error when no CGroupPath is set",
+ spec: specDefault,
+ mountOpts: mountOptsNsDelegate,
+ wantErr: assert.NoError,
+ },
+ {
+ name: "should not error when no nsdelegate is used",
+ spec: specWithCgroupPath,
+ mountOpts: mountOptsDefault,
+ wantErr: assert.NoError,
+ },
+ {
+ name: "should error when nsdelegate is used",
+ spec: specWithCgroupPath,
+ mountOpts: mountOptsNsDelegate,
+ wantErr: assert.Error,
+ },
+ }
+
+ defer func() { getMountOptions = defaultMountOptions }()
+ for _, tt := range tests {
+ getMountOptions = func(file, fstype string) ([]string, error) {
+ return tt.mountOpts, nil
+ }
+
+ t.Run(tt.name, func(t *testing.T) {
+ tt.wantErr(t, checkForCgroup2NsdelegateConflict(&tt.spec), fmt.Sprintf("CheckForCgroup2Nsdelegate(%+v, %+v)", tt.spec, tt.mountOpts))
+ })
+ }
+}