Schemahero controller and schemahero table #415
Conversation
Debarshi11
requested review from
anshulsao,
ishaankalra,
pramodh-ayyappan,
rr0hit,
unni-facets and
vishnukv-facets
as code owners
WalkthroughThis update introduces new modules and facets for deploying and managing SchemaHero controllers, databases, and tables in Kubernetes using Terraform and Helm. It adds robust configuration schemas, Helm chart definitions, Kubernetes CRDs, and health check modules. The status check facet is expanded with detailed validation, and supporting files for automation, documentation, and configuration are included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Facet
participant Terraform
participant Helm
participant Kubernetes
User->>Facet: Define SchemaHero controller/database/table/status_check resource
Facet->>Terraform: Pass configuration and schema
Terraform->>Helm: Render Helm chart with values and manifests
Helm->>Kubernetes: Install/upgrade resources (CRDs, StatefulSet, Service, etc.)
Kubernetes-->>User: Deploys SchemaHero controller and resources
User->>Facet: Configure health checks
Facet->>Terraform: Pass health check spec
Terraform->>Kubernetes: Create custom resource for health check
Kubernetes-->>User: Runs health checks as per configuration
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
♻️ Duplicate comments (3)
modules/schemahero_database/k8s/0.1/variables.tf (1)
3-22: Same variable issues as controller moduleAll observations made for
modules/schemahero_controller/variables.tf(untypedany, missing descriptions, no validation) apply here verbatim.modules/schemahero_table/k8s/0.1/variables.tf (1)
1-20: Same variable issues as controller moduleIdentical concerns: provide explicit types, descriptions, and validation.
modules/schemahero_controller/schemahero-helm/crds/schemas.schemahero.io_views.yaml (1)
102-105: Samestatussubresource concern as Migration CRDAdd the
statussubresource to follow Kubernetes API conventions and allow RBAC-scoped status updates.
🧹 Nitpick comments (28)
modules/status_check/default/0.1/variables.tf (1)
21-23:inputsis declared but never usedThe variable is not referenced in
locals.tformain.tf; keeping it around is misleading.-variable "inputs" { - type = any -}Either wire this variable into the module or drop it altogether.
modules/status_check/default/0.1/locals.tf (1)
7-8: Dead-code:local.resource_nameis never referencedUnless you plan to re-use it shortly, remove the declaration to avoid cognitive load.
modules/status_check/default/0.1/main.tf (1)
24-32: Suspicious default payload forrequestBodyDefaulting the body to
"100"is unexpected and might send meaningless data.- requestBody = lookup(each.value, "body", "100") + # Default to empty string to avoid sending unintended payloads + requestBody = lookup(each.value, "body", "")modules/schemahero_controller/outputs.tf (1)
1-4: Empty locals block is noise—either populate or remove.
localsis introduced solely to declareoutput_interfacesandoutput_attributes, both empty.
Until these maps carry content, the block adds no functional value and slightly clutters the module.-locals { - output_interfaces = {} - output_attributes = {} -}If placeholders are mandatory for some external tooling, add a short comment explaining the contract; otherwise delete until outputs are implemented.
modules/schemahero_table/k8s/0.1/outputs.tf (1)
1-4: Same placeholder issue as controller module—remove or document.Duplicating an empty
localsblock without explanation propagates dead code. Follow the advice given for the controller module: either document the need or drop the file for now.modules/schemahero_controller/schemahero-helm/.dockerignore (1)
1-4: Verify ignores—chart content may be accidentally excluded from image.Ignoring
templates/andcrds/means any Docker image built from this context won’t contain the Helm chart itself.
If the image is just a build/linter container that doesn’t need the chart, fine. Otherwise, remove those entries or build from a higher-level directory.Also,
*.jsonwill dropvalues.schema.json, which is useful for IDE validation.Please double-check the build use-case.
modules/schemahero_controller/schemahero-helm/.commitlintrc.yml (1)
5-11: Restrictivetype-enummay hinder future workOnly
chore,feat, andfixare allowed. Common types such asdocs,refactor,perf, andtestare currently blocked. If that is intentional, ignore this; otherwise, consider extending the list to avoid unnecessary lint failures.modules/schemahero_database/k8s/0.1/outputs.tf (2)
1-4: Remove unused locals or document their purposeBoth
output_attributesandoutput_interfacesare empty and never referenced.
Either delete them or add a comment explaining that they are placeholders for future outputs; dead code adds noise.
6-8: Output lacks description and sensitivity flagFor consistency with Terraform best practices, add a
descriptionand, ifinstance_namecan contain PII, mark it assensitive = true.output "name" { + description = "Name of the SchemaHero database instance." value = var.instance_name + # sensitive = true }modules/schemahero_controller/schemahero-helm/templates/service-account.yaml (1)
1-8: Consider hardening the ServiceAccountIf this account doesn’t need the default token injected into every pod, explicitly disable it:
kind: ServiceAccount metadata: name: {{ include "schemahero.fullname" . }} labels: {{- include "schemahero.labels" . | nindent 4 }} namespace: {{ .Release.Namespace }} +automountServiceAccountToken: falseThis follows the principle of least privilege.
modules/schemahero_controller/schemahero-helm/templates/cluster-role-binding.yaml (1)
4-4: Add labels for traceabilityIncluding standard chart labels aids troubleshooting and enables easy selection in cluster queries.
metadata: name: {{ include "schemahero.fullname" . }} + labels: + {{- include "schemahero.labels" . | nindent 4 }}modules/schemahero_controller/schemahero-helm/package.json (1)
1-10: Add minimal package metadata to avoid NPM warnings and improve discoverability
package.jsoncontains onlydevDependencies. Adding the standard top-level metadata (name,version,license, maybeprivate: true) prevents NPM/Yarn warnings and makes the package self-describing when published or inspected by tooling.{ + "name": "schemahero-helm", + "version": "0.1.0", + "license": "Apache-2.0", + "private": true, "devDependencies": { "@eshepelyuk/semantic-release-helm-oci": "^1.4.0", "@semantic-release/exec": "^5.0.0", "@semantic-release/git": "^9.0.0", "conventional-changelog-conventionalcommits": "^4.6.0", "semantic-release": "^17.4.4", "semantic-release-commitlint": "^1.2.2" } }modules/schemahero_controller/schemahero-helm/templates/service.yaml (1)
8-15: Allow overriding the Servicetypefor flexibilityThe Service is currently hard-coded to
ClusterIP. Exposing it as a value lets operators chooseNodePort/LoadBalancerwithout forking the chart.-spec: - type: ClusterIP +spec: + type: {{ .Values.service.type | default "ClusterIP" }}Remember to add
service.typetovalues.yamland the JSON schema.modules/schemahero_controller/facets.yaml (2)
25-32: Memory regex does not accept lowercase units & missing “MiB/GiB” variantsThe pattern disallows lowercase
k|m|gand composite units likeGiB. This will surprise users.- pattern: ^[0-9]+[KMGT]?[i]?[Bb]?$ + pattern: ^[0-9]+([kKmMgGtT])?([iI])?([bB])?$Alternatively drop the regex and delegate to Kubernetes’ native quantity parser.
10-33: AddadditionalProperties: falsefor strict schemaWithout it, typos in user-supplied keys silently pass through.
properties: size: ... + additionalProperties: falsemodules/schemahero_controller/schemahero-helm/README.md (1)
15-18: Helm OCI note outdatedFrom Helm 3.8 OCI support is GA; the
HELM_EXPERIMENTAL_OCItoggle is no longer needed.
Suggest dropping the environment variable to reduce noise.modules/schemahero_controller/schemahero-helm/templates/tests/testdb.yaml (1)
12-17: ConsiderimagePullPolicyand resource requests for test podExplicitly setting
imagePullPolicy(IfNotPresent) and minimal resources avoids noisy warnings in constrained clusters.modules/schemahero_controller/schemahero-helm/crds/schemas.schemahero.io_datatypes.yaml (2)
6-9: Remove Helm-generated noise (creationTimestamp: null)
creationTimestamp: nullis injected bykubectl get … -o yamland is not accepted bykubectl apply --server-sidewith--validate=true.
Delete it (or regenerate the CRD withcontroller-gen) to avoid validation warnings.
74-78: Drop emptystatus.acceptedNamesfields – they breakkubectl diff
acceptedNames.kind|pluralare populated by the API-server at run-time.
Bundling them empty causes perpetual diffs and noisy reconciliations.-status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] +status: {}modules/schemahero_controller/schemahero-helm/Chart.yaml (1)
5-8: Chart version & appVersion placeholders are non-semantic
version: 0.0.0andappVersion: 0.13.1hard-code values that will quickly drift and break semver-based tooling (e.g. Helm dependency updates, Artifact Hub). Inject them at release time or pin them to the upstream controller tag instead of0.0.0.-version: 0.0.0 # chart version introspected from git release tags -appVersion: 0.13.1 +version: 0.13.1 # keep in lock-step with upstream controller +appVersion: 0.13.1modules/schemahero_controller/schemahero-helm/crds/schemas.schemahero.io_migrations.yaml (1)
102-104: Enablestatussubresource for safer updates
Without thestatussubresource the controller must patch the entire object, increasing the chance of update conflicts. Consider:- subresources: {} + subresources: + status: {}modules/schemahero_controller/schemahero-helm/templates/manager.yaml (1)
50-53: Resource limits configurable but optional – provide sane defaults
If.Values.resourcesis omitted the container runs unbounded. Consider shipping conservative default requests/limits to prevent noisy-neighbour issues.modules/schemahero_table/k8s/0.1/main.tf (2)
4-4:instance_namelookup expression is overly convolutedThe ternary with two identical look-ups (
lookup(lookup(var.instance,"metadata",{}),"name",null)) does the same work twice.
Keep a single lookup and fall back tovar.instance_namefor clarity and micro-performance gain.- instance_name = lookup(lookup(var.instance, "metadata", {}), "name", null) == null ? var.instance_name : lookup(lookup(var.instance, "metadata", {}), "name", null) + instance_name = coalesce( + lookup(lookup(var.instance, "metadata", {}), "name", null), + var.instance_name + )
19-25: Local nameindexshadows Terraform’s built-inindex()functionWhile technically allowed, shadowing a standard function hurts readability and can confuse future maintainers.
Rename to something explicit such asindexes.modules/schemahero_controller/schemahero-helm/templates/cluster-role.yaml (1)
155-166: Indentation drift may break Helm renderingThese two rule blocks are indented with 4 spaces instead of 2, unlike previous blocks.
While YAML itself tolerates this, mixed indentation inside Helm templates has bitten us before when users copy/paste or mix tabs.modules/schemahero_controller/schemahero-helm/values.schema.json (1)
45-54:examples/examplefields are not valid JSON-Schema keywordsDraft-07 recognises
examples(array) or the olderexample(single), but not the current mixture:"examples": "- --log-level\n- debug", "comment": "some comment"This will be ignored by tools performing schema validation.
- "examples": "- --log-level\n- debug", "comment": "some comment" + "examples": ["--log-level", "debug"]modules/schemahero_table/k8s/0.1/facets.yaml (1)
44-47: Column keys restricted to digits; consider using column identifiers
patternProperties: "^[0-9]+$"forces opaque numeric keys ('1','2', …).
Using meaningful keys (e.g.,"code","name") improves readability and avoids merge conflicts.- patternProperties: - ^[0-9]+$: + patternProperties: + ^[a-zA-Z_][a-zA-Z0-9_]*$:modules/schemahero_database/k8s/0.1/facets.yaml (1)
76-76: Missing trailing newlineYAML-lint flags the absence of a newline at EOF; several tooling pipelines (git diff, POSIX text utils) expect one.
@@ - memory: "150Mi" \ No newline at end of file + memory: "150Mi" +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (42)
modules/schemahero_controller/facets.yaml(1 hunks)modules/schemahero_controller/main.tf(1 hunks)modules/schemahero_controller/outputs.tf(1 hunks)modules/schemahero_controller/schemahero-helm/.commitlintrc.yml(1 hunks)modules/schemahero_controller/schemahero-helm/.dockerignore(1 hunks)modules/schemahero_controller/schemahero-helm/.editorconfig(1 hunks)modules/schemahero_controller/schemahero-helm/.helmignore(1 hunks)modules/schemahero_controller/schemahero-helm/.releaserc.yaml(1 hunks)modules/schemahero_controller/schemahero-helm/Chart.yaml(1 hunks)modules/schemahero_controller/schemahero-helm/LICENSE(1 hunks)modules/schemahero_controller/schemahero-helm/README.md(1 hunks)modules/schemahero_controller/schemahero-helm/crds/schemas.schemahero.io_datatypes.yaml(1 hunks)modules/schemahero_controller/schemahero-helm/crds/schemas.schemahero.io_migrations.yaml(1 hunks)modules/schemahero_controller/schemahero-helm/crds/schemas.schemahero.io_tables.yaml(1 hunks)modules/schemahero_controller/schemahero-helm/crds/schemas.schemahero.io_views.yaml(1 hunks)modules/schemahero_controller/schemahero-helm/package.json(1 hunks)modules/schemahero_controller/schemahero-helm/skaffold.yaml(1 hunks)modules/schemahero_controller/schemahero-helm/templates/_helpers.tpl(1 hunks)modules/schemahero_controller/schemahero-helm/templates/cluster-role-binding.yaml(1 hunks)modules/schemahero_controller/schemahero-helm/templates/cluster-role.yaml(1 hunks)modules/schemahero_controller/schemahero-helm/templates/manager.yaml(1 hunks)modules/schemahero_controller/schemahero-helm/templates/secret.yaml(1 hunks)modules/schemahero_controller/schemahero-helm/templates/service-account.yaml(1 hunks)modules/schemahero_controller/schemahero-helm/templates/service.yaml(1 hunks)modules/schemahero_controller/schemahero-helm/templates/tests/testdb.yaml(1 hunks)modules/schemahero_controller/schemahero-helm/values.schema.json(1 hunks)modules/schemahero_controller/schemahero-helm/values.yaml(1 hunks)modules/schemahero_controller/variables.tf(1 hunks)modules/schemahero_database/k8s/0.1/facets.yaml(1 hunks)modules/schemahero_database/k8s/0.1/main.tf(1 hunks)modules/schemahero_database/k8s/0.1/module.json(1 hunks)modules/schemahero_database/k8s/0.1/outputs.tf(1 hunks)modules/schemahero_database/k8s/0.1/variables.tf(1 hunks)modules/schemahero_table/k8s/0.1/facets.yaml(1 hunks)modules/schemahero_table/k8s/0.1/main.tf(1 hunks)modules/schemahero_table/k8s/0.1/outputs.tf(1 hunks)modules/schemahero_table/k8s/0.1/variables.tf(1 hunks)modules/status_check/default/0.1/facets.yaml(1 hunks)modules/status_check/default/0.1/locals.tf(1 hunks)modules/status_check/default/0.1/main.tf(1 hunks)modules/status_check/default/0.1/outputs.tf(1 hunks)modules/status_check/default/0.1/variables.tf(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
modules/schemahero_controller/schemahero-helm/templates/service-account.yaml
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
modules/schemahero_controller/schemahero-helm/templates/service.yaml
[error] 4-4: syntax error: expected , but found ''
(syntax)
modules/schemahero_controller/schemahero-helm/templates/tests/testdb.yaml
[warning] 1-1: too many blank lines
(1 > 0) (empty-lines)
[error] 2-2: syntax error: expected the node content, but found '-'
(syntax)
modules/schemahero_controller/schemahero-helm/templates/manager.yaml
[error] 6-6: syntax error: expected the node content, but found '-'
(syntax)
modules/schemahero_database/k8s/0.1/facets.yaml
[error] 76-76: no new line character at the end of file
(new-line-at-end-of-file)
🪛 LanguageTool
modules/schemahero_controller/schemahero-helm/README.md
[uncategorized] ~20-~20: You might be missing the article “the” here.
Context: ...M_EXPERIMENTAL_OCI=1 ``` 1. Choose appropriate version from the [available releases](h...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~40-~40: You might be missing the article “the” here.
Context: ...helm/actions/workflows/release.yaml) in Actions section. 1. Click `Run workflow...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~44-~44: Possible missing article found.
Context: ... job is finished. Release job performs following actions. 1. Generates changelog from p...
(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~48-~48: You might be missing the article “a” here.
Context: ... version from the changelog. 1. Creates new GitHub release with git tag containing ...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~48-~48: You might be missing the article “a” here.
Context: ...log. 1. Creates new GitHub release with git tag containing new release number. 1. G...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
🪛 Checkov (3.2.334)
modules/status_check/default/0.1/main.tf
[MEDIUM] 3-34: Ensure Terraform module sources use a commit hash
(CKV_TF_1)
[HIGH] 3-34: Ensure Terraform module sources use a tag with a version number
(CKV_TF_2)
[MEDIUM] 36-60: Ensure Terraform module sources use a commit hash
(CKV_TF_1)
[HIGH] 36-60: Ensure Terraform module sources use a tag with a version number
(CKV_TF_2)
[MEDIUM] 62-86: Ensure Terraform module sources use a commit hash
(CKV_TF_1)
[HIGH] 62-86: Ensure Terraform module sources use a tag with a version number
(CKV_TF_2)
[MEDIUM] 88-112: Ensure Terraform module sources use a commit hash
(CKV_TF_1)
[HIGH] 88-112: Ensure Terraform module sources use a tag with a version number
(CKV_TF_2)
🔇 Additional comments (16)
modules/status_check/default/0.1/facets.yaml (1)
141-142:disabledat the root of the sample is not part of the declared schemaThe top-level schema only defines
http,mongo,redis,tcp. Keeping this field will cause validation failures once strict validation is enabled. Verify if it should be nested under each check or removed.modules/schemahero_controller/schemahero-helm/templates/secret.yaml (1)
1-5: ```shell
#!/bin/bash
set -eecho "Checking for existing 'type:' in secret.yaml"
rg -n "type:" modules/schemahero_controller/schemahero-helm/templates/secret.yaml || echo "→ No 'type:' directive found"echo
echo "Verifying 'schemahero.labels' helper definition"
rg -n "define "schemahero.labels"" -C3echo
echo "Searching for any existing 'helm.sh/resource-policy' annotations"
rg -n "helm.sh/resource-policy" || echo "→ No existing resource-policy annotations found"</details> <details> <summary>modules/schemahero_controller/schemahero-helm/.editorconfig (1)</summary> `1-12`: **👍 Consistent editor settings.** The `.editorconfig` enforces LF endings, UTF-8, 2-space indents, and trims whitespace—good for cross-team consistency. </details> <details> <summary>modules/schemahero_controller/schemahero-helm/.commitlintrc.yml (1)</summary> `1-12`: **Config structure looks valid** YAML indentation and commit-lint tuple syntax are correct. No action needed. </details> <details> <summary>modules/schemahero_controller/schemahero-helm/templates/service-account.yaml (1)</summary> `5-7`: **False-positive YAMLLint warning – no action required** The `nindent 4` pipe correctly indents the injected labels block; the lint tool does not understand Helm templating. </details> <details> <summary>modules/schemahero_controller/schemahero-helm/.helmignore (1)</summary> `1-32`: **Helm ignore list is comprehensive** Covers VCS, editor, build, and test artefacts—good job. </details> <details> <summary>modules/schemahero_controller/schemahero-helm/templates/cluster-role-binding.yaml (1)</summary> `4-12`: **Name collision risk across multiple releases** `{{ include "schemahero.fullname" . }}` may resolve to the same value for separate releases in different namespaces, causing a ClusterRoleBinding naming conflict (cluster-scoped resource). Confirm that the helper adds a release-specific suffix, or switch to `{{ include "schemahero.fullname" . }}-{{ .Release.Namespace }}` for uniqueness. </details> <details> <summary>modules/schemahero_controller/schemahero-helm/skaffold.yaml (1)</summary> `1-20`: **Confirm that `artifactOverrides.image` matches the chart’s value key** `skaffold` replaces chart values by matching the map key under `artifactOverrides` to the corresponding Helm value. If the SchemaHero chart expects something like `image.repository` or `controller.image`, the plain key `image` will silently do nothing and the built image won’t be used. Please verify the exact value path in `values.yaml`; if necessary, switch to `setValueTemplates` or update the key: ```diff - artifactOverrides: - image: schemahero/schemahero-e2e + setValueTemplates: + image.repository: "{{.IMAGE_NAME}}"modules/schemahero_controller/schemahero-helm/LICENSE (1)
1-202: License file looks goodFull Apache-2.0 text is included and unmodified. No issues.
modules/schemahero_controller/schemahero-helm/values.yaml (1)
11-16: Default limits/request mismatch with facet defaultsFacet defaults set
cpu: 100m / memory: 150Mi, but chart defaults requestcpu: 100m, memory: 50Miand limit CPU at1.
Aligning both sources avoids surprise capacity planning issues.modules/schemahero_database/k8s/0.1/module.json (1)
16-18: Verify module dependency name
"module.schemahero"may not exist – the controller module introduced here isschemahero_controller.
Please confirm the correct module ID to avoid Terraform graph failures.modules/schemahero_controller/schemahero-helm/README.md (1)
44-50: Minor language polish“Release job performs following actions” → “the following actions”, etc.
Helps readability but non-blocking.modules/schemahero_controller/schemahero-helm/Chart.yaml (1)
26-42: CRD annotation list is incomplete
The chart deploys additional CRDs (View,DataType) but they are missing fromartifacthub.io/crds, which leads to an incorrect metadata page on Artifact Hub and mis-reported installability.Add the missing kinds & keep the list in sync automatically (e.g.
{{- range $path, $_ := .Files.Glob "crds/*.yaml" }}in helpers).modules/schemahero_controller/schemahero-helm/templates/_helpers.tpl (1)
5-10: Helper looks goodConsistent labels & versions; no issues spotted.
modules/schemahero_table/k8s/0.1/facets.yaml (1)
2-2: Flavor value deviates from convention; verify downstream impactChanging
flavorfrom the conventionalk8s(ordefault) totest-schemawill break look-ups that assume the existing taxonomy (e.g., facet discovery, docs, CI matrices).
Confirm that all consumers (Terraform modules, UI registry, docs, tests) are updated accordingly or revert to the previous canonical flavor.modules/schemahero_database/k8s/0.1/facets.yaml (1)
58-66: Input type reference may be incorrect
inputs.kubernetes_details.typepoints to@outputs/kubernetes-cluster-details, whereas the table facet uses@outputs/kubernetes.
Align the alias or create a single canonical output; otherwise cross-facet composition will fail.
| variable "instance" { | ||
| type = any | ||
| default = {} | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid any – define a concrete object schema for var.instance
Using any defeats Terraform’s built-in validation and makes refactors risky. A minimal typed shape drastically improves IDE hints and prevents silent drift.
-variable "instance" {
- type = any
- default = {}
-}
+variable "instance" {
+ description = "Instance definition consumed by the status_check module"
+ type = object({
+ metadata = optional(object({
+ name = optional(string)
+ namespace = optional(string)
+ labels = optional(map(string))
+ annotations = optional(map(string))
+ }), {})
+
+ advanced = optional(object({
+ status_check = optional(any)
+ }), {})
+
+ spec = optional(any)
+ })
+ default = {}
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| variable "instance" { | |
| type = any | |
| default = {} | |
| } | |
| variable "instance" { | |
| description = "Instance definition consumed by the status_check module" | |
| type = object({ | |
| metadata = optional(object({ | |
| name = optional(string) | |
| namespace = optional(string) | |
| labels = optional(map(string)) | |
| annotations = optional(map(string)) | |
| }), {}) | |
| advanced = optional(object({ | |
| status_check = optional(any) | |
| }), {}) | |
| spec = optional(any) | |
| }) | |
| default = {} | |
| } |
🤖 Prompt for AI Agents
In modules/status_check/default/0.1/variables.tf at lines 3 to 6, the variable
"instance" is currently typed as "any", which disables Terraform's validation
and reduces IDE support. Replace the "any" type with a concrete object schema
that defines the expected attributes and their types for "instance". This will
enable better validation, improve IDE hints, and reduce risks during
refactoring.
| # Define your outputs here | ||
| locals { | ||
| output_interfaces = {} | ||
| output_attributes = {} | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
outputs.tf should expose outputs, not locals
Placing a locals block here is counter-intuitive and the data is never surfaced to callers.
-locals {
- output_interfaces = {}
- output_attributes = {}
-}
+output "interfaces" {
+ description = "Interfaces exposed by the status_check module"
+ value = {}
+}
+
+output "attributes" {
+ description = "Attributes exposed by the status_check module"
+ value = {}
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Define your outputs here | |
| locals { | |
| output_interfaces = {} | |
| output_attributes = {} | |
| } | |
| # Define your outputs here | |
| output "interfaces" { | |
| description = "Interfaces exposed by the status_check module" | |
| value = {} | |
| } | |
| output "attributes" { | |
| description = "Attributes exposed by the status_check module" | |
| value = {} | |
| } |
🤖 Prompt for AI Agents
In modules/status_check/default/0.1/outputs.tf at lines 1 to 5, the code
incorrectly uses a locals block to define output_interfaces and
output_attributes, which does not expose these values as outputs. Replace the
locals block with proper output blocks that explicitly define and expose
output_interfaces and output_attributes so they can be accessed by callers of
the module.
| module "uptime_http_checks" { | ||
| for_each = lookup(local.processed_names, "http", {}) | ||
| source = "github.com/Facets-cloud/facets-utility-modules//any-k8s-resource" | ||
| name = lower("${each.key}") | ||
| namespace = local.namespace | ||
| advanced_config = lookup(local.advanced_config, "values", {}) |
There was a problem hiding this comment.
Pin the module source to a tag/commit for reproducible builds
Open-ended Git sources trigger Checkov (CKV_TF_1/2) and make future runs nondeterministic.
- source = "github.com/Facets-cloud/facets-utility-modules//any-k8s-resource"
+ # Pin to a specific tag or commit to avoid unintentional upgrades
+ source = "github.com/Facets-cloud/facets-utility-modules//any-k8s-resource?ref=v1.2.3"Apply the same change to every uptime_*_checks module block.
Also applies to: 36-41, 62-67, 88-93
🧰 Tools
🪛 Checkov (3.2.334)
[MEDIUM] 3-34: Ensure Terraform module sources use a commit hash
(CKV_TF_1)
[HIGH] 3-34: Ensure Terraform module sources use a tag with a version number
(CKV_TF_2)
🤖 Prompt for AI Agents
In modules/status_check/default/0.1/main.tf around lines 3 to 8, the module
source uses an open-ended Git reference which causes nondeterministic builds and
triggers Checkov warnings. Update the source attribute to include a specific tag
or commit hash to pin the module version. Apply this change consistently to all
uptime_*_checks module blocks at lines 3-8, 36-41, 62-67, and 88-93 to ensure
reproducible builds.
| plugins: | ||
| - '@semantic-release/commit-analyzer' | ||
| - 'semantic-release-commitlint' | ||
| - | ||
| - "@semantic-release/release-notes-generator" | ||
| - preset: "conventionalcommits" | ||
| - | ||
| - "@semantic-release/git" | ||
| - assets: [] | ||
| messsage: false | ||
| - | ||
| - "@semantic-release/github" | ||
| - successComment: false | ||
| failComment: false | ||
| - | ||
| - '@eshepelyuk/semantic-release-helm-oci' | ||
| - registry: oci://ghcr.io/schemahero/helm | ||
| skipAppVersion: true |
There was a problem hiding this comment.
YAML structure breaks Semantic-Release – plugin entries & “messsage” typo
Several list items are malformed (nested array instead of [plugin, options]) and the key messsage is misspelled, so Semantic-Release will crash on startup.
Minimal fix:
plugins:
- '@semantic-release/commit-analyzer'
- 'semantic-release-commitlint'
- -
- - "@semantic-release/release-notes-generator"
- - preset: "conventionalcommits"
- -
- - "@semantic-release/git"
- - assets: []
- messsage: false
- -
- - "@semantic-release/github"
- - successComment: false
- failComment: false
- -
- - '@eshepelyuk/semantic-release-helm-oci'
- - registry: oci://ghcr.io/schemahero/helm
- skipAppVersion: true
+ - ['@semantic-release/release-notes-generator', {preset: 'conventionalcommits'}]
+ - ['@semantic-release/git', {assets: [], message: false}]
+ - ['@semantic-release/github', {successComment: false, failComment: false}]
+ - ['@eshepelyuk/semantic-release-helm-oci', {registry: 'oci://ghcr.io/schemahero/helm', skipAppVersion: true}]Without this change, CI/CD will fail at the release step.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| plugins: | |
| - '@semantic-release/commit-analyzer' | |
| - 'semantic-release-commitlint' | |
| - | |
| - "@semantic-release/release-notes-generator" | |
| - preset: "conventionalcommits" | |
| - | |
| - "@semantic-release/git" | |
| - assets: [] | |
| messsage: false | |
| - | |
| - "@semantic-release/github" | |
| - successComment: false | |
| failComment: false | |
| - | |
| - '@eshepelyuk/semantic-release-helm-oci' | |
| - registry: oci://ghcr.io/schemahero/helm | |
| skipAppVersion: true | |
| plugins: | |
| - '@semantic-release/commit-analyzer' | |
| - 'semantic-release-commitlint' | |
| - ['@semantic-release/release-notes-generator', {preset: 'conventionalcommits'}] | |
| - ['@semantic-release/git', {assets: [], message: false}] | |
| - ['@semantic-release/github', {successComment: false, failComment: false}] | |
| - ['@eshepelyuk/semantic-release-helm-oci', {registry: 'oci://ghcr.io/schemahero/helm', skipAppVersion: true}] |
🤖 Prompt for AI Agents
In modules/schemahero_controller/schemahero-helm/.releaserc.yaml between lines 6
and 23, the YAML structure for the semantic-release plugins is incorrect because
some plugins are defined as nested arrays instead of the correct [plugin,
options] format, and there is a typo in the key "messsage" which should be
"message". Fix the YAML by converting nested arrays into proper mappings with
plugin names as keys and their options as values, and correct the typo
"messsage" to "message" to ensure semantic-release parses the config correctly
and does not crash.
| cpu: | ||
| type: string | ||
| title: CPU Limit | ||
| description: CPU resource limit for the schemahero controller | ||
| default: 100m | ||
| pattern: ^[0-9]+[m]?$ | ||
| memory: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
CPU pattern too restrictive for valid Kubernetes quantities
^[0-9]+[m]?$ forbids fractional cores such as 0.5, which are valid CPU requests/limits.
Consider a more permissive regex (or simply rely on Kubernetes validation).
- pattern: ^[0-9]+[m]?$
+ # Accept integers, millicores (m) and decimal cores (e.g. 0.5)
+ pattern: ^([0-9]+m?|[0-9]*\.[0-9]+)$📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cpu: | |
| type: string | |
| title: CPU Limit | |
| description: CPU resource limit for the schemahero controller | |
| default: 100m | |
| pattern: ^[0-9]+[m]?$ | |
| memory: | |
| cpu: | |
| type: string | |
| title: CPU Limit | |
| description: CPU resource limit for the schemahero controller | |
| default: 100m | |
| # Accept integers, millicores (m) and decimal cores (e.g. 0.5) | |
| pattern: ^([0-9]+m?|[0-9]*\.[0-9]+)$ | |
| memory: |
🤖 Prompt for AI Agents
In modules/schemahero_controller/facets.yaml around lines 18 to 24, the regex
pattern for the CPU limit field is too restrictive and disallows valid
Kubernetes CPU quantities like fractional cores (e.g., 0.5). Update the pattern
to allow decimal numbers with optional 'm' suffix or remove the pattern entirely
to rely on Kubernetes native validation for CPU resource quantities.
| columns = local.spec.connection == "mysql" ? jsonencode([for v in local.spec.columns : | ||
| merge({ | ||
| name = v.name | ||
| type = v.type | ||
| constraints = lookup(v, "constraints", {}) | ||
| attributes = lookup(v, "attributes", {}) | ||
| }, lookup(v, "default", "") != "" ? { default = v.default } : {}) | ||
| ]) : jsonencode([for v in local.spec.columns : | ||
| merge({ | ||
| name = v.name | ||
| type = v.type | ||
| constraints = lookup(v, "constraints", {}) | ||
| }, lookup(v, "default", "") != "" ? { default = v.default } : {}) | ||
| ]) |
There was a problem hiding this comment.
Double jsonencode → jsondecode round-trip is unnecessary and error-prone
local.columns is first JSON-encoded and later JSON-decoded (line 48).
This extra serialisation:
- Wastes CPU cycles on every plan/apply.
- Obscures the real data shape (list → string → list).
- Risks silent type drift if SchemaHero changes expectations.
Keep the value as a list and let yamlencode in the Helm set block handle the final serialisation.
- columns = local.spec.connection == "mysql" ? jsonencode([for v in local.spec.columns :
+ columns = local.spec.connection == "mysql" ? [for v in local.spec.columns :
...
- ]) : jsonencode([for v in local.spec.columns :
+ ] : [for v in local.spec.columns :
...
- columns = jsondecode(local.columns)
+ columns = local.columns🤖 Prompt for AI Agents
In modules/schemahero_table/k8s/0.1/main.tf around lines 5 to 18, the code
unnecessarily JSON-encodes the columns list, which is later JSON-decoded,
causing inefficiency and potential type issues. Remove the jsonencode calls so
that the columns attribute remains a list of maps directly. This avoids
redundant serialization and keeps the data structure intact for further
processing like yamlencode in Helm.
| - deployments/status | ||
| - statefulset/status | ||
| verbs: |
There was a problem hiding this comment.
Pluralisation typo: statefulset/status → statefulsets/status
Kubernetes sub-resource paths must match the plural form.
Using the singular form prevents RBAC evaluation and the controller will receive 403s when patching StatefulSet status.
- - statefulset/status
+ - statefulsets/status📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - deployments/status | |
| - statefulset/status | |
| verbs: | |
| - deployments/status | |
| - statefulsets/status | |
| verbs: |
🤖 Prompt for AI Agents
In modules/schemahero_controller/schemahero-helm/templates/cluster-role.yaml at
lines 22 to 24, the resource path 'statefulset/status' is incorrectly singular.
Change it to the correct plural form 'statefulsets/status' to ensure proper
Kubernetes RBAC evaluation and avoid 403 errors when patching StatefulSet
status.
| json:triggers: | ||
| items: | ||
| properties: | ||
| arguments: | ||
| items: | ||
| type: string | ||
| type: array | ||
| condition: | ||
| type: string | ||
| constraintTrigger: | ||
| type: boolean | ||
| events: | ||
| items: | ||
| type: string | ||
| type: array | ||
| executeProcedure: | ||
| type: string | ||
| forEachRun: | ||
| type: boolean | ||
| forEachStatement: | ||
| type: boolean | ||
| name: | ||
| type: string | ||
| required: | ||
| - events | ||
| - executeProcedure | ||
| type: object | ||
| type: array | ||
| primaryKey: |
There was a problem hiding this comment.
Invalid property name json:triggers
The property is defined twice with a literal json: prefix (json:triggers).
This colon becomes part of the actual field name, so the controller will never match a spec that uses the documented triggers property.
It also violates the go-text/template tag expectations if generated code assumes triggers.
- json:triggers:
+ triggers:Apply the same fix for the second occurrence (Postgres block).
Also applies to: 399-427
🤖 Prompt for AI Agents
In
modules/schemahero_controller/schemahero-helm/crds/schemas.schemahero.io_tables.yaml
around lines 207 to 235 and also lines 399 to 427, the property name is
incorrectly defined as `json:triggers` with a colon prefix, which causes the
field name to include the literal `json:` and breaks matching and code
generation. Remove the `json:` prefix so the property is simply named `triggers`
to align with the expected spec and template usage. Apply this fix to both
occurrences in the specified line ranges.
| primary_key: | ||
| type: object | ||
| title: Primary Key Columns | ||
| description: Definition of primary key columns | ||
| x-ui-yaml-editor: true |
There was a problem hiding this comment.
primary_key type mismatch between spec and sample
spec.primary_key is declared as an object, yet the sample manifest supplies an array (- code). This will fail JSON-schema validation and block facet rendering.
- primary_key:
- type: object
+ primary_key:
+ type: array
+ items:
+ type: stringAlso applies to: 106-107
🤖 Prompt for AI Agents
In modules/schemahero_table/k8s/0.1/facets.yaml around lines 35 to 39 and also
lines 106 to 107, the primary_key field is declared as an object in the spec but
the sample manifest uses an array, causing a type mismatch and JSON-schema
validation failure. To fix this, change the type of primary_key in the spec from
object to array to match the sample manifest structure, ensuring consistency and
successful validation.
| intent: schemahero-controller | ||
| flavor: default | ||
| version: "0.1" | ||
| description: "Deploys SchemaHero controller for automated database schema management in Kubernetes environments" | ||
| lifecycle: ENVIRONMENT_BOOTSTRAP | ||
| clouds: | ||
| - aws | ||
| - gcp | ||
| - azure | ||
| - kubernetes | ||
| - kubernetes | ||
| conditional_on_intent: schemahero_database | ||
| input_type: config |
There was a problem hiding this comment.
Directory / intent / dependency triad is inconsistent
The file lives under schemahero_database/… but declares
intent: schemahero-controller and conditional_on_intent: schemahero_database.
This circular coupling will confuse intent resolution and may produce undeployable plans.
Either move the file to schemahero_controller/ or revert intent / conditional_on_intent so path-to-intent mapping stays 1-to-1.
🤖 Prompt for AI Agents
In modules/schemahero_database/k8s/0.1/facets.yaml lines 1 to 9, the intent and
conditional_on_intent fields conflict with the file's directory path, causing
circular dependency issues. To fix this, either move the file to a directory
matching the declared intent "schemahero-controller" or change the intent and
conditional_on_intent values to "schemahero_database" so the directory path and
intent mapping are consistent and avoid deployment problems.
|
Remove module.json from the modules and add release link, screenshot of forms |
Release Link schemahero table - https://facetsdemo.console.facets.cloud/capc/stack/infra-dev/releases/cluster/64b0fe30f8ae8b0007f5f010/dialog/release-details/685a9a7c311e7c3f5866e3bb
Form Screenshot-
<img width="1086" alt="Screenshot 2025-06-25 at 13 43 14" src="http
s://github.com/user-attachments/assets/992635fa-f7fc-45ee-aaa7-c2aab88cd1c0" />
Release Link schemahero-controller - https://facetsdemo.console.facets.cloud/capc/stack/infra-dev/releases/cluster/64b0fe30f8ae8b0007f5f010/dialog/release-details/685a921fe17f5506509d1269
Form Screenshot-
Release Link uptime_monitoring - https://facetsdemo.console.facets.cloud/capc/stack/infra-dev/releases/cluster/64b0fe30f8ae8b0007f5f010/dialog/release-details/68678029111ce3070e643aa7/terminal-dialog/terraform-logs?backUrl=%2Fstack%2Finfra-dev%2Freleases%2Fcluster%2F64b0fe30f8ae8b0007f5f010
Form Screenshot-
Description
Related issues
Type of change
Checklist
developbranchTesting
Reviewer instructions
Summary by CodeRabbit
New Features
Documentation
Chores