From 8e73fd4c44ddbe1d98079cd89b1f0fa0ff5de45d Mon Sep 17 00:00:00 2001 From: ^_^void Date: Sat, 6 May 2023 17:29:59 +0800 Subject: [PATCH 1/7] #1152 execute HostConfigModifier at last #1152 execute HostConfigModifier at last, or some values will be overwrite --- lifecycle.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lifecycle.go b/lifecycle.go index 9bbecc62ca..cec20846b0 100644 --- a/lifecycle.go +++ b/lifecycle.go @@ -289,11 +289,6 @@ func (p *DockerProvider) preCreateContainerHook(ctx context.Context, req Contain req.ConfigModifier(dockerInput) } - if req.HostConfigModifier == nil { - req.HostConfigModifier = defaultHostConfigModifier(req) - } - req.HostConfigModifier(hostConfig) - if req.EnpointSettingsModifier != nil { req.EnpointSettingsModifier(endpointSettings) } @@ -319,6 +314,11 @@ func (p *DockerProvider) preCreateContainerHook(ctx context.Context, req Contain dockerInput.ExposedPorts = exposedPortSet hostConfig.PortBindings = exposedPortMap + + if req.HostConfigModifier == nil { + req.HostConfigModifier = defaultHostConfigModifier(req) + } + req.HostConfigModifier(hostConfig) return nil } From fb47bdbb484ff19c5039865bb57e58e90886c183 Mon Sep 17 00:00:00 2001 From: ^_^void Date: Mon, 8 May 2023 09:21:41 +0800 Subject: [PATCH 2/7] add UT: Request contains exposed port modifiers --- lifecycle_test.go | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/lifecycle_test.go b/lifecycle_test.go index 092d2c9f61..1beefbccd4 100644 --- a/lifecycle_test.go +++ b/lifecycle_test.go @@ -300,6 +300,39 @@ func TestPreCreateModifierHook(t *testing.T) { "Networking config's network ID should be retrieved from Docker", ) }) + + t.Run("Request contains exposed port modifiers", func(t *testing.T) { + // reqWithModifiers { + req := ContainerRequest{ + Image: nginxAlpineImage, // alpine image does expose port 80 + HostConfigModifier: func(hostConfig *container.HostConfig) { + hostConfig.PortBindings = nat.PortMap{ + "80/tcp": []nat.PortBinding{ + { + HostIP: "localhost", + HostPort: "8080", + }, + }, + } + }, + ExposedPorts: []string{"80"}, + } + // } + + // define empty inputs to be overwritten by the pre create hook + inputConfig := &container.Config{ + Image: req.Image, + } + inputHostConfig := &container.HostConfig{} + inputNetworkingConfig := &network.NetworkingConfig{} + + err = provider.preCreateContainerHook(ctx, req, inputConfig, inputHostConfig, inputNetworkingConfig) + require.Nil(t, err) + + // assertions + assert.Equal(t, inputHostConfig.PortBindings["80/tcp"][0].HostIP, "localhost") + assert.Equal(t, inputHostConfig.PortBindings["80/tcp"][0].HostPort, "8080") + }) } func TestLifecycleHooks(t *testing.T) { From 8c992662c8e268db78206c6427312f7f9c0d47be Mon Sep 17 00:00:00 2001 From: ^_^void Date: Fri, 12 May 2023 10:37:12 +0800 Subject: [PATCH 3/7] merge nat.ParsePortSpecs and hostConfig.PortBindings --- go.mod | 5 ++- go.sum | 9 +++-- lifecycle.go | 27 ++++++++++--- lifecycle_test.go | 100 +++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 129 insertions(+), 12 deletions(-) diff --git a/go.mod b/go.mod index 9011230086..5b507b5f4b 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,7 @@ require ( github.com/moby/term v0.0.0-20221128092401-c43b287e0e0f github.com/opencontainers/image-spec v1.1.0-rc2 github.com/stretchr/testify v1.8.2 + golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea golang.org/x/sys v0.6.0 gotest.tools/gotestsum v1.10.0 ) @@ -45,12 +46,12 @@ require ( github.com/pmezard/go-difflib v1.0.0 // indirect github.com/rogpeppe/go-internal v1.8.1 // indirect github.com/sirupsen/logrus v1.9.0 // indirect - golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect + golang.org/x/mod v0.6.0 // indirect golang.org/x/net v0.7.0 // indirect golang.org/x/sync v0.1.0 // indirect golang.org/x/term v0.5.0 // indirect golang.org/x/time v0.0.0-20220210224613-90d013bbcef8 // indirect - golang.org/x/tools v0.1.12 // indirect + golang.org/x/tools v0.2.0 // indirect google.golang.org/genproto v0.0.0-20220617124728-180714bec0ad // indirect google.golang.org/grpc v1.47.0 // indirect google.golang.org/protobuf v1.28.0 // indirect diff --git a/go.sum b/go.sum index 280d071169..a7026bd906 100644 --- a/go.sum +++ b/go.sum @@ -177,13 +177,16 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= +golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea h1:vLCWI/yYrdEHyN2JzIzPO3aaQJHQdp89IZBA/+azVC4= +golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU= golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 h1:6zppjxzCulZykYSLyVDYbneBfbaBIQPYMevg0bEwv2s= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= +golang.org/x/mod v0.6.0 h1:b9gGHsz9/HhJ3HF5DHQytPpuwocVTChQJK3AvoLRD5I= +golang.org/x/mod v0.6.0/go.mod h1:4mET923SAdbXp2ki8ey+zGs1SLqsuM2Y0uvdZR/fUNI= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -263,8 +266,8 @@ golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roY golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= golang.org/x/tools v0.1.11/go.mod h1:SgwaegtQh8clINPpECJMqnxLv9I09HLqnW3RMqW0CA4= -golang.org/x/tools v0.1.12 h1:VveCTK38A2rkS8ZqFY25HIDFscX5X9OoEhJd3quQmXU= -golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= +golang.org/x/tools v0.2.0 h1:G6AHpWxTMGY1KyEYoAQ5WTtIekUUvDNjan3ugu60JvE= +golang.org/x/tools v0.2.0/go.mod h1:y4OqIKeOV/fWJetJ8bXPU1sEVniLMIyDAZWeHdV+NTA= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/lifecycle.go b/lifecycle.go index cec20846b0..d42421548f 100644 --- a/lifecycle.go +++ b/lifecycle.go @@ -2,10 +2,12 @@ package testcontainers import ( "context" + "strings" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/network" "github.com/docker/go-connections/nat" + "golang.org/x/exp/slices" ) // ContainerRequestHook is a hook that will be called before a container is created. @@ -289,6 +291,11 @@ func (p *DockerProvider) preCreateContainerHook(ctx context.Context, req Contain req.ConfigModifier(dockerInput) } + if req.HostConfigModifier == nil { + req.HostConfigModifier = defaultHostConfigModifier(req) + } + req.HostConfigModifier(hostConfig) + if req.EnpointSettingsModifier != nil { req.EnpointSettingsModifier(endpointSettings) } @@ -313,16 +320,24 @@ func (p *DockerProvider) preCreateContainerHook(ctx context.Context, req Contain } dockerInput.ExposedPorts = exposedPortSet - hostConfig.PortBindings = exposedPortMap - - if req.HostConfigModifier == nil { - req.HostConfigModifier = defaultHostConfigModifier(req) - } - req.HostConfigModifier(hostConfig) + hostConfig.PortBindings = mergePortBindings(hostConfig.PortBindings, exposedPortMap, req.ExposedPorts) return nil } +func mergePortBindings(configPortMap, exposedPortMap nat.PortMap, exposedPorts []string) nat.PortMap { + if exposedPortMap == nil { + exposedPortMap = make(map[nat.Port][]nat.PortBinding) + } + + for k, v := range configPortMap { + if slices.Contains(exposedPorts, strings.Split(string(k), "/")[0]) { + exposedPortMap[k] = v + } + } + return exposedPortMap +} + // defaultHostConfigModifier provides a default modifier including the deprecated fields func defaultHostConfigModifier(req ContainerRequest) func(hostConfig *container.HostConfig) { return func(hostConfig *container.HostConfig) { diff --git a/lifecycle_test.go b/lifecycle_test.go index 1beefbccd4..d738841857 100644 --- a/lifecycle_test.go +++ b/lifecycle_test.go @@ -300,7 +300,7 @@ func TestPreCreateModifierHook(t *testing.T) { "Networking config's network ID should be retrieved from Docker", ) }) - + t.Run("Request contains exposed port modifiers", func(t *testing.T) { // reqWithModifiers { req := ContainerRequest{ @@ -335,6 +335,104 @@ func TestPreCreateModifierHook(t *testing.T) { }) } +func Test_mergePortBindings(t *testing.T) { + type arg struct { + configPortMap nat.PortMap + parsedPortMap nat.PortMap + exposedPorts []string + } + cases := []struct { + name string + arg arg + expected nat.PortMap + }{ + { + name: "empty ports", + arg: arg{ + configPortMap: nil, + parsedPortMap: nil, + exposedPorts: nil, + }, + expected: map[nat.Port][]nat.PortBinding{}, + }, + { + name: "config port map but not exposed", + arg: arg{ + configPortMap: map[nat.Port][]nat.PortBinding{ + "80/tcp": {{HostIP: "1", HostPort: "2"}}, + }, + parsedPortMap: nil, + exposedPorts: nil, + }, + expected: map[nat.Port][]nat.PortBinding{}, + }, + { + name: "parsed port map without config", + arg: arg{ + configPortMap: nil, + parsedPortMap: map[nat.Port][]nat.PortBinding{ + "80/tcp": {{HostIP: "", HostPort: ""}}, + }, + exposedPorts: nil, + }, + expected: map[nat.Port][]nat.PortBinding{ + "80/tcp": {{HostIP: "", HostPort: ""}}, + }, + }, + { + name: "config port map without parsed", + arg: arg{ + configPortMap: map[nat.Port][]nat.PortBinding{ + "80/tcp": {{HostIP: "1", HostPort: "2"}}, + }, + parsedPortMap: nil, + exposedPorts: []string{"80"}, + }, + expected: map[nat.Port][]nat.PortBinding{ + "80/tcp": {{HostIP: "1", HostPort: "2"}}, + }, + }, + { + name: "config port map with parsed", + arg: arg{ + configPortMap: map[nat.Port][]nat.PortBinding{ + "80/tcp": {{HostIP: "1", HostPort: "2"}}, + }, + parsedPortMap: map[nat.Port][]nat.PortBinding{ + "80/tcp": {{HostIP: "", HostPort: ""}}, + }, + exposedPorts: []string{"80"}, + }, + expected: map[nat.Port][]nat.PortBinding{ + "80/tcp": {{HostIP: "1", HostPort: "2"}}, + }, + }, + { + name: "merge both parsed and config", + arg: arg{ + configPortMap: map[nat.Port][]nat.PortBinding{ + "80/tcp": {{HostIP: "1", HostPort: "2"}}, + }, + parsedPortMap: map[nat.Port][]nat.PortBinding{ + "90/tcp": {{HostIP: "", HostPort: ""}}, + }, + exposedPorts: []string{"80"}, + }, + expected: map[nat.Port][]nat.PortBinding{ + "80/tcp": {{HostIP: "1", HostPort: "2"}}, + "90/tcp": {{HostIP: "", HostPort: ""}}, + }, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + res := mergePortBindings(c.arg.configPortMap, c.arg.parsedPortMap, c.arg.exposedPorts) + assert.Equal(t, c.expected, res) + }) + } +} + func TestLifecycleHooks(t *testing.T) { tests := []struct { name string From eacb0ba5901a5c938324ac91ed3ba7cc1778494b Mon Sep 17 00:00:00 2001 From: ^_^void Date: Fri, 12 May 2023 14:30:27 +0800 Subject: [PATCH 4/7] update UT --- lifecycle_test.go | 36 ++++++------------------------------ 1 file changed, 6 insertions(+), 30 deletions(-) diff --git a/lifecycle_test.go b/lifecycle_test.go index d738841857..70e9666e04 100644 --- a/lifecycle_test.go +++ b/lifecycle_test.go @@ -335,7 +335,7 @@ func TestPreCreateModifierHook(t *testing.T) { }) } -func Test_mergePortBindings(t *testing.T) { +func TestMergePortBindings(t *testing.T) { type arg struct { configPortMap nat.PortMap parsedPortMap nat.PortMap @@ -379,46 +379,22 @@ func Test_mergePortBindings(t *testing.T) { "80/tcp": {{HostIP: "", HostPort: ""}}, }, }, - { - name: "config port map without parsed", - arg: arg{ - configPortMap: map[nat.Port][]nat.PortBinding{ - "80/tcp": {{HostIP: "1", HostPort: "2"}}, - }, - parsedPortMap: nil, - exposedPorts: []string{"80"}, - }, - expected: map[nat.Port][]nat.PortBinding{ - "80/tcp": {{HostIP: "1", HostPort: "2"}}, - }, - }, - { - name: "config port map with parsed", - arg: arg{ - configPortMap: map[nat.Port][]nat.PortBinding{ - "80/tcp": {{HostIP: "1", HostPort: "2"}}, - }, - parsedPortMap: map[nat.Port][]nat.PortBinding{ - "80/tcp": {{HostIP: "", HostPort: ""}}, - }, - exposedPorts: []string{"80"}, - }, - expected: map[nat.Port][]nat.PortBinding{ - "80/tcp": {{HostIP: "1", HostPort: "2"}}, - }, - }, { name: "merge both parsed and config", arg: arg{ configPortMap: map[nat.Port][]nat.PortBinding{ + "60/tcp": {{HostIP: "1", HostPort: "2"}}, + "70/tcp": {{HostIP: "1", HostPort: "2"}}, "80/tcp": {{HostIP: "1", HostPort: "2"}}, }, parsedPortMap: map[nat.Port][]nat.PortBinding{ + "80/tcp": {{HostIP: "", HostPort: ""}}, "90/tcp": {{HostIP: "", HostPort: ""}}, }, - exposedPorts: []string{"80"}, + exposedPorts: []string{"70", "80"}, }, expected: map[nat.Port][]nat.PortBinding{ + "70/tcp": {{HostIP: "1", HostPort: "2"}}, "80/tcp": {{HostIP: "1", HostPort: "2"}}, "90/tcp": {{HostIP: "", HostPort: ""}}, }, From 7f00b6af12f5aa5de0ec71d42512892dff63e056 Mon Sep 17 00:00:00 2001 From: ^_^void Date: Tue, 16 May 2023 09:29:57 +0800 Subject: [PATCH 5/7] clean code --- lifecycle_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/lifecycle_test.go b/lifecycle_test.go index 70e9666e04..027f4bf12c 100644 --- a/lifecycle_test.go +++ b/lifecycle_test.go @@ -302,7 +302,6 @@ func TestPreCreateModifierHook(t *testing.T) { }) t.Run("Request contains exposed port modifiers", func(t *testing.T) { - // reqWithModifiers { req := ContainerRequest{ Image: nginxAlpineImage, // alpine image does expose port 80 HostConfigModifier: func(hostConfig *container.HostConfig) { @@ -317,7 +316,6 @@ func TestPreCreateModifierHook(t *testing.T) { }, ExposedPorts: []string{"80"}, } - // } // define empty inputs to be overwritten by the pre create hook inputConfig := &container.Config{ From 59eca371280f75817d6dd596e40be846f7081332 Mon Sep 17 00:00:00 2001 From: ^_^void Date: Tue, 16 May 2023 09:47:26 +0800 Subject: [PATCH 6/7] add case to UT --- lifecycle_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lifecycle_test.go b/lifecycle_test.go index 027f4bf12c..2b5db5fcaa 100644 --- a/lifecycle_test.go +++ b/lifecycle_test.go @@ -377,6 +377,21 @@ func TestMergePortBindings(t *testing.T) { "80/tcp": {{HostIP: "", HostPort: ""}}, }, }, + { + name: "parsed and configured but not exposed", + arg: arg{ + configPortMap: map[nat.Port][]nat.PortBinding{ + "80/tcp": {{HostIP: "1", HostPort: "2"}}, + }, + parsedPortMap: map[nat.Port][]nat.PortBinding{ + "80/tcp": {{HostIP: "", HostPort: ""}}, + }, + exposedPorts: nil, + }, + expected: map[nat.Port][]nat.PortBinding{ + "80/tcp": {{HostIP: "", HostPort: ""}}, + }, + }, { name: "merge both parsed and config", arg: arg{ From 959635c6a6f2b36d0f0ef8ccb8bd0d490b7584ce Mon Sep 17 00:00:00 2001 From: ^_^void Date: Thu, 18 May 2023 15:07:30 +0800 Subject: [PATCH 7/7] update PortBinding logic --- lifecycle.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lifecycle.go b/lifecycle.go index d42421548f..600adbe4c7 100644 --- a/lifecycle.go +++ b/lifecycle.go @@ -320,7 +320,13 @@ func (p *DockerProvider) preCreateContainerHook(ctx context.Context, req Contain } dockerInput.ExposedPorts = exposedPortSet - hostConfig.PortBindings = mergePortBindings(hostConfig.PortBindings, exposedPortMap, req.ExposedPorts) + + // only exposing those ports automatically if the container request exposes zero ports and the container does not run in a container network + if len(exposedPorts) == 0 && !hostConfig.NetworkMode.IsContainer() { + hostConfig.PortBindings = exposedPortMap + } else { + hostConfig.PortBindings = mergePortBindings(hostConfig.PortBindings, exposedPortMap, req.ExposedPorts) + } return nil }