Skip to content

Commit

Permalink
Merge pull request #1221 from justinsb/storage_better_errors_update
Browse files Browse the repository at this point in the history 
mockgcp: Refactor errors from storage.Update and storage.Delete also
  • Loading branch information
@google-oss-prow
google-oss-prow[bot] authored Feb 9, 2024
2 parents a701f04 + 59fd880 commit 8d4ec8a
Show file tree
Hide file tree
Showing 21 changed files with 97 additions and 284 deletions.
8 changes: 1 addition & 7 deletions mockgcp/common/operations/operations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@ import (
rpcstatus "google.golang.org/genproto/googleapis/rpc/status"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/encoding/prototext"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/anypb"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/util/uuid"
"k8s.io/klog/v2"

Expand Down Expand Up @@ -122,11 +120,7 @@ func (s *Operations) GetOperation(ctx context.Context, req *pb.GetOperationReque

op := &pb.Operation{}
if err := s.storage.Get(ctx, fqn, op); err != nil {
if apierrors.IsNotFound(err) {
klog.Infof("LRO not found for %v", prototext.Format(req))
return nil, status.Errorf(codes.NotFound, "LRO %q not found", req.Name)
}
return nil, status.Errorf(codes.Internal, "error reading LRO: %v", err)
return nil, err
}

return op, nil
Expand Down
27 changes: 5 additions & 22 deletions mockgcp/mockapikeys/key.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"google.golang.org/grpc/status"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/fieldmaskpb"
apierrors "k8s.io/apimachinery/pkg/api/errors"

pb "github.com/GoogleCloudPlatform/k8s-config-connector/mockgcp/generated/mockgcp/api/apikeys/v2"
)
Expand All @@ -44,11 +43,7 @@ func (s *APIKeysV2) GetKey(ctx context.Context, req *pb.GetKeyRequest) (*pb.Key,

obj := &pb.Key{}
if err := s.storage.Get(ctx, fqn, obj); err != nil {
if apierrors.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, "key %q not found", name)
} else {
return nil, status.Errorf(codes.Internal, "error reading key: %v", err)
}
return nil, err
}

return obj, nil
Expand All @@ -64,11 +59,7 @@ func (s *APIKeysV2) GetKeyString(ctx context.Context, req *pb.GetKeyStringReques

obj := &pb.Key{}
if err := s.storage.Get(ctx, fqn, obj); err != nil {
if apierrors.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, "key %q not found", name)
} else {
return nil, status.Errorf(codes.Internal, "error reading key: %v", err)
}
return nil, err
}

keyString := "dummy-encrypted-value"
Expand Down Expand Up @@ -112,11 +103,7 @@ func (s *APIKeysV2) UpdateKey(ctx context.Context, req *pb.UpdateKeyRequest) (*l

obj := &pb.Key{}
if err := s.storage.Get(ctx, fqn, obj); err != nil {
if apierrors.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, "key %q not found", name)
} else {
return nil, status.Errorf(codes.Internal, "error reading key: %v", err)
}
return nil, err
}

// From the proto:
Expand Down Expand Up @@ -163,7 +150,7 @@ func (s *APIKeysV2) UpdateKey(ctx context.Context, req *pb.UpdateKeyRequest) (*l
}

if err := s.storage.Update(ctx, fqn, obj); err != nil {
return nil, status.Errorf(codes.Internal, "error updating key: %v", err)
return nil, err
}

return s.operations.NewLRO(ctx)
Expand All @@ -179,11 +166,7 @@ func (s *APIKeysV2) DeleteKey(ctx context.Context, req *pb.DeleteKeyRequest) (*l

deleted := &pb.Key{}
if err := s.storage.Delete(ctx, fqn, deleted); err != nil {
if apierrors.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, "key %q not found", name)
} else {
return nil, status.Errorf(codes.Internal, "error deleting key: %v", err)
}
return nil, err
}

return s.operations.NewLRO(ctx)
Expand Down
5 changes: 2 additions & 3 deletions mockgcp/mockbilling/billingv1.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,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/billing/v1"
Expand Down Expand Up @@ -51,13 +50,13 @@ func (s *BillingV1) GetProjectBillingInfo(ctx context.Context, req *pb.GetProjec

obj := &pb.ProjectBillingInfo{}
if err := s.storage.Get(ctx, fqn, obj); err != nil {
if apierrors.IsNotFound(err) {
if status.Code(err) == codes.NotFound {
// Expected if billing info has not yet been set
obj.Name = fqn
obj.BillingEnabled = false
obj.ProjectId = projectName.ProjectID
} else {
return nil, status.Errorf(codes.Internal, "error reading projectBillingInfo: %v", err)
return nil, err
}
}

Expand Down
77 changes: 16 additions & 61 deletions mockgcp/mockcertificatemanager/v1.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/proto"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/klog/v2"

pb "github.com/GoogleCloudPlatform/k8s-config-connector/mockgcp/generated/mockgcp/cloud/certificatemanager/v1"
Expand All @@ -42,11 +41,7 @@ func (s *CertificateManagerV1) GetCertificate(ctx context.Context, req *pb.GetCe

obj := &pb.Certificate{}
if err := s.storage.Get(ctx, fqn, obj); err != nil {
if apierrors.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, "certificate %q not found", name)
} else {
return nil, status.Errorf(codes.Internal, "error reading certificate: %v", err)
}
return nil, err
}

return obj, nil
Expand Down Expand Up @@ -82,10 +77,7 @@ func (s *CertificateManagerV1) UpdateCertificate(ctx context.Context, req *pb.Up
fqn := name.String()
obj := &pb.Certificate{}
if err := s.storage.Get(ctx, fqn, obj); err != nil {
if apierrors.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, "certificate %q not found", reqName)
}
return nil, status.Errorf(codes.Internal, "error reading certificate: %v", err)
return nil, err
}

// Required. The update mask applies to the resource.
Expand All @@ -107,7 +99,7 @@ func (s *CertificateManagerV1) UpdateCertificate(ctx context.Context, req *pb.Up
}

if err := s.storage.Update(ctx, fqn, obj); err != nil {
return nil, status.Errorf(codes.Internal, "error updating certificate: %v", err)
return nil, err
}

return s.operations.NewLRO(ctx)
Expand All @@ -123,11 +115,7 @@ func (s *CertificateManagerV1) DeleteCertificate(ctx context.Context, req *pb.De

deletedObj := &pb.Certificate{}
if err := s.storage.Delete(ctx, fqn, deletedObj); err != nil {
if apierrors.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, "certificate %q not found", name)
} else {
return nil, status.Errorf(codes.Internal, "error deleting certificate: %v", err)
}
return nil, err
}

return s.operations.NewLRO(ctx)
Expand All @@ -143,11 +131,7 @@ func (s *CertificateManagerV1) GetCertificateMap(ctx context.Context, req *pb.Ge

obj := &pb.CertificateMap{}
if err := s.storage.Get(ctx, fqn, obj); err != nil {
if apierrors.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, "certificateMap %q not found", name)
} else {
return nil, status.Errorf(codes.Internal, "error reading certificateMap: %v", err)
}
return nil, err
}

return obj, nil
Expand Down Expand Up @@ -183,10 +167,7 @@ func (s *CertificateManagerV1) UpdateCertificateMap(ctx context.Context, req *pb
fqn := name.String()
obj := &pb.CertificateMap{}
if err := s.storage.Get(ctx, fqn, obj); err != nil {
if apierrors.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, "certificateMap %q not found", reqName)
}
return nil, status.Errorf(codes.Internal, "error reading certificateMap: %v", err)
return nil, err
}

// Required. The update mask applies to the resource.
Expand All @@ -207,7 +188,7 @@ func (s *CertificateManagerV1) UpdateCertificateMap(ctx context.Context, req *pb
}

if err := s.storage.Update(ctx, fqn, obj); err != nil {
return nil, status.Errorf(codes.Internal, "error updating certificateMap: %v", err)
return nil, err
}

return s.operations.NewLRO(ctx)
Expand All @@ -223,11 +204,7 @@ func (s *CertificateManagerV1) DeleteCertificateMap(ctx context.Context, req *pb

oldObj := &pb.CertificateMap{}
if err := s.storage.Delete(ctx, fqn, oldObj); err != nil {
if apierrors.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, "certificate map %q not found", name)
} else {
return nil, status.Errorf(codes.Internal, "error deleting certificate map: %v", err)
}
return nil, err
}

return s.operations.NewLRO(ctx)
Expand All @@ -243,11 +220,7 @@ func (s *CertificateManagerV1) GetDnsAuthorization(ctx context.Context, req *pb.

obj := &pb.DnsAuthorization{}
if err := s.storage.Get(ctx, fqn, obj); err != nil {
if apierrors.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, "dns authorization %q not found", name)
} else {
return nil, status.Errorf(codes.Internal, "error reading dns authorization: %v", err)
}
return nil, err
}

return obj, nil
Expand Down Expand Up @@ -283,10 +256,7 @@ func (s *CertificateManagerV1) UpdateDnsAuthorization(ctx context.Context, req *
fqn := name.String()
obj := &pb.DnsAuthorization{}
if err := s.storage.Get(ctx, fqn, obj); err != nil {
if apierrors.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, "dnsAuthorization %q not found", reqName)
}
return nil, status.Errorf(codes.Internal, "error reading dnsAuthorization: %v", err)
return nil, err
}

// Required. The update mask applies to the resource.
Expand All @@ -308,7 +278,7 @@ func (s *CertificateManagerV1) UpdateDnsAuthorization(ctx context.Context, req *
}

