From fe42ff775a1c8107697f88b844e9d6ccb1dcbc15 Mon Sep 17 00:00:00 2001 From: rosstimothy <39066650+rosstimothy@users.noreply.github.com> Date: Mon, 18 Sep 2023 12:02:09 -0400 Subject: [PATCH] Prevent duplicate service registration (#32050) Enterprise tests do not always set the enterprise build type which causes test failures due to duplicate service registration. To prevent every test from needing to update the modules the PluginRegistry was modified to expose a mechanism to detect if a plugin is registered or not. The old check that registered a not implemented LoginRuleService now makes use of this mechanism to verify that the enterprise auth plugin is not registered. --- lib/auth/grpcserver.go | 3 +-- lib/plugin/registry.go | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/auth/grpcserver.go b/lib/auth/grpcserver.go index b5833e88bc2..4c998569584 100644 --- a/lib/auth/grpcserver.go +++ b/lib/auth/grpcserver.go @@ -71,7 +71,6 @@ import ( "github.com/gravitational/teleport/lib/events" "github.com/gravitational/teleport/lib/httplib" "github.com/gravitational/teleport/lib/joinserver" - "github.com/gravitational/teleport/lib/modules" "github.com/gravitational/teleport/lib/observability/metrics" "github.com/gravitational/teleport/lib/services" "github.com/gravitational/teleport/lib/services/local" @@ -5378,7 +5377,7 @@ func NewGRPCServer(cfg GRPCServerConfig) (*GRPCServer, error) { // Only register the service if this is an open source build. Enterprise builds // register the actual service via an auth plugin, if we register here then all // Enterprise builds would fail with a duplicate service registered error. - if modules.GetModules().BuildType() == modules.BuildOSS { + if cfg.PluginRegistry == nil || !cfg.PluginRegistry.IsRegistered("auth.enterprise") { loginrulepb.RegisterLoginRuleServiceServer(server, loginrule.NotImplementedService{}) } diff --git a/lib/plugin/registry.go b/lib/plugin/registry.go index ecccdd0dc48..2f169094a5b 100644 --- a/lib/plugin/registry.go +++ b/lib/plugin/registry.go @@ -32,13 +32,15 @@ type Plugin interface { // Registry is the plugin registry type Registry interface { + // IsRegistered returns whether a plugin with the give name exists. + IsRegistered(name string) bool // Add adds plugin to the registry Add(plugin Plugin) error // RegisterProxyWebHandlers registers Teleport Proxy web handlers - RegisterProxyWebHandlers(hander interface{}) error + RegisterProxyWebHandlers(handler interface{}) error // RegisterAuthWebHandlers registers Teleport Auth web handlers RegisterAuthWebHandlers(handler interface{}) error - // RegisterAuthServices registerse Teleport AuthServer services + // RegisterAuthServices registers Teleport AuthServer services RegisterAuthServices(server interface{}) error } @@ -53,6 +55,12 @@ type registry struct { plugins map[string]Plugin } +// IsRegistered returns whether a plugin with the give name exists. +func (r *registry) IsRegistered(name string) bool { + _, ok := r.plugins[name] + return ok +} + // Add adds plugin to the plugin registry func (r *registry) Add(p Plugin) error { if p == nil { @@ -64,8 +72,7 @@ func (r *registry) Add(p Plugin) error { return trace.BadParameter("missing plugin name") } - _, exists := r.plugins[name] - if exists { + if r.IsRegistered(name) { return trace.AlreadyExists("plugin %v already exists", name) } @@ -96,7 +103,7 @@ func (r *registry) RegisterAuthWebHandlers(handler interface{}) error { return nil } -// RegisterAuthServices registerse Teleport AuthServer services +// RegisterAuthServices registers Teleport AuthServer services func (r *registry) RegisterAuthServices(server interface{}) error { for _, p := range r.plugins { if err := p.RegisterAuthServices(server); err != nil {