feat(providerclient): wire ResourceDriver CRUD so apply mutates external providers (infra-admin PR13/13, ADR 0021)#851
Merged
Conversation
…PR-2)
Implements the deferred PR-2 migration in iac/providerclient.Adapter:
- Adds IaCServiceResourceDriver const ("workflow.plugin.external.iac.ResourceDriver")
- Adds resourceDriverAdapter implementing interfaces.ResourceDriver (all 8 methods:
Create/Read/Update/Delete/Diff/HealthCheck/Scale/SensitiveKeys) PLUS the optional
interfaces.Troubleshooter (Troubleshoot) using the same JSON-bytes convention
(config_json/outputs_json) as the existing typed adapter
- Compile-time guards: var _ interfaces.ResourceDriver and
var _ interfaces.Troubleshooter on *resourceDriverAdapter (mirror typedResourceDriver)
- Wires advertisement-gated pb.ResourceDriverClient in New(); nil when not advertised
- Replaces the ErrProviderMethodUnimplemented stub with a live implementation
- Adds ResourceDriverProvider accessor interface mirroring RegionListerProvider/
RunnerProvider pattern; not-advertised path still returns ErrProviderMethodUnimplemented
- mapResourceDriverGRPCError maps codes.NotFound/AlreadyExists/ResourceExhausted/
Unavailable/DeadlineExceeded/Unauthenticated/PermissionDenied/InvalidArgument/
FailedPrecondition/Unimplemented to engine sentinels. Wraps with %w/%w (sentinel +
original gRPC error) so callers recover BOTH the sentinel (errors.Is) AND the gRPC
status (status.Code walking the unwrap chain); default case keeps method attribution
while preserving the status via %w
Closes the runtime advertisement gap: plugin/external/adapter.go
advertisedOptionalIaCServices() now forwards IaCServiceResourceDriver (and
IaCServiceRunner, which #850 added but the switch never matched) from the
plugin's ContractRegistry to providerclient.New, so the WiringHook-registered
adapter constructs the real ResourceDriver client. Without this, the adapter
stayed nil and ApplyPlanWithHooks -> provider.ResourceDriver(type) still hit
ErrProviderMethodUnimplemented end-to-end.
- Tests (providerclient): bufconn round-trip for all 8 CRUD methods + Troubleshoot;
negative advertisement gate; table-driven gRPC error-mapping test covering all 10
status codes (asserts both the errors.Is sentinel AND status.Code recovery via the
unwrap chain)
- Tests (plugin/external): advertisedOptionalIaCServices forwards ResourceDriver
(and Runner) when advertised / excludes when not; end-to-end WiringHook ->
GetService -> ResourceDriver(type) returns the real bridge and Create round-trips
WITH config_json echoed back to prove the Config JSON survives the boundary;
unadvertised plugin still returns the unimplemented error
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR completes the runtime wiring for the optional IaC ResourceDriver service so step.iac_provider_apply can execute per-resource CRUD operations against external IaC provider plugins (per ADR 0021), by (1) bridging the pb.ResourceDriver gRPC client into an interfaces.ResourceDriver implementation and (2) ensuring service advertisement is forwarded through the WiringHook path.
Changes:
- Add
IaCServiceResourceDrivercapability wiring and aresourceDriverAdapterthat bridges CRUD/Diff/Scale/HealthCheck/SensitiveKeys/Troubleshoot over gRPC. - Forward optional IaC service advertisements (now including
RunnerandResourceDriver) from the pluginContractRegistryintoproviderclient.New(...). - Add end-to-end wiring tests and unit tests covering advertisement gating and gRPC error→sentinel mapping.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
plugin/external/iac_provider_wiring_test.go |
Extends wiring tests to exercise optional-service advertisement and end-to-end ResourceDriver bridging via WiringHook. |
plugin/external/adapter.go |
Forwards additional optional IaC service advertisements (Runner, ResourceDriver) from ContractRegistry into providerclient.New. |
iac/providerclient/adapter.go |
Implements ResourceDriverProvider + resourceDriverAdapter and wires optional ResourceDriver client construction based on advertisement. |
iac/providerclient/adapter_test.go |
Adds unit tests for ResourceDriver advertisement gating, CRUD round-trips, and gRPC error mapping (including unwrap-chain preservation). |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…ented wraps gRPC err (Copilot) - Scale rejects out-of-int32-range replicas explicitly (was clampInt32 → silent saturation → unintended scale); mirrors typedResourceDriver.Scale. - mapResourceDriverGRPCError Unimplemented case wraps the original gRPC error (%w) so status.Code stays recoverable, consistent with the other mapped cases + translateRPCErr. Co-Authored-By: Claude Opus 4.8 (1M context) <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.
PR13/13 — wire
providerclient.ResourceDriver(CRUD) [scope amendment, ADR 0021]Makes
step.iac_provider_applyactually create/update/delete resources against an external IaC provider. Discovered by the demonstration-fidelity gate (scenario 92 on the real v0.72.0 stack):iac/providerclient.Adapter.ResourceDriver()returnedErrProviderMethodUnimplemented(the migration deferred per-action CRUD as "PR-2"), so apply only planned + hash-guarded — it never mutated infra, and commit-back always sawpartial-apply. User-approved amendment (AskUserQuestion 2026-06-03).Changes
iac/providerclient.Adapter: newIaCServiceResourceDriveradvertised-service const; aresourceDriverAdapterbridging thepb.ResourceDriverClient→interfaces.ResourceDriver(Create/Read/Update/Delete/Diff/Scale/HealthCheck/SensitiveKeys/Troubleshoot), gated on advertisement (nil +ErrProviderMethodUnimplementedwhen unadvertised — unchanged regression path). JSON bridge for Config/Outputs (*_jsonbytes) field-for-field identical to the wfctl typed adapter (all 6 ResourceSpec fields;sensitivemap preserved). gRPC error mapping (NotFound→ErrResourceNotFound, AlreadyExists→ErrResourceAlreadyExists, …) with%w/%wwrapping so callers recover BOTH the sentinel (errors.Is) AND the gRPC status (status.Code). Compile-timevar _ interfaces.ResourceDriver/Troubleshooterguards.plugin/external/adapter.go:advertisedOptionalIaCServices()now forwardsIaCServiceResourceDriver(andIaCServiceRunner, which was also missing since Add optional IaC provider job runner contract #850 — a regression-guard bonus) from the ContractRegistry intoproviderclient.New, so the WiringHook actually activates the bridge at runtime.Tests
errors.Issentinel ANDstatus.Code).TestWiringHook_ResourceDriver_WiredEndToEnd— full WiringHook →app.GetService→Adapter.ResourceDriver(...)returns the real bridge + aCreateround-trips withconfig_jsonsurviving the gRPC boundary; + the unadvertised negative path.Verified:
go build ./...exit 0; fullgo test ./...exit 0 (151 ok); fullgolangci-lint v2.12.00 issues. No proto/contract change (theResourceDriverservice + stubs already shipped). After merge: cut workflow v0.73.0, re-pin scenario 92, un-SKIP the apply→commit-back assertion.🤖 Generated with Claude Code