Skip to content

[module-sdk] fix: prevent CRD update churn and prune unknown extensions - #128

Draft
fuldaxxx wants to merge 4 commits into
mainfrom
fix/crd-installer-prune-unknown-fields
Draft

[module-sdk] fix: prevent CRD update churn and prune unknown extensions#128
fuldaxxx wants to merge 4 commits into
mainfrom
fix/crd-installer-prune-unknown-fields

Conversation

@fuldaxxx

@fuldaxxx fuldaxxx commented Aug 5, 2026

Copy link
Copy Markdown
Member

Description

Prune CRD validation schemas to the fields the apiserver actually knows before applying
them, normalize the fields it fills in itself, and stop rewriting metadata that belongs to
other actors.

New package pkg/crd-installer/openapi:

  • a field-for-field fork of apiextensionsv1.JSONSchemaProps plus
    x-kubernetes-sensitive-data, with the union marshallers ported from upstream;
  • Prune decodes a schema through that type and re-encodes it, so every key that is not a
    schema field is dropped and whole numbers come back as int64 — exactly as the apiserver
    returns them;
  • drift guards: field set, json tag options (omitempty included), union field sets, and a
    byte-for-byte comparison of the fork's marshalled output against upstream's.

Installer:

  • sanitize rewrites only spec.versions[*].schema.openAPIV3Schema; the rest of the
    document is passed through untouched, so a CRD field only a newer or patched apiserver
    models still reaches it;
  • applyServerDefaults writes what the apiserver fills in itself — spec.names.singular,
    spec.names.listKind, and served/storage on every version (neither has omitempty
    upstream, so the stored object always carries both);
  • the update compares the lossless unstructured .spec, labels and annotations against the
    state the object must end up holding, and returns early when nothing differs;
  • labels and annotations are overlaid, never replaced, so keys written by other actors
    survive;
  • a document that cannot be read or sanitized no longer aborts the file: the remaining
    documents are still applied, all errors are joined, and a schema this build cannot decode
    is sent as it came rather than keeping the CRD out of the cluster.

Why do we need it, and what problem does it solve?

Reconcile churn. Since CRDs are applied verbatim (#122), the desired .spec carried
keys the apiserver prunes (x-doc-examples, x-doc-default, x-examples,
x-kubernetes-immutable, …), omitted the .spec fields it defaults, and encoded whole
numbers as float64. The stored spec could therefore never equal the desired one, so every
ensure_crds run — i.e. the startup of every module — issued a full Update of nearly
every CRD, bumping resourceVersion and re-waking every CRD informer in the cluster. The
apiserver also logged one "unknown field" warning per pruned key on every apply.

Vendor extensions still have to survive. x-kubernetes-sensitive-data only exists on
the Deckhouse apiserver, and the upstream Go type cannot hold it — dropping it would stop
the marked fields from being encrypted in etcd, RBAC-filtered and masked in audit logs.
That is the sole reason the schema type is forked instead of reusing
apiextensionsv1.JSONSchemaProps.

Foreign metadata was deleted. Replacing metadata.labels and metadata.annotations
wholesale dropped Helm's ownership keys — app.kubernetes.io/managed-by among the labels,
meta.helm.sh/release-name and -namespace among the annotations — so the next
helm upgrade of the chart that installed the CRD failed on
invalid ownership metadata.

One bad document took the file down with it. An undecodable document aborted
processCRD, silently skipping every CRD after it in the same file, and a schema this build
cannot decode kept the CRD itself out of the cluster — which rejects every custom resource
of that kind as an unknown kind.

What is the expected behaviour?

  • Applying a manifest over the state the apiserver derived from it is a no-op: no Update,
    no generation bump, no informer wakeups.
  • Keys the apiserver does not know never leave the installer for schemas, so the
    "unknown field" warnings are gone.
  • x-kubernetes-sensitive-data reaches the cluster, at every nesting position
    (properties, items, additionalProperties, allOf, …).
  • Labels and annotations written by Helm, kubectl or an operator survive every update;
    server-managed metadata (finalizers, ownerReferences, uid) and the in-cluster
    spec.conversion are preserved as before.
  • A broken document is reported and the rest of the file is still installed.

Deliberate trade-offs

  • .spec keys outside the schema are still sent. If this cluster's apiserver prunes one
    (selectableFields on an older cluster, x-kubernetes-sensitive-data on a stock control
    plane), that single CRD is updated on every reconcile. Accepted: the key is sent so an
    apiserver that does know it gets it.
  • Metadata keys are never deleted. A label or annotation the manifest stops declaring
    stays in the cluster; retracting one needs the field ownership the apiserver keeps for
    server-side apply, which would mean switching the whole update to Apply.
  • The fork is the allowlist in both directions. A schema field a newer apiserver
    understands and this build does not is dropped silently, and the drift guards only fire
    when this module bumps k8s.io/apiextensions-apiserver — keep the dependency in step with
    the apiserver Deckhouse ships.

Tests

New regression subtests in pkg/crd-installer:

  • unknown schema extensions are stripped while x-kubernetes-sensitive-data survives;
  • a CRD field outside the schema reaches the cluster and does not churn;
  • applying a manifest over its stored form (7_churn_stored.yaml8_churn_manifest.yaml,
    including a version whose storage the manifest omits) issues zero updates;
  • a bad document does not skip the rest of the file, and its own CRD is still installed with
    the schema sent as it came;
  • Helm's ownership label and annotation survive an update, and neither reads as a diff on
    the next run — on a fake client seeded by the subtest itself, so it holds in isolation.

In pkg/crd-installer/openapi: schema round-trip tests for kept and dropped keys, the three
drift guards, and the marshalled-bytes comparison against upstream. Each fix was checked by
reverting it and confirming the matching test fails.

@fuldaxxx fuldaxxx self-assigned this Aug 5, 2026
Signed-off-by: Ruslan Gorbunov <ruslan.gorbunov@flant.com>
@fuldaxxx
fuldaxxx force-pushed the fix/crd-installer-prune-unknown-fields branch from e4748bf to 586d874 Compare August 6, 2026 14:17
Signed-off-by: Ruslan Gorbunov <ruslan.gorbunov@flant.com>
Queue documents even when schema sanitization fails, while reporting the
error and continuing with later documents. Normalize version defaults
and
overlay metadata to avoid reconcile churn and deleting ownership fields.

Signed-off-by: Ruslan Gorbunov <ruslan.gorbunov@flant.com>
@fuldaxxx fuldaxxx changed the title fix: prevent CRD update churn and prune unknown extensions [module-sdk] fix: prevent CRD update churn and prune unknown extensions Aug 7, 2026
Signed-off-by: Ruslan Gorbunov <ruslan.gorbunov@flant.com>
@ldmonster
ldmonster self-requested a review August 7, 2026 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant