fix: do not override ImageBuildOptions.Labels when building from a Do…

…ckerfile (#2775) * Fix #2632 - ImageBuildOptions.Labels are overwritten * Fix #2632 - fix linter errors. * Update internal/core/labels_test.go Co-authored-by: Steven Hartland <[email protected]> * Update internal/core/labels_test.go Co-authored-by: Steven Hartland <[email protected]> * Update internal/core/labels_test.go Co-authored-by: Steven Hartland <[email protected]> * Update internal/core/labels_test.go Co-authored-by: Steven Hartland <[email protected]> * Update container.go Co-authored-by: Manuel de la Peña <[email protected]> * Update internal/core/labels.go Co-authored-by: Manuel de la Peña <[email protected]> * Update container_test.go Co-authored-by: Manuel de la Peña <[email protected]> * Fix #2632 - remove given, when, then comments. * Update internal/core/labels.go Co-authored-by: Manuel de la Peña <[email protected]> * Fix #2632 - unit test update. * Update internal/core/labels_test.go Co-authored-by: Steven Hartland <[email protected]> * Fix #2632 - return error when destination labels map is nil and custom labels are present. * Update internal/core/labels_test.go Co-authored-by: Steven Hartland <[email protected]> * Update internal/core/labels.go Co-authored-by: Steven Hartland <[email protected]> * Update internal/core/labels.go Co-authored-by: Steven Hartland <[email protected]> * Update internal/core/labels.go Co-authored-by: Steven Hartland <[email protected]> * Update internal/core/labels_test.go Co-authored-by: Steven Hartland <[email protected]> * Update internal/core/labels_test.go Co-authored-by: Steven Hartland <[email protected]> * Fix #2632 - fix test. * Update container_test.go Co-authored-by: Manuel de la Peña <[email protected]> --------- Co-authored-by: Steven Hartland <[email protected]> Co-authored-by: Manuel de la Peña <[email protected]>
testcontainers · Sep 18, 2024 · 31a033c · 31a033c
1 parent e2bd70f
commit 31a033c
Show file tree

Hide file tree

Showing 4 changed files with 121 additions and 1 deletion.
diff --git 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 := GenericLabels()
+		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
@@ -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"
@@ -302,6 +303,64 @@ func Test_BuildImageWithContexts(t *testing.T) {
 	}
 }
 
+func TestCustomLabelsImage(t *testing.T) {
+	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},
+		},
+	}
+
+	ctr, err := testcontainers.GenericContainer(ctx, req)
+
+	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) {
+	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},
+		},
+	}
+
+	ctr, err := testcontainers.GenericContainer(ctx, req)
+	testcontainers.CleanupContainer(t, ctr)
+	require.NoError(t, err)
+
+	ctrJSON, err := ctr.Inspect(ctx)
+	require.NoError(t, err)
+	require.Equal(t, myLabelValue, ctrJSON.Config.Labels[myLabelName])
+	require.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
@@ -1,6 +1,10 @@
 package core
 
 import (
+	"errors"
+	"fmt"
+	"strings"
+
 	"github.com/testcontainers/testcontainers-go/internal"
 )
 
@@ -21,3 +25,19 @@ func DefaultLabels(sessionID string) map[string]string {
 		LabelVersion:   internal.Version,
 	}
 }
+
+// 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")
+	}
+	for key, value := range src {
+		if strings.HasPrefix(key, LabelBase) {
+			return fmt.Errorf("key %q has %q prefix", key, LabelBase)
+		}
+		dst[key] = value
+	}
+	return nil
+}
diff --git a/internal/core/labels_test.go b/internal/core/labels_test.go
@@ -0,0 +1,34 @@
+package core
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestMergeCustomLabels(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"}
+
+		err := MergeCustomLabels(dst, src)
+		require.NoError(t, err)
+		require.Equal(t, map[string]string{"A": "1", "B": "X", "C": "3"}, dst)
+	})
+
+	t.Run("invalid-prefix", func(t *testing.T) {
+		dst := map[string]string{"A": "1", "B": "2"}
+		src := map[string]string{"B": "X", LabelLang: "go"}
+
+		err := MergeCustomLabels(dst, src)
+
+		require.EqualError(t, err, `key "org.testcontainers.lang" has "org.testcontainers" prefix`)
+		require.Equal(t, map[string]string{"A": "1", "B": "X"}, dst)
+	})
+
+	t.Run("nil-destination", func(t *testing.T) {
+		src := map[string]string{"A": "1"}
+		err := MergeCustomLabels(nil, src)
+		require.Error(t, err)
+	})
+}