Encapsulate proxy settings that get passed to PopulateInstanceConfig

The PopulateInstanceConfig function previously received various proxy-specific settings as arguments. Adding three extra arguments for the snap proxy settings is certainly non-ergonomic. Therefore, a much better approach is to encapsulate all proxy-related settings into a ProxyConfiguration object and pass that to PopulateInstanceConfig.
hatched · Sep 6, 2019 · 3267b3a · 3267b3a
1 parent 6f7591b
commit 3267b3a
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 23 deletions.
diff --git a/cloudconfig/instancecfg/instancecfg.go b/cloudconfig/instancecfg/instancecfg.go
@@ -832,6 +832,49 @@ func NewBootstrapInstanceConfig(
 	return icfg, nil
 }
 
+// ProxyConfiguration encapsulates all proxy-related settings that can be used
+// to populate an InstanceConfig.
+type ProxyConfiguration struct {
+	// Legacy proxy settings.
+	Legacy proxy.Settings
+
+	// Juju-specific proxy settings.
+	Juju proxy.Settings
+
+	// Apt-specific proxy settings.
+	Apt proxy.Settings
+
+	// Snap-specific proxy settings.
+	Snap proxy.Settings
+
+	// Apt mirror.
+	AptMirror string
+
+	// SnapStoreAssertions contains a list of assertions that must be
+	// passed to snapd together with a store proxy ID parameter before it
+	// can connect to a snap store proxy.
+	SnapStoreAssertions string
+
+	// SnapStoreProxyID references a store entry in the snap store
+	// assertion list that must be passed to snapd before it can connect to
+	// a snap store proxy.
+	SnapStoreProxyID string
+}
+
+// proxyConfigurationFromEnv populates a ProxyConfiguration object from an
+// environment Config value.
+func proxyConfigurationFromEnv(cfg *config.Config) ProxyConfiguration {
+	return ProxyConfiguration{
+		Legacy:              cfg.LegacyProxySettings(),
+		Juju:                cfg.JujuProxySettings(),
+		Apt:                 cfg.AptProxySettings(),
+		AptMirror:           cfg.AptMirror(),
+		Snap:                cfg.SnapProxySettings(),
+		SnapStoreAssertions: cfg.SnapStoreAssertions(),
+		SnapStoreProxyID:    cfg.SnapStoreProxy(),
+	}
+}
+
 // PopulateInstanceConfig is called both from the FinishInstanceConfig below,
 // which does have access to the environment config, and from the container
 // provisioners, which don't have access to the environment config. Everything
