Merge pull request #7162 from wupeka/b1666353

Make proxy settings truly global

## Description of change
Make proxy settings global - previously the settings were only used on 'ubuntu' user login shell, with this change the settings are global for all users using /etc/profile.d and, on systems using systemd, also set for services launched using systemd via /etc/systemd/{system,user}.conf.d/juju-proxy.conf. 

## QA steps
1. Run unit tests
2. Verify that if http-proxy option is set it's being set in the environment - e.g. services running via systemd should have proper env variables set.

## Documentation changes
This change has to be noted in release notes and docs.

## Bug reference
https://bugs.launchpad.net/juju/+bug/1666353

apiserver/provisioner/provisioner_test.go

@@ -1169,13 +1169,15 @@ func (s *withoutControllerSuite) TestContainerManagerConfig(c *gc.C) {
 func (s *withoutControllerSuite) TestContainerConfig(c *gc.C) {
 	attrs := map[string]interface{}{
 		"http-proxy":            "http://proxy.example.com:9000",
+		"apt-https-proxy":       "https://proxy.example.com:9000",
 		"allow-lxd-loop-mounts": true,
 		"apt-mirror":            "http://example.mirror.com",
 	}
 	err := s.State.UpdateModelConfig(attrs, nil, nil)
 	c.Assert(err, jc.ErrorIsNil)
 	expectedAPTProxy := proxy.Settings{
 		Http:    "http://proxy.example.com:9000",
+		Https:   "https://proxy.example.com:9000",
 		NoProxy: "127.0.0.1,localhost,::1",
 	}
 

apiserver/proxyupdater/proxyupdater.go

@@ -4,10 +4,7 @@
 package proxyupdater
 
 import (
-	"strings"
-
 	"github.com/juju/utils/proxy"
-	"github.com/juju/utils/set"
 	"gopkg.in/juju/names.v2"
 
 	"github.com/juju/juju/apiserver/common"
@@ -86,7 +83,7 @@ func proxyUtilsSettingsToProxySettingsParam(settings proxy.Settings) params.Prox
 		HTTP:    settings.Http,
 		HTTPS:   settings.Https,
 		FTP:     settings.Ftp,
-		NoProxy: settings.NoProxy,
+		NoProxy: settings.FullNoProxy(),
 	}
 }
 
@@ -127,26 +124,10 @@ func (api *ProxyUpdaterAPI) proxyConfig() params.ProxyConfigResult {
 		return result
 	}
 
-	result.ProxySettings = proxyUtilsSettingsToProxySettingsParam(env.ProxySettings())
+	proxySettings := env.ProxySettings()
+	proxySettings.AutoNoProxy = network.APIHostPortsToNoProxyString(apiHostPorts)
+	result.ProxySettings = proxyUtilsSettingsToProxySettingsParam(proxySettings)
 	result.APTProxySettings = proxyUtilsSettingsToProxySettingsParam(env.AptProxySettings())
-
-	var noProxy []string
-	if result.ProxySettings.NoProxy != "" {
-		noProxy = strings.Split(result.ProxySettings.NoProxy, ",")
-	}
-
-	noProxySet := set.NewStrings(noProxy...)
-	for _, host := range apiHostPorts {
-		for _, hp := range host {
-			if hp.Address.Scope == network.ScopeMachineLocal ||
-				hp.Address.Scope == network.ScopeLinkLocal {
-				continue
-			}
-			noProxySet.Add(hp.Address.Value)
-		}
-	}
-	result.ProxySettings.NoProxy = strings.Join(noProxySet.SortedValues(), ",")
-
 	return result
 }
 

apiserver/proxyupdater/proxyupdater_test.go

