Repository navigation
fix(keycloak, foundationdb): apply operator CRD updates on upgrade - #4946
Open
europrinter (yankawai) wants to merge 2 commits into
Open
europrinter (yankawai) wants to merge 2 commits into
europrinter (yankawai) wants to merge 2 commits into
Conversation
keycloak-operator ships its CRDs in the chart's crds/ directory and its PackageSource sets no upgradeCRDs policy, so helm-controller never replaces them. A cluster installed before v1.2.0 therefore runs the v1.32.0 operator against the v1.25.0 CRDs, whose ClusterKeycloakRealm schema has no spec.sessions, spec.login or spec.organizationsEnabled. The apiserver drops those fields from the realm keycloak-configure renders, so the operator never applies the session limits the platform sets. On a v1.6.4 cluster in that state Keycloak also rejected the dashboard client with HTTP 400, and replacing the CRDs from the chart by hand, with an operator restart, cleared it. The v1.32.0 CRDs are not purely additive. They drop the realm, targetRealm and keycloakOwner fields that v1.25.0 marked deprecated and require realmRef and keycloakRef instead. Every Keycloak object the platform renders already uses the refs, and each CRD keeps its single version, so replacing them loses nothing the platform relies on. helm-controller applies the policy only when it upgrades the release, and it runs an upgrade only when the release artifact or its values change, which a PackageSource edit does not do. The package test added here also changes the keycloak-operator artifact, so the replace runs on the upgrade that ships this change even where nothing else in the package changes. The test itself pins the realm fields keycloak-configure sets against the CRD the chart ships. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
The foundationdb-operator PackageSource sets no upgradeCRDs policy, so a cluster installed before v1.6.0 runs the v2.30.0 operator on the v2.13.0 CRDs it first installed. The apiserver prunes every field added since, status.backupDetails.restorable on FoundationDBBackup among them, so a backup never shows as restorable however long it has run. The v2.30.0 CRDs only add fields. They keep the single v1beta2 version and drop or retype nothing, and the one new constraint, a pattern on perpetual_storage_wiggle_locality, sits on a field the platform does not set, so CreateReplace is safe. Nothing in the foundationdb-operator package changes here, so a cluster picks the new CRDs up at the next upgrade of that release, which comes with the next change to the package. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
github-actions
Bot
added
area/database
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely describes the main change: enabling operator CRD updates during upgrades for Keycloak and FoundationDB. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (6 skipped: 6 unsupported.)
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Comment @coderabbitai help to get the list of available commands.
IvanHunters
approved these changes
Oct 23, 2026
IvanHunters
left a comment
IvanHunters
left a comment
Collaborator
There was a problem hiding this comment.
The mechanism is install.upgradeCRDs: CreateReplace on the PackageSource, applied by helm-controller itself, so there is no custom CRD-applying Job and no RBAC gap. hookpolicy found nothing in the two touched charts. Verified with the Go tests (TestParseCRDPolicy/CreateReplace_is_passed_through, TestPackageSourceCRDHasUpgradeCRDsEnum), a mutation that removes upgradeCRDs (both source suites then fail), and helm-unittest 1.1.2 (full platform suite 287/287). No blocking issues.
Non-blocking:
packages/core/platform/sources/keycloak-operator.yaml:29: the keycloak CRD replace is non-additive (v1.32.0 dropsrealm/targetRealm/keycloakOwner), andCreateReplacecan prune data from live objects using those fields. Mitigated: all 14 CRDs keep a single version (no stored-version drop) and the platform renders onlyrealmRef/keycloakRef, so residual risk is a hand-authored CR on a deprecated field. Documented in the PR.- The fix taking effect on this upgrade relies on the
tests/plus Makefile change flipping the keycloak-operator artifact digest. Plausible (content-addressed source-watcher) but not verifiable offline; worst case is a one-release delay, not a regression.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
The generated HelmReleases leave
spec.upgrade.crdsunset unless the PackageSource setsupgradeCRDs, so helm-controller never replaces a CRD that a chart ships incrds/. keycloak-operator and foundationdb-operator ship their CRDs that way and set no policy. On a v1.6.4 cluster that came through earlier releases, the CRDs of both operators were older than the ones in the v1.6.4 charts. The v1.25.0 ClusterKeycloakRealm schema has nospec.sessions,spec.loginorspec.organizationsEnabled, so the apiserver dropped the session settings keycloak-configure sets and the operator never applied them to the realm. Keycloak also rejected the dashboard KeycloakClient with HTTP 400 on that cluster, and replacing the 17 CRDs from the charts and restarting keycloak-operator cleared it.This opts both PackageSources into
upgradeCRDs: CreateReplace, the policy mongodb-operator, redis-operator and velero already use, and pins it with a platform test for each. The keycloak-operator change is not purely additive. v1.32.0 drops therealm,targetRealmandkeycloakOwnerfields that v1.25.0 marked deprecated and requiresrealmRefandkeycloakRefinstead. Every Keycloak object the platform renders already uses the refs, and each of the 14 CRDs keeps its single version, so the replace loses nothing the platform relies on and drops no stored version. A hand-made object that still uses a deprecated field would lose it on its next write, and that write needs the ref. The foundationdb-operator CRDs only add fields from v2.13.0 to v2.30.0, plus a pattern onperpetual_storage_wiggle_locality, which the platform does not set.helm-controller applies the policy only when it upgrades a release, and it upgrades only when the release artifact or its values change. A PackageSource edit changes neither. The PR therefore also gives the keycloak-operator package a helm-unittest suite, which pins the realm fields keycloak-configure sets against the ClusterKeycloakRealm CRD the chart ships and changes the keycloak-operator artifact, so the CRDs are replaced on the upgrade that brings this change even where nothing else in the package changes, as on a backport to release-1.6. The platform release writes the new PackageSource in the same apply that moves
cozystack-packagesto the new revision, so the policy should reach the HelmRelease before the new keycloak-operator artifact does. The foundationdb-operator package does not change here, so its CRDs are replaced at the next change to that package, and main already carries one since v1.6.5.Objects written while the CRDs were stale get the pruned fields back only when their chart applies them again. For the cozy realm that is the next keycloak-configure upgrade. On main it comes with the same platform upgrade, since keycloak-configure changed after v1.6.5, and its HelmRelease depends on the keycloak-operator one through the oidc variant. On a branch where keycloak-configure does not change, the realm keeps its current session limits until it does or until that release is forced to upgrade.
#4901 adds the same
upgradeCRDs: CreateReplaceto the foundationdb-operator PackageSource as part of the FoundationDB backup work, and it changes the foundationdb-operator package as well. The foundationdb commit here is separate from the keycloak one and does not depend on it, so it can be dropped if #4901 lands first.The other packages that ship CRDs only in
crds/and set no policy are bootbox, clickhouse-operator, clustersecret-operator, cozystack-scheduler, external-dns, grafana-operator, kafka-operator and snapshot-controller. None of their CRDs changed since v1.0.0, or since the package was added, apart from description text in external-dns, so no cluster is behind on them today. kafka-operator gets CreateReplace with the Strimzi move in #4900. gpu-operator replaces its CRDs with its own pre-upgrade Job, and the remaining packages render their CRDs from templates or leave them to an operator. Each of the eight will need the same review and opt-in the next time its CRDs change.make -C packages/core/platform testpasses with 58 suites and 287 tests, andmake -C packages/system/keycloak-operator testpasses. With either PackageSource reverted to main, its suite fails withunknown path spec.variants[0].components[0].install.upgradeCRDs. With the v1.25.0 ClusterKeycloakRealm CRD in the chart, the keycloak-operator suite fails all four assertions. The fullhack/helm-unit-tests.shsweep passes.Screenshots
Not a UI change.
Downstream repositories
Two PackageSources under
packages/core/platform/sources/gain an install option, three test suites are new, andpackages/system/keycloak-operator/Makefilegains the sametesttarget other packages have. Platformvalues.yaml, variants, bundles, CRDs,hack/and the installer are untouched. The website'sdevelopment.mddoes not describe per-package test targets, so the Makefile change needs nothing there, and no website page documents a manual CRD replace for these operators.Release note
Summary by CodeRabbit