Two follow-up findings from PR #44 review (deferred trusted_sources flush). Both are Minor — not blocking PR #44 itself, but worth fixing in a follow-up.
1. Orphan handling when DB type leaves plan.Actions
In `DOProvider.Apply` second-pass flush, the deferred-flush loop iterates `plan.Actions` by resource type. Per Copilot review on PR #44 commit 706138a (internal/provider.go:~362):
The deferred flush pass only considers resource types present in plan.Actions. If deferred updates are retained for retry (e.g. after a transient flush failure), a subsequent Plan that no longer contains the deferring resource type would skip the flush entirely — leaving deferred entries orphaned in driver state.
Concrete reproducer:
- Plan A: includes `infra.database` + `infra.app`; first apply succeeds for app+db creates but flush fails (transient DO API error).
- Operator removes the app from infra.yaml.
- Plan B: includes `infra.database` only (no app type); apply runs; deferred-flush loop iterates plan.Actions, sees only `infra.database` type, but the deferred entry is keyed for the app that no longer appears.
- Outcome depends on impl: if the queue is keyed by resource ref and FlushDeferredUpdates re-attempts, this is fine. If keyed by referenced-app and the app is gone from infra.yaml, the deferred entry is stranded.
Likely fix: iterate driver registry directly in the second pass (any driver with HasDeferredUpdates() == true), not plan.Actions. Each driver flushes its own queue regardless of whether its type appears in this plan.
2. UUID-resolvable type=app shortcut
Per Copilot review on PR #44 (internal/drivers/database.go:602 + :634):
buildCreateFirewallRulesExcludingApps + buildUpdateFirewallRulesExcludingApps drop ALL type=app entries unconditionally. If a type=app entry's value is already UUID-like, it's resolvable immediately and doesn't require Apps API resolution — could be passed through directly to the create/update call instead of being deferred.
Currently any type=app entry is deferred. This is correct but conservative — over-defers when the value is already a resolved UUID. Optimization, not correctness.
Likely fix: predicate the exclusion on "value matches non-UUID app-name pattern" (e.g., regex `^[a-z][a-z0-9-]+$` for app slugs vs UUID).
Scope
Both are follow-ups; no urgency. Should land as a small PR after PR #44 merges + downstream lockfile bumps catch up.
Related
Two follow-up findings from PR #44 review (deferred trusted_sources flush). Both are Minor — not blocking PR #44 itself, but worth fixing in a follow-up.
1. Orphan handling when DB type leaves plan.Actions
In `DOProvider.Apply` second-pass flush, the deferred-flush loop iterates `plan.Actions` by resource type. Per Copilot review on PR #44 commit 706138a (internal/provider.go:~362):
Concrete reproducer:
Likely fix: iterate driver registry directly in the second pass (any driver with HasDeferredUpdates() == true), not plan.Actions. Each driver flushes its own queue regardless of whether its type appears in this plan.
2. UUID-resolvable type=app shortcut
Per Copilot review on PR #44 (internal/drivers/database.go:602 + :634):
Currently any type=app entry is deferred. This is correct but conservative — over-defers when the value is already a resolved UUID. Optimization, not correctness.
Likely fix: predicate the exclusion on "value matches non-UUID app-name pattern" (e.g., regex `^[a-z][a-z0-9-]+$` for app slugs vs UUID).
Scope
Both are follow-ups; no urgency. Should land as a small PR after PR #44 merges + downstream lockfile bumps catch up.
Related