From 329b2a26f33c4bae8837489ceafbadc7fc55ea9b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 3 Feb 2025 10:35:25 +0100 Subject: [PATCH] client: normalize and validate empty ID / name arguments to fail early In situations where an empty ID was passed, the client would construct an invalid API endpoint URL, which either resulted in the "not found" handler being hit (resulting in a "page not found" error), or even the wrong endpoint being hit if the client follows redirects. For example, `/containers//json` (inspect) redirects to `/containers/json` (docker ps)) Given that empty IDs should never be expected (especially if they're part of the API URL path), we can validate these and return early. Its worth noting that a few methods already had an error in place; those methods were related to the situation mentioned above, where (e.g.) an "inspect" would redirect to a "list" endpoint. The existing errors, for convenience, mimicked a "not found" error; this patch changes such errors to an "Invalid Parameter" instead, which is more correct, but it could be a breaking change for some edge cases where users parsed the output; git grep 'objectNotFoundError{' client/config_inspect.go: return swarm.Config{}, nil, objectNotFoundError{object: "config", id: id} client/container_inspect.go: return container.InspectResponse{}, nil, objectNotFoundError{object: "container", id: containerID} client/container_inspect.go: return container.InspectResponse{}, objectNotFoundError{object: "container", id: containerID} client/distribution_inspect.go: return distributionInspect, objectNotFoundError{object: "distribution", id: imageRef} client/image_inspect.go: return image.InspectResponse{}, nil, objectNotFoundError{object: "image", id: imageID} client/network_inspect.go: return network.Inspect{}, nil, objectNotFoundError{object: "network", id: networkID} client/node_inspect.go: return swarm.Node{}, nil, objectNotFoundError{object: "node", id: nodeID} client/plugin_inspect.go: return nil, nil, objectNotFoundError{object: "plugin", id: name} client/secret_inspect.go: return swarm.Secret{}, nil, objectNotFoundError{object: "secret", id: id} client/service_inspect.go: return swarm.Service{}, nil, objectNotFoundError{object: "service", id: serviceID} client/task_inspect.go: return swarm.Task{}, nil, objectNotFoundError{object: "task", id: taskID} client/volume_inspect.go: return volume.Volume{}, nil, objectNotFoundError{object: "volume", id: volumeID} Two such errors are still left, as "ID or name" would probably be confusing, but perhaps we can use a more generic error to include those as well (e.g. "invalid reference: value is empty"); client/distribution_inspect.go: return distributionInspect, objectNotFoundError{object: "distribution", id: imageRef} client/image_inspect.go: return image.InspectResponse{}, nil, objectNotFoundError{object: "image", id: imageID} Before this patch: docker container start "" Error response from daemon: page not found Error: failed to start containers: docker container start " " Error response from daemon: No such container: Error: failed to start containers: With this patch: docker container start "" invalid container name or ID: value is empty Error: failed to start containers: docker container start " " invalid container name or ID: value is empty Error: failed to start containers: Signed-off-by: Sebastiaan van Stijn --- client/checkpoint_create.go | 9 +++++-- client/checkpoint_create_test.go | 10 +++++++- client/checkpoint_delete.go | 5 ++++ client/checkpoint_delete_test.go | 8 ++++++ client/config_inspect.go | 5 ++-- client/config_inspect_test.go | 7 +++++- client/config_remove.go | 4 +++ client/config_remove_test.go | 8 ++++++ client/config_update.go | 4 +++ client/config_update_test.go | 8 ++++++ client/container_attach.go | 9 +++++-- client/container_commit.go | 9 +++++-- client/container_commit_test.go | 8 ++++++ client/container_copy.go | 25 +++++++++++++------ client/container_copy_test.go | 24 ++++++++++++++++++ client/container_diff.go | 11 ++++++-- client/container_diff_test.go | 8 ++++++ client/container_exec.go | 17 ++++++++----- client/container_exec_test.go | 11 +++++++- client/container_export.go | 5 ++++ client/container_export_test.go | 8 ++++++ client/container_inspect.go | 12 ++++++--- client/container_inspect_test.go | 15 ++++++++++- client/container_kill.go | 5 ++++ client/container_kill_test.go | 8 ++++++ client/container_logs.go | 9 +++++-- client/container_logs_test.go | 8 ++++++ client/container_pause.go | 5 ++++ client/container_remove.go | 5 ++++ client/container_remove_test.go | 8 ++++++ client/container_rename.go | 5 ++++ client/container_rename_test.go | 8 ++++++ client/container_resize.go | 8 ++++++ client/container_resize_test.go | 8 ++++++ client/container_restart.go | 5 ++++ client/container_restart_test.go | 8 ++++++ client/container_start.go | 5 ++++ client/container_start_test.go | 8 ++++++ client/container_stats.go | 10 ++++++++ client/container_stats_test.go | 8 ++++++ client/container_stop.go | 5 ++++ client/container_stop_test.go | 14 ++++++++--- client/container_top.go | 9 +++++-- client/container_top_test.go | 8 ++++++ client/container_unpause.go | 5 ++++ client/container_unpause_test.go | 8 ++++++ client/container_update.go | 9 +++++-- client/container_update_test.go | 8 ++++++ client/container_wait.go | 6 +++++ client/network_connect.go | 10 ++++++++ client/network_connect_test.go | 9 +++++++ client/network_disconnect.go | 10 ++++++++ client/network_disconnect_test.go | 9 +++++++ client/network_inspect.go | 5 ++-- client/network_inspect_test.go | 7 +++++- client/network_remove.go | 4 +++ client/network_remove_test.go | 8 ++++++ client/node_inspect.go | 5 ++-- client/node_inspect_test.go | 7 +++++- client/node_remove.go | 5 ++++ client/node_remove_test.go | 8 ++++++ client/node_update.go | 5 ++++ client/node_update_test.go | 8 ++++++ client/plugin_disable.go | 4 +++ client/plugin_disable_test.go | 8 ++++++ client/plugin_enable.go | 4 +++ client/plugin_enable_test.go | 8 ++++++ client/plugin_inspect.go | 5 ++-- client/plugin_inspect_test.go | 7 +++++- client/plugin_push.go | 4 +++ client/plugin_push_test.go | 8 ++++++ client/plugin_remove.go | 5 ++++ client/plugin_remove_test.go | 8 ++++++ client/plugin_set.go | 5 ++++ client/plugin_set_test.go | 8 ++++++ client/plugin_upgrade.go | 7 +++++- client/secret_inspect.go | 7 +++--- client/secret_inspect_test.go | 7 +++++- client/secret_remove.go | 4 +++ client/secret_remove_test.go | 8 ++++++ client/secret_update.go | 4 +++ client/secret_update_test.go | 10 +++++++- client/service_inspect.go | 6 +++-- client/service_inspect_test.go | 7 +++++- client/service_logs.go | 5 ++++ client/service_logs_test.go | 8 ++++++ client/service_remove.go | 5 ++++ client/service_remove_test.go | 8 ++++++ client/service_update.go | 12 ++++++--- client/service_update_test.go | 8 ++++++ client/task_inspect.go | 6 +++-- client/task_inspect_test.go | 7 +++++- client/utils.go | 18 +++++++++++++ client/volume_inspect.go | 5 ++-- client/volume_inspect_test.go | 7 +++++- client/volume_remove.go | 5 ++++ client/volume_remove_test.go | 8 ++++++ client/volume_update.go | 4 +++ client/volume_update_test.go | 10 +++++++- integration-cli/docker_api_containers_test.go | 3 ++- 100 files changed, 706 insertions(+), 70 deletions(-) diff --git a/client/checkpoint_create.go b/client/checkpoint_create.go index 9746d288df..7b06fee31d 100644 --- a/client/checkpoint_create.go +++ b/client/checkpoint_create.go @@ -7,8 +7,13 @@ import ( ) // CheckpointCreate creates a checkpoint from the given container with the given name -func (cli *Client) CheckpointCreate(ctx context.Context, container string, options checkpoint.CreateOptions) error { - resp, err := cli.post(ctx, "/containers/"+container+"/checkpoints", nil, options, nil) +func (cli *Client) CheckpointCreate(ctx context.Context, containerID string, options checkpoint.CreateOptions) error { + containerID, err := trimID("container", containerID) + if err != nil { + return err + } + + resp, err := cli.post(ctx, "/containers/"+containerID+"/checkpoints", nil, options, nil) ensureReaderClosed(resp) return err } diff --git a/client/checkpoint_create_test.go b/client/checkpoint_create_test.go index 44dc4a3bc7..ee20caf54e 100644 --- a/client/checkpoint_create_test.go +++ b/client/checkpoint_create_test.go @@ -26,6 +26,14 @@ func TestCheckpointCreateError(t *testing.T) { }) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.CheckpointCreate(context.Background(), "", checkpoint.CreateOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.CheckpointCreate(context.Background(), " ", checkpoint.CreateOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestCheckpointCreate(t *testing.T) { @@ -36,7 +44,7 @@ func TestCheckpointCreate(t *testing.T) { client := &Client{ client: newMockClient(func(req *http.Request) (*http.Response, error) { if !strings.HasPrefix(req.URL.Path, expectedURL) { - return nil, fmt.Errorf("Expected URL '%s', got '%s'", expectedURL, req.URL) + return nil, fmt.Errorf("expected URL '%s', got '%s'", expectedURL, req.URL) } if req.Method != http.MethodPost { diff --git a/client/checkpoint_delete.go b/client/checkpoint_delete.go index b968c2b237..d15162ea04 100644 --- a/client/checkpoint_delete.go +++ b/client/checkpoint_delete.go @@ -9,6 +9,11 @@ import ( // CheckpointDelete deletes the checkpoint with the given name from the given container func (cli *Client) CheckpointDelete(ctx context.Context, containerID string, options checkpoint.DeleteOptions) error { + containerID, err := trimID("container", containerID) + if err != nil { + return err + } + query := url.Values{} if options.CheckpointDir != "" { query.Set("dir", options.CheckpointDir) diff --git a/client/checkpoint_delete_test.go b/client/checkpoint_delete_test.go index 7ce3af2a06..7e578256dd 100644 --- a/client/checkpoint_delete_test.go +++ b/client/checkpoint_delete_test.go @@ -25,6 +25,14 @@ func TestCheckpointDeleteError(t *testing.T) { }) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.CheckpointDelete(context.Background(), "", checkpoint.DeleteOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.CheckpointDelete(context.Background(), " ", checkpoint.DeleteOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestCheckpointDelete(t *testing.T) { diff --git a/client/config_inspect.go b/client/config_inspect.go index 2c6c7cb36f..9a16b3d4a9 100644 --- a/client/config_inspect.go +++ b/client/config_inspect.go @@ -11,8 +11,9 @@ import ( // ConfigInspectWithRaw returns the config information with raw data func (cli *Client) ConfigInspectWithRaw(ctx context.Context, id string) (swarm.Config, []byte, error) { - if id == "" { - return swarm.Config{}, nil, objectNotFoundError{object: "config", id: id} + id, err := trimID("contig", id) + if err != nil { + return swarm.Config{}, nil, err } if err := cli.NewVersionError(ctx, "1.30", "config inspect"); err != nil { return swarm.Config{}, nil, err diff --git a/client/config_inspect_test.go b/client/config_inspect_test.go index a47a8251b5..2aa5d3e6d1 100644 --- a/client/config_inspect_test.go +++ b/client/config_inspect_test.go @@ -33,7 +33,12 @@ func TestConfigInspectWithEmptyID(t *testing.T) { }), } _, _, err := client.ConfigInspectWithRaw(context.Background(), "") - assert.Check(t, is.ErrorType(err, errdefs.IsNotFound)) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, _, err = client.ConfigInspectWithRaw(context.Background(), " ") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestConfigInspectUnsupported(t *testing.T) { diff --git a/client/config_remove.go b/client/config_remove.go index d05b0113aa..a2955c6894 100644 --- a/client/config_remove.go +++ b/client/config_remove.go @@ -4,6 +4,10 @@ import "context" // ConfigRemove removes a config. func (cli *Client) ConfigRemove(ctx context.Context, id string) error { + id, err := trimID("config", id) + if err != nil { + return err + } if err := cli.NewVersionError(ctx, "1.30", "config remove"); err != nil { return err } diff --git a/client/config_remove_test.go b/client/config_remove_test.go index 99c4450b4e..1646962bf5 100644 --- a/client/config_remove_test.go +++ b/client/config_remove_test.go @@ -31,6 +31,14 @@ func TestConfigRemoveError(t *testing.T) { err := client.ConfigRemove(context.Background(), "config_id") assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.ConfigRemove(context.Background(), "") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.ConfigRemove(context.Background(), " ") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestConfigRemove(t *testing.T) { diff --git a/client/config_update.go b/client/config_update.go index 6995861df0..ddb219cf6a 100644 --- a/client/config_update.go +++ b/client/config_update.go @@ -9,6 +9,10 @@ import ( // ConfigUpdate attempts to update a config func (cli *Client) ConfigUpdate(ctx context.Context, id string, version swarm.Version, config swarm.ConfigSpec) error { + id, err := trimID("config", id) + if err != nil { + return err + } if err := cli.NewVersionError(ctx, "1.30", "config update"); err != nil { return err } diff --git a/client/config_update_test.go b/client/config_update_test.go index 59af49f2ea..cfb5a4aadb 100644 --- a/client/config_update_test.go +++ b/client/config_update_test.go @@ -32,6 +32,14 @@ func TestConfigUpdateError(t *testing.T) { err := client.ConfigUpdate(context.Background(), "config_id", swarm.Version{}, swarm.ConfigSpec{}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.ConfigUpdate(context.Background(), "", swarm.Version{}, swarm.ConfigSpec{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.ConfigUpdate(context.Background(), " ", swarm.Version{}, swarm.ConfigSpec{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestConfigUpdate(t *testing.T) { diff --git a/client/container_attach.go b/client/container_attach.go index 6a32e5f664..2e7a13e5c5 100644 --- a/client/container_attach.go +++ b/client/container_attach.go @@ -33,7 +33,12 @@ import ( // // You can use github.com/docker/docker/pkg/stdcopy.StdCopy to demultiplex this // stream. -func (cli *Client) ContainerAttach(ctx context.Context, container string, options container.AttachOptions) (types.HijackedResponse, error) { +func (cli *Client) ContainerAttach(ctx context.Context, containerID string, options container.AttachOptions) (types.HijackedResponse, error) { + containerID, err := trimID("container", containerID) + if err != nil { + return types.HijackedResponse{}, err + } + query := url.Values{} if options.Stream { query.Set("stream", "1") @@ -54,7 +59,7 @@ func (cli *Client) ContainerAttach(ctx context.Context, container string, option query.Set("logs", "1") } - return cli.postHijacked(ctx, "/containers/"+container+"/attach", query, nil, http.Header{ + return cli.postHijacked(ctx, "/containers/"+containerID+"/attach", query, nil, http.Header{ "Content-Type": {"text/plain"}, }) } diff --git a/client/container_commit.go b/client/container_commit.go index 26b3f09158..718465ce06 100644 --- a/client/container_commit.go +++ b/client/container_commit.go @@ -12,7 +12,12 @@ import ( ) // ContainerCommit applies changes to a container and creates a new tagged image. -func (cli *Client) ContainerCommit(ctx context.Context, container string, options container.CommitOptions) (types.IDResponse, error) { +func (cli *Client) ContainerCommit(ctx context.Context, containerID string, options container.CommitOptions) (types.IDResponse, error) { + containerID, err := trimID("container", containerID) + if err != nil { + return types.IDResponse{}, err + } + var repository, tag string if options.Reference != "" { ref, err := reference.ParseNormalizedNamed(options.Reference) @@ -32,7 +37,7 @@ func (cli *Client) ContainerCommit(ctx context.Context, container string, option } query := url.Values{} - query.Set("container", container) + query.Set("container", containerID) query.Set("repo", repository) query.Set("tag", tag) query.Set("comment", options.Comment) diff --git a/client/container_commit_test.go b/client/container_commit_test.go index 44980b1f76..c6cf94a74d 100644 --- a/client/container_commit_test.go +++ b/client/container_commit_test.go @@ -23,6 +23,14 @@ func TestContainerCommitError(t *testing.T) { } _, err := client.ContainerCommit(context.Background(), "nothing", container.CommitOptions{}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + _, err = client.ContainerCommit(context.Background(), "", container.CommitOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, err = client.ContainerCommit(context.Background(), " ", container.CommitOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestContainerCommit(t *testing.T) { diff --git a/client/container_copy.go b/client/container_copy.go index 8490a3b156..3604a0ad55 100644 --- a/client/container_copy.go +++ b/client/container_copy.go @@ -16,11 +16,15 @@ import ( // ContainerStatPath returns stat information about a path inside the container filesystem. func (cli *Client) ContainerStatPath(ctx context.Context, containerID, path string) (container.PathStat, error) { + containerID, err := trimID("container", containerID) + if err != nil { + return container.PathStat{}, err + } + query := url.Values{} query.Set("path", filepath.ToSlash(path)) // Normalize the paths used in the API. - urlStr := "/containers/" + containerID + "/archive" - response, err := cli.head(ctx, urlStr, query, nil) + response, err := cli.head(ctx, "/containers/"+containerID+"/archive", query, nil) defer ensureReaderClosed(response) if err != nil { return container.PathStat{}, err @@ -31,6 +35,11 @@ func (cli *Client) ContainerStatPath(ctx context.Context, containerID, path stri // CopyToContainer copies content into the container filesystem. // Note that `content` must be a Reader for a TAR archive func (cli *Client) CopyToContainer(ctx context.Context, containerID, dstPath string, content io.Reader, options container.CopyToContainerOptions) error { + containerID, err := trimID("container", containerID) + if err != nil { + return err + } + query := url.Values{} query.Set("path", filepath.ToSlash(dstPath)) // Normalize the paths used in the API. // Do not allow for an existing directory to be overwritten by a non-directory and vice versa. @@ -42,9 +51,7 @@ func (cli *Client) CopyToContainer(ctx context.Context, containerID, dstPath str query.Set("copyUIDGID", "true") } - apiPath := "/containers/" + containerID + "/archive" - - response, err := cli.putRaw(ctx, apiPath, query, content, nil) + response, err := cli.putRaw(ctx, "/containers/"+containerID+"/archive", query, content, nil) defer ensureReaderClosed(response) if err != nil { return err @@ -56,11 +63,15 @@ func (cli *Client) CopyToContainer(ctx context.Context, containerID, dstPath str // CopyFromContainer gets the content from the container and returns it as a Reader // for a TAR archive to manipulate it in the host. It's up to the caller to close the reader. func (cli *Client) CopyFromContainer(ctx context.Context, containerID, srcPath string) (io.ReadCloser, container.PathStat, error) { + containerID, err := trimID("container", containerID) + if err != nil { + return nil, container.PathStat{}, err + } + query := make(url.Values, 1) query.Set("path", filepath.ToSlash(srcPath)) // Normalize the paths used in the API. - apiPath := "/containers/" + containerID + "/archive" - response, err := cli.get(ctx, apiPath, query, nil) + response, err := cli.get(ctx, "/containers/"+containerID+"/archive", query, nil) if err != nil { return nil, container.PathStat{}, err } diff --git a/client/container_copy_test.go b/client/container_copy_test.go index 8f7d2ce78a..3007496628 100644 --- a/client/container_copy_test.go +++ b/client/container_copy_test.go @@ -23,6 +23,14 @@ func TestContainerStatPathError(t *testing.T) { } _, err := client.ContainerStatPath(context.Background(), "container_id", "path") assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + _, err = client.ContainerStatPath(context.Background(), "", "path") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, err = client.ContainerStatPath(context.Background(), " ", "path") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestContainerStatPathNotFoundError(t *testing.T) { @@ -99,6 +107,14 @@ func TestCopyToContainerError(t *testing.T) { } err := client.CopyToContainer(context.Background(), "container_id", "path/to/file", bytes.NewReader([]byte("")), container.CopyToContainerOptions{}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.CopyToContainer(context.Background(), "", "path/to/file", bytes.NewReader([]byte("")), container.CopyToContainerOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.CopyToContainer(context.Background(), " ", "path/to/file", bytes.NewReader([]byte("")), container.CopyToContainerOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestCopyToContainerNotFoundError(t *testing.T) { @@ -173,6 +189,14 @@ func TestCopyFromContainerError(t *testing.T) { } _, _, err := client.CopyFromContainer(context.Background(), "container_id", "path/to/file") assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + _, _, err = client.CopyFromContainer(context.Background(), "", "path/to/file") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, _, err = client.CopyFromContainer(context.Background(), " ", "path/to/file") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestCopyFromContainerNotFoundError(t *testing.T) { diff --git a/client/container_diff.go b/client/container_diff.go index c22c819a79..38e92dbfc5 100644 --- a/client/container_diff.go +++ b/client/container_diff.go @@ -10,14 +10,21 @@ import ( // ContainerDiff shows differences in a container filesystem since it was started. func (cli *Client) ContainerDiff(ctx context.Context, containerID string) ([]container.FilesystemChange, error) { - var changes []container.FilesystemChange + containerID, err := trimID("container", containerID) + if err != nil { + return nil, err + } serverResp, err := cli.get(ctx, "/containers/"+containerID+"/changes", url.Values{}, nil) defer ensureReaderClosed(serverResp) if err != nil { - return changes, err + return nil, err } + var changes []container.FilesystemChange err = json.NewDecoder(serverResp.body).Decode(&changes) + if err != nil { + return nil, err + } return changes, err } diff --git a/client/container_diff_test.go b/client/container_diff_test.go index 6fad8767b2..bf80a3d163 100644 --- a/client/container_diff_test.go +++ b/client/container_diff_test.go @@ -22,6 +22,14 @@ func TestContainerDiffError(t *testing.T) { } _, err := client.ContainerDiff(context.Background(), "nothing") assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + _, err = client.ContainerDiff(context.Background(), "") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, err = client.ContainerDiff(context.Background(), " ") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestContainerDiff(t *testing.T) { diff --git a/client/container_exec.go b/client/container_exec.go index 9379448d1a..4891b98e6e 100644 --- a/client/container_exec.go +++ b/client/container_exec.go @@ -11,8 +11,11 @@ import ( ) // ContainerExecCreate creates a new exec configuration to run an exec process. -func (cli *Client) ContainerExecCreate(ctx context.Context, container string, options container.ExecOptions) (types.IDResponse, error) { - var response types.IDResponse +func (cli *Client) ContainerExecCreate(ctx context.Context, containerID string, options container.ExecOptions) (types.IDResponse, error) { + containerID, err := trimID("container", containerID) + if err != nil { + return types.IDResponse{}, err + } // Make sure we negotiated (if the client is configured to do so), // as code below contains API-version specific handling of options. @@ -20,21 +23,23 @@ func (cli *Client) ContainerExecCreate(ctx context.Context, container string, op // Normally, version-negotiation (if enabled) would not happen until // the API request is made. if err := cli.checkVersion(ctx); err != nil { - return response, err + return types.IDResponse{}, err } if err := cli.NewVersionError(ctx, "1.25", "env"); len(options.Env) != 0 && err != nil { - return response, err + return types.IDResponse{}, err } if versions.LessThan(cli.ClientVersion(), "1.42") { options.ConsoleSize = nil } - resp, err := cli.post(ctx, "/containers/"+container+"/exec", nil, options, nil) + resp, err := cli.post(ctx, "/containers/"+containerID+"/exec", nil, options, nil) defer ensureReaderClosed(resp) if err != nil { - return response, err + return types.IDResponse{}, err } + + var response types.IDResponse err = json.NewDecoder(resp.body).Decode(&response) return response, err } diff --git a/client/container_exec_test.go b/client/container_exec_test.go index acae8026d8..f291bccf97 100644 --- a/client/container_exec_test.go +++ b/client/container_exec_test.go @@ -21,8 +21,17 @@ func TestContainerExecCreateError(t *testing.T) { client := &Client{ client: newMockClient(errorMock(http.StatusInternalServerError, "Server error")), } + _, err := client.ContainerExecCreate(context.Background(), "container_id", container.ExecOptions{}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + _, err = client.ContainerExecCreate(context.Background(), "", container.ExecOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, err = client.ContainerExecCreate(context.Background(), " ", container.ExecOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } // TestContainerExecCreateConnectionError verifies that connection errors occurring @@ -33,7 +42,7 @@ func TestContainerExecCreateConnectionError(t *testing.T) { client, err := NewClientWithOpts(WithAPIVersionNegotiation(), WithHost("tcp://no-such-host.invalid")) assert.NilError(t, err) - _, err = client.ContainerExecCreate(context.Background(), "", container.ExecOptions{}) + _, err = client.ContainerExecCreate(context.Background(), "container_id", container.ExecOptions{}) assert.Check(t, is.ErrorType(err, IsErrConnectionFailed)) } diff --git a/client/container_export.go b/client/container_export.go index d0c0a5cbad..1e45a9925c 100644 --- a/client/container_export.go +++ b/client/container_export.go @@ -10,6 +10,11 @@ import ( // and returns them as an io.ReadCloser. It's up to the caller // to close the stream. func (cli *Client) ContainerExport(ctx context.Context, containerID string) (io.ReadCloser, error) { + containerID, err := trimID("container", containerID) + if err != nil { + return nil, err + } + serverResp, err := cli.get(ctx, "/containers/"+containerID+"/export", url.Values{}, nil) if err != nil { return nil, err diff --git a/client/container_export_test.go b/client/container_export_test.go index dca4dd7636..3001b5c678 100644 --- a/client/container_export_test.go +++ b/client/container_export_test.go @@ -20,6 +20,14 @@ func TestContainerExportError(t *testing.T) { } _, err := client.ContainerExport(context.Background(), "nothing") assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + _, err = client.ContainerExport(context.Background(), "") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, err = client.ContainerExport(context.Background(), " ") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestContainerExport(t *testing.T) { diff --git a/client/container_inspect.go b/client/container_inspect.go index fa342e16b5..7fe86a2a88 100644 --- a/client/container_inspect.go +++ b/client/container_inspect.go @@ -12,9 +12,11 @@ import ( // ContainerInspect returns the container information. func (cli *Client) ContainerInspect(ctx context.Context, containerID string) (container.InspectResponse, error) { - if containerID == "" { - return container.InspectResponse{}, objectNotFoundError{object: "container", id: containerID} + containerID, err := trimID("container", containerID) + if err != nil { + return container.InspectResponse{}, err } + serverResp, err := cli.get(ctx, "/containers/"+containerID+"/json", nil, nil) defer ensureReaderClosed(serverResp) if err != nil { @@ -28,9 +30,11 @@ func (cli *Client) ContainerInspect(ctx context.Context, containerID string) (co // ContainerInspectWithRaw returns the container information and its raw representation. func (cli *Client) ContainerInspectWithRaw(ctx context.Context, containerID string, getSize bool) (container.InspectResponse, []byte, error) { - if containerID == "" { - return container.InspectResponse{}, nil, objectNotFoundError{object: "container", id: containerID} + containerID, err := trimID("container", containerID) + if err != nil { + return container.InspectResponse{}, nil, err } + query := url.Values{} if getSize { query.Set("size", "1") diff --git a/client/container_inspect_test.go b/client/container_inspect_test.go index c1fe267646..dbfd4641a8 100644 --- a/client/container_inspect_test.go +++ b/client/container_inspect_test.go @@ -24,6 +24,14 @@ func TestContainerInspectError(t *testing.T) { _, err := client.ContainerInspect(context.Background(), "nothing") assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + _, err = client.ContainerInspect(context.Background(), "") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, err = client.ContainerInspect(context.Background(), " ") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestContainerInspectContainerNotFound(t *testing.T) { @@ -42,7 +50,12 @@ func TestContainerInspectWithEmptyID(t *testing.T) { }), } _, _, err := client.ContainerInspectWithRaw(context.Background(), "", true) - assert.Check(t, is.ErrorType(err, errdefs.IsNotFound)) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, _, err = client.ContainerInspectWithRaw(context.Background(), " ", true) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestContainerInspect(t *testing.T) { diff --git a/client/container_kill.go b/client/container_kill.go index 7c9529f1e1..22767ae682 100644 --- a/client/container_kill.go +++ b/client/container_kill.go @@ -7,6 +7,11 @@ import ( // ContainerKill terminates the container process but does not remove the container from the docker host. func (cli *Client) ContainerKill(ctx context.Context, containerID, signal string) error { + containerID, err := trimID("container", containerID) + if err != nil { + return err + } + query := url.Values{} if signal != "" { query.Set("signal", signal) diff --git a/client/container_kill_test.go b/client/container_kill_test.go index 95d525ef5d..2a22dd78fd 100644 --- a/client/container_kill_test.go +++ b/client/container_kill_test.go @@ -20,6 +20,14 @@ func TestContainerKillError(t *testing.T) { } err := client.ContainerKill(context.Background(), "nothing", "SIGKILL") assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.ContainerKill(context.Background(), "", "") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.ContainerKill(context.Background(), " ", "") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestContainerKill(t *testing.T) { diff --git a/client/container_logs.go b/client/container_logs.go index 61197d8407..989a66a915 100644 --- a/client/container_logs.go +++ b/client/container_logs.go @@ -33,7 +33,12 @@ import ( // // You can use github.com/docker/docker/pkg/stdcopy.StdCopy to demultiplex this // stream. -func (cli *Client) ContainerLogs(ctx context.Context, container string, options container.LogsOptions) (io.ReadCloser, error) { +func (cli *Client) ContainerLogs(ctx context.Context, containerID string, options container.LogsOptions) (io.ReadCloser, error) { + containerID, err := trimID("container", containerID) + if err != nil { + return nil, err + } + query := url.Values{} if options.ShowStdout { query.Set("stdout", "1") @@ -72,7 +77,7 @@ func (cli *Client) ContainerLogs(ctx context.Context, container string, options } query.Set("tail", options.Tail) - resp, err := cli.get(ctx, "/containers/"+container+"/logs", query, nil) + resp, err := cli.get(ctx, "/containers/"+containerID+"/logs", query, nil) if err != nil { return nil, err } diff --git a/client/container_logs_test.go b/client/container_logs_test.go index 4e04da253b..defd6742cd 100644 --- a/client/container_logs_test.go +++ b/client/container_logs_test.go @@ -24,6 +24,14 @@ func TestContainerLogsNotFoundError(t *testing.T) { } _, err := client.ContainerLogs(context.Background(), "container_id", container.LogsOptions{}) assert.Check(t, is.ErrorType(err, errdefs.IsNotFound)) + + _, err = client.ContainerLogs(context.Background(), "", container.LogsOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, err = client.ContainerLogs(context.Background(), " ", container.LogsOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestContainerLogsError(t *testing.T) { diff --git a/client/container_pause.go b/client/container_pause.go index 5e7271a371..5cc2984013 100644 --- a/client/container_pause.go +++ b/client/container_pause.go @@ -4,6 +4,11 @@ import "context" // ContainerPause pauses the main process of a given container without terminating it. func (cli *Client) ContainerPause(ctx context.Context, containerID string) error { + containerID, err := trimID("container", containerID) + if err != nil { + return err + } + resp, err := cli.post(ctx, "/containers/"+containerID+"/pause", nil, nil, nil) ensureReaderClosed(resp) return err diff --git a/client/container_remove.go b/client/container_remove.go index 39f7b106a1..6661351a92 100644 --- a/client/container_remove.go +++ b/client/container_remove.go @@ -9,6 +9,11 @@ import ( // ContainerRemove kills and removes a container from the docker host. func (cli *Client) ContainerRemove(ctx context.Context, containerID string, options container.RemoveOptions) error { + containerID, err := trimID("container", containerID) + if err != nil { + return err + } + query := url.Values{} if options.RemoveVolumes { query.Set("v", "1") diff --git a/client/container_remove_test.go b/client/container_remove_test.go index eecbd71268..b11a98db85 100644 --- a/client/container_remove_test.go +++ b/client/container_remove_test.go @@ -21,6 +21,14 @@ func TestContainerRemoveError(t *testing.T) { } err := client.ContainerRemove(context.Background(), "container_id", container.RemoveOptions{}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.ContainerRemove(context.Background(), "", container.RemoveOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.ContainerRemove(context.Background(), " ", container.RemoveOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestContainerRemoveNotFoundError(t *testing.T) { diff --git a/client/container_rename.go b/client/container_rename.go index 240fdf552b..0a092310c6 100644 --- a/client/container_rename.go +++ b/client/container_rename.go @@ -7,6 +7,11 @@ import ( // ContainerRename changes the name of a given container. func (cli *Client) ContainerRename(ctx context.Context, containerID, newContainerName string) error { + containerID, err := trimID("container", containerID) + if err != nil { + return err + } + query := url.Values{} query.Set("name", newContainerName) resp, err := cli.post(ctx, "/containers/"+containerID+"/rename", query, nil, nil) diff --git a/client/container_rename_test.go b/client/container_rename_test.go index 0f606a0121..27215a0b5e 100644 --- a/client/container_rename_test.go +++ b/client/container_rename_test.go @@ -20,6 +20,14 @@ func TestContainerRenameError(t *testing.T) { } err := client.ContainerRename(context.Background(), "nothing", "newNothing") assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.ContainerRename(context.Background(), "", "newNothing") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.ContainerRename(context.Background(), " ", "newNothing") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestContainerRename(t *testing.T) { diff --git a/client/container_resize.go b/client/container_resize.go index 6f1a8f5605..725c08ad41 100644 --- a/client/container_resize.go +++ b/client/container_resize.go @@ -10,11 +10,19 @@ import ( // ContainerResize changes the size of the tty for a container. func (cli *Client) ContainerResize(ctx context.Context, containerID string, options container.ResizeOptions) error { + containerID, err := trimID("container", containerID) + if err != nil { + return err + } return cli.resize(ctx, "/containers/"+containerID, options.Height, options.Width) } // ContainerExecResize changes the size of the tty for an exec process running inside a container. func (cli *Client) ContainerExecResize(ctx context.Context, execID string, options container.ResizeOptions) error { + execID, err := trimID("exec", execID) + if err != nil { + return err + } return cli.resize(ctx, "/exec/"+execID, options.Height, options.Width) } diff --git a/client/container_resize_test.go b/client/container_resize_test.go index 00960c1774..7874421c79 100644 --- a/client/container_resize_test.go +++ b/client/container_resize_test.go @@ -20,6 +20,14 @@ func TestContainerResizeError(t *testing.T) { } err := client.ContainerResize(context.Background(), "container_id", container.ResizeOptions{}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.ContainerResize(context.Background(), "", container.ResizeOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.ContainerResize(context.Background(), " ", container.ResizeOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestContainerExecResizeError(t *testing.T) { diff --git a/client/container_restart.go b/client/container_restart.go index 02b5079bc4..50559ba6e4 100644 --- a/client/container_restart.go +++ b/client/container_restart.go @@ -13,6 +13,11 @@ import ( // It makes the daemon wait for the container to be up again for // a specific amount of time, given the timeout. func (cli *Client) ContainerRestart(ctx context.Context, containerID string, options container.StopOptions) error { + containerID, err := trimID("container", containerID) + if err != nil { + return err + } + query := url.Values{} if options.Timeout != nil { query.Set("t", strconv.Itoa(*options.Timeout)) diff --git a/client/container_restart_test.go b/client/container_restart_test.go index 95f3fceb19..6adc296139 100644 --- a/client/container_restart_test.go +++ b/client/container_restart_test.go @@ -21,6 +21,14 @@ func TestContainerRestartError(t *testing.T) { } err := client.ContainerRestart(context.Background(), "nothing", container.StopOptions{}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.ContainerRestart(context.Background(), "", container.StopOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.ContainerRestart(context.Background(), " ", container.StopOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } // TestContainerRestartConnectionError verifies that connection errors occurring diff --git a/client/container_start.go b/client/container_start.go index 33ba85f248..b81ed3ebc8 100644 --- a/client/container_start.go +++ b/client/container_start.go @@ -9,6 +9,11 @@ import ( // ContainerStart sends a request to the docker daemon to start a container. func (cli *Client) ContainerStart(ctx context.Context, containerID string, options container.StartOptions) error { + containerID, err := trimID("container", containerID) + if err != nil { + return err + } + query := url.Values{} if len(options.CheckpointID) != 0 { query.Set("checkpoint", options.CheckpointID) diff --git a/client/container_start_test.go b/client/container_start_test.go index d4d0fd16bc..d233c953c5 100644 --- a/client/container_start_test.go +++ b/client/container_start_test.go @@ -22,6 +22,14 @@ func TestContainerStartError(t *testing.T) { } err := client.ContainerStart(context.Background(), "nothing", container.StartOptions{}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.ContainerStart(context.Background(), "", container.StartOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.ContainerStart(context.Background(), " ", container.StartOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestContainerStart(t *testing.T) { diff --git a/client/container_stats.go b/client/container_stats.go index b5641daee9..f90d1c9a6b 100644 --- a/client/container_stats.go +++ b/client/container_stats.go @@ -10,6 +10,11 @@ import ( // ContainerStats returns near realtime stats for a given container. // It's up to the caller to close the io.ReadCloser returned. func (cli *Client) ContainerStats(ctx context.Context, containerID string, stream bool) (container.StatsResponseReader, error) { + containerID, err := trimID("container", containerID) + if err != nil { + return container.StatsResponseReader{}, err + } + query := url.Values{} query.Set("stream", "0") if stream { @@ -30,6 +35,11 @@ func (cli *Client) ContainerStats(ctx context.Context, containerID string, strea // ContainerStatsOneShot gets a single stat entry from a container. // It differs from `ContainerStats` in that the API should not wait to prime the stats func (cli *Client) ContainerStatsOneShot(ctx context.Context, containerID string) (container.StatsResponseReader, error) { + containerID, err := trimID("container", containerID) + if err != nil { + return container.StatsResponseReader{}, err + } + query := url.Values{} query.Set("stream", "0") query.Set("one-shot", "1") diff --git a/client/container_stats_test.go b/client/container_stats_test.go index f0e9c0d934..f10cf63ec3 100644 --- a/client/container_stats_test.go +++ b/client/container_stats_test.go @@ -20,6 +20,14 @@ func TestContainerStatsError(t *testing.T) { } _, err := client.ContainerStats(context.Background(), "nothing", false) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + _, err = client.ContainerStats(context.Background(), "", false) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, err = client.ContainerStats(context.Background(), " ", false) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestContainerStats(t *testing.T) { diff --git a/client/container_stop.go b/client/container_stop.go index 7c98a354b4..eb0129ce37 100644 --- a/client/container_stop.go +++ b/client/container_stop.go @@ -17,6 +17,11 @@ import ( // otherwise the engine default. A negative timeout value can be specified, // meaning no timeout, i.e. no forceful termination is performed. func (cli *Client) ContainerStop(ctx context.Context, containerID string, options container.StopOptions) error { + containerID, err := trimID("container", containerID) + if err != nil { + return err + } + query := url.Values{} if options.Timeout != nil { query.Set("t", strconv.Itoa(*options.Timeout)) diff --git a/client/container_stop_test.go b/client/container_stop_test.go index a0b38c10fd..98e4161a08 100644 --- a/client/container_stop_test.go +++ b/client/container_stop_test.go @@ -19,8 +19,16 @@ func TestContainerStopError(t *testing.T) { client := &Client{ client: newMockClient(errorMock(http.StatusInternalServerError, "Server error")), } - err := client.ContainerStop(context.Background(), "nothing", container.StopOptions{}) + err := client.ContainerStop(context.Background(), "container_id", container.StopOptions{}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.ContainerStop(context.Background(), "", container.StopOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.ContainerStop(context.Background(), " ", container.StopOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } // TestContainerStopConnectionError verifies that connection errors occurring @@ -31,7 +39,7 @@ func TestContainerStopConnectionError(t *testing.T) { client, err := NewClientWithOpts(WithAPIVersionNegotiation(), WithHost("tcp://no-such-host.invalid")) assert.NilError(t, err) - err = client.ContainerStop(context.Background(), "nothing", container.StopOptions{}) + err = client.ContainerStop(context.Background(), "container_id", container.StopOptions{}) assert.Check(t, is.ErrorType(err, IsErrConnectionFailed)) } @@ -40,7 +48,7 @@ func TestContainerStop(t *testing.T) { client := &Client{ client: newMockClient(func(req *http.Request) (*http.Response, error) { if !strings.HasPrefix(req.URL.Path, expectedURL) { - return nil, fmt.Errorf("Expected URL '%s', got '%s'", expectedURL, req.URL) + return nil, fmt.Errorf("expected URL '%s', got '%s'", expectedURL, req.URL) } s := req.URL.Query().Get("signal") if s != "SIGKILL" { diff --git a/client/container_top.go b/client/container_top.go index a5b78999bf..4eac031fae 100644 --- a/client/container_top.go +++ b/client/container_top.go @@ -11,7 +11,11 @@ import ( // ContainerTop shows process information from within a container. func (cli *Client) ContainerTop(ctx context.Context, containerID string, arguments []string) (container.ContainerTopOKBody, error) { - var response container.ContainerTopOKBody + containerID, err := trimID("container", containerID) + if err != nil { + return container.ContainerTopOKBody{}, err + } + query := url.Values{} if len(arguments) > 0 { query.Set("ps_args", strings.Join(arguments, " ")) @@ -20,9 +24,10 @@ func (cli *Client) ContainerTop(ctx context.Context, containerID string, argumen resp, err := cli.get(ctx, "/containers/"+containerID+"/top", query, nil) defer ensureReaderClosed(resp) if err != nil { - return response, err + return container.ContainerTopOKBody{}, err } + var response container.ContainerTopOKBody err = json.NewDecoder(resp.body).Decode(&response) return response, err } diff --git a/client/container_top_test.go b/client/container_top_test.go index 0c133f1383..92a6626881 100644 --- a/client/container_top_test.go +++ b/client/container_top_test.go @@ -23,6 +23,14 @@ func TestContainerTopError(t *testing.T) { } _, err := client.ContainerTop(context.Background(), "nothing", []string{}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + _, err = client.ContainerTop(context.Background(), "", []string{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, err = client.ContainerTop(context.Background(), " ", []string{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestContainerTop(t *testing.T) { diff --git a/client/container_unpause.go b/client/container_unpause.go index 1d8f873169..f602549bb2 100644 --- a/client/container_unpause.go +++ b/client/container_unpause.go @@ -4,6 +4,11 @@ import "context" // ContainerUnpause resumes the process execution within a container func (cli *Client) ContainerUnpause(ctx context.Context, containerID string) error { + containerID, err := trimID("container", containerID) + if err != nil { + return err + } + resp, err := cli.post(ctx, "/containers/"+containerID+"/unpause", nil, nil, nil) ensureReaderClosed(resp) return err diff --git a/client/container_unpause_test.go b/client/container_unpause_test.go index 9bdcfe1cdd..17ea5123bf 100644 --- a/client/container_unpause_test.go +++ b/client/container_unpause_test.go @@ -20,6 +20,14 @@ func TestContainerUnpauseError(t *testing.T) { } err := client.ContainerUnpause(context.Background(), "nothing") assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.ContainerUnpause(context.Background(), "") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.ContainerUnpause(context.Background(), " ") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestContainerUnpause(t *testing.T) { diff --git a/client/container_update.go b/client/container_update.go index bf68a5300e..d14b14af3c 100644 --- a/client/container_update.go +++ b/client/container_update.go @@ -9,13 +9,18 @@ import ( // ContainerUpdate updates the resources of a container. func (cli *Client) ContainerUpdate(ctx context.Context, containerID string, updateConfig container.UpdateConfig) (container.ContainerUpdateOKBody, error) { - var response container.ContainerUpdateOKBody + containerID, err := trimID("container", containerID) + if err != nil { + return container.ContainerUpdateOKBody{}, err + } + serverResp, err := cli.post(ctx, "/containers/"+containerID+"/update", nil, updateConfig, nil) defer ensureReaderClosed(serverResp) if err != nil { - return response, err + return container.ContainerUpdateOKBody{}, err } + var response container.ContainerUpdateOKBody err = json.NewDecoder(serverResp.body).Decode(&response) return response, err } diff --git a/client/container_update_test.go b/client/container_update_test.go index 10dc1ed633..ad9b586e80 100644 --- a/client/container_update_test.go +++ b/client/container_update_test.go @@ -22,6 +22,14 @@ func TestContainerUpdateError(t *testing.T) { } _, err := client.ContainerUpdate(context.Background(), "nothing", container.UpdateConfig{}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + _, err = client.ContainerUpdate(context.Background(), "", container.UpdateConfig{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, err = client.ContainerUpdate(context.Background(), " ", container.UpdateConfig{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestContainerUpdate(t *testing.T) { diff --git a/client/container_wait.go b/client/container_wait.go index 8bb6be0a18..fd025d25a6 100644 --- a/client/container_wait.go +++ b/client/container_wait.go @@ -33,6 +33,12 @@ func (cli *Client) ContainerWait(ctx context.Context, containerID string, condit resultC := make(chan container.WaitResponse) errC := make(chan error, 1) + containerID, err := trimID("container", containerID) + if err != nil { + errC <- err + return resultC, errC + } + // Make sure we negotiated (if the client is configured to do so), // as code below contains API-version specific handling of options. // diff --git a/client/network_connect.go b/client/network_connect.go index 8daf890635..fa7cc34faa 100644 --- a/client/network_connect.go +++ b/client/network_connect.go @@ -8,6 +8,16 @@ import ( // NetworkConnect connects a container to an existent network in the docker host. func (cli *Client) NetworkConnect(ctx context.Context, networkID, containerID string, config *network.EndpointSettings) error { + networkID, err := trimID("network", networkID) + if err != nil { + return err + } + + containerID, err = trimID("container", containerID) + if err != nil { + return err + } + nc := network.ConnectOptions{ Container: containerID, EndpointConfig: config, diff --git a/client/network_connect_test.go b/client/network_connect_test.go index 60005b9fac..8bb833d102 100644 --- a/client/network_connect_test.go +++ b/client/network_connect_test.go @@ -23,6 +23,15 @@ func TestNetworkConnectError(t *testing.T) { err := client.NetworkConnect(context.Background(), "network_id", "container_id", nil) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + // Empty network ID or container ID + err = client.NetworkConnect(context.Background(), "", "container_id", nil) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.NetworkConnect(context.Background(), "network_id", "", nil) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestNetworkConnectEmptyNilEndpointSettings(t *testing.T) { diff --git a/client/network_disconnect.go b/client/network_disconnect.go index aaf428d853..d8051df2fa 100644 --- a/client/network_disconnect.go +++ b/client/network_disconnect.go @@ -8,6 +8,16 @@ import ( // NetworkDisconnect disconnects a container from an existent network in the docker host. func (cli *Client) NetworkDisconnect(ctx context.Context, networkID, containerID string, force bool) error { + networkID, err := trimID("network", networkID) + if err != nil { + return err + } + + containerID, err = trimID("container", containerID) + if err != nil { + return err + } + nd := network.DisconnectOptions{ Container: containerID, Force: force, diff --git a/client/network_disconnect_test.go b/client/network_disconnect_test.go index 687a412595..bda1b3ce57 100644 --- a/client/network_disconnect_test.go +++ b/client/network_disconnect_test.go @@ -23,6 +23,15 @@ func TestNetworkDisconnectError(t *testing.T) { err := client.NetworkDisconnect(context.Background(), "network_id", "container_id", false) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + // Empty network ID or container ID + err = client.NetworkDisconnect(context.Background(), "", "container_id", false) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.NetworkDisconnect(context.Background(), "network_id", "", false) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestNetworkDisconnect(t *testing.T) { diff --git a/client/network_inspect.go b/client/network_inspect.go index afc47de6fa..ad34e80a09 100644 --- a/client/network_inspect.go +++ b/client/network_inspect.go @@ -18,8 +18,9 @@ func (cli *Client) NetworkInspect(ctx context.Context, networkID string, options // NetworkInspectWithRaw returns the information for a specific network configured in the docker host and its raw representation. func (cli *Client) NetworkInspectWithRaw(ctx context.Context, networkID string, options network.InspectOptions) (network.Inspect, []byte, error) { - if networkID == "" { - return network.Inspect{}, nil, objectNotFoundError{object: "network", id: networkID} + networkID, err := trimID("network", networkID) + if err != nil { + return network.Inspect{}, nil, err } query := url.Values{} if options.Verbose { diff --git a/client/network_inspect_test.go b/client/network_inspect_test.go index be7bb14a80..c15ebaf823 100644 --- a/client/network_inspect_test.go +++ b/client/network_inspect_test.go @@ -69,7 +69,12 @@ func TestNetworkInspect(t *testing.T) { t.Run("empty ID", func(t *testing.T) { // verify that the client does not create a request if the network-ID/name is empty. _, err := client.NetworkInspect(context.Background(), "", network.InspectOptions{}) - assert.Check(t, is.ErrorType(err, errdefs.IsNotFound)) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, err = client.NetworkInspect(context.Background(), " ", network.InspectOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) }) t.Run("no options", func(t *testing.T) { r, err := client.NetworkInspect(context.Background(), "network_id", network.InspectOptions{}) diff --git a/client/network_remove.go b/client/network_remove.go index 9d6c6cef07..89fdaaf3a8 100644 --- a/client/network_remove.go +++ b/client/network_remove.go @@ -4,6 +4,10 @@ import "context" // NetworkRemove removes an existent network from the docker host. func (cli *Client) NetworkRemove(ctx context.Context, networkID string) error { + networkID, err := trimID("network", networkID) + if err != nil { + return err + } resp, err := cli.delete(ctx, "/networks/"+networkID, nil, nil) defer ensureReaderClosed(resp) return err diff --git a/client/network_remove_test.go b/client/network_remove_test.go index 3a8a39cb18..3316f24344 100644 --- a/client/network_remove_test.go +++ b/client/network_remove_test.go @@ -21,6 +21,14 @@ func TestNetworkRemoveError(t *testing.T) { err := client.NetworkRemove(context.Background(), "network_id") assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.NetworkRemove(context.Background(), "") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.NetworkRemove(context.Background(), " ") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestNetworkRemove(t *testing.T) { diff --git a/client/node_inspect.go b/client/node_inspect.go index 95ab9b1be0..458fe616d9 100644 --- a/client/node_inspect.go +++ b/client/node_inspect.go @@ -11,8 +11,9 @@ import ( // NodeInspectWithRaw returns the node information. func (cli *Client) NodeInspectWithRaw(ctx context.Context, nodeID string) (swarm.Node, []byte, error) { - if nodeID == "" { - return swarm.Node{}, nil, objectNotFoundError{object: "node", id: nodeID} + nodeID, err := trimID("node", nodeID) + if err != nil { + return swarm.Node{}, nil, err } serverResp, err := cli.get(ctx, "/nodes/"+nodeID, nil, nil) defer ensureReaderClosed(serverResp) diff --git a/client/node_inspect_test.go b/client/node_inspect_test.go index ae547b6688..a0710c2ba7 100644 --- a/client/node_inspect_test.go +++ b/client/node_inspect_test.go @@ -42,7 +42,12 @@ func TestNodeInspectWithEmptyID(t *testing.T) { }), } _, _, err := client.NodeInspectWithRaw(context.Background(), "") - assert.Check(t, is.ErrorType(err, errdefs.IsNotFound)) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, _, err = client.NodeInspectWithRaw(context.Background(), " ") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestNodeInspect(t *testing.T) { diff --git a/client/node_remove.go b/client/node_remove.go index e44436debc..81f8fed6b5 100644 --- a/client/node_remove.go +++ b/client/node_remove.go @@ -9,6 +9,11 @@ import ( // NodeRemove removes a Node. func (cli *Client) NodeRemove(ctx context.Context, nodeID string, options types.NodeRemoveOptions) error { + nodeID, err := trimID("node", nodeID) + if err != nil { + return err + } + query := url.Values{} if options.Force { query.Set("force", "1") diff --git a/client/node_remove_test.go b/client/node_remove_test.go index b6c5065c6e..ab5a263e88 100644 --- a/client/node_remove_test.go +++ b/client/node_remove_test.go @@ -22,6 +22,14 @@ func TestNodeRemoveError(t *testing.T) { err := client.NodeRemove(context.Background(), "node_id", types.NodeRemoveOptions{Force: false}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.NodeRemove(context.Background(), "", types.NodeRemoveOptions{Force: false}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.NodeRemove(context.Background(), " ", types.NodeRemoveOptions{Force: false}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestNodeRemove(t *testing.T) { diff --git a/client/node_update.go b/client/node_update.go index 0d0fc3b788..10e2186615 100644 --- a/client/node_update.go +++ b/client/node_update.go @@ -9,6 +9,11 @@ import ( // NodeUpdate updates a Node. func (cli *Client) NodeUpdate(ctx context.Context, nodeID string, version swarm.Version, node swarm.NodeSpec) error { + nodeID, err := trimID("node", nodeID) + if err != nil { + return err + } + query := url.Values{} query.Set("version", version.String()) resp, err := cli.post(ctx, "/nodes/"+nodeID+"/update", query, node, nil) diff --git a/client/node_update_test.go b/client/node_update_test.go index a9e167bd6b..f0a8a368a9 100644 --- a/client/node_update_test.go +++ b/client/node_update_test.go @@ -22,6 +22,14 @@ func TestNodeUpdateError(t *testing.T) { err := client.NodeUpdate(context.Background(), "node_id", swarm.Version{}, swarm.NodeSpec{}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.NodeUpdate(context.Background(), "", swarm.Version{}, swarm.NodeSpec{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.NodeUpdate(context.Background(), " ", swarm.Version{}, swarm.NodeSpec{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestNodeUpdate(t *testing.T) { diff --git a/client/plugin_disable.go b/client/plugin_disable.go index 01f6574f95..9fabe77bf6 100644 --- a/client/plugin_disable.go +++ b/client/plugin_disable.go @@ -9,6 +9,10 @@ import ( // PluginDisable disables a plugin func (cli *Client) PluginDisable(ctx context.Context, name string, options types.PluginDisableOptions) error { + name, err := trimID("plugin", name) + if err != nil { + return err + } query := url.Values{} if options.Force { query.Set("force", "1") diff --git a/client/plugin_disable_test.go b/client/plugin_disable_test.go index 158e5e5a93..45ec3a8ffb 100644 --- a/client/plugin_disable_test.go +++ b/client/plugin_disable_test.go @@ -22,6 +22,14 @@ func TestPluginDisableError(t *testing.T) { err := client.PluginDisable(context.Background(), "plugin_name", types.PluginDisableOptions{}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.PluginDisable(context.Background(), "", types.PluginDisableOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.PluginDisable(context.Background(), " ", types.PluginDisableOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestPluginDisable(t *testing.T) { diff --git a/client/plugin_enable.go b/client/plugin_enable.go index 736da48bd1..492d0bcff5 100644 --- a/client/plugin_enable.go +++ b/client/plugin_enable.go @@ -10,6 +10,10 @@ import ( // PluginEnable enables a plugin func (cli *Client) PluginEnable(ctx context.Context, name string, options types.PluginEnableOptions) error { + name, err := trimID("plugin", name) + if err != nil { + return err + } query := url.Values{} query.Set("timeout", strconv.Itoa(options.Timeout)) diff --git a/client/plugin_enable_test.go b/client/plugin_enable_test.go index 645753fae8..ba01c61f4e 100644 --- a/client/plugin_enable_test.go +++ b/client/plugin_enable_test.go @@ -22,6 +22,14 @@ func TestPluginEnableError(t *testing.T) { err := client.PluginEnable(context.Background(), "plugin_name", types.PluginEnableOptions{}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.PluginEnable(context.Background(), "", types.PluginEnableOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.PluginEnable(context.Background(), " ", types.PluginEnableOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestPluginEnable(t *testing.T) { diff --git a/client/plugin_inspect.go b/client/plugin_inspect.go index f09e460660..6d8bb94df1 100644 --- a/client/plugin_inspect.go +++ b/client/plugin_inspect.go @@ -11,8 +11,9 @@ import ( // PluginInspectWithRaw inspects an existing plugin func (cli *Client) PluginInspectWithRaw(ctx context.Context, name string) (*types.Plugin, []byte, error) { - if name == "" { - return nil, nil, objectNotFoundError{object: "plugin", id: name} + name, err := trimID("plugin", name) + if err != nil { + return nil, nil, err } resp, err := cli.get(ctx, "/plugins/"+name+"/json", nil, nil) defer ensureReaderClosed(resp) diff --git a/client/plugin_inspect_test.go b/client/plugin_inspect_test.go index 015adf1ad1..02474ca66e 100644 --- a/client/plugin_inspect_test.go +++ b/client/plugin_inspect_test.go @@ -33,7 +33,12 @@ func TestPluginInspectWithEmptyID(t *testing.T) { }), } _, _, err := client.PluginInspectWithRaw(context.Background(), "") - assert.Check(t, is.ErrorType(err, errdefs.IsNotFound)) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, _, err = client.PluginInspectWithRaw(context.Background(), " ") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestPluginInspect(t *testing.T) { diff --git a/client/plugin_push.go b/client/plugin_push.go index 8f68a86eee..4624386dc2 100644 --- a/client/plugin_push.go +++ b/client/plugin_push.go @@ -10,6 +10,10 @@ import ( // PluginPush pushes a plugin to a registry func (cli *Client) PluginPush(ctx context.Context, name string, registryAuth string) (io.ReadCloser, error) { + name, err := trimID("plugin", name) + if err != nil { + return nil, err + } resp, err := cli.post(ctx, "/plugins/"+name+"/push", nil, nil, http.Header{ registry.AuthHeader: {registryAuth}, }) diff --git a/client/plugin_push_test.go b/client/plugin_push_test.go index 4e42fb0247..2e593f5524 100644 --- a/client/plugin_push_test.go +++ b/client/plugin_push_test.go @@ -22,6 +22,14 @@ func TestPluginPushError(t *testing.T) { _, err := client.PluginPush(context.Background(), "plugin_name", "") assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + _, err = client.PluginPush(context.Background(), "", "") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, err = client.PluginPush(context.Background(), " ", "") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestPluginPush(t *testing.T) { diff --git a/client/plugin_remove.go b/client/plugin_remove.go index 4cd66958c3..6ee107e3cc 100644 --- a/client/plugin_remove.go +++ b/client/plugin_remove.go @@ -9,6 +9,11 @@ import ( // PluginRemove removes a plugin func (cli *Client) PluginRemove(ctx context.Context, name string, options types.PluginRemoveOptions) error { + name, err := trimID("plugin", name) + if err != nil { + return err + } + query := url.Values{} if options.Force { query.Set("force", "1") diff --git a/client/plugin_remove_test.go b/client/plugin_remove_test.go index 9364ed2156..48ac649198 100644 --- a/client/plugin_remove_test.go +++ b/client/plugin_remove_test.go @@ -22,6 +22,14 @@ func TestPluginRemoveError(t *testing.T) { err := client.PluginRemove(context.Background(), "plugin_name", types.PluginRemoveOptions{}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.PluginRemove(context.Background(), "", types.PluginRemoveOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.PluginRemove(context.Background(), " ", types.PluginRemoveOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestPluginRemove(t *testing.T) { diff --git a/client/plugin_set.go b/client/plugin_set.go index dcf5752ca2..e2a79838d5 100644 --- a/client/plugin_set.go +++ b/client/plugin_set.go @@ -6,6 +6,11 @@ import ( // PluginSet modifies settings for an existing plugin func (cli *Client) PluginSet(ctx context.Context, name string, args []string) error { + name, err := trimID("plugin", name) + if err != nil { + return err + } + resp, err := cli.post(ctx, "/plugins/"+name+"/set", nil, args, nil) ensureReaderClosed(resp) return err diff --git a/client/plugin_set_test.go b/client/plugin_set_test.go index 31d71ff715..4a62db8152 100644 --- a/client/plugin_set_test.go +++ b/client/plugin_set_test.go @@ -21,6 +21,14 @@ func TestPluginSetError(t *testing.T) { err := client.PluginSet(context.Background(), "plugin_name", []string{}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.PluginSet(context.Background(), "", []string{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.PluginSet(context.Background(), " ", []string{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestPluginSet(t *testing.T) { diff --git a/client/plugin_upgrade.go b/client/plugin_upgrade.go index 5cade450f4..58f0d5f128 100644 --- a/client/plugin_upgrade.go +++ b/client/plugin_upgrade.go @@ -13,7 +13,12 @@ import ( ) // PluginUpgrade upgrades a plugin -func (cli *Client) PluginUpgrade(ctx context.Context, name string, options types.PluginInstallOptions) (rc io.ReadCloser, err error) { +func (cli *Client) PluginUpgrade(ctx context.Context, name string, options types.PluginInstallOptions) (io.ReadCloser, error) { + name, err := trimID("plugin", name) + if err != nil { + return nil, err + } + if err := cli.NewVersionError(ctx, "1.26", "plugin upgrade"); err != nil { return nil, err } diff --git a/client/secret_inspect.go b/client/secret_inspect.go index a9cb59889b..cf160c601d 100644 --- a/client/secret_inspect.go +++ b/client/secret_inspect.go @@ -11,11 +11,12 @@ import ( // SecretInspectWithRaw returns the secret information with raw data func (cli *Client) SecretInspectWithRaw(ctx context.Context, id string) (swarm.Secret, []byte, error) { - if err := cli.NewVersionError(ctx, "1.25", "secret inspect"); err != nil { + id, err := trimID("secret", id) + if err != nil { return swarm.Secret{}, nil, err } - if id == "" { - return swarm.Secret{}, nil, objectNotFoundError{object: "secret", id: id} + if err := cli.NewVersionError(ctx, "1.25", "secret inspect"); err != nil { + return swarm.Secret{}, nil, err } resp, err := cli.get(ctx, "/secrets/"+id, nil, nil) defer ensureReaderClosed(resp) diff --git a/client/secret_inspect_test.go b/client/secret_inspect_test.go index 187f2a4b67..67ee5175bc 100644 --- a/client/secret_inspect_test.go +++ b/client/secret_inspect_test.go @@ -53,7 +53,12 @@ func TestSecretInspectWithEmptyID(t *testing.T) { }), } _, _, err := client.SecretInspectWithRaw(context.Background(), "") - assert.Check(t, is.ErrorType(err, errdefs.IsNotFound)) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, _, err = client.SecretInspectWithRaw(context.Background(), " ") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestSecretInspect(t *testing.T) { diff --git a/client/secret_remove.go b/client/secret_remove.go index 079ed67394..7ea2acbf52 100644 --- a/client/secret_remove.go +++ b/client/secret_remove.go @@ -4,6 +4,10 @@ import "context" // SecretRemove removes a secret. func (cli *Client) SecretRemove(ctx context.Context, id string) error { + id, err := trimID("secret", id) + if err != nil { + return err + } if err := cli.NewVersionError(ctx, "1.25", "secret remove"); err != nil { return err } diff --git a/client/secret_remove_test.go b/client/secret_remove_test.go index 9edd177f18..9328d9a5f6 100644 --- a/client/secret_remove_test.go +++ b/client/secret_remove_test.go @@ -31,6 +31,14 @@ func TestSecretRemoveError(t *testing.T) { err := client.SecretRemove(context.Background(), "secret_id") assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.SecretRemove(context.Background(), "") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.SecretRemove(context.Background(), " ") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestSecretRemove(t *testing.T) { diff --git a/client/secret_update.go b/client/secret_update.go index 9dfe67198b..60d21a6f2c 100644 --- a/client/secret_update.go +++ b/client/secret_update.go @@ -9,6 +9,10 @@ import ( // SecretUpdate attempts to update a secret. func (cli *Client) SecretUpdate(ctx context.Context, id string, version swarm.Version, secret swarm.SecretSpec) error { + id, err := trimID("secret", id) + if err != nil { + return err + } if err := cli.NewVersionError(ctx, "1.25", "secret update"); err != nil { return err } diff --git a/client/secret_update_test.go b/client/secret_update_test.go index a20cd626b8..c41d584bdb 100644 --- a/client/secret_update_test.go +++ b/client/secret_update_test.go @@ -32,6 +32,14 @@ func TestSecretUpdateError(t *testing.T) { err := client.SecretUpdate(context.Background(), "secret_id", swarm.Version{}, swarm.SecretSpec{}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.SecretUpdate(context.Background(), "", swarm.Version{}, swarm.SecretSpec{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.SecretUpdate(context.Background(), " ", swarm.Version{}, swarm.SecretSpec{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestSecretUpdate(t *testing.T) { @@ -41,7 +49,7 @@ func TestSecretUpdate(t *testing.T) { version: "1.25", client: newMockClient(func(req *http.Request) (*http.Response, error) { if !strings.HasPrefix(req.URL.Path, expectedURL) { - return nil, fmt.Errorf("Expected URL '%s', got '%s'", expectedURL, req.URL) + return nil, fmt.Errorf("expected URL '%s', got '%s'", expectedURL, req.URL) } if req.Method != http.MethodPost { return nil, fmt.Errorf("expected POST method, got %s", req.Method) diff --git a/client/service_inspect.go b/client/service_inspect.go index cee020c98b..9efcca5800 100644 --- a/client/service_inspect.go +++ b/client/service_inspect.go @@ -14,9 +14,11 @@ import ( // ServiceInspectWithRaw returns the service information and the raw data. func (cli *Client) ServiceInspectWithRaw(ctx context.Context, serviceID string, opts types.ServiceInspectOptions) (swarm.Service, []byte, error) { - if serviceID == "" { - return swarm.Service{}, nil, objectNotFoundError{object: "service", id: serviceID} + serviceID, err := trimID("service", serviceID) + if err != nil { + return swarm.Service{}, nil, err } + query := url.Values{} query.Set("insertDefaults", fmt.Sprintf("%v", opts.InsertDefaults)) serverResp, err := cli.get(ctx, "/services/"+serviceID, query, nil) diff --git a/client/service_inspect_test.go b/client/service_inspect_test.go index 9acd7c1034..4fbbdcf4d2 100644 --- a/client/service_inspect_test.go +++ b/client/service_inspect_test.go @@ -43,7 +43,12 @@ func TestServiceInspectWithEmptyID(t *testing.T) { }), } _, _, err := client.ServiceInspectWithRaw(context.Background(), "", types.ServiceInspectOptions{}) - assert.Check(t, is.ErrorType(err, errdefs.IsNotFound)) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, _, err = client.ServiceInspectWithRaw(context.Background(), " ", types.ServiceInspectOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestServiceInspect(t *testing.T) { diff --git a/client/service_logs.go b/client/service_logs.go index e9e30a2ab4..9281194acd 100644 --- a/client/service_logs.go +++ b/client/service_logs.go @@ -14,6 +14,11 @@ import ( // ServiceLogs returns the logs generated by a service in an io.ReadCloser. // It's up to the caller to close the stream. func (cli *Client) ServiceLogs(ctx context.Context, serviceID string, options container.LogsOptions) (io.ReadCloser, error) { + serviceID, err := trimID("service", serviceID) + if err != nil { + return nil, err + } + query := url.Values{} if options.ShowStdout { query.Set("stdout", "1") diff --git a/client/service_logs_test.go b/client/service_logs_test.go index 0c557685be..917ebf602f 100644 --- a/client/service_logs_test.go +++ b/client/service_logs_test.go @@ -29,6 +29,14 @@ func TestServiceLogsError(t *testing.T) { Since: "2006-01-02TZ", }) assert.Check(t, is.ErrorContains(err, `parsing time "2006-01-02TZ"`)) + + _, err = client.ServiceLogs(context.Background(), "", container.LogsOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, err = client.ServiceLogs(context.Background(), " ", container.LogsOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestServiceLogs(t *testing.T) { diff --git a/client/service_remove.go b/client/service_remove.go index 2c46326ebc..93c949e44a 100644 --- a/client/service_remove.go +++ b/client/service_remove.go @@ -4,6 +4,11 @@ import "context" // ServiceRemove kills and removes a service. func (cli *Client) ServiceRemove(ctx context.Context, serviceID string) error { + serviceID, err := trimID("service", serviceID) + if err != nil { + return err + } + resp, err := cli.delete(ctx, "/services/"+serviceID, nil, nil) defer ensureReaderClosed(resp) return err diff --git a/client/service_remove_test.go b/client/service_remove_test.go index 15ea22db7b..196c17df46 100644 --- a/client/service_remove_test.go +++ b/client/service_remove_test.go @@ -21,6 +21,14 @@ func TestServiceRemoveError(t *testing.T) { err := client.ServiceRemove(context.Background(), "service_id") assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.ServiceRemove(context.Background(), "") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.ServiceRemove(context.Background(), " ") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestServiceRemoveNotFoundError(t *testing.T) { diff --git a/client/service_update.go b/client/service_update.go index d2f03f02f0..aa7da207c2 100644 --- a/client/service_update.go +++ b/client/service_update.go @@ -16,7 +16,10 @@ import ( // It should be the value as set *before* the update. You can find this value in the Meta field // of swarm.Service, which can be found using ServiceInspectWithRaw. func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version swarm.Version, service swarm.ServiceSpec, options types.ServiceUpdateOptions) (swarm.ServiceUpdateResponse, error) { - response := swarm.ServiceUpdateResponse{} + serviceID, err := trimID("service", serviceID) + if err != nil { + return swarm.ServiceUpdateResponse{}, err + } // Make sure we negotiated (if the client is configured to do so), // as code below contains API-version specific handling of options. @@ -24,7 +27,7 @@ func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version // Normally, version-negotiation (if enabled) would not happen until // the API request is made. if err := cli.checkVersion(ctx); err != nil { - return response, err + return swarm.ServiceUpdateResponse{}, err } query := url.Values{} @@ -39,7 +42,7 @@ func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version query.Set("version", version.String()) if err := validateServiceSpec(service); err != nil { - return response, err + return swarm.ServiceUpdateResponse{}, err } // ensure that the image is tagged @@ -74,9 +77,10 @@ func (cli *Client) ServiceUpdate(ctx context.Context, serviceID string, version resp, err := cli.post(ctx, "/services/"+serviceID+"/update", query, service, headers) defer ensureReaderClosed(resp) if err != nil { - return response, err + return swarm.ServiceUpdateResponse{}, err } + response := swarm.ServiceUpdateResponse{} err = json.NewDecoder(resp.body).Decode(&response) if resolveWarning != "" { response.Warnings = append(response.Warnings, resolveWarning) diff --git a/client/service_update_test.go b/client/service_update_test.go index 683c03f815..893d0ab103 100644 --- a/client/service_update_test.go +++ b/client/service_update_test.go @@ -23,6 +23,14 @@ func TestServiceUpdateError(t *testing.T) { _, err := client.ServiceUpdate(context.Background(), "service_id", swarm.Version{}, swarm.ServiceSpec{}, types.ServiceUpdateOptions{}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + _, err = client.ServiceUpdate(context.Background(), "", swarm.Version{}, swarm.ServiceSpec{}, types.ServiceUpdateOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, err = client.ServiceUpdate(context.Background(), " ", swarm.Version{}, swarm.ServiceSpec{}, types.ServiceUpdateOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } // TestServiceUpdateConnectionError verifies that connection errors occurring diff --git a/client/task_inspect.go b/client/task_inspect.go index dde1f6c59d..898ad3e455 100644 --- a/client/task_inspect.go +++ b/client/task_inspect.go @@ -11,9 +11,11 @@ import ( // TaskInspectWithRaw returns the task information and its raw representation. func (cli *Client) TaskInspectWithRaw(ctx context.Context, taskID string) (swarm.Task, []byte, error) { - if taskID == "" { - return swarm.Task{}, nil, objectNotFoundError{object: "task", id: taskID} + taskID, err := trimID("task", taskID) + if err != nil { + return swarm.Task{}, nil, err } + serverResp, err := cli.get(ctx, "/tasks/"+taskID, nil, nil) defer ensureReaderClosed(serverResp) if err != nil { diff --git a/client/task_inspect_test.go b/client/task_inspect_test.go index 7febd5a2c6..03db38b505 100644 --- a/client/task_inspect_test.go +++ b/client/task_inspect_test.go @@ -33,7 +33,12 @@ func TestTaskInspectWithEmptyID(t *testing.T) { }), } _, _, err := client.TaskInspectWithRaw(context.Background(), "") - assert.Check(t, is.ErrorType(err, errdefs.IsNotFound)) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, _, err = client.TaskInspectWithRaw(context.Background(), " ") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestTaskInspect(t *testing.T) { diff --git a/client/utils.go b/client/utils.go index 8cbd671792..925d4d8d38 100644 --- a/client/utils.go +++ b/client/utils.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "net/url" + "strings" "github.com/docker/docker/api/types/filters" "github.com/docker/docker/errdefs" @@ -13,6 +14,23 @@ import ( var headerRegexp = lazyregexp.New(`\ADocker/.+\s\((.+)\)\z`) +type emptyIDError string + +func (e emptyIDError) InvalidParameter() {} + +func (e emptyIDError) Error() string { + return "invalid " + string(e) + " name or ID: value is empty" +} + +// trimID trims the given object-ID / name, returning an error if it's empty. +func trimID(objType, id string) (string, error) { + id = strings.TrimSpace(id) + if len(id) == 0 { + return "", emptyIDError(objType) + } + return id, nil +} + // getDockerOS returns the operating system based on the server header from the daemon. func getDockerOS(serverHeader string) string { var osType string diff --git a/client/volume_inspect.go b/client/volume_inspect.go index b3ba4e6046..d841d1c59e 100644 --- a/client/volume_inspect.go +++ b/client/volume_inspect.go @@ -17,8 +17,9 @@ func (cli *Client) VolumeInspect(ctx context.Context, volumeID string) (volume.V // VolumeInspectWithRaw returns the information about a specific volume in the docker host and its raw representation func (cli *Client) VolumeInspectWithRaw(ctx context.Context, volumeID string) (volume.Volume, []byte, error) { - if volumeID == "" { - return volume.Volume{}, nil, objectNotFoundError{object: "volume", id: volumeID} + volumeID, err := trimID("volume", volumeID) + if err != nil { + return volume.Volume{}, nil, err } var vol volume.Volume diff --git a/client/volume_inspect_test.go b/client/volume_inspect_test.go index e5ad8f81f6..6bf245f323 100644 --- a/client/volume_inspect_test.go +++ b/client/volume_inspect_test.go @@ -42,7 +42,12 @@ func TestVolumeInspectWithEmptyID(t *testing.T) { }), } _, _, err := client.VolumeInspectWithRaw(context.Background(), "") - assert.Check(t, is.ErrorType(err, errdefs.IsNotFound)) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + _, _, err = client.VolumeInspectWithRaw(context.Background(), " ") + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestVolumeInspect(t *testing.T) { diff --git a/client/volume_remove.go b/client/volume_remove.go index b8bdc5ae85..eefd9ce437 100644 --- a/client/volume_remove.go +++ b/client/volume_remove.go @@ -9,6 +9,11 @@ import ( // VolumeRemove removes a volume from the docker host. func (cli *Client) VolumeRemove(ctx context.Context, volumeID string, force bool) error { + volumeID, err := trimID("volume", volumeID) + if err != nil { + return err + } + query := url.Values{} if force { // Make sure we negotiated (if the client is configured to do so), diff --git a/client/volume_remove_test.go b/client/volume_remove_test.go index f242a3892d..11fa99d033 100644 --- a/client/volume_remove_test.go +++ b/client/volume_remove_test.go @@ -21,6 +21,14 @@ func TestVolumeRemoveError(t *testing.T) { err := client.VolumeRemove(context.Background(), "volume_id", false) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.VolumeRemove(context.Background(), "", false) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.VolumeRemove(context.Background(), " ", false) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } // TestVolumeRemoveConnectionError verifies that connection errors occurring diff --git a/client/volume_update.go b/client/volume_update.go index 151863f07a..c91d5e984e 100644 --- a/client/volume_update.go +++ b/client/volume_update.go @@ -11,6 +11,10 @@ import ( // VolumeUpdate updates a volume. This only works for Cluster Volumes, and // only some fields can be updated. func (cli *Client) VolumeUpdate(ctx context.Context, volumeID string, version swarm.Version, options volume.UpdateOptions) error { + volumeID, err := trimID("volume", volumeID) + if err != nil { + return err + } if err := cli.NewVersionError(ctx, "1.42", "volume update"); err != nil { return err } diff --git a/client/volume_update_test.go b/client/volume_update_test.go index faec7ab440..64cc7e64c4 100644 --- a/client/volume_update_test.go +++ b/client/volume_update_test.go @@ -21,8 +21,16 @@ func TestVolumeUpdateError(t *testing.T) { client: newMockClient(errorMock(http.StatusInternalServerError, "Server error")), } - err := client.VolumeUpdate(context.Background(), "", swarm.Version{}, volumetypes.UpdateOptions{}) + err := client.VolumeUpdate(context.Background(), "volume", swarm.Version{}, volumetypes.UpdateOptions{}) assert.Check(t, is.ErrorType(err, errdefs.IsSystem)) + + err = client.VolumeUpdate(context.Background(), "", swarm.Version{}, volumetypes.UpdateOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) + + err = client.VolumeUpdate(context.Background(), " ", swarm.Version{}, volumetypes.UpdateOptions{}) + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(t, is.ErrorContains(err, "value is empty")) } func TestVolumeUpdate(t *testing.T) { diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go index dd96adbb2f..623a5cefcb 100644 --- a/integration-cli/docker_api_containers_test.go +++ b/integration-cli/docker_api_containers_test.go @@ -1391,7 +1391,8 @@ func (s *DockerAPISuite) TestContainerAPIDeleteWithEmptyName(c *testing.T) { defer apiClient.Close() err = apiClient.ContainerRemove(testutil.GetContext(c), "", container.RemoveOptions{}) - assert.Check(c, errdefs.IsNotFound(err)) + assert.Check(c, is.ErrorType(err, errdefs.IsInvalidParameter)) + assert.Check(c, is.ErrorContains(err, "value is empty")) } func (s *DockerAPISuite) TestContainerAPIStatsWithNetworkDisabled(c *testing.T) {