From 58e37b47e73b784e45568f97408238b22c0036c7 Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Tue, 26 Aug 2025 11:53:43 -0400 Subject: [PATCH] manager: restore NewServer to v2 signature While NewServer is probably not called by any code outside the swarmkit module, we should still be good maintainers by taking reasonable measures to avoid introducing breaking changes to the module. Instead provide a view-response mutator to controlapi by setting an exported struct field on the constructed Server. Since for similar compatibility reasons we can't ever add new methods to the view-response interface, rename it to better reflect its narrow scope. Signed-off-by: Cory Snider --- manager/controlapi/apihooks.go | 14 +++++++++++--- manager/controlapi/network.go | 4 ++-- manager/controlapi/server.go | 11 +++++------ manager/controlapi/server_test.go | 2 +- manager/manager.go | 3 ++- .../orchestrator_controlapi_integration_test.go | 2 +- 6 files changed, 22 insertions(+), 14 deletions(-) diff --git a/manager/controlapi/apihooks.go b/manager/controlapi/apihooks.go index 809d1f6ad4..4e4870c2c9 100644 --- a/manager/controlapi/apihooks.go +++ b/manager/controlapi/apihooks.go @@ -6,9 +6,10 @@ import ( "github.com/moby/swarmkit/v2/api" ) -// ViewResponseMutator provides callbacks which may modify the response objects -// for Get or List Control API requests before they are sent to the client. -type ViewResponseMutator interface { +// NetworkViewResponseMutator provides callbacks which may modify the response +// objects for GetNetwork and ListNetworks Control API requests before they are +// sent to the client. +type NetworkViewResponseMutator interface { OnGetNetwork(context.Context, *api.Network) error OnListNetworks(context.Context, []*api.Network) error } @@ -22,3 +23,10 @@ func (NoopViewResponseMutator) OnGetNetwork(ctx context.Context, n *api.Network) func (NoopViewResponseMutator) OnListNetworks(ctx context.Context, networks []*api.Network) error { return nil } + +func (s *Server) networkhooks() NetworkViewResponseMutator { + if s.NetworkHooks != nil { + return s.NetworkHooks + } + return NoopViewResponseMutator{} +} diff --git a/manager/controlapi/network.go b/manager/controlapi/network.go index bd78101528..2f335b743e 100644 --- a/manager/controlapi/network.go +++ b/manager/controlapi/network.go @@ -148,7 +148,7 @@ func (s *Server) GetNetwork(ctx context.Context, request *api.GetNetworkRequest) if n == nil { return nil, status.Errorf(codes.NotFound, "network %s not found", request.NetworkID) } - if err := s.viewhooks.OnGetNetwork(ctx, n); err != nil { + if err := s.networkhooks().OnGetNetwork(ctx, n); err != nil { return nil, err } return &api.GetNetworkResponse{ @@ -295,7 +295,7 @@ func (s *Server) ListNetworks(ctx context.Context, request *api.ListNetworksRequ ) } - if err := s.viewhooks.OnListNetworks(ctx, networks); err != nil { + if err := s.networkhooks().OnListNetworks(ctx, networks); err != nil { return nil, err } diff --git a/manager/controlapi/server.go b/manager/controlapi/server.go index 20e91e5165..31890f0c34 100644 --- a/manager/controlapi/server.go +++ b/manager/controlapi/server.go @@ -20,24 +20,23 @@ type Server struct { raft *raft.Node securityConfig *ca.SecurityConfig netvalidator networkallocator.DriverValidator - viewhooks ViewResponseMutator dr *drivers.DriverProvider + + // NetworkHooks intercept and mutate API server responses for GetNetwork + // and ListNetworks API requests when set. + NetworkHooks NetworkViewResponseMutator } // NewServer creates a Cluster API server. -func NewServer(store *store.MemoryStore, raft *raft.Node, securityConfig *ca.SecurityConfig, nv networkallocator.DriverValidator, vrm ViewResponseMutator, dr *drivers.DriverProvider) *Server { +func NewServer(store *store.MemoryStore, raft *raft.Node, securityConfig *ca.SecurityConfig, nv networkallocator.DriverValidator, dr *drivers.DriverProvider) *Server { if nv == nil { nv = networkallocator.InertProvider{} } - if vrm == nil { - vrm = NoopViewResponseMutator{} - } return &Server{ store: store, dr: dr, raft: raft, securityConfig: securityConfig, netvalidator: nv, - viewhooks: vrm, } } diff --git a/manager/controlapi/server_test.go b/manager/controlapi/server_test.go index eee99ef007..0d0cd26c7e 100644 --- a/manager/controlapi/server_test.go +++ b/manager/controlapi/server_test.go @@ -47,7 +47,7 @@ func newTestServer(t *testing.T) *testServer { ts.Store = store.NewMemoryStore(&stateutils.MockProposer{}) assert.NotNil(t, ts.Store) - ts.Server = NewServer(ts.Store, nil, securityConfig, nil, nil, nil) + ts.Server = NewServer(ts.Store, nil, securityConfig, nil, nil) assert.NotNil(t, ts.Server) temp, err := os.CreateTemp("", "test-socket") diff --git a/manager/manager.go b/manager/manager.go index 81173bbd58..edd9702072 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -472,7 +472,8 @@ func (m *Manager) Run(parent context.Context) error { return err } - baseControlAPI := controlapi.NewServer(m.raftNode.MemoryStore(), m.raftNode, m.config.SecurityConfig, m.config.networkProvider(), m, drivers.New(m.config.PluginGetter)) + baseControlAPI := controlapi.NewServer(m.raftNode.MemoryStore(), m.raftNode, m.config.SecurityConfig, m.config.networkProvider(), drivers.New(m.config.PluginGetter)) + baseControlAPI.NetworkHooks = m baseResourceAPI := resourceapi.New(m.raftNode.MemoryStore()) healthServer := health.NewHealthServer() localHealthServer := health.NewHealthServer() diff --git a/manager/orchestrator/jobs/orchestrator_controlapi_integration_test.go b/manager/orchestrator/jobs/orchestrator_controlapi_integration_test.go index fe3e44ea21..396deac830 100644 --- a/manager/orchestrator/jobs/orchestrator_controlapi_integration_test.go +++ b/manager/orchestrator/jobs/orchestrator_controlapi_integration_test.go @@ -54,7 +54,7 @@ var _ = Describe("Integration between the controlapi and jobs orchestrator", fun // but it's unexported. in any case, re-writing it in ginkgo isn't // unwarranted. s = store.NewMemoryStore(&stateutils.MockProposer{}) - server = controlapi.NewServer(s, nil, nil, nil, nil, nil) + server = controlapi.NewServer(s, nil, nil, nil, nil) // we need a temporary unix socket to server on temp, err := os.CreateTemp("", "test-socket")