Skip to content

fix: Reject zero and empty values admission used to let through - #73

Merged
as51340 merged 1 commit into
mainfrom
fix/reject-zero-values
Oct 13, 2026
Merged

as51340 merged 1 commit into
mainfrom
fix/reject-zero-values

Conversation

@as51340

@as51340 as51340 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Why

Follow-up to #72's API audit: "if you really want to disallow zero values or empty strings for a field, use MinLength=1 / Minimum=1". Four fields accepted a value that only failed after admission (as ApplyFailed or a pod that never starts), or that silently meant something else.

What

Field Accepted What happened Now
monitoring.vmagentRemote.scrapeInterval "" (every unit in the duration pattern is optional) Stored as "", because a schema default only fills a missing field; the builder then quietly used 15s MinLength=1
securityContext.runAsUser 0 The Memgraph container sets runAsNonRoot, so the kubelet refuses to start it Minimum=1 (runAsGroup/fsGroup keep 0, a legitimate gid)
coreDumps.uploader.envFromSecrets[] anything, including "" Became secretRef.name, so the StatefulSet apply failed with ApplyFailed Same MinLength/MaxLength/DNS-subdomain rules as every secretName
storage.<role>.libPVCSize, coreDumps.<role>.size 0, -1Gi at creation (the existing rules only block a shrink) The claim template was refused at apply time CEL: must be greater than zero

Left alone on purpose: the storage class names ("" disables dynamic provisioning, as documented), EnvVar.value, FlagValue, the uploader's command/args items, and priorityClassName/serviceMonitor.interval (optional, where "" already means unset).

Compatibility: this only tightens validation. An existing object holding one of these values fails its next update until it is corrected; nothing changes for valid objects or their pods.

Chart version 1.0.1-rc.1 → 1.0.1-rc.2 for the CRD change. #72 makes the same bump, so whichever merges second needs rc.3.

Testing

  • New envtest rejection cases for each field: root uid, zero lib claim, negative core dumps claim, empty and non-DNS uploader Secret, and a blanked-out scrape interval sent as a raw manifest (the typed client drops ""). The raw manifests now share a rawCluster helper.
  • make test, make lint, make chart-verify, make helm-lint, make chart-version-check.

Four fields accepted a value that only failed, or silently changed meaning,
after admission:

- monitoring.vmagentRemote.scrapeInterval matched "" (every unit in its
  duration pattern is optional). A written "" is stored as is, since a schema
  default fills only a missing field, and the builder then ran vmagent on its
  own 15s fallback. MinLength=1.
- securityContext.runAsUser allowed 0, but the Memgraph container sets
  runAsNonRoot, so the kubelet refused to start it. Minimum=1; runAsGroup and
  fsGroup keep 0, which is a legitimate gid.
- coreDumps.uploader.envFromSecrets items had no constraint, so "" or a
  non-DNS name surfaced as ApplyFailed. Items now carry the Secret name rules
  every other secretName has.
- storage.<role>.libPVCSize and coreDumps.<role>.size accepted 0 and negative
  quantities at creation; the existing rules only stop a shrink. Both must
  now be greater than zero.

An existing object holding one of these values fails its next update until
it is corrected. The validation test's raw manifests share a rawCluster
helper now that there are three.
@as51340 as51340 self-assigned this Oct 13, 2026
@as51340
as51340 merged commit 491ba65 into main Oct 13, 2026
6 checks passed
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