From a4793404832016c0c948e6cd48c47af54885a28f Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Thu, 16 Feb 2017 16:50:36 +0800 Subject: [PATCH] provider/lxd: improve endpoint handling We improve the cloud endpoint handling so that users can now specify any of the following: - (empty) => connect to the unix socket, identify the bridge device used for default profile's eth0 NIC, and identify the IP address assigned to the bridge device on the host. Query LXD for the port it is listening on for HTTPS requests. - host => connect to the specified host on the default port (8443) over HTTPS. - host:port => connect to the specified host and port over HTTPS. - https://host[:port]/path => connect to the specified URL over HTTPS. In the future we should also support unix://, but for now there are too many assumptions, so we reject any endpoint starting with unix://. We will no longer use the unix socket just because the endpoint is a local address. We only use the unix socket to create certificate credentials for the local LXD, and to configure LXD to listen on HTTPS and obtain the appropriate endpoint for communication from within. --- provider/lxd/credentials.go | 32 +++++++++++- provider/lxd/environ.go | 4 -- provider/lxd/environ_raw.go | 1 + provider/lxd/environ_test.go | 8 --- provider/lxd/provider.go | 83 +++++++++++++++++++++++--------- provider/lxd/provider_test.go | 63 ++++++++++++++++++++++-- provider/lxd/testing_test.go | 33 +++++++++---- tools/lxdclient/client.go | 13 +++-- tools/lxdclient/client_config.go | 6 +++ tools/lxdclient/utils.go | 15 ++++-- tools/lxdclient/utils_test.go | 45 +++++++++++++---- 11 files changed, 237 insertions(+), 66 deletions(-) diff --git a/provider/lxd/credentials.go b/provider/lxd/credentials.go index 75dcf1bdc67..01bb8032352 100644 --- a/provider/lxd/credentials.go +++ b/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) { diff --git a/provider/lxd/environ.go b/provider/lxd/environ.go index c7cf3aebcc0..07499eb7038 100644 --- a/provider/lxd/environ.go +++ b/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 } diff --git a/provider/lxd/environ_raw.go b/provider/lxd/environ_raw.go index 2fb568fe612..054e62479d8 100644 --- a/provider/lxd/environ_raw.go +++ b/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 diff --git a/provider/lxd/environ_test.go b/provider/lxd/environ_test.go index cd646b1e4ec..4ac7185a027 100644 --- a/provider/lxd/environ_test.go +++ b/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", "[::]"}}, - }) -} diff --git a/provider/lxd/provider.go b/provider/lxd/provider.go index ba909c61e85..8725661f308 100644 --- a/provider/lxd/provider.go +++ b/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: diff --git a/provider/lxd/provider_test.go b/provider/lxd/provider_test.go index 310f7286586..944091da87c 100644 --- a/provider/lxd/provider_test.go +++ b/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{ diff --git a/provider/lxd/testing_test.go b/provider/lxd/testing_test.go index 07cd78a745e..068e19f2cfd 100644 --- a/provider/lxd/testing_test.go +++ b/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 { diff --git a/tools/lxdclient/client.go b/tools/lxdclient/client.go index 78fd6898cff..5089339a690 100644 --- a/tools/lxdclient/client.go +++ b/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) diff --git a/tools/lxdclient/client_config.go b/tools/lxdclient/client_config.go index d6bed86a9a5..357075cd69c 100644 --- a/tools/lxdclient/client_config.go +++ b/tools/lxdclient/client_config.go @@ -12,6 +12,7 @@ import ( ) type rawConfigClient interface { + Addresses() ([]string, error) SetServerConfig(key, value string) (*lxd.Response, error) SetContainerConfig(container, key, value string) error @@ -51,3 +52,8 @@ func (c configClient) SetContainerConfig(container, key, value string) error { func (c configClient) ServerStatus() (*shared.ServerState, error) { return c.raw.ServerStatus() } + +// ServerAddresses reports the addresses that the server is listening on. +func (c configClient) ServerAddresses() ([]string, error) { + return c.raw.Addresses() +} diff --git a/tools/lxdclient/utils.go b/tools/lxdclient/utils.go index f47334e6808..55235bee6ca 100644 --- a/tools/lxdclient/utils.go +++ b/tools/lxdclient/utils.go @@ -11,6 +11,7 @@ import ( "github.com/juju/errors" "github.com/juju/utils/series" + "github.com/lxc/lxd/shared" "github.com/juju/juju/service" "github.com/juju/juju/service/common" @@ -71,17 +72,25 @@ const errIPV6NotSupported = `socket: address family not supported by protocol` // EnableHTTPSListener configures LXD to listen for HTTPS requests, // rather than only via the Unix socket. func EnableHTTPSListener(client interface { + ServerStatus() (*shared.ServerState, error) SetServerConfig(k, v string) error }) error { + // First check that the server is not already listening for HTTPS. + state, err := client.ServerStatus() + if err != nil { + return errors.Trace(err) + } + if _, ok := state.Config["core.https_address"]; ok { + return nil + } + // Make sure the LXD service is configured to listen to local https // requests, rather than only via the Unix socket. // TODO: jam 2016-02-25 This tells LXD to listen on all addresses, // which does expose the LXD to outside requests. It would // probably be better to only tell LXD to listen for requests on // the loopback and LXC bridges that we are using. - err := client.SetServerConfig("core.https_address", "[::]") - - if err != nil { + if err := client.SetServerConfig("core.https_address", "[::]"); err != nil { // if the error hints that the problem might be a protocol unsoported // such as what happens when IPV6 is disabled in kernel, we try IPV4 // as a fallback. diff --git a/tools/lxdclient/utils_test.go b/tools/lxdclient/utils_test.go index b13c42d8cad..38f1f45b4d4 100644 --- a/tools/lxdclient/utils_test.go +++ b/tools/lxdclient/utils_test.go @@ -10,6 +10,7 @@ import ( "github.com/juju/testing" jc "github.com/juju/testing/checkers" + "github.com/lxc/lxd/shared" gc "gopkg.in/check.v1" "github.com/juju/juju/tools/lxdclient" @@ -24,30 +25,54 @@ type utilsSuite struct { } func (s *utilsSuite) TestEnableHTTPSListener(c *gc.C) { - var client mockConfigSetter - err := lxdclient.EnableHTTPSListener(&client) + client := newMockConfigSetter() + err := lxdclient.EnableHTTPSListener(client) c.Assert(err, jc.ErrorIsNil) - client.CheckCall(c, 0, "SetServerConfig", "core.https_address", "[::]") + client.CheckCall(c, 0, "ServerStatus") + client.CheckCall(c, 1, "SetServerConfig", "core.https_address", "[::]") +} + +func (s *utilsSuite) TestEnableHTTPSListenerAlreadyEnabled(c *gc.C) { + client := newMockConfigSetter() + client.ServerState.Config["core.https_address"] = "foo" + err := lxdclient.EnableHTTPSListener(client) + c.Assert(err, jc.ErrorIsNil) + client.CheckCallNames(c, "ServerStatus") } func (s *utilsSuite) TestEnableHTTPSListenerError(c *gc.C) { - var client mockConfigSetter + client := newMockConfigSetter() client.SetErrors(errors.New("uh oh")) - err := lxdclient.EnableHTTPSListener(&client) + err := lxdclient.EnableHTTPSListener(client) c.Assert(err, gc.ErrorMatches, "uh oh") } func (s *utilsSuite) TestEnableHTTPSListenerIPV4Fallback(c *gc.C) { - var client mockConfigSetter - client.SetErrors(errors.New("any error string added by lxd: socket: address family not supported by protocol")) - err := lxdclient.EnableHTTPSListener(&client) + client := newMockConfigSetter() + client.SetErrors(nil, errors.New("any error string added by lxd: socket: address family not supported by protocol")) + err := lxdclient.EnableHTTPSListener(client) c.Assert(err, jc.ErrorIsNil) - client.CheckCall(c, 0, "SetServerConfig", "core.https_address", "[::]") - client.CheckCall(c, 1, "SetServerConfig", "core.https_address", "0.0.0.0") + client.CheckCall(c, 0, "ServerStatus") + client.CheckCall(c, 1, "SetServerConfig", "core.https_address", "[::]") + client.CheckCall(c, 2, "SetServerConfig", "core.https_address", "0.0.0.0") } type mockConfigSetter struct { testing.Stub + ServerState *shared.ServerState +} + +func newMockConfigSetter() *mockConfigSetter { + return &mockConfigSetter{ + ServerState: &shared.ServerState{ + Config: map[string]interface{}{}, + }, + } +} + +func (m *mockConfigSetter) ServerStatus() (*shared.ServerState, error) { + m.MethodCall(m, "ServerStatus") + return m.ServerState, m.NextErr() } func (m *mockConfigSetter) SetServerConfig(k, v string) error {