-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Problem
In v0.3.11, the pipelinesteps plugin now registers a built-in step.authz_check step type. External plugins (e.g., workflow-plugin-authz) that also register step.authz_check fail to load via engine.LoadPlugin() because the plugin loader (plugin/loader.go line 146) rejects duplicate step type names.
Current Behavior
step type "step.authz_check" already registered
The plugin loader checks for duplicates and returns an error, preventing the external plugin from loading entirely.
Expected Behavior
External plugins should be able to override built-in step types. This is important because:
- The built-in
step.authz_checkonly works withPolicyEngineModule(which only supports "mock" backend viaresolvePolicyEngine()concrete type assertion) - The external
workflow-plugin-authzprovides a Casbin-backed implementation that is production-ready - External plugins should be treated as intentional overrides of built-in defaults
Workaround
We bypass the plugin loader entirely by registering external plugin factories directly via engine.AddModuleType() / engine.AddStepType() (which silently overwrite):
func registerExternalPlugin(engine *wf.StdEngine, p plugin.EnginePlugin) error {
for typeName, factory := range p.ModuleFactories() {
f := factory
engine.AddModuleType(typeName, func(name string, cfg map[string]any) modular.Module {
return f(name, cfg)
})
}
for typeName, factory := range p.StepFactories() {
capturedType := typeName
capturedFactory := factory
engine.AddStepType(typeName, func(name string, cfg map[string]any, app modular.Application) (module.PipelineStep, error) {
result, err := capturedFactory(name, cfg, app)
if err != nil {
return nil, err
}
step, ok := result.(module.PipelineStep)
if !ok {
return nil, fmt.Errorf("external plugin step factory for %q returned non-PipelineStep type %T", capturedType, result)
}
return step, nil
})
}
return nil
}Suggested Fix
Option A: Allow LoadPlugin() to accept an override flag or option that permits replacing existing types.
Option B: Change the plugin loader to log a warning instead of returning an error when an external plugin registers a type that already exists, treating external plugins as intentional overrides.
Option C: Add an engine.LoadPluginWithOverride() method that uses the existing loader but skips the duplicate check.