OCI spec may contain duplicate environment variables
Closes #5226 PiperOrigin-RevId: 351259576
This commit is contained in:
parent
4c4de66443
commit
7e462a1c7f
|
@ -440,6 +440,10 @@ func createProcessArgs(id string, spec *specs.Spec, creds *auth.Credentials, k *
|
|||
if err != nil {
|
||||
return kernel.CreateProcessArgs{}, fmt.Errorf("creating limits: %v", err)
|
||||
}
|
||||
env, err := specutils.ResolveEnvs(spec.Process.Env)
|
||||
if err != nil {
|
||||
return kernel.CreateProcessArgs{}, fmt.Errorf("resolving env: %w", err)
|
||||
}
|
||||
|
||||
wd := spec.Process.Cwd
|
||||
if wd == "" {
|
||||
|
@ -449,7 +453,7 @@ func createProcessArgs(id string, spec *specs.Spec, creds *auth.Credentials, k *
|
|||
// Create the process arguments.
|
||||
procArgs := kernel.CreateProcessArgs{
|
||||
Argv: spec.Process.Args,
|
||||
Envv: spec.Process.Env,
|
||||
Envv: env,
|
||||
WorkingDirectory: wd,
|
||||
Credentials: creds,
|
||||
Umask: 0022,
|
||||
|
@ -933,6 +937,11 @@ func (l *Loader) executeAsync(args *control.ExecArgs) (kernel.ThreadID, error) {
|
|||
}
|
||||
}
|
||||
|
||||
args.Envv, err = specutils.ResolveEnvs(args.Envv)
|
||||
if err != nil {
|
||||
return 0, fmt.Errorf("resolving env: %w", err)
|
||||
}
|
||||
|
||||
// Add the HOME environment variable if it is not already set.
|
||||
if kernel.VFS2Enabled {
|
||||
root := args.MountNamespaceVFS2.Root()
|
||||
|
|
|
@ -118,14 +118,14 @@ func (ex *Exec) Execute(_ context.Context, f *flag.FlagSet, args ...interface{})
|
|||
}
|
||||
|
||||
log.Debugf("Exec arguments: %+v", e)
|
||||
log.Debugf("Exec capablities: %+v", e.Capabilities)
|
||||
log.Debugf("Exec capabilities: %+v", e.Capabilities)
|
||||
|
||||
// Replace empty settings with defaults from container.
|
||||
if e.WorkingDirectory == "" {
|
||||
e.WorkingDirectory = c.Spec.Process.Cwd
|
||||
}
|
||||
if e.Envv == nil {
|
||||
e.Envv, err = resolveEnvs(c.Spec.Process.Env, ex.env)
|
||||
e.Envv, err = specutils.ResolveEnvs(c.Spec.Process.Env, ex.env)
|
||||
if err != nil {
|
||||
Fatalf("getting environment variables: %v", err)
|
||||
}
|
||||
|
@ -382,31 +382,6 @@ func argsFromProcess(p *specs.Process, enableRaw bool) (*control.ExecArgs, error
|
|||
}, nil
|
||||
}
|
||||
|
||||
// resolveEnvs transforms lists of environment variables into a single list of
|
||||
// environment variables. If a variable is defined multiple times, the last
|
||||
// value is used.
|
||||
func resolveEnvs(envs ...[]string) ([]string, error) {
|
||||
// First create a map of variable names to values. This removes any
|
||||
// duplicates.
|
||||
envMap := make(map[string]string)
|
||||
for _, env := range envs {
|
||||
for _, str := range env {
|
||||
parts := strings.SplitN(str, "=", 2)
|
||||
if len(parts) != 2 {
|
||||
return nil, fmt.Errorf("invalid variable: %s", str)
|
||||
}
|
||||
envMap[parts[0]] = parts[1]
|
||||
}
|
||||
}
|
||||
// Reassemble envMap into a list of environment variables of the form
|
||||
// NAME=VALUE.
|
||||
env := make([]string, 0, len(envMap))
|
||||
for k, v := range envMap {
|
||||
env = append(env, fmt.Sprintf("%s=%s", k, v))
|
||||
}
|
||||
return env, nil
|
||||
}
|
||||
|
||||
// capabilities takes a list of capabilities as strings and returns an
|
||||
// auth.TaskCapabilities struct with those capabilities in every capability set.
|
||||
// This mimics runc's behavior.
|
||||
|
|
|
@ -1803,3 +1803,91 @@ func TestMultiContainerEvent(t *testing.T) {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Tests that duplicate variables in the spec are merged into a single one.
|
||||
func TestDuplicateEnvVariable(t *testing.T) {
|
||||
conf := testutil.TestConfig(t)
|
||||
|
||||
rootDir, cleanup, err := testutil.SetupRootDir()
|
||||
if err != nil {
|
||||
t.Fatalf("error creating root dir: %v", err)
|
||||
}
|
||||
defer cleanup()
|
||||
conf.RootDir = rootDir
|
||||
|
||||
// Create files to dump `env` output.
|
||||
files := [3]*os.File{}
|
||||
for i := 0; i < len(files); i++ {
|
||||
var err error
|
||||
files[i], err = ioutil.TempFile(testutil.TmpDir(), "env-var-test")
|
||||
if err != nil {
|
||||
t.Fatalf("creating temp file: %v", err)
|
||||
}
|
||||
defer files[i].Close()
|
||||
defer os.Remove(files[i].Name())
|
||||
}
|
||||
|
||||
// Setup the containers. Use root container to test exec too.
|
||||
cmd1 := fmt.Sprintf("env > %q; sleep 1000", files[0].Name())
|
||||
cmd2 := fmt.Sprintf("env > %q", files[1].Name())
|
||||
cmdExec := fmt.Sprintf("env > %q", files[2].Name())
|
||||
testSpecs, ids := createSpecs([]string{"/bin/bash", "-c", cmd1}, []string{"/bin/bash", "-c", cmd2})
|
||||
testSpecs[0].Process.Env = append(testSpecs[0].Process.Env, "VAR=foo", "VAR=bar")
|
||||
testSpecs[1].Process.Env = append(testSpecs[1].Process.Env, "VAR=foo", "VAR=bar")
|
||||
|
||||
containers, cleanup, err := startContainers(conf, testSpecs, ids)
|
||||
if err != nil {
|
||||
t.Fatalf("error starting containers: %v", err)
|
||||
}
|
||||
defer cleanup()
|
||||
|
||||
// Wait for the `env` from the root container to finish.
|
||||
expectedPL := []*control.Process{
|
||||
newProcessBuilder().Cmd("bash").Process(),
|
||||
newProcessBuilder().Cmd("sleep").Process(),
|
||||
}
|
||||
if err := waitForProcessList(containers[0], expectedPL); err != nil {
|
||||
t.Errorf("failed to wait for sleep to start: %v", err)
|
||||
}
|
||||
if ws, err := containers[1].Wait(); err != nil {
|
||||
t.Errorf("failed to wait container 1: %v", err)
|
||||
} else if es := ws.ExitStatus(); es != 0 {
|
||||
t.Errorf("container %s exited with non-zero status: %v", containers[1].ID, es)
|
||||
}
|
||||
|
||||
execArgs := &control.ExecArgs{
|
||||
Filename: "/bin/bash",
|
||||
Argv: []string{"/bin/bash", "-c", cmdExec},
|
||||
Envv: []string{"VAR=foo", "VAR=bar"},
|
||||
}
|
||||
if ws, err := containers[0].executeSync(execArgs); err != nil || ws.ExitStatus() != 0 {
|
||||
t.Fatalf("exec failed, ws: %v, err: %v", ws, err)
|
||||
}
|
||||
|
||||
// Now read and check that none of the env has repeated values.
|
||||
for _, file := range files {
|
||||
out, err := ioutil.ReadAll(file)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Logf("Checking env %q:\n%s", file.Name(), out)
|
||||
envs := make(map[string]string)
|
||||
for _, line := range strings.Split(string(out), "\n") {
|
||||
if len(line) == 0 {
|
||||
continue
|
||||
}
|
||||
envVar := strings.SplitN(line, "=", 2)
|
||||
if len(envVar) != 2 {
|
||||
t.Fatalf("invalid env variable: %s", line)
|
||||
}
|
||||
key := envVar[0]
|
||||
if val, ok := envs[key]; ok {
|
||||
t.Errorf("env variable %q is duplicated: %q and %q", key, val, envVar[1])
|
||||
}
|
||||
envs[key] = envVar[1]
|
||||
}
|
||||
if _, ok := envs["VAR"]; !ok {
|
||||
t.Errorf("variable VAR missing: %v", envs)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -493,6 +493,31 @@ func EnvVar(env []string, name string) (string, bool) {
|
|||
return "", false
|
||||
}
|
||||
|
||||
// ResolveEnvs transforms lists of environment variables into a single list of
|
||||
// environment variables. If a variable is defined multiple times, the last
|
||||
// value is used.
|
||||
func ResolveEnvs(envs ...[]string) ([]string, error) {
|
||||
// First create a map of variable names to values. This removes any
|
||||
// duplicates.
|
||||
envMap := make(map[string]string)
|
||||
for _, env := range envs {
|
||||
for _, str := range env {
|
||||
parts := strings.SplitN(str, "=", 2)
|
||||
if len(parts) != 2 {
|
||||
return nil, fmt.Errorf("invalid variable: %s", str)
|
||||
}
|
||||
envMap[parts[0]] = parts[1]
|
||||
}
|
||||
}
|
||||
// Reassemble envMap into a list of environment variables of the form
|
||||
// NAME=VALUE.
|
||||
env := make([]string, 0, len(envMap))
|
||||
for k, v := range envMap {
|
||||
env = append(env, fmt.Sprintf("%s=%s", k, v))
|
||||
}
|
||||
return env, nil
|
||||
}
|
||||
|
||||
// FaqErrorMsg returns an error message pointing to the FAQ.
|
||||
func FaqErrorMsg(anchor, msg string) string {
|
||||
return fmt.Sprintf("%s; see https://gvisor.dev/faq#%s for more details", msg, anchor)
|
||||
|
|
Loading…
Reference in New Issue