diff --git a/plugin/external/sdk/contracts.go b/plugin/external/sdk/contracts.go new file mode 100644 index 000000000..b9f28989a --- /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 000000000..b67aebaf8 --- /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/wftest/bdd/strict_iac.go b/wftest/bdd/strict_iac.go new file mode 100644 index 000000000..b811e36c3 --- /dev/null +++ b/wftest/bdd/strict_iac.go @@ -0,0 +1,165 @@ +package bdd + +import ( + "sort" + "strings" + + "google.golang.org/grpc" + + pb "github.com/GoCodeAlone/workflow/plugin/external/proto" +) + +// strictIaCT is the minimal testing.TB shape that +// AssertProviderCapabilitiesMatchRegistration uses. Defined as an +// interface (rather than testing.TB) so tests can substitute a +// recording double — the helper's failure path is itself unit-tested. +type strictIaCT interface { + Errorf(format string, args ...any) + Fatalf(format string, args ...any) + Helper() +} + +// iacServicePrefix identifies typed IaC service names emitted by +// iac.proto's package option. +const iacServicePrefix = "workflow.plugin.external.iac." + +// iacServiceCheck pairs a typed service name with a runtime check that +// reports whether a provider Go type satisfies the corresponding +// generated server interface. +type iacServiceCheck struct { + serviceName string + satisfies func(any) bool +} + +// iacServiceChecks lists every typed IaC service this helper knows +// about. New optional services added to iac.proto must be appended +// here; the cycle 4 belt-and-braces invariant is the test failing +// loudly for any service the SDK auto-registration helper already +// covers. +var iacServiceChecks = []iacServiceCheck{ + {"workflow.plugin.external.iac.IaCProviderRequired", func(p any) bool { + _, ok := p.(pb.IaCProviderRequiredServer) + return ok + }}, + {"workflow.plugin.external.iac.IaCProviderEnumerator", func(p any) bool { + _, ok := p.(pb.IaCProviderEnumeratorServer) + return ok + }}, + {"workflow.plugin.external.iac.IaCProviderDriftDetector", func(p any) bool { + _, ok := p.(pb.IaCProviderDriftDetectorServer) + return ok + }}, + {"workflow.plugin.external.iac.IaCProviderCredentialRevoker", func(p any) bool { + _, ok := p.(pb.IaCProviderCredentialRevokerServer) + return ok + }}, + {"workflow.plugin.external.iac.IaCProviderMigrationRepairer", func(p any) bool { + _, ok := p.(pb.IaCProviderMigrationRepairerServer) + return ok + }}, + {"workflow.plugin.external.iac.IaCProviderValidator", func(p any) bool { + _, ok := p.(pb.IaCProviderValidatorServer) + return ok + }}, + {"workflow.plugin.external.iac.IaCProviderDriftConfigDetector", func(p any) bool { + _, ok := p.(pb.IaCProviderDriftConfigDetectorServer) + return ok + }}, + {"workflow.plugin.external.iac.ResourceDriver", func(p any) bool { + _, ok := p.(pb.ResourceDriverServer) + return ok + }}, +} + +// AssertProviderCapabilitiesMatchRegistration asserts that every typed +// IaC gRPC service interface satisfied by provider's Go type IS +// registered on grpcSrv, AND no IaC service is registered that the +// provider's Go type does NOT satisfy. Reports specific missing / +// extra service names so test failures point at the exact omission. +// +// Per cycle 4 of the strict-contracts force-cutover design (belt-and- +// braces): the canonical registration path is +// sdk.RegisterAllIaCProviderServices, which uses Go type-assertion to +// auto-detect every interface and cannot omit a registration. The +// per-service Register* helpers are still exposed for advanced use +// cases (e.g., a plugin that registers a different Go type per +// optional service); this helper is the test-time guard that catches +// the manual-registration omission failure mode. +// +// The provider parameter is the live Go provider implementation +// (typically a *DOProvider or test stub). The grpcSrv parameter is +// the gRPC server with the plugin's service registrations on it +// (typically the result of grpc.NewServer() + RegisterAllIaCProviderServices, +// or — for the failure case this guard catches — a server with +// manual per-service registrations). +// +// Provider MUST satisfy pb.IaCProviderRequiredServer at minimum; if +// not, the helper reports a fatal-class failure naming the missing +// required interface (a broken fixture, not a runtime issue). +func AssertProviderCapabilitiesMatchRegistration(t strictIaCT, provider any, grpcSrv *grpc.Server) { + t.Helper() + if grpcSrv == nil { + t.Fatalf("AssertProviderCapabilitiesMatchRegistration: grpcSrv is nil") + return + } + if provider == nil { + t.Fatalf("AssertProviderCapabilitiesMatchRegistration: provider is nil") + return + } + if _, ok := provider.(pb.IaCProviderRequiredServer); !ok { + t.Fatalf( + "provider %T does not satisfy pb.IaCProviderRequiredServer "+ + "(broken test fixture); see decisions/0024-iac-typed-force-cutover.md", + provider, + ) + return + } + + registered := grpcSrv.GetServiceInfo() + expected := make(map[string]bool, len(iacServiceChecks)) + for _, c := range iacServiceChecks { + if c.satisfies(provider) { + expected[c.serviceName] = true + } + } + + // Pass 1: every interface the provider satisfies MUST be registered. + missing := make([]string, 0, len(expected)) + for name := range expected { + if _, ok := registered[name]; !ok { + missing = append(missing, name) + } + } + sort.Strings(missing) + for _, name := range missing { + t.Errorf( + "provider %T satisfies %s but the corresponding gRPC service "+ + "is NOT registered on the plugin handle; "+ + "call sdk.RegisterAllIaCProviderServices to auto-register", + provider, name, + ) + } + + // Pass 2: no IaC service registered that the provider doesn't satisfy. + // We only inspect services in the workflow.plugin.external.iac + // namespace — non-IaC services (e.g., the legacy PluginService) are + // out of scope. + extra := make([]string, 0) + for name := range registered { + if !strings.HasPrefix(name, iacServicePrefix) { + continue + } + if !expected[name] { + extra = append(extra, name) + } + } + sort.Strings(extra) + for _, name := range extra { + t.Errorf( + "gRPC service %s is registered on the plugin handle but "+ + "provider %T does not satisfy the corresponding Go interface; "+ + "manual Register* call appears to bind a different impl", + name, provider, + ) + } +} diff --git a/wftest/bdd/strict_iac_internal_test.go b/wftest/bdd/strict_iac_internal_test.go new file mode 100644 index 000000000..513d633c7 --- /dev/null +++ b/wftest/bdd/strict_iac_internal_test.go @@ -0,0 +1,86 @@ +package bdd + +// Internal-package tests so iacServiceChecks (unexported) is in scope. +// External-package tests for AssertProviderCapabilitiesMatchRegistration +// behavior are in strict_iac_test.go. + +import ( + "strings" + "testing" + + pb "github.com/GoCodeAlone/workflow/plugin/external/proto" +) + +// TestIaCServiceChecks_CoversEveryProtoService walks the iac.proto +// FileDescriptor at runtime and asserts every gRPC service whose +// fully-qualified name starts with iacServicePrefix is also listed +// in iacServiceChecks. Without this guard, a new optional service +// added to iac.proto silently passes through +// AssertProviderCapabilitiesMatchRegistration's two-pass logic +// (the satisfies check is never invoked for unlisted services), so +// a missing-from-iacServiceChecks bug leaves a hole in the cycle 4 +// belt-and-braces invariant. +// +// Per cycle 4 code-review PR 606 MINOR-1: tightens the manual- +// maintenance surface into a compile-time-discoverable test failure +// rather than a silent test-passing regression. +func TestIaCServiceChecks_CoversEveryProtoService(t *testing.T) { + fd := pb.File_iac_proto + if fd == nil { + t.Fatalf("pb.File_iac_proto is nil — proto bindings not loaded") + } + services := fd.Services() + if services.Len() == 0 { + t.Fatalf("iac.proto descriptor has no services — proto regeneration broke?") + } + + // Index of fully-qualified names that iacServiceChecks already covers. + covered := make(map[string]bool, len(iacServiceChecks)) + for _, c := range iacServiceChecks { + covered[c.serviceName] = true + } + + // Every IaC service name in the proto must appear in iacServiceChecks. + missing := make([]string, 0) + for i := 0; i < services.Len(); i++ { + fqn := string(services.Get(i).FullName()) + if !strings.HasPrefix(fqn, iacServicePrefix) { + // Not an IaC service — skip (defensive; iac.proto's package + // option pins every service to this prefix today). + continue + } + if !covered[fqn] { + missing = append(missing, fqn) + } + } + if len(missing) > 0 { + t.Errorf( + "iacServiceChecks (wftest/bdd/strict_iac.go) is missing %d entr(y/ies) that "+ + "iac.proto declares: %v\n"+ + "Add a corresponding iacServiceCheck row for each so "+ + "AssertProviderCapabilitiesMatchRegistration covers them.", + len(missing), missing, + ) + } + + // Inverse check: every iacServiceCheck names a service that the + // proto actually defines. Catches the rename-without-cleanup + // failure mode. + declared := make(map[string]bool, services.Len()) + for i := 0; i < services.Len(); i++ { + declared[string(services.Get(i).FullName())] = true + } + stale := make([]string, 0) + for _, c := range iacServiceChecks { + if !declared[c.serviceName] { + stale = append(stale, c.serviceName) + } + } + if len(stale) > 0 { + t.Errorf( + "iacServiceChecks references %d service(s) not declared in iac.proto: %v\n"+ + "The proto likely renamed/removed them; remove the stale rows.", + len(stale), stale, + ) + } +} diff --git a/wftest/bdd/strict_iac_test.go b/wftest/bdd/strict_iac_test.go new file mode 100644 index 000000000..ca87d352e --- /dev/null +++ b/wftest/bdd/strict_iac_test.go @@ -0,0 +1,160 @@ +package bdd_test + +import ( + "fmt" + "strings" + "testing" + + "google.golang.org/grpc" + + pb "github.com/GoCodeAlone/workflow/plugin/external/proto" + "github.com/GoCodeAlone/workflow/plugin/external/sdk" + "github.com/GoCodeAlone/workflow/wftest/bdd" +) + +// TestAssertProviderCapabilitiesMatchRegistration_AutoRegisteredAllOK +// asserts the helper passes silently when sdk.RegisterAllIaCProviderServices +// has been used as designed (every interface the provider satisfies IS +// also registered on the gRPC server). +func TestAssertProviderCapabilitiesMatchRegistration_AutoRegisteredAllOK(t *testing.T) { + srv := grpc.NewServer() + provider := &allCapabilitiesStub{} + if err := sdk.RegisterAllIaCProviderServices(srv, provider); err != nil { + t.Fatalf("auto-register: %v", err) + } + + rec := &recordingT{} + bdd.AssertProviderCapabilitiesMatchRegistration(rec, provider, srv) + if rec.failed { + t.Fatalf("expected silent pass; got failures: %v", rec.errors) + } +} + +// TestAssertProviderCapabilitiesMatchRegistration_ManuallyRegisteredMissingOptional_Fails +// asserts the cycle 4 belt-and-braces invariant: when a plugin author +// uses the per-service Register* helpers manually and forgets one, the +// helper surfaces the omission as a test failure with the missing +// service name in the error message. +// +// Reproduces the failure mode the SDK auto-registration helper closes +// (per cycle 3 I-1) — proves that even if a plugin author bypasses +// sdk.RegisterAllIaCProviderServices, the test-time guard still catches +// the omission. +func TestAssertProviderCapabilitiesMatchRegistration_ManuallyRegisteredMissingOptional_Fails(t *testing.T) { + srv := grpc.NewServer() + provider := &allCapabilitiesStub{} + // Register Required + Enumerator manually, OMIT DriftDetector and others. + pb.RegisterIaCProviderRequiredServer(srv, provider) + pb.RegisterIaCProviderEnumeratorServer(srv, provider) + // Intentionally not registering DriftDetector / CredentialRevoker / + // MigrationRepairer / Validator / DriftConfigDetector / ResourceDriver + // even though provider's Go type satisfies them all. + + rec := &recordingT{} + bdd.AssertProviderCapabilitiesMatchRegistration(rec, provider, srv) + if !rec.failed { + t.Fatalf("expected failure for missing service registration; rec.errors=%v", rec.errors) + } + wantContains := []string{ + "IaCProviderDriftDetector", + "IaCProviderCredentialRevoker", + "IaCProviderMigrationRepairer", + "IaCProviderValidator", + "IaCProviderDriftConfigDetector", + "ResourceDriver", + } + joined := strings.Join(rec.errors, "\n") + for _, name := range wantContains { + if !strings.Contains(joined, name) { + t.Errorf("expected error mentioning %q; errors:\n%s", name, joined) + } + } +} + +// TestAssertProviderCapabilitiesMatchRegistration_ProviderMissingRequired_Fails +// asserts the helper rejects a "provider" Go type that does not satisfy +// pb.IaCProviderRequiredServer at all. Distinct from +// RegisterAllIaCProviderServices's startup-time error: this is the +// test-author-writing-a-broken-fixture failure mode. +func TestAssertProviderCapabilitiesMatchRegistration_ProviderMissingRequired_Fails(t *testing.T) { + srv := grpc.NewServer() + provider := &noIaCStub{} + + rec := &recordingT{} + bdd.AssertProviderCapabilitiesMatchRegistration(rec, provider, srv) + if !rec.failed { + t.Fatalf("expected failure for provider missing required interface") + } + if !strings.Contains(strings.Join(rec.errors, "\n"), "IaCProviderRequiredServer") { + t.Errorf("error must name the missing required interface; got: %v", rec.errors) + } +} + +// TestAssertProviderCapabilitiesMatchRegistration_RegisteredButProviderDoesntSatisfy_Fails +// asserts the inverse of the previous case: a service is registered on +// the gRPC server, but the Go provider type does NOT satisfy the +// corresponding interface. This catches the case where the plugin +// author registered an OptionalServer with one type while the +// "provider" passed to the test is a different (narrower) type. +func TestAssertProviderCapabilitiesMatchRegistration_RegisteredButProviderDoesntSatisfy_Fails(t *testing.T) { + srv := grpc.NewServer() + all := &allCapabilitiesStub{} + pb.RegisterIaCProviderRequiredServer(srv, all) + pb.RegisterIaCProviderEnumeratorServer(srv, all) + + // Provider passed to the assertion only satisfies Required (no + // Enumerator embed) — yet the server has Enumerator registered. + provider := &requiredOnlyStub{} + + rec := &recordingT{} + bdd.AssertProviderCapabilitiesMatchRegistration(rec, provider, srv) + if !rec.failed { + t.Fatalf("expected failure for over-registration (server has Enumerator, provider doesn't satisfy)") + } + if !strings.Contains(strings.Join(rec.errors, "\n"), "IaCProviderEnumerator") { + t.Errorf("error must name the over-registered service; got: %v", rec.errors) + } +} + +// recordingT is a minimal testing.TB shape that captures Errorf and +// Fatalf without aborting the surrounding test goroutine. Used so the +// strict-IaC helper's failure path can be asserted without making the +// parent test fail. Only the methods AssertProviderCapabilitiesMatchRegistration +// touches are implemented; everything else delegates to a no-op base. +type recordingT struct { + failed bool + errors []string +} + +func (r *recordingT) Errorf(format string, args ...any) { + r.failed = true + r.errors = append(r.errors, fmt.Sprintf(format, args...)) +} + +func (r *recordingT) Fatalf(format string, args ...any) { + r.failed = true + r.errors = append(r.errors, fmt.Sprintf(format, args...)) +} + +func (r *recordingT) Helper() {} + +// allCapabilitiesStub satisfies every required + optional IaC service +// plus ResourceDriver. +type allCapabilitiesStub struct { + pb.UnimplementedIaCProviderRequiredServer + pb.UnimplementedIaCProviderEnumeratorServer + pb.UnimplementedIaCProviderDriftDetectorServer + pb.UnimplementedIaCProviderCredentialRevokerServer + pb.UnimplementedIaCProviderMigrationRepairerServer + pb.UnimplementedIaCProviderValidatorServer + pb.UnimplementedIaCProviderDriftConfigDetectorServer + pb.UnimplementedResourceDriverServer +} + +// requiredOnlyStub satisfies Required ONLY. +type requiredOnlyStub struct { + pb.UnimplementedIaCProviderRequiredServer +} + +// noIaCStub satisfies no IaC interface. +type noIaCStub struct{}