feat(iac): provider service-registration + IaCProviderRegionLister (infra-admin migration PR-1)#836
Merged
Conversation
Mirrors the Enumerator/DriftDetector/ProviderValidator optional-interface pattern. Callers type-assert and fall back to a static catalog when the provider doesn't implement it. Foundational for step.iac_provider_catalog and the core gRPC-client adapter (Task 2). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
….IaCProvider Core-importable adapter wrapping pb.IaCProviderRequiredClient + pb.IaCProviderRegionListerClient as interfaces.IaCProvider + interfaces.IaCProviderRegionLister. Replicates the proto↔interfaces conversion helpers from cmd/wfctl/iac_typed_adapter.go (package main, not importable). Enables WiringHook service registration in Task 3 so engine steps can resolve external iac.provider plugins via app.GetService. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…CProvider services via WiringHook ExternalPluginAdapter.WiringHooks() now detects iac.provider plugins by inspecting the ContractRegistry (and diskManifest.IaCServices fallback) for the IaCProviderRequired service name — the analog of IaCStateBackend detection. For each such plugin, a WiringHook is returned whose closure captures adapter.Conn() and calls app.RegisterService(<pluginName>, providerclient.New(conn)) when the engine runs hooks during BuildFromConfig (after app.Init(), before app.Start()). Steps resolve the provider via app.GetService(cfg.Provider, &provider) under the plugin name. This is the foundational gap that v1/v1.1 infra-admin silently lacked: external iac.provider plugins were loaded but never registered as services, so app.GetService(name, &provider) found nothing at step execution time. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
PR-1 of the infra-admin plugin migration: adds the foundational core capability that lets external iac.provider plugins be registered as interfaces.IaCProvider services in the modular DI graph. Introduces a new optional sub-interface, a core-importable gRPC-client adapter, and a WiringHook on ExternalPluginAdapter that performs the registration before any pipeline step runs.
Changes:
- New optional
interfaces.IaCProviderRegionListersub-interface (type-asserted; mirrors Enumerator/DriftDetector). - New
iac/providerclientpackage wrapping the proto IaC gRPC clients asinterfaces.IaCProvider(+ optional region lister), with bufconn-based tests. ExternalPluginAdapter.WiringHooks()now detectsiac.providerplugins via contract-registry / disk-manifest and registers aproviderclient.Adapterunder the plugin name.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| interfaces/iac_provider.go | Adds optional IaCProviderRegionLister interface. |
| interfaces/iac_provider_test.go | Asserts the new optional interface is genuinely optional. |
| iac/providerclient/adapter.go | New core-importable proto↔interfaces adapter implementing IaCProvider + IaCProviderRegionLister. |
| iac/providerclient/adapter_test.go | In-proc gRPC tests for the adapter and compile-time interface assertions. |
| plugin/external/adapter.go | Implements WiringHooks() to register iac.provider adapters as services. |
| plugin/external/iac_provider_wiring_test.go | Tests the new wiring hook against a fake gRPC server and the modular app. |
Comment on lines
+52
to
+58
| func New(conn grpc.ClientConnInterface) *Adapter { | ||
| return &Adapter{ | ||
| conn: conn, | ||
| required: pb.NewIaCProviderRequiredClient(conn), | ||
| regionLister: pb.NewIaCProviderRegionListerClient(conn), | ||
| } | ||
| } |
Comment on lines
+145
to
+152
| // DetectDrift calls IaCProviderRequired.Status to satisfy the IaCProvider interface. | ||
| // This is a stub implementation — the required interface needs DetectDrift, so we | ||
| // surface a not-implemented error. Full drift detection is available via the | ||
| // IaCProviderDriftDetector optional service, which is not in scope for PR-1. | ||
| func (a *Adapter) DetectDrift(ctx context.Context, resources []interfaces.ResourceRef) ([]interfaces.DriftResult, error) { | ||
| return nil, fmt.Errorf("%w: DetectDrift optional service not wired in PR-1 adapter", | ||
| interfaces.ErrProviderMethodUnimplemented) | ||
| } |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
…tDetector + fix Endpoint Addresses all PR-1 review fixes: 1. Advertisement-gated optional clients (THE key fix): New() now accepts advertisedServices map[string]bool; IaCProviderRegionLister and IaCProviderDriftDetector gRPC clients are constructed ONLY when their service name appears in the map. *Adapter no longer unconditionally satisfies interfaces.IaCProviderRegionLister — the RegionLister() accessor returns nil for unadvertised plugins, enabling catalog steps' static-fallback path to fire correctly. Accessor interfaces RegionListerProvider and DriftDetectorProvider defined in iac/providerclient for consuming steps. Negative-path test added: TestAdapter_RegionLister_Unadvertised confirms nil return + that *Adapter does NOT satisfy IaCProviderRegionLister directly. 2. DriftDetector wired: pb.NewIaCProviderDriftDetectorClient constructed when advertised; DetectDrift() routes through it (returns ErrProviderMethodUnimplemented when unadvertised); driftDetectorAdapter satisfies interfaces.DriftConfigDetector for DetectDriftWithSpecs. 3. BootstrapResult.Endpoint mapped: r.GetEndpoint() now included in the returned BootstrapResult literal (DO Spaces endpoint was silently lost). 4. Capabilities cached via sync.Once: SupportedCanonicalKeys() reuses fetchCapabilities() instead of issuing a second RPC. 5. plugin/external/adapter.go WiringHooks: collects optional service names via advertisedOptionalIaCServices() and forwards them to providerclient.New; adds diskManifest fallback rationale comment on advertisesIaCProviderRequiredService(). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
PR-1 of the infra-admin plugin migration (design + plan + ADRs, merged). Foundational core capability — this is the gap that made v1/v1.1 infra-admin never function at runtime (an external
iac.providerplugin was never registered as aninterfaces.IaCProviderservice).Tasks (TDD, one commit each)
interfaces.IaCProviderRegionLister— new optional sub-interface (type-asserted), mirrors Enumerator/DriftDetector.iac/providerclient— core-importable adapter wrapping theplugin/external/protoIaC gRPC clients (IaCProviderRequired+ optionalRegionLister/DriftDetector) asinterfaces.IaCProvider. Tested against an in-proc gRPC server.ExternalPluginAdapter.WiringHooks()(wasnil) now detectsiac.providerplugins viadiskManifest.IaCServicesand returns a hook thatapp.RegisterService(<moduleName>, providerclient.New(conn))(precedent:module/iac_module.go:94). Registers before any step executes (hook runs inbuildFromConfig, afterInit, beforeStart).ADR 0013 (steps bind
interfaces.IaCProvider) + ADR 0014 (this registration mechanism). No newIaCProviderProviderinterface; existingPlatformProviderstep.iac_*untouched.Lead-verified:
GOWORK=off go build ./...green;interfaces/iac/providerclient/plugin/externaltests pass;golangci-lint --new-from-rev=origin/main0 issues.🤖 Generated with Claude Code