From 9756c29de7376b661aa06c69968bddc2d8b06b06 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Fri, 15 May 2026 02:03:06 -0400 Subject: [PATCH 1/3] feat(sdk): IaCServeOptions.Modules + .Steps via mapBackedProvider adapter (decisions/0038) --- plugin/external/sdk/iacserver.go | 174 ++++++++++++++- plugin/external/sdk/iacserver_modules_test.go | 204 ++++++++++++++++++ 2 files changed, 376 insertions(+), 2 deletions(-) create mode 100644 plugin/external/sdk/iacserver_modules_test.go diff --git a/plugin/external/sdk/iacserver.go b/plugin/external/sdk/iacserver.go index 64e20190..673b1ce9 100644 --- a/plugin/external/sdk/iacserver.go +++ b/plugin/external/sdk/iacserver.go @@ -81,10 +81,22 @@ func registerAllIaCProviderServicesWithOpts(s *grpc.Server, provider any, opts I // (e.g. a mixed plugin that called sdk.Serve AND RegisterAllIaC). // gRPC panics on double-registration; the guard prevents that. if _, alreadyRegistered := s.GetServiceInfo()[pb.PluginService_ServiceDesc.ServiceName]; !alreadyRegistered { - pb.RegisterPluginServiceServer(s, &iacPluginServiceBridge{ + bridge := &iacPluginServiceBridge{ grpcSrv: s, 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 { + bridge.delegate = newGRPCServer(&mapBackedProvider{ + modules: opts.Modules, + steps: opts.Steps, + }) + } + pb.RegisterPluginServiceServer(s, bridge) } return nil } @@ -167,6 +179,87 @@ type iacPluginServiceBridge struct { pb.UnimplementedPluginServiceServer grpcSrv *grpc.Server diskManifest *pluginpkg.PluginManifest + + // delegate, when non-nil, handles GetModuleTypes / CreateModule / + // InitModule / StartModule / StopModule / DestroyModule / GetStepTypes / + // CreateStep / ExecuteStep / DestroyStep by forwarding to grpc_server.go's + // existing implementation. Constructed by + // registerAllIaCProviderServicesWithOpts when IaCServeOptions.Modules or + // .Steps is non-nil. Zero-value ⇒ those RPCs continue returning + // Unimplemented via UnimplementedPluginServiceServer. See decisions/0038. + delegate *grpcServer +} + +// GetModuleTypes forwards to the delegate when wired, else falls back to the +// Unimplemented default. Same pattern for the 9 sibling forwarding methods. +func (b *iacPluginServiceBridge) GetModuleTypes(ctx context.Context, req *emptypb.Empty) (*pb.TypeList, error) { + if b.delegate == nil { + return b.UnimplementedPluginServiceServer.GetModuleTypes(ctx, req) + } + return b.delegate.GetModuleTypes(ctx, req) +} + +func (b *iacPluginServiceBridge) CreateModule(ctx context.Context, req *pb.CreateModuleRequest) (*pb.HandleResponse, error) { + if b.delegate == nil { + return b.UnimplementedPluginServiceServer.CreateModule(ctx, req) + } + return b.delegate.CreateModule(ctx, req) +} + +func (b *iacPluginServiceBridge) InitModule(ctx context.Context, req *pb.HandleRequest) (*pb.ErrorResponse, error) { + if b.delegate == nil { + return b.UnimplementedPluginServiceServer.InitModule(ctx, req) + } + return b.delegate.InitModule(ctx, req) +} + +func (b *iacPluginServiceBridge) StartModule(ctx context.Context, req *pb.HandleRequest) (*pb.ErrorResponse, error) { + if b.delegate == nil { + return b.UnimplementedPluginServiceServer.StartModule(ctx, req) + } + return b.delegate.StartModule(ctx, req) +} + +func (b *iacPluginServiceBridge) StopModule(ctx context.Context, req *pb.HandleRequest) (*pb.ErrorResponse, error) { + if b.delegate == nil { + return b.UnimplementedPluginServiceServer.StopModule(ctx, req) + } + return b.delegate.StopModule(ctx, req) +} + +func (b *iacPluginServiceBridge) DestroyModule(ctx context.Context, req *pb.HandleRequest) (*pb.ErrorResponse, error) { + if b.delegate == nil { + return b.UnimplementedPluginServiceServer.DestroyModule(ctx, req) + } + return b.delegate.DestroyModule(ctx, req) +} + +func (b *iacPluginServiceBridge) GetStepTypes(ctx context.Context, req *emptypb.Empty) (*pb.TypeList, error) { + if b.delegate == nil { + return b.UnimplementedPluginServiceServer.GetStepTypes(ctx, req) + } + return b.delegate.GetStepTypes(ctx, req) +} + +func (b *iacPluginServiceBridge) CreateStep(ctx context.Context, req *pb.CreateStepRequest) (*pb.HandleResponse, error) { + if b.delegate == nil { + return b.UnimplementedPluginServiceServer.CreateStep(ctx, req) + } + return b.delegate.CreateStep(ctx, req) +} + +func (b *iacPluginServiceBridge) ExecuteStep(ctx context.Context, req *pb.ExecuteStepRequest) (*pb.ExecuteStepResponse, error) { + if b.delegate == nil { + return b.UnimplementedPluginServiceServer.ExecuteStep(ctx, req) + } + return b.delegate.ExecuteStep(ctx, req) +} + +func (b *iacPluginServiceBridge) DestroyStep(ctx context.Context, req *pb.HandleRequest) (*pb.ErrorResponse, error) { + if b.delegate == nil { + return b.UnimplementedPluginServiceServer.DestroyStep(ctx, req) + } + return b.delegate.DestroyStep(ctx, req) } // GetContractRegistry returns the set of gRPC services registered on @@ -215,6 +308,83 @@ type IaCServeOptions struct { // engine falls back to its manager.go-loaded plugin.json (workflow plan // Task 1). ManifestProvider *pluginpkg.PluginManifest + + // Modules supplies plugin-native module providers. When non-nil, the + // bridge wires GetModuleTypes / CreateModule / InitModule / StartModule / + // StopModule / DestroyModule to delegate to grpc_server.go's existing + // PluginService implementation via a thin mapBackedProvider adapter. + // Zero-value = current behavior (Unimplemented for those RPCs). + // See decisions/0038. + Modules map[string]ModuleProvider + + // Steps supplies plugin-native step providers. Same wiring rationale as + // Modules; values are sdk.StepProvider — the same interface non-IaC + // plugins consume via sdk.Serve. + Steps map[string]StepProvider +} + +// mapBackedProvider adapts user-supplied module/step provider maps to the +// sdk.PluginProvider + sdk.ModuleProvider + sdk.StepProvider interfaces that +// grpc_server.go's existing PluginService implementation expects. Per +// decisions/0038, this is the smallest viable extraction path that lets the +// IaC bridge reuse newGRPCServer's handle-state + lifecycle code without +// refactoring grpc_server.go. +// +// The adapter is intentionally thin: ModuleTypes/StepTypes return map keys; +// CreateModule/CreateStep look up the named provider in the map and delegate. +// Manifest returns a zero-valued PluginManifest — the iacPluginServiceBridge +// implements GetManifest directly (using IaCServeOptions.ManifestProvider) +// and never calls back through this adapter, so Manifest's return value is +// never observed; it exists solely to satisfy the PluginProvider interface +// contract that newGRPCServer requires. ContractRegistry is intentionally NOT +// implemented — the iacPluginServiceBridge implements GetContractRegistry +// 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 +} + +// Manifest satisfies sdk.PluginProvider. Return value is unobserved (the +// bridge handles GetManifest directly via IaCServeOptions.ManifestProvider) +// — the method exists only to satisfy the interface so newGRPCServer's +// PluginProvider parameter type-checks at compile time. +func (p *mapBackedProvider) Manifest() PluginManifest { return PluginManifest{} } + +// ModuleTypes returns the keys of the modules map. +func (p *mapBackedProvider) ModuleTypes() []string { + out := make([]string, 0, len(p.modules)) + for name := range p.modules { + out = append(out, name) + } + return out +} + +// CreateModule looks up the named module provider and delegates to it. +func (p *mapBackedProvider) CreateModule(typeName, name string, config map[string]any) (ModuleInstance, error) { + mp, ok := p.modules[typeName] + if !ok { + return nil, fmt.Errorf("mapBackedProvider: unknown module type %q", typeName) + } + return mp.CreateModule(typeName, name, config) +} + +// StepTypes returns the keys of the steps map. +func (p *mapBackedProvider) StepTypes() []string { + out := make([]string, 0, len(p.steps)) + for name := range p.steps { + out = append(out, name) + } + return out +} + +// CreateStep looks up the named step provider and delegates to it. +func (p *mapBackedProvider) CreateStep(typeName, name string, config map[string]any) (StepInstance, error) { + sp, ok := p.steps[typeName] + if !ok { + return nil, fmt.Errorf("mapBackedProvider: unknown step type %q", typeName) + } + return sp.CreateStep(typeName, name, config) } // PluginInfo carries the metadata that go-plugin needs to serve an IaC diff --git a/plugin/external/sdk/iacserver_modules_test.go b/plugin/external/sdk/iacserver_modules_test.go new file mode 100644 index 00000000..054e4747 --- /dev/null +++ b/plugin/external/sdk/iacserver_modules_test.go @@ -0,0 +1,204 @@ +package sdk + +// Internal test (package sdk) — exercises mapBackedProvider + the bridge's +// optional delegate field that wires GetModuleTypes / CreateModule / etc. +// through to grpc_server.go's existing PluginService implementation when +// IaCServeOptions.Modules or .Steps is non-nil. Per decisions/0038. + +import ( + "context" + "net" + "testing" + + pb "github.com/GoCodeAlone/workflow/plugin/external/proto" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/status" + "google.golang.org/grpc/test/bufconn" + "google.golang.org/protobuf/types/known/emptypb" +) + +// ── fakes ─────────────────────────────────────────────────────────────────── + +// fakeIaCRequiredProvider satisfies pb.IaCProviderRequiredServer (the +// REQUIRED service registerAllIaCProviderServicesWithOpts asserts) so the +// bridge wiring under test exercises the same code path production plugins do. +type fakeIaCRequiredProvider struct { + pb.UnimplementedIaCProviderRequiredServer +} + +// fakeModuleProvider implements sdk.ModuleProvider. +type fakeModuleProvider struct { + types []string + instance ModuleInstance // returned from CreateModule when non-nil +} + +func (f *fakeModuleProvider) ModuleTypes() []string { return f.types } +func (f *fakeModuleProvider) CreateModule(_, _ string, _ map[string]any) (ModuleInstance, error) { + if f.instance != nil { + return f.instance, nil + } + return &fakeModuleInstance{}, nil +} + +// fakeStepProvider implements sdk.StepProvider. +type fakeStepProvider struct { + types []string +} + +func (f *fakeStepProvider) StepTypes() []string { return f.types } +func (f *fakeStepProvider) CreateStep(_, _ string, _ map[string]any) (StepInstance, error) { + return &fakeStepInstance{}, nil +} + +// fakeModuleInstance is a no-op ModuleInstance. +type fakeModuleInstance struct{} + +func (*fakeModuleInstance) Init() error { return nil } +func (*fakeModuleInstance) Start(context.Context) error { return nil } +func (*fakeModuleInstance) Stop(context.Context) error { return nil } + +// fakeStepInstance is a no-op StepInstance. +type fakeStepInstance struct{} + +func (*fakeStepInstance) Execute(context.Context, map[string]any, map[string]map[string]any, map[string]any, map[string]any, map[string]any) (*StepResult, error) { + return &StepResult{}, nil +} + +// fakeMessageAwareModule records whether SetMessagePublisher / +// SetMessageSubscriber were called. Regression guard for the +// no-broker-plumbed Non-Goal in the bridge path. +type fakeMessageAwareModule struct { + fakeModuleInstance + SetMessagePublisherCalled bool + SetMessageSubscriberCalled bool +} + +func (f *fakeMessageAwareModule) SetMessagePublisher(MessagePublisher) { + f.SetMessagePublisherCalled = true +} +func (f *fakeMessageAwareModule) SetMessageSubscriber(MessageSubscriber) { + f.SetMessageSubscriberCalled = true +} + +// ── bufconn dial helper ───────────────────────────────────────────────────── + +// dialBridge serves the gRPC server on a bufconn listener and returns a +// PluginServiceClient connected to it. Caller defers conn.Close() / s.Stop(). +func dialBridge(t *testing.T, s *grpc.Server) pb.PluginServiceClient { + t.Helper() + lis := bufconn.Listen(1 << 20) + go func() { _ = s.Serve(lis) }() + t.Cleanup(s.Stop) + conn, err := grpc.NewClient("passthrough://bufnet", + grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { + return lis.DialContext(ctx) + }), + grpc.WithTransportCredentials(insecure.NewCredentials()), + ) + if err != nil { + t.Fatalf("dial bufconn: %v", err) + } + t.Cleanup(func() { _ = conn.Close() }) + return pb.NewPluginServiceClient(conn) +} + +// ── tests ─────────────────────────────────────────────────────────────────── + +// TestIaCBridge_ModulesAndSteps_Delegate locks the wire-up of the optional +// delegate: when IaCServeOptions.Modules / .Steps are non-nil, +// GetModuleTypes / GetStepTypes return the keys of those maps (proving the +// bridge forwards to a grpc_server.go-backed mapBackedProvider). +func TestIaCBridge_ModulesAndSteps_Delegate(t *testing.T) { + opts := IaCServeOptions{ + Modules: map[string]ModuleProvider{ + "storage.test": &fakeModuleProvider{types: []string{"storage.test"}}, + }, + Steps: map[string]StepProvider{ + "step.test": &fakeStepProvider{types: []string{"step.test"}}, + }, + } + s := grpc.NewServer() + if err := registerAllIaCProviderServicesWithOpts(s, &fakeIaCRequiredProvider{}, opts); err != nil { + t.Fatalf("register: %v", err) + } + client := dialBridge(t, s) + ctx := context.Background() + + modTypes, err := client.GetModuleTypes(ctx, &emptypb.Empty{}) + if err != nil { + t.Fatalf("GetModuleTypes: %v", err) + } + if len(modTypes.GetTypes()) != 1 || modTypes.GetTypes()[0] != "storage.test" { + t.Errorf("GetModuleTypes = %v, want [storage.test]", modTypes.GetTypes()) + } + + stepTypes, err := client.GetStepTypes(ctx, &emptypb.Empty{}) + if err != nil { + t.Fatalf("GetStepTypes: %v", err) + } + if len(stepTypes.GetTypes()) != 1 || stepTypes.GetTypes()[0] != "step.test" { + t.Errorf("GetStepTypes = %v, want [step.test]", stepTypes.GetTypes()) + } +} + +// TestIaCBridge_ZeroValueOptions_ModulesUnimplemented is the backwards-compat +// invariant: zero-value IaCServeOptions {} keeps the bridge's pre-PR +// behavior — module/step RPCs return Unimplemented (via +// UnimplementedPluginServiceServer). Existing IaC-only plugins MUST be +// unaffected. +func TestIaCBridge_ZeroValueOptions_ModulesUnimplemented(t *testing.T) { + s := grpc.NewServer() + if err := registerAllIaCProviderServicesWithOpts(s, &fakeIaCRequiredProvider{}, IaCServeOptions{}); err != nil { + t.Fatalf("register: %v", err) + } + client := dialBridge(t, s) + _, err := client.GetModuleTypes(context.Background(), &emptypb.Empty{}) + if err == nil { + t.Fatal("GetModuleTypes must return Unimplemented on zero-value options") + } + if got := status.Code(err); got != codes.Unimplemented { + t.Errorf("GetModuleTypes code = %v, want Unimplemented", got) + } +} + +// TestIaCBridge_NilBroker_NoMessagePublisherCall is the regression guard for +// the Non-Goal in decisions/0038: no broker is plumbed through iacGRPCPlugin, +// so a MessageAwareModule registered via the bridge MUST never have +// SetMessagePublisher / SetMessageSubscriber called. If a future change wires +// the broker up, this test fails loudly so the implementer remembers to also +// add a positive pub/sub test (otherwise the path silently regresses to +// "broker plumbed but Publish/Subscribe still nil-deref"). +func TestIaCBridge_NilBroker_NoMessagePublisherCall(t *testing.T) { + mam := &fakeMessageAwareModule{} + opts := IaCServeOptions{ + Modules: map[string]ModuleProvider{ + "storage.test": &fakeModuleProvider{ + types: []string{"storage.test"}, + instance: mam, + }, + }, + } + 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.test", + Name: "test-instance", + }) + if err != nil { + t.Fatalf("CreateModule: %v", err) + } + if resp.GetError() != "" { + t.Fatalf("CreateModule plugin-side error: %s", resp.GetError()) + } + if mam.SetMessagePublisherCalled { + t.Error("SetMessagePublisher MUST NOT be called via the IaC bridge path (no broker plumbed)") + } + if mam.SetMessageSubscriberCalled { + t.Error("SetMessageSubscriber MUST NOT be called via the IaC bridge path") + } +} From 42e4c84308145e00cdfbff78c2dc1115bcfbc00a Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Fri, 15 May 2026 02:15:55 -0400 Subject: [PATCH 2/3] =?UTF-8?q?test(sdk):=20end-to-end=20IaC=20bridge=20?= =?UTF-8?q?=E2=86=94=20pb.PluginServiceClient=20integration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- plugin/external/sdk/iac_modules_e2e_test.go | 71 +++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 plugin/external/sdk/iac_modules_e2e_test.go diff --git a/plugin/external/sdk/iac_modules_e2e_test.go b/plugin/external/sdk/iac_modules_e2e_test.go new file mode 100644 index 00000000..4db1498b --- /dev/null +++ b/plugin/external/sdk/iac_modules_e2e_test.go @@ -0,0 +1,71 @@ +package sdk + +// End-to-end integration test (internal package sdk) — exercises the IaC +// bridge through a real pb.PluginServiceClient over bufconn, the canonical +// runtime-launch-validation evidence for the workflow-side plumbing path per +// decisions/0038. The engine's ExternalPluginAdapter.ModuleFactories() calls +// GetModuleTypes + CreateModule on this exact client interface — bufconn uses +// the same gRPC dispatch the production engine does. Subprocess-handshake +// coverage lives in plan-2 Tasks 7+11 (plugin repos) against real binaries. + +import ( + "context" + "testing" + + pb "github.com/GoCodeAlone/workflow/plugin/external/proto" + "google.golang.org/grpc" + "google.golang.org/protobuf/types/known/emptypb" + "google.golang.org/protobuf/types/known/structpb" +) + +// TestEndToEnd_IaCBridge_EngineAdapterSeesModules spins up an IaC bridge with +// IaCServeOptions.Modules wired, dials it via the standard +// pb.PluginServiceClient over bufconn, and exercises the GetModuleTypes + +// CreateModule pair the engine adapter calls — proving the engine sees the +// modules without any engine-side change. Locks the design's "engine-side: +// zero change" claim. +func TestEndToEnd_IaCBridge_EngineAdapterSeesModules(t *testing.T) { + fakeMod := &fakeModuleProvider{ + types: []string{"storage.test"}, + instance: &fakeModuleInstance{}, + } + opts := IaCServeOptions{ + Modules: map[string]ModuleProvider{"storage.test": fakeMod}, + } + s := grpc.NewServer() + if err := registerAllIaCProviderServicesWithOpts(s, &fakeIaCRequiredProvider{}, opts); err != nil { + t.Fatalf("register: %v", err) + } + client := dialBridge(t, s) + ctx := context.Background() + + // GetModuleTypes — the adapter's first call when populating ModuleFactories. + types, err := client.GetModuleTypes(ctx, &emptypb.Empty{}) + if err != nil { + t.Fatalf("GetModuleTypes: %v", err) + } + if len(types.GetTypes()) != 1 || types.GetTypes()[0] != "storage.test" { + t.Fatalf("GetModuleTypes = %v, want [storage.test]", types.GetTypes()) + } + + // CreateModule — the adapter's second call (per-instance) — exercises the + // full module-creation handshake the engine's RemoteModule path triggers. + cfg, err := structpb.NewStruct(map[string]any{"k": "v"}) + if err != nil { + t.Fatalf("structpb.NewStruct: %v", err) + } + resp, err := client.CreateModule(ctx, &pb.CreateModuleRequest{ + Type: "storage.test", + Name: "n", + Config: cfg, + }) + if err != nil { + t.Fatalf("CreateModule: %v", err) + } + if resp.GetError() != "" { + t.Fatalf("CreateModule plugin-side error: %s", resp.GetError()) + } + if resp.GetHandleId() == "" { + t.Fatal("CreateModule must return a non-empty HandleId — the engine's RemoteModule keys all subsequent lifecycle RPCs by it") + } +} From 0bca64698572dfa4fd5307ac2ce842ddc4e8df67 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Fri, 15 May 2026 02:56:07 -0400 Subject: [PATCH 3/3] fix(sdk): deterministic ModuleTypes/StepTypes + correct bufconn URI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses 2 Copilot findings on PR #683 (plan-2 Tasks 1+2): 1. mapBackedProvider.ModuleTypes / StepTypes — Go map iteration is randomized, so the previous unsorted slice would differ run-to-run, breaking cache keys + any caller that compares as an ordered list. Sort the keys lexicographically before returning. 2. Bufconn dial target — the iacserver_modules_test.go dial used `passthrough://bufnet` (double-slash, wrong); the rest of the sdk bufconn tests use `passthrough:///bufnet` (triple-slash, gRPC URI form). Aligned with the in-tree convention. Added TestIaCBridge_ModuleStepTypes_Deterministic: 3 entries inserted non-alphabetically, expects alphabetical back, asserts across 5 iterations to catch a non-sorted impl that happens to win on a single race. Co-Authored-By: Claude Opus 4.7 --- plugin/external/sdk/iacserver.go | 12 +++- plugin/external/sdk/iacserver_modules_test.go | 63 ++++++++++++++++++- 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/plugin/external/sdk/iacserver.go b/plugin/external/sdk/iacserver.go index 673b1ce9..4131c373 100644 --- a/plugin/external/sdk/iacserver.go +++ b/plugin/external/sdk/iacserver.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "reflect" + "sort" goplugin "github.com/GoCodeAlone/go-plugin" "google.golang.org/grpc" @@ -351,12 +352,17 @@ 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. +// ModuleTypes returns the keys of the 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. func (p *mapBackedProvider) ModuleTypes() []string { out := make([]string, 0, len(p.modules)) for name := range p.modules { out = append(out, name) } + sort.Strings(out) return out } @@ -369,12 +375,14 @@ func (p *mapBackedProvider) CreateModule(typeName, name string, config map[strin return mp.CreateModule(typeName, name, config) } -// StepTypes returns the keys of the steps map. +// StepTypes returns the keys of the steps map in deterministic +// (lexicographic) order — same rationale as ModuleTypes. func (p *mapBackedProvider) StepTypes() []string { out := make([]string, 0, len(p.steps)) for name := range p.steps { out = append(out, name) } + sort.Strings(out) return out } diff --git a/plugin/external/sdk/iacserver_modules_test.go b/plugin/external/sdk/iacserver_modules_test.go index 054e4747..2c07f6c3 100644 --- a/plugin/external/sdk/iacserver_modules_test.go +++ b/plugin/external/sdk/iacserver_modules_test.go @@ -91,7 +91,7 @@ func dialBridge(t *testing.T, s *grpc.Server) pb.PluginServiceClient { lis := bufconn.Listen(1 << 20) go func() { _ = s.Serve(lis) }() t.Cleanup(s.Stop) - conn, err := grpc.NewClient("passthrough://bufnet", + conn, err := grpc.NewClient("passthrough:///bufnet", grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { return lis.DialContext(ctx) }), @@ -143,6 +143,67 @@ func TestIaCBridge_ModulesAndSteps_Delegate(t *testing.T) { } } +// TestIaCBridge_ModuleStepTypes_Deterministic locks the lexicographic-order +// contract: Go map iteration is randomized, so without sorting the +// GetModuleTypes / GetStepTypes responses would differ run-to-run, breaking +// cache keys + any caller that compares the list as an ordered sequence. +// Three entries inserted in a non-alphabetical order; expect alphabetical back. +func TestIaCBridge_ModuleStepTypes_Deterministic(t *testing.T) { + opts := IaCServeOptions{ + Modules: map[string]ModuleProvider{ + "storage.zeta": &fakeModuleProvider{types: []string{"storage.zeta"}}, + "storage.alpha": &fakeModuleProvider{types: []string{"storage.alpha"}}, + "storage.beta": &fakeModuleProvider{types: []string{"storage.beta"}}, + }, + Steps: map[string]StepProvider{ + "step.zeta": &fakeStepProvider{types: []string{"step.zeta"}}, + "step.alpha": &fakeStepProvider{types: []string{"step.alpha"}}, + "step.beta": &fakeStepProvider{types: []string{"step.beta"}}, + }, + } + s := grpc.NewServer() + if err := registerAllIaCProviderServicesWithOpts(s, &fakeIaCRequiredProvider{}, opts); err != nil { + t.Fatalf("register: %v", err) + } + client := dialBridge(t, s) + ctx := context.Background() + + wantMods := []string{"storage.alpha", "storage.beta", "storage.zeta"} + wantSteps := []string{"step.alpha", "step.beta", "step.zeta"} + + // Multiple iterations guard against an unsorted impl happening to win the + // race on the first call; a non-sorted slice WILL eventually differ across + // runs given Go's randomized map iteration. + for i := 0; i < 5; i++ { + modTypes, err := client.GetModuleTypes(ctx, &emptypb.Empty{}) + if err != nil { + t.Fatalf("GetModuleTypes iter %d: %v", i, err) + } + if got := modTypes.GetTypes(); !stringSliceEqual(got, wantMods) { + t.Fatalf("GetModuleTypes iter %d = %v, want %v (must be lexicographic)", i, got, wantMods) + } + stepTypes, err := client.GetStepTypes(ctx, &emptypb.Empty{}) + if err != nil { + t.Fatalf("GetStepTypes iter %d: %v", i, err) + } + if got := stepTypes.GetTypes(); !stringSliceEqual(got, wantSteps) { + t.Fatalf("GetStepTypes iter %d = %v, want %v (must be lexicographic)", i, got, wantSteps) + } + } +} + +func stringSliceEqual(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + 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