feat: add dedicated database resources - #34
Open
levivannoort wants to merge 1 commit into
Open
Conversation
Onboards Appwrite dedicated databases (managed PostgreSQL, MySQL, and Mongo instances) onto the provider: - appwrite_dedicated_database: CRUD + import, waits for async provisioning so connection details are populated, exposes connection and status/sizing fields (password/URI marked sensitive). - appwrite_dedicated_database_backup_policy: CRUD + import. A single resource selects the engine via an `engine` attribute. The three engines share byte-identical params, models, and REST paths (differing only in the URL segment), so this package issues raw engine-keyed REST calls and decodes into the shared SDK models, mirroring the existing GetColumnRaw helper rather than triplicating typed option builders. Built against the unreleased dedicated-databases SDK, which currently lives on a fork with a different module path. To avoid a module-identity conflict the fork is required additionally and used only by this new package (which builds its own client from the raw provider credentials); no existing imports change. Swap the import prefix back to appwrite/sdk-for-go and drop the fork require once the feature ships in the official SDK. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Greptile SummaryThis PR adds Terraform support for Appwrite dedicated databases. The main changes are:
Confidence Score: 4/5The async database lifecycle paths need fixes before merging.
internal/services/dedicateddatabase/resource.go; internal/services/dedicateddatabase/helpers.go Important Files Changed
Prompt To Fix All With AIFix the following 4 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 4
internal/services/dedicateddatabase/resource.go:212-219
**Created Database Loses State**
When the POST succeeds but readiness polling later times out, is canceled, or hits a transient read error, this returns before writing the created database ID to state. Terraform then forgets the remote database it just created, so the next apply can create another dedicated database instead of reconciling the first one.
### Issue 2 of 4
internal/services/dedicateddatabase/resource.go:293-305
**Update Stores Transitional State**
Changing fields such as `specification`, `replicas`, storage autoscaling, or PITR can start asynchronous reconfiguration, but this path writes the immediate PATCH response directly to state. If the API returns `scaling` or another transitional response with stale or empty connection details, downstream Terraform references can receive unusable `hostname` or `connection_string` values until a later refresh.
### Issue 3 of 4
internal/services/dedicateddatabase/helpers.go:126-136
**Terminal Statuses Keep Polling**
The wait loop only treats `failed` and `deleted` as terminal errors, so any other terminal non-ready status keeps polling until the full timeout. If the API reports a state such as `error` or `unhealthy`, Terraform waits up to 30 minutes and then reports a generic timeout instead of failing on the actual database state.
### Issue 4 of 4
internal/services/dedicateddatabase/backup_policy_resource.go:254-265
**Slash IDs Shift Import Parts**
The import parser accepts extra slashes inside the middle segment because each call to `splitTwo` only splits once. An import like `postgresql/db/with/slash/policy` becomes `database_id = "db"` and `policy_id = "with/slash/policy"`, so the later read builds a different policy path than the user intended and can import the wrong object or fail with a misleading not-found error.
Reviews (1): Last reviewed commit: "feat: add dedicated database resources" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+212
to
+219
|
|
||
| // Provisioning is asynchronous; wait so connection details are populated. | ||
| if err := waitForDatabaseReady(ctx, func() (string, error) { | ||
| var cur models.DedicatedDatabase | ||
| if err := apiCall(c, r.clients.UserAgent, "GET", "/"+engine+"/"+db.Id, nil, &cur); err != nil { | ||
| return "", err | ||
| } | ||
| return cur.Status, nil |
Contributor
There was a problem hiding this comment.
When the POST succeeds but readiness polling later times out, is canceled, or hits a transient read error, this returns before writing the created database ID to state. Terraform then forgets the remote database it just created, so the next apply can create another dedicated database instead of reconciling the first one.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/services/dedicateddatabase/resource.go
Line: 212-219
Comment:
**Created Database Loses State**
When the POST succeeds but readiness polling later times out, is canceled, or hits a transient read error, this returns before writing the created database ID to state. Terraform then forgets the remote database it just created, so the next apply can create another dedicated database instead of reconciling the first one.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+293
to
+305
| params["networkIPAllowlist"] = allow | ||
| } | ||
| if resp.Diagnostics.HasError() { | ||
| return | ||
| } | ||
|
|
||
| var db models.DedicatedDatabase | ||
| if err := apiCall(c, r.clients.UserAgent, "PATCH", "/"+engine+"/"+plan.ID.ValueString(), params, &db); err != nil { | ||
| resp.Diagnostics.AddError("Error updating dedicated database", common.FormatError(err)) | ||
| return | ||
| } | ||
|
|
||
| plan.ProjectID = types.StringValue(projectID) |
Contributor
There was a problem hiding this comment.
Changing fields such as specification, replicas, storage autoscaling, or PITR can start asynchronous reconfiguration, but this path writes the immediate PATCH response directly to state. If the API returns scaling or another transitional response with stale or empty connection details, downstream Terraform references can receive unusable hostname or connection_string values until a later refresh.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/services/dedicateddatabase/resource.go
Line: 293-305
Comment:
**Update Stores Transitional State**
Changing fields such as `specification`, `replicas`, storage autoscaling, or PITR can start asynchronous reconfiguration, but this path writes the immediate PATCH response directly to state. If the API returns `scaling` or another transitional response with stale or empty connection details, downstream Terraform references can receive unusable `hostname` or `connection_string` values until a later refresh.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+126
to
+136
| if err != nil { | ||
| return fmt.Errorf("error checking database %q status: %w", databaseID, err) | ||
| } | ||
| switch status { | ||
| case "ready", "inactive", "paused": | ||
| return nil | ||
| case "failed": | ||
| return fmt.Errorf("database %q entered failed state", databaseID) | ||
| case "deleted": | ||
| return fmt.Errorf("database %q was deleted during provisioning", databaseID) | ||
| } |
Contributor
There was a problem hiding this comment.
The wait loop only treats failed and deleted as terminal errors, so any other terminal non-ready status keeps polling until the full timeout. If the API reports a state such as error or unhealthy, Terraform waits up to 30 minutes and then reports a generic timeout instead of failing on the actual database state.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/services/dedicateddatabase/helpers.go
Line: 126-136
Comment:
**Terminal Statuses Keep Polling**
The wait loop only treats `failed` and `deleted` as terminal errors, so any other terminal non-ready status keeps polling until the full timeout. If the API reports a state such as `error` or `unhealthy`, Terraform waits up to 30 minutes and then reports a generic timeout instead of failing on the actual database state.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+254
to
+265
| func (r *backupPolicyResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { | ||
| engine, rest, ok := splitTwo(req.ID) | ||
| if ok { | ||
| var dbID, policyID string | ||
| dbID, policyID, ok = splitTwo(rest) | ||
| if ok { | ||
| if _, valid := engines[engine]; !valid { | ||
| resp.Diagnostics.AddError("Invalid engine", fmt.Sprintf("Engine must be one of: %s", validEngines())) | ||
| return | ||
| } | ||
| resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("engine"), engine)...) | ||
| resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("database_id"), dbID)...) |
Contributor
There was a problem hiding this comment.
The import parser accepts extra slashes inside the middle segment because each call to splitTwo only splits once. An import like postgresql/db/with/slash/policy becomes database_id = "db" and policy_id = "with/slash/policy", so the later read builds a different policy path than the user intended and can import the wrong object or fail with a misleading not-found error.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/services/dedicateddatabase/backup_policy_resource.go
Line: 254-265
Comment:
**Slash IDs Shift Import Parts**
The import parser accepts extra slashes inside the middle segment because each call to `splitTwo` only splits once. An import like `postgresql/db/with/slash/policy` becomes `database_id = "db"` and `policy_id = "with/slash/policy"`, so the later read builds a different policy path than the user intended and can import the wrong object or fail with a misleading not-found error.
How can I resolve this? If you propose a fix, please make it concise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Onboards Appwrite dedicated databases (managed PostgreSQL, MySQL, and Mongo instances) onto the provider.
Resources
appwrite_dedicated_database— CRUD + import for a managed database instance, engine chosen via anengineattribute (postgresql/mysql/mongo, ForceNew). Waits for async provisioning to finish soconnection_string/hostname/ etc. are populated; connection password and URI are marked sensitive.appwrite_dedicated_database_backup_policy— CRUD + import for a database's backup policy.Design notes
engineselector. The three engines share byte-identical create/update params, response models, and REST paths — only the/postgresql|/mysql|/mongoURL segment differs. Rather than triplicate ~30 typed option-builders per method, this package issues raw engine-keyed REST calls and decodes into the shared SDK models, mirroring the existingGetColumnRawhelper.github.com/aw-tests/sdk-for-go/v6) that self-references that path — so areplacecan't stand in for the real path. The fork is therefore required additionally and used only by this new package (which builds its own client from the raw provider credentials); no existing imports change. Flip this package's import prefix back toappwrite/sdk-for-goand drop the forkrequireonce the feature ships in the official SDK (noted in the package doc comment). This is why the PR targets abetabase branch rather thanmain.Out of scope (follow-ups)
Day-2 imperative ops that don't fit Terraform's declarative model — failover, migration, in-place upgrade, restorations, branches, SQL executions, connection pooler, PG extensions — and update-only tuning knobs (
sql_api_*,metrics_*).Verification
make lint+make build+ package unit tests pass.terraform providers schema -jsonboots the provider and passes the framework's schema validation for both resources.make docswith zero churn to existing pages.TF_ACC=1) — need a live Appwrite server with the feature enabled.🤖 Generated with Claude Code