From 49f115191a6dc83a175eff4f6af999f9277d5bb5 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Wed, 3 Sep 2025 09:12:00 +0200 Subject: [PATCH 01/16] Introduces namespaces for memberships. --- api/v1/project_member.pb.go | 35 +++++++++++++++++++++++++++-------- api/v1/tenant_member.pb.go | 29 ++++++++++++++++++++++++----- client/main.go | 2 +- pkg/client/client.go | 18 +++++++++++++++++- pkg/service/projectmember.go | 18 +++++++++++++++++- pkg/service/tenantmember.go | 17 +++++++++++++++-- proto/v1/project_member.proto | 3 +++ proto/v1/tenant_member.proto | 3 +++ 8 files changed, 107 insertions(+), 18 deletions(-) diff --git a/api/v1/project_member.pb.go b/api/v1/project_member.pb.go index 1611dd7..556b3b3 100644 --- a/api/v1/project_member.pb.go +++ b/api/v1/project_member.pb.go @@ -23,10 +23,12 @@ const ( // ProjectMember is the database model type ProjectMember struct { - state protoimpl.MessageState `protogen:"open.v1"` - Meta *Meta `protobuf:"bytes,1,opt,name=meta,proto3" json:"meta,omitempty"` - ProjectId string `protobuf:"bytes,2,opt,name=project_id,json=projectId,proto3" json:"project_id,omitempty"` - TenantId string `protobuf:"bytes,4,opt,name=tenant_id,json=tenantId,proto3" json:"tenant_id,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + Meta *Meta `protobuf:"bytes,1,opt,name=meta,proto3" json:"meta,omitempty"` + ProjectId string `protobuf:"bytes,2,opt,name=project_id,json=projectId,proto3" json:"project_id,omitempty"` + TenantId string `protobuf:"bytes,4,opt,name=tenant_id,json=tenantId,proto3" json:"tenant_id,omitempty"` + // Namespace introduces the possibility to associate memberships for different applications that use the masterdata-api as a backend. + Namespace string `protobuf:"bytes,5,opt,name=namespace,proto3" json:"namespace,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -82,6 +84,13 @@ func (x *ProjectMember) GetTenantId() string { return "" } +func (x *ProjectMember) GetNamespace() string { + if x != nil { + return x.Namespace + } + return "" +} + type ProjectMemberCreateRequest struct { state protoimpl.MessageState `protogen:"open.v1"` ProjectMember *ProjectMember `protobuf:"bytes,1,opt,name=project_member,json=projectMember,proto3" json:"project_member,omitempty"` @@ -263,6 +272,7 @@ type ProjectMemberFindRequest struct { ProjectId *string `protobuf:"bytes,1,opt,name=project_id,json=projectId,proto3,oneof" json:"project_id,omitempty"` TenantId *string `protobuf:"bytes,2,opt,name=tenant_id,json=tenantId,proto3,oneof" json:"tenant_id,omitempty"` Annotations map[string]string `protobuf:"bytes,6,rep,name=annotations,proto3" json:"annotations,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"` + Namespace string `protobuf:"bytes,7,opt,name=namespace,proto3" json:"namespace,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -318,6 +328,13 @@ func (x *ProjectMemberFindRequest) GetAnnotations() map[string]string { return nil } +func (x *ProjectMemberFindRequest) GetNamespace() string { + if x != nil { + return x.Namespace + } + return "" +} + type ProjectMemberResponse struct { state protoimpl.MessageState `protogen:"open.v1"` ProjectMember *ProjectMember `protobuf:"bytes,1,opt,name=project_member,json=projectMember,proto3" json:"project_member,omitempty"` @@ -410,12 +427,13 @@ var File_v1_project_member_proto protoreflect.FileDescriptor const file_v1_project_member_proto_rawDesc = "" + "\n" + - "\x17v1/project_member.proto\x12\x02v1\x1a\rv1/meta.proto\"i\n" + + "\x17v1/project_member.proto\x12\x02v1\x1a\rv1/meta.proto\"\x87\x01\n" + "\rProjectMember\x12\x1c\n" + "\x04meta\x18\x01 \x01(\v2\b.v1.MetaR\x04meta\x12\x1d\n" + "\n" + "project_id\x18\x02 \x01(\tR\tprojectId\x12\x1b\n" + - "\ttenant_id\x18\x04 \x01(\tR\btenantId\"V\n" + + "\ttenant_id\x18\x04 \x01(\tR\btenantId\x12\x1c\n" + + "\tnamespace\x18\x05 \x01(\tR\tnamespace\"V\n" + "\x1aProjectMemberCreateRequest\x128\n" + "\x0eproject_member\x18\x01 \x01(\v2\x11.v1.ProjectMemberR\rprojectMember\"V\n" + "\x1aProjectMemberUpdateRequest\x128\n" + @@ -423,12 +441,13 @@ const file_v1_project_member_proto_rawDesc = "" + "\x1aProjectMemberDeleteRequest\x12\x0e\n" + "\x02id\x18\x01 \x01(\tR\x02id\")\n" + "\x17ProjectMemberGetRequest\x12\x0e\n" + - "\x02id\x18\x01 \x01(\tR\x02id\"\x8e\x02\n" + + "\x02id\x18\x01 \x01(\tR\x02id\"\xac\x02\n" + "\x18ProjectMemberFindRequest\x12\"\n" + "\n" + "project_id\x18\x01 \x01(\tH\x00R\tprojectId\x88\x01\x01\x12 \n" + "\ttenant_id\x18\x02 \x01(\tH\x01R\btenantId\x88\x01\x01\x12O\n" + - "\vannotations\x18\x06 \x03(\v2-.v1.ProjectMemberFindRequest.AnnotationsEntryR\vannotations\x1a>\n" + + "\vannotations\x18\x06 \x03(\v2-.v1.ProjectMemberFindRequest.AnnotationsEntryR\vannotations\x12\x1c\n" + + "\tnamespace\x18\a \x01(\tR\tnamespace\x1a>\n" + "\x10AnnotationsEntry\x12\x10\n" + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01B\r\n" + diff --git a/api/v1/tenant_member.pb.go b/api/v1/tenant_member.pb.go index fb27b51..bc6e8e7 100644 --- a/api/v1/tenant_member.pb.go +++ b/api/v1/tenant_member.pb.go @@ -28,7 +28,9 @@ type TenantMember struct { // TenantId is the id of the parent tenant TenantId string `protobuf:"bytes,2,opt,name=tenant_id,json=tenantId,proto3" json:"tenant_id,omitempty"` // MemberId is the id of the member tenant - MemberId string `protobuf:"bytes,3,opt,name=member_id,json=memberId,proto3" json:"member_id,omitempty"` + MemberId string `protobuf:"bytes,3,opt,name=member_id,json=memberId,proto3" json:"member_id,omitempty"` + // Namespace introduces the possibility to associate memberships for different applications that use the masterdata-api as a backend. + Namespace string `protobuf:"bytes,4,opt,name=namespace,proto3" json:"namespace,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -84,6 +86,13 @@ func (x *TenantMember) GetMemberId() string { return "" } +func (x *TenantMember) GetNamespace() string { + if x != nil { + return x.Namespace + } + return "" +} + type TenantMemberCreateRequest struct { state protoimpl.MessageState `protogen:"open.v1"` TenantMember *TenantMember `protobuf:"bytes,1,opt,name=tenant_member,json=tenantMember,proto3" json:"tenant_member,omitempty"` @@ -265,6 +274,7 @@ type TenantMemberFindRequest struct { TenantId *string `protobuf:"bytes,1,opt,name=tenant_id,json=tenantId,proto3,oneof" json:"tenant_id,omitempty"` MemberId *string `protobuf:"bytes,2,opt,name=member_id,json=memberId,proto3,oneof" json:"member_id,omitempty"` Annotations map[string]string `protobuf:"bytes,6,rep,name=annotations,proto3" json:"annotations,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"` + Namespace string `protobuf:"bytes,7,opt,name=namespace,proto3" json:"namespace,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -320,6 +330,13 @@ func (x *TenantMemberFindRequest) GetAnnotations() map[string]string { return nil } +func (x *TenantMemberFindRequest) GetNamespace() string { + if x != nil { + return x.Namespace + } + return "" +} + type TenantMemberResponse struct { state protoimpl.MessageState `protogen:"open.v1"` TenantMember *TenantMember `protobuf:"bytes,1,opt,name=tenant_member,json=tenantMember,proto3" json:"tenant_member,omitempty"` @@ -412,11 +429,12 @@ var File_v1_tenant_member_proto protoreflect.FileDescriptor const file_v1_tenant_member_proto_rawDesc = "" + "\n" + - "\x16v1/tenant_member.proto\x12\x02v1\x1a\rv1/meta.proto\"f\n" + + "\x16v1/tenant_member.proto\x12\x02v1\x1a\rv1/meta.proto\"\x84\x01\n" + "\fTenantMember\x12\x1c\n" + "\x04meta\x18\x01 \x01(\v2\b.v1.MetaR\x04meta\x12\x1b\n" + "\ttenant_id\x18\x02 \x01(\tR\btenantId\x12\x1b\n" + - "\tmember_id\x18\x03 \x01(\tR\bmemberId\"R\n" + + "\tmember_id\x18\x03 \x01(\tR\bmemberId\x12\x1c\n" + + "\tnamespace\x18\x04 \x01(\tR\tnamespace\"R\n" + "\x19TenantMemberCreateRequest\x125\n" + "\rtenant_member\x18\x01 \x01(\v2\x10.v1.TenantMemberR\ftenantMember\"R\n" + "\x19TenantMemberUpdateRequest\x125\n" + @@ -424,11 +442,12 @@ const file_v1_tenant_member_proto_rawDesc = "" + "\x19TenantMemberDeleteRequest\x12\x0e\n" + "\x02id\x18\x01 \x01(\tR\x02id\"(\n" + "\x16TenantMemberGetRequest\x12\x0e\n" + - "\x02id\x18\x01 \x01(\tR\x02id\"\x89\x02\n" + + "\x02id\x18\x01 \x01(\tR\x02id\"\xa7\x02\n" + "\x17TenantMemberFindRequest\x12 \n" + "\ttenant_id\x18\x01 \x01(\tH\x00R\btenantId\x88\x01\x01\x12 \n" + "\tmember_id\x18\x02 \x01(\tH\x01R\bmemberId\x88\x01\x01\x12N\n" + - "\vannotations\x18\x06 \x03(\v2,.v1.TenantMemberFindRequest.AnnotationsEntryR\vannotations\x1a>\n" + + "\vannotations\x18\x06 \x03(\v2,.v1.TenantMemberFindRequest.AnnotationsEntryR\vannotations\x12\x1c\n" + + "\tnamespace\x18\a \x01(\tR\tnamespace\x1a>\n" + "\x10AnnotationsEntry\x12\x10\n" + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01B\f\n" + diff --git a/client/main.go b/client/main.go index 20f252d..b9cbd47 100644 --- a/client/main.go +++ b/client/main.go @@ -31,7 +31,7 @@ func main() { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - c, err := client.NewClient(ctx, "localhost", 50051, "certs/client.pem", "certs/client-key.pem", "certs/ca.pem", hmacKey, true, logger) + c, err := client.NewClient(ctx, "localhost", 50051, "certs/client.pem", "certs/client-key.pem", "certs/ca.pem", hmacKey, true, logger, "test") if err != nil { logger.Error(err.Error()) panic(err) diff --git a/pkg/client/client.go b/pkg/client/client.go index ea2a933..4c8cbdc 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -34,7 +34,7 @@ type GRPCClient struct { } // NewClient creates a new client for the services for the given address, with the certificate and hmac. -func NewClient(ctx context.Context, hostname string, port int, certFile string, keyFile string, caFile string, hmacKey string, insecure bool, logger *slog.Logger) (Client, error) { +func NewClient(ctx context.Context, hostname string, port int, certFile string, keyFile string, caFile string, hmacKey string, insecure bool, logger *slog.Logger, namespace string) (Client, error) { address := fmt.Sprintf("%s:%d", hostname, port) @@ -83,6 +83,20 @@ func NewClient(ctx context.Context, hostname string, port int, certFile string, return nil, fmt.Errorf("failed to create hmac-authenticator: %w", err) } + namespaceInterceptor := func(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { + switch r := req.(type) { + case *v1.TenantMemberCreateRequest: + r.TenantMember.Namespace = namespace + case *v1.TenantMemberFindRequest: + r.Namespace = namespace + case *v1.ProjectMemberCreateRequest: + r.ProjectMember.Namespace = namespace + case *v1.ProjectMemberFindRequest: + r.Namespace = namespace + } + return invoker(ctx, method, req, reply, cc, opts...) + } + opts := []grpc.DialOption{ // In addition to the following grpc.DialOption, callers may also use // the grpc.CallOption grpc.PerRPCCredentials with the RPC invocation @@ -93,6 +107,8 @@ func NewClient(ctx context.Context, hostname string, port int, certFile string, // credentials. grpc.WithTransportCredentials(creds), + grpc.WithChainUnaryInterceptor(namespaceInterceptor), + // grpc.WithInsecure(), } // Set up a connection to the server. diff --git a/pkg/service/projectmember.go b/pkg/service/projectmember.go index 4ba70fa..5f39bec 100644 --- a/pkg/service/projectmember.go +++ b/pkg/service/projectmember.go @@ -53,11 +53,24 @@ func (s *projectMemberService) Create(ctx context.Context, req *v1.ProjectMember err = s.projectMemberStore.Create(ctx, projectMember) return projectMember.NewProjectMemberResponse(), err } + func (s *projectMemberService) Update(ctx context.Context, req *v1.ProjectMemberUpdateRequest) (*v1.ProjectMemberResponse, error) { projectMember := req.ProjectMember - err := s.projectMemberStore.Update(ctx, projectMember) + + old, err := s.projectMemberStore.Get(ctx, projectMember.Meta.Id) + if err != nil { + return nil, err + } + + if old.Namespace != projectMember.Namespace { + return nil, status.Error(codes.InvalidArgument, "updating the namespace of a project member is not allowed") + } + + err = s.projectMemberStore.Update(ctx, projectMember) + return projectMember.NewProjectMemberResponse(), err } + func (s *projectMemberService) Delete(ctx context.Context, req *v1.ProjectMemberDeleteRequest) (*v1.ProjectMemberResponse, error) { projectMember := req.NewProjectMember() err := s.projectMemberStore.Delete(ctx, projectMember.Meta.Id) @@ -78,6 +91,9 @@ func (s *projectMemberService) Find(ctx context.Context, req *v1.ProjectMemberFi if req.TenantId != nil { filter["projectmember ->> 'tenant_id'"] = req.TenantId } + if req.TenantId != nil { + filter["projectmember ->> 'tenant_id'"] = req.TenantId + } for key, value := range req.Annotations { // select * from projectMember where projectMember -> 'meta' -> 'annotations' ->> 'metal-stack.io/role' = 'owner'; f := fmt.Sprintf("projectmember -> 'meta' -> 'annotations' ->> '%s'", key) diff --git a/pkg/service/tenantmember.go b/pkg/service/tenantmember.go index 6eae834..0d82de5 100644 --- a/pkg/service/tenantmember.go +++ b/pkg/service/tenantmember.go @@ -53,7 +53,18 @@ func (s *tenantMemberService) Create(ctx context.Context, req *v1.TenantMemberCr } func (s *tenantMemberService) Update(ctx context.Context, req *v1.TenantMemberUpdateRequest) (*v1.TenantMemberResponse, error) { tenantMember := req.TenantMember - err := s.tenantMemberStore.Update(ctx, tenantMember) + + old, err := s.tenantMemberStore.Get(ctx, tenantMember.Meta.Id) + if err != nil { + return nil, err + } + + if old.Namespace != tenantMember.Namespace { + return nil, status.Error(codes.InvalidArgument, "updating the namespace of a tenant member is not allowed") + } + + err = s.tenantMemberStore.Update(ctx, tenantMember) + return tenantMember.NewTenantMemberResponse(), err } func (s *tenantMemberService) Delete(ctx context.Context, req *v1.TenantMemberDeleteRequest) (*v1.TenantMemberResponse, error) { @@ -69,7 +80,9 @@ func (s *tenantMemberService) Get(ctx context.Context, req *v1.TenantMemberGetRe return tenantMember.NewTenantMemberResponse(), nil } func (s *tenantMemberService) Find(ctx context.Context, req *v1.TenantMemberFindRequest) (*v1.TenantMemberListResponse, error) { - filter := make(map[string]any) + filter := map[string]any{ + "tenantmember ->> 'namespace'": req.Namespace, + } if req.TenantId != nil { filter["tenantmember ->> 'tenant_id'"] = req.TenantId } diff --git a/proto/v1/project_member.proto b/proto/v1/project_member.proto index 91d5772..76946f6 100644 --- a/proto/v1/project_member.proto +++ b/proto/v1/project_member.proto @@ -17,6 +17,8 @@ message ProjectMember { Meta meta = 1; string project_id = 2; string tenant_id = 4; + // Namespace introduces the possibility to associate memberships for different applications that use the masterdata-api as a backend. + string namespace = 5; } message ProjectMemberCreateRequest { @@ -39,6 +41,7 @@ message ProjectMemberFindRequest { optional string project_id = 1; optional string tenant_id = 2; map annotations = 6; + string namespace = 7; } message ProjectMemberResponse { diff --git a/proto/v1/tenant_member.proto b/proto/v1/tenant_member.proto index 08235a6..1d7bafa 100644 --- a/proto/v1/tenant_member.proto +++ b/proto/v1/tenant_member.proto @@ -19,6 +19,8 @@ message TenantMember { string tenant_id = 2; // MemberId is the id of the member tenant string member_id = 3; + // Namespace introduces the possibility to associate memberships for different applications that use the masterdata-api as a backend. + string namespace = 4; } message TenantMemberCreateRequest { @@ -41,6 +43,7 @@ message TenantMemberFindRequest { optional string tenant_id = 1; optional string member_id = 2; map annotations = 6; + string namespace = 7; } message TenantMemberResponse { From 10a34a7216f3d36c40309f8db5fb35499a9dbc3b Mon Sep 17 00:00:00 2001 From: Gerrit Date: Wed, 3 Sep 2025 10:23:39 +0200 Subject: [PATCH 02/16] Add client test for namespace interceptor. --- pkg/client/client.go | 116 ++++++++++++--------- pkg/client/client_test.go | 205 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 275 insertions(+), 46 deletions(-) create mode 100644 pkg/client/client_test.go diff --git a/pkg/client/client.go b/pkg/client/client.go index 4c8cbdc..5437652 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -4,16 +4,16 @@ import ( "context" "crypto/tls" "crypto/x509" - "errors" "fmt" "log/slog" "os" - "github.com/metal-stack/masterdata-api/pkg/auth" "google.golang.org/grpc" "google.golang.org/grpc/credentials" + grpcinsecure "google.golang.org/grpc/credentials/insecure" v1 "github.com/metal-stack/masterdata-api/api/v1" + "github.com/metal-stack/masterdata-api/pkg/auth" ) // Client defines the client API @@ -35,7 +35,6 @@ type GRPCClient struct { // NewClient creates a new client for the services for the given address, with the certificate and hmac. func NewClient(ctx context.Context, hostname string, port int, certFile string, keyFile string, caFile string, hmacKey string, insecure bool, logger *slog.Logger, namespace string) (Client, error) { - address := fmt.Sprintf("%s:%d", hostname, port) certPool, err := x509.SystemCertPool() @@ -55,21 +54,51 @@ func NewClient(ctx context.Context, hostname string, port int, certFile string, } } - clientCertificate, err := tls.LoadX509KeyPair(certFile, keyFile) - if err != nil { - return nil, fmt.Errorf("could not load client key pair: %w", err) + var ( + certificates []tls.Certificate + opts []grpc.DialOption + ) + + if certFile != "" && keyFile != "" { + clientCertificate, err := tls.LoadX509KeyPair(certFile, keyFile) + if err != nil { + return nil, fmt.Errorf("could not load client key pair: %w", err) + } + + certificates = append(certificates, clientCertificate) + + creds := credentials.NewTLS(&tls.Config{ + ServerName: hostname, + Certificates: certificates, + RootCAs: certPool, + MinVersion: tls.VersionTLS12, + InsecureSkipVerify: insecure, // nolint:gosec + }) + + opts = append(opts, + // oauth.NewOauthAccess requires the configuration of transport + // credentials. + grpc.WithTransportCredentials(creds), + ) + } else { + opts = append(opts, + grpc.WithTransportCredentials(grpcinsecure.NewCredentials()), + ) } - creds := credentials.NewTLS(&tls.Config{ - ServerName: hostname, - Certificates: []tls.Certificate{clientCertificate}, - RootCAs: certPool, - MinVersion: tls.VersionTLS12, - InsecureSkipVerify: insecure, // nolint:gosec - }) + if hmacKey != "" { + // Set up the credentials for the connection. + perRPCHMACAuthenticator, err := auth.NewHMACAuther(hmacKey, auth.EditUser) + if err != nil { + return nil, fmt.Errorf("failed to create hmac-authenticator: %w", err) + } - if hmacKey == "" { - return nil, errors.New("no hmac-key specified") + opts = append(opts, + // In addition to the following grpc.DialOption, callers may also use + // the grpc.CallOption grpc.PerRPCCredentials with the RPC invocation + // itself. + // See: https://godoc.org/google.golang.org/grpc#PerRPCCredentials + grpc.WithPerRPCCredentials(perRPCHMACAuthenticator)) } client := GRPCClient{ @@ -77,45 +106,40 @@ func NewClient(ctx context.Context, hostname string, port int, certFile string, hmacKey: hmacKey, } - // Set up the credentials for the connection. - perRPCHMACAuthenticator, err := auth.NewHMACAuther(hmacKey, auth.EditUser) - if err != nil { - return nil, fmt.Errorf("failed to create hmac-authenticator: %w", err) - } - - namespaceInterceptor := func(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { - switch r := req.(type) { - case *v1.TenantMemberCreateRequest: - r.TenantMember.Namespace = namespace - case *v1.TenantMemberFindRequest: - r.Namespace = namespace - case *v1.ProjectMemberCreateRequest: - r.ProjectMember.Namespace = namespace - case *v1.ProjectMemberFindRequest: - r.Namespace = namespace + if namespace != "" { + namespaceInterceptor := func(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { + switch r := req.(type) { + case *v1.TenantMemberCreateRequest: + if r.TenantMember.Namespace == "" { + r.TenantMember.Namespace = namespace + } + case *v1.TenantMemberFindRequest: + if r.Namespace == "" { + r.Namespace = namespace + } + case *v1.ProjectMemberCreateRequest: + if r.ProjectMember.Namespace == "" { + r.ProjectMember.Namespace = namespace + } + case *v1.ProjectMemberFindRequest: + if r.Namespace == "" { + r.Namespace = namespace + } + } + return invoker(ctx, method, req, reply, cc, opts...) } - return invoker(ctx, method, req, reply, cc, opts...) - } - opts := []grpc.DialOption{ - // In addition to the following grpc.DialOption, callers may also use - // the grpc.CallOption grpc.PerRPCCredentials with the RPC invocation - // itself. - // See: https://godoc.org/google.golang.org/grpc#PerRPCCredentials - grpc.WithPerRPCCredentials(perRPCHMACAuthenticator), - // oauth.NewOauthAccess requires the configuration of transport - // credentials. - grpc.WithTransportCredentials(creds), - - grpc.WithChainUnaryInterceptor(namespaceInterceptor), - - // grpc.WithInsecure(), + opts = append(opts, + grpc.WithChainUnaryInterceptor(namespaceInterceptor), + ) } + // Set up a connection to the server. conn, err := grpc.NewClient(address, opts...) if err != nil { return nil, err } + client.conn = conn return client, nil diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go new file mode 100644 index 0000000..989b00b --- /dev/null +++ b/pkg/client/client_test.go @@ -0,0 +1,205 @@ +package client + +import ( + "context" + "log/slog" + "net" + "strconv" + "testing" + + v1 "github.com/metal-stack/masterdata-api/api/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/reflection" +) + +func Test_Client(t *testing.T) { + const ( + namespace = "a" + ) + + var ( + log = slog.Default() + grpcServer = grpc.NewServer() + projectMemberServer = &projectMemberServer{} + tenantMemberServer = &tenantMemberServer{} + ) + + v1.RegisterProjectMemberServiceServer(grpcServer, projectMemberServer) + v1.RegisterTenantMemberServiceServer(grpcServer, tenantMemberServer) + + reflection.Register(grpcServer) + + lis, err := net.Listen("tcp", "") + require.NoError(t, err) + + go func() { + err = grpcServer.Serve(lis) + require.NoError(t, err) + }() + defer func() { + grpcServer.Stop() + }() + + _, portString, err := net.SplitHostPort(lis.Addr().String()) + require.NoError(t, err) + + port, err := strconv.Atoi(portString) + require.NoError(t, err) + + client, err := NewClient(t.Context(), "localhost", port, "", "", "", "", true, log, namespace) + require.NoError(t, err) + + t.Run("check namespace interceptor sets missing namespace", func(t *testing.T) { + t.Run("project member", func(t *testing.T) { + projectMemberServer.create = func(ctx context.Context, pmcr *v1.ProjectMemberCreateRequest) (*v1.ProjectMemberResponse, error) { + assert.Equal(t, "project-a", pmcr.ProjectMember.ProjectId) + assert.Equal(t, "tenant-a", pmcr.ProjectMember.TenantId) + assert.Equal(t, namespace, pmcr.ProjectMember.Namespace) + return &v1.ProjectMemberResponse{}, nil + } + projectMemberServer.find = func(ctx context.Context, pmfr *v1.ProjectMemberFindRequest) (*v1.ProjectMemberListResponse, error) { + assert.Equal(t, namespace, pmfr.Namespace) + return &v1.ProjectMemberListResponse{}, nil + } + + _, err = client.ProjectMember().Create(t.Context(), &v1.ProjectMemberCreateRequest{ + ProjectMember: &v1.ProjectMember{ + ProjectId: "project-a", + TenantId: "tenant-a", + }, + }) + require.NoError(t, err) + + _, err = client.ProjectMember().Find(t.Context(), &v1.ProjectMemberFindRequest{}) + require.NoError(t, err) + }) + + t.Run("tenant member", func(t *testing.T) { + tenantMemberServer.create = func(ctx context.Context, tmcr *v1.TenantMemberCreateRequest) (*v1.TenantMemberResponse, error) { + assert.Equal(t, "tenant-a", tmcr.TenantMember.TenantId) + assert.Equal(t, namespace, tmcr.TenantMember.Namespace) + return &v1.TenantMemberResponse{}, nil + } + tenantMemberServer.find = func(ctx context.Context, tmfr *v1.TenantMemberFindRequest) (*v1.TenantMemberListResponse, error) { + assert.Equal(t, namespace, tmfr.Namespace) + return &v1.TenantMemberListResponse{}, nil + } + + _, err = client.TenantMember().Create(t.Context(), &v1.TenantMemberCreateRequest{ + TenantMember: &v1.TenantMember{ + TenantId: "tenant-a", + }, + }) + require.NoError(t, err) + + _, err = client.TenantMember().Find(t.Context(), &v1.TenantMemberFindRequest{}) + require.NoError(t, err) + }) + }) + + t.Run("check explicit namespace can be set anyway", func(t *testing.T) { + t.Run("project member", func(t *testing.T) { + projectMemberServer.create = func(ctx context.Context, pmcr *v1.ProjectMemberCreateRequest) (*v1.ProjectMemberResponse, error) { + assert.Equal(t, "project-a", pmcr.ProjectMember.ProjectId) + assert.Equal(t, "tenant-a", pmcr.ProjectMember.TenantId) + assert.Equal(t, "b", pmcr.ProjectMember.Namespace) + return &v1.ProjectMemberResponse{}, nil + } + projectMemberServer.find = func(ctx context.Context, pmfr *v1.ProjectMemberFindRequest) (*v1.ProjectMemberListResponse, error) { + assert.Equal(t, "b", pmfr.Namespace) + return &v1.ProjectMemberListResponse{}, nil + } + + _, err = client.ProjectMember().Create(t.Context(), &v1.ProjectMemberCreateRequest{ + ProjectMember: &v1.ProjectMember{ + ProjectId: "project-a", + TenantId: "tenant-a", + Namespace: "b", + }, + }) + require.NoError(t, err) + + _, err = client.ProjectMember().Find(t.Context(), &v1.ProjectMemberFindRequest{ + Namespace: "b", + }) + require.NoError(t, err) + }) + + t.Run("tenant member", func(t *testing.T) { + tenantMemberServer.create = func(ctx context.Context, tmcr *v1.TenantMemberCreateRequest) (*v1.TenantMemberResponse, error) { + assert.Equal(t, "tenant-a", tmcr.TenantMember.TenantId) + assert.Equal(t, "b", tmcr.TenantMember.Namespace) + return &v1.TenantMemberResponse{}, nil + } + tenantMemberServer.find = func(ctx context.Context, tmfr *v1.TenantMemberFindRequest) (*v1.TenantMemberListResponse, error) { + assert.Equal(t, "b", tmfr.Namespace) + return &v1.TenantMemberListResponse{}, nil + } + + _, err = client.TenantMember().Create(t.Context(), &v1.TenantMemberCreateRequest{ + TenantMember: &v1.TenantMember{ + TenantId: "tenant-a", + Namespace: "b", + }, + }) + require.NoError(t, err) + + _, err = client.TenantMember().Find(t.Context(), &v1.TenantMemberFindRequest{ + Namespace: "b", + }) + require.NoError(t, err) + }) + }) +} + +type projectMemberServer struct { + create func(context.Context, *v1.ProjectMemberCreateRequest) (*v1.ProjectMemberResponse, error) + find func(context.Context, *v1.ProjectMemberFindRequest) (*v1.ProjectMemberListResponse, error) +} + +func (t *projectMemberServer) Create(ctx context.Context, r *v1.ProjectMemberCreateRequest) (*v1.ProjectMemberResponse, error) { + return t.create(ctx, r) +} + +func (t *projectMemberServer) Delete(context.Context, *v1.ProjectMemberDeleteRequest) (*v1.ProjectMemberResponse, error) { + panic("unimplemented") +} + +func (t *projectMemberServer) Find(ctx context.Context, r *v1.ProjectMemberFindRequest) (*v1.ProjectMemberListResponse, error) { + return t.find(ctx, r) +} + +func (t *projectMemberServer) Get(context.Context, *v1.ProjectMemberGetRequest) (*v1.ProjectMemberResponse, error) { + panic("unimplemented") +} + +func (t *projectMemberServer) Update(context.Context, *v1.ProjectMemberUpdateRequest) (*v1.ProjectMemberResponse, error) { + panic("unimplemented") +} + +type tenantMemberServer struct { + create func(context.Context, *v1.TenantMemberCreateRequest) (*v1.TenantMemberResponse, error) + find func(context.Context, *v1.TenantMemberFindRequest) (*v1.TenantMemberListResponse, error) +} + +func (t *tenantMemberServer) Create(ctx context.Context, r *v1.TenantMemberCreateRequest) (*v1.TenantMemberResponse, error) { + return t.create(ctx, r) +} + +func (t *tenantMemberServer) Delete(context.Context, *v1.TenantMemberDeleteRequest) (*v1.TenantMemberResponse, error) { + panic("unimplemented") +} + +func (t *tenantMemberServer) Find(ctx context.Context, r *v1.TenantMemberFindRequest) (*v1.TenantMemberListResponse, error) { + return t.find(ctx, r) +} + +func (t *tenantMemberServer) Get(context.Context, *v1.TenantMemberGetRequest) (*v1.TenantMemberResponse, error) { + panic("unimplemented") +} + +func (t *tenantMemberServer) Update(context.Context, *v1.TenantMemberUpdateRequest) (*v1.TenantMemberResponse, error) { + panic("unimplemented") +} From 935ebbeeda5d8dc84b5a1e5b57673845588f31d2 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Wed, 3 Sep 2025 11:37:57 +0200 Subject: [PATCH 03/16] Fix tests. --- pkg/service/projectmember.go | 4 +++- pkg/service/projectmember_test.go | 7 ++++++- pkg/service/tenantmember_test.go | 9 +++++++-- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/pkg/service/projectmember.go b/pkg/service/projectmember.go index 5f39bec..0a97176 100644 --- a/pkg/service/projectmember.go +++ b/pkg/service/projectmember.go @@ -84,7 +84,9 @@ func (s *projectMemberService) Get(ctx context.Context, req *v1.ProjectMemberGet return projectMember.NewProjectMemberResponse(), nil } func (s *projectMemberService) Find(ctx context.Context, req *v1.ProjectMemberFindRequest) (*v1.ProjectMemberListResponse, error) { - filter := make(map[string]any) + filter := map[string]any{ + "projectmember ->> 'namespace'": req.Namespace, + } if req.ProjectId != nil { filter["projectmember ->> 'project_id'"] = req.ProjectId } diff --git a/pkg/service/projectmember_test.go b/pkg/service/projectmember_test.go index 3268b38..eef687b 100644 --- a/pkg/service/projectmember_test.go +++ b/pkg/service/projectmember_test.go @@ -73,6 +73,7 @@ func TestUpdateProjectMember(t *testing.T) { }, } + storageMock.On("Get", ctx, pm1.Meta.Id).Return(pm1, nil) storageMock.On("Update", ctx, pm1).Return(nil) resp, err := ts.Update(ctx, pmur) require.NoError(t, err) @@ -149,10 +150,12 @@ func TestFindProjectMemberByProject(t *testing.T) { var t6s []*v1.ProjectMember tfr := &v1.ProjectMemberFindRequest{ ProjectId: pointer.Pointer("p1"), + Namespace: "a", } f2 := make(map[string]any) f2["projectmember ->> 'project_id'"] = pointer.Pointer("p1") + f2["projectmember ->> 'namespace'"] = "a" storageMock.On("Find", ctx, mock.AnythingOfType("*v1.Paging"), []any{f2}).Return(t6s, nil, nil) resp, err := ts.Find(ctx, tfr) require.NoError(t, err) @@ -174,11 +177,13 @@ func TestFindProjectMemberByTenant(t *testing.T) { // filter by name var t6s []*v1.ProjectMember tfr := &v1.ProjectMemberFindRequest{ - TenantId: pointer.Pointer("t1"), + TenantId: pointer.Pointer("t1"), + Namespace: "a", } f2 := make(map[string]any) f2["projectmember ->> 'tenant_id'"] = pointer.Pointer("t1") + f2["projectmember ->> 'namespace'"] = "a" storageMock.On("Find", ctx, mock.AnythingOfType("*v1.Paging"), []any{f2}).Return(t6s, nil, nil) resp, err := ts.Find(ctx, tfr) require.NoError(t, err) diff --git a/pkg/service/tenantmember_test.go b/pkg/service/tenantmember_test.go index 6762531..19c3d0a 100644 --- a/pkg/service/tenantmember_test.go +++ b/pkg/service/tenantmember_test.go @@ -69,6 +69,7 @@ func TestUpdateTenantMember(t *testing.T) { }, } + storageMock.On("Get", ctx, pm1.Meta.Id).Return(pm1, nil) storageMock.On("Update", ctx, pm1).Return(nil) resp, err := ts.Update(ctx, pmur) require.NoError(t, err) @@ -138,11 +139,13 @@ func TestFindTenantMemberByTenant(t *testing.T) { // filter by name var t6s []*v1.TenantMember tfr := &v1.TenantMemberFindRequest{ - TenantId: pointer.Pointer("p1"), + TenantId: pointer.Pointer("p1"), + Namespace: "a", } f2 := make(map[string]any) f2["tenantmember ->> 'tenant_id'"] = pointer.Pointer("p1") + f2["tenantmember ->> 'namespace'"] = "a" storageMock.On("Find", ctx, mock.AnythingOfType("*v1.Paging"), []any{f2}).Return(t6s, nil, nil) resp, err := ts.Find(ctx, tfr) require.NoError(t, err) @@ -162,11 +165,13 @@ func TestFindTenantMemberByMember(t *testing.T) { // filter by name var t6s []*v1.TenantMember tfr := &v1.TenantMemberFindRequest{ - MemberId: pointer.Pointer("t1"), + MemberId: pointer.Pointer("t1"), + Namespace: "a", } f2 := make(map[string]any) f2["tenantmember ->> 'member_id'"] = pointer.Pointer("t1") + f2["tenantmember ->> 'namespace'"] = "a" storageMock.On("Find", ctx, mock.AnythingOfType("*v1.Paging"), []any{f2}).Return(t6s, nil, nil) resp, err := ts.Find(ctx, tfr) require.NoError(t, err) From cfb42067b4c526a0c56dd7ac5822da18c893d03d Mon Sep 17 00:00:00 2001 From: Gerrit Date: Wed, 3 Sep 2025 13:12:49 +0200 Subject: [PATCH 04/16] Add namespace to member queries. --- api/v1/tenant.pb.go | 39 +++++++++++++++++++++++++++++------ pkg/datastore/query_runner.go | 2 +- pkg/service/tenant.go | 26 ++++++++++++++++------- pkg/service/tenant_test.go | 2 +- proto/v1/tenant.proto | 3 +++ 5 files changed, 56 insertions(+), 16 deletions(-) diff --git a/api/v1/tenant.pb.go b/api/v1/tenant.pb.go index 4b0c73e..c3907f3 100644 --- a/api/v1/tenant.pb.go +++ b/api/v1/tenant.pb.go @@ -27,6 +27,7 @@ type FindParticipatingProjectsRequest struct { state protoimpl.MessageState `protogen:"open.v1"` TenantId string `protobuf:"bytes,1,opt,name=tenant_id,json=tenantId,proto3" json:"tenant_id,omitempty"` IncludeInherited *bool `protobuf:"varint,2,opt,name=include_inherited,json=includeInherited,proto3,oneof" json:"include_inherited,omitempty"` + Namespace string `protobuf:"bytes,3,opt,name=namespace,proto3" json:"namespace,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -75,10 +76,18 @@ func (x *FindParticipatingProjectsRequest) GetIncludeInherited() bool { return false } +func (x *FindParticipatingProjectsRequest) GetNamespace() string { + if x != nil { + return x.Namespace + } + return "" +} + type FindParticipatingTenantsRequest struct { state protoimpl.MessageState `protogen:"open.v1"` TenantId string `protobuf:"bytes,1,opt,name=tenant_id,json=tenantId,proto3" json:"tenant_id,omitempty"` IncludeInherited *bool `protobuf:"varint,2,opt,name=include_inherited,json=includeInherited,proto3,oneof" json:"include_inherited,omitempty"` + Namespace string `protobuf:"bytes,3,opt,name=namespace,proto3" json:"namespace,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -127,10 +136,18 @@ func (x *FindParticipatingTenantsRequest) GetIncludeInherited() bool { return false } +func (x *FindParticipatingTenantsRequest) GetNamespace() string { + if x != nil { + return x.Namespace + } + return "" +} + type ListTenantMembersRequest struct { state protoimpl.MessageState `protogen:"open.v1"` TenantId string `protobuf:"bytes,1,opt,name=tenant_id,json=tenantId,proto3" json:"tenant_id,omitempty"` IncludeInherited *bool `protobuf:"varint,2,opt,name=include_inherited,json=includeInherited,proto3,oneof" json:"include_inherited,omitempty"` + Namespace string `protobuf:"bytes,3,opt,name=namespace,proto3" json:"namespace,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -179,6 +196,13 @@ func (x *ListTenantMembersRequest) GetIncludeInherited() bool { return false } +func (x *ListTenantMembersRequest) GetNamespace() string { + if x != nil { + return x.Namespace + } + return "" +} + type ListTenantMembersResponse struct { state protoimpl.MessageState `protogen:"open.v1"` Tenants []*TenantWithMembershipAnnotations `protobuf:"bytes,1,rep,name=tenants,proto3" json:"tenants,omitempty"` @@ -951,18 +975,21 @@ var File_v1_tenant_proto protoreflect.FileDescriptor const file_v1_tenant_proto_rawDesc = "" + "\n" + - "\x0fv1/tenant.proto\x12\x02v1\x1a\x1fgoogle/protobuf/timestamp.proto\x1a\x1egoogle/protobuf/wrappers.proto\x1a\x0fv1/common.proto\x1a\fv1/iam.proto\x1a\rv1/meta.proto\x1a\x10v1/project.proto\x1a\x0ev1/quota.proto\"\x87\x01\n" + + "\x0fv1/tenant.proto\x12\x02v1\x1a\x1fgoogle/protobuf/timestamp.proto\x1a\x1egoogle/protobuf/wrappers.proto\x1a\x0fv1/common.proto\x1a\fv1/iam.proto\x1a\rv1/meta.proto\x1a\x10v1/project.proto\x1a\x0ev1/quota.proto\"\xa5\x01\n" + " FindParticipatingProjectsRequest\x12\x1b\n" + "\ttenant_id\x18\x01 \x01(\tR\btenantId\x120\n" + - "\x11include_inherited\x18\x02 \x01(\bH\x00R\x10includeInherited\x88\x01\x01B\x14\n" + - "\x12_include_inherited\"\x86\x01\n" + + "\x11include_inherited\x18\x02 \x01(\bH\x00R\x10includeInherited\x88\x01\x01\x12\x1c\n" + + "\tnamespace\x18\x03 \x01(\tR\tnamespaceB\x14\n" + + "\x12_include_inherited\"\xa4\x01\n" + "\x1fFindParticipatingTenantsRequest\x12\x1b\n" + "\ttenant_id\x18\x01 \x01(\tR\btenantId\x120\n" + - "\x11include_inherited\x18\x02 \x01(\bH\x00R\x10includeInherited\x88\x01\x01B\x14\n" + - "\x12_include_inherited\"\x7f\n" + + "\x11include_inherited\x18\x02 \x01(\bH\x00R\x10includeInherited\x88\x01\x01\x12\x1c\n" + + "\tnamespace\x18\x03 \x01(\tR\tnamespaceB\x14\n" + + "\x12_include_inherited\"\x9d\x01\n" + "\x18ListTenantMembersRequest\x12\x1b\n" + "\ttenant_id\x18\x01 \x01(\tR\btenantId\x120\n" + - "\x11include_inherited\x18\x02 \x01(\bH\x00R\x10includeInherited\x88\x01\x01B\x14\n" + + "\x11include_inherited\x18\x02 \x01(\bH\x00R\x10includeInherited\x88\x01\x01\x12\x1c\n" + + "\tnamespace\x18\x03 \x01(\tR\tnamespaceB\x14\n" + "\x12_include_inherited\"Z\n" + "\x19ListTenantMembersResponse\x12=\n" + "\atenants\x18\x01 \x03(\v2#.v1.TenantWithMembershipAnnotationsR\atenants\"e\n" + diff --git a/pkg/datastore/query_runner.go b/pkg/datastore/query_runner.go index 5cfe9f6..3c10395 100644 --- a/pkg/datastore/query_runner.go +++ b/pkg/datastore/query_runner.go @@ -15,7 +15,7 @@ func RunQuery[E any](ctx context.Context, log *slog.Logger, db *sqlx.DB, builder } if log.Enabled(ctx, slog.LevelDebug) { - log.Debug("query", "sql", query, "values", vals) + log.Debug("query", "sql", query, "values", vals, "input", input) } rows, err := db.NamedQueryContext(ctx, query, input) diff --git a/pkg/service/tenant.go b/pkg/service/tenant.go index 95ecff0..69bdd18 100644 --- a/pkg/service/tenant.go +++ b/pkg/service/tenant.go @@ -175,7 +175,9 @@ var ( ). From(projectMembers.TableName()). Join(projects.TableName() + " ON " + projects.TableName() + ".id = " + projectMembers.JSONField() + "->>'project_id'"). - Where(projectMembers.JSONField() + "->>'tenant_id' = :tenantId") + Where(projectMembers.JSONField() + "->>'tenant_id' = :tenantId"). + // COALESCE is required to provide an empty string as default value in case the namespace field is not present + Where("COALESCE(" + projectMembers.JSONField() + "->> 'namespace', '') = :namespace") queryInheritedProjectParticipations = sq.Select( projects.JSONField(), @@ -183,7 +185,9 @@ var ( ). From(tenantMembers.TableName()). Join(projects.TableName() + " ON " + projects.JSONField() + "->>'tenant_id' = " + tenantMembers.JSONField() + "->>'tenant_id'"). - Where(tenantMembers.JSONField() + "->>'member_id' = :tenantId") + Where(tenantMembers.JSONField() + "->>'member_id' = :tenantId"). + // COALESCE is required to provide an empty string as default value in case the namespace field is not present + Where("COALESCE(" + tenantMembers.JSONField() + "->> 'namespace', '') = :namespace") ) // FindParticipatingProjects returns all projects in which a member participates. @@ -200,7 +204,7 @@ func (s *tenantService) FindParticipatingProjects(ctx context.Context, req *v1.F res []*v1.ProjectWithMembershipAnnotations resultMap = map[string]*v1.ProjectWithMembershipAnnotations{} - input = map[string]any{"tenantId": req.TenantId} + input = map[string]any{"tenantId": req.TenantId, "namespace": req.Namespace} resultFn = func(e result) error { p, ok := resultMap[e.Project.Meta.Id] @@ -261,7 +265,9 @@ var ( ). From(tenantMembers.TableName()). Join(tenants.TableName() + " ON " + tenants.TableName() + ".id = " + tenantMembers.JSONField() + "->>'tenant_id'"). - Where(tenantMembers.JSONField() + "->>'member_id' = :tenantId") + Where(tenantMembers.JSONField() + "->>'member_id' = :tenantId"). + // COALESCE is required to provide an empty string as default value in case the namespace field is not present + Where("COALESCE(" + tenantMembers.JSONField() + "->> 'namespace', '') = :namespace") queryInheritedTenantParticipations = sq.Select( tenants.JSONField(), @@ -270,7 +276,9 @@ var ( From(projectMembers.TableName()). Join(projects.TableName() + " ON " + projects.TableName() + ".id = " + projectMembers.JSONField() + "->>'project_id'"). Join(tenants.TableName() + " ON " + tenants.TableName() + ".id = " + projects.JSONField() + "->>'tenant_id'"). - Where(projectMembers.JSONField() + "->>'tenant_id' = :tenantId") + Where(projectMembers.JSONField() + "->>'tenant_id' = :tenantId"). + // COALESCE is required to provide an empty string as default value in case the namespace field is not present + Where("COALESCE(" + projectMembers.JSONField() + "->> 'namespace', '') = :namespace") ) // FindParticipatingTenants returns all tenants in which a member participates. @@ -284,7 +292,7 @@ func (s *tenantService) FindParticipatingTenants(ctx context.Context, req *v1.Fi } var ( - input = map[string]any{"tenantId": req.TenantId} + input = map[string]any{"tenantId": req.TenantId, "namespace": req.Namespace} res []*v1.TenantWithMembershipAnnotations resultMap = map[string]*v1.TenantWithMembershipAnnotations{} @@ -348,7 +356,9 @@ var ( ). From(tenantMembers.TableName()). Join(tenants.TableName() + " ON " + tenants.TableName() + ".id = " + tenantMembers.JSONField() + "->>'member_id'"). - Where(tenantMembers.JSONField() + "->>'tenant_id' = :tenantId") + Where(tenantMembers.JSONField() + "->>'tenant_id' = :tenantId"). + // COALESCE is required to provide an empty string as default value in case the namespace field is not present + Where("COALESCE(" + tenantMembers.JSONField() + "->> 'namespace', '') = :namespace") queryInheritedTenantMembers = sq.Select( tenants.JSONField(), @@ -374,7 +384,7 @@ func (s *tenantService) ListTenantMembers(ctx context.Context, req *v1.ListTenan res []*v1.TenantWithMembershipAnnotations resultMap = map[string]*v1.TenantWithMembershipAnnotations{} - input = map[string]any{"tenantId": req.TenantId} + input = map[string]any{"tenantId": req.TenantId, "namespace": req.Namespace} resultFn = func(e result) error { t, ok := resultMap[e.Tenant.Meta.Id] diff --git a/pkg/service/tenant_test.go b/pkg/service/tenant_test.go index 1a037a5..aec19b7 100644 --- a/pkg/service/tenant_test.go +++ b/pkg/service/tenant_test.go @@ -895,7 +895,7 @@ func Test_tenantService_ListTenantMembers(t *testing.T) { s := &tenantService{ db: db, - log: slog.Default(), + log: slog.New(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug})), } var ( diff --git a/proto/v1/tenant.proto b/proto/v1/tenant.proto index b1f808c..1f5bda3 100644 --- a/proto/v1/tenant.proto +++ b/proto/v1/tenant.proto @@ -25,16 +25,19 @@ service TenantService { message FindParticipatingProjectsRequest { string tenant_id = 1; optional bool include_inherited = 2; + string namespace = 3; } message FindParticipatingTenantsRequest { string tenant_id = 1; optional bool include_inherited = 2; + string namespace = 3; } message ListTenantMembersRequest { string tenant_id = 1; optional bool include_inherited = 2; + string namespace = 3; } message ListTenantMembersResponse { From 1ff78a36f659aff0ec4a44eccabea9ca34d00b9b Mon Sep 17 00:00:00 2001 From: Gerrit Date: Wed, 3 Sep 2025 13:32:10 +0200 Subject: [PATCH 05/16] Expose namespace interceptor. --- pkg/client/client.go | 50 ++++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index 5437652..8f31350 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -107,31 +107,7 @@ func NewClient(ctx context.Context, hostname string, port int, certFile string, } if namespace != "" { - namespaceInterceptor := func(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { - switch r := req.(type) { - case *v1.TenantMemberCreateRequest: - if r.TenantMember.Namespace == "" { - r.TenantMember.Namespace = namespace - } - case *v1.TenantMemberFindRequest: - if r.Namespace == "" { - r.Namespace = namespace - } - case *v1.ProjectMemberCreateRequest: - if r.ProjectMember.Namespace == "" { - r.ProjectMember.Namespace = namespace - } - case *v1.ProjectMemberFindRequest: - if r.Namespace == "" { - r.Namespace = namespace - } - } - return invoker(ctx, method, req, reply, cc, opts...) - } - - opts = append(opts, - grpc.WithChainUnaryInterceptor(namespaceInterceptor), - ) + opts = append(opts, NamespaceInterceptor(namespace)) } // Set up a connection to the server. @@ -145,6 +121,30 @@ func NewClient(ctx context.Context, hostname string, port int, certFile string, return client, nil } +func NamespaceInterceptor(namespace string) grpc.DialOption { + return grpc.WithChainUnaryInterceptor(func(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { + switch r := req.(type) { + case *v1.TenantMemberCreateRequest: + if r.TenantMember.Namespace == "" { + r.TenantMember.Namespace = namespace + } + case *v1.TenantMemberFindRequest: + if r.Namespace == "" { + r.Namespace = namespace + } + case *v1.ProjectMemberCreateRequest: + if r.ProjectMember.Namespace == "" { + r.ProjectMember.Namespace = namespace + } + case *v1.ProjectMemberFindRequest: + if r.Namespace == "" { + r.Namespace = namespace + } + } + return invoker(ctx, method, req, reply, cc, opts...) + }) +} + // Close the underlying connection func (c GRPCClient) Close() error { return c.conn.Close() From 7e9b567a79b46f86e10dca500b12f034920a1ac5 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Wed, 3 Sep 2025 13:37:29 +0200 Subject: [PATCH 06/16] More COALESCE. --- pkg/service/projectmember.go | 2 +- pkg/service/projectmember_test.go | 4 ++-- pkg/service/tenantmember.go | 2 +- pkg/service/tenantmember_test.go | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/service/projectmember.go b/pkg/service/projectmember.go index 0a97176..abdeea3 100644 --- a/pkg/service/projectmember.go +++ b/pkg/service/projectmember.go @@ -85,7 +85,7 @@ func (s *projectMemberService) Get(ctx context.Context, req *v1.ProjectMemberGet } func (s *projectMemberService) Find(ctx context.Context, req *v1.ProjectMemberFindRequest) (*v1.ProjectMemberListResponse, error) { filter := map[string]any{ - "projectmember ->> 'namespace'": req.Namespace, + "COALESCE(projectmember ->> 'namespace', '')": req.Namespace, } if req.ProjectId != nil { filter["projectmember ->> 'project_id'"] = req.ProjectId diff --git a/pkg/service/projectmember_test.go b/pkg/service/projectmember_test.go index eef687b..390d6df 100644 --- a/pkg/service/projectmember_test.go +++ b/pkg/service/projectmember_test.go @@ -155,7 +155,7 @@ func TestFindProjectMemberByProject(t *testing.T) { f2 := make(map[string]any) f2["projectmember ->> 'project_id'"] = pointer.Pointer("p1") - f2["projectmember ->> 'namespace'"] = "a" + f2["COALESCE(projectmember ->> 'namespace', '')"] = "a" storageMock.On("Find", ctx, mock.AnythingOfType("*v1.Paging"), []any{f2}).Return(t6s, nil, nil) resp, err := ts.Find(ctx, tfr) require.NoError(t, err) @@ -183,7 +183,7 @@ func TestFindProjectMemberByTenant(t *testing.T) { f2 := make(map[string]any) f2["projectmember ->> 'tenant_id'"] = pointer.Pointer("t1") - f2["projectmember ->> 'namespace'"] = "a" + f2["COALESCE(projectmember ->> 'namespace', '')"] = "a" storageMock.On("Find", ctx, mock.AnythingOfType("*v1.Paging"), []any{f2}).Return(t6s, nil, nil) resp, err := ts.Find(ctx, tfr) require.NoError(t, err) diff --git a/pkg/service/tenantmember.go b/pkg/service/tenantmember.go index 0d82de5..f719cb9 100644 --- a/pkg/service/tenantmember.go +++ b/pkg/service/tenantmember.go @@ -81,7 +81,7 @@ func (s *tenantMemberService) Get(ctx context.Context, req *v1.TenantMemberGetRe } func (s *tenantMemberService) Find(ctx context.Context, req *v1.TenantMemberFindRequest) (*v1.TenantMemberListResponse, error) { filter := map[string]any{ - "tenantmember ->> 'namespace'": req.Namespace, + "COALESCE(tenantmember ->> 'namespace', '')": req.Namespace, } if req.TenantId != nil { filter["tenantmember ->> 'tenant_id'"] = req.TenantId diff --git a/pkg/service/tenantmember_test.go b/pkg/service/tenantmember_test.go index 19c3d0a..70d3b2b 100644 --- a/pkg/service/tenantmember_test.go +++ b/pkg/service/tenantmember_test.go @@ -145,7 +145,7 @@ func TestFindTenantMemberByTenant(t *testing.T) { f2 := make(map[string]any) f2["tenantmember ->> 'tenant_id'"] = pointer.Pointer("p1") - f2["tenantmember ->> 'namespace'"] = "a" + f2["COALESCE(tenantmember ->> 'namespace', '')"] = "a" storageMock.On("Find", ctx, mock.AnythingOfType("*v1.Paging"), []any{f2}).Return(t6s, nil, nil) resp, err := ts.Find(ctx, tfr) require.NoError(t, err) @@ -171,7 +171,7 @@ func TestFindTenantMemberByMember(t *testing.T) { f2 := make(map[string]any) f2["tenantmember ->> 'member_id'"] = pointer.Pointer("t1") - f2["tenantmember ->> 'namespace'"] = "a" + f2["COALESCE(tenantmember ->> 'namespace', '')"] = "a" storageMock.On("Find", ctx, mock.AnythingOfType("*v1.Paging"), []any{f2}).Return(t6s, nil, nil) resp, err := ts.Find(ctx, tfr) require.NoError(t, err) From e904f874ee16ac39547da9347b1b45590f248ad0 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Wed, 3 Sep 2025 13:55:10 +0200 Subject: [PATCH 07/16] Interceptor extend. --- pkg/client/client.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index 8f31350..0041e7c 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -128,18 +128,30 @@ func NamespaceInterceptor(namespace string) grpc.DialOption { if r.TenantMember.Namespace == "" { r.TenantMember.Namespace = namespace } - case *v1.TenantMemberFindRequest: - if r.Namespace == "" { - r.Namespace = namespace - } case *v1.ProjectMemberCreateRequest: if r.ProjectMember.Namespace == "" { r.ProjectMember.Namespace = namespace } + case *v1.TenantMemberFindRequest: + if r.Namespace == "" { + r.Namespace = namespace + } case *v1.ProjectMemberFindRequest: if r.Namespace == "" { r.Namespace = namespace } + case *v1.FindParticipatingProjectsRequest: + if r.Namespace != "" { + r.Namespace = namespace + } + case *v1.FindParticipatingTenantsRequest: + if r.Namespace != "" { + r.Namespace = namespace + } + case *v1.ListTenantMembersRequest: + if r.Namespace != "" { + r.Namespace = namespace + } } return invoker(ctx, method, req, reply, cc, opts...) }) From aef669f9fbeda48f56c35f763b084cbb37cd90a6 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Wed, 3 Sep 2025 14:16:44 +0200 Subject: [PATCH 08/16] One missing query adjustment. --- pkg/service/tenant.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/service/tenant.go b/pkg/service/tenant.go index 69bdd18..ecfa529 100644 --- a/pkg/service/tenant.go +++ b/pkg/service/tenant.go @@ -367,7 +367,9 @@ var ( From(projectMembers.TableName()). Join(projects.TableName() + " ON " + projects.TableName() + ".id = " + projectMembers.JSONField() + "->>'project_id'"). Join(tenants.TableName() + " ON " + tenants.TableName() + ".id = " + projectMembers.JSONField() + "->>'tenant_id'"). - Where(projects.JSONField() + "->>'tenant_id' = :tenantId") + Where(projects.JSONField() + "->>'tenant_id' = :tenantId"). + // COALESCE is required to provide an empty string as default value in case the namespace field is not present + Where("COALESCE(" + projectMembers.JSONField() + "->> 'namespace', '') = :namespace") ) // ListTenantMembers returns all members of a tenant. From ccbce37983bc212af9822813bfe335927551c246 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Wed, 3 Sep 2025 14:47:45 +0200 Subject: [PATCH 09/16] Fix. --- pkg/client/client.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index 0041e7c..b375fa1 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -141,15 +141,15 @@ func NamespaceInterceptor(namespace string) grpc.DialOption { r.Namespace = namespace } case *v1.FindParticipatingProjectsRequest: - if r.Namespace != "" { + if r.Namespace == "" { r.Namespace = namespace } case *v1.FindParticipatingTenantsRequest: - if r.Namespace != "" { + if r.Namespace == "" { r.Namespace = namespace } case *v1.ListTenantMembersRequest: - if r.Namespace != "" { + if r.Namespace == "" { r.Namespace = namespace } } From a24e1473983d6a4a1985abda48da36ca78b33f84 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Wed, 10 Sep 2025 09:43:38 +0200 Subject: [PATCH 10/16] First set of tests. --- pkg/service/projectmember.go | 5 + pkg/service/projectmember_test.go | 275 ++++++++++++++++++++++++++++++ pkg/service/tenantmember.go | 9 + pkg/service/tenantmember_test.go | 255 +++++++++++++++++++++++++++ 4 files changed, 544 insertions(+) diff --git a/pkg/service/projectmember.go b/pkg/service/projectmember.go index abdeea3..d033199 100644 --- a/pkg/service/projectmember.go +++ b/pkg/service/projectmember.go @@ -76,6 +76,7 @@ func (s *projectMemberService) Delete(ctx context.Context, req *v1.ProjectMember err := s.projectMemberStore.Delete(ctx, projectMember.Meta.Id) return projectMember.NewProjectMemberResponse(), err } + func (s *projectMemberService) Get(ctx context.Context, req *v1.ProjectMemberGetRequest) (*v1.ProjectMemberResponse, error) { projectMember, err := s.projectMemberStore.Get(ctx, req.Id) if err != nil { @@ -83,6 +84,7 @@ func (s *projectMemberService) Get(ctx context.Context, req *v1.ProjectMemberGet } return projectMember.NewProjectMemberResponse(), nil } + func (s *projectMemberService) Find(ctx context.Context, req *v1.ProjectMemberFindRequest) (*v1.ProjectMemberListResponse, error) { filter := map[string]any{ "COALESCE(projectmember ->> 'namespace', '')": req.Namespace, @@ -101,11 +103,14 @@ func (s *projectMemberService) Find(ctx context.Context, req *v1.ProjectMemberFi f := fmt.Sprintf("projectmember -> 'meta' -> 'annotations' ->> '%s'", key) filter[f] = value } + res, _, err := s.projectMemberStore.Find(ctx, nil, filter) if err != nil { return nil, err } + resp := new(v1.ProjectMemberListResponse) resp.ProjectMembers = append(resp.ProjectMembers, res...) + return resp, nil } diff --git a/pkg/service/projectmember_test.go b/pkg/service/projectmember_test.go index 390d6df..310d831 100644 --- a/pkg/service/projectmember_test.go +++ b/pkg/service/projectmember_test.go @@ -3,15 +3,21 @@ package service import ( "context" "log/slog" + "slices" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" v1 "github.com/metal-stack/masterdata-api/api/v1" "github.com/metal-stack/metal-lib/pkg/pointer" + "github.com/metal-stack/metal-lib/pkg/testcommon" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + "google.golang.org/protobuf/runtime/protoimpl" "testing" + "github.com/metal-stack/masterdata-api/pkg/datastore" "github.com/metal-stack/masterdata-api/pkg/test/mocks" ) @@ -189,3 +195,272 @@ func TestFindProjectMemberByTenant(t *testing.T) { require.NoError(t, err) assert.NotNil(t, resp) } + +func TestFindProjectMember(t *testing.T) { + ctx := t.Context() + ves := []datastore.Entity{ + &v1.Project{}, + &v1.ProjectMember{}, + &v1.Tenant{}, + &v1.TenantMember{}, + } + + container, db, err := StartPostgres(ctx, ves...) + require.NoError(t, err) + defer func() { + require.NoError(t, db.Close()) + require.NoError(t, container.Terminate(ctx)) + }() + + var ( + projectMemberStore = datastore.New(log, db, &v1.ProjectMember{}) + projectStore = datastore.New(log, db, &v1.Project{}) + tenantStore = datastore.New(log, db, &v1.Tenant{}) + + testTenant1 = &v1.Tenant{ + Meta: &v1.Meta{ + Id: "tenant-1", + Kind: "Tenant", + Apiversion: "v1", + Version: 1, + }, + Name: "tenant 1", + Description: "test tenant 1", + } + testTenant2 = &v1.Tenant{ + Meta: &v1.Meta{ + Id: "tenant-2", + Kind: "Tenant", + Apiversion: "v1", + Version: 1, + }, + Name: "tenant 2", + Description: "test tenant 2", + } + testProject1 = &v1.Project{ + Meta: &v1.Meta{ + Id: "project-1", + Kind: "Project", + Apiversion: "v1", + Version: 1, + }, + Name: "project 1", + Description: "test project 1", + TenantId: "tenant-1", + } + testProjectMember1 = &v1.ProjectMember{ + Meta: &v1.Meta{ + Id: "1", + Kind: "ProjectMember", + Apiversion: "v1", + Version: 1, + Annotations: map[string]string{ + "role": "owner", + }, + Labels: []string{"a", "b"}, + }, + ProjectId: "project-1", + TenantId: "tenant-1", + Namespace: "a", + } + testProjectMember2 = &v1.ProjectMember{ + Meta: &v1.Meta{ + Id: "2", + Kind: "ProjectMember", + Apiversion: "v1", + Version: 1, + Annotations: map[string]string{ + "role": "viewer", + }, + Labels: []string{"c", "d"}, + }, + ProjectId: "project-1", + TenantId: "tenant-2", + Namespace: "a", + } + testProjectMember3 = &v1.ProjectMember{ + Meta: &v1.Meta{ + Id: "3", + Kind: "ProjectMember", + Apiversion: "v1", + Version: 1, + Annotations: map[string]string{ + "role": "owner", + }, + Labels: []string{"e", "f"}, + }, + ProjectId: "project-2", + TenantId: "tenant-2", + Namespace: "a", + } + testProjectMember4 = &v1.ProjectMember{ + Meta: &v1.Meta{ + Id: "4", + Kind: "ProjectMember", + Apiversion: "v1", + Version: 1, + Annotations: map[string]string{ + "role": "owner", + }, + }, + ProjectId: "project-2", + TenantId: "tenant-2", + Namespace: "", + } + + service = &projectMemberService{ + log: log, + projectMemberStore: projectMemberStore, + tenantStore: tenantStore, + projectStore: projectStore, + } + ) + + tests := []struct { + name string + prepare func() + req *v1.ProjectMemberFindRequest + want *v1.ProjectMemberListResponse + wantErr error + }{ + { + name: "find by project", + req: &v1.ProjectMemberFindRequest{ + ProjectId: pointer.Pointer("project-1"), + Namespace: "a", + }, + prepare: func() { + require.NoError(t, tenantStore.Create(ctx, testTenant1)) + require.NoError(t, tenantStore.Create(ctx, testTenant2)) + require.NoError(t, projectStore.Create(ctx, testProject1)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember1)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember2)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember3)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember4)) + }, + want: &v1.ProjectMemberListResponse{ + ProjectMembers: []*v1.ProjectMember{ + testProjectMember1, + testProjectMember2, + }, + }, + wantErr: nil, + }, + { + name: "find by project id (no results) #1", + req: &v1.ProjectMemberFindRequest{ + ProjectId: pointer.Pointer("no-result"), + Namespace: "a", + }, + prepare: func() { + require.NoError(t, tenantStore.Create(ctx, testTenant1)) + require.NoError(t, tenantStore.Create(ctx, testTenant2)) + require.NoError(t, projectStore.Create(ctx, testProject1)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember1)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember2)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember3)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember4)) + }, + want: &v1.ProjectMemberListResponse{ + ProjectMembers: nil, + }, + wantErr: nil, + }, + { + name: "find by project id (no results) #2", + req: &v1.ProjectMemberFindRequest{ + ProjectId: pointer.Pointer("project-1"), + Namespace: "wrong-namespace", + }, + prepare: func() { + require.NoError(t, tenantStore.Create(ctx, testTenant1)) + require.NoError(t, tenantStore.Create(ctx, testTenant2)) + require.NoError(t, projectStore.Create(ctx, testProject1)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember1)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember2)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember3)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember4)) + }, + want: &v1.ProjectMemberListResponse{ + ProjectMembers: nil, + }, + wantErr: nil, + }, + { + name: "find by tenant", + req: &v1.ProjectMemberFindRequest{ + TenantId: pointer.Pointer("tenant-2"), + Namespace: "a", + }, + prepare: func() { + require.NoError(t, tenantStore.Create(ctx, testTenant1)) + require.NoError(t, tenantStore.Create(ctx, testTenant2)) + require.NoError(t, projectStore.Create(ctx, testProject1)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember1)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember2)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember3)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember4)) + }, + want: &v1.ProjectMemberListResponse{ + ProjectMembers: []*v1.ProjectMember{ + testProjectMember2, + testProjectMember3, + }, + }, + wantErr: nil, + }, + { + name: "find by annotation", + req: &v1.ProjectMemberFindRequest{ + Annotations: map[string]string{"role": "owner"}, + Namespace: "a", + }, + prepare: func() { + require.NoError(t, tenantStore.Create(ctx, testTenant1)) + require.NoError(t, tenantStore.Create(ctx, testTenant2)) + require.NoError(t, projectStore.Create(ctx, testProject1)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember1)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember2)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember3)) + require.NoError(t, projectMemberStore.Create(ctx, testProjectMember4)) + }, + want: &v1.ProjectMemberListResponse{ + ProjectMembers: []*v1.ProjectMember{ + testProjectMember1, + testProjectMember3, + }, + }, + wantErr: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for _, ve := range ves { + _, err := db.ExecContext(ctx, "TRUNCATE TABLE "+ve.TableName()) + require.NoError(t, err) + } + + if tt.prepare != nil { + tt.prepare() + } + + got, err := service.Find(ctx, tt.req) + if diff := cmp.Diff(err, tt.wantErr); diff != "" { + t.Errorf("(-want +got):\n%s", diff) + return + } + + slices.SortFunc(got.ProjectMembers, func(i, j *v1.ProjectMember) int { + if i.Meta.Id < j.Meta.Id { + return -1 + } else { + return 1 + } + }) + + if diff := cmp.Diff(tt.want, got, cmpopts.IgnoreTypes(protoimpl.MessageState{}), cmpopts.IgnoreFields(v1.Meta{}, "CreatedTime"), testcommon.IgnoreUnexported()); diff != "" { + t.Errorf("(-want +got):\n%s", diff) + } + }) + } +} diff --git a/pkg/service/tenantmember.go b/pkg/service/tenantmember.go index f719cb9..aa6798a 100644 --- a/pkg/service/tenantmember.go +++ b/pkg/service/tenantmember.go @@ -51,6 +51,7 @@ func (s *tenantMemberService) Create(ctx context.Context, req *v1.TenantMemberCr err = s.tenantMemberStore.Create(ctx, tenantMember) return tenantMember.NewTenantMemberResponse(), err } + func (s *tenantMemberService) Update(ctx context.Context, req *v1.TenantMemberUpdateRequest) (*v1.TenantMemberResponse, error) { tenantMember := req.TenantMember @@ -67,22 +68,27 @@ func (s *tenantMemberService) Update(ctx context.Context, req *v1.TenantMemberUp return tenantMember.NewTenantMemberResponse(), err } + func (s *tenantMemberService) Delete(ctx context.Context, req *v1.TenantMemberDeleteRequest) (*v1.TenantMemberResponse, error) { tenantMember := req.NewTenantMember() err := s.tenantMemberStore.Delete(ctx, tenantMember.Meta.Id) return tenantMember.NewTenantMemberResponse(), err } + func (s *tenantMemberService) Get(ctx context.Context, req *v1.TenantMemberGetRequest) (*v1.TenantMemberResponse, error) { tenantMember, err := s.tenantMemberStore.Get(ctx, req.Id) if err != nil { return nil, err } + return tenantMember.NewTenantMemberResponse(), nil } + func (s *tenantMemberService) Find(ctx context.Context, req *v1.TenantMemberFindRequest) (*v1.TenantMemberListResponse, error) { filter := map[string]any{ "COALESCE(tenantmember ->> 'namespace', '')": req.Namespace, } + if req.TenantId != nil { filter["tenantmember ->> 'tenant_id'"] = req.TenantId } @@ -94,11 +100,14 @@ func (s *tenantMemberService) Find(ctx context.Context, req *v1.TenantMemberFind f := fmt.Sprintf("tenantmember -> 'meta' -> 'annotations' ->> '%s'", key) filter[f] = value } + res, _, err := s.tenantMemberStore.Find(ctx, nil, filter) if err != nil { return nil, err } + resp := new(v1.TenantMemberListResponse) resp.TenantMembers = append(resp.TenantMembers, res...) + return resp, nil } diff --git a/pkg/service/tenantmember_test.go b/pkg/service/tenantmember_test.go index 70d3b2b..f47dc20 100644 --- a/pkg/service/tenantmember_test.go +++ b/pkg/service/tenantmember_test.go @@ -3,15 +3,21 @@ package service import ( "context" "log/slog" + "slices" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" v1 "github.com/metal-stack/masterdata-api/api/v1" "github.com/metal-stack/metal-lib/pkg/pointer" + "github.com/metal-stack/metal-lib/pkg/testcommon" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + "google.golang.org/protobuf/runtime/protoimpl" "testing" + "github.com/metal-stack/masterdata-api/pkg/datastore" "github.com/metal-stack/masterdata-api/pkg/test/mocks" ) @@ -177,3 +183,252 @@ func TestFindTenantMemberByMember(t *testing.T) { require.NoError(t, err) assert.NotNil(t, resp) } + +func TestFindTenantMember(t *testing.T) { + ctx := t.Context() + ves := []datastore.Entity{ + &v1.Tenant{}, + &v1.TenantMember{}, + } + + container, db, err := StartPostgres(ctx, ves...) + require.NoError(t, err) + defer func() { + require.NoError(t, db.Close()) + require.NoError(t, container.Terminate(ctx)) + }() + + var ( + tenantMemberStore = datastore.New(log, db, &v1.TenantMember{}) + tenantStore = datastore.New(log, db, &v1.Tenant{}) + + testTenant1 = &v1.Tenant{ + Meta: &v1.Meta{ + Id: "tenant-1", + Kind: "Tenant", + Apiversion: "v1", + Version: 1, + }, + Name: "tenant 1", + Description: "test tenant 1", + } + testTenant2 = &v1.Tenant{ + Meta: &v1.Meta{ + Id: "tenant-2", + Kind: "Tenant", + Apiversion: "v1", + Version: 1, + }, + Name: "tenant 2", + Description: "test tenant 2", + } + testTenantMember1 = &v1.TenantMember{ + Meta: &v1.Meta{ + Id: "1", + Kind: "TenantMember", + Apiversion: "v1", + Version: 1, + Annotations: map[string]string{ + "role": "owner", + }, + Labels: []string{"a", "b"}, + }, + TenantId: "tenant-1", + MemberId: "tenant-1", + Namespace: "a", + } + testTenantMember2 = &v1.TenantMember{ + Meta: &v1.Meta{ + Id: "2", + Kind: "TenantMember", + Apiversion: "v1", + Version: 1, + Annotations: map[string]string{ + "role": "owner", + }, + Labels: []string{"c", "d"}, + }, + TenantId: "tenant-2", + MemberId: "tenant-2", + Namespace: "a", + } + testTenantMember3 = &v1.TenantMember{ + Meta: &v1.Meta{ + Id: "3", + Kind: "TenantMember", + Apiversion: "v1", + Version: 1, + Annotations: map[string]string{ + "role": "editor", + }, + Labels: []string{"e", "f"}, + }, + TenantId: "tenant-1", + MemberId: "tenant-2", + Namespace: "a", + } + testTenantMember4 = &v1.TenantMember{ + Meta: &v1.Meta{ + Id: "4", + Kind: "TenantMember", + Apiversion: "v1", + Version: 1, + Annotations: map[string]string{ + "role": "editor", + }, + Labels: []string{"e", "f"}, + }, + TenantId: "tenant-1", + MemberId: "tenant-2", + Namespace: "", + } + + service = &tenantMemberService{ + log: log, + tenantMemberStore: tenantMemberStore, + tenantStore: tenantStore, + } + ) + + tests := []struct { + name string + prepare func() + req *v1.TenantMemberFindRequest + want *v1.TenantMemberListResponse + wantErr error + }{ + { + name: "find by tenant", + req: &v1.TenantMemberFindRequest{ + TenantId: pointer.Pointer("tenant-1"), + Namespace: "a", + }, + prepare: func() { + require.NoError(t, tenantStore.Create(ctx, testTenant1)) + require.NoError(t, tenantStore.Create(ctx, testTenant2)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember1)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember2)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember3)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember4)) + }, + want: &v1.TenantMemberListResponse{ + TenantMembers: []*v1.TenantMember{ + testTenantMember1, + testTenantMember3, + }, + }, + wantErr: nil, + }, + { + name: "find by tenant id (no results) #1", + req: &v1.TenantMemberFindRequest{ + TenantId: pointer.Pointer("no-result"), + Namespace: "a", + }, + prepare: func() { + require.NoError(t, tenantStore.Create(ctx, testTenant1)) + require.NoError(t, tenantStore.Create(ctx, testTenant2)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember1)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember2)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember3)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember4)) + }, + want: &v1.TenantMemberListResponse{ + TenantMembers: nil, + }, + wantErr: nil, + }, + { + name: "find by tenant id (no results) #2", + req: &v1.TenantMemberFindRequest{ + TenantId: pointer.Pointer("tenant-1"), + Namespace: "wrong-namespace", + }, + prepare: func() { + require.NoError(t, tenantStore.Create(ctx, testTenant1)) + require.NoError(t, tenantStore.Create(ctx, testTenant2)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember1)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember2)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember3)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember4)) + }, + want: &v1.TenantMemberListResponse{ + TenantMembers: nil, + }, + wantErr: nil, + }, + { + name: "find by tenant", + req: &v1.TenantMemberFindRequest{ + TenantId: pointer.Pointer("tenant-2"), + Namespace: "a", + }, + prepare: func() { + require.NoError(t, tenantStore.Create(ctx, testTenant1)) + require.NoError(t, tenantStore.Create(ctx, testTenant2)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember1)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember2)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember3)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember4)) + }, + want: &v1.TenantMemberListResponse{ + TenantMembers: []*v1.TenantMember{ + testTenantMember2, + }, + }, + wantErr: nil, + }, + { + name: "find by annotation", + req: &v1.TenantMemberFindRequest{ + Annotations: map[string]string{"role": "owner"}, + Namespace: "a", + }, + prepare: func() { + require.NoError(t, tenantStore.Create(ctx, testTenant1)) + require.NoError(t, tenantStore.Create(ctx, testTenant2)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember1)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember2)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember3)) + require.NoError(t, tenantMemberStore.Create(ctx, testTenantMember4)) + }, + want: &v1.TenantMemberListResponse{ + TenantMembers: []*v1.TenantMember{ + testTenantMember1, + testTenantMember2, + }, + }, + wantErr: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for _, ve := range ves { + _, err := db.ExecContext(ctx, "TRUNCATE TABLE "+ve.TableName()) + require.NoError(t, err) + } + + if tt.prepare != nil { + tt.prepare() + } + + got, err := service.Find(ctx, tt.req) + if diff := cmp.Diff(err, tt.wantErr); diff != "" { + t.Errorf("(-want +got):\n%s", diff) + return + } + + slices.SortFunc(got.TenantMembers, func(i, j *v1.TenantMember) int { + if i.Meta.Id < j.Meta.Id { + return -1 + } else { + return 1 + } + }) + + if diff := cmp.Diff(tt.want, got, cmpopts.IgnoreTypes(protoimpl.MessageState{}), cmpopts.IgnoreFields(v1.Meta{}, "CreatedTime"), testcommon.IgnoreUnexported()); diff != "" { + t.Errorf("(-want +got):\n%s", diff) + } + }) + } +} From e07e836321421e4541eadb8db77e6dca48967cf1 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Wed, 10 Sep 2025 11:34:29 +0200 Subject: [PATCH 11/16] Add tests for update and prevent some field updates on members. --- pkg/service/projectmember.go | 6 + pkg/service/projectmember_test.go | 296 ++++++++++++++++++++---------- pkg/service/tenantmember.go | 6 + pkg/service/tenantmember_test.go | 286 ++++++++++++++++++++--------- 4 files changed, 414 insertions(+), 180 deletions(-) diff --git a/pkg/service/projectmember.go b/pkg/service/projectmember.go index d033199..ae9f19a 100644 --- a/pkg/service/projectmember.go +++ b/pkg/service/projectmember.go @@ -62,6 +62,12 @@ func (s *projectMemberService) Update(ctx context.Context, req *v1.ProjectMember return nil, err } + if old.ProjectId != projectMember.ProjectId { + return nil, status.Error(codes.InvalidArgument, "updating the project id of a project member is not allowed") + } + if old.TenantId != projectMember.TenantId { + return nil, status.Error(codes.InvalidArgument, "updating the tenant id of a project member is not allowed") + } if old.Namespace != projectMember.Namespace { return nil, status.Error(codes.InvalidArgument, "updating the namespace of a project member is not allowed") } diff --git a/pkg/service/projectmember_test.go b/pkg/service/projectmember_test.go index 310d831..5765820 100644 --- a/pkg/service/projectmember_test.go +++ b/pkg/service/projectmember_test.go @@ -11,8 +11,9 @@ import ( "github.com/metal-stack/metal-lib/pkg/pointer" "github.com/metal-stack/metal-lib/pkg/testcommon" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "google.golang.org/protobuf/runtime/protoimpl" "testing" @@ -52,42 +53,6 @@ func TestCreateProjectMember(t *testing.T) { assert.Equal(t, pmcr.ProjectMember.ProjectId, resp.GetProjectMember().GetProjectId()) } -func TestUpdateProjectMember(t *testing.T) { - storageMock := mocks.NewMockStorage[*v1.ProjectMember](t) - tenantStorageMock := mocks.NewMockStorage[*v1.Tenant](t) - projectStorageMock := mocks.NewMockStorage[*v1.Project](t) - ts := &projectMemberService{ - projectMemberStore: storageMock, - tenantStore: tenantStorageMock, - projectStore: projectStorageMock, - log: slog.Default(), - } - ctx := context.Background() - - meta := &v1.Meta{Id: "p2", Annotations: map[string]string{"key": "value"}} - pm1 := &v1.ProjectMember{ - Meta: meta, - ProjectId: "p1", - TenantId: "t1", - } - meta.Annotations = map[string]string{"key": "value2"} - pmur := &v1.ProjectMemberUpdateRequest{ - ProjectMember: &v1.ProjectMember{ - Meta: meta, - ProjectId: "p1", - TenantId: "t1", - }, - } - - storageMock.On("Get", ctx, pm1.Meta.Id).Return(pm1, nil) - storageMock.On("Update", ctx, pm1).Return(nil) - resp, err := ts.Update(ctx, pmur) - require.NoError(t, err) - assert.NotNil(t, resp) - assert.NotNil(t, resp.GetProjectMember()) - assert.Equal(t, pmur.GetProjectMember().Meta.Annotations, resp.GetProjectMember().Meta.Annotations) -} - func TestDeleteProjectMember(t *testing.T) { storageMock := mocks.NewMockStorage[*v1.ProjectMember](t) tenantStorageMock := mocks.NewMockStorage[*v1.Tenant](t) @@ -140,62 +105,6 @@ func TestGetProjectMember(t *testing.T) { assert.Equal(t, tgr.Id, resp.GetProjectMember().GetMeta().GetId()) } -func TestFindProjectMemberByProject(t *testing.T) { - storageMock := mocks.NewMockStorage[*v1.ProjectMember](t) - tenantStorageMock := mocks.NewMockStorage[*v1.Tenant](t) - projectStorageMock := mocks.NewMockStorage[*v1.Project](t) - ts := &projectMemberService{ - projectMemberStore: storageMock, - tenantStore: tenantStorageMock, - projectStore: projectStorageMock, - log: slog.Default(), - } - ctx := context.Background() - - // filter by name - var t6s []*v1.ProjectMember - tfr := &v1.ProjectMemberFindRequest{ - ProjectId: pointer.Pointer("p1"), - Namespace: "a", - } - - f2 := make(map[string]any) - f2["projectmember ->> 'project_id'"] = pointer.Pointer("p1") - f2["COALESCE(projectmember ->> 'namespace', '')"] = "a" - storageMock.On("Find", ctx, mock.AnythingOfType("*v1.Paging"), []any{f2}).Return(t6s, nil, nil) - resp, err := ts.Find(ctx, tfr) - require.NoError(t, err) - assert.NotNil(t, resp) -} - -func TestFindProjectMemberByTenant(t *testing.T) { - storageMock := mocks.NewMockStorage[*v1.ProjectMember](t) - tenantStorageMock := mocks.NewMockStorage[*v1.Tenant](t) - projectStorageMock := mocks.NewMockStorage[*v1.Project](t) - ts := &projectMemberService{ - projectMemberStore: storageMock, - tenantStore: tenantStorageMock, - projectStore: projectStorageMock, - log: slog.Default(), - } - ctx := context.Background() - - // filter by name - var t6s []*v1.ProjectMember - tfr := &v1.ProjectMemberFindRequest{ - TenantId: pointer.Pointer("t1"), - Namespace: "a", - } - - f2 := make(map[string]any) - f2["projectmember ->> 'tenant_id'"] = pointer.Pointer("t1") - f2["COALESCE(projectmember ->> 'namespace', '')"] = "a" - storageMock.On("Find", ctx, mock.AnythingOfType("*v1.Paging"), []any{f2}).Return(t6s, nil, nil) - resp, err := ts.Find(ctx, tfr) - require.NoError(t, err) - assert.NotNil(t, resp) -} - func TestFindProjectMember(t *testing.T) { ctx := t.Context() ves := []datastore.Entity{ @@ -464,3 +373,204 @@ func TestFindProjectMember(t *testing.T) { }) } } + +func TestUpdateProjectMember(t *testing.T) { + ctx := t.Context() + ves := []datastore.Entity{ + &v1.Project{}, + &v1.ProjectMember{}, + &v1.Tenant{}, + &v1.TenantMember{}, + } + + container, db, err := StartPostgres(ctx, ves...) + require.NoError(t, err) + defer func() { + require.NoError(t, db.Close()) + require.NoError(t, container.Terminate(ctx)) + }() + + var ( + projectMemberStore = datastore.New(log, db, &v1.ProjectMember{}) + projectStore = datastore.New(log, db, &v1.Project{}) + tenantStore = datastore.New(log, db, &v1.Tenant{}) + + service = &projectMemberService{ + log: log, + projectMemberStore: projectMemberStore, + tenantStore: tenantStore, + projectStore: projectStore, + } + ) + + tests := []struct { + name string + prepare func() + req *v1.ProjectMemberUpdateRequest + want *v1.ProjectMemberResponse + wantErr error + }{ + { + name: "update mutable fields", + req: &v1.ProjectMemberUpdateRequest{ + ProjectMember: &v1.ProjectMember{ + Meta: &v1.Meta{ + Id: "1", + Version: 1, + Annotations: map[string]string{ + "role": "owner", + }, + Labels: []string{"a", "b"}, + }, + ProjectId: "project-1", + TenantId: "tenant-1", + Namespace: "a", + }, + }, + prepare: func() { + require.NoError(t, projectMemberStore.Create(ctx, &v1.ProjectMember{ + Meta: &v1.Meta{ + Id: "1", + Kind: "ProjectMember", + Apiversion: "v1", + Version: 1, + }, + ProjectId: "project-1", + TenantId: "tenant-1", + Namespace: "a", + })) + }, + want: &v1.ProjectMemberResponse{ + ProjectMember: &v1.ProjectMember{ + Meta: &v1.Meta{ + Id: "1", + Kind: "ProjectMember", + Apiversion: "v1", + Version: 2, + Annotations: map[string]string{ + "role": "owner", + }, + Labels: []string{"a", "b"}, + }, + ProjectId: "project-1", + TenantId: "tenant-1", + Namespace: "a", + }, + }, + wantErr: nil, + }, + { + name: "unable to update namespace", + req: &v1.ProjectMemberUpdateRequest{ + ProjectMember: &v1.ProjectMember{ + Meta: &v1.Meta{ + Id: "1", + Version: 1, + }, + ProjectId: "project-1", + TenantId: "tenant-1", + Namespace: "b", + }, + }, + prepare: func() { + require.NoError(t, projectMemberStore.Create(ctx, &v1.ProjectMember{ + Meta: &v1.Meta{ + Id: "1", + Kind: "ProjectMember", + Apiversion: "v1", + Version: 1, + }, + ProjectId: "project-1", + TenantId: "tenant-1", + Namespace: "a", + })) + }, + want: nil, + wantErr: status.Error(codes.InvalidArgument, "updating the namespace of a project member is not allowed"), + }, + { + name: "unable to update project", + req: &v1.ProjectMemberUpdateRequest{ + ProjectMember: &v1.ProjectMember{ + Meta: &v1.Meta{ + Id: "1", + Version: 1, + }, + ProjectId: "project-2", + TenantId: "tenant-1", + Namespace: "a", + }, + }, + prepare: func() { + require.NoError(t, projectMemberStore.Create(ctx, &v1.ProjectMember{ + Meta: &v1.Meta{ + Id: "1", + Kind: "ProjectMember", + Apiversion: "v1", + Version: 1, + }, + ProjectId: "project-1", + TenantId: "tenant-1", + Namespace: "a", + })) + }, + want: nil, + wantErr: status.Error(codes.InvalidArgument, "updating the project id of a project member is not allowed"), + }, + { + name: "unable to update tenant", + req: &v1.ProjectMemberUpdateRequest{ + ProjectMember: &v1.ProjectMember{ + Meta: &v1.Meta{ + Id: "1", + Version: 1, + }, + ProjectId: "project-1", + TenantId: "tenant-2", + Namespace: "a", + }, + }, + prepare: func() { + require.NoError(t, projectMemberStore.Create(ctx, &v1.ProjectMember{ + Meta: &v1.Meta{ + Id: "1", + Kind: "ProjectMember", + Apiversion: "v1", + Version: 1, + }, + ProjectId: "project-1", + TenantId: "tenant-1", + Namespace: "a", + })) + }, + want: nil, + wantErr: status.Error(codes.InvalidArgument, "updating the tenant id of a project member is not allowed"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for _, ve := range ves { + _, err := db.ExecContext(ctx, "TRUNCATE TABLE "+ve.TableName()) + require.NoError(t, err) + } + + if tt.prepare != nil { + tt.prepare() + } + + got, err := service.Update(ctx, tt.req) + if diff := cmp.Diff(err, tt.wantErr, cmpopts.EquateErrors()); diff != "" { + t.Errorf("(-want +got):\n%s", diff) + return + } + + if err == nil { + assert.NotNil(t, got.ProjectMember.Meta.UpdatedTime) + } + + if diff := cmp.Diff(tt.want, got, cmpopts.IgnoreTypes(protoimpl.MessageState{}), cmpopts.IgnoreFields(v1.Meta{}, "CreatedTime", "UpdatedTime"), testcommon.IgnoreUnexported()); diff != "" { + t.Errorf("(-want +got):\n%s", diff) + } + }) + } +} diff --git a/pkg/service/tenantmember.go b/pkg/service/tenantmember.go index aa6798a..3a0a019 100644 --- a/pkg/service/tenantmember.go +++ b/pkg/service/tenantmember.go @@ -60,6 +60,12 @@ func (s *tenantMemberService) Update(ctx context.Context, req *v1.TenantMemberUp return nil, err } + if old.TenantId != tenantMember.TenantId { + return nil, status.Error(codes.InvalidArgument, "updating the tenant id of a tenant member is not allowed") + } + if old.MemberId != tenantMember.MemberId { + return nil, status.Error(codes.InvalidArgument, "updating the member id of a tenant member is not allowed") + } if old.Namespace != tenantMember.Namespace { return nil, status.Error(codes.InvalidArgument, "updating the namespace of a tenant member is not allowed") } diff --git a/pkg/service/tenantmember_test.go b/pkg/service/tenantmember_test.go index f47dc20..ebd5207 100644 --- a/pkg/service/tenantmember_test.go +++ b/pkg/service/tenantmember_test.go @@ -11,8 +11,9 @@ import ( "github.com/metal-stack/metal-lib/pkg/pointer" "github.com/metal-stack/metal-lib/pkg/testcommon" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "google.golang.org/protobuf/runtime/protoimpl" "testing" @@ -50,40 +51,6 @@ func TestCreateTenantMember(t *testing.T) { assert.Equal(t, pmcr.TenantMember.TenantId, resp.GetTenantMember().GetTenantId()) } -func TestUpdateTenantMember(t *testing.T) { - storageMock := mocks.NewMockStorage[*v1.TenantMember](t) - tenantStorageMock := mocks.NewMockStorage[*v1.Tenant](t) - ts := &tenantMemberService{ - tenantMemberStore: storageMock, - tenantStore: tenantStorageMock, - log: slog.Default(), - } - ctx := context.Background() - - meta := &v1.Meta{Id: "p2", Annotations: map[string]string{"key": "value"}} - pm1 := &v1.TenantMember{ - Meta: meta, - TenantId: "p1", - MemberId: "t1", - } - meta.Annotations = map[string]string{"key": "value2"} - pmur := &v1.TenantMemberUpdateRequest{ - TenantMember: &v1.TenantMember{ - Meta: meta, - TenantId: "p1", - MemberId: "t1", - }, - } - - storageMock.On("Get", ctx, pm1.Meta.Id).Return(pm1, nil) - storageMock.On("Update", ctx, pm1).Return(nil) - resp, err := ts.Update(ctx, pmur) - require.NoError(t, err) - assert.NotNil(t, resp) - assert.NotNil(t, resp.GetTenantMember()) - assert.Equal(t, pmur.GetTenantMember().Meta.Annotations, resp.GetTenantMember().Meta.Annotations) -} - func TestDeleteTenantMember(t *testing.T) { storageMock := mocks.NewMockStorage[*v1.TenantMember](t) tenantStorageMock := mocks.NewMockStorage[*v1.Tenant](t) @@ -132,58 +99,6 @@ func TestGetTenantMember(t *testing.T) { assert.Equal(t, tgr.Id, resp.GetTenantMember().GetMeta().GetId()) } -func TestFindTenantMemberByTenant(t *testing.T) { - storageMock := mocks.NewMockStorage[*v1.TenantMember](t) - tenantStorageMock := mocks.NewMockStorage[*v1.Tenant](t) - ts := &tenantMemberService{ - tenantMemberStore: storageMock, - tenantStore: tenantStorageMock, - log: slog.Default(), - } - ctx := context.Background() - - // filter by name - var t6s []*v1.TenantMember - tfr := &v1.TenantMemberFindRequest{ - TenantId: pointer.Pointer("p1"), - Namespace: "a", - } - - f2 := make(map[string]any) - f2["tenantmember ->> 'tenant_id'"] = pointer.Pointer("p1") - f2["COALESCE(tenantmember ->> 'namespace', '')"] = "a" - storageMock.On("Find", ctx, mock.AnythingOfType("*v1.Paging"), []any{f2}).Return(t6s, nil, nil) - resp, err := ts.Find(ctx, tfr) - require.NoError(t, err) - assert.NotNil(t, resp) -} - -func TestFindTenantMemberByMember(t *testing.T) { - storageMock := mocks.NewMockStorage[*v1.TenantMember](t) - tenantStorageMock := mocks.NewMockStorage[*v1.Tenant](t) - ts := &tenantMemberService{ - tenantMemberStore: storageMock, - tenantStore: tenantStorageMock, - log: slog.Default(), - } - ctx := context.Background() - - // filter by name - var t6s []*v1.TenantMember - tfr := &v1.TenantMemberFindRequest{ - MemberId: pointer.Pointer("t1"), - Namespace: "a", - } - - f2 := make(map[string]any) - f2["tenantmember ->> 'member_id'"] = pointer.Pointer("t1") - f2["COALESCE(tenantmember ->> 'namespace', '')"] = "a" - storageMock.On("Find", ctx, mock.AnythingOfType("*v1.Paging"), []any{f2}).Return(t6s, nil, nil) - resp, err := ts.Find(ctx, tfr) - require.NoError(t, err) - assert.NotNil(t, resp) -} - func TestFindTenantMember(t *testing.T) { ctx := t.Context() ves := []datastore.Entity{ @@ -432,3 +347,200 @@ func TestFindTenantMember(t *testing.T) { }) } } + +func TestUpdateTenantMember(t *testing.T) { + ctx := t.Context() + ves := []datastore.Entity{ + &v1.Tenant{}, + &v1.TenantMember{}, + } + + container, db, err := StartPostgres(ctx, ves...) + require.NoError(t, err) + defer func() { + require.NoError(t, db.Close()) + require.NoError(t, container.Terminate(ctx)) + }() + + var ( + tenantMemberStore = datastore.New(log, db, &v1.TenantMember{}) + tenantStore = datastore.New(log, db, &v1.Tenant{}) + + service = &tenantMemberService{ + log: log, + tenantMemberStore: tenantMemberStore, + tenantStore: tenantStore, + } + ) + + tests := []struct { + name string + prepare func() + req *v1.TenantMemberUpdateRequest + want *v1.TenantMemberResponse + wantErr error + }{ + { + name: "update mutable fields", + req: &v1.TenantMemberUpdateRequest{ + TenantMember: &v1.TenantMember{ + Meta: &v1.Meta{ + Id: "1", + Version: 1, + Annotations: map[string]string{ + "role": "owner", + }, + Labels: []string{"a", "b"}, + }, + TenantId: "tenant-1", + MemberId: "tenant-1", + Namespace: "a", + }, + }, + prepare: func() { + require.NoError(t, tenantMemberStore.Create(ctx, &v1.TenantMember{ + Meta: &v1.Meta{ + Id: "1", + Kind: "TenantMember", + Apiversion: "v1", + Version: 1, + }, + TenantId: "tenant-1", + MemberId: "tenant-1", + Namespace: "a", + })) + }, + want: &v1.TenantMemberResponse{ + TenantMember: &v1.TenantMember{ + Meta: &v1.Meta{ + Id: "1", + Kind: "TenantMember", + Apiversion: "v1", + Version: 2, + Annotations: map[string]string{ + "role": "owner", + }, + Labels: []string{"a", "b"}, + }, + TenantId: "tenant-1", + MemberId: "tenant-1", + Namespace: "a", + }, + }, + wantErr: nil, + }, + { + name: "unable to update namespace", + req: &v1.TenantMemberUpdateRequest{ + TenantMember: &v1.TenantMember{ + Meta: &v1.Meta{ + Id: "1", + Version: 1, + }, + TenantId: "tenant-1", + MemberId: "tenant-1", + Namespace: "b", + }, + }, + prepare: func() { + require.NoError(t, tenantMemberStore.Create(ctx, &v1.TenantMember{ + Meta: &v1.Meta{ + Id: "1", + Kind: "TenantMember", + Apiversion: "v1", + Version: 1, + }, + TenantId: "tenant-1", + MemberId: "tenant-1", + Namespace: "a", + })) + }, + want: nil, + wantErr: status.Error(codes.InvalidArgument, "updating the namespace of a tenant member is not allowed"), + }, + { + name: "unable to update tenant id", + req: &v1.TenantMemberUpdateRequest{ + TenantMember: &v1.TenantMember{ + Meta: &v1.Meta{ + Id: "1", + Version: 1, + }, + TenantId: "tenant-2", + MemberId: "tenant-1", + Namespace: "a", + }, + }, + prepare: func() { + require.NoError(t, tenantMemberStore.Create(ctx, &v1.TenantMember{ + Meta: &v1.Meta{ + Id: "1", + Kind: "TenantMember", + Apiversion: "v1", + Version: 1, + }, + TenantId: "tenant-1", + MemberId: "tenant-1", + Namespace: "a", + })) + }, + want: nil, + wantErr: status.Error(codes.InvalidArgument, "updating the tenant id of a tenant member is not allowed"), + }, + { + name: "unable to update member id", + req: &v1.TenantMemberUpdateRequest{ + TenantMember: &v1.TenantMember{ + Meta: &v1.Meta{ + Id: "1", + Version: 1, + }, + TenantId: "tenant-1", + MemberId: "tenant-2", + Namespace: "a", + }, + }, + prepare: func() { + require.NoError(t, tenantMemberStore.Create(ctx, &v1.TenantMember{ + Meta: &v1.Meta{ + Id: "1", + Kind: "TenantMember", + Apiversion: "v1", + Version: 1, + }, + TenantId: "tenant-1", + MemberId: "tenant-1", + Namespace: "a", + })) + }, + want: nil, + wantErr: status.Error(codes.InvalidArgument, "updating the member id of a tenant member is not allowed"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for _, ve := range ves { + _, err := db.ExecContext(ctx, "TRUNCATE TABLE "+ve.TableName()) + require.NoError(t, err) + } + + if tt.prepare != nil { + tt.prepare() + } + + got, err := service.Update(ctx, tt.req) + if diff := cmp.Diff(err, tt.wantErr, cmpopts.EquateErrors()); diff != "" { + t.Errorf("(-want +got):\n%s", diff) + return + } + + if err == nil { + assert.NotNil(t, got.TenantMember.Meta.UpdatedTime) + } + + if diff := cmp.Diff(tt.want, got, cmpopts.IgnoreTypes(protoimpl.MessageState{}), cmpopts.IgnoreFields(v1.Meta{}, "CreatedTime", "UpdatedTime"), testcommon.IgnoreUnexported()); diff != "" { + t.Errorf("(-want +got):\n%s", diff) + } + }) + } +} From 454273c959b3e78228c9fc3d339646b92538179a Mon Sep 17 00:00:00 2001 From: Gerrit Date: Wed, 10 Sep 2025 12:34:32 +0200 Subject: [PATCH 12/16] Tests on tenant methods. --- pkg/service/tenant_test.go | 260 +++++++++++++++++++++++++++++-------- 1 file changed, 209 insertions(+), 51 deletions(-) diff --git a/pkg/service/tenant_test.go b/pkg/service/tenant_test.go index aec19b7..376ccc4 100644 --- a/pkg/service/tenant_test.go +++ b/pkg/service/tenant_test.go @@ -457,10 +457,12 @@ func Test_tenantService_FindParticipatingProjects(t *testing.T) { IncludeInherited: pointer.Pointer(true), }, prepare: func() { - err := projectStore.Create(ctx, &v1.Project{Meta: &v1.Meta{Id: "1"}}) - require.NoError(t, err) - err = projectMemberStore.Create(ctx, &v1.ProjectMember{Meta: &v1.Meta{Annotations: map[string]string{"role": "admin"}}, ProjectId: "1", TenantId: "someone else"}) - require.NoError(t, err) + require.NoError(t, projectStore.Create(ctx, &v1.Project{Meta: &v1.Meta{Id: "1"}})) + require.NoError(t, projectMemberStore.Create(ctx, &v1.ProjectMember{ + Meta: &v1.Meta{Annotations: map[string]string{"role": "admin"}}, + ProjectId: "1", + TenantId: "someone else", + })) }, want: &v1.FindParticipatingProjectsResponse{}, wantErr: nil, @@ -472,10 +474,63 @@ func Test_tenantService_FindParticipatingProjects(t *testing.T) { IncludeInherited: pointer.Pointer(true), }, prepare: func() { - err := projectStore.Create(ctx, &v1.Project{Meta: &v1.Meta{Id: "1"}}) - require.NoError(t, err) - err = projectMemberStore.Create(ctx, &v1.ProjectMember{Meta: &v1.Meta{Annotations: map[string]string{"role": "admin"}}, ProjectId: "1", TenantId: "a"}) - require.NoError(t, err) + require.NoError(t, projectStore.Create(ctx, &v1.Project{Meta: &v1.Meta{Id: "1"}})) + require.NoError(t, projectMemberStore.Create(ctx, &v1.ProjectMember{ + Meta: &v1.Meta{Annotations: map[string]string{"role": "admin"}}, + ProjectId: "1", + TenantId: "a", + })) + }, + want: &v1.FindParticipatingProjectsResponse{ + Projects: []*v1.ProjectWithMembershipAnnotations{{ + Project: &v1.Project{ + Meta: &v1.Meta{ + Kind: "Project", + Apiversion: "v1", + Id: "1", + }, + }, + ProjectAnnotations: map[string]string{"role": "admin"}, + TenantAnnotations: nil, + }}, + }, + wantErr: nil, + }, + { + name: "no direct membership in other namespace", + req: &v1.FindParticipatingProjectsRequest{ + TenantId: "a", + IncludeInherited: pointer.Pointer(true), + Namespace: "other", + }, + prepare: func() { + require.NoError(t, projectStore.Create(ctx, &v1.Project{Meta: &v1.Meta{Id: "1"}})) + require.NoError(t, projectMemberStore.Create(ctx, &v1.ProjectMember{ + Meta: &v1.Meta{Annotations: map[string]string{"role": "admin"}}, + ProjectId: "1", + TenantId: "a", + })) + }, + want: &v1.FindParticipatingProjectsResponse{ + Projects: nil, + }, + wantErr: nil, + }, + { + name: "direct membership in a namespace", + req: &v1.FindParticipatingProjectsRequest{ + TenantId: "a", + IncludeInherited: pointer.Pointer(true), + Namespace: "a", + }, + prepare: func() { + require.NoError(t, projectStore.Create(ctx, &v1.Project{Meta: &v1.Meta{Id: "1"}})) + require.NoError(t, projectMemberStore.Create(ctx, &v1.ProjectMember{ + Meta: &v1.Meta{Annotations: map[string]string{"role": "admin"}}, + ProjectId: "1", + TenantId: "a", + Namespace: "a", + })) }, want: &v1.FindParticipatingProjectsResponse{ Projects: []*v1.ProjectWithMembershipAnnotations{{ @@ -499,16 +554,23 @@ func Test_tenantService_FindParticipatingProjects(t *testing.T) { IncludeInherited: pointer.Pointer(false), }, prepare: func() { - err := projectStore.Create(ctx, &v1.Project{Meta: &v1.Meta{Id: "1"}}) - require.NoError(t, err) - err = projectStore.Create(ctx, &v1.Project{Meta: &v1.Meta{Id: "2"}, TenantId: "b"}) - require.NoError(t, err) - err = projectMemberStore.Create(ctx, &v1.ProjectMember{Meta: &v1.Meta{Annotations: map[string]string{"role": "admin"}}, ProjectId: "1", TenantId: "a"}) - require.NoError(t, err) - err = projectMemberStore.Create(ctx, &v1.ProjectMember{Meta: &v1.Meta{Annotations: map[string]string{"role": "admin"}}, ProjectId: "2", TenantId: "b"}) - require.NoError(t, err) - err = tenantMemberStore.Create(ctx, &v1.TenantMember{Meta: &v1.Meta{Annotations: map[string]string{"role": "editor"}}, MemberId: "a", TenantId: "b"}) - require.NoError(t, err) + require.NoError(t, projectStore.Create(ctx, &v1.Project{Meta: &v1.Meta{Id: "1"}})) + require.NoError(t, projectStore.Create(ctx, &v1.Project{Meta: &v1.Meta{Id: "2"}, TenantId: "b"})) + require.NoError(t, projectMemberStore.Create(ctx, &v1.ProjectMember{ + Meta: &v1.Meta{Annotations: map[string]string{"role": "admin"}}, + ProjectId: "1", + TenantId: "a", + })) + require.NoError(t, projectMemberStore.Create(ctx, &v1.ProjectMember{ + Meta: &v1.Meta{Annotations: map[string]string{"role": "admin"}}, + ProjectId: "2", + TenantId: "b", + })) + require.NoError(t, tenantMemberStore.Create(ctx, &v1.TenantMember{ + Meta: &v1.Meta{Annotations: map[string]string{"role": "editor"}}, + MemberId: "a", + TenantId: "b", + })) }, want: &v1.FindParticipatingProjectsResponse{ Projects: []*v1.ProjectWithMembershipAnnotations{{ @@ -532,10 +594,8 @@ func Test_tenantService_FindParticipatingProjects(t *testing.T) { IncludeInherited: pointer.Pointer(true), }, prepare: func() { - err := projectStore.Create(ctx, &v1.Project{Meta: &v1.Meta{Id: "1"}, TenantId: "b"}) - require.NoError(t, err) - err = tenantMemberStore.Create(ctx, &v1.TenantMember{Meta: &v1.Meta{Annotations: map[string]string{"role": "viewer"}}, TenantId: "b", MemberId: "a"}) - require.NoError(t, err) + require.NoError(t, projectStore.Create(ctx, &v1.Project{Meta: &v1.Meta{Id: "1"}, TenantId: "b"})) + require.NoError(t, tenantMemberStore.Create(ctx, &v1.TenantMember{Meta: &v1.Meta{Annotations: map[string]string{"role": "viewer"}}, TenantId: "b", MemberId: "a"})) }, want: &v1.FindParticipatingProjectsResponse{ Projects: []*v1.ProjectWithMembershipAnnotations{{ @@ -560,34 +620,29 @@ func Test_tenantService_FindParticipatingProjects(t *testing.T) { IncludeInherited: pointer.Pointer(true), }, prepare: func() { - err := projectStore.Create(ctx, &v1.Project{ + require.NoError(t, projectStore.Create(ctx, &v1.Project{ Meta: &v1.Meta{Id: "direct-1"}, TenantId: "req-tenant", - }) - require.NoError(t, err) - err = projectMemberStore.Create(ctx, &v1.ProjectMember{ + })) + require.NoError(t, projectMemberStore.Create(ctx, &v1.ProjectMember{ Meta: &v1.Meta{Annotations: map[string]string{"role": "owner"}}, ProjectId: "direct-1", TenantId: "req-tenant", - }) - require.NoError(t, err) - err = tenantMemberStore.Create(ctx, &v1.TenantMember{ + })) + require.NoError(t, tenantMemberStore.Create(ctx, &v1.TenantMember{ Meta: &v1.Meta{Annotations: map[string]string{"role": "editor"}}, MemberId: "req-tenant", TenantId: "parent", - }) - require.NoError(t, err) - err = projectStore.Create(ctx, &v1.Project{ + })) + require.NoError(t, projectStore.Create(ctx, &v1.Project{ Meta: &v1.Meta{Id: "indirect-2"}, TenantId: "parent", - }) - require.NoError(t, err) - err = projectMemberStore.Create(ctx, &v1.ProjectMember{ + })) + require.NoError(t, projectMemberStore.Create(ctx, &v1.ProjectMember{ Meta: &v1.Meta{Annotations: map[string]string{"role": "admin"}}, ProjectId: "indirect-2", TenantId: "parent", - }) - require.NoError(t, err) + })) }, want: &v1.FindParticipatingProjectsResponse{ Projects: []*v1.ProjectWithMembershipAnnotations{ @@ -742,6 +797,53 @@ func Test_tenantService_FindParticipatingTenants(t *testing.T) { }, wantErr: nil, }, + { + name: "no direct membership when in different namespace", + req: &v1.FindParticipatingTenantsRequest{ + TenantId: "a", + IncludeInherited: pointer.Pointer(true), + Namespace: "other", + }, + prepare: func() { + err := tenantStore.Create(ctx, &v1.Tenant{Meta: &v1.Meta{Id: "b"}}) + require.NoError(t, err) + err = tenantMemberStore.Create(ctx, &v1.TenantMember{Meta: &v1.Meta{Annotations: map[string]string{"role": "admin"}}, MemberId: "a", TenantId: "b"}) + require.NoError(t, err) + }, + want: &v1.FindParticipatingTenantsResponse{ + Tenants: nil, + }, + wantErr: nil, + }, + { + name: "direct membership in namespace", + req: &v1.FindParticipatingTenantsRequest{ + TenantId: "a", + IncludeInherited: pointer.Pointer(true), + Namespace: "a", + }, + prepare: func() { + err := tenantStore.Create(ctx, &v1.Tenant{Meta: &v1.Meta{Id: "b"}}) + require.NoError(t, err) + err = tenantMemberStore.Create(ctx, &v1.TenantMember{Meta: &v1.Meta{Annotations: map[string]string{"role": "admin"}}, Namespace: "a", MemberId: "a", TenantId: "b"}) + require.NoError(t, err) + }, + want: &v1.FindParticipatingTenantsResponse{ + Tenants: []*v1.TenantWithMembershipAnnotations{ + { + Tenant: &v1.Tenant{ + Meta: &v1.Meta{ + Kind: "Tenant", + Apiversion: "v1", + Id: "b", + }, + }, + TenantAnnotations: map[string]string{"role": "admin"}, + }, + }, + }, + wantErr: nil, + }, { name: "indirect membership", req: &v1.FindParticipatingTenantsRequest{ @@ -790,34 +892,43 @@ func Test_tenantService_FindParticipatingTenants(t *testing.T) { wantErr: nil, }, { - name: "direct and indirect memberships", + name: "direct and indirect memberships (without interference with other namespaces)", req: &v1.FindParticipatingTenantsRequest{ TenantId: "req-tnt", IncludeInherited: pointer.Pointer(true), }, prepare: func() { - err = tenantStore.Create(ctx, &v1.Tenant{Meta: &v1.Meta{Id: "indirect-tnt"}}) - require.NoError(t, err) - err := projectStore.Create(ctx, &v1.Project{ + require.NoError(t, tenantStore.Create(ctx, &v1.Tenant{Meta: &v1.Meta{Id: "indirect-tnt"}})) + require.NoError(t, projectStore.Create(ctx, &v1.Project{ Meta: &v1.Meta{Id: "indirect"}, TenantId: "indirect-tnt", - }) - require.NoError(t, err) - err = projectMemberStore.Create(ctx, &v1.ProjectMember{ + })) + require.NoError(t, projectMemberStore.Create(ctx, &v1.ProjectMember{ Meta: &v1.Meta{Annotations: map[string]string{"role": "admin"}}, ProjectId: "indirect", TenantId: "req-tnt", - }) - require.NoError(t, err) + })) + // should not interfere: + require.NoError(t, projectMemberStore.Create(ctx, &v1.ProjectMember{ + Meta: &v1.Meta{Annotations: map[string]string{"role": "admin"}}, + ProjectId: "indirect", + TenantId: "req-tnt", + Namespace: "other", + })) - err = tenantStore.Create(ctx, &v1.Tenant{Meta: &v1.Meta{Id: "direct-tnt"}}) - require.NoError(t, err) - err = tenantMemberStore.Create(ctx, &v1.TenantMember{ + require.NoError(t, tenantStore.Create(ctx, &v1.Tenant{Meta: &v1.Meta{Id: "direct-tnt"}})) + require.NoError(t, tenantMemberStore.Create(ctx, &v1.TenantMember{ Meta: &v1.Meta{Annotations: map[string]string{"relation": "direct"}}, TenantId: "direct-tnt", MemberId: "req-tnt", - }) - require.NoError(t, err) + })) + // should not interfere: + require.NoError(t, projectMemberStore.Create(ctx, &v1.ProjectMember{ + Meta: &v1.Meta{Annotations: map[string]string{"role": "admin"}}, + ProjectId: "indirect", + TenantId: "req-tnt", + Namespace: "other", + })) }, want: &v1.FindParticipatingTenantsResponse{ Tenants: []*v1.TenantWithMembershipAnnotations{ @@ -970,6 +1081,53 @@ func Test_tenantService_ListTenantMembers(t *testing.T) { }, wantErr: nil, }, + { + name: "no direct membership in other namespace", + req: &v1.ListTenantMembersRequest{ + TenantId: "acme", + IncludeInherited: pointer.Pointer(true), + Namespace: "other", + }, + prepare: func() { + err := tenantStore.Create(ctx, &v1.Tenant{Meta: &v1.Meta{Id: "azure"}}) + require.NoError(t, err) + err = tenantMemberStore.Create(ctx, &v1.TenantMember{Meta: &v1.Meta{Annotations: map[string]string{"role": "admin"}}, MemberId: "azure", TenantId: "acme"}) + require.NoError(t, err) + }, + want: &v1.ListTenantMembersResponse{ + Tenants: nil, + }, + wantErr: nil, + }, + { + name: "direct membership in namespace", + req: &v1.ListTenantMembersRequest{ + TenantId: "acme", + IncludeInherited: pointer.Pointer(true), + Namespace: "a", + }, + prepare: func() { + err := tenantStore.Create(ctx, &v1.Tenant{Meta: &v1.Meta{Id: "azure"}}) + require.NoError(t, err) + err = tenantMemberStore.Create(ctx, &v1.TenantMember{Meta: &v1.Meta{Annotations: map[string]string{"role": "admin"}}, Namespace: "a", MemberId: "azure", TenantId: "acme"}) + require.NoError(t, err) + }, + want: &v1.ListTenantMembersResponse{ + Tenants: []*v1.TenantWithMembershipAnnotations{ + { + Tenant: &v1.Tenant{ + Meta: &v1.Meta{ + Kind: "Tenant", + Apiversion: "v1", + Id: "azure", + }, + }, + TenantAnnotations: map[string]string{"role": "admin"}, + }, + }, + }, + wantErr: nil, + }, { name: "indirect membership", req: &v1.ListTenantMembersRequest{ From d0a8c94768afa1e38313ee79dafee50d04b31d4c Mon Sep 17 00:00:00 2001 From: Gerrit Date: Wed, 10 Sep 2025 13:20:13 +0200 Subject: [PATCH 13/16] Remove duplicate filter. --- pkg/service/projectmember.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/service/projectmember.go b/pkg/service/projectmember.go index ae9f19a..e3acf11 100644 --- a/pkg/service/projectmember.go +++ b/pkg/service/projectmember.go @@ -101,9 +101,6 @@ func (s *projectMemberService) Find(ctx context.Context, req *v1.ProjectMemberFi if req.TenantId != nil { filter["projectmember ->> 'tenant_id'"] = req.TenantId } - if req.TenantId != nil { - filter["projectmember ->> 'tenant_id'"] = req.TenantId - } for key, value := range req.Annotations { // select * from projectMember where projectMember -> 'meta' -> 'annotations' ->> 'metal-stack.io/role' = 'owner'; f := fmt.Sprintf("projectmember -> 'meta' -> 'annotations' ->> '%s'", key) From fd62e99d3d8fae093213e9b312985a7c587f537e Mon Sep 17 00:00:00 2001 From: Gerrit Date: Wed, 10 Sep 2025 13:37:13 +0200 Subject: [PATCH 14/16] Provide config struct for client. --- client/main.go | 14 +++++-- pkg/client/client.go | 79 +++++++++++++++++++++++++++++++-------- pkg/client/client_test.go | 8 +++- 3 files changed, 81 insertions(+), 20 deletions(-) diff --git a/client/main.go b/client/main.go index b9cbd47..b7cd5bb 100644 --- a/client/main.go +++ b/client/main.go @@ -29,9 +29,17 @@ func main() { hmacKey = auth.HmacDefaultKey } - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - c, err := client.NewClient(ctx, "localhost", 50051, "certs/client.pem", "certs/client-key.pem", "certs/ca.pem", hmacKey, true, logger, "test") + c, err := client.NewClient(&client.Config{ + Logger: logger, + Hostname: "localhost", + Port: 50051, + CertFile: "certs/client.pem", + KeyFile: "certs/client-key.pem", + CaFile: "certs/ca.pem", + Insecure: true, + HmacKey: hmacKey, + Namespace: "test", + }) if err != nil { logger.Error(err.Error()) panic(err) diff --git a/pkg/client/client.go b/pkg/client/client.go index b375fa1..13e5396 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -33,16 +33,62 @@ type GRPCClient struct { hmacKey string } +type Config struct { + Logger *slog.Logger + + Hostname string + Port int + + CertFile string + KeyFile string + CaFile string + Insecure bool + + HmacKey string + + // Namespace if set adds this namespace to namespaced requests such that it does not need to be passed all the time + Namespace string +} + +func (c *Config) validate() error { + if c == nil { + return fmt.Errorf("config must not be nil") + } + + if c.Hostname == "" { + return fmt.Errorf("hostname must be specified") + } + if c.Port < 0 { + return fmt.Errorf("port must not be a negative number") + } + + if c.KeyFile != "" || c.CertFile != "" { + if c.KeyFile == "" || c.CertFile == "" { + return fmt.Errorf("either both key and cert file must be specified or none of them") + } + } + + return nil +} + // NewClient creates a new client for the services for the given address, with the certificate and hmac. -func NewClient(ctx context.Context, hostname string, port int, certFile string, keyFile string, caFile string, hmacKey string, insecure bool, logger *slog.Logger, namespace string) (Client, error) { - address := fmt.Sprintf("%s:%d", hostname, port) +func NewClient(config *Config) (Client, error) { + if err := config.validate(); err != nil { + return nil, err + } + + if config.Logger == nil { + config.Logger = slog.Default() + } + + address := fmt.Sprintf("%s:%d", config.Hostname, config.Port) certPool, err := x509.SystemCertPool() if err != nil { return nil, fmt.Errorf("failed to load system credentials: %w", err) } - if caFile != "" { + if caFile := config.CaFile; caFile != "" { ca, err := os.ReadFile(caFile) if err != nil { return nil, fmt.Errorf("could not read ca certificate: %w", err) @@ -59,8 +105,8 @@ func NewClient(ctx context.Context, hostname string, port int, certFile string, opts []grpc.DialOption ) - if certFile != "" && keyFile != "" { - clientCertificate, err := tls.LoadX509KeyPair(certFile, keyFile) + if config.CertFile != "" && config.KeyFile != "" { + clientCertificate, err := tls.LoadX509KeyPair(config.CertFile, config.KeyFile) if err != nil { return nil, fmt.Errorf("could not load client key pair: %w", err) } @@ -68,11 +114,11 @@ func NewClient(ctx context.Context, hostname string, port int, certFile string, certificates = append(certificates, clientCertificate) creds := credentials.NewTLS(&tls.Config{ - ServerName: hostname, + ServerName: config.Hostname, Certificates: certificates, RootCAs: certPool, MinVersion: tls.VersionTLS12, - InsecureSkipVerify: insecure, // nolint:gosec + InsecureSkipVerify: config.Insecure, // nolint:gosec }) opts = append(opts, @@ -86,9 +132,15 @@ func NewClient(ctx context.Context, hostname string, port int, certFile string, ) } - if hmacKey != "" { + client := GRPCClient{ + log: config.Logger, + } + + if config.HmacKey != "" { + client.hmacKey = config.HmacKey + // Set up the credentials for the connection. - perRPCHMACAuthenticator, err := auth.NewHMACAuther(hmacKey, auth.EditUser) + perRPCHMACAuthenticator, err := auth.NewHMACAuther(config.HmacKey, auth.EditUser) if err != nil { return nil, fmt.Errorf("failed to create hmac-authenticator: %w", err) } @@ -101,13 +153,8 @@ func NewClient(ctx context.Context, hostname string, port int, certFile string, grpc.WithPerRPCCredentials(perRPCHMACAuthenticator)) } - client := GRPCClient{ - log: logger, - hmacKey: hmacKey, - } - - if namespace != "" { - opts = append(opts, NamespaceInterceptor(namespace)) + if config.Namespace != "" { + opts = append(opts, NamespaceInterceptor(config.Namespace)) } // Set up a connection to the server. diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 989b00b..1ab8751 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -48,7 +48,13 @@ func Test_Client(t *testing.T) { port, err := strconv.Atoi(portString) require.NoError(t, err) - client, err := NewClient(t.Context(), "localhost", port, "", "", "", "", true, log, namespace) + client, err := NewClient(&Config{ + Hostname: "localhost", + Port: port, + Insecure: true, + Logger: log, + Namespace: namespace, + }) require.NoError(t, err) t.Run("check namespace interceptor sets missing namespace", func(t *testing.T) { From 4720577f0bd7ca7df866b9c9935879dd0a06a0cc Mon Sep 17 00:00:00 2001 From: Gerrit Date: Wed, 10 Sep 2025 13:40:11 +0200 Subject: [PATCH 15/16] Fix flaky test. --- pkg/service/tenant_test.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/pkg/service/tenant_test.go b/pkg/service/tenant_test.go index 376ccc4..daedfea 100644 --- a/pkg/service/tenant_test.go +++ b/pkg/service/tenant_test.go @@ -1208,12 +1208,10 @@ func Test_tenantService_ListTenantMembers(t *testing.T) { Meta: &v1.Meta{ Kind: "Tenant", Apiversion: "v1", - Id: "github", + Id: "azure", }, }, - TenantAnnotations: map[string]string{"tenant-role": "owner"}, ProjectIds: []string{ - "1", "2", }, }, @@ -1222,10 +1220,12 @@ func Test_tenantService_ListTenantMembers(t *testing.T) { Meta: &v1.Meta{ Kind: "Tenant", Apiversion: "v1", - Id: "azure", + Id: "github", }, }, + TenantAnnotations: map[string]string{"tenant-role": "owner"}, ProjectIds: []string{ + "1", "2", }, }, @@ -1250,6 +1250,15 @@ func Test_tenantService_ListTenantMembers(t *testing.T) { t.Errorf("(-want +got):\n%s", diff) return } + + slices.SortFunc(got.Tenants, func(i, j *v1.TenantWithMembershipAnnotations) int { + if i.Tenant.Meta.Id < j.Tenant.Meta.Id { + return -1 + } else { + return 1 + } + }) + if diff := cmp.Diff(tt.want, got, cmpopts.IgnoreTypes(protoimpl.MessageState{}), cmpopts.IgnoreFields(v1.Meta{}, "CreatedTime"), testcommon.IgnoreUnexported()); diff != "" { t.Errorf("(-want +got):\n%s", diff) } From 2af1307e06f0f656a2c578eadcdc2f07ed65f473 Mon Sep 17 00:00:00 2001 From: Gerrit Date: Thu, 11 Sep 2025 09:19:58 +0200 Subject: [PATCH 16/16] Use unsigned. --- pkg/client/client.go | 5 +---- pkg/client/client_test.go | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index 13e5396..5353082 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -37,7 +37,7 @@ type Config struct { Logger *slog.Logger Hostname string - Port int + Port uint CertFile string KeyFile string @@ -58,9 +58,6 @@ func (c *Config) validate() error { if c.Hostname == "" { return fmt.Errorf("hostname must be specified") } - if c.Port < 0 { - return fmt.Errorf("port must not be a negative number") - } if c.KeyFile != "" || c.CertFile != "" { if c.KeyFile == "" || c.CertFile == "" { diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 1ab8751..4eece9c 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -50,7 +50,7 @@ func Test_Client(t *testing.T) { client, err := NewClient(&Config{ Hostname: "localhost", - Port: port, + Port: uint(port), Insecure: true, Logger: log, Namespace: namespace,