From 0e73a153cdb306c4c8bd52cdefabbb60471fb5a7 Mon Sep 17 00:00:00 2001 From: Nick Trevino Date: Tue, 16 Sep 2025 11:07:57 -0600 Subject: [PATCH 1/3] fix(core): return service sin the order they were registered --- service/pkg/server/services.go | 6 +++++- service/pkg/serviceregistry/serviceregistry.go | 11 +++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/service/pkg/server/services.go b/service/pkg/server/services.go index 776acac73..18aec1bf6 100644 --- a/service/pkg/server/services.go +++ b/service/pkg/server/services.go @@ -127,7 +127,11 @@ func startServices(ctx context.Context, params startServicesParams) (func(), err keyManagerFactories := params.keyManagerFactories // Iterate through the registered namespaces - for ns, namespace := range reg.GetNamespaces() { + for _, ns := range reg.GetNamespaces() { + namespace, err := reg.GetNamespace(ns) + if err != nil { + return nil, err + } // Check if this namespace should be enabled based on configured modes modeEnabled := namespace.IsEnabled(cfg.Mode) diff --git a/service/pkg/serviceregistry/serviceregistry.go b/service/pkg/serviceregistry/serviceregistry.go index bf2f4f526..e5e6bc26d 100644 --- a/service/pkg/serviceregistry/serviceregistry.go +++ b/service/pkg/serviceregistry/serviceregistry.go @@ -281,15 +281,14 @@ func NewServiceRegistry() *Registry { } // GetNamespaces returns all namespaces in the registry -func (reg *Registry) GetNamespaces() map[string]*Namespace { +func (reg *Registry) GetNamespaces() []string { reg.mu.RLock() defer reg.mu.RUnlock() - result := make(map[string]*Namespace, len(reg.namespaces)) - for k, v := range reg.namespaces { - result[k] = v - } - return result + // Return a copy to prevent modification of the internal order slice. + orderCopy := make([]string, len(reg.order)) + copy(orderCopy, reg.order) + return orderCopy } // RegisterService registers a service in the service registry. From 65c5acf4870cebd1aa7fdf57295c3b9e2e10efb3 Mon Sep 17 00:00:00 2001 From: Nick Trevino Date: Tue, 16 Sep 2025 13:57:49 -0600 Subject: [PATCH 2/3] Update GetNamespaces() to return info struct slice. --- service/pkg/server/services.go | 9 +- service/pkg/server/services_test.go | 101 ++++++++++++++++++ .../pkg/serviceregistry/serviceregistry.go | 16 ++- 3 files changed, 116 insertions(+), 10 deletions(-) diff --git a/service/pkg/server/services.go b/service/pkg/server/services.go index 18aec1bf6..c9e81ea3f 100644 --- a/service/pkg/server/services.go +++ b/service/pkg/server/services.go @@ -127,11 +127,10 @@ func startServices(ctx context.Context, params startServicesParams) (func(), err keyManagerFactories := params.keyManagerFactories // Iterate through the registered namespaces - for _, ns := range reg.GetNamespaces() { - namespace, err := reg.GetNamespace(ns) - if err != nil { - return nil, err - } + for _, nsInfo := range reg.GetNamespaces() { + ns := nsInfo.Name + namespace := nsInfo.Namespace + // Check if this namespace should be enabled based on configured modes modeEnabled := namespace.IsEnabled(cfg.Mode) diff --git a/service/pkg/server/services_test.go b/service/pkg/server/services_test.go index ed6bd2da4..a768bd544 100644 --- a/service/pkg/server/services_test.go +++ b/service/pkg/server/services_test.go @@ -2,9 +2,11 @@ package server import ( "context" + "embed" "testing" "github.com/grpc-ecosystem/grpc-gateway/v2/runtime" + "github.com/opentdf/platform/service/internal/server" "github.com/opentdf/platform/service/logger" "github.com/opentdf/platform/service/pkg/config" "github.com/opentdf/platform/service/pkg/serviceregistry" @@ -470,3 +472,102 @@ func (suite *ServiceTestSuite) TestIsNamespaceEnabled() { }) } } + +// mockOrderTrackingService is a mock implementation of IService for testing start order. +type mockOrderTrackingService struct { + namespace string + serviceName string + startOrderTracker *[]string +} + +func (m *mockOrderTrackingService) GetNamespace() string { return m.namespace } +func (m *mockOrderTrackingService) GetVersion() string { return "v1" } +func (m *mockOrderTrackingService) GetServiceDesc() *grpc.ServiceDesc { + return &grpc.ServiceDesc{ServiceName: m.serviceName} +} +func (m *mockOrderTrackingService) IsDBRequired() bool { return false } +func (m *mockOrderTrackingService) DBMigrations() *embed.FS { return nil } +func (m *mockOrderTrackingService) IsStarted() bool { return true } +func (m *mockOrderTrackingService) Shutdown() error { return nil } +func (m *mockOrderTrackingService) Start(_ context.Context, _ serviceregistry.RegistrationParams) error { + *m.startOrderTracker = append(*m.startOrderTracker, m.serviceName) + return nil +} +func (m *mockOrderTrackingService) RegisterConfigUpdateHook(context.Context, func(config.ChangeHook)) error { + return nil +} +func (m *mockOrderTrackingService) RegisterConnectRPCServiceHandler(context.Context, *server.ConnectRPC) error { + return nil +} +func (m *mockOrderTrackingService) RegisterGRPCGatewayHandler(context.Context, *runtime.ServeMux, *grpc.ClientConn) error { + return nil +} +func (m *mockOrderTrackingService) RegisterHTTPHandlers(context.Context, *runtime.ServeMux) error { + return nil +} + +func (suite *ServiceTestSuite) TestStartServices_StartsInRegistrationOrder() { + ctx := context.Background() + startOrderTracker := make([]string, 0) + registry := serviceregistry.NewServiceRegistry() + + // Define the services and their registration order + servicesToRegister := []struct { + name string + namespace string + mode serviceregistry.ModeName + }{ + {"ServiceA", "namespace1", "test"}, + {"ServiceB", "namespace2", "test"}, + {"ServiceC", "namespace1", "test"}, + {"ServiceD", "namespace3", "test"}, + {"ServiceE", "namespace2", "test"}, + } + + for _, s := range servicesToRegister { + mockSvc := &mockOrderTrackingService{ + namespace: s.namespace, + serviceName: s.name, + startOrderTracker: &startOrderTracker, + } + err := registry.RegisterService(mockSvc, s.mode) + suite.Require().NoError(err) + } + + // Prepare to call startServices + otdf, err := mockOpenTDFServer() + suite.Require().NoError(err) + defer otdf.Stop() + + newLogger, err := logger.NewLogger(logger.Config{Output: "stdout", Level: "info", Type: "json"}) + suite.Require().NoError(err) + + cleanup, err := startServices(ctx, startServicesParams{ + cfg: &config.Config{ + Mode: []string{"test"}, // Enable the mode for our test services + Services: map[string]config.ServiceConfig{ + "namespace1": {}, + "namespace2": {}, + "namespace3": {}, + }, + }, + otdf: otdf, + logger: newLogger, + reg: registry, + }) + suite.Require().NoError(err) + defer cleanup() + + // The startServices function iterates through namespaces in the order they were first registered, + // and then through the services within that namespace in their registration order. + // Namespace registration order: namespace1, namespace2, namespace3 + // Services in namespace1: ServiceA, ServiceC + // Services in namespace2: ServiceB, ServiceE + // Services in namespace3: ServiceD + expectedStartOrder := []string{"ServiceA", "ServiceC", "ServiceB", "ServiceE", "ServiceD"} + + suite.Require().Equal(expectedStartOrder, startOrderTracker, "Services should start in the order they were registered, grouped by namespace") + + // call close function + registry.Shutdown() +} diff --git a/service/pkg/serviceregistry/serviceregistry.go b/service/pkg/serviceregistry/serviceregistry.go index e5e6bc26d..2a328e1e8 100644 --- a/service/pkg/serviceregistry/serviceregistry.go +++ b/service/pkg/serviceregistry/serviceregistry.go @@ -280,15 +280,21 @@ func NewServiceRegistry() *Registry { } } +type NamespaceInfo struct { + Name string + Namespace *Namespace +} + // GetNamespaces returns all namespaces in the registry -func (reg *Registry) GetNamespaces() []string { +func (reg *Registry) GetNamespaces() []NamespaceInfo { reg.mu.RLock() defer reg.mu.RUnlock() - // Return a copy to prevent modification of the internal order slice. - orderCopy := make([]string, len(reg.order)) - copy(orderCopy, reg.order) - return orderCopy + namespaceInfo := make([]NamespaceInfo, len(reg.order)) + for i, name := range reg.order { + namespaceInfo[i] = NamespaceInfo{Name: name, Namespace: reg.namespaces[name]} + } + return namespaceInfo } // RegisterService registers a service in the service registry. From 9e6b28b4ed7fd0fb4d4ebd3b09620993c799948a Mon Sep 17 00:00:00 2001 From: Nick Trevino Date: Tue, 16 Sep 2025 14:29:43 -0600 Subject: [PATCH 3/3] update formatting --- service/pkg/server/services_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/service/pkg/server/services_test.go b/service/pkg/server/services_test.go index a768bd544..4eb1f47e3 100644 --- a/service/pkg/server/services_test.go +++ b/service/pkg/server/services_test.go @@ -493,15 +493,19 @@ func (m *mockOrderTrackingService) Start(_ context.Context, _ serviceregistry.Re *m.startOrderTracker = append(*m.startOrderTracker, m.serviceName) return nil } + func (m *mockOrderTrackingService) RegisterConfigUpdateHook(context.Context, func(config.ChangeHook)) error { return nil } + func (m *mockOrderTrackingService) RegisterConnectRPCServiceHandler(context.Context, *server.ConnectRPC) error { return nil } + func (m *mockOrderTrackingService) RegisterGRPCGatewayHandler(context.Context, *runtime.ServeMux, *grpc.ClientConn) error { return nil } + func (m *mockOrderTrackingService) RegisterHTTPHandlers(context.Context, *runtime.ServeMux) error { return nil }