@@ -841,8 +884,7 @@ func NewBootstrapInstanceConfig(
 func PopulateInstanceConfig(icfg *InstanceConfig,
 	providerType, authorizedKeys string,
 	sslHostnameVerification bool,
-	legacyProxySettings, jujuProxySettings, aptProxySettings proxy.Settings,
-	aptMirror string,
+	proxyCfg ProxyConfiguration,
 	enableOSRefreshUpdates bool,
 	enableOSUpgrade bool,
 	cloudInitUserData map[string]interface{},
@@ -855,12 +897,15 @@ func PopulateInstanceConfig(icfg *InstanceConfig,
 	icfg.AgentEnvironment[agent.ProviderType] = providerType
 	icfg.AgentEnvironment[agent.ContainerType] = string(icfg.MachineContainerType)
 	icfg.DisableSSLHostnameVerification = !sslHostnameVerification
-	icfg.LegacyProxySettings = legacyProxySettings
+	icfg.LegacyProxySettings = proxyCfg.Legacy
 	icfg.LegacyProxySettings.AutoNoProxy = strings.Join(icfg.APIHosts(), ",")
-	icfg.JujuProxySettings = jujuProxySettings
+	icfg.JujuProxySettings = proxyCfg.Juju
 	// No AutoNoProxy needed as juju no proxy values are CIDR aware.
-	icfg.AptProxySettings = aptProxySettings
-	icfg.AptMirror = aptMirror
+	icfg.AptProxySettings = proxyCfg.Apt
+	icfg.AptMirror = proxyCfg.AptMirror
+	icfg.SnapProxySettings = proxyCfg.Snap
+	icfg.SnapStoreAssertions = proxyCfg.SnapStoreAssertions
+	icfg.SnapStoreProxyID = proxyCfg.SnapStoreProxyID
 	icfg.EnableOSRefreshUpdate = enableOSRefreshUpdates
 	icfg.EnableOSUpgrade = enableOSUpgrade
 	icfg.CloudInitUserData = cloudInitUserData
@@ -885,10 +930,7 @@ func FinishInstanceConfig(icfg *InstanceConfig, cfg *config.Config) (err error)
 		cfg.Type(),
 		cfg.AuthorizedKeys(),
 		cfg.SSLHostnameVerification(),
-		cfg.LegacyProxySettings(),
-		cfg.JujuProxySettings(),
-		cfg.AptProxySettings(),
-		cfg.AptMirror(),
+		proxyConfigurationFromEnv(cfg),
 		cfg.EnableOSRefreshUpdate(),
 		cfg.EnableOSUpgrade(),
 		cfg.CloudInitUserData(),

diff --git a/container/broker/broker.go b/container/broker/broker.go
@@ -15,6 +15,7 @@ import (
 	apiprovisioner "github.com/juju/juju/api/provisioner"
 	"github.com/juju/juju/apiserver/params"
 	"github.com/juju/juju/cloudconfig"
+	"github.com/juju/juju/cloudconfig/instancecfg"
 	"github.com/juju/juju/container"
 	"github.com/juju/juju/core/instance"
 	corenetwork "github.com/juju/juju/core/network"
@@ -245,3 +246,17 @@ func combinedCloudInitData(
 
 	return cloudInitData, nil
 }
+
+// proxyConfigurationFromContainerCfg populates a ProxyConfiguration object
+// from an ContenerConfig API response.
+func proxyConfigurationFromContainerCfg(cfg params.ContainerConfig) instancecfg.ProxyConfiguration {
+	return instancecfg.ProxyConfiguration{
+		Legacy:              cfg.LegacyProxy,
+		Juju:                cfg.JujuProxy,
+		Apt:                 cfg.AptProxy,
+		AptMirror:           cfg.AptMirror,
+		Snap:                cfg.SnapProxy,
+		SnapStoreAssertions: cfg.SnapStoreAssertions,
+		SnapStoreProxyID:    cfg.SnapStoreProxyID,
+	}
+}
diff --git a/container/broker/kvm-broker.go b/container/broker/kvm-broker.go
@@ -129,10 +129,7 @@ func (broker *kvmBroker) StartInstance(ctx context.ProviderCallContext, args env
 		config.ProviderType,
 		config.AuthorizedKeys,
 		config.SSLHostnameVerification,
-		config.LegacyProxy,
-		config.JujuProxy,
-		config.AptProxy,
-		config.AptMirror,
+		proxyConfigurationFromContainerCfg(config),
 		config.EnableOSRefreshUpdate,
 		config.EnableOSUpgrade,
 		cloudInitUserData,

diff --git a/container/broker/lxd-broker.go b/container/broker/lxd-broker.go
@@ -128,10 +128,7 @@ func (broker *lxdBroker) StartInstance(ctx context.ProviderCallContext, args env
 		config.ProviderType,
 		config.AuthorizedKeys,
 		config.SSLHostnameVerification,
-		config.LegacyProxy,
-		config.JujuProxy,
-		config.AptProxy,
-		config.AptMirror,
+		proxyConfigurationFromContainerCfg(config),
 		config.EnableOSRefreshUpdate,
 		config.EnableOSUpgrade,
 		cloudInitUserData,

diff --git a/container/lxd/manager_test.go b/container/lxd/manager_test.go
@@ -8,7 +8,6 @@ import (
 	stdtesting "testing"
 
 	"github.com/golang/mock/gomock"
-	"github.com/juju/proxy"
 	jc "github.com/juju/testing/checkers"
 	"github.com/juju/version"
 	lxdclient "github.com/lxc/lxd/client"
@@ -99,10 +98,7 @@ func prepInstanceConfig(c *gc.C) *instancecfg.InstanceConfig {
 		"lxd",
 		"",
 		false,
-		proxy.Settings{},
-		proxy.Settings{},
-		proxy.Settings{},
-		"",
+		instancecfg.ProxyConfiguration{},
 		false,
 		false,
 		nil,