From 0cdf7ffa2080028bebed9bc6978486a27d70e62a Mon Sep 17 00:00:00 2001 From: justinsb Date: Wed, 7 Feb 2024 20:12:40 -0500 Subject: [PATCH 1/3] mockgcp: Refactor errors from storage.Get Going through one method at a time, but the code is much simplified by having storage.Get return an object that follows the GRPC design patterns, where codes.NotFound is returned when the object is not found. --- mockgcp/common/operations/operations.go | 8 +--- mockgcp/mockapikeys/key.go | 18 ++------ mockgcp/mockbilling/billingv1.go | 5 +-- mockgcp/mockcertificatemanager/v1.go | 44 ++++--------------- mockgcp/mockcloudfunctions/v1.go | 11 +---- mockgcp/mockcompute/networksv1.go | 11 +---- mockgcp/mockcompute/subnetsv1.go | 11 +---- mockgcp/mockedgecontainer/cluster.go | 6 +-- mockgcp/mockedgecontainer/nodepool.go | 6 +-- mockgcp/mockedgenetwork/network.go | 6 +-- mockgcp/mockedgenetwork/subnet.go | 6 +-- mockgcp/mockgkemulticloud/attachedcluster.go | 11 +---- mockgcp/mockiam/serviceaccounts.go | 11 +---- .../mocknetworkservices/networkservices.go | 11 +---- mockgcp/mockprivateca/capool.go | 11 +---- .../mockresourcemanager/projects_internal.go | 5 +-- mockgcp/mocksecretmanager/secrets.go | 6 +-- mockgcp/mocksecretmanager/secretversions.go | 8 +--- mockgcp/mockserviceusage/serviceusagev1.go | 13 +++--- mockgcp/pkg/storage/interfaces.go | 5 ++- mockgcp/pkg/storage/memory.go | 14 ++++-- 21 files changed, 57 insertions(+), 170 deletions(-) diff --git a/mockgcp/common/operations/operations.go b/mockgcp/common/operations/operations.go index 2c1f22f265..849d324b8d 100644 --- a/mockgcp/common/operations/operations.go +++ b/mockgcp/common/operations/operations.go @@ -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" @@ -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 diff --git a/mockgcp/mockapikeys/key.go b/mockgcp/mockapikeys/key.go index 1b93534be4..31b840e74b 100644 --- a/mockgcp/mockapikeys/key.go +++ b/mockgcp/mockapikeys/key.go @@ -44,11 +44,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 @@ -64,11 +60,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" @@ -112,11 +104,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: diff --git a/mockgcp/mockbilling/billingv1.go b/mockgcp/mockbilling/billingv1.go index 4120cf77f8..bfb0064f8d 100644 --- a/mockgcp/mockbilling/billingv1.go +++ b/mockgcp/mockbilling/billingv1.go @@ -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" @@ -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 } } diff --git a/mockgcp/mockcertificatemanager/v1.go b/mockgcp/mockcertificatemanager/v1.go index ecd4c6de0d..65388ef34e 100644 --- a/mockgcp/mockcertificatemanager/v1.go +++ b/mockgcp/mockcertificatemanager/v1.go @@ -42,11 +42,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 @@ -82,10 +78,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. @@ -143,11 +136,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 @@ -183,10 +172,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. @@ -243,11 +229,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 @@ -283,10 +265,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. @@ -344,11 +323,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 @@ -384,10 +359,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. diff --git a/mockgcp/mockcloudfunctions/v1.go b/mockgcp/mockcloudfunctions/v1.go index a8b0963b60..4c5eb8f05c 100644 --- a/mockgcp/mockcloudfunctions/v1.go +++ b/mockgcp/mockcloudfunctions/v1.go @@ -42,11 +42,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 @@ -81,10 +77,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. diff --git a/mockgcp/mockcompute/networksv1.go b/mockgcp/mockcompute/networksv1.go index 4b0b04f0ca..d392a2ab92 100644 --- a/mockgcp/mockcompute/networksv1.go +++ b/mockgcp/mockcompute/networksv1.go @@ -43,11 +43,7 @@ func (s *NetworksV1) Get(ctx context.Context, req *pb.GetNetworkRequest) (*pb.Ne obj := &pb.Network{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "network %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error reading network: %v", err) - } + return nil, err } return obj, nil @@ -88,10 +84,7 @@ func (s *NetworksV1) Patch(ctx context.Context, req *pb.PatchNetworkRequest) (*p fqn := name.String() obj := &pb.Network{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "network %q not found", fqn) - } - return nil, status.Errorf(codes.Internal, "error reading network: %v", err) + return nil, err } if req.GetNetworkResource().RoutingConfig != nil { diff --git a/mockgcp/mockcompute/subnetsv1.go b/mockgcp/mockcompute/subnetsv1.go index c965f8de1f..598d653335 100644 --- a/mockgcp/mockcompute/subnetsv1.go +++ b/mockgcp/mockcompute/subnetsv1.go @@ -42,11 +42,7 @@ func (s *SubnetsV1) Get(ctx context.Context, req *pb.GetSubnetworkRequest) (*pb. obj := &pb.Subnetwork{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "subnet %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error reading subnet: %v", err) - } + return nil, err } return obj, nil @@ -104,10 +100,7 @@ func (s *SubnetsV1) SetPrivateIpGoogleAccess(ctx context.Context, req *pb.SetPri fqn := name.String() obj := &pb.Subnetwork{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "subnet %q not found", fqn) - } - return nil, status.Errorf(codes.Internal, "error reading subnet: %v", err) + return nil, err } obj.PrivateIpGoogleAccess = PtrTo(req.GetSubnetworksSetPrivateIpGoogleAccessRequestResource().GetPrivateIpGoogleAccess()) diff --git a/mockgcp/mockedgecontainer/cluster.go b/mockgcp/mockedgecontainer/cluster.go index d3b541e84d..cf94187f38 100644 --- a/mockgcp/mockedgecontainer/cluster.go +++ b/mockgcp/mockedgecontainer/cluster.go @@ -35,11 +35,7 @@ func (s *EdgeContainerV1) GetCluster(ctx context.Context, req *pb.GetClusterRequ fqn := name.String() obj := &pb.Cluster{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "cluster %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error reading cluster: %v", err) - } + return nil, err } return obj, nil diff --git a/mockgcp/mockedgecontainer/nodepool.go b/mockgcp/mockedgecontainer/nodepool.go index 44566a01d7..a550a0cb1e 100644 --- a/mockgcp/mockedgecontainer/nodepool.go +++ b/mockgcp/mockedgecontainer/nodepool.go @@ -35,11 +35,7 @@ func (s *EdgeContainerV1) GetNodePool(ctx context.Context, req *pb.GetNodePoolRe fqn := name.String() obj := &pb.NodePool{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "nodePool %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error reading nodePool: %v", err) - } + return nil, err } return obj, nil diff --git a/mockgcp/mockedgenetwork/network.go b/mockgcp/mockedgenetwork/network.go index 86002cb646..218c169e12 100644 --- a/mockgcp/mockedgenetwork/network.go +++ b/mockgcp/mockedgenetwork/network.go @@ -36,11 +36,7 @@ func (s *EdgenetworkV1) GetNetwork(ctx context.Context, req *pb.GetNetworkReques fqn := name.String() obj := &pb.Network{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "network %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error reading network: %v", err) - } + return nil, err } return obj, nil diff --git a/mockgcp/mockedgenetwork/subnet.go b/mockgcp/mockedgenetwork/subnet.go index 0d5b274354..e8d35cba64 100644 --- a/mockgcp/mockedgenetwork/subnet.go +++ b/mockgcp/mockedgenetwork/subnet.go @@ -35,11 +35,7 @@ func (s *EdgenetworkV1) GetSubnet(ctx context.Context, req *pb.GetSubnetRequest) fqn := name.String() obj := &pb.Subnet{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "subnet %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error reading subnet: %v", err) - } + return nil, err } return obj, nil diff --git a/mockgcp/mockgkemulticloud/attachedcluster.go b/mockgcp/mockgkemulticloud/attachedcluster.go index 591957f67b..f1fa2d566c 100644 --- a/mockgcp/mockgkemulticloud/attachedcluster.go +++ b/mockgcp/mockgkemulticloud/attachedcluster.go @@ -41,11 +41,7 @@ func (s *GKEMulticloudV1) GetAttachedCluster(ctx context.Context, req *pb.GetAtt obj := &pb.AttachedCluster{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "attachedCluster %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error reading attachedCluster: %v", err) - } + return nil, err } return obj, nil @@ -79,10 +75,7 @@ func (s *GKEMulticloudV1) UpdateAttachedCluster(ctx context.Context, req *pb.Upd fqn := name.String() obj := &pb.AttachedCluster{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "attachedCluster %q not found", fqn) - } - return nil, status.Errorf(codes.Internal, "error reading attachedCluster: %v", err) + return nil, err } // Mask of fields to update. At least one path must be supplied in // this field. The elements of the repeated paths field can only include these diff --git a/mockgcp/mockiam/serviceaccounts.go b/mockgcp/mockiam/serviceaccounts.go index 262f64fd56..362af7eea4 100644 --- a/mockgcp/mockiam/serviceaccounts.go +++ b/mockgcp/mockiam/serviceaccounts.go @@ -25,7 +25,6 @@ import ( "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/emptypb" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/klog/v2" "github.com/GoogleCloudPlatform/k8s-config-connector/mockgcp/common/projects" @@ -70,10 +69,7 @@ func (s *ServerV1) GetServiceAccount(ctx context.Context, req *pb.GetServiceAcco sa := &pb.ServiceAccount{} fqn := name.String() if err := s.storage.Get(ctx, fqn, sa); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "Service account %q not found", req.Name) - } - return nil, status.Errorf(codes.Internal, "error reading serviceaccount: %v", err) + return nil, err } return sa, nil @@ -159,10 +155,7 @@ func (s *ServerV1) PatchServiceAccount(ctx context.Context, req *pb.PatchService fqn := name.String() sa := &pb.ServiceAccount{} if err := s.storage.Get(ctx, fqn, sa); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "serviceaccount %q not found", reqName) - } - return nil, status.Errorf(codes.Internal, "error reading serviceaccount: %v", err) + return nil, err } // You can patch only the `display_name` and `description` fields. diff --git a/mockgcp/mocknetworkservices/networkservices.go b/mockgcp/mocknetworkservices/networkservices.go index c1e3355646..2b5acced3d 100644 --- a/mockgcp/mocknetworkservices/networkservices.go +++ b/mockgcp/mocknetworkservices/networkservices.go @@ -45,11 +45,7 @@ func (s *NetworkServicesServer) GetMesh(ctx context.Context, req *pb.GetMeshRequ obj := &pb.Mesh{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "mesh %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error reading mesh: %v", err) - } + return nil, err } return obj, nil @@ -84,10 +80,7 @@ func (s *NetworkServicesServer) UpdateMesh(ctx context.Context, req *pb.UpdateMe fqn := name.String() obj := &pb.Mesh{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "mesh %q not found", reqName) - } - return nil, status.Errorf(codes.Internal, "error reading mesh: %v", err) + return nil, err } // Field mask is used to specify the fields to be overwritten in the diff --git a/mockgcp/mockprivateca/capool.go b/mockgcp/mockprivateca/capool.go index 5119ab80f3..73292cc5f4 100644 --- a/mockgcp/mockprivateca/capool.go +++ b/mockgcp/mockprivateca/capool.go @@ -41,11 +41,7 @@ func (s *PrivateCAV1) GetCaPool(ctx context.Context, req *pb.GetCaPoolRequest) ( obj := &pb.CaPool{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "caPool %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error reading caPool: %v", err) - } + return nil, err } return obj, nil @@ -81,10 +77,7 @@ func (s *PrivateCAV1) UpdateCaPool(ctx context.Context, req *pb.UpdateCaPoolRequ fqn := name.String() obj := &pb.CaPool{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "caPool %q not found", reqName) - } - return nil, status.Errorf(codes.Internal, "error reading caPool: %v", err) + return nil, err } // Required. A list of fields to be updated in this request. diff --git a/mockgcp/mockresourcemanager/projects_internal.go b/mockgcp/mockresourcemanager/projects_internal.go index 3f0da265f2..1718d497f0 100644 --- a/mockgcp/mockresourcemanager/projects_internal.go +++ b/mockgcp/mockresourcemanager/projects_internal.go @@ -26,7 +26,6 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" - apierrors "k8s.io/apimachinery/pkg/api/errors" ) type ProjectsInternal struct { @@ -109,10 +108,10 @@ func (s *ProjectsInternal) tryGetProjectByID(ctx context.Context, projectID stri obj := &pb.Project{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { + if status.Code(err) == codes.NotFound { return nil, nil } else { - return nil, status.Errorf(codes.Internal, "error reading project: %v", err) + return nil, err } } diff --git a/mockgcp/mocksecretmanager/secrets.go b/mockgcp/mocksecretmanager/secrets.go index 900b2a02a6..5b041d257f 100644 --- a/mockgcp/mocksecretmanager/secrets.go +++ b/mockgcp/mocksecretmanager/secrets.go @@ -88,11 +88,7 @@ func (s *SecretsV1) GetSecret(ctx context.Context, req *pb.GetSecretRequest) (*p var secret pb.Secret fqn := name.String() if err := s.storage.Get(ctx, fqn, &secret); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "secret %q not found", req.Name) - - } - return nil, status.Errorf(codes.Internal, "error reading secret: %v", err) + return nil, err } return &secret, nil diff --git a/mockgcp/mocksecretmanager/secretversions.go b/mockgcp/mocksecretmanager/secretversions.go index c632b78c2c..fdd62e5b18 100644 --- a/mockgcp/mocksecretmanager/secretversions.go +++ b/mockgcp/mocksecretmanager/secretversions.go @@ -45,11 +45,7 @@ func (s *SecretsV1) AddSecretVersion(ctx context.Context, req *pb.AddSecretVersi var secret pb.Secret if err := s.storage.Get(ctx, secretName.String(), &secret); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "secret %q not found", req.Parent) - - } - return nil, status.Errorf(codes.Internal, "error reading secret: %v", err) + return nil, err } secretVersionKind := (&pb.SecretVersion{}).ProtoReflect().Descriptor() @@ -168,7 +164,7 @@ func (s *MockService) getSecretVersion(ctx context.Context, name *secretVersionN if err := s.storage.Get(ctx, fqn, secretVersionObj); err != nil { // TODO: Delete secret data? // TODO: Owner ref? - return nil, status.Errorf(codes.Internal, "error creating secret version: %v", err) + return nil, err } return secretVersionObj, nil } diff --git a/mockgcp/mockserviceusage/serviceusagev1.go b/mockgcp/mockserviceusage/serviceusagev1.go index 4032a7223c..f59410a6c5 100644 --- a/mockgcp/mockserviceusage/serviceusagev1.go +++ b/mockgcp/mockserviceusage/serviceusagev1.go @@ -23,7 +23,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" "github.com/GoogleCloudPlatform/k8s-config-connector/mockgcp/common/projects" @@ -54,10 +53,10 @@ func (s *ServiceUsageV1) EnableService(ctx context.Context, req *pb.EnableServic create := false service := &pb.Service{} if err := s.storage.Get(ctx, fqn, service); err != nil { - if apierrors.IsNotFound(err) { + if status.Code(err) == codes.NotFound { create = true } else { - return nil, status.Errorf(codes.Internal, "error reading service: %v", err) + return nil, err } } @@ -99,10 +98,10 @@ func (s *ServiceUsageV1) DisableService(ctx context.Context, req *pb.DisableServ exists := true service := &pb.Service{} if err := s.storage.Get(ctx, fqn, service); err != nil { - if apierrors.IsNotFound(err) { + if status.Code(err) == codes.NotFound { exists = false } else { - return nil, status.Errorf(codes.Internal, "error reading service: %v", err) + return nil, err } } @@ -128,14 +127,14 @@ func (s *ServiceUsageV1) GetService(ctx context.Context, req *pb.GetServiceReque obj := &pb.Service{} if err := s.storage.Get(ctx, fqn, obj); err != nil { - if apierrors.IsNotFound(err) { + if status.Code(err) == codes.NotFound { // Mock: everything is enabled obj = &pb.Service{ Name: serviceName.String(), State: pb.State_DISABLED, } } else { - return nil, status.Errorf(codes.Internal, "error reading service: %v", err) + return nil, err } } diff --git a/mockgcp/pkg/storage/interfaces.go b/mockgcp/pkg/storage/interfaces.go index 15adf8538e..cf7cf38309 100644 --- a/mockgcp/pkg/storage/interfaces.go +++ b/mockgcp/pkg/storage/interfaces.go @@ -26,8 +26,11 @@ type Storage interface { Create(ctx context.Context, fqn string, create proto.Message) error // Update stores a new version of an object, erroring if it does not already exist Update(ctx context.Context, fqn string, update proto.Message) error - // Get returns an existing object + + // Get returns an existing object. + // The error is "ready to return"; we return codes.NotFound if not found. Get(ctx context.Context, fqn string, dest proto.Message) error + // List returns all matching objects List(ctx context.Context, kind protoreflect.Descriptor, options ListOptions, callback func(obj proto.Message) error) error // Deleting deletes the object, returning a not found error if it does not exist. diff --git a/mockgcp/pkg/storage/memory.go b/mockgcp/pkg/storage/memory.go index 9e0d9ad423..c64e2825e3 100644 --- a/mockgcp/pkg/storage/memory.go +++ b/mockgcp/pkg/storage/memory.go @@ -19,6 +19,8 @@ import ( "strings" "sync" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -33,8 +35,9 @@ type InMemoryStorage struct { // typeStorage stores objects of a given type type typeStorage struct { - mutex sync.Mutex - byKey map[string]proto.Message + mutex sync.Mutex + objectTypeName string + byKey map[string]proto.Message } var _ Storage = &InMemoryStorage{} @@ -52,8 +55,11 @@ func (s *InMemoryStorage) getTypeStorage(name protoreflect.FullName) *typeStorag ts := s.byType[name] if ts == nil { + objectTypeName := string(name.Name()) + objectTypeName = strings.ToLower(string(objectTypeName[0])) + objectTypeName[1:] ts = &typeStorage{ - byKey: make(map[string]protoreflect.ProtoMessage), + objectTypeName: objectTypeName, + byKey: make(map[string]protoreflect.ProtoMessage), } s.byType[name] = ts } @@ -125,7 +131,7 @@ func (s *typeStorage) Get(ctx context.Context, fqn string, dest proto.Message) e existing, found := s.byKey[fqn] if !found { - return apierrors.NewNotFound(schema.GroupResource{}, fqn) + return status.Errorf(codes.NotFound, "%v %q not found", s.objectTypeName, fqn) } proto.Merge(dest, existing) return nil From 8fde5bc4cec23dc2cabea722b82c83da09c5c69c Mon Sep 17 00:00:00 2001 From: justinsb Date: Wed, 7 Feb 2024 20:23:24 -0500 Subject: [PATCH 2/3] mockgcp: Refactor errors from storage.Update storage.Update now returns a suitable error for returning to the caller. --- mockgcp/mockapikeys/key.go | 2 +- mockgcp/mockcertificatemanager/v1.go | 8 ++++---- mockgcp/mockcloudfunctions/v1.go | 2 +- mockgcp/mockcompute/networksv1.go | 2 +- mockgcp/mockcompute/subnetsv1.go | 2 +- mockgcp/mockgkemulticloud/attachedcluster.go | 2 +- mockgcp/mockiam/serviceaccounts.go | 2 +- mockgcp/mocknetworkservices/networkservices.go | 2 +- mockgcp/mockprivateca/capool.go | 2 +- mockgcp/mockresourcemanager/projects_internal.go | 2 +- mockgcp/mocksecretmanager/secretversions.go | 6 +++--- mockgcp/mockserviceusage/serviceusagev1.go | 4 ++-- mockgcp/pkg/storage/memory.go | 2 +- 13 files changed, 19 insertions(+), 19 deletions(-) diff --git a/mockgcp/mockapikeys/key.go b/mockgcp/mockapikeys/key.go index 31b840e74b..2aee2e947b 100644 --- a/mockgcp/mockapikeys/key.go +++ b/mockgcp/mockapikeys/key.go @@ -151,7 +151,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) diff --git a/mockgcp/mockcertificatemanager/v1.go b/mockgcp/mockcertificatemanager/v1.go index 65388ef34e..f34495abc2 100644 --- a/mockgcp/mockcertificatemanager/v1.go +++ b/mockgcp/mockcertificatemanager/v1.go @@ -100,7 +100,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) @@ -193,7 +193,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) @@ -287,7 +287,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) @@ -380,7 +380,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) diff --git a/mockgcp/mockcloudfunctions/v1.go b/mockgcp/mockcloudfunctions/v1.go index 4c5eb8f05c..cfdbfe8540 100644 --- a/mockgcp/mockcloudfunctions/v1.go +++ b/mockgcp/mockcloudfunctions/v1.go @@ -101,7 +101,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) diff --git a/mockgcp/mockcompute/networksv1.go b/mockgcp/mockcompute/networksv1.go index d392a2ab92..9986a70200 100644 --- a/mockgcp/mockcompute/networksv1.go +++ b/mockgcp/mockcompute/networksv1.go @@ -97,7 +97,7 @@ func (s *NetworksV1) Patch(ctx context.Context, req *pb.PatchNetworkRequest) (*p } if err := s.storage.Update(ctx, fqn, obj); err != nil { - return nil, status.Errorf(codes.Internal, "error updating network: %v", err) + return nil, err } return s.newLRO(ctx, name.Project.ID) diff --git a/mockgcp/mockcompute/subnetsv1.go b/mockgcp/mockcompute/subnetsv1.go index 598d653335..1438d31bbb 100644 --- a/mockgcp/mockcompute/subnetsv1.go +++ b/mockgcp/mockcompute/subnetsv1.go @@ -106,7 +106,7 @@ func (s *SubnetsV1) SetPrivateIpGoogleAccess(ctx context.Context, req *pb.SetPri obj.PrivateIpGoogleAccess = PtrTo(req.GetSubnetworksSetPrivateIpGoogleAccessRequestResource().GetPrivateIpGoogleAccess()) if err := s.storage.Update(ctx, fqn, obj); err != nil { - return nil, status.Errorf(codes.Internal, "error updating subnet: %v", err) + return nil, err } return s.newLRO(ctx, name.Project.ID) diff --git a/mockgcp/mockgkemulticloud/attachedcluster.go b/mockgcp/mockgkemulticloud/attachedcluster.go index f1fa2d566c..40b8ccd311 100644 --- a/mockgcp/mockgkemulticloud/attachedcluster.go +++ b/mockgcp/mockgkemulticloud/attachedcluster.go @@ -105,7 +105,7 @@ func (s *GKEMulticloudV1) UpdateAttachedCluster(ctx context.Context, req *pb.Upd } if err := s.storage.Update(ctx, fqn, obj); err != nil { - return nil, status.Errorf(codes.Internal, "error updating attachedCluster: %v", err) + return nil, err } return s.operations.NewLRO(ctx) } diff --git a/mockgcp/mockiam/serviceaccounts.go b/mockgcp/mockiam/serviceaccounts.go index 362af7eea4..a2329a7ada 100644 --- a/mockgcp/mockiam/serviceaccounts.go +++ b/mockgcp/mockiam/serviceaccounts.go @@ -173,7 +173,7 @@ func (s *ServerV1) PatchServiceAccount(ctx context.Context, req *pb.PatchService } if err := s.storage.Update(ctx, fqn, sa); err != nil { - return nil, status.Errorf(codes.Internal, "error updating serviceaccount: %v", err) + return nil, err } return sa, nil } diff --git a/mockgcp/mocknetworkservices/networkservices.go b/mockgcp/mocknetworkservices/networkservices.go index 2b5acced3d..3912391eeb 100644 --- a/mockgcp/mocknetworkservices/networkservices.go +++ b/mockgcp/mocknetworkservices/networkservices.go @@ -104,7 +104,7 @@ func (s *NetworkServicesServer) UpdateMesh(ctx context.Context, req *pb.UpdateMe } if err := s.storage.Update(ctx, fqn, obj); err != nil { - return nil, status.Errorf(codes.Internal, "error updating mesh: %v", err) + return nil, err } return s.operations.NewLRO(ctx) } diff --git a/mockgcp/mockprivateca/capool.go b/mockgcp/mockprivateca/capool.go index 73292cc5f4..8eba0f3aa3 100644 --- a/mockgcp/mockprivateca/capool.go +++ b/mockgcp/mockprivateca/capool.go @@ -98,7 +98,7 @@ func (s *PrivateCAV1) UpdateCaPool(ctx context.Context, req *pb.UpdateCaPoolRequ } if err := s.storage.Update(ctx, fqn, obj); err != nil { - return nil, status.Errorf(codes.Internal, "error updating caPool: %v", err) + return nil, err } return s.operations.NewLRO(ctx) diff --git a/mockgcp/mockresourcemanager/projects_internal.go b/mockgcp/mockresourcemanager/projects_internal.go index 1718d497f0..adfbacdc6f 100644 --- a/mockgcp/mockresourcemanager/projects_internal.go +++ b/mockgcp/mockresourcemanager/projects_internal.go @@ -218,7 +218,7 @@ func (s *ProjectsInternal) mutateProject(ctx context.Context, name string, mutat } if err := s.storage.Update(ctx, fqn, obj); err != nil { - return nil, status.Errorf(codes.Internal, "error updating project: %v", err) + return nil, err } return obj, nil diff --git a/mockgcp/mocksecretmanager/secretversions.go b/mockgcp/mocksecretmanager/secretversions.go index fdd62e5b18..0eccb88e7c 100644 --- a/mockgcp/mocksecretmanager/secretversions.go +++ b/mockgcp/mocksecretmanager/secretversions.go @@ -243,7 +243,7 @@ func (s *SecretsV1) EnableSecretVersion(ctx context.Context, req *pb.EnableSecre secretVersion.State = pb.SecretVersion_ENABLED fqn := secretVersion.Name if err := s.storage.Update(ctx, fqn, secretVersion); err != nil { - return nil, status.Errorf(codes.Internal, "error updating secret version: %v", err) + return nil, err } return secretVersion, nil @@ -263,7 +263,7 @@ func (s *SecretsV1) DisableSecretVersion(ctx context.Context, req *pb.DisableSec secretVersion.State = pb.SecretVersion_DISABLED fqn := secretVersion.Name if err := s.storage.Update(ctx, fqn, secretVersion); err != nil { - return nil, status.Errorf(codes.Internal, "error updating secret version: %v", err) + return nil, err } return secretVersion, nil @@ -291,7 +291,7 @@ func (s *SecretsV1) DestroySecretVersion(ctx context.Context, req *pb.DestroySec secretVersion.State = pb.SecretVersion_DESTROYED fqn := secretVersion.Name if err := s.storage.Update(ctx, fqn, secretVersion); err != nil { - return nil, status.Errorf(codes.Internal, "error updating secret version: %v", err) + return nil, err } return secretVersion, nil diff --git a/mockgcp/mockserviceusage/serviceusagev1.go b/mockgcp/mockserviceusage/serviceusagev1.go index f59410a6c5..b3d58f0207 100644 --- a/mockgcp/mockserviceusage/serviceusagev1.go +++ b/mockgcp/mockserviceusage/serviceusagev1.go @@ -76,7 +76,7 @@ func (s *ServiceUsageV1) EnableService(ctx context.Context, req *pb.EnableServic service.State = pb.State_ENABLED if err := s.storage.Update(ctx, fqn, service); err != nil { - return nil, status.Errorf(codes.Internal, "error updating service: %v", err) + return nil, err } return s.operations.NewLRO(ctx) @@ -110,7 +110,7 @@ func (s *ServiceUsageV1) DisableService(ctx context.Context, req *pb.DisableServ } else { service.State = pb.State_DISABLED if err := s.storage.Update(ctx, fqn, service); err != nil { - return nil, status.Errorf(codes.Internal, "error updating service: %v", err) + return nil, err } } diff --git a/mockgcp/pkg/storage/memory.go b/mockgcp/pkg/storage/memory.go index c64e2825e3..9cdabd1114 100644 --- a/mockgcp/pkg/storage/memory.go +++ b/mockgcp/pkg/storage/memory.go @@ -114,7 +114,7 @@ func (s *typeStorage) Update(ctx context.Context, fqn string, update proto.Messa _, found := s.byKey[fqn] if !found { - return apierrors.NewNotFound(schema.GroupResource{}, fqn) + return status.Errorf(codes.NotFound, "%v %q not found", s.objectTypeName, fqn) } s.byKey[fqn] = proto.Clone(update) return nil From 59fd880d9065191c9dac6fe0a7729a23ae5da81d Mon Sep 17 00:00:00 2001 From: justinsb Date: Wed, 7 Feb 2024 20:27:22 -0500 Subject: [PATCH 3/3] mockgcp: Refactor errors from storage.Delete storage.Delete now returns a suitable error for returning to the caller. --- mockgcp/mockapikeys/key.go | 7 +----- mockgcp/mockcertificatemanager/v1.go | 25 +++---------------- mockgcp/mockcloudfunctions/v1.go | 7 +----- mockgcp/mockcompute/networksv1.go | 7 +----- mockgcp/mockcompute/subnetsv1.go | 7 +----- mockgcp/mockedgecontainer/cluster.go | 7 +----- mockgcp/mockedgecontainer/nodepool.go | 7 +----- mockgcp/mockedgenetwork/network.go | 7 +----- mockgcp/mockedgenetwork/subnet.go | 7 +----- mockgcp/mockgkemulticloud/attachedcluster.go | 7 +----- mockgcp/mockiam/serviceaccounts.go | 2 +- .../mocknetworkservices/networkservices.go | 7 +----- mockgcp/mockprivateca/capool.go | 7 +----- mockgcp/mocksecretmanager/secrets.go | 7 +----- mockgcp/pkg/storage/interfaces.go | 3 +++ mockgcp/pkg/storage/memory.go | 2 +- 16 files changed, 21 insertions(+), 95 deletions(-) diff --git a/mockgcp/mockapikeys/key.go b/mockgcp/mockapikeys/key.go index 2aee2e947b..2773fc1bfa 100644 --- a/mockgcp/mockapikeys/key.go +++ b/mockgcp/mockapikeys/key.go @@ -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" ) @@ -167,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) diff --git a/mockgcp/mockcertificatemanager/v1.go b/mockgcp/mockcertificatemanager/v1.go index f34495abc2..f25315cffb 100644 --- a/mockgcp/mockcertificatemanager/v1.go +++ b/mockgcp/mockcertificatemanager/v1.go @@ -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" @@ -116,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) @@ -209,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) @@ -303,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) @@ -396,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) diff --git a/mockgcp/mockcloudfunctions/v1.go b/mockgcp/mockcloudfunctions/v1.go index cfdbfe8540..b2a1652f32 100644 --- a/mockgcp/mockcloudfunctions/v1.go +++ b/mockgcp/mockcloudfunctions/v1.go @@ -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" @@ -117,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) diff --git a/mockgcp/mockcompute/networksv1.go b/mockgcp/mockcompute/networksv1.go index 9986a70200..b29e2417d9 100644 --- a/mockgcp/mockcompute/networksv1.go +++ b/mockgcp/mockcompute/networksv1.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" @@ -113,11 +112,7 @@ func (s *NetworksV1) Delete(ctx context.Context, req *pb.DeleteNetworkRequest) ( deleted := &pb.Network{} if err := s.storage.Delete(ctx, fqn, deleted); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "network %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error deleting network: %v", err) - } + return nil, err } return s.newLRO(ctx, name.Project.ID) diff --git a/mockgcp/mockcompute/subnetsv1.go b/mockgcp/mockcompute/subnetsv1.go index 1438d31bbb..629f20d8c7 100644 --- a/mockgcp/mockcompute/subnetsv1.go +++ b/mockgcp/mockcompute/subnetsv1.go @@ -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/compute/v1" @@ -81,11 +80,7 @@ func (s *SubnetsV1) Delete(ctx context.Context, req *pb.DeleteSubnetworkRequest) deleted := &pb.Subnetwork{} if err := s.storage.Delete(ctx, fqn, deleted); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "subnet %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error deleting subnet: %v", err) - } + return nil, err } return s.newLRO(ctx, name.Project.ID) diff --git a/mockgcp/mockedgecontainer/cluster.go b/mockgcp/mockedgecontainer/cluster.go index cf94187f38..2af0f4a5c9 100644 --- a/mockgcp/mockedgecontainer/cluster.go +++ b/mockgcp/mockedgecontainer/cluster.go @@ -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" pb "github.com/GoogleCloudPlatform/k8s-config-connector/mockgcp/generated/mockgcp/cloud/edgecontainer/v1" ) @@ -70,11 +69,7 @@ func (s *EdgeContainerV1) DeleteCluster(ctx context.Context, req *pb.DeleteClust deletedObj := &pb.Cluster{} if err := s.storage.Delete(ctx, fqn, deletedObj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "cluster %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error deleting cluster: %v", err) - } + return nil, err } return s.operations.NewLRO(ctx) diff --git a/mockgcp/mockedgecontainer/nodepool.go b/mockgcp/mockedgecontainer/nodepool.go index a550a0cb1e..c5f1af3366 100644 --- a/mockgcp/mockedgecontainer/nodepool.go +++ b/mockgcp/mockedgecontainer/nodepool.go @@ -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" pb "github.com/GoogleCloudPlatform/k8s-config-connector/mockgcp/generated/mockgcp/cloud/edgecontainer/v1" ) @@ -70,11 +69,7 @@ func (s *EdgeContainerV1) DeleteNodePool(ctx context.Context, req *pb.DeleteNode deletedObj := &pb.NodePool{} if err := s.storage.Delete(ctx, fqn, deletedObj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "nodePool %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error deleting nodePool: %v", err) - } + return nil, err } return s.operations.NewLRO(ctx) diff --git a/mockgcp/mockedgenetwork/network.go b/mockgcp/mockedgenetwork/network.go index 218c169e12..3f16be0bd0 100644 --- a/mockgcp/mockedgenetwork/network.go +++ b/mockgcp/mockedgenetwork/network.go @@ -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" pb "github.com/GoogleCloudPlatform/k8s-config-connector/mockgcp/generated/mockgcp/cloud/edgenetwork/v1" "github.com/GoogleCloudPlatform/k8s-config-connector/mockgcp/pkg/storage" @@ -88,11 +87,7 @@ func (s *EdgenetworkV1) DeleteNetwork(ctx context.Context, req *pb.DeleteNetwork deletedObj := &pb.Network{} if err := s.storage.Delete(ctx, fqn, deletedObj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "network %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error deleting network: %v", err) - } + return nil, err } return s.operations.NewLRO(ctx) diff --git a/mockgcp/mockedgenetwork/subnet.go b/mockgcp/mockedgenetwork/subnet.go index e8d35cba64..f6a877647d 100644 --- a/mockgcp/mockedgenetwork/subnet.go +++ b/mockgcp/mockedgenetwork/subnet.go @@ -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" pb "github.com/GoogleCloudPlatform/k8s-config-connector/mockgcp/generated/mockgcp/cloud/edgenetwork/v1" ) @@ -76,11 +75,7 @@ func (s *EdgenetworkV1) DeleteSubnet(ctx context.Context, req *pb.DeleteSubnetRe deletedObj := &pb.Subnet{} if err := s.storage.Delete(ctx, fqn, deletedObj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "subnet %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error deleting subnet: %v", err) - } + return nil, err } return s.operations.NewLRO(ctx) diff --git a/mockgcp/mockgkemulticloud/attachedcluster.go b/mockgcp/mockgkemulticloud/attachedcluster.go index 40b8ccd311..d9a7b96de6 100644 --- a/mockgcp/mockgkemulticloud/attachedcluster.go +++ b/mockgcp/mockgkemulticloud/attachedcluster.go @@ -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" pb "github.com/GoogleCloudPlatform/k8s-config-connector/mockgcp/generated/mockgcp/cloud/gkemulticloud/v1" ) @@ -120,11 +119,7 @@ func (s *GKEMulticloudV1) DeleteAttachedCluster(ctx context.Context, req *pb.Del oldObj := &pb.AttachedCluster{} if err := s.storage.Delete(ctx, fqn, oldObj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "attachedCluster %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error deleting attachedCluster: %v", err) - } + return nil, err } return s.operations.NewLRO(ctx) diff --git a/mockgcp/mockiam/serviceaccounts.go b/mockgcp/mockiam/serviceaccounts.go index a2329a7ada..44d2b066d2 100644 --- a/mockgcp/mockiam/serviceaccounts.go +++ b/mockgcp/mockiam/serviceaccounts.go @@ -138,7 +138,7 @@ func (s *ServerV1) DeleteServiceAccount(ctx context.Context, req *pb.DeleteServi fqn := name.String() deletedObj := &pb.ServiceAccount{} if err := s.storage.Delete(ctx, fqn, deletedObj); err != nil { - return nil, status.Errorf(codes.Internal, "error deleting serviceaccount: %v", err) + return nil, err } return &emptypb.Empty{}, nil diff --git a/mockgcp/mocknetworkservices/networkservices.go b/mockgcp/mocknetworkservices/networkservices.go index 3912391eeb..6f1fa88d87 100644 --- a/mockgcp/mocknetworkservices/networkservices.go +++ b/mockgcp/mocknetworkservices/networkservices.go @@ -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" pb "github.com/GoogleCloudPlatform/k8s-config-connector/mockgcp/generated/mockgcp/cloud/networkservices/v1" ) @@ -119,11 +118,7 @@ func (s *NetworkServicesServer) DeleteMesh(ctx context.Context, req *pb.DeleteMe deletedObj := &pb.Mesh{} if err := s.storage.Delete(ctx, fqn, deletedObj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "mesh %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error deleting mesh: %v", err) - } + return nil, err } return s.operations.NewLRO(ctx) diff --git a/mockgcp/mockprivateca/capool.go b/mockgcp/mockprivateca/capool.go index 8eba0f3aa3..a2d6a868f8 100644 --- a/mockgcp/mockprivateca/capool.go +++ b/mockgcp/mockprivateca/capool.go @@ -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" pb "github.com/GoogleCloudPlatform/k8s-config-connector/mockgcp/generated/mockgcp/cloud/security/privateca/v1" ) @@ -114,11 +113,7 @@ func (s *PrivateCAV1) DeleteCaPool(ctx context.Context, req *pb.DeleteCaPoolRequ oldObj := &pb.CaPool{} if err := s.storage.Delete(ctx, fqn, oldObj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "caPool %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error deleting caPool: %v", err) - } + return nil, err } return s.operations.NewLRO(ctx) diff --git a/mockgcp/mocksecretmanager/secrets.go b/mockgcp/mocksecretmanager/secrets.go index 5b041d257f..2fc7c575d2 100644 --- a/mockgcp/mocksecretmanager/secrets.go +++ b/mockgcp/mocksecretmanager/secrets.go @@ -24,7 +24,6 @@ import ( "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/emptypb" "google.golang.org/protobuf/types/known/timestamppb" - 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/secretmanager/v1" @@ -105,11 +104,7 @@ func (s *SecretsV1) DeleteSecret(ctx context.Context, req *pb.DeleteSecretReques oldObj := &pb.Secret{} if err := s.storage.Delete(ctx, fqn, oldObj); err != nil { - if apierrors.IsNotFound(err) { - return nil, status.Errorf(codes.NotFound, "secret %q not found", name) - } else { - return nil, status.Errorf(codes.Internal, "error deleting secret: %v", err) - } + return nil, err } // TODO: Delete secret versions? diff --git a/mockgcp/pkg/storage/interfaces.go b/mockgcp/pkg/storage/interfaces.go index cf7cf38309..37db950503 100644 --- a/mockgcp/pkg/storage/interfaces.go +++ b/mockgcp/pkg/storage/interfaces.go @@ -24,6 +24,7 @@ import ( type Storage interface { // Create stores the object, erroring if it already exists Create(ctx context.Context, fqn string, create proto.Message) error + // Update stores a new version of an object, erroring if it does not already exist Update(ctx context.Context, fqn string, update proto.Message) error @@ -33,7 +34,9 @@ type Storage interface { // List returns all matching objects List(ctx context.Context, kind protoreflect.Descriptor, options ListOptions, callback func(obj proto.Message) error) error + // Deleting deletes the object, returning a not found error if it does not exist. + // The error is "ready to return", e.g. we return codes.NotFound if not found. Delete(ctx context.Context, fqn string, dest proto.Message) error } diff --git a/mockgcp/pkg/storage/memory.go b/mockgcp/pkg/storage/memory.go index 9cdabd1114..45f8c80d8a 100644 --- a/mockgcp/pkg/storage/memory.go +++ b/mockgcp/pkg/storage/memory.go @@ -95,7 +95,7 @@ func (s *typeStorage) Delete(ctx context.Context, fqn string, dest proto.Message existing, found := s.byKey[fqn] if !found { - return apierrors.NewNotFound(schema.GroupResource{}, fqn) + return status.Errorf(codes.NotFound, "%v %q not found", s.objectTypeName, fqn) } proto.Merge(dest, existing) delete(s.byKey, fqn)