From 104b3b3d2b1c25cc06a86669238cad3b744c86d3 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Mon, 20 Apr 2026 18:43:02 +0300 Subject: [PATCH 1/4] feat(operator): add per-package upgradeCRDs policy for HelmRelease Add an opt-in UpgradeCRDs field to ComponentInstall that maps to HelmRelease.Spec.Upgrade.CRDs, allowing a PackageSource component to declare how Flux should handle CRDs from the chart's crds/ directory on upgrade. The helm-controller default on upgrade is Skip, which means new CRDs added between chart versions never reach existing clusters and must be applied manually. Setting upgradeCRDs: CreateReplace makes Flux apply new CRDs declaratively with the chart. Allowed values are restricted to Skip, Create, CreateReplace via a kubebuilder enum marker. Empty / unset preserves the existing Flux default, so all existing PackageSource resources keep working. Assisted-By: Claude Signed-off-by: Aleksei Sviridkin --- api/v1alpha1/packagesource_types.go | 10 ++ .../cozystack.io_packagesources.yaml | 13 ++ internal/operator/package_reconciler.go | 11 ++ internal/operator/package_reconciler_test.go | 138 ++++++++++++++++++ 4 files changed, 172 insertions(+) create mode 100644 internal/operator/package_reconciler_test.go diff --git a/api/v1alpha1/packagesource_types.go b/api/v1alpha1/packagesource_types.go index c0b943e8..75bf217b 100644 --- a/api/v1alpha1/packagesource_types.go +++ b/api/v1alpha1/packagesource_types.go @@ -132,6 +132,16 @@ type ComponentInstall struct { // DependsOn is a list of component names that must be installed before this component // +optional DependsOn []string `json:"dependsOn,omitempty"` + + // UpgradeCRDs controls how CRDs from the chart's crds/ directory are + // handled on HelmRelease upgrades. Maps to HelmRelease.Spec.Upgrade.CRDs. + // Empty string (default) preserves the helm-controller default (Skip). + // Use "CreateReplace" for operators that evolve their CRD set between + // versions. Warning: CreateReplace overwrites CRDs and may cause data + // loss if upstream drops fields from a CRD with live objects. + // +optional + // +kubebuilder:validation:Enum=Skip;Create;CreateReplace + UpgradeCRDs string `json:"upgradeCRDs,omitempty"` } // Component defines a single Helm release component within a package source diff --git a/internal/crdinstall/manifests/cozystack.io_packagesources.yaml b/internal/crdinstall/manifests/cozystack.io_packagesources.yaml index 0acfcdd2..1d0037df 100644 --- a/internal/crdinstall/manifests/cozystack.io_packagesources.yaml +++ b/internal/crdinstall/manifests/cozystack.io_packagesources.yaml @@ -118,6 +118,19 @@ spec: ReleaseName is the name of the HelmRelease resource that will be created If not specified, defaults to the component Name field type: string + upgradeCRDs: + description: |- + UpgradeCRDs controls how CRDs from the chart's crds/ directory are + handled on HelmRelease upgrades. Maps to HelmRelease.Spec.Upgrade.CRDs. + Empty string (default) preserves the helm-controller default (Skip). + Use "CreateReplace" for operators that evolve their CRD set between + versions. Warning: CreateReplace overwrites CRDs and may cause data + loss if upstream drops fields from a CRD with live objects. + enum: + - Skip + - Create + - CreateReplace + type: string type: object libraries: description: |- diff --git a/internal/operator/package_reconciler.go b/internal/operator/package_reconciler.go index 32e667e4..0e724e49 100644 --- a/internal/operator/package_reconciler.go +++ b/internal/operator/package_reconciler.go @@ -45,6 +45,16 @@ const ( SecretCozystackValues = "cozystack-values" ) +// parseCRDPolicy maps ComponentInstall.UpgradeCRDs to a helmv2.CRDsPolicy. +// Empty / nil preserves the helm-controller default (Skip on upgrade); +// the CRD enum marker restricts the string to Skip/Create/CreateReplace. +func parseCRDPolicy(install *cozyv1alpha1.ComponentInstall) helmv2.CRDsPolicy { + if install == nil || install.UpgradeCRDs == "" { + return "" + } + return helmv2.CRDsPolicy(install.UpgradeCRDs) +} + // PackageReconciler reconciles Package resources type PackageReconciler struct { client.Client @@ -221,6 +231,7 @@ func (r *PackageReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct Remediation: &helmv2.UpgradeRemediation{ Retries: -1, }, + CRDs: parseCRDPolicy(component.Install), }, }, } diff --git a/internal/operator/package_reconciler_test.go b/internal/operator/package_reconciler_test.go new file mode 100644 index 00000000..f0ee2c19 --- /dev/null +++ b/internal/operator/package_reconciler_test.go @@ -0,0 +1,138 @@ +/* +Copyright 2025 The Cozystack Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package operator + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + cozyv1alpha1 "github.com/cozystack/cozystack/api/v1alpha1" + helmv2 "github.com/fluxcd/helm-controller/api/v2" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "sigs.k8s.io/yaml" +) + +func TestParseCRDPolicy(t *testing.T) { + tests := []struct { + name string + install *cozyv1alpha1.ComponentInstall + want helmv2.CRDsPolicy + }{ + { + name: "nil install leaves flux default", + install: nil, + want: "", + }, + { + name: "empty upgradeCRDs leaves flux default", + install: &cozyv1alpha1.ComponentInstall{}, + want: "", + }, + { + name: "Skip is passed through", + install: &cozyv1alpha1.ComponentInstall{UpgradeCRDs: "Skip"}, + want: helmv2.Skip, + }, + { + name: "Create is passed through", + install: &cozyv1alpha1.ComponentInstall{UpgradeCRDs: "Create"}, + want: helmv2.Create, + }, + { + name: "CreateReplace is passed through", + install: &cozyv1alpha1.ComponentInstall{UpgradeCRDs: "CreateReplace"}, + want: helmv2.CreateReplace, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := parseCRDPolicy(tc.install) + if got != tc.want { + t.Errorf("parseCRDPolicy() = %q, want %q", got, tc.want) + } + }) + } +} + +// TestPackageSourceCRDHasUpgradeCRDsEnum guards the generated CRD schema: the +// invalid-value case from the spec is enforced at the API server via a +// kubebuilder enum marker, not in the reconciler. If someone drops the marker +// and forgets to regenerate, this test catches it. +func TestPackageSourceCRDHasUpgradeCRDsEnum(t *testing.T) { + path := filepath.Join("..", "crdinstall", "manifests", "cozystack.io_packagesources.yaml") + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + + var crd apiextensionsv1.CustomResourceDefinition + if err := yaml.Unmarshal(data, &crd); err != nil { + t.Fatalf("unmarshal CRD: %v", err) + } + + var field *apiextensionsv1.JSONSchemaProps + for i := range crd.Spec.Versions { + v := &crd.Spec.Versions[i] + if v.Schema == nil || v.Schema.OpenAPIV3Schema == nil { + continue + } + spec, ok := v.Schema.OpenAPIV3Schema.Properties["spec"] + if !ok { + continue + } + variants, ok := spec.Properties["variants"] + if !ok || variants.Items == nil || variants.Items.Schema == nil { + continue + } + components, ok := variants.Items.Schema.Properties["components"] + if !ok || components.Items == nil || components.Items.Schema == nil { + continue + } + install, ok := components.Items.Schema.Properties["install"] + if !ok { + continue + } + f, ok := install.Properties["upgradeCRDs"] + if !ok { + continue + } + field = &f + break + } + + if field == nil { + t.Fatal("upgradeCRDs field not found in PackageSource CRD schema") + } + + got := map[string]bool{} + for _, e := range field.Enum { + var s string + if err := json.Unmarshal(e.Raw, &s); err != nil { + t.Fatalf("unmarshal enum value %q: %v", e.Raw, err) + } + got[s] = true + } + + for _, want := range []string{"Skip", "Create", "CreateReplace"} { + if !got[want] { + t.Errorf("enum value %q missing from upgradeCRDs; got %v", want, got) + } + } +} From d86bc7760a77c93c6fd837ef5e23c3df3b9b5898 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Mon, 20 Apr 2026 18:43:18 +0300 Subject: [PATCH 2/4] docs(agents): document PackageSource upgradeCRDs field Describe when to set upgradeCRDs: CreateReplace (operators that evolve their CRD set additively between versions) and the data-loss risk of enabling it on operators that drop fields. Assisted-By: Claude Signed-off-by: Aleksei Sviridkin --- docs/agents/overview.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/agents/overview.md b/docs/agents/overview.md index 35798961..ae0fd1c4 100644 --- a/docs/agents/overview.md +++ b/docs/agents/overview.md @@ -83,6 +83,14 @@ packages/// - Reference PR numbers when available - Keep commits atomic and focused +### PackageSource CRD upgrade policy + +Each component in a `PackageSource` may set `install.upgradeCRDs` to control how CRDs from the chart's `crds/` directory are handled on `HelmRelease` upgrades. Allowed values: `Skip` (default — helm-controller does not touch CRDs on upgrade), `Create` (create new CRDs only), `CreateReplace` (create new and overwrite existing). + +Set `upgradeCRDs: CreateReplace` for operators whose upstream regularly adds new CRDs between versions (etcd-operator, cnpg, kubevirt, kamaji). Without it, new CRDs from a chart bump do not land on existing clusters — only fresh installs get them. + +Do **not** set `CreateReplace` blindly: it overwrites every CRD in `crds/` and can cause silent data loss if upstream drops a field from a CRD that has live objects. Only enable it for operators whose schema evolution is additive-only. When in doubt, leave it unset and apply new CRDs manually. + ### Documentation Documentation is organized as follows: From f527ce683b28401c62cdc420188392b32367401f Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Mon, 20 Apr 2026 19:08:11 +0300 Subject: [PATCH 3/4] docs(agents): clarify that Scopes list is illustrative The Scopes section was read as an exhaustive enumeration, which led to review feedback flagging any scope outside the list as invalid. The intent has always been that contributors pick the most specific scope for the change and extend the list when a genuinely new area appears. Reword the section accordingly and add operator as an example scope. Assisted-By: Claude Signed-off-by: Aleksei Sviridkin --- docs/agents/contributing.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/agents/contributing.md b/docs/agents/contributing.md index 658dab74..9f3ff779 100644 --- a/docs/agents/contributing.md +++ b/docs/agents/contributing.md @@ -43,10 +43,11 @@ git commit --signoff -m "type(scope): brief description" **Types:** `feat`, `fix`, `docs`, `style`, `refactor`, `perf`, `test`, `build`, `ci`, `chore` -**Scopes** (e.g., not exhaustive — use any scope that names the component you are touching): -- System: `dashboard`, `platform`, `cilium`, `kube-ovn`, `linstor`, `fluxcd`, `cluster-api` -- Apps: `postgres`, `mariadb`, `redis`, `kafka`, `clickhouse`, `virtual-machine`, `kubernetes` -- Other: `api`, `hack`, `tests`, `ci`, `docs`, `agents`, `maintenance` +**Scopes** (examples — not an exhaustive list; pick the most specific scope that describes the change, and introduce a new one if a genuinely new area needs its own): + +- System, e.g.: `dashboard`, `platform`, `operator`, `cilium`, `kube-ovn`, `linstor`, `fluxcd`, `cluster-api` +- Apps, e.g.: `postgres`, `mariadb`, `redis`, `kafka`, `clickhouse`, `virtual-machine`, `kubernetes` +- Other, e.g.: `api`, `hack`, `tests`, `ci`, `docs`, `agents`, `maintenance` Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer. From 32ae993d3ebb51a34ece2b6bbe8e6626c72ac300 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Mon, 20 Apr 2026 19:13:06 +0300 Subject: [PATCH 4/4] docs(maintenance): mirror illustrative-scopes wording in PR template Match the wording adopted in docs/agents/contributing.md so that human contributors and AI agents see the same guidance in both places. Assisted-By: Claude Signed-off-by: Aleksei Sviridkin --- .github/PULL_REQUEST_TEMPLATE.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index b9475846..044dd6a0 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,9 +1,10 @@