Implementing IsAccessDeniedErr method on the objstore

alanprot · alanprot · commit f10e813caaa7 · 2023-08-15T22:49:45.000-07:00
Signed-off-by: Alan Protasio &lt;alanprot@gmail.com&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -35,5 +35,5 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re
 - [#35](https://github.com/thanos-io/objstore/pull/35) Azure: Update Azure SDK and fix breaking changes.
 - [#65](https://github.com/thanos-io/objstore/pull/65) *: Upgrade minio-go version to `v7.0.61`.
 - [#70](https://github.com/thanos-io/objstore/pull/70) GCS: Update cloud.google.com/go/storage version to `v1.27.0`.
-
+- [#71](https://github.com/thanos-io/objstore/pull/71) Replace method `IsCustomerManagedKeyError` for a more generic `IsAccessDeniedErr` on the bucket interface.
 ### Removed
diff --git a/README.md b/README.md
@@ -88,7 +88,8 @@ type BucketReader interface {
 	// IsObjNotFoundErr returns true if error means that object is not found. Relevant to Get operations.
 	IsObjNotFoundErr(err error) bool
 
-	// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.
+	// IsAccessDeniedErr returns true if access to object is denied.
+	IsAccessDeniedErr(err error) bool
 ```
 
 Those interfaces represent the object storage operations your code can use from `objstore` clients.
diff --git a/inmem.go b/inmem.go
@@ -207,8 +207,8 @@ func (b *InMemBucket) IsObjNotFoundErr(err error) bool {
 	return errors.Is(err, errNotFound)
 }
 
-// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.
-func (b *InMemBucket) IsCustomerManagedKeyError(_ error) bool {
+// IsAccessDeniedErr returns true if access to object is denied.
+func (b *InMemBucket) IsAccessDeniedErr(err error) bool {
 	return false
 }
 
diff --git a/objstore.go b/objstore.go
@@ -85,8 +85,8 @@ type BucketReader interface {
 	// IsObjNotFoundErr returns true if error means that object is not found. Relevant to Get operations.
 	IsObjNotFoundErr(err error) bool
 
-	// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.
-	IsCustomerManagedKeyError(err error) bool
+	// IsAccessDeniedErr returns true if acces to object is denied.
+	IsAccessDeniedErr(err error) bool
 
 	// Attributes returns information about the specified object.
 	Attributes(ctx context.Context, name string) (ObjectAttributes, error)
@@ -624,8 +624,8 @@ func (b *metricBucket) IsObjNotFoundErr(err error) bool {
 	return b.bkt.IsObjNotFoundErr(err)
 }
 
-func (b *metricBucket) IsCustomerManagedKeyError(err error) bool {
-	return b.bkt.IsCustomerManagedKeyError(err)
+func (b *metricBucket) IsAccessDeniedErr(err error) bool {
+	return b.bkt.IsAccessDeniedErr(err)
 }
 
 func (b *metricBucket) Close() error {
diff --git a/prefixed_bucket.go b/prefixed_bucket.go
@@ -74,9 +74,9 @@ func (p *PrefixedBucket) IsObjNotFoundErr(err error) bool {
 	return p.bkt.IsObjNotFoundErr(err)
 }
 
-// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.
-func (p *PrefixedBucket) IsCustomerManagedKeyError(err error) bool {
-	return p.bkt.IsCustomerManagedKeyError(err)
+// IsAccessDeniedErr returns true if access to object is denied.
+func (p *PrefixedBucket) IsAccessDeniedErr(err error) bool {
+	return p.bkt.IsAccessDeniedErr(err)
 }
 
 // Attributes returns information about the specified object.
diff --git a/providers/azure/azure.go b/providers/azure/azure.go
@@ -235,9 +235,12 @@ func (b *Bucket) IsObjNotFoundErr(err error) bool {
 	return bloberror.HasCode(err, bloberror.BlobNotFound) || bloberror.HasCode(err, bloberror.InvalidURI)
 }
 
-// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.
-func (b *Bucket) IsCustomerManagedKeyError(_ error) bool {
-	return false
+// IsAccessDeniedErr returns true if access to object is denied.
+func (b *Bucket) IsAccessDeniedErr(err error) bool {
+	if err == nil {
+		return false
+	}
+	return bloberror.HasCode(err, bloberror.AuthorizationPermissionMismatch) || bloberror.HasCode(err, bloberror.InsufficientAccountPermissions)
 }
 
 func (b *Bucket) getBlobReader(ctx context.Context, name string, httpRange blob.HTTPRange) (io.ReadCloser, error) {
diff --git a/providers/bos/bos.go b/providers/bos/bos.go
@@ -287,8 +287,8 @@ func (b *Bucket) IsObjNotFoundErr(err error) bool {
 	return false
 }
 
-// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.
-func (b *Bucket) IsCustomerManagedKeyError(_ error) bool {
+// IsAccessDeniedErr returns true if access to object is denied.
+func (b *Bucket) IsAccessDeniedErr(_ error) bool {
 	return false
 }
 
diff --git a/providers/cos/cos.go b/providers/cos/cos.go
@@ -364,8 +364,8 @@ func (b *Bucket) IsObjNotFoundErr(err error) bool {
 	}
 }
 
-// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.
-func (b *Bucket) IsCustomerManagedKeyError(_ error) bool {
+// IsAccessDeniedErr returns true if access to object is denied.
+func (b *Bucket) IsAccessDeniedErr(_ error) bool {
 	return false
 }
 
diff --git a/providers/filesystem/filesystem.go b/providers/filesystem/filesystem.go
@@ -258,8 +258,8 @@ func (b *Bucket) IsObjNotFoundErr(err error) bool {
 	return os.IsNotExist(errors.Cause(err))
 }
 
-// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.
-func (b *Bucket) IsCustomerManagedKeyError(_ error) bool {
+// IsAccessDeniedErr returns true if access to object is denied.
+func (b *Bucket) IsAccessDeniedErr(_ error) bool {
 	return false
 }
 
diff --git a/providers/gcs/gcs.go b/providers/gcs/gcs.go
@@ -19,6 +19,8 @@ import (
 	"golang.org/x/oauth2/google"
 	"google.golang.org/api/iterator"
 	"google.golang.org/api/option"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 	"gopkg.in/yaml.v2"
 
 	"github.com/thanos-io/objstore"
@@ -188,8 +190,11 @@ func (b *Bucket) IsObjNotFoundErr(err error) bool {
 	return errors.Is(err, storage.ErrObjectNotExist)
 }
 
-// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.
-func (b *Bucket) IsCustomerManagedKeyError(_ error) bool {
+// IsAccessDeniedErr returns true if access to object is denied.
+func (b *Bucket) IsAccessDeniedErr(err error) bool {
+	if s, ok := status.FromError(err); ok && s.Code() == codes.PermissionDenied {
+		return true
+	}
 	return false
 }
 
diff --git a/providers/obs/obs.go b/providers/obs/obs.go
@@ -327,8 +327,8 @@ func (b *Bucket) IsObjNotFoundErr(err error) bool {
 	return false
 }
 
-// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.
-func (b *Bucket) IsCustomerManagedKeyError(_ error) bool {
+// IsAccessDeniedErr returns true if access to object is denied.
+func (b *Bucket) IsAccessDeniedErr(_ error) bool {
 	return false
 }
 
diff --git a/providers/oci/oci.go b/providers/oci/oci.go
@@ -227,8 +227,12 @@ func (b *Bucket) IsObjNotFoundErr(err error) bool {
 	return false
 }
 
-// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.
-func (b *Bucket) IsCustomerManagedKeyError(_ error) bool {
+// IsAccessDeniedErr returns true if access to object is denied.
+func (b *Bucket) IsAccessDeniedErr(err error) bool {
+	failure, isServiceError := common.IsServiceError(err)
+	if isServiceError {
+		return failure.GetHTTPStatusCode() == http.StatusForbidden
+	}
 	return false
 }
 
diff --git a/providers/oss/oss.go b/providers/oss/oss.go
@@ -379,7 +379,13 @@ func (b *Bucket) IsObjNotFoundErr(err error) bool {
 	return false
 }
 
-// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.
-func (b *Bucket) IsCustomerManagedKeyError(_ error) bool {
+// IsAccessDeniedErr returns true if access to object is denied.
+func (b *Bucket) IsAccessDeniedErr(err error) bool {
+	switch aliErr := errors.Cause(err).(type) {
+	case alioss.ServiceError:
+		if aliErr.StatusCode == http.StatusForbidden {
+			return true
+		}
+	}
 	return false
 }
diff --git a/providers/s3/s3.go b/providers/s3/s3.go
@@ -98,9 +98,6 @@ const (
 
 	// Storage class header.
 	amzStorageClass = "X-Amz-Storage-Class"
-
-	// amzKmsKeyAccessDeniedErrorMessage is the error message returned by s3 when the permissions to the KMS key is revoked.
-	amzKmsKeyAccessDeniedErrorMessage = "The ciphertext refers to a customer master key that does not exist, does not exist in this region, or you are not allowed to access."
 )
 
 var DefaultConfig = Config{
@@ -541,10 +538,9 @@ func (b *Bucket) IsObjNotFoundErr(err error) bool {
 	return minio.ToErrorResponse(errors.Cause(err)).Code == "NoSuchKey"
 }
 
-// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.
-func (b *Bucket) IsCustomerManagedKeyError(err error) bool {
-	errResponse := minio.ToErrorResponse(errors.Cause(err))
-	return errResponse.Code == "AccessDenied" && errResponse.Message == amzKmsKeyAccessDeniedErrorMessage
+// IsAccessDeniedErr returns true if access to object is denied.
+func (b *Bucket) IsAccessDeniedErr(err error) bool {
+	return minio.ToErrorResponse(errors.Cause(err)).Code == "AccessDenied"
 }
 
 func (b *Bucket) Close() error { return nil }
diff --git a/providers/swift/swift.go b/providers/swift/swift.go
@@ -290,9 +290,9 @@ func (c *Container) IsObjNotFoundErr(err error) bool {
 	return errors.Is(err, swift.ObjectNotFound)
 }
 
-// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.
-func (b *Container) IsCustomerManagedKeyError(_ error) bool {
-	return false
+// IsAccessDeniedErr returns true if access to object is denied.
+func (c *Container) IsAccessDeniedErr(err error) bool {
+	return errors.Is(err, swift.Forbidden)
 }
 
 // Upload writes the contents of the reader as an object into the container.
diff --git a/testing.go b/testing.go
@@ -309,6 +309,6 @@ func (d *delayingBucket) IsObjNotFoundErr(err error) bool {
 	return d.bkt.IsObjNotFoundErr(err)
 }
 
-func (d *delayingBucket) IsCustomerManagedKeyError(err error) bool {
-	return d.bkt.IsCustomerManagedKeyError(err)
+func (d *delayingBucket) IsAccessDeniedErr(err error) bool {
+	return d.bkt.IsAccessDeniedErr(err)
 }
diff --git a/tracing/opentelemetry/opentelemetry.go b/tracing/opentelemetry/opentelemetry.go
@@ -128,8 +128,8 @@ func (t TracingBucket) IsObjNotFoundErr(err error) bool {
 	return t.bkt.IsObjNotFoundErr(err)
 }
 
-func (t TracingBucket) IsCustomerManagedKeyError(err error) bool {
-	return t.bkt.IsCustomerManagedKeyError(err)
+func (t TracingBucket) IsAccessDeniedErr(err error) bool {
+	return t.bkt.IsAccessDeniedErr(err)
 }
 
 func (t TracingBucket) WithExpectedErrs(expectedFunc objstore.IsOpFailureExpectedFunc) objstore.Bucket {
diff --git a/tracing/opentracing/opentracing.go b/tracing/opentracing/opentracing.go
@@ -124,8 +124,8 @@ func (t TracingBucket) IsObjNotFoundErr(err error) bool {
 	return t.bkt.IsObjNotFoundErr(err)
 }
 
-func (t TracingBucket) IsCustomerManagedKeyError(err error) bool {
-	return t.bkt.IsCustomerManagedKeyError(err)
+func (t TracingBucket) IsAccessDeniedErr(err error) bool {
+	return t.bkt.IsAccessDeniedErr(err)
 }
 
 func (t TracingBucket) WithExpectedErrs(expectedFunc objstore.IsOpFailureExpectedFunc) objstore.Bucket {

Original file line number	Diff line number	Diff line change
`@@ -207,8 +207,8 @@ func (b *InMemBucket) IsObjNotFoundErr(err error) bool {`
`207`	`207`	`return errors.Is(err, errNotFound)`
`208`	`208`	`}`
`209`	`209`
`210`		`-// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.`
`211`		`-func (b *InMemBucket) IsCustomerManagedKeyError(_ error) bool {`
	`210`	`+// IsAccessDeniedErr returns true if access to object is denied.`
	`211`	`+func (b *InMemBucket) IsAccessDeniedErr(err error) bool {`
`212`	`212`	`return false`
`213`	`213`	`}`
`214`	`214`
Original file line number	Diff line number	Diff line change
`@@ -74,9 +74,9 @@ func (p *PrefixedBucket) IsObjNotFoundErr(err error) bool {`
`74`	`74`	`return p.bkt.IsObjNotFoundErr(err)`
`75`	`75`	`}`
`76`	`76`
`77`		`-// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.`
`78`		`-func (p *PrefixedBucket) IsCustomerManagedKeyError(err error) bool {`
`79`		`- return p.bkt.IsCustomerManagedKeyError(err)`
	`77`	`+// IsAccessDeniedErr returns true if access to object is denied.`
	`78`	`+func (p *PrefixedBucket) IsAccessDeniedErr(err error) bool {`
	`79`	`+ return p.bkt.IsAccessDeniedErr(err)`
`80`	`80`	`}`
`81`	`81`
`82`	`82`	`// Attributes returns information about the specified object.`
Original file line number	Diff line number	Diff line change
`@@ -235,9 +235,12 @@ func (b *Bucket) IsObjNotFoundErr(err error) bool {`
`235`	`235`	`return bloberror.HasCode(err, bloberror.BlobNotFound) \|\| bloberror.HasCode(err, bloberror.InvalidURI)`
`236`	`236`	`}`
`237`	`237`
`238`		`-// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.`
`239`		`-func (b *Bucket) IsCustomerManagedKeyError(_ error) bool {`
`240`		`- return false`
	`238`	`+// IsAccessDeniedErr returns true if access to object is denied.`
	`239`	`+func (b *Bucket) IsAccessDeniedErr(err error) bool {`
	`240`	`+ if err == nil {`
	`241`	`+ return false`
	`242`	`+ }`
	`243`	`+ return bloberror.HasCode(err, bloberror.AuthorizationPermissionMismatch) \|\| bloberror.HasCode(err, bloberror.InsufficientAccountPermissions)`
`241`	`244`	`}`
`242`	`245`
`243`	`246`	`func (b *Bucket) getBlobReader(ctx context.Context, name string, httpRange blob.HTTPRange) (io.ReadCloser, error) {`
Original file line number	Diff line number	Diff line change
`@@ -287,8 +287,8 @@ func (b *Bucket) IsObjNotFoundErr(err error) bool {`
`287`	`287`	`return false`
`288`	`288`	`}`
`289`	`289`
`290`		`-// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.`
`291`		`-func (b *Bucket) IsCustomerManagedKeyError(_ error) bool {`
	`290`	`+// IsAccessDeniedErr returns true if access to object is denied.`
	`291`	`+func (b *Bucket) IsAccessDeniedErr(_ error) bool {`
`292`	`292`	`return false`
`293`	`293`	`}`
`294`	`294`
Original file line number	Diff line number	Diff line change
`@@ -364,8 +364,8 @@ func (b *Bucket) IsObjNotFoundErr(err error) bool {`
`364`	`364`	`}`
`365`	`365`	`}`
`366`	`366`
`367`		`-// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.`
`368`		`-func (b *Bucket) IsCustomerManagedKeyError(_ error) bool {`
	`367`	`+// IsAccessDeniedErr returns true if access to object is denied.`
	`368`	`+func (b *Bucket) IsAccessDeniedErr(_ error) bool {`
`369`	`369`	`return false`
`370`	`370`	`}`
`371`	`371`
Original file line number	Diff line number	Diff line change
`@@ -258,8 +258,8 @@ func (b *Bucket) IsObjNotFoundErr(err error) bool {`
`258`	`258`	`return os.IsNotExist(errors.Cause(err))`
`259`	`259`	`}`
`260`	`260`
`261`		`-// IsCustomerManagedKeyError returns true if the permissions for key used to encrypt the object was revoked.`
`262`		`-func (b *Bucket) IsCustomerManagedKeyError(_ error) bool {`
	`261`	`+// IsAccessDeniedErr returns true if access to object is denied.`
	`262`	`+func (b *Bucket) IsAccessDeniedErr(_ error) bool {`
`263`	`263`	`return false`
`264`	`264`	`}`
`265`	`265`