Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit e828efa

Browse filesBrowse files
dnephindnephin
committed
Set APIVersion on the client, even when Ping fails
Refactor to support testing Also add tests Signed-off-by: Daniel Nephin <dnephin@docker.com>
1 parent 10e292d commit e828efa
Copy full SHA for e828efa

File tree

2 files changed

+177
-26
lines changed
Filter options

2 files changed

+177
-26
lines changed

‎cli/command/cli.go

Copy file name to clipboardExpand all lines: cli/command/cli.go
+35-25
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/docker/docker/client"
1818
"github.com/docker/go-connections/sockets"
1919
"github.com/docker/go-connections/tlsconfig"
20+
"github.com/docker/notary"
2021
"github.com/docker/notary/passphrase"
2122
"github.com/pkg/errors"
2223
"github.com/spf13/cobra"
@@ -111,44 +112,53 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions) error {
111112
var err error
112113
cli.client, err = NewAPIClientFromFlags(opts.Common, cli.configFile)
113114
if tlsconfig.IsErrEncryptedKey(err) {
114-
var (
115-
passwd string
116-
giveup bool
117-
)
118115
passRetriever := passphrase.PromptRetrieverWithInOut(cli.In(), cli.Out(), nil)
119-
120-
for attempts := 0; tlsconfig.IsErrEncryptedKey(err); attempts++ {
121-
// some code and comments borrowed from notary/trustmanager/keystore.go
122-
passwd, giveup, err = passRetriever("private", "encrypted TLS private", false, attempts)
123-
// Check if the passphrase retriever got an error or if it is telling us to give up
124-
if giveup || err != nil {
125-
return errors.Wrap(err, "private key is encrypted, but could not get passphrase")
126-
}
127-
128-
opts.Common.TLSOptions.Passphrase = passwd
129-
cli.client, err = NewAPIClientFromFlags(opts.Common, cli.configFile)
116+
newClient := func(password string) (client.APIClient, error) {
117+
opts.Common.TLSOptions.Passphrase = password
118+
return NewAPIClientFromFlags(opts.Common, cli.configFile)
130119
}
120+
cli.client, err = getClientWithPassword(passRetriever, newClient)
131121
}
132-
133122
if err != nil {
134123
return err
135124
}
125+
cli.initializeFromClient()
126+
return nil
127+
}
136128

129+
func (cli *DockerCli) initializeFromClient() {
137130
cli.defaultVersion = cli.client.ClientVersion()
138131

139-
if ping, err := cli.client.Ping(context.Background()); err == nil {
140-
cli.server = ServerInfo{
141-
HasExperimental: ping.Experimental,
142-
OSType: ping.OSType,
143-
}
144-
145-
cli.client.NegotiateAPIVersionPing(ping)
146-
} else {
132+
ping, err := cli.client.Ping(context.Background())
133+
if err != nil {
147134
// Default to true if we fail to connect to daemon
148135
cli.server = ServerInfo{HasExperimental: true}
136+
137+
if ping.APIVersion != "" {
138+
cli.client.NegotiateAPIVersionPing(ping)
139+
}
140+
return
149141
}
150142

151-
return nil
143+
cli.server = ServerInfo{
144+
HasExperimental: ping.Experimental,
145+
OSType: ping.OSType,
146+
}
147+
cli.client.NegotiateAPIVersionPing(ping)
148+
}
149+
150+
func getClientWithPassword(passRetriever notary.PassRetriever, newClient func(password string) (client.APIClient, error)) (client.APIClient, error) {
151+
for attempts := 0; ; attempts++ {
152+
passwd, giveup, err := passRetriever("private", "encrypted TLS private", false, attempts)
153+
if giveup || err != nil {
154+
return nil, errors.Wrap(err, "private key is encrypted, but could not get passphrase")
155+
}
156+
157+
apiclient, err := newClient(passwd)
158+
if !tlsconfig.IsErrEncryptedKey(err) {
159+
return apiclient, err
160+
}
161+
}
152162
}
153163

154164
// ServerInfo stores details about the supported features and platform of the

‎cli/command/cli_test.go

Copy file name to clipboardExpand all lines: cli/command/cli_test.go
+142-1
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,18 @@ import (
44
"os"
55
"testing"
66

7+
"crypto/x509"
8+
79
"github.com/docker/cli/cli/config/configfile"
810
"github.com/docker/cli/cli/flags"
11+
"github.com/docker/cli/internal/test/testutil"
912
"github.com/docker/docker/api"
13+
"github.com/docker/docker/api/types"
1014
"github.com/docker/docker/client"
15+
"github.com/pkg/errors"
1116
"github.com/stretchr/testify/assert"
1217
"github.com/stretchr/testify/require"
18+
"golang.org/x/net/context"
1319
)
1420

1521
func TestNewAPIClientFromFlags(t *testing.T) {
@@ -43,7 +49,7 @@ func TestNewAPIClientFromFlagsWithAPIVersionFromEnv(t *testing.T) {
4349
assert.Equal(t, customVersion, apiclient.ClientVersion())
4450
}
4551

46-
// TODO: move to gotestyourself
52+
// TODO: use gotestyourself/env.Patch
4753
func patchEnvVariable(t *testing.T, key, value string) func() {
4854
oldValue, ok := os.LookupEnv(key)
4955
require.NoError(t, os.Setenv(key, value))
@@ -55,3 +61,138 @@ func patchEnvVariable(t *testing.T, key, value string) func() {
5561
require.NoError(t, os.Setenv(key, oldValue))
5662
}
5763
}
64+
65+
type fakeClient struct {
66+
client.Client
67+
pingFunc func() (types.Ping, error)
68+
version string
69+
negotiated bool
70+
}
71+
72+
func (c *fakeClient) Ping(_ context.Context) (types.Ping, error) {
73+
return c.pingFunc()
74+
}
75+
76+
func (c *fakeClient) ClientVersion() string {
77+
return c.version
78+
}
79+
80+
func (c *fakeClient) NegotiateAPIVersionPing(types.Ping) {
81+
c.negotiated = true
82+
}
83+
84+
func TestInitializeFromClient(t *testing.T) {
85+
defaultVersion := "v1.55"
86+
87+
var testcases = []struct {
88+
doc string
89+
pingFunc func() (types.Ping, error)
90+
expectedServer ServerInfo
91+
negotiated bool
92+
}{
93+
{
94+
doc: "successful ping",
95+
pingFunc: func() (types.Ping, error) {
96+
return types.Ping{Experimental: true, OSType: "linux", APIVersion: "v1.30"}, nil
97+
},
98+
expectedServer: ServerInfo{HasExperimental: true, OSType: "linux"},
99+
negotiated: true,
100+
},
101+
{
102+
doc: "failed ping, no API version",
103+
pingFunc: func() (types.Ping, error) {
104+
return types.Ping{}, errors.New("failed")
105+
},
106+
expectedServer: ServerInfo{HasExperimental: true},
107+
},
108+
{
109+
doc: "failed ping, with API version",
110+
pingFunc: func() (types.Ping, error) {
111+
return types.Ping{APIVersion: "v1.33"}, errors.New("failed")
112+
},
113+
expectedServer: ServerInfo{HasExperimental: true},
114+
negotiated: true,
115+
},
116+
}
117+
118+
for _, testcase := range testcases {
119+
t.Run(testcase.doc, func(t *testing.T) {
120+
apiclient := &fakeClient{
121+
pingFunc: testcase.pingFunc,
122+
version: defaultVersion,
123+
}
124+
125+
cli := &DockerCli{client: apiclient}
126+
cli.initializeFromClient()
127+
assert.Equal(t, defaultVersion, cli.defaultVersion)
128+
assert.Equal(t, testcase.expectedServer, cli.server)
129+
assert.Equal(t, testcase.negotiated, apiclient.negotiated)
130+
})
131+
}
132+
}
133+
134+
func TestGetClientWithPassword(t *testing.T) {
135+
expected := "password"
136+
137+
var testcases = []struct {
138+
doc string
139+
password string
140+
retrieverErr error
141+
retrieverGiveup bool
142+
newClientErr error
143+
expectedErr string
144+
}{
145+
{
146+
doc: "successful connect",
147+
password: expected,
148+
},
149+
{
150+
doc: "password retriever exhausted",
151+
retrieverGiveup: true,
152+
retrieverErr: errors.New("failed"),
153+
expectedErr: "private key is encrypted, but could not get passphrase",
154+
},
155+
{
156+
doc: "password retriever error",
157+
retrieverErr: errors.New("failed"),
158+
expectedErr: "failed",
159+
},
160+
{
161+
doc: "newClient error",
162+
newClientErr: errors.New("failed to connect"),
163+
expectedErr: "failed to connect",
164+
},
165+
}
166+
167+
for _, testcase := range testcases {
168+
t.Run(testcase.doc, func(t *testing.T) {
169+
passRetriever := func(_, _ string, _ bool, attempts int) (passphrase string, giveup bool, err error) {
170+
// Always return an invalid pass first to test iteration
171+
switch attempts {
172+
case 0:
173+
return "something else", false, nil
174+
default:
175+
return testcase.password, testcase.retrieverGiveup, testcase.retrieverErr
176+
}
177+
}
178+
179+
newClient := func(currentPassword string) (client.APIClient, error) {
180+
if testcase.newClientErr != nil {
181+
return nil, testcase.newClientErr
182+
}
183+
if currentPassword == expected {
184+
return &client.Client{}, nil
185+
}
186+
return &client.Client{}, x509.IncorrectPasswordError
187+
}
188+
189+
_, err := getClientWithPassword(passRetriever, newClient)
190+
if testcase.expectedErr != "" {
191+
testutil.ErrorContains(t, err, testcase.expectedErr)
192+
return
193+
}
194+
195+
assert.NoError(t, err)
196+
})
197+
}
198+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.