From 8d2ae425f567277a80fe6729230ec27d55572062 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 3 Jun 2026 03:22:09 -0400 Subject: [PATCH 1/2] feat(providerclient): wire ResourceDriver gRPC service into Adapter (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) --- iac/providerclient/adapter.go | 339 ++++++++++++++++- iac/providerclient/adapter_test.go | 382 ++++++++++++++++++++ plugin/external/adapter.go | 14 +- plugin/external/iac_provider_wiring_test.go | 208 ++++++++++- 4 files changed, 918 insertions(+), 25 deletions(-) diff --git a/iac/providerclient/adapter.go b/iac/providerclient/adapter.go index 53a7b98c7..af1ee72d5 100644 --- a/iac/providerclient/adapter.go +++ b/iac/providerclient/adapter.go @@ -49,6 +49,11 @@ const ( // IaCServiceRunner is the gRPC service name for the optional // IaCProviderRunner service. IaCServiceRunner = "workflow.plugin.external.iac.IaCProviderRunner" + // IaCServiceResourceDriver is the gRPC service name for the optional + // ResourceDriver service. When advertised, Adapter.ResourceDriver(type) + // returns a per-resource-type bridge that routes Create/Read/Update/Delete/ + // Diff/HealthCheck/Scale/SensitiveKeys through the plugin's gRPC process. + IaCServiceResourceDriver = "workflow.plugin.external.iac.ResourceDriver" ) // RegionListerProvider is a capability-discovery interface implemented by @@ -187,6 +192,304 @@ func (r *runnerAdapter) JobLogs(ctx context.Context, handle interfaces.JobHandle } } +// ResourceDriverProvider is a capability-discovery interface implemented by +// *Adapter. Steps that need per-action CRUD type-assert the registered +// interfaces.IaCProvider to ResourceDriverProvider and call +// ResourceDriver(resourceType). The accessor is always present on *Adapter; +// absence of the underlying gRPC service is signalled by the returned +// ErrProviderMethodUnimplemented error (NOT a nil first return — when the +// service IS advertised the bridge is returned, and when it is NOT the error is +// returned with a nil driver). Callers use errors.Is to detect absence and skip +// or fall back. +type ResourceDriverProvider interface { + // ResourceDriver returns the per-resource-type CRUD driver for resourceType, + // or a nil driver wrapping ErrProviderMethodUnimplemented when the plugin did + // not advertise the ResourceDriver service. + ResourceDriver(resourceType string) (interfaces.ResourceDriver, error) +} + +// resourceDriverAdapter wraps pb.ResourceDriverClient and satisfies +// interfaces.ResourceDriver for a specific resource type. It is the concrete +// type underlying all objects returned by Adapter.ResourceDriver(). The +// resource_type field is carried on every RPC so the plugin can route to the +// correct per-type implementation (the DO plugin's 14-driver pattern). +type resourceDriverAdapter struct { + client pb.ResourceDriverClient + resourceType string +} + +// Create calls ResourceDriver.Create with JSON-encoded spec.Config. +func (r *resourceDriverAdapter) Create(ctx context.Context, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + pbSpec, err := specToPB(spec) + if err != nil { + return nil, fmt.Errorf("providerclient: encode Create spec: %w", err) + } + resp, err := r.client.Create(ctx, &pb.ResourceCreateRequest{ + ResourceType: r.resourceType, + Spec: pbSpec, + }) + if err != nil { + return nil, mapResourceDriverGRPCError(err, "Create") + } + return resourceOutputFromPB(resp.GetOutput()) +} + +// Read calls ResourceDriver.Read with the resource ref. +func (r *resourceDriverAdapter) Read(ctx context.Context, ref interfaces.ResourceRef) (*interfaces.ResourceOutput, error) { + resp, err := r.client.Read(ctx, &pb.ResourceReadRequest{ + ResourceType: r.resourceType, + Ref: refToPB(ref), + }) + if err != nil { + return nil, mapResourceDriverGRPCError(err, "Read") + } + return resourceOutputFromPB(resp.GetOutput()) +} + +// Update calls ResourceDriver.Update with the resource ref and desired spec. +func (r *resourceDriverAdapter) Update(ctx context.Context, ref interfaces.ResourceRef, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + pbSpec, err := specToPB(spec) + if err != nil { + return nil, fmt.Errorf("providerclient: encode Update spec: %w", err) + } + resp, err := r.client.Update(ctx, &pb.ResourceUpdateRequest{ + ResourceType: r.resourceType, + Ref: refToPB(ref), + Spec: pbSpec, + }) + if err != nil { + return nil, mapResourceDriverGRPCError(err, "Update") + } + return resourceOutputFromPB(resp.GetOutput()) +} + +// Delete calls ResourceDriver.Delete with the resource ref. +func (r *resourceDriverAdapter) Delete(ctx context.Context, ref interfaces.ResourceRef) error { + _, err := r.client.Delete(ctx, &pb.ResourceDeleteRequest{ + ResourceType: r.resourceType, + Ref: refToPB(ref), + }) + if err != nil { + return mapResourceDriverGRPCError(err, "Delete") + } + return nil +} + +// Diff calls ResourceDriver.Diff with the desired spec and current output. +func (r *resourceDriverAdapter) Diff(ctx context.Context, desired interfaces.ResourceSpec, current *interfaces.ResourceOutput) (*interfaces.DiffResult, error) { + pbSpec, err := specToPB(desired) + if err != nil { + return nil, fmt.Errorf("providerclient: encode Diff desired spec: %w", err) + } + pbCurrent, err := resourceOutputToPB(current) + if err != nil { + return nil, fmt.Errorf("providerclient: encode Diff current output: %w", err) + } + resp, err := r.client.Diff(ctx, &pb.ResourceDiffRequest{ + ResourceType: r.resourceType, + Desired: pbSpec, + Current: pbCurrent, + }) + if err != nil { + return nil, mapResourceDriverGRPCError(err, "Diff") + } + return diffResultFromPB(resp.GetResult()) +} + +// HealthCheck calls ResourceDriver.HealthCheck with the resource ref. +func (r *resourceDriverAdapter) HealthCheck(ctx context.Context, ref interfaces.ResourceRef) (*interfaces.HealthResult, error) { + resp, err := r.client.HealthCheck(ctx, &pb.ResourceHealthCheckRequest{ + ResourceType: r.resourceType, + Ref: refToPB(ref), + }) + if err != nil { + return nil, mapResourceDriverGRPCError(err, "HealthCheck") + } + return healthResultFromPB(resp.GetResult()), nil +} + +// Scale calls ResourceDriver.Scale with the resource ref and replica count. +func (r *resourceDriverAdapter) Scale(ctx context.Context, ref interfaces.ResourceRef, replicas int) (*interfaces.ResourceOutput, error) { + resp, err := r.client.Scale(ctx, &pb.ResourceScaleRequest{ + ResourceType: r.resourceType, + Ref: refToPB(ref), + Replicas: clampInt32(replicas), + }) + if err != nil { + return nil, mapResourceDriverGRPCError(err, "Scale") + } + return resourceOutputFromPB(resp.GetOutput()) +} + +// SensitiveKeys calls ResourceDriver.SensitiveKeys (no ctx — mirrors the +// interfaces.ResourceDriver contract which is a non-context accessor). +func (r *resourceDriverAdapter) SensitiveKeys() []string { + resp, err := r.client.SensitiveKeys(context.Background(), &pb.SensitiveKeysRequest{ + ResourceType: r.resourceType, + }) + if err != nil { + // Non-fatal — log and return empty; callers must tolerate nil/empty. + log.Printf("providerclient: ResourceDriver.SensitiveKeys(%s) RPC failed: %v", r.resourceType, err) + return nil + } + return append([]string(nil), resp.GetKeys()...) +} + +// Troubleshoot satisfies the optional interfaces.Troubleshooter. wfctl calls it +// (via troubleshootAfterFailure's driver.(interfaces.Troubleshooter) assert) when +// a health-check poll times out or a deploy returns a generic error, surfacing +// provider-side diagnostics. gRPC Unimplemented (the legitimate negative signal +// when the plugin's driver does not implement Troubleshoot) is translated to +// interfaces.ErrProviderMethodUnimplemented so callers can errors.Is and fall +// back to the original failure message. Mirrors typedResourceDriver.Troubleshoot. +func (r *resourceDriverAdapter) Troubleshoot(ctx context.Context, ref interfaces.ResourceRef, failureMsg string) ([]interfaces.Diagnostic, error) { + resp, err := r.client.Troubleshoot(ctx, &pb.TroubleshootRequest{ + ResourceType: r.resourceType, + Ref: refToPB(ref), + FailureMsg: failureMsg, + }) + if err != nil { + return nil, mapResourceDriverGRPCError(err, "Troubleshoot") + } + return diagnosticsFromPB(resp.GetDiagnostics()), nil +} + +// mapResourceDriverGRPCError translates well-known gRPC status codes to engine +// sentinel errors so callers can use errors.Is for typed identification. +// +// Each mapped case wraps with %w/%w (sentinel + original gRPC error), NOT %w/%v, +// so callers recover BOTH the interfaces sentinel via errors.Is AND the +// underlying gRPC status via status.FromError walking the unwrap chain (mirrors +// cmd/wfctl/iac_typed_adapter.go translateRPCErr's load-bearing comment). With +// %v the status code/details get demoted to a flat string and consumers that +// classify by code (rate-limit retry, transient backoff, etc.) lose the signal. +func mapResourceDriverGRPCError(err error, method string) error { + if err == nil { + return nil + } + switch status.Code(err) { + case codes.NotFound: + return fmt.Errorf("%w: ResourceDriver.%s: %w", interfaces.ErrResourceNotFound, method, err) + case codes.AlreadyExists: + return fmt.Errorf("%w: ResourceDriver.%s: %w", interfaces.ErrResourceAlreadyExists, method, err) + case codes.ResourceExhausted: + return fmt.Errorf("%w: ResourceDriver.%s: %w", interfaces.ErrRateLimited, method, err) + case codes.Unavailable, codes.DeadlineExceeded: + return fmt.Errorf("%w: ResourceDriver.%s: %w", interfaces.ErrTransient, method, err) + case codes.Unauthenticated: + return fmt.Errorf("%w: ResourceDriver.%s: %w", interfaces.ErrUnauthorized, method, err) + case codes.PermissionDenied: + return fmt.Errorf("%w: ResourceDriver.%s: %w", interfaces.ErrForbidden, method, err) + case codes.InvalidArgument, codes.FailedPrecondition: + return fmt.Errorf("%w: ResourceDriver.%s: %w", interfaces.ErrValidation, method, err) + case codes.Unimplemented: + return fmt.Errorf("%w: ResourceDriver.%s not implemented by plugin", interfaces.ErrProviderMethodUnimplemented, method) + default: + // Preserve attribution (method name) while keeping the gRPC status + // observable through the unwrap chain. + return fmt.Errorf("ResourceDriver.%s: %w", method, err) + } +} + +// resourceOutputFromPB converts a proto ResourceOutput to interfaces.ResourceOutput. +func resourceOutputFromPB(o *pb.ResourceOutput) (*interfaces.ResourceOutput, error) { + if o == nil { + return nil, nil + } + outputs, err := unmarshalJSONMap(o.GetOutputsJson()) + if err != nil { + return nil, fmt.Errorf("providerclient: unmarshal ResourceOutput.outputs_json: %w", err) + } + sensitive := make(map[string]bool, len(o.GetSensitive())) + for k, v := range o.GetSensitive() { + sensitive[k] = v + } + return &interfaces.ResourceOutput{ + Name: o.GetName(), + Type: o.GetType(), + ProviderID: o.GetProviderId(), + Outputs: outputs, + Sensitive: sensitive, + Status: o.GetStatus(), + }, nil +} + +// resourceOutputToPB converts an interfaces.ResourceOutput to proto ResourceOutput. +func resourceOutputToPB(o *interfaces.ResourceOutput) (*pb.ResourceOutput, error) { + if o == nil { + return nil, nil + } + outputsJSON, err := marshalJSONMap(o.Outputs) + if err != nil { + return nil, err + } + sensitive := make(map[string]bool, len(o.Sensitive)) + for k, v := range o.Sensitive { + sensitive[k] = v + } + return &pb.ResourceOutput{ + Name: o.Name, + Type: o.Type, + ProviderId: o.ProviderID, + OutputsJson: outputsJSON, + Sensitive: sensitive, + Status: o.Status, + }, nil +} + +// diffResultFromPB converts a proto DiffResult to interfaces.DiffResult. +func diffResultFromPB(d *pb.DiffResult) (*interfaces.DiffResult, error) { + if d == nil { + return nil, nil + } + changes, err := changesFromPB(d.GetChanges()) + if err != nil { + return nil, err + } + return &interfaces.DiffResult{ + NeedsUpdate: d.GetNeedsUpdate(), + NeedsReplace: d.GetNeedsReplace(), + Changes: changes, + }, nil +} + +// healthResultFromPB converts a proto HealthResult to interfaces.HealthResult. +func healthResultFromPB(h *pb.HealthResult) *interfaces.HealthResult { + if h == nil { + return nil + } + return &interfaces.HealthResult{ + Healthy: h.GetHealthy(), + Message: h.GetMessage(), + } +} + +// diagnosticsFromPB converts []*pb.Diagnostic to []interfaces.Diagnostic +// (Troubleshooter results). Mirrors typedResourceDriver.Troubleshoot's mapping. +func diagnosticsFromPB(diags []*pb.Diagnostic) []interfaces.Diagnostic { + out := make([]interfaces.Diagnostic, 0, len(diags)) + for _, d := range diags { + out = append(out, interfaces.Diagnostic{ + ID: d.GetId(), + Phase: d.GetPhase(), + Cause: d.GetCause(), + At: timeFromPB(d.GetAt()), + Detail: d.GetDetail(), + }) + } + return out +} + +// Compile-time guards: resourceDriverAdapter must satisfy interfaces.ResourceDriver +// (the required per-action CRUD surface) AND interfaces.Troubleshooter (the +// optional diagnostics surface troubleshootAfterFailure type-asserts). Mirrors +// the typedResourceDriver guards in cmd/wfctl/iac_typed_adapter.go; they catch +// signature drift between the interface and this bridge at compile time. +var ( + _ interfaces.ResourceDriver = (*resourceDriverAdapter)(nil) + _ interfaces.Troubleshooter = (*resourceDriverAdapter)(nil) +) + // Adapter wraps the pb.IaCProviderRequired gRPC client (and advertisement-gated // optional clients) as interfaces.IaCProvider. Optional sub-interfaces // (IaCProviderRegionLister, DriftConfigDetector) are exposed via typed accessors @@ -202,11 +505,12 @@ func (r *runnerAdapter) JobLogs(ctx context.Context, handle interfaces.JobHandle // // Compile-time guards are in adapter_test.go. type Adapter struct { - conn grpc.ClientConnInterface - required pb.IaCProviderRequiredClient - regionLister *regionListerImpl // nil when IaCServiceRegionLister not advertised - drift *driftDetectorAdapter // nil when IaCServiceDriftDetector not advertised - runner *runnerAdapter // nil when IaCServiceRunner not advertised + conn grpc.ClientConnInterface + required pb.IaCProviderRequiredClient + regionLister *regionListerImpl // nil when IaCServiceRegionLister not advertised + drift *driftDetectorAdapter // nil when IaCServiceDriftDetector not advertised + runner *runnerAdapter // nil when IaCServiceRunner not advertised + resourceDriver pb.ResourceDriverClient // nil when IaCServiceResourceDriver not advertised // Capabilities cache. Populated on first call to fetchCapabilities via // capsOnce; reused for the adapter's lifetime (capabilities don't change @@ -264,6 +568,9 @@ func New(conn grpc.ClientConnInterface, advertisedServices map[string]bool) *Ada if advertisedServices[IaCServiceRunner] { a.runner = &runnerAdapter{client: pb.NewIaCProviderRunnerClient(conn)} } + if advertisedServices[IaCServiceResourceDriver] { + a.resourceDriver = pb.NewResourceDriverClient(conn) + } return a } @@ -441,12 +748,24 @@ func (a *Adapter) ResolveSizing(resourceType string, size interfaces.Size, hints return sizingFromPB(resp.GetSizing()) } -// ResourceDriver returns an error — the full ResourceDriver optional service is -// not in PR-1 scope. Steps that need per-action CRUD use wfctlhelpers.ApplyPlanWithHooks -// which dispatches through the provider's ResourceDriver on the plugin side. +// ResourceDriver returns a per-resource-type driver bridged over the optional +// ResourceDriver gRPC service. When the plugin advertised IaCServiceResourceDriver, +// the returned driver routes Create/Read/Update/Delete/Diff/HealthCheck/Scale/ +// SensitiveKeys through the plugin's gRPC process carrying resource_type on every +// RPC (the plugin dispatches internally to the correct per-type driver, mirroring +// the DO plugin's 14-driver type-routing pattern). When the service was not +// advertised, returns ErrProviderMethodUnimplemented so callers can errors.Is-check +// and skip or fall back appropriately — matching the DetectDrift/RegionLister +// precedent for other optional services. func (a *Adapter) ResourceDriver(resourceType string) (interfaces.ResourceDriver, error) { - return nil, fmt.Errorf("%w: ResourceDriver optional service not wired in PR-1 adapter", - interfaces.ErrProviderMethodUnimplemented) + if a.resourceDriver == nil { + return nil, fmt.Errorf("%w: ResourceDriver service not advertised by plugin", + interfaces.ErrProviderMethodUnimplemented) + } + return &resourceDriverAdapter{ + client: a.resourceDriver, + resourceType: resourceType, + }, nil } // SupportedCanonicalKeys returns the canonical keys from the cached diff --git a/iac/providerclient/adapter_test.go b/iac/providerclient/adapter_test.go index 639515ab6..f2a701587 100644 --- a/iac/providerclient/adapter_test.go +++ b/iac/providerclient/adapter_test.go @@ -3,6 +3,7 @@ package providerclient_test import ( "context" "encoding/json" + "errors" "net" "testing" @@ -531,6 +532,387 @@ func TestAdapter_Runner_Unadvertised(t *testing.T) { } } +// ─── ResourceDriver advertisement-gating ───────────────────────────────────── + +// fakeResourceDriverServer implements ResourceDriverServer for unit tests. +// It captures the last request for assertion and returns canned responses. +type fakeResourceDriverServer struct { + pb.UnimplementedResourceDriverServer + + lastCreateReq *pb.ResourceCreateRequest + lastUpdateReq *pb.ResourceUpdateRequest + lastDeleteReq *pb.ResourceDeleteRequest + lastReadReq *pb.ResourceReadRequest + lastDiffReq *pb.ResourceDiffRequest + lastHealthReq *pb.ResourceHealthCheckRequest + lastScaleReq *pb.ResourceScaleRequest + lastSensitiveReq *pb.SensitiveKeysRequest + lastTroubleshootReq *pb.TroubleshootRequest +} + +func (s *fakeResourceDriverServer) Create(_ context.Context, req *pb.ResourceCreateRequest) (*pb.ResourceCreateResponse, error) { + s.lastCreateReq = req + outputsJSON, _ := json.Marshal(map[string]any{"endpoint": "db.example.com", "port": float64(5432)}) + return &pb.ResourceCreateResponse{ + Output: &pb.ResourceOutput{ + Name: req.GetSpec().GetName(), + Type: req.GetResourceType(), + ProviderId: "prov-123", + OutputsJson: outputsJSON, + Sensitive: map[string]bool{"password": true}, + Status: "active", + }, + }, nil +} + +func (s *fakeResourceDriverServer) Read(_ context.Context, req *pb.ResourceReadRequest) (*pb.ResourceReadResponse, error) { + s.lastReadReq = req + outputsJSON, _ := json.Marshal(map[string]any{"endpoint": "db.example.com"}) + return &pb.ResourceReadResponse{ + Output: &pb.ResourceOutput{ + Name: req.GetRef().GetName(), + Type: req.GetResourceType(), + ProviderId: req.GetRef().GetProviderId(), + OutputsJson: outputsJSON, + Status: "active", + }, + }, nil +} + +func (s *fakeResourceDriverServer) Update(_ context.Context, req *pb.ResourceUpdateRequest) (*pb.ResourceUpdateResponse, error) { + s.lastUpdateReq = req + outputsJSON, _ := json.Marshal(map[string]any{"endpoint": "db.example.com"}) + return &pb.ResourceUpdateResponse{ + Output: &pb.ResourceOutput{ + Name: req.GetRef().GetName(), + Type: req.GetResourceType(), + ProviderId: req.GetRef().GetProviderId(), + OutputsJson: outputsJSON, + Status: "active", + }, + }, nil +} + +func (s *fakeResourceDriverServer) Delete(_ context.Context, req *pb.ResourceDeleteRequest) (*pb.ResourceDeleteResponse, error) { + s.lastDeleteReq = req + return &pb.ResourceDeleteResponse{}, nil +} + +func (s *fakeResourceDriverServer) Diff(_ context.Context, req *pb.ResourceDiffRequest) (*pb.ResourceDiffResponse, error) { + s.lastDiffReq = req + oldJSON, _ := json.Marshal("t3.micro") + newJSON, _ := json.Marshal("t3.large") + return &pb.ResourceDiffResponse{ + Result: &pb.DiffResult{ + NeedsUpdate: true, + Changes: []*pb.FieldChange{ + {Path: "size", OldJson: oldJSON, NewJson: newJSON, ForceNew: false}, + }, + }, + }, nil +} + +func (s *fakeResourceDriverServer) HealthCheck(_ context.Context, req *pb.ResourceHealthCheckRequest) (*pb.ResourceHealthCheckResponse, error) { + s.lastHealthReq = req + return &pb.ResourceHealthCheckResponse{ + Result: &pb.HealthResult{Healthy: true, Message: "all systems nominal"}, + }, nil +} + +func (s *fakeResourceDriverServer) Scale(_ context.Context, req *pb.ResourceScaleRequest) (*pb.ResourceScaleResponse, error) { + s.lastScaleReq = req + outputsJSON, _ := json.Marshal(map[string]any{"replicas": float64(req.GetReplicas())}) + return &pb.ResourceScaleResponse{ + Output: &pb.ResourceOutput{ + Name: req.GetRef().GetName(), + Type: req.GetResourceType(), + ProviderId: req.GetRef().GetProviderId(), + OutputsJson: outputsJSON, + Status: "active", + }, + }, nil +} + +func (s *fakeResourceDriverServer) SensitiveKeys(_ context.Context, req *pb.SensitiveKeysRequest) (*pb.SensitiveKeysResponse, error) { + s.lastSensitiveReq = req + return &pb.SensitiveKeysResponse{Keys: []string{"password", "api_key"}}, nil +} + +func (s *fakeResourceDriverServer) Troubleshoot(_ context.Context, req *pb.TroubleshootRequest) (*pb.TroubleshootResponse, error) { + s.lastTroubleshootReq = req + return &pb.TroubleshootResponse{ + Diagnostics: []*pb.Diagnostic{ + {Id: "deploy-7", Phase: "ERROR", Cause: "image pull backoff", Detail: "manifest unknown"}, + }, + }, nil +} + +// TestAdapter_ResourceDriver_WiredWhenAdvertised verifies end-to-end CRUD +// round-trip when IaCServiceResourceDriver is in advertisedServices. +func TestAdapter_ResourceDriver_WiredWhenAdvertised(t *testing.T) { + srv := grpc.NewServer() + pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) + rdServer := &fakeResourceDriverServer{} + pb.RegisterResourceDriverServer(srv, rdServer) + conn := startFakeServer(t, srv) + + a := providerclient.New(conn, map[string]bool{ + providerclient.IaCServiceResourceDriver: true, + }) + + // ResourceDriverProvider accessor. + rdp, ok := any(a).(providerclient.ResourceDriverProvider) + if !ok { + t.Fatal("*Adapter must satisfy ResourceDriverProvider when IaCServiceResourceDriver advertised") + } + + driver, err := rdp.ResourceDriver("stub.database") + if err != nil { + t.Fatalf("ResourceDriver() returned error: %v", err) + } + if driver == nil { + t.Fatal("ResourceDriver() returned nil driver when service was advertised") + } + + ctx := context.Background() + + // ── Create ────────────────────────────────────────────────────────────── + configMap := map[string]any{"instance_class": "db.t3.medium", "engine": "postgres"} + spec := interfaces.ResourceSpec{Name: "mydb", Type: "stub.database", Config: configMap} + out, err := driver.Create(ctx, spec) + if err != nil { + t.Fatalf("Create() returned error: %v", err) + } + if out == nil { + t.Fatal("Create() returned nil output") + } + if out.ProviderID != "prov-123" { + t.Errorf("Create() ProviderID = %q, want %q", out.ProviderID, "prov-123") + } + if out.Outputs["endpoint"] != "db.example.com" { + t.Errorf("Create() Outputs[endpoint] = %v, want db.example.com", out.Outputs["endpoint"]) + } + if !out.Sensitive["password"] { + t.Error("Create() Sensitive[password] should be true") + } + // Assert that Config JSON reached the server. + if rdServer.lastCreateReq == nil { + t.Fatal("Create() did not reach server") + } + if rdServer.lastCreateReq.GetResourceType() != "stub.database" { + t.Errorf("server saw resource_type = %q, want %q", rdServer.lastCreateReq.GetResourceType(), "stub.database") + } + var serverCfg map[string]any + if err := json.Unmarshal(rdServer.lastCreateReq.GetSpec().GetConfigJson(), &serverCfg); err != nil { + t.Fatalf("server config_json not valid JSON: %v", err) + } + if serverCfg["engine"] != "postgres" { + t.Errorf("server config engine = %v, want postgres", serverCfg["engine"]) + } + + // ── Read ───────────────────────────────────────────────────────────────── + ref := interfaces.ResourceRef{Name: "mydb", Type: "stub.database", ProviderID: "prov-123"} + readOut, err := driver.Read(ctx, ref) + if err != nil { + t.Fatalf("Read() returned error: %v", err) + } + if readOut == nil { + t.Fatal("Read() returned nil output") + } + if rdServer.lastReadReq.GetResourceType() != "stub.database" { + t.Errorf("Read server resource_type = %q, want stub.database", rdServer.lastReadReq.GetResourceType()) + } + + // ── Update ─────────────────────────────────────────────────────────────── + updSpec := interfaces.ResourceSpec{Name: "mydb", Type: "stub.database", Config: map[string]any{"instance_class": "db.t3.large"}} + updateOut, err := driver.Update(ctx, ref, updSpec) + if err != nil { + t.Fatalf("Update() returned error: %v", err) + } + if updateOut == nil { + t.Fatal("Update() returned nil output") + } + if rdServer.lastUpdateReq.GetRef().GetProviderId() != "prov-123" { + t.Errorf("Update server ref provider_id = %q, want prov-123", rdServer.lastUpdateReq.GetRef().GetProviderId()) + } + + // ── Delete ─────────────────────────────────────────────────────────────── + if err := driver.Delete(ctx, ref); err != nil { + t.Fatalf("Delete() returned error: %v", err) + } + if rdServer.lastDeleteReq.GetResourceType() != "stub.database" { + t.Errorf("Delete server resource_type = %q, want stub.database", rdServer.lastDeleteReq.GetResourceType()) + } + + // ── Diff ───────────────────────────────────────────────────────────────── + diffResult, err := driver.Diff(ctx, spec, out) + if err != nil { + t.Fatalf("Diff() returned error: %v", err) + } + if diffResult == nil { + t.Fatal("Diff() returned nil result") + } + if !diffResult.NeedsUpdate { + t.Error("Diff() NeedsUpdate should be true") + } + if len(diffResult.Changes) != 1 || diffResult.Changes[0].Path != "size" { + t.Errorf("Diff() Changes = %+v, want 1 change with path=size", diffResult.Changes) + } + + // ── HealthCheck ───────────────────────────────────────────────────────── + health, err := driver.HealthCheck(ctx, ref) + if err != nil { + t.Fatalf("HealthCheck() returned error: %v", err) + } + if health == nil || !health.Healthy { + t.Errorf("HealthCheck() = %+v, want Healthy=true", health) + } + if health.Message != "all systems nominal" { + t.Errorf("HealthCheck() message = %q, want %q", health.Message, "all systems nominal") + } + + // ── Scale ──────────────────────────────────────────────────────────────── + scaleOut, err := driver.Scale(ctx, ref, 3) + if err != nil { + t.Fatalf("Scale() returned error: %v", err) + } + if scaleOut == nil { + t.Fatal("Scale() returned nil output") + } + if rdServer.lastScaleReq.GetReplicas() != 3 { + t.Errorf("Scale server replicas = %d, want 3", rdServer.lastScaleReq.GetReplicas()) + } + + // ── SensitiveKeys ──────────────────────────────────────────────────────── + keys := driver.SensitiveKeys() + if len(keys) != 2 { + t.Errorf("SensitiveKeys() = %v, want [password api_key]", keys) + } + if rdServer.lastSensitiveReq.GetResourceType() != "stub.database" { + t.Errorf("SensitiveKeys server resource_type = %q, want stub.database", rdServer.lastSensitiveReq.GetResourceType()) + } + + // ── Troubleshoot (optional interfaces.Troubleshooter) ──────────────────── + troubleshooter, ok := driver.(interfaces.Troubleshooter) + if !ok { + t.Fatal("ResourceDriver bridge must satisfy interfaces.Troubleshooter") + } + diags, err := troubleshooter.Troubleshoot(ctx, ref, "health check timed out") + if err != nil { + t.Fatalf("Troubleshoot() returned error: %v", err) + } + if len(diags) != 1 || diags[0].ID != "deploy-7" || diags[0].Cause != "image pull backoff" { + t.Errorf("Troubleshoot() = %+v, want 1 diagnostic id=deploy-7 cause='image pull backoff'", diags) + } + if rdServer.lastTroubleshootReq.GetResourceType() != "stub.database" { + t.Errorf("Troubleshoot server resource_type = %q, want stub.database", rdServer.lastTroubleshootReq.GetResourceType()) + } + if rdServer.lastTroubleshootReq.GetFailureMsg() != "health check timed out" { + t.Errorf("Troubleshoot server failure_msg = %q, want %q", rdServer.lastTroubleshootReq.GetFailureMsg(), "health check timed out") + } +} + +// TestAdapter_ResourceDriver_UnimplementedWhenNotAdvertised verifies that when +// the ResourceDriver service is not in advertisedServices, ResourceDriver() +// returns ErrProviderMethodUnimplemented. +func TestAdapter_ResourceDriver_UnimplementedWhenNotAdvertised(t *testing.T) { + srv := grpc.NewServer() + pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) + // NOTE: ResourceDriver is deliberately NOT registered on the server. + conn := startFakeServer(t, srv) + + a := providerclient.New(conn, nil) + + // ResourceDriverProvider accessor must always be present on *Adapter. + rdp, ok := any(a).(providerclient.ResourceDriverProvider) + if !ok { + t.Fatal("*Adapter must satisfy ResourceDriverProvider regardless of advertisement") + } + + _, err := rdp.ResourceDriver("stub.database") + if err == nil { + t.Fatal("ResourceDriver() on unadvertised adapter must return error") + } + if !isUnimplemented(err) { + t.Errorf("ResourceDriver() error = %v, want ErrProviderMethodUnimplemented", err) + } +} + +// codeReturningResourceDriverServer returns a configurable gRPC status code from +// its Read RPC so the error-mapping table test can drive each named case. +type codeReturningResourceDriverServer struct { + pb.UnimplementedResourceDriverServer + code codes.Code +} + +func (s *codeReturningResourceDriverServer) Read(_ context.Context, _ *pb.ResourceReadRequest) (*pb.ResourceReadResponse, error) { + return nil, status.Error(s.code, "stub error from server") +} + +// TestAdapter_ResourceDriver_GRPCErrorMapping verifies that each well-known gRPC +// status code returned by a ResourceDriver RPC is mapped to the correct engine +// sentinel (recoverable via errors.Is) AND that the underlying gRPC status code +// remains recoverable via status.Code walking the unwrap chain (the %w/%w fix — +// without it status.Code would degrade to Unknown). The AlreadyExists → upsert +// recovery path (wfctlhelpers/apply.go errors.Is(err, ErrResourceAlreadyExists)) +// is load-bearing. +func TestAdapter_ResourceDriver_GRPCErrorMapping(t *testing.T) { + cases := []struct { + name string + code codes.Code + wantSentinel error + }{ + {"NotFound", codes.NotFound, interfaces.ErrResourceNotFound}, + {"AlreadyExists", codes.AlreadyExists, interfaces.ErrResourceAlreadyExists}, + {"ResourceExhausted", codes.ResourceExhausted, interfaces.ErrRateLimited}, + {"Unavailable", codes.Unavailable, interfaces.ErrTransient}, + {"DeadlineExceeded", codes.DeadlineExceeded, interfaces.ErrTransient}, + {"Unauthenticated", codes.Unauthenticated, interfaces.ErrUnauthorized}, + {"PermissionDenied", codes.PermissionDenied, interfaces.ErrForbidden}, + {"InvalidArgument", codes.InvalidArgument, interfaces.ErrValidation}, + {"FailedPrecondition", codes.FailedPrecondition, interfaces.ErrValidation}, + {"Unimplemented", codes.Unimplemented, interfaces.ErrProviderMethodUnimplemented}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + srv := grpc.NewServer() + pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) + pb.RegisterResourceDriverServer(srv, &codeReturningResourceDriverServer{code: tc.code}) + conn := startFakeServer(t, srv) + + a := providerclient.New(conn, map[string]bool{ + providerclient.IaCServiceResourceDriver: true, + }) + rdp := any(a).(providerclient.ResourceDriverProvider) + driver, err := rdp.ResourceDriver("stub.database") + if err != nil { + t.Fatalf("ResourceDriver() returned error: %v", err) + } + + _, err = driver.Read(context.Background(), interfaces.ResourceRef{Name: "x", Type: "stub.database", ProviderID: "id-1"}) + if err == nil { + t.Fatalf("expected error for code %v", tc.code) + } + + // 1. Sentinel must be recoverable via errors.Is. + if !errors.Is(err, tc.wantSentinel) { + t.Errorf("code %v: errors.Is(err, %v) = false; err = %v", tc.code, tc.wantSentinel, err) + } + + // 2. The underlying gRPC status code must STILL be recoverable from the + // unwrap chain (the %w/%w fix). For Unimplemented the original error + // is intentionally NOT chained (sentinel-only message), so skip the + // status-code recovery assertion for that case. + if tc.code != codes.Unimplemented { + if got := status.Code(err); got != tc.code { + t.Errorf("code %v: status.Code(err) = %v, want %v (status lost from unwrap chain — %%w/%%v regression?)", tc.code, got, tc.code) + } + } + }) + } +} + type capturingLogSink struct { data []byte eof bool diff --git a/plugin/external/adapter.go b/plugin/external/adapter.go index c7074d547..a0a7c1c9d 100644 --- a/plugin/external/adapter.go +++ b/plugin/external/adapter.go @@ -645,7 +645,10 @@ func (a *ExternalPluginAdapter) advertisedOptionalIaCServices() map[string]bool continue } switch d.ServiceName { - case providerclient.IaCServiceRegionLister, providerclient.IaCServiceDriftDetector: + case providerclient.IaCServiceRegionLister, + providerclient.IaCServiceDriftDetector, + providerclient.IaCServiceRunner, + providerclient.IaCServiceResourceDriver: out[d.ServiceName] = true } } @@ -666,11 +669,12 @@ func (a *ExternalPluginAdapter) advertisedOptionalIaCServices() map[string]bool // (no wiring needed — not an iac.provider plugin). // // Optional services (IaCProviderRegionLister, IaCProviderDriftDetector, -// IaCProviderRunner) are collected from the ContractRegistry and forwarded to -// providerclient.New. +// IaCProviderRunner, ResourceDriver) are collected from the ContractRegistry by +// advertisedOptionalIaCServices and forwarded to providerclient.New. // Optional gRPC clients are constructed only for advertised services, so the -// adapter's capability accessors (RegionLister(), DriftDetector(), Runner()) -// return nil for unadvertised services — ensuring fallback paths remain reachable. +// adapter's capability accessors (RegionLister(), DriftDetector(), Runner(), +// ResourceDriver()) return nil / ErrProviderMethodUnimplemented for unadvertised +// services — ensuring fallback paths remain reachable. func (a *ExternalPluginAdapter) WiringHooks() []plugin.WiringHook { if !a.advertisesIaCProviderRequiredService() { return nil diff --git a/plugin/external/iac_provider_wiring_test.go b/plugin/external/iac_provider_wiring_test.go index d05af57aa..194b4da9e 100644 --- a/plugin/external/iac_provider_wiring_test.go +++ b/plugin/external/iac_provider_wiring_test.go @@ -12,6 +12,7 @@ package external import ( "context" + "encoding/json" "net" "testing" @@ -76,14 +77,59 @@ func (s *minimalIaCProviderServer) BootstrapStateBackend(_ context.Context, _ *p return nil, status.Error(codes.Unimplemented, "not needed in test") } +// minimalResourceDriverServer implements just enough of ResourceDriverServer for +// the end-to-end wiring assertion: a Create call must reach the plugin and return +// a canned ResourceOutput, proving the WiringHook constructed a live driver +// bridge (not the unimplemented stub). It echoes the received spec.config_json +// back into the output's outputs_json under "received_config" so the test can +// assert the Config JSON round-tripped across the gRPC boundary. +type minimalResourceDriverServer struct { + pb.UnimplementedResourceDriverServer +} + +func (s *minimalResourceDriverServer) Create(_ context.Context, req *pb.ResourceCreateRequest) (*pb.ResourceCreateResponse, error) { + // Decode the inbound config_json and echo it back so the caller can verify + // the spec.Config survived marshaling across the wire. + var cfg map[string]any + if raw := req.GetSpec().GetConfigJson(); len(raw) > 0 { + if err := json.Unmarshal(raw, &cfg); err != nil { + return nil, status.Errorf(codes.InvalidArgument, "config_json not valid JSON: %v", err) + } + } + outputsJSON, err := json.Marshal(map[string]any{"received_config": cfg}) + if err != nil { + return nil, status.Errorf(codes.Internal, "marshal outputs: %v", err) + } + return &pb.ResourceCreateResponse{ + Output: &pb.ResourceOutput{ + Name: req.GetSpec().GetName(), + Type: req.GetResourceType(), + ProviderId: "wired-prov-1", + OutputsJson: outputsJSON, + Status: "active", + }, + }, nil +} + // startIaCProviderServer starts an in-process gRPC server with the required // IaCProviderRequired service registered. Returns a *grpc.ClientConn. func startIaCProviderServer(t *testing.T) *grpc.ClientConn { + t.Helper() + return startIaCProviderServerWith(t, false) +} + +// startIaCProviderServerWith starts an in-process gRPC server with the required +// IaCProviderRequired service registered, optionally also registering the +// ResourceDriver service. Returns a *grpc.ClientConn. +func startIaCProviderServerWith(t *testing.T, withResourceDriver bool) *grpc.ClientConn { t.Helper() lis := bufconn.Listen(4 << 20) t.Cleanup(func() { _ = lis.Close() }) srv := grpc.NewServer() pb.RegisterIaCProviderRequiredServer(srv, &minimalIaCProviderServer{}) + if withResourceDriver { + pb.RegisterResourceDriverServer(srv, &minimalResourceDriverServer{}) + } go func() { _ = srv.Serve(lis) }() t.Cleanup(srv.Stop) @@ -104,23 +150,39 @@ func startIaCProviderServer(t *testing.T) *grpc.ClientConn { // gRPC connection to the fake server. Uses package-internal access to // construct the adapter without a full go-plugin subprocess. func newIaCProviderAdapter(t *testing.T, pluginName string, conn *grpc.ClientConn) *ExternalPluginAdapter { + t.Helper() + return newIaCProviderAdapterWithOptional(t, pluginName, conn) +} + +// newIaCProviderAdapterWithOptional builds an ExternalPluginAdapter that +// advertises IaCProviderRequired plus any extra optional service names supplied, +// each as a CONTRACT_KIND_SERVICE descriptor in the ContractRegistry. This lets +// tests drive advertisedOptionalIaCServices() and the end-to-end WiringHook path +// for optional services (ResourceDriver, Runner, etc.). +func newIaCProviderAdapterWithOptional(t *testing.T, pluginName string, conn *grpc.ClientConn, optionalServiceNames ...string) *ExternalPluginAdapter { t.Helper() iacServiceName := pb.IaCProviderRequired_ServiceDesc.ServiceName diskManifest := &plugin.PluginManifest{ Name: pluginName, Version: "1.0.0", - IaCServices: []string{iacServiceName}, - } - // Build a ContractRegistry advertising the IaCProviderRequired service. - registry := &pb.ContractRegistry{ - Contracts: []*pb.ContractDescriptor{ - { - Kind: pb.ContractKind_CONTRACT_KIND_SERVICE, - ServiceName: iacServiceName, - Method: "Plan", // representative method - }, + IaCServices: append([]string{iacServiceName}, optionalServiceNames...), + } + // Build a ContractRegistry advertising the IaCProviderRequired service plus + // any optional services. + contracts := []*pb.ContractDescriptor{ + { + Kind: pb.ContractKind_CONTRACT_KIND_SERVICE, + ServiceName: iacServiceName, + Method: "Plan", // representative method }, } + for _, name := range optionalServiceNames { + contracts = append(contracts, &pb.ContractDescriptor{ + Kind: pb.ContractKind_CONTRACT_KIND_SERVICE, + ServiceName: name, + }) + } + registry := &pb.ContractRegistry{Contracts: contracts} a := &ExternalPluginAdapter{ name: pluginName, manifest: &pb.Manifest{Name: pluginName, Version: "1.0.0"}, @@ -242,6 +304,132 @@ func TestWiringHook_TwoPlugins_RegisterUnderDistinctNames(t *testing.T) { } } +// TestAdvertisedOptionalIaCServices_ResourceDriver asserts that a ContractRegistry +// advertising the ResourceDriver service is forwarded by advertisedOptionalIaCServices, +// and that a registry WITHOUT it excludes it. This is the gap PR13 left open: the +// adapter wired ResourceDriver, but the engine never advertised it, so it stayed nil. +func TestAdvertisedOptionalIaCServices_ResourceDriver(t *testing.T) { + conn := startIaCProviderServer(t) + + // WITH ResourceDriver advertised → present in the map. + withRD := newIaCProviderAdapterWithOptional(t, "rd-provider", conn, providerclient.IaCServiceResourceDriver) + got := withRD.advertisedOptionalIaCServices() + if !got[providerclient.IaCServiceResourceDriver] { + t.Errorf("advertisedOptionalIaCServices() = %v, want IaCServiceResourceDriver=true", got) + } + + // WITHOUT ResourceDriver advertised → absent from the map. + withoutRD := newIaCProviderAdapter(t, "plain-provider", conn) + got2 := withoutRD.advertisedOptionalIaCServices() + if got2[providerclient.IaCServiceResourceDriver] { + t.Errorf("advertisedOptionalIaCServices() = %v, want IaCServiceResourceDriver absent when unadvertised", got2) + } +} + +// TestAdvertisedOptionalIaCServices_Runner asserts the Runner service (added in +// #850) is likewise forwarded when advertised — the switch previously omitted it +// despite the doc comment claiming Runner was collected. +func TestAdvertisedOptionalIaCServices_Runner(t *testing.T) { + conn := startIaCProviderServer(t) + + withRunner := newIaCProviderAdapterWithOptional(t, "runner-provider", conn, providerclient.IaCServiceRunner) + got := withRunner.advertisedOptionalIaCServices() + if !got[providerclient.IaCServiceRunner] { + t.Errorf("advertisedOptionalIaCServices() = %v, want IaCServiceRunner=true", got) + } +} + +// TestWiringHook_ResourceDriver_WiredEndToEnd is the end-to-end assertion: a plugin +// that advertises ResourceDriver, run through the WiringHook, registers a +// *providerclient.Adapter whose ResourceDriver(type) returns the REAL bridge (not +// ErrProviderMethodUnimplemented), and a Create round-trips to the fake server. +// This closes the half-connected runtime path PR13's lead-verification found. +func TestWiringHook_ResourceDriver_WiredEndToEnd(t *testing.T) { + conn := startIaCProviderServerWith(t, true /* withResourceDriver */) + a := newIaCProviderAdapterWithOptional(t, "rd-provider", conn, providerclient.IaCServiceResourceDriver) + + hooks := a.WiringHooks() + if len(hooks) == 0 { + t.Fatal("expected at least one WiringHook") + } + + app := modular.NewStdApplication(modular.NewStdConfigProvider(nil), nil) + if err := hooks[0].Hook(app, nil); err != nil { + t.Fatalf("WiringHook.Hook returned error: %v", err) + } + + var p interfaces.IaCProvider + if err := app.GetService("rd-provider", &p); err != nil { + t.Fatalf("app.GetService(rd-provider): %v", err) + } + + // The registered provider must expose the ResourceDriver via the accessor. + rdp, ok := p.(providerclient.ResourceDriverProvider) + if !ok { + t.Fatalf("registered provider %T does not satisfy ResourceDriverProvider", p) + } + driver, err := rdp.ResourceDriver("stub.database") + if err != nil { + t.Fatalf("ResourceDriver() returned error (advertisement not forwarded?): %v", err) + } + if driver == nil { + t.Fatal("ResourceDriver() returned nil bridge despite advertisement") + } + + // Create must round-trip to the fake ResourceDriver server. + out, err := driver.Create(context.Background(), interfaces.ResourceSpec{ + Name: "mydb", + Type: "stub.database", + Config: map[string]any{"engine": "postgres"}, + }) + if err != nil { + t.Fatalf("Create() returned error: %v", err) + } + if out == nil || out.ProviderID != "wired-prov-1" { + t.Fatalf("Create() output = %+v, want ProviderID=wired-prov-1", out) + } + + // Assert the spec.Config JSON round-tripped across the gRPC boundary: the + // fake server echoes the decoded config back under "received_config". + received, ok := out.Outputs["received_config"].(map[string]any) + if !ok { + t.Fatalf("Create() output missing received_config echo; outputs = %+v", out.Outputs) + } + if received["engine"] != "postgres" { + t.Errorf("config_json round-trip: received_config[engine] = %v, want postgres", received["engine"]) + } +} + +// TestWiringHook_ResourceDriver_UnimplementedWhenUnadvertised asserts the negative +// path survives the fix: a plugin that does NOT advertise ResourceDriver still +// yields ErrProviderMethodUnimplemented from ResourceDriver(), so callers' skip/ +// fallback logic remains reachable. +func TestWiringHook_ResourceDriver_UnimplementedWhenUnadvertised(t *testing.T) { + conn := startIaCProviderServer(t) + a := newIaCProviderAdapter(t, "plain-provider", conn) + + hooks := a.WiringHooks() + if len(hooks) == 0 { + t.Fatal("expected at least one WiringHook") + } + app := modular.NewStdApplication(modular.NewStdConfigProvider(nil), nil) + if err := hooks[0].Hook(app, nil); err != nil { + t.Fatalf("WiringHook.Hook returned error: %v", err) + } + + var p interfaces.IaCProvider + if err := app.GetService("plain-provider", &p); err != nil { + t.Fatalf("app.GetService(plain-provider): %v", err) + } + rdp, ok := p.(providerclient.ResourceDriverProvider) + if !ok { + t.Fatalf("registered provider %T does not satisfy ResourceDriverProvider", p) + } + if _, err := rdp.ResourceDriver("stub.database"); err == nil { + t.Fatal("ResourceDriver() on unadvertised plugin must return error") + } +} + // TestWiringHook_IaCProviderPlugin_ReturnsHook and the functions below must // not reference the 'external' package (we're inside it). // Ensure the used import is live by using it in one test. From 5cd5f27d537ab801a6ee44c1432bbec2efff0bce Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 3 Jun 2026 04:04:40 -0400 Subject: [PATCH 2/2] fix(providerclient): Scale errors on out-of-int32 replicas + Unimplemented wraps gRPC err (Copilot) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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) --- iac/providerclient/adapter.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/iac/providerclient/adapter.go b/iac/providerclient/adapter.go index af1ee72d5..1764a3ff7 100644 --- a/iac/providerclient/adapter.go +++ b/iac/providerclient/adapter.go @@ -310,10 +310,16 @@ func (r *resourceDriverAdapter) HealthCheck(ctx context.Context, ref interfaces. // Scale calls ResourceDriver.Scale with the resource ref and replica count. func (r *resourceDriverAdapter) Scale(ctx context.Context, ref interfaces.ResourceRef, replicas int) (*interfaces.ResourceOutput, error) { + // Reject out-of-int32-range replica counts explicitly rather than silently + // saturating (mirrors cmd/wfctl typedResourceDriver.Scale) — a clamp could + // trigger an unintended scale operation. + if replicas < math.MinInt32 || replicas > math.MaxInt32 { + return nil, fmt.Errorf("providerclient %s: scale replicas %d out of int32 range", r.resourceType, replicas) + } resp, err := r.client.Scale(ctx, &pb.ResourceScaleRequest{ ResourceType: r.resourceType, Ref: refToPB(ref), - Replicas: clampInt32(replicas), + Replicas: int32(replicas), //nolint:gosec // G115: range-checked above }) if err != nil { return nil, mapResourceDriverGRPCError(err, "Scale") @@ -383,7 +389,7 @@ func mapResourceDriverGRPCError(err error, method string) error { case codes.InvalidArgument, codes.FailedPrecondition: return fmt.Errorf("%w: ResourceDriver.%s: %w", interfaces.ErrValidation, method, err) case codes.Unimplemented: - return fmt.Errorf("%w: ResourceDriver.%s not implemented by plugin", interfaces.ErrProviderMethodUnimplemented, method) + return fmt.Errorf("%w: ResourceDriver.%s not implemented by plugin: %w", interfaces.ErrProviderMethodUnimplemented, method, err) default: // Preserve attribution (method name) while keeping the gRPC status // observable through the unwrap chain.