if err := s.storage.Update(ctx, fqn, obj); err != nil {
return nil, status.Errorf(codes.Internal, "error updating dnsAuthorization: %v", err)
return nil, err
}

return s.operations.NewLRO(ctx)
Expand All @@ -324,11 +294,7 @@ func (s *CertificateManagerV1) DeleteDnsAuthorization(ctx context.Context, req *

oldObj := &pb.DnsAuthorization{}
if err := s.storage.Delete(ctx, fqn, oldObj); err != nil {
if apierrors.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, "dns authorization %q not found", name)
} else {
return nil, status.Errorf(codes.Internal, "error deleting dns authorization: %v", err)
}
return nil, err
}

return s.operations.NewLRO(ctx)
Expand All @@ -344,11 +310,7 @@ func (s *CertificateManagerV1) GetCertificateMapEntry(ctx context.Context, req *

obj := &pb.CertificateMapEntry{}
if err := s.storage.Get(ctx, fqn, obj); err != nil {
if apierrors.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, "certificate map entry %q not found", name)
} else {
return nil, status.Errorf(codes.Internal, "error reading certificate map entry: %v", err)
}
return nil, err
}

return obj, nil
Expand Down Expand Up @@ -384,10 +346,7 @@ func (s *CertificateManagerV1) UpdateCertificateMapEntry(ctx context.Context, re
fqn := name.String()
obj := &pb.CertificateMapEntry{}
if err := s.storage.Get(ctx, fqn, obj); err != nil {
if apierrors.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, "certificateMapEntry %q not found", reqName)
}
return nil, status.Errorf(codes.Internal, "error reading certificateMapEntry: %v", err)
return nil, err
}

// Required. The update mask applies to the resource.
Expand All @@ -408,7 +367,7 @@ func (s *CertificateManagerV1) UpdateCertificateMapEntry(ctx context.Context, re
}

if err := s.storage.Update(ctx, fqn, obj); err != nil {
return nil, status.Errorf(codes.Internal, "error updating certificateMapEntry: %v", err)
return nil, err
}

return s.operations.NewLRO(ctx)
Expand All @@ -424,11 +383,7 @@ func (s *CertificateManagerV1) DeleteCertificateMapEntry(ctx context.Context, re

deletedObj := &pb.CertificateMapEntry{}
if err := s.storage.Delete(ctx, fqn, deletedObj); err != nil {
if apierrors.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, "certificate map entry %q not found", name)
} else {
return nil, status.Errorf(codes.Internal, "error deleting certificate map entry: %v", err)
}
return nil, err
}

return s.operations.NewLRO(ctx)
Expand Down
20 changes: 4 additions & 16 deletions mockgcp/mockcloudfunctions/v1.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/proto"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/klog/v2"

pb "github.com/GoogleCloudPlatform/k8s-config-connector/mockgcp/generated/mockgcp/cloud/functions/v1"
Expand All @@ -42,11 +41,7 @@ func (s *CloudFunctionsV1) GetFunction(ctx context.Context, req *pb.GetFunctionR

obj := &pb.CloudFunction{}
if err := s.storage.Get(ctx, fqn, obj); err != nil {
if apierrors.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, "function %q not found", name)
} else {
return nil, status.Errorf(codes.Internal, "error reading function: %v", err)
}
return nil, err
}

return obj, nil
Expand Down Expand Up @@ -81,10 +76,7 @@ func (s *CloudFunctionsV1) UpdateFunction(ctx context.Context, req *pb.UpdateFun
fqn := name.String()
obj := &pb.CloudFunction{}
if err := s.storage.Get(ctx, fqn, obj); err != nil {
if apierrors.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, "cloudFunction %q not found", reqName)
}
return nil, status.Errorf(codes.Internal, "error reading cloudFunction: %v", err)
return nil, err
}

// Required. The update mask applies to the resource.
Expand All @@ -108,7 +100,7 @@ func (s *CloudFunctionsV1) UpdateFunction(ctx context.Context, req *pb.UpdateFun
}

if err := s.storage.Update(ctx, fqn, obj); err != nil {
return nil, status.Errorf(codes.Internal, "error updating cloudFunction: %v", err)
return nil, err
}

return s.operations.NewLRO(ctx)
Expand All @@ -124,11 +116,7 @@ func (s *CloudFunctionsV1) DeleteFunction(ctx context.Context, req *pb.DeleteFun

oldObj := &pb.CloudFunction{}
if err := s.storage.Delete(ctx, fqn, oldObj); err != nil {
if apierrors.IsNotFound(err) {
return nil, status.Errorf(codes.NotFound, "function %q not found", name)
} else {
return nil, status.Errorf(codes.Internal, "error deleting function: %v", err)
}
return nil, err
}

return s.operations.NewLRO(ctx)
Expand Down
Loading

0 comments on commit 8d4ec8a

Please sign in to comment.