The 1970..2262 check refused times on parsing, including a start that
the window clamps to the maximum range before anything is sent and a
since that only becomes a capped start_offset, so an old start or a
tail resume from before 1970 answered 400 where the documented answer
is to clamp. It had also detached the parseTime doc comment.
The check now applies to the times actually sent, after clamping, and
uses the range vlselect can hold, int64 nanoseconds (1677..2262); 1970
is not a limit of vlselect.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The slow-body and stalled-client tests checked the slot by sending
requests of their own, which competed for the one slot under test: a
probe that held the slot when the request under test arrived got that
request refused, and the test failed. The tests now read the limiter's
slot count directly.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The second nginx of the not-installed test reused the stand-in port
of the first one right after killing it, behind a fixed sleep, so a
slow exit could leave the port taken. It now waits for the first nginx
to exit and takes a new port. The file header also names the fourth
property it now checks.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
An empty fields_limit answered 400, but an empty limit or facets cap
fell back to the default, so one malformed parameter got two answers
depending on its name. Every cap now refuses an empty value with 400:
a cap the client sent but left blank is a client bug, and reading it
as the default would hide it.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The proxy sends vlselect RFC3339 times, which it never reads as a
guessed-unit number, but it accepted any RFC3339 year and unix
seconds up to 2286. vlselect keeps times as int64 nanoseconds through
time.UnixNano, which is undefined outside 1678..2262, so such a
window landed anywhere. An absolute time outside 1970..2262 now
answers 400; nothing was logged before 1970.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
A request refused before its body was read (401, 403, 405, or a rate
or concurrency 429) had no read deadline yet, and net/http drains up
to 256 KiB of an unread body before it sends the headers. A client
that declared a body and never sent it got no answer, and held the
handler, for as long as it kept the connection; for a second form POST
arriving while the concurrency slot was taken, that was more than five
request timeouts.
The read deadline is now set when the request arrives and lifted once
the body is read, so tails are not cut. A refusal sent with the body
still unread closes the connection instead of draining it, so it is
answered at once.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The test only checked that a Retry-After was present, while the rate
limit test pins its value, so a change to the promised hint passed.
It now expects the literal value. The note on the empty fields_limit
case now gives the reason it is refused.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Parsing the scheme with a case-insensitive split changed which header
forms reach the token review; a bare scheme, an empty token and a
scheme run into the token must still answer 401.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The fields_limit cap applied only when the client sent the parameter.
vlselect returns every group when it is absent, as it does for 0, so
leaving it out bypassed the cap. The capped value is now always sent.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Only the rate-limit 429 said when to retry. A concurrency or tail 429
came without it, so a client had no hint and could retry at once in a
loop. It now carries a one-second hint; when a slot frees depends on
the user's other requests, which the proxy cannot predict.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The authorization scheme is case-insensitive, and kube-apiserver
accepts "bearer" as well as "Bearer", so a client that works against
the API server got 401 from the proxy with the same header.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
An empty fields_limit fell back to the default of 0 and reached
vlselect as fields_limit=0, instead of being refused like any other
malformed limit.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The proxy can be left out of an installation, and then the console
nginx answered the observability roots with its own 502. The console
read that as an unreachable store: three errors, a Retry button and a
live tail reconnecting forever, instead of a notice that the feature
is not available on this cluster.
nginx now turns the 502 and 504 it generates itself, when it cannot
resolve or reach the proxy, into a 404 Kubernetes Status, which the
console already treats as not installed. Upstream errors are not
intercepted, so the proxy's own 502 and 504 for a store still reach
the console unchanged. The bats test runs the extracted locations in a
real nginx when one is installed.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
ParseForm reads no body on a GET or on a POST that is not a form. When
such a request declared a body and never sent it, net/http drained it
after the handler flushed the headers, with no read deadline, so the
request held its slot for as long as the client kept the connection.
The handler now reads the rest of the body itself, under the same
deadline and size cap as the form. Draining was chosen over refusing a
declared body so that clients which send one stay working. A body
over the cap now answers 413 instead of a generic 400.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The library is sourced by sh scripts and has no shebang, so shellcheck
could not tell which shell to check it as and refused the file.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The chart tests named the tenant egress policy as one of the parties
the pod label and port are pinned for. That policy is gone, so the
notes now name only the Service and the console nginx, which still
depend on them.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
go build ./cmd/observability-proxy drops the binary at the repository
root like every other command, and unlike them it was not ignored, so
git add . would commit it.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The default of 11000 buckets was justified by a metrics proxy that
does not exist on this branch. The number is Prometheus's cap on
points per series in a range query, which the metrics root reuses.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The rule was added for a tenant Grafana reading through the proxy, but
the tenant Grafana datasources still point straight at vlselect and
vmselect, and the console reaches the proxy from cozy-dashboard. The
rule therefore served no caller and only let every pod of every tenant
reach the proxy directly, outside the console route, where each
request with a fresh token costs a TokenReview.
It can come back together with a tenant client that needs it.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The background tail goes through the console route and the proxy's
reviews before it subscribes, so the early ingest can land first, and
a tail without start_offset returns only lines written after it
starts. The tail now replays the last minute, which still leaves the
hold and the late-line check meaningful.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
An absent namespace resolves to the root store, and before the
informer's first list every namespace is absent, so a request served
in that window read a labeled namespace's logs from the wrong store.
Readiness keeps the Service from routing there, but a client that
addresses the pod directly is not stopped by it.
The resolver now fails while the cache is unsynced, which answers
503 like any other resolution error.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
vlselect widens the window to step boundaries and returns a
zero-filled bucket for each one, so a start off a boundary yields one
bucket more than the span divided by the step. Sizing the step from
the unaligned span let hits and stats_query_range return MaxBuckets+1
buckets.
The minimum step now spans MaxBuckets-1 steps, and the smallest
accepted --logs-max-buckets rises to 4 so that this step still fits
the third-of-range cap that keeps alignment inside the window.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The flag parser stops at the first argument that is not a flag, so
a stray word in the container args made every flag after it fall back
to its default without an error, the root tenant included.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
NaN fails the negative check, as every comparison does, and +Inf
refills a bucket instantly, so either value switched the per-user rate
limit off while the flag looked set. Both now stop the proxy at
startup.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
When the bucket map was full and the sweep freed nothing, the new
user's bucket was added anyway, so the map grew past its bound and
every further newcomer paid a full scan under the limiter lock.
A newcomer is now refused with 429 until the first bucket refills,
and the Retry-After tells it when. Resetting the map instead would
hand every throttled user a fresh burst.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
A TokenReview whose authenticator failed comes back unauthenticated
with status.error set, and a SubjectAccessReview whose authorizer
failed comes back with an evaluation error and no decision. Both were
cached as refusals, so a valid caller stayed locked out for the cache
TTL after the outage ended.
Such answers are now refused for the current request only. A review
that failed to decide still answers 401 or 403 rather than 503: RBAC
sets an evaluation error for any binding to a missing role, which a
retryable status would turn into a permanent 503 for that user.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Once the status line has gone out, a read error from the store left
the handler returning normally, so the chunked body ended cleanly and
the client took a partial answer for a complete one.
The handler now aborts the response in that case. The request is
counted before the abort, because the abort skips the rest of the
handler.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The tail idle timer started only once the store had sent its headers,
and a tail request carries no deadline, so a store that accepted the
request and stayed silent held the tail slot until the client left.
The timer now starts before the request, and the same timer bounds the
header wait and the gap between lines. A store that never answers
gets 504, like a non-tail request past its deadline.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
A client that stopped reading kept its slot for good: the first flush
ran before the tail idle timer started, and cancelling the upstream
request does not interrupt a write that is blocked on the client.
Every response now carries a write deadline. A tail renews it by the
idle timeout before each line, so a tail that keeps receiving stays
open while a stalled reader is cut and frees its slot; any other
response must be written by the request deadline.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The form was parsed before the concurrency slot was taken, and the
listener sets no body-read deadline, so a client that never finished a
POST body held a handler outside every limit for as long as it liked.
Tenant workloads reach the Service directly, so nginx buffering does
not cover this path.
The slot is now taken first and the body is read under the request
timeout; a body that does not arrive in time answers 408 and frees the
slot. The deadline is lifted once the body is read, because the
server's background read would take a later expiry for a client that
went away and cancel a live tail.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Only tails opt out of the transport's gzip negotiation; bounded
requests keep it on purpose. Assert that side too, so a change that
applies identity to every request fails a test instead of silently
dropping compression between the proxy and the store.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Go's transport asks for gzip on its own when a request sets no
Accept-Encoding. vlselect answers a gzip-negotiated tail through a gzip
writer that holds even the response headers until the first line, so the
proxy could not forward its 200 and the console kept showing a
connecting state for quiet applications; buffered lines were also
delayed.
The tail request now sends Accept-Encoding: identity, which also turns
off the transport's transparent gzip. Other endpoints keep it: they are
bounded responses read to the end, compression saves bandwidth between
the proxy and the store, and the transport decompresses before the body
is copied to the client.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The wait until the next token was converted from float to Duration
without a range check, so a rate limit below about 1e-10 requests per
second produced a negative Retry-After on amd64. The wait now saturates
at the largest Duration.
The earlier 429 change said range, step, bucket and limit violations
keep 422; on the logs root they are clamped or answer 400, and nothing
answers 422.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
parseDuration summed duration parts without an overflow check and
converted out-of-range floats to Duration, which Go leaves
implementation-defined. A start_offset such as
99999999999h99999999999h1s wrapped to 999ms and was accepted, and a
huge step saturated on some architectures and wrapped on others. Such
values now answer 400 like any other invalid duration.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Nothing the proxy does reads ApplicationDefinitions, so the rule for
them is dropped. Namespaces are read through an informer, which only
lists and watches, so get is dropped as well.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Clients such as Grafana and browser retry logic back off on 429 Too
Many Requests, not on 422, which they treat as a final rejection of
the query. The per-user rate limit, the per-user concurrency limit and
the open-tail caps are about the caller's pace, not the query, so they
now answer 429. The rate limiter knows when its bucket refills, so that
answer carries Retry-After; the concurrency caps cannot know when a
slot frees and omit it. Range, step, bucket and limit violations keep
422.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The suite checked that B's line and the account-7 line are absent from
A's answers but only waited for A's line to become searchable, so an
absence could come from a line the store had not indexed yet. The step
budgets in the comments now count the calls the steps make.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Two of the three windows divided into whole milliseconds, so they
passed with the old truncating step and guarded nothing. Each window
now leaves a sub-millisecond step remainder.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Unit tests mock the API server and the store, so nothing shows that the
console's nginx, the proxy's TokenReview and SubjectAccessReview and a
real VictoriaLogs agree. The suite drives a viewer of one tenant through
the console against a seeded store: it reads its own namespace, is
refused a sibling's, cannot widen the scope with client filters, an
AccountID header or dot segments, and keeps a tail open past a minute.
The root store is off in e2e, so the suite runs its own VictoriaLogs
under the root store's Service name rather than waiting on Fluent Bit.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The step is sent to vlselect in milliseconds and was truncated, so a
window that the bucket cap does not divide evenly got one bucket more
than --logs-max-buckets allows. Round it up before the window is
shrunk to fit the step, so both caps still hold.
Bare-seconds steps and start offsets are now tested on the accepting
side too: only the refused forms were, and disabling the digit branch
passed the suite.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Startup refuses --logs-max-buckets below 3 and --logs-max-range below
3s, but only the refusals were tested, so moving either bound up by
one went unnoticed. Pin the smallest accepted pair.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The console nginx config now ships as an entrypoint template, and a
failure message naming the old conf.d path sends the reader to a file
the Containerfile no longer writes.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The logs and metrics roots under /apis/cozystack.io and
/apis/apps.cozystack.io are served by the observability proxy, not by
the API server, so the console nginx sends them there ahead of its
/apis prefix location, unbuffered and with the same long timeouts, so
that a live tail reaches the browser as it is written.
nginx resolves a literal upstream once at startup and refuses to start
when the name does not resolve, which would take the console down on
every cluster that does not run the proxy. The upstream is therefore a
variable resolved per request, and such a cluster answers 502 on those
paths only. That resolver does not apply the pod's search domains, so
the config becomes a template the image's entrypoint renders with the
pod's nameservers and the platform cluster domain, which the chart now
passes to the console.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
A tenant's Grafana reads the tenant's logs and metrics through the
observability proxy, and tenant egress is default-deny with one
allow-to-* policy per platform service. The rule is narrowed to the
proxy pods and their port, since the proxy's namespace holds nothing
else a tenant should reach.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The proxy only answers for stores that the monitoring stack runs, so it
follows monitoring-agents: on by default in the full variants and
opt-in through enabledPackages on the slim ones, where monitoring is
opt-in too. It is a stateless reader with no CRDs, so networking is its
only dependency.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Tenants read their logs, and later their metrics, through an
authorizing proxy that sits in front of the VictoriaLogs and
VictoriaMetrics read paths, so the proxy needs a deployable package
before the console or tenant Grafana can reach it.
The proxy reviews every token and access decision against the API
server, so its own role is limited to creating TokenReviews and
SubjectAccessReviews and reading ApplicationDefinitions and namespaces.
The namespace logs root is authorized as list on cozystack.io logs,
which no API server serves; the rule is aggregated into the tenant view
role so that a namespace viewer passes that review, as upstream grants
pods/log at view.
The image ref lands without a digest until the first build stamps it,
and the package joins the root build list so every build path refreshes
it.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
The step is capped at a third of the maximum range so that vlselect's
alignment to whole steps cannot widen the window past that range. With
fewer than three buckets, or a range under three seconds, the minimum
step exceeds that cap and the bound no longer holds, so such settings
are refused at startup.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Bare seconds were parsed with ParseFloat, which also takes signs,
exponents, Inf and NaN. Such a start fell through to the relative
duration form and silently moved the window, so 1.7937864e9 meant
1970 rather than an error, and NaN converted differently per CPU
architecture. Only digits with an optional fraction are taken now;
everything else is a 400.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
Zero is the documented way to disable a limit; a negative value
disabled it too, silently. Refuse it so a typo in the chart cannot turn
off a per-user or total cap unnoticed.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
vlselect widens [start, end] outward to whole steps before scanning, so
a step of a hundred years turned the clamped week into the whole
retention. The step is now capped at the window and at a third of the
maximum range, and the window is shrunk from its start until the
widened span still fits the maximum range.
Numeric times are accepted only as unambiguous unix seconds, because
vlselect reads the unit from the digit count and a millisecond stamp or
a bare year would otherwise land in the wrong place. The standalone
ignore_global_time_filter substring check is dropped: the options block
check already covers it, and the substring only refused plain searches
for that word.
Assisted-by: LLM
Signed-off-by: ZverGuy <maximbel2003@gmail.com>
ZverGuy