Skip to content

Commit 1d661db

Browse files
fix(core): return services in the order they were registered (#2733)
### Proposed Changes So right now I have inter-PEP dependencies, so config/PEP-A for example. PEP-A wants to set its config to the config service, for later viewing/editing in another interface. Currently the only place available for this is PEP-A's Init (RegisterFunc). So, right now, this is the solution I'm using. I think there is an alternative with some "OnServicesStarted" type hook or something as well (which might be more useful maybe). ### Checklist - [ ] I have added or updated unit tests - [ ] I have added or updated integration tests (if appropriate) - [ ] I have added or updated documentation ### Testing Instructions
1 parent 4ee9544 commit 1d661db

File tree

3 files changed

+119
-6
lines changed

3 files changed

+119
-6
lines changed

service/pkg/server/services.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,10 @@ func startServices(ctx context.Context, params startServicesParams) (func(), err
139139
}
140140

141141
// Iterate through the registered namespaces
142-
for ns, namespace := range reg.GetNamespaces() {
142+
for _, nsInfo := range reg.GetNamespaces() {
143+
ns := nsInfo.Name
144+
namespace := nsInfo.Namespace
145+
143146
// Check if this namespace should be enabled based on configured modes
144147
modeEnabled := namespace.IsEnabled(cfg.Mode)
145148

service/pkg/server/services_test.go

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@ package server
22

33
import (
44
"context"
5+
"embed"
56
"testing"
67

78
"github.com/grpc-ecosystem/grpc-gateway/v2/runtime"
9+
"github.com/opentdf/platform/service/internal/server"
810
"github.com/opentdf/platform/service/logger"
911
"github.com/opentdf/platform/service/pkg/config"
1012
"github.com/opentdf/platform/service/pkg/serviceregistry"
@@ -468,3 +470,106 @@ func (suite *ServiceTestSuite) TestIsNamespaceEnabled() {
468470
})
469471
}
470472
}
473+
474+
// mockOrderTrackingService is a mock implementation of IService for testing start order.
475+
type mockOrderTrackingService struct {
476+
namespace string
477+
serviceName string
478+
startOrderTracker *[]string
479+
}
480+
481+
func (m *mockOrderTrackingService) GetNamespace() string { return m.namespace }
482+
func (m *mockOrderTrackingService) GetVersion() string { return "v1" }
483+
func (m *mockOrderTrackingService) GetServiceDesc() *grpc.ServiceDesc {
484+
return &grpc.ServiceDesc{ServiceName: m.serviceName}
485+
}
486+
func (m *mockOrderTrackingService) IsDBRequired() bool { return false }
487+
func (m *mockOrderTrackingService) DBMigrations() *embed.FS { return nil }
488+
func (m *mockOrderTrackingService) IsStarted() bool { return true }
489+
func (m *mockOrderTrackingService) Shutdown() error { return nil }
490+
func (m *mockOrderTrackingService) Start(_ context.Context, _ serviceregistry.RegistrationParams) error {
491+
*m.startOrderTracker = append(*m.startOrderTracker, m.serviceName)
492+
return nil
493+
}
494+
495+
func (m *mockOrderTrackingService) RegisterConfigUpdateHook(context.Context, func(config.ChangeHook)) error {
496+
return nil
497+
}
498+
499+
func (m *mockOrderTrackingService) RegisterConnectRPCServiceHandler(context.Context, *server.ConnectRPC) error {
500+
return nil
501+
}
502+
503+
func (m *mockOrderTrackingService) RegisterGRPCGatewayHandler(context.Context, *runtime.ServeMux, *grpc.ClientConn) error {
504+
return nil
505+
}
506+
507+
func (m *mockOrderTrackingService) RegisterHTTPHandlers(context.Context, *runtime.ServeMux) error {
508+
return nil
509+
}
510+
511+
func (suite *ServiceTestSuite) TestStartServices_StartsInRegistrationOrder() {
512+
ctx := context.Background()
513+
startOrderTracker := make([]string, 0)
514+
registry := serviceregistry.NewServiceRegistry()
515+
516+
// Define the services and their registration order
517+
servicesToRegister := []struct {
518+
name string
519+
namespace string
520+
mode serviceregistry.ModeName
521+
}{
522+
{"ServiceA", "namespace1", "test"},
523+
{"ServiceB", "namespace2", "test"},
524+
{"ServiceC", "namespace1", "test"},
525+
{"ServiceD", "namespace3", "test"},
526+
{"ServiceE", "namespace2", "test"},
527+
}
528+
529+
for _, s := range servicesToRegister {
530+
mockSvc := &mockOrderTrackingService{
531+
namespace: s.namespace,
532+
serviceName: s.name,
533+
startOrderTracker: &startOrderTracker,
534+
}
535+
err := registry.RegisterService(mockSvc, s.mode)
536+
suite.Require().NoError(err)
537+
}
538+
539+
// Prepare to call startServices
540+
otdf, err := mockOpenTDFServer()
541+
suite.Require().NoError(err)
542+
defer otdf.Stop()
543+
544+
newLogger, err := logger.NewLogger(logger.Config{Output: "stdout", Level: "info", Type: "json"})
545+
suite.Require().NoError(err)
546+
547+
cleanup, err := startServices(ctx, startServicesParams{
548+
cfg: &config.Config{
549+
Mode: []string{"test"}, // Enable the mode for our test services
550+
Services: map[string]config.ServiceConfig{
551+
"namespace1": {},
552+
"namespace2": {},
553+
"namespace3": {},
554+
},
555+
},
556+
otdf: otdf,
557+
logger: newLogger,
558+
reg: registry,
559+
})
560+
suite.Require().NoError(err)
561+
defer cleanup()
562+
563+
// The startServices function iterates through namespaces in the order they were first registered,
564+
// and then through the services within that namespace in their registration order.
565+
// Namespace registration order: namespace1, namespace2, namespace3
566+
// Services in namespace1: ServiceA, ServiceC
567+
// Services in namespace2: ServiceB, ServiceE
568+
// Services in namespace3: ServiceD
569+
expectedStartOrder := []string{"ServiceA", "ServiceC", "ServiceB", "ServiceE", "ServiceD"}
570+
571+
suite.Require().Equal(expectedStartOrder, startOrderTracker, "Services should start in the order they were registered, grouped by namespace")
572+
573+
// call close function
574+
registry.Shutdown()
575+
}

service/pkg/serviceregistry/serviceregistry.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -289,16 +289,21 @@ func NewServiceRegistry() *Registry {
289289
}
290290
}
291291

292+
type NamespaceInfo struct {
293+
Name string
294+
Namespace *Namespace
295+
}
296+
292297
// GetNamespaces returns all namespaces in the registry
293-
func (reg *Registry) GetNamespaces() map[string]*Namespace {
298+
func (reg *Registry) GetNamespaces() []NamespaceInfo {
294299
reg.mu.RLock()
295300
defer reg.mu.RUnlock()
296301

297-
result := make(map[string]*Namespace, len(reg.namespaces))
298-
for k, v := range reg.namespaces {
299-
result[k] = v
302+
namespaceInfo := make([]NamespaceInfo, len(reg.order))
303+
for i, name := range reg.order {
304+
namespaceInfo[i] = NamespaceInfo{Name: name, Namespace: reg.namespaces[name]}
300305
}
301-
return result
306+
return namespaceInfo
302307
}
303308

304309
// RegisterService registers a service in the service registry.

0 commit comments

Comments
 (0)