/health returned 200 without asking anything. Whatever it was meant to say, an
orchestrator reading it learned only that a process was accepting connections.
The two questions are not the same one, and the answers differ:
- /health stays a bare 200. It answers "should I restart you", and a process
whose database is unreachable does not want restarting - that turns one
outage into a crash loop and discards the connection pool, the cache and
every request in flight on the way.
- /ready is new. It answers "should I send you requests", fails while a
dependency is unreachable, and fails from the moment shutdown begins.
That last part is what the life-cycle phases bought. BeginDraining sits next to
BeginShutdown, before the server stops accepting, so a load balancer is told to
stop sending while this instance can still finish what it holds. Reversed - and
that is where it was - the connections are cut first and the probe reports it
afterwards.
The queue is deliberately not checked. Nothing on AdapterQueue answers "are you
reachable" without publishing something, the memory backend cannot fail, and a
queue that is down degrades logging rather than stopping requests: a reason to
alert, not a reason to leave the pool.
The cache probe writes and reads back rather than only reading. A cache that
answers "miss" for every key - a client pointed at the wrong server - is
indistinguishable from a healthy one on a read alone.
Every check runs behind a recover, and that is not defensive habit. The test for
"nothing configured" found the reason: GetCacheAdapter builds a wrapper around
whatever is configured and returns it even when nothing is, so the value is not
nil, the cache inside it is, and Set dereferences it. A nil check cannot see
that, and GetQueueAdapter behaves the same way. Whatever the cause, a probe is
the last thing that should be able to take the process down - the caller is
asking whether this instance is well, and killing it is the wrong reply.
The counter-proof compiles and fails: without the recover, the unconfigured case
panics rather than reporting two failed checks.
Two properties the previous shape broke silently: GetHandlerFunc must
report ok for the JwtToken key, and every module must read back the same
instance. Reverting the registration to the unbound method expression
still compiles and turns the first of these red, which is the failure
this pins.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
Four modules each called AuthInit and built their own instance, so which
one Runtime handed back was decided by whichever module initialised last.
The JwtToken key was also registered as an unbound method expression,
which GetHandlerFunc's type assertion can never match - the key was
registered and unusable at the same time.
The instance is now built once here and registered as a bound closure.
Modules read it back through GetAuthMiddleware, which is fatal rather
than nil when called before InitMiddleware has run: a process without a
JWT middleware should not reach the point of serving a request.
Only one call site needs the instance itself rather than the handler
(admin's /login, for LoginHandler); the thirty-odd MiddlewareFunc() call
sites are unchanged.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
PUT /api/v1/sys-user sits in CasbinExclude so the profile page can reach it,
which means AuthCheckRole never runs for this route. The handler took the
target user id from the request body, so any authenticated caller could edit
another user's record - including their roleId.
The route has to stay excluded: the profile page and the admin user list share
this one endpoint, so removing the exclusion would break self-service editing
for every non-admin role. The check therefore moves into the handler: when the
target is not the caller, the request is put through Casbin explicitly.
EnforceRoleFor carries the same admin short-circuit and enforcement AuthCheckRole
uses, so a route that opts out of the middleware can still ask the same question.
Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx
The warning on Authorizator matters to anyone keeping a copy of that file, not
to one particular consumer, and it reads better addressed to all of them: check
what reads those context keys before taking this change.
The map Authorizator receives is built by IdentityHandler in the same file and
carries IdentityKey / UserName / RoleKey / UserId / RoleIds / DataScope - not
user and role. Both assertions failed on every request, and because the ok
result was discarded the five c.Set calls stored zero values and the function
returned true anyway. Nothing in this repository or in core reads those keys.
go-admin-pro has its own copy of this file and does read them; this change
must not be carried over there verbatim.
common/middleware imported app/admin/service/dto for two string constants,
which made a package apps are told to build on depend on one particular app.
LoggerToFile is registered on the engine, so every POST, PUT, GET and
DELETE had its body copied into memory - through a bytes.Buffer, a
ReadAll and a string conversion - before any handler ran. The only
consumer is operParam on the operation-log row, which is written when
logger.enableddb is on, and that is off in the shipped configuration.
There was no size limit either, and a file upload is a POST like any
other: a 1MB request allocated 4.3MB here and a 16MB upload allocated
about 67MB, to build a value nobody stored.
The body is now read only when the operation log will use it, and at
most 32KB of it. The handler still receives the whole request: it reads
the copied part from memory and the rest from the connection, so what
this holds is bounded however large the request is. 32KB also keeps the
value inside the TEXT column it is written to.
The bufio.Writer this replaces was never flushed. Nothing was truncated
only because bytes.Buffer implements io.ReaderFrom, so io.Copy bypassed
the buffer entirely - a different destination would have dropped the
tail of every request body.
AuthCheckRole walks CasbinExclude for every non-admin request, and used
casbin's util.KeyMatch2 to test each entry. That delegates to
util.RegexMatch, which is regexp.MatchString - it compiles its pattern on
every call - so a 32-entry list cost about 2,566 allocations per request
before the request reached Enforce.
Test the method first, which rules out most entries with a string
compare, and take the path test from go-admin-core, whose KeyMatch2
answers the same thing without recompiling. The scan drops to 52ns and no
allocations.
The loop moves out of AuthCheckRole so the tests exercise the code a
request runs rather than a copy of it, and an allocation budget fails if
the uncached matcher comes back.
A rejected request answered 200 with the failure only in the body, so every
layer that reads the status line counted it as served: load balancers, metrics,
client-side retry. A load test against this reported the limiter's own
rejections as successful traffic and overstated throughput more than tenfold.
The threshold was a constant in the middleware, which made 200 QPS the ceiling
of every deployment with nothing in the configuration to reveal it. It now
reads extend.rateLimit.inboundQPS; an absent value keeps 200, so an upgrade
changes nothing, and zero disables the limiter for a deployment behind its own
gateway.
Also drops Strategy: system.BBR. Reading sentinel's source, the adaptive
strategy is consulted only for Load and CpuUsage - for InboundQPS the trigger
count is compared directly - so it read as if the limit adapted to the machine
when it never did.
The scope is decided by the user id, the role id, the department and the
data_scope string. Three of the four were already in the token; deptid
was not, though core's user.GetDeptId has always read that claim. Adding
it removes a sys_user join from every list, detail, update and delete.
This goes no more stale than rolekey does, which Casbin has read from
the token since the beginning: both settle on the next login.
A token minted before this still works. Its claims are incomplete, and
the lookup runs for it as before.
Every import of the module changes, not only the seven packages that
moved out of sdk/pkg: Go requires the major version in the path from v2
on. Both happen in one pass —
go run github.com/go-admin-team/go-admin-core/tools/coreupgrade@v2.0.0 -w -v2 .
go mod tidy
— which is the command the release notes give, run here as a consumer
would run it. 210 imports across 95 files.
The compatibility shims this used are gone in v2, so the paths that
moved had to move: sdk/pkg/captcha, sdk/pkg/jwtauth and its user
package, sdk/pkg/response and sdk/pkg/casbin.
The count of unformatted files is unchanged at 34, none of them touched
by this: the tool reformats a file only if it was already gofmt clean,
so a migration cannot disappear into whitespace.
The pinned core dated from April, before sdk stopped being a separate module,
so the build resolved sdk packages from the old module and core packages from
the new one. Dropping the separate requirement is what makes the two agree
again.
Most of the diff is renames that came with that: the tenant accessors gained a
ByTenant suffix, GetDb now returns one database and GetAllDb the map, and
casbin moved to v3.
The change that matters is four call sites moving from GetMemoryQueue to
GetQueuePrefix. GetMemoryQueue returns a queue fixed at construction, so the
login log, the operate log and the api check ran in process no matter what the
settings file selected — a second instance saw none of it. GetQueuePrefix
returns whatever the configuration built, which is the point of being able to
configure a queue at all.
Verified against core at main: build and vet clean. The two file_store failures
are unchanged from before this branch; they need cloud credentials.
zhangwenjian