From c3d1dc7d73c705b991b7b94b609eb0ac0bca1ea4 Mon Sep 17 00:00:00 2001 From: justinsb Date: Tue, 13 Feb 2024 14:56:31 -0500 Subject: [PATCH] mockgcp: Don't wrap errors in the storage layer It is unnecessary now that we return "grpc ready" errors from the storage layer, and may also break HTTP status codes (e.g. 404 on not found). --- mockgcp/mockcompute/disksv1.go | 20 ++++---------------- mockgcp/mockcompute/regiondisksv1.go | 20 ++++---------------- mockgcp/mockresourcemanager/tagkeys.go | 18 +++--------------- 3 files changed, 11 insertions(+), 47 deletions(-) diff --git a/mockgcp/mockcompute/disksv1.go b/mockgcp/mockcompute/disksv1.go index 2b5b703545..38a9140edc 100644 --- a/mockgcp/mockcompute/disksv1.go +++ b/mockgcp/mockcompute/disksv1.go @@ -22,7 +22,6 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" - apierrors "k8s.io/apimachinery/pkg/api/errors" "github.com/GoogleCloudPlatform/k8s-config-connector/mockgcp/common/projects" pb "github.com/GoogleCloudPlatform/k8s-config-connector/mockgcp/generated/mockgcp/cloud/compute/v1" @@ -44,11 +43,7 @@ func (s *DisksV1) Get(ctx context.Context, req *pb.GetDiskRequest) (*pb.Disk, er obj := &pb.Disk{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "disk %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error reading disk: %v", err) - } + return nil, err } return obj, nil @@ -81,7 +76,7 @@ func (s *DisksV1) Insert(ctx context.Context, req *pb.InsertDiskRequest) (*pb.Op } if err := s.storage.Create(ctx, fqn, obj); err != nil { - return nil, status.Errorf(codes.Internal, "error creating disk: %v", err) + return nil, err } return s.newLRO(ctx, name.Project.ID) @@ -98,10 +93,7 @@ func (s *DisksV1) Update(ctx context.Context, req *pb.UpdateDiskRequest) (*pb.Op fqn := name.String() obj := &pb.Disk{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "disk %q not found", fqn) - } - return nil, status.Errorf(codes.Internal, "error reading disk: %v", err) + return nil, err } // TODO: Implement helper to implement the full rules here @@ -125,11 +117,7 @@ func (s *DisksV1) Delete(ctx context.Context, req *pb.DeleteDiskRequest) (*pb.Op deleted := (&pb.Disk{}) if err := s.storage.Delete(ctx, fqn, deleted); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "disk %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error deleting disk: %v", err) - } + return nil, err } return s.newLRO(ctx, name.Project.ID) diff --git a/mockgcp/mockcompute/regiondisksv1.go b/mockgcp/mockcompute/regiondisksv1.go index 0abcda322a..2034609141 100644 --- a/mockgcp/mockcompute/regiondisksv1.go +++ b/mockgcp/mockcompute/regiondisksv1.go @@ -22,7 +22,6 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" - apierrors "k8s.io/apimachinery/pkg/api/errors" "github.com/GoogleCloudPlatform/k8s-config-connector/mockgcp/common/projects" pb "github.com/GoogleCloudPlatform/k8s-config-connector/mockgcp/generated/mockgcp/cloud/compute/v1" @@ -44,11 +43,7 @@ func (s *RegionalDisksV1) Get(ctx context.Context, req *pb.GetRegionDiskRequest) obj := &pb.Disk{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "disk %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error reading disk: %v", err) - } + return nil, err } return obj, nil @@ -81,7 +76,7 @@ func (s *RegionalDisksV1) Insert(ctx context.Context, req *pb.InsertRegionDiskRe } if err := s.storage.Create(ctx, fqn, obj); err != nil { - return nil, status.Errorf(codes.Internal, "error creating disk: %v", err) + return nil, err } return s.newLRO(ctx, name.Project.ID) @@ -98,10 +93,7 @@ func (s *RegionalDisksV1) Update(ctx context.Context, req *pb.UpdateRegionDiskRe fqn := name.String() obj := &pb.Disk{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "disk %q not found", fqn) - } - return nil, status.Errorf(codes.Internal, "error reading disk: %v", err) + return nil, err } // TODO: Implement helper to implement the full rules here @@ -125,11 +117,7 @@ func (s *RegionalDisksV1) Delete(ctx context.Context, req *pb.DeleteRegionDiskRe deleted := &pb.Disk{} if err := s.storage.Delete(ctx, fqn, deleted); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "disk %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error deleting disk: %v", err) - } + return nil, err } return s.newLRO(ctx, name.Project.ID) diff --git a/mockgcp/mockresourcemanager/tagkeys.go b/mockgcp/mockresourcemanager/tagkeys.go index 59ad35497e..e94180e5fa 100644 --- a/mockgcp/mockresourcemanager/tagkeys.go +++ b/mockgcp/mockresourcemanager/tagkeys.go @@ -27,7 +27,6 @@ import ( "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/timestamppb" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/klog/v2" "github.com/GoogleCloudPlatform/k8s-config-connector/mockgcp/common/projects" @@ -50,11 +49,7 @@ func (s *TagKeys) GetTagKey(ctx context.Context, req *pb.GetTagKeyRequest) (*pb. obj := &pb.TagKey{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "tagKey %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error reading tagKey: %v", err) - } + return nil, err } // We should verify that this is part of on of our projects, but ... it's a mock @@ -128,10 +123,7 @@ func (s *TagKeys) UpdateTagKey(ctx context.Context, req *pb.UpdateTagKeyRequest) fqn := name.String() obj := &pb.TagKey{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "tagKey %q not found", reqName) - } - return nil, status.Errorf(codes.Internal, "error reading tagKey: %v", err) + return nil, err } // We should verify that this is part of on of our projects, but ... it's a mock @@ -172,11 +164,7 @@ func (s *TagKeys) DeleteTagKey(ctx context.Context, req *pb.DeleteTagKeyRequest) deleted := &pb.TagKey{} if err := s.storage.Delete(ctx, fqn, deleted); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "tagKey %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error deleting tagKey: %v", err) - } + return nil, err } // We should verify that this is part of on of our projects, but ... it's a mock