Skip to content

Commit

Permalink
Handle a Docker daemon without registry info
Browse files Browse the repository at this point in the history
The current implementation of the ElectAuthServer doesn't handle well when the
default Registry server is not included in the response from the daemon Info
endpoint.

That leads to the storage and usage of the credentials for the default registry
(`https://index.docker.io/v1/`) under an empty string on the client config file.

Sample config file after a login via a Docker Daemon without Registry
information:
```json
{
	"auths": {
		"": {
			"auth": "***"
		}
	}
}
```

That can lead to duplication of the password for the default registry and
authentication failures against the default registry if a pull/push is performed
without first authenticating via the misbehaving daemon.

Also, changes the output of the warning message from stdout to sdterr as
per dnephin suggestion.

Signed-off-by: Marcus Martins <marcus@docker.com>
  • Loading branch information
marcusmartins committed May 26, 2017
1 parent 61c0b9f commit 8626497
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 1 deletion.
4 changes: 3 additions & 1 deletion cli/command/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ func ElectAuthServer(ctx context.Context, cli Cli) string {
// the default registry URL might be Windows specific.
serverAddress := registry.IndexServer
if info, err := cli.Client().Info(ctx); err != nil {
fmt.Fprintf(cli.Out(), "Warning: failed to get default registry endpoint from daemon (%v). Using system default: %s\n", err, serverAddress)
fmt.Fprintf(cli.Err(), "Warning: failed to get default registry endpoint from daemon (%v). Using system default: %s\n", err, serverAddress)
} else if info.IndexServerAddress == "" {
fmt.Fprintf(cli.Err(), "Warning: Empty registry endpoint from daemon. Using system default: %s\n", serverAddress)
} else {
serverAddress = info.IndexServerAddress
}
Expand Down
79 changes: 79 additions & 0 deletions cli/command/registry_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package command_test

import (
"bytes"
"testing"

"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
"golang.org/x/net/context"

// Prevents a circular import with "github.com/docker/cli/cli/internal/test"
. "github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/internal/test"
"github.com/docker/docker/api/types"
"github.com/docker/docker/client"
)

type fakeClient struct {
client.Client
infoFunc func() (types.Info, error)
}

func (cli *fakeClient) Info(_ context.Context) (types.Info, error) {
if cli.infoFunc != nil {
return cli.infoFunc()
}
return types.Info{}, nil
}

func TestElectAuthServer(t *testing.T) {
testCases := []struct {
expectedAuthServer string
expectedWarning string
infoFunc func() (types.Info, error)
}{
{
expectedAuthServer: "https://index.docker.io/v1/",
expectedWarning: "",
infoFunc: func() (types.Info, error) {
return types.Info{IndexServerAddress: "https://index.docker.io/v1/"}, nil
},
},
{
expectedAuthServer: "https://index.docker.io/v1/",
expectedWarning: "Empty registry endpoint from daemon",
infoFunc: func() (types.Info, error) {
return types.Info{IndexServerAddress: ""}, nil
},
},
{
expectedAuthServer: "https://foo.bar",
expectedWarning: "",
infoFunc: func() (types.Info, error) {
return types.Info{IndexServerAddress: "https://foo.bar"}, nil
},
},
{
expectedAuthServer: "https://index.docker.io/v1/",
expectedWarning: "failed to get default registry endpoint from daemon",
infoFunc: func() (types.Info, error) {
return types.Info{}, errors.Errorf("error getting info")
},
},
}
for _, tc := range testCases {
buf := new(bytes.Buffer)
cli := test.NewFakeCli(&fakeClient{infoFunc: tc.infoFunc}, buf)
errBuf := new(bytes.Buffer)
cli.SetErr(errBuf)
server := ElectAuthServer(context.Background(), cli)
assert.Equal(t, tc.expectedAuthServer, server)
actual := errBuf.String()
if tc.expectedWarning == "" {
assert.Empty(t, actual)
} else {
assert.Contains(t, actual, tc.expectedWarning)
}
}
}

0 comments on commit 8626497

Please sign in to comment.