Merge pull request juju#9468 from SimonRichardson/register-lxd

juju#9468 ## Description of change The following registers the alternative name for the localhost lxd provider. This way we can bootstrap juju just using the alternative name "lxd" rather than the perceived more popular one "localhost". The question of if this is correct or not is a bit of a concern. Why we don't reconcile both localhost and lxd names and normalise them to mean the same one when bootstrapping seems a mystery. Anyway, I'll supply a good deal of Cunningham's Law to this and see what the fallout to that is. ## QA steps ```sh juju bootstrap lxd ``` ## Documentation changes This is a regression from 2.4, where you could bootstrap using "lxd" correctly. ## Bug reference N/A
MalloZup · Nov 20, 2018 · 9b1bc1d · 9b1bc1d
2 parents 30db4a0 + 1958ca7
commit 9b1bc1d
Show file tree

Hide file tree

Showing 12 changed files with 141 additions and 36 deletions.
diff --git a/cmd/juju/commands/bootstrap.go b/cmd/juju/commands/bootstrap.go
@@ -876,7 +876,9 @@ func (c *bootstrapCommand) credentialsAndRegionName(
 
 	// When looking for credentials, we should attempt to see if there are any
 	// credentials that should be registered, before we get or detect them
-	err = common.RegisterCredentials(ctx, store, provider)
+	err = common.RegisterCredentials(ctx, store, provider, modelcmd.RegisterCredentialsParams{
+		Cloud: cloud,
+	})
 	if err != nil {
 		logger.Errorf("registering credentials errored %s", err)
 	}

diff --git a/cmd/juju/common/cloudcredential.go b/cmd/juju/common/cloudcredential.go
@@ -32,8 +32,9 @@ func RegisterCredentials(
 	ctx *cmd.Context,
 	store jujuclient.CredentialStore,
 	provider environs.EnvironProvider,
+	args modelcmd.RegisterCredentialsParams,
 ) error {
-	credentials, err := modelcmd.RegisterCredentials(provider)
+	credentials, err := modelcmd.RegisterCredentials(provider, args)
 	switch {
 	case errors.IsNotFound(err):
 		return nil

diff --git a/cmd/juju/common/cloudcredential_test.go b/cmd/juju/common/cloudcredential_test.go
@@ -16,6 +16,7 @@ import (
 
 	"github.com/juju/juju/cloud"
 	"github.com/juju/juju/cmd/juju/common"
+	"github.com/juju/juju/cmd/modelcmd"
 )
 
 var _ = gc.Suite(&cloudCredentialSuite{})
@@ -67,7 +68,9 @@ func (*cloudCredentialSuite) TestRegisterCredentials(c *gc.C) {
 	}
 
 	mockProvider := common.NewMockTestCloudProvider(ctrl)
-	mockProvider.EXPECT().RegisterCredentials().Return(map[string]*cloud.CloudCredential{
+	mockProvider.EXPECT().RegisterCredentials(cloud.Cloud{
+		Name: "fake",
+	}).Return(map[string]*cloud.CloudCredential{
 		"fake": credential,
 	}, nil)
 	mockStore := common.NewMockCredentialStore(ctrl)
@@ -77,7 +80,11 @@ func (*cloudCredentialSuite) TestRegisterCredentials(c *gc.C) {
 
 	err := common.RegisterCredentials(&cmd.Context{
 		Stderr: stderr,
-	}, mockStore, mockProvider)
+	}, mockStore, mockProvider, modelcmd.RegisterCredentialsParams{
+		Cloud: cloud.Cloud{
+			Name: "fake",
+		},
+	})
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(stderr.String(), gc.Equals, "")
 }
@@ -87,14 +94,20 @@ func (*cloudCredentialSuite) TestRegisterCredentialsWithNoCredentials(c *gc.C) {
 	defer ctrl.Finish()
 
 	mockProvider := common.NewMockTestCloudProvider(ctrl)
-	mockProvider.EXPECT().RegisterCredentials().Return(map[string]*cloud.CloudCredential{}, nil)
+	mockProvider.EXPECT().RegisterCredentials(cloud.Cloud{
+		Name: "fake",
+	}).Return(map[string]*cloud.CloudCredential{}, nil)
 	mockStore := common.NewMockCredentialStore(ctrl)
 
 	stderr := new(bytes.Buffer)
 
 	err := common.RegisterCredentials(&cmd.Context{
 		Stderr: stderr,
-	}, mockStore, mockProvider)
+	}, mockStore, mockProvider, modelcmd.RegisterCredentialsParams{
+		Cloud: cloud.Cloud{
+			Name: "fake",
+		},
+	})
 	c.Assert(err, jc.ErrorIsNil)
 }
 
@@ -103,13 +116,19 @@ func (*cloudCredentialSuite) TestRegisterCredentialsWithCallFailure(c *gc.C) {
 	defer ctrl.Finish()
 
 	mockProvider := common.NewMockTestCloudProvider(ctrl)
-	mockProvider.EXPECT().RegisterCredentials().Return(nil, errors.New("bad"))
+	mockProvider.EXPECT().RegisterCredentials(cloud.Cloud{
+		Name: "fake",
+	}).Return(nil, errors.New("bad"))
 	mockStore := common.NewMockCredentialStore(ctrl)
 
 	stderr := new(bytes.Buffer)
 
 	err := common.RegisterCredentials(&cmd.Context{
 		Stderr: stderr,
-	}, mockStore, mockProvider)
+	}, mockStore, mockProvider, modelcmd.RegisterCredentialsParams{
+		Cloud: cloud.Cloud{
+			Name: "fake",
+		},
+	})
 	c.Assert(errors.Cause(err).Error(), gc.Matches, "bad")
 }
