Open
Conversation
Adds a `Variables` field to the `resource.Context` type. Variables are useful for dynamic variables that do not fit neatly within a single resource attribute, and that may have effects that across multiple resources. A subsequent commit will use this field for a variable that indicates whether the database has already been created. PLAT-543
Adds a `NotCreated` field to track when a database has not yet been created. This field is set to `true` when the `database.Database` record is first created. It is changed to `false` after it becomes available for the first time. This means that if a `create-database` operation fails, and a subsequent `update-database` succeeds, this flag is only set to `false` after the `update-database` completes. Note that this field is implemented in the negative, as `NotCreated` instead of `Created`, so that it will default to `false` for existing databases. This lets us avoid a migration. PLAT-543
Plumbs the database `NotCreated` field into all of the workflows via `resource.Variables` in the `resource.Context`. PLAT-543
Adds a `scripts` field to the database spec in the API and in the `database` package. The contents of this field are validated to be syntactically correct SQL. PLAT-543
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 10 minutes and 42 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (20)
📝 WalkthroughWalkthroughThis pull request adds support for user-defined SQL scripts in database creation. It introduces new API types for defining scripts ( Changes
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Up to standards ✅🟢 Issues
|
| Category | Results |
|---|---|
| Complexity | 2 medium |
🟢 Metrics 43 complexity · -7 duplication
Metric Results Complexity 43 Duplication -7
TIP This summary will be updated as you push new changes. Give us feedback
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/internal/database/script.go`:
- Around line 134-136: The result struct's Error field isn't cleared on success,
so previous failure text can persist; in the function where you set
result.CompletedAt and result.Succeeded (look for the block assigning
result.CompletedAt = time.Now() and result.Succeeded = len(errs) == 0), update
it to reset result.Error to an empty string when result.Succeeded is true (and
otherwise populate it from errs as appropriate). Ensure you reference and mutate
the same result variable (e.g., result.Error = "" when len(errs) == 0) so a
successful rerun clears stale error text.
- Around line 98-101: Before starting the SQL transaction, gate execution by
inspecting the persisted result from svc.GetScriptResult: check the returned
result's Succeeded flag (from the GetScriptResult call using script.DatabaseID,
script.Name, script.NodeName) and if it is true, return early (no-op) so the
non-idempotent SQL is not re-run; only proceed to BeginTx/transaction and
subsequent store-update if the persisted result indicates not succeeded. Ensure
you use the fetched result (not the possibly stale script.Succeeded) for this
check and handle any nil/absent result appropriately.
- Around line 121-123: The code currently embeds the raw SQL variable statement
into the wrapped error and persists it to result.Error (via fmt.Errorf("failed
to execute statement '%s': %w", statement, err) and result.Error = err.Error()),
which can leak secrets; change the wrapping so it does not include the raw SQL
(e.g. use fmt.Errorf("failed to execute statement: %w", execErr) or a redacted
message) and ensure result.Error stores a non-sensitive, generic or sanitized
message (or a fingerprint/hash of the statement) instead of the full statement;
update places that append to errs so they append the sanitized/wrapped error
(referencing variables statement, execErr/err, result.Error, and errs).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ce6e556-735b-4e3c-b8b7-1d4ccd394ab6
⛔ Files ignored due to path filters (10)
api/apiv1/gen/control_plane/service.gois excluded by!**/gen/**api/apiv1/gen/http/control_plane/client/encode_decode.gois excluded by!**/gen/**api/apiv1/gen/http/control_plane/client/types.gois excluded by!**/gen/**api/apiv1/gen/http/control_plane/server/encode_decode.gois excluded by!**/gen/**api/apiv1/gen/http/control_plane/server/types.gois excluded by!**/gen/**api/apiv1/gen/http/openapi.jsonis excluded by!**/gen/**api/apiv1/gen/http/openapi.yamlis excluded by!**/gen/**api/apiv1/gen/http/openapi3.jsonis excluded by!**/gen/**api/apiv1/gen/http/openapi3.yamlis excluded by!**/gen/**go.sumis excluded by!**/*.sum
📒 Files selected for processing (39)
NOTICE.txtapi/apiv1/design/database.gochanges/unreleased/Added-20260412-202528.yamldocs/using/create-db.mde2e/scripts_test.gogo.modserver/internal/api/apiv1/convert.goserver/internal/api/apiv1/post_init_handlers.goserver/internal/api/apiv1/validate.goserver/internal/api/apiv1/validate_test.goserver/internal/database/database.goserver/internal/database/database_store.goserver/internal/database/instance_resource.goserver/internal/database/operations/common.goserver/internal/database/operations/restore_database.goserver/internal/database/orchestrator.goserver/internal/database/postgres_database.goserver/internal/database/script.goserver/internal/database/script_result_store.goserver/internal/database/service.goserver/internal/database/spec.goserver/internal/database/store.goserver/internal/orchestrator/swarm/orchestrator.goserver/internal/orchestrator/systemd/orchestrator.goserver/internal/resource/resource.goserver/internal/resource/resource_test.goserver/internal/workflows/activities/activities.goserver/internal/workflows/activities/apply_event.goserver/internal/workflows/activities/executor.goserver/internal/workflows/activities/get_instance_resources.goserver/internal/workflows/activities/get_restore_resources.goserver/internal/workflows/activities/get_script_results.goserver/internal/workflows/common.goserver/internal/workflows/delete_database.goserver/internal/workflows/pgbackrest_restore.goserver/internal/workflows/plan_restore.goserver/internal/workflows/refresh_current_state.goserver/internal/workflows/service.goserver/internal/workflows/update_database.go
Adds script entities to the `database` package, including utilities and types that can be used by resources to execute scripts at different points in the database creation process, as well as storage for script results. PLAT-543
Adds scripts to two resources in the `database` package: - `InstanceResource.PostInit`: executes immediately after connecting to the database for the first time, before any users are created. - `PostgresDatabaseResource.PostDatabaseCreate`: executes immediately after Spock is initialized. PLAT-543
Adds an E2E test to exercise the new `scripts` field. PLAT-543
jason-lynch
force-pushed
the
feat/PLAT-543/user-defined-scripts
branch
from
2b652a3 to
71093cc
Compare
jason-lynch
force-pushed
the
feat/PLAT-543/user-defined-scripts
branch
from
71093cc to
f3805f2
Compare
rshoemaker
approved these changes
Apr 13, 2026
| return err | ||
| } | ||
| if !ok { | ||
| return nil |
Contributor
There was a problem hiding this comment.
Thoughts on explicitly logging when scripts are NOT executed? I think this can only happen during update-database, which might be valuable to see in the logs. I'm not sure.
Member
Author
There was a problem hiding this comment.
Is there a specific use case that you're thinking of when those logs would be helpful?
As it is now, these are executed only once per node/database during the database's lifetime. They can execute during update-database if you're using update-database to retry a create-database that failed before the scripts executed successfully. If we were to log on non-executions, you'd see "not executing scripts" on every update or restore operation after that point.
Contributor
There was a problem hiding this comment.
If there are too many then the message wouldn't be useful. I was just thinking that people who don't read the docs might be confused why the script isn't running.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the ability to run arbitrary SQL at two different points in the database creation process:
post_init: This script runs immediately after the database is created, before any users are created.post_database_create: This script runs immediately after Spock is initialized, before any subscriptions have been created.This feature can be used to perform first-time setup for roles and similar operations.
Changes
scriptsfield to the database spec:Testing
Basic usage:
After that, you can try changing your statements and validating that the changed statements do not run.
You can also try specifying an invalid statement in
post_initfor a new database, likeCREATE DATABASE foo, and observe that the database creation process fails with an error about howCREATE DATABASEis not allowed within a transaction. Then remove or replace the bad statement, submit anupdate-databaserequest, and verify that the creation process resumes where it left off and executes your updated statement.You can also look at the included examples in the API reference, docs, and tests for other things to try.
Notes for Reviewers
Roles added in the
pre_initfield will not carry over to new nodes. This will be addressed in a subsequent PR.PLAT-543