fix(kubeblocks): lookup-safe optional fields, dead-code toggles, output schema fixes#368
Merged
Merged
Conversation
…ixes (RULE-006/012/015/017/025)
restore is optional(object({...})) with no default, so an unset restore
block resolves to null (not absent). lookup(var.instance.spec, "restore",
{}) only substitutes its default for genuinely absent keys, so it returned
the literal null, and the following lookup(local.restore_config, "enabled",
false) crashed with "argument must not be null" on every deploy that
doesn't configure a restore (the common case). Replaced with
try(var.instance.spec.restore.enabled/.backup_name, default) and gave the
restore object real defaults in variables.tf. Same RULE-015 class as the
karpenter/kubernetes_cluster fixes on this branch.
Also, while auditing this module against rules.md:
- RULE-006: kubeblocks_operator input was missing the interfaces wrapper
(source emits @facets/kubeblocks-operator under `default`, needs both
attributes and interfaces). kubernetes_cluster input was wrapped in an
extra attributes{} layer when the source emits it flat.
- RULE-025: node_pool.interfaces was typed `any`; replaced with an explicit
optional(object({}), {}) matching what karpenter actually emits.
- RULE-012: outputs/mongo_k8s/outputs.yaml declared external_endpoints as
type: object, but the module emits jsonencode(...) (a string, as the
README already documents). Fixed the schema to type: string.
- RULE-012: outputs/mongo/outputs.yaml's interfaces schema only declared
the three fixed keys (cluster/reader/writer), but output_interfaces
dynamically merges in a key per configured external_access entry. Added
additionalProperties to the schema rather than restrict the feature.
- storage_class was read in main.tf and documented in the README, but was
never exposed in facets.yaml's storage schema, so it could not actually
be set via Facets. Added it as an optional string (default '').
- Removed a redundant depends_on on the credentials data source; the
try(...) reference to the same data source already creates it implicitly.
Version bumped 1.1 -> 1.2 for the input/output schema corrections.
Validated via `raptor create iac-module --dry-run`.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… restore, schema fixes (RULE-006/012/015/017/025)
create_read_service was hardcoded true, so the reader-fallback ternary in
outputs.tf was dead code: in standalone mode (spec.mode != replication),
the read Service selects kubeblocks.io/role: secondary pods, which don't
exist outside HA. Every standalone deploy handed out a reader endpoint
that resolved to nothing while the writer endpoint worked fine - a silent
failure, no plan/apply error. Fixed to create_read_service = local.ha_enabled,
so standalone correctly falls back to the writer/primary host, matching
mongo's existing (correct) behavior. Traced the fix through both branches
of the ternary post-change to confirm.
restore is optional(object({...})) with no default, so an unset restore
block resolves to null (not absent) and lookup(local.restore_config,
"enabled", false) crashed with "argument must not be null" on every
deploy that doesn't configure a restore. Replaced with try() directly on
var.instance.spec.restore.*, matching the mongo/RULE-015 fix.
Also, matching the audit done on the mongo sibling module:
- RULE-006: kubeblocks_operator input was missing the interfaces wrapper;
kubernetes_cluster input was wrapped in an extra attributes{} layer when
the source emits it flat. Verified kubernetes_cluster is unreferenced
elsewhere in this module before flattening.
- RULE-025: node_pool.interfaces was typed `any`; replaced with an explicit
optional(object({}), {}).
- RULE-017: removed a redundant depends_on already implied by a try()
reference to the same data source.
- storage_class was read in main.tf but never exposed in facets.yaml;
added it as an optional string (default ''), and guarded the PVC
template so an empty value doesn't emit storageClassName: "" (which
disables dynamic provisioning in Kubernetes, unlike omitting the field).
- RULE-012: port was emitted as a number but the @facets/postgres output
schema types it string; wrapped in tostring(). Added the missing
`database` property to the interfaces schema (the module already emits
it, the schema just never declared it).
Validated via `raptor create iac-module --dry-run`.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…store, schema fixes (RULE-006/015/017/025)
Same bugs as the postgres sibling module in this branch:
create_read_service was hardcoded true, so in standalone mode (non-HA)
the reader output pointed at a Kubernetes Service selecting
kubeblocks.io/role: secondary pods, which don't exist outside replication
mode. Fixed to create_read_service = local.ha_enabled, matching mongo's
already-correct fallback to the writer/primary host.
restore is optional(object({...})) with no default, so an unset restore
block resolved to null and crashed lookup(local.restore_config, "enabled",
false) with "argument must not be null" on every deploy that doesn't
configure a restore. Replaced with try() directly on
var.instance.spec.restore.*.
Also matching the mongo/postgres audit:
- RULE-006: kubeblocks_operator input missing the interfaces wrapper;
kubernetes_cluster wrapped in an extra attributes{} layer. Verified
kubernetes_cluster is unreferenced elsewhere in this module.
- RULE-025: node_pool.interfaces typed `any` -> explicit
optional(object({}), {}).
- RULE-017: removed a redundant depends_on already implied by a try()
reference to the same data source.
- storage_class was read in main.tf but never exposed in facets.yaml;
added as an optional string (default ''), guarded so an empty value
doesn't emit storageClassName: "" (disables dynamic provisioning).
mysql's port/database output types already matched the @facets/mysql
schema, so unlike postgres no RULE-012 output-type fix was needed here.
Validated via `raptor create iac-module --dry-run`.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rces, schema fixes (RULE-006/015/025)
resources is optional(object({...})) with no default, so an unset
resources block resolved to null and crashed the nested
lookup(lookup(var.instance.spec, "resources", {}), "cpu_limit", ...)
chain with "argument must not be null" on every deploy that doesn't
override CPU/memory. Gave the object real defaults (matching the literals
already hardcoded as lookup() fallbacks in main.tf) instead of patching
the call site. Same RULE-015 class as mongo/postgres/mysql's restore fix
in this branch. Reproduced the crash locally via `terraform apply` before
fixing, and confirmed the fix resolves it even when resources is sent as
an explicit null.
Real bug: spec.database_addons declared five boolean toggles
(postgresql/mysql/mongodb/redis/kafka) meant to control which KubeBlocks
addon Helm charts install, but main.tf's enabled_addons was never
filtered by them - the comment claimed "Filter only enabled addons" but
installed all five unconditionally. Setting redis=false did nothing, and
because the addon releases are wired atomic+wait+prevent_destroy, an
unrelated redis/kafka chart failure could block the entire operator
apply. Fixed by filtering enabled_addons against
var.instance.spec.database_addons (defaulting each to true, so existing
deployments that don't set this field see no behavior change). The addon
releases were already a single for_each block, so a filtered-out addon is
now genuinely absent from the plan, not just skipped-but-still-installed.
Also added database_addons to facets.yaml, which never exposed it.
Also matching the audit done on the datastore siblings:
- RULE-006: kubernetes_cluster input was wrapped in attributes{}+
interfaces{} when the source emits @facets/kubernetes-details flat
under `attributes`. Verified unreferenced elsewhere before flattening.
- RULE-025: node_pool.interfaces typed `any` -> explicit
optional(object({}), {}).
- Swapped the bitnamilegacy/kubectl:1.33.4 image for the equivalent
registry.k8s.io/kubectl:v1.33.4 (verified the tag exists) - bitnami's
legacy shelf isn't where this should be pulled from.
Not changed: prevent_destroy = true on both helm releases contradicts
the module's own teardown logic (wait_for_cleanup sleep, keepAddons=false,
cleanup_on_fail=true all exist specifically to let destroy work cleanly,
but prevent_destroy blocks terraform destroy from running at all). Left
as-is pending a decision on whether that protection is intentional.
Version kept at 1.0: the output type (@facets/kubeblocks-operator) is
unchanged and the new database_addons field is additive, so nothing
consuming this module's outputs breaks.
Validated via `raptor create iac-module --dry-run`.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
RoguedBear
changed the title
unni-facets
approved these changes
Jul 6, 2026
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
2 participants
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
Four commits, one per module, all in the KubeBlocks family (mongo/postgres/mysql datastore modules + the shared kubeblocks-operator):
mongo/kubeblocks:restoreoptional-object null-crash (RULE-015) + RULE-006/012/017/025 output-wiring and schema fixes + missingstorage_classschema field. Version1.1→1.2.postgres/kubeblocks: samerestorenull-crash, plus a real bug —create_read_servicewas hardcodedtrue, so standalone (non-HA) deploys handed out areaderendpoint pointing at a Service with zero backing pods. Fixed to followlocal.ha_enabled. Same RULE-006/012/017/025 fixes as mongo.mysql/kubeblocks: samerestorenull-crash and samecreate_read_servicebug as postgres, plus the same RULE-006/017/025 fixes (mysql's output types were already correct, so no RULE-012 fix needed here).kubeblocks-operator:resourcesoptional-object null-crash (RULE-015, reproduced locally viaterraform applybefore fixing), plus a real bug —spec.database_addonswas supposed to toggle which addon Helm charts install but was never actually wired up; all five installed unconditionally regardless of the setting, and an unrelated redis/kafka chart failure could block the whole operator apply. Fixed by filteringenabled_addonsagainst the spec (defaulting totrue, so no behavior change for existing deployments that don't set it). Also swapped a straybitnamilegacy/kubectlimage for the canonicalregistry.k8s.io/kubectltag.Test plan
raptor create iac-module --dry-runpasses clean on all four modulesterraform fmt/terraform validateclean on all four (only pre-existing, unrelated deprecation warnings)create_read_servicefix through both standalone and replication branches of the output ternary on postgres and mysqlkubeblocks-operatorresourcesnull-crash locally viaterraform applybefore fixing, confirmed the fix resolves it