Skip to content

Commit d55ec69

Browse files
authored
chore: consistency across modules (#76)
* chore(client): do not expose NewClientOption * chore: automatically add the SDK labels in the Create operations * fix(client): do not allow altering the SDK labels * chore(network): rename for consistency Prefer FindBy over GetBy * fix: handle nil maps when adding SDK labels
1 parent 990161d commit d55ec69

24 files changed

+133
-48
lines changed

client/client.container.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ func (c *Client) ContainerCreate(ctx context.Context, config *container.Config,
2121
return container.CreateResponse{}, fmt.Errorf("docker client: %w", err)
2222
}
2323

24+
// Add the labels that identify this as a container created by the SDK.
25+
AddSDKLabels(config.Labels)
26+
2427
return dockerClient.ContainerCreate(ctx, config, hostConfig, networkingConfig, platform, name)
2528
}
2629

client/client.image.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ func (c *Client) ImageBuild(ctx context.Context, options build.ImageBuildOptions
2222
return build.ImageBuildResponse{}, errors.New("build context is nil")
2323
}
2424

25+
// Add client labels
26+
AddSDKLabels(options.Labels)
27+
2528
return dockerClient.ImageBuild(ctx, options.Context, options)
2629
}
2730

client/client.network.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ func (c *Client) NetworkCreate(ctx context.Context, name string, options network
2424
return network.CreateResponse{}, fmt.Errorf("docker client: %w", err)
2525
}
2626

27+
// Add the labels that identify this as a network created by the SDK.
28+
AddSDKLabels(options.Labels)
29+
2730
return dockerClient.NetworkCreate(ctx, name, options)
2831
}
2932

client/client.volume.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ func (c *Client) VolumeCreate(ctx context.Context, options volume.CreateOptions)
1414
return volume.Volume{}, fmt.Errorf("docker client: %w", err)
1515
}
1616

17+
// Add the labels that identify this as a volume created by the SDK.
18+
AddSDKLabels(options.Labels)
19+
1720
return dockerClient.VolumeCreate(ctx, options)
1821
}
1922

client/client_test.go

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package client_test
22

33
import (
44
"context"
5-
"errors"
65
"path/filepath"
76
"sync"
87
"testing"
@@ -88,19 +87,6 @@ func TestNew(t *testing.T) {
8887
require.Nil(t, cli)
8988
})
9089

91-
t.Run("error/apply-option", func(t *testing.T) {
92-
// custom option that always fails to apply
93-
customOpt := func() client.ClientOption {
94-
return client.NewClientOption(func(_ *client.Client) error {
95-
return errors.New("apply option")
96-
})
97-
}
98-
99-
cli, err := client.New(context.Background(), customOpt())
100-
require.ErrorContains(t, err, "apply option")
101-
require.Nil(t, cli)
102-
})
103-
10490
t.Run("healthcheck/nil", func(t *testing.T) {
10591
cli, err := client.New(context.Background(), client.WithHealthCheck(nil))
10692
require.ErrorContains(t, err, "health check is nil")

client/client_unit_test.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,4 +150,17 @@ func TestNew_internal_state(t *testing.T) {
150150
require.ErrorContains(t, err, "docker host from context")
151151
require.Nil(t, client)
152152
})
153+
154+
t.Run("error/apply-option", func(t *testing.T) {
155+
// custom option that always fails to apply
156+
customOpt := func() ClientOption {
157+
return newClientOption(func(_ *Client) error {
158+
return errors.New("apply option")
159+
})
160+
}
161+
162+
cli, err := New(context.Background(), customOpt())
163+
require.ErrorContains(t, err, "apply option")
164+
require.Nil(t, cli)
165+
})
153166
}

client/labels.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,28 @@ const (
1313
LabelVersion = LabelBase + ".version"
1414
)
1515

16-
// SDKLabels returns a map of labels that can be used to identify resources
16+
// sdkLabels is a map of labels that can be used to identify resources
1717
// created by this library.
18-
var SDKLabels = map[string]string{
18+
var sdkLabels = map[string]string{
1919
LabelBase: "true",
2020
LabelLang: "go",
2121
LabelVersion: Version(),
2222
}
2323

2424
// AddSDKLabels adds the SDK labels to target.
2525
func AddSDKLabels(target map[string]string) {
26-
maps.Copy(target, SDKLabels)
26+
if target == nil {
27+
target = make(map[string]string)
28+
}
29+
maps.Copy(target, sdkLabels)
30+
}
31+
32+
// SDKLabels returns a map of labels that can be used to identify resources
33+
// created by this library.
34+
func SDKLabels() map[string]string {
35+
return map[string]string{
36+
LabelBase: "true",
37+
LabelLang: "go",
38+
LabelVersion: Version(),
39+
}
2740
}

client/labels_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,14 @@ func TestAddSDKLabels(t *testing.T) {
1515
require.Contains(t, labels, client.LabelBase)
1616
require.Contains(t, labels, client.LabelLang)
1717
require.Contains(t, labels, client.LabelVersion)
18+
19+
t.Run("idempotent", func(t *testing.T) {
20+
sdkLabels := client.SDKLabels()
21+
sdkLabels["foo"] = "bar"
22+
23+
labels := make(map[string]string)
24+
client.AddSDKLabels(labels)
25+
require.NotEqual(t, sdkLabels, labels)
26+
require.NotContains(t, labels, "foo")
27+
})
1828
}

client/options.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,14 @@ func (f funcOpt) Apply(c *Client) error {
4040
return f(c)
4141
}
4242

43-
// NewClientOption creates a new ClientOption from a function
44-
func NewClientOption(f func(*Client) error) ClientOption {
43+
// newClientOption creates a new ClientOption from a function
44+
func newClientOption(f func(*Client) error) ClientOption {
4545
return funcOpt(f)
4646
}
4747

4848
// WithDockerHost returns a client option that sets the docker host for the client.
4949
func WithDockerHost(dockerHost string) ClientOption {
50-
return NewClientOption(func(c *Client) error {
50+
return newClientOption(func(c *Client) error {
5151
c.dockerHost = dockerHost
5252
return nil
5353
})
@@ -57,15 +57,15 @@ func WithDockerHost(dockerHost string) ClientOption {
5757
// If set, the client will use the docker context to determine the docker host.
5858
// If used in combination with [WithDockerHost], the host in the context will take precedence.
5959
func WithDockerContext(dockerContext string) ClientOption {
60-
return NewClientOption(func(c *Client) error {
60+
return newClientOption(func(c *Client) error {
6161
c.dockerContext = dockerContext
6262
return nil
6363
})
6464
}
6565

6666
// WithExtraHeaders returns a client option that sets the extra headers for the client.
6767
func WithExtraHeaders(headers map[string]string) ClientOption {
68-
return NewClientOption(func(c *Client) error {
68+
return newClientOption(func(c *Client) error {
6969
c.extraHeaders = headers
7070
return nil
7171
})
@@ -75,7 +75,7 @@ func WithExtraHeaders(headers map[string]string) ClientOption {
7575
// If not set, the default health check will be used, which retries the ping to the
7676
// docker daemon until it is ready, three times, or the context is done.
7777
func WithHealthCheck(healthCheck func(ctx context.Context) func(c *Client) error) ClientOption {
78-
return NewClientOption(func(c *Client) error {
78+
return newClientOption(func(c *Client) error {
7979
if healthCheck == nil {
8080
return errors.New("health check is nil")
8181
}
@@ -87,7 +87,7 @@ func WithHealthCheck(healthCheck func(ctx context.Context) func(c *Client) error
8787

8888
// WithLogger returns a client option that sets the logger for the client.
8989
func WithLogger(log *slog.Logger) ClientOption {
90-
return NewClientOption(func(c *Client) error {
90+
return newClientOption(func(c *Client) error {
9191
c.log = log
9292
return nil
9393
})

container/container.run.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,14 +99,13 @@ func Run(ctx context.Context, opts ...ContainerCustomizer) (*Container, error) {
9999
}
100100
}
101101

102-
// Add the labels that identify this as a container created by the SDK.
103-
client.AddSDKLabels(def.labels)
102+
def.labels[moduleLabel] = Version()
104103

105104
dockerInput := &container.Config{
106105
Entrypoint: def.entrypoint,
107106
Image: def.image,
108107
Env: env,
109-
Labels: def.labels,
108+
Labels: def.labels, // the Client will add the SDK labels automatically
110109
Cmd: def.cmd,
111110
}
112111

0 commit comments

Comments
 (0)