Add ageing cache to dns alias driver - #1071
Conversation
- Cache getAliases responses with a configurable TTL (default 30s).
- Use a sentinel value ("null") to cache 404 (no aliases) without
poisoning the cache with nil.
- Invalidate the cache after every mutation (create, update, delete).
- Explicitly clear the cache before Start, Stop, and Status to ensure
fresh data.
- Expose CacheTTL on the mgr struct so tests can disable the cache
(set to 0) and avoid filesystem permission issues.
Changes:
- drivers/resipsgcp_dnsalias/main.go
- drivers/resipsgcp_dnsalias/mgr.go
- drivers/resipsgcp_dnsalias/main_test.go
53a0b94 to
40f3543
Compare
![aikido-pr-checks[bot]](https://avatars.githubusercontent.com/in/898896?s=60&v=4){kind=small}
| } | ||
|
|
||
| func (m *mgr) cacheSig() string { | ||
| return fmt.Sprintf("dnsalias:%s:%s:%s", m.alias.ZoneID, m.alias.Name, m.alias.UUID) |
There was a problem hiding this comment.
🟡 Medium - Scope the alias cache by SGCP authentication context
The cache signature is derived only from zone, name, and UUID, while each driver instance can use a different secret/SGCP account and the ageing cache is shared under the node-wide cache directory. Parallel status or reconcile operations for two such resources can therefore return one account's cached alias listing to the other; that can expose the other tenant's alias metadata and make the second resource report the wrong state or attempt an update/delete using the cached UUID. Include a stable endpoint and authenticated account/secret identity in a sanitized cache key (or otherwise isolate cache entries by API context).
Show fix
Include a stable, non-secret identity for the configured SGCP endpoint and credentials (for example the auth-info signature/account identity) in the cache signature, sanitize each component before using it in cache/lock paths, and add a parallel two-manager test proving that entries cannot be reused across authentication contexts.
More info - Reply on this comment to give feedback or ignore the issue.
| t.Log().Debugf("cache clear error: %s", err) | ||
| } | ||
| aliases, err := t.mgr.getAliases(ctx) |
There was a problem hiding this comment.
🔵 Low - Do not invalidate the alias cache immediately before every read
Every public operation clears the current alias cache key and then performs its only getAliases call, so the entry written by one operation is deleted before the next operation can reuse it. In the scheduler's repeated Status path this leaves DNS API traffic unchanged while adding ageing-cache clear/output locking and filesystem churn, defeating the cache integration and its intended scalability benefit. Invalidate after Start/Stop mutations, and only bypass the cache for explicitly fresh, non-scheduler status requests.
Show fix
Remove the unconditional pre-read cacheClear calls from Start, Stop, and Status; invalidate the relevant key after successful create/update/delete, and use the action context to force a fresh read only for explicit non-scheduler status commands. Add a test that invokes Status twice with CacheTTL enabled and verifies the second call does not invoke GetAliases.
More info - Reply on this comment to give feedback or ignore the issue.
1 participant
Changes: