diff --git a/.github/workflows/cross-plugin-build-test.yml b/.github/workflows/cross-plugin-build-test.yml index c82bf216..e8404c88 100644 --- a/.github/workflows/cross-plugin-build-test.yml +++ b/.github/workflows/cross-plugin-build-test.yml @@ -10,6 +10,15 @@ on: - 'iac/**' - 'platform/**' - 'plugin/sdk/**' + # Strict-contracts cutover Task 6 — typed IaC contract + adapters live + # under plugin/external/{proto,sdk}; downstream IaC dispatch + remote- + # plugin orchestration code lives elsewhere under plugin/external/. + # Both surfaces affect the iac-typed-e2e job (cross-plugin gRPC wire + # behavior + handshake/loader changes); use the broad gate on + # plugin/external/** so any structural change to the external-plugin + # boundary triggers the cross-plugin smoke. CI cost is bounded — + # changes outside this dir already skip via the rest of the filter. + - 'plugin/external/**' - 'go.mod' - 'go.sum' - '.github/workflows/cross-plugin-build-test.yml' @@ -61,3 +70,31 @@ jobs: go mod tidy go build ./... # NOTE: not `go test` — those plugins have their own CI; we just verify compile-compat + + # Strict-contracts force-cutover Task 6: typed-IaC E2E integration test. + # + # The cross-plugin-build job above only verifies `go build` against AWS/GCP/ + # Azure plugins. For the typed IaC contract introduced by the strict- + # contracts cutover (PR 2 of the plan), `go build` is necessary but not + # sufficient — wire incompat between workflow and plugin grpc-go versions + # would slip through (per cycle 1 I-2 + cycle 2 I-1-NEW). + # + # This job runs the workflow-side in-process E2E test built with + # -tags=integration. The same test exercises the typed gRPC services + # registered via sdk.RegisterAllIaCProviderServices through a real bufconn + # gRPC channel, asserting the typed roundtrip preserves shape (including + # the map sensitive field that the legacy structpb path + # silently dropped). + # + # The subprocess wire-test variant against the real DO plugin v1.0.0 binary + # is added once that plugin ships — see plan §PR 3 (Task 7+). + iac-typed-e2e: + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: { go-version-file: go.mod } + - name: Typed-IaC E2E test (in-process gRPC roundtrip) + run: GOWORK=off go test -tags=integration ./plugin/external/sdk/... -run TestIaC_EndToEnd -count=1 -v diff --git a/plugin/external/sdk/contracts.go b/plugin/external/sdk/contracts.go new file mode 100644 index 00000000..b9f28989 --- /dev/null +++ b/plugin/external/sdk/contracts.go @@ -0,0 +1,60 @@ +package sdk + +import ( + "sort" + + "google.golang.org/grpc" + + pb "github.com/GoCodeAlone/workflow/plugin/external/proto" +) + +// BuildContractRegistry enumerates the gRPC services registered on +// grpcSrv and returns a *pb.ContractRegistry with a SERVICE-kind +// ContractDescriptor for each one. Mode is set to +// CONTRACT_MODE_STRICT_PROTO so the host can distinguish typed IaC +// services from the legacy structpb-mode contracts produced by +// Module/Step/Trigger ContractProvider implementations. +// +// Why this exists (per cycle 3 I-1 of the strict-contracts force-cutover +// design): wfctl needs a single mechanism to discover "is the optional +// service registered on this plugin handle?". Reusing the existing +// ContractRegistry shape keeps Module/Step/Trigger and IaC capability +// discovery on the same wire surface — no new server-reflection +// dependency required. +// +// The helper is safe to call with a nil server; it returns an empty +// (but non-nil) ContractRegistry. Service descriptors are emitted in a +// deterministic alphabetical order so callers can rely on stable +// FileDescriptorSet-adjacent output for diff/compare operations and +// the wftest BDD test in Task 15. +// +// IaC plugin authors typically wire this into their ContractProvider +// implementation: +// +// func (p *plugin) ContractRegistry() *pb.ContractRegistry { +// return sdk.BuildContractRegistry(p.grpcServer) +// } +// +// where p.grpcServer was captured inside the iacGRPCPlugin.GRPCServer +// callback at startup. The ContractProvider hook keeps the wfctl-side +// GetContractRegistry RPC path unchanged. +func BuildContractRegistry(grpcSrv *grpc.Server) *pb.ContractRegistry { + registry := &pb.ContractRegistry{} + if grpcSrv == nil { + return registry + } + info := grpcSrv.GetServiceInfo() + names := make([]string, 0, len(info)) + for name := range info { + names = append(names, name) + } + sort.Strings(names) + for _, name := range names { + registry.Contracts = append(registry.Contracts, &pb.ContractDescriptor{ + Kind: pb.ContractKind_CONTRACT_KIND_SERVICE, + ServiceName: name, + Mode: pb.ContractMode_CONTRACT_MODE_STRICT_PROTO, + }) + } + return registry +} diff --git a/plugin/external/sdk/contracts_iac_test.go b/plugin/external/sdk/contracts_iac_test.go new file mode 100644 index 00000000..b67aebaf --- /dev/null +++ b/plugin/external/sdk/contracts_iac_test.go @@ -0,0 +1,95 @@ +package sdk_test + +import ( + "testing" + + "google.golang.org/grpc" + + pb "github.com/GoCodeAlone/workflow/plugin/external/proto" + "github.com/GoCodeAlone/workflow/plugin/external/sdk" +) + +// TestBuildContractRegistry_AdvertisesRegisteredIaCServices asserts that +// after calling RegisterAllIaCProviderServices, BuildContractRegistry +// returns a *pb.ContractRegistry that lists the registered IaC services +// as SERVICE-kind ContractDescriptors. wfctl uses this for capability +// discovery against IaC plugins (per design §Optional services — single +// mechanism, no new server-reflection dependency). +func TestBuildContractRegistry_AdvertisesRegisteredIaCServices(t *testing.T) { + grpcSrv := grpc.NewServer() + provider := &iacContractProviderStub{} + if err := sdk.RegisterAllIaCProviderServices(grpcSrv, provider); err != nil { + t.Fatalf("register: %v", err) + } + + registry := sdk.BuildContractRegistry(grpcSrv) + if registry == nil { + t.Fatalf("expected non-nil ContractRegistry") + } + + services := serviceNamesFromRegistry(registry) + want := []string{ + "workflow.plugin.external.iac.IaCProviderRequired", + "workflow.plugin.external.iac.IaCProviderEnumerator", + "workflow.plugin.external.iac.IaCProviderDriftDetector", + } + for _, name := range want { + if !services[name] { + t.Errorf("ContractRegistry missing service %q; have: %v", name, services) + } + } +} + +// TestBuildContractRegistry_ServiceContractsUseStrictProtoMode asserts +// that auto-emitted IaC service descriptors carry Mode=STRICT_PROTO so +// the host can distinguish them from the legacy structpb-mode contracts +// produced by Module/Step/Trigger ContractProvider implementations. +func TestBuildContractRegistry_ServiceContractsUseStrictProtoMode(t *testing.T) { + grpcSrv := grpc.NewServer() + if err := sdk.RegisterAllIaCProviderServices(grpcSrv, &iacContractProviderStub{}); err != nil { + t.Fatalf("register: %v", err) + } + registry := sdk.BuildContractRegistry(grpcSrv) + + for _, c := range registry.Contracts { + if c.Kind != pb.ContractKind_CONTRACT_KIND_SERVICE { + t.Errorf("unexpected non-service contract kind %v for %q", c.Kind, c.ServiceName) + continue + } + if c.Mode != pb.ContractMode_CONTRACT_MODE_STRICT_PROTO { + t.Errorf("service %q should be STRICT_PROTO mode; got %v", c.ServiceName, c.Mode) + } + } +} + +// TestBuildContractRegistry_NilServer_ReturnsEmpty asserts the helper is +// safe to call with a nil server (returns an empty registry rather than +// panicking) — defensive contract for callers that may construct the +// helper before the gRPC server exists. +func TestBuildContractRegistry_NilServer_ReturnsEmpty(t *testing.T) { + registry := sdk.BuildContractRegistry(nil) + if registry == nil { + t.Fatalf("expected non-nil empty ContractRegistry") + } + if len(registry.Contracts) != 0 { + t.Fatalf("expected empty contracts; got %d", len(registry.Contracts)) + } +} + +func serviceNamesFromRegistry(r *pb.ContractRegistry) map[string]bool { + out := make(map[string]bool, len(r.Contracts)) + for _, c := range r.Contracts { + if c.Kind == pb.ContractKind_CONTRACT_KIND_SERVICE { + out[c.ServiceName] = true + } + } + return out +} + +// iacContractProviderStub satisfies Required + Enumerator + DriftDetector +// to exercise the multi-service registration path. +type iacContractProviderStub struct { + pb.UnimplementedIaCProviderRequiredServer + pb.UnimplementedIaCProviderEnumeratorServer + pb.UnimplementedIaCProviderDriftDetectorServer +} diff --git a/plugin/external/sdk/iac_e2e_test.go b/plugin/external/sdk/iac_e2e_test.go new file mode 100644 index 00000000..eea0ea00 --- /dev/null +++ b/plugin/external/sdk/iac_e2e_test.go @@ -0,0 +1,228 @@ +//go:build integration +// +build integration + +// Package sdk integration tests — only built with `-tags=integration`. +// These exercise the typed IaC contract end-to-end through a real +// in-process gRPC channel (bufconn), catching the bug class the +// strict-contracts cutover closes: +// +// - missing client method bridge — the typed pb.IaCProvider*Client +// would fail to compile if iac.proto dropped a method +// - missing server dispatcher — the auto-registration helper would +// reject a stub that doesn't satisfy IaCProviderRequiredServer +// - structpb conversion drops — typed messages mean no +// map[string]any roundtrip surface +// +// Subprocess wire-test variant runs in +// .github/workflows/cross-plugin-build-test.yml against the real DO +// plugin binary once it ships at v1.0.0; the in-process variant runs +// in workflow's own CI on every IaC-touching PR. +package sdk_test + +import ( + "context" + "net" + "testing" + "time" + + "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" + + pb "github.com/GoCodeAlone/workflow/plugin/external/proto" + "github.com/GoCodeAlone/workflow/plugin/external/sdk" +) + +const ( + e2eBufSize = 1024 * 1024 + e2eRPCDeadline = 5 * time.Second +) + +// TestIaC_EndToEnd_RequiredAndOptional_TypedDispatch starts an +// in-process gRPC server with the typed IaCProviderRequired + +// Enumerator services registered via sdk.RegisterAllIaCProviderServices, +// dials it through bufconn, invokes typed RPCs on both services, and +// asserts the responses come back with the expected shape. +// +// This is the canonical workflow-side smoke test for the typed IaC +// contract. The cross-plugin-build matrix runs an analogous test +// against the real DO plugin binary; the in-process flavor here +// exercises the same code paths through a real gRPC channel without +// requiring the DO plugin to be cross-built first. +func TestIaC_EndToEnd_RequiredAndOptional_TypedDispatch(t *testing.T) { + listener := bufconn.Listen(e2eBufSize) + t.Cleanup(func() { _ = listener.Close() }) + server := grpc.NewServer() + + provider := &e2eProvider{ + name: "fake-iac", + version: "0.0.0", + enumerateAllResp: &pb.EnumerateAllResponse{ + Outputs: []*pb.ResourceOutput{ + { + Name: "test-spaces-key", + Type: "infra.spaces_key", + ProviderId: "ABC123", + Status: "active", + Sensitive: map[string]bool{"secret": true}, + }, + }, + }, + } + if err := sdk.RegisterAllIaCProviderServices(server, provider); err != nil { + t.Fatalf("RegisterAllIaCProviderServices: %v", err) + } + go func() { _ = server.Serve(listener) }() + t.Cleanup(server.Stop) + + conn, err := grpc.NewClient("passthrough:///bufnet", + grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { + return listener.DialContext(ctx) + }), + grpc.WithTransportCredentials(insecure.NewCredentials()), + ) + if err != nil { + t.Fatalf("grpc.NewClient: %v", err) + } + t.Cleanup(func() { _ = conn.Close() }) + + // Per cycle 4 review PR 603: bound RPC deadline so a transport- + // level hang doesn't pin the CI worker until the suite-wide + // timeout. Same instance covers all 3 RPCs in this test. + ctx, cancel := context.WithTimeout(context.Background(), e2eRPCDeadline) + t.Cleanup(cancel) + + // Required service: Name + Version typed RPCs. + requiredClient := pb.NewIaCProviderRequiredClient(conn) + nameResp, err := requiredClient.Name(ctx, &pb.NameRequest{}) + if err != nil { + t.Fatalf("Name: %v", err) + } + if nameResp.Name != "fake-iac" { + t.Errorf("expected name=fake-iac, got %q", nameResp.Name) + } + versionResp, err := requiredClient.Version(ctx, &pb.VersionRequest{}) + if err != nil { + t.Fatalf("Version: %v", err) + } + if versionResp.Version != "0.0.0" { + t.Errorf("expected version=0.0.0, got %q", versionResp.Version) + } + + // Optional service: Enumerator.EnumerateAll typed RPC. + enumClient := pb.NewIaCProviderEnumeratorClient(conn) + enumResp, err := enumClient.EnumerateAll(ctx, &pb.EnumerateAllRequest{ + ResourceType: "infra.spaces_key", + }) + if err != nil { + t.Fatalf("EnumerateAll: %v", err) + } + if got := len(enumResp.Outputs); got != 1 { + t.Fatalf("expected 1 output; got %d", got) + } + out := enumResp.Outputs[0] + if out.Name != "test-spaces-key" { + t.Errorf("output.Name = %q; want test-spaces-key", out.Name) + } + if out.ProviderId != "ABC123" { + t.Errorf("output.ProviderId = %q; want ABC123", out.ProviderId) + } + // Crucial assertion: typed map survived the roundtrip. + // The pre-cutover structpb path silently dropped this map. + if got := out.Sensitive["secret"]; !got { + t.Errorf("output.Sensitive[\"secret\"] = false; want true (typed map roundtrip lost?)") + } +} + +// TestIaC_EndToEnd_OptionalNotRegistered_ClientFailsTyped asserts that +// when a provider does NOT satisfy the optional Enumerator interface, +// the typed enumerator client receives a typed gRPC error (codes. +// Unimplemented). This is the "absence of registration IS the negative +// signal" contract from the design — wfctl observes the absence at the +// gRPC layer rather than via a NotSupported flag in the response body. +func TestIaC_EndToEnd_OptionalNotRegistered_ClientFailsTyped(t *testing.T) { + listener := bufconn.Listen(e2eBufSize) + t.Cleanup(func() { _ = listener.Close() }) + server := grpc.NewServer() + provider := &requiredOnlyProvider{} + // requiredOnlyProvider does NOT embed UnimplementedIaCProviderEnumeratorServer, + // so RegisterAllIaCProviderServices skips Enumerator registration — + // absence of the registration IS the negative signal per design. + if err := sdk.RegisterAllIaCProviderServices(server, provider); err != nil { + t.Fatalf("register: %v", err) + } + go func() { _ = server.Serve(listener) }() + t.Cleanup(server.Stop) + + conn, err := grpc.NewClient("passthrough:///bufnet", + grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { + return listener.DialContext(ctx) + }), + grpc.WithTransportCredentials(insecure.NewCredentials()), + ) + if err != nil { + t.Fatalf("grpc.NewClient: %v", err) + } + t.Cleanup(func() { _ = conn.Close() }) + + // Per cycle 4 review PR 603: bound RPC deadline so a transport- + // level hang doesn't pin the CI worker until the suite-wide + // timeout. + ctx, cancel := context.WithTimeout(context.Background(), e2eRPCDeadline) + t.Cleanup(cancel) + + enumClient := pb.NewIaCProviderEnumeratorClient(conn) + _, err = enumClient.EnumerateAll(ctx, &pb.EnumerateAllRequest{ResourceType: "x"}) + if err == nil { + t.Fatalf("expected typed gRPC error for unregistered Enumerator service; got nil") + } + // Per cycle 4 review MINOR-1: pin the failure to codes.Unimplemented + // specifically so a future bufconn/transport behavior change can't + // silently mask the absence-of-registration signal under a different + // status code. + if got := status.Code(err); got != codes.Unimplemented { + t.Fatalf("expected codes.Unimplemented for unregistered service; got %v (err=%v)", got, err) + } +} + +// e2eProvider satisfies pb.IaCProviderRequiredServer + pb.IaCProviderEnumeratorServer +// for the in-process E2E test. It overrides only Name, Version, and +// EnumerateAll; every other Required RPC delegates to the +// Unimplemented*Server defaults (codes.Unimplemented), which is fine +// for this smoke test — we are exercising the wire layer, not provider +// behavior. +type e2eProvider struct { + pb.UnimplementedIaCProviderRequiredServer + pb.UnimplementedIaCProviderEnumeratorServer + + name string + version string + enumerateAllResp *pb.EnumerateAllResponse +} + +func (p *e2eProvider) Name(_ context.Context, _ *pb.NameRequest) (*pb.NameResponse, error) { + return &pb.NameResponse{Name: p.name}, nil +} + +func (p *e2eProvider) Version(_ context.Context, _ *pb.VersionRequest) (*pb.VersionResponse, error) { + return &pb.VersionResponse{Version: p.version}, nil +} + +func (p *e2eProvider) EnumerateAll(_ context.Context, _ *pb.EnumerateAllRequest) (*pb.EnumerateAllResponse, error) { + if p.enumerateAllResp == nil { + return &pb.EnumerateAllResponse{}, nil + } + return p.enumerateAllResp, nil +} + +// requiredOnlyProvider satisfies pb.IaCProviderRequiredServer ONLY — +// it deliberately omits the UnimplementedIaCProviderEnumeratorServer +// embed so the auto-registration helper does NOT advertise the +// optional Enumerator service. The "absence of registration IS the +// negative signal" contract from the design is exercised end-to-end +// via TestIaC_EndToEnd_OptionalNotRegistered_ClientFailsTyped. +type requiredOnlyProvider struct { + pb.UnimplementedIaCProviderRequiredServer +}