Skip to content

bug(api): @immutable (x-kubernetes-validations transition rules) not enforced on Application update #3970

Closed as not planned

Description

@scooby87

Summary

The @immutable annotation on an app's values.yaml is translated by cozyvalues-gen into an x-kubernetes-validations transition rule (self == oldSelf) that reaches the ApplicationDefinition's openAPISchema, but the apps.cozystack.io aggregated apiserver never evaluates it on update. Immutability is therefore silently unenforced at admission for every Application kind, not just one app.

Mechanism

buildSpecSchema builds a structural schema from the embedded openAPISchema, but the only consumer of that schema is defaulting — pkg/registry/apps/application/rest_defaulting.go calls defaultLikeKubernetes(&m, r.specSchema). The update path in pkg/registry/apps/application/rest.go validates only the object name (ValidateApplicationName); there is no CEL / x-kubernetes-validations evaluation wired in, so self == oldSelf rules are carried in the schema and never checked. The unit test pkg/registry/apps/application/rest_spec_schema_test.go pins only that the schema builds with XValidations preserved through conversion — it does not assert that the rule is enforced on update.

Reproduce (verified live)

  1. Take any VMDisk (its storageClass already carries self == oldSelf / "storageClass is immutable" in the shipped ApplicationDefinition).
  2. kubectl patch vmdisk <name> -n <tenant> --type=merge -p '{"spec":{"storageClass":"local"}}'
  3. Expected: rejected by admission ("storageClass is immutable"). Actual: vmdisk ... patched — the change is accepted.

Impact

Every field marked @immutable across all apps (e.g. MariaDB storageClass, and storageClass/source on vm-disk after #3969) is advisory only. Downstream helm templates that reuse an immutable resource's spec then silently no-op the edit; #3969 adds a fail() backstop in the vm-disk chart, but the proper fix is to enforce transition rules at the apiserver so the edit is rejected at kubectl apply time.

Suggested direction

Evaluate the embedded schema's x-kubernetes-validations (including transition rules that reference oldSelf) during update in the Application registry, and add a test that a self == oldSelf field actually rejects a changed value on update — not just that the schema builds.

Related: #2985, #3969.

Activity

  1. added
    kind/bugCategorizes issue or PR as related to a bug
    area/apiIssues or PRs related to the cozystack-api aggregated API server
    on Aug 26, 2026
  2. dosubot commented on Aug 26, 2026

    @dosubot

    scooby87 the analysis in the report is accurate — I can confirm the gap from the code.

    buildSpecSchema is defaulting-only. Its comment explicitly says it returns "the structural-schema form used for defaulting" [1], and the only consumer (applySpecDefaults) calls defaultLikeKubernetes with no CEL evaluation path at all [2].

    The Update path has no schema-based validation. After updateValidation (the generic apiserver-level hook — not CEL from x-kubernetes-validations), it runs validateNoInternalKeys, tenant quota checks, and legacy-field warnings, then goes straight to ConvertApplicationToHelmRelease and writes [3]. Nothing in that path evaluates transition rules.

    k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel is not imported anywhere in the registry package. The only CEL import in the codebase is celopenapi "k8s.io/apiserver/pkg/cel/openapi" in pkg/cmd/server/openapi_test.go — that's for VAP type-checking of the published OpenAPI schema, unrelated to admission [4].

    Existing tests cover name validation and schema construction — not enforcement. The test file rest_validation_test.go pins name-format validation, name-length limits, and tenant namespace overflow, but contains nothing about x-kubernetes-validations or transition rules being enforced on update. The line-1673 comment in rest.go about "kstatus readiness" refers to the HelmRelease HealthCheckExprs, not to schema CEL rules [5].

    For the fix, the right upstream primitive is k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel — specifically the Validator it exposes, which handles oldSelf transition rules (this is the same package CRD admission uses). The specSchema *structuralschema.Structural is already built and stored on REST [6], so the wiring would be:

    1. Compile CEL rules at NewREST time using cel.NewValidator(specSchema, ...).
    2. In REST.Update, after applySpecDefaults, call validator.Validate(...) with both old and new spec objects.
    3. Return the resulting field.ErrorList as a StatusReasonInvalid error if non-empty.
    4. Add a test that patches a self == oldSelf field and asserts a 422 Invalid response.

    To reply, just mention @dosu.


    Share context across your team and agents. Try Dosu.

    Leave Feedback Learn about cozystack with Dosu Add Dosu to your team

  3. scooby87 commented on Aug 27, 2026

    @scooby87
    ContributorAuthor

    Duplicate of #2657, which already tracks enforcing x-kubernetes-validations CEL rules (including self == oldSelf transition rules) in the apps.cozystack.io aggregated apiserver. Closing in favour of #2657.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/apiIssues or PRs related to the cozystack-api aggregated API serverkind/bugCategorizes issue or PR as related to a bugtriage/needs-triageIndicates an issue needs triage by a maintainer

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions