feat(middleware): add authz cache, circuit breaker, and bounded retry#129
Merged
Conversation
Every authorized request made a synchronous POST /v1/authorize with no cache, breaker, or retry, so an authz-service blip or outage amplified straight into the request path. The call was also built with http.NewRequest (no context), so a caller's deadline/cancellation never reached it. Add three opt-in resilience layers (all default-off, so default behavior is byte-for-byte unchanged) plus the context fix, composed as breaker(retry(httpCall(ctx))) with the cache checked before the breaker: - Prereq fix: build the authz request with http.NewRequestWithContext, and wrap each call in context.WithTimeout(ctx, AUTH_TIMEOUT) (default 30s = behavior-neutral). The caller's cancellation now aborts the call, and the deadline caps the retry budget. - TTL decision cache (AUTH_CACHE_TTL > 0) keyed by (sub, resource, action, product) — never the raw token. Bounded, sharded, no-dependency map with lazy expiry (no background goroutine to leak). get never returns an expired entry. Documented staleness/revocation-lag window = TTL. - Circuit breaker (AUTH_BREAKER_ENABLED, sony/gobreaker promoted to direct). On open it serves ONLY a fresh positive cache hit (checked before the breaker), otherwise denies — never a stale grant. - Bounded retry (AUTH_RETRY_MAX, cenkalti/backoff/v5) on TRANSIENT failures only (network/timeout/5xx); authoritative 401/403 are never retried. Every fallback — breaker open, retry exhausted, timeout — denies (fail closed, 403 / PermissionDenied). This complements #107: misconfig at construction -> 503 (never mounts open); runtime outage -> deny. Two triggers, both fail-closed. Stacks on #107 (PR #127): built on its Required fail-closed flag. Must merge after #127; rebase onto develop and retarget base to develop once #127 lands. Closes #108 Claude-Session: https://claude.ai/code/session_01RcQLK4qK1kbpvXWjuAR3Xh
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches✨ Simplify code
Comment |
|
🎉 This PR is included in version 3.0.0-beta.7 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 3.0.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds three opt-in authorization resilience layers (all default-off → default behavior byte-for-byte unchanged) plus a prerequisite context fix, composed as
breaker( retry( httpCall(ctx) ) )with the cache checked before the breaker.Prerequisite bug fix
checkAuthorizationbuilt the authz POST withhttp.NewRequest(no context), so the caller's deadline/cancellation never reached the call. Now useshttp.NewRequestWithContext(ctx, ...)wrapped incontext.WithTimeout(ctx, AUTH_TIMEOUT).Layers
AUTH_TIMEOUT, default 30s (behavior-neutral; was the client-wide timeout). Now genuinely per-request, cancellable, and caps the retry budget.AUTH_CACHE_TTL > 0. Keyed by(sub, resource, action, product)— never the raw token. Bounded, 16-way sharded, no-dependency map with lazy expiry (no background goroutine to leak).getnever returns an expired entry. Caches positive and negative decisions. Staleness window = TTL (documented revocation-lag tradeoff).AUTH_BREAKER_ENABLED(sony/gobreaker, promoted to direct). On open: serves only a fresh positive cache hit (checked before the breaker), otherwise denies. Never serves a stale grant.AUTH_RETRY_MAX(cenkalti/backoff/v5, already a dep). Retries only transient failures (network/timeout/5xx); never 401/403.MaxElapsedTimecapped atAUTH_TIMEOUT.Fail-closed contract
Every fallback — breaker open, retry exhausted, timeout, any error — denies (Fiber 403 / gRPC
PermissionDenied). Complements #107: misconfig at construction → 503 (#107, never mounts open) vs runtime outage → deny/403 (#108). Two triggers, both fail-closed. No path fails open — proven by tests for breaker-open-no-cache, retry-exhausted, timeout, and expired-cache-under-outage.Composition & default path
When all knobs are off,
checkAuthorizationmakes a single call and returns the outcome exactly as before (all pre-existing tests pass, including the 500-on-authz-down and 403-with-error-body cases). The transient-vs-authoritative classification and 403-deny only engage when a resilience layer is active.Tests
18 new tests: cache hit avoids a second POST; TTL expiry re-queries; negative decision cached; retry on 5xx eventually succeeds; retry NOT on 403; breaker opens after N and denies without touching the service; breaker-open serves a fresh positive cache hit but denies an uncached key; per-request timeout and caller-cancellation abort the call; cache is bounded and race-clean; config wiring (defaults / all-enabled / invalid-values).
Verification
go build,go vet,go test -race,golangci-lint run ./...(gocognit/gocyclo under ceilings — no//nolint), andgosec ./...all clean locally.go mod tidyrun after promotinggobreakerto a direct dependency.Closes #108
https://claude.ai/code/session_01RcQLK4qK1kbpvXWjuAR3Xh