Skip to content

Commit 5b9d046

Browse files
committed
config create: refactor, use limit reader, and touch up errors
Swarm has size constraints on the size of configs, but the client-side would read content into memory, regardless its size. This could lead to either the client reading too much into memory, or it sending data that's larger than the size limit of gRPC, which resulted in the error not being handled by SwarmKit and a generic gRPC error returned. Reading a config from a file used a system.OpenSequential for reading ([FILE_FLAG_SEQUENTIAL_SCAN]). While there could be a very marginal benefit to prevent polluting the system's cache (Windows won’t aggressively keep it in the cache, freeing up system memory for other tasks). These details were not documented in code, and possibly may be too marginal, but adding a comment to outline won't hurt so this patch also adds a comment. This patch: - Factors out the reading code to a readConfigData, analogous to the equivalent in secret create. - Implements reading the data with a limit-reader to prevent reading large files into memory. - The limit is based on SwarmKits limits ([MaxConfigSize]), but made twice that size, just in case larger sizes are supported in future; the main goal is to have some constraints, and to prevent hitting the gRPC limit. - Updates some error messages to include STDIN (when used), or the filename (when used). Before this patch: ls -lh largefile -rw------- 1 thajeztah staff 8.1M Mar 9 00:19 largefile docker config create nosuchfile ./nosuchfile Error reading content from "./nosuchfile": open ./nosuchfile: no such file or directory docker config create toolarge ./largefile Error response from daemon: rpc error: code = ResourceExhausted desc = grpc: received message larger than max (8462870 vs. 4194304) docker config create empty ./emptyfile Error response from daemon: rpc error: code = InvalidArgument desc = config data must be larger than 0 and less than 1024000 bytes cat ./largefile | docker config create toolarge - Error response from daemon: rpc error: code = ResourceExhausted desc = grpc: received message larger than max (8462870 vs. 4194304) cat ./emptyfile | docker config create empty - Error response from daemon: rpc error: code = InvalidArgument desc = config data must be larger than 0 and less than 1024000 bytes With this patch: docker config create nosuchfile ./nosuchfile error reading from ./nosuchfile: open ./nosuchfile: no such file or directory docker config create empty ./emptyfile error reading from ./emptyfile: data is empty docker config create toolarge ./largefile Error response from daemon: rpc error: code = InvalidArgument desc = config data must be larger than 0 and less than 1024000 bytes cat ./largefile | docker config create toolarge - Error response from daemon: rpc error: code = InvalidArgument desc = secret data must be larger than 0 and less than 1024000 bytes cat ./emptyfile | docker config create empty - error reading from STDIN: data is empty [FILE_FLAG_SEQUENTIAL_SCAN]: https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea#FILE_FLAG_SEQUENTIAL_SCAN [MaxConfigSize]: https://pkg.go.dev/github.com/moby/swarmkit/v2@v2.0.0-20250103191802-8c1959736554/manager/controlapi#MaxConfigSize Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
1 parent 311480c commit 5b9d046

File tree

2 files changed

+50
-15
lines changed

2 files changed

+50
-15
lines changed

cli/command/config/create.go

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -51,17 +51,7 @@ func newConfigCreateCommand(dockerCli command.Cli) *cobra.Command {
5151
func RunConfigCreate(ctx context.Context, dockerCLI command.Cli, options CreateOptions) error {
5252
apiClient := dockerCLI.Client()
5353

54-
var in io.Reader = dockerCLI.In()
55-
if options.File != "-" {
56-
file, err := sequential.Open(options.File)
57-
if err != nil {
58-
return err
59-
}
60-
in = file
61-
defer file.Close()
62-
}
63-
64-
configData, err := io.ReadAll(in)
54+
configData, err := readConfigData(dockerCLI.In(), options.File)
6555
if err != nil {
6656
return errors.Errorf("Error reading content from %q: %v", options.File, err)
6757
}
@@ -83,6 +73,51 @@ func RunConfigCreate(ctx context.Context, dockerCLI command.Cli, options CreateO
8373
return err
8474
}
8575

86-
fmt.Fprintln(dockerCLI.Out(), r.ID)
76+
_, _ = fmt.Fprintln(dockerCLI.Out(), r.ID)
8777
return nil
8878
}
79+
80+
// maxConfigSize is the maximum byte length of the [swarm.ConfigSpec.Data] field,
81+
// as defined by [MaxConfigSize] in SwarmKit.
82+
//
83+
// [MaxConfigSize]: https://pkg.go.dev/github.com/moby/swarmkit/v2@v2.0.0-20250103191802-8c1959736554/manager/controlapi#MaxConfigSize
84+
const maxConfigSize = 1000 * 1024 // 1000KB
85+
86+
// readConfigData reads the config from either stdin or the given fileName.
87+
//
88+
// It reads up to twice the maximum size of the config ([maxConfigSize]),
89+
// just in case swarm's limit changes; this is only a safeguard to prevent
90+
// reading arbitrary files into memory.
91+
func readConfigData(in io.Reader, fileName string) (data []byte, retErr error) {
92+
if fileName != "" {
93+
defer func() {
94+
if retErr != nil {
95+
retErr = fmt.Errorf("error reading from %s: %w", fileName, retErr)
96+
} else if len(data) == 0 {
97+
retErr = fmt.Errorf("error reading from %s: data is empty", fileName)
98+
}
99+
}()
100+
}
101+
102+
switch fileName {
103+
case "-":
104+
fileName = "STDIN"
105+
return io.ReadAll(io.LimitReader(in, 2*maxConfigSize))
106+
case "":
107+
return nil, errors.New("config file is required")
108+
default:
109+
// Open file with [FILE_FLAG_SEQUENTIAL_SCAN] on Windows, which
110+
// prevents Windows from aggressively caching it. We expect this
111+
// file to be only read once. Given that this is expected to be
112+
// a small file, this may not be a significant optimization, so
113+
// we could choose to omit this, and use a regular [os.Open].
114+
//
115+
// [FILE_FLAG_SEQUENTIAL_SCAN]: https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea#FILE_FLAG_SEQUENTIAL_SCAN
116+
f, err := sequential.Open(fileName)
117+
if err != nil {
118+
return nil, err
119+
}
120+
defer f.Close()
121+
return io.ReadAll(io.LimitReader(f, 2*maxConfigSize))
122+
}
123+
}

cli/command/config/create_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func TestConfigCreateErrors(t *testing.T) {
5959
}
6060

6161
func TestConfigCreateWithName(t *testing.T) {
62-
name := "foo"
62+
const name = "config-with-name"
6363
var actual []byte
6464
cli := test.NewFakeCli(&fakeClient{
6565
configCreateFunc: func(_ context.Context, spec swarm.ConfigSpec) (types.ConfigCreateResponse, error) {
@@ -87,7 +87,7 @@ func TestConfigCreateWithLabels(t *testing.T) {
8787
"lbl1": "Label-foo",
8888
"lbl2": "Label-bar",
8989
}
90-
name := "foo"
90+
const name = "config-with-labels"
9191

9292
data, err := os.ReadFile(filepath.Join("testdata", configDataFile))
9393
assert.NilError(t, err)
@@ -124,7 +124,7 @@ func TestConfigCreateWithTemplatingDriver(t *testing.T) {
124124
expectedDriver := &swarm.Driver{
125125
Name: "template-driver",
126126
}
127-
name := "foo"
127+
const name = "config-with-template-driver"
128128

129129
cli := test.NewFakeCli(&fakeClient{
130130
configCreateFunc: func(_ context.Context, spec swarm.ConfigSpec) (types.ConfigCreateResponse, error) {

0 commit comments

Comments
 (0)