@@ -111,17 +111,19 @@ func (s *ProxyUpdaterSuite) TestProxyConfig(c *gc.C) {
 		ProxySettings: params.ProxyConfig{
 			HTTP: "http proxy", HTTPS: "https proxy", FTP: "", NoProxy: noProxy},
 		APTProxySettings: params.ProxyConfig{
-			HTTP: "http://http proxy", HTTPS: "https://https proxy", FTP: "", NoProxy: ""},
+			HTTP: "http://apt http proxy", HTTPS: "https://apt https proxy", FTP: "", NoProxy: ""},
 	}
 	c.Assert(cfg.Results[0], jc.DeepEquals, r)
 }
 
 func (s *ProxyUpdaterSuite) TestProxyConfigExtendsExisting(c *gc.C) {
 	// Check that the ProxyConfig combines data from ModelConfig and APIHostPorts
 	s.state.SetModelConfig(coretesting.Attrs{
-		"http-proxy":  "http proxy",
-		"https-proxy": "https proxy",
-		"no-proxy":    "9.9.9.9",
+		"http-proxy":      "http proxy",
+		"https-proxy":     "https proxy",
+		"apt-http-proxy":  "apt http proxy",
+		"apt-https-proxy": "apt https proxy",
+		"no-proxy":        "9.9.9.9",
 	})
 	cfg := s.facade.ProxyConfig(s.oneEntity())
 	s.state.Stub.CheckCallNames(c,
@@ -130,21 +132,24 @@ func (s *ProxyUpdaterSuite) TestProxyConfigExtendsExisting(c *gc.C) {
 	)
 
 	expectedNoProxy := "0.1.2.3,0.1.2.4,0.1.2.5,9.9.9.9"
+	expectedAptNoProxy := "9.9.9.9"
 
 	c.Assert(cfg.Results[0], jc.DeepEquals, params.ProxyConfigResult{
 		ProxySettings: params.ProxyConfig{
 			HTTP: "http proxy", HTTPS: "https proxy", FTP: "", NoProxy: expectedNoProxy},
 		APTProxySettings: params.ProxyConfig{
-			HTTP: "http://http proxy", HTTPS: "https://https proxy", FTP: "", NoProxy: ""},
+			HTTP: "http://apt http proxy", HTTPS: "https://apt https proxy", FTP: "", NoProxy: expectedAptNoProxy},
 	})
 }
 
 func (s *ProxyUpdaterSuite) TestProxyConfigNoDuplicates(c *gc.C) {
 	// Check that the ProxyConfig combines data from ModelConfig and APIHostPorts
 	s.state.SetModelConfig(coretesting.Attrs{
-		"http-proxy":  "http proxy",
-		"https-proxy": "https proxy",
-		"no-proxy":    "0.1.2.3",
+		"http-proxy":      "http proxy",
+		"https-proxy":     "https proxy",
+		"apt-http-proxy":  "apt http proxy",
+		"apt-https-proxy": "apt https proxy",
+		"no-proxy":        "0.1.2.3",
 	})
 	cfg := s.facade.ProxyConfig(s.oneEntity())
 	s.state.Stub.CheckCallNames(c,
@@ -153,12 +158,13 @@ func (s *ProxyUpdaterSuite) TestProxyConfigNoDuplicates(c *gc.C) {
 	)
 
 	expectedNoProxy := "0.1.2.3,0.1.2.4,0.1.2.5"
+	expectedAptNoProxy := "0.1.2.3"
 
 	c.Assert(cfg.Results[0], jc.DeepEquals, params.ProxyConfigResult{
 		ProxySettings: params.ProxyConfig{
 			HTTP: "http proxy", HTTPS: "https proxy", FTP: "", NoProxy: expectedNoProxy},
 		APTProxySettings: params.ProxyConfig{
-			HTTP: "http://http proxy", HTTPS: "https://https proxy", FTP: "", NoProxy: ""},
+			HTTP: "http://apt http proxy", HTTPS: "https://apt https proxy", FTP: "", NoProxy: expectedAptNoProxy},
 	})
 }
 
