Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions cmd/wfctl/iac_typed_adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"io"
"log"
"math"
"sort"
"strings"
"time"

Expand All @@ -53,6 +54,7 @@ const (
iacServiceEnumerator = "workflow.plugin.external.iac.IaCProviderEnumerator"
iacServiceDriftDetector = "workflow.plugin.external.iac.IaCProviderDriftDetector"
iacServiceCredentialRevoker = "workflow.plugin.external.iac.IaCProviderCredentialRevoker"
iacServiceRegionLister = "workflow.plugin.external.iac.IaCProviderRegionLister"
iacServiceMigrationRepairer = "workflow.plugin.external.iac.IaCProviderMigrationRepairer"
iacServiceValidator = "workflow.plugin.external.iac.IaCProviderValidator"
iacServiceDriftConfigDetect = "workflow.plugin.external.iac.IaCProviderDriftConfigDetector"
Expand Down Expand Up @@ -80,6 +82,7 @@ type typedIaCAdapter struct {
enumerator pb.IaCProviderEnumeratorClient
drift pb.IaCProviderDriftDetectorClient
revoker pb.IaCProviderCredentialRevokerClient
regionLister pb.IaCProviderRegionListerClient
repairer pb.IaCProviderMigrationRepairerClient
validator pb.IaCProviderValidatorClient
driftCfg pb.IaCProviderDriftConfigDetectorClient
Expand Down Expand Up @@ -115,6 +118,9 @@ func newTypedIaCAdapter(conn *grpc.ClientConn, registered map[string]bool) *type
if registered[iacServiceCredentialRevoker] {
a.revoker = pb.NewIaCProviderCredentialRevokerClient(conn)
}
if registered[iacServiceRegionLister] {
a.regionLister = pb.NewIaCProviderRegionListerClient(conn)
}
if registered[iacServiceMigrationRepairer] {
a.repairer = pb.NewIaCProviderMigrationRepairerClient(conn)
}
Expand Down Expand Up @@ -221,6 +227,37 @@ func (a *typedIaCAdapter) CredentialRevoker() pb.IaCProviderCredentialRevokerCli
return a.revoker
}

// RegionLister returns the typed pb.IaCProviderRegionListerClient or nil
// when the plugin did not register IaCProviderRegionLister. Used by
// infra-admin provider metadata to prefer provider-sourced region lists
// over the host's local catalog.
func (a *typedIaCAdapter) RegionLister() pb.IaCProviderRegionListerClient {
return a.regionLister
}

// ListProviderRegions queries the optional region lister and returns sorted
// provider region identifiers. The display name is intentionally ignored here
// because infra-admin's current response shape carries region IDs only.
func (a *typedIaCAdapter) ListProviderRegions(ctx context.Context, envName string) ([]string, error) {
if a.regionLister == nil {
return nil, unimplementedOptional(iacServiceRegionLister)
}
Comment thread
intel352 marked this conversation as resolved.
resp, err := a.regionLister.ListRegions(ctx, &pb.ListRegionsRequest{EnvName: envName})
if err != nil {
return nil, translateRPCErr(err)
}
regions := make([]string, 0, len(resp.GetRegions()))
for _, region := range resp.GetRegions() {
name := strings.TrimSpace(region.GetName())
if name == "" {
continue
}
regions = append(regions, name)
}
sort.Strings(regions)
return regions, nil
}

// MigrationRepairer returns the typed
// pb.IaCProviderMigrationRepairerClient or nil when the plugin did not
// register IaCProviderMigrationRepairer.
Expand Down
59 changes: 59 additions & 0 deletions cmd/wfctl/iac_typed_adapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ func TestTypedAdapter_OptionalReturnsUnimplementedSentinel(t *testing.T) {
{"RevokeProviderCredential", func() error {
return a.RevokeProviderCredential(context.Background(), "digitalocean.spaces", "key-1")
}},
{"ListProviderRegions", func() error {
_, err := a.ListProviderRegions(context.Background(), "staging")
return err
}},
{"RepairDirtyMigration", func() error {
_, err := a.RepairDirtyMigration(context.Background(), interfaces.MigrationRepairRequest{})
return err
Expand Down Expand Up @@ -396,6 +400,61 @@ func TestTypedAdapter_Finalizer_NilWhenNotRegistered(t *testing.T) {
}
}

func TestTypedAdapter_RegionLister_PopulatedWhenRegistered(t *testing.T) {
conn := dialLazyConn(t)
adapter := newTypedIaCAdapter(conn, map[string]bool{
iacServiceRegionLister: true,
})
if adapter.RegionLister() == nil {
t.Error("RegionLister() returned nil when IaCProviderRegionLister is in registered set")
}
}

func TestTypedAdapter_RegionLister_NilWhenNotRegistered(t *testing.T) {
conn := dialLazyConn(t)
adapter := newTypedIaCAdapter(conn, map[string]bool{
iacServiceEnumerator: true,
})
if adapter.RegionLister() != nil {
t.Error("RegionLister() returned non-nil when IaCProviderRegionLister not registered")
}
}

func TestTypedAdapter_ListProviderRegions(t *testing.T) {
adapter := fixtureTypedAdapter{
RegionLister: regionListerStub{},
}.build(t)

got, err := adapter.ListProviderRegions(context.Background(), "staging")
if err != nil {
t.Fatalf("ListProviderRegions: %v", err)
}
want := []string{"nyc3", "sfo3"}
if len(got) != len(want) {
t.Fatalf("regions = %v, want %v", got, want)
}
for i := range want {
if got[i] != want[i] {
t.Fatalf("regions = %v, want %v", got, want)
}
}
}

type regionListerStub struct {
pb.UnimplementedIaCProviderRegionListerServer
}

func (regionListerStub) ListRegions(ctx context.Context, req *pb.ListRegionsRequest) (*pb.ListRegionsResponse, error) {
if req.GetEnvName() != "staging" {
return nil, status.Errorf(codes.InvalidArgument, "env = %q, want staging", req.GetEnvName())
}
return &pb.ListRegionsResponse{Regions: []*pb.ProviderRegion{
{Name: "sfo3"},
{Name: "nyc3"},
{Name: ""},
}}, nil
}

// dialLazyConn returns a real *grpc.ClientConn pointing at an in-process
// gRPC server with zero services registered. Used by adapter field-wiring
// tests that need newTypedIaCAdapter's `pb.NewXxxClient(conn)` calls to
Expand Down
5 changes: 5 additions & 0 deletions cmd/wfctl/iac_typed_fixture_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ type fixtureTypedAdapter struct {
Enumerator pb.IaCProviderEnumeratorServer
DriftDetector pb.IaCProviderDriftDetectorServer
CredentialRevoker pb.IaCProviderCredentialRevokerServer
RegionLister pb.IaCProviderRegionListerServer
MigrationRepairer pb.IaCProviderMigrationRepairerServer
Validator pb.IaCProviderValidatorServer
DriftConfigDetect pb.IaCProviderDriftConfigDetectorServer
Expand Down Expand Up @@ -131,6 +132,10 @@ func (f fixtureTypedAdapter) build(t *testing.T) *typedIaCAdapter {
pb.RegisterIaCProviderCredentialRevokerServer(server, f.CredentialRevoker)
registered[iacServiceCredentialRevoker] = true
}
if f.RegionLister != nil {
pb.RegisterIaCProviderRegionListerServer(server, f.RegionLister)
registered[iacServiceRegionLister] = true
}
if f.MigrationRepairer != nil {
pb.RegisterIaCProviderMigrationRepairerServer(server, f.MigrationRepairer)
registered[iacServiceMigrationRepairer] = true
Expand Down
122 changes: 122 additions & 0 deletions docs/plans/2026-06-01-iac-provider-region-lister.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
# IaC Provider Region Lister Implementation Plan

> **For the implementing agent:** REQUIRED SUB-SKILL: Use autodev:executing-plans to implement this plan task-by-task.

**Goal:** Add the optional `IaCProviderRegionLister` gRPC contract and make infra-admin use provider-sourced regions when advertised, falling back to the local catalog otherwise.

**Architecture:** Follow the existing optional IaC service pattern: additive proto service, SDK auto-registration by Go type assertion, typed wfctl adapter gated by ContractRegistry, and infra-admin handler probing an optional host-side interface. Absence or failure of the optional service keeps the current `local-catalog` behavior.

**Tech Stack:** Go 1.26, protobuf/gRPC via `buf generate`, Workflow plugin SDK, `wfctl` typed IaC adapter, infra-admin handler.

**Base branch:** main

---

## Scope Manifest

**PR Count:** 1
**Tasks:** 4
**Estimated Lines of Change:** ~600 including generated protobuf files

**Out of scope:**
- Cloud provider plugin implementations; they need a follow-up cascade after this core contract PR lands.
- Changes to `workflow-plugin-infra`; it does not own cloud credentials or the engine/plugin contract.
- New UI fields beyond existing `supported_regions` and `regions_source`.

**PR Grouping:**

| PR # | Title | Tasks | Branch |
|------|-------|-------|--------|
| 1 | Add IaCProviderRegionLister contract and infra-admin consumer | Task 1, Task 2, Task 3, Task 4 | feat/813-iac-provider-region-lister |

**Status:** Draft

## Global Design Guidance

Source: `AGENTS.md`, `docs/AGENT_GUIDE.md`, `docs/REPO_LAYOUT.md`, `decisions/0025-iac-optional-method-typed-services-not-bool.md`

| Guidance | Plan response |
|---|---|
| Core owns shared contracts; plugins own provider-specific runtime integrations. | Add the shared proto/SDK/adapter contract in workflow only; defer cloud SDK implementations to provider repos. |
| Optional IaC capabilities are advertised by ContractRegistry; absence is the negative signal. | Region lister is an optional service registered only when providers implement it. |
| Use `GOWORK=off` and focused tests first. | Verification starts with proto/SDK/adapter/handler tests, then broadens to package tests and lint. |
| Do not add scratch example/test directories. | No new root directories; only source, tests, generated proto, and this plan. |

## Task 1: Proto Contract

**Files:**
- Modify: `plugin/external/proto/iac.proto`
- Regenerate: `plugin/external/proto/iac.pb.go`
- Regenerate: `plugin/external/proto/iac_grpc.pb.go`
- Test: `plugin/external/proto/iac_proto_test.go`

**Steps:**
1. Add optional service `IaCProviderRegionLister` with `ListRegions(ListRegionsRequest) returns (ListRegionsResponse)`.
2. Add strict typed messages with scalar fields only: `ListRegionsRequest { string env_name = 1; }`, `ProviderRegion { string name = 1; string display_name = 2; }`, and `ListRegionsResponse { repeated ProviderRegion regions = 1; }`.
3. Run `buf generate` from repo root.
4. Add/extend compile-time tests proving the service exists and remains optional.

**Verification:**
- `GOWORK=off go test ./plugin/external/proto -run 'TestIaCProto|TestRegion' -count=1`
- Expected: package passes and generated Go exposes `pb.IaCProviderRegionListerServer`.

**Rollback:** Revert the proto + generated files; provider plugins keep using local catalog fallback.

## Task 2: SDK Registration and Contract Advertisement

**Files:**
- Modify: `plugin/external/sdk/iacserver.go`
- Modify: `plugin/external/sdk/contracts_iac_test.go`
- Modify as needed: `plugin/external/sdk/iacserver_test.go`

**Steps:**
1. Add `pb.IaCProviderRegionListerServer` to the optional service list in SDK docs/comments.
2. Register it in `registerIaCServicesOnly` via the existing type-assertion pattern.
3. Add a contract registry test proving the service is advertised when implemented and absent when not implemented.

**Verification:**
- `GOWORK=off go test ./plugin/external/sdk -run 'TestBuildContractRegistry|TestRegisterAllIaCProviderServices' -count=1`
- Expected: service descriptor includes `workflow.plugin.external.iac.IaCProviderRegionLister` only for implementing providers.

**Rollback:** Revert SDK registration; plugins can still compile with the proto but the host will not discover the service.

## Task 3: wfctl Typed Adapter

**Files:**
- Modify: `cmd/wfctl/iac_typed_adapter.go`
- Modify: `cmd/wfctl/iac_typed_adapter_test.go`

**Steps:**
1. Add `iacServiceRegionLister` constant and gated client construction in `newTypedIaCAdapter`.
2. Add `RegionLister()` accessor and a small `ListProviderRegions(ctx, envName)` helper returning sorted region names or `interfaces.ErrProviderMethodUnimplemented` when not advertised.
3. Add tests for advertised and absent region lister behavior.

**Verification:**
- `GOWORK=off go test ./cmd/wfctl -run 'TestTypedIaCAdapter.*Region|TestNewTypedIaCAdapter' -count=1`
- Expected: advertised service returns provider regions; absent service returns the existing unimplemented sentinel.

**Rollback:** Revert adapter changes; infra-admin falls back to local catalog.

## Task 4: Infra-Admin Consumer

**Files:**
- Modify: `iac/admin/handler/list_providers.go`
- Modify: `iac/admin/handler/list_providers_test.go`
- Modify: `iac/admin/proto/infra_admin.proto`
- Regenerate if proto comments or fields change: `iac/admin/proto/infra_admin.pb.go`
- Modify docs if needed: `docs/WFCTL.md`

**Steps:**
1. Add a narrow handler-local interface for providers that can list regions.
2. In `ListProviders`, call the optional lister when available. On nil error, use provider regions and set `regions_source = "provider-lister"`; on absence or error, keep existing `local-catalog` fallback.
3. Update tests for provider-sourced regions, error fallback, and existing local-catalog behavior.
4. Update comments/docs to remove the “future v1.1” wording.

**Verification:**
- `GOWORK=off go test ./iac/admin/handler -run 'TestListProviders' -count=1`
- `GOWORK=off go test ./cmd/wfctl -run 'TestInfraAdminCLI_ListProvidersOutput_RoundTrip' -count=1`
- `GOWORK=off go test ./plugin/external/proto ./plugin/external/sdk ./iac/admin/handler ./cmd/wfctl -count=1`
- `GOWORK=off golangci-lint run ./plugin/external/... ./iac/admin/... ./cmd/wfctl`
- Expected: all tests/lint pass; local catalog behavior remains unchanged when the optional service is absent.

**Rollback:** Revert handler + docs; infra-admin returns to host local catalog without schema migration.
38 changes: 33 additions & 5 deletions iac/admin/handler/list_providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,17 @@ package handler
import (
"context"
"sort"
"strings"

"github.com/GoCodeAlone/workflow/iac/admin/catalog"
adminpb "github.com/GoCodeAlone/workflow/iac/admin/proto"
"github.com/GoCodeAlone/workflow/interfaces"
)

type providerRegionLister interface {
ListProviderRegions(context.Context, string) ([]string, error)
}

// ListProviders implements InfraAdminService.ListProviders by
// walking the provided `providers` map (keyed by host YAML module
// name) and emitting one AdminProviderSummary per entry. The
Expand Down Expand Up @@ -44,9 +49,9 @@ import (
// fix requires providerTypeByModule. Final shape is 7 params;
// design line 233 was underspecified.
//
// regions_source is the literal "local-catalog" per design §FieldSpec
// Catalog so consumers can distinguish v1's local lookup from a
// future v1.1 IaCProviderRegionLister gRPC service.
// regions_source is "provider-lister" when the provider advertises and
// successfully serves IaCProviderRegionLister; otherwise it remains the
// literal "local-catalog" fallback.
//
// Per design §Authz: default-deny via the shared authz guard.
func ListProviders(
Expand Down Expand Up @@ -81,15 +86,38 @@ func ListProviders(
out := &adminpb.AdminListProvidersOutput{}
for _, modName := range moduleNames {
providerType := providerTypeByModule[modName] // may be "" if Init didn't populate
supportedRegions := regionCat.For(providerType)
regionsSource := "local-catalog"
if lister, ok := providers[modName].(providerRegionLister); ok {
if regions, err := lister.ListProviderRegions(ctx, in.GetEnvName()); err == nil {
supportedRegions = normalizeProviderRegions(regions)
regionsSource = "provider-lister"
}
}
summary := &adminpb.AdminProviderSummary{
ModuleName: modName,
ProviderType: providerType,
SupportedRegions: regionCat.For(providerType),
SupportedRegions: supportedRegions,
SupportedEngines: engineCat.For(providerType),
SupportedTypes: append([]string(nil), allTypes...),
RegionsSource: "local-catalog",
RegionsSource: regionsSource,
}
out.Providers = append(out.Providers, summary)
}
return out, nil
}

func normalizeProviderRegions(regions []string) []string {
out := make([]string, 0, len(regions))
seen := make(map[string]bool, len(regions))
for _, region := range regions {
region = strings.TrimSpace(region)
if region == "" || seen[region] {
continue
}
seen[region] = true
out = append(out, region)
}
sort.Strings(out)
return out
}
Loading
Loading