Repository navigation
apps.cozystack.io aggregated API ignores dryRun: kubectl --dry-run=server persists writes #3982
Description
Summary
kubectl --dry-run=server against the aggregated apps.cozystack.io/v1alpha1 API persists the change. The apps registry (pkg/registry/apps/application/rest.go) never inspects options.DryRun on Create, Update or Delete, so a request that the client explicitly marked as a simulation is executed for real. The same is true on main today (no DryRun reference in the file).
Environment
Cozystack v1.6.1 (cozystack-api:v1.6.1-rc.1), Kubernetes v1.34.3, Talos v1.13.6. Observed 2026-08-28 during a security review of a lab cluster.
Reproduction
$ kubectl -n <tenant-ns> annotate vminstances.apps.cozystack.io <vm> probe.example.org/dry-run=marker --dry-run=server
vminstance.apps.cozystack.io/<vm> annotated (server dry run)
$ kubectl -n <tenant-ns> get vminstances.apps.cozystack.io <vm> -o jsonpath='{.metadata.annotations}'
{"probe.example.org/dry-run":"marker"} # persisted
Done twice with previously unseen annotation keys, read back with a different kubeconfig: the write reaches etcd every time. (Cleaned up afterwards with kubectl annotate ... probe.example.org/dry-run-.)
Expected
Per the Kubernetes API conventions, a custom rest.Storage implementation must honour DryRun itself: when dryrun.IsDryRun(options.DryRun) is true, run admission/validation and return the object that would be written without persisting anything. Delete is the worrying case — by the same code path a kubectl delete ... --dry-run=server on a VMInstance/VMDisk would delete the underlying HelmRelease for real (inferred from the code, deliberately not reproduced).
Impact
Anyone relying on --dry-run=server to preview a change on any apps.cozystack.io kind (VMInstance, VMDisk, Bucket, Postgres, ...) applies it instead, without any warning. It also breaks tooling that uses server-side dry-run for validation (e.g. kubectl diff, GitOps pre-flight checks, policy engines). No authorization boundary is crossed — the caller must already have write rights — but the semantics of a standard, documented client flag are violated.
Suggested fix
In pkg/registry/apps/application/rest.go, check dryrun.IsDryRun(options.DryRun) in Create, Update and Delete (and Patch if implemented separately): validate and build the HelmRelease as today, but return before any call to the underlying client. A conformance test asserting that a dry-run request leaves the backing HelmRelease untouched would prevent regressions.
Activity
dosubot commentedon Aug 28, 2026
Hi lf (@lfinmauritius)! I'm Dosu and I'm helping the Cozystack team.
Your analysis is correct and matches two earlier reports of the same bug: #2981 (first reported against the
Updatepath on v1.5.0) and #3971 (confirming the behavior across the aggregated apiserver). There is already an open fix in PR #3937 that addresses the root cause acrossCreate,Update, andDelete.The PR description explains that the problem goes deeper than simply not checking
dryrun.IsDryRun(): passing the incomingmetav1.*Optionsthroughclient.*Options{Raw: options}is also insufficient because controller-runtime'sAs*Options()methods rebuildRawfrom the typed struct fields, silently droppingDryRun,FieldManager,FieldValidation, delete propagation, grace periods, and preconditions [1].On
maintoday, the specific code paths are:&client.CreateOptions{Raw: options}[2] — looks correct but still loses options through controller-runtime's rebuilding.&metav1.UpdateOptions{}, discarding the caller's options entirely [3].r.Create(ctx, obj, createValidation, &metav1.CreateOptions{})with no options at all [4].&client.DeleteOptions{Raw: options}[5] — same controller-runtime rebuilding issue as Create.PR #3937 replaces all of these with new typed-option converters (
ClientCreateOptions,CreateOptionsFromUpdate,ClientUpdateOptions,ClientDeleteOptions) in a newpkg/registry/write_options.go, and also fixes the same class of bug inpkg/registry/core/tenantsecret/rest.goandpkg/registry/sdn/securitygroup/rest.go[1]. This covers theDeletecase you flagged as most concerning — a--dry-run=serverdelete would indeed have removed the backing HelmRelease for real under the current code.As of now, PR #3937 is still open and not yet merged, so both
mainand v1.6.1 remain affected [6].To reply, just mention @dosu.
Share context across your team and agents. Try Dosu.
myasnikovdaniil commentedon Aug 28, 2026
Hi! I'll take a look on this soon.
lexfrei commentedon Sep 11, 2026
One aggregated storage sits outside the fix in #3937 and still writes on a server dry-run:
taps.core.cozystack.io.pkg/registry/core/tap/rest.gotakes the options and discards them —Createat line 182 andDeleteat 276 both bind the incoming*metav1.CreateOptions/*metav1.DeleteOptionsto_and construct fresh ones for their dynamic client. So there is nothing to nil out, which is how it escaped the controller-runtime mechanism this issue is about: the other storages lostDryRunbecauseAsCreateOptions()overwroteRawfrom zero-valued typed fields, while this one never carries it in the first place. Same observable, different cause.The storage is registered at
pkg/apiserver/apiserver.go:234, so the path is live. Effect ofkubectl --dry-run=serveron a Tap:Createreally creates the FluxOCIRepositoryincozy-system, andDeletereally removes thePackageSourceand possibly theOCIRepositorywith it.Worth stating for whoever closes this issue: fixing the three storages in #3937 does not close the class, and a reader checking "is dryRun honoured now" against the apps registry will get a yes that does not generalise.
Related, and cheap to add while the area is open: nothing stops the next aggregated storage from hand-building
client.CreateOptions{Raw: opts}and regressing this silently. There is no lint rule and no test that scans for it, so the only thing standing between the tree and a repeat is whoever reviews that PR remembering this one.lexfrei commentedon Sep 14, 2026
The
Raw: optsshape does not work either, so the set is in worse shape than my earlier comment said.On the pinned
sigs.k8s.io/controller-runtime v0.23.1,CreateOptions.ApplyToCreate(options.go:303) copies the typedDryRunonly when it is non-nil, so a caller who fills in onlyRawleaves the typed field nil.AsCreateOptionsthen runso.Raw.DryRun = o.DryRunat options.go:287 and overwrites the populated raw marker with that nil. Four sites do it: 287, 371, 815, 913. The one at 371,AsDeleteOptions, also wipesGracePeriodSeconds,PreconditionsandPropagationPolicy.So
&client.CreateOptions{Raw: opts}is a no-op for dry-run, and on main today all three storages lose the directive:The worse half is that the test meant to pin this cannot fail.
TestUpdateForceCreatePropagatesDryRunatpkg/registry/sdn/securitygroup/rest_test.go:755capturesco.Raw.DryRunfrom inside aWithInterceptorFuncscreate interceptor, and that interceptor runs before any conversion, so it reads the marker while it is still there and goes green over a production path that drops it.createOptionsFromUpdateat:494is written correctly; the wrapper eats its output afterwards.Two things for whoever picks up #3937.
application/rest.go:609is a plain empty options struct rather than theRawconversion, so a fix aimed only at the conversion layer will miss it. And the securitygroup test needs to assert downstream of the conversion instead of upstream of it, otherwise it keeps passing either way.lexfrei commentedon Sep 23, 2026
Fixed on main. #3937 passes dry-run through the application, tenantsecret and securitygroup storages, and #4225 does the same for tap. #2981 is the same bug and goes with this one.