Skip to content

OADP-8085: fix(gcp): use errors.Is/As for GCS bucket not-found checks #2232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PrasadJoshi12 wants to merge 1 commit into
base: oadp-1.6
Choose a base branch
from
+19 −5
Open

OADP-8085: fix(gcp): use errors.Is/As for GCS bucket not-found checks #2232

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions pkg/bucket/gcp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package bucket
import (
"context"
"encoding/json"
"errors"
"fmt"
"math/rand"
"net/http"
Expand Down Expand Up @@ -38,7 +39,7 @@ func (g gcpBucketClient) Exists() (bool, error) {
bucket := gcsClient.Bucket(g.bucket.Spec.Name)
_, err = bucket.Attrs(ctx)
if err != nil {
if err == storage.ErrBucketNotExist {
if isBucketNotFoundError(err) {
return false, nil
}
// Return true for permission errors - unable to determine if bucket exists
Expand Down Expand Up @@ -116,7 +117,7 @@ func (g gcpBucketClient) Delete() (bool, error) {
// Check if bucket exists first
_, err = bucket.Attrs(ctx)
if err != nil {
if err == storage.ErrBucketNotExist {
if isBucketNotFoundError(err) {
// Bucket doesn't exist - idempotent behavior
return true, nil
}
Expand Down Expand Up @@ -486,7 +487,8 @@ func isGCSRetryableError(err error) bool {
return false
}

if gerr, ok := err.(*googleapi.Error); ok {
var gerr *googleapi.Error
if errors.As(err, &gerr) {
switch gerr.Code {
Comment on lines +490 to 492
case http.StatusTooManyRequests: // Too Many Requests
return true
Expand All @@ -513,16 +515,28 @@ func isGCSRetryableError(err error) bool {
return false
}

// isBucketNotFoundError reports whether err indicates a GCS bucket does not exist.
// The GCS library may return storage.ErrBucketNotExist directly, wrap it via fmt.Errorf("%w", ...),
// or surface a *googleapi.Error with HTTP 404 — all three must be handled.
func isBucketNotFoundError(err error) bool {
if errors.Is(err, storage.ErrBucketNotExist) {
return true
}
var gerr *googleapi.Error
return errors.As(err, &gerr) && gerr.Code == http.StatusNotFound
Comment on lines +522 to +526
Comment on lines +518 to +526
}

// handleGCSError provides consistent error handling for GCS operations
func handleGCSError(err error, operation string, bucketName string) error {
if err == storage.ErrBucketNotExist {
if errors.Is(err, storage.ErrBucketNotExist) {
if operation == "exists" || operation == "delete" {
return nil // Expected for these operations
}
return fmt.Errorf("bucket '%s' not found", bucketName)
}

if gerr, ok := err.(*googleapi.Error); ok {
var gerr *googleapi.Error
if errors.As(err, &gerr) {
Comment on lines +538 to +539
switch gerr.Code {
case http.StatusConflict: // Conflict - Bucket already exists
if operation == "create" {
Expand Down