diff --git a/client/client.go b/client/client.go index f7a8c07d3a..85a71b3659 100644 --- a/client/client.go +++ b/client/client.go @@ -220,18 +220,11 @@ func (cli *Client) getAPIPath(p string, query url.Values) string { var apiPath string if cli.version != "" { v := strings.TrimPrefix(cli.version, "v") - apiPath = path.Join(cli.basePath, "/v"+v+p) + apiPath = path.Join(cli.basePath, "/v"+v, p) } else { apiPath = path.Join(cli.basePath, p) } - - u := &url.URL{ - Path: apiPath, - } - if len(query) > 0 { - u.RawQuery = query.Encode() - } - return u.String() + return (&url.URL{Path: apiPath, RawQuery: query.Encode()}).String() } // ClientVersion returns the version string associated with this diff --git a/client/client_test.go b/client/client_test.go index bc911c0c4a..322849d59a 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -104,11 +104,11 @@ func TestNewEnvClient(t *testing.T) { } func TestGetAPIPath(t *testing.T) { - cases := []struct { - v string - p string - q url.Values - e string + testcases := []struct { + version string + path string + query url.Values + expected string }{ {"", "/containers/json", nil, "/containers/json"}, {"", "/containers/json", url.Values{}, "/containers/json"}, @@ -122,16 +122,10 @@ func TestGetAPIPath(t *testing.T) { {"v1.22", "/networks/kiwl$%^", nil, "/v1.22/networks/kiwl$%25%5E"}, } - for _, cs := range cases { - c, err := NewClient("unix:///var/run/docker.sock", cs.v, nil, nil) - if err != nil { - t.Fatal(err) - } - g := c.getAPIPath(cs.p, cs.q) - assert.Equal(t, g, cs.e) - - err = c.Close() - assert.NoError(t, err) + for _, testcase := range testcases { + c := Client{version: testcase.version, basePath: "/"} + actual := c.getAPIPath(testcase.path, testcase.query) + assert.Equal(t, actual, testcase.expected) } } diff --git a/client/volume_inspect.go b/client/volume_inspect.go index 3860e9b22c..4722d467ca 100644 --- a/client/volume_inspect.go +++ b/client/volume_inspect.go @@ -5,6 +5,7 @@ import ( "encoding/json" "io/ioutil" "net/http" + "path" "github.com/docker/docker/api/types" "golang.org/x/net/context" @@ -18,8 +19,15 @@ 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, volumeNotFoundError{volumeID} + } + var volume types.Volume - resp, err := cli.get(ctx, "/volumes/"+volumeID, nil, nil) + resp, err := cli.get(ctx, path.Join("/volumes", volumeID), nil, nil) if err != nil { if resp.statusCode == http.StatusNotFound { return volume, nil, volumeNotFoundError{volumeID} diff --git a/client/volume_inspect_test.go b/client/volume_inspect_test.go index 0d1d118828..7d01f44ed2 100644 --- a/client/volume_inspect_test.go +++ b/client/volume_inspect_test.go @@ -10,6 +10,9 @@ import ( "testing" "github.com/docker/docker/api/types" + "github.com/docker/docker/internal/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "golang.org/x/net/context" ) @@ -19,9 +22,7 @@ func TestVolumeInspectError(t *testing.T) { } _, err := client.VolumeInspect(context.Background(), "nothing") - if err == nil || err.Error() != "Error response from daemon: Server error" { - t.Fatalf("expected a Server Error, got %v", err) - } + testutil.ErrorContains(t, err, "Error response from daemon: Server error") } func TestVolumeInspectNotFound(t *testing.T) { @@ -30,13 +31,34 @@ func TestVolumeInspectNotFound(t *testing.T) { } _, err := client.VolumeInspect(context.Background(), "unknown") - if err == nil || !IsErrVolumeNotFound(err) { - t.Fatalf("expected a volumeNotFound error, got %v", err) + assert.True(t, IsErrNotFound(err)) +} + +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 + }), } + _, err := client.VolumeInspect(context.Background(), "") + testutil.ErrorContains(t, err, "No such volume: ") + } func TestVolumeInspect(t *testing.T) { expectedURL := "/volumes/volume_id" + expected := types.Volume{ + Name: "name", + Driver: "driver", + Mountpoint: "mountpoint", + } + client := &Client{ client: newMockClient(func(req *http.Request) (*http.Response, error) { if !strings.HasPrefix(req.URL.Path, expectedURL) { @@ -45,11 +67,7 @@ func TestVolumeInspect(t *testing.T) { if req.Method != "GET" { return nil, fmt.Errorf("expected GET method, got %s", req.Method) } - content, err := json.Marshal(types.Volume{ - Name: "name", - Driver: "driver", - Mountpoint: "mountpoint", - }) + content, err := json.Marshal(expected) if err != nil { return nil, err } @@ -60,17 +78,7 @@ func TestVolumeInspect(t *testing.T) { }), } - v, err := client.VolumeInspect(context.Background(), "volume_id") - if err != nil { - t.Fatal(err) - } - if v.Name != "name" { - t.Fatalf("expected `name`, got %s", v.Name) - } - if v.Driver != "driver" { - t.Fatalf("expected `driver`, got %s", v.Driver) - } - if v.Mountpoint != "mountpoint" { - t.Fatalf("expected `mountpoint`, got %s", v.Mountpoint) - } + volume, err := client.VolumeInspect(context.Background(), "volume_id") + require.NoError(t, err) + assert.Equal(t, expected, volume) }