feat!: backport portable chart enhancements and release v25.3.8 - #140
Closed
mlwelles wants to merge 12 commits into
Closed
feat!: backport portable chart enhancements and release v25.3.8#140mlwelles wants to merge 12 commits into
mlwelles wants to merge 12 commits into
Conversation
…tatefulsets - zero Service read the wrong externalTrafficPolicy key path, so the field never rendered - ACL/encryption/TLS Secrets stamped an undefined annotations key (dead code) - the backup PersistentVolume carried a meaningless namespace (cluster-scoped) - /dgraph was unmounted when persistence was disabled - an inert explicit datadir volume collided with volumeClaimTemplates - fullname left a trailing dash when truncated at 24 chars - serviceAccountName could render empty; route it through the helper - the alpha command was not exec'd, so SIGTERM was not forwarded
…less Service Declare http (8080) and grpc (9080) on the alpha headless Service. The backup CronJobs reach a pinned alpha-0 through this Service; under a STRICT mTLS mesh the client sidecar only builds an mTLS route for ports the Service declares, so without them the /admin login falls through to plaintext and the sidecar resets it. Guard each backup.sh curl with require_json: capture the curl exit status errexit-safely and validate the body is JSON before parsing. A mesh or network failure returns a plaintext body (e.g. an Envoy connect error) that now surfaces as a clear curl-status / URL / body message instead of a cryptic parse failure or a silently empty token. Convert the login bodies and response parsing to jq: get_token_rest and get_token_graphql build their request bodies with 'jq -n --arg' instead of hand-interpolating user/password into a JSON string, so credentials containing quotes or other JSON-breaking characters can no longer corrupt or inject into the request. All four functions parse responses with 'jq -r ... // empty' instead of grep -oP/grep -q errors, removing the dependency on grep's PCRE mode. backup_graphql's destination stays hand-interpolated since it is operator-controlled, not user credentials. Reset HEADERS and CERTOPTS at the top of get_token and backup so a second call in the same shell does not accumulate headers left over from an earlier call. Quote the get_token arguments in the full and incremental CronJobs so an admin user or password containing whitespace is passed as a single argument.
…admin credentials
BREAKING CHANGE: alpha.acl.enabled and alpha.encryption.enabled now synthesize the --acl / --encryption superflags. Remove any --acl/--encryption from alpha.extraFlags; the chart fails rendering rather than pass a flag twice.
…theusRule, and NetworkPolicy All four resources default off. - pdb.yaml: alpha/zero PodDisruptionBudgets, default-off to avoid the single-replica drain deadlock (minAvailable >= replicaCount blocks kubectl drain / cluster-autoscaler eviction indefinitely). - servicemonitor.yaml: companion to commit 2's headless-Service ports; scoped to the ClusterIP Services via the monitor label. - prometheusrule.yaml: conservative default alerts on stable metrics (up, kube_job_status_failed); extraRules for image-specific alerts. - networkpolicy.yaml: ingress-only, intra-cluster allow plus clientPodLabels-gated alpha client access. ServiceMonitor/PrometheusRule require the Prometheus Operator CRDs; default-off means a stock install imposes no CRD dependency.
…curityContext, securityContext passthrough, preUpgradeHook toggle, log flags, helper dedup
…TPS when TLS is enabled BREAKING CHANGE: alpha.tls.enabled/zero.tls.enabled now synthesize the --tls superflag from the tls block (internalPort/clientName/clientAuthType) and switch health probes to HTTPS. Remove any --tls from extraFlags. A cert-requiring clientAuthType (REQUIREANY/REQUIREANDVERIFY) with httpGet probes fails rendering; use VERIFYIFGIVEN or exec probes.
…validation subsystem Both are default-off and inert on a stock install: nothing renders unless explicitly enabled. The ACL bootstrap reconciler is a post-install/post-upgrade hook Job that rotates groot off its default password and converges the declared users and groups. It is idempotent, so it is safe to re-run on every helm upgrade. The validation subsystem asserts the running cluster matches what the chart rendered (health, membership, ACL enforcement, admin and per-user logins, group rules, and optionally backup CronJob schedules). It can gate a release via a post-install hook Job or run on demand via helm test; a suspended manual-trigger CronJob is also provided.
…og-flag robustness - alpha/svc-headless.yaml: wrap the http-alpha (8080) and grpc-alpha (9080) port entries in an or backups.full.enabled/backups.incremental.enabled guard, since their only consumer is the backup CronJobs. The grpc-alpha-int (7080) entry stays unconditional. Update the comment to describe the new gating. - acl/bootstrap-job.yaml: require registry, repository, and tag to all be set before using the bootstrap image override, matching the templates/validation/_pod.tpl pattern. A partial override (e.g. registry only) now falls back to the shared dgraph image instead of rendering a malformed reference. - _helpers.tpl: make dgraph.logFlags nil-safe on .logtostderr by defaulting it to true before use in both the emptiness check and the rendered flag, so an omitted key no longer emits an extra --logtostderr flag at defaults. - values.yaml: refresh the two commented-out example override tags (alpha.acl.bootstrap.image, validation.image) from v25.3.1 to v25.3.8 to match the released default.
Sprig's default treats a boolean false as empty, so dgraph.logFlags read .logtostderr | default true and silently flipped an explicit logtostderr: false back to true. Switch to the nil-check pattern dgraph.tlsFlag already uses for internalPort: nil/omitted still defaults to true, but an explicit false is now honored.
This was referenced Jul 21, 2026
Open
Open
Author
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
1 participant
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.
Summary
Backports the portable, non-vendor-specific enhancements we have been carrying in a downstream fork of this chart, so the upstream
dgraphchart gains them and we stop diverging. Every new feature and resource is default-off: a stockhelm installrenders identically to the previous release except for a set of named, documented changes (bug fixes, a pre-upgrade-hook hardening, and the version bump).Twelve commits, ordered so each is independently reviewable and renders cleanly on its own.
What's included
externalTrafficPolicykey path; deadtls.annotationsblocks on Secrets; a meaninglessnamespaceon the cluster-scoped backup PV;/dgraphunmounted when persistence is off; an inertdatadirvolume that duplicatedvolumeClaimTemplates; a trailing dash infullnameat 24-char truncation; an emptyserviceAccountName; and a missingexecso SIGTERM reached Dgraph).jqparsing andjq -n --arginjection-safe login bodies replacing fragilegrep -oP, an errexit-safe curl guard, and the alpha headless Service exposing 8080/9080 when backups are enabled (so a pinned-pod/adminlogin survives a strict mTLS mesh).existingSecretfor the ACL HMAC, encryption key, and backup-admin password, so key material never renders through Helm values or release state.--acl/--encryptionnow auto-activate fromalpha.acl.enabled/alpha.encryption.enabled(see Breaking changes).PodDisruptionBudget,ServiceMonitor,PrometheusRule, andNetworkPolicy.imagePullSecrets, per-containersecurityContext,securityContextpassthrough, apreUpgradeHook.enabledtoggle plus hook-script hardening, glog log flags, and de-duplicated domain-suffix / ingress-path helpers.--tlssuperflag from thetlsblock and switches health probes to HTTPS (see Breaking changes).persistentVolumeClaimRetentionPolicyfor alpha and zero (folds in feat(dgraph): add persistentVolumeClaimRetentionPolicy support #136).v25.3.8and document the breaking changes.Breaking changes
All three fail loudly at template time rather than misbehaving at runtime; migration notes are in the chart README under
### Upgrading→#### Additional v25 breaking changes.alpha.acl.enabled(oralpha.encryption.enabled) now synthesizes the--acl(or--encryption) superflag. If you currently pass either throughalpha.extraFlags, remove it; the chart fails rendering rather than pass the flag twice.tlsblock. Settingalpha.tls.enabled/zero.tls.enablednow synthesizes--tlsfrom thetlssettings (internalPort,clientName,clientAuthType). Remove any hand-set--tlsfromextraFlags.scheme: HTTPS. A cert-requiringclientAuthType(REQUIREANY/REQUIREANDVERIFY) cannot work with the built-in httpGet probes (the kubelet has no client cert), so the chart fails rendering and points you atVERIFYIFGIVENor exec probes.Default-off and verification
There is no PR-time test workflow in this repo, so verification was done locally and is reproducible:
v25.3.8image tag / chart labels, theexecprefix, thedatadir-volume normalization, and the pre-upgrade-hook script hardening. Every new feature contributes zero lines to the default render until explicitly enabled.helm template+helm lintsweep (ACL, encryption,existingSecret, TLS acrossinternalPort/clientAuthType, backups s3/minio/nfs/volume, each ingress example, persistence off, each default-off resource enabled,global.ingress, and the legacyexample_values/v20.11/set). All pass; the one pre-existing malformedv20.11/zero-tls-secrets.yamlexample is unchanged by this PR./dgraphmount fix) and again with ACL enabled (a groot login returned a valid JWT, confirming the synthesized--acl).volumeClaimTemplatesandspec.selectorare never modified (both immutable on StatefulSet update).Version
Bumps chart
versionto25.3.8,appVersiontov25.3.8, and the shared image tag tov25.3.8(the ratel and kubectl-hook image tags are unchanged). Theversionbump is what triggers chart-releaser to publish on merge.Relationship to #136
This folds in #136's
persistentVolumeClaimRetentionPolicychange verbatim. #136 will be closed in favor of this PR — it also never bumpedChart.yaml, so it would not have published on its own.Notes for reviewers
The ACL bootstrap reconciler and the validation subsystem are the most opinionated pieces. Both are entirely default-off and land as the final feature commit, so they can be dropped without unwinding the rest of the PR if you would prefer to take them separately. Specifically:
grootpassword on first run in order to rotate it, and aborts ifgrootis neither the target nor the default password.helm upgradewhen a validation check fails (off by default).suspend: trueas akubectl create job --from=cronjob/...trigger idiom.Known cosmetic follow-ups intentionally deferred: a one-character README config-table column alignment drift in the observability block; a
{{- /* … */ -}}vs*/}}comment-close style difference in the four new observability templates; a stray blank line from an emptyimagePullSecretsin the validation pod template.