Repository navigation
feat(api): typed waitStrategy/healthCheckExprs for kstatus readiness (#2642) - #3273
Conversation
📝 WalkthroughWalkthroughAdds typed ChangesHealth-check and wait-strategy flow
OpenAPI model naming
Backup controller schema refresh
Generator and dependency updates
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ApplicationDefinition
participant CozyServerOptions
participant PackageReconciler
participant HelmRelease
ApplicationDefinition->>CozyServerOptions: provide typed release configuration
CozyServerOptions->>PackageReconciler: provide component installation configuration
PackageReconciler->>HelmRelease: set health checks and resolved wait strategy
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
github-actions
Bot
added
area/api
Verification note — the CEL enums this API will carry are grounded in live CRsRead-only checks against a live cluster confirm the convention-based status values the Phase-3 leaves will declare via these fields, with zero corrections needed from what was observed:
Not yet observed live (apps not deployed on the verification cluster): Percona |
Andrei Kvapil (kvaps)
left a comment
Andrei Kvapil (kvaps)
left a comment
There was a problem hiding this comment.
Implementation checked against the API-owner review in #3263. Every point (Q1–Q3, R1–R5) is addressed; the design matches what was approved. Verified locally: go build ./... clean, go vet ./... clean, go test ./pkg/config/... ./internal/operator/... ./pkg/registry/apps/application/... all pass, and make generate at HEAD produces zero drift.
Review points
Q1 — upstream type reused. HealthCheckExprs []kustomize.CustomHealthCheck, i.e. the exact type of the field it maps to (helm-controller/[email protected] v2/helmrelease_types.go:199). Note there is no helmv2.CustomHealthCheck; the correct reference is github.com/fluxcd/pkg/apis/kustomize, correctly promoted indirect → direct in go.mod.
Q2 — placement. spec.release on ApplicationDefinition (api/v1alpha1/applicationdefinitions_types.go:84-101), flat on ComponentInstall next to upgradeCRDs (api/v1alpha1/packagesource_types.go:146-163). As agreed.
Q3 — dependency bump. Isolated step-0 commit touching only go.mod/go.sum. github.com/fluxcd/helm-controller/api tag api/v1.5.1 exists. The apimachinery fork checks out: cozystack/[email protected] head is 763e1451713b, whose parent 72d71eac is exactly the commit behind upstream kubernetes/apimachinery tag v0.35.0, and the single commit on top touches only pkg/runtime/scheme.go — the ConvertToVersion patch. The go.mod pseudo-version matches that commit. The structured-merge-diff/v6 pseudo-version is inherited from helm-controller/[email protected] and [email protected] (confirmed via go mod graph), not hand-picked. The premise holds: the shipped runtime is already ghcr.io/fluxcd/helm-controller:v1.5.0 (internal/fluxinstall/manifests/fluxcd.yaml:8097) and its HelmRelease CRD carries both fields, so this closes a Go-types-vs-runtime skew rather than getting ahead of it.
R1 — waitStrategy shape. Scalar enum kept, and the deliberate simplification is documented on both fields ("maps to HelmReleaseSpec.WaitStrategy.Name — a deliberate scalar simplification of the upstream {name} object"), with the object constructed in config.ResolveWaitStrategy. That is what was asked for.
R2 — coupled default. config.ResolveWaitStrategy is shared by both builders (pkg/registry/apps/application/rest.go:1646, internal/operator/package_reconciler.go:105) and defaults to poller when expressions are present and no strategy is set. Unit-tested across all five combinations, plus mirrored per-builder tests. See the one caveat below.
R3 — disableWait. Explicitly called out as a conscious YAGNI cut in both the PR body and the commit message, including the legacy ≠ disableWait distinction.
R4 — serialization. No YAML round-trip exists: ReleaseConfig is built in-memory at pkg/cmd/server/start.go:252 and reaches json.Marshal only via ResourceConfig at start.go:363, so the kustomize.CustomHealthCheck json tags are honored. Deepcopy is generated and correct — CustomHealthCheck is all-string, so make([]…) + copy is the right shape.
R5 — failure mode. Documented on both fields and carried into the generated CRD descriptions: a bad CEL expression or a not-yet-installed CRD makes the HelmRelease hang until its timeout, which is the backstop.
No behavior change today is accurate: no in-repo ApplicationDefinition or ComponentInstall sets either field, and ResolveWaitStrategy returns nil in that case, so generated HelmReleases are byte-identical. The waitStrategy/healthCheckExprs already present in packages/extra/{seaweedfs,monitoring} are hand-written HelmRelease manifests and are untouched by either builder.
Findings
Finding 1 — disableWait silently defeats healthCheckExprs on the apiserver path
Upstream is explicit (helm-controller/[email protected] v2/helmrelease_types.go:193-199):
The expressions are evaluated only when the specific Helm action taking place has wait enabled, i.e. DisableWait is false, and the 'poller' WaitStrategy is used.
In pkg/registry/apps/application/rest.go, lines 1635-1638 set Install.DisableWait / Upgrade.DisableWait from the release.cozystack.io/helm-install-disable-wait annotation, and lines 1645-1646 then set the expressions plus the poller strategy. An ApplicationDefinition carrying both would emit a HelmRelease whose health expressions are never evaluated, with no error and no warning — the release just reports Ready as before.
This is reachable: packages/system/kubernetes-rd/cozyrds/kubernetes.yaml:28 already sets that annotation, and it is a plausible Phase-3 candidate for CEL expressions. ResolveWaitStrategy couples the poller default precisely to remove a silent-no-op footgun; disableWait is the other half of the same trap and is currently unguarded.
Minimum: state the precedence in the healthCheckExprs field doc and next to HelmInstallDisableWaitAnnotation. Better: log a warning at startup (or reject) when a kind sets both. The operator path is unaffected, since ComponentInstall has no disableWait — which is consistent with the YAGNI cut in R3.
Finding 2 — the step-0 commit leaves codegen drift behind
pkg/generated/openapi/zz_generated.openapi.go (adds observedGeneration to the CRD condition schemas, drops an x-kubernetes-list-type extension) and the four strategy.backups.cozystack.io_* CRDs (description-only PodSpec drift) change purely as a consequence of the k8s 0.35 bump — nothing in them relates to the feature. They currently land in the feat(api) commit, so cf08add3 on its own does not satisfy make generate. .github/workflows/codegen-drift.yml only gates the PR head, so CI stays green, but bisect and any per-commit gate would not. Moving those five generated files into the build(deps) commit keeps the isolated step-0 genuinely isolated and shrinks the feature diff to the feature.
Finding 3 — slice aliasing between the long-lived config and every generated HelmRelease
pkg/registry/apps/application/rest.go:1645 assigns r.releaseConfig.HealthCheckExprs directly, so one backing array is shared by the per-kind ReleaseConfig and every HelmRelease built from it (same pattern at internal/operator/package_reconciler.go:104). Nothing mutates the slice today, so this is not a live bug, but a slices.Clone at the assignment removes the class of failure for one line.
Finding 4 — two doc comments were orphaned by the insert
internal/operator/package_reconciler_test.go:158-164 — the new TestBuildHelmReleaseSpecHealthCheckExprs was inserted between the TestBuildHelmReleaseSpecZeroMaxHistory doc comment and its function, so that comment now documents the wrong test and TestBuildHelmReleaseSpecZeroMaxHistory has none. Same in pkg/registry/apps/application/rest_helmrelease_spec_test.go:131-140, leaving TestConvertApplicationToHelmRelease_ZeroMaxHistory (line 181) undocumented.
Finding 5 — nits
ResolveWaitStrategypasses any non-empty string through toWaitStrategyName. The CRD enum guards both paths in practice, so this is defensive only, but a fallback or validation would make the helper safe on its own.- The commit and PR bodies say the bump cascades "the go directive to 1.26" — the
godirective is unchanged,go.mod:5isgo 1.26.4on bothmainand this branch. - Per
docs/agents/contributing.md:113-123, AI assistance is disclosed with anAssisted-By:trailer. Both commits useCo-authored-by: Claude Opus 4.8 <[email protected]>instead, and the PR body ends with a "Generated with Claude Code" line.Co-authored-by:carries real authorship semantics, so please switch both commits toAssisted-By: Claude <[email protected]>and drop the PR-body footer before this leaves draft.
Verdict
The API surface and the plumbing are right, both builders stay in lockstep, and the tests pin the part worth pinning. Nothing here blocks the design. Before this leaves draft: fix the commit trailers (finding 5), and please address the disableWait precedence (finding 1) — a field-doc sentence now, a real guard before any leaf adopts both. Folding the dep-bump codegen into commit 1 (finding 2) is worth doing while the branch is still rebaseable; findings 3 and 4 are take-them-or-leave-them.
|
Andrei Kvapil (@kvaps) ready for another look — un-drafted, E2E is green now: https://github.com/cozystack/cozystack/actions/runs/29317176726 The helm-controller v1.5 / k8s 0.35 bump surfaced two aggregated-apiserver regressions on top of the original change; both are fixed:
Drift-check and build stayed green throughout — only E2E-with-a-cluster caught these. |
dosubot
Bot
added
the
kind/api-change
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces typed fields for waitStrategy and healthCheckExprs to allow applications to declare kstatus health gates, ensuring that HelmRelease readiness accurately reflects the actual health of rendered custom resources. The changes include a significant dependency stack upgrade to Kubernetes 0.35 and Flux helm-controller 1.5.1, along with necessary updates to CRDs, codegen, and internal reconciliation logic to support these new readiness features. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for Kstatus readiness gates in Helm releases by adding WaitStrategy and HealthCheckExprs to ApplicationDefinition and PackageSource types, allowing releases to report readiness based on CEL health expressions. It also upgrades Kubernetes dependencies to v0.35.0 and resolves Server-Side Apply (SSA) field management issues by defining OpenAPIModelName methods for the apps.cozystack.io types. Feedback on the code changes suggests using pointer receivers for the OpenAPIModelName methods to avoid copying large structs, and notes that the PR description must be updated to include a release-note block as required by the repository style guide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| func (in Application) OpenAPIModelName() string { | ||
| return "com.github.cozystack.cozystack.pkg.apis.apps.v1alpha1.Application" | ||
| } | ||
|
|
||
| func (in ApplicationList) OpenAPIModelName() string { | ||
| return "com.github.cozystack.cozystack.pkg.apis.apps.v1alpha1.ApplicationList" | ||
| } | ||
|
|
||
| func (in ApplicationStatus) OpenAPIModelName() string { | ||
| return "com.github.cozystack.cozystack.pkg.apis.apps.v1alpha1.ApplicationStatus" | ||
| } |
There was a problem hiding this comment.
In Go, it is a standard performance best practice to use pointer receivers for large structs to avoid copying the entire struct on every method call.
Since Application, ApplicationList, and ApplicationStatus are Kubernetes API resource structs that can grow quite large, passing them by value to OpenAPIModelName() will cause a full copy of the struct on each invocation. Using pointer receivers avoids this overhead entirely.
| func (in Application) OpenAPIModelName() string { | |
| return "com.github.cozystack.cozystack.pkg.apis.apps.v1alpha1.Application" | |
| } | |
| func (in ApplicationList) OpenAPIModelName() string { | |
| return "com.github.cozystack.cozystack.pkg.apis.apps.v1alpha1.ApplicationList" | |
| } | |
| func (in ApplicationStatus) OpenAPIModelName() string { | |
| return "com.github.cozystack.cozystack.pkg.apis.apps.v1alpha1.ApplicationStatus" | |
| } | |
| func (in *Application) OpenAPIModelName() string { | |
| return "com.github.cozystack.cozystack.pkg.apis.apps.v1alpha1.Application" | |
| } | |
| func (in *ApplicationList) OpenAPIModelName() string { | |
| return "com.github.cozystack.cozystack.pkg.apis.apps.v1alpha1.ApplicationList" | |
| } | |
| func (in *ApplicationStatus) OpenAPIModelName() string { | |
| return "com.github.cozystack.cozystack.pkg.apis.apps.v1alpha1.ApplicationStatus" | |
| } |
| // WaitStrategy maps to HelmReleaseSpec.WaitStrategy.Name — a deliberate | ||
| // scalar simplification of the upstream {name} object, since there is only | ||
| // one value to set. One of poller|legacy. When healthCheckExprs is set and | ||
| // this is empty, the generated HelmRelease defaults to poller, because | ||
| // healthCheckExprs are only evaluated under the poller wait strategy. | ||
| // +optional | ||
| // +kubebuilder:validation:Enum=poller;legacy | ||
| WaitStrategy string `json:"waitStrategy,omitempty"` |
There was a problem hiding this comment.
According to the repository style guide (lines 51-59), the PR body must contain a release-note block in the following format:
```release-note
type(scope): human-readable changelog entry
Please update the PR description to include this block so that the release automation can parse it correctly.
<details>
<summary>References</summary>
1. PR body must contain a release note block in the specified format. <sup>([link](https://github.com/cozystack/cozystack/blob/main/.gemini/styleguide.md))</sup>
</details>
Andrei Kvapil (kvaps)
left a comment
Andrei Kvapil (kvaps)
left a comment
There was a problem hiding this comment.
Re-reviewed after the latest push (head 84773db). The two new fix commits are good and one of them clears the codegen-drift nit from my last pass, so approving.
New commits — LGTM.
fix(api): regenerate OpenAPI with code-generator v0.35.0— correct. Matching the openapi-gen pin to the 0.35 runtime fixes the aggregated-apiserver crash, andmake generateon this head is now byte-for-byte clean (emptygit status).fix(api): declare OpenAPIModelName for apps types— correct and well-documented. build / vet / targeted tests all green.
Two small follow-ups to tidy before/at merge (not blocking the approval):
- Commit trailers: all four commits still carry
Co-authored-by: Claude Opus 4.8; ourdocs/agents/contributing.md(AI Agent Attribution) asks forAssisted-By: Claude <[email protected]>instead. The PR body also still has theGenerated with Claude Codefooter. Worth normalizing on the squash/rebase. - Doc: the field docs now note that
healthCheckExprsare only evaluated under thepollerstrategy (good). One more sentence — thatdisableWait(e.g. thehelm-install-disable-waitannotation) makes them a silent no-op regardless of strategy — would round it out.
Non-blocking gemini nits. The "use pointer receivers for OpenAPIModelName" suggestion should not be applied — value receivers are required: the generated openapi map calls the method on a non-addressable v1alpha1.Application{} literal, where a pointer-receiver method can't be called (it would break compilation). The "add a release-note block to the PR body" one is valid and easy to add while tidying the body.
The kstatus readiness work (#2642) sets WaitStrategy / kustomize.CustomHealthCheck / HelmReleaseSpec.HealthCheckExprs on generated HelmReleases. Those Go types do not exist at helm-controller/api v1.4.3 — the runtime CRD is already v1.5.0 (shipped by #2602), so this closes a Go-types-vs-runtime skew rather than getting ahead of the runtime. The bump is not isolated: helm-controller/api v1.5.x requires k8s 0.35 + controller-runtime 0.23, cascading the k8s stack 0.34 -> 0.35. cozystack replaces k8s.io/apimachinery with a fork carrying the (still-open) upstream ConvertToVersion patch k/k#135537; that single commit is rebased cleanly onto apimachinery v0.35.0 at cozystack/[email protected]. The repo builds with no source changes under the new stack. Refs #2642 Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
Add first-class fields so a package/app can declare the Flux helm-controller v1.5.0 kstatus health gate for the custom resource it wraps, so HelmRelease.Ready means "the CR is actually healthy" instead of "helm applied the CR" (#2642). API-owner approved in #3263. - api: WaitStrategy (scalar, maps to HelmReleaseSpec.WaitStrategy.Name — a deliberate simplification) + HealthCheckExprs ([]kustomize.CustomHealthCheck, the upstream type verbatim) on both ApplicationDefinition.spec.release (the apiserver path) and ComponentInstall (the operator path). - config.ResolveWaitStrategy: shared by both HelmRelease builders. Couples the default — healthCheckExprs imply the poller strategy when none is set, since exprs are only evaluated under poller. This removes the "must also set waitStrategy: poller" footgun and does not depend on the controller's global default. - thread through buildHelmReleaseSpec (operator) and convertApplicationToHelmRelease (apiserver); the apiserver loads the typed spec.release fields in start.go. - regenerate deepcopy, the ApplicationDefinition and PackageSource CRDs, and openapi. The backupstrategy CRDs also regenerate — benign k8s-0.35 PodSpec description drift, not a functional change. disableWait is intentionally not added (YAGNI): the only known need (the kubernetes tenant-addon deadlock) was resolved child-side, and waitStrategy: legacy is not equivalent (legacy still waits Helm-v3-style; disableWait skips waiting entirely). Add it later when a concrete leaf needs a true no-gate escape hatch. Refs #2642 Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
This PR bumps the runtime k8s libraries to 0.35 (k8s.io/apiserver,
kube-openapi 20250910) but hack/update-codegen.sh still pinned the
openapi code-generator at v0.34.1.
The 0.34.1 openapi-gen keys the version.Info definition by its Go
import path ("k8s.io/apimachinery/pkg/version.Info"), whereas
kube-openapi 0.35 now resolves that model via the new OpenAPIModelNamer
interface — version.Info{}.OpenAPIModelName() ->
"io.k8s.apimachinery.pkg.version.Info". The key mismatch makes the
aggregated apiserver fatal while building its root OpenAPI spec
("cannot find model definition for io.k8s.apimachinery.pkg.version.Info"
-> klog.Fatalf, exit 255), crashlooping cozystack-api. That takes down
the apps.cozystack.io aggregated API, so the Bucket kind can no longer
be resolved and the backupstrategy-controller HelmRelease install fails,
reddening E2E.
Bump the code-generator pin to v0.35.0 (matching the runtime) and commit
the regenerated artifacts. No hand edits to generated files.
Assisted-By: Claude <[email protected]>
Signed-off-by: Myasnikov Daniil <[email protected]>
…alog kinds After the k8s 0.35 bump, server-side apply of any apps.cozystack.io kind (e.g. Bucket) fails at field management with "no corresponding type for apps.cozystack.io/v1alpha1, Kind=Bucket", blocking backupstrategy-controller and the whole install. The apps API multiplexes every catalog kind (Bucket, Postgres, …) onto the single Application Go type via Scheme.AddKnownTypeWithName. The SSA field manager attaches a kind's GroupVersionKind to the Application OpenAPI model only when the model's name matches the name the apiserver's DefinitionNamer computed for it. Since Kubernetes 0.35 the DefinitionNamer keys that lookup by Scheme.ToOpenAPIDefinitionName (the "friendly" reversed-path form, com.github.cozystack…Application) while the model builder still uses the Go import-path form. With no OpenAPIModelName the two disagree, the GVK extension is never attached, and the type converter ends up with no apps kinds at all. Declare OpenAPIModelName on Application/ApplicationList/ApplicationStatus so GetCanonicalTypeName, ToOpenAPIDefinitionName and the generated openapi map key all agree on the friendly form. This is done via the +k8s:openapi-model-package package tag plus hand-written methods rather than code-generator's --output-model-name-file flag, because that flag also rewrites zz_generated.model_name.go inside the read-only apimachinery / apiextensions module-cache packages the shared gen_openapi helper always passes as inputs, which fails on any consumer (including CI) that vendors deps from the cache. Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
The field docs already say healthCheckExprs are only evaluated under the poller wait strategy, but that is only half the precedence rule. Upstream evaluates them only when the Helm action itself has wait enabled (helm-controller/[email protected] v2/helmrelease_types.go:193-199), so the release.cozystack.io/helm-install-disable-wait annotation makes them a silent no-op whatever waitStrategy says — no error, no warning, the release just reports Ready as before. This is reachable today: packages/system/kubernetes-rd already sets that annotation and is a plausible candidate for CEL expressions in phase 3. Document it on the ApplicationDefinition field (so it reaches the CRD description) and next to HelmInstallDisableWaitAnnotation. ComponentInstall has no disableWait, so its field doc is unchanged. Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
84773db to
f161921
Compare
## What this PR does Fixes #3806. Declares `OpenAPIModelName` for the `core` and `sdn` API groups the way #3273 already did for `apps`, so every published OpenAPI definition name is the dotted Kubernetes model name rather than a Go import path. Worth noting the two are the same event. #3273 landed the k8s 0.35 work, hit the loud half of this in server-side apply where no apps kind resolved at all, fixed `apps`, and left `core` and `sdn` with the quieter half. Its merge commit `960d85370` is exactly where the window opens: 190 failed-E2E logs from 2026-06-25 to 07-19 carry no occurrence, and the first one is 11 hours after it. Before this, 20 definitions shipped under names like `github.com/cozystack/cozystack/pkg/apis/core/v1alpha1.Option`, while every `$ref` pointing at them spelled each slash `~1`. Clients resolve a `$ref` by trimming `#/definitions/` and looking the remainder up verbatim with no unescaping, so the two spellings never meet and the reference dangles. `kubectl apply --validate` against a cluster running a `cozystack-api` built after 2026-07-21 therefore fails on any resource, and in e2e the platform install fails on `cozy-backup-controller/backupstrategy-controller` and never completes. The mechanism is a Kubernetes 0.35 change: `DefinitionNamer.GetDefinitionName` now returns the model name verbatim, where it previously converted the Go import-path form into the friendly reversed-path form. So whatever name the generated map uses becomes both the published key and the escaped `$ref`, and slash-freedom stops being a nice-to-have and becomes the correctness condition. ### Why hand-written methods openapi-gen can emit these with `--output-model-name-file`, but that also rewrites `zz_generated.model_name.go` inside the read-only apimachinery and apiextensions module-cache packages that the shared `gen_openapi` helper always passes as inputs, which fails for any consumer that vendors deps from the cache, CI included. The `apps` group settled this the same way and the reasoning is repeated in each new `model_name.go` so nobody simplifies it back into the flag. Once a package carries `+k8s:openapi-model-package`, openapi-gen emits `Type{}.OpenAPIModelName()` for every type it writes a schema or a `$ref` for, so a missing method is a build failure rather than a silent skip. That makes a future omission inside these groups loud by construction, and the new tests make an omission on a future group loud at the group level. ### Guard tests `pkg/generated/openapi/definitions_test.go` adds two, both asserting the invariant rather than today's 20 names, and both refusing to pass on an empty or cozystack-free definition map so they cannot go vacuously green. `TestDefinitionNamesAreDottedModelNames` fails on any definition name containing a slash, naming the offender and the fix. `TestDefinitionRefsResolve` builds each `$ref` the way kube-openapi's builder does and resolves it the way a client does, which checks the failure directly rather than through the slash-freedom proxy, and also catches a group whose types disagree with each other. Nothing asserted this before, which is why the defect reached v1.6.0 and v1.6.1 unnoticed. ### Screenshots Not applicable. ### Downstream repositories Walked the trigger map against the diff. Published model names change, so I swept for consumers pinning the old form: every hit on `github.com/cozystack/cozystack/pkg/apis` in the tree is a Go import, which is unaffected because the package path does not change, and `api/api-rules/cozystack_api_violation_exceptions.list` keys off the Go package path in its own format and was unchanged by regeneration. No string literal anywhere pins the old model-name form, and `apiPrefix` in `pkg/cmd/server/openapi.go` is an apps-only const that this does not touch. - [x] No downstream repository is affected by this change ### Testing - `go build ./... && go vet ./...` - `go test ./...` - `make generate && git diff --exit-code`, clean and idempotent - `pre-commit run --all-files` - Both guard tests run against the pre-fix tree first: `TestDefinitionNamesAreDottedModelNames` named all 20 offenders and `TestDefinitionRefsResolve` reproduced the reported error string with 15 dangling refs, fewer than 20 because some of the 20 are only referenced-from and never referencing. - An independent check driving the real `openapi.NewDefinitionNamer(apiserver.Scheme)` reports 104 published definitions with 0 slashes, 106 emitted refs with 0 dangling, and all 23 cozystack kinds resolving to a published definition carrying `x-kubernetes-group-version-kind`, which also confirms the `apps` server-side-apply fix still holds. `make unit-tests` aborts at `hack/ghcr-mirror_test.bats`, which fails on a pristine `origin/main` in my environment while passing in CI, so I ran the chain standalone: the other 60 bats files pass, as do `helm-unit-tests`, `go-unit-tests`, `rd-presets-check`, `test-check-readiness` and `migrations-target-check`. ### Release note ```release-note fix(api): publish OpenAPI model names for the core and sdn API groups as dotted Kubernetes names, so `$ref`s resolve and client-side validation works again against the aggregated apiserver ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Enhancements** * Added OpenAPI metadata for Core v1alpha1 API models. * Added OpenAPI metadata for SDN v1alpha1 API models. * API schemas now provide consistent canonical model names, supporting more reliable documentation and schema references. * Added package-level OpenAPI markers to improve API discovery and tooling compatibility. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
What
Adds typed
waitStrategy+healthCheckExprsfields so a package/app can declare the Flux helm-controller v1.5.0 kstatus health gate for the custom resource it wraps — makingHelmRelease.Readymean "the CR is actually healthy" instead of "helm applied the CR". This is the operator/apiserver foundation of the kstatus epic (#2642); the API surface was reviewed and approved by the API owner in #3263.Commits (per the review's "isolated step-0" request):
build(deps)— the dependency bump (see below).feat(api)— the fields, plumbing, tests, and regenerated code.fix(api)— match the openapi code-generator pin to the 0.35 runtime (fixes an aggregated-apiserver crash the bump surfaced).fix(api)— declareOpenAPIModelNameon the apps types so SSA resolves catalog kinds.docs(api)— note thatdisableWaitsilently defeatshealthCheckExprs.API surface
Typed fields on both HelmRelease-building paths:
ApplicationDefinition.spec.release(apiserver / catalog apps →convertApplicationToHelmRelease)ComponentInstall(operator / platform packages →buildHelmReleaseSpec)config.ResolveWaitStrategyis shared by both builders and couples the default:healthCheckExprsare only evaluated under thepollerstrategy, so when expressions are set and no strategy is given, the generated HelmRelease defaults topoller. A package that sets onlyhealthCheckExprsis therefore self-contained, independent of the controller's global default.The dependency bump (commit 1)
The
WaitStrategy/kustomize.CustomHealthCheck/HelmReleaseSpec.HealthCheckExprstypes do not exist belowhelm-controller/apiv1.5.0 — the runtime is already v1.5.0 (shipped by #2602), so this closes a Go-types-vs-runtime skew. It is not an isolated bump: v1.5.x requires k8s 0.35 + controller-runtime 0.23, cascading the k8s stack 0.34 → 0.35 (and the go directive to 1.26).cozystack replaces
k8s.io/apimachinerywith a fork carrying the still-open upstreamConvertToVersionpatch (k/k#135537). That single commit rebases cleanly onto apimachinery v0.35.0; published atcozystack/[email protected], andgo.modpins its pseudo-version. The repo builds with zero source changes under the new stack.Not included (conscious YAGNI)
disableWaitis intentionally omitted. The only known need (the kubernetes tenant-addon deadlock) was resolved child-side, andwaitStrategy: legacyis not equivalent (legacy still waits Helm-v3-style;disableWaitskips waiting entirely). Add it later when a concrete leaf needs a true no-gate escape hatch.Tests
config.TestResolveWaitStrategy— the couple-the-default logic (unset+exprs→poller; explicit legacy honored; unset+no-exprs→nil).TestBuildHelmReleaseSpecHealthCheckExprs/TestConvertApplicationToHelmRelease_HealthCheckExprs).go build ./...clean,go vetclean,gofmtclean, codegen regenerated and committed.The 4 backupstrategy CRDs regenerate with benign k8s-0.35 PodSpec description drift (DynamicResourceAllocation alpha→stable, toleration Lt/Gt, resizePolicy note) — no functional change.
Follow-up
Phase 3 (per-leaf adoption — postgres/kafka/mongodb/etc. declaring their CEL via these fields) builds on this. The full CRD→CEL catalog is maintained in the epic's readiness audit.
Refs #2642
Summary by CodeRabbit
New Features
waitStrategysupport (pollerandlegacy) with automaticpollerselection when health checks are configured and no strategy is set.Documentation
healthCheckExprsandwaitStrategy.