Add credential invalidation to OCI provider

This commit improves our ability to gracefully handle authentication and authorisation errors. It makes use of the provider/common/errors.HandleCredentialError callback mechanism to mark credentials as invalid client-side. Changes within oci/common/errors*.go: - Change IsAuthenticationError() to rely on the SDK's ServiceError interface. This improves the ability for us to test that it's working. - Increase the test coverage by generating non-auth errors and ensuring that they fail. Changes within provider/oci/*.go: - Add HandleCredentialError() to places where the code interacts with the API server. - Ensure that errors.Trace(err) is added to errors where appropriate. - Remove unnecessary aliases - Update methods accessing instance information to require ProviderCallContext
MalloZup · Nov 29, 2018 · b7782b8 · b7782b8
1 parent ca80373
commit b7782b8
Show file tree

Hide file tree

Showing 9 changed files with 310 additions and 45 deletions.
diff --git a/provider/common/errors.go b/provider/common/errors.go
@@ -84,8 +84,7 @@ var AuthorisationFailureStatusCodes = set.NewInts(
 )
 
 // MaybeHandleCredentialError determines if a given error relates to an invalid credential.
-//  If it is, the credential is invalidated and the return bool is true.
-// Original error is returned untouched.
+// If it is, the credential is invalidated and the return bool is true.
 func MaybeHandleCredentialError(isAuthError func(error) bool, err error, ctx context.ProviderCallContext) bool {
 	denied := isAuthError(errors.Cause(err))
 	if ctx != nil && denied {
@@ -98,7 +97,6 @@ func MaybeHandleCredentialError(isAuthError func(error) bool, err error, ctx con
 }
 
 // HandleCredentialError determines if a given error relates to an invalid credential.
-// If it is, the credential is invalidated. Original error is returned untouched.
 func HandleCredentialError(isAuthError func(error) bool, err error, ctx context.ProviderCallContext) {
 	MaybeHandleCredentialError(isAuthError, err, ctx)
 }
diff --git a/provider/oci/common/errors.go b/provider/oci/common/errors.go
@@ -0,0 +1,61 @@
+// Copyright 2018 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details
+
+package common
+
+import (
+	"github.com/juju/collections/set"
+	ociCommon "github.com/oracle/oci-go-sdk/common"
+
+	"github.com/juju/juju/environs/context"
+	"github.com/juju/juju/provider/common"
+)
+
+// Oracle bundles authorisation errors into HTTP 401, HTTP 404 and
+// HTTP 409, without specifically indicating that it's an authorisation
+// failure. Each response includes a Code field that we can match against,
+// but they remain intentionally ambiguous.
+//
+// The codes we match against are:
+//   - HTTP 401 ("NotAuthenticated")
+//   - HTTP 404 ("NotAuthorizedOrNotFound")
+//   - HTTP 409 ("NotAuthorizedOrResourceAlreadyExists")
+//
+// As we're not generating any API calls manually, it's unlikely
+// that we'll be striking URIs that don't exist at all, therefore we assume
+// auth issues are causing the errors.
+//
+// For more details, see https://docs.cloud.oracle.com/iaas/Content/API/References/apierrors.htm
+var authErrorCodes = set.NewStrings(
+	"NotAuthenticated",
+	"NotAuthorizedOrResourceAlreadyExists",
+	"NotAuthorizedOrNotFound",
+)
+
+// IsAuthorisationFailure reports whether the error is related to
+// attempting to access the provider with invalid or expired credentials.
+func IsAuthorisationFailure(err error) bool {
+	if err == nil {
+		return false
+	}
+
+	serviceError, ok := err.(ociCommon.ServiceError)
+	if !ok {
+		// Just to double check, also try the SDK's
+		// implementation. This isn't checked first, because
+		// it is hard to test.
+		serviceError, ok = ociCommon.IsServiceError(err)
+	}
+
+	if ok && authErrorCodes.Contains(serviceError.GetCode()) {
+		return true
+	}
+
+	return false
+}
+
+// HandleCredentialError marks the current credentials as invalid internally
+//  if Oracle believes that they are expired
+func HandleCredentialError(err error, ctx context.ProviderCallContext) {
+	common.HandleCredentialError(IsAuthorisationFailure, err, ctx)
+}
diff --git a/provider/oci/common/errors_test.go b/provider/oci/common/errors_test.go
@@ -0,0 +1,60 @@
+// Copyright 2018 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package common_test
+
+import (
+	"fmt"
+
+	ocicommon "github.com/oracle/oci-go-sdk/common"
+	gc "gopkg.in/check.v1"
+
+	"github.com/juju/errors"
+	"github.com/juju/juju/provider/oci/common"
+	"github.com/juju/juju/testing"
+	jc "github.com/juju/testing/checkers"
+)
+
+type errorsSuite struct {
+	testing.BaseSuite
+}
+
+var _ = gc.Suite(&errorsSuite{})
+
+type MockServiceError struct {
+	ocicommon.ServiceError
+
+	code string
+}
+
+func (a MockServiceError) Error() string {
+	return fmt.Sprintf("Mocked error %s", a.GetCode())
+}
+
+func (a MockServiceError) GetCode() string { return a.code }
+
+func (s *errorsSuite) TestServiceErrorsCanTriggerIsAuthorisationFailure(c *gc.C) {
+	err := MockServiceError{code: "NotAuthenticated"}
+	result := common.IsAuthorisationFailure(err)
+	c.Assert(result, jc.IsTrue)
+
+	err = MockServiceError{code: "InternalServerError"}
+	result = common.IsAuthorisationFailure(err)
+	c.Assert(result, jc.IsFalse)
+}
+
+func (s *errorsSuite) TestUnknownErrorsDoNotTriggerIsAuthorisationFailure(c *gc.C) {
+	err1 := errors.New("unknown")
+	for _, err := range []error{
+		err1,
+		errors.Trace(err1),
+		errors.Annotate(err1, "really unknown"),
+	} {
+		c.Assert(common.IsAuthorisationFailure(err), jc.IsFalse)
+	}
+}
+
+func (s *errorsSuite) TestNilDoesNotTriggerIsAuthorisationFailure(c *gc.C) {
+	result := common.IsAuthorisationFailure(nil)
+	c.Assert(result, jc.IsFalse)
+}