Merge pull request juju#9424 from Veebers/lxd-credential-error

juju#9424 ## Description of change Invalidate the credential whenever we get an authorisation error back from any provider action.
MalloZup · Nov 9, 2018 · e0e5888 · e0e5888
2 parents b20e640 + 444dc49
commit e0e5888
Show file tree

Hide file tree

Showing 10 changed files with 586 additions and 8 deletions.
diff --git a/provider/lxd/environ.go b/provider/lxd/environ.go
@@ -155,10 +155,12 @@ func (env *environ) Bootstrap(ctx environs.BootstrapContext, callCtx context.Pro
 // known environment.
 func (env *environ) Destroy(ctx context.ProviderCallContext) error {
 	if err := env.base.DestroyEnv(ctx); err != nil {
+		common.HandleCredentialError(IsAuthorisationFailure, err, ctx)
 		return errors.Trace(err)
 	}
 	if env.storageSupported() {
 		if err := destroyModelFilesystems(env); err != nil {
+			common.HandleCredentialError(IsAuthorisationFailure, err, ctx)
 			return errors.Annotate(err, "destroying LXD filesystems for model")
 		}
 	}
@@ -171,10 +173,12 @@ func (env *environ) DestroyController(ctx context.ProviderCallContext, controlle
 		return errors.Trace(err)
 	}
 	if err := env.destroyHostedModelResources(controllerUUID); err != nil {
+		common.HandleCredentialError(IsAuthorisationFailure, err, ctx)
 		return errors.Trace(err)
 	}
 	if env.storageSupported() {
 		if err := destroyControllerFilesystems(env, controllerUUID); err != nil {
+			common.HandleCredentialError(IsAuthorisationFailure, err, ctx)
 			return errors.Annotate(err, "destroying LXD filesystems for controller")
 		}
 	}
@@ -239,6 +243,7 @@ func (env *environ) AvailabilityZones(ctx context.ProviderCallContext) ([]common
 
 	nodes, err := env.server.GetClusterMembers()
 	if err != nil {
+		common.HandleCredentialError(IsAuthorisationFailure, err, ctx)
 		return nil, errors.Annotate(err, "listing cluster members")
 	}
 	aZones := make([]common.AvailabilityZone, len(nodes))

diff --git a/provider/lxd/environ_broker.go b/provider/lxd/environ_broker.go
@@ -42,6 +42,7 @@ func (env *environ) StartInstance(
 
 	container, err := env.newContainer(ctx, args, arch)
 	if err != nil {
+		common.HandleCredentialError(IsAuthorisationFailure, err, ctx)
 		if args.StatusCallback != nil {
 			args.StatusCallback(status.ProvisioningError, err.Error(), nil)
 		}
@@ -335,5 +336,9 @@ func (env *environ) StopInstances(ctx context.ProviderCallContext, instances ...
 		}
 	}
 
-	return errors.Trace(env.server.RemoveContainers(names))
+	err := env.server.RemoveContainers(names)
+	if err != nil {
+		common.HandleCredentialError(IsAuthorisationFailure, err, ctx)
+	}
+	return errors.Trace(err)
 }
diff --git a/provider/lxd/environ_broker_test.go b/provider/lxd/environ_broker_test.go
@@ -23,15 +23,21 @@ import (
 type environBrokerSuite struct {
 	lxd.EnvironSuite
 
-	callCtx        context.ProviderCallContext
-	defaultProfile *api.Profile
+	callCtx           *context.CloudCallContext
+	defaultProfile    *api.Profile
+	invalidCredential bool
 }
 
 var _ = gc.Suite(&environBrokerSuite{})
 
 func (s *environBrokerSuite) SetUpTest(c *gc.C) {
 	s.BaseSuite.SetUpTest(c)
-	s.callCtx = context.NewCloudCallContext()
+	s.callCtx = &context.CloudCallContext{
+		InvalidateCredentialFunc: func(string) error {
+			s.invalidCredential = true
+			return nil
+		},
+	}
 	s.defaultProfile = &api.Profile{
 		ProfilePut: api.ProfilePut{
 			Devices: map[string]map[string]string{
@@ -41,6 +47,11 @@ func (s *environBrokerSuite) SetUpTest(c *gc.C) {
 	}
 }
 
+func (s *environBrokerSuite) TearDownTest(c *gc.C) {
+	s.invalidCredential = false
+	s.BaseSuite.TearDownTest(c)
+}
+
 // containerSpecMatcher is a gomock matcher for testing a container spec
 // with a supplied validation func.
 type containerSpecMatcher struct {
@@ -350,6 +361,26 @@ func (s *environBrokerSuite) TestStartInstanceNoTools(c *gc.C) {
 	c.Assert(err, gc.ErrorMatches, "no matching agent binaries available")
 }
 
+func (s *environBrokerSuite) TestStartInstanceInvalidCredentials(c *gc.C) {
+	c.Assert(s.invalidCredential, jc.IsFalse)
+	ctrl := gomock.NewController(c)
+	defer ctrl.Finish()
+	svr := lxd.NewMockServer(ctrl)
+
+	exp := svr.EXPECT()
+	gomock.InOrder(
+		exp.HostArch().Return(arch.AMD64),
+		exp.FindImage("bionic", arch.AMD64, gomock.Any(), true, gomock.Any()).Return(containerlxd.SourcedImage{}, nil),
+		exp.GetNICsFromProfile("default").Return(s.defaultProfile.Devices, nil),
+		exp.CreateContainerFromSpec(gomock.Any()).Return(&containerlxd.Container{}, fmt.Errorf("not authorized")),
+	)
+
+	env := s.NewEnviron(c, svr, nil)
+	_, err := env.StartInstance(s.callCtx, s.GetStartInstanceArgs(c, "bionic"))
+	c.Assert(err, gc.ErrorMatches, "not authorized")
+	c.Assert(s.invalidCredential, jc.IsTrue)
+}
+
 func (s *environBrokerSuite) TestStopInstances(c *gc.C) {
 	ctrl := gomock.NewController(c)
 	defer ctrl.Finish()
@@ -362,6 +393,20 @@ func (s *environBrokerSuite) TestStopInstances(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 }
 
+func (s *environBrokerSuite) TestStopInstancesInvalidCredentials(c *gc.C) {
+	c.Assert(s.invalidCredential, jc.IsFalse)
+	ctrl := gomock.NewController(c)
+	defer ctrl.Finish()
+	svr := lxd.NewMockServer(ctrl)
+
+	svr.EXPECT().RemoveContainers([]string{"juju-f75cba-1", "juju-f75cba-2"}).Return(fmt.Errorf("not authorized"))
+
+	env := s.NewEnviron(c, svr, nil)
+	err := env.StopInstances(s.callCtx, "juju-f75cba-1", "juju-f75cba-2", "not-in-namespace-so-ignored")
+	c.Assert(err, gc.ErrorMatches, "not authorized")
+	c.Assert(s.invalidCredential, jc.IsTrue)
+}
+
 func (s *environBrokerSuite) TestImageSourcesDefault(c *gc.C) {
 	ctrl := gomock.NewController(c)
 	defer ctrl.Finish()

diff --git a/provider/lxd/environ_instance.go b/provider/lxd/environ_instance.go
@@ -12,6 +12,7 @@ import (
 	"github.com/juju/juju/environs/context"
 	"github.com/juju/juju/environs/tags"
 	"github.com/juju/juju/instance"
+	"github.com/juju/juju/provider/common"
 )
 
 // Instances returns the available instances in the environment that
@@ -31,6 +32,7 @@ func (env *environ) Instances(ctx context.ProviderCallContext, ids []instance.Id
 		// will return either ErrPartialInstances or ErrNoInstances.
 		// TODO(ericsnow) Skip returning here only for certain errors?
 		logger.Errorf("failed to get instances from LXD: %v", err)
+		common.HandleCredentialError(IsAuthorisationFailure, err, ctx)
 		err = errors.Trace(err)
 	}
 
@@ -95,6 +97,7 @@ func (env *environ) prefixedInstances(prefix string) ([]*environInstance, error)
 func (env *environ) ControllerInstances(ctx context.ProviderCallContext, controllerUUID string) ([]instance.Id, error) {
 	containers, err := env.server.AliveContainers("juju-")
 	if err != nil {
+		common.HandleCredentialError(IsAuthorisationFailure, err, ctx)
 		return nil, errors.Trace(err)
 	}
 
@@ -118,6 +121,7 @@ func (env *environ) ControllerInstances(ctx context.ProviderCallContext, control
 func (env *environ) AdoptResources(ctx context.ProviderCallContext, controllerUUID string, fromVersion version.Number) error {
 	instances, err := env.AllInstances(ctx)
 	if err != nil {
+		common.HandleCredentialError(IsAuthorisationFailure, err, ctx)
 		return errors.Annotate(err, "all instances")
 	}
 

diff --git a/provider/lxd/environ_instance_test.go b/provider/lxd/environ_instance_test.go
@@ -93,6 +93,23 @@ func (s *environInstSuite) TestInstancesNoMatch(c *gc.C) {
 	c.Check(errors.Cause(err), gc.Equals, environs.ErrNoInstances)
 }
 
+func (s *environInstSuite) TestInstancesInvalidCredentials(c *gc.C) {
+	var invalidCred = false
+	// allInstances will ultimately return the error.
+	s.Client.Stub.SetErrors(errTestUnAuth)
+
+	ids := []instance.Id{"eggs"}
+	_, err := s.Env.Instances(&context.CloudCallContext{
+		InvalidateCredentialFunc: func(string) error {
+			invalidCred = true
+			return nil
+		},
+	}, ids)
+
+	c.Check(err, gc.ErrorMatches, "not authorized")
+	c.Assert(invalidCred, jc.IsTrue)
+}
+
 func (s *environInstSuite) TestControllerInstancesOkay(c *gc.C) {
 	s.Client.Containers = []containerlxd.Container{*s.Container}
 
@@ -123,6 +140,22 @@ func (s *environInstSuite) TestControllerInstancesMixed(c *gc.C) {
 	c.Check(ids, jc.DeepEquals, []instance.Id{"spam"})
 }
 
+func (s *environInstSuite) TestControllerInvalidCredentials(c *gc.C) {
+	var invalidCred = false
+	// AliveContainers will return an error.
+	s.Client.Stub.SetErrors(errTestUnAuth)
+
+	_, err := s.Env.ControllerInstances(
+		&context.CloudCallContext{
+			InvalidateCredentialFunc: func(string) error {
+				invalidCred = true
+				return nil
+			},
+		}, coretesting.ControllerTag.Id())
+	c.Check(err, gc.ErrorMatches, "not authorized")
+	c.Assert(invalidCred, jc.IsTrue)
+}
+
 func (s *environInstSuite) TestAdoptResources(c *gc.C) {
 	one := s.NewContainer(c, "smoosh")
 	two := s.NewContainer(c, "guild-league")
@@ -159,3 +192,20 @@ func (s *environInstSuite) TestAdoptResourcesError(c *gc.C) {
 	s.BaseSuite.Client.CheckCall(
 		c, 3, "UpdateContainerConfig", "tall-dwarfs", map[string]string{"user.juju-controller-uuid": "target-uuid"})
 }
+
+func (s *environInstSuite) TestAdoptResourcesInvalidResources(c *gc.C) {
+	var invalidCred = false
+	// allInstances will ultimately return the error.
+	s.Client.Stub.SetErrors(errTestUnAuth)
+
+	err := s.Env.AdoptResources(&context.CloudCallContext{
+		InvalidateCredentialFunc: func(string) error {
+			invalidCred = true
+			return nil
+		},
+	}, "target-uuid", version.MustParse("3.4.5"))
+
+	c.Check(err, gc.ErrorMatches, ".*not authorized")
+	c.Assert(invalidCred, jc.IsTrue)
+	s.BaseSuite.Client.CheckCall(c, 0, "AliveContainers", "juju-f75cba-")
+}