@@ -176,8 +182,10 @@ func (sb *stubBackend) SetUp(c *gc.C) {
 	sb.Stub = &testing.Stub{}
 	sb.c = c
 	sb.configAttrs = coretesting.Attrs{
-		"http-proxy":  "http proxy",
-		"https-proxy": "https proxy",
+		"http-proxy":      "http proxy",
+		"https-proxy":     "https proxy",
+		"apt-http-proxy":  "apt http proxy",
+		"apt-https-proxy": "apt https proxy",
 	}
 	sb.hpWatcher = workertest.NewFakeWatcher(1, 1)
 	sb.confWatcher = workertest.NewFakeWatcher(1, 1)

cloudconfig/instancecfg/instancecfg.go

@@ -11,6 +11,7 @@ import (
 	"path"
 	"reflect"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/juju/errors"
@@ -466,6 +467,24 @@ func (cfg *InstanceConfig) APIHostAddrs() []string {
 	return hosts
 }
 
+func (cfg *InstanceConfig) APIHosts() []string {
+	var hosts []string
+	if cfg.Bootstrap != nil {
+		hosts = append(hosts, "localhost")
+	}
+	if cfg.APIInfo != nil {
+		for _, addr := range cfg.APIInfo.Addrs {
+			host, _, err := net.SplitHostPort(addr)
+			if err != nil {
+				logger.Errorf("Can't split API address %q to host:port - %q", host, err)
+				continue
+			}
+			hosts = append(hosts, host)
+		}
+	}
+	return hosts
+}
+
 // AgentVersion returns the version of the Juju agent that will be configured
 // on the instance. The zero value will be returned if there are no tools set.
 func (cfg *InstanceConfig) AgentVersion() version.Binary {
@@ -771,6 +790,7 @@ func PopulateInstanceConfig(icfg *InstanceConfig,
 	icfg.AgentEnvironment[agent.ContainerType] = string(icfg.MachineContainerType)
 	icfg.DisableSSLHostnameVerification = !sslHostnameVerification
 	icfg.ProxySettings = proxySettings
+	icfg.ProxySettings.AutoNoProxy = strings.Join(icfg.APIHosts(), ",")
 	icfg.AptProxySettings = aptProxySettings
 	icfg.AptMirror = aptMirror
 	icfg.EnableOSRefreshUpdate = enableOSRefreshUpdates

cloudconfig/userdatacfg_test.go

@@ -330,7 +330,7 @@ printf '%s\\n' '.*"Stop all network interfaces.*' > '/etc/init/juju-clean-shutdo
 install -D -m 644 /dev/null '/var/lib/juju/nonce.txt'
 printf '%s\\n' 'FAKE_NONCE' > '/var/lib/juju/nonce.txt'
 test -n "\$JUJU_PROGRESS_FD" \|\| \(exec \{JUJU_PROGRESS_FD\}>&2\) 2>/dev/null && exec \{JUJU_PROGRESS_FD\}>&2 \|\| JUJU_PROGRESS_FD=2
-\(\[ ! -e /home/ubuntu/.profile \] \|\| grep -q '.juju-proxy' /home/ubuntu/.profile\) \|\| printf .* >> /home/ubuntu/.profile
+\[ -e /etc/profile.d/juju-proxy.sh \] \|\| printf .* >> /etc/profile.d/juju-proxy.sh
 mkdir -p /var/lib/juju/locks
 \(id ubuntu &> /dev/null\) && chown ubuntu:ubuntu /var/lib/juju/locks
 mkdir -p /var/log/juju
@@ -387,7 +387,7 @@ printf '%s\\n' '.*"Stop all network interfaces on shutdown".*' > '/etc/init/juju
 install -D -m 644 /dev/null '/var/lib/juju/nonce.txt'
 printf '%s\\n' 'FAKE_NONCE' > '/var/lib/juju/nonce.txt'
 test -n "\$JUJU_PROGRESS_FD" \|\| \(exec \{JUJU_PROGRESS_FD\}>&2\) 2>/dev/null && exec \{JUJU_PROGRESS_FD\}>&2 \|\| JUJU_PROGRESS_FD=2
-\(\[ ! -e /home/ubuntu/\.profile \] \|\| grep -q '.juju-proxy' /home/ubuntu/.profile\) \|\| printf .* >> /home/ubuntu/.profile
+\[ -e /etc/profile.d/juju-proxy.sh \] \|\| printf .* >> /etc/profile.d/juju-proxy.sh
 mkdir -p /var/lib/juju/locks
 \(id ubuntu &> /dev/null\) && chown ubuntu:ubuntu /var/lib/juju/locks
 mkdir -p /var/log/juju
@@ -1164,21 +1164,27 @@ func (s *cloudinitSuite) TestProxyWritten(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 
 	cmds := cloudcfg.RunCmds()
-	first := `([ ! -e /home/ubuntu/.profile ] || grep -q '.juju-proxy' /home/ubuntu/.profile) || printf '\n# Added by juju\n[ -f "$HOME/.juju-proxy" ] && . "$HOME/.juju-proxy"\n' >> /home/ubuntu/.profile`
+	first := `[ -e /etc/profile.d/juju-proxy.sh ] || printf '\n# Added by juju\n[ -f "/etc/juju-proxy.conf" ] && . "/etc/juju-proxy.conf"\n' >> /etc/profile.d/juju-proxy.sh`
 	expected := []string{
 		`export http_proxy=http://user@10.0.0.1`,
 		`export HTTP_PROXY=http://user@10.0.0.1`,
-		`export no_proxy=10.0.3.1,localhost`,
-		`export NO_PROXY=10.0.3.1,localhost`,
-		`(id ubuntu &> /dev/null) && (printf '%s\n' 'export http_proxy=http://user@10.0.0.1
+		`export no_proxy=0.1.2.3,10.0.3.1,localhost`,
+		`export NO_PROXY=0.1.2.3,10.0.3.1,localhost`,
+		`(printf '%s\n' 'export http_proxy=http://user@10.0.0.1
 export HTTP_PROXY=http://user@10.0.0.1
-export no_proxy=10.0.3.1,localhost
-export NO_PROXY=10.0.3.1,localhost' > /home/ubuntu/.juju-proxy && chown ubuntu:ubuntu /home/ubuntu/.juju-proxy)`,
+export no_proxy=0.1.2.3,10.0.3.1,localhost
+export NO_PROXY=0.1.2.3,10.0.3.1,localhost' > /etc/juju-proxy.conf && chmod 0644 /etc/juju-proxy.conf)`,
+		`printf '%s\n' '# To allow juju to control the global systemd proxy settings,
+# create symbolic links to this file from within /etc/systemd/system.conf.d/
+# and /etc/systemd/users.conf.d/.
+[Manager]
+DefaultEnvironment="http_proxy=http://user@10.0.0.1" "HTTP_PROXY=http://user@10.0.0.1" "no_proxy=0.1.2.3,10.0.3.1,localhost" "NO_PROXY=0.1.2.3,10.0.3.1,localhost" 
+' > /etc/juju-proxy-systemd.conf`,
 	}
 	found := false
 	for i, cmd := range cmds {
 		if cmd == first {
-			c.Assert(cmds[i+1:i+6], jc.DeepEquals, expected)
+			c.Assert(cmds[i+1:i+7], jc.DeepEquals, expected)
 			found = true
 			break
 		}

cloudconfig/userdatacfg_unix.go

@@ -228,17 +228,20 @@ func (w *unixConfigure) ConfigureJuju() error {
 	// sourced by bash, and ssh through that.
 	w.conf.AddScripts(
 		// We look to see if the proxy line is there already as
-		// the manual provider may have had it already. The ubuntu
-		// user may not exist.
-		`([ ! -e /home/ubuntu/.profile ] || grep -q '.juju-proxy' /home/ubuntu/.profile) || ` +
-			`printf '\n# Added by juju\n[ -f "$HOME/.juju-proxy" ] && . "$HOME/.juju-proxy"\n' >> /home/ubuntu/.profile`)
+		// the manual provider may have had it already.
+		`[ -e /etc/profile.d/juju-proxy.sh ] || ` +
+			`printf '\n# Added by juju\n[ -f "/etc/juju-proxy.conf" ] && . "/etc/juju-proxy.conf"\n' >> /etc/profile.d/juju-proxy.sh`)
 	if (w.icfg.ProxySettings != proxy.Settings{}) {
 		exportedProxyEnv := w.icfg.ProxySettings.AsScriptEnvironment()
 		w.conf.AddScripts(strings.Split(exportedProxyEnv, "\n")...)
 		w.conf.AddScripts(
 			fmt.Sprintf(
-				`(id ubuntu &> /dev/null) && (printf '%%s\n' %s > /home/ubuntu/.juju-proxy && chown ubuntu:ubuntu /home/ubuntu/.juju-proxy)`,
+				`(printf '%%s\n' %s > /etc/juju-proxy.conf && chmod 0644 /etc/juju-proxy.conf)`,
 				shquote(w.icfg.ProxySettings.AsScriptEnvironment())))
+
+		// Write out systemd proxy settings
+		w.conf.AddScripts(fmt.Sprintf(`printf '%%s\n' %[1]s > /etc/juju-proxy-systemd.conf`,
+			shquote(w.icfg.ProxySettings.AsSystemdDefaultEnv())))
 	}
 
 	if w.icfg.Controller != nil && w.icfg.Controller.PublicImageSigningKey != "" {

dependencies.tsv

@@ -48,7 +48,7 @@ github.com/juju/terms-client	git	9b925afd677234e4146dde3cb1a11e187cbed64e	2016-0
 github.com/juju/testing	git	06d21ddace802a83d08c82f513e30d84010ce31f	2017-05-01T02:35:42Z
 github.com/juju/txn	git	5c6f1c49ecbd4a916ed7b5a8f81ee67203e86043	2017-04-21T05:33:50Z
 github.com/juju/usso	git	68a59c96c178fbbad65926e7f93db50a2cd14f33	2016-04-01T10:44:24Z
-github.com/juju/utils	git	6622634c402775ec773bb05427bccf976e6917e7	2017-04-27T05:01:38Z
+github.com/juju/utils	git	b16611e1db90dde02a570236b12d59ad54d53488	2017-05-02T10:24:56Z
 github.com/juju/version	git	1f41e27e54f21acccf9b2dddae063a782a8a7ceb	2016-10-31T05:19:06Z
 github.com/juju/webbrowser	git	54b8c57083b4afb7dc75da7f13e2967b2606a507	2016-03-09T14:36:29Z
 github.com/juju/xml	git	eb759a627588d35166bc505fceb51b88500e291e	2015-04-13T13:11:21Z

environs/config/config.go

@@ -360,7 +360,7 @@ var defaultConfigValues = map[string]interface{}{
 	AptHTTPProxyKey:  "",
 	AptHTTPSProxyKey: "",
 	AptFTPProxyKey:   "",
-	AptNoProxyKey:    "127.0.0.1,localhost,::1",
+	AptNoProxyKey:    "",
 	"apt-mirror":     "",
 
 	// Status history settings
@@ -704,7 +704,7 @@ func (c *Config) AptFTPProxy() string {
 
 // AptNoProxy returns the 'apt-no-proxy' for the environment.
 func (c *Config) AptNoProxy() string {
-	return c.asString(AptNoProxyKey)
+	return c.getWithFallback(AptNoProxyKey, NoProxyKey)
 }
 
 // AptMirror sets the apt mirror for the environment.

environs/config/config_test.go

@@ -1015,6 +1015,7 @@ func (s *ConfigSuite) TestProxyValuesWithFallback(c *gc.C) {
 	c.Assert(config.FTPProxy(), gc.Equals, "ftp://user@10.0.0.1")
 	c.Assert(config.AptFTPProxy(), gc.Equals, "ftp://user@10.0.0.1")
 	c.Assert(config.NoProxy(), gc.Equals, "localhost,10.0.3.1")
+	c.Assert(config.AptNoProxy(), gc.Equals, "localhost,10.0.3.1")
 }
 
 func (s *ConfigSuite) TestProxyValuesWithFallbackNoScheme(c *gc.C) {
@@ -1033,6 +1034,7 @@ func (s *ConfigSuite) TestProxyValuesWithFallbackNoScheme(c *gc.C) {
 	c.Assert(config.FTPProxy(), gc.Equals, "user@10.0.0.1")
 	c.Assert(config.AptFTPProxy(), gc.Equals, "ftp://user@10.0.0.1")
 	c.Assert(config.NoProxy(), gc.Equals, "localhost,10.0.3.1")
+	c.Assert(config.AptNoProxy(), gc.Equals, "localhost,10.0.3.1")
 }
 
 func (s *ConfigSuite) TestProxyValues(c *gc.C) {
@@ -1078,12 +1080,13 @@ func (s *ConfigSuite) TestProxyConfigMap(c *gc.C) {
 		Http:    "http://http proxy",
 		Https:   "https://https proxy",
 		Ftp:     "ftp://ftp proxy",
-		NoProxy: "127.0.0.1,localhost,::1",
+		NoProxy: "no proxy",
 	}
 	cfg, err := cfg.Apply(config.ProxyConfigMap(proxySettings))
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(cfg.ProxySettings(), gc.DeepEquals, proxySettings)
-	// Apt proxy settings always include the scheme. NoProxy is set to system defaults.
+	cfg, err = cfg.Apply(config.AptProxyConfigMap(proxySettings))
+	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(cfg.AptProxySettings(), gc.DeepEquals, expectedProxySettings)
 }
 

network/hostport.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/juju/errors"
 	"github.com/juju/utils/set"
+	"strings"
 )
 
 // HostPort associates an address with a port.
@@ -194,6 +195,22 @@ func HostPortsToStrings(hps []HostPort) []string {
 	return result
 }
 
+// APIHostPortsToNoProxyString converts list of lists of NetAddrs() to
+// a NoProxy-like comma separated string, ignoring local addresses
+func APIHostPortsToNoProxyString(ahp [][]HostPort) string {
+	noProxySet := set.NewStrings()
+	for _, host := range ahp {
+		for _, hp := range host {
+			if hp.Address.Scope == ScopeMachineLocal ||
+				hp.Address.Scope == ScopeLinkLocal {
+				continue
+			}
+			noProxySet.Add(hp.Address.Value)
+		}
+	}
+	return strings.Join(noProxySet.SortedValues(), ",")
+}
+
 // CollapseHostPorts returns a flattened list of HostPorts keeping the
 // same order they appear in serversHostPorts.
 func CollapseHostPorts(serversHostPorts [][]HostPort) []HostPort {

utils/proxy/proxyconfig.go

@@ -36,7 +36,7 @@ func (pc *ProxyConfig) Set(newSettings proxyutils.Settings) error {
 	}
 	pc.http = httpUrl
 	pc.https = httpsUrl
-	pc.noProxy = newSettings.NoProxy
+	pc.noProxy = newSettings.FullNoProxy()
 	return nil
 }
 

worker/proxyupdater/manifold.go

@@ -53,9 +53,9 @@ func Manifold(config ManifoldConfig) dependency.Manifold {
 				return nil, err
 			}
 			w, err := config.WorkerFunc(Config{
-				Directory:       "/home/ubuntu",
+				SystemdFiles:    []string{"/etc/juju-proxy-systemd.conf"},
+				EnvFiles:        []string{"/etc/juju-proxy.conf"},
 				RegistryPath:    `HKCU:\Software\Microsoft\Windows\CurrentVersion\Internet Settings`,
-				Filename:        ".juju-proxy",
 				API:             proxyAPI,
 				ExternalUpdate:  config.ExternalUpdate,
 				InProcessUpdate: config.InProcessUpdate,

worker/proxyupdater/manifold_test.go

@@ -118,9 +118,9 @@ func (s *ManifoldSuite) TestStartSuccess(c *gc.C) {
 	c.Check(err, jc.ErrorIsNil)
 	dummy, ok := worker.(*dummyWorker)
 	c.Assert(ok, jc.IsTrue)
-	c.Check(dummy.config.Directory, gc.Equals, "/home/ubuntu")
+	c.Check(dummy.config.SystemdFiles, gc.DeepEquals, []string{"/etc/juju-proxy-systemd.conf"})
+	c.Check(dummy.config.EnvFiles, gc.DeepEquals, []string{"/etc/juju-proxy.conf"})
 	c.Check(dummy.config.RegistryPath, gc.Equals, `HKCU:\Software\Microsoft\Windows\CurrentVersion\Internet Settings`)
-	c.Check(dummy.config.Filename, gc.Equals, ".juju-proxy")
 	c.Check(dummy.config.API, gc.NotNil)
 	// Checking function equality is problematic, use the errors they
 	// return.