Browse code

Fix volume inspect with empty ID

Signed-off-by: Daniel Nephin <dnephin@docker.com>

Daniel Nephin authored on 2017/09/08 02:46:23
Showing 4 changed files
... ...
@@ -220,18 +220,11 @@ func (cli *Client) getAPIPath(p string, query url.Values) string {
220 220
 	var apiPath string
221 221
 	if cli.version != "" {
222 222
 		v := strings.TrimPrefix(cli.version, "v")
223
-		apiPath = path.Join(cli.basePath, "/v"+v+p)
223
+		apiPath = path.Join(cli.basePath, "/v"+v, p)
224 224
 	} else {
225 225
 		apiPath = path.Join(cli.basePath, p)
226 226
 	}
227
-
228
-	u := &url.URL{
229
-		Path: apiPath,
230
-	}
231
-	if len(query) > 0 {
232
-		u.RawQuery = query.Encode()
233
-	}
234
-	return u.String()
227
+	return (&url.URL{Path: apiPath, RawQuery: query.Encode()}).String()
235 228
 }
236 229
 
237 230
 // ClientVersion returns the version string associated with this
... ...
@@ -104,11 +104,11 @@ func TestNewEnvClient(t *testing.T) {
104 104
 }
105 105
 
106 106
 func TestGetAPIPath(t *testing.T) {
107
-	cases := []struct {
108
-		v string
109
-		p string
110
-		q url.Values
111
-		e string
107
+	testcases := []struct {
108
+		version  string
109
+		path     string
110
+		query    url.Values
111
+		expected string
112 112
 	}{
113 113
 		{"", "/containers/json", nil, "/containers/json"},
114 114
 		{"", "/containers/json", url.Values{}, "/containers/json"},
... ...
@@ -122,16 +122,10 @@ func TestGetAPIPath(t *testing.T) {
122 122
 		{"v1.22", "/networks/kiwl$%^", nil, "/v1.22/networks/kiwl$%25%5E"},
123 123
 	}
124 124
 
125
-	for _, cs := range cases {
126
-		c, err := NewClient("unix:///var/run/docker.sock", cs.v, nil, nil)
127
-		if err != nil {
128
-			t.Fatal(err)
129
-		}
130
-		g := c.getAPIPath(cs.p, cs.q)
131
-		assert.Equal(t, g, cs.e)
132
-
133
-		err = c.Close()
134
-		assert.NoError(t, err)
125
+	for _, testcase := range testcases {
126
+		c := Client{version: testcase.version, basePath: "/"}
127
+		actual := c.getAPIPath(testcase.path, testcase.query)
128
+		assert.Equal(t, actual, testcase.expected)
135 129
 	}
136 130
 }
137 131
 
... ...
@@ -5,6 +5,7 @@ import (
5 5
 	"encoding/json"
6 6
 	"io/ioutil"
7 7
 	"net/http"
8
+	"path"
8 9
 
9 10
 	"github.com/docker/docker/api/types"
10 11
 	"golang.org/x/net/context"
... ...
@@ -18,8 +19,15 @@ func (cli *Client) VolumeInspect(ctx context.Context, volumeID string) (types.Vo
18 18
 
19 19
 // VolumeInspectWithRaw returns the information about a specific volume in the docker host and its raw representation
20 20
 func (cli *Client) VolumeInspectWithRaw(ctx context.Context, volumeID string) (types.Volume, []byte, error) {
21
+	// The empty ID needs to be handled here because with an empty ID the
22
+	// request url will not contain a trailing / which calls the volume list API
23
+	// instead of volume inspect
24
+	if volumeID == "" {
25
+		return types.Volume{}, nil, volumeNotFoundError{volumeID}
26
+	}
27
+
21 28
 	var volume types.Volume
22
-	resp, err := cli.get(ctx, "/volumes/"+volumeID, nil, nil)
29
+	resp, err := cli.get(ctx, path.Join("/volumes", volumeID), nil, nil)
23 30
 	if err != nil {
24 31
 		if resp.statusCode == http.StatusNotFound {
25 32
 			return volume, nil, volumeNotFoundError{volumeID}
... ...
@@ -10,6 +10,9 @@ import (
10 10
 	"testing"
11 11
 
12 12
 	"github.com/docker/docker/api/types"
13
+	"github.com/docker/docker/internal/testutil"
14
+	"github.com/stretchr/testify/assert"
15
+	"github.com/stretchr/testify/require"
13 16
 	"golang.org/x/net/context"
14 17
 )
15 18
 
... ...
@@ -19,9 +22,7 @@ func TestVolumeInspectError(t *testing.T) {
19 19
 	}
20 20
 
21 21
 	_, err := client.VolumeInspect(context.Background(), "nothing")
22
-	if err == nil || err.Error() != "Error response from daemon: Server error" {
23
-		t.Fatalf("expected a Server Error, got %v", err)
24
-	}
22
+	testutil.ErrorContains(t, err, "Error response from daemon: Server error")
25 23
 }
26 24
 
27 25
 func TestVolumeInspectNotFound(t *testing.T) {
... ...
@@ -30,13 +31,34 @@ func TestVolumeInspectNotFound(t *testing.T) {
30 30
 	}
31 31
 
32 32
 	_, err := client.VolumeInspect(context.Background(), "unknown")
33
-	if err == nil || !IsErrVolumeNotFound(err) {
34
-		t.Fatalf("expected a volumeNotFound error, got %v", err)
33
+	assert.True(t, IsErrNotFound(err))
34
+}
35
+
36
+func TestVolumeInspectWithEmptyID(t *testing.T) {
37
+	expectedURL := "/volumes/"
38
+
39
+	client := &Client{
40
+		client: newMockClient(func(req *http.Request) (*http.Response, error) {
41
+			assert.Equal(t, req.URL.Path, expectedURL)
42
+			return &http.Response{
43
+				StatusCode: http.StatusNotFound,
44
+				Body:       ioutil.NopCloser(bytes.NewReader(nil)),
45
+			}, nil
46
+		}),
35 47
 	}
48
+	_, err := client.VolumeInspect(context.Background(), "")
49
+	testutil.ErrorContains(t, err, "No such volume: ")
50
+
36 51
 }
37 52
 
38 53
 func TestVolumeInspect(t *testing.T) {
39 54
 	expectedURL := "/volumes/volume_id"
55
+	expected := types.Volume{
56
+		Name:       "name",
57
+		Driver:     "driver",
58
+		Mountpoint: "mountpoint",
59
+	}
60
+
40 61
 	client := &Client{
41 62
 		client: newMockClient(func(req *http.Request) (*http.Response, error) {
42 63
 			if !strings.HasPrefix(req.URL.Path, expectedURL) {
... ...
@@ -45,11 +67,7 @@ func TestVolumeInspect(t *testing.T) {
45 45
 			if req.Method != "GET" {
46 46
 				return nil, fmt.Errorf("expected GET method, got %s", req.Method)
47 47
 			}
48
-			content, err := json.Marshal(types.Volume{
49
-				Name:       "name",
50
-				Driver:     "driver",
51
-				Mountpoint: "mountpoint",
52
-			})
48
+			content, err := json.Marshal(expected)
53 49
 			if err != nil {
54 50
 				return nil, err
55 51
 			}
... ...
@@ -60,17 +78,7 @@ func TestVolumeInspect(t *testing.T) {
60 60
 		}),
61 61
 	}
62 62
 
63
-	v, err := client.VolumeInspect(context.Background(), "volume_id")
64
-	if err != nil {
65
-		t.Fatal(err)
66
-	}
67
-	if v.Name != "name" {
68
-		t.Fatalf("expected `name`, got %s", v.Name)
69
-	}
70
-	if v.Driver != "driver" {
71
-		t.Fatalf("expected `driver`, got %s", v.Driver)
72
-	}
73
-	if v.Mountpoint != "mountpoint" {
74
-		t.Fatalf("expected `mountpoint`, got %s", v.Mountpoint)
75
-	}
63
+	volume, err := client.VolumeInspect(context.Background(), "volume_id")
64
+	require.NoError(t, err)
65
+	assert.Equal(t, expected, volume)
76 66
 }