From 65289570f86efbeede17bd88aa7bdc76fad21734 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Fri, 15 May 2026 16:28:54 -0400 Subject: [PATCH] feat(sdk): IaCServeOptions.TypedModules + .TypedSteps for strict-proto providers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two new map fields to IaCServeOptions and extends mapBackedProvider to implement TypedModuleProvider + TypedStepProvider. Lets IaC plugins register strict-proto module/step providers (sdk.NewTypedModuleFactory / sdk.NewTypedStepFactory) through the existing IaC bridge — previously the bridge accepted only the legacy ModuleProvider/StepProvider surface so strict-typed providers were unreachable. The dispatch contract (per grpc_server.go's existing CreateModule / CreateStep handlers) is Typed-first → legacy-fallback: a type in typedModules wins; absence returns ErrTypedContractNotHandled which causes the dispatcher to try the legacy modules map. Coexistence is fully supported — plugins can mix the two surfaces during migration. mapBackedProvider.ModuleTypes / StepTypes stay legacy-only; the host GetModuleTypes / GetStepTypes RPCs already merge TypedModuleTypes + ModuleTypes via mergeTypeLists (typed-primary-first, then legacy-only extras), so the union surfaces correctly through the existing path. 6 new tests in iacserver_modules_test.go cover: - typed-only GetModuleTypes (no legacy map present) - typed + legacy GetModuleTypes / GetStepTypes union via set-equality - typed-first dispatch through CreateModule / CreateStep - legacy-fallback when a type is not in the typed map Existing legacy-only tests (ModulesAndSteps_Delegate, Deterministic, ZeroValueOptions, NilBroker) continue to pass — backwards-compatible. Full sdk + full workflow test suite green under -race; vet clean. See decisions/0039 (forthcoming). --- plugin/external/sdk/iacserver.go | 112 ++++++- plugin/external/sdk/iacserver_modules_test.go | 279 ++++++++++++++++++ 2 files changed, 375 insertions(+), 16 deletions(-) diff --git a/plugin/external/sdk/iacserver.go b/plugin/external/sdk/iacserver.go index 4131c373..39d1a590 100644 --- a/plugin/external/sdk/iacserver.go +++ b/plugin/external/sdk/iacserver.go @@ -10,6 +10,7 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "google.golang.org/protobuf/types/known/anypb" "google.golang.org/protobuf/types/known/emptypb" pluginpkg "github.com/GoCodeAlone/workflow/plugin" @@ -87,14 +88,16 @@ func registerAllIaCProviderServicesWithOpts(s *grpc.Server, provider any, opts I diskManifest: opts.ManifestProvider, } // Wire the optional grpc_server.go delegate when the caller supplied - // module or step providers. Zero-value Modules/Steps ⇒ delegate stays - // nil ⇒ module/step RPCs continue returning Unimplemented (current - // behavior preserved for strict-cutover IaC plugins). Per - // decisions/0038. - if opts.Modules != nil || opts.Steps != nil { + // any (legacy or typed) module/step providers. Zero-value across all + // four maps ⇒ delegate stays nil ⇒ module/step RPCs continue returning + // Unimplemented (current behavior preserved for strict-cutover IaC + // plugins). Per decisions/0038 + decisions/0039. + if opts.Modules != nil || opts.Steps != nil || opts.TypedModules != nil || opts.TypedSteps != nil { bridge.delegate = newGRPCServer(&mapBackedProvider{ - modules: opts.Modules, - steps: opts.Steps, + modules: opts.Modules, + steps: opts.Steps, + typedModules: opts.TypedModules, + typedSteps: opts.TypedSteps, }) } pb.RegisterPluginServiceServer(s, bridge) @@ -322,6 +325,21 @@ type IaCServeOptions struct { // Modules; values are sdk.StepProvider — the same interface non-IaC // plugins consume via sdk.Serve. Steps map[string]StepProvider + + // TypedModules supplies plugin-native module providers that implement the + // strict-proto TypedModuleProvider surface (sdk.TypedModuleFactory or a + // custom implementor). When non-nil, mapBackedProvider implements + // TypedModuleProvider and grpc_server.go's CreateModule path dispatches + // CreateTypedModule on the looked-up entry — passing the host-supplied + // *anypb.Any TypedConfig directly to the typed factory's proto-message + // unpack. The legacy Modules map remains supported alongside (the + // dispatch is Typed-first, then legacy-fallback). See decisions/0039. + TypedModules map[string]TypedModuleProvider + + // TypedSteps supplies plugin-native step providers that implement + // TypedStepProvider. Same wiring rationale as TypedModules. See + // decisions/0039. + TypedSteps map[string]TypedStepProvider } // mapBackedProvider adapts user-supplied module/step provider maps to the @@ -342,8 +360,10 @@ type IaCServeOptions struct { // directly (walks the gRPC server's registered services) and never calls // back through the delegate. type mapBackedProvider struct { - modules map[string]ModuleProvider - steps map[string]StepProvider + modules map[string]ModuleProvider + steps map[string]StepProvider + typedModules map[string]TypedModuleProvider + typedSteps map[string]TypedStepProvider } // Manifest satisfies sdk.PluginProvider. Return value is unobserved (the @@ -352,11 +372,16 @@ type mapBackedProvider struct { // PluginProvider parameter type-checks at compile time. func (p *mapBackedProvider) Manifest() PluginManifest { return PluginManifest{} } -// ModuleTypes returns the keys of the modules map in deterministic +// ModuleTypes returns the keys of the legacy modules map in deterministic // (lexicographic) order. Sorting matters because Go map iteration is // randomized — without it, GetModuleTypes responses would differ run-to-run, -// breaking cache keys, golden files, and any caller that compares the list as -// an ordered sequence. +// breaking cache keys, golden files, and any caller that compares the list +// as an ordered sequence. +// +// The typed-module names are surfaced separately via TypedModuleTypes(). +// grpc_server.go's GetModuleTypes calls both methods and merges the lists +// when the provider implements TypedModuleProvider (Typed-primary-first, +// then legacy-only extras, with duplicates skipped). See decisions/0039. func (p *mapBackedProvider) ModuleTypes() []string { out := make([]string, 0, len(p.modules)) for name := range p.modules { @@ -366,7 +391,13 @@ func (p *mapBackedProvider) ModuleTypes() []string { return out } -// CreateModule looks up the named module provider and delegates to it. +// CreateModule looks up the named module provider in the legacy map and +// delegates to it. The typed map is checked separately via +// CreateTypedModule (the gRPC dispatcher tries the typed path first); a +// type registered ONLY in typedModules will surface here as "unknown" so the +// host can distinguish "not in legacy" from "not at all" — but in practice +// grpc_server.CreateModule short-circuits on the typed path and never reaches +// this code for a typed-only type. func (p *mapBackedProvider) CreateModule(typeName, name string, config map[string]any) (ModuleInstance, error) { mp, ok := p.modules[typeName] if !ok { @@ -375,8 +406,32 @@ func (p *mapBackedProvider) CreateModule(typeName, name string, config map[strin return mp.CreateModule(typeName, name, config) } -// StepTypes returns the keys of the steps map in deterministic -// (lexicographic) order — same rationale as ModuleTypes. +// TypedModuleTypes returns the keys of the typed-module map in deterministic +// (lexicographic) order. Required by TypedModuleProvider; consumed by +// grpc_server.go's GetModuleTypes when it merges typed + legacy names. +func (p *mapBackedProvider) TypedModuleTypes() []string { + out := make([]string, 0, len(p.typedModules)) + for name := range p.typedModules { + out = append(out, name) + } + sort.Strings(out) + return out +} + +// CreateTypedModule looks up the named typed-module provider and delegates +// the strict-proto factory call to it. Returns ErrTypedContractNotHandled +// when the type is not in the typed map — the gRPC dispatcher then falls +// back to the legacy CreateModule path. See decisions/0039. +func (p *mapBackedProvider) CreateTypedModule(typeName, name string, config *anypb.Any) (ModuleInstance, error) { + mp, ok := p.typedModules[typeName] + if !ok { + return nil, fmt.Errorf("%w: module type %q", ErrTypedContractNotHandled, typeName) + } + return mp.CreateTypedModule(typeName, name, config) +} + +// StepTypes returns the keys of the legacy steps map in deterministic +// (lexicographic) order. Same rationale + merge contract as ModuleTypes. func (p *mapBackedProvider) StepTypes() []string { out := make([]string, 0, len(p.steps)) for name := range p.steps { @@ -386,7 +441,8 @@ func (p *mapBackedProvider) StepTypes() []string { return out } -// CreateStep looks up the named step provider and delegates to it. +// CreateStep looks up the named step provider in the legacy map and +// delegates to it. Same Typed-first dispatch semantics as CreateModule. func (p *mapBackedProvider) CreateStep(typeName, name string, config map[string]any) (StepInstance, error) { sp, ok := p.steps[typeName] if !ok { @@ -395,6 +451,30 @@ func (p *mapBackedProvider) CreateStep(typeName, name string, config map[string] return sp.CreateStep(typeName, name, config) } +// TypedStepTypes returns the keys of the typed-step map in deterministic +// (lexicographic) order. Required by TypedStepProvider; consumed by +// grpc_server.go's GetStepTypes when it merges typed + legacy names. +func (p *mapBackedProvider) TypedStepTypes() []string { + out := make([]string, 0, len(p.typedSteps)) + for name := range p.typedSteps { + out = append(out, name) + } + sort.Strings(out) + return out +} + +// CreateTypedStep looks up the named typed-step provider and delegates the +// strict-proto factory call to it. Returns ErrTypedContractNotHandled when +// the type is not in the typed map — the gRPC dispatcher then falls back to +// the legacy CreateStep path. +func (p *mapBackedProvider) CreateTypedStep(typeName, name string, config *anypb.Any) (StepInstance, error) { + sp, ok := p.typedSteps[typeName] + if !ok { + return nil, fmt.Errorf("%w: step type %q", ErrTypedContractNotHandled, typeName) + } + return sp.CreateTypedStep(typeName, name, config) +} + // PluginInfo carries the metadata that go-plugin needs to serve an IaC // plugin. Currently only HandshakeConfig is meaningful; reserved as the // extension point for future Name/Version metadata fields without diff --git a/plugin/external/sdk/iacserver_modules_test.go b/plugin/external/sdk/iacserver_modules_test.go index 2c07f6c3..6e73fb2b 100644 --- a/plugin/external/sdk/iacserver_modules_test.go +++ b/plugin/external/sdk/iacserver_modules_test.go @@ -16,6 +16,7 @@ import ( "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/status" "google.golang.org/grpc/test/bufconn" + "google.golang.org/protobuf/types/known/anypb" "google.golang.org/protobuf/types/known/emptypb" ) @@ -204,6 +205,29 @@ func stringSliceEqual(a, b []string) bool { return true } +// stringSetEqual reports whether a and b contain the same elements +// regardless of order or duplicates. Used to assert union-merge results +// where the merge order is contract-irrelevant. +func stringSetEqual(a, b []string) bool { + set := func(xs []string) map[string]struct{} { + m := make(map[string]struct{}, len(xs)) + for _, x := range xs { + m[x] = struct{}{} + } + return m + } + sa, sb := set(a), set(b) + if len(sa) != len(sb) { + return false + } + for k := range sa { + if _, ok := sb[k]; !ok { + return false + } + } + return true +} + // TestIaCBridge_ZeroValueOptions_ModulesUnimplemented is the backwards-compat // invariant: zero-value IaCServeOptions {} keeps the bridge's pre-PR // behavior — module/step RPCs return Unimplemented (via @@ -263,3 +287,258 @@ func TestIaCBridge_NilBroker_NoMessagePublisherCall(t *testing.T) { t.Error("SetMessageSubscriber MUST NOT be called via the IaC bridge path") } } + +// ── Typed-module / Typed-step dispatch ─────────────────────────────────────── +// +// The TypedModules / TypedSteps surface added per decisions/0039 lets a +// plugin register strict-proto providers (sdk.NewTypedModuleFactory / +// sdk.NewTypedStepFactory) alongside or in place of the legacy ModuleProvider +// / StepProvider maps. grpc_server.CreateModule / CreateStep dispatch typed- +// first; mapBackedProvider's CreateTypedModule / CreateTypedStep delegate to +// the named entry in the typed map (returning ErrTypedContractNotHandled to +// fall through to the legacy path when not found). + +// TestIaCBridge_TypedModules_GetModuleTypes_Union locks the contract that +// GetModuleTypes returns the union of typed + legacy module-type keys (so +// the host's discovery surface sees every advertised type). +func TestIaCBridge_TypedModules_GetModuleTypes_Union(t *testing.T) { + opts := IaCServeOptions{ + Modules: map[string]ModuleProvider{ + "storage.legacy": &fakeModuleProvider{types: []string{"storage.legacy"}}, + }, + TypedModules: map[string]TypedModuleProvider{ + "storage.typed": NewTypedModuleFactory( + "storage.typed", + &emptypb.Empty{}, + func(_ string, _ *emptypb.Empty) (ModuleInstance, error) { + return &fakeModuleInstance{}, nil + }, + ), + }, + } + s := grpc.NewServer() + if err := registerAllIaCProviderServicesWithOpts(s, &fakeIaCRequiredProvider{}, opts); err != nil { + t.Fatalf("register: %v", err) + } + client := dialBridge(t, s) + resp, err := client.GetModuleTypes(context.Background(), &emptypb.Empty{}) + if err != nil { + t.Fatalf("GetModuleTypes: %v", err) + } + // grpc_server.GetModuleTypes merges TypedModuleTypes + ModuleTypes via + // mergeTypeLists (typed-primary-first, legacy-only-extras, no + // re-sort). The contract is set-equality, not ordered equality — the + // host treats GetModuleTypes as a set lookup. So assert as a set. + if got := resp.GetTypes(); !stringSetEqual(got, []string{"storage.legacy", "storage.typed"}) { + t.Errorf("GetModuleTypes set = %v, want set {storage.legacy, storage.typed}", got) + } +} + +// TestIaCBridge_TypedModules_CreateModule_DispatchesTypedFirst verifies that +// when a module type is in TypedModules, CreateModule hits the typed factory +// (using the host-supplied TypedConfig *anypb.Any), not the legacy path. +func TestIaCBridge_TypedModules_CreateModule_DispatchesTypedFirst(t *testing.T) { + var typedCalled bool + opts := IaCServeOptions{ + TypedModules: map[string]TypedModuleProvider{ + "storage.typed": NewTypedModuleFactory( + "storage.typed", + &emptypb.Empty{}, + func(_ string, _ *emptypb.Empty) (ModuleInstance, error) { + typedCalled = true + return &fakeModuleInstance{}, nil + }, + ), + }, + } + s := grpc.NewServer() + if err := registerAllIaCProviderServicesWithOpts(s, &fakeIaCRequiredProvider{}, opts); err != nil { + t.Fatalf("register: %v", err) + } + client := dialBridge(t, s) + + cfgAny, err := anypb.New(&emptypb.Empty{}) + if err != nil { + t.Fatalf("anypb.New: %v", err) + } + resp, err := client.CreateModule(context.Background(), &pb.CreateModuleRequest{ + Type: "storage.typed", + Name: "instance-1", + TypedConfig: cfgAny, + }) + if err != nil { + t.Fatalf("CreateModule: %v", err) + } + if resp.GetError() != "" { + t.Fatalf("CreateModule plugin-side error: %s", resp.GetError()) + } + if !typedCalled { + t.Error("typed factory was not called — dispatch did not route through TypedModules") + } +} + +// TestIaCBridge_TypedModules_LegacyFallback_WhenNotInTypedMap verifies the +// Typed-first → legacy-fallback contract: a type registered ONLY in Modules +// still works because mapBackedProvider.CreateTypedModule returns +// ErrTypedContractNotHandled, and grpc_server.CreateModule falls through to +// the legacy CreateModule path. +func TestIaCBridge_TypedModules_LegacyFallback_WhenNotInTypedMap(t *testing.T) { + var legacyCalled bool + legacyProvider := &fakeModuleProviderWithFlag{ + fakeModuleProvider: fakeModuleProvider{types: []string{"storage.legacy"}}, + called: &legacyCalled, + } + opts := IaCServeOptions{ + Modules: map[string]ModuleProvider{ + "storage.legacy": legacyProvider, + }, + TypedModules: map[string]TypedModuleProvider{ + "storage.typed": NewTypedModuleFactory( + "storage.typed", + &emptypb.Empty{}, + func(_ string, _ *emptypb.Empty) (ModuleInstance, error) { + return &fakeModuleInstance{}, nil + }, + ), + }, + } + s := grpc.NewServer() + if err := registerAllIaCProviderServicesWithOpts(s, &fakeIaCRequiredProvider{}, opts); err != nil { + t.Fatalf("register: %v", err) + } + client := dialBridge(t, s) + + resp, err := client.CreateModule(context.Background(), &pb.CreateModuleRequest{ + Type: "storage.legacy", + Name: "instance-legacy", + }) + if err != nil { + t.Fatalf("CreateModule: %v", err) + } + if resp.GetError() != "" { + t.Fatalf("CreateModule plugin-side error: %s", resp.GetError()) + } + if !legacyCalled { + t.Error("legacy CreateModule was not called — fallback did not engage for non-typed type") + } +} + +// TestIaCBridge_TypedSteps_GetStepTypes_Union mirrors the module union test +// for steps. +func TestIaCBridge_TypedSteps_GetStepTypes_Union(t *testing.T) { + opts := IaCServeOptions{ + Steps: map[string]StepProvider{ + "step.legacy": &fakeStepProvider{types: []string{"step.legacy"}}, + }, + TypedSteps: map[string]TypedStepProvider{ + "step.typed": NewTypedStepFactory( + "step.typed", + &emptypb.Empty{}, + &emptypb.Empty{}, + func(_ context.Context, _ TypedStepRequest[*emptypb.Empty, *emptypb.Empty]) (*TypedStepResult[*emptypb.Empty], error) { + return &TypedStepResult[*emptypb.Empty]{Output: &emptypb.Empty{}}, nil + }, + ), + }, + } + s := grpc.NewServer() + if err := registerAllIaCProviderServicesWithOpts(s, &fakeIaCRequiredProvider{}, opts); err != nil { + t.Fatalf("register: %v", err) + } + client := dialBridge(t, s) + resp, err := client.GetStepTypes(context.Background(), &emptypb.Empty{}) + if err != nil { + t.Fatalf("GetStepTypes: %v", err) + } + // Same set-equality rationale as GetModuleTypes_Union. + if got := resp.GetTypes(); !stringSetEqual(got, []string{"step.legacy", "step.typed"}) { + t.Errorf("GetStepTypes set = %v, want set {step.legacy, step.typed}", got) + } +} + +// TestIaCBridge_TypedSteps_CreateStep_DispatchesTypedFirst — same shape as +// the module-side test, for steps. +func TestIaCBridge_TypedSteps_CreateStep_DispatchesTypedFirst(t *testing.T) { + var typedCalled bool + opts := IaCServeOptions{ + TypedSteps: map[string]TypedStepProvider{ + "step.typed": NewTypedStepFactory( + "step.typed", + &emptypb.Empty{}, + &emptypb.Empty{}, + func(_ context.Context, _ TypedStepRequest[*emptypb.Empty, *emptypb.Empty]) (*TypedStepResult[*emptypb.Empty], error) { + typedCalled = true + return &TypedStepResult[*emptypb.Empty]{Output: &emptypb.Empty{}}, nil + }, + ), + }, + } + s := grpc.NewServer() + if err := registerAllIaCProviderServicesWithOpts(s, &fakeIaCRequiredProvider{}, opts); err != nil { + t.Fatalf("register: %v", err) + } + client := dialBridge(t, s) + + cfgAny, err := anypb.New(&emptypb.Empty{}) + if err != nil { + t.Fatalf("anypb.New: %v", err) + } + resp, err := client.CreateStep(context.Background(), &pb.CreateStepRequest{ + Type: "step.typed", + Name: "step-1", + TypedConfig: cfgAny, + }) + if err != nil { + t.Fatalf("CreateStep: %v", err) + } + if resp.GetError() != "" { + t.Fatalf("CreateStep plugin-side error: %s", resp.GetError()) + } + // CreateTypedStep is called during create; the factory's callback + // (which sets typedCalled) only fires on Execute, but the CreateStep + // success itself confirms TypedStepProvider.CreateTypedStep was used + // (the legacy path would have errored because no Steps entry exists). + _ = typedCalled +} + +// TestIaCBridge_TypedOnly_GetModuleTypes_NoLegacyMapPresent confirms that +// TypedModules alone (no Modules map at all) wires the bridge and surfaces +// typed module types — i.e. opting fully in to strict-proto providers does +// not require also setting Modules. +func TestIaCBridge_TypedOnly_GetModuleTypes_NoLegacyMapPresent(t *testing.T) { + opts := IaCServeOptions{ + TypedModules: map[string]TypedModuleProvider{ + "storage.typed-only": NewTypedModuleFactory( + "storage.typed-only", + &emptypb.Empty{}, + func(_ string, _ *emptypb.Empty) (ModuleInstance, error) { + return &fakeModuleInstance{}, nil + }, + ), + }, + } + s := grpc.NewServer() + if err := registerAllIaCProviderServicesWithOpts(s, &fakeIaCRequiredProvider{}, opts); err != nil { + t.Fatalf("register: %v", err) + } + client := dialBridge(t, s) + resp, err := client.GetModuleTypes(context.Background(), &emptypb.Empty{}) + if err != nil { + t.Fatalf("GetModuleTypes (typed-only): %v", err) + } + if got := resp.GetTypes(); len(got) != 1 || got[0] != "storage.typed-only" { + t.Errorf("GetModuleTypes (typed-only) = %v, want [storage.typed-only]", got) + } +} + +// fakeModuleProviderWithFlag tracks whether the legacy CreateModule path +// was hit, for the typed-first fallback test. +type fakeModuleProviderWithFlag struct { + fakeModuleProvider + called *bool +} + +func (f *fakeModuleProviderWithFlag) CreateModule(t, n string, c map[string]any) (ModuleInstance, error) { + *f.called = true + return f.fakeModuleProvider.CreateModule(t, n, c) +}