From 05b2bc9ff28bea2f6b2b29cb07ddad842bb10117 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 2 Jun 2026 12:13:12 -0400 Subject: [PATCH 1/2] feat(provider): EnumerateAll lists domains for infra.dns_delegation Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/provider.go | 17 +++++++++------- internal/provider_test.go | 43 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/internal/provider.go b/internal/provider.go index e29b3ec..c008dcc 100644 --- a/internal/provider.go +++ b/internal/provider.go @@ -326,26 +326,29 @@ func (p *HoverProvider) EnumerateAll(ctx context.Context, resourceType string) ( if p.domains == nil { return nil, fmt.Errorf("hover: EnumerateAll called on provider that is not initialized — call Initialize first") } - if resourceType != "infra.dns" { + if resourceType != "infra.dns" && resourceType != "infra.dns_delegation" { return nil, fmt.Errorf("hover: EnumerateAll: resource type %q not supported", resourceType) } domains, err := p.domains.ListDomains(ctx) if err != nil { - return nil, fmt.Errorf("hover: EnumerateAll infra.dns: %w", err) + return nil, fmt.Errorf("hover: EnumerateAll %s: %w", resourceType, err) } out := make([]*interfaces.ResourceOutput, 0, len(domains)) for _, d := range domains { if d.Name == "" { continue } - out = append(out, &interfaces.ResourceOutput{ + o := &interfaces.ResourceOutput{ ProviderID: d.Name, - Type: "infra.dns", - Outputs: map[string]any{ + Type: resourceType, + } + if resourceType == "infra.dns" { + o.Outputs = map[string]any{ "zone": d.Name, "domain_id": d.ID, - }, - }) + } + } + out = append(out, o) } return out, nil } diff --git a/internal/provider_test.go b/internal/provider_test.go index c8cbcc9..fd2844d 100644 --- a/internal/provider_test.go +++ b/internal/provider_test.go @@ -201,6 +201,49 @@ func TestHoverProvider_EnumerateAll_DNS_skipsBlankName(t *testing.T) { } } +// ── EnumerateAll(infra.dns_delegation) coverage ───────────────────────────── + +// TestEnumerateAll_DelegationListsDomains verifies that EnumerateAll for +// "infra.dns_delegation" returns one ResourceOutput per domain with +// ProviderID == domain.Name and Type == "infra.dns_delegation", and that +// an unknown resource type still returns the unsupported error. +func TestEnumerateAll_DelegationListsDomains(t *testing.T) { + stub := &fakeHoverClient{ + domains: []hoverclient.Domain{ + {ID: "1", Name: "a.com"}, + {ID: "2", Name: "b.com"}, + }, + } + p := &HoverProvider{domains: stub} + + out, err := p.EnumerateAll(context.Background(), "infra.dns_delegation") + if err != nil { + t.Fatalf("EnumerateAll(infra.dns_delegation): %v", err) + } + if len(out) != 2 { + t.Fatalf("want 2 outputs; got %d", len(out)) + } + for i, want := range []string{"a.com", "b.com"} { + if out[i].ProviderID != want { + t.Errorf("out[%d].ProviderID = %q; want %q", i, out[i].ProviderID, want) + } + if out[i].Type != "infra.dns_delegation" { + t.Errorf("out[%d].Type = %q; want infra.dns_delegation", i, out[i].Type) + } + } + if stub.calls != 1 { + t.Errorf("ListDomains called %d times; want 1", stub.calls) + } + + // Unknown resource type must still return the unsupported error. + stub2 := &fakeHoverClient{} + p2 := &HoverProvider{domains: stub2} + _, err2 := p2.EnumerateAll(context.Background(), "infra.compute") + if err2 == nil { + t.Fatal("want error for unsupported resource type; got nil") + } +} + // ── browser config tests ────────────────────────────────────────────────────── // TestInitialize_ParsesBrowserConfig verifies that browser_path, browser_download, From 952d5a8e66211c25484027807a8bf2c28f018c89 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 2 Jun 2026 12:16:56 -0400 Subject: [PATCH 2/2] feat(provider): Import dual-fetches registrar+live NS for delegation Add DelegationDriver.ReadForImport which fetches the Hover registrar NS (authoritative intent) first, then the live public NS best-effort, and returns both as separate Outputs keys. HoverProvider.Import now routes infra.dns_delegation through this path so a catalog import captures registrar intent rather than TTL-cached live NS during an NS switch. The primary "nameservers" key is set to the registrar NS, keeping existing Diff/nameserversFromOutputs semantics consistent. DelegationDriver.Read is unchanged (drift/apply path unaffected). Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/drivers/delegation.go | 44 ++++++++++++ internal/drivers/delegation_test.go | 98 +++++++++++++++++++++++++ internal/provider.go | 39 +++++++++- internal/provider_test.go | 106 ++++++++++++++++++++++++++++ 4 files changed, 285 insertions(+), 2 deletions(-) diff --git a/internal/drivers/delegation.go b/internal/drivers/delegation.go index 498fd86..00c5e1b 100644 --- a/internal/drivers/delegation.go +++ b/internal/drivers/delegation.go @@ -209,6 +209,50 @@ func lookupPublicNameservers(ctx context.Context, domain string) ([]string, erro return out, nil } +// ReadForImport fetches both the registrar (authoritative) and live public +// nameservers, returning them in a single ResourceOutput. Unlike Read, this +// method always calls GetDomainDelegation first (registrar = authoritative +// intent); the live resolver is called best-effort and its result is omitted +// if unavailable. The primary "nameservers" key equals the registrar NS so +// existing Diff / nameserversFromOutputs semantics remain consistent. +// +// This is the import-path equivalent of Read; it MUST NOT be used for drift +// detection or apply (those continue to use Read). +func (d *DelegationDriver) ReadForImport(ctx context.Context, ref interfaces.ResourceRef) (*interfaces.ResourceOutput, error) { + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("dns_delegation read-for-import %q: %w", ref.Name, err) + } + domain := ref.ProviderID + if domain == "" { + domain = ref.Name + } + + // Registrar is authoritative — any error is a hard failure. + dom, err := d.client.GetDomainDelegation(ctx, domain) + if err != nil { + return nil, fmt.Errorf("dns_delegation read-for-import %q: %w", ref.Name, err) + } + + outputs := map[string]any{ + "nameservers": nameserversToAny(dom.Nameservers), + "registrar_nameservers": nameserversToAny(dom.Nameservers), + } + + // Live resolver is best-effort; omit live_nameservers on any failure. + if d.nsResolver != nil { + if liveNS, liveErr := d.nsResolver(ctx, domain); liveErr == nil && len(liveNS) > 0 { + outputs["live_nameservers"] = nameserversToAny(liveNS) + } + } + + return &interfaces.ResourceOutput{ + Name: ref.Name, + Type: "infra.dns_delegation", + ProviderID: domain, + Outputs: outputs, + }, nil +} + // Update replaces the registrar nameservers. Rejects in-place domain // renames (those must route through Diff → NeedsReplace → Delete-then-Create). func (d *DelegationDriver) Update(ctx context.Context, ref interfaces.ResourceRef, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { diff --git a/internal/drivers/delegation_test.go b/internal/drivers/delegation_test.go index fc95736..780e2e9 100644 --- a/internal/drivers/delegation_test.go +++ b/internal/drivers/delegation_test.go @@ -534,3 +534,101 @@ func TestDelegationDriver_Diff_DomainChange_SetsBothNeedsUpdateAndReplace(t *tes t.Error("NeedsUpdate=false; should be true alongside NeedsReplace") } } + +func TestDelegationReadForImport_DualNS(t *testing.T) { + // Registrar (authoritative intent) returns dnsimple NS. + // Live public DNS returns digitalocean NS (simulates in-flight NS switch). + fc := &fakeDelegationClient{ + getResult: &hoverclient.DomainDelegation{ + ID: "domain-x.com", + Name: "x.com", + Nameservers: []string{"ns1.dnsimple.com"}, + }, + } + liveResolver := func(_ context.Context, domain string) ([]string, error) { + if domain != "x.com" { + t.Fatalf("resolver domain = %q, want x.com", domain) + } + return []string{"ns1.digitalocean.com"}, nil + } + d := NewDelegationDriverWithClientAndResolver(fc, liveResolver) + + ref := interfaces.ResourceRef{Name: "x.com", Type: "infra.dns_delegation", ProviderID: "x.com"} + out, err := d.ReadForImport(context.Background(), ref) + if err != nil { + t.Fatalf("ReadForImport: %v", err) + } + if out == nil { + t.Fatal("ReadForImport returned nil output") + } + + // Primary "nameservers" key MUST equal the registrar NS (authoritative intent). + ns, ok := out.Outputs["nameservers"].([]any) + if !ok || len(ns) != 1 || ns[0] != "ns1.dnsimple.com" { + t.Errorf("nameservers = %v, want [ns1.dnsimple.com]", out.Outputs["nameservers"]) + } + + // registrar_nameservers MUST equal the registrar NS. + regNS, ok := out.Outputs["registrar_nameservers"].([]any) + if !ok || len(regNS) != 1 || regNS[0] != "ns1.dnsimple.com" { + t.Errorf("registrar_nameservers = %v, want [ns1.dnsimple.com]", out.Outputs["registrar_nameservers"]) + } + + // live_nameservers MUST equal the live DNS NS. + liveNS, ok := out.Outputs["live_nameservers"].([]any) + if !ok || len(liveNS) != 1 || liveNS[0] != "ns1.digitalocean.com" { + t.Errorf("live_nameservers = %v, want [ns1.digitalocean.com]", out.Outputs["live_nameservers"]) + } + + // Structural invariants. + if out.Name != "x.com" { + t.Errorf("Name = %q, want x.com", out.Name) + } + if out.Type != "infra.dns_delegation" { + t.Errorf("Type = %q, want infra.dns_delegation", out.Type) + } + if out.ProviderID != "x.com" { + t.Errorf("ProviderID = %q, want x.com", out.ProviderID) + } +} + +func TestDelegationReadForImport_LiveLookupFailsGracefully(t *testing.T) { + // Registrar returns NS; live resolver errors → live_nameservers key omitted. + fc := &fakeDelegationClient{ + getResult: &hoverclient.DomainDelegation{ + ID: "domain-y.com", + Name: "y.com", + Nameservers: []string{"ns1.dnsimple.com"}, + }, + } + failingResolver := func(_ context.Context, _ string) ([]string, error) { + return nil, errors.New("DNS lookup failed") + } + d := NewDelegationDriverWithClientAndResolver(fc, failingResolver) + + ref := interfaces.ResourceRef{Name: "y.com", Type: "infra.dns_delegation", ProviderID: "y.com"} + out, err := d.ReadForImport(context.Background(), ref) + if err != nil { + t.Fatalf("ReadForImport: %v", err) + } + if out == nil { + t.Fatal("ReadForImport returned nil output") + } + + // Primary "nameservers" key MUST be present from registrar. + ns, ok := out.Outputs["nameservers"].([]any) + if !ok || len(ns) != 1 || ns[0] != "ns1.dnsimple.com" { + t.Errorf("nameservers = %v, want [ns1.dnsimple.com]", out.Outputs["nameservers"]) + } + + // registrar_nameservers MUST be present. + regNS, ok := out.Outputs["registrar_nameservers"].([]any) + if !ok || len(regNS) != 1 || regNS[0] != "ns1.dnsimple.com" { + t.Errorf("registrar_nameservers = %v, want [ns1.dnsimple.com]", out.Outputs["registrar_nameservers"]) + } + + // live_nameservers MUST be absent when live lookup fails. + if _, present := out.Outputs["live_nameservers"]; present { + t.Errorf("live_nameservers should be absent when live lookup fails; got %v", out.Outputs["live_nameservers"]) + } +} diff --git a/internal/provider.go b/internal/provider.go index c008dcc..a8bc178 100644 --- a/internal/provider.go +++ b/internal/provider.go @@ -257,6 +257,12 @@ func (p *HoverProvider) DetectDrift(ctx context.Context, resources []interfaces. // Import reads an existing Hover-managed resource and returns IaC adoption // state. cloudID is the domain name for both infra.dns and infra.dns_delegation. +// +// For infra.dns_delegation, Import uses DelegationDriver.ReadForImport so that +// the registrar NS (authoritative intent) is captured as the primary value, and +// the live public NS is recorded as a propagation annotation. This avoids the +// live-first semantics of DelegationDriver.Read, which would capture stale TTL- +// cached NS during an in-flight NS switch. func (p *HoverProvider) Import(ctx context.Context, cloudID string, resourceType string) (*interfaces.ResourceState, error) { if cloudID == "" { return nil, fmt.Errorf("hover import: provider_id is required") @@ -264,17 +270,46 @@ func (p *HoverProvider) Import(ctx context.Context, cloudID string, resourceType if resourceType == "" { resourceType = "infra.dns" } + + ref := interfaces.ResourceRef{Name: cloudID, Type: resourceType, ProviderID: cloudID} + + // Delegation import: use the dual-fetch ReadForImport path so the + // registrar NS (not the live-first Read result) is the authoritative value. + if resourceType == "infra.dns_delegation" { + d, err := p.ResourceDriver(resourceType) + if err != nil { + return nil, err + } + if dd, ok := d.(*drivers.DelegationDriver); ok { + out, err := dd.ReadForImport(ctx, ref) + if err != nil { + return nil, fmt.Errorf("hover import %q: %w", cloudID, err) + } + if out == nil { + return nil, fmt.Errorf("hover import %q: driver returned nil output", cloudID) + } + return buildResourceState(cloudID, out), nil + } + } + d, err := p.ResourceDriver(resourceType) if err != nil { return nil, err } - out, err := d.Read(ctx, interfaces.ResourceRef{Name: cloudID, Type: resourceType, ProviderID: cloudID}) + out, err := d.Read(ctx, ref) if err != nil { return nil, fmt.Errorf("hover import %q: %w", cloudID, err) } if out == nil { return nil, fmt.Errorf("hover import %q: driver returned nil output", cloudID) } + return buildResourceState(cloudID, out), nil +} + +// buildResourceState constructs a ResourceState from a ResourceOutput. +// Used by Import to ensure the delegation and generic Read paths produce an +// identical ResourceState shape. +func buildResourceState(cloudID string, out *interfaces.ResourceOutput) *interfaces.ResourceState { now := time.Now() id := out.ProviderID if id == "" { @@ -290,7 +325,7 @@ func (p *HoverProvider) Import(ctx context.Context, cloudID string, resourceType Outputs: out.Outputs, CreatedAt: now, UpdatedAt: now, - }, nil + } } // ResolveSizing is a stub: Hover has no compute sizing. diff --git a/internal/provider_test.go b/internal/provider_test.go index fd2844d..59a6917 100644 --- a/internal/provider_test.go +++ b/internal/provider_test.go @@ -8,6 +8,7 @@ import ( "sync/atomic" "testing" + "github.com/GoCodeAlone/workflow-plugin-hover/internal/drivers" "github.com/GoCodeAlone/workflow-plugin-hover/pkg/hoverclient" "github.com/GoCodeAlone/workflow/interfaces" ) @@ -244,6 +245,111 @@ func TestEnumerateAll_DelegationListsDomains(t *testing.T) { } } +// ── Import delegation dual-fetch coverage ──────────────────────────────────── + +// fakeDelegationClientForImport satisfies HoverDelegationClient and +// hoverDomainLister so it can be injected into both the DelegationDriver +// and HoverProvider.domains field. It also satisfies hoverclient.HoverClient +// via a nil client stored in drivers so we need a separate provider-level stub. +type fakeDelegationClientForImport struct { + registrarNS []string + registrarErr error +} + +func (f *fakeDelegationClientForImport) GetDomainDelegation(_ context.Context, _ string) (*hoverclient.DomainDelegation, error) { + if f.registrarErr != nil { + return nil, f.registrarErr + } + return &hoverclient.DomainDelegation{ + ID: "domain-x.com", + Name: "x.com", + Nameservers: f.registrarNS, + }, nil +} + +func (f *fakeDelegationClientForImport) SetNameservers(_ context.Context, _ string, _ []string) error { + return nil +} + +// TestImport_DelegationUsesRegistrarNotLiveRead verifies that +// HoverProvider.Import for infra.dns_delegation calls ReadForImport (which +// fetches the registrar NS authoritatively) rather than falling through to +// the live-first DelegationDriver.Read path. +func TestImport_DelegationUsesRegistrarNotLiveRead(t *testing.T) { + registrarNS := []string{"ns1.dnsimple.com"} + liveNS := []string{"ns1.digitalocean.com"} + + fc := &fakeDelegationClientForImport{registrarNS: registrarNS} + + // Build a DelegationDriver with a resolver that returns different (live) NS. + liveResolver := func(_ context.Context, _ string) ([]string, error) { + return liveNS, nil + } + + delegDriver := drivers.NewDelegationDriverWithClientAndResolver(fc, liveResolver) + + p := &HoverProvider{ + drivers: map[string]interfaces.ResourceDriver{ + "infra.dns_delegation": delegDriver, + "infra.dns": &noopCreateDriver{}, + }, + } + + state, err := p.Import(context.Background(), "x.com", "infra.dns_delegation") + if err != nil { + t.Fatalf("Import(infra.dns_delegation): %v", err) + } + if state == nil { + t.Fatal("Import returned nil state") + } + + // registrar_nameservers must be the REGISTRAR value (not live). + regNS, ok := state.Outputs["registrar_nameservers"].([]any) + if !ok || len(regNS) != 1 || regNS[0] != "ns1.dnsimple.com" { + t.Errorf("registrar_nameservers = %v, want [ns1.dnsimple.com]", state.Outputs["registrar_nameservers"]) + } + + // live_nameservers must be the LIVE DNS value. + liveNSOut, ok := state.Outputs["live_nameservers"].([]any) + if !ok || len(liveNSOut) != 1 || liveNSOut[0] != "ns1.digitalocean.com" { + t.Errorf("live_nameservers = %v, want [ns1.digitalocean.com]", state.Outputs["live_nameservers"]) + } + + // Primary nameservers must equal registrar (authoritative intent). + ns, ok := state.Outputs["nameservers"].([]any) + if !ok || len(ns) != 1 || ns[0] != "ns1.dnsimple.com" { + t.Errorf("nameservers = %v, want [ns1.dnsimple.com]", state.Outputs["nameservers"]) + } + + // ResourceState structural invariants. + if state.Provider != "hover" { + t.Errorf("Provider = %q, want hover", state.Provider) + } + if state.Type != "infra.dns_delegation" { + t.Errorf("Type = %q, want infra.dns_delegation", state.Type) + } + if state.AppliedConfigSource != "adoption" { + t.Errorf("AppliedConfigSource = %q, want adoption", state.AppliedConfigSource) + } +} + +// TestImport_DNSUnchanged verifies that Import for infra.dns still goes +// through the generic d.Read path (not the delegation dual-fetch). +func TestImport_DNSUnchanged(t *testing.T) { + // noopCreateDriver.Read returns nil — Import must not panic or break. + p := &HoverProvider{ + drivers: map[string]interfaces.ResourceDriver{ + "infra.dns": &noopCreateDriver{}, + "infra.dns_delegation": &noopCreateDriver{}, + }, + } + // infra.dns with nil Read output → Import returns an error (nil output guard). + _, err := p.Import(context.Background(), "x.com", "infra.dns") + if err == nil { + t.Fatal("expected error when driver.Read returns nil output for infra.dns") + } +} + // ── browser config tests ────────────────────────────────────────────────────── // TestInitialize_ParsesBrowserConfig verifies that browser_path, browser_download,