mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-12 23:54:01 +09:00
28b391ff176943ccd54fdfdbaef3f7f0beb9ccc6
7783
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 ZverGuy
|
28b391ff17 |
fix(observability-proxy): round the step up to whole milliseconds
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> |
||
|
ZverGuy
|
a56ff5050f |
test(observability-proxy): accept the smallest satisfiable step cap
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> |
||
|
ZverGuy
|
d452b30e42 |
test(dashboard): name the templated nginx config in the buffer check
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> |
||
|
ZverGuy
|
74a7a44423 |
feat(dashboard): route the observability proxy roots from the console
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> |
||
|
ZverGuy
|
f7719cf6e2 |
feat(tenant): allow egress to the observability proxy
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> |
||
|
ZverGuy
|
bbdf05c6c7 |
feat(platform): deploy the observability-proxy
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> |
||
|
ZverGuy
|
7ec648d863 |
feat(observability-proxy): add the chart and image build
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> |
||
|
ZverGuy
|
ca7f283b6a |
fix(observability-proxy): require a range and bucket count the step cap fits
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> |
||
|
ZverGuy
|
4e8071aa2b |
fix(observability-proxy): accept bare seconds only as plain digits
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> |
||
|
ZverGuy
|
8eef2fa5d2 |
fix(observability-proxy): reject negative limits at startup
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> |
||
|
ZverGuy
|
d1c49b38d4 |
fix(observability-proxy): keep step from widening the logs window
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
|
1c345da367 |
fix(observability-proxy): reject negative timeouts at startup
A negative duration silently disables the bound it configures, so a typo in the chart would turn off the request deadline or the tail idle close without any sign. Refuse it when the flags are parsed. Assisted-by: LLM Signed-off-by: ZverGuy <maximbel2003@gmail.com> |
||
|
ZverGuy
|
159f90691e |
fix(observability-proxy): tell a slow store apart and bound tail memory
A request that hits the proxy's own deadline was answered like an unreachable store, so a heavy query looked like an outage; it now answers 504, and a client that went away is no longer logged as a store failure. A tail line is read through a fixed buffer, so a single huge log line cannot make one stream hold unbounded memory. Assisted-by: LLM Signed-off-by: ZverGuy <maximbel2003@gmail.com> |
||
|
ZverGuy
|
5fc2cba2d5 |
fix(observability-proxy): refuse a monitoring label that is not a namespace
The label value is spliced into the vlselect host name. A value with a dot would address a host in some other namespace instead of a tenant's store, so it is treated as a resolution error rather than followed. Assisted-by: LLM Signed-off-by: ZverGuy <maximbel2003@gmail.com> |
||
|
ZverGuy
|
8fe3c222d3 |
fix(observability-proxy): reject non-canonical request paths
The console nginx selects its location on the normalized path but forwards the original request URI. A path such as namespaces/tenant-b/../tenant-a/logs/ reaches the proxy as written, so any interpretation the proxy makes of dot segments, empty segments or percent-encoding could make the namespace in the SubjectAccessReview differ from the one a normalizer downstream would see. Such paths are answered 400 instead of being interpreted. Assisted-by: LLM Signed-off-by: ZverGuy <maximbel2003@gmail.com> |
||
|
ZverGuy
|
cbca8ac3c9 |
fix(observability-proxy): refuse every LogsQL options block
A substring match on ignore_global_time_filter does not see the option once it is written as a quoted name with an escape, or split by a comment: the LogsQL lexer unquotes option names and joins words across a comment, so the option still applies and lifts the time window the proxy enforces. None of the query options is needed by a tenant, so the options keyword followed by a parenthesis is refused outright, skipping whitespace and comments exactly as the lexer does. Assisted-by: LLM Signed-off-by: ZverGuy <maximbel2003@gmail.com> |
||
|
ZverGuy
|
8b0ac342b5 |
feat(observability-proxy): add the observability-proxy command
The deploy unit builds an image and chart around this binary, so its flags follow the agreed contract: listen and probe addresses, cluster domain, root tenant and auth cache TTL, plus the logs limits. Defaults are chosen so a stock install is safe without tuning; readiness waits for the namespace cache because store resolution reads it on every request. On shutdown open tails are cut after a grace period so their clients reconnect with since to another replica. Assisted-by: LLM Signed-off-by: ZverGuy <maximbel2003@gmail.com> |
||
|
ZverGuy
|
689405127d |
feat(observability-proxy): serve the namespace logs root
Tenants get a way to read their namespace's logs without Grafana and without reaching vlselect directly, which would give them neither an RBAC check nor a forced scope. The handler authenticates, authorizes from the path, rebuilds the upstream request with fixed tenant headers and no client headers, and returns VictoriaLogs' native response. A live tail is flushed after every line, is not subject to the request timeout, and is closed once idle, so it survives far past the apiserver request timeout without leaving forgotten browser tabs holding streams open. An unreachable store answers 502 so the dashboard can tell an outage from an empty result. Assisted-by: LLM Signed-off-by: ZverGuy <maximbel2003@gmail.com> |
||
|
ZverGuy
|
61275df240 |
feat(observability-proxy): cap per-user concurrency, rate and open tails
The log stores are shared by every namespace routed to them, and the root store also serves platform Grafana, so one tenant's dashboard must not be able to monopolise vlselect. Live tails are counted apart from request concurrency because they hold their slot for minutes and would otherwise starve the same user's panels. Assisted-by: LLM Signed-off-by: ZverGuy <maximbel2003@gmail.com> |
||
|
ZverGuy
|
eb0d6e5957 |
feat(observability-proxy): rebuild logs queries from an allowlist
vlselect has no notion of a tenant: whatever filter reaches it decides what is returned. The upstream parameters are therefore rebuilt from a per-endpoint allowlist with the namespace scope as the only extra_stream_filters value, so a client cannot widen it through extra_filters, tenant headers or an alternate parameter spelling. VictoriaLogs ANDs extra_stream_filters into every subquery, so union and in() queries pass through unparsed. What the query can still do is cost: a missing start scans the whole retention, a tiny step returns a bucket per millisecond, query has no default limit, and ignore_global_time_filter lifts the window altogether. Each of those is bounded or refused here, and a tail resume point becomes a capped start_offset. Assisted-by: LLM Signed-off-by: ZverGuy <maximbel2003@gmail.com> |
||
|
ZverGuy
|
e02d0fa16e |
feat(observability-proxy): resolve a namespace to its log store
Fluent Bit writes a namespace's logs to the store its monitoring label names, falling back to the root tenant, so exactly one store answers for a namespace. Reading with any other rule would show an empty log view or the wrong tenant's store, so the resolver repeats the collector's rule and is tested against the same namespace set as its chart test. It reads a namespace lister, keeping store lookups off the apiserver. Assisted-by: LLM Signed-off-by: ZverGuy <maximbel2003@gmail.com> |
||
|
ZverGuy
|
448cd2b5bd |
feat(observability-proxy): authenticate and authorize with cached reviews
The proxy holds no credentials of its own: kube-apiserver answers who a bearer token belongs to and whether that identity may read a scope, the same way an aggregated API server would ask through delegated auth. Both answers are cached briefly so a dashboard polling several panels does not turn into a review per request, while transport errors are left uncached so an apiserver blip does not lock users out for the TTL. Assisted-by: LLM Signed-off-by: ZverGuy <maximbel2003@gmail.com> |
||
|
ZverGuy
|
8e35ff521c |
feat(observability-proxy): parse the four tenant observability roots
The proxy derives its single SubjectAccessReview from the request path, so the path has to be split and validated before anything else runs. Names are DNS-1123 checked because they end up quoted inside LogsQL and PromQL scopes, and percent-encoding is refused so a decoded slash cannot shift the segments authorization is built from. Assisted-by: LLM Signed-off-by: ZverGuy <maximbel2003@gmail.com> |
||
|
myasnikovdaniil
|
4087218a02 |
docs: move the repo-only guides onto the website (#5034)
## What this PR does Seven guides under `docs/` were operator facing and written alongside the code, and left in the source tree where no user looks. They are on the website now, so this deletes them here and repoints every reference. ``` docs/operations/backup-classes.md -> operations/services/backup-classes docs/operations/resource-presets.md -> guides/resource-management docs/storage-immutability.md -> operations/storage-class-immutability docs/operations/system-memory-limits.md -> operations/configuration/system-memory-limits docs/gateway-classes.md -> networking/gateway-classes docs/observability/distributed-tracing.md -> operations/services/monitoring/distributed-tracing docs/operations/seaweedfs-capacity-for-backups.md -> operations/services/object-storage/capacity-for-backups ``` `docs/release.md`, `docs/deprecating-applications.md` and the two SeaweedFS runbooks in `docs/operations/` stay. A workflow, `AGENTS.md`, a migration and a bats suite read them by path, which is the test from #3777. References point at `https://cozystack.io/docs/v1/...`. The site rewrites that short prefix to the latest version in `layouts/404.html`, so the links follow releases and pin none. Backup Classes, resource management and `storageClass` immutability exist in the latest version already. The other four pages are only in `next/` until 1.7.0 is promoted, and their links start to resolve then. The generated mongodb files come from `values.yaml` through `make generate` with cozyvalues-gen v1.7.0, the version pinned in `.github/workflows/pre-commit.yml`. Tests: helm unit tests for mongodb (151), postgres (209), vm-disk (114) and kubernetes-nodes (223), go tests for backupcontroller, presets and the operator, rd-presets-check, the migration-50 bats suite. ### Downstream repositories - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: cozystack/website#763 Walked the rest of the trigger map against the diff. No provider follow-up: the mongodb `values.schema.json` diff is one `description` string, with no field added, removed or renamed, no enum, no default, and no `kind`, `plural` or `release.prefix` change. `packages/core/platform/values.yaml` changed in comments only and no key, so the hand written platform-package table on the site is unaffected. Please merge after cozystack/website#763, the links in the code only resolve once those pages deploy. ### Release note ```release-note docs: seven guides move from the docs/ directory of this repository to cozystack.io (Backup Classes, resource presets, storageClass immutability, system component memory limits, GatewayClass selection, distributed tracing, SeaweedFS capacity for backups), and in-repo references point there. ``` |
||
|
Timofei Larkin
|
e31f518df7 |
ci(api-review-gate): publish the gate as a commit status (#3440)
## What this PR does Publishes the `API Review Gate` merge context as a commit status, so a skipped or cancelled run cannot pass it or poison it, and gives fork PRs the same gate through a privileged `workflow_run` workflow. ### Why The merge context was a job name. A required job that skips counts as passed, and check runs are not deduplicated per name, so one cancelled run on a head keeps the context failed until the next push. Measured on this repo: `41d1153c` has four check runs with the gate's job name (one `cancelled`, three `success`). Commit statuses are deduplicated per context and the newest wins, so a run with no verdict posts nothing and the previous status stays. ### What changes Same-repo PRs get status from `api-gate-report`. A `pending` opener marks the head before the evaluation on approve, dismiss and request-changes reviews, so an old `success` is not mergeable for the minute the evaluation takes. `COMMENTED` reviews no longer count as a reviewer's latest review, so an owner who approved and then commented keeps the approval. Concurrency is split, so bot comment run cannot cancel live evaluation. Fork PRs: `api-review-gate-fork.yaml` runs from the default branch and reaches the verdict itself, the fork's run is only a trigger. `resolve` finds PR by head SHA in open PR list (review runs on a fork report base repo as head repository and have no pull requests). `detect` fetches the head as data and runs the detector in a job with `contents: read` only. `conclude` applies the approval rule and posts. A sweep every 15 minutes moves the status of fork PRs whose runs wait for a maintainer in `action_required`, those never fire `workflow_run`. This is lllamnyp's patch, applied whole, with these changes on top: - two open fork PRs on one head fail closed in `resolve` and in the sweep, a head carried by a same-repo PR stays with `api-gate-report` - detector is built from default branch only, merge base is picked by PR author and an old one would mean an old detector - the fork `detect` step runs under `bash -e` and detector exit 2 ended it before the code was read, so it has `set +e` like the same-repo job - head is checked out with links as links, like the same-repo gate does, because flattening them hid a CRD behind a file link from the detector while Helm still ships it ### Limits - A fork PR whose push run waits for a maintainer's approval has no status until that run executes, same as `E2E Tests`. - A fork PR on a head that a same-repo PR also carries shows the verdict standing there, one commit status cannot hold a verdict per base. - The sweep can publish a stale `success` if it reads an approval, stalls, and the owner withdraws it meanwhile. The next sweep flips it back, so the window is up to 15 minutes. - Two copies of one first-party CRD and CRDs behind directory links are gaps of the detector on main, same for same-repo PRs, they are in https://github.com/cozystack/cozystack/issues/5170. ### Testing `actionlint` is clean on both workflows. `zizmor` is clean too, but `dangerous-triggers` and `template-injection` are disabled repo-wide in `.github/zizmor.yml`, so that says little about a privileged workflow. I ran the `github-script` bodies under node with a mocked API (every conclusion, approval and shared head case) and the run steps under `bash -e` with a stub detector exiting 0, 2 and 1. `make unit-tests` passes with `LC_ALL=C` (a kafka bats test depends on the locale, nothing to do with this PR). Not exercised live: the fork path and the sweep run only from the default branch, so their first run is after merge. ### Repo admin Branch protection requires the old job name `Require API owner review for sizeable API changes`. This PR renames that job, so after merge the required context has to be changed to the `API Review Gate` status. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note NONE ``` |
||
|
Aleksei Sviridkin
|
a2cc9a3016 |
fix(dashboard): link a tenant row to its detail page (#4004)
## What this PR does Fixes #3822 Tenant list showed name as plain text and the only link in a row was Edit button, so tenant detail page with its tabs and Delete was reachable only by opening edit form and cancelling out of it. Name cell is a link to that page now, it appears exactly where Edit appears and points to the same CR. Plain click switches selected tenant to the row's parent first, same as Edit does, because detail page takes namespace from selected tenant. Ctrl, cmd and shift click don't run that switch, so the tab you open a tenant from stays where it was. First commit fixes what the link depends on. Console recovers a tenant's parent from namespace name (`tenant-a-b` sits under `tenant-a`), but root is the one level that naming skips: tenant `acme` ordered in `tenant-root` owns `tenant-acme`, not `tenant-root-acme`. So `realParentNamespace` returned undefined for every top level tenant, which is most tenants on a normal cluster, and Edit button has been missing there for as long as it exists. Ancestor labels carry the parent that the name does not. Root level tenant named `root-x` resolves to CR `root-x` now, not `x`. ### Notes I first tried to make links carry their tenant in the url (`?tenant=`), so a detail page opened in new tab resolves on its own. That turned into questions about tenant context itself: what to do with a tenant in a link the user can't see, what localStorage may hold, and TenantNamespace LIST ignoring `labelSelector`. Detail route addresses a Tenant CR by relative name and takes namespace from selected tenant, and making that robust is a refactor of tenant context and routing, which should start with study and planning, not with patches on patches. So this PR only closes the UX gap (no way from tenant list into a tenant's page), going deeper is out of scope. Known limit that comes with it: link has no tenant in it, so opening it in a new tab (ctrl/cmd/middle click, open in new tab, copied url) resolves against the tenant last selected in any tab, because selection lives in shared localStorage. For a row directly under that tenant it opens the right page. For a row deeper in the tree the page says `Application <name> not found.` and there is no Delete. If that tenant also has a direct child tenant with the same relative name, new tab opens that other tenant's page with working Edit and Delete, and delete confirm says only `Delete Tenant "<name>"?`. Plain click in the same tab is not affected. Detail page's own tabs and Edit link already behave this way, it's how detail route resolves its namespace today and fixing it properly is the refactor above. A row links, and offers Edit, exactly when namespace holding its Tenant CR is one the apiserver returns to this user. It returns a tenant namespace only if the user is subject of a RoleBinding in it, so sub-tenant admin that cant act in `tenant-root` sees no link there. No row level Delete button. Issue allows either that or a route to detail page, I took the route so confirm and mutation stay in one place. Covered by tests, every source change here fails a test when reverted: - top level tenant, nested one, root level tenant named `root-x`, forest root without ancestor labels - bridged row whose real parent is invisible gets neither link nor Edit - plain click switches to the row's parent and opens detail page - ctrl click leaves selection and location alone ### Screenshots Tenant list under root on a dev cluster. Every row name is a link to tenant page now (`pr4078` is hovered), top level tenants had neither link nor Edit before. Host column redacted. <!-- SCREENSHOT: drag in 01-tenant-list-links.png here --> <img width="1440" height="900" alt="01-tenant-list-links" src="https://github.com/user-attachments/assets/39cd723a-1ac5-4d3b-8c36-b70ddc626961" /> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: All changed files are under `packages/system/dashboard/images/console`, console has no downstream copy (standalone UI repo is archived). ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(dashboard): tenant name in the tenant list links to its detail page now (where Delete is), and top level tenants get their Edit button back. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * Root-level tenant names are now displayed without incorrectly removing prefixes, and tenants ordered directly under the root are correctly recognized in the hierarchy. * Tenant rows link to their detail pages when the parent tenant is accessible. Same-tab navigation updates the active tenant, while opening a link in another tab leaves the current selection unchanged. * Edit links and navigation now use consistent tenant routes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
b5e3b37a9a |
ci(api-review-gate): build the fork detector from the default branch
The merge base is the commit the fork's author branched from, so a branch cut from an old commit selected an old detector, and a fix to the detector never reached the PRs an attacker controls. The workflow itself comes from the default branch, so the detector now does too. The head is also checked out with its links intact. Flattening them hid a CRD behind a file link from the detector while Helm still shipped it, and made the verdict differ from the one the same-repo gate reaches. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Aleksei Sviridkin
|
d32092ac9a |
feat(release): attach an SBOM to every release image (#4997)
## What this PR does
This PR attaches an SPDX SBOM to every first-party image of a release.
BuildKit adds it at build time as an attestation, so it sits in the
image's index and moves with the digest the release pins, through
promotion. `docker buildx imagetools inspect <image>@<digest> --format
'{{ json .SBOM }}'` prints it, one document per platform. Releases cut
from this change on have it. The Talos installer image (built by the
imager, pushed by skopeo) and the third-party images don't.
It is more than `SBOM: '1'` in `tags.yaml`, because three things broke
on the way:
- An image built with an attestation is an index of the image and an
attestation manifest. `image_arch` in `hack/stitch-multiarch.sh` read
that as not one amd64 and one arm64 image, so the stitch skipped every
such image and still exited 0, and `hack/verify-multiarch.sh` then
failed the release with "index without linux/arm64". `image_arch` and
the rerun check look through the index now, and the stitched index keeps
both attestations.
- The release `make build` ran on the default docker driver, which
cannot attach attestations. It runs on the docker-container builder with
the docker.io mirror now, like the arm64 leg. `use: false` stays, so a
plain `docker build` keeps the default driver.
- `--sbom=true` pulls `docker/buildkit-syft-scanner:stable-1`, a
floating tag. `SBOM=1` passes `--attest type=sbom,generator=` with the
scanner pinned by digest (1.12.0). The comment in `hack/common-envs.mk`
called the output CycloneDX, BuildKit writes SPDX, and the same word in
`docs/security/self-assessment.md` is fixed too.
Only the release lane changes. PR, main and nightly builds push to OCIR
and are not what users install, so the OCIR attestation probe the
tracker (A2 in #3278) listed is not on this path, release images go to
GHCR.
### How it was checked
- Bats 1.14.0: four new tests in `hack/stitch-multiarch_test.bats` (the
arm64 leg's workflow shape, a stitch of SBOM images, a rerun after one,
an index of two linux images that is skipped), two in
`hack/common-envs_test.bats` (the flags, and the Renovate manager held
to the `SBOM_SCANNER` line), and the matchbox workflow test rewritten
for the new builder. The stitch tests fail on `main`'s
`stitch-multiarch.sh`, and each of these mutations fails one: dropping
the `length == 1` check, counting the attestation as a platform, looking
for the amd64 index itself in the rerun, taking `SBOM`, `BUILDER` or the
mirror config out of `tags.yaml`, going back to `--sbom=true`, dropping
the scanner digest, turning `SBOM` on by default, reformatting the
`SBOM_SCANNER` line out of the Renovate manager's regexp.
- A rehearsal on a localhost registry (docker 29.8, buildx 0.37.1) with
the real scripts. Images built through the `BUILDX_ARGS` of
`common-envs.mk` for amd64 and arm64, one without SBOM beside them, then
`stitch-multiarch.sh` (2 stitched, 0 skipped), `verify-multiarch.sh` (0
failed), `imagetools inspect` reading SPDX 2.3 with 15 packages per
platform from the stitched index, and `skopeo copy --multi-arch all`
keeping the index digest. `containerimage.digest` in `--metadata-file`
equals the pushed index digest, so the digest the package Makefiles
stamp is the index. The same run with `main`'s stitch skips the SBOM
image, exits 0 and fails the gate.
- `make bats-unit-tests`: 2444 of 2445. The one failure, "all-topics
backup skips double-underscore internals and keeps single-underscore
user topics" in `hack/kafka-topic-backup-strategy.bats`, also fails on
unmodified `main` on the same machine. `make bats-posix-compat-tests`
passes, and the stitch test file passes under dash through
`cozytest.sh`. `hack/check-commit-trailers.sh` passes. zizmor and
actionlint aren't installed here, so the workflow change (two env keys
and one renamed builder step) is covered only by the tests above.
- Not checked: that GHCR takes the attestations, what retention does to
the child manifests, the fork lane's OCI export (it doesn't set `SBOM`),
and a real release build. The first alpha cut after the merge tests the
first and the last. Renovate itself did not run on the new manager: the
config passes the Renovate JSON schema, and the regexp takes the tag and
the digest off the live line, which a test holds.
### Decisions for the release owners
- Scope is `tags.yaml` only. SBOMs on main and nightly images would need
OCIR to take attestations (not probed) and would change what the nightly
mirror copies.
- The scanner is 1.12.0 with syft 1.51.0 inside, which is in the range
of GHSA-mw2c-m758-9v5q (memory exhaustion parsing binary inputs, fixed
in syft 1.52.0). It reads images our own builds produce, so I didn't
hold the change for it. A Renovate manager on the `SBOM_SCANNER` line
moves the version and the digest together when the scanner ships a newer
syft.
- The scanner adds a step per image and I didn't time it on the release
runners.
- A stitch rerun reads the amd64 index the tree pins, which stays in the
registry untagged after a stitch. If the registry drops untagged
manifests before the rerun, it fails at that read rather than repinning.
- After the first alpha with attestations I'll move A2b in #3278 and the
SBOM item in #2937 to done. The SBOM row in
`docs/security/self-assessment.md` still says SBOMs aren't published,
and stays until that alpha shows they are.
### Screenshots
<!-- REQUIRED for UI changes: attach screenshots or screen recordings
demonstrating
the visual impact of your changes. PRs with UI changes without
screenshots will not be merged. -->
### Downstream repositories
- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:
I walked the trigger map against the diff. It touches `tags.yaml`,
`hack/common-envs.mk`, the stitch and verify scripts and three docs. No
release asset is renamed, `hack/package.mk` and the commit convention
are untouched, and the flow a maintainer follows to cut and promote a
release is the same.
### Release note
```release-note
feat(release): every first-party container image of a release carries an SPDX SBOM as a BuildKit attestation in its index, readable with `docker buildx imagetools inspect <image>@<digest> --format '{{ json .SBOM }}'`. Releases cut from this change on have it, the Talos installer image and third-party images don't.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->
## Summary by CodeRabbit
- **New Features**
- Supported release image builds now include SPDX-format software bills
of materials (SBOMs) as attestations.
- Multi-architecture image stitching, retagging, and mirroring preserve
SBOM attestations.
- **Documentation**
- Updated image-reference guidance to explain how to handle attestation
entries in multi-architecture indexes.
- Clarified which images include SBOMs, how to inspect them, and that
SBOM generation remains disabled by default outside configured release
builds.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
IvanHunters
|
94ada11eef |
fix(marketplace): pin a tapped repository to a digest instead of a mutable tag (#5082)
## What this PR does A tapped (community-connected) repository followed a mutable OCI tag: the Flux `OCIRepository` carried only `spec.ref.tag` with a 5-minute interval, so whoever controlled the tag controlled what ran in the management cluster within that interval, down to the HelmReleases of already-running tenant application instances. Fixes #4979. A tap is now pinned to a digest. `cozypkg tap` and the connect API accept an explicit `oci://host/org/repo@sha256:...` reference; a tag reference is resolved and pinned to its digest by the tap materializer once the artifact for the current spec generation is ready, from the digest Flux reports in `status.artifact.revision`, with the tag kept informational. A tap connected before this change is pinned to its current digest on the first reconcile after upgrade, and a re-connect to a new tag waits for source-controller to re-resolve before pinning, so a stale revision is never pinned. Moving the tag upstream no longer changes anything in the cluster; an explicit re-connect moves the pin. Connect also accepts an optional keyless cosign verification policy (`--certificate-identity` / `--certificate-oidc-issuer` on the CLI, `spec.verify` on the API), anchored to an exact identity so a superstring cannot match, and preserves it, and the pull secret, across repeat connects rather than silently dropping them. With those flags the CLI verifies the signature at tap time and pins the verified digest before pulling, so the content it applies is what a trusted identity signed; it fails closed if the verified digest cannot be determined. ### Screenshots Not applicable: no UI changes. ### Downstream repositories I walked the trigger map against the diff. The change is confined to `cozypkg`, the `Tap` aggregated API (a virtual, cluster-scoped resource with no CRD), the tap materializer, and the community-index docs. It does not touch `packages/apps`, `packages/core/platform/values.yaml`, installer values, platform variants/components, release assets, `ApplicationDefinition` semantics, the `hack/` make targets, or the commit convention, so the provider, ansible, ccp and external-apps paths are not reached. One path is arguable and needs a human decision, so I have left every box unticked rather than tick or dismiss it: - `cozystack/website` — `cozypkg tap` gains flags and digest-pinning behaviour. The marketplace tap flow is new and I am not sure it has a reference page on the site yet, so whether a docs follow-up is warranted (and where) is a maintainer call. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(marketplace): a tapped repository is pinned to a digest instead of following a mutable tag, so moving the tag upstream no longer changes what runs in the cluster; connect accepts oci://...@sha256:... and an optional keyless cosign verify policy (anchored to an exact identity and preserved across re-connects), and with verify flags the CLI verifies the signature and pins the verified digest at tap time. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit - **New Features** - OCI repositories can be connected using a SHA-256 digest, with or without a tag. Tag-based sources are pinned to the resolved artifact digest. - Optional Cosign keyless verification is available during connection using certificate identity and OIDC issuer details. - Reconnecting preserves existing verification and pull-credential settings when replacement values aren’t provided. - **Bug Fixes** - Reconnecting with a new tag clears previous operator-managed pins so the requested tag can resolve to its current artifact. - **Documentation** - Clarified tag pinning, signature verification, and where tapped app charts run. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
ccc216652e |
fix(monitoring-agents)!: isolate merged log payload under log_processed (#5167)
## What this PR does Fluent Bit's `kubernetes` filter runs with `Merge_Log On` but without `Merge_Log_Key`, so a parsed JSON log line is merged into the record root. An application that logs JSON can thereby set top-level fields that later pipeline stages trust for routing and tenant isolation: `kubernetes_namespace_name` (rewrite_tag routing and the `_stream_fields` stream identity), `kubernetes.labels`, and the `tenant` / `cluster` / `log_source` markers. A tenant workload can forge another tenant's values on its own log lines. Setting `Merge_Log_Key log_processed` nests the parsed payload under a dedicated key, so application log content can no longer override the top-level service fields. The change is applied to both Fluent Bit configurations that carry this filter: the platform agent (`packages/system/monitoring-agents/values.yaml`) and the guest-cluster agent (`packages/apps/kubernetes`). ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, file by file. The change touches only the Fluent Bit filter config inside `monitoring-agents`; it does not alter package layout, any `values.schema.json`, platform values keys, `ApplicationDefinition`, any CRD, `hack/`, namespaces, telemetry metric names, or the commit convention. None of the listed couplings match. - [x] No downstream repository is affected by this change ### Release note ```release-note fix(monitoring-agents)!: Fluent Bit now nests parsed JSON log content under a `log_processed` key instead of merging it into the record root. This prevents an application log line from forging the service fields used for log routing and tenant isolation. BREAKING: top-level fields previously extracted from JSON logs (for example `level`, `msg`, and any application-specific keys) now live under `log_processed.*`; external dashboards and LogsQL queries that read them from the record root must be updated. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Improvements** * Merged Kubernetes log content is now grouped under `log_processed` in Fluent Bit output. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
7f55ff5d85 |
fix(dashboard): link a tenant row to its detail page
The tenant list rendered the name as plain text and put the row's only link on an Edit button, so nothing in the list reached the detail page that every other kind gets, tabs and a Delete action included. The only way there was to open the edit form and cancel out of it. The name cell becomes a link wherever Edit is offered, to the same CR. The detail page reads the namespace from the tenant context, so a plain click aligns the context to the row's parent first, the way Edit already does. React runs that handler for a ctrl-, cmd- or shift-click too, which the browser turns into a new tab or window, so it runs only for the click that navigates its own tab; otherwise opening a tenant beside the current one would move the tab left behind as well. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
e44e41d307 |
fix(dashboard): resolve the Tenant CR of a tenant ordered in the root
A tenant's CR lives in its parent's namespace, and the console recovered the parent from the namespace name: `tenant-a-b` sits under `tenant-a`. The root is the one level that naming skips. A tenant `acme` ordered in `tenant-root` owns `tenant-acme`, not `tenant-root-acme`, so the prefix test never matched a top-level tenant and `realParentNamespace` returned undefined for it. That is most tenants on a normal cluster, and the Edit button, offered only where the CR's namespace resolves, was missing for all of them. The ancestor labels carry the parent the name does not, so the lookup falls back to `tenant-root` when the chain names it. Deriving the CR name keys on the parent being the root rather than on the child's name happening to start with it, which keeps a root-level tenant called `root-x` from resolving to the CR `x`, and the row label agrees with it rather than showing `x`. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
dfa2283a13 |
ci(api-review-gate): close a standing success on a shared fork head
A commit status belongs to the commit. When two open fork PRs carry the same head, a success already standing on it may have been earned by one of them on another base, or may rest on an approval that was withdrawn on a PR whose run waits for a maintainer and so never reaches resolve. The sweep skipped such a head and left that success in place, which let a duplicate fork PR keep a withdrawn approval from taking effect. The sweep now writes the same failure resolve writes, once per head, when a success set by a workflow stands on a head that only fork PRs carry. It still never creates a status, and a head that an open same-repo PR carries stays with api-gate-report. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
0538e70591 |
ci(api-review-gate): let a sizeable fork change reach the approval check
api-gate exits 2 to report a sizeable change, and the runner runs a step without an explicit shell as bash -e. The fork detect step switched errexit off only after reading the exit code, so the first failing command ended the step with 2 before sizeable=true was recorded. Every fork PR with a sizeable API change then got "gate evaluation did not complete", which the sweep deliberately ignores, so an owner's approval could not turn it green by either path. The same-repo step already switches errexit off before the call. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
878f569e61 |
ci(api-review-gate): leave a shared head to its same-repo publisher
A commit status belongs to the commit, so two open PRs that carry the same head share one "API Review Gate" status. The fork workflow attributed that status to a fork PR by head SHA alone. A fork PR opened at a commit of this repository could therefore overwrite the verdict of the same-repo PR that owns it: resolve wrote a failure after api-gate-report had posted success, and the sweep applied the fork PR's reviews to a status the other PR had earned. A fork can reference any commit of the repository's network, so anyone able to open a PR could hold another PR's gate red. The fork workflow now leaves a head that an open same-repo PR carries to api-gate-report, and the sweep skips any head carried by more than one open PR. Two fork PRs at one head still fail closed in resolve, since their bases, and so their verdicts, can differ. The header of the same-repo workflow said skipped and cancelled runs post nothing. That stopped being true once a review posts pending ahead of its evaluation, so it now says they post no verdict. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
4c37ef3a61 |
ci(api-review-gate): let owner approval reach fork PRs
On a fork PR an API owner's approval could never turn the status green. For a pull_request_review run on a fork PR, workflow_run.head_repository names the base repository and pull_requests is empty, so the fork publisher took every review-triggered run for a same-repo one and skipped it, while the same-repo reporter skipped it for being a fork. A sizeable fork PR stayed on the failure its push run had posted. Fixing the guard alone would leave most forks stuck. The repository makes every run from an external contributor wait for a maintainer's approval, and a run in action_required never fires workflow_run, so their review-triggered runs never reach the publisher. And a fork's run executes the fork's own copy of the gate workflow, so the conclusion the publisher relayed was whatever the fork made it. The privileged workflow therefore reaches the verdict itself, from inputs a fork cannot forge, and a scheduled sweep covers the runs that never execute. The sweep only moves a status the full evaluation already set after finding a sizeable change, so it cannot create a pass. The same-repo gate now posts pending as soon as an actionable review arrives: a dismissal can turn a green head red, and the old success would stay mergeable for the minute the evaluation takes. Both paths also ignore COMMENTED reviews when taking each reviewer's latest, since an owner who approved and then commented had withdrawn their own approval as far as the gate was concerned, which GitHub does not do. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
939496ad78 |
docs(e2e): record the gate-status pattern's second consumer
The "E2E Tests" commit-status pattern in §10 now has a second consumer in api-review-gate.yaml, and the two GitHub behaviours that make it load-bearing were not written down anywhere. Check runs are not deduplicated per name across workflow runs — each run gets its own check suite, so filter=latest returns the same set as filter=all and the aggregate is worst-of, meaning a cancelled run poisons a context until the next push — while commit statuses are deduplicated per context with the newest winning. And the fork read-only token restriction covers pull_request_review as well as pull_request, so an approval on a fork PR cannot publish a status either, which is why both gates need a privileged workflow_run half rather than only handling the push path. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
b771962eb8 |
ci(api-review-gate): publish the gate as a commit status
The gate's merge context was its job name, which inherits two GitHub behaviours that together make it both unenforceable and self-poisoning. A required *job* that skips counts as PASSED by branch protection, so any path that skips the evaluating job hands out a free pass — the hole that let fork PRs merge with e2e never run (#3257). And check runs are not deduplicated per name: every workflow run gets its own check suite, so `GET /commits/{sha}/check-runs?filter=latest` returns the same set as `filter=all`, and the rollup is worst-of. Two runs on one head SHA therefore leave two verdicts standing for one context name, and a cancelled run poisons the context until the next push. Commit statuses are deduplicated per context, newest wins. Rename the evaluating job off the gate context and publish "API Review Gate" as a commit status: `api-gate-report` for same-repo PRs, and a privileged workflow_run for forks, whose GITHUB_TOKEN is read-only on both the pull_request and the pull_request_review trigger and so can never post a status itself. Skipped and cancelled runs post nothing, so declining to post can only ever preserve a verdict some real run reached — it can never invent one. That invariant is what makes it safe to stop re-evaluating on the plain `commented` reviews the review bots post: such a review cannot move a verdict that counts only APPROVED reviews, and the run that skips now leaves the previous status untouched instead of overwriting it. Also split the concurrency group so a run that computes no verdict cannot cancel one that does. A bot commenting during the ~30s evaluation used to cancel it and then contribute nothing, leaving a cancelled run as that SHA's only api-gate verdict (41d1153c, 4b1966c5). For the gate to actually block a merge, main branch protection must require the "API Review Gate" status context; until then it is advisory, as the job name was. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
IvanHunters
|
a444ac4498 |
Merge branch 'main' into fix/tap-pin-digest
Reconcile the tap digest-pin change with #5071 (no-install-on-connect), now merged into main. Keep this branch's resetTapPinState, the verify-before-pull Trust text, and the index-README trust model, while adopting #5071's wording that installing (not tapping) runs charts and dropping the stale "re-registration" phrasing from the shared tap.go and rest.go comments. Conflicts resolved in cmd/cozypkg/cmd/tap.go, pkg/registry/core/tap/rest.go, and hack/packages-index/README.md. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
e1e925a092 |
docs(agents): steer agents away from Helm lookup (#5168)
## What this PR does Adds a rule to the Helm Charts conventions in `docs/agents/overview.md` about `lookup`. A lookup reads cluster state at render time and nothing re-renders the HelmRelease when that state changes, so a lookup that copies another object's content into the output (a bucket name, an endpoint, a CA, a StorageClass list) goes stale; several have failed this way ( |
||
|
Timofei Larkin
|
321baf5707 |
docs(agents): narrow lookup rule to copied state
The staleness problem is specific to a lookup that carries another object's content into the rendered output; a lookup whose repeated render is identical, such as a guard or keeping a generated Secret, cannot go stale, and banning it pushed agents toward nothing better. The testing caveat held only for runs without a kubernetesProvider mock, and the alternatives now name the mechanisms the tree already has, so an agent can reach for them instead of a new lookup. Assisted-by: LLM Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Aleksei Sviridkin
|
ba0b1ac6ef |
fix(e2e): validate the leak allowlist against every suite layout (#4972)
`lg_allowlist_validate` in `hack/e2e-chainsaw/_lib/leak-guard.sh`
accepted only `<suite>/chainsaw-test.y*ml`, while the selectors already
see step-file suites and a Test nested below the suite directory
(#4630). A suite in one of those layouts could not be put on the leak
allowlist at all: validation failed and the run stopped before the first
suite.
- `lg_suite_has_test` validates an allowlist entry with the rule
`hack/select-e2e.sh` enumerates suites with: `chainsaw-test.yaml`,
`chainsaw-test.yml`, or a numbered step file, anywhere in or below the
suite directory, with the selector's type test (a step-file name counts
unless it is a directory, a `chainsaw-test.y*ml` counts whatever its
type, so Chainsaw reports a directory by that name); a parked
`chainsaw-test.yaml.disabled` matches none. Both sides carry a comment
pointing at the other so they move together.
- `hack/e2e-chainsaw/README.md` describes the three layouts once, under
Layout, and lists `kubernetes-proxmox` as the parked suite.
`docs/agents/e2e-testing.md` points at it.
- Unit tests: every layout is accepted, including a `chainsaw-test.yaml`
that is a directory; a parked, empty, or not-a-step-file directory is
refused, including `1a-install.yaml` (passes the `find` glob, not the
step-file regex) and a directory named `01-install.yaml`.
Left as they are, with the reasons:
- `docs/agents/e2e-testing.md` calling `kubernetes-proxmox` the one
parked suite is correct on main now: `hack/e2e-chainsaw/backup/` was
removed in
|
||
|
Aleksei Sviridkin
|
013b166967 |
fix(e2e): read the KubernetesNodes pool release in the HelmRelease guard (#4970)
`cozy_guard_all_helmreleases` in `hack/e2e-chainsaw/_lib/run-kubernetes.sh` reads the parent `kubernetes-<test>` release and the addons selected by the `kubernetes-<test>-` prefix. The worker pool is `kubernetes-nodes-<test>-md0`, an Application HelmRelease the aggregated apiserver stamps with `RetryOnFailure` like the parent, and it sits on the node-join path through the talos-reconcile Job it renders. Its name does not start with the addon prefix, so a failed action on it was never read, on the passing path or in the failure-path report. This reads it on both paths, with the `required` strategy check the parent gets, since the apiserver stamps both. The pool name is derived from the parent (`cozy_pool_helmrelease_name`), the way the addon prefix is, so the two cannot drift. The failure-path report checks that the release exists before reading it, with `get hr --ignore-not-found -o name`: a run whose pool apply failed has no pool release (exit 0, empty output), and the report says so instead of reporting a failed read. For that run to reach the report at all, `run_kubernetes_test` now arms the exit handler (`_tenant_failure_on_exit`, with `CURRENT_TENANT_PARENT_HR`) right after the parent `Kubernetes` apply instead of after the `KubernetesNodes` apply: before, a failing pool apply exited under errexit with no handler installed, and the parent went unclassified. The snapshot half still no-ops until the tenant kubeconfig is set. A non-zero exit (a timeout, a kill, a Forbidden) is reported as the failed read it is, through `_cozy_guard_read_failure`, never as absence. The pool reads are gated on the report's own deadline as well as the phase clock, like the addon listing. Unit tests in `hack/run-kubernetes-remediation_test.bats` cover a failing pool release on the passing path, a clean one (the composite returns zero), an empty strategy on the pool, the report with and without the pool release, a cut-off existence read, and a pool read past the report budget; the ordering pin now also requires the handler to be armed before the pool apply. The kubectl stub gained an existence read: an `ABSENT` marker prints nothing and exits 0 under `--ignore-not-found` (NotFound without it, as kubectl does), and the failure markers apply to it as `<name>.exists`. Verified with `hack/cozytest.sh` and Bats on `hack/run-kubernetes-remediation_test.bats` (51 tests), `hack/run-kubernetes-node-join_test.bats` and `hack/remediation-guard.bats`. Closes #4737 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * Kubernetes release checks now include worker-pool releases alongside the parent release and add-ons, and flag missing or invalid install strategies and remediation failures. * Failure reports distinguish worker-pool releases that are absent from those whose status cannot be read. Existing releases are assessed against the phase gate and report time budget; when the budget is exhausted, skipped checks are identified. * Failure handling now begins immediately after the parent resource is applied, covering errors that occur while applying the worker pool. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Timofei Larkin
|
648dcd2aa3 |
docs(agents): steer agents away from Helm lookup
Nothing triggers a reconcile of a HelmRelease when an object one of its templates looks up changes, so a lookup leaves the release on a stale answer, and the project has worked to remove them. They keep spreading because agents read the charts, find over a hundred lookups, and take them as the convention. The guidance says they are not, names the one accepted kind, and what to do instead. Assisted-by: LLM Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
IvanHunters
|
8d96c46994 |
fix(monitoring-agents): isolate merged log payload under log_processed
Fluent Bit's kubernetes filter runs with Merge_Log On but without Merge_Log_Key, so a parsed JSON log line is merged into the record root. An application log line can thereby forge the service fields that later pipeline stages rely on for routing and tenant isolation (kubernetes_namespace_name, kubernetes.labels, tenant, cluster, log_source). Setting Merge_Log_Key log_processed nests the parsed payload under a dedicated key so application content can no longer override top-level service fields. Applies to both the platform (system/monitoring-agents) and guest (apps/kubernetes) Fluent Bit configurations. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Marian Koreniuk
|
b61ad29783 |
fix(e2e): point the selector's suite rule back at the allowlist rule
The first commit says the two rules are cross-referenced so they move together, but only lg_suite_has_test pointed at the selector. The selector's suite enumeration comment now names lg_suite_has_test as well, so a change to either side prompts the other. The layout case also pins a chainsaw-test.yml that is a directory, the other half of the type test: dropping that name from the find now turns the case red. Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua> Assisted-by: LLM |
||
|
Marian Koreniuk
|
75bb70e58d |
fix(e2e): arm the tenant failure handler before the pool apply
The exit handler was armed after the KubernetesNodes apply, so a failing pool apply exited under errexit with no handler installed: the parent went unclassified, and the report's "pool release does not exist" line described a run that could never reach it. Arm it right after the parent Kubernetes apply instead. The snapshot half still no-ops until the tenant kubeconfig is set, so the only change is that a failed pool apply now gets the parent classified and the absent pool reported. The ordering pin now also requires the handler to be armed before the pool apply. Assisted-by: LLM Signed-off-by: Marian Koreniuk <moriarti@cp.if.ua> |
||
|
Aleksei Sviridkin
|
048c029e29 |
fix(e2e): keep the Proxmox tenant kubeconfig per run and remove it (#4971)
`hack/e2e-chainsaw/_lib/run-proxmox.sh` wrote the tenant's super-admin kubeconfig to `$TMPDIR/proxmox-e2e-tenant.kubeconfig` with the process umask and never removed it, so it outlived the run on the self-hosted runner, world-readable under the usual umask. `dump_diagnostics` then read whatever sat at that path, so a run that failed before `fetch_tenant_kubeconfig` reported the previous run's tenant as its own. - The file now lives under `RUNNER_TEMP`, which the runner exports into every step and empties between jobs; `COZY_PROXMOX_WORKDIR` overrides it for a local run, and `TMPDIR` is the last fallback. The workflow sets no path of its own: the `runner` context is not available in job-level `env`. - `fetch_tenant_kubeconfig` writes it under `umask 077` in a subshell, since the workflow's `umask 077` covers only the staging step. - `apply-tenant`, the suite's first step, removes a leftover before doing anything else, so diagnostics in this run can only see this run's file. - The suite's `finally` removes it, before the `keep_tenant` exit, and the workflow's "Remove staged credentials" step removes it again for a run killed before `finally`. Verification: `sh -n` and shellcheck on the script; actionlint on `e2e-proxmox.yaml` (only the custom `proxmox` runner label is reported, as on main). `hack/proxmox-e2e-fixture_test.bats` has three new cases with a stub `kubectl`: the fetched file has no group or other bits under `umask 022`, `apply-tenant` removes a planted leftover, and the suite's `finally` (run out of the Chainsaw file on the `keep_tenant` path) removes the file, so `diagnostics` after it prints no tenant section. Each goes red with its line (`umask 077`, the leftover `rm`, the `finally` `rm`) removed. All `hack/proxmox-*.bats` pass (fixture 9, teardown contract 8, step1b 4, talos template 12), also under `hack/cozytest.sh`. Not run on a Proxmox stand: the only stand I have cannot dispatch the workflow at the moment, so the workflow steps themselves are unexercised beyond actionlint. Closes #4806 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Chores** * Proxmox end-to-end test runs store tenant credentials in a temporary work directory and create kubeconfig files with restricted permissions. * Stale tenant kubeconfig files are cleared before a new tenant fixture is applied. Cleanup removes the file even when tenant data is retained or a run is interrupted, preventing credentials from appearing in later diagnostics. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
3abdff5900 |
fix(kubernetes-nodes): refuse a partial resources spec on every pool (#4973)
The `resources.cpu`/`resources.memory` pair check in `nodegroup.yaml` ran only when an `instanceType` was set, while the field description (values.yaml, README, schema, the KubernetesNodes OpenAPI schema) promised a render-time error for a partial spec on any pool. Without an instancetype a cpu-only pool renders a VM with no memory, which KubeVirt refuses when the Machine is created, and a memory-only pool silently sizes the guest at KubeVirt's default of one vCPU. - The check now runs on every kubevirt pool. The proxmox branch already required both, so this is the kubevirt half of one rule, and the field descriptions are true as written (no doc change). On a pool with no `instanceType` the message ends in "or neither and set instanceType", so it does not send the user straight into the next refusal. - A kubevirt pool with neither an `instanceType` nor both resources fails at render with a message naming both, the way `vm-instance` does. No such pool could have passed admission (no memory), so this refusal breaks nothing that runs today. This is the decision the issue asked for; if the gate should stay narrower, this commit can be dropped on its own. - **Upgrade impact.** One shape that renders on main stops rendering: `instanceType: ""` with only `resources.memory`. On main it gets `domain.memory.guest` and no `domain.cpu`, so KubeVirt runs the worker with one vCPU; with this PR it fails with the partial-spec message, and the pool's HelmRelease sits in a failed upgrade until the values are edited. To keep the current size such a pool needs `resources.cpu: "1"`. Adding that line renames the machine template (for the test values, `kubernetes-myk8s-md0-28d812` becomes `kubernetes-myk8s-md0-1ce103`, the same name on main and on this branch), so the pool's workers are replaced once. cpu-only with `instanceType: ""` is not in this class: it renders a VM with no memory, which KubeVirt refuses, so it never ran. - `cores:` printed an unquoted (float64) core count of one million or more in exponent form (`1e+06`). It is printed as an integer now, with the same bytes for every smaller whole number. Tests: `tests/resources_partial_test.yaml` covers both halves with and without an instancetype, the no-sizing case, both fields set, and the float64 formatting. The existing unquoted-zero case in `resources_cpu_test.yaml` now reads the field as unset on an instancetype-sized pool (with a `kubernetesProvider` stub for the lookup), and zero beside memory is pinned as a partial spec. Byte stability, checked by snapshotting the `KubevirtMachineTemplate` on main for nine spellings of `resources.cpu` and replaying the snapshots on this branch: `"2"`, `2`, `2.0`, `"2.0"`, `"+2"`, `"1e1"`, `"010"`, `999999` render identical bytes and template names. Only unquoted `4294967295` changes, from `cores: 4.294967295e+09` to `cores: 4294967295`. Verified with `helm unittest .` in `packages/apps/kubernetes-nodes` (38 suites, 233 tests, 12 snapshots), and on a trial merge with current main (39 suites, 237 tests). Closes #4546. The leading-zero inconsistency from the issue's comment is addressed separately, together with #4554. ### Release note ```release-note fix(kubernetes-nodes): a partial `resources` spec (only `cpu` or only `memory`) is now refused at render time on every kubevirt worker pool, not only on a pool with an `instanceType`, and a pool with neither an `instanceType` nor both resources is refused as well. Action required: a pool with `instanceType: ""` and only `resources.memory` rendered before (KubeVirt ran it with one vCPU) and now fails to upgrade; add `resources.cpu: "1"` to keep its current size. Adding that line renames the pool's machine template, so its workers are replaced once. Whole-number core counts of one million or more are now printed as integers instead of in exponent form. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * KubeVirt worker pools now require either an instance type or both CPU and memory values. Configurations with only one resource or no sizing information are rejected. * Whole-number CPU values render without exponent notation, including values written with a decimal fraction. * An unquoted zero CPU value is treated as unset when an instance type is provided; without an instance type, pairing zero CPU with memory fails validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
60d1f53e82 |
fix(kubernetes-nodes): plan autoscaler capacity on the cores the VM actually gets (#4974)
On the kubevirt substrate `resources.cpu` is written into `domain.cpu.cores` as spelled and decoded as YAML, so `"010"` gives the VM eight cores, while the cluster-autoscaler capacity annotation parsed the same string as decimal ten (`capacity.cluster-autoscaler.kubernetes.io/cpu: "9000m"` on a node that offers 7000m). This is the inconsistency reported in #4554 and in the comment on #4546. - The YAML-decoded value the range check already computes is kept, and on kubevirt the capacity annotation and the reserved-CPU check start from it. - The kubelet reservations are not touched. The worker machineconfig (`_talosconfigtemplate.tpl`) sizes them from the raw string read as a decimal, and the TalosConfigTemplate name is a hash of that config, so reading the decoded value there would roll every leading-zero pool once. Instead the annotation subtracts the reservations the machineconfig really carries from the cores the VM really has: for `"010"` that is 8000m − 2 × 500m = `"7000m"`, the node's real allocatable, while `"8"` keeps `"7200m"` (2 × 400m). Tests pin both sides: the annotation and the bootstrap `configRef` name in `resources_cpu_test.yaml`, and `systemReserved.cpu` / `kubeReserved.cpu` in the machineconfig in `talos_worker_config_test.yaml`. - No pool rolls. Checked by rendering `nodegroup.yaml` on main and on this branch for nine spellings (`"2"`, `2`, `2.0`, `"2.0"`, `"+2"`, `"1e1"`, `"010"`, `999999`, `4294967295`): the only difference is the capacity annotation of the `"010"` case, so the `KubevirtMachineTemplate` name and the TalosConfigTemplate `configRef` stay as they are. The machineconfig template itself is not changed. - Proxmox parses the quantity as decimal for the machine template and for the reservations alike, so it never had the inconsistency and is unchanged (pinned by a test). - The `resources` description says the value is decoded as YAML, what that means for a leading zero, and that the reservations still read it as a decimal. README, schema, the RD and the Go type are regenerated from it. This closes #4554 without the migration and the render-time refusal it proposes. A refusal at render breaks a stored pool on its next upgrade, a pattern in the OpenAPI schema breaks every update of an existing CR, and a platform-wide migration over every `KubernetesNodes` and parent `nodeGroups` entry is a lot of blast radius for a spelling nobody has reported in a live cluster. With this change the autoscaler plans for the node the VM really is. What is left of a leading zero is the octal reading itself and reservations sized from the decimal value, and both are documented. The unquoted zero of the issue is already harmless: it is falsy, counts as unset, and renders the same bytes as an absent field. If a migration is still wanted, please reopen. Verified with `helm unittest .` in `packages/apps/kubernetes-nodes` (38 suites, 233 tests, 12 snapshots; pointing the annotation back at the decimal string turns the `"010"` case red with `9000m`), `make generate` (no diff), and a trial merge with #4973 (clean, 243 tests green). Closes #4554 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * KubeVirt VM sizing and autoscaler capacity now reflect YAML-decoded CPU values. For example, `"010"` represents eight VM cores, while kubelet reservations interpret it as decimal ten. `"8"` and `"10"` yield `7200m` and `9000m` of autoscaler capacity, respectively. Proxmox continues to interpret `"010"` as ten cores, yielding `9000m` of capacity. * **Documentation** * Clarified explicit worker CPU and memory sizing behavior, including how leading-zero CPU values affect VM sizing, reservations, and capacity calculations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |