test(wftest/bdd): AssertProviderCapabilitiesMatchRegistration belt-and-braces guard (Task 15)#606
Merged
Merged
Conversation
…ices + ResourceDriver Task 3 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds plugin/external/proto/iac.proto defining the typed gRPC contract that supersedes the legacy InvokeService/structpb dispatch path for IaCProvider + ResourceDriver: - service IaCProviderRequired: 11 RPCs every IaC plugin MUST implement (Initialize, Name, Version, Capabilities, Plan, Apply, Destroy, Status, Import, ResolveSizing, BootstrapStateBackend). Compile-time enforced via the SDK type-assert in Task 4. - 6 optional services — providers register only the ones they support: IaCProviderEnumerator (EnumerateAll, EnumerateByTag), IaCProviderDriftDetector (DetectDrift, DetectDriftWithSpecs), IaCProviderCredentialRevoker (RevokeProviderCredential), IaCProviderMigrationRepairer (RepairDirtyMigration), IaCProviderValidator (ValidatePlan), IaCProviderDriftConfigDetector (DetectDriftConfig). Absence of registration IS the negative signal — no NotSupported field on any optional response (per design §Optional services). - service ResourceDriver: 9 RPCs for per-resource-type CRUD dispatch (Create, Read, Update, Delete, Diff, Scale, HealthCheck, SensitiveKeys, Troubleshoot), each carrying resource_type so a single server can route to the per-type driver implementation. Hard invariants honored: - NO google.protobuf.Struct, NO google.protobuf.Any anywhere. - Free-form per-resource Config/Outputs payloads cross the wire as bytes <name>_json (the plugin owns json.Marshal/Unmarshal); this eliminates the structpb conversion surface that previously dropped map[string]bool entries silently (T3.9 finding). - ResourceOutput.sensitive uses typed map<string, bool> per design. Generated iac.pb.go + iac_grpc.pb.go via protoc v34.1 + protoc-gen-go v1.36.11 + protoc-gen-go-grpc v1.6.1. Failing test (plugin/external/proto/iac_proto_test.go) asserts the generated server interfaces exist and have the methods the design requires — drops in iac.proto cause the test file to fail to compile. Verification: GOWORK=off go test ./plugin/external/proto/... PASSES; GOWORK=off go build ./plugin/... ./cmd/... ./module/... clean. Rollback: revert this commit; legacy InvokeService dispatch in plugin.proto remains functional; the additive-only nature of this PR means no consumer is affected until subsequent tasks wire callers.
Task 4 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds plugin/external/sdk/iacserver.go: a single helper that uses Go type-assertion to register every typed IaC gRPC service the provider satisfies, in one call. REQUIRED service: pb.IaCProviderRequiredServer — surfaced as a clear startup-time error if the provider type doesn't satisfy it (rather than failing at the first RPC dispatch with a generic "unimplemented" status). OPTIONAL services (auto-detected): IaCProviderEnumerator, IaCProviderDriftDetector, IaCProviderCredentialRevoker, IaCProviderMigrationRepairer, IaCProviderValidator, IaCProviderDriftConfigDetector. Plus ResourceDriver. Per cycle 3 I-1 of the design: plugin authors write ONE call; they cannot omit registration for a capability they implemented. This removes the registration-omission bug class (the same shape as the legacy InvokeService case-string-typo bug) by removing the manual step entirely. Tests cover four cases: - required-satisfied → required service registered + advertised by grpcSrv.GetServiceInfo(). - enumerator-only → only the optional Enumerator service registered; other optionals stay absent (auto-detection precision). - empty-stub → returns an error naming the unsatisfied required interface, with a docs pointer. - all-capabilities-stub → all 8 typed services (Required + 6 optional + ResourceDriver) registered. Stacked on feat/iac-proto-task3 (Task 3 PR #598 provides the generated server interfaces this helper consumes). Verification: GOWORK=off go test -race ./plugin/external/sdk/... PASS; GOWORK=off go build ./plugin/... ./cmd/... ./module/... clean; GOWORK=off go vet ./plugin/external/... clean. Rollback: revert this commit; SDK consumers can still register services manually via the per-service Register* helpers protoc generated.
…rver callback
Task 29 of the strict-contracts force-cutover plan
(docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5).
Adds the high-level plugin-author API on top of Task 4's
RegisterAllIaCProviderServices:
func main() {
sdk.ServeIaCPlugin(&doProvider{}, sdk.IaCServeOptions{})
}
Per cycle 3 I-1 of the design, service registration happens INSIDE
go-plugin's GRPCServer callback (iacGRPCPlugin.GRPCServer) — the
framework owns *grpc.Server lifecycle, so plugin authors cannot
pre-create a server and forget to register a typed service on it.
API surface (all in plugin/external/sdk/iacserver.go):
- IaCServeOptions{ PluginInfo *PluginInfo } — caller-side options.
- PluginInfo{ HandshakeConfig goplugin.HandshakeConfig } — extension
point for future Name/Version metadata; defaults to ext.Handshake
(the canonical wfctl<->plugin handshake) when zero-valued.
- iacGRPCPlugin{provider any} — implements goplugin.Plugin
(GRPCServer + GRPCClient). The GoCodeAlone fork of go-plugin v1.7.0
is gRPC-only and exposes only the canonical Plugin interface; there
is no GRPCPlugin alias or NetRPCUnsupportedPlugin embed to use.
- ServeIaCPlugin(provider, opts) — wraps goplugin.Serve with the
resolved handshake + a single iacGRPCPlugin entry under the "iac"
key.
- resolveServeHandshake(opts) — extracted helper so the override-vs-
default rule is unit-testable without invoking the blocking
goplugin.Serve loop.
Tests (iacserver_serve_test.go) cover six cases via internal-package
tests (so the unexported plugin type is exercisable without a real
subprocess; subprocess-level coverage lands in Task 6's typed-IaC E2E
test):
- iacGRPCPlugin.GRPCServer registers all satisfied services on the
framework-managed *grpc.Server (Required + Enumerator + ResourceDriver
for the all-stub).
- iacGRPCPlugin.GRPCServer propagates the auto-register error for an
empty stub — go-plugin aborts plugin startup with an actionable
message.
- iacGRPCPlugin.GRPCClient is a no-op (host builds typed clients
directly).
- iacGRPCPlugin satisfies goplugin.Plugin at compile time (refactor
guard).
- ServeIaCPlugin defaults to ext.Handshake when PluginInfo is nil.
- ServeIaCPlugin honors a non-zero override handshake when provided.
Stacked on feat/iac-sdk-auto-register-task4 (Task 4 PR #599 provides
RegisterAllIaCProviderServices, which the GRPCServer callback delegates
to).
Verification: GOWORK=off go test -race ./plugin/external/sdk/... PASS;
GOWORK=off go build ./plugin/... ./cmd/... ./module/... clean;
GOWORK=off go vet ./plugin/external/... clean.
Rollback: revert this commit; plugin authors can fall back to
manually constructing goplugin.Serve + Plugins map referencing
RegisterAllIaCProviderServices in their own GRPCServer callback.
Task 5 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds plugin/external/sdk/contracts.go with the BuildContractRegistry helper that enumerates grpc.Server.GetServiceInfo() and emits a SERVICE-kind ContractDescriptor for each registered service. ContractMode is set to STRICT_PROTO so the host can distinguish typed IaC services from the legacy structpb-mode contracts produced by Module/Step/Trigger ContractProvider implementations. Per cycle 3 I-1 of the 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 gRPC server-reflection dependency required. Service descriptors are emitted in deterministic alphabetical order so callers can rely on stable output for diff/compare operations and the wftest BDD test in Task 15. The helper is safe to call with a nil server (returns an empty but non-nil ContractRegistry) so callers that may construct it before the gRPC server exists do not panic. Tests (contracts_iac_test.go) cover three cases — all pass: - AdvertisesRegisteredIaCServices: a Required + Enumerator + DriftDetector stub yields exactly those service descriptors. - ServiceContractsUseStrictProtoMode: every emitted descriptor is Kind=SERVICE + Mode=STRICT_PROTO (host-side discriminator). - NilServer_ReturnsEmpty: defensive contract for nil input. Stacked on feat/iac-sdk-serve-task29 (Task 29 PR #600 provides ServeIaCPlugin which IaC plugins use to register the services this helper enumerates). Verification: GOWORK=off go test -race ./plugin/external/sdk/... PASS; GOWORK=off go build ./plugin/... ./cmd/... ./module/... clean; GOWORK=off go vet ./plugin/external/... clean. Rollback: revert this commit; ContractRegistry returns the prior shape (Module/Step/Trigger only via the existing ContractProvider hook in grpc_server.go).
…d-braces guard Task 15 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5). Adds wftest/bdd/strict_iac.go with the AssertProviderCapabilitiesMatchRegistration helper. Given a Go provider implementation + the gRPC server with the plugin's service registrations, the helper asserts: - Pass 1: every typed IaC service interface satisfied by the provider's Go type IS registered on the gRPC server. - Pass 2: no IaC service is registered on the gRPC server that the provider's Go type does NOT satisfy (catches the "manual Register* call binds a different Go impl" failure mode). Per cycle 4 belt-and-braces of the design: 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; this helper is the test-time guard that catches the manual-registration omission failure mode. iacServiceChecks lists every typed IaC service the helper knows about (Required + 6 optional + ResourceDriver). New optional services added to iac.proto must be appended here. Helper takes a strictIaCT interface (Errorf + Fatalf + Helper) rather than *testing.T directly so the failure path itself is unit-testable via a recordingT double — the four tests in strict_iac_test.go cover the four behavior axes: - AutoRegisteredAllOK: silent pass when sdk.RegisterAllIaCProviderServices was used as designed. - ManuallyRegisteredMissingOptional_Fails: provider satisfies every optional + ResourceDriver but author registered only Required + Enumerator manually → helper names every missing service in the error messages. - ProviderMissingRequired_Fails: broken test fixture (provider doesn't satisfy IaCProviderRequiredServer) → fatal-class failure with the missing interface named. - RegisteredButProviderDoesntSatisfy_Fails: server has Enumerator registered but provider passed to assert is requiredOnlyStub → over-registration named in error. Stacked on feat/iac-sdk-contracts-task5 (Task 5 PR #602 provides the typed proto interfaces this helper inspects via reflection- free type-assert chain). Verification: GOWORK=off go test -race ./wftest/... PASS; GOWORK=off go build ./... clean; GOWORK=off go vet ./wftest/... clean. Rollback: revert this commit; canonical registration path (sdk.RegisterAllIaCProviderServices) is unaffected.
Per cycle 4 code-review PR 606 MINOR-1: iacServiceChecks in wftest/bdd/strict_iac.go is a manually-maintained list of typed IaC services. If iac.proto adds a new optional service later without appending to iacServiceChecks, AssertProviderCapabilitiesMatchRegistration silently passes for the new service (the satisfies check is never invoked for unlisted services), leaving a hole in the cycle 4 belt- and-braces invariant. Adds wftest/bdd/strict_iac_internal_test.go (package bdd, internal test so iacServiceChecks is in scope) with TestIaCServiceChecks_CoversEveryProtoService — walks pb.File_plugin_external_proto_iac_proto.Services() at runtime and asserts: - Forward: every IaC service name in the proto (prefix-matched on iacServicePrefix) appears in iacServiceChecks. Missing entries produce a test failure naming the service(s) the proto declares but the helper doesn't cover. - Inverse: every iacServiceCheck row names a service the proto actually declares. Catches the rename-without-cleanup failure mode (proto renames a service; iacServiceChecks still references the old name). Tightens the manual-maintenance surface into a compile-time- discoverable test failure rather than a silent test-passing regression. Verification: GOWORK=off go test ./wftest/bdd/... -run "TestIaCServiceChecks|TestAssertProviderCapabilities" -count=1 → PASS (5/5); gofmt clean. Rollback: revert this commit; the cycle 4 belt-and-braces guard returns to the manual-maintenance form (still works, just no test-time coverage check on the helper itself).
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new wftest/BBD helper that cross-checks a provider’s Go-level IaC gRPC capabilities against what’s actually registered on a grpc.Server, to catch manual registration omissions/over-registrations during tests as part of the strict-contracts force-cutover plan.
Changes:
- Introduces
bdd.AssertProviderCapabilitiesMatchRegistration(t, provider, grpcSrv)with a two-pass check for missing vs extra IaC service registrations. - Adds external-package tests covering OK, missing optional registrations, missing required interface, and over-registration scenarios.
- Adds an internal-package guard test that ensures
iacServiceChecksstays in sync with the services declared iniac.proto.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| wftest/bdd/strict_iac.go | Implements the assertion helper and the known IaC service checklist. |
| wftest/bdd/strict_iac_test.go | Black-box tests validating helper behavior across key failure modes. |
| wftest/bdd/strict_iac_internal_test.go | Ensures the helper’s service checklist matches iac.proto service declarations. |
| 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", |
intel352
force-pushed
the
feat/iac-sdk-contracts-task5
branch
from
c03c871 to
8bb6e52
Compare
…ask15 # Conflicts: # plugin/external/proto/iac.pb.go # plugin/external/proto/iac.proto # plugin/external/proto/iac_grpc.pb.go # plugin/external/proto/iac_proto_test.go # plugin/external/sdk/iacserver.go # plugin/external/sdk/iacserver_serve_test.go # plugin/external/sdk/iacserver_test.go
…e-merge) Generated proto descriptor variable is File_iac_proto (per iac.proto's go_package option), not File_plugin_external_proto_iac_proto. Reference fixed; all 5 BDD strict-IaC tests PASS. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Task 15 of the strict-contracts force-cutover plan (docs/plans/2026-05-10-strict-contracts-force-cutover.md, rev5).
Adds
wftest/bdd/strict_iac.gowithAssertProviderCapabilitiesMatchRegistration(t, provider, grpcSrv)— the cycle 4 belt-and-braces test-time guard.The canonical registration path is
sdk.RegisterAllIaCProviderServices(Task 4, PR #599), which uses Go type-assertion to auto-detect every interface and cannot omit a registration. The per-serviceRegister*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.Stacked on PR #602 (Task 5). Base branch is
feat/iac-sdk-contracts-task5.Behavior
The helper does two passes:
requiredOnlyStub) is reported.iacServiceCheckslists every typed IaC service the helper knows about (Required + 6 optional + ResourceDriver). New optional services added toiac.protomust be appended here.Helper takes a
strictIaCTinterface (Errorf + Fatalf + Helper) rather than*testing.Tdirectly so the failure path itself is unit-testable via a recording double.Tests (4 cases, all PASS, race-mode included)
AutoRegisteredAllOK— silent pass whensdk.RegisterAllIaCProviderServicesis used as designedManuallyRegisteredMissingOptional_Fails— provider satisfies every optional + ResourceDriver but author registered only Required + Enumerator manually → helper names DriftDetector / CredentialRevoker / MigrationRepairer / Validator / DriftConfigDetector / ResourceDriver in the error messagesProviderMissingRequired_Fails— broken test fixture (provider doesn't satisfyIaCProviderRequiredServer) → fatal-class failure naming the missing interfaceRegisteredButProviderDoesntSatisfy_Fails— server has Enumerator registered but provider isrequiredOnlyStub→ over-registration named in errorVerification
GOWORK=off go test -race ./wftest/...→ PASSGOWORK=off go build ./...→ cleanGOWORK=off go vet ./wftest/...→ cleanRollback
Revert this commit; canonical registration path (
sdk.RegisterAllIaCProviderServices) is unaffected.Test plan
🤖 Generated with Claude Code