- 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>
(cherry picked from commit 61ed7ad89c)
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>
(cherry picked from commit 7257b6aed4)
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>
(cherry picked from commit 5b2501db91)
Tenant applications live in a child namespace computed from the Tenant
name, not in the HelmRelease's namespace. Scope the WorkloadMonitor
watch cluster-wide when kindName is Tenant and reverse-map WM events
back to the owning HelmRelease via computeTenantNamespace. The
condition-enrichment path now looks up monitors in the computed child
namespace too, so WorkloadsReady is populated for Tenants.
Buffer WorkloadMonitor events that arrive before the initial-events-end
bookmark and replay them once the bookmark is emitted, to preserve the
watch-list contract while not dropping workload-state transitions that
happen during the snapshot window.
Pass the fresh WorkloadMonitor object from the watch event into the
conversion path so WorkloadsReady reflects the state that triggered
the event, even when the cache client is lagging behind the watch
client.
Derive WorkloadsReady.LastTransitionTime from a stable source
(max of HelmRelease creation and condition timestamps, plus monitor
timestamps) instead of metav1.Now(), so repeated conversions of the
same underlying state produce identical timestamps.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Add two features to improve application observability in the dashboard:
1. WorkloadsReady condition on Application status
- Query WorkloadMonitor resources to determine if all pods are running
- Expose WorkloadsReady as a separate condition alongside Ready
- Ready continues to reflect HelmRelease state only (no override) to
preserve backward compatibility with tooling and avoid false-negatives
during normal startup windows when pods are still coming up
- Handle three states: operational, not operational, unknown (pending)
- Fail-open on WorkloadMonitor query errors
- Integrate with Application Watch to emit MODIFIED events on
WorkloadMonitor changes (with initial-events-end safety)
- Register cozystack.io/v1alpha1 types in API server scheme
2. Events tab in dashboard
- Show Kubernetes Events scoped to application namespace
- Use status.namespace for Tenant applications
- Include both lastTimestamp and eventTime columns for K8s compat
3. Bug fix: WorkloadMonitor Operational status persistence
- Operational was written to stale 'monitor' instead of 'fresh'
inside RetryOnConflict, so it was never persisted to the cluster
Closes#2359Closes#2360
Assisted-By: Claude <noreply@anthropic.com>
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>
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>
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>
Import SchedulingClassAnnotation, SchedulingClassLabel, SchedulerName,
and GVR components from the cozystack-scheduler/pkg/apis sub-module
instead of defining them locally. This ensures the webhook and scheduler
stay in sync on label/annotation keys.
Also standardize the namespace label from scheduling.cozystack.io/class
to scheduler.cozystack.io/scheduling-class for consistency with the
scheduler, and resolve scheduling class from the owner Application CR
(via a new SchedulingClass() stub method) before falling back to the
namespace label.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
The OpenAPI PostProcessSpec callback is invoked for every group-version
(apps, core, version, etc.), but the Application schema cloning logic
only applies to apps.cozystack.io. When called for other GVs the base
Application schemas are absent, causing a spurious error log on every
API server start.
Return early instead of erroring when the base schemas are not found.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
## What this PR does
Fix build error in `pkg/generated/applyconfiguration/utils.go` caused by
a reference to `testing.TypeConverter` which was removed in client-go
v0.34.1.
The root cause was that `hack/update-codegen.sh` called `gen_helpers`
and
`gen_openapi` but never called `gen_client`, so the applyconfiguration
code
was never regenerated after the client-go upgrade.
Changes:
- Fix `THIS_PKG` from `k8s.io/sample-apiserver` template leftover to
correct module path
- Add `kube::codegen::gen_client` call with `--with-applyconfig` flag
- Regenerate applyconfiguration (now uses `managedfields.TypeConverter`)
- Add tests for `ForKind` and `NewTypeConverter` functions
### Release note
```release-note
[maintenance] Regenerate applyconfiguration code for client-go v0.34.1 compatibility
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Documentation**
* Updated backup class definitions example to reference MariaDB instead
of MySQL.
* **Chores**
* Updated code generation tooling and module dependencies to support
enhanced functionality.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
The applyconfiguration code referenced testing.TypeConverter from
k8s.io/client-go/testing, which was removed in client-go v0.34.1.
Root cause: hack/update-codegen.sh called gen_helpers and gen_openapi
but not gen_client, so applyconfiguration was never regenerated after
the client-go upgrade.
Changes:
- Fix THIS_PKG from sample-apiserver template leftover to correct
module path
- Add kube::codegen::gen_client call with --with-applyconfig flag
- Regenerate applyconfiguration (now uses managedfields.TypeConverter)
- Add tests for ForKind and NewTypeConverter functions
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Root-host validation for Tenant names is no longer needed here.
The underlying issue (namespace.cozystack.io/host label exceeding
63-char limit) will be addressed in #2002 by moving the label
to an annotation.
Name length validation now only checks the Helm release name
limit (53 - prefix length), which applies uniformly to all
application types.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
- Fix goimports order: duration before validation/field
- Show rootHost in error messages only for Tenant kind where it
actually affects the length calculation
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
- Return field.ErrorList from validateNameLength for consistent
apierrors.NewInvalid error shape (was NewBadRequest)
- Add klog warning when YAML parsing fails in parseRootHostFromSecret
- Fix maxHelmReleaseName comment to accurately describe Helm convention
- Add note that root-host changes require API server restart
- Replace interface{} with any throughout openapi.go and rest.go
- Remove trailing blank line in const block
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Remove patchObjectMetaNameValidation and patchObjectMetaNameValidationV2
functions that were modifying the global ObjectMeta schema. This patching
affected ALL resources served by the API server, not just Application
resources. Backend validation in Create() is sufficient for enforcing
name constraints.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Replace custom DNS-1035 regex with k8s.io/apimachinery IsDNS1035Label.
Remove hardcoded maxApplicationNameLength=40 from both validation and
OpenAPI — length validation is now handled entirely by validateNameLength
which computes dynamic limits based on Helm release prefix and root-host.
Fix README to reflect that max length depends on cluster configuration.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Skip DNS-1035 and length validation on Update since Kubernetes names
are immutable — validating would block updates to pre-existing resources
with non-conforming names. Replace fmt.Printf with klog for structured
logging consistency.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
- Add DNS-1035 format validation to Update path (was only in Create)
- Simplify Secret reading by reusing existing scheme instead of
creating a separate client
- Add nil secret test case for parseRootHostFromSecret
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Add protection against negative or zero maxLen when release prefix or
root host are too long, returning a clear configuration error instead of
a confusing "name too long" message. Add corresponding test cases.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Read root-host from cozystack-values secret at API server startup
and use it to compute maximum allowed name length for applications.
For all apps: validates prefix + name fits within the Helm release
name limit (53 chars). For Tenants: additionally checks that the
host label (name + "." + rootHost) fits within the Kubernetes label
value limit (63 chars).
This replaces the static 40-char limit with a dynamic calculation
that accounts for the actual cluster root host length.
Ref: https://github.com/cozystack/cozystack/issues/2001
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Add pattern and maxLength constraints to ObjectMeta.name in OpenAPI schema.
This enables UI form validation when openapi-k8s-toolkit supports it.
- Pattern: ^[a-z]([-a-z0-9]*[a-z0-9])?$ (DNS-1035)
- MaxLength: 40
Depends on: cozystack/openapi-k8s-toolkit#1
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Add validation to ensure Application names (including Tenants) conform
to DNS-1035 format. This prevents creation of resources with names
starting with digits, which would cause Kubernetes resource creation
failures (e.g., Services, Namespaces).
DNS-1035 requires names to:
- Start with a lowercase letter [a-z]
- Contain only lowercase alphanumeric or hyphens [-a-z0-9]
- End with an alphanumeric character [a-z0-9]
Also fixes broken validation.go that referenced non-existent internal
types (apps.Application, apps.ApplicationSpec).
Fixes: https://github.com/cozystack/cozystack/issues/1538
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
- 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>
Rename the CRD and all related types for better clarity:
- CozystackResourceDefinition -> ApplicationDefinition
- CozystackResourceDefinitionList -> ApplicationDefinitionList
- CozystackResourceDefinitionSpec -> ApplicationDefinitionSpec
- All related nested types updated accordingly
Updated components:
- API types and generated deepcopy code
- Controllers and reconcilers
- Dashboard, lineagecontrollerwebhook, crdmem packages
- CRD YAML definition and Helm chart
- All 25 cozyrds YAML manifests
- Migration scripts and documentation
Added migration 23 to remove old cozystack-resource-definition-crd HelmRelease.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Replace the chart field with chartRef for referencing Helm charts via
ExternalArtifact resources. This enables the Package controller to
manage chart sources centrally.
Changes:
- Add chartRef field to CozystackResourceDefinition spec
- Remove chart field (deprecated)
- Remove validation (moved to controller)
- Update lineage mapper for new field structure
- Regenerate openapi specs
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>
Replace Helm lookup functions with FluxCD valuesFrom mechanism for
reading cluster and namespace configuration.
Changes:
- Create Secret cozystack-values in each namespace with values.yaml key
containing _cluster and _namespace configuration as nested YAML
- Configure HelmReleases to read from this Secret via valuesFrom
(valuesKey defaults to values.yaml, so it can be omitted)
- Update cozy-lib helpers to access config via .Values._cluster
- Add default values for required _cluster keys to ensure all fields exist
- Update Go code (cozystack-api and helm reconciler) to use new format
This eliminates the need for Helm lookup functions while maintaining
the same configuration interface for charts.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
- Simplify ByName with strings.Compare
- Avoid string allocation in ByNamespacedName by comparing fields separately
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
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 change is extracted from
- https://github.com/cozystack/cozystack/pull/1641
and reworked to work standalone
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
<!-- Thank you for making a contribution! Here are some tips for you:
- Start the PR title with the [label] of Cozystack component:
- For system components: [platform], [system], [linstor], [cilium],
[kube-ovn], [dashboard], [cluster-api], etc.
- For managed apps: [apps], [tenant], [kubernetes], [postgres],
[virtual-machine] etc.
- For development and maintenance: [tests], [ci], [docs], [maintenance].
- If it's a work in progress, consider creating this PR as a draft.
- Don't hesistate to ask for opinion and review in the community chats,
even if it's still a draft.
- Add the label `backport` if it's a bugfix that needs to be backported
to a previous version.
-->
## What this PR does
### Release note
<!-- Write a release note:
- Explain what has changed internally and for users.
- Start with the same [label] as in the PR title
- Follow the guidelines at
https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md.
-->
```release-note
[cozystack] Update go modules
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Chores**
* Updated Go toolchain to 1.25.0 and upgraded core Kubernetes libraries,
OpenTelemetry, Prometheus, gRPC/protobuf and many indirect dependencies.
Bumped builder base images to golang:1.25-alpine across multiple
components.
* **Refactor**
* Removed legacy component versioning/emulation and simplified server
startup and configuration paths.
* **Tests**
* Removed tests related to the legacy versioning/emulation behavior.
<sub>✏️ Tip: You can customize this high-level summary in your review
settings.</sub>
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
