Skip to content

Less interfaces #3316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kolyshkin wants to merge 21 commits into from
Closed

Less interfaces #3316

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
604917e
libct/README: rm Cgroupfs
kolyshkin Jan 25, 2022
8b9fa19
libct/configs: rm Windows TODO
kolyshkin Dec 8, 2021
923a8c9
libct: initContainer: rename Id -> ID
kolyshkin Jan 21, 2022
e1147d6
libct/config: rm type Capabilities type
kolyshkin Dec 8, 2021
d0ba8c6
libct: remove Factory interface and unused methods
kolyshkin Dec 8, 2021
3148aef
libct: rename factory_linux* -> factory
kolyshkin Dec 9, 2021
94f6098
libct: rm BaseContainer and Container interfaces
kolyshkin Dec 9, 2021
e4557c1
libct: remove some factory methods
kolyshkin Dec 9, 2021
344ef42
libct: rm TestGetContainerStats, mockIntelRdtManager
kolyshkin Dec 9, 2021
0e6bf47
libct: rm intelrtd.Manager interface, NewIntelRdtManager
kolyshkin Dec 9, 2021
232bd46
libct: StartInitialization: decouple from factory
kolyshkin Dec 9, 2021
ac0f26b
libct/int: remove logger from init
kolyshkin Dec 10, 2021
742be71
init.go: move logger setup to StartInitialization
kolyshkin Dec 9, 2021
270cf70
Fix some revive warnings
kolyshkin Dec 11, 2021
0d9a69a
libct/init: call Init from containerInit
kolyshkin Dec 9, 2021
b9b9fe1
libct: Container, Factory: rm InitPath, InitArgs
kolyshkin Jan 20, 2022
e4c0fef
libct: remove Validator interface
kolyshkin Jan 20, 2022
0c6d53d
libct: Container, Factory: rm newuidmap/newgidmap
kolyshkin Jan 21, 2022
13c6cae
libct: New: rm opts arguments
kolyshkin Jan 21, 2022
5ce43c4
runc: remove --criu option
kolyshkin Jan 21, 2022
3d5fd4a
utils: remove loadFactory
kolyshkin Jan 21, 2022
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
4 changes: 2 additions & 2 deletions libcontainer/configs/validate/rootless.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import (
"github.com/opencontainers/runc/libcontainer/configs"
)

