Merge pull request #50719 from akerouanton/deprecate-legacy-links-env-vars

daemon: deprecate env vars set by legacy links

daemon/container_operations_unix.go

@@ -46,15 +46,18 @@ func (daemon *Daemon) setupLinkedContainers(ctr *container.Container) ([]string,
 			return nil, fmt.Errorf("container %s not attached to default bridge network", child.ID)
 		}
 
-		linkEnvVars := links.EnvVars(
-			bridgeSettings.IPAddress,
-			childBridgeSettings.IPAddress,
-			linkAlias,
-			child.Config.Env,
-			child.Config.ExposedPorts,
-		)
-
-		env = append(env, linkEnvVars...)
+		// Environment variables defined when using legacy links are deprecated and will be removed in a future release.
+		// Allow users to restore the old behavior through this escape hatch.
+		if os.Getenv("DOCKER_KEEP_DEPRECATED_LEGACY_LINKS_ENV_VARS") == "1" {
+			linkEnvVars := links.EnvVars(
+				bridgeSettings.IPAddress,
+				childBridgeSettings.IPAddress,
+				linkAlias,
+				child.Config.Env,
+				child.Config.ExposedPorts,
+			)
+			env = append(env, linkEnvVars...)
+		}
 	}
 
 	return env, nil

hack/make/test-docker-py

@@ -37,6 +37,11 @@ PY_TEST_OPTIONS="$PY_TEST_OPTIONS --deselect=tests/integration/api_image_test.py
 
 # TODO(laurazard): re-enable after https://github.com/docker/docker-py/pull/3290 is included in the DOCKER_PY_COMMIT release.
 PY_TEST_OPTIONS="$PY_TEST_OPTIONS --deselect=tests/integration/models_containers_test.py::ContainerTest::test_exec_run_failed"
+
+# Legacy links are deprecated since a long time, and starting with v29.0, the Engine won't set legacy links env vars
+# automatically in linking containers. Thus, this test is expected to fail — skip it. See https://github.com/moby/moby/pull/50719
+PY_TEST_OPTIONS="${PY_TEST_OPTIONS} --deselect=tests/integration/api_container_test.py::CreateContainerTest::test_create_with_links"
+
 (
 	bundle .integration-daemon-start
 

integration-cli/docker_cli_exec_test.go

@@ -532,12 +532,3 @@ func (s *DockerCLIExecSuite) TestExecWindowsPathNotWiped(c *testing.T) {
 	out = strings.ToLower(strings.Trim(out, "\r\n"))
 	assert.Assert(c, is.Contains(out, `windowspowershell\v1.0`))
 }
-
-func (s *DockerCLIExecSuite) TestExecEnvLinksHost(c *testing.T) {
-	testRequires(c, DaemonIsLinux)
-	runSleepingContainer(c, "-d", "--name", "foo")
-	runSleepingContainer(c, "-d", "--link", "foo:db", "--hostname", "myhost", "--name", "bar")
-	out := cli.DockerCmd(c, "exec", "bar", "env").Stdout()
-	assert.Check(c, is.Contains(out, "HOSTNAME=myhost"))
-	assert.Check(c, is.Contains(out, "DB_NAME=/bar/db"))
-}

integration-cli/docker_cli_links_test.go

@@ -193,15 +193,6 @@ func (s *DockerCLILinksSuite) TestLinksUpdateOnRestart(c *testing.T) {
 	assert.Check(c, is.Equal(ip, realIP))
 }
 
-func (s *DockerCLILinksSuite) TestLinksEnvs(c *testing.T) {
-	testRequires(c, DaemonIsLinux)
-	cli.DockerCmd(c, "run", "-d", "-e", "e1=", "-e", "e2=v2", "-e", "e3=v3=v3", "--name=first", "busybox", "top")
-	out := cli.DockerCmd(c, "run", "--name=second", "--link=first:first", "busybox", "env").Stdout()
-	assert.Assert(c, is.Contains(out, "FIRST_ENV_e1=\n"))
-	assert.Assert(c, is.Contains(out, "FIRST_ENV_e2=v2"))
-	assert.Assert(c, is.Contains(out, "FIRST_ENV_e3=v3=v3"))
-}
-
 func (s *DockerCLILinksSuite) TestLinkShortDefinition(c *testing.T) {
 	testRequires(c, DaemonIsLinux)
 	cid := cli.DockerCmd(c, "run", "-d", "--name", "shortlinkdef", "busybox", "top").Stdout()

integration/networking/bridge_linux_test.go

@@ -1894,3 +1894,60 @@ func TestDropInForwardChain(t *testing.T) {
 		}
 	})
 }
+
+// TestLegacyLinksEnvVars verify that legacy links environment variables are set in containers when the daemon is
+// started with DOCKER_KEEP_DEPRECATED_LEGACY_LINKS_ENV_VARS=1, and are skipped when the daemon is started without that
+// environment variable.
+func TestLegacyLinksEnvVars(t *testing.T) {
+	for _, tc := range []struct {
+		name          string
+		expectEnvVars bool
+	}{
+		{"with legacy links env vars", true},
+		{"without legacy links env vars", false},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			var dEnv []string
+			if tc.expectEnvVars {
+				dEnv = []string{"DOCKER_KEEP_DEPRECATED_LEGACY_LINKS_ENV_VARS=1"}
+			}
+
+			ctx := setupTest(t)
+			d := daemon.New(t, daemon.WithEnvVars(dEnv...))
+			d.StartWithBusybox(ctx, t)
+			defer d.Stop(t)
+			c := d.NewClientT(t)
+			defer c.Close()
+
+			ctr1 := container.Run(ctx, t, c,
+				container.WithName("ctr1"),
+				container.WithCmd("httpd", "-f"))
+			defer c.ContainerRemove(ctx, ctr1, containertypes.RemoveOptions{Force: true})
+
+			exportRes := container.RunAttach(ctx, t, c,
+				container.WithName("ctr2"),
+				container.WithLinks("ctr1"),
+				container.WithCmd("/bin/sh", "-c", "export"),
+				container.WithAutoRemove)
+
+			// Check the list of environment variables set in the linking container.
+			var found bool
+			for _, l := range strings.Split(exportRes.Stdout.String(), "\n") {
+				if strings.HasPrefix(l, "export CTR1_") {
+					// Legacy links env var found, but not expected.
+					if !tc.expectEnvVars {
+						t.Fatalf("unexpected env var %q", l)
+					}
+
+					// Legacy links env var found, and expected. No need to check further.
+					found = true
+					break
+				}
+			}
+
+			if !found && tc.expectEnvVars {
+				t.Fatal("no legacy links env vars found")
+			}
+		})
+	}
+}