From bb62a90283a98c02e39c1634fb4d8e6738e83d50 Mon Sep 17 00:00:00 2001 From: Pedro Gomes Date: Sun, 28 Jun 2026 07:19:57 +0100 Subject: [PATCH] fix(registry): region-scope hcloud/cloudflare provider URNs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit program.Run builds one provider registry per realization scope — one per region plus the region-less global slice — all over the same Pulumi context. Each registry lazily registered its hcloud and cloudflare *provider* resources under the fixed names "hcloud"/"cloudflare", so once a config realized Hetzner/Cloudflare resources in more than one scope the two registrations collided on URN and failed the whole preview with `Duplicate resource URN '…::pulumi:providers:hcloud::hcloud'`. Scope the provider resource names by region (`hcloud-`, `cloudflare-`); r.region is "global" for the global slice and the region name otherwise, so it is always present and unique per scope. Add a regression test that builds two registries over one context and asserts the provider names are unique per scope (the Pulumi mock monitor does not itself reject duplicate URNs), and a rule documenting the invariant for future singleton providers added to the registry. Claude-Session: https://claude.ai/code/session_017Kyd98NzojozMZ19d5UCZ2 --- ...gistry-provider-names-are-region-scoped.md | 27 ++++++ internal/registry/registry.go | 12 ++- internal/registry/registry_test.go | 89 +++++++++++++++++++ 3 files changed, 126 insertions(+), 2 deletions(-) create mode 100644 .agents/rules/registry-provider-names-are-region-scoped.md diff --git a/.agents/rules/registry-provider-names-are-region-scoped.md b/.agents/rules/registry-provider-names-are-region-scoped.md new file mode 100644 index 0000000..320d8ca --- /dev/null +++ b/.agents/rules/registry-provider-names-are-region-scoped.md @@ -0,0 +1,27 @@ +# Registry provider resource names must be region-scoped + +`program.Run` builds one `registry.BuildRegistry` per realization scope — one per region **plus** +the region-less global slice — all sharing the same Pulumi `ctx`. Each registry lazily registers its +own cloud-provider resources (`hcloud.NewProvider`, `cf.NewProvider`). If two registries register a +provider under the same resource name, Pulumi fails the whole preview/up with +`Duplicate resource URN '…::pulumi:providers:hcloud::hcloud'`. + +So every provider resource the registry creates MUST embed `r.region` in its name +(`fmt.Sprintf("hcloud-%s", r.region)`, `fmt.Sprintf("cloudflare-%s", r.region)`). `r.region` is +`"global"` for the global slice and the region name otherwise, so it is always present and unique +across the scopes a single program run builds. + +## Applies to + +`internal/registry/registry.go` — `hetznerProv()` and `cfProv()`. When adding any new +singleton provider resource to the registry (a `*.NewProvider(r.ctx, name, …)` call memoised via a +`sync.Once`), the `name` must be region-scoped the same way. Per-resource registrations that already +carry a unique name (the neon/infisical `ctx.RegisterResource` calls) are not affected. + +## Why + +A single-scope config (one region, or global-only) never exercised the collision, so a fixed name +like `"hcloud"` looked correct. The first config to realize Hetzner/Cloudflare resources in **both** +the global slice and a region — or in two regions — registers the provider twice under one URN and +fails at preview before any resource is touched. Region-scoping the name makes the provider unique +per scope while staying stable across runs of that scope. diff --git a/internal/registry/registry.go b/internal/registry/registry.go index 3a7dcfc..011716f 100644 --- a/internal/registry/registry.go +++ b/internal/registry/registry.go @@ -101,7 +101,12 @@ func (r *registry) hetznerProv() *hcloud.Provider { return } token := providerCfgString(r.config, "hetzner", "apiToken") - p, _ := hcloud.NewProvider(r.ctx, "hcloud", &hcloud.ProviderArgs{ + // The provider resource name is scoped by region so that a program building + // more than one registry over the same Pulumi context — one per region plus + // the region-less global slice — does not register two providers under the + // same URN (pulumi:providers:hcloud::hcloud). r.region is "global" for the + // global slice and the region name otherwise, so it is always unique. + p, _ := hcloud.NewProvider(r.ctx, fmt.Sprintf("hcloud-%s", r.region), &hcloud.ProviderArgs{ Token: pulumi.String(token), }) r.hetznerProvider = p @@ -165,7 +170,10 @@ func (r *registry) cfProv() *cf.Provider { return } token := providerCfgString(r.config, "cloudflare", "apiToken") - p, _ := cf.NewProvider(r.ctx, "cloudflare", &cf.ProviderArgs{ + // Scoped by region for the same reason as the hcloud provider: each scope's + // registry registers its own provider, and a shared name would collide on + // URN once more than one scope realizes DNS records. + p, _ := cf.NewProvider(r.ctx, fmt.Sprintf("cloudflare-%s", r.region), &cf.ProviderArgs{ ApiToken: pulumi.StringPtr(token), }) r.cfProvider = p diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go index ff09a0e..99175f9 100644 --- a/internal/registry/registry_test.go +++ b/internal/registry/registry_test.go @@ -1,10 +1,15 @@ package registry import ( + "strings" + "sync" "testing" + "github.com/pulumi/pulumi/sdk/v3/go/common/resource" + "github.com/pulumi/pulumi/sdk/v3/go/pulumi" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/wardnet/inforge/internal/regions" "github.com/wardnet/inforge/internal/tags" "github.com/wardnet/inforge/internal/types" cfprovider "github.com/wardnet/inforge/providers/cloudflare" @@ -13,6 +18,90 @@ import ( "github.com/wardnet/inforge/providers/neon" ) +// provMocks records the (typeToken, name) of every resource registered through it. +// Provider resources (e.g. type "pulumi:providers:hcloud") are registered like any +// other, so the mock sees them and we can assert their names are unique. +type provMocks struct { + mu sync.Mutex + providers map[string][]string // typeToken -> resource names registered +} + +func (m *provMocks) Call(pulumi.MockCallArgs) (resource.PropertyMap, error) { + return resource.PropertyMap{}, nil +} + +func (m *provMocks) NewResource(args pulumi.MockResourceArgs) (string, resource.PropertyMap, error) { + if strings.HasPrefix(args.TypeToken, "pulumi:providers:") { + m.mu.Lock() + if m.providers == nil { + m.providers = map[string][]string{} + } + m.providers[args.TypeToken] = append(m.providers[args.TypeToken], args.Name) + m.mu.Unlock() + } + return args.Name + "-id", resource.PropertyMap{}, nil +} + +// TestProviderResourceNamesAreScopePerRegistry guards against the duplicate-URN +// regression that broke `inforge preview` once a config realized Hetzner + +// Cloudflare resources in more than one scope. program.Run builds one registry per +// scope — one per region plus the region-less global slice — all over the SAME +// Pulumi context. Each registry lazily registers its own hcloud/cloudflare +// *provider* resource; two registries registering a provider under the same name +// produce the same URN, and the real engine then fails the whole run with +// `Duplicate resource URN '…::pulumi:providers:hcloud::hcloud'`. +// +// The provider resource name is what determines the URN, so this asserts the +// invariant directly: across the two scopes, each provider type must be registered +// under distinct names. (The mock monitor does not itself reject duplicate URNs, +// so we check the names rather than rely on RunErr returning an error.) +func TestProviderResourceNamesAreScopePerRegistry(t *testing.T) { + config := map[string]map[string]any{ + "hetzner": {"apiToken": "t"}, + "cloudflare": {"apiToken": "t"}, + } + mocks := &provMocks{} + + err := pulumi.RunErr(func(ctx *pulumi.Context) error { + // Mirror program.Run: a global-slice registry and a regional registry over + // the same context. Distinct region strings → distinct scopes. + for _, region := range []string{"global", "us-east-1"} { + reg := BuildRegistry(ctx, config, ®ions.DnsAuthority{Zone: "z"}, types.SSHConfig{}, regions.Table{}, "proj", "prd", region, tags.Ephemeral{}) + // Compute("hetzner") forces hcloud.NewProvider; DNS("cloudflare") forces + // cf.NewProvider. Both are the singleton provider resources that collide. + if _, cerr := reg.Compute("hetzner"); cerr != nil { + return cerr + } + if _, derr := reg.DNS("cloudflare"); derr != nil { + return derr + } + } + return nil + }, pulumi.WithMocks("proj", "stack", mocks)) + require.NoError(t, err) + + // Both provider types must have been registered once per scope (2 scopes), and + // every registration must carry a distinct name — else the URNs collide. + for _, typeToken := range []string{"pulumi:providers:hcloud", "pulumi:providers:cloudflare"} { + names := mocks.providers[typeToken] + require.Len(t, names, 2, "%s: expected one provider per scope", typeToken) + assert.Len(t, dedupe(names), len(names), + "%s: provider names must be unique per scope, got %v (duplicate URN regression)", typeToken, names) + } +} + +func dedupe(in []string) []string { + seen := map[string]struct{}{} + var out []string + for _, s := range in { + if _, ok := seen[s]; !ok { + seen[s] = struct{}{} + out = append(out, s) + } + } + return out +} + func TestRegistryUnknownProvider(t *testing.T) { // nil ctx + nil regionTable: providers are built lazily, so construction is // not triggered during this test. Only the "unknown provider" paths are hit.