mirror of
https://github.com/moby/moby.git
synced 2026-01-11 18:51:37 +00:00
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/<empty id>/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 <object> 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 <github@gone.nl>
174 lines
4.8 KiB
Go
174 lines
4.8 KiB
Go
package client // import "github.com/docker/docker/client"
|
|
|
|
import (
|
|
"bytes"
|
|
"context"
|
|
"fmt"
|
|
"io"
|
|
"log"
|
|
"net/http"
|
|
"os"
|
|
"strings"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/docker/docker/api/types/container"
|
|
"github.com/docker/docker/errdefs"
|
|
"gotest.tools/v3/assert"
|
|
is "gotest.tools/v3/assert/cmp"
|
|
)
|
|
|
|
func TestContainerLogsNotFoundError(t *testing.T) {
|
|
client := &Client{
|
|
client: newMockClient(errorMock(http.StatusNotFound, "Not found")),
|
|
}
|
|
_, 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) {
|
|
client := &Client{
|
|
client: newMockClient(errorMock(http.StatusInternalServerError, "Server error")),
|
|
}
|
|
_, err := client.ContainerLogs(context.Background(), "container_id", container.LogsOptions{})
|
|
assert.Check(t, is.ErrorType(err, errdefs.IsSystem))
|
|
|
|
_, err = client.ContainerLogs(context.Background(), "container_id", container.LogsOptions{
|
|
Since: "2006-01-02TZ",
|
|
})
|
|
assert.Check(t, is.ErrorContains(err, `parsing time "2006-01-02TZ"`))
|
|
_, err = client.ContainerLogs(context.Background(), "container_id", container.LogsOptions{
|
|
Until: "2006-01-02TZ",
|
|
})
|
|
assert.Check(t, is.ErrorContains(err, `parsing time "2006-01-02TZ"`))
|
|
}
|
|
|
|
func TestContainerLogs(t *testing.T) {
|
|
expectedURL := "/containers/container_id/logs"
|
|
cases := []struct {
|
|
options container.LogsOptions
|
|
expectedQueryParams map[string]string
|
|
expectedError string
|
|
}{
|
|
{
|
|
expectedQueryParams: map[string]string{
|
|
"tail": "",
|
|
},
|
|
},
|
|
{
|
|
options: container.LogsOptions{
|
|
Tail: "any",
|
|
},
|
|
expectedQueryParams: map[string]string{
|
|
"tail": "any",
|
|
},
|
|
},
|
|
{
|
|
options: container.LogsOptions{
|
|
ShowStdout: true,
|
|
ShowStderr: true,
|
|
Timestamps: true,
|
|
Details: true,
|
|
Follow: true,
|
|
},
|
|
expectedQueryParams: map[string]string{
|
|
"tail": "",
|
|
"stdout": "1",
|
|
"stderr": "1",
|
|
"timestamps": "1",
|
|
"details": "1",
|
|
"follow": "1",
|
|
},
|
|
},
|
|
{
|
|
options: container.LogsOptions{
|
|
// timestamp will be passed as is
|
|
Since: "1136073600.000000001",
|
|
},
|
|
expectedQueryParams: map[string]string{
|
|
"tail": "",
|
|
"since": "1136073600.000000001",
|
|
},
|
|
},
|
|
{
|
|
options: container.LogsOptions{
|
|
// timestamp will be passed as is
|
|
Until: "1136073600.000000001",
|
|
},
|
|
expectedQueryParams: map[string]string{
|
|
"tail": "",
|
|
"until": "1136073600.000000001",
|
|
},
|
|
},
|
|
{
|
|
options: container.LogsOptions{
|
|
// An complete invalid date will not be passed
|
|
Since: "invalid value",
|
|
},
|
|
expectedError: `invalid value for "since": failed to parse value as time or duration: "invalid value"`,
|
|
},
|
|
{
|
|
options: container.LogsOptions{
|
|
// An complete invalid date will not be passed
|
|
Until: "invalid value",
|
|
},
|
|
expectedError: `invalid value for "until": failed to parse value as time or duration: "invalid value"`,
|
|
},
|
|
}
|
|
for _, logCase := range cases {
|
|
client := &Client{
|
|
client: newMockClient(func(r *http.Request) (*http.Response, error) {
|
|
if !strings.HasPrefix(r.URL.Path, expectedURL) {
|
|
return nil, fmt.Errorf("expected URL '%s', got '%s'", expectedURL, r.URL)
|
|
}
|
|
// Check query parameters
|
|
query := r.URL.Query()
|
|
for key, expected := range logCase.expectedQueryParams {
|
|
actual := query.Get(key)
|
|
if actual != expected {
|
|
return nil, fmt.Errorf("%s not set in URL query properly. Expected '%s', got %s", key, expected, actual)
|
|
}
|
|
}
|
|
return &http.Response{
|
|
StatusCode: http.StatusOK,
|
|
Body: io.NopCloser(bytes.NewReader([]byte("response"))),
|
|
}, nil
|
|
}),
|
|
}
|
|
body, err := client.ContainerLogs(context.Background(), "container_id", logCase.options)
|
|
if logCase.expectedError != "" {
|
|
assert.Check(t, is.Error(err, logCase.expectedError))
|
|
continue
|
|
}
|
|
assert.NilError(t, err)
|
|
defer body.Close()
|
|
content, err := io.ReadAll(body)
|
|
assert.NilError(t, err)
|
|
assert.Check(t, is.Contains(string(content), "response"))
|
|
}
|
|
}
|
|
|
|
func ExampleClient_ContainerLogs_withTimeout() {
|
|
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
|
defer cancel()
|
|
|
|
client, _ := NewClientWithOpts(FromEnv)
|
|
reader, err := client.ContainerLogs(ctx, "container_id", container.LogsOptions{})
|
|
if err != nil {
|
|
log.Fatal(err)
|
|
}
|
|
|
|
_, err = io.Copy(os.Stdout, reader)
|
|
if err != nil && err != io.EOF {
|
|
log.Fatal(err)
|
|
}
|
|
}
|