provider/lxd: improve endpoint handling

provider/lxd/credentials.go

@@ -10,6 +10,7 @@ import (
 	"io"
 	"io/ioutil"
 	"net"
+	"net/url"
 	"os"
 	"path/filepath"
 
@@ -314,7 +315,15 @@ func (p environProviderCredentials) isLocalEndpoint(endpoint string) (bool, erro
 		// LXD controller.
 		return true, nil
 	}
-	endpointAddrs, err := p.lookupHost(endpoint)
+	endpointURL, err := endpointURL(endpoint)
+	if err != nil {
+		return false, errors.Trace(err)
+	}
+	host, _, err := net.SplitHostPort(endpointURL.Host)
+	if err != nil {
+		host = endpointURL.Host
+	}
+	endpointAddrs, err := p.lookupHost(host)
 	if err != nil {
 		return false, errors.Trace(err)
 	}
@@ -325,6 +334,27 @@ func (p environProviderCredentials) isLocalEndpoint(endpoint string) (bool, erro
 	return addrsContainsAny(localAddrs, endpointAddrs), nil
 }
 
+func endpointURL(endpoint string) (*url.URL, error) {
+	remoteURL, err := url.Parse(endpoint)
+	if err != nil || remoteURL.Scheme == "" {
+		remoteURL = &url.URL{
+			Scheme: "https",
+			Host:   endpoint,
+		}
+	} else {
+		// If the user specifies an endpoint, it must be either
+		// host:port, or https://host:port. We do not support
+		// unix:// endpoints at present.
+		if remoteURL.Scheme != "https" {
+			return nil, errors.Errorf(
+				"invalid URL %q: only HTTPS is supported",
+				endpoint,
+			)
+		}
+	}
+	return remoteURL, nil
+}
+
 func addrsContainsAny(haystack []net.Addr, needles []string) bool {
 	for _, needle := range needles {
 		if addrsContains(haystack, needle) {

provider/lxd/environ.go

@@ -15,7 +15,6 @@ import (
 	"github.com/juju/juju/environs/tags"
 	"github.com/juju/juju/instance"
 	"github.com/juju/juju/provider/common"
-	"github.com/juju/juju/tools/lxdclient"
 )
 
 const bootstrapMessage = `To configure your system to better support LXD containers, please see: https://github.com/lxc/lxd/blob/master/doc/production-setup.md`
@@ -140,9 +139,6 @@ func (env *environ) Config() *config.Config {
 
 // PrepareForBootstrap implements environs.Environ.
 func (env *environ) PrepareForBootstrap(ctx environs.BootstrapContext) error {
-	if err := lxdclient.EnableHTTPSListener(env.raw); err != nil {
-		return errors.Annotate(err, "enabling HTTPS listener")
-	}
 	return nil
 }
 

provider/lxd/environ_raw.go

@@ -33,6 +33,7 @@ type lxdCerts interface {
 }
 
 type lxdConfig interface {
+	ServerAddresses() ([]string, error)
 	ServerStatus() (*lxdshared.ServerState, error)
 	SetServerConfig(k, v string) error
 	SetContainerConfig(container, key, value string) error

provider/lxd/environ_test.go

@@ -149,11 +149,3 @@ func (s *environSuite) TestDestroyHostedModels(c *gc.C) {
 		{"RemoveInstances", []interface{}{"juju-", []string{machine1.Name}}},
 	})
 }
-
-func (s *environSuite) TestPrepareForBootstrap(c *gc.C) {
-	err := s.Env.PrepareForBootstrap(envtesting.BootstrapContext(c))
-	c.Assert(err, jc.ErrorIsNil)
-	s.Stub.CheckCalls(c, []gitjujutesting.StubCall{
-		{"SetServerConfig", []interface{}{"core.https_address", "[::]"}},
-	})
-}

provider/lxd/provider.go

@@ -7,6 +7,7 @@ package lxd
 
 import (
 	"net"
+	"strings"
 
 	"github.com/juju/errors"
 	"github.com/juju/jsonschema"
@@ -19,6 +20,7 @@ import (
 	"github.com/juju/juju/environs"
 	"github.com/juju/juju/environs/config"
 	"github.com/juju/juju/provider/lxd/lxdnames"
+	"github.com/juju/juju/tools/lxdclient"
 )
 
 type environProvider struct {
@@ -101,35 +103,22 @@ func (p *environProvider) FinalizeCloud(
 	in cloud.Cloud,
 ) (cloud.Cloud, error) {
 
-	var hostAddress string
-	getHostAddress := func() (string, error) {
-		raw, err := p.newLocalRawProvider()
-		if err != nil {
-			return "", errors.Trace(err)
-		}
-		bridgeName := raw.DefaultProfileBridgeName()
-		hostAddress, err = p.interfaceAddress(bridgeName)
-		if err != nil {
-			return "", errors.Trace(err)
-		}
-		ctx.Verbosef(
-			"Resolved LXD host address on bridge %s: %s",
-			bridgeName, hostAddress,
-		)
-		return hostAddress, nil
-	}
+	var endpoint string
 	resolveEndpoint := func(ep *string) error {
 		if *ep != "" {
 			return nil
 		}
-		if hostAddress == "" {
+		if endpoint == "" {
+			// The cloud endpoint is empty, which means
+			// that we should connect to the local LXD.
 			var err error
-			hostAddress, err = getHostAddress()
+			hostAddress, err := p.getLocalHostAddress(ctx)
 			if err != nil {
 				return err
 			}
+			endpoint = hostAddress
 		}
-		*ep = hostAddress
+		*ep = endpoint
 		return nil
 	}
 
@@ -144,6 +133,48 @@ func (p *environProvider) FinalizeCloud(
 	return in, nil
 }
 
+func (p *environProvider) getLocalHostAddress(ctx environs.FinalizeCloudContext) (string, error) {
+	raw, err := p.newLocalRawProvider()
+	if err != nil {
+		return "", errors.Trace(err)
+	}
+	bridgeName := raw.DefaultProfileBridgeName()
+	hostAddress, err := p.interfaceAddress(bridgeName)
+	if err != nil {
+		return "", errors.Trace(err)
+	}
+	// LXD itself reports the host:ports that is listens on.
+	// Cross-check the address we have with the values
+	// reported by LXD.
+	if err := lxdclient.EnableHTTPSListener(raw); err != nil {
+		return "", errors.Annotate(err, "enabling HTTPS listener")
+	}
+	serverAddresses, err := raw.ServerAddresses()
+	if err != nil {
+		return "", errors.Trace(err)
+	}
+	var found bool
+	for _, addr := range serverAddresses {
+		if strings.HasPrefix(addr, hostAddress+":") {
+			hostAddress = addr
+			found = true
+			break
+		}
+	}
+	if !found {
+		return "", errors.Errorf(
+			"LXD is not listening on address %s ("+
+				"reported addresses: %s)",
+			hostAddress, serverAddresses,
+		)
+	}
+	ctx.Verbosef(
+		"Resolved LXD host address on bridge %s: %s",
+		bridgeName, hostAddress,
+	)
+	return hostAddress, nil
+}
+
 // localhostCloud is the predefined "localhost" LXD cloud. We leave the
 // endpoints empty to indicate that LXD is on the local host. When the
 // cloud is finalized (see FinalizeCloud), we resolve the bridge address
@@ -187,9 +218,15 @@ func (p *environProvider) validateCloudSpec(spec environs.CloudSpec) (local bool
 	if spec.Credential == nil {
 		return false, errors.NotValidf("missing credential")
 	}
-	local, err := p.isLocalEndpoint(spec.Endpoint)
-	if err != nil {
-		return false, errors.Trace(err)
+	if spec.Endpoint == "" {
+		// If we're dealing with an old controller, or we're preparing
+		// a local LXD, we'll have an empty endpoint. Connect to the
+		// default Unix socket.
+		local = true
+	} else {
+		if _, err := endpointURL(spec.Endpoint); err != nil {
+			return false, errors.Trace(err)
+		}
 	}
 	switch authType := spec.Credential.AuthType(); authType {
 	case cloud.CertificateAuthType:

provider/lxd/provider_test.go

@@ -106,17 +106,64 @@ func (s *providerSuite) TestFinalizeCloud(c *gc.C) {
 		Name:      "foo",
 		Type:      "lxd",
 		AuthTypes: []cloud.AuthType{cloud.CertificateAuthType},
-		Endpoint:  "1.2.3.4",
+		Endpoint:  "1.2.3.4:1234",
 		Regions: []cloud.Region{{
 			Name:     "bar",
-			Endpoint: "1.2.3.4",
+			Endpoint: "1.2.3.4:1234",
 		}},
 	})
 	ctx.CheckCallNames(c, "Verbosef")
 	ctx.CheckCall(
 		c, 0, "Verbosef", "Resolved LXD host address on bridge %s: %s",
-		[]interface{}{"test-bridge", "1.2.3.4"},
+		[]interface{}{"test-bridge", "1.2.3.4:1234"},
 	)
+
+	// Finalizing a CloudSpec with an empty endpoint involves
+	// configuring the local LXD to listen for HTTPS.
+	s.Stub.CheckCalls(c, []gitjujutesting.StubCall{
+		{"DefaultProfileBridgeName", nil},
+		{"InterfaceAddress", []interface{}{"test-bridge"}},
+		{"ServerStatus", nil},
+		{"SetServerConfig", []interface{}{"core.https_address", "[::]"}},
+		{"ServerAddresses", nil},
+	})
+}
+
+func (s *providerSuite) TestFinalizeCloudNotListening(c *gc.C) {
+	var ctx mockContext
+	s.PatchValue(&s.InterfaceAddr, "8.8.8.8")
+	_, err := s.Provider.FinalizeCloud(&ctx, cloud.Cloud{
+		Name:      "foo",
+		Type:      "lxd",
+		AuthTypes: []cloud.AuthType{cloud.CertificateAuthType},
+		Regions: []cloud.Region{{
+			Name: "bar",
+		}},
+	})
+	c.Assert(err, gc.NotNil)
+	c.Assert(err.Error(), gc.Equals,
+		`LXD is not listening on address 8.8.8.8 `+
+			`(reported addresses: [127.0.0.1:1234 1.2.3.4:1234])`)
+}
+
+func (s *providerSuite) TestFinalizeCloudAlreadyListeningHTTPS(c *gc.C) {
+	s.Client.ServerState.Config["core.https_address"] = "[::]:9999"
+	var ctx mockContext
+	_, err := s.Provider.FinalizeCloud(&ctx, cloud.Cloud{
+		Name:      "foo",
+		Type:      "lxd",
+		AuthTypes: []cloud.AuthType{cloud.CertificateAuthType},
+	})
+	c.Assert(err, jc.ErrorIsNil)
+
+	// The LXD is already listening on HTTPS, so there should be
+	// no SetServerConfig call.
+	s.Stub.CheckCalls(c, []gitjujutesting.StubCall{
+		{"DefaultProfileBridgeName", nil},
+		{"InterfaceAddress", []interface{}{"test-bridge"}},
+		{"ServerStatus", nil},
+		{"ServerAddresses", nil},
+	})
 }
 
 func (s *providerSuite) TestDetectRegions(c *gc.C) {
@@ -175,6 +222,16 @@ func (s *ProviderFunctionalSuite) TestPrepareConfig(c *gc.C) {
 	c.Check(cfg, gc.NotNil)
 }
 
+func (s *ProviderFunctionalSuite) TestPrepareConfigUnsupportedEndpointScheme(c *gc.C) {
+	cloudSpec := lxdCloudSpec()
+	cloudSpec.Endpoint = "unix://foo"
+	_, err := s.provider.PrepareConfig(environs.PrepareConfigParams{
+		Cloud:  cloudSpec,
+		Config: s.Config,
+	})
+	c.Assert(err, gc.ErrorMatches, `validating cloud spec: invalid URL "unix://foo": only HTTPS is supported`)
+}
+
 func (s *ProviderFunctionalSuite) TestPrepareConfigUnsupportedAuthType(c *gc.C) {
 	cred := cloud.NewCredential("foo", nil)
 	_, err := s.provider.PrepareConfig(environs.PrepareConfigParams{

provider/lxd/testing_test.go

@@ -111,6 +111,7 @@ type BaseSuiteUnpatched struct {
 
 	Ports          []network.PortRange
 	EndpointAddrs  []string
+	InterfaceAddr  string
 	InterfaceAddrs []net.Addr
 }
 
@@ -139,6 +140,7 @@ func (s *BaseSuiteUnpatched) SetUpTest(c *gc.C) {
 func (s *BaseSuiteUnpatched) initProvider(c *gc.C) {
 	s.Provider = &environProvider{}
 	s.EndpointAddrs = []string{"1.2.3.4"}
+	s.InterfaceAddr = "1.2.3.4"
 	s.InterfaceAddrs = []net.Addr{
 		&net.IPNet{IP: net.ParseIP("127.0.0.1")},
 		&net.IPNet{IP: net.ParseIP("1.2.3.4")},
@@ -329,7 +331,15 @@ func (s *BaseSuite) SetUpTest(c *gc.C) {
 	s.BaseSuiteUnpatched.SetUpTest(c)
 
 	s.Stub = &gitjujutesting.Stub{}
-	s.Client = &StubClient{Stub: s.Stub}
+	s.Client = &StubClient{
+		Stub: s.Stub,
+		ServerState: &shared.ServerState{
+			Config: map[string]interface{}{},
+			Environment: shared.ServerStateEnvironment{
+				Certificate: "server-cert",
+			},
+		},
+	}
 	s.Firewaller = &stubFirewaller{stub: s.Stub}
 	s.Common = &stubCommon{stub: s.Stub}
 
@@ -365,7 +375,7 @@ func (s *BaseSuite) SetUpTest(c *gc.C) {
 	}
 	s.Provider.interfaceAddress = func(iface string) (string, error) {
 		s.Stub.AddCall("InterfaceAddress", iface)
-		return "1.2.3.4", s.Stub.NextErr()
+		return s.InterfaceAddr, s.Stub.NextErr()
 	}
 	s.Provider.interfaceAddrs = func() ([]net.Addr, error) {
 		s.Stub.AddCall("InterfaceAddrs")
@@ -475,8 +485,9 @@ func (sc *stubCommon) DestroyEnv() error {
 type StubClient struct {
 	*gitjujutesting.Stub
 
-	Insts []lxdclient.Instance
-	Inst  *lxdclient.Instance
+	Insts       []lxdclient.Instance
+	Inst        *lxdclient.Instance
+	ServerState *shared.ServerState
 }
 
 func (conn *StubClient) Instances(prefix string, statuses ...string) ([]lxdclient.Instance, error) {
@@ -548,11 +559,15 @@ func (conn *StubClient) ServerStatus() (*shared.ServerState, error) {
 	if err := conn.NextErr(); err != nil {
 		return nil, err
 	}
-	return &shared.ServerState{
-		Environment: shared.ServerStateEnvironment{
-			Certificate: "server-cert",
-		},
-	}, nil
+	return conn.ServerState, nil
+}
+
+func (conn *StubClient) ServerAddresses() ([]string, error) {
+	conn.AddCall("ServerAddresses")
+	return []string{
+		"127.0.0.1:1234",
+		"1.2.3.4:1234",
+	}, conn.NextErr()
 }
 
 func (conn *StubClient) SetServerConfig(k, v string) error {

tools/lxdclient/client.go

@@ -193,11 +193,14 @@ func newRawClient(remote Remote) (*lxd.Client, error) {
 
 	if remote.ID() == remoteIDForLocal && host == "" {
 		host = "unix://" + lxdshared.VarPath("unix.socket")
-	} else if !strings.HasPrefix(host, "unix://") {
-		_, _, err := net.SplitHostPort(host)
-		if err != nil {
-			// There is no port here
-			host = net.JoinHostPort(host, lxdshared.DefaultPort)
+	} else {
+		// If it's a URL, leave it alone. Otherwise, we
+		// assume it's a hostname, optionally with port.
+		url, err := url.Parse(host)
+		if err != nil || url.Scheme == "" {
+			if _, _, err := net.SplitHostPort(host); err != nil {
+				host = net.JoinHostPort(host, lxdshared.DefaultPort)
+			}
 		}
 	}
 	logger.Debugf("connecting to LXD remote %q: %q", remote.ID(), host)