Skip to content

Commit 26168fc

Browse files
Copilotintel352
Copilot
and
intel352
authored
Add LoadPluginWithOverride to allow external plugins to replace built-in types (#263)
* Initial plan * Add LoadPluginWithOverride to PluginLoader and StdEngine Allow external plugins to override built-in step, module, trigger, handler, deploy target, and sidecar provider types. When a duplicate type is encountered via LoadPluginWithOverride, a warning is logged instead of returning an error. The original LoadPlugin behavior (rejecting duplicates) is preserved. Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> * Fix contradictory comment in LoadPluginWithOverride doc Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> * Address review feedback: LoadBinaryPluginWithOverride, doc fix, extended tests - Add LoadBinaryPluginWithOverride to PluginLoader for override-capable binary plugin loading - Fix engine.go doc to mention deploy targets and sidecar providers - Add LoadPlugin duplicate rejection assertion in step override test - Extend AllTypes test to cover deploy targets and sidecar providers; add fullPlugin, mockDeployTarget, mockSidecarProvider test helpers Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
1 parent 68b49d1 commit 26168fc

3 files changed

Lines changed: 289 additions & 14 deletions

File tree

‎engine.go‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,29 @@ func (e *StdEngine) SetPluginLoader(loader *plugin.PluginLoader) {
212212

213213
// LoadPlugin loads an EnginePlugin into the engine.
214214
func (e *StdEngine) LoadPlugin(p plugin.EnginePlugin) error {
215+
return e.loadPluginInternal(p, false)
216+
}
217+
218+
// LoadPluginWithOverride loads an EnginePlugin into the engine, allowing it to
219+
// override existing module, step, trigger, handler, deploy target, and sidecar
220+
// provider registrations. When a duplicate type is encountered, the new factory
221+
// replaces the previous one and a warning is logged, instead of returning an
222+
// error. This is intended for external plugins that intentionally replace
223+
// built-in defaults (e.g., replacing a mock step with a production
224+
// implementation, or swapping out deploy targets/sidecar providers).
225+
func (e *StdEngine) LoadPluginWithOverride(p plugin.EnginePlugin) error {
226+
return e.loadPluginInternal(p, true)
227+
}
228+
229+
func (e *StdEngine) loadPluginInternal(p plugin.EnginePlugin, allowOverride bool) error {
215230
loader := e.PluginLoader()
216-
if err := loader.LoadPlugin(p); err != nil {
231+
var err error
232+
if allowOverride {
233+
err = loader.LoadPluginWithOverride(p)
234+
} else {
235+
err = loader.LoadPlugin(p)
236+
}
237+
if err != nil {
217238
return fmt.Errorf("load plugin: %w", err)
218239
}
219240
for typeName, factory := range p.ModuleFactories() {

‎plugin/loader.go‎

Lines changed: 61 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,23 @@ func (l *PluginLoader) LoadBinaryPlugin(p EnginePlugin, binaryPath, sigPath, cer
7676
return l.LoadPlugin(p)
7777
}
7878

79-
// ValidateTier checks whether a plugin's tier is allowed given the current
79+
// LoadBinaryPluginWithOverride is the override-capable counterpart to
80+
// LoadBinaryPlugin. It verifies the plugin binary with cosign (for premium
81+
// plugins) and then loads the plugin, allowing it to override existing module,
82+
// step, trigger, handler, deploy target, and sidecar provider registrations.
83+
// When a duplicate type is encountered, the new factory replaces the previous
84+
// one and a warning is logged instead of returning an error.
85+
func (l *PluginLoader) LoadBinaryPluginWithOverride(p EnginePlugin, binaryPath, sigPath, certPath string) error {
86+
manifest := p.EngineManifest()
87+
if manifest.Tier == TierPremium && l.cosignVerifier != nil {
88+
if err := l.cosignVerifier.Verify(binaryPath, sigPath, certPath); err != nil {
89+
return fmt.Errorf("plugin %q: binary verification failed: %w", manifest.Name, err)
90+
}
91+
}
92+
return l.LoadPluginWithOverride(p)
93+
}
94+
95+
8096
// license validator configuration:
8197
// - Core and Community plugins are always allowed.
8298
// - Premium plugins are validated against the LicenseValidator if one is set.
@@ -106,6 +122,20 @@ func (l *PluginLoader) ValidateTier(manifest *PluginManifest) error {
106122
// schemas, and wiring hooks. Returns an error if any factory type conflicts with
107123
// an existing registration.
108124
func (l *PluginLoader) LoadPlugin(p EnginePlugin) error {
125+
return l.loadPlugin(p, false)
126+
}
127+
128+
// LoadPluginWithOverride is like LoadPlugin but allows the plugin to override
129+
// existing module, step, trigger, handler, deploy target, and sidecar provider
130+
// registrations. When a duplicate type is encountered, the new factory replaces
131+
// the previous one and a warning is logged instead of returning an error.
132+
// This is intended for external plugins that intentionally replace built-in
133+
// defaults (e.g., replacing a mock authz step with a production implementation).
134+
func (l *PluginLoader) LoadPluginWithOverride(p EnginePlugin) error {
135+
return l.loadPlugin(p, true)
136+
}
137+
138+
func (l *PluginLoader) loadPlugin(p EnginePlugin, allowOverride bool) error {
109139
manifest := p.EngineManifest()
110140
if err := manifest.Validate(); err != nil {
111141
return fmt.Errorf("plugin %q: %w", manifest.Name, err)
@@ -132,34 +162,46 @@ func (l *PluginLoader) LoadPlugin(p EnginePlugin) error {
132162
}
133163
}
134164

135-
// Register module factories — conflict on duplicate type.
165+
// Register module factories — conflict on duplicate type unless override allowed.
136166
for typeName, factory := range p.ModuleFactories() {
137167
if _, exists := l.moduleFactories[typeName]; exists {
138-
return fmt.Errorf("plugin %q: module type %q already registered", manifest.Name, typeName)
168+
if !allowOverride {
169+
return fmt.Errorf("plugin %q: module type %q already registered", manifest.Name, typeName)
170+
}
171+
slog.Warn("plugin overriding existing module type", "plugin", manifest.Name, "type", typeName)
139172
}
140173
l.moduleFactories[typeName] = factory
141174
}
142175

143-
// Register step factories — conflict on duplicate type.
176+
// Register step factories — conflict on duplicate type unless override allowed.
144177
for typeName, factory := range p.StepFactories() {
145178
if _, exists := l.stepFactories[typeName]; exists {
146-
return fmt.Errorf("plugin %q: step type %q already registered", manifest.Name, typeName)
179+
if !allowOverride {
180+
return fmt.Errorf("plugin %q: step type %q already registered", manifest.Name, typeName)
181+
}
182+
slog.Warn("plugin overriding existing step type", "plugin", manifest.Name, "type", typeName)
147183
}
148184
l.stepFactories[typeName] = factory
149185
}
150186

151-
// Register trigger factories — conflict on duplicate type.
187+
// Register trigger factories — conflict on duplicate type unless override allowed.
152188
for typeName, factory := range p.TriggerFactories() {
153189
if _, exists := l.triggerFactories[typeName]; exists {
154-
return fmt.Errorf("plugin %q: trigger type %q already registered", manifest.Name, typeName)
190+
if !allowOverride {
191+
return fmt.Errorf("plugin %q: trigger type %q already registered", manifest.Name, typeName)
192+
}
193+
slog.Warn("plugin overriding existing trigger type", "plugin", manifest.Name, "type", typeName)
155194
}
156195
l.triggerFactories[typeName] = factory
157196
}
158197

159-
// Register workflow handler factories — conflict on duplicate type.
198+
// Register workflow handler factories — conflict on duplicate type unless override allowed.
160199
for typeName, factory := range p.WorkflowHandlers() {
161200
if _, exists := l.handlerFactories[typeName]; exists {
162-
return fmt.Errorf("plugin %q: workflow handler type %q already registered", manifest.Name, typeName)
201+
if !allowOverride {
202+
return fmt.Errorf("plugin %q: workflow handler type %q already registered", manifest.Name, typeName)
203+
}
204+
slog.Warn("plugin overriding existing workflow handler type", "plugin", manifest.Name, "type", typeName)
163205
}
164206
l.handlerFactories[typeName] = factory
165207
}
@@ -175,18 +217,24 @@ func (l *PluginLoader) LoadPlugin(p EnginePlugin) error {
175217
// Collect config transform hooks.
176218
l.configTransformHooks = append(l.configTransformHooks, p.ConfigTransformHooks()...)
177219

178-
// Register deploy targets — conflict on duplicate name.
220+
// Register deploy targets — conflict on duplicate name unless override allowed.
179221
for name, target := range p.DeployTargets() {
180222
if _, exists := l.deployTargets[name]; exists {
181-
return fmt.Errorf("plugin %q: deploy target %q already registered", manifest.Name, name)
223+
if !allowOverride {
224+
return fmt.Errorf("plugin %q: deploy target %q already registered", manifest.Name, name)
225+
}
226+
slog.Warn("plugin overriding existing deploy target", "plugin", manifest.Name, "target", name)
182227
}
183228
l.deployTargets[name] = target
184229
}
185230

186-
// Register sidecar providers — conflict on duplicate type.
231+
// Register sidecar providers — conflict on duplicate type unless override allowed.
187232
for typeName, provider := range p.SidecarProviders() {
188233
if _, exists := l.sidecarProviders[typeName]; exists {
189-
return fmt.Errorf("plugin %q: sidecar provider %q already registered", manifest.Name, typeName)
234+
if !allowOverride {
235+
return fmt.Errorf("plugin %q: sidecar provider %q already registered", manifest.Name, typeName)
236+
}
237+
slog.Warn("plugin overriding existing sidecar provider", "plugin", manifest.Name, "type", typeName)
190238
}
191239
l.sidecarProviders[typeName] = provider
192240
}

‎plugin/loader_test.go‎

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
package plugin
22

33
import (
4+
"context"
5+
"io"
46
"testing"
57

68
"github.com/CrisisTextLine/modular"
79
"github.com/GoCodeAlone/workflow/capability"
810
"github.com/GoCodeAlone/workflow/config"
11+
"github.com/GoCodeAlone/workflow/deploy"
912
"github.com/GoCodeAlone/workflow/schema"
1013
)
1114

@@ -166,6 +169,158 @@ func TestPluginLoader_DuplicateModuleTypeConflict(t *testing.T) {
166169
}
167170
}
168171

172+
func TestPluginLoader_LoadPluginWithOverride_ModuleType(t *testing.T) {
173+
loader := newTestEngineLoader()
174+
175+
p1 := &modulePlugin{
176+
BaseEnginePlugin: *makeEnginePlugin("builtin-plugin", "1.0.0", nil),
177+
modules: map[string]ModuleFactory{
178+
"shared.module": func(name string, cfg map[string]any) modular.Module {
179+
return nil
180+
},
181+
},
182+
}
183+
p2 := &modulePlugin{
184+
BaseEnginePlugin: *makeEnginePlugin("external-plugin", "1.0.0", nil),
185+
modules: map[string]ModuleFactory{
186+
"shared.module": func(name string, cfg map[string]any) modular.Module {
187+
return nil
188+
},
189+
},
190+
}
191+
192+
if err := loader.LoadPlugin(p1); err != nil {
193+
t.Fatalf("first load should succeed: %v", err)
194+
}
195+
// LoadPlugin should still reject duplicates.
196+
if err := loader.LoadPlugin(p2); err == nil {
197+
t.Fatal("expected duplicate module type error from LoadPlugin")
198+
}
199+
// LoadPluginWithOverride should allow replacing the type.
200+
if err := loader.LoadPluginWithOverride(p2); err != nil {
201+
t.Fatalf("LoadPluginWithOverride should succeed: %v", err)
202+
}
203+
if got := len(loader.ModuleFactories()); got != 1 {
204+
t.Errorf("expected 1 module factory after override, got %d", got)
205+
}
206+
if got := len(loader.LoadedPlugins()); got != 2 {
207+
t.Errorf("expected 2 loaded plugins, got %d", got)
208+
}
209+
}
210+
211+
func TestPluginLoader_LoadPluginWithOverride_StepType(t *testing.T) {
212+
loader := newTestEngineLoader()
213+
214+
p1 := &modulePlugin{
215+
BaseEnginePlugin: *makeEnginePlugin("builtin-steps", "1.0.0", nil),
216+
steps: map[string]StepFactory{
217+
"step.authz_check": func(name string, cfg map[string]any, _ modular.Application) (any, error) {
218+
return "builtin", nil
219+
},
220+
},
221+
}
222+
p2 := &modulePlugin{
223+
BaseEnginePlugin: *makeEnginePlugin("external-authz", "1.0.0", nil),
224+
steps: map[string]StepFactory{
225+
"step.authz_check": func(name string, cfg map[string]any, _ modular.Application) (any, error) {
226+
return "external", nil
227+
},
228+
},
229+
}
230+
231+
if err := loader.LoadPlugin(p1); err != nil {
232+
t.Fatalf("first load should succeed: %v", err)
233+
}
234+
// LoadPlugin should still reject duplicate step types.
235+
if err := loader.LoadPlugin(p2); err == nil {
236+
t.Fatal("expected duplicate step type error from LoadPlugin")
237+
}
238+
if err := loader.LoadPluginWithOverride(p2); err != nil {
239+
t.Fatalf("LoadPluginWithOverride should succeed: %v", err)
240+
}
241+
242+
// Verify the override replaced the factory.
243+
factories := loader.StepFactories()
244+
if got := len(factories); got != 1 {
245+
t.Fatalf("expected 1 step factory, got %d", got)
246+
}
247+
result, err := factories["step.authz_check"]("test", nil, nil)
248+
if err != nil {
249+
t.Fatalf("step factory returned error: %v", err)
250+
}
251+
if result != "external" {
252+
t.Errorf("expected overridden factory to return %q, got %q", "external", result)
253+
}
254+
}
255+
256+
func TestPluginLoader_LoadPluginWithOverride_AllTypes(t *testing.T) {
257+
loader := newTestEngineLoader()
258+
259+
p1 := &fullPlugin{
260+
BaseEnginePlugin: *makeEnginePlugin("builtin", "1.0.0", nil),
261+
modules: map[string]ModuleFactory{
262+
"mod.type": func(name string, cfg map[string]any) modular.Module { return nil },
263+
},
264+
steps: map[string]StepFactory{
265+
"step.type": func(name string, cfg map[string]any, _ modular.Application) (any, error) { return nil, nil },
266+
},
267+
triggers: map[string]TriggerFactory{
268+
"trigger.type": func() any { return nil },
269+
},
270+
handlers: map[string]WorkflowHandlerFactory{
271+
"handler.type": func() any { return nil },
272+
},
273+
deployTargets: map[string]deploy.DeployTarget{"deploy.target": &mockDeployTarget{name: "builtin-target"}},
274+
sidecarProviders: map[string]deploy.SidecarProvider{"sidecar.type": &mockSidecarProvider{typeName: "builtin-sidecar"}},
275+
}
276+
p2 := &fullPlugin{
277+
BaseEnginePlugin: *makeEnginePlugin("external", "1.0.0", nil),
278+
modules: map[string]ModuleFactory{
279+
"mod.type": func(name string, cfg map[string]any) modular.Module { return nil },
280+
},
281+
steps: map[string]StepFactory{
282+
"step.type": func(name string, cfg map[string]any, _ modular.Application) (any, error) { return nil, nil },
283+
},
284+
triggers: map[string]TriggerFactory{
285+
"trigger.type": func() any { return nil },
286+
},
287+
handlers: map[string]WorkflowHandlerFactory{
288+
"handler.type": func() any { return nil },
289+
},
290+
deployTargets: map[string]deploy.DeployTarget{"deploy.target": &mockDeployTarget{name: "external-target"}},
291+
sidecarProviders: map[string]deploy.SidecarProvider{"sidecar.type": &mockSidecarProvider{typeName: "external-sidecar"}},
292+
}
293+
294+
if err := loader.LoadPlugin(p1); err != nil {
295+
t.Fatalf("first load should succeed: %v", err)
296+
}
297+
// Verify LoadPlugin rejects all duplicate types.
298+
if err := loader.LoadPlugin(p2); err == nil {
299+
t.Fatal("expected duplicate type error from LoadPlugin")
300+
}
301+
if err := loader.LoadPluginWithOverride(p2); err != nil {
302+
t.Fatalf("LoadPluginWithOverride should succeed for all types: %v", err)
303+
}
304+
if got := len(loader.ModuleFactories()); got != 1 {
305+
t.Errorf("expected 1 module factory, got %d", got)
306+
}
307+
if got := len(loader.StepFactories()); got != 1 {
308+
t.Errorf("expected 1 step factory, got %d", got)
309+
}
310+
if got := len(loader.TriggerFactories()); got != 1 {
311+
t.Errorf("expected 1 trigger factory, got %d", got)
312+
}
313+
if got := len(loader.WorkflowHandlerFactories()); got != 1 {
314+
t.Errorf("expected 1 handler factory, got %d", got)
315+
}
316+
if got := len(loader.DeployTargets()); got != 1 {
317+
t.Errorf("expected 1 deploy target, got %d", got)
318+
}
319+
if got := len(loader.SidecarProviders()); got != 1 {
320+
t.Errorf("expected 1 sidecar provider, got %d", got)
321+
}
322+
}
323+
169324
func TestPluginLoader_WiringHooksSortedByPriority(t *testing.T) {
170325
loader := newTestEngineLoader()
171326

@@ -235,3 +390,54 @@ type hookPlugin struct {
235390
}
236391

237392
func (p *hookPlugin) WiringHooks() []WiringHook { return p.hooks }
393+
394+
// fullPlugin embeds BaseEnginePlugin and overrides all factory methods including
395+
// deploy targets and sidecar providers.
396+
type fullPlugin struct {
397+
BaseEnginePlugin
398+
modules map[string]ModuleFactory
399+
steps map[string]StepFactory
400+
triggers map[string]TriggerFactory
401+
handlers map[string]WorkflowHandlerFactory
402+
deployTargets map[string]deploy.DeployTarget
403+
sidecarProviders map[string]deploy.SidecarProvider
404+
}
405+
406+
func (p *fullPlugin) ModuleFactories() map[string]ModuleFactory { return p.modules }
407+
func (p *fullPlugin) StepFactories() map[string]StepFactory { return p.steps }
408+
func (p *fullPlugin) TriggerFactories() map[string]TriggerFactory { return p.triggers }
409+
func (p *fullPlugin) WorkflowHandlers() map[string]WorkflowHandlerFactory { return p.handlers }
410+
func (p *fullPlugin) DeployTargets() map[string]deploy.DeployTarget { return p.deployTargets }
411+
func (p *fullPlugin) SidecarProviders() map[string]deploy.SidecarProvider {
412+
return p.sidecarProviders
413+
}
414+
415+
// mockDeployTarget is a no-op deploy target for tests.
416+
type mockDeployTarget struct{ name string }
417+
418+
func (m *mockDeployTarget) Name() string { return m.name }
419+
func (m *mockDeployTarget) Generate(_ context.Context, _ *deploy.DeployRequest) (*deploy.DeployArtifacts, error) {
420+
return nil, nil
421+
}
422+
func (m *mockDeployTarget) Apply(_ context.Context, _ *deploy.DeployArtifacts, _ deploy.ApplyOpts) (*deploy.DeployResult, error) {
423+
return nil, nil
424+
}
425+
func (m *mockDeployTarget) Destroy(_ context.Context, _, _ string) error { return nil }
426+
func (m *mockDeployTarget) Status(_ context.Context, _, _ string) (*deploy.DeployStatus, error) {
427+
return nil, nil
428+
}
429+
func (m *mockDeployTarget) Diff(_ context.Context, _ *deploy.DeployArtifacts) (string, error) {
430+
return "", nil
431+
}
432+
func (m *mockDeployTarget) Logs(_ context.Context, _, _ string, _ deploy.LogOpts) (io.ReadCloser, error) {
433+
return nil, nil
434+
}
435+
436+
// mockSidecarProvider is a no-op sidecar provider for tests.
437+
type mockSidecarProvider struct{ typeName string }
438+
439+
func (m *mockSidecarProvider) Type() string { return m.typeName }
440+
func (m *mockSidecarProvider) Validate(_ config.SidecarConfig) error { return nil }
441+
func (m *mockSidecarProvider) Resolve(_ config.SidecarConfig, _ string) (*deploy.SidecarSpec, error) {
442+
return nil, nil
443+
}

0 commit comments

Comments
 (0)