Skip to content

Commit f0f3511

Browse files
thaJeztahthaJeztah
committed
Refactor ProxyConfig
Signed-off-by: Sebastiaan van Stijn <[email protected]>
1 parent cc6a92c commit f0f3511

6 files changed

Lines changed: 68 additions & 80 deletions

File tree

‎cli/command/container/create.go‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,7 @@ func NewCreateCommand(dockerCli command.Cli) *cobra.Command {
6161
}
6262

6363
func runCreate(dockerCli command.Cli, flags *pflag.FlagSet, options *createOptions, copts *containerOptions) error {
64-
proxyConfig := dockerCli.ConfigFile().ParseProxyConfig(dockerCli.Client().DaemonHost(), copts.env.GetAll())
65-
newEnv := []string{}
66-
for k, v := range proxyConfig {
67-
if v == nil {
68-
newEnv = append(newEnv, k)
69-
} else {
70-
newEnv = append(newEnv, fmt.Sprintf("%s=%s", k, *v))
71-
}
72-
}
64+
newEnv := dockerCli.ConfigFile().ParseProxyConfig(dockerCli.Client().DaemonHost(), copts.env.GetAll())
7365
copts.env = *opts.NewListOptsRef(&newEnv, nil)
7466
containerConfig, err := parse(flags, copts)
7567
if err != nil {

‎cli/command/container/run.go‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -98,15 +98,7 @@ func isLocalhost(ip string) bool {
9898
}
9999

100100
func runRun(dockerCli command.Cli, flags *pflag.FlagSet, ropts *runOptions, copts *containerOptions) error {
101-
proxyConfig := dockerCli.ConfigFile().ParseProxyConfig(dockerCli.Client().DaemonHost(), copts.env.GetAll())
102-
newEnv := []string{}
103-
for k, v := range proxyConfig {
104-
if v == nil {
105-
newEnv = append(newEnv, k)
106-
} else {
107-
newEnv = append(newEnv, fmt.Sprintf("%s=%s", k, *v))
108-
}
109-
}
101+
newEnv := dockerCli.ConfigFile().ParseProxyConfig(dockerCli.Client().DaemonHost(), copts.env.GetAll())
110102
copts.env = *opts.NewListOptsRef(&newEnv, nil)
111103
containerConfig, err := parse(flags, copts)
112104
// just in case the parse does not exit

‎cli/command/image/build.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,10 @@ func runBuild(dockerCli command.Cli, options buildOptions) error {
384384

385385
configFile := dockerCli.ConfigFile()
386386
authConfigs, _ := configFile.GetAllCredentials()
387-
buildOptions := imageBuildOptions(dockerCli, options)
387+
newEnv := configFile.ParseProxyConfig(dockerCli.Client().DaemonHost(), options.buildArgs.GetAll())
388+
options.buildArgs = *opts.NewListOptsRef(&newEnv, nil)
389+
390+
buildOptions := imageBuildOptions(options)
388391
buildOptions.Version = types.BuilderV1
389392
buildOptions.Dockerfile = relDockerfile
390393
buildOptions.AuthConfigs = authConfigs
@@ -599,8 +602,7 @@ func replaceDockerfileForContentTrust(ctx context.Context, inputTarStream io.Rea
599602
return pipeReader
600603
}
601604

602-
func imageBuildOptions(dockerCli command.Cli, options buildOptions) types.ImageBuildOptions {
603-
configFile := dockerCli.ConfigFile()
605+
func imageBuildOptions(options buildOptions) types.ImageBuildOptions {
604606
return types.ImageBuildOptions{
605607
Memory: options.memory.Value(),
606608
MemorySwap: options.memorySwap.Value(),
@@ -619,7 +621,7 @@ func imageBuildOptions(dockerCli command.Cli, options buildOptions) types.ImageB
619621
CgroupParent: options.cgroupParent,
620622
ShmSize: options.shmSize.Value(),
621623
Ulimits: options.ulimits.GetList(),
622-
BuildArgs: configFile.ParseProxyConfig(dockerCli.Client().DaemonHost(), options.buildArgs.GetAll()),
624+
BuildArgs: opts.ConvertKVStringsToMapWithNil(options.buildArgs.GetAll()),
623625
Labels: opts.ConvertKVStringsToMap(options.labels.GetAll()),
624626
CacheFrom: options.cacheFrom,
625627
SecurityOpt: options.securityOpt,

‎cli/command/image/build_buildkit.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func runBuildBuildKit(dockerCli command.Cli, options buildOptions) error {
175175
s.Close()
176176
}()
177177

178-
buildOptions := imageBuildOptions(dockerCli, options)
178+
buildOptions := imageBuildOptions(options)
179179
buildOptions.Version = types.BuilderBuildKit
180180
buildOptions.Dockerfile = dockerfileName
181181
//buildOptions.AuthConfigs = authConfigs // handled by session

‎cli/config/configfile/file.go‎

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -194,35 +194,39 @@ func (configFile *ConfigFile) Save() error {
194194

195195
// ParseProxyConfig computes proxy configuration by retrieving the config for the provided host and
196196
// then checking this against any environment variables provided to the container
197-
func (configFile *ConfigFile) ParseProxyConfig(host string, runOpts []string) map[string]*string {
198-
var cfgKey string
197+
func (configFile *ConfigFile) ParseProxyConfig(host string, runOpts []string) []string {
198+
m := opts.ConvertKVStringsToMap(runOpts)
199+
for k, v := range configFile.GetProxyConfig(host) {
200+
if *v == "" {
201+
continue
202+
}
203+
if _, ok := m[k]; !ok {
204+
runOpts = append(runOpts, fmt.Sprintf("%s=%s", k, *v))
205+
}
206+
}
207+
return runOpts
208+
}
199209

210+
// GetProxyConfig returns the proxy configuration for the provided host, or the
211+
// default proxy configuration if no configuration was found for the provided host.
212+
func (configFile *ConfigFile) GetProxyConfig(host string) map[string]*string {
213+
var config ProxyConfig
200214
if _, ok := configFile.Proxies[host]; !ok {
201-
cfgKey = "default"
215+
config = configFile.Proxies["default"]
202216
} else {
203-
cfgKey = host
217+
config = configFile.Proxies[host]
204218
}
205219

206-
config := configFile.Proxies[cfgKey]
207-
permitted := map[string]*string{
220+
return map[string]*string{
208221
"HTTP_PROXY": &config.HTTPProxy,
222+
"http_proxy": &config.HTTPProxy,
209223
"HTTPS_PROXY": &config.HTTPSProxy,
224+
"https_proxy": &config.HTTPSProxy,
210225
"NO_PROXY": &config.NoProxy,
226+
"no_proxy": &config.NoProxy,
211227
"FTP_PROXY": &config.FTPProxy,
228+
"ftp_proxy": &config.FTPProxy,
212229
}
213-
m := opts.ConvertKVStringsToMapWithNil(runOpts)
214-
for k := range permitted {
215-
if *permitted[k] == "" {
216-
continue
217-
}
218-
if _, ok := m[k]; !ok {
219-
m[k] = permitted[k]
220-
}
221-
if _, ok := m[strings.ToLower(k)]; !ok {
222-
m[strings.ToLower(k)] = permitted[k]
223-
}
224-
}
225-
return m
226230
}
227231

228232
// encodeAuth creates a base64 encoded string to containing authorization information

‎cli/config/configfile/file_test.go‎

Lines changed: 36 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"fmt"
55
"io/ioutil"
66
"os"
7+
"sort"
78
"testing"
89

910
"github.com/docker/cli/cli/config/credentials"
@@ -28,30 +29,31 @@ func TestProxyConfig(t *testing.T) {
2829
httpsProxy := "https://user:[email protected]:3129"
2930
ftpProxy := "http://ftpproxy.mycorp.com:21"
3031
noProxy := "*.intra.mycorp.com"
31-
defaultProxyConfig := ProxyConfig{
32-
HTTPProxy: httpProxy,
33-
HTTPSProxy: httpsProxy,
34-
FTPProxy: ftpProxy,
35-
NoProxy: noProxy,
36-
}
3732

3833
cfg := ConfigFile{
3934
Proxies: map[string]ProxyConfig{
40-
"default": defaultProxyConfig,
35+
"default": {
36+
HTTPProxy: httpProxy,
37+
HTTPSProxy: httpsProxy,
38+
FTPProxy: ftpProxy,
39+
NoProxy: noProxy,
40+
},
4141
},
4242
}
4343

4444
proxyConfig := cfg.ParseProxyConfig("/var/run/docker.sock", []string{})
45-
expected := map[string]*string{
46-
"HTTP_PROXY": &httpProxy,
47-
"http_proxy": &httpProxy,
48-
"HTTPS_PROXY": &httpsProxy,
49-
"https_proxy": &httpsProxy,
50-
"FTP_PROXY": &ftpProxy,
51-
"ftp_proxy": &ftpProxy,
52-
"NO_PROXY": &noProxy,
53-
"no_proxy": &noProxy,
45+
expected := []string{
46+
"HTTP_PROXY=" + httpProxy,
47+
"http_proxy=" + httpProxy,
48+
"HTTPS_PROXY=" + httpsProxy,
49+
"https_proxy=" + httpsProxy,
50+
"FTP_PROXY=" + ftpProxy,
51+
"ftp_proxy=" + ftpProxy,
52+
"NO_PROXY=" + noProxy,
53+
"no_proxy=" + noProxy,
5454
}
55+
sort.Strings(proxyConfig)
56+
sort.Strings(expected)
5557
assert.Check(t, is.DeepEqual(expected, proxyConfig))
5658
}
5759

@@ -62,16 +64,15 @@ func TestProxyConfigOverride(t *testing.T) {
6264
httpsProxy := "https://user:[email protected]:3129"
6365
ftpProxy := "http://ftpproxy.mycorp.com:21"
6466
noProxy := "*.intra.mycorp.com"
65-
defaultProxyConfig := ProxyConfig{
66-
HTTPProxy: httpProxy,
67-
HTTPSProxy: httpsProxy,
68-
FTPProxy: ftpProxy,
69-
NoProxy: noProxy,
70-
}
7167

7268
cfg := ConfigFile{
7369
Proxies: map[string]ProxyConfig{
74-
"default": defaultProxyConfig,
70+
"default": {
71+
HTTPProxy: httpProxy,
72+
HTTPSProxy: httpsProxy,
73+
FTPProxy: ftpProxy,
74+
NoProxy: noProxy,
75+
},
7576
},
7677
}
7778

@@ -104,23 +105,20 @@ func TestProxyConfigPerHost(t *testing.T) {
104105
extFTPProxy := "http://ftpproxy.example.com:21"
105106
extNoProxy := "*.intra.example.com"
106107

107-
defaultProxyConfig := ProxyConfig{
108-
HTTPProxy: httpProxy,
109-
HTTPSProxy: httpsProxy,
110-
FTPProxy: ftpProxy,
111-
NoProxy: noProxy,
112-
}
113-
externalProxyConfig := ProxyConfig{
114-
HTTPProxy: extHTTPProxy,
115-
HTTPSProxy: extHTTPSProxy,
116-
FTPProxy: extFTPProxy,
117-
NoProxy: extNoProxy,
118-
}
119-
120108
cfg := ConfigFile{
121109
Proxies: map[string]ProxyConfig{
122-
"default": defaultProxyConfig,
123-
"tcp://example.docker.com:2376": externalProxyConfig,
110+
"default": {
111+
HTTPProxy: httpProxy,
112+
HTTPSProxy: httpsProxy,
113+
FTPProxy: ftpProxy,
114+
NoProxy: noProxy,
115+
},
116+
"tcp://example.docker.com:2376": {
117+
HTTPProxy: extHTTPProxy,
118+
HTTPSProxy: extHTTPSProxy,
119+
FTPProxy: extFTPProxy,
120+
NoProxy: extNoProxy,
121+
},
124122
},
125123
}
126124

0 commit comments

Comments
 (0)