Skip to content
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

Less interfaces #3316

Closed
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/capabilities/capabilities.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"sort"
"strings"

"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runtime-spec/specs-go"
"github.com/sirupsen/logrus"
"github.com/syndtr/gocapability/capability"
)
Expand Down Expand Up @@ -49,7 +49,7 @@ func KnownCapabilities() []string {
// New creates a new Caps from the given Capabilities config. Unknown Capabilities
// or Capabilities that are unavailable in the current environment are ignored,
// printing a warning instead.
func New(capConfig *configs.Capabilities) (*Caps, error) {
func New(capConfig *specs.LinuxCapabilities) (*Caps, error) {
var (
err error
c Caps
Expand Down
4 changes: 2 additions & 2 deletions libcontainer/capabilities/capabilities_linux_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ import (
"os"
"testing"

"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runtime-spec/specs-go"
"github.com/sirupsen/logrus"
"github.com/sirupsen/logrus/hooks/test"
"github.com/syndtr/gocapability/capability"
)

func TestNew(t *testing.T) {
cs := []string{"CAP_CHOWN", "CAP_UNKNOWN", "CAP_UNKNOWN2"}
conf := configs.Capabilities{
conf := specs.LinuxCapabilities{
Bounding: cs,
Effective: cs,
Inheritable: cs,
Expand Down
15 changes: 1 addition & 14 deletions libcontainer/configs/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ type Config struct {

// Capabilities specify the capabilities to keep when executing the process inside the container
// All capabilities not specified will be dropped from the processes capability mask
Capabilities *Capabilities `json:"capabilities"`
Capabilities *specs.LinuxCapabilities `json:"capabilities"`

// Networks specifies the container's network setup to be created
Networks []*Network `json:"networks"`
Expand Down Expand Up @@ -261,19 +261,6 @@ func KnownHookNames() []string {
}
}

type Capabilities struct {
// Bounding is the set of capabilities checked by the kernel.
Bounding []string
// Effective is the set of capabilities checked by the kernel.
Effective []string
// Inheritable is the capabilities preserved across execve.
Inheritable []string
// Permitted is the limiting superset for effective capabilities.
Permitted []string
// Ambient is the ambient set of capabilities that are kept.
Ambient []string
}
Comment on lines -267 to -278
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it will cause any issues, but mentioninging it (just in case); the type is the same from a "go" perspective, but the runtime-spec type uses lowercase names (and omitempty) for the JSON representation; https://github.com/opencontainers/runtime-spec/blob/a3c33d663ebc56c4d35dbceaa447c7bf37f6fab3/specs-go/config.go#L65-L78

// LinuxCapabilities specifies the list of allowed capabilities that are kept for a process.
// http://man7.org/linux/man-pages/man7/capabilities.7.html
type LinuxCapabilities struct {
	// Bounding is the set of capabilities checked by the kernel.
	Bounding []string `json:"bounding,omitempty" platform:"linux"`
	// Effective is the set of capabilities checked by the kernel.
	Effective []string `json:"effective,omitempty" platform:"linux"`
	// Inheritable is the capabilities preserved across execve.
	Inheritable []string `json:"inheritable,omitempty" platform:"linux"`
	// Permitted is the limiting superset for effective capabilities.
	Permitted []string `json:"permitted,omitempty" platform:"linux"`
	// Ambient is the ambient set of capabilities that are kept.
	Ambient []string `json:"ambient,omitempty" platform:"linux"`
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, this might open the hell gates:

[root@kir-rhat runc]# jq < /run/runc/xxx/state.json | grep -A10 capab
    "capabilities": {
      "Bounding": [
        "CAP_AUDIT_WRITE",
        "CAP_KILL",
        "CAP_NET_BIND_SERVICE"
      ],
      "Effective": [
        "CAP_AUDIT_WRITE",
        "CAP_KILL",
        "CAP_NET_BIND_SERVICE"
      ],
[root@kir-rhat runc]# jq < /run/runc/xx344/state.json | grep -A10 capab
    "capabilities": {
      "bounding": [
        "CAP_AUDIT_WRITE",
        "CAP_KILL",
        "CAP_NET_BIND_SERVICE"
      ],
      "effective": [
        "CAP_AUDIT_WRITE",
        "CAP_KILL",
        "CAP_NET_BIND_SERVICE"
      ],

From what I can see, this is not used anywhere, and yet it might become problematic.

I guess I'll revert this.

Copy link
Member

Choose a reason for hiding this comment

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

If these files are only used by runc and/or only used by Go code, then it shouldn't (AFAICS) be an issue. json.Unmarshal() is case insensitive for keys (for better or worse 😅)

https://go.dev/play/p/9bo-nSvEARD

package main

import (
	"encoding/json"
	"fmt"
	"reflect"
)

type Capabilities struct {
	// Bounding is the set of capabilities checked by the kernel.
	Bounding []string
	// Effective is the set of capabilities checked by the kernel.
	Effective []string
	// Inheritable is the capabilities preserved across execve.
	Inheritable []string
	// Permitted is the limiting superset for effective capabilities.
	Permitted []string
	// Ambient is the ambient set of capabilities that are kept.
	Ambient []string
}

type LinuxCapabilities struct {
	// Bounding is the set of capabilities checked by the kernel.
	Bounding []string `json:"bounding,omitempty" platform:"linux"`
	// Effective is the set of capabilities checked by the kernel.
	Effective []string `json:"effective,omitempty" platform:"linux"`
	// Inheritable is the capabilities preserved across execve.
	Inheritable []string `json:"inheritable,omitempty" platform:"linux"`
	// Permitted is the limiting superset for effective capabilities.
	Permitted []string `json:"permitted,omitempty" platform:"linux"`
	// Ambient is the ambient set of capabilities that are kept.
	Ambient []string `json:"ambient,omitempty" platform:"linux"`
}

type runc struct {
	Capabilities *Capabilities `json:"capabilities"`
}

type oci struct {
	Capabilities *LinuxCapabilities `json:"capabilities"`
}

const jsonLower = `
{
  "capabilities": {
    "bounding": [
      "CAP_AUDIT_WRITE",
      "CAP_KILL",
      "CAP_NET_BIND_SERVICE"
    ],
    "effective": [
      "CAP_AUDIT_WRITE",
      "CAP_KILL",
      "CAP_NET_BIND_SERVICE"
    ]
  }
}`

const jsonUpper = `
{
  "capabilities": {
    "Bounding": [
      "CAP_AUDIT_WRITE",
      "CAP_KILL",
      "CAP_NET_BIND_SERVICE"
    ],
    "Effective": [
      "CAP_AUDIT_WRITE",
      "CAP_KILL",
      "CAP_NET_BIND_SERVICE"
    ]
  }
}`

func main() {
	var (
		runcLower runc
		runcUpper runc
		ociLower  oci
		ociUpper  oci
	)

	if err := json.Unmarshal([]byte(jsonLower), &runcLower); err != nil {
		fmt.Println(err)
	}
	if err := json.Unmarshal([]byte(jsonUpper), &runcUpper); err != nil {
		fmt.Println(err)
	}
	if err := json.Unmarshal([]byte(jsonLower), &ociLower); err != nil {
		fmt.Println(err)
	}
	if err := json.Unmarshal([]byte(jsonUpper), &ociUpper); err != nil {
		fmt.Println(err)
	}

	if reflect.DeepEqual(runcLower, runcUpper) {
		fmt.Printf("SAME: %v <--> %v\n", runcLower.Capabilities, runcUpper.Capabilities)
	}
	if reflect.DeepEqual(ociLower, ociUpper) {
		fmt.Printf("SAME: %v <--> %v\n", ociLower.Capabilities, runcUpper.Capabilities)
	}
}


func (hooks HookList) RunHooks(state *specs.State) error {
for i, h := range hooks {
if err := h.Run(state); err != nil {
Expand Down
44 changes: 22 additions & 22 deletions libcontainer/init_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,27 +49,27 @@ type network struct {

// initConfig is used for transferring parameters from Exec() to Init()
type initConfig struct {
Args []string `json:"args"`
Env []string `json:"env"`
Cwd string `json:"cwd"`
Capabilities *configs.Capabilities `json:"capabilities"`
ProcessLabel string `json:"process_label"`
AppArmorProfile string `json:"apparmor_profile"`
NoNewPrivileges bool `json:"no_new_privileges"`
User string `json:"user"`
AdditionalGroups []string `json:"additional_groups"`
Config *configs.Config `json:"config"`
Networks []*network `json:"network"`
PassedFilesCount int `json:"passed_files_count"`
ContainerID string `json:"containerid"`
Rlimits []configs.Rlimit `json:"rlimits"`
CreateConsole bool `json:"create_console"`
ConsoleWidth uint16 `json:"console_width"`
ConsoleHeight uint16 `json:"console_height"`
RootlessEUID bool `json:"rootless_euid,omitempty"`
RootlessCgroups bool `json:"rootless_cgroups,omitempty"`
SpecState *specs.State `json:"spec_state,omitempty"`
Cgroup2Path string `json:"cgroup2_path,omitempty"`
Args []string `json:"args"`
Env []string `json:"env"`
Cwd string `json:"cwd"`
Capabilities *specs.LinuxCapabilities `json:"capabilities"`
ProcessLabel string `json:"process_label"`
AppArmorProfile string `json:"apparmor_profile"`
NoNewPrivileges bool `json:"no_new_privileges"`
User string `json:"user"`
AdditionalGroups []string `json:"additional_groups"`
Config *configs.Config `json:"config"`
Networks []*network `json:"network"`
PassedFilesCount int `json:"passed_files_count"`
ContainerID string `json:"containerid"`
Rlimits []configs.Rlimit `json:"rlimits"`
CreateConsole bool `json:"create_console"`
ConsoleWidth uint16 `json:"console_width"`
ConsoleHeight uint16 `json:"console_height"`
RootlessEUID bool `json:"rootless_euid,omitempty"`
RootlessCgroups bool `json:"rootless_cgroups,omitempty"`
SpecState *specs.State `json:"spec_state,omitempty"`
Cgroup2Path string `json:"cgroup2_path,omitempty"`
}

type initer interface {
Expand Down Expand Up @@ -167,7 +167,7 @@ func finalizeNamespace(config *initConfig) error {
}
}

caps := &configs.Capabilities{}
caps := &specs.LinuxCapabilities{}
if config.Capabilities != nil {
caps = config.Capabilities
} else if config.Config.Capabilities != nil {
Expand Down
4 changes: 2 additions & 2 deletions libcontainer/integration/exec_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ func TestProcessCaps(t *testing.T) {
Env: standardEnvironment,
Stdin: nil,
Stdout: &stdout,
Capabilities: &configs.Capabilities{},
Capabilities: &specs.LinuxCapabilities{},
Init: true,
}
pconfig.Capabilities.Bounding = append(config.Capabilities.Bounding, "CAP_NET_ADMIN")
Expand Down Expand Up @@ -1402,7 +1402,7 @@ func TestRootfsPropagationSharedMount(t *testing.T) {
Env: standardEnvironment,
Stdin: stdinR2,
Stdout: &stdout2,
Capabilities: &configs.Capabilities{},
Capabilities: &specs.LinuxCapabilities{},
}

// Provide CAP_SYS_ADMIN
Expand Down
3 changes: 2 additions & 1 deletion libcontainer/integration/template_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runc/libcontainer/devices"
"github.com/opencontainers/runc/libcontainer/specconv"
"github.com/opencontainers/runtime-spec/specs-go"
"golang.org/x/sys/unix"
)

Expand Down Expand Up @@ -42,7 +43,7 @@ func newTemplateConfig(t *testing.T, p *tParam) *configs.Config {
}
config := &configs.Config{
Rootfs: newRootfs(t),
Capabilities: &configs.Capabilities{
Capabilities: &specs.LinuxCapabilities{
Bounding: []string{
"CAP_CHOWN",
"CAP_DAC_OVERRIDE",
Expand Down
3 changes: 2 additions & 1 deletion libcontainer/process.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"os"

"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runtime-spec/specs-go"
)

var errInvalidProcess = errors.New("invalid process")
Expand Down Expand Up @@ -55,7 +56,7 @@ type Process struct {

// Capabilities specify the capabilities to keep when executing the process inside the container
// All capabilities not specified will be dropped from the processes capability mask
Capabilities *configs.Capabilities
Capabilities *specs.LinuxCapabilities

// AppArmorProfile specifies the profile to apply to the process and is
// changed at the time the process is execed
Expand Down
10 changes: 1 addition & 9 deletions libcontainer/specconv/spec_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,15 +481,7 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) {
config.NoNewPrivileges = spec.Process.NoNewPrivileges
config.Umask = spec.Process.User.Umask
config.ProcessLabel = spec.Process.SelinuxLabel
if spec.Process.Capabilities != nil {
config.Capabilities = &configs.Capabilities{
Bounding: spec.Process.Capabilities.Bounding,
Effective: spec.Process.Capabilities.Effective,
Permitted: spec.Process.Capabilities.Permitted,
Inheritable: spec.Process.Capabilities.Inheritable,
Ambient: spec.Process.Capabilities.Ambient,
}
}
config.Capabilities = spec.Process.Capabilities
}
createHooks(spec, config)
config.Version = specs.Version
Expand Down
10 changes: 1 addition & 9 deletions utils_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"golang.org/x/sys/unix"

"github.com/opencontainers/runc/libcontainer"
"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runc/libcontainer/specconv"
"github.com/opencontainers/runc/libcontainer/utils"
)
Expand Down Expand Up @@ -86,21 +85,14 @@ func newProcess(p specs.Process) (*libcontainer.Process, error) {
Label: p.SelinuxLabel,
NoNewPrivileges: &p.NoNewPrivileges,
AppArmorProfile: p.ApparmorProfile,
Capabilities: p.Capabilities,
}

if p.ConsoleSize != nil {
lp.ConsoleWidth = uint16(p.ConsoleSize.Width)
lp.ConsoleHeight = uint16(p.ConsoleSize.Height)
}

if p.Capabilities != nil {
lp.Capabilities = &configs.Capabilities{}
lp.Capabilities.Bounding = p.Capabilities.Bounding
lp.Capabilities.Effective = p.Capabilities.Effective
lp.Capabilities.Inheritable = p.Capabilities.Inheritable
lp.Capabilities.Permitted = p.Capabilities.Permitted
lp.Capabilities.Ambient = p.Capabilities.Ambient
}
for _, gid := range p.User.AdditionalGids {
lp.AdditionalGroups = append(lp.AdditionalGroups, strconv.FormatUint(uint64(gid), 10))
}
Expand Down