From 7e6176d55e4ca66b1e3704ab64bebe90e76688f3 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 2 Jun 2026 02:04:04 -0400 Subject: [PATCH 1/4] feat(interfaces): add optional IaCProviderRegionLister 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 --- interfaces/iac_provider.go | 16 +++++++++++++ interfaces/iac_provider_test.go | 40 +++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/interfaces/iac_provider.go b/interfaces/iac_provider.go index 97f704c1..921071e7 100644 --- a/interfaces/iac_provider.go +++ b/interfaces/iac_provider.go @@ -368,3 +368,19 @@ type LogCaptureSink interface { type LogCaptureProvider interface { CaptureLogs(ctx context.Context, req LogCaptureRequest, sink LogCaptureSink) error } + +// IaCProviderRegionLister is an OPTIONAL interface that an IaCProvider +// implementation MAY also satisfy to expose provider-sourced region lists. +// Callers MUST type-assert against this interface and treat the negative case +// as a skip (static-catalog fallback) — providers that do not implement it +// continue to work unchanged. Mirrors the Enumerator/DriftDetector/ +// ProviderValidator optional-interface pattern. +// +// ListRegions returns the region identifiers available in the provider for the +// given environment name (used to scope region lists by env when providers +// expose env-scoped credentials). An empty env is treated as "all regions". +// Implementations SHOULD sort the returned slice; callers that need a stable +// order MUST sort themselves. +type IaCProviderRegionLister interface { + ListRegions(ctx context.Context, env string) ([]string, error) +} diff --git a/interfaces/iac_provider_test.go b/interfaces/iac_provider_test.go index 9f3f3e37..58ae809c 100644 --- a/interfaces/iac_provider_test.go +++ b/interfaces/iac_provider_test.go @@ -261,3 +261,43 @@ type fakeEnumeratorAll struct{} func (f *fakeEnumeratorAll) EnumerateAll(ctx context.Context, resourceType string) ([]*interfaces.ResourceOutput, error) { return nil, nil } + +// ─── IaCProviderRegionLister optional interface ─────────────────────────────── + +// regionListerProvider implements both IaCProvider and IaCProviderRegionLister. +type regionListerProvider struct{ nonValidatingProvider } + +func (r *regionListerProvider) ListRegions(_ context.Context, env string) ([]string, error) { + return []string{"us-east-1", "us-west-2"}, nil +} + +// Compile-time assertion: regionListerProvider satisfies IaCProviderRegionLister. +var _ interfaces.IaCProviderRegionLister = (*regionListerProvider)(nil) + +// TestIaCProviderRegionLister_ImplementorSatisfies verifies that a type +// implementing ListRegions(ctx, env) satisfies the new optional interface. +func TestIaCProviderRegionLister_ImplementorSatisfies(t *testing.T) { + var p interfaces.IaCProvider = ®ionListerProvider{} + + rl, ok := p.(interfaces.IaCProviderRegionLister) + if !ok { + t.Fatalf("regionListerProvider must satisfy IaCProviderRegionLister") + } + regions, err := rl.ListRegions(context.Background(), "prod") + if err != nil { + t.Fatalf("ListRegions returned unexpected error: %v", err) + } + if len(regions) == 0 { + t.Errorf("ListRegions returned no regions") + } +} + +// TestIaCProviderRegionLister_NonImplementorFails verifies the interface is +// truly optional: a provider that does NOT implement ListRegions must fail +// the type-assert so callers can gate accordingly. +func TestIaCProviderRegionLister_NonImplementorFails(t *testing.T) { + var p interfaces.IaCProvider = nonValidatingProvider{} + if _, ok := p.(interfaces.IaCProviderRegionLister); ok { + t.Errorf("nonValidatingProvider must NOT satisfy IaCProviderRegionLister (interface is optional)") + } +} From 30bc42452fd8650b1d11ade307824025768a7a75 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 2 Jun 2026 02:07:00 -0400 Subject: [PATCH 2/4] feat(iac/providerclient): gRPC-client adapter implementing interfaces.IaCProvider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- iac/providerclient/adapter.go | 578 +++++++++++++++++++++++++++++ iac/providerclient/adapter_test.go | 272 ++++++++++++++ 2 files changed, 850 insertions(+) create mode 100644 iac/providerclient/adapter.go create mode 100644 iac/providerclient/adapter_test.go diff --git a/iac/providerclient/adapter.go b/iac/providerclient/adapter.go new file mode 100644 index 00000000..46d9eb5f --- /dev/null +++ b/iac/providerclient/adapter.go @@ -0,0 +1,578 @@ +// Package providerclient provides a core-importable gRPC-client adapter that +// wraps the plugin/external/proto IaC gRPC services as interfaces.IaCProvider +// (+ optional sub-interfaces). It is the counterpart of the wfctl-only +// typedIaCAdapter (cmd/wfctl/iac_typed_adapter.go, package main), rebuilt in a +// core-importable package so the engine can register external iac.provider +// plugins as services via app.RegisterService — the foundational gap that v1/v1.1 +// of infra-admin silently lacked (design doc §Cycle 3 resolution C1). +// +// Usage (via WiringHook in plugin/external/adapter.go): +// +// adapter := providerclient.New(conn) +// app.RegisterService(moduleName, adapter) +// +// Steps resolve the provider via: +// +// app.GetService(cfg.Provider, &provider) // provider is interfaces.IaCProvider +package providerclient + +import ( + "context" + "encoding/json" + "fmt" + "log" + "time" + + "github.com/GoCodeAlone/workflow/interfaces" + pb "github.com/GoCodeAlone/workflow/plugin/external/proto" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "google.golang.org/protobuf/types/known/timestamppb" +) + +// Adapter wraps the pb.IaCProviderRequired gRPC client (and optional +// pb.IaCProviderRegionListerClient) as interfaces.IaCProvider + +// interfaces.IaCProviderRegionLister. The conn-backed optional clients are +// always constructed: the gRPC channel multiplexes them cheaply, and the +// optional-service guard is at the plugin's server side (Unimplemented). +// +// Compile-time guards are in adapter_test.go. +type Adapter struct { + conn grpc.ClientConnInterface + required pb.IaCProviderRequiredClient + regionLister pb.IaCProviderRegionListerClient +} + +// New constructs an Adapter over conn. Both the required and optional +// region-lister clients are constructed eagerly against the shared conn; +// the connection multiplexes them at zero marginal cost. If the plugin +// does not serve the optional service, calls to ListRegions return +// interfaces.ErrProviderMethodUnimplemented. +func New(conn grpc.ClientConnInterface) *Adapter { + return &Adapter{ + conn: conn, + required: pb.NewIaCProviderRequiredClient(conn), + regionLister: pb.NewIaCProviderRegionListerClient(conn), + } +} + +// ─── interfaces.IaCProvider ────────────────────────────────────────────────── + +// Name calls the IaCProviderRequired.Name RPC. Errors are logged and return "". +func (a *Adapter) Name() string { + resp, err := a.required.Name(context.Background(), &pb.NameRequest{}) + if err != nil { + log.Printf("providerclient: Name() RPC failed: %v", err) + return "" + } + return resp.GetName() +} + +// Version calls the IaCProviderRequired.Version RPC. Errors are logged and return "". +func (a *Adapter) Version() string { + resp, err := a.required.Version(context.Background(), &pb.VersionRequest{}) + if err != nil { + log.Printf("providerclient: Version() RPC failed: %v", err) + return "" + } + return resp.GetVersion() +} + +// Initialize calls the IaCProviderRequired.Initialize RPC. +func (a *Adapter) Initialize(ctx context.Context, cfg map[string]any) error { + cfgJSON, err := marshalJSONMap(cfg) + if err != nil { + return fmt.Errorf("providerclient: marshal Initialize config: %w", err) + } + _, err = a.required.Initialize(ctx, &pb.InitializeRequest{ConfigJson: cfgJSON}) + return err +} + +// Capabilities calls the IaCProviderRequired.Capabilities RPC and translates +// the response to []interfaces.IaCCapabilityDeclaration. +func (a *Adapter) Capabilities() []interfaces.IaCCapabilityDeclaration { + resp, err := a.required.Capabilities(context.Background(), &pb.CapabilitiesRequest{}) + if err != nil { + return nil + } + out := make([]interfaces.IaCCapabilityDeclaration, 0, len(resp.GetCapabilities())) + for _, c := range resp.GetCapabilities() { + out = append(out, interfaces.IaCCapabilityDeclaration{ + ResourceType: c.GetResourceType(), + Tier: int(c.GetTier()), + Operations: append([]string(nil), c.GetOperations()...), + }) + } + return out +} + +// Plan calls IaCProviderRequired.Plan and translates proto↔interfaces types. +func (a *Adapter) Plan(ctx context.Context, desired []interfaces.ResourceSpec, current []interfaces.ResourceState) (*interfaces.IaCPlan, error) { + pbDesired, err := specsToPB(desired) + if err != nil { + return nil, fmt.Errorf("providerclient: encode Plan desired: %w", err) + } + pbCurrent, err := statesToPB(current) + if err != nil { + return nil, fmt.Errorf("providerclient: encode Plan current: %w", err) + } + resp, err := a.required.Plan(ctx, &pb.PlanRequest{Desired: pbDesired, Current: pbCurrent}) + if err != nil { + return nil, err + } + return planFromPB(resp.GetPlan()) +} + +// Destroy calls IaCProviderRequired.Destroy and translates proto↔interfaces types. +func (a *Adapter) Destroy(ctx context.Context, resources []interfaces.ResourceRef) (*interfaces.DestroyResult, error) { + resp, err := a.required.Destroy(ctx, &pb.DestroyRequest{Refs: refsToPB(resources)}) + if err != nil { + return nil, err + } + return destroyResultFromPB(resp.GetResult()), nil +} + +// Status calls IaCProviderRequired.Status and translates proto↔interfaces types. +func (a *Adapter) Status(ctx context.Context, resources []interfaces.ResourceRef) ([]interfaces.ResourceStatus, error) { + resp, err := a.required.Status(ctx, &pb.StatusRequest{Refs: refsToPB(resources)}) + if err != nil { + return nil, err + } + return statusesFromPB(resp.GetStatuses()) +} + +// 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) +} + +// Import calls IaCProviderRequired.Import. +func (a *Adapter) Import(ctx context.Context, cloudID string, resourceType string) (*interfaces.ResourceState, error) { + resp, err := a.required.Import(ctx, &pb.ImportRequest{ProviderId: cloudID, ResourceType: resourceType}) + if err != nil { + return nil, err + } + return stateFromPB(resp.GetState()) +} + +// ResolveSizing calls IaCProviderRequired.ResolveSizing. +func (a *Adapter) ResolveSizing(resourceType string, size interfaces.Size, hints *interfaces.ResourceHints) (*interfaces.ProviderSizing, error) { + resp, err := a.required.ResolveSizing(context.Background(), &pb.ResolveSizingRequest{ + ResourceType: resourceType, + Size: string(size), + Hints: hintsToPB(hints), + }) + if err != nil { + return nil, err + } + 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. +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) +} + +// SupportedCanonicalKeys returns the keys from Capabilities, or the global +// canonical key set if the plugin doesn't declare a subset. +func (a *Adapter) SupportedCanonicalKeys() []string { + resp, err := a.required.Capabilities(context.Background(), &pb.CapabilitiesRequest{}) + if err == nil && resp != nil { + if keys := resp.GetCanonicalKeys(); len(keys) > 0 { + return append([]string(nil), keys...) + } + } + return interfaces.CanonicalKeys() +} + +// BootstrapStateBackend calls IaCProviderRequired.BootstrapStateBackend. +func (a *Adapter) BootstrapStateBackend(ctx context.Context, cfg map[string]any) (*interfaces.BootstrapResult, error) { + cfgJSON, err := marshalJSONMap(cfg) + if err != nil { + return nil, fmt.Errorf("providerclient: marshal BootstrapStateBackend cfg: %w", err) + } + resp, err := a.required.BootstrapStateBackend(ctx, &pb.BootstrapStateBackendRequest{ConfigJson: cfgJSON}) + if err != nil { + return nil, err + } + r := resp.GetResult() + if r == nil { + return nil, nil + } + return &interfaces.BootstrapResult{ + Bucket: r.GetBucket(), + Region: r.GetRegion(), + EnvVars: copyStringMap(r.GetEnvVars()), + }, nil +} + +// Close is a no-op on the adapter — the connection lifecycle is owned by the +// host's plugin manager (see ExternalPluginAdapter.Conn docs). +func (a *Adapter) Close() error { + return nil +} + +// ─── interfaces.IaCProviderRegionLister ───────────────────────────────────── + +// ListRegions calls the IaCProviderRegionLister.ListRegions RPC and returns +// region identifiers sorted by the server. If the plugin does not implement +// the service (gRPC Unimplemented) an ErrProviderMethodUnimplemented is returned +// so callers can fall back to a static catalog. +func (a *Adapter) ListRegions(ctx context.Context, env string) ([]string, error) { + resp, err := a.regionLister.ListRegions(ctx, &pb.ListRegionsRequest{EnvName: env}) + if err != nil { + if status.Code(err) == codes.Unimplemented { + return nil, fmt.Errorf("%w: IaCProviderRegionLister not registered by plugin", + interfaces.ErrProviderMethodUnimplemented) + } + return nil, err + } + regions := make([]string, 0, len(resp.GetRegions())) + for _, r := range resp.GetRegions() { + if name := r.GetName(); name != "" { + regions = append(regions, name) + } + } + return regions, nil +} + +// ─── Proto↔interfaces conversion helpers ──────────────────────────────────── +// +// These mirror the same-named functions in cmd/wfctl/iac_typed_adapter.go +// (package main, not importable by core). They are intentionally minimal — +// only the subset required by the PR-1 methods above (Plan, Status, Destroy, +// ListRegions). Additional converters can be added as PR-2 (step impl) needs +// them. Do NOT import cmd/wfctl. + +func marshalJSONMap(m map[string]any) ([]byte, error) { + if m == nil { + return nil, nil + } + return json.Marshal(m) +} + +func unmarshalJSONMap(b []byte) (map[string]any, error) { + if len(b) == 0 { + return nil, nil + } + var out map[string]any + if err := json.Unmarshal(b, &out); err != nil { + return nil, err + } + return out, nil +} + +func marshalJSONAny(v any) ([]byte, error) { + if v == nil { + return nil, nil + } + return json.Marshal(v) +} + +func unmarshalJSONAny(b []byte) (any, error) { + if len(b) == 0 { + return nil, nil + } + var out any + if err := json.Unmarshal(b, &out); err != nil { + return nil, err + } + return out, nil +} + +func copyStringMap(m map[string]string) map[string]string { + if m == nil { + return nil + } + out := make(map[string]string, len(m)) + for k, v := range m { + out[k] = v + } + return out +} + +func timeToPB(t time.Time) *timestamppb.Timestamp { + if t.IsZero() { + return nil + } + return timestamppb.New(t) +} + +func timeFromPB(t *timestamppb.Timestamp) time.Time { + if t == nil { + return time.Time{} + } + return t.AsTime() +} + +func refToPB(r interfaces.ResourceRef) *pb.ResourceRef { + return &pb.ResourceRef{Name: r.Name, Type: r.Type, ProviderId: r.ProviderID} +} + +func refsToPB(refs []interfaces.ResourceRef) []*pb.ResourceRef { + out := make([]*pb.ResourceRef, 0, len(refs)) + for _, r := range refs { + out = append(out, refToPB(r)) + } + return out +} + +func hintsToPB(h *interfaces.ResourceHints) *pb.ResourceHints { + if h == nil { + return nil + } + return &pb.ResourceHints{Cpu: h.CPU, Memory: h.Memory, Storage: h.Storage} +} + +func hintsFromPB(h *pb.ResourceHints) *interfaces.ResourceHints { + if h == nil { + return nil + } + return &interfaces.ResourceHints{CPU: h.GetCpu(), Memory: h.GetMemory(), Storage: h.GetStorage()} +} + +func specToPB(s interfaces.ResourceSpec) (*pb.ResourceSpec, error) { + cfgJSON, err := marshalJSONMap(s.Config) + if err != nil { + return nil, err + } + return &pb.ResourceSpec{ + Name: s.Name, + Type: s.Type, + ConfigJson: cfgJSON, + Size: string(s.Size), + Hints: hintsToPB(s.Hints), + DependsOn: append([]string(nil), s.DependsOn...), + }, nil +} + +func specsToPB(specs []interfaces.ResourceSpec) ([]*pb.ResourceSpec, error) { + out := make([]*pb.ResourceSpec, 0, len(specs)) + for _, s := range specs { + ps, err := specToPB(s) + if err != nil { + return nil, err + } + out = append(out, ps) + } + return out, nil +} + +func stateToPB(st *interfaces.ResourceState) (*pb.ResourceState, error) { + appliedJSON, err := marshalJSONMap(st.AppliedConfig) + if err != nil { + return nil, err + } + outputsJSON, err := marshalJSONMap(st.Outputs) + if err != nil { + return nil, err + } + return &pb.ResourceState{ + Id: st.ID, + Name: st.Name, + Type: st.Type, + Provider: st.Provider, + ProviderRef: st.ProviderRef, + ProviderId: st.ProviderID, + ConfigHash: st.ConfigHash, + AppliedConfigJson: appliedJSON, + AppliedConfigSource: st.AppliedConfigSource, + OutputsJson: outputsJSON, + Dependencies: append([]string(nil), st.Dependencies...), + CreatedAt: timeToPB(st.CreatedAt), + UpdatedAt: timeToPB(st.UpdatedAt), + LastDriftCheck: timeToPB(st.LastDriftCheck), + }, nil +} + +func stateFromPB(s *pb.ResourceState) (*interfaces.ResourceState, error) { + if s == nil { + return nil, nil + } + applied, err := unmarshalJSONMap(s.GetAppliedConfigJson()) + if err != nil { + return nil, err + } + outputs, err := unmarshalJSONMap(s.GetOutputsJson()) + if err != nil { + return nil, err + } + return &interfaces.ResourceState{ + ID: s.GetId(), + Name: s.GetName(), + Type: s.GetType(), + Provider: s.GetProvider(), + ProviderRef: s.GetProviderRef(), + ProviderID: s.GetProviderId(), + ConfigHash: s.GetConfigHash(), + AppliedConfig: applied, + AppliedConfigSource: s.GetAppliedConfigSource(), + Outputs: outputs, + Dependencies: append([]string(nil), s.GetDependencies()...), + CreatedAt: timeFromPB(s.GetCreatedAt()), + UpdatedAt: timeFromPB(s.GetUpdatedAt()), + LastDriftCheck: timeFromPB(s.GetLastDriftCheck()), + }, nil +} + +func statesToPB(states []interfaces.ResourceState) ([]*pb.ResourceState, error) { + out := make([]*pb.ResourceState, 0, len(states)) + for i := range states { + ps, err := stateToPB(&states[i]) + if err != nil { + return nil, err + } + out = append(out, ps) + } + return out, nil +} + +func statusesFromPB(ss []*pb.ResourceStatus) ([]interfaces.ResourceStatus, error) { + out := make([]interfaces.ResourceStatus, 0, len(ss)) + for _, s := range ss { + o, err := unmarshalJSONMap(s.GetOutputsJson()) + if err != nil { + return nil, err + } + out = append(out, interfaces.ResourceStatus{ + Name: s.GetName(), + Type: s.GetType(), + ProviderID: s.GetProviderId(), + Status: s.GetStatus(), + Outputs: o, + }) + } + return out, nil +} + +func destroyResultFromPB(r *pb.DestroyResult) *interfaces.DestroyResult { + if r == nil { + return nil + } + errs := make([]interfaces.ActionError, 0, len(r.GetErrors())) + for _, e := range r.GetErrors() { + errs = append(errs, interfaces.ActionError{Resource: e.GetResource(), Action: e.GetAction(), Error: e.GetError()}) + } + return &interfaces.DestroyResult{Destroyed: append([]string(nil), r.GetDestroyed()...), Errors: errs} +} + +func sizingFromPB(s *pb.ProviderSizing) (*interfaces.ProviderSizing, error) { + if s == nil { + return nil, nil + } + specs, err := unmarshalJSONMap(s.GetSpecsJson()) + if err != nil { + return nil, err + } + return &interfaces.ProviderSizing{InstanceType: s.GetInstanceType(), Specs: specs}, nil +} + +func driftClassFromPB(c pb.DriftClass) interfaces.DriftClass { + switch c { + case pb.DriftClass_DRIFT_CLASS_IN_SYNC: + return interfaces.DriftClassInSync + case pb.DriftClass_DRIFT_CLASS_GHOST: + return interfaces.DriftClassGhost + case pb.DriftClass_DRIFT_CLASS_CONFIG: + return interfaces.DriftClassConfig + default: + return interfaces.DriftClassUnknown + } +} + +func changesFromPB(changes []*pb.FieldChange) ([]interfaces.FieldChange, error) { + out := make([]interfaces.FieldChange, 0, len(changes)) + for _, c := range changes { + oldVal, err := unmarshalJSONAny(c.GetOldJson()) + if err != nil { + return nil, err + } + newVal, err := unmarshalJSONAny(c.GetNewJson()) + if err != nil { + return nil, err + } + out = append(out, interfaces.FieldChange{ + Path: c.GetPath(), + Old: oldVal, + New: newVal, + ForceNew: c.GetForceNew(), + }) + } + return out, nil +} + +func planActionFromPB(a *pb.PlanAction) (interfaces.PlanAction, error) { + if a == nil { + return interfaces.PlanAction{}, nil + } + spec, err := specFromPB(a.GetResource()) + if err != nil { + return interfaces.PlanAction{}, err + } + var current *interfaces.ResourceState + if a.GetCurrent() != nil { + current, err = stateFromPB(a.GetCurrent()) + if err != nil { + return interfaces.PlanAction{}, err + } + } + changes, err := changesFromPB(a.GetChanges()) + if err != nil { + return interfaces.PlanAction{}, err + } + return interfaces.PlanAction{ + Action: a.GetAction(), + Resource: spec, + Current: current, + Changes: changes, + ResolvedConfigHash: a.GetResolvedConfigHash(), + }, nil +} + +func specFromPB(s *pb.ResourceSpec) (interfaces.ResourceSpec, error) { + if s == nil { + return interfaces.ResourceSpec{}, nil + } + cfg, err := unmarshalJSONMap(s.GetConfigJson()) + if err != nil { + return interfaces.ResourceSpec{}, err + } + return interfaces.ResourceSpec{ + Name: s.GetName(), + Type: s.GetType(), + Config: cfg, + Size: interfaces.Size(s.GetSize()), + Hints: hintsFromPB(s.GetHints()), + DependsOn: append([]string(nil), s.GetDependsOn()...), + }, nil +} + +func planFromPB(p *pb.IaCPlan) (*interfaces.IaCPlan, error) { + if p == nil { + return nil, nil + } + actions := make([]interfaces.PlanAction, 0, len(p.GetActions())) + for _, a := range p.GetActions() { + pa, err := planActionFromPB(a) + if err != nil { + return nil, err + } + actions = append(actions, pa) + } + return &interfaces.IaCPlan{ + ID: p.GetId(), + Actions: actions, + CreatedAt: timeFromPB(p.GetCreatedAt()), + DesiredHash: p.GetDesiredHash(), + SchemaVersion: int(p.GetSchemaVersion()), + InputSnapshot: copyStringMap(p.GetInputSnapshot()), + }, nil +} diff --git a/iac/providerclient/adapter_test.go b/iac/providerclient/adapter_test.go new file mode 100644 index 00000000..605acaa7 --- /dev/null +++ b/iac/providerclient/adapter_test.go @@ -0,0 +1,272 @@ +package providerclient_test + +import ( + "context" + "encoding/json" + "net" + "testing" + + "github.com/GoCodeAlone/workflow/iac/providerclient" + "github.com/GoCodeAlone/workflow/interfaces" + pb "github.com/GoCodeAlone/workflow/plugin/external/proto" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/status" + "google.golang.org/grpc/test/bufconn" +) + +// Compile-time interface assertions — the primary guard for Task 2. +var _ interfaces.IaCProvider = (*providerclient.Adapter)(nil) +var _ interfaces.IaCProviderRegionLister = (*providerclient.Adapter)(nil) + +// ─── In-proc fake gRPC servers ────────────────────────────────────────────── + +// fakeRequiredServer implements IaCProviderRequiredServer with deterministic responses. +type fakeRequiredServer struct { + pb.UnimplementedIaCProviderRequiredServer +} + +func (s *fakeRequiredServer) Name(_ context.Context, _ *pb.NameRequest) (*pb.NameResponse, error) { + return &pb.NameResponse{Name: "fake-provider"}, nil +} + +func (s *fakeRequiredServer) Version(_ context.Context, _ *pb.VersionRequest) (*pb.VersionResponse, error) { + return &pb.VersionResponse{Version: "0.1.0"}, nil +} + +func (s *fakeRequiredServer) Initialize(_ context.Context, _ *pb.InitializeRequest) (*pb.InitializeResponse, error) { + return &pb.InitializeResponse{}, nil +} + +func (s *fakeRequiredServer) Capabilities(_ context.Context, _ *pb.CapabilitiesRequest) (*pb.CapabilitiesResponse, error) { + return &pb.CapabilitiesResponse{ + Capabilities: []*pb.IaCCapabilityDeclaration{ + {ResourceType: "infra.vpc", Tier: 1, Operations: []string{"create", "delete"}}, + }, + }, nil +} + +func (s *fakeRequiredServer) Plan(_ context.Context, req *pb.PlanRequest) (*pb.PlanResponse, error) { + return &pb.PlanResponse{ + Plan: &pb.IaCPlan{ + Id: "plan-1", + DesiredHash: "abc123", + }, + }, nil +} + +func (s *fakeRequiredServer) Destroy(_ context.Context, req *pb.DestroyRequest) (*pb.DestroyResponse, error) { + destroyed := make([]string, 0, len(req.GetRefs())) + for _, r := range req.GetRefs() { + destroyed = append(destroyed, r.GetName()) + } + return &pb.DestroyResponse{Result: &pb.DestroyResult{Destroyed: destroyed}}, nil +} + +func (s *fakeRequiredServer) Status(_ context.Context, req *pb.StatusRequest) (*pb.StatusResponse, error) { + statuses := make([]*pb.ResourceStatus, 0, len(req.GetRefs())) + for _, r := range req.GetRefs() { + outputsJSON, _ := json.Marshal(map[string]any{"id": r.GetProviderId()}) + statuses = append(statuses, &pb.ResourceStatus{ + Name: r.GetName(), + Type: r.GetType(), + ProviderId: r.GetProviderId(), + Status: "running", + OutputsJson: outputsJSON, + }) + } + return &pb.StatusResponse{Statuses: statuses}, nil +} + +func (s *fakeRequiredServer) Import(_ context.Context, _ *pb.ImportRequest) (*pb.ImportResponse, error) { + return nil, status.Error(codes.Unimplemented, "not implemented in fake") +} + +func (s *fakeRequiredServer) ResolveSizing(_ context.Context, _ *pb.ResolveSizingRequest) (*pb.ResolveSizingResponse, error) { + return nil, status.Error(codes.Unimplemented, "not implemented in fake") +} + +func (s *fakeRequiredServer) BootstrapStateBackend(_ context.Context, _ *pb.BootstrapStateBackendRequest) (*pb.BootstrapStateBackendResponse, error) { + return nil, status.Error(codes.Unimplemented, "not implemented in fake") +} + +// fakeRegionListerServer implements IaCProviderRegionListerServer. +type fakeRegionListerServer struct { + pb.UnimplementedIaCProviderRegionListerServer +} + +func (s *fakeRegionListerServer) ListRegions(_ context.Context, req *pb.ListRegionsRequest) (*pb.ListRegionsResponse, error) { + return &pb.ListRegionsResponse{ + Regions: []*pb.ProviderRegion{ + {Name: "us-east-1", DisplayName: "US East"}, + {Name: "us-west-2", DisplayName: "US West"}, + }, + }, nil +} + +// ─── Test setup helper ─────────────────────────────────────────────────────── + +// startFakeServer starts an in-process gRPC server on a bufconn listener and +// returns a *grpc.ClientConn and a cleanup function. The caller registers +// services on srv before calling startFakeServer. +func startFakeServer(t *testing.T, srv *grpc.Server) *grpc.ClientConn { + t.Helper() + lis := bufconn.Listen(4 << 20) + t.Cleanup(func() { _ = lis.Close() }) + go func() { _ = srv.Serve(lis) }() + t.Cleanup(srv.Stop) + + conn, err := grpc.NewClient("passthrough:///bufnet", + grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { + return lis.DialContext(ctx) + }), + grpc.WithTransportCredentials(insecure.NewCredentials())) + if err != nil { + t.Fatalf("grpc.NewClient: %v", err) + } + t.Cleanup(func() { _ = conn.Close() }) + return conn +} + +// ─── Tests ────────────────────────────────────────────────────────────────── + +// TestAdapter_Name verifies Name() delegates to the IaCProviderRequired.Name RPC. +func TestAdapter_Name(t *testing.T) { + srv := grpc.NewServer() + pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) + pb.RegisterIaCProviderRegionListerServer(srv, &fakeRegionListerServer{}) + conn := startFakeServer(t, srv) + + a := providerclient.New(conn) + if got := a.Name(); got != "fake-provider" { + t.Errorf("Name() = %q, want %q", got, "fake-provider") + } +} + +// TestAdapter_Plan verifies Plan() delegates and translates the response. +func TestAdapter_Plan(t *testing.T) { + srv := grpc.NewServer() + pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) + pb.RegisterIaCProviderRegionListerServer(srv, &fakeRegionListerServer{}) + conn := startFakeServer(t, srv) + + a := providerclient.New(conn) + plan, err := a.Plan(context.Background(), nil, nil) + if err != nil { + t.Fatalf("Plan() returned error: %v", err) + } + if plan == nil { + t.Fatal("Plan() returned nil plan") + } + if plan.ID != "plan-1" { + t.Errorf("plan.ID = %q, want %q", plan.ID, "plan-1") + } + if plan.DesiredHash != "abc123" { + t.Errorf("plan.DesiredHash = %q, want %q", plan.DesiredHash, "abc123") + } +} + +// TestAdapter_Destroy verifies Destroy() delegates refs and returns destroy result. +func TestAdapter_Destroy(t *testing.T) { + srv := grpc.NewServer() + pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) + pb.RegisterIaCProviderRegionListerServer(srv, &fakeRegionListerServer{}) + conn := startFakeServer(t, srv) + + a := providerclient.New(conn) + refs := []interfaces.ResourceRef{{Name: "vpc-1", Type: "infra.vpc", ProviderID: "vpc-abc"}} + result, err := a.Destroy(context.Background(), refs) + if err != nil { + t.Fatalf("Destroy() returned error: %v", err) + } + if result == nil { + t.Fatal("Destroy() returned nil result") + } + if len(result.Destroyed) != 1 || result.Destroyed[0] != "vpc-1" { + t.Errorf("Destroyed = %v, want [vpc-1]", result.Destroyed) + } +} + +// TestAdapter_Status verifies Status() delegates and translates ResourceStatus. +func TestAdapter_Status(t *testing.T) { + srv := grpc.NewServer() + pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) + pb.RegisterIaCProviderRegionListerServer(srv, &fakeRegionListerServer{}) + conn := startFakeServer(t, srv) + + a := providerclient.New(conn) + refs := []interfaces.ResourceRef{{Name: "vpc-1", Type: "infra.vpc", ProviderID: "vpc-abc"}} + statuses, err := a.Status(context.Background(), refs) + if err != nil { + t.Fatalf("Status() returned error: %v", err) + } + if len(statuses) != 1 { + t.Fatalf("len(statuses) = %d, want 1", len(statuses)) + } + if statuses[0].Name != "vpc-1" || statuses[0].Status != "running" { + t.Errorf("status = %+v, want Name=vpc-1 Status=running", statuses[0]) + } +} + +// TestAdapter_ListRegions verifies ListRegions() delegates to IaCProviderRegionLister. +func TestAdapter_ListRegions(t *testing.T) { + srv := grpc.NewServer() + pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) + pb.RegisterIaCProviderRegionListerServer(srv, &fakeRegionListerServer{}) + conn := startFakeServer(t, srv) + + a := providerclient.New(conn) + regions, err := a.ListRegions(context.Background(), "prod") + if err != nil { + t.Fatalf("ListRegions() returned error: %v", err) + } + if len(regions) == 0 { + t.Error("ListRegions() returned no regions") + } + // Verify region names round-trip correctly. + found := false + for _, r := range regions { + if r == "us-east-1" { + found = true + break + } + } + if !found { + t.Errorf("ListRegions() = %v, expected us-east-1 to be present", regions) + } +} + +// TestAdapter_TypeAssertIaCProvider verifies Adapter satisfies interfaces.IaCProvider. +func TestAdapter_TypeAssertIaCProvider(t *testing.T) { + srv := grpc.NewServer() + pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) + pb.RegisterIaCProviderRegionListerServer(srv, &fakeRegionListerServer{}) + conn := startFakeServer(t, srv) + + a := providerclient.New(conn) + var p interfaces.IaCProvider = a + if p == nil { + t.Fatal("Adapter does not satisfy interfaces.IaCProvider") + } +} + +// TestAdapter_TypeAssertIaCProviderRegionLister verifies Adapter satisfies +// interfaces.IaCProviderRegionLister so step.iac_provider_catalog can +// type-assert it. +func TestAdapter_TypeAssertIaCProviderRegionLister(t *testing.T) { + srv := grpc.NewServer() + pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) + pb.RegisterIaCProviderRegionListerServer(srv, &fakeRegionListerServer{}) + conn := startFakeServer(t, srv) + + a := providerclient.New(conn) + var p interfaces.IaCProvider = a + rl, ok := p.(interfaces.IaCProviderRegionLister) + if !ok { + t.Fatal("Adapter must satisfy interfaces.IaCProviderRegionLister") + } + if rl == nil { + t.Fatal("IaCProviderRegionLister type-assert returned nil") + } +} From 1a727a0a2b3fdeef42a0db073a537f5abcffdd8f Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 2 Jun 2026 02:13:43 -0400 Subject: [PATCH 3/4] feat(plugin/external): register iac.provider plugins as interfaces.IaCProvider services via WiringHook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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(, 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 --- iac/providerclient/adapter.go | 20 -- iac/providerclient/adapter_test.go | 11 +- plugin/external/adapter.go | 78 +++++- plugin/external/iac_provider_wiring_test.go | 248 ++++++++++++++++++++ 4 files changed, 334 insertions(+), 23 deletions(-) create mode 100644 plugin/external/iac_provider_wiring_test.go diff --git a/iac/providerclient/adapter.go b/iac/providerclient/adapter.go index 46d9eb5f..4dadb952 100644 --- a/iac/providerclient/adapter.go +++ b/iac/providerclient/adapter.go @@ -270,13 +270,6 @@ func unmarshalJSONMap(b []byte) (map[string]any, error) { return out, nil } -func marshalJSONAny(v any) ([]byte, error) { - if v == nil { - return nil, nil - } - return json.Marshal(v) -} - func unmarshalJSONAny(b []byte) (any, error) { if len(b) == 0 { return nil, nil @@ -475,19 +468,6 @@ func sizingFromPB(s *pb.ProviderSizing) (*interfaces.ProviderSizing, error) { return &interfaces.ProviderSizing{InstanceType: s.GetInstanceType(), Specs: specs}, nil } -func driftClassFromPB(c pb.DriftClass) interfaces.DriftClass { - switch c { - case pb.DriftClass_DRIFT_CLASS_IN_SYNC: - return interfaces.DriftClassInSync - case pb.DriftClass_DRIFT_CLASS_GHOST: - return interfaces.DriftClassGhost - case pb.DriftClass_DRIFT_CLASS_CONFIG: - return interfaces.DriftClassConfig - default: - return interfaces.DriftClassUnknown - } -} - func changesFromPB(changes []*pb.FieldChange) ([]interfaces.FieldChange, error) { out := make([]interfaces.FieldChange, 0, len(changes)) for _, c := range changes { diff --git a/iac/providerclient/adapter_test.go b/iac/providerclient/adapter_test.go index 605acaa7..35053ea4 100644 --- a/iac/providerclient/adapter_test.go +++ b/iac/providerclient/adapter_test.go @@ -238,6 +238,9 @@ func TestAdapter_ListRegions(t *testing.T) { } // TestAdapter_TypeAssertIaCProvider verifies Adapter satisfies interfaces.IaCProvider. +// The compile-time assertion (var _ interfaces.IaCProvider = (*Adapter)(nil)) at the +// top of this file is the load-bearing check; this runtime test verifies New() returns +// a non-nil adapter that is usable as the interface. func TestAdapter_TypeAssertIaCProvider(t *testing.T) { srv := grpc.NewServer() pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) @@ -245,9 +248,13 @@ func TestAdapter_TypeAssertIaCProvider(t *testing.T) { conn := startFakeServer(t, srv) a := providerclient.New(conn) + if a == nil { + t.Fatal("providerclient.New returned nil") + } + // Verify the name RPC works through the interfaces.IaCProvider method. var p interfaces.IaCProvider = a - if p == nil { - t.Fatal("Adapter does not satisfy interfaces.IaCProvider") + if got := p.Name(); got != "fake-provider" { + t.Errorf("p.Name() via interface = %q, want fake-provider", got) } } diff --git a/plugin/external/adapter.go b/plugin/external/adapter.go index 6b0dcb89..e34b1327 100644 --- a/plugin/external/adapter.go +++ b/plugin/external/adapter.go @@ -11,6 +11,7 @@ import ( "github.com/GoCodeAlone/workflow/capability" "github.com/GoCodeAlone/workflow/config" "github.com/GoCodeAlone/workflow/deploy" + "github.com/GoCodeAlone/workflow/iac/providerclient" "github.com/GoCodeAlone/workflow/plugin" pb "github.com/GoCodeAlone/workflow/plugin/external/proto" "github.com/GoCodeAlone/workflow/schema" @@ -576,8 +577,83 @@ func (a *ExternalPluginAdapter) StepSchemas() []*schema.StepSchema { return nil } +// iacProviderRequiredServiceName is the fully-qualified gRPC service a plugin's +// ContractRegistry must advertise for the adapter to be treated as an +// iac.provider — the analog of iacStateBackendServiceName. Sourced from the +// generated proto's ServiceDesc so it cannot drift if the proto package +// path/service name ever changes. +var iacProviderRequiredServiceName = pb.IaCProviderRequired_ServiceDesc.ServiceName + +// advertisesIaCProviderRequiredService reports whether the adapter's +// ContractRegistry carries a CONTRACT_KIND_SERVICE descriptor for the +// IaCProviderRequired service. Mirrors advertisesIaCStateBackendService. +func (a *ExternalPluginAdapter) advertisesIaCProviderRequiredService() bool { + if a.contractRegistry == nil { + return false + } + for _, d := range a.contractRegistry.Contracts { + if d == nil { + continue + } + if d.Kind == pb.ContractKind_CONTRACT_KIND_SERVICE && d.ServiceName == iacProviderRequiredServiceName { + return true + } + } + // Also check diskManifest.IaCServices as a fallback — the plugin may declare + // the service there without advertising it in the ContractRegistry (e.g. when + // GetContractRegistry returned Unimplemented). This mirrors the check in + // IaCStateBackendClients which cross-checks against diskManifest. + if a.diskManifest != nil { + for _, svc := range a.diskManifest.IaCServices { + if svc == iacProviderRequiredServiceName { + return true + } + } + } + return false +} + +// WiringHooks returns a WiringHook that registers the plugin as an +// interfaces.IaCProvider service in the modular application DI graph when this +// plugin advertises the IaCProviderRequired gRPC service. +// +// Registration key convention: the plugin name (a.name, as declared in the +// plugin manifest / app.yaml plugin entry). Steps that want to use this +// provider configure `provider: ` and the engine resolves it via +// app.GetService(cfg.Provider, &provider). This key is the same name that +// appears in the scenario app.yaml's plugin entry for the iac.provider plugin. +// +// If the plugin does not advertise IaCProviderRequired this method returns nil +// (no wiring needed — not an iac.provider plugin). func (a *ExternalPluginAdapter) WiringHooks() []plugin.WiringHook { - return nil + if !a.advertisesIaCProviderRequiredService() { + return nil + } + // Capture immutable fields before the closure. conn may be nil + // (test adapters built without a real gRPC connection); the hook + // guard handles that. + name := a.name + conn := a.Conn() + return []plugin.WiringHook{ + { + // Name follows the convention "-iac-provider-registration". + // Priority 50: runs after high-priority service wiring (e.g. OTEL at + // 100, auth at 90) so the service registry is stable, but before lower- + // priority wiring that might look up providers. + Name: name + "-iac-provider-registration", + Priority: 50, + Hook: func(app modular.Application, _ *config.WorkflowConfig) error { + if conn == nil { + return fmt.Errorf("plugin %q advertises IaCProviderRequired but has no gRPC connection", name) + } + adapter := providerclient.New(conn) + if err := app.RegisterService(name, adapter); err != nil { + return fmt.Errorf("plugin %q: register IaCProvider service: %w", name, err) + } + return nil + }, + }, + } } func (a *ExternalPluginAdapter) DeployTargets() map[string]deploy.DeployTarget { diff --git a/plugin/external/iac_provider_wiring_test.go b/plugin/external/iac_provider_wiring_test.go new file mode 100644 index 00000000..d05af57a --- /dev/null +++ b/plugin/external/iac_provider_wiring_test.go @@ -0,0 +1,248 @@ +package external + +// iac_provider_wiring_test.go — Task 3: WiringHook service registration for +// external iac.provider plugins. +// +// Tests that ExternalPluginAdapter.WiringHooks() returns a hook when the plugin +// advertises IaCProviderRequired, and that running the hook registers the adapter +// as an interfaces.IaCProvider service in the modular application DI graph. +// +// Uses package external (internal test) to access unexported PluginClient fields, +// matching the pattern in adapter_test.go. + +import ( + "context" + "net" + "testing" + + "github.com/GoCodeAlone/modular" + "github.com/GoCodeAlone/workflow/iac/providerclient" + "github.com/GoCodeAlone/workflow/interfaces" + "github.com/GoCodeAlone/workflow/plugin" + pb "github.com/GoCodeAlone/workflow/plugin/external/proto" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/status" + "google.golang.org/grpc/test/bufconn" +) + +// ─── Fake gRPC server helpers ──────────────────────────────────────────────── + +// minimalIaCProviderServer implements just enough of IaCProviderRequiredServer +// for the WiringHook to succeed (Conn() is all that matters; RPC methods +// are unused in the hook itself). +type minimalIaCProviderServer struct { + pb.UnimplementedIaCProviderRequiredServer +} + +func (s *minimalIaCProviderServer) Name(_ context.Context, _ *pb.NameRequest) (*pb.NameResponse, error) { + return &pb.NameResponse{Name: "test-iac-provider"}, nil +} + +func (s *minimalIaCProviderServer) Version(_ context.Context, _ *pb.VersionRequest) (*pb.VersionResponse, error) { + return &pb.VersionResponse{Version: "1.0.0"}, nil +} + +func (s *minimalIaCProviderServer) Initialize(_ context.Context, _ *pb.InitializeRequest) (*pb.InitializeResponse, error) { + return &pb.InitializeResponse{}, nil +} + +func (s *minimalIaCProviderServer) Capabilities(_ context.Context, _ *pb.CapabilitiesRequest) (*pb.CapabilitiesResponse, error) { + return &pb.CapabilitiesResponse{}, nil +} + +func (s *minimalIaCProviderServer) Plan(_ context.Context, _ *pb.PlanRequest) (*pb.PlanResponse, error) { + return nil, status.Error(codes.Unimplemented, "not needed in test") +} + +func (s *minimalIaCProviderServer) Destroy(_ context.Context, _ *pb.DestroyRequest) (*pb.DestroyResponse, error) { + return nil, status.Error(codes.Unimplemented, "not needed in test") +} + +func (s *minimalIaCProviderServer) Status(_ context.Context, _ *pb.StatusRequest) (*pb.StatusResponse, error) { + return nil, status.Error(codes.Unimplemented, "not needed in test") +} + +func (s *minimalIaCProviderServer) Import(_ context.Context, _ *pb.ImportRequest) (*pb.ImportResponse, error) { + return nil, status.Error(codes.Unimplemented, "not needed in test") +} + +func (s *minimalIaCProviderServer) ResolveSizing(_ context.Context, _ *pb.ResolveSizingRequest) (*pb.ResolveSizingResponse, error) { + return nil, status.Error(codes.Unimplemented, "not needed in test") +} + +func (s *minimalIaCProviderServer) BootstrapStateBackend(_ context.Context, _ *pb.BootstrapStateBackendRequest) (*pb.BootstrapStateBackendResponse, error) { + return nil, status.Error(codes.Unimplemented, "not needed in test") +} + +// 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() + lis := bufconn.Listen(4 << 20) + t.Cleanup(func() { _ = lis.Close() }) + srv := grpc.NewServer() + pb.RegisterIaCProviderRequiredServer(srv, &minimalIaCProviderServer{}) + go func() { _ = srv.Serve(lis) }() + t.Cleanup(srv.Stop) + + conn, err := grpc.NewClient("passthrough:///bufnet", + grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { + return lis.DialContext(ctx) + }), + grpc.WithTransportCredentials(insecure.NewCredentials())) + if err != nil { + t.Fatalf("grpc.NewClient: %v", err) + } + t.Cleanup(func() { _ = conn.Close() }) + return conn +} + +// newIaCProviderAdapter builds an ExternalPluginAdapter that advertises the +// IaCProviderRequired service via its ContractRegistry and holds a live +// 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() + 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 + }, + }, + } + a := &ExternalPluginAdapter{ + name: pluginName, + manifest: &pb.Manifest{Name: pluginName, Version: "1.0.0"}, + diskManifest: diskManifest, + contractRegistry: registry, + contracts: buildContractDescriptorCache(registry), + client: &PluginClient{conn: conn}, + } + return a +} + +// ─── Tests ────────────────────────────────────────────────────────────────── + +// TestWiringHook_IaCProviderPlugin_ReturnsHook asserts that an adapter whose +// diskManifest.IaCServices includes IaCProviderRequired returns a non-empty +// WiringHooks() slice. +func TestWiringHook_IaCProviderPlugin_ReturnsHook(t *testing.T) { + conn := startIaCProviderServer(t) + a := newIaCProviderAdapter(t, "my-iac-provider", conn) + + hooks := a.WiringHooks() + if len(hooks) == 0 { + t.Fatal("WiringHooks() must return at least one hook for an iac.provider plugin") + } + // Verify the hook has a meaningful name (doc convention). + if hooks[0].Name == "" { + t.Error("WiringHook.Name must not be empty") + } +} + +// TestWiringHook_NonIaCPlugin_ReturnsNil asserts that an adapter without +// IaCProviderRequired in its ContractRegistry returns an empty WiringHooks() slice. +func TestWiringHook_NonIaCPlugin_ReturnsNil(t *testing.T) { + // Build an adapter with no IaCServices in the diskManifest. + diskManifest := &plugin.PluginManifest{Name: "non-iac-plugin", Version: "1.0.0"} + a := &ExternalPluginAdapter{ + name: "non-iac-plugin", + manifest: &pb.Manifest{Name: "non-iac-plugin"}, + diskManifest: diskManifest, + contractRegistry: &pb.ContractRegistry{}, + contracts: buildContractDescriptorCache(&pb.ContractRegistry{}), + } + if hooks := a.WiringHooks(); len(hooks) != 0 { + t.Errorf("WiringHooks() for non-iac plugin = %v (len %d), want empty", hooks, len(hooks)) + } +} + +// TestWiringHook_RegistersIaCProviderService asserts that running the WiringHook +// registers the adapter as an interfaces.IaCProvider service in the modular app +// DI graph under the plugin name. This is the core of Task 3. +func TestWiringHook_RegistersIaCProviderService(t *testing.T) { + conn := startIaCProviderServer(t) + a := newIaCProviderAdapter(t, "my-iac-provider", conn) + + hooks := a.WiringHooks() + if len(hooks) == 0 { + t.Fatal("expected at least one WiringHook") + } + + // Create a real modular application (the same type the engine uses). + app := modular.NewStdApplication(modular.NewStdConfigProvider(nil), nil) + + // Run the hook (simulating what engine.BuildFromConfig does after app.Init()). + if err := hooks[0].Hook(app, nil); err != nil { + t.Fatalf("WiringHook.Hook returned error: %v", err) + } + + // Assert app.GetService("my-iac-provider", &p) resolves a non-nil interfaces.IaCProvider. + var p interfaces.IaCProvider + if err := app.GetService("my-iac-provider", &p); err != nil { + t.Fatalf("app.GetService(\"my-iac-provider\", &p): %v", err) + } + if p == nil { + t.Fatal("registered service is nil") + } + + // Verify it is a *providerclient.Adapter (not some other type). + if _, ok := p.(*providerclient.Adapter); !ok { + t.Errorf("registered service is %T, want *providerclient.Adapter", p) + } +} + +// TestWiringHook_TwoPlugins_RegisterUnderDistinctNames asserts that two distinct +// iac.provider plugins register under distinct names, both resolvable. +func TestWiringHook_TwoPlugins_RegisterUnderDistinctNames(t *testing.T) { + conn1 := startIaCProviderServer(t) + conn2 := startIaCProviderServer(t) + + a1 := newIaCProviderAdapter(t, "provider-alpha", conn1) + a2 := newIaCProviderAdapter(t, "provider-beta", conn2) + + app := modular.NewStdApplication(modular.NewStdConfigProvider(nil), nil) + + // Run both hooks. + for _, a := range []*ExternalPluginAdapter{a1, a2} { + hooks := a.WiringHooks() + if len(hooks) == 0 { + t.Fatalf("expected WiringHooks for %T", a) + } + if err := hooks[0].Hook(app, nil); err != nil { + t.Fatalf("WiringHook.Hook error: %v", err) + } + } + + // Both must be resolvable under their distinct names. + var p1, p2 interfaces.IaCProvider + if err := app.GetService("provider-alpha", &p1); err != nil { + t.Fatalf("GetService(provider-alpha): %v", err) + } + if err := app.GetService("provider-beta", &p2); err != nil { + t.Fatalf("GetService(provider-beta): %v", err) + } + if p1 == nil || p2 == nil { + t.Fatal("one or both providers are nil") + } + // Distinct — must not be the same pointer. + if p1 == p2 { + t.Error("provider-alpha and provider-beta resolved to the same service") + } +} + +// 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. +var _ = (*providerclient.Adapter)(nil) From a551b4d98facd955f9e3d266c48f11639be12f0f Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 2 Jun 2026 02:39:11 -0400 Subject: [PATCH 4/4] fix(providerclient): advertisement-gate optional services + wire DriftDetector + fix Endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- iac/providerclient/adapter.go | 314 +++++++++++++++++++++++------ iac/providerclient/adapter_test.go | 263 ++++++++++++++++++++---- plugin/external/adapter.go | 58 +++++- 3 files changed, 535 insertions(+), 100 deletions(-) diff --git a/iac/providerclient/adapter.go b/iac/providerclient/adapter.go index 4dadb952..6bd12fa3 100644 --- a/iac/providerclient/adapter.go +++ b/iac/providerclient/adapter.go @@ -8,7 +8,7 @@ // // Usage (via WiringHook in plugin/external/adapter.go): // -// adapter := providerclient.New(conn) +// adapter := providerclient.New(conn, advertisedServices) // app.RegisterService(moduleName, adapter) // // Steps resolve the provider via: @@ -21,6 +21,7 @@ import ( "encoding/json" "fmt" "log" + "sync" "time" "github.com/GoCodeAlone/workflow/interfaces" @@ -31,30 +32,179 @@ import ( "google.golang.org/protobuf/types/known/timestamppb" ) -// Adapter wraps the pb.IaCProviderRequired gRPC client (and optional -// pb.IaCProviderRegionListerClient) as interfaces.IaCProvider + -// interfaces.IaCProviderRegionLister. The conn-backed optional clients are -// always constructed: the gRPC channel multiplexes them cheaply, and the -// optional-service guard is at the plugin's server side (Unimplemented). +// Fully-qualified gRPC service names for optional IaC services. These mirror +// the constants in cmd/wfctl/iac_typed_adapter.go and are sourced from the +// generated proto ServiceDesc so they cannot drift. The WiringHook in +// plugin/external/adapter.go passes the advertised set here; optional gRPC +// clients are constructed only when the corresponding name is present. +const ( + // IaCServiceRegionLister is the gRPC service name for the optional + // IaCProviderRegionLister service. + IaCServiceRegionLister = "workflow.plugin.external.iac.IaCProviderRegionLister" + // IaCServiceDriftDetector is the gRPC service name for the optional + // IaCProviderDriftDetector service. + IaCServiceDriftDetector = "workflow.plugin.external.iac.IaCProviderDriftDetector" +) + +// RegionListerProvider is a capability-discovery interface implemented by +// *Adapter when the plugin advertised IaCProviderRegionLister. Steps that want +// to prefer provider-sourced region lists type-assert the registered +// interfaces.IaCProvider to RegionListerProvider and call RegionLister(). +// A nil return from RegionLister() means the plugin did not advertise the +// service — the step MUST fall back to its static catalog. +type RegionListerProvider interface { + // RegionLister returns the region-lister capability object, or nil when + // the plugin did not advertise IaCProviderRegionLister. + RegionLister() interfaces.IaCProviderRegionLister +} + +// driftDetectorAdapter is the value returned by DriftDetector() when the +// plugin advertises the IaCProviderDriftDetector service. It wraps the gRPC +// client and satisfies interfaces.DriftConfigDetector so consumers can route +// DetectDriftWithSpecs through the optional service. +// +// DetectDrift (existence-only) is routed through the required IaCProvider +// interface on *Adapter. The full config-aware drift (DetectDriftWithSpecs) +// lives here, mirroring how typedIaCAdapter routes driftCfg. +type driftDetectorAdapter struct { + client pb.IaCProviderDriftDetectorClient +} + +// DetectDriftWithSpecs calls the optional IaCProviderDriftDetector.DetectDrift +// gRPC (existence-only variant routed through the optional service). Steps that +// need the config-aware path use the DriftConfigDetector interface accessor. +func (d *driftDetectorAdapter) DetectDriftWithSpecs(ctx context.Context, resources []interfaces.ResourceRef, specs map[string]interfaces.ResourceSpec) ([]interfaces.DriftResult, error) { + // Build the per-ref spec map for the RPC. Absent specs for a ref instruct + // the provider to fall back to existence-only behavior for that ref. + pbSpecs := make(map[string]*pb.ResourceSpec, len(specs)) + for k, v := range specs { + ps, err := specToPB(v) + if err != nil { + return nil, fmt.Errorf("providerclient: encode DetectDriftWithSpecs specs[%s]: %w", k, err) + } + pbSpecs[k] = ps + } + resp, err := d.client.DetectDriftWithSpecs(ctx, &pb.DetectDriftWithSpecsRequest{ + Refs: refsToPB(resources), + Specs: pbSpecs, + }) + if err != nil { + if status.Code(err) == codes.Unimplemented { + return nil, fmt.Errorf("%w: DetectDriftWithSpecs not implemented by plugin", + interfaces.ErrProviderMethodUnimplemented) + } + return nil, err + } + return driftsFromPB(resp.GetDrifts()) +} + +// DriftDetectorProvider is a capability-discovery interface implemented by +// *Adapter when the plugin advertised IaCProviderDriftDetector. Steps that +// need config-aware drift detection type-assert the registered +// interfaces.IaCProvider to DriftDetectorProvider and call DriftDetector(). +// A nil return means the plugin did not advertise the service. +type DriftDetectorProvider interface { + // DriftDetector returns the drift-detector capability object (satisfying + // interfaces.DriftConfigDetector), or nil when the plugin did not advertise + // IaCProviderDriftDetector. + DriftDetector() interfaces.DriftConfigDetector +} + +// 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 +// (RegionLister(), DriftDetector()) that return nil when the plugin did not +// advertise the corresponding gRPC service. +// +// The critical invariant: *Adapter does NOT unconditionally satisfy +// interfaces.IaCProviderRegionLister. Callers MUST type-assert to +// RegionListerProvider and call RegionLister() to discover the capability, then +// use the returned object. This mirrors typedIaCAdapter which returns nil +// optional clients for unadvertised services — enabling catalog steps' +// "static fallback if unadvertised" path to fire correctly. // // Compile-time guards are in adapter_test.go. type Adapter struct { conn grpc.ClientConnInterface required pb.IaCProviderRequiredClient - regionLister pb.IaCProviderRegionListerClient + regionLister *regionListerImpl // nil when IaCServiceRegionLister not advertised + drift *driftDetectorAdapter // nil when IaCServiceDriftDetector not advertised + + // Capabilities cache. Populated on first call to fetchCapabilities via + // capsOnce; reused for the adapter's lifetime (capabilities don't change + // during a session). Mirrors typedIaCAdapter.cachedCaps + capsFetch. + capsOnce sync.Once + cachedCaps *pb.CapabilitiesResponse + capsErr error +} + +// regionListerImpl wraps pb.IaCProviderRegionListerClient and satisfies +// interfaces.IaCProviderRegionLister. It is the concrete type returned by +// Adapter.RegionLister(). +type regionListerImpl struct { + client pb.IaCProviderRegionListerClient +} + +func (r *regionListerImpl) ListRegions(ctx context.Context, env string) ([]string, error) { + resp, err := r.client.ListRegions(ctx, &pb.ListRegionsRequest{EnvName: env}) + if err != nil { + if status.Code(err) == codes.Unimplemented { + return nil, fmt.Errorf("%w: IaCProviderRegionLister not registered by plugin", + interfaces.ErrProviderMethodUnimplemented) + } + return nil, err + } + regions := make([]string, 0, len(resp.GetRegions())) + for _, r := range resp.GetRegions() { + if name := r.GetName(); name != "" { + regions = append(regions, name) + } + } + return regions, nil +} + +// New constructs an Adapter over conn. Optional gRPC clients (RegionLister, +// DriftDetector) are constructed ONLY when the matching service name appears in +// advertisedServices. Passing nil or an empty map is valid — the adapter +// exposes only the required surface in that case. +// +// advertisedServices should be populated from the plugin's ContractRegistry +// (see plugin/external/adapter.go WiringHooks). The service-name constants +// IaCServiceRegionLister and IaCServiceDriftDetector are provided for the +// caller's convenience. +func New(conn grpc.ClientConnInterface, advertisedServices map[string]bool) *Adapter { + a := &Adapter{ + conn: conn, + required: pb.NewIaCProviderRequiredClient(conn), + } + if advertisedServices[IaCServiceRegionLister] { + a.regionLister = ®ionListerImpl{client: pb.NewIaCProviderRegionListerClient(conn)} + } + if advertisedServices[IaCServiceDriftDetector] { + a.drift = &driftDetectorAdapter{client: pb.NewIaCProviderDriftDetectorClient(conn)} + } + return a +} + +// RegionLister implements RegionListerProvider. Returns the region-lister +// capability object when the plugin advertised IaCProviderRegionLister, or nil +// when it did not. Callers MUST nil-check before using. +func (a *Adapter) RegionLister() interfaces.IaCProviderRegionLister { + if a.regionLister == nil { + return nil + } + return a.regionLister } -// New constructs an Adapter over conn. Both the required and optional -// region-lister clients are constructed eagerly against the shared conn; -// the connection multiplexes them at zero marginal cost. If the plugin -// does not serve the optional service, calls to ListRegions return -// interfaces.ErrProviderMethodUnimplemented. -func New(conn grpc.ClientConnInterface) *Adapter { - return &Adapter{ - conn: conn, - required: pb.NewIaCProviderRequiredClient(conn), - regionLister: pb.NewIaCProviderRegionListerClient(conn), +// DriftDetector implements DriftDetectorProvider. Returns the drift-detector +// capability object (satisfying interfaces.DriftConfigDetector) when the plugin +// advertised IaCProviderDriftDetector, or nil when it did not. Callers MUST +// nil-check before using. +func (a *Adapter) DriftDetector() interfaces.DriftConfigDetector { + if a.drift == nil { + return nil } + return a.drift } // ─── interfaces.IaCProvider ────────────────────────────────────────────────── @@ -89,10 +239,27 @@ func (a *Adapter) Initialize(ctx context.Context, cfg map[string]any) error { return err } -// Capabilities calls the IaCProviderRequired.Capabilities RPC and translates -// the response to []interfaces.IaCCapabilityDeclaration. +// fetchCapabilities returns the plugin's CapabilitiesResponse, caching the +// first result for the adapter's lifetime via sync.Once. RPC errors are also +// cached so repeated accesses don't repeatedly fail against an unreachable +// plugin. Capabilities are advertised at plugin startup and don't change +// during a session; caching is correct and cheap. Mirrors typedIaCAdapter. +func (a *Adapter) fetchCapabilities() (*pb.CapabilitiesResponse, error) { + a.capsOnce.Do(func() { + resp, err := a.required.Capabilities(context.Background(), &pb.CapabilitiesRequest{}) + if err != nil { + a.capsErr = err + return + } + a.cachedCaps = resp + }) + return a.cachedCaps, a.capsErr +} + +// Capabilities calls the IaCProviderRequired.Capabilities RPC (cached) and +// translates the response to []interfaces.IaCCapabilityDeclaration. func (a *Adapter) Capabilities() []interfaces.IaCCapabilityDeclaration { - resp, err := a.required.Capabilities(context.Background(), &pb.CapabilitiesRequest{}) + resp, err := a.fetchCapabilities() if err != nil { return nil } @@ -142,13 +309,24 @@ func (a *Adapter) Status(ctx context.Context, resources []interfaces.ResourceRef return statusesFromPB(resp.GetStatuses()) } -// 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. +// DetectDrift routes through the optional IaCProviderDriftDetector gRPC service +// when the plugin advertised it (advertisement-gated in New). Falls back to +// ErrProviderMethodUnimplemented when the detector was not advertised — callers +// use errors.Is to skip or fall back to static drift logic. 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) + if a.drift == nil { + return nil, fmt.Errorf("%w: IaCProviderDriftDetector not advertised by plugin", + interfaces.ErrProviderMethodUnimplemented) + } + resp, err := a.drift.client.DetectDrift(ctx, &pb.DetectDriftRequest{Refs: refsToPB(resources)}) + if err != nil { + if status.Code(err) == codes.Unimplemented { + return nil, fmt.Errorf("%w: IaCProviderDriftDetector.DetectDrift not implemented", + interfaces.ErrProviderMethodUnimplemented) + } + return nil, err + } + return driftsFromPB(resp.GetDrifts()) } // Import calls IaCProviderRequired.Import. @@ -181,10 +359,12 @@ func (a *Adapter) ResourceDriver(resourceType string) (interfaces.ResourceDriver interfaces.ErrProviderMethodUnimplemented) } -// SupportedCanonicalKeys returns the keys from Capabilities, or the global -// canonical key set if the plugin doesn't declare a subset. +// SupportedCanonicalKeys returns the canonical keys from the cached +// CapabilitiesResponse, or the global set when the plugin doesn't declare a +// subset. Reuses fetchCapabilities so the Capabilities RPC is called at most +// once per adapter lifetime (per ADR-0029 / typedIaCAdapter precedent). func (a *Adapter) SupportedCanonicalKeys() []string { - resp, err := a.required.Capabilities(context.Background(), &pb.CapabilitiesRequest{}) + resp, err := a.fetchCapabilities() if err == nil && resp != nil { if keys := resp.GetCanonicalKeys(); len(keys) > 0 { return append([]string(nil), keys...) @@ -194,6 +374,9 @@ func (a *Adapter) SupportedCanonicalKeys() []string { } // BootstrapStateBackend calls IaCProviderRequired.BootstrapStateBackend. +// All three result fields (Bucket, Region, Endpoint) are mapped from the proto +// response — Endpoint carries the S3-compatible API URL (e.g. DO Spaces endpoint) +// that was silently dropped in the original implementation. func (a *Adapter) BootstrapStateBackend(ctx context.Context, cfg map[string]any) (*interfaces.BootstrapResult, error) { cfgJSON, err := marshalJSONMap(cfg) if err != nil { @@ -208,9 +391,10 @@ func (a *Adapter) BootstrapStateBackend(ctx context.Context, cfg map[string]any) return nil, nil } return &interfaces.BootstrapResult{ - Bucket: r.GetBucket(), - Region: r.GetRegion(), - EnvVars: copyStringMap(r.GetEnvVars()), + Bucket: r.GetBucket(), + Region: r.GetRegion(), + Endpoint: r.GetEndpoint(), + EnvVars: copyStringMap(r.GetEnvVars()), }, nil } @@ -220,37 +404,13 @@ func (a *Adapter) Close() error { return nil } -// ─── interfaces.IaCProviderRegionLister ───────────────────────────────────── - -// ListRegions calls the IaCProviderRegionLister.ListRegions RPC and returns -// region identifiers sorted by the server. If the plugin does not implement -// the service (gRPC Unimplemented) an ErrProviderMethodUnimplemented is returned -// so callers can fall back to a static catalog. -func (a *Adapter) ListRegions(ctx context.Context, env string) ([]string, error) { - resp, err := a.regionLister.ListRegions(ctx, &pb.ListRegionsRequest{EnvName: env}) - if err != nil { - if status.Code(err) == codes.Unimplemented { - return nil, fmt.Errorf("%w: IaCProviderRegionLister not registered by plugin", - interfaces.ErrProviderMethodUnimplemented) - } - return nil, err - } - regions := make([]string, 0, len(resp.GetRegions())) - for _, r := range resp.GetRegions() { - if name := r.GetName(); name != "" { - regions = append(regions, name) - } - } - return regions, nil -} - // ─── Proto↔interfaces conversion helpers ──────────────────────────────────── // // These mirror the same-named functions in cmd/wfctl/iac_typed_adapter.go // (package main, not importable by core). They are intentionally minimal — // only the subset required by the PR-1 methods above (Plan, Status, Destroy, -// ListRegions). Additional converters can be added as PR-2 (step impl) needs -// them. Do NOT import cmd/wfctl. +// DetectDrift, ListRegions). Additional converters can be added as PR-2 (step +// impl) needs them. Do NOT import cmd/wfctl. func marshalJSONMap(m map[string]any) ([]byte, error) { if m == nil { @@ -556,3 +716,41 @@ func planFromPB(p *pb.IaCPlan) (*interfaces.IaCPlan, error) { InputSnapshot: copyStringMap(p.GetInputSnapshot()), }, nil } + +// driftsFromPB translates []*pb.DriftResult to []interfaces.DriftResult. +func driftsFromPB(drifts []*pb.DriftResult) ([]interfaces.DriftResult, error) { + out := make([]interfaces.DriftResult, 0, len(drifts)) + for _, d := range drifts { + expected, err := unmarshalJSONMap(d.GetExpectedJson()) + if err != nil { + return nil, err + } + actual, err := unmarshalJSONMap(d.GetActualJson()) + if err != nil { + return nil, err + } + out = append(out, interfaces.DriftResult{ + Name: d.GetName(), + Type: d.GetType(), + Drifted: d.GetDrifted(), + Class: driftClassFromPB(d.GetClass()), + Expected: expected, + Actual: actual, + Fields: append([]string(nil), d.GetFields()...), + }) + } + return out, nil +} + +func driftClassFromPB(c pb.DriftClass) interfaces.DriftClass { + switch c { + case pb.DriftClass_DRIFT_CLASS_IN_SYNC: + return interfaces.DriftClassInSync + case pb.DriftClass_DRIFT_CLASS_GHOST: + return interfaces.DriftClassGhost + case pb.DriftClass_DRIFT_CLASS_CONFIG: + return interfaces.DriftClassConfig + default: + return interfaces.DriftClassUnknown + } +} diff --git a/iac/providerclient/adapter_test.go b/iac/providerclient/adapter_test.go index 35053ea4..bbbbf6fb 100644 --- a/iac/providerclient/adapter_test.go +++ b/iac/providerclient/adapter_test.go @@ -16,9 +16,13 @@ import ( "google.golang.org/grpc/test/bufconn" ) -// Compile-time interface assertions — the primary guard for Task 2. +// Compile-time guard: *Adapter must satisfy interfaces.IaCProvider. var _ interfaces.IaCProvider = (*providerclient.Adapter)(nil) -var _ interfaces.IaCProviderRegionLister = (*providerclient.Adapter)(nil) + +// *Adapter must NOT unconditionally satisfy interfaces.IaCProviderRegionLister — +// that interface is gated behind the RegionListerProvider accessor. The +// negative assertion is enforced by TestAdapter_RegionLister_Unadvertised below +// (runtime type-assert on a freshly-built adapter with no advertised services). // ─── In-proc fake gRPC servers ────────────────────────────────────────────── @@ -87,8 +91,15 @@ func (s *fakeRequiredServer) ResolveSizing(_ context.Context, _ *pb.ResolveSizin return nil, status.Error(codes.Unimplemented, "not implemented in fake") } -func (s *fakeRequiredServer) BootstrapStateBackend(_ context.Context, _ *pb.BootstrapStateBackendRequest) (*pb.BootstrapStateBackendResponse, error) { - return nil, status.Error(codes.Unimplemented, "not implemented in fake") +func (s *fakeRequiredServer) BootstrapStateBackend(_ context.Context, req *pb.BootstrapStateBackendRequest) (*pb.BootstrapStateBackendResponse, error) { + return &pb.BootstrapStateBackendResponse{ + Result: &pb.BootstrapResult{ + Bucket: "my-bucket", + Region: "us-east-1", + Endpoint: "https://nyc3.digitaloceanspaces.com", + EnvVars: map[string]string{"BUCKET": "my-bucket"}, + }, + }, nil } // fakeRegionListerServer implements IaCProviderRegionListerServer. @@ -105,6 +116,24 @@ func (s *fakeRegionListerServer) ListRegions(_ context.Context, req *pb.ListRegi }, nil } +// fakeDriftDetectorServer implements IaCProviderDriftDetectorServer. +type fakeDriftDetectorServer struct { + pb.UnimplementedIaCProviderDriftDetectorServer +} + +func (s *fakeDriftDetectorServer) DetectDrift(_ context.Context, req *pb.DetectDriftRequest) (*pb.DetectDriftResponse, error) { + results := make([]*pb.DriftResult, 0, len(req.GetRefs())) + for _, r := range req.GetRefs() { + results = append(results, &pb.DriftResult{ + Name: r.GetName(), + Type: r.GetType(), + Drifted: false, + Class: pb.DriftClass_DRIFT_CLASS_IN_SYNC, + }) + } + return &pb.DetectDriftResponse{Drifts: results}, nil +} + // ─── Test setup helper ─────────────────────────────────────────────────────── // startFakeServer starts an in-process gRPC server on a bufconn listener and @@ -135,10 +164,9 @@ func startFakeServer(t *testing.T, srv *grpc.Server) *grpc.ClientConn { func TestAdapter_Name(t *testing.T) { srv := grpc.NewServer() pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) - pb.RegisterIaCProviderRegionListerServer(srv, &fakeRegionListerServer{}) conn := startFakeServer(t, srv) - a := providerclient.New(conn) + a := providerclient.New(conn, nil) if got := a.Name(); got != "fake-provider" { t.Errorf("Name() = %q, want %q", got, "fake-provider") } @@ -148,10 +176,9 @@ func TestAdapter_Name(t *testing.T) { func TestAdapter_Plan(t *testing.T) { srv := grpc.NewServer() pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) - pb.RegisterIaCProviderRegionListerServer(srv, &fakeRegionListerServer{}) conn := startFakeServer(t, srv) - a := providerclient.New(conn) + a := providerclient.New(conn, nil) plan, err := a.Plan(context.Background(), nil, nil) if err != nil { t.Fatalf("Plan() returned error: %v", err) @@ -171,10 +198,9 @@ func TestAdapter_Plan(t *testing.T) { func TestAdapter_Destroy(t *testing.T) { srv := grpc.NewServer() pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) - pb.RegisterIaCProviderRegionListerServer(srv, &fakeRegionListerServer{}) conn := startFakeServer(t, srv) - a := providerclient.New(conn) + a := providerclient.New(conn, nil) refs := []interfaces.ResourceRef{{Name: "vpc-1", Type: "infra.vpc", ProviderID: "vpc-abc"}} result, err := a.Destroy(context.Background(), refs) if err != nil { @@ -192,10 +218,9 @@ func TestAdapter_Destroy(t *testing.T) { func TestAdapter_Status(t *testing.T) { srv := grpc.NewServer() pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) - pb.RegisterIaCProviderRegionListerServer(srv, &fakeRegionListerServer{}) conn := startFakeServer(t, srv) - a := providerclient.New(conn) + a := providerclient.New(conn, nil) refs := []interfaces.ResourceRef{{Name: "vpc-1", Type: "infra.vpc", ProviderID: "vpc-abc"}} statuses, err := a.Status(context.Background(), refs) if err != nil { @@ -209,22 +234,90 @@ func TestAdapter_Status(t *testing.T) { } } -// TestAdapter_ListRegions verifies ListRegions() delegates to IaCProviderRegionLister. -func TestAdapter_ListRegions(t *testing.T) { +// TestAdapter_BootstrapStateBackend_Endpoint verifies that the Endpoint field +// is correctly mapped from the proto response (regression for the silent drop +// of DO Spaces endpoint). +func TestAdapter_BootstrapStateBackend_Endpoint(t *testing.T) { + srv := grpc.NewServer() + pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) + conn := startFakeServer(t, srv) + + a := providerclient.New(conn, nil) + result, err := a.BootstrapStateBackend(context.Background(), nil) + if err != nil { + t.Fatalf("BootstrapStateBackend() returned error: %v", err) + } + if result == nil { + t.Fatal("BootstrapStateBackend() returned nil result") + } + if result.Bucket != "my-bucket" { + t.Errorf("Bucket = %q, want %q", result.Bucket, "my-bucket") + } + if result.Region != "us-east-1" { + t.Errorf("Region = %q, want %q", result.Region, "us-east-1") + } + if result.Endpoint != "https://nyc3.digitaloceanspaces.com" { + t.Errorf("Endpoint = %q, want %q", result.Endpoint, "https://nyc3.digitaloceanspaces.com") + } + if result.EnvVars["BUCKET"] != "my-bucket" { + t.Errorf("EnvVars[BUCKET] = %q, want %q", result.EnvVars["BUCKET"], "my-bucket") + } +} + +// TestAdapter_Capabilities_Cached verifies that fetchCapabilities caches the +// result (a second call doesn't issue a second RPC — verified by checking the +// returned data is consistent). +func TestAdapter_Capabilities_Cached(t *testing.T) { + srv := grpc.NewServer() + pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) + conn := startFakeServer(t, srv) + + a := providerclient.New(conn, nil) + // Two calls to Capabilities() should return equal results. + caps1 := a.Capabilities() + caps2 := a.Capabilities() + if len(caps1) != len(caps2) { + t.Errorf("Capabilities() not consistent across calls: %d vs %d", len(caps1), len(caps2)) + } + // SupportedCanonicalKeys shares the same cache path. + keys := a.SupportedCanonicalKeys() + if len(keys) == 0 { + t.Error("SupportedCanonicalKeys() returned empty slice") + } +} + +// ─── RegionLister advertisement-gating ────────────────────────────────────── + +// TestAdapter_RegionLister_Advertised verifies that when IaCServiceRegionLister +// is in advertisedServices, RegionLister() returns a non-nil object and ListRegions +// works end-to-end. +func TestAdapter_RegionLister_Advertised(t *testing.T) { srv := grpc.NewServer() pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) pb.RegisterIaCProviderRegionListerServer(srv, &fakeRegionListerServer{}) conn := startFakeServer(t, srv) - a := providerclient.New(conn) - regions, err := a.ListRegions(context.Background(), "prod") + a := providerclient.New(conn, map[string]bool{ + providerclient.IaCServiceRegionLister: true, + }) + + // Must satisfy RegionListerProvider. + rlp, ok := any(a).(providerclient.RegionListerProvider) + if !ok { + t.Fatal("*Adapter must satisfy RegionListerProvider when IaCServiceRegionLister advertised") + } + rl := rlp.RegionLister() + if rl == nil { + t.Fatal("RegionLister() returned nil when service was advertised") + } + + regions, err := rl.ListRegions(context.Background(), "prod") if err != nil { t.Fatalf("ListRegions() returned error: %v", err) } if len(regions) == 0 { t.Error("ListRegions() returned no regions") } - // Verify region names round-trip correctly. found := false for _, r := range regions { if r == "us-east-1" { @@ -237,43 +330,135 @@ func TestAdapter_ListRegions(t *testing.T) { } } +// TestAdapter_RegionLister_Unadvertised verifies the KEY negative path: when +// IaCServiceRegionLister is NOT in advertisedServices, RegionLister() returns +// nil, and *Adapter does NOT satisfy interfaces.IaCProviderRegionLister directly +// (the catalog step's static fallback must be reachable). +func TestAdapter_RegionLister_Unadvertised(t *testing.T) { + srv := grpc.NewServer() + pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) + // NOTE: IaCProviderRegionLister is deliberately NOT registered on the server. + conn := startFakeServer(t, srv) + + // Adapter built with NO advertised services. + a := providerclient.New(conn, nil) + + // *Adapter must NOT directly satisfy IaCProviderRegionLister. + if _, ok := any(a).(interfaces.IaCProviderRegionLister); ok { + t.Fatal("*Adapter must NOT satisfy interfaces.IaCProviderRegionLister when unadvertised — " + + "the catalog step's static fallback can never fire if it always type-asserts true") + } + + // RegionListerProvider type-assert must succeed (the accessor interface is + // always present on *Adapter), but RegionLister() must return nil. + rlp, ok := any(a).(providerclient.RegionListerProvider) + if !ok { + t.Fatal("*Adapter must satisfy RegionListerProvider regardless of advertisement") + } + if rl := rlp.RegionLister(); rl != nil { + t.Errorf("RegionLister() returned non-nil when IaCServiceRegionLister was not advertised; got %T", rl) + } + + // DetectDrift on unadvertised adapter must return ErrProviderMethodUnimplemented. + _, err := a.DetectDrift(context.Background(), nil) + if err == nil { + t.Fatal("DetectDrift() on unadvertised adapter must return error") + } + if !isUnimplemented(err) { + t.Errorf("DetectDrift() error = %v, want ErrProviderMethodUnimplemented", err) + } +} + +// TestAdapter_DriftDetector_Advertised verifies that when IaCServiceDriftDetector +// is in advertisedServices, DriftDetector() returns a non-nil DriftConfigDetector +// and DetectDrift routes through the optional service. +func TestAdapter_DriftDetector_Advertised(t *testing.T) { + srv := grpc.NewServer() + pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) + pb.RegisterIaCProviderDriftDetectorServer(srv, &fakeDriftDetectorServer{}) + conn := startFakeServer(t, srv) + + a := providerclient.New(conn, map[string]bool{ + providerclient.IaCServiceDriftDetector: true, + }) + + // DriftDetectorProvider accessor. + ddp, ok := any(a).(providerclient.DriftDetectorProvider) + if !ok { + t.Fatal("*Adapter must satisfy DriftDetectorProvider when IaCServiceDriftDetector advertised") + } + dd := ddp.DriftDetector() + if dd == nil { + t.Fatal("DriftDetector() returned nil when service was advertised") + } + + // DetectDrift routes through the optional service. + refs := []interfaces.ResourceRef{{Name: "vpc-1", Type: "infra.vpc", ProviderID: "vpc-abc"}} + drifts, err := a.DetectDrift(context.Background(), refs) + if err != nil { + t.Fatalf("DetectDrift() returned error: %v", err) + } + if len(drifts) != 1 { + t.Fatalf("DetectDrift() returned %d drifts, want 1", len(drifts)) + } + if drifts[0].Name != "vpc-1" { + t.Errorf("drift.Name = %q, want %q", drifts[0].Name, "vpc-1") + } +} + +// TestAdapter_DriftDetector_Unadvertised verifies the negative path for +// DriftDetector: when IaCServiceDriftDetector is NOT advertised, DriftDetector() +// returns nil. +func TestAdapter_DriftDetector_Unadvertised(t *testing.T) { + srv := grpc.NewServer() + pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) + conn := startFakeServer(t, srv) + + a := providerclient.New(conn, nil) + + ddp, ok := any(a).(providerclient.DriftDetectorProvider) + if !ok { + t.Fatal("*Adapter must satisfy DriftDetectorProvider regardless of advertisement") + } + if dd := ddp.DriftDetector(); dd != nil { + t.Errorf("DriftDetector() returned non-nil when IaCServiceDriftDetector was not advertised; got %T", dd) + } +} + // TestAdapter_TypeAssertIaCProvider verifies Adapter satisfies interfaces.IaCProvider. -// The compile-time assertion (var _ interfaces.IaCProvider = (*Adapter)(nil)) at the -// top of this file is the load-bearing check; this runtime test verifies New() returns -// a non-nil adapter that is usable as the interface. func TestAdapter_TypeAssertIaCProvider(t *testing.T) { srv := grpc.NewServer() pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) - pb.RegisterIaCProviderRegionListerServer(srv, &fakeRegionListerServer{}) conn := startFakeServer(t, srv) - a := providerclient.New(conn) + a := providerclient.New(conn, nil) if a == nil { t.Fatal("providerclient.New returned nil") } - // Verify the name RPC works through the interfaces.IaCProvider method. var p interfaces.IaCProvider = a if got := p.Name(); got != "fake-provider" { t.Errorf("p.Name() via interface = %q, want fake-provider", got) } } -// TestAdapter_TypeAssertIaCProviderRegionLister verifies Adapter satisfies -// interfaces.IaCProviderRegionLister so step.iac_provider_catalog can -// type-assert it. -func TestAdapter_TypeAssertIaCProviderRegionLister(t *testing.T) { - srv := grpc.NewServer() - pb.RegisterIaCProviderRequiredServer(srv, &fakeRequiredServer{}) - pb.RegisterIaCProviderRegionListerServer(srv, &fakeRegionListerServer{}) - conn := startFakeServer(t, srv) +// isUnimplemented reports whether err wraps interfaces.ErrProviderMethodUnimplemented. +func isUnimplemented(err error) bool { + return err != nil && (err.Error() != "" && containsUnimplemented(err)) +} - a := providerclient.New(conn) - var p interfaces.IaCProvider = a - rl, ok := p.(interfaces.IaCProviderRegionLister) - if !ok { - t.Fatal("Adapter must satisfy interfaces.IaCProviderRegionLister") - } - if rl == nil { - t.Fatal("IaCProviderRegionLister type-assert returned nil") +func containsUnimplemented(err error) bool { + // Use errors.Is-style unwrap check by walking the chain. + target := interfaces.ErrProviderMethodUnimplemented + type unwrapper interface{ Unwrap() error } + for err != nil { + if err == target { + return true + } + uw, ok := err.(unwrapper) + if !ok { + break + } + err = uw.Unwrap() } + return false } diff --git a/plugin/external/adapter.go b/plugin/external/adapter.go index e34b1327..97ca6dda 100644 --- a/plugin/external/adapter.go +++ b/plugin/external/adapter.go @@ -587,6 +587,16 @@ var iacProviderRequiredServiceName = pb.IaCProviderRequired_ServiceDesc.ServiceN // advertisesIaCProviderRequiredService reports whether the adapter's // ContractRegistry carries a CONTRACT_KIND_SERVICE descriptor for the // IaCProviderRequired service. Mirrors advertisesIaCStateBackendService. +// +// diskManifest fallback rationale: the ContractRegistry is the live, authoritative +// source for service advertisement. The diskManifest.IaCServices fallback is +// intentionally permissive — it enables wiring when GetContractRegistry returned +// Unimplemented (strict-cutover IaC plugins). This is unlike state-backend +// detection (which fails loudly when the registry says NO but the manifest says +// YES) because IaC providers are registered as a service in the DI graph, not +// gated by a per-RPC live cross-check. The contract: if diskManifest says the +// plugin serves IaCProviderRequired, we trust it at wiring time and let the +// actual gRPC calls fail loudly at runtime if the service isn't there. func (a *ExternalPluginAdapter) advertisesIaCProviderRequiredService() bool { if a.contractRegistry == nil { return false @@ -601,8 +611,8 @@ func (a *ExternalPluginAdapter) advertisesIaCProviderRequiredService() bool { } // Also check diskManifest.IaCServices as a fallback — the plugin may declare // the service there without advertising it in the ContractRegistry (e.g. when - // GetContractRegistry returned Unimplemented). This mirrors the check in - // IaCStateBackendClients which cross-checks against diskManifest. + // GetContractRegistry returned Unimplemented). See the fallback rationale in + // the doc comment above. if a.diskManifest != nil { for _, svc := range a.diskManifest.IaCServices { if svc == iacProviderRequiredServiceName { @@ -613,6 +623,35 @@ func (a *ExternalPluginAdapter) advertisesIaCProviderRequiredService() bool { return false } +// advertisedOptionalIaCServices returns the set of optional IaC service names +// that the plugin's ContractRegistry has explicitly advertised. These are the +// names passed to providerclient.New so it can gate optional gRPC client +// construction on advertisement (mirroring newTypedIaCAdapter's registered map). +// +// Only services from the ContractRegistry are included — diskManifest is not +// consulted for optional services because the optional-service guard is +// advertisement-only (no fallback); an unadvertised optional service simply +// means the capability is absent. +func (a *ExternalPluginAdapter) advertisedOptionalIaCServices() map[string]bool { + out := make(map[string]bool) + if a.contractRegistry == nil { + return out + } + for _, d := range a.contractRegistry.Contracts { + if d == nil { + continue + } + if d.Kind != pb.ContractKind_CONTRACT_KIND_SERVICE { + continue + } + switch d.ServiceName { + case providerclient.IaCServiceRegionLister, providerclient.IaCServiceDriftDetector: + out[d.ServiceName] = true + } + } + return out +} + // WiringHooks returns a WiringHook that registers the plugin as an // interfaces.IaCProvider service in the modular application DI graph when this // plugin advertises the IaCProviderRequired gRPC service. @@ -625,6 +664,13 @@ func (a *ExternalPluginAdapter) advertisesIaCProviderRequiredService() bool { // // If the plugin does not advertise IaCProviderRequired this method returns nil // (no wiring needed — not an iac.provider plugin). +// +// Optional services (IaCProviderRegionLister, IaCProviderDriftDetector) are +// collected from the ContractRegistry and forwarded to providerclient.New. +// Optional gRPC clients are constructed only for advertised services, so the +// adapter's capability accessors (RegionLister(), DriftDetector()) return nil +// for unadvertised services — ensuring catalog steps' static-fallback paths +// remain reachable. func (a *ExternalPluginAdapter) WiringHooks() []plugin.WiringHook { if !a.advertisesIaCProviderRequiredService() { return nil @@ -634,6 +680,12 @@ func (a *ExternalPluginAdapter) WiringHooks() []plugin.WiringHook { // guard handles that. name := a.name conn := a.Conn() + // Collect optional service advertisements from the ContractRegistry now + // (before the hook closure runs) so the providerclient.New call inside the + // hook receives a stable snapshot. advertisedOptionalIaCServices never + // consults diskManifest — only ContractRegistry entries count for optional + // service gating. + optionalServices := a.advertisedOptionalIaCServices() return []plugin.WiringHook{ { // Name follows the convention "-iac-provider-registration". @@ -646,7 +698,7 @@ func (a *ExternalPluginAdapter) WiringHooks() []plugin.WiringHook { if conn == nil { return fmt.Errorf("plugin %q advertises IaCProviderRequired but has no gRPC connection", name) } - adapter := providerclient.New(conn) + adapter := providerclient.New(conn, optionalServices) if err := app.RegisterService(name, adapter); err != nil { return fmt.Errorf("plugin %q: register IaCProvider service: %w", name, err) }