From 08c3f1c2324d79d6080fbf73f11b4fa715bec4cb Mon Sep 17 00:00:00 2001 From: Leonardo Cecchi Date: Thu, 25 Sep 2025 10:36:48 +0200 Subject: [PATCH] feat: return proper gRPC error codes for expected conditions (#549) The plugin now returns a 404 status code when a requested WAL file does not exist in the object store. This prevents misleading log entries such as "Error while handling gRPC request" for expected missing-file scenarios. The `ErrEndOfWALStreamReached` now returns `OutOfRange` error. The `ErrMissingPermissions` now returns `FailedPrecondition` error. Signed-off-by: Leonardo Cecchi Signed-off-by: Armando Ruocco Signed-off-by: Marco Nenciarini Co-authored-by: Armando Ruocco Co-authored-by: Marco Nenciarini --- internal/cnpgi/common/errors.go | 31 ++++++++++++++++++++----------- internal/cnpgi/common/wal.go | 22 ++++++++-------------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/internal/cnpgi/common/errors.go b/internal/cnpgi/common/errors.go index 48b2cde..3fc4fe5 100644 --- a/internal/cnpgi/common/errors.go +++ b/internal/cnpgi/common/errors.go @@ -1,16 +1,25 @@ package common -// walNotFoundError is raised when a WAL file has not been found in the object store -type walNotFoundError struct{} +import ( + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) -func newWALNotFoundError() *walNotFoundError { return &walNotFoundError{} } +// ErrEndOfWALStreamReached is returned when end of WAL is detected in the cloud archive. +var ErrEndOfWALStreamReached = status.Errorf(codes.OutOfRange, "end of WAL reached") -// ShouldPrintStackTrace tells whether the sidecar log stream should contain the stack trace -func (e walNotFoundError) ShouldPrintStackTrace() bool { - return false -} - -// Error implements the error interface -func (e walNotFoundError) Error() string { - return "WAL file not found" +// ErrMissingPermissions is raised when the sidecar has no +// permission to download the credentials needed to reach +// the object storage. +// This will be fixed by the reconciliation loop in the +// operator plugin. +var ErrMissingPermissions = status.Errorf(codes.FailedPrecondition, + "no permission to download the backup credentials, retrying") + +// newWALNotFoundError returns a error that states that a +// certain WAL file has not been found. This error is +// compatible with GRPC status codes, resulting in a 404 +// being used as a response code. +func newWALNotFoundError(walName string) error { + return status.Errorf(codes.NotFound, "wal %q not found", walName) } diff --git a/internal/cnpgi/common/wal.go b/internal/cnpgi/common/wal.go index 5f6c856..e15e987 100644 --- a/internal/cnpgi/common/wal.go +++ b/internal/cnpgi/common/wal.go @@ -27,13 +27,6 @@ import ( "github.com/cloudnative-pg/plugin-barman-cloud/internal/cnpgi/operator/config" ) -// ErrMissingPermissions is raised when the sidecar has no -// permission to download the credentials needed to reach -// the object storage. -// This will be fixed by the reconciliation loop in the -// operator plugin. -var ErrMissingPermissions = fmt.Errorf("no permission to download the backup credentials, retrying") - // SpoolManagementError is raised when a spool management // error has been detected type SpoolManagementError struct { @@ -95,11 +88,14 @@ func (w WALServiceImplementation) Archive( ctx context.Context, request *wal.WALArchiveRequest, ) (*wal.WALArchiveResult, error) { - contextLogger := log.FromContext(ctx) - contextLogger.Debug("starting wal archive") - baseWalName := path.Base(request.GetSourceFileName()) + contextLogger := log.FromContext(ctx) + contextLogger.Debug("wal archive start", "walName", baseWalName) + defer func() { + contextLogger.Debug("wal archive end", "walName", baseWalName) + }() + // Step 1: parse the configuration and get the environment variables needed // for barman-cloud-wal-archive configuration, err := config.NewFromClusterJSON(request.ClusterDefinition) @@ -122,6 +118,7 @@ func (w WALServiceImplementation) Archive( ) if err != nil { if apierrors.IsForbidden(err) { + contextLogger.Info(ErrMissingPermissions.Error()) return nil, ErrMissingPermissions } return nil, err @@ -350,7 +347,7 @@ func (w WALServiceImplementation) restoreFromBarmanObjectStore( // The failure has already been logged in walRestorer.RestoreList method if walStatus[0].Err != nil { if errors.Is(walStatus[0].Err, barmanRestorer.ErrWALNotFound) { - return newWALNotFoundError() + return newWALNotFoundError(walStatus[0].WalName) } return walStatus[0].Err @@ -457,9 +454,6 @@ func gatherWALFilesToRestore(walName string, parallel int) (walList []string, er return walList, err } -// ErrEndOfWALStreamReached is returned when end of WAL is detected in the cloud archive. -var ErrEndOfWALStreamReached = errors.New("end of WAL reached") - // checkEndOfWALStreamFlag returns ErrEndOfWALStreamReached if the flag is set in the restorer. func checkEndOfWALStreamFlag(walRestorer *barmanRestorer.WALRestorer) error { contain, err := walRestorer.IsEndOfWALStream()