- Gate defensive label post-filter on event type: DELETED events now always
pass through so cached clients can evict stale entries when a Secret's
labels mutate out of the watched selector
- Extract buildTenantSelector helper to eliminate duplicated selector-merge
code between List and Watch
- Add TestList_WithNothingSelector_ReturnsEmpty and
TestWatch_WithNothingSelector_ClosesImmediately to cover the previously
untested labels.Nothing() path
Signed-off-by: IvanHunters <ivan.okhotnikov@aenix.io>
Watch() in the TenantSecret REST handler hard-coded the selector to the
internal.cozystack.io/tenantresource=true label and discarded
opts.LabelSelector entirely. Watch streams therefore returned every
tenant Secret in the namespace regardless of the user-provided selector,
while List() filtered correctly. Clients that switch to Watch after the
initial List surfaced unrelated Secrets.
Merge opts.LabelSelector into the underlying watch selector the same
way List() already does, handle non-selectable selectors by returning a
closed watcher, and add a defensive post-filter in the streaming
goroutine so events never escape the requested selector even if a future
client implementation skips label filtering on Watch.
Adds rest_test.go covering label-selector merging in List and a Watch
test that creates Secrets after the watch starts (since fake.Client.Watch
does not emit initial ADDED events) to assert only matching events are
observed.
Signed-off-by: IvanHunters <ivan.okhotnikov@aenix.io>
- Hoist user identity extraction out of the Watch goroutine; reuse a
cached username and groups map across events instead of re-fetching
them per event. Watch now returns Unauthorized up front when no user
is present in the context, rather than failing silently per event.
- Switch the per-event access-check error log to structured klog.ErrorS
to comply with the project Go style guide.
- Strengthen TestGet_WithAccess to assert the concrete *TenantNamespace
type plus Name, Kind, and APIVersion, so type or metadata regressions
fail fast.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Address review feedback from sircthulhu and CodeRabbit:
1. Return Forbidden instead of NotFound for unauthorized access
- Get() now returns 403 Forbidden to follow standard K8s RBAC behavior
- Previously returned 404 NotFound for security-by-obscurity
- Updated test expectations to match new behavior
2. Propagate field and label selectors in Watch handler
- Pass opts.FieldSelector and opts.LabelSelector to upstream Watch
- Add defensive filtering before authorization to prevent RBAC bypass
- Fixes potential issue with resourceNames restrictions
3. Refactor subject-matching logic to eliminate duplication
- Extract matchesSubject() helper for Group/User/ServiceAccount checks
- Remove duplicated code from filterAccessible and hasAccessToNamespace
- Consolidates ServiceAccount namespace fallback logic
All tests pass successfully.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Added comprehensive unit tests for authorization logic:
- hasAccessToNamespace() tests:
- User subject access (positive and negative cases)
- Group subject access
- ServiceAccount subject access
- ServiceAccount with empty namespace (defaults to RoleBinding ns)
- Privileged groups (system:masters, cozystack-cluster-admin)
- Get() handler tests:
- Returns namespace when user has access
- Returns NotFound when user lacks access (not Forbidden)
- Returns NotFound for non-tenant namespaces
All tests verify that authorization correctly enforces RoleBinding-based
access control and prevents IDOR vulnerabilities.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
- Add resourceVersion handling for Watch requests by filtering ADDED events
based on the resourceVersion provided by the client
- Forward bookmark events from underlying HelmRelease watchers to clients
for proper resourceVersion synchronization
- Extract MaxResourceVersion helper using meta.EachListItem for cleaner code
- This ensures clients don't receive duplicate objects they already have
from List+Watch patterns
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Fix getVersion to parse "0.1.4+abcdef" format (with "+" separator)
instead of incorrectly looking for "sha256:" prefix.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Controller-runtime cache doesn't support field selectors, causing
incorrect filtering when using kubectl with field selectors like
--field-selector=metadata.namespace=tenant-kvaps or metadata.name=test.
Changes:
- Created pkg/registry/fields package with ParseFieldSelector utility
- Refactored field selector parsing logic in application, tenantmodule,
and tenantsecret registries to use common implementation
- Implemented manual filtering for metadata.name and metadata.namespace
in List() and Watch() methods
- Removed Raw field usage and field selectors from client.ListOptions
- Label selectors passed directly via LabelSelector field
Field selectors now properly filter resources by name and namespace
through manual post-processing after label-based filtering.
See: https://github.com/kubernetes-sigs/controller-runtime/issues/612
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Add sorting helper package with generic functions for sorting
resources by name (cluster-scoped) or namespace/name (namespace-scoped).
Apply sorting to:
- TenantNamespace (by name)
- TenantSecret (refactored to use helper)
- TenantModule (new)
- Application (new)
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
This commit refactors the apiserver REST handlers to use typed objects
(appsv1alpha1.Application) instead of unstructured.Unstructured, eliminating
the need for runtime conversions and simplifying the codebase.
Additionally, it fixes an issue where UnstructuredList objects were using
the first registered kind from typeToGVK instead of the kind from the
object's field when multiple kinds are registered with the same Go type.
This is a more comprehensive fix for the problem addressed in
https://github.com/cozystack/cozystack/pull/1630, which was reverted in
https://github.com/cozystack/cozystack/pull/1677.
The fix includes the upstream fix from kubernetes/kubernetes#135537,
which enables short-circuit path for UnstructuredList similar to regular
Unstructured objects, using GVK from the object field instead of
typeToGVK.
Changes:
- Refactored rest.go handlers to use typed Application objects
- Removed unstructured.Unstructured conversions
- Fixed UnstructuredList GVK handling
- Updated dependencies in go.mod/go.sum
- Added e2e test for OpenAPI validation
- Updated Dockerfile
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
This patch changes all clients in the Cozystack API server to typed
ones from the controller runtime. This should improve the performance of
the API server and simplifies the code by removing work with
unstructured objects and dynamic clients.
```release-note
[api] Use typed and cache-backed k8s clients in the Cozystack API to
improve performance. Get rid of operations on unstructured objects and
use of dynamic clients.
```
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
The Cozystack API server lists TenantNamespaces by running a
SubjectAccessReview against every single requested namespace to see if
the user can create a WorkloadMonitor there. Will this is robust in
terms of permissions, delegating the authorization decision to the k8s
API, this is incredibly inefficient and has caused high latency to the
API. This patch simplifies the logic by instead getting the user's
groups and checking if the namespace contains a rolebinding for that
group. That way listing TenantNamespaces is reduced to a list call to
the k8s API for namespaces and another list call for rolebindings across
all namespaces, while authorization is done on the Cozystack API server
instead of making further calls to the k8s API.
```release-note
[api] Optimize listing of TenantNamespaces, fixes a bug causing very
high latency to the k8s API.
```
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
This patch refactors the secret selectors to use the
`internal.cozystack.io/tenantresource` label for managing secret
visibility and removes any selectors based on it or the previous
`apps.cozystack.io/tenantresource` label, the idea being that this label
will only ever be set by the controller.
```
[controller,api] Refactor labels for the secret selector.
```
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
[cozystack-controller] Introduce new dashboard-controller
[dashboard] Introduce new dashboard based on openapi-ui
Co-authored-by: kklinch0 <kklinch0@gmail.com>
Signed-off-by: kklinch0 <kklinch0@gmail.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
IvanHunters