From 9417385677581e27fc163296f8aec036f0aeebed Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Wed, 24 Aug 2016 13:06:35 +0800 Subject: [PATCH] provider/lxd: drop all existing config The LXD provider is updated such that the LXD host address and certificates are no longer stored in model config. This makes it possible to drop the following: - EnvironProvider.RestrictedConfigAttributes - EnvironProvider.SecretAttrs - on-the-fly addition and removal of credential attributes to/from model config --- cmd/juju/backups/restore_test.go | 2 +- provider/lxd/config.go | 269 +------------------------------ provider/lxd/config_test.go | 182 +-------------------- provider/lxd/credentials.go | 14 +- provider/lxd/credentials_test.go | 2 +- provider/lxd/environ.go | 64 ++------ provider/lxd/environ_broker.go | 44 ++++- provider/lxd/environ_raw.go | 120 ++++++++++++-- provider/lxd/environ_raw_test.go | 97 +++++++++++ provider/lxd/environ_test.go | 11 +- provider/lxd/provider.go | 59 +++---- provider/lxd/provider_test.go | 27 +++- provider/lxd/testing_test.go | 98 +++-------- tools/lxdclient/config.go | 13 +- tools/lxdclient/config_test.go | 2 + tools/lxdclient/remote.go | 8 +- tools/lxdclient/remote_test.go | 15 +- 17 files changed, 359 insertions(+), 668 deletions(-) create mode 100644 provider/lxd/environ_raw_test.go diff --git a/cmd/juju/backups/restore_test.go b/cmd/juju/backups/restore_test.go index 9d59ad9950d..8ebff2a3321 100644 --- a/cmd/juju/backups/restore_test.go +++ b/cmd/juju/backups/restore_test.go @@ -272,7 +272,7 @@ func (s *restoreSuite) TestRestoreReboostrapBuiltInProvider(c *gc.C) { sort.Sort(args.Cloud.AuthTypes) c.Assert(args.Cloud, jc.DeepEquals, cloud.Cloud{ Type: "lxd", - AuthTypes: []cloud.AuthType{"certificate", "empty"}, + AuthTypes: []cloud.AuthType{"empty"}, Regions: []cloud.Region{{Name: "localhost"}}, }) return nil diff --git a/provider/lxd/config.go b/provider/lxd/config.go index d91f5353af9..e9d7b11c523 100644 --- a/provider/lxd/config.go +++ b/provider/lxd/config.go @@ -6,111 +6,25 @@ package lxd import ( - "fmt" - "github.com/juju/errors" "github.com/juju/schema" "gopkg.in/juju/environschema.v1" "github.com/juju/juju/environs/config" - "github.com/juju/juju/tools/lxdclient" -) - -// TODO(ericsnow) Support providing cert/key file. - -// The LXD-specific config keys. -const ( - cfgRemoteURL = "remote-url" - cfgClientCert = "client-cert" - cfgClientKey = "client-key" - cfgServerPEMCert = "server-cert" ) -// configSchema defines the schema for the configuration attributes -// defined by the LXD provider. -var configSchema = environschema.Fields{ - cfgRemoteURL: { - Description: `Identifies the LXD API server to use for managing containers, if any.`, - Type: environschema.Tstring, - Immutable: true, - }, - cfgClientKey: { - Description: `The client key used for connecting to a LXD host machine.`, - Type: environschema.Tstring, - Immutable: true, - }, - cfgClientCert: { - Description: `The client cert used for connecting to a LXD host machine.`, - Type: environschema.Tstring, - Immutable: true, - }, - cfgServerPEMCert: { - Description: `The certificate of the LXD server on the host machine.`, - Type: environschema.Tstring, - Immutable: true, - }, -} - var ( - // TODO(ericsnow) Extract the defaults from configSchema as soon as - // (or if) environschema.Attr supports defaults. - - configBaseDefaults = schema.Defaults{ - cfgRemoteURL: "", - cfgClientCert: "", - cfgClientKey: "", - cfgServerPEMCert: "", - } - + configSchema = environschema.Fields{} + configBaseDefaults = schema.Defaults{} configFields, configDefaults = func() (schema.Fields, schema.Defaults) { fields, defaults, err := configSchema.ValidationSchema() if err != nil { panic(err) } - defaults = updateDefaults(defaults, configBaseDefaults) return fields, defaults }() - - configSecretFields = []string{ - cfgClientKey, // only privileged agents should get to talk to LXD - } ) -func updateDefaults(defaults schema.Defaults, updates schema.Defaults) schema.Defaults { - updated := schema.Defaults{} - for k, v := range defaults { - updated[k] = v - } - for k, v := range updates { - // TODO(ericsnow) Delete the item if v is nil? - updated[k] = v - } - return updated -} - -func adjustDefaults(cfg *config.Config, defaults map[string]interface{}) (map[string]interface{}, []string) { - var unset []string - updated := make(map[string]interface{}) - for k, v := range defaults { - updated[k] = v - } - - return updated, unset -} - -// TODO(ericsnow) environschema.Fields should have this... -func ensureImmutableFields(oldAttrs, newAttrs map[string]interface{}) error { - for name, attr := range configSchema { - if !attr.Immutable { - continue - } - if newAttrs[name] != oldAttrs[name] { - return errors.Errorf("%s: cannot change from %v to %v", name, oldAttrs[name], newAttrs[name]) - } - } - return nil -} - type environConfig struct { *config.Config attrs map[string]interface{} @@ -128,41 +42,14 @@ func newConfig(cfg *config.Config) *environConfig { // newValidConfig builds a new environConfig from the provided Config // and returns it. This includes applying the provided defaults // values, if any. The resulting config values are validated. -func newValidConfig(cfg *config.Config, defaults map[string]interface{}) (*environConfig, error) { - // Any auth credentials handling should happen first... - +func newValidConfig(cfg *config.Config) (*environConfig, error) { // Ensure that the provided config is valid. if err := config.Validate(cfg, nil); err != nil { return nil, errors.Trace(err) } - // Apply the defaults and coerce/validate the custom config attrs. - fixedDefaults, unset := adjustDefaults(cfg, defaults) - cfg, err := cfg.Remove(unset) - if err != nil { - return nil, errors.Trace(err) - } - validated, err := cfg.ValidateUnknownAttrs(configFields, fixedDefaults) - if err != nil { - return nil, errors.Trace(err) - } - validCfg, err := cfg.Apply(validated) - if err != nil { - return nil, errors.Trace(err) - } - // Build the config. - ecfg := newConfig(validCfg) - - // Update to defaults set via client config. - clientCfg, err := ecfg.clientConfig() - if err != nil { - return nil, errors.Trace(err) - } - ecfg, err = ecfg.updateForClientConfig(clientCfg) - if err != nil { - return nil, errors.Trace(err) - } + ecfg := newConfig(cfg) // Do final (more complex, provider-specific) validation. if err := ecfg.validate(); err != nil { @@ -172,153 +59,7 @@ func newValidConfig(cfg *config.Config, defaults map[string]interface{}) (*envir return ecfg, nil } -func (c *environConfig) dirname() string { - // TODO(ericsnow) Put it under one of the juju/paths.*() directories. - return "" -} - -func (c *environConfig) remoteURL() string { - raw := c.attrs[cfgRemoteURL] - return raw.(string) -} - -func (c *environConfig) clientCert() string { - raw := c.attrs[cfgClientCert] - return raw.(string) -} - -func (c *environConfig) clientKey() string { - raw := c.attrs[cfgClientKey] - return raw.(string) -} - -func (c *environConfig) serverPEMCert() string { - raw := c.attrs[cfgServerPEMCert] - return raw.(string) -} - -// clientConfig builds a LXD Config based on the env config and returns it. -func (c *environConfig) clientConfig() (lxdclient.Config, error) { - remote := lxdclient.Remote{ - Name: "juju-remote", - Host: c.remoteURL(), - ServerPEMCert: c.serverPEMCert(), - } - if c.clientCert() != "" { - certPEM := []byte(c.clientCert()) - keyPEM := []byte(c.clientKey()) - cert := lxdclient.NewCert(certPEM, keyPEM) - cert.Name = fmt.Sprintf("juju cert for env %q", c.Name()) - remote.Cert = &cert - } - - cfg := lxdclient.Config{ - Remote: remote, - } - cfg, err := cfg.WithDefaults() - if err != nil { - return cfg, errors.Trace(err) - } - return cfg, nil -} - -// TODO(ericsnow) Switch to a DI testing approach and eliminiate this var. -var asNonLocal = lxdclient.Config.UsingTCPRemote - -func (c *environConfig) updateForClientConfig(clientCfg lxdclient.Config) (*environConfig, error) { - nonlocal, err := asNonLocal(clientCfg) - if err != nil { - return nil, errors.Trace(err) - } - clientCfg = nonlocal - - c.attrs[cfgRemoteURL] = clientCfg.Remote.Host - c.attrs[cfgServerPEMCert] = clientCfg.Remote.ServerPEMCert - - var cert lxdclient.Cert - if clientCfg.Remote.Cert != nil { - cert = *clientCfg.Remote.Cert - } - c.attrs[cfgClientCert] = string(cert.CertPEM) - c.attrs[cfgClientKey] = string(cert.KeyPEM) - - // Apply the updates. - cfg, err := c.Config.Apply(c.attrs) - if err != nil { - return nil, errors.Trace(err) - } - return newConfig(cfg), nil -} - -// secret gathers the "secret" config values and returns them. -func (c *environConfig) secret() map[string]string { - if len(configSecretFields) == 0 { - return nil - } - - secretAttrs := make(map[string]string, len(configSecretFields)) - for _, key := range configSecretFields { - secretAttrs[key] = c.attrs[key].(string) - } - return secretAttrs -} - -// validate checks more complex LCD-specific config values. +// validate validates LXD-specific configuration. func (c *environConfig) validate() error { - // All fields must be populated, even with just the default. - // TODO(ericsnow) Shouldn't configSchema support this? - for field := range configFields { - if dflt, ok := configDefaults[field]; ok && dflt == "" { - continue - } - if c.attrs[field].(string) == "" { - return errors.Errorf("%s: must not be empty", field) - } - } - - // If cert is provided then key must be (and vice versa). - if c.clientCert() == "" && c.clientKey() != "" { - return errors.Errorf("missing %s (got %s value %q)", cfgClientCert, cfgClientKey, c.clientKey()) - } - if c.clientCert() != "" && c.clientKey() == "" { - return errors.Errorf("missing %s (got %s value %q)", cfgClientKey, cfgClientCert, c.clientCert()) - } - - // Check sanity of complex provider-specific fields. - cfg, err := c.clientConfig() - if err != nil { - return errors.Trace(err) - } - if err := cfg.Validate(); err != nil { - return errors.Trace(err) - } - - return nil -} - -// update applies changes from the provided config to the env config. -// Changes to any immutable attributes result in an error. -func (c *environConfig) update(cfg *config.Config) error { - // Validate the updates. newValidConfig does not modify the "known" - // config attributes so it is safe to call Validate here first. - if err := config.Validate(cfg, c.Config); err != nil { - return errors.Trace(err) - } - - updates, err := newValidConfig(cfg, configDefaults) - if err != nil { - return errors.Trace(err) - } - - // Check that no immutable fields have changed. - attrs := updates.UnknownAttrs() - if err := ensureImmutableFields(c.attrs, attrs); err != nil { - return errors.Trace(err) - } - - // Apply the updates. - // TODO(ericsnow) Should updates.Config be set in instead of cfg? - c.Config = cfg - c.attrs = cfg.UnknownAttrs() return nil } diff --git a/provider/lxd/config_test.go b/provider/lxd/config_test.go index 409c9e1695e..f7be11b78b4 100644 --- a/provider/lxd/config_test.go +++ b/provider/lxd/config_test.go @@ -6,8 +6,6 @@ package lxd_test import ( - "fmt" - jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" "gopkg.in/juju/environschema.v1" @@ -16,7 +14,6 @@ import ( "github.com/juju/juju/environs/config" "github.com/juju/juju/provider/lxd" "github.com/juju/juju/testing" - "github.com/juju/juju/tools/lxdclient" ) type configSuite struct { @@ -42,154 +39,7 @@ func (s *configSuite) TestDefaults(c *gc.C) { values, extras := ecfg.Values(c) c.Assert(extras, gc.HasLen, 0) - c.Check(values, jc.DeepEquals, lxd.ConfigValues{ - RemoteURL: "", - ClientCert: "", - ClientKey: "", - ServerCert: "", - }) -} - -func (s *configSuite) TestClientConfigLocal(c *gc.C) { - cfg := lxd.NewBaseConfig(c) - ecfg := lxd.NewConfig(cfg) - values, _ := ecfg.Values(c) - c.Assert(values.RemoteURL, gc.Equals, "") - - clientCfg, err := ecfg.ClientConfig() - c.Assert(err, jc.ErrorIsNil) - - c.Check(clientCfg, jc.DeepEquals, lxdclient.Config{ - Remote: lxdclient.Remote{ - Name: "juju-remote", - Host: "", - Protocol: lxdclient.LXDProtocol, - Cert: nil, - ServerPEMCert: "", - }, - }) -} - -func (s *configSuite) TestClientConfigNonLocal(c *gc.C) { - cfg := lxd.NewBaseConfig(c) - ecfg := lxd.NewConfig(cfg) - ecfg = ecfg.Apply(c, map[string]interface{}{ - "remote-url": "10.0.0.1", - "client-cert": "", - "client-key": "", - "server-cert": "", - }) - - clientCfg, err := ecfg.ClientConfig() - c.Assert(err, jc.ErrorIsNil) - - c.Check(clientCfg, jc.DeepEquals, lxdclient.Config{ - Remote: lxdclient.Remote{ - Name: "juju-remote", - Host: "10.0.0.1", - Protocol: lxdclient.LXDProtocol, - Cert: &lxdclient.Cert{ - Name: fmt.Sprintf("juju cert for env %q", s.config.Name()), - CertPEM: []byte(""), - KeyPEM: []byte(""), - }, - ServerPEMCert: "", - }, - }) -} - -func (s *configSuite) TestUpdateForClientConfigLocal(c *gc.C) { - cfg := lxd.NewBaseConfig(c) - ecfg := lxd.NewConfig(cfg) - - clientCfg, err := ecfg.ClientConfig() - c.Assert(err, jc.ErrorIsNil) - updated, err := ecfg.UpdateForClientConfig(clientCfg) - c.Assert(err, jc.ErrorIsNil) - - values, extras := updated.Values(c) - c.Assert(extras, gc.HasLen, 0) - - c.Check(values, jc.DeepEquals, lxd.ConfigValues{ - RemoteURL: "", - ClientCert: "", - ClientKey: "", - ServerCert: "", - }) -} - -func (s *configSuite) TestUpdateForClientConfigNonLocal(c *gc.C) { - cfg := lxd.NewBaseConfig(c) - ecfg := lxd.NewConfig(cfg) - ecfg = ecfg.Apply(c, map[string]interface{}{ - "remote-url": "10.0.0.1", - "client-cert": "", - "client-key": "", - "server-cert": "", - }) - - before, extras := ecfg.Values(c) - c.Assert(extras, gc.HasLen, 0) - - clientCfg, err := ecfg.ClientConfig() - c.Assert(err, jc.ErrorIsNil) - updated, err := ecfg.UpdateForClientConfig(clientCfg) - c.Assert(err, jc.ErrorIsNil) - - after, extras := updated.Values(c) - c.Assert(extras, gc.HasLen, 0) - - c.Check(before, jc.DeepEquals, lxd.ConfigValues{ - RemoteURL: "10.0.0.1", - ClientCert: "", - ClientKey: "", - ServerCert: "", - }) - c.Check(after, jc.DeepEquals, lxd.ConfigValues{ - RemoteURL: "10.0.0.1", - ClientCert: "", - ClientKey: "", - ServerCert: "", - }) -} - -func (s *configSuite) TestUpdateForClientConfigGeneratedCert(c *gc.C) { - cfg := lxd.NewBaseConfig(c) - ecfg := lxd.NewConfig(cfg) - ecfg = ecfg.Apply(c, map[string]interface{}{ - "remote-url": "10.0.0.1", - "client-cert": "", - "client-key": "", - "server-cert": "", - }) - - before, extras := ecfg.Values(c) - c.Assert(extras, gc.HasLen, 0) - - clientCfg, err := ecfg.ClientConfig() - c.Assert(err, jc.ErrorIsNil) - updated, err := ecfg.UpdateForClientConfig(clientCfg) - c.Assert(err, jc.ErrorIsNil) - - after, extras := updated.Values(c) - c.Assert(extras, gc.HasLen, 0) - - c.Check(before, jc.DeepEquals, lxd.ConfigValues{ - RemoteURL: "10.0.0.1", - ClientCert: "", - ClientKey: "", - ServerCert: "", - }) - after.CheckCert(c) - after.ClientCert = "" - after.ClientKey = "" - after.ServerCert = "" - c.Check(after, jc.DeepEquals, lxd.ConfigValues{ - RemoteURL: "10.0.0.1", - ClientCert: "", - ClientKey: "", - ServerCert: "", - }) + c.Check(values, jc.DeepEquals, lxd.ConfigValues{}) } // TODO(ericsnow) Each test only deals with a single field, so having @@ -275,34 +125,6 @@ func updateAttrs(attrs, updates testing.Attrs) testing.Attrs { } var newConfigTests = []configTestSpec{{ - info: "remote-url is optional", - remove: []string{"remote-url"}, - expect: testing.Attrs{"remote-url": ""}, -}, { - info: "remote-url can be empty", - insert: testing.Attrs{"remote-url": ""}, - expect: testing.Attrs{"remote-url": ""}, -}, { - info: "client-cert is optional", - remove: []string{"client-cert"}, - expect: testing.Attrs{"client-cert": ""}, -}, { - info: "client-cert can be empty", - insert: testing.Attrs{"client-cert": ""}, - expect: testing.Attrs{"client-cert": ""}, -}, { - info: "client-key is optional", - remove: []string{"client-key"}, - expect: testing.Attrs{"client-key": ""}, -}, { - info: "client-key can be empty", - insert: testing.Attrs{"client-key": ""}, - expect: testing.Attrs{"client-key": ""}, -}, { - info: "server-cert is optional", - remove: []string{"server-cert"}, - expect: testing.Attrs{"server-cert": ""}, -}, { info: "unknown field is not touched", insert: testing.Attrs{"unknown-field": 12345}, expect: testing.Attrs{"unknown-field": 12345}, @@ -381,8 +203,6 @@ func (s *configSuite) TestValidateOldConfig(c *gc.C) { } } -// TODO(ericsnow) Add tests for client-cert and client-key. - var changeConfigTests = []configTestSpec{{ info: "no change, no error", expect: lxd.ConfigAttrs, diff --git a/provider/lxd/credentials.go b/provider/lxd/credentials.go index cdb8f681ea9..b16f26abc4e 100644 --- a/provider/lxd/credentials.go +++ b/provider/lxd/credentials.go @@ -16,19 +16,7 @@ func (environProviderCredentials) CredentialSchemas() map[cloud.AuthType]cloud.C // TODO (anastasiamac 2016-04-14) When/If this value changes, // verify that juju/juju/cloud/clouds.go#BuiltInClouds // with lxd type are up to-date. - // TODO(wallyworld) update BuiltInClouds to match when we actually take notice of TLSAuthType - return map[cloud.AuthType]cloud.CredentialSchema{ - cloud.EmptyAuthType: {}, - cloud.CertificateAuthType: { - { - cfgClientCert, cloud.CredentialAttr{Description: "The client cert used for connecting to a LXD host machine."}, - }, { - cfgClientKey, cloud.CredentialAttr{Description: "The client key used for connecting to a LXD host machine."}, - }, { - cfgServerPEMCert, cloud.CredentialAttr{Description: "The certificate of the LXD server on the host machine."}, - }, - }, - } + return map[cloud.AuthType]cloud.CredentialSchema{cloud.EmptyAuthType: {}} } // DetectCredentials is part of the environs.ProviderCredentials interface. diff --git a/provider/lxd/credentials_test.go b/provider/lxd/credentials_test.go index 0540f3f8925..d8b57ab8e92 100644 --- a/provider/lxd/credentials_test.go +++ b/provider/lxd/credentials_test.go @@ -29,7 +29,7 @@ func (s *credentialsSuite) SetUpTest(c *gc.C) { } func (s *credentialsSuite) TestCredentialSchemas(c *gc.C) { - envtesting.AssertProviderAuthTypes(c, s.provider, "certificate", "empty") + envtesting.AssertProviderAuthTypes(c, s.provider, "empty") } func (s *credentialsSuite) TestDetectCredentials(c *gc.C) { diff --git a/provider/lxd/environ.go b/provider/lxd/environ.go index 58f5e6c76c5..b00bf558f57 100644 --- a/provider/lxd/environ.go +++ b/provider/lxd/environ.go @@ -38,29 +38,32 @@ type environ struct { ecfg *environConfig } -type newRawProviderFunc func(*environConfig) (*rawProvider, error) +type newRawProviderFunc func(environs.CloudSpec) (*rawProvider, error) -func newEnviron(cfg *config.Config, newRawProvider newRawProviderFunc) (*environ, error) { - ecfg, err := newValidConfig(cfg, configDefaults) +func newEnviron(spec environs.CloudSpec, cfg *config.Config, newRawProvider newRawProviderFunc) (*environ, error) { + ecfg, err := newValidConfig(cfg) if err != nil { return nil, errors.Annotate(err, "invalid config") } - // Connect and authenticate. - raw, err := newRawProvider(ecfg) + namespace, err := instance.NewNamespace(cfg.UUID()) if err != nil { return nil, errors.Trace(err) } - env, err := newEnvironRaw(ecfg, raw) + raw, err := newRawProvider(spec) if err != nil { return nil, errors.Trace(err) } - env.namespace, err = instance.NewNamespace(cfg.UUID()) - if err != nil { - return nil, errors.Trace(err) + env := &environ{ + name: ecfg.Name(), + uuid: ecfg.UUID(), + raw: raw, + namespace: namespace, + ecfg: ecfg, } + env.base = common.DefaultProvider{Env: env} //TODO(wwitzel3) make sure we are also cleaning up profiles during destroy if err := env.initProfile(); err != nil { @@ -70,17 +73,6 @@ func newEnviron(cfg *config.Config, newRawProvider newRawProviderFunc) (*environ return env, nil } -func newEnvironRaw(ecfg *environConfig, raw *rawProvider) (*environ, error) { - env := &environ{ - name: ecfg.Name(), - uuid: ecfg.UUID(), - ecfg: ecfg, - raw: raw, - } - env.base = common.DefaultProvider{Env: env} - return env, nil -} - var defaultProfileConfig = map[string]string{ "boot.autostart": "true", "security.nesting": "true", @@ -117,14 +109,11 @@ func (*environ) Provider() environs.EnvironProvider { func (env *environ) SetConfig(cfg *config.Config) error { env.lock.Lock() defer env.lock.Unlock() - - if env.ecfg == nil { - return errors.New("cannot set config on uninitialized env") - } - - if err := env.ecfg.update(cfg); err != nil { - return errors.Annotate(err, "invalid config change") + ecfg, err := newValidConfig(cfg) + if err != nil { + return errors.Trace(err) } + env.ecfg = ecfg return nil } @@ -138,30 +127,16 @@ func (env *environ) Config() *config.Config { // PrepareForBootstrap implements environs.Environ. func (env *environ) PrepareForBootstrap(ctx environs.BootstrapContext) error { - if ctx.ShouldVerifyCredentials() { - if err := env.verifyCredentials(); err != nil { - return errors.Trace(err) - } - } return nil } // Create implements environs.Environ. func (env *environ) Create(environs.CreateParams) error { - if err := env.verifyCredentials(); err != nil { - return errors.Trace(err) - } return nil } -// Bootstrap creates a new instance, chosing the series and arch out of -// available tools. The series and arch are returned along with a func -// that must be called to finalize the bootstrap process by transferring -// the tools and installing the initial juju controller. +// Bootstrap implements environs.Environ. func (env *environ) Bootstrap(ctx environs.BootstrapContext, params environs.BootstrapParams) (*environs.BootstrapResult, error) { - // TODO(ericsnow) Ensure currently not the root user - // if remote is local host? - // Using the Bootstrap func from provider/common should be fine. // Local provider does its own thing because it has to deal directly // with localhost rather than using SSH. @@ -219,8 +194,3 @@ func (env *environ) destroyHostedModelResources(controllerUUID string) error { } return nil } - -func (env *environ) verifyCredentials() error { - // TODO(ericsnow) Do something here? - return nil -} diff --git a/provider/lxd/environ_broker.go b/provider/lxd/environ_broker.go index 4de0a0a18ea..69ec86fef14 100644 --- a/provider/lxd/environ_broker.go +++ b/provider/lxd/environ_broker.go @@ -11,7 +11,9 @@ import ( "github.com/juju/errors" "github.com/juju/utils/arch" + lxdshared "github.com/lxc/lxd/shared" + "github.com/juju/juju/cloudconfig/cloudinit" "github.com/juju/juju/cloudconfig/instancecfg" "github.com/juju/juju/cloudconfig/providerinit" "github.com/juju/juju/environs" @@ -72,13 +74,9 @@ func (env *environ) finishInstanceConfig(args environs.StartInstanceParams) erro if err != nil { return errors.Trace(err) } - if len(tools) == 0 { - return errors.Errorf("No tools available for architecture %q", arch.HostArch()) - } if err := args.InstanceConfig.SetTools(tools); err != nil { return errors.Trace(err) } - logger.Debugf("tools: %#v", args.InstanceConfig.ToolsList()) if err := instancecfg.FinishInstanceConfig(args.InstanceConfig, env.ecfg.Config); err != nil { return errors.Trace(err) @@ -144,7 +142,7 @@ func (env *environ) newRawInstance(args environs.StartInstanceParams) (*lxdclien return nil, errors.Trace(err) } - series := args.Tools.OneSeries() + series := args.InstanceConfig.Series // TODO(jam): We should get this information from EnsureImageExists, or // something given to us from 'raw', not assume it ourselves. image := "ubuntu-" + series @@ -179,10 +177,40 @@ func (env *environ) newRawInstance(args environs.StartInstanceParams) (*lxdclien } cleanupCallback() // Clean out any long line of completed download status - metadata, err := getMetadata(args) + cloudcfg, err := cloudinit.New(series) if err != nil { return nil, errors.Trace(err) } + if args.InstanceConfig.Controller != nil { + // For controller machines, generate a certificate pair and write + // them to the instance's disk in a well-defined location, along + // with the server's certificate. + certPEM, keyPEM, err := lxdshared.GenerateMemCert() + if err != nil { + return nil, errors.Trace(err) + } + cert := lxdclient.NewCert(certPEM, keyPEM) + cert.Name = hostname + // TODO(axw) 2016-08-24 #1616346 + // We need to remove this cert when removing + // the machine and/or destroying the controller. + if err := env.raw.AddCert(cert); err != nil { + return nil, errors.Annotatef(err, "adding certificate %q", cert.Name) + } + serverState, err := env.raw.ServerStatus() + if err != nil { + return nil, errors.Annotate(err, "getting server status") + } + cloudcfg.AddRunTextFile(clientCertPath, string(certPEM), 0600) + cloudcfg.AddRunTextFile(clientKeyPath, string(keyPEM), 0600) + cloudcfg.AddRunTextFile(serverCertPath, serverState.Environment.Certificate, 0600) + } + + metadata, err := getMetadata(cloudcfg, args) + if err != nil { + return nil, errors.Trace(err) + } + //tags := []string{ // env.globalFirewallName(), // machineID, @@ -223,9 +251,9 @@ func (env *environ) newRawInstance(args environs.StartInstanceParams) (*lxdclien // getMetadata builds the raw "user-defined" metadata for the new // instance (relative to the provided args) and returns it. -func getMetadata(args environs.StartInstanceParams) (map[string]string, error) { +func getMetadata(cloudcfg cloudinit.CloudConfig, args environs.StartInstanceParams) (map[string]string, error) { renderer := lxdRenderer{} - uncompressed, err := providerinit.ComposeUserData(args.InstanceConfig, nil, renderer) + uncompressed, err := providerinit.ComposeUserData(args.InstanceConfig, cloudcfg, renderer) if err != nil { return nil, errors.Annotate(err, "cannot make user data") } diff --git a/provider/lxd/environ_raw.go b/provider/lxd/environ_raw.go index 74beb0c4c01..5b06506529a 100644 --- a/provider/lxd/environ_raw.go +++ b/provider/lxd/environ_raw.go @@ -6,20 +6,47 @@ package lxd import ( + "io/ioutil" + "os" + "path" + "strings" + "github.com/juju/errors" + "github.com/juju/utils" + "github.com/juju/utils/series" + lxdshared "github.com/lxc/lxd/shared" + "github.com/juju/juju/environs" + jujupaths "github.com/juju/juju/juju/paths" "github.com/juju/juju/network" "github.com/juju/juju/provider/common" "github.com/juju/juju/tools/lxdclient" ) +var ( + jujuConfDir = jujupaths.MustSucceed(jujupaths.ConfDir(series.LatestLts())) + clientCertPath = path.Join(jujuConfDir, "lxd-client.crt") + clientKeyPath = path.Join(jujuConfDir, "lxd-client.key") + serverCertPath = path.Join(jujuConfDir, "lxd-server.crt") +) + type rawProvider struct { + lxdCerts + lxdConfig lxdInstances lxdProfiles lxdImages common.Firewaller } +type lxdCerts interface { + AddCert(lxdclient.Cert) error +} + +type lxdConfig interface { + ServerStatus() (*lxdshared.ServerState, error) +} + type lxdInstances interface { Instances(string, ...string) ([]lxdclient.Instance, error) AddInstance(lxdclient.InstanceSpec) (*lxdclient.Instance, error) @@ -36,40 +63,101 @@ type lxdImages interface { EnsureImageExists(series string, sources []lxdclient.Remote, copyProgressHandler func(string)) error } -func newRawProvider(ecfg *environConfig) (*rawProvider, error) { - client, err := newClient(ecfg) - if err != nil { - return nil, errors.Trace(err) - } - - firewaller, err := newFirewaller(ecfg) +func newRawProvider(spec environs.CloudSpec) (*rawProvider, error) { + client, err := newClient(spec, ioutil.ReadFile, utils.RunCommand) if err != nil { - return nil, errors.Trace(err) + return nil, errors.Annotate(err, "creating LXD client") } raw := &rawProvider{ + lxdCerts: client, + lxdConfig: client, lxdInstances: client, lxdProfiles: client, lxdImages: client, - Firewaller: firewaller, + Firewaller: common.NewFirewaller(), } return raw, nil } -func newClient(ecfg *environConfig) (*lxdclient.Client, error) { - clientCfg, err := ecfg.clientConfig() - if err != nil { +type readFileFunc func(string) ([]byte, error) +type runCommandFunc func(string, ...string) (string, error) + +func newClient( + spec environs.CloudSpec, + readFile readFileFunc, + runCommand runCommandFunc, +) (*lxdclient.Client, error) { + if spec.Endpoint != "" { + // We don't handle connecting to non-local lxd at present. + return nil, errors.NotValidf("endpoint %q", spec.Endpoint) + } + + config, err := getRemoteConfig(readFile, runCommand) + if errors.IsNotFound(err) { + config = &lxdclient.Config{Remote: lxdclient.Local} + } else if err != nil { return nil, errors.Trace(err) } - client, err := lxdclient.Connect(clientCfg) + client, err := lxdclient.Connect(*config) if err != nil { return nil, errors.Trace(err) } - return client, nil } -func newFirewaller(ecfg *environConfig) (common.Firewaller, error) { - return common.NewFirewaller(), nil +// getRemoteConfig returns a lxdclient.Config using a TCP-based remote +// if called from within an instance started by the LXD provider. Otherwise, +// it returns an errors satisfying errors.IsNotFound. +func getRemoteConfig(readFile readFileFunc, runCommand runCommandFunc) (*lxdclient.Config, error) { + readFileOrig := readFile + readFile = func(path string) ([]byte, error) { + data, err := readFileOrig(path) + if err != nil { + if os.IsNotExist(err) { + err = errors.NotFoundf("%s", path) + } + return nil, err + } + return data, nil + } + clientCert, err := readFile(clientCertPath) + if err != nil { + return nil, errors.Annotate(err, "reading client certificate") + } + clientKey, err := readFile(clientKeyPath) + if err != nil { + return nil, errors.Annotate(err, "reading client key") + } + serverCert, err := readFile(serverCertPath) + if err != nil { + return nil, errors.Annotate(err, "reading server certificate") + } + cert := lxdclient.NewCert(clientCert, clientKey) + hostAddress, err := getDefaultGateway(runCommand) + if err != nil { + return nil, errors.Annotate(err, "getting gateway address") + } + return &lxdclient.Config{ + lxdclient.Remote{ + Name: "remote", + Host: hostAddress, + Protocol: lxdclient.LXDProtocol, + Cert: &cert, + ServerPEMCert: string(serverCert), + }, + }, nil +} + +func getDefaultGateway(runCommand runCommandFunc) (string, error) { + out, err := runCommand("ip", "route", "list", "match", "0/0") + if err != nil { + return "", errors.Trace(err) + } + if !strings.HasPrefix(string(out), "default via") { + return "", errors.Errorf(`unexpected output from "ip route": %s`, out) + } + fields := strings.Fields(string(out)) + return fields[2], nil } diff --git a/provider/lxd/environ_raw_test.go b/provider/lxd/environ_raw_test.go new file mode 100644 index 00000000000..65943c6842a --- /dev/null +++ b/provider/lxd/environ_raw_test.go @@ -0,0 +1,97 @@ +// Copyright 2016 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +// +build go1.3 + +package lxd + +import ( + "os" + + "github.com/juju/errors" + "github.com/juju/testing" + jc "github.com/juju/testing/checkers" + gc "gopkg.in/check.v1" + + "github.com/juju/juju/tools/lxdclient" +) + +type environRawSuite struct { + testing.IsolationSuite + testing.Stub + readFile readFileFunc + runCommand runCommandFunc +} + +var _ = gc.Suite(&environRawSuite{}) + +func (s *environRawSuite) SetUpTest(c *gc.C) { + s.IsolationSuite.SetUpTest(c) + s.Stub.ResetCalls() + s.readFile = func(path string) ([]byte, error) { + s.AddCall("readFile", path) + if err := s.NextErr(); err != nil { + return nil, err + } + return []byte("content:" + path), nil + } + s.runCommand = func(command string, args ...string) (string, error) { + s.AddCall("runCommand", command, args) + if err := s.NextErr(); err != nil { + return "", err + } + return "default via 10.0.8.1 dev eth0", nil + } +} + +func (s *environRawSuite) TestGetRemoteConfig(c *gc.C) { + cfg, err := getRemoteConfig(s.readFile, s.runCommand) + c.Assert(err, jc.ErrorIsNil) + c.Assert(cfg, jc.DeepEquals, &lxdclient.Config{ + Remote: lxdclient.Remote{ + Name: "remote", + Host: "10.0.8.1", + Protocol: "lxd", + Cert: &lxdclient.Cert{ + CertPEM: []byte("content:/etc/juju/lxd-client.crt"), + KeyPEM: []byte("content:/etc/juju/lxd-client.key"), + }, + ServerPEMCert: "content:/etc/juju/lxd-server.crt", + }, + }) + s.Stub.CheckCalls(c, []testing.StubCall{ + {"readFile", []interface{}{"/etc/juju/lxd-client.crt"}}, + {"readFile", []interface{}{"/etc/juju/lxd-client.key"}}, + {"readFile", []interface{}{"/etc/juju/lxd-server.crt"}}, + {"runCommand", []interface{}{"ip", []string{"route", "list", "match", "0/0"}}}, + }) +} + +func (s *environRawSuite) TestGetRemoteConfigFileNotExist(c *gc.C) { + s.SetErrors(os.ErrNotExist) + _, err := getRemoteConfig(s.readFile, s.runCommand) + // os.IsNotExist is translated to errors.IsNotFound + c.Assert(err, jc.Satisfies, errors.IsNotFound) + c.Assert(err, gc.ErrorMatches, "reading client certificate: /etc/juju/lxd-client.crt not found") +} + +func (s *environRawSuite) TestGetRemoteConfigFileError(c *gc.C) { + s.SetErrors(nil, errors.New("i/o error")) + _, err := getRemoteConfig(s.readFile, s.runCommand) + c.Assert(err, gc.ErrorMatches, "reading client key: i/o error") +} + +func (s *environRawSuite) TestGetRemoteConfigIPRouteFormatError(c *gc.C) { + s.runCommand = func(string, ...string) (string, error) { + return "this is not the prefix you're looking for", nil + } + _, err := getRemoteConfig(s.readFile, s.runCommand) + c.Assert(err, gc.ErrorMatches, + `getting gateway address: unexpected output from "ip route": this is not the prefix you're looking for`) +} + +func (s *environRawSuite) TestGetRemoteConfigIPRouteCommandError(c *gc.C) { + s.SetErrors(nil, nil, nil, errors.New("buh bow")) + _, err := getRemoteConfig(s.readFile, s.runCommand) + c.Assert(err, gc.ErrorMatches, `getting gateway address: buh bow`) +} diff --git a/provider/lxd/environ_test.go b/provider/lxd/environ_test.go index 24fc9f411bc..2a29435d901 100644 --- a/provider/lxd/environ_test.go +++ b/provider/lxd/environ_test.go @@ -48,17 +48,8 @@ func (s *environSuite) TestSetConfigOkay(c *gc.C) { func (s *environSuite) TestSetConfigNoAPI(c *gc.C) { err := s.Env.SetConfig(s.Config) - c.Assert(err, jc.ErrorIsNil) - - s.Stub.CheckCallNames(c, "asNonLocal") -} - -func (s *environSuite) TestSetConfigMissing(c *gc.C) { - lxd.UnsetEnvConfig(s.Env) - err := s.Env.SetConfig(s.Config) - - c.Check(err, gc.ErrorMatches, "cannot set config on uninitialized env") + c.Assert(err, jc.ErrorIsNil) } func (s *environSuite) TestConfig(c *gc.C) { diff --git a/provider/lxd/provider.go b/provider/lxd/provider.go index 5bbcf7a2298..063b9aa9a5d 100644 --- a/provider/lxd/provider.go +++ b/provider/lxd/provider.go @@ -23,60 +23,38 @@ var providerInstance environProvider // Open implements environs.EnvironProvider. func (environProvider) Open(args environs.OpenParams) (environs.Environ, error) { + if err := validateCloudSpec(args.Cloud); err != nil { + return nil, errors.Annotate(err, "validating cloud spec") + } // TODO(ericsnow) verify prerequisites (see provider/local/prereq.go)? - // TODO(ericsnow) do something similar to correctLocalhostURLs() - // (in provider/local/environprovider.go)? - - env, err := newEnviron(args.Config, newRawProvider) + env, err := newEnviron(args.Cloud, args.Config, newRawProvider) return env, errors.Trace(err) } // PrepareConfig implements environs.EnvironProvider. func (p environProvider) PrepareConfig(args environs.PrepareConfigParams) (*config.Config, error) { + if err := validateCloudSpec(args.Cloud); err != nil { + return nil, errors.Annotate(err, "validating cloud spec") + } return args.Config, nil } // RestrictedConfigAttributes is specified in the EnvironProvider interface. func (environProvider) RestrictedConfigAttributes() []string { - return []string{ - "remote-url", - "client-cert", - "client-key", - "server-cert", - } + return []string{} } // Validate implements environs.EnvironProvider. func (environProvider) Validate(cfg, old *config.Config) (valid *config.Config, err error) { - if old == nil { - ecfg, err := newValidConfig(cfg, configDefaults) - if err != nil { - return nil, errors.Annotate(err, "invalid config") - } - return ecfg.Config, nil - } - - // The defaults should be set already, so we pass nil. - ecfg, err := newValidConfig(old, nil) - if err != nil { + if _, err := newValidConfig(cfg); err != nil { return nil, errors.Annotate(err, "invalid base config") } - - if err := ecfg.update(cfg); err != nil { - return nil, errors.Annotate(err, "invalid config change") - } - - return ecfg.Config, nil + return cfg, nil } // SecretAttrs implements environs.EnvironProvider. func (environProvider) SecretAttrs(cfg *config.Config) (map[string]string, error) { - // The defaults should be set already, so we pass nil. - ecfg, err := newValidConfig(cfg, nil) - if err != nil { - return nil, errors.Trace(err) - } - return ecfg.secret(), nil + return map[string]string{}, nil } // DetectRegions implements environs.CloudRegionDetector. @@ -95,3 +73,18 @@ func (environProvider) Schema() environschema.Fields { } return fields } + +func validateCloudSpec(spec environs.CloudSpec) error { + if err := spec.Validate(); err != nil { + return errors.Trace(err) + } + if spec.Endpoint != "" { + return errors.NotValidf("non-empty endpoint %q", spec.Endpoint) + } + if spec.Credential != nil { + if authType := spec.Credential.AuthType(); authType != cloud.EmptyAuthType { + return errors.NotSupportedf("%q auth-type", authType) + } + } + return nil +} diff --git a/provider/lxd/provider_test.go b/provider/lxd/provider_test.go index 3db230ee175..620d5edd6bc 100644 --- a/provider/lxd/provider_test.go +++ b/provider/lxd/provider_test.go @@ -77,8 +77,7 @@ func (s *providerSuite) TestValidate(c *gc.C) { func (s *providerSuite) TestSecretAttrs(c *gc.C) { obtainedAttrs, err := s.provider.SecretAttrs(s.Config) c.Assert(err, jc.ErrorIsNil) - - c.Check(obtainedAttrs, gc.DeepEquals, map[string]string{"client-key": ""}) + c.Assert(obtainedAttrs, gc.HasLen, 0) } type ProviderFunctionalSuite struct { @@ -116,8 +115,32 @@ func (s *ProviderFunctionalSuite) TestOpen(c *gc.C) { func (s *ProviderFunctionalSuite) TestPrepareConfig(c *gc.C) { cfg, err := s.provider.PrepareConfig(environs.PrepareConfigParams{ + Cloud: lxdCloudSpec(), Config: s.Config, }) c.Assert(err, jc.ErrorIsNil) c.Check(cfg, gc.NotNil) } + +func (s *ProviderFunctionalSuite) TestPrepareConfigUnsupportedAuthType(c *gc.C) { + cred := cloud.NewCredential(cloud.CertificateAuthType, nil) + _, err := s.provider.PrepareConfig(environs.PrepareConfigParams{ + Cloud: environs.CloudSpec{ + Type: "lxd", + Name: "remotehost", + Credential: &cred, + }, + }) + c.Assert(err, gc.ErrorMatches, `validating cloud spec: "certificate" auth-type not supported`) +} + +func (s *ProviderFunctionalSuite) TestPrepareConfigNonEmptyEndpoint(c *gc.C) { + _, err := s.provider.PrepareConfig(environs.PrepareConfigParams{ + Cloud: environs.CloudSpec{ + Type: "lxd", + Name: "remotehost", + Endpoint: "1.2.3.4", + }, + }) + c.Assert(err, gc.ErrorMatches, `validating cloud spec: non-empty endpoint "1.2.3.4" not valid`) +} diff --git a/provider/lxd/testing_test.go b/provider/lxd/testing_test.go index b72faa39007..6cde4d45d5a 100644 --- a/provider/lxd/testing_test.go +++ b/provider/lxd/testing_test.go @@ -6,14 +6,13 @@ package lxd import ( - "crypto/tls" - "encoding/pem" "os" "github.com/juju/errors" gitjujutesting "github.com/juju/testing" jc "github.com/juju/testing/checkers" "github.com/juju/utils/arch" + "github.com/lxc/lxd/shared" gc "gopkg.in/check.v1" "github.com/juju/juju/cloudconfig/instancecfg" @@ -71,12 +70,8 @@ const ( // These are stub config values for use in tests. var ( ConfigAttrs = testing.FakeConfig().Merge(testing.Attrs{ - "type": "lxd", - "remote-url": "", - "client-cert": "", - "client-key": "", - "server-cert": "", - "uuid": "2d02eeac-9dbb-11e4-89d3-123b93f75cba", + "type": "lxd", + "uuid": "2d02eeac-9dbb-11e4-89d3-123b93f75cba", }) ) @@ -220,7 +215,7 @@ func (s *BaseSuiteUnpatched) initNet(c *gc.C) { func (s *BaseSuiteUnpatched) setConfig(c *gc.C, cfg *config.Config) { s.Config = cfg - ecfg, err := newValidConfig(cfg, configDefaults) + ecfg, err := newValidConfig(cfg) c.Assert(err, jc.ErrorIsNil) s.EnvConfig = ecfg uuid := cfg.UUID() @@ -296,7 +291,6 @@ type BaseSuite struct { func (s *BaseSuite) SetUpSuite(c *gc.C) { s.BaseSuiteUnpatched.SetUpSuite(c) // Do this *before* s.initEnv() gets called in BaseSuiteUnpatched.SetUpTest - s.PatchValue(&asNonLocal, s.asNonLocal) } func (s *BaseSuite) SetUpTest(c *gc.C) { @@ -309,6 +303,8 @@ func (s *BaseSuite) SetUpTest(c *gc.C) { // Patch out all expensive external deps. s.Env.raw = &rawProvider{ + lxdCerts: s.Client, + lxdConfig: s.Client, lxdInstances: s.Client, lxdImages: s.Client, Firewaller: s.Firewaller, @@ -320,18 +316,6 @@ func (s *BaseSuite) CheckNoAPI(c *gc.C) { s.Stub.CheckCalls(c, nil) } -func (s *BaseSuite) asNonLocal(clientCfg lxdclient.Config) (lxdclient.Config, error) { - if s.Stub == nil { - return clientCfg, nil - } - s.Stub.AddCall("asNonLocal", clientCfg) - if err := s.Stub.NextErr(); err != nil { - return clientCfg, errors.Trace(err) - } - - return clientCfg, nil -} - func NewBaseConfig(c *gc.C) *config.Config { var err error cfg := testing.ModelConfig(c) @@ -356,32 +340,6 @@ func NewCustomBaseConfig(c *gc.C, updates map[string]interface{}) *config.Config } type ConfigValues struct { - RemoteURL string - ClientCert string - ClientKey string - ServerCert string -} - -func (cv ConfigValues) CheckCert(c *gc.C) { - certPEM := []byte(cv.ClientCert) - keyPEM := []byte(cv.ClientKey) - - _, err := tls.X509KeyPair(certPEM, keyPEM) - c.Check(err, jc.ErrorIsNil) - - block, remainder := pem.Decode(certPEM) - c.Check(block.Type, gc.Equals, "CERTIFICATE") - c.Check(remainder, gc.HasLen, 0) - - block, remainder = pem.Decode(keyPEM) - c.Check(block.Type, gc.Equals, "RSA PRIVATE KEY") - c.Check(remainder, gc.HasLen, 0) - - if cv.ServerCert != "" { - block, remainder = pem.Decode([]byte(cv.ServerCert)) - c.Check(block.Type, gc.Equals, "CERTIFICATE") - c.Check(remainder, gc.HasLen, 1) - } } type Config struct { @@ -393,16 +351,6 @@ func NewConfig(cfg *config.Config) *Config { return &Config{ecfg} } -func NewValidConfig(cfg *config.Config) (*Config, error) { - ecfg, err := newValidConfig(cfg, nil) - return &Config{ecfg}, err -} - -func NewValidDefaultConfig(cfg *config.Config) (*Config, error) { - ecfg, err := newValidConfig(cfg, configDefaults) - return &Config{ecfg}, err -} - func (ecfg *Config) Values(c *gc.C) (ConfigValues, map[string]interface{}) { c.Assert(ecfg.attrs, jc.DeepEquals, ecfg.UnknownAttrs()) @@ -410,14 +358,6 @@ func (ecfg *Config) Values(c *gc.C) (ConfigValues, map[string]interface{}) { extras := make(map[string]interface{}) for k, v := range ecfg.attrs { switch k { - case cfgRemoteURL: - values.RemoteURL = v.(string) - case cfgClientCert: - values.ClientCert = v.(string) - case cfgClientKey: - values.ClientKey = v.(string) - case cfgServerPEMCert: - values.ServerCert = v.(string) default: extras[k] = v } @@ -435,15 +375,6 @@ func (ecfg *Config) Validate() error { return ecfg.validate() } -func (ecfg *Config) ClientConfig() (lxdclient.Config, error) { - return ecfg.clientConfig() -} - -func (ecfg *Config) UpdateForClientConfig(clientCfg lxdclient.Config) (*Config, error) { - updated, err := ecfg.updateForClientConfig(clientCfg) - return &Config{updated}, err -} - type stubCommon struct { stub *gitjujutesting.Stub @@ -524,6 +455,23 @@ func (conn *StubClient) Addresses(name string) ([]network.Address, error) { }}, nil } +func (conn *StubClient) AddCert(cert lxdclient.Cert) error { + conn.AddCall("AddCert", cert) + return conn.NextErr() +} + +func (conn *StubClient) ServerStatus() (*shared.ServerState, error) { + conn.AddCall("ServerStatus") + if err := conn.NextErr(); err != nil { + return nil, err + } + return &shared.ServerState{ + Environment: shared.ServerStateEnvironment{ + Certificate: "server-cert", + }, + }, nil +} + // TODO(ericsnow) Move stubFirewaller to environs/testing or provider/common/testing. type stubFirewaller struct { diff --git a/tools/lxdclient/config.go b/tools/lxdclient/config.go index 69a493f48fb..64fdb5f3732 100644 --- a/tools/lxdclient/config.go +++ b/tools/lxdclient/config.go @@ -95,13 +95,12 @@ func prepareRemote(client *Client, newCert *Cert) (string, error) { return "", errors.Trace(err) } - if newCert == nil { - return "", nil - } - - // Make sure the LXD service will allow our certificate to connect - if err := client.AddCert(*newCert); err != nil { - return "", errors.Trace(err) + if newCert != nil { + // Make sure the LXD service will allow + // our certificate to authenticate. + if err := client.AddCert(*newCert); err != nil { + return "", errors.Trace(err) + } } st, err := client.ServerStatus() diff --git a/tools/lxdclient/config_test.go b/tools/lxdclient/config_test.go index 8622e356198..d2bf172cfc4 100644 --- a/tools/lxdclient/config_test.go +++ b/tools/lxdclient/config_test.go @@ -159,6 +159,8 @@ func (s *configFunctionalSuite) TestUsingTCPRemote(c *gc.C) { } nonlocal, err := cfg.UsingTCPRemote() c.Assert(err, jc.ErrorIsNil) + nonlocal, err = nonlocal.WithDefaults() + c.Assert(err, jc.ErrorIsNil) checkValidRemote(c, &nonlocal.Remote) c.Check(nonlocal, jc.DeepEquals, lxdclient.Config{ diff --git a/tools/lxdclient/remote.go b/tools/lxdclient/remote.go index be0fdf1d553..b8f37e17cbf 100644 --- a/tools/lxdclient/remote.go +++ b/tools/lxdclient/remote.go @@ -177,7 +177,8 @@ func (r Remote) validateLocal() error { // // For a "local" remote (see Local), the remote is changed to a one with the // host set to the first IPv4 address assigned to the given bridgeName. The -// remote is also set up for remote access, setting the cert if not already set. +// remote's certificate will be unchanged; to set a certificate, the +// Remote.WithDefaults method may be called. func (r Remote) UsingTCP(bridgeName string) (Remote, error) { // Note that r is a value receiver, so it is an implicit copy. @@ -193,10 +194,5 @@ func (r Remote) UsingTCP(bridgeName string) (Remote, error) { // TODO(ericsnow) Change r.Name if "local"? Prepend "juju-"? - r, err = r.WithDefaults() - if err != nil { - return r, errors.Trace(err) - } - return r, nil } diff --git a/tools/lxdclient/remote_test.go b/tools/lxdclient/remote_test.go index 147c5018a31..038ef7b2c04 100644 --- a/tools/lxdclient/remote_test.go +++ b/tools/lxdclient/remote_test.go @@ -427,14 +427,21 @@ func (s *remoteFunctionalSuite) TestUsingTCP(c *gc.C) { lxdclient.PatchGenerateCertificate(&s.CleanupSuite, testingCert, testingKey) remote := lxdclient.Remote{ - Name: "my-remote", - Host: "", - Cert: nil, + Name: "my-remote", + Host: "", + Cert: nil, + Protocol: lxdclient.LXDProtocol, } nonlocal, err := remote.UsingTCP("lo") c.Assert(err, jc.ErrorIsNil) - checkValidRemote(c, &nonlocal) + withCert, err := nonlocal.WithDefaults() + withoutCert := withCert + withoutCert.Cert = nil + c.Check(withoutCert, jc.DeepEquals, nonlocal) + + checkValidRemote(c, &withCert) + c.Check(nonlocal, jc.DeepEquals, lxdclient.Remote{ Name: "my-remote", Host: nonlocal.Host,