// rootlessEUID makes sure that the config can be applied when runc
// rootlessEUIDCheck makes sure that the config can be applied when runc
// is being executed as a non-root user (euid != 0) in the current user namespace.
func (v *ConfigValidator) rootlessEUID(config *configs.Config) error {
func rootlessEUIDCheck(config *configs.Config) error {
if !config.RootlessEUID {
return nil
}
Expand Down
40 changes: 14 additions & 26 deletions libcontainer/configs/validate/rootless_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,42 +34,34 @@ func rootlessEUIDConfig() *configs.Config {
}

func TestValidateRootlessEUID(t *testing.T) {
validator := New()

config := rootlessEUIDConfig()
if err := validator.Validate(config); err != nil {
if err := Validate(config); err != nil {
t.Errorf("Expected error to not occur: %+v", err)
}
}

/* rootlessEUIDMappings */

func TestValidateRootlessEUIDUserns(t *testing.T) {
validator := New()

config := rootlessEUIDConfig()
config.Namespaces = nil
if err := validator.Validate(config); err == nil {
if err := Validate(config); err == nil {
t.Errorf("Expected error to occur if user namespaces not set")
}
}

func TestValidateRootlessEUIDMappingUid(t *testing.T) {
validator := New()

config := rootlessEUIDConfig()
config.UidMappings = nil
if err := validator.Validate(config); err == nil {
if err := Validate(config); err == nil {
t.Errorf("Expected error to occur if no uid mappings provided")
}
}

func TestValidateNonZeroEUIDMappingGid(t *testing.T) {
validator := New()

config := rootlessEUIDConfig()
config.GidMappings = nil
if err := validator.Validate(config); err == nil {
if err := Validate(config); err == nil {
t.Errorf("Expected error to occur if no gid mappings provided")
}
}
Expand All @@ -78,8 +70,6 @@ func TestValidateNonZeroEUIDMappingGid(t *testing.T) {

func TestValidateRootlessEUIDMountUid(t *testing.T) {
config := rootlessEUIDConfig()
validator := New()

config.Mounts = []*configs.Mount{
{
Source: "devpts",
Expand All @@ -88,37 +78,35 @@ func TestValidateRootlessEUIDMountUid(t *testing.T) {
},
}

if err := validator.Validate(config); err != nil {
if err := Validate(config); err != nil {
t.Errorf("Expected error to not occur when uid= not set in mount options: %+v", err)
}

config.Mounts[0].Data = "uid=5"
if err := validator.Validate(config); err == nil {
if err := Validate(config); err == nil {
t.Errorf("Expected error to occur when setting uid=5 in mount options")
}

config.Mounts[0].Data = "uid=0"
if err := validator.Validate(config); err != nil {
if err := Validate(config); err != nil {
t.Errorf("Expected error to not occur when setting uid=0 in mount options: %+v", err)
}

config.Mounts[0].Data = "uid=2"
config.UidMappings[0].Size = 10
if err := validator.Validate(config); err != nil {
if err := Validate(config); err != nil {
t.Errorf("Expected error to not occur when setting uid=2 in mount options and UidMapping[0].size is 10")
}

config.Mounts[0].Data = "uid=20"
config.UidMappings[0].Size = 10
if err := validator.Validate(config); err == nil {
if err := Validate(config); err == nil {
t.Errorf("Expected error to occur when setting uid=20 in mount options and UidMapping[0].size is 10")
}
}

func TestValidateRootlessEUIDMountGid(t *testing.T) {
config := rootlessEUIDConfig()
validator := New()

config.Mounts = []*configs.Mount{
{
Source: "devpts",
Expand All @@ -127,29 +115,29 @@ func TestValidateRootlessEUIDMountGid(t *testing.T) {
},
}

if err := validator.Validate(config); err != nil {
if err := Validate(config); err != nil {
t.Errorf("Expected error to not occur when gid= not set in mount options: %+v", err)
}

config.Mounts[0].Data = "gid=5"
if err := validator.Validate(config); err == nil {
if err := Validate(config); err == nil {
t.Errorf("Expected error to occur when setting gid=5 in mount options")
}

config.Mounts[0].Data = "gid=0"
if err := validator.Validate(config); err != nil {
if err := Validate(config); err != nil {
t.Errorf("Expected error to not occur when setting gid=0 in mount options: %+v", err)
}

config.Mounts[0].Data = "gid=5"
config.GidMappings[0].Size = 10
if err := validator.Validate(config); err != nil {
if err := Validate(config); err != nil {
t.Errorf("Expected error to not occur when setting gid=5 in mount options and GidMapping[0].size is 10")
}

config.Mounts[0].Data = "gid=11"
config.GidMappings[0].Size = 10
if err := validator.Validate(config); err == nil {
if err := Validate(config); err == nil {
t.Errorf("Expected error to occur when setting gid=11 in mount options and GidMapping[0].size is 10")
}
}
55 changes: 21 additions & 34 deletions libcontainer/configs/validate/validator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,30 +16,19 @@ import (
"golang.org/x/sys/unix"
)

type Validator interface {
Validate(*configs.Config) error
}

func New() Validator {
return &ConfigValidator{}
}

type ConfigValidator struct{}

type check func(config *configs.Config) error

func (v *ConfigValidator) Validate(config *configs.Config) error {
func Validate(config *configs.Config) error {
Copy link
Member

@thaJeztah thaJeztah Jan 26, 2022

Choose a reason for hiding this comment

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

Thanks for doing this! I recall looking at this a couple of times (why the "Validator"??)

Perhaps as a follow-up, we should move configs/validate to configs. It's a bit weird to have it in a separate package, as it's validating the config. Moving it into configs, if could just be validate(config) (no need to export), or

func (config *configs.Config) validate() error

Copy link
Contributor Author

@kolyshkin kolyshkin Jan 27, 2022

Choose a reason for hiding this comment

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

Perhaps as a follow-up, we should move configs/validate to configs.

I gave it a try and got circular deps as a result. Will take a look later, but it seems this was the reason why it's in a separate package.

Oh, and we still have to export it because it's also used from libcontainer/specconv/spec_linux_test.go

checks := []check{
v.cgroups,
v.rootfs,
v.network,
v.hostname,
v.security,
v.usernamespace,
v.cgroupnamespace,
v.sysctl,
v.intelrdt,
v.rootlessEUID,
cgroupsCheck,
rootfs,
network,
hostname,
security,
namespaces,
sysctl,
intelrdtCheck,
rootlessEUIDCheck,
}
for _, c := range checks {
if err := c(config); err != nil {
Expand All @@ -48,7 +37,7 @@ func (v *ConfigValidator) Validate(config *configs.Config) error {
}
// Relaxed validation rules for backward compatibility
warns := []check{
v.mounts, // TODO (runc v1.x.x): make this an error instead of a warning
mounts, // TODO (runc v1.x.x): make this an error instead of a warning
}
for _, c := range warns {
if err := c(config); err != nil {
Expand All @@ -60,7 +49,7 @@ func (v *ConfigValidator) Validate(config *configs.Config) error {

// rootfs validates if the rootfs is an absolute path and is not a symlink
// to the container's root filesystem.
func (v *ConfigValidator) rootfs(config *configs.Config) error {
func rootfs(config *configs.Config) error {
if _, err := os.Stat(config.Rootfs); err != nil {
return fmt.Errorf("invalid rootfs: %w", err)
}
Expand All @@ -77,7 +66,7 @@ func (v *ConfigValidator) rootfs(config *configs.Config) error {
return nil
}

func (v *ConfigValidator) network(config *configs.Config) error {
func network(config *configs.Config) error {
if !config.Namespaces.Contains(configs.NEWNET) {
if len(config.Networks) > 0 || len(config.Routes) > 0 {
return errors.New("unable to apply network settings without a private NET namespace")
Expand All @@ -86,14 +75,14 @@ func (v *ConfigValidator) network(config *configs.Config) error {
return nil
}

func (v *ConfigValidator) hostname(config *configs.Config) error {
func hostname(config *configs.Config) error {
if config.Hostname != "" && !config.Namespaces.Contains(configs.NEWUTS) {
return errors.New("unable to set hostname without a private UTS namespace")
}
return nil
}

func (v *ConfigValidator) security(config *configs.Config) error {
func security(config *configs.Config) error {
// restrict sys without mount namespace
if (len(config.MaskPaths) > 0 || len(config.ReadonlyPaths) > 0) &&
!config.Namespaces.Contains(configs.NEWNS) {
Expand All @@ -106,7 +95,7 @@ func (v *ConfigValidator) security(config *configs.Config) error {
return nil
}

func (v *ConfigValidator) usernamespace(config *configs.Config) error {
func namespaces(config *configs.Config) error {
if config.Namespaces.Contains(configs.NEWUSER) {
if _, err := os.Stat("/proc/self/ns/user"); os.IsNotExist(err) {
return errors.New("USER namespaces aren't enabled in the kernel")
Expand All @@ -116,15 +105,13 @@ func (v *ConfigValidator) usernamespace(config *configs.Config) error {
return errors.New("User namespace mappings specified, but USER namespace isn't enabled in the config")
}
}
return nil
}

func (v *ConfigValidator) cgroupnamespace(config *configs.Config) error {
if config.Namespaces.Contains(configs.NEWCGROUP) {
if _, err := os.Stat("/proc/self/ns/cgroup"); os.IsNotExist(err) {
return errors.New("cgroup namespaces aren't enabled in the kernel")
}
}

return nil
}

Expand Down Expand Up @@ -161,7 +148,7 @@ func convertSysctlVariableToDotsSeparator(val string) string {
// sysctl validates that the specified sysctl keys are valid or not.
// /proc/sys isn't completely namespaced and depending on which namespaces
// are specified, a subset of sysctls are permitted.
func (v *ConfigValidator) sysctl(config *configs.Config) error {
func sysctl(config *configs.Config) error {
validSysctlMap := map[string]bool{
"kernel.msgmax": true,
"kernel.msgmnb": true,
Expand Down Expand Up @@ -227,7 +214,7 @@ func (v *ConfigValidator) sysctl(config *configs.Config) error {
return nil
}

func (v *ConfigValidator) intelrdt(config *configs.Config) error {
func intelrdtCheck(config *configs.Config) error {
if config.IntelRdt != nil {
if !intelrdt.IsCATEnabled() && !intelrdt.IsMBAEnabled() {
return errors.New("intelRdt is specified in config, but Intel RDT is not supported or enabled")
Expand All @@ -248,7 +235,7 @@ func (v *ConfigValidator) intelrdt(config *configs.Config) error {
return nil
}

func (v *ConfigValidator) cgroups(config *configs.Config) error {
func cgroupsCheck(config *configs.Config) error {
c := config.Cgroups
if c == nil {
return nil
Expand Down Expand Up @@ -277,7 +264,7 @@ func (v *ConfigValidator) cgroups(config *configs.Config) error {
return nil
}

func (v *ConfigValidator) mounts(config *configs.Config) error {
func mounts(config *configs.Config) error {
for _, m := range config.Mounts {
if !filepath.IsAbs(m.Destination) {
return fmt.Errorf("invalid mount %+v: mount destination not absolute", m)
Expand Down
Loading