Features/database persistence#55
Merged
Merged
Conversation
…micUI-conditional-fields Feat(portal): add conditional database fields in UI for advanced node.js template
…fold-dynamic-wizard-extension [Feat] Add DatabasePicker extension for database selection
…elopment - Introduced a Taskfile to automate setup and development tasks for the Helios Platform. - Added a .env.example file to provide a template for environment variables. - Updated .gitignore to exclude .env files while keeping .env.example. - Enhanced documentation in APP_STARTUP_GUIDE.md and SETUP.md for quick start and manual setup instructions. - Implemented prerequisite check scripts for both Windows and Unix-like systems.
…askfile - Changed GIT_AUTHOR_NAME in .env.example to use quotes for consistency. - Removed the setup:tekton-resources task from Taskfile.yml as it is no longer needed.
…base-picker-component Impl: add databasePicker component logic
… "- Move database provisioning above image validation guard - Fix panic on invalid storage by using ParseQuantity - Add PGPORT env var and update health probes to use custom port - Handle StatefulSet and Service drift by actively updating mutable fields
…stgres-provisioning feat: implement postgres provisioning logic for database trait (#34)
…#39) Add Phase 0.9 to the operator reconcile loop that injects DB_HOST, DB_USER, DB_PASS env vars (via secretKeyRef) into backend Deployments. Create a NestJS + Prisma Backstage template with Prisma 7 driver adapter support. Operator changes: - Add InjectDatabaseEnvVars() and reconcileDatabaseSecretInjection() to patch live Deployments with database credential references - Add 6 unit/integration tests for injection logic (idempotency, empty containers, missing deployments) - Update RBAC role for statefulsets and ingresses - Bump golangci-lint to v2.11.3, CUE language to v0.16.0 NestJS Prisma template: - Full Backstage scaffolder template with NestJS 11 + Prisma 7 + pg adapter - prisma.config.ts for Prisma 7 datasource URL (replaces schema.prisma url) - PrismaService with namespace import (import * as pg) for CommonJS compat - Multi-stage Dockerfile with migration support - Helios GitOps manifest with database trait Documentation: - Manual verification guide for k3d cluster testing - Implementation plan, task tracking, and walkthrough docs - Claude Code skills for implementation workflow and Go/CUE guidelines Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…l-dev-env-script [Feat] Automate local dev setup with Taskfile and k3d
PR review fixes: - Validate existing env var sources in InjectDatabaseEnvVars, not just names - Revert accidental local-dev image tag in kustomization.yaml - Requeue when Deployment is missing for database secret injection - Add ArgoCD ignoreDifferences to prevent self-heal reverting injected env vars - Fix broken NestJS template main.ts syntax - Fix catalog-info.yaml referencing undefined template variables - Remove committed planning docs (task.md, implementation_plan.md, walkthrough.md) Go 1.26.1 modernization: - Update Dockerfile and devcontainer from golang:1.24 to golang:1.26 - Use t.Context() instead of context.Background() in tests - Use for-range over integers instead of C-style loops - Use strings.SplitSeq instead of strings.Split in for-range - Update Go version references in documentation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…mated-secret-injection
…ated-secret-injection feat: implement automated secret injection and NestJS Prisma template…
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThêm provisioning cơ sở dữ liệu và tiêm bí mật tự động: operator sinh credentials, tạo Secret + StatefulSet Postgres, inject env vars vào Deployment; mở rộng reconciler với pha database; bổ sung template NestJS+Prisma, Taskfile, script kiểm tra, docs và cập nhật RBAC/tooling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Controller as "HeliosApp Controller"
participant DB as "Database Manager"
participant K8s as "Kubernetes API"
participant App as "Backend Deployment"
User->>Controller: Apply HeliosApp với database trait
Controller->>DB: Extract database traits
DB->>DB: Generate secure credentials
DB->>K8s: Create Secret (DB_USER, DB_PASS, DB_HOST)
K8s-->>Controller: Secret created
Controller->>DB: Provision Postgres StatefulSet & Service
DB->>K8s: Create StatefulSet + Service
K8s-->>Controller: Database ready
Controller->>K8s: Get Backend Deployment
K8s-->>Controller: Return Deployment / NotFound
Controller->>DB: Inject DB env vars (secretKeyRef) or mark pending
DB->>K8s: Patch Deployment when present
K8s-->>App: Deployment updated
Controller->>User: Reconcile complete or requeue if pending
sequenceDiagram
participant Portal as "Backstage Portal"
participant Scaffolder as "Scaffolder Engine"
participant Extension as "DatabasePicker Extension"
participant Template as "Template Executor"
participant GitHub as "GitHub"
participant K8s as "Kubernetes API"
Portal->>Scaffolder: Mở form Create
Scaffolder->>Extension: Render DatabasePicker
Extension-->>Scaffolder: dbType / dbName chọn xong
Scaffolder->>Template: Execute nestjs-prisma-template với DB inputs
Template->>GitHub: Publish source + gitops repos
GitHub-->>Template: Repos created
Template->>K8s: Apply HeliosApp (có database trait)
K8s-->>Template: HeliosApp created
Template->>Portal: Trả links & register catalog entry
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Implements end-to-end “database persistence” for Helios by provisioning per-component PostgreSQL instances, generating/injecting credentials at runtime, and adding supporting CUE/Backstage scaffolder/template + developer tooling/docs.
Changes:
- Operator: add reconcile phases for DB Secret generation, Postgres StatefulSet/Service provisioning, and live Deployment env injection; adjust RBAC and ArgoCD diff handling.
- CUE: add/update
databasetrait output (ConfigMap metadata) and bump CUE language/runtime versions. - Portal/devex: add Backstage
DatabasePickerfield extension, introduce a NestJS+Prisma template, and add Taskfile + prerequisite checker scripts + docs.
Reviewed changes
Copilot reviewed 48 out of 51 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/start-portal.ps1 | Generates ArgoCD token on Windows before starting Backstage |
| scripts/check-prereqs.sh | Cross-platform prereq checker for local dev (bash) |
| scripts/check-prereqs.bat | Windows prereq checker (cmd) |
| docs/manual-verification-guide.md | Manual verification steps for DB secret injection/provisioning |
| docs/SETUP.md | Updates setup prerequisites and k3d instructions |
| docs/OPERATOR.md | Updates operator prerequisites (Go version) |
| docs/APP_STARTUP_GUIDE.md | Adds Task-based quick start and k3d-based manual steps |
| cue/definitions/traits/database.cue | Updates database trait outputs/conventions and clarifies responsibilities |
| cue/cue.mod/module.cue | Bumps CUE language version |
| apps/portal/packages/app/src/scaffolder/index.ts | Exports scaffolder field extension entrypoint |
| apps/portal/packages/app/src/scaffolder/DatabasePickerExtension/index.ts | Barrel exports for DatabasePicker extension |
| apps/portal/packages/app/src/scaffolder/DatabasePickerExtension/extension.ts | Registers DatabasePicker scaffolder field extension |
| apps/portal/packages/app/src/scaffolder/DatabasePickerExtension/DatabasePicker.tsx | Implements UI for choosing DB type/name |
| apps/portal/packages/app/src/App.tsx | Wires scaffolder field extensions into /create route |
| apps/portal/examples/nestjs-prisma-template/template.yaml | Adds new Backstage scaffolder template for NestJS+Prisma |
| apps/portal/examples/nestjs-prisma-template/content/source/tsconfig.json | Template TypeScript config |
| apps/portal/examples/nestjs-prisma-template/content/source/tsconfig.build.json | Template build tsconfig |
| apps/portal/examples/nestjs-prisma-template/content/source/src/prisma/prisma.service.ts | Template Prisma service using env-injected DB creds |
| apps/portal/examples/nestjs-prisma-template/content/source/src/prisma/prisma.module.ts | Template Prisma module |
| apps/portal/examples/nestjs-prisma-template/content/source/src/main.ts | Template NestJS bootstrap |
| apps/portal/examples/nestjs-prisma-template/content/source/src/app.service.ts | Template app service |
| apps/portal/examples/nestjs-prisma-template/content/source/src/app.module.ts | Template app module |
| apps/portal/examples/nestjs-prisma-template/content/source/src/app.controller.ts | Template app controller incl. health endpoint |
| apps/portal/examples/nestjs-prisma-template/content/source/prisma/schema.prisma | Template Prisma schema |
| apps/portal/examples/nestjs-prisma-template/content/source/prisma/migrations/migration_lock.toml | Template Prisma migrate lockfile |
| apps/portal/examples/nestjs-prisma-template/content/source/prisma.config.ts | Template Prisma CLI config |
| apps/portal/examples/nestjs-prisma-template/content/source/package.json | Template dependencies/scripts |
| apps/portal/examples/nestjs-prisma-template/content/source/nest-cli.json | Template Nest CLI config |
| apps/portal/examples/nestjs-prisma-template/content/source/catalog-info.yaml | Template Backstage catalog descriptor |
| apps/portal/examples/nestjs-prisma-template/content/source/Dockerfile | Template multi-stage Docker build |
| apps/portal/examples/nestjs-prisma-template/content/source/.env.example | Template env example for DB/app vars |
| apps/portal/examples/nestjs-prisma-template/content/gitops/helios-app.yaml | Template HeliosApp manifest (GitOps) |
| apps/portal/examples/advanced-template/template.yaml | Adds DB picker step to advanced template wizard |
| apps/operator/test/utils/utils.go | Updates line-splitting helper for tests |
| apps/operator/internal/controller/suite_test.go | Improves envtest binary discovery |
| apps/operator/internal/controller/heliosapp_controller_unit_test.go | Uses t.Context() in unit tests |
| apps/operator/internal/controller/heliosapp_controller.go | Adds DB phases + requeue + StatefulSet ownership + RBAC update |
| apps/operator/internal/controller/database_resources_test.go | Adds extensive tests for DB secret/provisioning/injection helpers |
| apps/operator/internal/controller/database_resources.go | Implements DB secret generation, Postgres provisioning, and env injection logic |
| apps/operator/internal/controller/argocd_resources.go | Adds ArgoCD ignoreDifferences to prevent revert of injected env vars |
| apps/operator/go.sum | Dependency updates (CUE/controller-runtime/etc.) |
| apps/operator/go.mod | Bumps Go/CUE/controller-runtime versions |
| apps/operator/config/rbac/role.yaml | Expands RBAC for secrets/statefulsets |
| apps/operator/README.md | Updates Go prerequisite version |
| apps/operator/Makefile | Bumps golangci-lint version |
| apps/operator/Dockerfile | Updates builder Go image |
| apps/operator/.devcontainer/devcontainer.json | Updates devcontainer Go image |
| Taskfile.yml | Adds root Taskfile for check/setup/dev/test/clean workflows |
| .gitignore | Ignores .env, keeps .env.example |
| .env.example | Adds repository-root environment variable template |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 33
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitignore:
- Around line 5-7: Add a glob to ignore all .env variants so secrets like
.env.local or .env.development aren't committed: update the .gitignore entries
that currently list ".env" and "!.env.example" to also include ".env.*" (keeping
the existing exception for ".env.example"). Ensure the pattern ".env.*" appears
alongside the existing ".env" entry and that the "!.env.example" negate rule
remains so the example file is still tracked.
In `@apps/operator/internal/controller/argocd_resources.go`:
- Around line 48-58: The current ignoreDifferences entry skips the entire env
array for containers/0 which is brittle; replace the jsonPointers entry with a
jqPathExpressions rule that matches any container and only ignores env vars
whose name starts with "DB_". Update the map under "ignoreDifferences" (the one
containing "group":"apps" and "kind":"Deployment") to remove the
"/spec/template/spec/containers/0/env" jsonPointers and add a
"jqPathExpressions" array that uses a select() expression targeting
.spec.template.spec.containers[].env[] | select(.name | test("^DB_")) so only
DB_* env vars are ignored and container ordering is no longer an issue.
In `@apps/operator/internal/controller/database_resources.go`:
- Around line 464-469: InjectDatabaseEnvVars is incorrectly always selecting
deploy.Spec.Template.Spec.Containers[0], which breaks deployments with sidecars;
instead, locate the proper app container by iterating
deploy.Spec.Template.Spec.Containers and matching the container name to the
deployment/component identity available on the PodTemplate (e.g. compare
container.Name to deploy.Name or to labels like "app" or
"app.kubernetes.io/component" on deploy.Spec.Template.ObjectMeta.Labels), set
the DB_* env vars on that matched container, and only fall back to index 0 if no
match is found; update references to the local variable container and any later
code that assumes containers[0] accordingly.
- Around line 230-243: The current reconcile early-continues when an
existingSecret (checked via r.Get for secretName) is present, but it doesn't
validate that required keys DB_USER, DB_PASS, and DB_HOST exist; update the
logic in the reconcile loop around existingSecret so that after r.Get succeeds
you validate those keys in existingSecret.Data (or existingSecret.StringData),
and only skip creation if all required keys are present; if any key is missing,
proceed to create/patch the secret (or log and requeue) so secretKeyRef
references used later (from dbTrait.ComponentName) point to valid keys rather
than leaving the reconciler to "skip" on a drifted/manual secret.
- Around line 684-701: ReadinessProbe and LivenessProbe currently use
ExecAction.Command with literal $(POSTGRES_USER)/$(PGPORT) which are not
shell-expanded by kubelet; update both Probe definitions (the corev1.Probe
blocks that set ProbeHandler.Exec.Command) to invoke a shell so environment
variables are expanded: replace the Command slice with ["sh","-ec","pg_isready
-U $POSTGRES_USER -d <dbName> -p $PGPORT"] (use the dbName variable already used
in the template) so the shell expands $POSTGRES_USER and $PGPORT; apply the same
change to both ReadinessProbe and LivenessProbe ExecAction.Command entries.
In `@apps/operator/internal/controller/heliosapp_controller.go`:
- Around line 542-547: The controller currently sets owner refs for DB secrets
(see database_resources.go) but the controller builder in
heliosapp_controller.go doesn't watch Secret objects, so rotations or deletions
of {component}-db-secret won't trigger reconciles; update the controller builder
chain in the Reconciler setup (the
ctrl.NewControllerManagedBy(...).For(...).Owns(...)) to include
Owns(&corev1.Secret{}), and ensure corev1 is imported; alternatively, if you
prefer targeted watches, extend the secret lookup in findObjectsForSecret to
also match the `{component}-db-secret` naming pattern, but the simplest fix is
adding Owns(&corev1.Secret{}) to the builder.
In `@apps/operator/internal/controller/suite_test.go`:
- Around line 106-115: The WalkDir callback currently swallows traversal errors;
update the filepath.WalkDir usage so any error returned by WalkDir is captured
(e.g., assign the WalkDir call result to walkErr) and logged (use the test
logger like t.Logf or t.Errorf) while preserving the existing fallback behavior
(still skip unreadable dirs in the callback and keep setting found when the etcd
binary is located via found and filepath.SkipAll). Specifically, modify the
anonymous function passed to filepath.WalkDir to return nil for unreadable dirs
but let WalkDir surface other errors to be assigned to walkErr, then after
calling filepath.WalkDir log that walkErr if non-nil.
In `@apps/portal/examples/advanced-template/template.yaml`:
- Around line 46-53: The template defines a databaseConfig UI field
(DatabasePicker) but never wires it into the steps, so user choices are ignored;
update the wizard steps to propagate databaseConfig into the fetch-gitops step
(and fetch-source if relevant) by adding the databaseConfig parameter/reference
to the step inputs for the fetch-gitops step so the operator can read it when
creating the database trait—look for symbols databaseConfig, fetch-gitops,
fetch-source, and DatabasePicker in template.yaml and add the parameter binding
so fetch-gitops receives the selected values.
In `@apps/portal/examples/nestjs-prisma-template/content/source/.env.example`:
- Around line 1-9: Add surrounding quotes to the DB_NAME value in the
.env.example template so the rendered value `${{ values.name }}-db` is quoted;
update the DB_NAME entry to use "..." (keep the existing `${{ values.name }}`
placeholder intact) to ensure the final environment variable is a single quoted
string.
In
`@apps/portal/examples/nestjs-prisma-template/content/source/catalog-info.yaml`:
- Line 7: Giải thích ngắn: trường values.owner trả về Backstage entity reference
(ví dụ "user:john") nhưng annotation github.com/project-slug cần GitHub
owner/org; sửa bằng cách không gán trực tiếp values.owner vào
github.com/project-slug. Hướng dẫn sửa: thêm một trường input mới (ví dụ
values.githubOwner hoặc values.repoOwner) trong template để nhận GitHub
owner/org từ người dùng, hoặc bỏ annotation github.com/project-slug nếu không
cần; cập nhật mọi chỗ tham chiếu hiện tại từ values.owner sang
values.githubOwner cho annotation github.com/project-slug; tham khảo các tên duy
nhất trong diff: values.owner, github.com/project-slug, spec.owner và
OwnerPicker để xác định vị trí cần thay đổi.
In `@apps/portal/examples/nestjs-prisma-template/content/source/Dockerfile`:
- Around line 39-40: Add a Docker HEALTHCHECK to the runtime image so the
orchestrator can detect hung or failed instances; after the existing CMD ["npm",
"run", "start:migrate:prod"] add a HEALTHCHECK using CMD-SHELL that queries your
app's liveness endpoint (e.g., GET /health or /healthz on the app port) with a
short timeout and sensible interval/retries/start-period (for example: interval
30s, timeout 5s, start-period 30s, retries 3) so the container returns non-zero
on failure; ensure the endpoint used matches the app in package scripts and the
server port.
- Around line 20-40: Container runs as root at runtime; modify the Dockerfile to
create a non-root user, change ownership of /app and any required files, and
switch to that user before CMD. Specifically: after copying files (after the
COPY --from=builder ... /app/dist ./dist/ step) add steps to create a group/user
(e.g., addgroup/appadduser or use the built-in node user), chown -R /app to that
user, and then set USER <username|uid> so the CMD
["npm","run","start:migrate:prod"] executes without root privileges; ensure
WORKDIR /app and the copied Prisma/node_modules files are owned by the non-root
user so runtime writes (migrations, logs) succeed.
In `@apps/portal/examples/nestjs-prisma-template/content/source/package.json`:
- Line 13: Script "lint" calls eslint but eslint and the TypeScript
plugins/configs are missing from devDependencies; update package.json to add
"eslint" plus the needed TypeScript-related packages (e.g.,
"@typescript-eslint/parser", "@typescript-eslint/eslint-plugin" and any shared
eslint-config used) into devDependencies so npm run lint can run successfully,
ensuring the "lint" script (named "lint") remains unchanged and that versions
are compatible with the project TypeScript/Node setup.
- Line 12: The script "start:migrate:prod" runs "npx prisma migrate deploy" but
the Prisma CLI is only in devDependencies, so production installs will lack the
runtime binary and the container will fail; fix by ensuring the Prisma CLI is
available at runtime — either move "prisma" from devDependencies to
dependencies, or remove the runtime migration call from the start script and run
migrations during build/startup outside the container (e.g., Dockerfile or an
entrypoint/migration script) so that start:migrate:prod no longer depends on a
dev-only package; update package.json's "start:migrate:prod" or dependency list
accordingly and keep the symbol names "start:migrate:prod" and "prisma" in mind
when making the change.
In `@apps/portal/examples/nestjs-prisma-template/content/source/prisma.config.ts`:
- Around line 9-17: The buildDatabaseUrl function duplicates logic that also
exists in src/prisma/prisma.service.ts; extract this URL-building logic into a
single shared helper (e.g., export a buildDatabaseUrl utility) and replace the
inline implementations in both buildDatabaseUrl (current file) and the
corresponding method in PrismaService with calls to that helper; ensure the
helper preserves behavior (env keys DB_HOST/DB_USER/DB_PASS/DB_NAME/DB_PORT,
encodeURIComponent on DB_PASS, default values, and the ?schema=public suffix)
and export/import it from the shared location so both modules use the same
implementation.
- Around line 10-15: The current defaults for DB config (host, user, pass, name,
port) silently fall back to localhost/postgres; change this to fail-fast in
production by removing implicit defaults and validating
DB_HOST/DB_USER/DB_PASS/DB_NAME/DB_PORT when NODE_ENV === 'production' (or an
equivalent production flag) inside prisma.config.ts: read env vars into
host/user/pass/name/port (apply encodeURIComponent to pass only if present), and
if any required var is missing in production throw a descriptive Error
(including which var is missing) so the app fails to start instead of connecting
to a wrong DB; keep permissive defaults only for non-production environments if
desired.
In
`@apps/portal/examples/nestjs-prisma-template/content/source/src/app.service.ts`:
- Line 6: PrismaService được injected vào constructor của AppService nhưng biến
prisma không được sử dụng, gây dead code; either remove the injection or
demonstrate usage—update AppService by either (A) removing the constructor
parameter "private readonly prisma: PrismaService" if Prisma isn't needed, or
(B) keep the injection and use prisma inside a method like healthCheck (e.g.,
call prisma.$queryRaw or prisma.user.count to validate DB connectivity) and add
a short comment showing it's a template example; refer to the constructor,
PrismaService, prisma, and healthCheck to locate where to change.
In `@apps/portal/examples/nestjs-prisma-template/content/source/src/main.ts`:
- Line 10: Replace the direct console.log call in main.ts with NestJS Logger:
import Logger from '@nestjs/common', create a Logger instance (e.g., new
Logger('Bootstrap') or use Logger.log) and replace console.log(`Application is
running on port ${port}`) with logger.log(...) so logs follow NestJS formatting
and levels; ensure the Logger instance is used consistently for
bootstrap/startup messages.
In
`@apps/portal/examples/nestjs-prisma-template/content/source/src/prisma/prisma.service.ts`:
- Around line 1-4: Service đang fallback về database tên "postgres" thay vì dùng
DB đã provisioned; update PrismaService so that buildDatabaseUrl() and the
PrismaClient datasource use the provisioned DB name (read from the injected env
var used by the scaffold, e.g. DB_NAME or the component-specific env) instead of
a hardcoded default, and ensure PrismaPg/pg connection setup and any
migrate/query paths reference that same URL; specifically modify
buildDatabaseUrl(), the PrismaService constructor/initializer where PrismaClient
is instantiated, and any places using PrismaPg to derive the connection string
from the provisioned DB name.
In `@apps/portal/examples/nestjs-prisma-template/template.yaml`:
- Around line 48-53: The template declares a databaseConfig UI field (ui:field:
DatabasePicker) but no workflow step forwards that value into the scaffold
steps; update the workflow so the value from the databaseConfig form is passed
into the fetch-source and/or fetch-gitops steps (or into an intermediate step)
as an input/output mapping and used when generating the scaffold outputs;
specifically locate the DatabasePicker usage in template.yaml (property
databaseConfig) and add wiring so fetch-source and fetch-gitops read the
databaseConfig input (or consume an output produced by a step that maps
databaseConfig) so changes in the wizard actually affect the generated scaffold.
- Around line 15-46: The schema currently defines properties dockerOrg and
repoName but doesn't require them, causing invalid image refs when left empty;
update the Component Information schema by adding "dockerOrg" and "repoName" to
the required array (the same place that currently lists name, owner, port) so
the form enforces values for properties dockerOrg and repoName and prevents
generating manifests with empty image references.
In
`@apps/portal/packages/app/src/scaffolder/DatabasePickerExtension/extension.ts`:
- Around line 11-14: Khi kiểm tra PostgreSQL (value?.dbType === 'postgres') hiện
chỉ kiểm tra sự tồn tại của value?.dbName mà chưa chặn chuỗi chỉ chứa khoảng
trắng; thay điều kiện thành kiểm tra sau khi trim: nếu value?.dbName không tồn
tại hoặc value.dbName.trim().length === 0 thì gọi validation.addError('Database
Name is required when PostgreSQL is selected'); đảm bảo dùng value.dbName.trim()
để xác định chuỗi rỗng sau khi loại bỏ khoảng trắng.
In `@cue/definitions/traits/database.cue`:
- Around line 12-13: Update the inline comment that references the wrong issue
ID: change the parenthetical "(see issue `#33`)" to "(see issue `#45`)" in the
comment that starts "Credentials (Secret) are generated by the
HeliosAppReconciler" so the comment for the credentials correctly points to
issue `#45`; search for and replace any other occurrences of the same incorrect
issue reference in the same file (the comment text around
HeliosAppReconciler/Credentials) to keep traceability consistent.
In `@docs/APP_STARTUP_GUIDE.md`:
- Around line 366-372: The table entry for `task setup` is misleading — update
the table row for `task setup` (and optionally `task dev`) to clarify that `task
setup` performs steps 1–8 (environment/cluster setup) but does not start the
portal/operator; explicitly state that the app is started with `task dev` (or
add “does not start services; run `task dev` to run operator + portal”) so new
users won't expect the app to be running after setup.
In `@docs/manual-verification-guide.md`:
- Around line 37-42: The shell snippets are GNU-specific: the PORT extraction
uses grep -oP (PCRE) and other steps use base64 -d which fails on macOS BSD;
update the PORT command (the line with PORT=$(kubectl config view -o
jsonpath='{.clusters[?(@.name=="k3d-helios-test")].cluster.server}' | grep -oP
':\K\d+$')) to a POSIX-friendly extraction such as piping to sed -E
's/.*:([0-9]+)$/\1/' or awk -F: '{print $NF}', and replace any uses of base64 -d
with a portable decode fallback (e.g., try base64 --decode, then base64 -D for
macOS, or use openssl base64 -d / a short python3 -c 'import
sys,base64;print(base64.b64decode(sys.stdin.read()).decode())') so the manual
works on both GNU/Linux and macOS.
In `@scripts/check-prereqs.bat`:
- Around line 1-155: The batch file is using LF-only line endings which breaks
Windows batch parsing (affecting labels like :check_tool and :check_env_var and
CALL behavior); convert this file to CRLF line endings, commit the change, and
prevent regressions by adding a .gitattributes entry (e.g. for *.bat set
eol=crlf) or ensure core.autocrlf is configured so future edits keep CRLF for
scripts like this; after conversion verify the script runs and labels
(:check_tool, :check_env_var) resolve correctly.
- Line 11: The script enables delayed expansion globally with setlocal
enabledelayedexpansion which causes any '!' in .env values to be lost when
parsing (see the for /f ... loop that reads ENV_FILE and the later lookup using
set "val=!%~1!"). Fix by ensuring delayed expansion is not active while
loading/parsing the .env: move the setlocal enabledelayedexpansion call to after
the .env parsing block or wrap the .env read loop with an endlocal before
parsing and then re-enable delayed expansion (setlocal enabledelayedexpansion)
afterward so that the for /f loop and subsequent set "line=..."/set "%%i=%%j"
operations run without delayed expansion enabled.
In `@scripts/check-prereqs.sh`:
- Around line 70-72: The Go version check in the prereq script uses check_tool
with a minimum of "1.24" but the operator module requires Go 1.26.1; update the
check_tool invocation for "go" to require "1.26.1" (i.e., change the second
argument passed to check_tool for "go" from "1.24" to "1.26.1") so the prereq
check matches the operator's go.mod requirement and prevents users from passing
prereqs but failing the operator build.
- Around line 132-133: Không dùng `source "$ENV_FILE"` vì nó thực thi bất kỳ
shell code trong file; thay vào đó, read và parse `ENV_FILE` an toàn: mở
`$ENV_FILE`, lọc bỏ comment/blank lines, validate mỗi dòng theo pattern đơn giản
`^[A-Za-z_][A-Za-z0-9_]*=.+$`, và chỉ export các biến nằm trong mảng
REQUIRED_VARS (tham chiếu đến REQUIRED_VARS và ENV_FILE trong diff) hoặc sử dụng
một tool an toàn như direnv/dotenv-linter/dotenv-validator để load/validate; cập
nhật script để thực hiện parsing/validation thay vì source để tránh thực thi
code độc hại.
- Around line 71-97: The version-extraction commands used by the check_tool
invocations (e.g., the commands for "go", "docker", "kubectl", "k3d", "cue",
"node", "yarn") rely on GNU-only grep -oP which breaks on macOS; update each
check_tool command to use a portable extraction method (e.g., use grep -E or sed
-E with a POSIX ERE, or a short perl one-liner) instead of grep -oP so the
script works with BSD grep on macOS (ensure the patterns still capture the same
numeric groups and pipe to head -1 where needed).
In `@scripts/start-portal.ps1`:
- Around line 21-27: Thay thế các lệnh Write-Host trong khối tạo token ArgoCD
bằng các stream PowerShell phù hợp: dùng Write-Information cho thông báo thành
công ("ArgoCD token generated."), Write-Warning cho các cảnh báo ("Could not
generate ArgoCD token. ArgoCD features may not work.") và Write-Error trong
catch để ghi lỗi chi tiết (bao gồm $_). Cập nhật tất cả các gọi Write-Host trong
cùng try/catch (tìm theo biểu tượng Write-Host và khối catch) để dùng
Write-Information/Write-Warning/Write-Error tương ứng và đảm bảo thông điệp vẫn
rõ ràng.
- Around line 13-18: The script currently sets
[System.Net.ServicePointManager]::ServerCertificateValidationCallback = { $true
} globally and never restores it, disabling TLS validation for the entire
process; change this to save the original callback into a local variable, set
ServerCertificateValidationCallback to { $true } only immediately before the
Invoke-RestMethod call that uses $ArgocdPort and $body, wrap that call in a
try/finally, and in the finally restore the saved original callback so
subsequent steps (like yarn start) use the original TLS validation.
In `@Taskfile.yml`:
- Around line 219-231: The ARGOCD password decoding uses GNU-only `base64 -d`
which fails on macOS; update the command that sets ARGOCD_PASS (the base64
invocation) to be portable: try `base64 --decode` or fallback to `base64 -D`, or
detect Darwin via `uname` and use `-D` there, so the ARGOCD_PASS extraction line
and subsequent token generation (the TOKEN_JSON and ARGOCD_AUTH_TOKEN logic)
work on both Linux and macOS; modify the ARGOCD_PASS assignment in the
dev:portal:start command to implement this fallback/detection approach.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 54aa0627-68d9-4ff1-ba7c-e84a17c4cf17
⛔ Files ignored due to path filters (2)
apps/operator/go.sumis excluded by!**/*.sumapps/portal/examples/nestjs-prisma-template/content/source/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (49)
.env.example.gitignoreTaskfile.ymlapps/operator/.devcontainer/devcontainer.jsonapps/operator/Dockerfileapps/operator/Makefileapps/operator/README.mdapps/operator/config/rbac/role.yamlapps/operator/go.modapps/operator/internal/controller/argocd_resources.goapps/operator/internal/controller/database_resources.goapps/operator/internal/controller/database_resources_test.goapps/operator/internal/controller/heliosapp_controller.goapps/operator/internal/controller/heliosapp_controller_unit_test.goapps/operator/internal/controller/suite_test.goapps/operator/test/utils/utils.goapps/portal/examples/advanced-template/template.yamlapps/portal/examples/nestjs-prisma-template/content/gitops/helios-app.yamlapps/portal/examples/nestjs-prisma-template/content/source/.env.exampleapps/portal/examples/nestjs-prisma-template/content/source/Dockerfileapps/portal/examples/nestjs-prisma-template/content/source/catalog-info.yamlapps/portal/examples/nestjs-prisma-template/content/source/nest-cli.jsonapps/portal/examples/nestjs-prisma-template/content/source/package.jsonapps/portal/examples/nestjs-prisma-template/content/source/prisma.config.tsapps/portal/examples/nestjs-prisma-template/content/source/prisma/migrations/migration_lock.tomlapps/portal/examples/nestjs-prisma-template/content/source/prisma/schema.prismaapps/portal/examples/nestjs-prisma-template/content/source/src/app.controller.tsapps/portal/examples/nestjs-prisma-template/content/source/src/app.module.tsapps/portal/examples/nestjs-prisma-template/content/source/src/app.service.tsapps/portal/examples/nestjs-prisma-template/content/source/src/main.tsapps/portal/examples/nestjs-prisma-template/content/source/src/prisma/prisma.module.tsapps/portal/examples/nestjs-prisma-template/content/source/src/prisma/prisma.service.tsapps/portal/examples/nestjs-prisma-template/content/source/tsconfig.build.jsonapps/portal/examples/nestjs-prisma-template/content/source/tsconfig.jsonapps/portal/examples/nestjs-prisma-template/template.yamlapps/portal/packages/app/src/App.tsxapps/portal/packages/app/src/scaffolder/DatabasePickerExtension/DatabasePicker.tsxapps/portal/packages/app/src/scaffolder/DatabasePickerExtension/extension.tsapps/portal/packages/app/src/scaffolder/DatabasePickerExtension/index.tsapps/portal/packages/app/src/scaffolder/index.tscue/cue.mod/module.cuecue/definitions/traits/database.cuedocs/APP_STARTUP_GUIDE.mddocs/OPERATOR.mddocs/SETUP.mddocs/manual-verification-guide.mdscripts/check-prereqs.batscripts/check-prereqs.shscripts/start-portal.ps1
- harden operator database reconciliation with required secret validation, postgres-only handling, targeted env injection, and probe command fixes - narrow Argo CD ignoreDifferences to DB env vars and update related operator tests - upgrade portal dependencies to Backstage 1.49.1 and fix EntityPage compatibility - wire database settings through scaffolder templates and generated GitOps manifests - improve cross-platform startup/prereq scripts with safe .env parsing and portable base64 decoding - update CI workflow action versions, docs Go requirement, and env-file gitignore patterns
There was a problem hiding this comment.
Actionable comments posted: 16
♻️ Duplicate comments (1)
apps/portal/packages/app/src/scaffolder/DatabasePickerExtension/extension.ts (1)
9-16:⚠️ Potential issue | 🟠 MajorChặn
dbNamechỉ gồm khoảng trắng.Điều kiện ở Line 12 vẫn cho giá trị như
" "đi qua, nên form hợp lệ nhưng tên database thực tế vẫn rỗng về mặt logic. Hãytrim()trước khi validate.🐛 Đề xuất sửa
validation: (value: any, validation: any) => { // Custom validation: If postgres is selected, dbName MUST be provided if (value?.dbType === 'postgres') { - if (!value?.dbName) { + const dbName = + typeof value?.dbName === 'string' ? value.dbName.trim() : ''; + if (!dbName) { validation.addError( 'Database Name is required when PostgreSQL is selected', ); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/packages/app/src/scaffolder/DatabasePickerExtension/extension.ts` around lines 9 - 16, The custom validator in the validation function currently allows dbName values that are only whitespace; update the check for value?.dbName inside the if (value?.dbType === 'postgres') branch to trim() the string and validate its length (e.g., if (!value?.dbName || value.dbName.trim().length === 0) ) before calling validation.addError('Database Name is required when PostgreSQL is selected'), so names like " " are rejected; ensure you reference the same validation.addError call and value?.dbName property when implementing this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/operator/.github/workflows/lint.yml`:
- Around line 13-23: Các workflow steps sử dụng mutable tags
(actions/checkout@v6, actions/setup-go@v6, golangci/golangci-lint-action@v9) nên
pin lại bằng full commit SHA; tìm trong repository của từng action (GitHub)
commit SHA tương ứng với phiên bản mong muốn và thay các giá trị của "uses" cho
các steps "actions/checkout", "actions/setup-go" và
"golangci/golangci-lint-action" bằng ref dạng @<full-commit-sha> (không dùng
`@v6/`@v9), đảm bảo mỗi replacement giữ nguyên cấu trúc step và các khóa "with"
hiện có.
In `@apps/operator/internal/controller/argocd_resources.go`:
- Around line 54-56: The jq expression in the jqPathExpressions array assumes
env always exists and can throw "Cannot iterate over null"; update the
expression string in the jqPathExpressions entry (the one containing
`.spec.template.spec.containers[].env[] | select(.name == "DB_HOST" or .name ==
"DB_USER" or .name == "DB_PASS")`) to use the optional iterator so it safely
handles missing/null env (e.g., change the `.env[]` part to `.env[]?`).
In `@apps/operator/internal/controller/database_resources_test.go`:
- Around line 744-955: Add a new subtest inside TestReconcileDatabaseInstance
that exercises the "update" branch: create an initial HeliosApp with a database
trait and pre-populate the fake client with an existing StatefulSet and Service
that have different storage (PVC/volumeClaimTemplates) or port values than the
desired spec, then call reconcileDatabaseInstance and assert the reconciler
updates those resources to match the desired values; reference
reconcileDatabaseInstance, the TestReconcileDatabaseInstance test harness, and
the StatefulSet/Service objects (e.g., checking
sts.Spec.VolumeClaimTemplates[*].Spec.Resources.Requests["storage"] and
svc.Spec.Ports[*].Port) so the test fails if updates are not performed. Ensure
the fake client is built with the existing resources and the assertions verify
the updated storage and port values after reconcileDatabaseInstance returns.
In `@apps/operator/internal/controller/database_resources.go`:
- Around line 359-363: The reconcile currently reads dbTrait.Properties.Storage
(falling back to DefaultDatabaseStorage) but then silently keeps
existingSts.Spec.VolumeClaimTemplates, so changes to storage in the trait are
ignored on updates; update the reconcile path (where storage is read and where
existingSts is used) to detect when dbTrait.Properties.Storage (or the effective
storage after defaulting) differs from the current PVC size in
existingSts.Spec.VolumeClaimTemplates and either (a) set a clear Condition /
return an error indicating storage drift was detected, or (b) implement PVC
expansion logic to patch the VolumeClaimTemplates spec safely; specifically add
a comparison between the computed storage variable and the PVC size in
existingSts.Spec.VolumeClaimTemplates, and on mismatch update status/return a
meaningful error (or call a dedicated expandPVC function) so storage changes are
not silently ignored (apply same fix to the other occurrence around the block
handling storage at the second location).
- Around line 353-367: The app injection currently sets DB_HOST/DB_USER/DB_PASS
but omits the Postgres port, so non-default dbTrait.Properties.Port (used by
GenerateDatabaseStatefulSet and Service) won’t be seen by the app; update the
contract/env injection to include DB_PORT (use dbTrait.Properties.Port with
fallback to DefaultPostgresPort) wherever DB_* envs are set for the app (also
apply the same fix in the other injection site referenced around lines 470-472)
so the running application receives the correct port.
In `@apps/portal/examples/advanced-template/template.yaml`:
- Around line 47-53: The template is missing a default for the "databaseConfig"
parameter so parameters.databaseConfig may be undefined while DatabasePicker.tsx
only displays formData?.dbType || 'none' and downstream code dereferences
dbType/dbName; add a default object for databaseConfig in the template (e.g.
provide default dbType: "none" and dbName: "" or appropriate defaults) so the
scaffolder form state always includes databaseConfig and avoids runtime
dereference errors; update the "databaseConfig" properties/default in
template.yaml rather than changing the picker component or making the field
required.
In
`@apps/portal/examples/nestjs-prisma-template/content/source/src/prisma/prisma.service.ts`:
- Line 34: The cast "as any" on the pooled client hides type errors for the
PrismaPg constructor — either remove the cast and fix the types so pool
(pg.Pool) matches what PrismaPg expects or, if this is a deliberate temporary
workaround, replace the cast with a clear comment that explains why the mismatch
exists, references the upstream issue/PR to follow, and add a TODO with an
issue/PR link; locate the cast at the PrismaPg instantiation (const adapter =
new PrismaPg(pool as any)) and update the code or comment accordingly so type
safety is preserved or the reason for opting out is documented.
In `@apps/portal/packages/app/package.json`:
- Around line 62-63: The `@types` packages are currently unpinned ("@types/react"
and "@types/react-dom" set to "*") which can resolve to a newer major than the
runtime React ("react": "^18.0.2"); update the package.json entries for
"@types/react" and "@types/react-dom" to pin them to React major 18 (for example
change "*" to "^18.0.0" or an 18.x exact version) so TypeScript types remain
compatible with the runtime React version.
In
`@apps/portal/packages/app/src/scaffolder/DatabasePickerExtension/DatabasePicker.tsx`:
- Around line 27-34: The event type for the Select.onChange handler in
handleTypeChange is too narrow for MUI v4; update the parameter type to
React.ChangeEvent<{ name?: string; value: unknown }> (i.e., include name?:
string) so it matches `@material-ui/core`'s signature, then keep the existing
newType cast (event.target.value as string) and the same onChange logic in
handleTypeChange to avoid type warnings.
In `@apps/portal/start-dev.sh`:
- Around line 4-44: The trim_ws and read_env_value functions are duplicated;
extract them into a single shared helper script (e.g., a sourced utilities file)
and remove the duplicate definitions from this script so both scripts source
that helper instead; update this script to source the shared file and rely on
the existing trim_ws and read_env_value symbols (no other behavioral changes),
ensuring the helper exports the same function names and behavior used by
start-dev and the precheck script.
- Line 76: The pipeline that sets ARGOCD_PASS uses "kubectl -n argocd get secret
argocd-initial-admin-secret -o jsonpath=\"{.data.password}\" | decode_base64"
which can hide a failing kubectl because pipefail is not enabled; either enable
shell pipefail (set -o pipefail) at the top of start-dev.sh, or split into two
steps: run the kubectl command and check its exit status (or capture its output
into a temp var) and only then run decode_base64, ensuring you check for empty
output and handle/report errors before assigning ARGOCD_PASS.
In `@docs/APP_STARTUP_GUIDE.md`:
- Around line 148-150: Remove the trailing whitespace detected on the Markdown
line containing "k3d typically ships with a recent k3s version (1.28+), so this
patch is rarely needed. Check with `kubectl version`." — edit that line to
delete the trailing space at the end (so there is no extra space character
before the newline) and re-run markdownlint to confirm the formatting error is
resolved.
In `@scripts/check-prereqs.sh`:
- Around line 63-67: The quote-stripping logic is redundant: the first glob [[
"$value" == \"*\" && "$value" == *\" ]] can be simplified because the glob
"\"*\"" already ensures the string starts and ends with a double-quote (same for
single-quote). Replace the two-part checks with single-pattern tests — e.g., use
if [[ "$value" == \"*\" ]]; then value="${value:1:${`#value`}-2}"; elif [[
"$value" == \'*\' ]]; then value="${value:1:${`#value`}-2}"; fi — referencing the
variable value in the existing block to remove the extra redundant condition.
In `@Taskfile.yml`:
- Line 1: Remove the UTF-8 BOM (U+FEFF) at the start of Taskfile.yml so the
first character is not the BOM; open Taskfile.yml in a text editor or use your
tooling to re-save it as UTF-8 without BOM (ensuring the leading character is
removed), then commit the cleaned file so YAML parsers and CI tools consume a
BOM-free Taskfile.yml.
- Line 246: The current extraction of ARGOCD_AUTH_TOKEN from TOKEN_JSON using
sed is fragile; change it to parse JSON with jq (e.g., use jq -r '.token' on
TOKEN_JSON) and add a fallback to the existing sed extraction only when jq is
not available; ensure you reference the ARGOCD_AUTH_TOKEN and TOKEN_JSON
variables, check for jq presence before using it, and keep the sed extraction as
the last-resort fallback to preserve behavior on systems without jq.
- Around line 148-151: The current use of "|| true" after the "kubectl patch sa
pipeline -p '{"secrets": [{"name": "docker-credentials"}]}'" command masks all
failures; instead, detect and handle only the expected non-fatal case
(serviceaccount not found) by first checking existence with "kubectl get sa
pipeline" and only attempting the patch if it exists, otherwise emit a clear
note that the SA will be created later; alternatively, run the patch and inspect
the exit code / stderr to differentiate permission/connection errors from the
not-found case and fail on real errors rather than silencing everything with "||
true".
---
Duplicate comments:
In
`@apps/portal/packages/app/src/scaffolder/DatabasePickerExtension/extension.ts`:
- Around line 9-16: The custom validator in the validation function currently
allows dbName values that are only whitespace; update the check for
value?.dbName inside the if (value?.dbType === 'postgres') branch to trim() the
string and validate its length (e.g., if (!value?.dbName ||
value.dbName.trim().length === 0) ) before calling validation.addError('Database
Name is required when PostgreSQL is selected'), so names like " " are
rejected; ensure you reference the same validation.addError call and
value?.dbName property when implementing this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c0b4f154-dbd7-41de-96bb-89d9079e8034
⛔ Files ignored due to path filters (1)
apps/portal/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (32)
.github/workflows/operator-ci.yml.github/workflows/portal-ci.yml.gitignoreTaskfile.ymlapps/operator/.github/workflows/lint.ymlapps/operator/.github/workflows/test-e2e.ymlapps/operator/.github/workflows/test.ymlapps/operator/config/rbac/role.yamlapps/operator/go.modapps/operator/internal/controller/argocd_resources.goapps/operator/internal/controller/database_resources.goapps/operator/internal/controller/database_resources_test.goapps/portal/backstage.jsonapps/portal/examples/advanced-template/content/gitops/helios-app.yamlapps/portal/examples/advanced-template/template.yamlapps/portal/examples/nestjs-prisma-template/content/gitops/helios-app.yamlapps/portal/examples/nestjs-prisma-template/content/source/src/prisma/prisma.service.tsapps/portal/examples/nestjs-prisma-template/template.yamlapps/portal/package.jsonapps/portal/packages/app/package.jsonapps/portal/packages/app/src/App.tsxapps/portal/packages/app/src/components/catalog/EntityPage.tsxapps/portal/packages/app/src/scaffolder/DatabasePickerExtension/DatabasePicker.tsxapps/portal/packages/app/src/scaffolder/DatabasePickerExtension/extension.tsapps/portal/packages/app/src/scaffolder/DatabasePickerExtension/index.tsapps/portal/packages/app/src/scaffolder/index.tsapps/portal/packages/backend/package.jsonapps/portal/packages/backend/src/index.tsapps/portal/start-dev.shdocs/APP_STARTUP_GUIDE.mdscripts/check-prereqs.shscripts/start-portal.ps1
💤 Files with no reviewable changes (1)
- apps/operator/config/rbac/role.yaml
- include CUE definitions in operator image and fix docker build context for e2e - harden ArgoCD DB_* diff ignore with null-safe jq expression - add DB_PORT injection and storage-drift detection in database reconciliation with tests - improve Taskfile and shell robustness (shared env helpers, safer SA patching, jq token fallback, pipefail) - tighten scaffolder template and validation defaults plus React type pinning - pin workflow action refs to immutable SHAs and clean docs whitespace
There was a problem hiding this comment.
Actionable comments posted: 12
♻️ Duplicate comments (1)
docs/APP_STARTUP_GUIDE.md (1)
368-371:⚠️ Potential issue | 🟡 MinorMô tả
task setupở bảng tóm tắt vẫn gây hiểu nhầm.Hàng này vẫn nói
task setupgộp “Steps 1-8”, trong khi chính phần trên của tài liệu chỉ start operator/portal ởtask dev. Với wording hiện tại, người mới rất dễ hiểu rằng chạytask setupxong là app đã chạy.🔧 Đề xuất sửa
-| `task setup` | Steps 1-8 in one command | +| `task setup` | Chuẩn bị môi trường local (cluster, CRD, secrets, portal deps); chưa chạy services |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/APP_STARTUP_GUIDE.md` around lines 368 - 371, Cập nhật mô tả trong bảng tóm tắt: thay dòng cho `task setup` để rõ rằng nó chỉ thực hiện các bước thiết lập/ cấu hình (Steps 1-8) chứ không khởi chạy ứng dụng hoặc dịch vụ; ví dụ đổi thành "Run setup steps 1–8 (install/config) — does not start services"; đồng thời đảm bảo `task dev` vẫn mô tả là "Run operator + portal" để người đọc biết `task dev` mới khởi chạy app.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/operator/internal/controller/database_resources_test.go`:
- Around line 780-793: Extract the repeated test setup into a helper (e.g.,
newTestReconciler or buildFakeReconciler) that creates runtime.NewScheme(),
calls corev1.AddToScheme, appv1alpha1.AddToScheme, appsv1.AddToScheme, builds
the fake client with
fake.NewClientBuilder().WithScheme(...).WithObjects(...).Build(), and returns a
*HeliosAppReconciler (or the scheme and client) so tests can call it instead of
duplicating the block; update occurrences that currently construct fullScheme,
call AddToScheme, build fakeClient, and instantiate
&HeliosAppReconciler{Client:..., Scheme:...} to use the new helper.
- Around line 3-7: The test in database_resources_test.go that currently claims
to check "valid base64" only verifies string length; replace the length-only
assertion with a real base64 decode check by attempting to decode the encoded
string (use encoding/base64) and asserting that base64.StdEncoding.DecodeString
(or the appropriate decoder) returns no error and yields expected bytes; update
the assertion on the variable used in that test (the encoded value inspected in
the "valid base64" check) and remove the length-based check so the test fails on
invalid base64 formats rather than on length alone.
- Around line 433-437: In the ExistingSecretPreserved subtest, extend the single
DB_USER assertion to also assert that secret.Data["DB_PASS"] equals
"existing-pass" and secret.Data["DB_HOST"] equals "existing-host" so the test
verifies all preserved fields; update the assertions in the test function
ExistingSecretPreserved to check DB_USER, DB_PASS and DB_HOST (using
string(secret.Data["..."]) comparisons and t.Errorf on mismatch).
In `@apps/operator/internal/controller/database_resources.go`:
- Around line 590-591: The current branch returns (0, true) when
preferredContainerName == "" which incorrectly signals an exactMatch; change the
return to (0, false) so exactMatch is false for the empty-preference fallback
case (update the function that checks preferredContainerName and the returned
exactMatch flag accordingly), and run/update any tests or callers that rely on
the exactMatch boolean to treat empty preferredContainerName as a fallback
rather than an exact match.
- Line 41: PasswordCharset currently contains characters like $, !, ^ and other
shell-sensitive symbols which can cause issues in shell commands, connection
strings or URL encoding; update the PasswordCharset constant to a safer charset
(e.g., letters, digits and a limited set of non-problematic symbols like -._) by
removing shell-special characters, and ensure any code that generates or
validates passwords (references to PasswordCharset) uses the new charset and
handles encoding/quoting where secrets are injected.
In `@apps/operator/Makefile`:
- Around line 169-171: The docker-buildx multi-arch target is still building
with context "." while the docker-build target uses the repo-root context
"../..", so update the docker-buildx rule (the Makefile target named
docker-buildx and any use of $(CONTAINER_TOOL) within it) to use the same build
context and Dockerfile path as docker-build (use -f
../../apps/operator/Dockerfile and build context ../..) so COPYs like COPY
apps/operator/... and COPY cue/ resolve correctly for multi-arch builds; ensure
IMG and any platform/buildx flags remain unchanged.
In
`@apps/portal/examples/nestjs-prisma-template/content/source/src/prisma/prisma.service.ts`:
- Line 30: The service currently falls back to 'postgres' because
prisma.service.ts reads database: process.env.DB_NAME || 'postgres' but the
operator never injects DB_NAME; update the operator to include DB_NAME in the
injected Secret/env (add DB_NAME to databaseSecretEnvVarNames and to
GenerateDatabaseSecret output using dbTrait.Properties.DBName or the default
{component}-db), or alternatively change prisma.service.ts to derive the DB name
from an injected var the operator already provides (e.g., use
process.env.DB_DATABASE or a combined value from existing envs) so the database
name matches the provisioned db; target the DB_NAME usage in prisma.service.ts
and the operator symbols databaseSecretEnvVarNames and
GenerateDatabaseSecret/dbTrait.Properties.DBName when making the fix.
In
`@apps/portal/packages/app/src/scaffolder/DatabasePickerExtension/extension.ts`:
- Line 5: Replace the loose any type on the exported DatabasePickerExtension
with Backstage's proper FieldExtensionOptions type to restore type safety:
update the declaration of DatabasePickerExtension (the symbol exported and
returned from scaffolderPlugin.provide) to use FieldExtensionOptions (or the
appropriate generic variant) instead of any, and adjust any import/usage so
FieldExtensionOptions is imported from `@backstage/plugin-scaffolder` or the
correct package; this ensures the extension's shape is statically checked while
leaving scaffolderPlugin.provide usage intact.
In `@docs/APP_STARTUP_GUIDE.md`:
- Around line 147-150: Xóa dòng trống giữa các dòng blockquote để tránh tách
khối và sửa lỗi markdownlint MD028: nối các dòng bắt đầu bằng "> [!WARNING]" và
dòng tiếp theo "**Only apply this patch..." cũng như dòng "k3d typically..." vào
cùng khối blockquote (loại bỏ dòng trống giữa các dòng ">" trong đoạn chứa cảnh
báo).
In `@Taskfile.yml`:
- Around line 190-191: The Taskfile tasks dev:portal:proxy and dev:portal:start
start background processes (port-forward and kubectl proxy) but never record or
clean up their PIDs; update these tasks to mirror the cleanup flow used in
apps/portal/start-dev.sh (the trap around Lines 71-81) by capturing PIDs for
port-forward and kubectl proxy, registering an exit/trap handler to kill those
PIDs on SIGINT/EXIT, and ensuring the handler runs before handing control to
yarn start so no orphaned processes keep ports 8080/8001 bound.
- Around line 76-78: Change the k3d API binding from 0.0.0.0 to localhost:
update the k3d invocation that uses the --api-port flag (the line with "k3d
cluster create {{.CLUSTER_NAME}} --agents 1 --wait --api-port 0.0.0.0:6550") to
bind to 127.0.0.1:6550 instead, and update the kubectl config set-cluster call
(the line with "kubectl config set-cluster k3d-{{.CLUSTER_NAME}} --server=...")
to use https://127.0.0.1:6550 so the API server is only accessible from
localhost.
- Around line 171-175: Remove the passthrough shell-expansion variables from the
env block so they don't override dotenv-loaded values: delete the lines setting
GITHUB_TOKEN: $GITHUB_TOKEN and GITHUB_USER: $GITHUB_USER (keep HELIOS_CUE_PATH
as-is), or if you need to explicitly read from the dotenv file use the Taskfile
templating form {{env "GITHUB_TOKEN"}} and {{env "GITHUB_USER"}} instead; ensure
the top-level dotenv: ['.env'] remains and that env only contains explicit
template lookups or actual values to avoid empty overrides.
---
Duplicate comments:
In `@docs/APP_STARTUP_GUIDE.md`:
- Around line 368-371: Cập nhật mô tả trong bảng tóm tắt: thay dòng cho `task
setup` để rõ rằng nó chỉ thực hiện các bước thiết lập/ cấu hình (Steps 1-8) chứ
không khởi chạy ứng dụng hoặc dịch vụ; ví dụ đổi thành "Run setup steps 1–8
(install/config) — does not start services"; đồng thời đảm bảo `task dev` vẫn mô
tả là "Run operator + portal" để người đọc biết `task dev` mới khởi chạy app.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f750c8ca-3713-40a7-a20f-53191f9f26b7
⛔ Files ignored due to path filters (1)
apps/portal/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
Taskfile.ymlapps/operator/.github/workflows/lint.ymlapps/operator/Dockerfileapps/operator/Makefileapps/operator/internal/controller/argocd_resources.goapps/operator/internal/controller/database_resources.goapps/operator/internal/controller/database_resources_test.goapps/portal/examples/advanced-template/template.yamlapps/portal/examples/nestjs-prisma-template/content/source/src/prisma/prisma.service.tsapps/portal/examples/nestjs-prisma-template/template.yamlapps/portal/packages/app/package.jsonapps/portal/packages/app/src/scaffolder/DatabasePickerExtension/DatabasePicker.tsxapps/portal/packages/app/src/scaffolder/DatabasePickerExtension/extension.tsapps/portal/start-dev.shdocs/APP_STARTUP_GUIDE.mdscripts/check-prereqs.shscripts/lib/env_helpers.sh
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/operator/internal/controller/database_resources.go`:
- Around line 197-201: The json.Unmarshal error case for trait.Properties.Raw
(inside the block checking trait.Properties != nil && trait.Properties.Raw !=
nil) currently just continues without logging; update this error branch to log
the failure including the error and the offending raw payload (or a truncated
preview) so malformed trait properties are visible—use the existing
reconciler/logger/context if available (or add a logger/context parameter to the
enclosing function) and call it with a clear message referencing trait, err, and
trait.Properties.Raw before continuing.
In
`@apps/portal/packages/app/src/scaffolder/DatabasePickerExtension/DatabasePicker.tsx`:
- Around line 12-14: Replace the loose string for DatabaseConfig.dbType with a
concrete union (or exported type/enum) to improve type safety and IntelliSense;
define a DatabaseType union (e.g., type DatabaseType = 'postgres' | 'mysql' |
'sqlite' | 'mongodb' or your supported DB names), change the DatabaseConfig
interface to use dbType: DatabaseType, and update all usages/type annotations
that reference DatabaseConfig or dbType (constructors, props, parsing logic) to
the new DatabaseType so callers get compile-time errors for invalid values.
- Line 64: The TextField error display is inconsistent with the validation in
extension.ts because the error check uses !dbName without trimming; update the
error condition in DatabasePicker (the JSX where error={rawErrors?.length > 0 &&
!dbName}) to trim dbName before checking (e.g., use dbName?.trim() or
equivalent) so that inputs of only whitespace trigger the error UI consistently
with the validation logic; ensure any other UI checks in DatabasePicker that
depend on dbName (display, helper text) use the same trimmed check.
In
`@apps/portal/packages/app/src/scaffolder/DatabasePickerExtension/extension.ts`:
- Around line 9-19: Replace the loose any types in the validation function with
concrete types to improve type safety: change the signature of validation to use
the DatabaseConfig type (import DatabaseConfig from ./DatabasePicker) for the
value parameter and the appropriate validation type (e.g., FieldValidation or
the project's validation type) for the validation parameter; update references
inside the function to rely on DatabaseConfig.dbType and DatabaseConfig.dbName
(still using the existing trim logic) so TypeScript can enforce correct shapes
and eliminate any usage.
In `@Taskfile.yml`:
- Around line 81-104: The Tekton install steps in the setup:tekton task use
kubectl apply against remote manifests without any timeout, which can hang;
update each kubectl apply command (the three lines invoking kubectl apply
--filename https://...) to include a request timeout (e.g.,
--request-timeout=<duration> like 60s) or wrap the apply in a
platform-appropriate timeout wrapper so the Taskfile target setup:tekton will
fail fast on network stalls; ensure you modify the exact kubectl apply lines
referenced in setup:tekton to include the timeout flag consistently.
- Around line 133-153: The setup:credentials task uses POSIX-only shell
constructs in preconditions ([ -n "$VAR" ]) and a bash if/then/else block (if
kubectl get sa ... then ... fi) which will fail on Windows; fix by making the
task platform-aware: add a platforms: [linux,darwin] guard to setup:credentials
(or split into two tasks) so the existing preconditions and bash if/then/else
run only on Unix-like systems, and add a separate Windows variant that performs
equivalent checks and the kubectl secret/patch logic using PowerShell-compatible
checks/conditionals; ensure you update the preconditions and the kubectl
SA-check/patch logic referenced in setup:credentials to point to the
platform-appropriate implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d03c7030-b7ac-4a51-a236-fdd658108a25
📒 Files selected for processing (6)
Taskfile.ymlapps/operator/internal/controller/database_resources.goapps/operator/internal/controller/database_resources_test.goapps/portal/packages/app/src/scaffolder/DatabasePickerExtension/DatabasePicker.tsxapps/portal/packages/app/src/scaffolder/DatabasePickerExtension/extension.tsdocs/APP_STARTUP_GUIDE.md
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/operator/internal/controller/database_resources.go`:
- Around line 673-679: The log accesses
deploy.Spec.Template.Spec.Containers[0].Name which can panic when the Deployment
has no containers (InjectDatabaseEnvVarsForContainer returns (false,false) and
exactContainerMatch is true); update the branch handling where
!exactContainerMatch is true (the code around InjectDatabaseEnvVarsForContainer,
dbTrait.ComponentName, deployName and exactContainerMatch) to first check
len(deploy.Spec.Template.Spec.Containers) > 0 before referencing index 0, and if
the slice is empty log a safe fallback string (e.g. "<no-containers>") or omit
the fallbackContainer field; also ensure any subsequent code that assumes a
container exists either returns early or guards against empty Containers to
prevent a runtime panic.
- Around line 774-796: The PGDATA env var currently appends PostgresDataSubPath
to PostgresDataPath, causing a nested pgdata directory when VolumeMount uses
SubPath; update the PGDATA env entry (named "PGDATA" where Value is set using
PostgresDataPath + "/" + PostgresDataSubPath) to use only PostgresDataPath
(i.e., remove the concatenation of PostgresDataSubPath) so the mounted PVC path
and PGDATA match the VolumeMount (which uses SubPath = PostgresDataSubPath).
- Line 718: The probeCommand currently interpolates dbName via fmt.Sprintf
(probeCommand) which allows shell expansion/subshell injection (e.g., $(...));
instead of embedding dbName, change the probe to reference the already-set
environment variable POSTGRES_DB (use -d "$POSTGRES_DB") so the value is read
from the environment rather than injected into the shell command; update code
around probeCommand and where POSTGRES_DB is exported (the block that sets
POSTGRES_DB) to ensure the probe uses that env var and remove direct use of
dbName in the command string.
In `@Taskfile.yml`:
- Around line 127-131: Dòng tạo ClusterRoleBinding hiện tại ("kubectl create
clusterrolebinding tekton-triggers-sa-admin --clusterrole=cluster-admin
--serviceaccount=default:tekton-triggers-sa") cấp quyền quá rộng; replace it by
defining a minimal Role or ClusterRole with only the verbs/resources Tekton
Triggers needs and bind that to the tekton-triggers-sa (or if permissions are
namespace-scoped, create a Role + RoleBinding in the tekton namespace instead of
a ClusterRoleBinding). Concretely: remove the kubectl create clusterrolebinding
tekton-triggers-sa-admin command, add a limited Role/ClusterRole manifest that
lists specific resources/verbs, and create a RoleBinding/ClusterRoleBinding that
references that new role and the serviceAccount tekton-triggers-sa.
- Around line 73-75: The current status task uses a POSIX pipeline ("k3d cluster
list 2>/dev/null | grep -q '{{.CLUSTER_NAME}}'") which is not portable to
Windows and also relies on shell-specific redirection/grep; instead change the
check to use k3d's supported commands such as "k3d cluster list --output json"
and parse the JSON for the cluster name or use "k3d kubeconfig get
{{.CLUSTER_NAME}}" to verify existence, updating the Taskfile `status` entry to
run one of these portable checks and handle errors/exit codes rather than
relying on grep/2>/dev/null.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f8adeefd-5bef-4ea9-97df-65301f818181
📒 Files selected for processing (4)
Taskfile.ymlapps/operator/internal/controller/database_resources.goapps/portal/packages/app/src/scaffolder/DatabasePickerExtension/DatabasePicker.tsxapps/portal/packages/app/src/scaffolder/DatabasePickerExtension/extension.ts
PhuocHoan
requested review from
NgocAnhDo26,
PhamHoangKha1403,
VH3956,
hoangphuc841 and
nghiaz160904
NgocAnhDo26
approved these changes
Mar 23, 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
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.
🗄️ Database Persistence — End-to-End Feature
Summary
This PR implements the complete database persistence feature for the Helios Platform, enabling backend applications to automatically provision a PostgreSQL database and receive injected credentials — with zero manual configuration.
What's Included
🔧 Operator — Database Lifecycle Management
databasetrait is detectedDB_HOST,DB_USER,DB_PASSviasecretKeyRef, keeping secrets out of GitOps repos📐 CUE — Database Trait Schema
#DatabaseTraitdefinition incue/definitions/traits/database.cuecue/examples/app-with-database.cue🚀 NestJS + Prisma Template
apps/portal/examples/nestjs-prisma-template/@prisma/adapter-pg), Node.js 24 LTSPrismaServiceconstructs connection from injected env vars (DB_HOST,DB_USER,DB_PASS)prisma migrate deployat startupprisma.config.tsfor Prisma 7 CLI configuration🎨 Portal — DatabasePicker Extension
DatabasePicker📚 Documentation & Developer Experience
docs/manual-verification-guide.md— Step-by-step k3d testing guideSETUP.md,APP_STARTUP_GUIDE.md,OPERATOR.mdTaskfile.ymlfor one-command local developmentcheck-prereqs.sh,check-prereqs.bat)Closes
Testing
go build ./...— compiles cleanlygo vet ./...— no issuesmake test— all tests pass (68.5% controller coverage, 71.3% CUE coverage)DB_HOST,DB_USER,DB_PASSinto backend pods viasecretKeyRefSummary by CodeRabbit
New Features
Improvements
Documentation