Merge pull request #36144 from emil2k/node-id-required

Produce errors when empty ids are passed into inspect calls.

client/config_inspect.go

@@ -11,6 +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}
+	}
 	if err := cli.NewVersionError("1.30", "config inspect"); err != nil {
 		return swarm.Config{}, nil, err
 	}

client/config_inspect_test.go

@@ -10,10 +10,34 @@ import (
 	"testing"
 
 	"github.com/docker/docker/api/types/swarm"
+	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"golang.org/x/net/context"
 )
 
+func TestConfigInspectNotFound(t *testing.T) {
+	client := &Client{
+		client: newMockClient(errorMock(http.StatusNotFound, "Server error")),
+	}
+
+	_, _, err := client.ConfigInspectWithRaw(context.Background(), "unknown")
+	if err == nil || !IsErrNotFound(err) {
+		t.Fatalf("expected a NotFoundError error, got %v", err)
+	}
+}
+
+func TestConfigInspectWithEmptyID(t *testing.T) {
+	client := &Client{
+		client: newMockClient(func(req *http.Request) (*http.Response, error) {
+			return nil, errors.New("should not make request")
+		}),
+	}
+	_, _, err := client.ConfigInspectWithRaw(context.Background(), "")
+	if !IsErrNotFound(err) {
+		t.Fatalf("Expected NotFoundError, got %v", err)
+	}
+}
+
 func TestConfigInspectUnsupported(t *testing.T) {
 	client := &Client{
 		version: "1.29",

client/container_inspect.go

@@ -12,6 +12,9 @@ import (
 
 // ContainerInspect returns the container information.
 func (cli *Client) ContainerInspect(ctx context.Context, containerID string) (types.ContainerJSON, error) {
+	if containerID == "" {
+		return types.ContainerJSON{}, objectNotFoundError{object: "container", id: containerID}
+	}
 	serverResp, err := cli.get(ctx, "/containers/"+containerID+"/json", nil, nil)
 	if err != nil {
 		return types.ContainerJSON{}, wrapResponseError(err, serverResp, "container", containerID)
@@ -25,6 +28,9 @@ func (cli *Client) ContainerInspect(ctx context.Context, containerID string) (ty
 
 // ContainerInspectWithRaw returns the container information and its raw representation.
 func (cli *Client) ContainerInspectWithRaw(ctx context.Context, containerID string, getSize bool) (types.ContainerJSON, []byte, error) {
+	if containerID == "" {
+		return types.ContainerJSON{}, nil, objectNotFoundError{object: "container", id: containerID}
+	}
 	query := url.Values{}
 	if getSize {
 		query.Set("size", "1")

client/container_inspect_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 
 	"github.com/docker/docker/api/types"
+	"github.com/pkg/errors"
 	"golang.org/x/net/context"
 )
 
@@ -35,6 +36,18 @@ func TestContainerInspectContainerNotFound(t *testing.T) {
 	}
 }
 
+func TestContainerInspectWithEmptyID(t *testing.T) {
+	client := &Client{
+		client: newMockClient(func(req *http.Request) (*http.Response, error) {
+			return nil, errors.New("should not make request")
+		}),
+	}
+	_, _, err := client.ContainerInspectWithRaw(context.Background(), "", true)
+	if !IsErrNotFound(err) {
+		t.Fatalf("Expected NotFoundError, got %v", err)
+	}
+}
+
 func TestContainerInspect(t *testing.T) {
 	expectedURL := "/containers/container_id/json"
 	client := &Client{

client/distribution_inspect.go

@@ -12,6 +12,9 @@ import (
 func (cli *Client) DistributionInspect(ctx context.Context, image, encodedRegistryAuth string) (registrytypes.DistributionInspect, error) {
 	// Contact the registry to retrieve digest and platform information
 	var distributionInspect registrytypes.DistributionInspect
+	if image == "" {
+		return distributionInspect, objectNotFoundError{object: "distribution", id: image}
+	}
 
 	if err := cli.NewVersionError("1.30", "distribution inspect"); err != nil {
 		return distributionInspect, err

client/distribution_inspect_test.go

@@ -4,6 +4,7 @@ import (
 	"net/http"
 	"testing"
 
+	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"golang.org/x/net/context"
 )
@@ -16,3 +17,15 @@ func TestDistributionInspectUnsupported(t *testing.T) {
 	_, err := client.DistributionInspect(context.Background(), "foobar:1.0", "")
 	assert.EqualError(t, err, `"distribution inspect" requires API version 1.30, but the Docker daemon API version is 1.29`)
 }
+
+func TestDistributionInspectWithEmptyID(t *testing.T) {
+	client := &Client{
+		client: newMockClient(func(req *http.Request) (*http.Response, error) {
+			return nil, errors.New("should not make request")
+		}),
+	}
+	_, err := client.DistributionInspect(context.Background(), "", "")
+	if !IsErrNotFound(err) {
+		t.Fatalf("Expected NotFoundError, got %v", err)
+	}
+}

client/image_inspect.go

@@ -11,6 +11,9 @@ import (
 
 // ImageInspectWithRaw returns the image information and its raw representation.
 func (cli *Client) ImageInspectWithRaw(ctx context.Context, imageID string) (types.ImageInspect, []byte, error) {
+	if imageID == "" {
+		return types.ImageInspect{}, nil, objectNotFoundError{object: "image", id: imageID}
+	}
 	serverResp, err := cli.get(ctx, "/images/"+imageID+"/json", nil, nil)
 	if err != nil {
 		return types.ImageInspect{}, nil, wrapResponseError(err, serverResp, "image", imageID)

client/image_inspect_test.go

@@ -11,6 +11,7 @@ import (
 	"testing"
 
 	"github.com/docker/docker/api/types"
+	"github.com/pkg/errors"
 	"golang.org/x/net/context"
 )
 
@@ -36,6 +37,18 @@ func TestImageInspectImageNotFound(t *testing.T) {
 	}
 }
 
+func TestImageInspectWithEmptyID(t *testing.T) {
+	client := &Client{
+		client: newMockClient(func(req *http.Request) (*http.Response, error) {
+			return nil, errors.New("should not make request")
+		}),
+	}
+	_, _, err := client.ImageInspectWithRaw(context.Background(), "")
+	if !IsErrNotFound(err) {
+		t.Fatalf("Expected NotFoundError, got %v", err)
+	}
+}
+
 func TestImageInspect(t *testing.T) {
 	expectedURL := "/images/image_id/json"
 	expectedTags := []string{"tag1", "tag2"}

client/network_inspect.go

@@ -18,6 +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 types.NetworkInspectOptions) (types.NetworkResource, []byte, error) {
+	if networkID == "" {
+		return types.NetworkResource{}, nil, objectNotFoundError{object: "network", id: networkID}
+	}
 	var (
 		networkResource types.NetworkResource
 		resp            serverResponse

client/network_inspect_test.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/network"
+	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"golang.org/x/net/context"
 )
@@ -34,6 +35,18 @@ func TestNetworkInspectNotFoundError(t *testing.T) {
 	assert.True(t, IsErrNotFound(err))
 }
 
+func TestNetworkInspectWithEmptyID(t *testing.T) {
+	client := &Client{
+		client: newMockClient(func(req *http.Request) (*http.Response, error) {
+			return nil, errors.New("should not make request")
+		}),
+	}
+	_, _, err := client.NetworkInspectWithRaw(context.Background(), "", types.NetworkInspectOptions{})
+	if !IsErrNotFound(err) {
+		t.Fatalf("Expected NotFoundError, got %v", err)
+	}
+}
+
 func TestNetworkInspect(t *testing.T) {
 	expectedURL := "/networks/network_id"
 	client := &Client{

client/node_inspect.go

@@ -11,6 +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}
+	}
 	serverResp, err := cli.get(ctx, "/nodes/"+nodeID, nil, nil)
 	if err != nil {
 		return swarm.Node{}, nil, wrapResponseError(err, serverResp, "node", nodeID)

client/node_inspect_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 
 	"github.com/docker/docker/api/types/swarm"
+	"github.com/pkg/errors"
 	"golang.org/x/net/context"
 )
 
@@ -35,6 +36,18 @@ func TestNodeInspectNodeNotFound(t *testing.T) {
 	}
 }
 
+func TestNodeInspectWithEmptyID(t *testing.T) {
+	client := &Client{
+		client: newMockClient(func(req *http.Request) (*http.Response, error) {
+			return nil, errors.New("should not make request")
+		}),
+	}
+	_, _, err := client.NodeInspectWithRaw(context.Background(), "")
+	if !IsErrNotFound(err) {
+		t.Fatalf("Expected NotFoundError, got %v", err)
+	}
+}
+
 func TestNodeInspect(t *testing.T) {
 	expectedURL := "/nodes/node_id"
 	client := &Client{

client/plugin_inspect.go

@@ -11,6 +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}
+	}
 	resp, err := cli.get(ctx, "/plugins/"+name+"/json", nil, nil)
 	if err != nil {
 		return nil, nil, wrapResponseError(err, resp, "plugin", name)

client/plugin_inspect_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 
 	"github.com/docker/docker/api/types"
+	"github.com/pkg/errors"
 	"golang.org/x/net/context"
 )
 
@@ -24,6 +25,18 @@ func TestPluginInspectError(t *testing.T) {
 	}
 }
 
+func TestPluginInspectWithEmptyID(t *testing.T) {
+	client := &Client{
+		client: newMockClient(func(req *http.Request) (*http.Response, error) {
+			return nil, errors.New("should not make request")
+		}),
+	}
+	_, _, err := client.PluginInspectWithRaw(context.Background(), "")
+	if !IsErrNotFound(err) {
+		t.Fatalf("Expected NotFoundError, got %v", err)
+	}
+}
+
 func TestPluginInspect(t *testing.T) {
 	expectedURL := "/plugins/plugin_name"
 	client := &Client{

client/secret_inspect.go

@@ -14,6 +14,9 @@ func (cli *Client) SecretInspectWithRaw(ctx context.Context, id string) (swarm.S
 	if err := cli.NewVersionError("1.25", "secret inspect"); err != nil {
 		return swarm.Secret{}, nil, err
 	}
+	if id == "" {
+		return swarm.Secret{}, nil, objectNotFoundError{object: "secret", id: id}
+	}
 	resp, err := cli.get(ctx, "/secrets/"+id, nil, nil)
 	if err != nil {
 		return swarm.Secret{}, nil, wrapResponseError(err, resp, "secret", id)

client/secret_inspect_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 
 	"github.com/docker/docker/api/types/swarm"
+	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"golang.org/x/net/context"
 )
@@ -47,6 +48,18 @@ func TestSecretInspectSecretNotFound(t *testing.T) {
 	}
 }
 
+func TestSecretInspectWithEmptyID(t *testing.T) {
+	client := &Client{
+		client: newMockClient(func(req *http.Request) (*http.Response, error) {
+			return nil, errors.New("should not make request")
+		}),
+	}
+	_, _, err := client.SecretInspectWithRaw(context.Background(), "")
+	if !IsErrNotFound(err) {
+		t.Fatalf("Expected NotFoundError, got %v", err)
+	}
+}
+
 func TestSecretInspect(t *testing.T) {
 	expectedURL := "/v1.25/secrets/secret_id"
 	client := &Client{

client/service_inspect.go

@@ -14,6 +14,9 @@ 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}
+	}
 	query := url.Values{}
 	query.Set("insertDefaults", fmt.Sprintf("%v", opts.InsertDefaults))
 	serverResp, err := cli.get(ctx, "/services/"+serviceID, query, nil)

client/service_inspect_test.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/swarm"
+	"github.com/pkg/errors"
 	"golang.org/x/net/context"
 )
 
@@ -36,6 +37,18 @@ func TestServiceInspectServiceNotFound(t *testing.T) {
 	}
 }
 
+func TestServiceInspectWithEmptyID(t *testing.T) {
+	client := &Client{
+		client: newMockClient(func(req *http.Request) (*http.Response, error) {
+			return nil, errors.New("should not make request")
+		}),
+	}
+	_, _, err := client.ServiceInspectWithRaw(context.Background(), "", types.ServiceInspectOptions{})
+	if !IsErrNotFound(err) {
+		t.Fatalf("Expected NotFoundError, got %v", err)
+	}
+}
+
 func TestServiceInspect(t *testing.T) {
 	expectedURL := "/services/service_id"
 	client := &Client{

client/task_inspect.go

@@ -11,6 +11,9 @@ 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}
+	}
 	serverResp, err := cli.get(ctx, "/tasks/"+taskID, nil, nil)
 	if err != nil {
 		return swarm.Task{}, nil, wrapResponseError(err, serverResp, "task", taskID)

client/task_inspect_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 
 	"github.com/docker/docker/api/types/swarm"
+	"github.com/pkg/errors"
 	"golang.org/x/net/context"
 )
 
@@ -24,6 +25,18 @@ func TestTaskInspectError(t *testing.T) {
 	}
 }
 
+func TestTaskInspectWithEmptyID(t *testing.T) {
+	client := &Client{
+		client: newMockClient(func(req *http.Request) (*http.Response, error) {
+			return nil, errors.New("should not make request")
+		}),
+	}
+	_, _, err := client.TaskInspectWithRaw(context.Background(), "")
+	if !IsErrNotFound(err) {
+		t.Fatalf("Expected NotFoundError, got %v", err)
+	}
+}
+
 func TestTaskInspect(t *testing.T) {
 	expectedURL := "/tasks/task_id"
 	client := &Client{

client/volume_inspect.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"encoding/json"
 	"io/ioutil"
-	"path"
 
 	"github.com/docker/docker/api/types"
 	"golang.org/x/net/context"
@@ -18,15 +17,12 @@ func (cli *Client) VolumeInspect(ctx context.Context, volumeID string) (types.Vo
 
 // 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) (types.Volume, []byte, error) {
-	// The empty ID needs to be handled here because with an empty ID the
-	// request url will not contain a trailing / which calls the volume list API
-	// instead of volume inspect
 	if volumeID == "" {
 		return types.Volume{}, nil, objectNotFoundError{object: "volume", id: volumeID}
 	}
 
 	var volume types.Volume
-	resp, err := cli.get(ctx, path.Join("/volumes", volumeID), nil, nil)
+	resp, err := cli.get(ctx, "/volumes/"+volumeID, nil, nil)
 	if err != nil {
 		return volume, nil, wrapResponseError(err, resp, "volume", volumeID)
 	}

client/volume_inspect_test.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/internal/testutil"
+	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/net/context"
@@ -35,20 +36,15 @@ func TestVolumeInspectNotFound(t *testing.T) {
 }
 
 func TestVolumeInspectWithEmptyID(t *testing.T) {
-	expectedURL := "/volumes/"
-
 	client := &Client{
 		client: newMockClient(func(req *http.Request) (*http.Response, error) {
-			assert.Equal(t, req.URL.Path, expectedURL)
-			return &http.Response{
-				StatusCode: http.StatusNotFound,
-				Body:       ioutil.NopCloser(bytes.NewReader(nil)),
-			}, nil
+			return nil, errors.New("should not make request")
 		}),
 	}
-	_, err := client.VolumeInspect(context.Background(), "")
-	testutil.ErrorContains(t, err, "No such volume: ")
-
+	_, _, err := client.VolumeInspectWithRaw(context.Background(), "")
+	if !IsErrNotFound(err) {
+		t.Fatalf("Expected NotFoundError, got %v", err)
+	}
 }
 
 func TestVolumeInspect(t *testing.T) {