From 6bccda194560eb17bc7c427631d1fafc2e8d3e3c Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Mon, 20 Apr 2026 20:14:21 -0400 Subject: [PATCH 1/2] feat(deploy): wire real plugin loader into resolveIaCProvider - Replace the no-op stub with discoverAndLoadIaCProvider, which scans plugin.json files for capabilities.iacProvider.name, loads the matching plugin via ExternalPluginManager, and type-asserts the iac.provider module factory result to interfaces.IaCProvider. - Each failure mode (no plugin dir, no matching plugin, binary absent, no iac.provider module type, wrong type) returns an actionable error pointing at the fix command. - Refactor pluginDeployProvider to resolve lazily in ensureProvider(ctx) so the real request context is threaded through to Initialize. - Derive resourceType from the matched module's Type field instead of hardcoding "infra.container_service". - Add --plugin-dir flag to wfctl ci run (sets WFCTL_PLUGIN_DIR). Note: workflow-plugin-digitalocean v0.1.0 does not yet expose an iac.provider module type or ServiceInvoker handlers. Until it does, deploying via that plugin returns a clear "upgrade with wfctl plugin update" error rather than a silent no-op. Co-Authored-By: Claude Sonnet 4.6 --- cmd/wfctl/ci_run.go | 4 + cmd/wfctl/deploy_plugin_loader_test.go | 166 +++++++++++++++++++++++++ cmd/wfctl/deploy_providers.go | 149 +++++++++++++++++++--- 3 files changed, 304 insertions(+), 15 deletions(-) create mode 100644 cmd/wfctl/deploy_plugin_loader_test.go diff --git a/cmd/wfctl/ci_run.go b/cmd/wfctl/ci_run.go index a873a6082..4e6a3a978 100644 --- a/cmd/wfctl/ci_run.go +++ b/cmd/wfctl/ci_run.go @@ -20,6 +20,7 @@ func runCIRun(args []string) error { phases := fs.String("phase", "build,test", "Comma-separated phases: build, test, deploy") env := fs.String("env", "", "Target environment (required for deploy phase)") verbose := fs.Bool("verbose", false, "Show detailed output") + pluginDir := fs.String("plugin-dir", "", "Directory containing installed plugins (default: $WFCTL_PLUGIN_DIR or ./data/plugins)") fs.Usage = func() { fmt.Fprintf(fs.Output(), "Usage: wfctl ci run [options]\n\nRun CI phases from workflow config.\n\nOptions:\n") fs.PrintDefaults() @@ -60,6 +61,9 @@ func runCIRun(args []string) error { if *env == "" { return fmt.Errorf("--env is required for deploy phase") } + if *pluginDir != "" { + os.Setenv("WFCTL_PLUGIN_DIR", *pluginDir) //nolint:errcheck + } if len(cfg.Services) > 0 { if err := runMultiServiceDeploy(cfg.CI.Deploy, *env, &cfg, cfg.Services, *verbose); err != nil { return fmt.Errorf("deploy phase failed: %w", err) diff --git a/cmd/wfctl/deploy_plugin_loader_test.go b/cmd/wfctl/deploy_plugin_loader_test.go new file mode 100644 index 000000000..b9d2cd661 --- /dev/null +++ b/cmd/wfctl/deploy_plugin_loader_test.go @@ -0,0 +1,166 @@ +package main + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/GoCodeAlone/workflow/config" + "github.com/GoCodeAlone/workflow/interfaces" +) + +// TestDefaultResolveIaCProvider_IsNotPlaceholder verifies the default +// resolveIaCProvider no longer returns the old "no in-process provider loader" +// stub message. +func TestDefaultResolveIaCProvider_IsNotPlaceholder(t *testing.T) { + t.Setenv("WFCTL_PLUGIN_DIR", t.TempDir()) // empty dir — no plugins + _, err := resolveIaCProvider(context.Background(), "any-provider", nil) + if err == nil { + t.Skip("resolveIaCProvider succeeded unexpectedly") + } + if strings.Contains(err.Error(), "no in-process provider loader") { + t.Errorf("default resolveIaCProvider still returns old placeholder message: %v", err) + } +} + +// TestDefaultResolveIaCProvider_NoPluginDir verifies the error hints at +// 'wfctl plugin install' when the plugin directory does not exist. +func TestDefaultResolveIaCProvider_NoPluginDir(t *testing.T) { + t.Setenv("WFCTL_PLUGIN_DIR", filepath.Join(t.TempDir(), "nonexistent")) + _, err := resolveIaCProvider(context.Background(), "fake-provider", nil) + if err == nil { + t.Fatal("expected error when plugin dir does not exist") + } + if !strings.Contains(err.Error(), "wfctl plugin install") { + t.Errorf("expected hint to run 'wfctl plugin install', got: %v", err) + } +} + +// TestDefaultResolveIaCProvider_NoMatchingPlugin verifies the error hints at +// 'wfctl plugin install' when no plugin declares the requested iacProvider name. +func TestDefaultResolveIaCProvider_NoMatchingPlugin(t *testing.T) { + dir := t.TempDir() + pluginSubDir := filepath.Join(dir, "some-plugin") + if err := os.MkdirAll(pluginSubDir, 0o755); err != nil { + t.Fatal(err) + } + manifest := `{"name":"some-plugin","version":"0.1.0","capabilities":{"iacProvider":{"name":"other-provider"}}}` + if err := os.WriteFile(filepath.Join(pluginSubDir, "plugin.json"), []byte(manifest), 0o644); err != nil { + t.Fatal(err) + } + + t.Setenv("WFCTL_PLUGIN_DIR", dir) + _, err := resolveIaCProvider(context.Background(), "digitalocean", nil) + if err == nil { + t.Fatal("expected error when no matching plugin found") + } + if !strings.Contains(err.Error(), "wfctl plugin install") { + t.Errorf("expected hint to run 'wfctl plugin install', got: %v", err) + } +} + +// TestDefaultResolveIaCProvider_MatchingPlugin_NotLoaded verifies that when a +// matching plugin is found (correct iacProvider.name) but the plugin binary is +// absent, the error is specific about the plugin name rather than silently +// falling through to a generic message. +func TestDefaultResolveIaCProvider_MatchingPlugin_NotLoaded(t *testing.T) { + dir := t.TempDir() + pluginSubDir := filepath.Join(dir, "workflow-plugin-digitalocean") + if err := os.MkdirAll(pluginSubDir, 0o755); err != nil { + t.Fatal(err) + } + manifest := `{"name":"workflow-plugin-digitalocean","version":"0.1.0","capabilities":{"iacProvider":{"name":"digitalocean"}}}` + if err := os.WriteFile(filepath.Join(pluginSubDir, "plugin.json"), []byte(manifest), 0o644); err != nil { + t.Fatal(err) + } + // No binary present → load will fail. + + t.Setenv("WFCTL_PLUGIN_DIR", dir) + _, err := resolveIaCProvider(context.Background(), "digitalocean", nil) + if err == nil { + t.Fatal("expected error when plugin binary is absent") + } + if !strings.Contains(err.Error(), "workflow-plugin-digitalocean") { + t.Errorf("expected plugin name in error message, got: %v", err) + } +} + +// TestPluginDeployProvider_LazyResolution verifies that resolveIaCProvider is +// NOT called at construction time and IS called on the first Deploy. +func TestPluginDeployProvider_LazyResolution(t *testing.T) { + callCount := 0 + orig := resolveIaCProvider + defer func() { resolveIaCProvider = orig }() + + driver := &fakeResourceDriver{} + fake := &fakeIaCProvider{ + name: "fake-cloud", + drivers: map[string]interfaces.ResourceDriver{"infra.container_service": driver}, + } + resolveIaCProvider = func(_ context.Context, _ string, _ map[string]any) (interfaces.IaCProvider, error) { + callCount++ + return fake, nil + } + + cfg := makePluginTestConfig("fake-cloud", "fake-provider") + p, err := newDeployProvider("fake-cloud", cfg) + if err != nil { + t.Fatalf("newDeployProvider: %v", err) + } + + if callCount != 0 { + t.Errorf("resolveIaCProvider called at construction (%d times); want 0", callCount) + } + + deployCfg := DeployConfig{ + AppName: "my-app", + ImageTag: "registry.example.com/myapp:v1", + Env: &config.CIDeployEnvironment{}, + } + if err := p.Deploy(context.Background(), deployCfg); err != nil { + t.Fatalf("Deploy: %v", err) + } + if callCount != 1 { + t.Errorf("resolveIaCProvider call count after first Deploy: got %d, want 1", callCount) + } + + // Second Deploy must reuse cached provider. + if err := p.Deploy(context.Background(), deployCfg); err != nil { + t.Fatalf("Deploy (second): %v", err) + } + if callCount != 1 { + t.Errorf("resolveIaCProvider called again on second Deploy: got %d total calls, want 1", callCount) + } +} + +// TestPluginDeployProvider_ResourceTypeFromModule verifies the resource type +// is taken from the infra module's Type field, not hardcoded. +func TestPluginDeployProvider_ResourceTypeFromModule(t *testing.T) { + driver := &fakeResourceDriver{} + fake := &fakeIaCProvider{ + name: "fake-cloud", + drivers: map[string]interfaces.ResourceDriver{"infra.container_service": driver}, + } + + orig := resolveIaCProvider + defer func() { resolveIaCProvider = orig }() + resolveIaCProvider = func(_ context.Context, _ string, _ map[string]any) (interfaces.IaCProvider, error) { + return fake, nil + } + + cfg := makePluginTestConfig("fake-cloud", "fake-provider") + p, err := newDeployProvider("fake-cloud", cfg) + if err != nil { + t.Fatalf("newDeployProvider: %v", err) + } + + pp, ok := p.(*pluginDeployProvider) + if !ok { + t.Fatalf("expected *pluginDeployProvider, got %T", p) + } + if pp.resourceType != "infra.container_service" { + t.Errorf("resourceType = %q; want %q", pp.resourceType, "infra.container_service") + } +} diff --git a/cmd/wfctl/deploy_providers.go b/cmd/wfctl/deploy_providers.go index d62fe3dfd..c727556d0 100644 --- a/cmd/wfctl/deploy_providers.go +++ b/cmd/wfctl/deploy_providers.go @@ -3,16 +3,19 @@ package main import ( "bytes" "context" + "encoding/json" "fmt" "net/http" "os" "os/exec" + "path/filepath" "strings" "text/template" "time" "github.com/GoCodeAlone/workflow/config" "github.com/GoCodeAlone/workflow/interfaces" + "github.com/GoCodeAlone/workflow/plugin/external" ) // DeployConfig holds all parameters needed to execute a deployment. @@ -58,10 +61,104 @@ func newDeployProvider(provider string, wfCfg *config.WorkflowConfig) (DeployPro } } -// resolveIaCProvider is the factory used by newPluginDeployProvider to obtain a -// live IaCProvider from module config. Tests override this to inject fakes. -var resolveIaCProvider = func(_ context.Context, _ string, _ map[string]any) (interfaces.IaCProvider, error) { - return nil, fmt.Errorf("no in-process provider loader available; use pre-deploy steps with 'wfctl infra apply' to deploy via a workflow plugin") +// resolveIaCProvider is the factory used by pluginDeployProvider.ensureProvider +// to load a live IaCProvider from an installed external plugin. Tests override +// this var to inject fakes without touching the filesystem. +var resolveIaCProvider = func(ctx context.Context, providerName string, cfg map[string]any) (interfaces.IaCProvider, error) { + return discoverAndLoadIaCProvider(ctx, providerName, cfg) +} + +// iacPluginManifest is the minimal shape needed to read capabilities.iacProvider.name +// from a plugin.json without relying on the full PluginCapabilities struct. +type iacPluginManifest struct { + Capabilities struct { + IaCProvider struct { + Name string `json:"name"` + } `json:"iacProvider"` + } `json:"capabilities"` +} + +// findIaCPluginDir scans pluginDir subdirectories for a plugin.json that +// declares capabilities.iacProvider.name == providerName. +// Returns ("", false, nil) when not found; ("name", true/false, nil) when the +// manifest matches (hasBinary indicates whether the executable is present). +func findIaCPluginDir(pluginDir, providerName string) (name string, hasBinary bool, err error) { + entries, err := os.ReadDir(pluginDir) + if err != nil { + if os.IsNotExist(err) { + return "", false, nil + } + return "", false, fmt.Errorf("scan plugin directory %q: %w", pluginDir, err) + } + for _, entry := range entries { + if !entry.IsDir() { + continue + } + pluginName := entry.Name() + data, readErr := os.ReadFile(filepath.Join(pluginDir, pluginName, "plugin.json")) + if readErr != nil { + continue + } + var m iacPluginManifest + if jsonErr := json.Unmarshal(data, &m); jsonErr != nil { + continue + } + if m.Capabilities.IaCProvider.Name != providerName { + continue + } + binaryPath := filepath.Join(pluginDir, pluginName, pluginName) + _, statErr := os.Stat(binaryPath) + return pluginName, statErr == nil, nil + } + return "", false, nil +} + +// discoverAndLoadIaCProvider implements the default resolveIaCProvider: it scans +// the plugin directory for a plugin that declares iacProvider.name == providerName, +// loads it via ExternalPluginManager, and returns the IaCProvider. +func discoverAndLoadIaCProvider(ctx context.Context, providerName string, cfg map[string]any) (interfaces.IaCProvider, error) { + pluginDir := os.Getenv("WFCTL_PLUGIN_DIR") + if pluginDir == "" { + pluginDir = "./data/plugins" + } + + pluginName, hasBinary, err := findIaCPluginDir(pluginDir, providerName) + if err != nil { + return nil, fmt.Errorf("resolve IaC provider %q: %w", providerName, err) + } + if pluginName == "" { + return nil, fmt.Errorf("no plugin found for IaC provider %q in %s — run: wfctl plugin install ", providerName, pluginDir) + } + if !hasBinary { + return nil, fmt.Errorf("plugin %q declares provider %q but binary is missing — run: wfctl plugin install %s", pluginName, providerName, pluginName) + } + + mgr := external.NewExternalPluginManager(pluginDir, nil) + adapter, loadErr := mgr.LoadPlugin(pluginName) + if loadErr != nil { + return nil, fmt.Errorf("load plugin %q for provider %q: %w", pluginName, providerName, loadErr) + } + + factories := adapter.ModuleFactories() + factory, ok := factories["iac.provider"] + if !ok { + return nil, fmt.Errorf("plugin %q does not expose an iac.provider module type — upgrade with: wfctl plugin update %s", pluginName, pluginName) + } + + mod := factory("iac-provider", cfg) + if mod == nil { + return nil, fmt.Errorf("plugin %q iac.provider factory returned nil", pluginName) + } + + iacProvider, ok := mod.(interfaces.IaCProvider) + if !ok { + return nil, fmt.Errorf("plugin %q iac.provider module (%T) does not implement interfaces.IaCProvider — upgrade with: wfctl plugin update %s", pluginName, mod, pluginName) + } + + if initErr := iacProvider.Initialize(ctx, cfg); initErr != nil { + return nil, fmt.Errorf("initialize provider %q: %w", providerName, initErr) + } + return iacProvider, nil } // newPluginDeployProvider looks up a matching iac.provider + infra.container_service @@ -90,45 +187,64 @@ func newPluginDeployProvider(providerName string, wfCfg *config.WorkflowConfig) return nil, fmt.Errorf("unsupported deploy provider %q (built-ins: kubernetes, docker, aws-ecs; to use a plugin provider, declare an iac.provider module in your workflow config)%s", providerName, fmt.Sprintf(hint, providerName)) } - // Find the first infra.container_service module referencing this provider. - var resourceName string + // Find the first infra resource module referencing this provider. + var resourceName, resourceType string var resourceCfg map[string]any for _, m := range wfCfg.Modules { - if m.Type != "infra.container_service" { + if m.Type == "iac.provider" || m.Type == "" { continue } if p, _ := m.Config["provider"].(string); p == providerModName { resourceName = m.Name + resourceType = m.Type resourceCfg = m.Config break } } if resourceName == "" { - return nil, fmt.Errorf("no infra.container_service module found for provider %q in workflow config", providerModName) - } - - iacProvider, err := resolveIaCProvider(context.Background(), providerName, providerModCfg) - if err != nil { - return nil, fmt.Errorf("resolve provider %q: %w", providerName, err) + return nil, fmt.Errorf("no infra resource module found for provider %q in workflow config", providerModName) } + // Provider is resolved lazily on first Deploy/HealthCheck to thread the real ctx. return &pluginDeployProvider{ - provider: iacProvider, + providerName: providerName, + providerCfg: providerModCfg, resourceName: resourceName, - resourceType: "infra.container_service", + resourceType: resourceType, resourceCfg: resourceCfg, }, nil } // pluginDeployProvider wraps an IaCProvider and a single infra resource as a DeployProvider. +// The IaCProvider is resolved lazily on first use so the real request context is threaded +// through to Initialize rather than a synthetic context.Background(). type pluginDeployProvider struct { + // lazy-resolution fields (set at construction) + providerName string + providerCfg map[string]any + // resolved on first ensureProvider call provider interfaces.IaCProvider resourceName string resourceType string resourceCfg map[string]any } +func (p *pluginDeployProvider) ensureProvider(ctx context.Context) error { + if p.provider != nil { + return nil + } + prov, err := resolveIaCProvider(ctx, p.providerName, p.providerCfg) + if err != nil { + return fmt.Errorf("resolve provider %q: %w", p.providerName, err) + } + p.provider = prov + return nil +} + func (p *pluginDeployProvider) Deploy(ctx context.Context, cfg DeployConfig) error { + if err := p.ensureProvider(ctx); err != nil { + return err + } driver, err := p.provider.ResourceDriver(p.resourceType) if err != nil { return fmt.Errorf("plugin deploy: no driver for %q: %w", p.resourceType, err) @@ -151,6 +267,9 @@ func (p *pluginDeployProvider) HealthCheck(ctx context.Context, cfg DeployConfig if cfg.Env == nil || cfg.Env.HealthCheck == nil { return nil } + if err := p.ensureProvider(ctx); err != nil { + return err + } driver, err := p.provider.ResourceDriver(p.resourceType) if err != nil { return fmt.Errorf("plugin health check: no driver for %q: %w", p.resourceType, err) From 2a219866c3203d786602814bb909ac7d6791951a Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Mon, 20 Apr 2026 20:26:16 -0400 Subject: [PATCH 2/2] fix(deploy): add plugin subprocess lifecycle + sync.Once for provider resolution - resolveIaCProvider now returns (IaCProvider, io.Closer, error); the Closer wraps mgr.Shutdown() via closerFunc so callers can reap the plugin subprocess when done. - discoverAndLoadIaCProvider calls mgr.Shutdown() on every early-exit error path so no subprocess is leaked when loading fails. - runDeployPhaseWithConfig defers Close() on any provider that implements io.Closer, ensuring the subprocess is reaped after deploy. - pluginDeployProvider.ensureProvider is guarded by sync.Once (safe for concurrent Deploy/HealthCheck) and short-circuits when provider is already set (preserves test injection pattern). - Update all resolveIaCProvider test overrides to the new 3-return signature with nil closer. Co-Authored-By: Claude Sonnet 4.6 --- cmd/wfctl/ci_run.go | 4 ++ cmd/wfctl/deploy_plugin_loader_test.go | 17 ++--- cmd/wfctl/deploy_providers.go | 78 ++++++++++++++++------- cmd/wfctl/deploy_providers_plugin_test.go | 5 +- 4 files changed, 72 insertions(+), 32 deletions(-) diff --git a/cmd/wfctl/ci_run.go b/cmd/wfctl/ci_run.go index 4e6a3a978..a723391ba 100644 --- a/cmd/wfctl/ci_run.go +++ b/cmd/wfctl/ci_run.go @@ -4,6 +4,7 @@ import ( "context" "flag" "fmt" + "io" "os" "os/exec" "runtime" @@ -366,6 +367,9 @@ func runDeployPhaseWithConfig( if err != nil { return err } + if c, ok := provider.(io.Closer); ok { + defer c.Close() //nolint:errcheck + } deployCfg := DeployConfig{ EnvName: envName, diff --git a/cmd/wfctl/deploy_plugin_loader_test.go b/cmd/wfctl/deploy_plugin_loader_test.go index b9d2cd661..18943991e 100644 --- a/cmd/wfctl/deploy_plugin_loader_test.go +++ b/cmd/wfctl/deploy_plugin_loader_test.go @@ -2,6 +2,7 @@ package main import ( "context" + "io" "os" "path/filepath" "strings" @@ -16,7 +17,7 @@ import ( // stub message. func TestDefaultResolveIaCProvider_IsNotPlaceholder(t *testing.T) { t.Setenv("WFCTL_PLUGIN_DIR", t.TempDir()) // empty dir — no plugins - _, err := resolveIaCProvider(context.Background(), "any-provider", nil) + _, _, err := resolveIaCProvider(context.Background(), "any-provider", nil) if err == nil { t.Skip("resolveIaCProvider succeeded unexpectedly") } @@ -29,7 +30,7 @@ func TestDefaultResolveIaCProvider_IsNotPlaceholder(t *testing.T) { // 'wfctl plugin install' when the plugin directory does not exist. func TestDefaultResolveIaCProvider_NoPluginDir(t *testing.T) { t.Setenv("WFCTL_PLUGIN_DIR", filepath.Join(t.TempDir(), "nonexistent")) - _, err := resolveIaCProvider(context.Background(), "fake-provider", nil) + _, _, err := resolveIaCProvider(context.Background(), "fake-provider", nil) if err == nil { t.Fatal("expected error when plugin dir does not exist") } @@ -52,7 +53,7 @@ func TestDefaultResolveIaCProvider_NoMatchingPlugin(t *testing.T) { } t.Setenv("WFCTL_PLUGIN_DIR", dir) - _, err := resolveIaCProvider(context.Background(), "digitalocean", nil) + _, _, err := resolveIaCProvider(context.Background(), "digitalocean", nil) if err == nil { t.Fatal("expected error when no matching plugin found") } @@ -78,7 +79,7 @@ func TestDefaultResolveIaCProvider_MatchingPlugin_NotLoaded(t *testing.T) { // No binary present → load will fail. t.Setenv("WFCTL_PLUGIN_DIR", dir) - _, err := resolveIaCProvider(context.Background(), "digitalocean", nil) + _, _, err := resolveIaCProvider(context.Background(), "digitalocean", nil) if err == nil { t.Fatal("expected error when plugin binary is absent") } @@ -99,9 +100,9 @@ func TestPluginDeployProvider_LazyResolution(t *testing.T) { name: "fake-cloud", drivers: map[string]interfaces.ResourceDriver{"infra.container_service": driver}, } - resolveIaCProvider = func(_ context.Context, _ string, _ map[string]any) (interfaces.IaCProvider, error) { + resolveIaCProvider = func(_ context.Context, _ string, _ map[string]any) (interfaces.IaCProvider, io.Closer, error) { callCount++ - return fake, nil + return fake, nil, nil } cfg := makePluginTestConfig("fake-cloud", "fake-provider") @@ -146,8 +147,8 @@ func TestPluginDeployProvider_ResourceTypeFromModule(t *testing.T) { orig := resolveIaCProvider defer func() { resolveIaCProvider = orig }() - resolveIaCProvider = func(_ context.Context, _ string, _ map[string]any) (interfaces.IaCProvider, error) { - return fake, nil + resolveIaCProvider = func(_ context.Context, _ string, _ map[string]any) (interfaces.IaCProvider, io.Closer, error) { + return fake, nil, nil } cfg := makePluginTestConfig("fake-cloud", "fake-provider") diff --git a/cmd/wfctl/deploy_providers.go b/cmd/wfctl/deploy_providers.go index c727556d0..4408bffaf 100644 --- a/cmd/wfctl/deploy_providers.go +++ b/cmd/wfctl/deploy_providers.go @@ -5,11 +5,13 @@ import ( "context" "encoding/json" "fmt" + "io" "net/http" "os" "os/exec" "path/filepath" "strings" + "sync" "text/template" "time" @@ -62,9 +64,11 @@ func newDeployProvider(provider string, wfCfg *config.WorkflowConfig) (DeployPro } // resolveIaCProvider is the factory used by pluginDeployProvider.ensureProvider -// to load a live IaCProvider from an installed external plugin. Tests override -// this var to inject fakes without touching the filesystem. -var resolveIaCProvider = func(ctx context.Context, providerName string, cfg map[string]any) (interfaces.IaCProvider, error) { +// to load a live IaCProvider from an installed external plugin. It returns both +// the provider and an io.Closer that shuts down any background subprocess. +// Tests override this var to inject fakes without touching the filesystem; +// they may return nil for the closer. +var resolveIaCProvider = func(ctx context.Context, providerName string, cfg map[string]any) (interfaces.IaCProvider, io.Closer, error) { return discoverAndLoadIaCProvider(ctx, providerName, cfg) } @@ -115,8 +119,9 @@ func findIaCPluginDir(pluginDir, providerName string) (name string, hasBinary bo // discoverAndLoadIaCProvider implements the default resolveIaCProvider: it scans // the plugin directory for a plugin that declares iacProvider.name == providerName, -// loads it via ExternalPluginManager, and returns the IaCProvider. -func discoverAndLoadIaCProvider(ctx context.Context, providerName string, cfg map[string]any) (interfaces.IaCProvider, error) { +// loads it via ExternalPluginManager, and returns the IaCProvider plus a Closer +// that shuts down the plugin subprocess. The caller must call Close() when done. +func discoverAndLoadIaCProvider(ctx context.Context, providerName string, cfg map[string]any) (interfaces.IaCProvider, io.Closer, error) { pluginDir := os.Getenv("WFCTL_PLUGIN_DIR") if pluginDir == "" { pluginDir = "./data/plugins" @@ -124,43 +129,55 @@ func discoverAndLoadIaCProvider(ctx context.Context, providerName string, cfg ma pluginName, hasBinary, err := findIaCPluginDir(pluginDir, providerName) if err != nil { - return nil, fmt.Errorf("resolve IaC provider %q: %w", providerName, err) + return nil, nil, fmt.Errorf("resolve IaC provider %q: %w", providerName, err) } if pluginName == "" { - return nil, fmt.Errorf("no plugin found for IaC provider %q in %s — run: wfctl plugin install ", providerName, pluginDir) + return nil, nil, fmt.Errorf("no plugin found for IaC provider %q in %s — run: wfctl plugin install ", providerName, pluginDir) } if !hasBinary { - return nil, fmt.Errorf("plugin %q declares provider %q but binary is missing — run: wfctl plugin install %s", pluginName, providerName, pluginName) + return nil, nil, fmt.Errorf("plugin %q declares provider %q but binary is missing — run: wfctl plugin install %s", pluginName, providerName, pluginName) } mgr := external.NewExternalPluginManager(pluginDir, nil) + closer := closerFunc(func() error { mgr.Shutdown(); return nil }) + adapter, loadErr := mgr.LoadPlugin(pluginName) if loadErr != nil { - return nil, fmt.Errorf("load plugin %q for provider %q: %w", pluginName, providerName, loadErr) + mgr.Shutdown() + return nil, nil, fmt.Errorf("load plugin %q for provider %q: %w", pluginName, providerName, loadErr) } factories := adapter.ModuleFactories() factory, ok := factories["iac.provider"] if !ok { - return nil, fmt.Errorf("plugin %q does not expose an iac.provider module type — upgrade with: wfctl plugin update %s", pluginName, pluginName) + mgr.Shutdown() + return nil, nil, fmt.Errorf("plugin %q does not expose an iac.provider module type — upgrade with: wfctl plugin update %s", pluginName, pluginName) } mod := factory("iac-provider", cfg) if mod == nil { - return nil, fmt.Errorf("plugin %q iac.provider factory returned nil", pluginName) + mgr.Shutdown() + return nil, nil, fmt.Errorf("plugin %q iac.provider factory returned nil", pluginName) } iacProvider, ok := mod.(interfaces.IaCProvider) if !ok { - return nil, fmt.Errorf("plugin %q iac.provider module (%T) does not implement interfaces.IaCProvider — upgrade with: wfctl plugin update %s", pluginName, mod, pluginName) + mgr.Shutdown() + return nil, nil, fmt.Errorf("plugin %q iac.provider module (%T) does not implement interfaces.IaCProvider — upgrade with: wfctl plugin update %s", pluginName, mod, pluginName) } if initErr := iacProvider.Initialize(ctx, cfg); initErr != nil { - return nil, fmt.Errorf("initialize provider %q: %w", providerName, initErr) + mgr.Shutdown() + return nil, nil, fmt.Errorf("initialize provider %q: %w", providerName, initErr) } - return iacProvider, nil + return iacProvider, closer, nil } +// closerFunc adapts a func() error to io.Closer. +type closerFunc func() error + +func (f closerFunc) Close() error { return f() } + // newPluginDeployProvider looks up a matching iac.provider + infra.container_service // module pair in wfCfg and wraps them as a DeployProvider. func newPluginDeployProvider(providerName string, wfCfg *config.WorkflowConfig) (DeployProvider, error) { @@ -222,22 +239,39 @@ type pluginDeployProvider struct { // lazy-resolution fields (set at construction) providerName string providerCfg map[string]any - // resolved on first ensureProvider call - provider interfaces.IaCProvider + // resource target (set at construction) resourceName string resourceType string resourceCfg map[string]any + // resolved once on first ensureProvider call + once sync.Once + provider interfaces.IaCProvider + provErr error + closer io.Closer } func (p *pluginDeployProvider) ensureProvider(ctx context.Context) error { - if p.provider != nil { - return nil + p.once.Do(func() { + if p.provider != nil { + return // already injected (e.g. by tests constructing the struct directly) + } + prov, closer, err := resolveIaCProvider(ctx, p.providerName, p.providerCfg) + p.provider = prov + p.closer = closer + p.provErr = err + }) + if p.provErr != nil { + return fmt.Errorf("resolve provider %q: %w", p.providerName, p.provErr) } - prov, err := resolveIaCProvider(ctx, p.providerName, p.providerCfg) - if err != nil { - return fmt.Errorf("resolve provider %q: %w", p.providerName, err) + return nil +} + +// Close shuts down the plugin subprocess, if any. The DeployProvider interface +// does not include Close; callers should type-assert to io.Closer after use. +func (p *pluginDeployProvider) Close() error { + if p.closer != nil { + return p.closer.Close() } - p.provider = prov return nil } diff --git a/cmd/wfctl/deploy_providers_plugin_test.go b/cmd/wfctl/deploy_providers_plugin_test.go index 65c204113..34febbd63 100644 --- a/cmd/wfctl/deploy_providers_plugin_test.go +++ b/cmd/wfctl/deploy_providers_plugin_test.go @@ -3,6 +3,7 @@ package main import ( "context" "fmt" + "io" "strings" "testing" @@ -137,8 +138,8 @@ func TestNewDeployProvider_PluginProvider_Resolves(t *testing.T) { orig := resolveIaCProvider defer func() { resolveIaCProvider = orig }() - resolveIaCProvider = func(_ context.Context, _ string, _ map[string]any) (interfaces.IaCProvider, error) { - return fake, nil + resolveIaCProvider = func(_ context.Context, _ string, _ map[string]any) (interfaces.IaCProvider, io.Closer, error) { + return fake, nil, nil } cfg := makePluginTestConfig("fake-cloud", "fake-provider")