opencontainers / runtime-spec

OCI Runtime Specification
http://www.opencontainers.org
Apache License 2.0
3.15k stars 539 forks source link

Should linux fields be in process directly, or a level down? #831

Open lowenna opened 7 years ago

lowenna commented 7 years ago

Looking at the spec all-up, it seems that the Process struct is imbalanced relative to the spec in general, and shouldn't directly contain the Linux-specific fields, namely Capabilities, Rlimits, NoNewPrivileges, ApparmorProfile, OOMScoreAdj, SelinuxLabel and instead mirror the top-level Spec structure and have a LinuxProcess struct with those fields.

Something like

// Process contains information to start a specific application inside the container.
type Process struct {
    // Terminal creates an interactive terminal for the container.
    Terminal bool `json:"terminal,omitempty"`
    // ConsoleSize specifies the size of the console.
    ConsoleSize *Box `json:"consoleSize,omitempty"`
    // User specifies user information for the process.
    User User `json:"user"`
    // Args specifies the binary and arguments for the application to execute.
    Args []string `json:"args"`
    // Env populates the process environment for the process.
    Env []string `json:"env,omitempty"`
    // Cwd is the current working directory for the process and must be
    // relative to the container's root.
    Cwd string `json:"cwd"`
    // LinuxProcess is platform-specific configuration for Linux processes.
    LinuxProcess *LinuxProcess `json:"linuxprocess,omitempty" platform:"linux"`
}

// LinuxProcess contains platform-specific configurations for Linux processes in containers.
type LinuxProcess struct {  
    // Capabilities are Linux capabilities that are kept for the process.
    Capabilities *LinuxCapabilities `json:"capabilities,omitempty" platform:"linux"`
    // Rlimits specifies rlimit options to apply to the process.
    Rlimits []LinuxRlimit `json:"rlimits,omitempty" platform:"linux"`
    // NoNewPrivileges controls whether additional privileges could be gained by processes in the container.
    NoNewPrivileges bool `json:"noNewPrivileges,omitempty" platform:"linux"`
    // ApparmorProfile specifies the apparmor profile for the container.
    ApparmorProfile string `json:"apparmorProfile,omitempty" platform:"linux"`
    // Specify an oom_score_adj for the container.
    OOMScoreAdj *int `json:"oomScoreAdj,omitempty"`
    // SelinuxLabel specifies the selinux context that the container process is run as.
    SelinuxLabel string `json:"selinuxLabel,omitempty" platform:"linux"`
}

Thoughts?

vbatts commented 7 years ago

we've gone back on forth on this. I'm sure @wking could give a historical breakdown.

wking commented 7 years ago

On Tue, May 16, 2017 at 10:54:40AM -0700, v1.0.0.batts wrote:

I'm sure @wking could give a historical breakdown.

The current policy (since at least 2016-05-18) seems to be 1:

If any platform cannot use your setting, namespace it, unless that would seem weird for aesthetic reasons we're punting on defining.

Maybe this is a call to define those aesthetic reasons or drop the loophole? The initial shift of Linux-specific stuff into ‘process’ happened in #329, and there was a more recent shift in #789, neither of which has a lot of discussion about whether namespacing is appropriate. There is some previous discussion about namespacing in

405; for example, see [2,3,4].