Skip to content

feat(apps): mark stateful-app storageClass fields as immutable - #2639

Merged
Aleksei Sviridkin (lexfrei) merged 1 commit into
mainfrom
feat/mariadb-storageclass-immutable
May 27, 2026
Merged

feat(apps): mark stateful-app storageClass fields as immutable#2639
Aleksei Sviridkin (lexfrei) merged 1 commit into
mainfrom
feat/mariadb-storageclass-immutable

Conversation

@lexfrei

@lexfrei Aleksei Sviridkin (lexfrei) commented May 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Annotate storageClass as immutable across every stateful chart that exposes one: clickhouse, foundationdb, harbor, http-cache, kafka (Kafka + ZooKeeper), kubernetes (top-level + nodeGroups[]), mariadb, mongodb, nats, openbao, opensearch, postgres, qdrant, rabbitmq, redis, vm-disk. The PVC-pinning argument is identical for every chart — Kubernetes fixes storageClassName on each PVC at creation, so editing this field on an existing resource never migrated data; the schema annotation makes that contract explicit and lets schema-aware consumers refuse the edit up-front.

Schema-aware consumers today: the Cozystack UI (cozystack/cozystack-ui#6) reads the rule from the chart's openAPISchema and renders the field read-only on edit forms.

This PR is schema-side only. The cozystack aggregated apiserver does not yet evaluate CEL rules in openAPISchema (MariaDB / Postgres / etc. are application-backed, not CRDs); tracking issue #2657 covers wiring CEL into pkg/registry/apps/application/rest.go::Update. Until that lands, an apiserver-level kubectl patch that changes storageClass is still accepted — but the underlying StatefulSet PVCs do not move, so there's no data-corruption risk, only a confusing UX. The UI catches that case today; the apiserver gate closes it permanently.

What changes

For each app in the list above:

  • packages/apps/<app>/values.yaml — gain a ## @immutable directive directly above (or below, for fields inside a @typedef) the storageClass line.
  • Regenerated via make generate in each package:
    • api/apps/v1alpha1/<app>/types.go — kubebuilder XValidation marker on the relevant field(s).
    • packages/apps/<app>/values.schema.jsonx-kubernetes-validations: [{rule: "self == oldSelf", message: …}] on storageClass.
    • packages/apps/<app>/README.md — regenerated parameter table.
    • packages/system/<app>-rd/cozyrds/<app>.yaml — the embedded openAPISchema on the ApplicationDefinition reflects the change.

Refactor: pkg/registry/apps/application/rest.go factors the inline schema-build path into a new buildSpecSchema helper so it can be unit-tested without standing up a fake apiserver. Three regression tests in pkg/registry/apps/application/rest_spec_schema_test.go pin the contract that x-kubernetes-validations survives the v1→internal→structural conversion (test loads the actual MariaDB ApplicationDefinition fixture on disk, asserts the rule reaches the structural schema). Empty-input and malformed-JSON paths are also pinned.

No e2e admission test is added because the apiserver does not yet enforce the rule — see #2657.

No changelog entry: schema-side annotation is invisible at the apiserver layer today (UI-only consumer) and the release-cut PR will add the entry when admission enforcement lands.

Dependencies

Requires cozystack/cozyvalues-gen#24 to be merged and released, and the cozyvalues-gen binary pin in .github/workflows/pre-commit.yml line 50 bumped to the release that includes that PR. Pre-commit CI will fail until both happen — the stock v1.3.0 binary silently ignores ## @immutable and regenerates artefacts without the rule, producing a drift against what's committed.

Once that ordering is satisfied, rebase this branch and the pre-commit job will be green.

Summary by CodeRabbit

  • New Features

    • storageClass is now treated as immutable after creation for many stateful apps (enforced by chart/CRD schema validation).
  • Documentation

    • Added a storage immutability guide explaining the contract and enforcement.
    • Updated chart READMEs and values docs to mark storageClass as immutable and clarified nodeGroup storage remains intentionally mutable.
  • Tests

    • Added tests validating schema parsing and preservation of immutability rules.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds CEL immutability validations (rule: self == oldSelf) for storageClass across CRD Go types, Helm values schemas, and ApplicationDefinition openAPISchema; refactors Application REST spec parsing into buildSpecSchema; adds tests and a docs page describing the contract.

Changes

storageClass immutability enforcement across CRDs, Helm charts, and ApplicationDefinitions

Layer / File(s) Summary
CRD Go type annotations
api/apps/v1alpha1/*/types.go
Adds +kubebuilder:validation:XValidation:rule="self == oldSelf",message="storageClass is immutable" to storageClass fields in API types.
Helm chart values schemas & docs
packages/apps/*/values.schema.json, packages/apps/*/values.yaml, packages/apps/*/README.md
Adds x-kubernetes-validations immutability rules and ## @immutable`` markers; updates READMEs to note storageClass is immutable and link to docs/storage-immutability.md.
System ApplicationDefinition schemas
packages/system/*-rd/cozyrds/*.yaml
Adds x-kubernetes-validations immutability rules for chart storageClass entries in embedded openAPISchema payloads.
Spec-schema parsing & tests
pkg/registry/apps/application/rest.go, pkg/registry/apps/application/rest_spec_schema_test.go
Introduces buildSpecSchema to parse/convert raw openAPISchema JSON into a structural schema, wires it into NewREST, and adds tests covering conversion with CEL rules, empty input, and malformed JSON.
Documentation
docs/storage-immutability.md
New document describing the storageClass immutability contract, where it’s enforced, covered charts, exceptions, and authoring guidance.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Suggested labels

size/XL, area/platform

Suggested reviewers

  • kvaps
  • myasnikovdaniil
  • lllamnyp
  • sircthulhu

Poem

🐰 A carrot compiler, soft and bright,

I hopped through schemas late last night.
I stitched "self == oldSelf" in place,
Now storageClass keeps its grace.
Hooray — immutable, snug and tight!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding immutability annotations to storageClass fields across stateful application charts, which is the primary objective of this comprehensive changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/mariadb-storageclass-immutable

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added area/database Issues or PRs related to managed databases (postgres, mariadb, redis, etcd, kafka, clickhouse) kind/feature Categorizes issue or PR as related to a new feature size/S This PR changes 10-29 lines, ignoring generated files labels May 12, 2026
@lexfrei
Aleksei Sviridkin (lexfrei) force-pushed the feat/mariadb-storageclass-immutable branch 3 times, most recently from e2676c8 to 2dd4407 Compare May 14, 2026 16:49
@github-actions github-actions Bot added size/M This PR changes 30-99 lines, ignoring generated files and removed size/S This PR changes 10-29 lines, ignoring generated files labels May 14, 2026
@lexfrei
Aleksei Sviridkin (lexfrei) force-pushed the feat/mariadb-storageclass-immutable branch from 2dd4407 to 5f71dae Compare May 14, 2026 21:41
@github-actions github-actions Bot added size/L This PR changes 100-499 lines, ignoring generated files and removed size/M This PR changes 30-99 lines, ignoring generated files labels May 14, 2026
@lexfrei
Aleksei Sviridkin (lexfrei) force-pushed the feat/mariadb-storageclass-immutable branch 2 times, most recently from 4c2a7c3 to ad99cbc Compare May 15, 2026 08:27
@lexfrei Aleksei Sviridkin (lexfrei) changed the title feat(mariadb): mark storageClass as immutable feat(apps): mark stateful-app storageClass fields as immutable May 15, 2026
@github-actions github-actions Bot added the area/uncategorized PR auto-labeler could not map title scope to a known area/*; please review label May 15, 2026
@lexfrei
Aleksei Sviridkin (lexfrei) force-pushed the feat/mariadb-storageclass-immutable branch 2 times, most recently from f419939 to 84c0ffd Compare May 15, 2026 08:50
@github-actions github-actions Bot added size/XL This PR changes 500-999 lines, ignoring generated files and removed size/L This PR changes 100-499 lines, ignoring generated files labels May 15, 2026
@lexfrei
Aleksei Sviridkin (lexfrei) force-pushed the feat/mariadb-storageclass-immutable branch from 84c0ffd to 25721d8 Compare May 15, 2026 09:02
@github-actions github-actions Bot added size/L This PR changes 100-499 lines, ignoring generated files and removed size/XL This PR changes 500-999 lines, ignoring generated files labels May 15, 2026
@lexfrei
Aleksei Sviridkin (lexfrei) marked this pull request as ready for review May 18, 2026 08:05

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request enforces the immutability of the storageClass field across multiple stateful applications by adding CEL validation rules to API types and Helm chart schemas. It also includes documentation updates and a refactor of the schema building logic in the application registry, accompanied by new unit tests. Feedback was provided regarding a violation of the repository style guide where errors were being discarded using blank identifiers during scheme initialization.

Comment on lines +114 to +115
_ = internalapiext.AddToScheme(scheme)
_ = apiextv1.AddToScheme(scheme)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Discarding errors with _ when adding types to the scheme violates the repository style guide. Although these calls are unlikely to fail in this context, they should be handled explicitly to ensure the scheme is correctly initialized before conversion.

Suggested change
_ = internalapiext.AddToScheme(scheme)
_ = apiextv1.AddToScheme(scheme)
if err := internalapiext.AddToScheme(scheme); err != nil {
return nil, fmt.Errorf("add internalapiext to scheme: %w", err)
}
if err := apiextv1.AddToScheme(scheme); err != nil {
return nil, fmt.Errorf("add apiextv1 to scheme: %w", err)
}
References
  1. Handle errors explicitly. Discarding meaningful errors with _ is a bug. (link)

@lexfrei
Aleksei Sviridkin (lexfrei) force-pushed the feat/mariadb-storageclass-immutable branch from 25721d8 to 06b6dc9 Compare May 25, 2026 12:47
@lexfrei
Aleksei Sviridkin (lexfrei) marked this pull request as draft May 25, 2026 12:49
@lexfrei
Aleksei Sviridkin (lexfrei) marked this pull request as ready for review May 25, 2026 14:42
@lexfrei
Aleksei Sviridkin (lexfrei) force-pushed the feat/mariadb-storageclass-immutable branch from 06b6dc9 to 440a059 Compare May 25, 2026 14:42
@dosubot dosubot Bot added the quality-of-life QoL improvements label May 25, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request enforces the immutability of the storageClass field across multiple stateful applications by introducing CEL validation rules in API types, Helm schemas, and application definitions. It also refactors the OpenAPI schema parsing logic and adds unit tests to ensure validation rules survive structural schema conversion. Review feedback highlighted a violation of the repository's error handling policy regarding discarded errors and noted an inconsistent validation message in the PostgreSQL configuration.

Comment on lines +114 to +115
_ = internalapiext.AddToScheme(scheme)
_ = apiextv1.AddToScheme(scheme)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Discarding errors from AddToScheme calls violates the repository style guide (Rule 88), which states that discarding meaningful errors with _ is a bug. Although these calls are unlikely to fail in practice, they should be handled explicitly to ensure the scheme is correctly initialized before conversion.

Suggested change
_ = internalapiext.AddToScheme(scheme)
_ = apiextv1.AddToScheme(scheme)
if err := internalapiext.AddToScheme(scheme); err != nil {
return nil, fmt.Errorf("add internal apiextensions to scheme: %w", err)
}
if err := apiextv1.AddToScheme(scheme); err != nil {
return nil, fmt.Errorf("add v1 apiextensions to scheme: %w", err)
}
References
  1. Handle errors explicitly. Discarding meaningful errors with _ is a bug. (link)

Size resource.Quantity `json:"size"`
// StorageClass used to store the data.
// +kubebuilder:default:=""
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="storageClass is immutable"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

The validation message for postgresql is inconsistent with other applications in this PR (e.g., mariadb, clickhouse, harbor), which use "storageClass is immutable after creation". Maintaining a consistent message across all stateful apps improves the user experience and ensures uniform schema-aware validation feedback.

Suggested change
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="storageClass is immutable"
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="storageClass is immutable after creation"

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/apps/postgres/values.schema.json`:
- Around line 115-120: Update the source annotation that generates the JSON
validation so the message matches other apps: locate the
"x-kubernetes-validations" entry that contains the rule "self == oldSelf" and
change its "message" value from "storageClass is immutable" to "storageClass is
immutable after creation" in packages/apps/postgres/values.yaml so the generated
values.schema.json uses the standardized, more descriptive message.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f87c129b-350d-4c6e-b595-fa48b21bbf79

📥 Commits

Reviewing files that changed from the base of the PR and between 06b6dc9 and b09b44d.

📒 Files selected for processing (83)
  • api/apps/v1alpha1/clickhouse/types.go
  • api/apps/v1alpha1/foundationdb/types.go
  • api/apps/v1alpha1/harbor/types.go
  • api/apps/v1alpha1/httpcache/types.go
  • api/apps/v1alpha1/kafka/types.go
  • api/apps/v1alpha1/kubernetes/types.go
  • api/apps/v1alpha1/mariadb/types.go
  • api/apps/v1alpha1/mongodb/types.go
  • api/apps/v1alpha1/nats/types.go
  • api/apps/v1alpha1/openbao/types.go
  • api/apps/v1alpha1/opensearch/types.go
  • api/apps/v1alpha1/postgresql/types.go
  • api/apps/v1alpha1/qdrant/types.go
  • api/apps/v1alpha1/rabbitmq/types.go
  • api/apps/v1alpha1/redis/types.go
  • api/apps/v1alpha1/vmdisk/types.go
  • docs/storage-immutability.md
  • packages/apps/clickhouse/README.md
  • packages/apps/clickhouse/values.schema.json
  • packages/apps/clickhouse/values.yaml
  • packages/apps/foundationdb/README.md
  • packages/apps/foundationdb/values.schema.json
  • packages/apps/foundationdb/values.yaml
  • packages/apps/harbor/README.md
  • packages/apps/harbor/values.schema.json
  • packages/apps/harbor/values.yaml
  • packages/apps/http-cache/README.md
  • packages/apps/http-cache/values.schema.json
  • packages/apps/http-cache/values.yaml
  • packages/apps/kafka/README.md
  • packages/apps/kafka/values.schema.json
  • packages/apps/kafka/values.yaml
  • packages/apps/kubernetes/README.md
  • packages/apps/kubernetes/values.schema.json
  • packages/apps/kubernetes/values.yaml
  • packages/apps/mariadb/README.md
  • packages/apps/mariadb/values.schema.json
  • packages/apps/mariadb/values.yaml
  • packages/apps/mongodb/README.md
  • packages/apps/mongodb/values.schema.json
  • packages/apps/mongodb/values.yaml
  • packages/apps/nats/README.md
  • packages/apps/nats/values.schema.json
  • packages/apps/nats/values.yaml
  • packages/apps/openbao/README.md
  • packages/apps/openbao/values.schema.json
  • packages/apps/openbao/values.yaml
  • packages/apps/opensearch/README.md
  • packages/apps/opensearch/values.schema.json
  • packages/apps/opensearch/values.yaml
  • packages/apps/postgres/README.md
  • packages/apps/postgres/values.schema.json
  • packages/apps/postgres/values.yaml
  • packages/apps/qdrant/README.md
  • packages/apps/qdrant/values.schema.json
  • packages/apps/qdrant/values.yaml
  • packages/apps/rabbitmq/README.md
  • packages/apps/rabbitmq/values.schema.json
  • packages/apps/rabbitmq/values.yaml
  • packages/apps/redis/README.md
  • packages/apps/redis/values.schema.json
  • packages/apps/redis/values.yaml
  • packages/apps/vm-disk/README.md
  • packages/apps/vm-disk/values.schema.json
  • packages/apps/vm-disk/values.yaml
  • packages/system/clickhouse-rd/cozyrds/clickhouse.yaml
  • packages/system/foundationdb-rd/cozyrds/foundationdb.yaml
  • packages/system/harbor-rd/cozyrds/harbor.yaml
  • packages/system/http-cache-rd/cozyrds/http-cache.yaml
  • packages/system/kafka-rd/cozyrds/kafka.yaml
  • packages/system/kubernetes-rd/cozyrds/kubernetes.yaml
  • packages/system/mariadb-rd/cozyrds/mariadb.yaml
  • packages/system/mongodb-rd/cozyrds/mongodb.yaml
  • packages/system/nats-rd/cozyrds/nats.yaml
  • packages/system/openbao-rd/cozyrds/openbao.yaml
  • packages/system/opensearch-rd/cozyrds/opensearch.yaml
  • packages/system/postgres-rd/cozyrds/postgres.yaml
  • packages/system/qdrant-rd/cozyrds/qdrant.yaml
  • packages/system/rabbitmq-rd/cozyrds/rabbitmq.yaml
  • packages/system/redis-rd/cozyrds/redis.yaml
  • packages/system/vm-disk-rd/cozyrds/vm-disk.yaml
  • pkg/registry/apps/application/rest.go
  • pkg/registry/apps/application/rest_spec_schema_test.go
✅ Files skipped from review due to trivial changes (29)
  • packages/apps/foundationdb/values.yaml
  • packages/apps/harbor/values.yaml
  • packages/apps/rabbitmq/values.yaml
  • packages/apps/kafka/values.yaml
  • packages/apps/http-cache/values.yaml
  • packages/apps/mongodb/README.md
  • api/apps/v1alpha1/harbor/types.go
  • packages/apps/http-cache/README.md
  • packages/apps/rabbitmq/README.md
  • packages/apps/kafka/README.md
  • packages/apps/opensearch/README.md
  • packages/apps/vm-disk/README.md
  • packages/apps/harbor/README.md
  • packages/system/postgres-rd/cozyrds/postgres.yaml
  • packages/apps/redis/README.md
  • docs/storage-immutability.md
  • packages/apps/postgres/README.md
  • packages/apps/openbao/values.yaml
  • packages/apps/clickhouse/README.md
  • packages/apps/nats/README.md
  • packages/apps/redis/values.yaml
  • packages/apps/nats/values.yaml
  • packages/apps/mariadb/values.yaml
  • packages/apps/qdrant/README.md
  • packages/apps/opensearch/values.yaml
  • packages/apps/mongodb/values.yaml
  • packages/apps/kubernetes/README.md
  • packages/apps/foundationdb/README.md
  • packages/apps/qdrant/values.yaml

Comment thread packages/apps/postgres/values.schema.json
Aleksei Sviridkin (lexfrei) added a commit that referenced this pull request May 25, 2026
v1.5.0 adds the @immutable annotation support
(cozystack/cozyvalues-gen#24). Without the bump, pre-commit re-runs
make generate with the v1.4.0 binary, silently strips any
x-kubernetes-validations entries that downstream PRs add to
values.schema.json / embedded openAPISchema, and reports drift against
the committed artefacts.

This is a no-op for the current main: no chart currently uses
@immutable, so regeneration with v1.5.0 produces identical output to
v1.4.0. Unblocks #2639 (apply @immutable to
storageClass across stateful apps).

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Aleksei Sviridkin (lexfrei) added a commit that referenced this pull request May 25, 2026
## Summary

Bump the `cozyvalues-gen` binary pinned in
`.github/workflows/pre-commit.yml` from `v1.4.0` to `v1.5.0`.

`v1.5.0` (cozystack/cozyvalues-gen#24) adds support for the `@immutable`
annotation. Without this bump, pre-commit re-runs `make generate` with
the old `v1.4.0` binary on every PR; the old binary silently ignores `##
@immutable` directives and regenerates `values.schema.json` / embedded
`openAPISchema` without the corresponding `x-kubernetes-validations`
entries — which then shows up as committed-vs-regenerated drift on any
PR that tries to use the new annotation.

## No-op for main today

No chart on main currently uses `@immutable` (zero hits for `##
@immutable` under `packages/apps/*/values.yaml`), so this bump
regenerates byte-identical output. The change is purely lifting the
floor for downstream PRs that will start using the annotation.

## Unblocks

- #2639 — apply `@immutable` to `storageClass` across
every stateful Cozystack app (currently fails pre-commit on the v1.4.0
pin).

## Release info

- v1.5.0 release notes:
https://github.com/cozystack/cozyvalues-gen/releases/tag/v1.5.0
- Tag signed by F20B7CCBF00A5CAE8F9F8B8C7988329FDF395282 (Aleksey
Sviridkin)
- Goreleaser uploaded `cozyvalues-gen-linux-amd64.tar.gz` (the artefact
this workflow downloads) along with the rest of the OS/arch matrix.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Chores**
* Updated development tooling dependencies for build process
improvements.

<!-- review_stack_entry_start -->

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2730?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)

<!-- review_stack_entry_end -->

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@lexfrei
Aleksei Sviridkin (lexfrei) force-pushed the feat/mariadb-storageclass-immutable branch 2 times, most recently from 18dd65d to 9a6313b Compare May 25, 2026 17:11
@lexfrei Aleksei Sviridkin (lexfrei) added area/storage Issues or PRs related to storage (linstor, seaweedfs, bucket, velero, harbor) and removed area/uncategorized PR auto-labeler could not map title scope to a known area/*; please review labels May 25, 2026

@myasnikovdaniil myasnikovdaniil left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Coverage is complete across all 16 stateful apps, the buildSpecSchema refactor is a good call, and the new tests pass. nodeGroups exclusion is correct and the v1.5.0 dependency is already satisfied on main.

Two non-blocking doc nits:

  1. The PR description says kubernetes (top-level + nodeGroups[]), but nodeGroups[].storageClass is intentionally excluded. The "Dependencies" section is also stale — cozyvalues-gen v1.5.0 is already pinned on main and CI is green. Worth syncing the body to match.

Comment thread packages/apps/kubernetes/values.yaml Outdated
## @field {string} instanceType="u1.medium" - Virtual machine instance type.
## @field {quantity} diskSize="20Gi" - Persistent disk size for kubelet and containerd data.
## @field {string} [storageClass] - StorageClass for worker node persistent disks. When empty, uses the management cluster default StorageClass (the one annotated storageclass.kubernetes.io/is-default-class: true).
## @field {string} [storageClass] - StorageClass for worker node persistent disks. When empty, uses the management cluster default StorageClass (the one annotated storageclass.kubernetes.io/is-default-class: true). NOTE: deliberately NOT @immutable — the field is optional and undefaulted, so a strict `self == oldSelf` rule would block any future attempt to set it on an existing node group.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description embeds the literal @immutable token ("deliberately NOT Immutable (@immutable) …"). v1.5.0 parses it correctly today (the generated schema has no rule on this field), but it's fragile against future parser changes. Rewording to "deliberately not marked immutable" sidesteps it.

Annotate storageClass as immutable across every stateful app whose
chart exposes one — clickhouse, foundationdb, harbor, http-cache,
kafka (Kafka + ZooKeeper), kubernetes (top-level only), mariadb,
mongodb, nats, openbao, opensearch, postgres, qdrant, rabbitmq,
redis, vm-disk. Without coverage across the catalogue the UI would
render storageClass read-only on some apps and editable on others —
a worse mental model than uniform behaviour.

The annotation lands at the schema layer (CEL XValidation rule
'self == oldSelf' on values.schema.json / types.go / embedded
openAPISchema in cozyrds/<app>.yaml). The cozystack aggregated
apiserver does not yet evaluate CEL on application-backed resources;
that is tracked at #2657. The UI
(cozystack/cozystack-ui#6) consumes the marker today.

The PVC-pinning argument is identical for every chart: Kubernetes
fixes a PVC's storageClassName at creation time, so editing this
field on an existing resource never migrated data. The schema
annotation makes that contract explicit.

kubernetes.nodeGroups[name].storageClass is intentionally NOT
annotated: the field is optional and undefaulted, so a strict
'self == oldSelf' rule would block any future attempt to set it on
an existing node group. A richer ratcheting form is needed there —
see docs/storage-immutability.md.

A refactor in pkg/registry/apps/application/rest.go extracts the
inline schema-build path into buildSpecSchema so it can be
unit-tested. Three regression tests in rest_spec_schema_test.go pin
the contract that x-kubernetes-validations survives the
v1->internal->structural conversion (loaded from the actual MariaDB
ApplicationDefinition fixture on disk, not a hand-edited mock).

Each per-app README gains a one-line blockquote pointing to a new
docs/storage-immutability.md that explains the contract and which
consumers enforce it today.

Requires cozystack/cozyvalues-gen with @immutable support — bump the
pre-commit pin in .github/workflows/pre-commit.yml once the release
exists.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
@lexfrei
Aleksei Sviridkin (lexfrei) force-pushed the feat/mariadb-storageclass-immutable branch from 9a6313b to 5af5962 Compare May 26, 2026 20:51
@github-actions github-actions Bot added the area/uncategorized PR auto-labeler could not map title scope to a known area/*; please review label May 26, 2026
@lexfrei
Aleksei Sviridkin (lexfrei) merged commit b630706 into main May 27, 2026
10 checks passed
@lexfrei
Aleksei Sviridkin (lexfrei) deleted the feat/mariadb-storageclass-immutable branch May 27, 2026 07:57
myasnikovdaniil added a commit that referenced this pull request Jun 22, 2026
Merge the AI-generated v1.5.0 changelog with a fuller pass:

- Add an "Upgrade Notes and Required Actions" section (K8s 1.33+, removal
  of upgrade.force, GPU permittedHostDevices ownership, MetalLB FRR-K8s /
  HTTPS metrics, automatic TLS on externally-published services).
- Expand "Platform Components" with per-component upstream highlights and
  CVE references (Flux v2.8 CVE-2026-23990, MetalLB v0.16 CVE-2025-22874),
  plus the new frr-k8s subchart and kuberture package.
- Scope the storageClass-immutability entry (#2639) to UI-only enforcement,
  with apiserver enforcement tracked in #2657.
- Broaden Development/Testing/CI coverage and add the v1.4.x website docs
  backports (#565, #566, #567, #580).
- Correct the loop-device LVM-filter attribution to talm#215 (the prior
  ansible-cozystack#51 reference was a dependency bump, not this change).

Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/api Issues or PRs related to the cozystack-api aggregated API server area/database Issues or PRs related to managed databases (postgres, mariadb, redis, etcd, kafka, clickhouse) area/storage Issues or PRs related to storage (linstor, seaweedfs, bucket, velero, harbor) area/uncategorized PR auto-labeler could not map title scope to a known area/*; please review kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/feature Categorizes issue or PR as related to a new feature quality-of-life QoL improvements size/L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants