Extends the pattern from #19 (nbsetupkey) to the three Network-family
controllers. Before this change, a transient failure on the by-ID Get
in Observe was silently swallowed (ResourceExists: false, nil error),
which then drove Crossplane into Create — producing duplicate netbird
resources whenever the upstream API hiccupped. We also defaulted the
external-name annotation to the Kubernetes object name on first
reconcile, so subsequent reconciles would 404 against the API and
trigger Create again.
For each of NbNetwork, NbNetworkResource, NbNetworkRouter:
- Disable the NameAsExternalName default via managed.WithInitializers()
on the reconciler so external-name stays empty until we set it to
the real provider ID.
- Add resolve<Type>LookupID(cr) that prefers external-name when it
looks like a real provider ID, and falls back to
status.atProvider.id when external-name is empty or was defaulted
to the k8s object name by an older reconcile.
- Add is<Type>NotFoundError(err) that matches the
"<type>: <id> not found" message returned by the netbird REST API.
- In Observe: gate the by-ID branch on the resolved lookup ID.
- If the lookup ID is empty, fall through to the existing
by-name adoption path (unchanged — these controllers have richer
adoption than nbsetupkey did).
- When the by-ID Get fails: treat real not-found as
ResourceExists: false (delegate to Create); wrap and return
every other error so Crossplane requeues without calling Create.
- After a successful by-ID Get, repair a stale external-name
annotation against the real ID.
- Update/Delete use the same resolver and error out on empty rather
than passing an empty string to the API.
- Extract authClient interface on the external struct so tests can
inject a netbird client backed by httptest.
Adds table-driven tests covering: empty external-name + empty status
ID, by-ID 404, transient by-ID error, and stale-external-name repair
via status fallback. The existing TestResolveGroupIDs cases on
nbnetworkresource (from PR #20) are preserved.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`Create` / `Update` previously did:
for j, provgroup := range *cr.Spec.ForProvider.Groups {
for _, apigroup := range groups {
if apigroup.Name == *provgroup.Name { ... }
}
}
`provgroup.Name` is `*string` and the schema marks it optional. When a
user references a group by Id alone — the canonical pattern, since Id
is stable and Name is a free-text label — the unconditional `*Name`
deref panics the entire provider with nil pointer at this line.
Even when Name is supplied, the loop ignored the spec's Id entirely
and forced an account-wide group-name lookup, silently leaving the
slot empty (and the resource created with an empty group ref) if no
match was found.
Extract `resolveGroupIDs(spec, apiGroups)`:
- Prefer Id when present and non-empty.
- Fall back to Name lookup, guarded.
- Return an explicit error if neither is usable, or if the named
group does not exist on the account.
Both Create and Update now share the helper. Adds table-driven unit
test covering id-wins-over-name, name-fallback, nil-everything,
unknown-name, and empty-spec.
Repro: apply an `NbNetworkResource` whose `spec.forProvider.groups`
entries are `{id: <existing-group-id>}` (Name omitted). With v0.4.2
the provider crashes on the first reconcile; with this fix the
resource creates successfully.
Callum MacDonald
David Fry
David Fry
David Fry