From 2b634f5e6f7e142ce1996d27f16e474808161fc4 Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Mon, 9 Sep 2024 19:30:46 +0200 Subject: [PATCH 01/22] Fix #2632 - ImageBuildOptions.Labels are overwritten --- container.go | 9 ++++- container_test.go | 66 ++++++++++++++++++++++++++++++++++++ internal/core/labels.go | 20 +++++++++++ internal/core/labels_test.go | 36 ++++++++++++++++++++ 4 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 internal/core/labels_test.go diff --git a/container.go b/container.go index 8747335a28..3ec5155bfb 100644 --- a/container.go +++ b/container.go @@ -460,7 +460,14 @@ func (c *ContainerRequest) BuildOptions() (types.ImageBuildOptions, error) { } if !c.ShouldKeepBuiltImage() { - buildOptions.Labels = core.DefaultLabels(core.SessionID()) + dst := core.DefaultLabels(core.SessionID()) + if err = core.MergeCustomLabels(dst, c.Labels); err != nil { + return types.ImageBuildOptions{}, err + } + if err = core.MergeCustomLabels(dst, buildOptions.Labels); err != nil { + return types.ImageBuildOptions{}, err + } + buildOptions.Labels = dst } // Do this as late as possible to ensure we don't leak the context on error/panic. diff --git a/container_test.go b/container_test.go index 3cb14ac296..0d015c3cab 100644 --- a/container_test.go +++ b/container_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -303,6 +304,71 @@ func Test_BuildImageWithContexts(t *testing.T) { } } +func TestCustomLabelsImage(t *testing.T) { + // --- Given --- + const ( + myLabelName = "org.my.label" + myLabelValue = "my-label-value" + ) + + ctx := context.Background() + req := testcontainers.GenericContainerRequest{ + ContainerRequest: testcontainers.ContainerRequest{ + Image: "alpine:latest", + Labels: map[string]string{myLabelName: myLabelValue}, + }, + } + + // --- When --- + ctr, err := testcontainers.GenericContainer(ctx, req) + + // --- Then --- + require.NoError(t, err) + t.Cleanup(func() { assert.NoError(t, ctr.Terminate(ctx)) }) + + ctrJSON, err := ctr.Inspect(ctx) + require.NoError(t, err) + assert.Equal(t, myLabelValue, ctrJSON.Config.Labels[myLabelName]) +} + +func TestCustomLabelsBuildOptionsModifier(t *testing.T) { + // --- Given --- + const ( + myLabelName = "org.my.label" + myLabelValue = "my-label-value" + myBuildOptionLabel = "org.my.bo.label" + myBuildOptionValue = "my-bo-label-value" + ) + + ctx := context.Background() + req := testcontainers.GenericContainerRequest{ + ContainerRequest: testcontainers.ContainerRequest{ + FromDockerfile: testcontainers.FromDockerfile{ + Context: "./testdata", + Dockerfile: "Dockerfile", + BuildOptionsModifier: func(opts *types.ImageBuildOptions) { + opts.Labels = map[string]string{ + myBuildOptionLabel: myBuildOptionValue, + } + }, + }, + Labels: map[string]string{myLabelName: myLabelValue}, + }, + } + + // --- When --- + ctr, err := testcontainers.GenericContainer(ctx, req) + + // --- Then --- + require.NoError(t, err) + t.Cleanup(func() { assert.NoError(t, ctr.Terminate(ctx)) }) + + ctrJSON, err := ctr.Inspect(ctx) + require.NoError(t, err) + assert.Equal(t, myLabelValue, ctrJSON.Config.Labels[myLabelName]) + assert.Equal(t, myBuildOptionValue, ctrJSON.Config.Labels[myBuildOptionLabel]) +} + func Test_GetLogsFromFailedContainer(t *testing.T) { ctx := context.Background() // directDockerHubReference { diff --git a/internal/core/labels.go b/internal/core/labels.go index 58b054ab95..8ed1c5066a 100644 --- a/internal/core/labels.go +++ b/internal/core/labels.go @@ -1,6 +1,9 @@ package core import ( + "fmt" + "strings" + "github.com/testcontainers/testcontainers-go/internal" ) @@ -21,3 +24,20 @@ func DefaultLabels(sessionID string) map[string]string { LabelVersion: internal.Version, } } + +// MergeCustomLabels sets labels from src to dst. Returns error if src label +// name has [LabelBase] prefix. +// +// NOTICE: Default labels must not be nil. +func MergeCustomLabels(dst, src map[string]string) error { + for key := range src { + if strings.HasPrefix(key, LabelBase) { + format := "cannot use prefix %q for custom labels" + return fmt.Errorf(format, LabelBase) + } + } + for key, value := range src { + dst[key] = value + } + return nil +} diff --git a/internal/core/labels_test.go b/internal/core/labels_test.go new file mode 100644 index 0000000000..359e662661 --- /dev/null +++ b/internal/core/labels_test.go @@ -0,0 +1,36 @@ +package core + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMergeCustomLabels(t *testing.T) { + t.Run("merge success", func(t *testing.T) { + // --- Given --- + dst := map[string]string{"A": "1", "B": "2"} + src := map[string]string{"B": "X", "C": "3"} + + // --- When --- + err := MergeCustomLabels(dst, src) + + // --- Then --- + assert.NoError(t, err) + assert.Equal(t, map[string]string{"A": "1", "B": "X", "C": "3"}, dst) + }) + + t.Run("src cannot have keys starting with LabelBase", func(t *testing.T) { + // --- Given --- + dst := map[string]string{"A": "1", "B": "2"} + src := map[string]string{"B": "X", LabelLang: "go"} + + // --- When --- + err := MergeCustomLabels(dst, src) + + // --- Then --- + want := `cannot use prefix "org.testcontainers" for custom labels` + assert.Error(t, err, want) + assert.Equal(t, map[string]string{"A": "1", "B": "2"}, dst) + }) +} From b653dc41c79da07c4adccbae0944d4eefd511653 Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Mon, 9 Sep 2024 19:37:19 +0200 Subject: [PATCH 02/22] Fix #2632 - fix linter errors. --- internal/core/labels_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/core/labels_test.go b/internal/core/labels_test.go index 359e662661..4e00f5d6e8 100644 --- a/internal/core/labels_test.go +++ b/internal/core/labels_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestMergeCustomLabels(t *testing.T) { @@ -16,7 +17,7 @@ func TestMergeCustomLabels(t *testing.T) { err := MergeCustomLabels(dst, src) // --- Then --- - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, map[string]string{"A": "1", "B": "X", "C": "3"}, dst) }) @@ -30,7 +31,7 @@ func TestMergeCustomLabels(t *testing.T) { // --- Then --- want := `cannot use prefix "org.testcontainers" for custom labels` - assert.Error(t, err, want) + require.Error(t, err, want) assert.Equal(t, map[string]string{"A": "1", "B": "2"}, dst) }) } From 1f7c06020ac006f9e1d6685b2503e9aba9547a71 Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Sun, 15 Sep 2024 00:13:11 +0200 Subject: [PATCH 03/22] Update internal/core/labels_test.go Co-authored-by: Steven Hartland --- internal/core/labels_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/core/labels_test.go b/internal/core/labels_test.go index 4e00f5d6e8..092d4cb62d 100644 --- a/internal/core/labels_test.go +++ b/internal/core/labels_test.go @@ -30,8 +30,7 @@ func TestMergeCustomLabels(t *testing.T) { err := MergeCustomLabels(dst, src) // --- Then --- - want := `cannot use prefix "org.testcontainers" for custom labels` - require.Error(t, err, want) + require.EqualError(t, err, `cannot use prefix "org.testcontainers" for custom labels`) assert.Equal(t, map[string]string{"A": "1", "B": "2"}, dst) }) } From 316bbfe3e56067298aa41feec17ea07f2f52fd6c Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Sun, 15 Sep 2024 00:13:19 +0200 Subject: [PATCH 04/22] Update internal/core/labels_test.go Co-authored-by: Steven Hartland --- internal/core/labels_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/core/labels_test.go b/internal/core/labels_test.go index 092d4cb62d..5d9ac7d607 100644 --- a/internal/core/labels_test.go +++ b/internal/core/labels_test.go @@ -18,7 +18,7 @@ func TestMergeCustomLabels(t *testing.T) { // --- Then --- require.NoError(t, err) - assert.Equal(t, map[string]string{"A": "1", "B": "X", "C": "3"}, dst) + require.Equal(t, map[string]string{"A": "1", "B": "X", "C": "3"}, dst) }) t.Run("src cannot have keys starting with LabelBase", func(t *testing.T) { From 3b8a6a07e82568c2907483a8bed2ac972a294d02 Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Sun, 15 Sep 2024 00:13:58 +0200 Subject: [PATCH 05/22] Update internal/core/labels_test.go Co-authored-by: Steven Hartland --- internal/core/labels_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/core/labels_test.go b/internal/core/labels_test.go index 5d9ac7d607..b4b8f81428 100644 --- a/internal/core/labels_test.go +++ b/internal/core/labels_test.go @@ -9,14 +9,10 @@ import ( func TestMergeCustomLabels(t *testing.T) { t.Run("merge success", func(t *testing.T) { - // --- Given --- dst := map[string]string{"A": "1", "B": "2"} src := map[string]string{"B": "X", "C": "3"} - // --- When --- err := MergeCustomLabels(dst, src) - - // --- Then --- require.NoError(t, err) require.Equal(t, map[string]string{"A": "1", "B": "X", "C": "3"}, dst) }) From 0481053cd2be7fe0628ff10e08b7fe89574b6e0f Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Sun, 15 Sep 2024 00:14:09 +0200 Subject: [PATCH 06/22] Update internal/core/labels_test.go Co-authored-by: Steven Hartland --- internal/core/labels_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/core/labels_test.go b/internal/core/labels_test.go index b4b8f81428..2cec306cad 100644 --- a/internal/core/labels_test.go +++ b/internal/core/labels_test.go @@ -8,7 +8,7 @@ import ( ) func TestMergeCustomLabels(t *testing.T) { - t.Run("merge success", func(t *testing.T) { + t.Run("success", func(t *testing.T) { dst := map[string]string{"A": "1", "B": "2"} src := map[string]string{"B": "X", "C": "3"} From a63823805ca07781937493ee1d1dd15589dc15fa Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Mon, 16 Sep 2024 16:58:27 +0200 Subject: [PATCH 07/22] Update container.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Manuel de la Peña --- container.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/container.go b/container.go index 3ec5155bfb..54d89c08e5 100644 --- a/container.go +++ b/container.go @@ -460,7 +460,7 @@ func (c *ContainerRequest) BuildOptions() (types.ImageBuildOptions, error) { } if !c.ShouldKeepBuiltImage() { - dst := core.DefaultLabels(core.SessionID()) + dst := GenericLabels() if err = core.MergeCustomLabels(dst, c.Labels); err != nil { return types.ImageBuildOptions{}, err } From 4c12f9c5ae905c0a8aa8350a2b986682a8afae5f Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Mon, 16 Sep 2024 16:58:58 +0200 Subject: [PATCH 08/22] Update internal/core/labels.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Manuel de la Peña --- internal/core/labels.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/core/labels.go b/internal/core/labels.go index 8ed1c5066a..041dccaaf1 100644 --- a/internal/core/labels.go +++ b/internal/core/labels.go @@ -28,7 +28,7 @@ func DefaultLabels(sessionID string) map[string]string { // MergeCustomLabels sets labels from src to dst. Returns error if src label // name has [LabelBase] prefix. // -// NOTICE: Default labels must not be nil. +// NOTICE: dst labels must not be nil. func MergeCustomLabels(dst, src map[string]string) error { for key := range src { if strings.HasPrefix(key, LabelBase) { From f9b95ef4dffa6b0ac750b2e6d230b870d24182a5 Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Mon, 16 Sep 2024 16:59:13 +0200 Subject: [PATCH 09/22] Update container_test.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Manuel de la Peña --- container_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/container_test.go b/container_test.go index 0d015c3cab..615e24359b 100644 --- a/container_test.go +++ b/container_test.go @@ -365,8 +365,8 @@ func TestCustomLabelsBuildOptionsModifier(t *testing.T) { ctrJSON, err := ctr.Inspect(ctx) require.NoError(t, err) - assert.Equal(t, myLabelValue, ctrJSON.Config.Labels[myLabelName]) - assert.Equal(t, myBuildOptionValue, ctrJSON.Config.Labels[myBuildOptionLabel]) + require.Equal(t, myLabelValue, ctrJSON.Config.Labels[myLabelName]) + require.Equal(t, myBuildOptionValue, ctrJSON.Config.Labels[myBuildOptionLabel]) } func Test_GetLogsFromFailedContainer(t *testing.T) { From 19474d4f155684b7b8415521542ee3402dd6158a Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Mon, 16 Sep 2024 17:04:35 +0200 Subject: [PATCH 10/22] Fix #2632 - remove given, when, then comments. --- container_test.go | 6 ------ internal/core/labels_test.go | 3 --- 2 files changed, 9 deletions(-) diff --git a/container_test.go b/container_test.go index 615e24359b..26848bd67f 100644 --- a/container_test.go +++ b/container_test.go @@ -305,7 +305,6 @@ func Test_BuildImageWithContexts(t *testing.T) { } func TestCustomLabelsImage(t *testing.T) { - // --- Given --- const ( myLabelName = "org.my.label" myLabelValue = "my-label-value" @@ -319,10 +318,8 @@ func TestCustomLabelsImage(t *testing.T) { }, } - // --- When --- ctr, err := testcontainers.GenericContainer(ctx, req) - // --- Then --- require.NoError(t, err) t.Cleanup(func() { assert.NoError(t, ctr.Terminate(ctx)) }) @@ -332,7 +329,6 @@ func TestCustomLabelsImage(t *testing.T) { } func TestCustomLabelsBuildOptionsModifier(t *testing.T) { - // --- Given --- const ( myLabelName = "org.my.label" myLabelValue = "my-label-value" @@ -356,10 +352,8 @@ func TestCustomLabelsBuildOptionsModifier(t *testing.T) { }, } - // --- When --- ctr, err := testcontainers.GenericContainer(ctx, req) - // --- Then --- require.NoError(t, err) t.Cleanup(func() { assert.NoError(t, ctr.Terminate(ctx)) }) diff --git a/internal/core/labels_test.go b/internal/core/labels_test.go index 2cec306cad..15b84aeed4 100644 --- a/internal/core/labels_test.go +++ b/internal/core/labels_test.go @@ -18,14 +18,11 @@ func TestMergeCustomLabels(t *testing.T) { }) t.Run("src cannot have keys starting with LabelBase", func(t *testing.T) { - // --- Given --- dst := map[string]string{"A": "1", "B": "2"} src := map[string]string{"B": "X", LabelLang: "go"} - // --- When --- err := MergeCustomLabels(dst, src) - // --- Then --- require.EqualError(t, err, `cannot use prefix "org.testcontainers" for custom labels`) assert.Equal(t, map[string]string{"A": "1", "B": "2"}, dst) }) From e51843b705ff4832018d90fab8b97ef2dcd18125 Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Mon, 16 Sep 2024 17:06:23 +0200 Subject: [PATCH 11/22] Update internal/core/labels.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Manuel de la Peña --- internal/core/labels.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/core/labels.go b/internal/core/labels.go index 041dccaaf1..79bd1985b8 100644 --- a/internal/core/labels.go +++ b/internal/core/labels.go @@ -30,13 +30,12 @@ func DefaultLabels(sessionID string) map[string]string { // // NOTICE: dst labels must not be nil. func MergeCustomLabels(dst, src map[string]string) error { - for key := range src { + for key, value := range src { if strings.HasPrefix(key, LabelBase) { format := "cannot use prefix %q for custom labels" return fmt.Errorf(format, LabelBase) } - } - for key, value := range src { + dst[key] = value } return nil From 0f4c7ad0f15490a1c4a08d850a66a4ab89d745b2 Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Mon, 16 Sep 2024 17:08:08 +0200 Subject: [PATCH 12/22] Fix #2632 - unit test update. --- internal/core/labels_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/core/labels_test.go b/internal/core/labels_test.go index 15b84aeed4..c2e78c606b 100644 --- a/internal/core/labels_test.go +++ b/internal/core/labels_test.go @@ -24,6 +24,6 @@ func TestMergeCustomLabels(t *testing.T) { err := MergeCustomLabels(dst, src) require.EqualError(t, err, `cannot use prefix "org.testcontainers" for custom labels`) - assert.Equal(t, map[string]string{"A": "1", "B": "2"}, dst) + assert.Equal(t, map[string]string{"A": "1", "B": "X"}, dst) }) } From 6dd21d2c3859c7abbd84d27b98fc87eb5d05f769 Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Tue, 17 Sep 2024 10:40:59 +0200 Subject: [PATCH 13/22] Update internal/core/labels_test.go Co-authored-by: Steven Hartland --- internal/core/labels_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/core/labels_test.go b/internal/core/labels_test.go index c2e78c606b..2038366dc4 100644 --- a/internal/core/labels_test.go +++ b/internal/core/labels_test.go @@ -24,6 +24,6 @@ func TestMergeCustomLabels(t *testing.T) { err := MergeCustomLabels(dst, src) require.EqualError(t, err, `cannot use prefix "org.testcontainers" for custom labels`) - assert.Equal(t, map[string]string{"A": "1", "B": "X"}, dst) + require.Equal(t, map[string]string{"A": "1", "B": "X"}, dst) }) } From 560bc3891d174cde4dbb14ab40f93c9f70829f9a Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Tue, 17 Sep 2024 11:03:37 +0200 Subject: [PATCH 14/22] Fix #2632 - return error when destination labels map is nil and custom labels are present. --- internal/core/labels.go | 7 +++++-- internal/core/labels_test.go | 17 ++++++++++++++++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/internal/core/labels.go b/internal/core/labels.go index 79bd1985b8..d265441458 100644 --- a/internal/core/labels.go +++ b/internal/core/labels.go @@ -1,6 +1,7 @@ package core import ( + "errors" "fmt" "strings" @@ -28,14 +29,16 @@ func DefaultLabels(sessionID string) map[string]string { // MergeCustomLabels sets labels from src to dst. Returns error if src label // name has [LabelBase] prefix. // -// NOTICE: dst labels must not be nil. +// NOTICE: The dst labels must not be nil so we can set the labels. func MergeCustomLabels(dst, src map[string]string) error { + if dst == nil && len(src) > 0 { + return errors.New("cannot merge custom labels because destination map is nil") + } for key, value := range src { if strings.HasPrefix(key, LabelBase) { format := "cannot use prefix %q for custom labels" return fmt.Errorf(format, LabelBase) } - dst[key] = value } return nil diff --git a/internal/core/labels_test.go b/internal/core/labels_test.go index 2038366dc4..d211140918 100644 --- a/internal/core/labels_test.go +++ b/internal/core/labels_test.go @@ -3,7 +3,6 @@ package core import ( "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -26,4 +25,20 @@ func TestMergeCustomLabels(t *testing.T) { require.EqualError(t, err, `cannot use prefix "org.testcontainers" for custom labels`) require.Equal(t, map[string]string{"A": "1", "B": "X"}, dst) }) + + t.Run("nil destination and empty source", func(t *testing.T) { + src := map[string]string{} + + err := MergeCustomLabels(nil, src) + + require.NoError(t, err) + }) + + t.Run("nil destination", func(t *testing.T) { + src := map[string]string{"A": "1"} + + err := MergeCustomLabels(nil, src) + + require.Error(t, err) + }) } From a5ecd0d3cb96bd698ac05b3b9f626e62a2d7674f Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Tue, 17 Sep 2024 21:00:43 +0200 Subject: [PATCH 15/22] Update internal/core/labels_test.go Co-authored-by: Steven Hartland --- internal/core/labels_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/core/labels_test.go b/internal/core/labels_test.go index d211140918..2edc6a908d 100644 --- a/internal/core/labels_test.go +++ b/internal/core/labels_test.go @@ -34,11 +34,9 @@ func TestMergeCustomLabels(t *testing.T) { require.NoError(t, err) }) - t.Run("nil destination", func(t *testing.T) { + t.Run("nil-destination", func(t *testing.T) { src := map[string]string{"A": "1"} - err := MergeCustomLabels(nil, src) - require.Error(t, err) }) } From 11b9551da16deff2f1d885ab03dcd200d9b4be49 Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Tue, 17 Sep 2024 21:01:11 +0200 Subject: [PATCH 16/22] Update internal/core/labels.go Co-authored-by: Steven Hartland --- internal/core/labels.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/core/labels.go b/internal/core/labels.go index d265441458..d9e39f6a00 100644 --- a/internal/core/labels.go +++ b/internal/core/labels.go @@ -31,8 +31,8 @@ func DefaultLabels(sessionID string) map[string]string { // // NOTICE: The dst labels must not be nil so we can set the labels. func MergeCustomLabels(dst, src map[string]string) error { - if dst == nil && len(src) > 0 { - return errors.New("cannot merge custom labels because destination map is nil") + if dst == nil { + return errors.New("destination map is nil") } for key, value := range src { if strings.HasPrefix(key, LabelBase) { From b1aa4233a7ecf5f8d8e62f8849ddeb9fb1319bde Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Tue, 17 Sep 2024 21:01:31 +0200 Subject: [PATCH 17/22] Update internal/core/labels.go Co-authored-by: Steven Hartland --- internal/core/labels.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/core/labels.go b/internal/core/labels.go index d9e39f6a00..e8e68bd75f 100644 --- a/internal/core/labels.go +++ b/internal/core/labels.go @@ -26,10 +26,9 @@ func DefaultLabels(sessionID string) map[string]string { } } -// MergeCustomLabels sets labels from src to dst. Returns error if src label -// name has [LabelBase] prefix. -// -// NOTICE: The dst labels must not be nil so we can set the labels. +// MergeCustomLabels sets labels from src to dst. +// If a key in src has [LabelBase] prefix returns an error. +// If dst is nil returns an error. func MergeCustomLabels(dst, src map[string]string) error { if dst == nil { return errors.New("destination map is nil") From a2f93939ed756afe6204e81bb57b2297c1235cc4 Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Tue, 17 Sep 2024 21:02:01 +0200 Subject: [PATCH 18/22] Update internal/core/labels.go Co-authored-by: Steven Hartland --- internal/core/labels.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/core/labels.go b/internal/core/labels.go index e8e68bd75f..b5da2fb29d 100644 --- a/internal/core/labels.go +++ b/internal/core/labels.go @@ -35,8 +35,7 @@ func MergeCustomLabels(dst, src map[string]string) error { } for key, value := range src { if strings.HasPrefix(key, LabelBase) { - format := "cannot use prefix %q for custom labels" - return fmt.Errorf(format, LabelBase) + return fmt.Errorf("key %q has %q prefix", key, LabelBase) } dst[key] = value } From e126d915e96959376423e4abeadbb676a794143e Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Tue, 17 Sep 2024 21:03:16 +0200 Subject: [PATCH 19/22] Update internal/core/labels_test.go Co-authored-by: Steven Hartland --- internal/core/labels_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/core/labels_test.go b/internal/core/labels_test.go index 2edc6a908d..c1319a2773 100644 --- a/internal/core/labels_test.go +++ b/internal/core/labels_test.go @@ -16,7 +16,7 @@ func TestMergeCustomLabels(t *testing.T) { require.Equal(t, map[string]string{"A": "1", "B": "X", "C": "3"}, dst) }) - t.Run("src cannot have keys starting with LabelBase", func(t *testing.T) { + t.Run("invalid-prefix", func(t *testing.T) { dst := map[string]string{"A": "1", "B": "2"} src := map[string]string{"B": "X", LabelLang: "go"} From f1f928664c31981877e4789ab9460fa569e1c26e Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Tue, 17 Sep 2024 21:03:45 +0200 Subject: [PATCH 20/22] Update internal/core/labels_test.go Co-authored-by: Steven Hartland --- internal/core/labels_test.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/internal/core/labels_test.go b/internal/core/labels_test.go index c1319a2773..a71f2b8928 100644 --- a/internal/core/labels_test.go +++ b/internal/core/labels_test.go @@ -26,14 +26,6 @@ func TestMergeCustomLabels(t *testing.T) { require.Equal(t, map[string]string{"A": "1", "B": "X"}, dst) }) - t.Run("nil destination and empty source", func(t *testing.T) { - src := map[string]string{} - - err := MergeCustomLabels(nil, src) - - require.NoError(t, err) - }) - t.Run("nil-destination", func(t *testing.T) { src := map[string]string{"A": "1"} err := MergeCustomLabels(nil, src) From 0caf24d01bbb941f2ad8f56e244d37d1fbf624c5 Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Tue, 17 Sep 2024 21:23:04 +0200 Subject: [PATCH 21/22] Fix #2632 - fix test. --- internal/core/labels_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/core/labels_test.go b/internal/core/labels_test.go index a71f2b8928..e382a0ad48 100644 --- a/internal/core/labels_test.go +++ b/internal/core/labels_test.go @@ -22,7 +22,7 @@ func TestMergeCustomLabels(t *testing.T) { err := MergeCustomLabels(dst, src) - require.EqualError(t, err, `cannot use prefix "org.testcontainers" for custom labels`) + require.EqualError(t, err, `key "org.testcontainers.lang" has "org.testcontainers" prefix`) require.Equal(t, map[string]string{"A": "1", "B": "X"}, dst) }) From ec0d18b62263ffc7168d61fc3a0bfbfbf142cb8e Mon Sep 17 00:00:00 2001 From: Rafal Zajac Date: Wed, 18 Sep 2024 13:41:27 +0200 Subject: [PATCH 22/22] Update container_test.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Manuel de la Peña --- container_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/container_test.go b/container_test.go index 26848bd67f..40dc7e414d 100644 --- a/container_test.go +++ b/container_test.go @@ -353,9 +353,8 @@ func TestCustomLabelsBuildOptionsModifier(t *testing.T) { } ctr, err := testcontainers.GenericContainer(ctx, req) - + testcontainers.CleanupContainer(t, ctr) require.NoError(t, err) - t.Cleanup(func() { assert.NoError(t, ctr.Terminate(ctx)) }) ctrJSON, err := ctr.Inspect(ctx) require.NoError(t, err)