diff --git a/cmd/juju/common/cloudprovider_mock_test.go b/cmd/juju/common/cloudprovider_mock_test.go
diff --git a/cmd/modelcmd/cloudprovider_mock_test.go b/cmd/modelcmd/cloudprovider_mock_test.go
diff --git a/cmd/modelcmd/credentials.go b/cmd/modelcmd/credentials.go
@@ -186,11 +186,17 @@ func DetectCredential(cloudName string, provider environs.EnvironProvider) (*clo
 	return detected, nil
 }
 
+// RegisterCredentialsParams contains parameters for the RegisterCredentials function.
+type RegisterCredentialsParams struct {
+	// Cloud is the cloud definition.
+	Cloud cloud.Cloud
+}
+
 // RegisterCredentials returns any credentials that need to be registered for
 // a provider.
-func RegisterCredentials(provider environs.EnvironProvider) (map[string]*cloud.CloudCredential, error) {
+func RegisterCredentials(provider environs.EnvironProvider, args RegisterCredentialsParams) (map[string]*cloud.CloudCredential, error) {
 	if register, ok := provider.(environs.ProviderCredentialsRegister); ok {
-		found, err := register.RegisterCredentials()
+		found, err := register.RegisterCredentials(args.Cloud)
 		if err != nil {
 			return nil, errors.Annotatef(
 				err, "registering credentials for provider",

diff --git a/cmd/modelcmd/credentials_test.go b/cmd/modelcmd/credentials_test.go
@@ -178,9 +178,15 @@ func (s *credentialsSuite) TestRegisterCredentials(c *gc.C) {
 	}
 
 	exp := mockProvider.EXPECT()
-	exp.RegisterCredentials().Return(credential, nil)
+	exp.RegisterCredentials(cloud.Cloud{
+		Name: "fake",
+	}).Return(credential, nil)
 
-	credentials, err := modelcmd.RegisterCredentials(mockProvider)
+	credentials, err := modelcmd.RegisterCredentials(mockProvider, modelcmd.RegisterCredentialsParams{
+		Cloud: cloud.Cloud{
+			Name: "fake",
+		},
+	})
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(credentials, gc.DeepEquals, credential)
 }
@@ -194,9 +200,15 @@ func (s *credentialsSuite) TestRegisterCredentialsWithNoCredentials(c *gc.C) {
 	credential := map[string]*cloud.CloudCredential{}
 
 	exp := mockProvider.EXPECT()
-	exp.RegisterCredentials().Return(credential, nil)
+	exp.RegisterCredentials(cloud.Cloud{
+		Name: "fake",
+	}).Return(credential, nil)
 
-	credentials, err := modelcmd.RegisterCredentials(mockProvider)
+	credentials, err := modelcmd.RegisterCredentials(mockProvider, modelcmd.RegisterCredentialsParams{
+		Cloud: cloud.Cloud{
+			Name: "fake",
+		},
+	})
 	c.Assert(errors.Cause(err).Error(), gc.Matches, `credentials for provider not found`)
 	c.Assert(credentials, gc.IsNil)
 }
@@ -208,9 +220,15 @@ func (s *credentialsSuite) TestRegisterCredentialsWithCallFailure(c *gc.C) {
 	mockProvider := modelcmd.NewMockTestCloudProvider(ctrl)
 
 	exp := mockProvider.EXPECT()
-	exp.RegisterCredentials().Return(nil, errors.New("bad"))
+	exp.RegisterCredentials(cloud.Cloud{
+		Name: "fake",
+	}).Return(nil, errors.New("bad"))
 
-	credentials, err := modelcmd.RegisterCredentials(mockProvider)
+	credentials, err := modelcmd.RegisterCredentials(mockProvider, modelcmd.RegisterCredentialsParams{
+		Cloud: cloud.Cloud{
+			Name: "fake",
+		},
+	})
 	c.Assert(err.Error(), gc.Matches, `registering credentials for provider: bad`)
 	c.Assert(credentials, gc.IsNil)
 }
diff --git a/environs/interface.go b/environs/interface.go
@@ -137,7 +137,7 @@ type ProviderCredentialsRegister interface {
 	//
 	// If no credentials can be found, RegisterCredentials should return
 	// an error satisfying errors.IsNotFound.
-	RegisterCredentials() (map[string]*cloud.CloudCredential, error)
+	RegisterCredentials(cloud.Cloud) (map[string]*cloud.CloudCredential, error)
 }
 
 // RequestFinalizeCredential is an interface that an EnvironProvider implements

diff --git a/environs/testing/package_mock.go b/environs/testing/package_mock.go
diff --git a/provider/lxd/credentials.go b/provider/lxd/credentials.go
@@ -111,7 +111,14 @@ func (environProviderCredentials) CredentialSchemas() map[cloud.AuthType]cloud.C
 }
 
 // RegisterCredentials is part of the environs.ProviderCredentialsRegister interface.
-func (p environProviderCredentials) RegisterCredentials() (map[string]*cloud.CloudCredential, error) {
+func (p environProviderCredentials) RegisterCredentials(cld cloud.Cloud) (map[string]*cloud.CloudCredential, error) {
+	// only register credentials if the operator is attempting to access "lxd"
+	// or "localhost"
+	cloudName := cld.Name
+	if cloudName != lxdnames.DefaultCloud && cloudName != lxdnames.DefaultCloudAltName {
+		return make(map[string]*cloud.CloudCredential), nil
+	}
+
 	nopLogf := func(msg string, args ...interface{}) {}
 	certPEM, keyPEM, err := p.readOrGenerateCert(nopLogf)
 	if err != nil {
@@ -124,10 +131,10 @@ func (p environProviderCredentials) RegisterCredentials() (map[string]*cloud.Clo
 	}
 
 	return map[string]*cloud.CloudCredential{
-		lxdnames.DefaultCloud: {
-			DefaultCredential: lxdnames.DefaultCloud,
+		cloudName: {
+			DefaultCredential: cloudName,
 			AuthCredentials: map[string]cloud.Credential{
-				lxdnames.DefaultCloud: *localCertCredential,
+				cloudName: *localCertCredential,
 			},
 		},
 	}, nil

diff --git a/provider/lxd/credentials_test.go b/provider/lxd/credentials_test.go
@@ -267,7 +267,9 @@ func (s *credentialsSuite) TestRegisterCredentials(c *gc.C) {
 	expected.Label = `LXD credential "localhost"`
 
 	provider := deps.provider.(environs.ProviderCredentialsRegister)
-	credentials, err := provider.RegisterCredentials()
+	credentials, err := provider.RegisterCredentials(cloud.Cloud{
+		Name: "localhost",
+	})
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(credentials, jc.DeepEquals, map[string]*cloud.CloudCredential{
 		"localhost": {
@@ -279,6 +281,48 @@ func (s *credentialsSuite) TestRegisterCredentials(c *gc.C) {
 	})
 }
 
+func (s *credentialsSuite) TestRegisterCredentialsWithAlternativeCloudName(c *gc.C) {
+	ctrl := gomock.NewController(c)
+	defer ctrl.Finish()
+
+	deps := s.createProvider(ctrl)
+
+	deps.server.EXPECT().GetCertificate(s.clientCertFingerprint(c)).Return(nil, "", nil)
+	deps.server.EXPECT().ServerCertificate().Return("server-cert")
+
+	path := osenv.JujuXDGDataHomePath("lxd")
+	deps.certReadWriter.EXPECT().Read(path).Return(nil, nil, os.ErrNotExist)
+	deps.certReadWriter.EXPECT().Write(path, []byte(coretesting.CACert), []byte(coretesting.CAKey)).Return(nil)
+
+	path = filepath.Join(utils.Home(), ".config", "lxc")
+	deps.certReadWriter.EXPECT().Read(path).Return(nil, nil, os.ErrNotExist)
+	deps.certGenerator.EXPECT().Generate(true).Return([]byte(coretesting.CACert), []byte(coretesting.CAKey), nil)
+
+	expected := cloud.NewCredential(
+		cloud.CertificateAuthType,
+		map[string]string{
+			"client-cert": coretesting.CACert,
+			"client-key":  coretesting.CAKey,
+			"server-cert": "server-cert",
+		},
+	)
+	expected.Label = `LXD credential "localhost"`
+
+	provider := deps.provider.(environs.ProviderCredentialsRegister)
+	credentials, err := provider.RegisterCredentials(cloud.Cloud{
+		Name: "lxd",
+	})
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(credentials, jc.DeepEquals, map[string]*cloud.CloudCredential{
+		"lxd": {
+			DefaultCredential: "lxd",
+			AuthCredentials: map[string]cloud.Credential{
+				"lxd": expected,
+			},
+		},
+	})
+}
+
 func (s *credentialsSuite) TestRegisterCredentialsUsesJujuCert(c *gc.C) {
 	ctrl := gomock.NewController(c)
 	defer ctrl.Finish()
@@ -292,7 +336,9 @@ func (s *credentialsSuite) TestRegisterCredentialsUsesJujuCert(c *gc.C) {
 	deps.certReadWriter.EXPECT().Read(path).Return([]byte(coretesting.CACert), []byte(coretesting.CAKey), nil)
 
 	provider := deps.provider.(environs.ProviderCredentialsRegister)
-	credentials, err := provider.RegisterCredentials()
+	credentials, err := provider.RegisterCredentials(cloud.Cloud{
+		Name: "localhost",
+	})
 	c.Assert(err, jc.ErrorIsNil)
 
 	expected := cloud.NewCredential(
@@ -331,7 +377,9 @@ func (s *credentialsSuite) TestRegisterCredentialsUsesLXCCert(c *gc.C) {
 	deps.certReadWriter.EXPECT().Read(path).Return([]byte(coretesting.CACert), []byte(coretesting.CAKey), nil)
 
 	provider := deps.provider.(environs.ProviderCredentialsRegister)
-	credentials, err := provider.RegisterCredentials()
+	credentials, err := provider.RegisterCredentials(cloud.Cloud{
+		Name: "localhost",
+	})
 	c.Assert(err, jc.ErrorIsNil)
 
 	expected := cloud.NewCredential(

diff --git a/provider/lxd/lxdnames/names.go b/provider/lxd/lxdnames/names.go
@@ -11,6 +11,10 @@ package lxdnames
 // the local lxd daemon.
 const DefaultCloud = "localhost"
 
+// DefaultCloudAltName is the alternative name of the default lxd cloud,
+// which corresponds to the local lxd daemon.
+const DefaultCloudAltName = "lxd"
+
 // DefaultLocalRegion is the name of the "region" we support in a local lxd,
 // which corresponds to the local lxd daemon.
 const DefaultLocalRegion = "localhost"