mirror of
https://github.com/netbirdio/netbird.git
synced 2026-08-04 03:35:09 -04:00
[management] Generic gRPC extension seam for external modules (#6894)
## Describe your changes This adds an extension point to the management server for registering additional gRPC services. We already have a generic integrations system and dependency injection for server components. This closes the gap on being able to also extend the gRPC API cleanly. ## Issue ticket number and link N/A ## Stack <!-- branch-stack --> ### Checklist - [ ] Is it a bug fix - [ ] Is a typo/documentation fix - [x] Is a feature enhancement - [ ] It is a refactor - [ ] Created tests that fail without the change (if possible) - [ ] This change does **not** modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — **OR** I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See [CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first). > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) No docs needed. This is strictly a small internal plumbing enhancement / refactor. <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/netbirdio/codesmith/netbird/pr/6894"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-light-v2.svg"><img alt="View with [code]smith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"></picture></a> <a href="https://backend.blacksmith.sh/track/enable-autofix?expires=1787582002&installation_model_id=427504&pr_number=6894&repository=netbirdio%2Fnetbird&return_to=https%3A%2F%2Fgithub.com%2Fnetbirdio%2Fnetbird%2Fpull%2F6894&signature=3288061677db243031830964fec8f0f34c82f7fc63a39298cd0b4e3490551060"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-light.svg"><img alt="Autofix with [code]smith" src="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>@codesmith-bot</code> with what you need. Autofix is disabled.</sup> <!-- codesmith:autofix:disabled --> <!-- /codesmith:footer --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a gRPC extension mechanism to contribute additional services and automatically chain extra unary and stream interceptors. * Extension shutdown hooks now run as part of server stop. * Added exported proxy token generation via `GenerateProxyToken()` for external integrations. * **Tests** * Added coverage for extension interceptor/service wiring, extension shutdown execution, and proxy token generation validation (including hash consistency and prefix). <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -24,13 +24,13 @@ import (
|
||||
|
||||
"github.com/netbirdio/netbird/encryption"
|
||||
"github.com/netbirdio/netbird/formatter/hook"
|
||||
"github.com/netbirdio/netbird/management/internals/modules/agentnetwork"
|
||||
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/accesslogs"
|
||||
accesslogsmanager "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/accesslogs/manager"
|
||||
rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
|
||||
nbgrpc "github.com/netbirdio/netbird/management/internals/shared/grpc"
|
||||
"github.com/netbirdio/netbird/management/server/activity"
|
||||
activitystore "github.com/netbirdio/netbird/management/server/activity/store"
|
||||
"github.com/netbirdio/netbird/management/internals/modules/agentnetwork"
|
||||
nbcache "github.com/netbirdio/netbird/management/server/cache"
|
||||
nbContext "github.com/netbirdio/netbird/management/server/context"
|
||||
nbhttp "github.com/netbirdio/netbird/management/server/http"
|
||||
@@ -184,6 +184,10 @@ func (s *BaseServer) GRPCServer() *grpc.Server {
|
||||
grpc.ChainStreamInterceptor(realip.StreamServerInterceptorOpts(realipOpts...), streamInterceptor, proxyStream),
|
||||
}
|
||||
|
||||
// Append interceptors contributed by registered gRPC extensions. These
|
||||
// run after the built-in chain (ChainUnaryInterceptor is additive).
|
||||
gRPCOpts = appendExtensionInterceptors(gRPCOpts, s.grpcExtensions)
|
||||
|
||||
if s.Config.HttpConfig.LetsEncryptDomain != "" {
|
||||
certManager, err := encryption.CreateCertManager(s.Config.Datadir, s.Config.HttpConfig.LetsEncryptDomain)
|
||||
if err != nil {
|
||||
@@ -215,6 +219,9 @@ func (s *BaseServer) GRPCServer() *grpc.Server {
|
||||
mgmtProto.RegisterProxyServiceServer(gRPCAPIHandler, s.ReverseProxyGRPCServer())
|
||||
log.Info("ProxyService registered on gRPC server")
|
||||
|
||||
// Register services contributed by external modules via the extension seam.
|
||||
registerExtensions(gRPCAPIHandler, s.grpcExtensions)
|
||||
|
||||
return gRPCAPIHandler
|
||||
})
|
||||
}
|
||||
|
||||
74
management/internals/server/grpc_extension.go
Normal file
74
management/internals/server/grpc_extension.go
Normal file
@@ -0,0 +1,74 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
||||
"google.golang.org/grpc"
|
||||
)
|
||||
|
||||
// GRPCExtension bundles an external module's contribution to the management
|
||||
// gRPC server: the registration of one or more services onto the shared
|
||||
// grpc.Server, any server-wide interceptors those services require, and an
|
||||
// optional shutdown hook. It is a generic extension point with no knowledge of
|
||||
// any specific service.
|
||||
type GRPCExtension struct {
|
||||
// Register is invoked with the shared grpc.Server (as a ServiceRegistrar)
|
||||
// after the built-in services are registered. It may register any number of
|
||||
// services. May be nil.
|
||||
Register func(grpc.ServiceRegistrar)
|
||||
// UnaryInterceptors are appended to the server's unary interceptor chain,
|
||||
// running after the built-in interceptors. May be empty.
|
||||
UnaryInterceptors []grpc.UnaryServerInterceptor
|
||||
// StreamInterceptors are appended to the server's stream interceptor chain,
|
||||
// running after the built-in interceptors. May be empty.
|
||||
StreamInterceptors []grpc.StreamServerInterceptor
|
||||
// Shutdown, if non-nil, is called once during Stop() with the context
|
||||
// governing server shutdown, which carries a deadline. The hook MUST
|
||||
// return promptly and MUST abandon its work once that context is
|
||||
// cancelled or expires: it runs before the rest of Stop()'s cleanup
|
||||
// (store, event store, embedded IdP) and before Stop() itself checks the
|
||||
// context's deadline, so a hook that ignores the context will delay all
|
||||
// of that cleanup and prevent Stop() from returning on time. May be nil.
|
||||
Shutdown func(ctx context.Context)
|
||||
}
|
||||
|
||||
// RegisterGRPCExtension registers a gRPC extension. Call before the gRPC server
|
||||
// is first built (i.e. before Start); registrations after that have no effect.
|
||||
func (s *BaseServer) RegisterGRPCExtension(ext GRPCExtension) {
|
||||
s.grpcExtensions = append(s.grpcExtensions, ext)
|
||||
}
|
||||
|
||||
// appendExtensionInterceptors appends each extension's interceptors to the gRPC
|
||||
// server options as additional chained interceptors. grpc.ChainUnaryInterceptor
|
||||
// and grpc.ChainStreamInterceptor are additive, so the returned options run the
|
||||
// extension interceptors after any interceptors already present in opts.
|
||||
func appendExtensionInterceptors(opts []grpc.ServerOption, exts []GRPCExtension) []grpc.ServerOption {
|
||||
for _, ext := range exts {
|
||||
if len(ext.UnaryInterceptors) > 0 {
|
||||
opts = append(opts, grpc.ChainUnaryInterceptor(ext.UnaryInterceptors...))
|
||||
}
|
||||
if len(ext.StreamInterceptors) > 0 {
|
||||
opts = append(opts, grpc.ChainStreamInterceptor(ext.StreamInterceptors...))
|
||||
}
|
||||
}
|
||||
return opts
|
||||
}
|
||||
|
||||
// registerExtensions registers each extension's services onto reg.
|
||||
func registerExtensions(reg grpc.ServiceRegistrar, exts []GRPCExtension) {
|
||||
for _, ext := range exts {
|
||||
if ext.Register != nil {
|
||||
ext.Register(reg)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// runExtensionShutdownHooks calls each extension's shutdown hook, if set,
|
||||
// passing ctx through so hooks can honor its deadline/cancellation.
|
||||
func runExtensionShutdownHooks(ctx context.Context, exts []GRPCExtension) {
|
||||
for _, ext := range exts {
|
||||
if ext.Shutdown != nil {
|
||||
ext.Shutdown(ctx)
|
||||
}
|
||||
}
|
||||
}
|
||||
160
management/internals/server/grpc_extension_test.go
Normal file
160
management/internals/server/grpc_extension_test.go
Normal file
@@ -0,0 +1,160 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
|
||||
"google.golang.org/grpc"
|
||||
"google.golang.org/grpc/credentials/insecure"
|
||||
"google.golang.org/grpc/health"
|
||||
healthgrpc "google.golang.org/grpc/health/grpc_health_v1"
|
||||
"google.golang.org/grpc/test/bufconn"
|
||||
)
|
||||
|
||||
// Test that an extension's interceptors and service registration are actually
|
||||
// wired onto a real in-process gRPC server via the helpers, and that shutdown
|
||||
// hooks run. This validates the load-bearing assumption that
|
||||
// grpc.ChainUnaryInterceptor is additive (extension interceptors run in
|
||||
// addition to any base chain).
|
||||
func TestGRPCExtensionAppliedToServer(t *testing.T) {
|
||||
var unaryCalls atomic.Int32
|
||||
var streamShutdownCalled atomic.Bool
|
||||
|
||||
ext := GRPCExtension{
|
||||
Register: func(reg grpc.ServiceRegistrar) {
|
||||
healthgrpc.RegisterHealthServer(reg, health.NewServer())
|
||||
},
|
||||
UnaryInterceptors: []grpc.UnaryServerInterceptor{
|
||||
func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) {
|
||||
unaryCalls.Add(1)
|
||||
return handler(ctx, req)
|
||||
},
|
||||
},
|
||||
Shutdown: func(ctx context.Context) { streamShutdownCalled.Store(true) },
|
||||
}
|
||||
exts := []GRPCExtension{ext}
|
||||
|
||||
// Base options mimic GRPCServer(): a pre-existing chain the extension appends to.
|
||||
var baseUnaryCalls atomic.Int32
|
||||
opts := []grpc.ServerOption{
|
||||
grpc.ChainUnaryInterceptor(func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) {
|
||||
baseUnaryCalls.Add(1)
|
||||
return handler(ctx, req)
|
||||
}),
|
||||
}
|
||||
opts = appendExtensionInterceptors(opts, exts)
|
||||
|
||||
srv := grpc.NewServer(opts...)
|
||||
registerExtensions(srv, exts)
|
||||
|
||||
lis := bufconn.Listen(1024 * 1024)
|
||||
go func() { _ = srv.Serve(lis) }()
|
||||
t.Cleanup(srv.Stop)
|
||||
|
||||
conn, err := grpc.NewClient("passthrough:///bufnet",
|
||||
grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { return lis.DialContext(ctx) }),
|
||||
grpc.WithTransportCredentials(insecure.NewCredentials()))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Cleanup(func() { _ = conn.Close() })
|
||||
|
||||
_, err = healthgrpc.NewHealthClient(conn).Check(context.Background(), &healthgrpc.HealthCheckRequest{})
|
||||
if err != nil {
|
||||
t.Fatalf("health check via extension-registered service failed: %v", err)
|
||||
}
|
||||
if baseUnaryCalls.Load() != 1 {
|
||||
t.Errorf("base interceptor calls = %d, want 1 (base chain must be preserved)", baseUnaryCalls.Load())
|
||||
}
|
||||
if unaryCalls.Load() != 1 {
|
||||
t.Errorf("extension interceptor calls = %d, want 1", unaryCalls.Load())
|
||||
}
|
||||
|
||||
runExtensionShutdownHooks(context.Background(), exts)
|
||||
if !streamShutdownCalled.Load() {
|
||||
t.Error("extension shutdown hook was not called")
|
||||
}
|
||||
}
|
||||
|
||||
// TestGRPCExtensionShutdownHookReceivesCallerContext asserts that each hook receives
|
||||
// a non-nil context and that it is the very same context the caller passed
|
||||
// in, so hooks can rely on values/deadlines placed on it by Stop().
|
||||
func TestGRPCExtensionShutdownHookReceivesCallerContext(t *testing.T) {
|
||||
type sentinelKey struct{}
|
||||
want := "shutdown-ctx-sentinel"
|
||||
ctx := context.WithValue(context.Background(), sentinelKey{}, want)
|
||||
|
||||
var called bool
|
||||
ext := GRPCExtension{
|
||||
Shutdown: func(hookCtx context.Context) {
|
||||
called = true
|
||||
if hookCtx == nil {
|
||||
t.Fatal("hook received a nil context")
|
||||
}
|
||||
got, _ := hookCtx.Value(sentinelKey{}).(string)
|
||||
if got != want {
|
||||
t.Errorf("hook context sentinel = %q, want %q (not the caller's context)", got, want)
|
||||
}
|
||||
},
|
||||
}
|
||||
|
||||
runExtensionShutdownHooks(ctx, []GRPCExtension{ext})
|
||||
if !called {
|
||||
t.Fatal("shutdown hook was not called")
|
||||
}
|
||||
}
|
||||
|
||||
// TestGRPCExtensionShutdownHookObservesCancellation documents, by test, that
|
||||
// hooks can honor cancellation/deadlines: a hook given an already-cancelled
|
||||
// context must see ctx.Err() != nil and a closed Done() channel.
|
||||
func TestGRPCExtensionShutdownHookObservesCancellation(t *testing.T) {
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
cancel()
|
||||
|
||||
var called bool
|
||||
ext := GRPCExtension{
|
||||
Shutdown: func(hookCtx context.Context) {
|
||||
called = true
|
||||
if hookCtx.Err() == nil {
|
||||
t.Error("hook context Err() = nil, want non-nil for a cancelled context")
|
||||
}
|
||||
select {
|
||||
case <-hookCtx.Done():
|
||||
default:
|
||||
t.Error("hook context Done() channel is not closed for a cancelled context")
|
||||
}
|
||||
},
|
||||
}
|
||||
|
||||
runExtensionShutdownHooks(ctx, []GRPCExtension{ext})
|
||||
if !called {
|
||||
t.Fatal("shutdown hook was not called")
|
||||
}
|
||||
}
|
||||
|
||||
// TestGRPCExtensionShutdownHookNilSkipped asserts that an extension
|
||||
// with a nil Shutdown hook is skipped without panicking, and that hooks for
|
||||
// other extensions still run.
|
||||
func TestGRPCExtensionShutdownHookNilSkipped(t *testing.T) {
|
||||
var called atomic.Bool
|
||||
exts := []GRPCExtension{
|
||||
{Shutdown: nil},
|
||||
{Shutdown: func(context.Context) { called.Store(true) }},
|
||||
}
|
||||
|
||||
runExtensionShutdownHooks(context.Background(), exts)
|
||||
if !called.Load() {
|
||||
t.Error("shutdown hook for non-nil extension was not called")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRegisterGRPCExtensionAccumulates(t *testing.T) {
|
||||
s := &BaseServer{}
|
||||
s.RegisterGRPCExtension(GRPCExtension{})
|
||||
s.RegisterGRPCExtension(GRPCExtension{})
|
||||
if len(s.grpcExtensions) != 2 {
|
||||
t.Fatalf("grpcExtensions len = %d, want 2", len(s.grpcExtensions))
|
||||
}
|
||||
}
|
||||
@@ -68,6 +68,11 @@ type BaseServer struct {
|
||||
|
||||
proxyAuthClose func()
|
||||
|
||||
// grpcExtensions holds additional gRPC services, interceptors, and shutdown
|
||||
// hooks registered by external modules via RegisterGRPCExtension. Populated
|
||||
// during boot (single-threaded), consumed by GRPCServer() and Stop().
|
||||
grpcExtensions []GRPCExtension
|
||||
|
||||
listener net.Listener
|
||||
certManager *autocert.Manager
|
||||
update *version.Update
|
||||
@@ -257,6 +262,7 @@ func (s *BaseServer) Stop() error {
|
||||
s.proxyAuthClose()
|
||||
s.proxyAuthClose = nil
|
||||
}
|
||||
runExtensionShutdownHooks(ctx, s.grpcExtensions)
|
||||
_ = s.Store().Close(ctx)
|
||||
_ = s.EventStore().Close(ctx)
|
||||
if s.update != nil {
|
||||
|
||||
@@ -68,7 +68,7 @@ type ProxyAccessTokenGenerated struct {
|
||||
// CreateNewProxyAccessToken generates a new proxy access token.
|
||||
// Returns the token with hashed value stored and plain token for one-time display.
|
||||
func CreateNewProxyAccessToken(name string, expiresIn time.Duration, accountID *string, createdBy string) (*ProxyAccessTokenGenerated, error) {
|
||||
hashedToken, plainToken, err := generateProxyToken()
|
||||
hashedToken, plainToken, err := GenerateProxyToken()
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -94,7 +94,10 @@ func CreateNewProxyAccessToken(name string, expiresIn time.Duration, accountID *
|
||||
}, nil
|
||||
}
|
||||
|
||||
func generateProxyToken() (HashedProxyToken, PlainProxyToken, error) {
|
||||
// GenerateProxyToken generates a new random proxy token, returning its SHA-256
|
||||
// hash (for storage) and the one-time plaintext. Exported so external modules
|
||||
// can mint tokens in the canonical proxy-token format.
|
||||
func GenerateProxyToken() (HashedProxyToken, PlainProxyToken, error) {
|
||||
secret, err := b.Random(ProxyTokenSecretLength)
|
||||
if err != nil {
|
||||
return "", "", err
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package types
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -123,6 +124,22 @@ func TestCreateNewProxyAccessToken(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
func TestGenerateProxyToken(t *testing.T) {
|
||||
hashed, plain, err := GenerateProxyToken()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := plain.Validate(); err != nil {
|
||||
t.Errorf("generated token failed Validate(): %v", err)
|
||||
}
|
||||
if plain.Hash() != hashed {
|
||||
t.Error("returned hashed token does not match Hash(plain)")
|
||||
}
|
||||
if !strings.HasPrefix(string(plain), ProxyTokenPrefix) {
|
||||
t.Errorf("token %q missing prefix %q", plain, ProxyTokenPrefix)
|
||||
}
|
||||
}
|
||||
|
||||
func TestProxyAccessToken_IsExpired(t *testing.T) {
|
||||
past := time.Now().Add(-1 * time.Hour)
|
||||
future := time.Now().Add(1 * time.Hour)
|
||||
|
||||
Reference in New Issue
Block a user