1berry/docker-cli
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

config create: refactor, use limit reader, and touch up errors

Browse Source 
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>
This commit is contained in:
Sebastiaan van Stijn 2025-03-09 14:31:56 +01:00
parent 3a35b16669
commit d6d8ca6ebe
No known key found for this signature in database
GPG Key ID: 76698F39D527CE8C
2 changed files with 53 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

62
cli/command/config/create.go
View File

@ -51,17 +51,7 @@ func newConfigCreateCommand(dockerCli command.Cli) *cobra.Command {
func RunConfigCreate(ctx context.Context, dockerCLI command.Cli, options CreateOptions) error {
apiClient := dockerCLI.Client()
var in io.Reader = dockerCLI.In()
if options.File != "-" {
file, err := sequential.Open(options.File)
if err != nil {
return err
}
in = file
defer file.Close()
}
configData, err := io.ReadAll(in)
configData, err := readConfigData(dockerCLI.In(), options.File)
if err != nil {
return errors.Errorf("Error reading content from %q: %v", options.File, err)
}
@ -83,6 +73,54 @@ func RunConfigCreate(ctx context.Context, dockerCLI command.Cli, options CreateO
return err
}
fmt.Fprintln(dockerCLI.Out(), r.ID)
_, _ = fmt.Fprintln(dockerCLI.Out(), r.ID)
return nil
}
// maxConfigSize is the maximum byte length of the [swarm.ConfigSpec.Data] field,
// as defined by [MaxConfigSize] in SwarmKit.
//
// [MaxConfigSize]: https://pkg.go.dev/github.com/moby/swarmkit/v2@v2.0.0-20250103191802-8c1959736554/manager/controlapi#MaxConfigSize
const maxConfigSize = 1000 * 1024 // 1000KB
// readConfigData reads the config from either stdin or the given fileName.
//
// It reads up to twice the maximum size of the config ([maxConfigSize]),
// just in case swarm's limit changes; this is only a safeguard to prevent
// reading arbitrary files into memory.
func readConfigData(in io.Reader, fileName string) ([]byte, error) {
switch fileName {
case "-":
data, err := io.ReadAll(io.LimitReader(in, 2*maxConfigSize))
if err != nil {
return nil, fmt.Errorf("error reading from STDIN: %w", err)
}
if len(data) == 0 {
return nil, errors.New("error reading from STDIN: data is empty")
}
return data, nil
case "":
return nil, errors.New("config file is required")
default:
// Open file with [FILE_FLAG_SEQUENTIAL_SCAN] on Windows, which
// prevents Windows from aggressively caching it. We expect this
// file to be only read once. Given that this is expected to be
// a small file, this may not be a significant optimization, so
// we could choose to omit this, and use a regular [os.Open].
//
// [FILE_FLAG_SEQUENTIAL_SCAN]: https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea#FILE_FLAG_SEQUENTIAL_SCAN
f, err := sequential.Open(fileName)
if err != nil {
return nil, fmt.Errorf("error reading from %s: %w", fileName, err)
}
defer f.Close()
data, err := io.ReadAll(io.LimitReader(f, 2*maxConfigSize))
if err != nil {
return nil, fmt.Errorf("error reading from %s: %w", fileName, err)
}
if len(data) == 0 {
return nil, fmt.Errorf("error reading from %s: data is empty", fileName)
}
return data, nil
}
}

6
cli/command/config/create_test.go
View File

@ -59,7 +59,7 @@ func TestConfigCreateErrors(t *testing.T) {
}
func TestConfigCreateWithName(t *testing.T) {
name := "foo"
const name = "config-with-name"
var actual []byte
cli := test.NewFakeCli(&fakeClient{
configCreateFunc: func(_ context.Context, spec swarm.ConfigSpec) (types.ConfigCreateResponse, error) {
@ -87,7 +87,7 @@ func TestConfigCreateWithLabels(t *testing.T) {
"lbl1": "Label-foo",
"lbl2": "Label-bar",
}
name := "foo"
const name = "config-with-labels"
data, err := os.ReadFile(filepath.Join("testdata", configDataFile))
assert.NilError(t, err)
@ -124,7 +124,7 @@ func TestConfigCreateWithTemplatingDriver(t *testing.T) {
expectedDriver := &swarm.Driver{
Name: "template-driver",
}
name := "foo"
const name = "config-with-template-driver"
cli := test.NewFakeCli(&fakeClient{
configCreateFunc: func(_ context.Context, spec swarm.ConfigSpec) (types.ConfigCreateResponse, error) {