Label-based objectSelector (cozystack.io/deletion-protected=true) ensures
zero overhead for non-protected resources. Reuses the existing
lineage-controller-webhook Service and TLS certificates.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
## What this PR does
Adds configuration for the two AI reviewers active on this repository —
Gemini Code Assist and CodeRabbit.
Previously neither had a repo-level config, so both ran with defaults:
Gemini had no knowledge of the project's vendoring model and would flag
issues in vendored upstream charts and generated code; CodeRabbit only
ran on initial PR open, with no review on subsequent pushes.
Changes:
- `.gemini/config.yaml` — ignore patterns for vendored charts,
`vendor/`, generated code, build artifacts, dashboard JSONs, and
upstream patch files; severity threshold set to `LOW` for strict review
of hand-written code; comment cap to prevent floods on chart-update PRs.
- `.gemini/styleguide.md` — natural-language guide covering project
architecture, vendoring rules (where to send bug fixes instead of
editing `charts/`), commit and PR format requirements, sensitive
components, Go code standards, and explicit anti-patterns the reviewer
should not flag.
- `.coderabbit.yaml` — enables incremental reviews so CodeRabbit
re-reviews each push, and skips drafts.
### Release note
```release-note
[ci] Add Gemini Code Assist and CodeRabbit configuration
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Chores**
* Added code review automation configuration settings
* Added repository style guide and development conventions documentation
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## What this PR does
Makes `cozystack-scheduler` LINSTOR storage-aware by configuring it to
call the existing `linstor-scheduler-extender` as a scheduler extender.
This fixes#2328 (phase 1): pods with both a `SchedulingClass` and
LINSTOR PVCs now get storage-locality-aware placement instead of
bypassing LINSTOR's filter/prioritize logic.
Changes:
- **linstor-scheduler package**: expose the extender sidecar (port 8099)
via a new ClusterIP Service; patch the vendored deployment to add a
`app.kubernetes.io/component: scheduler` label for clean Service
targeting
- **cozystack-scheduler package**: bump to v0.3.0 which adds
configurable `extenders` support; set the LINSTOR extender URL in
wrapper values
- **tests**: helm-unittest for extender rendering in the ConfigMap
### Release note
```release-note
[scheduler] cozystack-scheduler now calls the LINSTOR scheduler extender for storage-aware pod placement. Pods assigned to a SchedulingClass that also use LINSTOR-backed PVCs will prefer nodes with local volume replicas.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added scheduler extender support: exposes an extender Service and adds
pod metadata to enable extender routing.
* **Chores**
* Bumped scheduler chart and image versions to 0.3.0.
* Added default extender configuration and a sample remote extender
entry.
* Updated build scripts to apply an additional patch and added a test
target.
* **Tests**
* Added Helm tests validating rendered ConfigMap with and without
extender configuration.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary
- Replace `3297617+cozystack-ci[bot]@users.noreply.github.com` with
`cozystack@users.noreply.github.com` across all CI workflows
- DCO probot rejects the `[bot]` brackets in email as invalid, causing
release PRs to fail DCO checks
## Test plan
- [ ] Verify DCO passes on release PRs after backport to release
branches
The cozystack-ci[bot] noreply email with brackets is rejected by DCO
probot as an invalid email address, causing release PRs to fail DCO
checks.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
## What this PR does
The aggregated API previously accepted tenant names containing hyphens
(e.g. `foo-bar`) because `ValidateApplicationName` delegated entirely to
`IsDNS1035Label`, which permits them. The tenant Helm chart's
`tenant.name` helper then rejected the release at template time — so
users saw a successful `kubectl apply` followed by a confusing
Flux/HelmRelease reconciliation error.
This PR extends `ValidateApplicationName` with a `kindName` parameter
and enforces an alphanumeric-only rule (`^[a-z][a-z0-9]*$`) for the
Tenant kind. A `TenantKind` constant centralizes the kind string, and
`REST.validateNameFormat` wraps the check symmetrically with the
existing `validateNameLength`. The tenant chart README is updated to
match the already-correct website documentation.
Test coverage includes:
- Unit tests for the validation package (format + error message
contract)
- REST wrapper and Update→Create fall-through path tests with a fake
client
- E2E BATS regression test with explicit exit-code checks and cleanup
Fixes#2375
### Release note
```release-note
[api] Tenant names containing dashes are now rejected at API level during creation, with a specific error message, instead of being silently accepted and failing later during Helm reconciliation.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Tenant names now require lowercase letters and digits only, must start
with a letter; dashes are rejected with a tenant-specific error.
* **Documentation**
* Tenant naming guidelines updated to reflect the stricter
alphanumeric-only requirement.
* **Tests**
* New unit and end-to-end tests added to verify tenant naming
enforcement and tenant-specific error messaging.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Three corrections from PR review:
- Package group list was incomplete. Added library/ (reusable helper
charts) and tests/ (tests for library charts, which are not directly
testable otherwise).
- CRD generation was described incorrectly. Static CRDs are generated
by hack/update-codegen.sh from types under api/v1alpha1/, api/backups/,
and api/dashboard/. Types under api/apps/v1alpha1/ are not static
CRDs — they are registered at runtime from ApplicationDefinition.
- Removed the tgz anti-pattern. No *.tgz files exist outside _out/,
which is gitignored. The guidance was fabricated.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Expose the existing linstor-scheduler-extender sidecar as a ClusterIP
Service and configure cozystack-scheduler to call it during the
scheduling cycle. Pods with both a SchedulingClass and LINSTOR PVCs
now get storage-locality-aware placement.
- Add extender Service (port 8099) for linstor-scheduler
- Patch vendored deployment to label pods for Service selector
- Bump cozystack-scheduler to v0.3.0 (configurable extenders)
- Add "linstor" PackageSource variant with extender values
- Default variant ships without extender for non-LINSTOR clusters
- Select linstor variant in system bundle
- Add helm-unittest tests for both packages
Ref: #2328
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
## Summary
- Add `--match 'v*'` to all `git describe` calls in
`hack/common-envs.mk`
- The `api/apps/v1alpha1/*` subtags share the same commit as release
tags, causing `git describe --exact-match` to pick
`api/apps/v1alpha1/vX.Y.Z` instead of `vX.Y.Z`, producing invalid Docker
image tags like `ghcr.io/.../image:v1.35-api/apps/v1alpha1/v1.1.6`
## Test plan
- [ ] Rerun Versioned Tag pipelines after merge and backport to release
branches
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Chores**
* Refined version and tag derivation in the build system to selectively
recognize Git tags, improving consistency in how release versions are
identified.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
The api/apps/v1alpha1/* subtags share the same commit as the release
tags. git describe --exact-match picks the first match alphabetically,
returning api/apps/v1alpha1/vX.Y.Z instead of vX.Y.Z, which produces
invalid Docker image tags.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Add a minimal .coderabbit.yaml that ensures CodeRabbit re-reviews each
push to a PR (incremental review on new commits) and skips drafts.
Without this file, the org-level configuration left incremental reviews
disabled, so only the initial PR open triggered a review.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Fix two issues caught in PR review:
- Use quadruple-backtick fence around the release-note example so the
inner triple-backtick block renders correctly and satisfies markdownlint.
- Correct the platform migration path. The previous text pointed at
scripts/migrations/, which does not exist. The actual migration flow
lives in packages/core/platform/templates/migration-hook.yaml and
packages/core/platform/images/migrations/.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
## Summary
- Replace `GH_PAT` (cozystack-bot PAT) with `cozystack-ci` GitHub App
token in `pull-requests.yaml`
- This was missed in #2351 and broke the `resolve_assets` / `e2e` jobs
for release PRs (draft releases not visible with invalid token)
## Test plan
- [ ] Re-run the release PR pipeline for `release-1.1.6` and verify
`resolve_assets` job passes
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Chores**
* Updated continuous integration authentication mechanism to use GitHub
App tokens instead of personal access tokens.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
The GH_PAT secret tied to cozystack-bot is no longer valid after
migrating release workflows to the cozystack-ci GitHub App in #2351.
This broke the resolve_assets and e2e jobs for release PRs.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Configure the Gemini Code Assist GitHub reviewer with project-specific
ignore patterns and a natural-language style guide.
The config silences noise from vendored upstream Helm charts, generated
Go code, build artifacts, and Grafana dashboard JSONs, while keeping the
severity threshold at LOW for strict review of hand-written code.
The style guide teaches the reviewer about the vendoring model, commit
format requirements, sensitive components like packages/core/platform/,
and lists anti-patterns not to flag (patch files, go.sum, upstream forks).
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Replace bare "Tenant" string literals with the validation.TenantKind constant in all test table entries, struct fields, and TypeMeta. Prevents silent drift if the canonical kind string is ever renamed.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The convertHelmReleaseToApplication fall-through that populates Status.Namespace for tenant resources still compared r.kindName against the bare string "Tenant". The validation package already exposes TenantKind for exactly this reason: if the kind string ever diverges from the ApplicationDefinition source of truth, the constant and the call site drift together rather than silently desynchronizing.
Add TestConvertHelmReleaseToApplication_TenantNamespaceKindGate to pin both sides of the gate: tenant kind populates Status.Namespace, non-tenant kind leaves it empty. Also rewrite the TestUpdate comment to describe the upsert invariant directly instead of referencing an issue number.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Four follow-ups from review round 4:
1. BLOCKER: the Update(forceAllowCreate=true) path delegates to Create() when the object does not yet exist (rest.go:452) — the typical kubectl apply upsert flow. Add TestUpdate_ForceAllowCreate_RejectsTenantDashName using a fake client so a future refactor of that delegation cannot silently bypass the tenant name check that r.validateNameFormat alone cannot catch.
2. BLOCKER: the e2e BATS test used || true inside the command substitution, which swallowed the kubectl exit code. Rework the test to capture exit code and stdout+stderr explicitly, then assert the exit code is non-zero before asserting on the error message. This distinguishes validation-success (kubectl exit 0 — regression) from environmental failures (exit non-zero but wrong message) from the happy path.
3. Extract TenantKind = "Tenant" as a named constant in the validation package with a comment pointing at the upstream ApplicationDefinition source of truth, and switch the kindName check to use it.
4. Add a clarifying comment on TestValidateApplicationName_TenantLengthFallthrough that it pins an architectural layering decision and is not a user-facing requirement, so a future promotion of tenant length into tenant-specific wording is a legitimate change rather than a test regression.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Three follow-ups from review:
1. Add defensive cleanup of the foo-bar tenant before and after the e2e regression test. If a prior run left the object in the cluster (e.g. a transient validation regression), the test no longer inherits that state; and if the test itself trips a regression in the future, the cluster is left clean for subsequent tests.
2. Switch kubectl --validate=false to --validate=ignore. The bool form was deprecated in kubectl 1.25 and only kept as an alias; --validate=ignore is the modern, stable spelling.
3. Drop the brittle Contains("63") assertion in TestValidateApplicationName_TenantLengthFallthrough. It pinned the test to the exact DNS-1035 error text from k8s.io/apimachinery, which could break on unrelated upstream wording changes. The surviving assertions still cover the intent: an error exists and it is not the tenant-specific message.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Follow-up from review round 2:
The BATS regression test now pins a namespace precondition, disables kubectl client-side validation with --validate=false so the request is guaranteed to reach the server-side name check, and documents the intent of each assertion. Previously a client-side schema rejection could have produced a false negative on the tenant-specific error grep, and a missing tenant-root namespace could have produced an unrelated failure that was hard to diagnose.
Also pin the edge case where a tenant name consists of valid characters but exceeds the DNS-1035 63-char label limit. The resulting error is intentionally the generic DNS-1035 message because length is not a tenant-specific constraint — the package-level function is not responsible for the stricter Helm-release-prefix length budget that REST.validateNameLength enforces.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Two follow-ups from review:
1. The original tenantNameRegex ^[a-z0-9]+$ let leading-digit names like 123foo slip past the tenant check and receive a generic DNS-1035 error, which defeated the goal of surfacing a tenant-specific message. Require a leading lowercase letter so every tenant-invalid name returns the tenant-contract error.
2. Add a BATS regression test in hack/e2e-install-cozystack.bats that exercises the aggregated API end-to-end. Unit tests construct REST{kindName: "Tenant"} by hand, so a future change to ApplicationDefinition kind registration could break real behavior without breaking the unit tests.
Also pin the error-message contract with a new TestValidateApplicationName_TenantErrorMessage that asserts every tenant-invalid input returns a message containing 'tenant names must' (not the generic DNS-1035 text).
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The README stated that dashes are 'allowed but discouraged' in tenant names, but the tenant.name Helm helper explicitly fails on release names that contain more than one dash, and the platform guide on the website already says tenant names must be alphanumeric. Bring the chart README in line with both the website documentation and the enforcement now present on the aggregated API.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
ValidateApplicationName previously delegated to IsDNS1035Label, which permits hyphens. The tenant Helm chart's tenant.name helper rejects them at template time, so users saw a successful kubectl apply followed by a Flux reconciliation error.
Extend ValidateApplicationName with a kindName parameter and enforce alphanumeric-only names for the Tenant kind, matching the documented naming rules. Wire the check into REST.Create via a small validateNameFormat wrapper that mirrors validateNameLength.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
## What this PR does
Adds a host preflight diagnostic script (`hack/check-host-runtime.sh`)
that warns operators when a standalone `containerd.service` or
`docker.service` is running on the host alongside the embedded k3s
runtime used by the cozystack `generic` variant (k3s / kubeadm on
Ubuntu).
**Why it matters.** K3s ships its own containerd at
`/run/k3s/containerd/containerd.sock` and
`/var/lib/rancher/k3s/agent/containerd`, while a system-package
containerd or docker uses `/run/containerd/containerd.sock` and
`/var/lib/containerd`. The two runtimes do not fight over sockets, so
both keep running silently. Over time the standalone one accumulates
unpruned images and build cache in `/var/lib/containerd` — enough to
fill the root disk, trigger `DiskPressure`, and put `cozystack-api` into
an eviction loop. This is silent on day zero and surfaces as a
mysterious production incident weeks later. The script exists to warn
the next operator before the failure mode surfaces.
The script is warning-only — it always exits 0 and never blocks the
install. It detects:
- `containerd.service` or `docker.service` active via `systemctl
is-active`
- Standalone runtime sockets at well-known paths, with a fallback that
works on hosts without systemd
- Standalone data directory sizes via `du -sh`
When a warning fires, the HINT names only the detected services and
instructs the operator to disable them with `sudo systemctl disable
--now <service>`. Reclaiming the data directory is called out separately
with an explicit note not to delete it blindly — the data may still be
in use.
**Entry points.**
- `make preflight` runs the script directly (for operators preparing a
generic-variant host)
- `make unit-tests` now runs `bats-unit-tests` alongside
`helm-unit-tests`, auto-discovering every `hack/*.bats` file that is not
an e2e test
**Test coverage.** `hack/check-host-runtime.bats` (11 cases, run via
`hack/cozytest.sh`) covers clean hosts with and without systemd,
single-service detection, both services simultaneously, socket-only
fallback for both runtimes, glob-expansion regression guard, explicit
exit-code-0 assertion, `sudo` prefix assertion, `du` failure robustness,
and the "service + socket = exactly one warning" de-duplication
invariant. Every test is self-contained with `trap 'rm -rf $STUB_DIR'
EXIT` for clean recovery on assertion failure, and has no runtime
dependencies beyond bash and core utilities — no python3, no real
systemd.
Irrelevant on Talos where the container runtime lifecycle is fully
managed by the distribution.
### Release note
```release-note
[hack] Add `check-host-runtime.sh` and `make preflight` target that warn when a standalone containerd or docker runtime is running alongside the embedded k3s runtime on the cozystack generic variant.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added a non-blocking preflight check that detects standalone container
runtimes, reports disk-usage estimates, and displays an actionable hint
to disable detected services.
* **Tests**
* Added BATS-based unit tests with comprehensive coverage for the
preflight validations and various host scenarios.
* **Chores**
* Build updated to run the BATS unit tests alongside the existing test
suite.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## What this PR does
The aggregated `cozystack-api` validates each individual Tenant name
length against the Helm release limit (46 chars for the `tenant-`
prefix), but it never validates the **computed workload namespace** that
is built by dash-joining the parent namespace with the tenant name. A
user can therefore `kubectl apply` a nested `Tenant` whose own name
passes the per-name check, only to have Kubernetes later reject the
generated `Namespace` for exceeding the 63-character DNS-1123 label
limit. The failure surfaces as an opaque HelmRelease reconcile error,
and the Tenant CR is left stranded in the cluster with a
`status.namespace` that can never exist.
This PR closes that gap at the aggregation layer by rejecting such
requests synchronously in `Create()` with a clear `field.Invalid` error
that names the computed namespace and its length, so the failure is
reported at `kubectl apply` time before any HelmRelease is created.
The change is scoped to the `Tenant` kind only. `Update()` is not
touched because Kubernetes names and namespaces are immutable and the
existing code already intentionally skips name validation there. No Helm
chart, CRD, or codegen changes are required.
The PR is split into two commits that document the bug and its fix via
TDD:
1. **Pin the gap** — adds a test that demonstrates the pre-fix behavior:
the pre-fix Create() path has no function that rejects a realistic
nested-tenant combination whose computed namespace exceeds 63 chars.
Reviewers can check out this commit and observe the gap.
2. **The fix** — adds `validateTenantNamespaceLength`, the `kindName ==
"Tenant"` gate in `Create()`, and `TestValidateTenantNamespaceLength`
covering boundary cases (pass at 63, fail at 64, fail at 68). The
pinning test from commit 1 is removed in the same commit now that the
gap is closed.
### Release note
```release-note
[platform] cozystack-api now rejects Tenant creation at admission time when the ancestor-chain namespace would exceed the 63-character Kubernetes namespace limit, instead of allowing the resource through and failing later at HelmRelease reconcile.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Tenant application creation now validates computed tenant namespace
length against the Kubernetes DNS-1123 63-character limit; creations
that would produce too-long namespace names now fail with a clear
validation error.
* **Tests**
* Added tests covering tenant namespace length validation, including
multiple parent/tenant-name scenarios, boundary conditions, and
verification that error messages include the offending namespace and its
length.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- Thank you for making a contribution! -->
## What this PR does
Cilium 1.19 init containers `mount-cgroup`, `apply-sysctl-overwrites`
and `clean-cilium-state` call `nsenter` to join the host cgroup/mount
namespace. On any distribution that loads the
`cri-containerd.apparmor.d` AppArmor profile by default for containerd
workloads (Ubuntu 22.04+, Debian with AppArmor), the kernel denies the
operation and the audit log reports it as a blocked `ptrace` operation:
```text
apparmor="DENIED" operation="ptrace" class="ptrace"
profile="cri-containerd.apparmor.d" comm="nsenter"
requested_mask="read" denied_mask="read" peer="unconfined"
```
Symptom on non-Talos clusters: after a cilium rollout the new pod stays
in `Init:CrashLoopBackOff`, and the rest of the platform cascades into
`dependency not ready` until cilium is healthy on every node.
This PR opts the affected containers out of the
`cri-containerd.apparmor.d` profile via deprecated per-container
`container.apparmor.security.beta.kubernetes.io/<name>: unconfined`
annotations. Four containers are annotated: `cilium-agent`,
`clean-cilium-state`, `mount-cgroup`, `apply-sysctl-overwrites`.
`mount-bpf-fs` is intentionally excluded because it renders with its own
`securityContext.privileged: true`, and the kernel does not apply
AppArmor profiles to privileged containers.
### Why pod annotations instead of `podSecurityContext.appArmorProfile`
The upstream Cilium chart already sets
`podSecurityContext.appArmorProfile.type: Unconfined` (the modern k8s >=
1.30 API), but containerd's CRI implementation silently ignores this
field today and keeps applying `cri-containerd.apparmor.d` — see
kubernetes/kubernetes#125069 (still open). Only the deprecated
per-container annotations take effect end-to-end on current k3s /
kubeadm + containerd setups.
### Why a dedicated values file limited to non-Talos variants
On Talos (`isp-full` bundle) `values-talos.yaml` sets
`cgroup.autoMount.enabled: false`, so the upstream chart does not render
the `mount-cgroup` init container. The kube-apiserver rejects a
DaemonSet that carries a per-container AppArmor annotation for a
container that is not in the pod spec:
```
DaemonSet.apps "cilium" is invalid: spec.template.annotations[
container.apparmor.security.beta.kubernetes.io/mount-cgroup]:
Invalid value: "mount-cgroup": container not found
```
To keep the Talos path untouched and only add the annotations where they
are both safe and useful, this PR puts the `cilium.podAnnotations` block
in a new `packages/system/cilium/values-apparmor.yaml` and wires it into
only the non-Talos PackageSource variants (`cilium-generic`,
`kubeovn-cilium-generic`). Those variants always run with
`cgroup.autoMount.enabled: true` (explicit bundle override in
`packages/core/platform/templates/bundles/system.yaml:83-86`), so every
annotated container really exists in the rendered DaemonSet.
On Talos the annotations are omitted entirely — that is safe because the
Talos kernel does not load the AppArmor LSM anyway.
### Vendored chart patch to avoid duplicate keys on unsupported k8s <
1.30
On k8s < 1.30 the upstream cilium chart template emits the same
annotation keys from its own `semverCompare "<1.30.0"` branch in
`cilium-agent/daemonset.yaml`. Letting both sources write to the
annotations mapping would produce a YAML map with duplicate keys.
Cozystack's supported k8s matrix starts at 1.30
(`packages/apps/kubernetes/files/versions.yaml`), so the duplicate
rendering is never observed in practice, but to keep the rendered
manifest clean on any k8s version this PR also adds a `perl -i -0pe`
multi-line delete to `packages/system/cilium/Makefile` that strips the
entire upstream `{{- if not .Values.securityContext.privileged }} / {{-
if semverCompare "<1.30.0" }}` block from the vendored template. A
fail-fast `grep` guard after the `perl` invocation turns a
silently-failed patch (e.g. if upstream reformats the block) into a loud
error. The same patch is applied to the currently vendored copy so the
repository state matches what `make update` produces. This follows the
existing sed-patch pattern already used for `values.yaml` (`Used in
iptables`, `SYS_MODULE`).
### Scope
- `packages/system/cilium/values-apparmor.yaml` — new file containing
the four `cilium.podAnnotations`
- `packages/system/cilium/values.yaml` — unchanged apart from reverting
the previously-added annotations block (the annotations now live in
`values-apparmor.yaml`)
- `packages/system/cilium/Makefile` — extends the `update:` target with
a `perl` multi-line delete (plus a `grep` fail-fast guard) that drops
the entire upstream AppArmor annotation block on every vendoring refresh
-
`packages/system/cilium/charts/cilium/templates/cilium-agent/daemonset.yaml`
— the vendored file with that block already removed to match the
Makefile patch
- `packages/core/platform/sources/networking.yaml` — adds
`values-apparmor.yaml` to the `cilium-generic` and
`kubeovn-cilium-generic` variants only
### Compatibility across supported platforms
- **Talos Linux** (`isp-full` bundle, `kubeovn-cilium` variant) —
AppArmor LSM is not loaded, `cgroup.autoMount.enabled: false`,
`values-apparmor.yaml` is not included. Zero annotations in the rendered
DaemonSet. No behavioural change. Verified with `helm template` +
`kubectl apply --dry-run=server` against a live k3s v1.35.2 cluster: no
errors, no AppArmor lines.
- **Ubuntu 22.04 / 24.04, Debian with AppArmor** (`isp-full-generic`
bundle, `kubeovn-cilium-generic` / `cilium-generic` variants) — the four
annotations are present in the rendered DaemonSet, kubelet starts the
listed containers with the unconfined profile, `nsenter` succeeds.
Verified with `helm template` + `kubectl apply --dry-run=server` against
a live k3s v1.35.2 cluster: no errors, exactly four
`container.apparmor.security.beta.kubernetes.io/*: unconfined` lines.
Verified on a fresh k3s + Ubuntu 24.04 deployment end-to-end.
- **RHEL / Rocky / AlmaLinux / Fedora** — use SELinux, not AppArmor.
When deployed via `*-generic` variants these platforms pick up the
annotations; kubelet silently ignores AppArmor annotations when the LSM
is not loaded, SELinux policy is unaffected.
- **k8s < 1.30 and k8s >= 1.30** — both now render the same four
annotations (on non-Talos), sourced exclusively from
`cilium.podAnnotations`. Verified via `helm template --kube-version
1.29.0` and `--kube-version 1.35.0` → exactly 4 annotations with no
duplicates.
### Release note
```release-note
[cilium] Opt four cilium-agent containers (cilium-agent, clean-cilium-state, mount-cgroup, apply-sysctl-overwrites) out of the cri-containerd.apparmor.d AppArmor profile via per-container annotations, applied only on non-Talos cilium variants (cilium-generic, kubeovn-cilium-generic). This unblocks cilium rollouts on Ubuntu 22.04+, Debian and other distributions that load this profile for containerd by default, where the kernel previously denied nsenter's namespace entry (reported as a blocked ptrace operation) and the cilium agent landed in Init:CrashLoopBackOff. Talos variants are untouched.
```
## Summary
Replaces the `cozystack-bot` user account + personal access token
(`GH_PAT`) with the `cozystack-ci` GitHub App across all CI/CD release
workflows.
### Why
- **Security**: GitHub App tokens are short-lived (1h) and scoped, vs. a
long-lived PAT tied to a user account
- **Auditability**: App actions are clearly attributed to
`cozystack-ci[bot]`
- **No 2FA issue**: The bot account lacked 2FA; GitHub Apps don't
require it
- **Best practice**: GitHub recommends Apps over PATs for automation
### Changes
| Workflow | What changed |
|---|---|
| `tags.yaml` | App token in all 3 jobs: prepare-release,
generate-changelog, update-website-docs |
| `auto-release.yaml` | App token for daily auto-patch tagging |
| `pull-requests-release.yaml` | App token for release PR finalization |
All `cozystack-bot` git identity replaced with `cozystack-ci[bot]`.
### New secrets (already configured at org level)
- `COZYSTACK_CI_APP_ID` — GitHub App ID
- `COZYSTACK_CI_PRIVATE_KEY` — GitHub App private key
### After merge
- [ ] Delete `GH_PAT` repo-level secret
- [ ] Remove `cozystack-bot` from the cozystack org
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Chores**
* Updated CI/CD automation authentication mechanisms across release and
tagging workflows for improved security practices.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Reject Tenant creation when the computed workload namespace (parent
namespace + "-" + tenant name) would exceed the 63-character DNS-1123
label limit. Previously only the per-name length was checked against
the Helm release limit, so a deeply-nested tenant could pass Create()
and then fail later as an opaque HelmRelease reconcile error.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The disk_usage helper shells out to 'du -sh $path' for reporting in
the warning message. On the exact directories this script is meant
to warn about (a /var/lib/containerd accumulating millions of files
from months of unpruned builds), traversing the tree with du can
take minutes and stall the preflight indefinitely.
Wrap the call in 'timeout 5s' so the helper returns quickly even on
a pathologically slow filesystem. If the timeout binary is absent
(e.g. minimal busybox userland), the pipeline still exits 0 via the
existing '|| true' guard and usage stays empty — the warning still
prints, just without the size detail — so the change is backward
compatible.
Addresses an inline review comment from gemini-code-assist on the
open PR.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
E2E revealed that the kube-apiserver rejects a DaemonSet carrying a
per-container AppArmor annotation for a container that is not present
in the pod spec:
DaemonSet.apps 'cilium' is invalid:
spec.template.annotations[container.apparmor.security.beta.kubernetes.io/mount-cgroup]:
Invalid value: 'mount-cgroup': container not found
On Talos (isp-full bundle) values-talos.yaml sets
cgroup.autoMount.enabled=false, so the upstream chart does not render
the mount-cgroup init container, and our unconditional podAnnotations
entry for it becomes an invalid reference.
Move the podAnnotations block from the shared values.yaml into a new
values-apparmor.yaml and include it only in the cilium-generic and
kubeovn-cilium-generic PackageSource variants, where
cgroup.autoMount.enabled is explicitly true. Talos variants
(cilium, cilium-kilo, kubeovn-cilium) get no AppArmor annotations,
which is safe because the Talos kernel does not load the AppArmor
LSM anyway.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
- Broaden the grep guard in the Makefile update: target to match any
container.apparmor.security.beta.kubernetes.io key, not only the
quoted cilium-agent line. The previous pattern would silently pass
if upstream switched to an unquoted annotation value, masking a
failed perl patch; grepping for the prefix catches any residual
hardcoded AppArmor annotation regardless of format.
- Rewrite the values.yaml comment to distinguish the two Talos-safe
reasons: mount-cgroup is simply not rendered when
cgroup.autoMount.enabled is false (Talos default), while the other
annotations are harmless because kubelet ignores AppArmor metadata
on nodes without the LSM loaded.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Replace the misleading 'nsenter --ptrace' shorthand with an accurate
description: nsenter joins the host cgroup/mount namespace, and the
cri-containerd.apparmor.d profile reports the denial as a blocked
'ptrace' operation (operation name, not an nsenter CLI flag).
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The perl multi-line delete in the update: target silently becomes a
no-op if the upstream template is ever reformatted. Verify the patch
applied by grepping for one of the stripped annotation keys after the
perl runs; fail the update: target loudly if it still exists, so a
future upstream reformat is caught instead of silently reintroducing
the duplicate-key rendering on k8s < 1.30.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The previous sed patch removed only the four annotation lines and
left an empty nested '{{- if ... }}{{- end }}{{- end }}{{- end }}'
skeleton behind. That skeleton was dead code, but it also meant that
the next make update would re-fetch the upstream chart with the
annotations back inside the same block, and the sed would strip them
again — leaving the skeleton behind forever.
Replace the per-line sed patch with a single perl -i -0pe multi-line
delete that removes the entire <1.30 AppArmor block in one step, and
apply the same patch to the currently vendored copy for consistency.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The upstream chart emits per-container AppArmor unconfined annotations
inside a k8s<1.30 branch of cilium-agent/daemonset.yaml. Cozystack now
owns these annotations via cilium.podAnnotations in values.yaml so they
also take effect on k8s>=1.30 (where upstream drops them in favour of
the containerd-broken podSecurityContext.appArmorProfile). Having both
sources emit the same keys produced a duplicate mapping on k8s<1.30.
Add a new SED_INPLACE invocation to the package Makefile that removes
the four hardcoded annotation lines from the vendored template, and
apply the patch to the currently vendored copy so make update will
reproduce the same state on future refreshes.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
$(wildcard ...) returns a space-separated list, so a bats file with
a literal space in its name would silently split into multiple
tokens in the subsequent 'for' loop. All current bats files use
hyphen-separated names, so this is not an immediate risk, but the
caveat is worth calling out so a future contributor who adopts a
different naming convention rewrites the recipe instead of tripping
over it.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Address three review findings on the bats suite:
- The 'standalone containerd service active' and 'standalone docker
service active' tests previously asserted 'systemctl disable --now
<service>' but did not require the 'sudo ' prefix, so the two-
service test was the only guard against a silent drop of the
prefix. Extend both single-service tests to require the prefix
exactly as the 'both services active' test does.
- The 'both services active' test now captures the exit code
explicitly with 'bash ... || status=$?' and asserts '$status -eq
0'. The script contract ('always exits 0') was previously enforced
implicitly via 'set -e', which produces a generic test failure on
a regression instead of a contract-specific error. The explicit
check locks in the contract and makes regressions readable.
- New test: 'docker socket paths with glob chars do not expand' —
sets COZYSTACK_DOCKER_SOCKET_PATHS to a literal glob against real
directories and asserts no docker warning fires. Locks in the
array-based parsing of the path list.
- Extend the file header with a 'title syntax constraints' section
documenting how cozytest.sh's awk parser treats double quotes and
non-alphanumeric characters in @test titles, so future
contributors do not stumble on the parser's quirks.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
'set -euo pipefail' does not include 'set -f', so the previous
'for sock in $DOCKER_SOCKET_PATHS' loop relied on both word splitting
AND unintended glob expansion. If the variable ever contained a
literal '*' or '?' — for example a test setting
COZYSTACK_DOCKER_SOCKET_PATHS='/run/docker-*.sock' against a tree
that happens to have real files matching the pattern — the loop
would iterate over directory entries instead of the intended socket
paths, producing false-positive docker warnings.
Replace the implicit split+glob loop with 'read -ra' into a bash
array and iterate 'for sock in "${_socks[@]}"'. This keeps the
space-separated input format, disables glob expansion, and removes
the fragile word splitting that shellcheck otherwise has to be
talked out of.
Covered by a new regression test ('docker socket paths with glob
chars do not expand') that sets the env var to a literal '*' pattern
pointing at real directories; without this fix the test fails with
an unexpected docker.service warning.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
DOCKER_SOCKET_PATHS is a space separated list of socket paths (its
default is "/run/docker.sock /var/run/docker.sock") that the loop
iterates by word splitting. Socket paths never contain whitespace on
Linux hosts, so this is the documented and correct idiom — not a
shellcheck oversight. Add an inline comment so a future maintainer (or
a linter that has not seen the comment) does not break the loop by
adding quotes around the expansion.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Without a guard, running make bats-unit-tests in a tree where the
hack/*.bats wildcard matches nothing (for example a rename, a shallow
clone, or a future reorganization that moves the tests elsewhere)
would silently succeed with zero tests run. Add an explicit guard that
fails the target with a clear error message when BATS_UNIT_FILES
expands to an empty list, so the developer cannot accidentally believe
tests passed when no tests executed.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Two review findings:
1. The socket fallback tests previously branched on 'command -v
python3' and fell through to 'return 0' when it was missing.
cozytest.sh has no SKIP concept — 'return 0' is indistinguishable
from a real pass, so on a runner without python3 the socket
fallback paths were silently unverified. Since the script uses
'[ -e "$sock" ]' rather than '[ -S ... ]', a regular file is
sufficient to exercise the detection path. Replace the python3
unix-socket creation with 'touch "$SOCK"' so the tests run
unconditionally on every runner.
2. Extend the 'both services active' HINT assertion to require the
'sudo ' prefix on the 'systemctl disable --now' line. Without sudo
the operator would be instructed to run a privileged command as a
non-root user and hit a silent failure; the prefix is as important
as the verb itself and must be locked in by a test.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Two small Makefile tweaks:
1. bats-unit-tests now iterates over every hack/*.bats file that is
not an e2e-*.bats file (wildcard + filter-out), so dropping a new
unit bats file into hack/ is enough to get it picked up by CI on
the next run. The previous hard coded single file path would have
required a Makefile edit for every new unit suite.
2. Add a new 'preflight' target that runs hack/check-host-runtime.sh.
This gives the script a discoverable entry point — an operator
preparing a generic (k3s/kubeadm) host for cozystack can now run
'make preflight' from the repository root to get the same warning
that is described in the script header, without having to know
the exact path under hack/.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Expand the bats suite to address a second review pass:
- Assert that a containerd only warning produces a HINT that names
containerd.service and NOT docker.service, and mirror the check in
the docker only test. When both fire, assert the HINT lists both
services in a single systemctl disable invocation.
- New test: 'clean host without systemctl exits silently' — exercises
the COZYSTACK_PREFLIGHT_FORCE_NO_SYSTEMCTL=1 path when no standalone
sockets exist. Previously the tests only covered the systemd enabled
clean host, leaving the non systemd clean path unverified.
- New test: 'docker service plus socket still emits exactly one
warning' — mirrors the existing containerd service+socket test and
locks in the gated check in check_docker.
- Replace silent 'return 0' on missing python3 with a visible
'# SKIP: python3 unavailable' message on stderr so CI logs make it
obvious which tests were skipped on a given runner.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
When only containerd.service was active, the HINT block still advised
the operator to disable both containerd.service and docker.service,
which is misleading and potentially dangerous on hosts where docker is
legitimately in use. Track per service warnings (CONTAINERD_WARN and
DOCKER_WARN) and build the HINT systemctl disable argument from the
services that actually fired.
Also guard the ANSI color escapes behind an 'is stderr a TTY' check so
log files (CI and reviewer captures) do not accumulate raw escape
sequences.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Address review feedback on the preflight test suite:
- Add a trap 'rm -rf $STUB_DIR' EXIT in every test immediately after
mktemp -d, so temp dirs are cleaned up even when an assertion fails
and terminates the test early under set -e.
- Assert the HINT block and 'systemctl disable --now' line in the
'both services active' test so a future silent removal or typo in
the HINT output is caught by CI.
- New test: 'docker socket fallback fires when systemctl is
unavailable' — mirrors the existing containerd socket fallback test
and exercises the same code path in check_docker.
- New test: 'containerd service plus socket still emits exactly one
warning' — documents and locks in the intent of the symmetric gated
check in check_containerd. Uses grep -c to assert the warning is not
double printed when both signals fire.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Myasnikov Daniil