From 3ddce626474c57a3317c7b4a5faa0a5ee1456733 Mon Sep 17 00:00:00 2001 From: max-voitcov Date: Wed, 26 Aug 2026 00:40:03 +0300 Subject: [PATCH] Reject volume mounts that silently never persist A `dokploy_mount` with `type = "volume"` and no `volume_name` was accepted by both this provider and Dokploy. Dokploy renders the mount as `{Source: volumeName || "", Target: mountPath}`, and Docker reads an empty source as an anonymous volume: every deploy created a fresh one and orphaned the last, so the data never survived a redeploy while disk usage climbed. Nothing errored at any point, which is what made it worth catching here. The pairing is now checked at plan time, before anything is created, and the error explains the consequence rather than only the rule. The same validator covers `bind` without `host_path` and `file` without `file_path`, and rejects a field set against the wrong type, which Dokploy would otherwise ignore. Verified against a live v0.30.2 instance: the offending config plans cleanly before the change and is refused after it. --- docs/resources/mount.md | 5 +- internal/provider/resource_mount_test.go | 94 ++++++++++++++++++++++++ internal/provider/resource_networking.go | 48 +++++++++++- internal/provider/validators.go | 78 ++++++++++++++++++++ 4 files changed, 223 insertions(+), 2 deletions(-) create mode 100644 internal/provider/resource_mount_test.go create mode 100644 internal/provider/validators.go diff --git a/docs/resources/mount.md b/docs/resources/mount.md index ff1b968..b87eb15 100644 --- a/docs/resources/mount.md +++ b/docs/resources/mount.md @@ -5,6 +5,7 @@ subcategory: "" description: |- A volume, bind mount, or config file attached to a Dokploy service. type = "volume" — a named Docker volume; set volume_name.type = "bind" — a path on the host; set host_path.type = "file" — a file rendered from content; set file_path. + ~> A volume mount must set volume_name. Dokploy hands an unset name to Docker as an empty source, which creates a fresh anonymous volume on every deploy and orphans the previous one — the data never survives a redeploy. The provider rejects that combination at plan time. --- # dokploy_mount (Resource) @@ -15,6 +16,8 @@ A volume, bind mount, or config file attached to a Dokploy service. * `type = "bind"` — a path on the host; set `host_path`. * `type = "file"` — a file rendered from `content`; set `file_path`. +~> **A `volume` mount must set `volume_name`.** Dokploy hands an unset name to Docker as an empty source, which creates a fresh anonymous volume on every deploy and orphans the previous one — the data never survives a redeploy. The provider rejects that combination at plan time. + @@ -24,7 +27,7 @@ A volume, bind mount, or config file attached to a Dokploy service. - `mount_path` (String) Path inside the container where the mount appears. - `service_id` (String) ID of the service this mount attaches to. Must match `service_type` — an application ID, a compose ID, a postgres ID, and so on. -- `service_type` (String) The kind of service this mount attaches to. Valid values: `application`, `postgres`, `mysql`, `mariadb`, `mongo`, `redis`, `compose`. +- `service_type` (String) The kind of service this mount attaches to. Valid values: `application`, `postgres`, `mysql`, `mariadb`, `mongo`, `redis`, `compose`, `libsql`. - `type` (String) The kind of mount to create. Valid values: `bind`, `volume`, `file`. ### Optional diff --git a/internal/provider/resource_mount_test.go b/internal/provider/resource_mount_test.go new file mode 100644 index 0000000..2856316 --- /dev/null +++ b/internal/provider/resource_mount_test.go @@ -0,0 +1,94 @@ +package provider_test + +import ( + "regexp" + "testing" + + "github.com/hashicorp/terraform-plugin-testing/helper/resource" +) + +const testMountProviderBlock = ` +provider "dokploy" { + host = "https://dokploy.invalid" + api_key = "not-used-plan-only" +} +` + +// A `volume` mount without a volume_name is the dangerous case: Dokploy's +// generateVolumeMounts maps a null volumeName to `Source: ""`, which Docker +// reads as an anonymous volume. Every deploy then gets a brand-new volume and +// the previous one is orphaned, so the data silently does not survive. +// +// These validations run at plan time, so they need no Dokploy instance. +func TestMountValidation(t *testing.T) { + for name, tc := range map[string]struct { + config string + expectError *regexp.Regexp + }{ + "volume without volume_name": { + config: testMountProviderBlock + ` +resource "dokploy_mount" "test" { + type = "volume" + mount_path = "/data" + service_type = "application" + service_id = "app-123" +}`, + expectError: regexp.MustCompile(`volume_name`), + }, + "volume with empty volume_name": { + config: testMountProviderBlock + ` +resource "dokploy_mount" "test" { + type = "volume" + mount_path = "/data" + volume_name = "" + service_type = "application" + service_id = "app-123" +}`, + expectError: regexp.MustCompile(`volume_name`), + }, + "bind without host_path": { + config: testMountProviderBlock + ` +resource "dokploy_mount" "test" { + type = "bind" + mount_path = "/data" + service_type = "application" + service_id = "app-123" +}`, + expectError: regexp.MustCompile(`host_path`), + }, + "file without file_path": { + config: testMountProviderBlock + ` +resource "dokploy_mount" "test" { + type = "file" + mount_path = "/etc/app" + content = "hello" + service_type = "application" + service_id = "app-123" +}`, + expectError: regexp.MustCompile(`file_path`), + }, + "volume_name set on a bind mount": { + config: testMountProviderBlock + ` +resource "dokploy_mount" "test" { + type = "bind" + mount_path = "/data" + host_path = "/srv/data" + volume_name = "stray" + service_type = "application" + service_id = "app-123" +}`, + expectError: regexp.MustCompile(`volume_name`), + }, + } { + t.Run(name, func(t *testing.T) { + resource.UnitTest(t, resource.TestCase{ + ProtoV6ProviderFactories: protoV6ProviderFactories, + Steps: []resource.TestStep{{ + Config: tc.config, + PlanOnly: true, + ExpectError: tc.expectError, + }}, + }) + }) + } +} diff --git a/internal/provider/resource_networking.go b/internal/provider/resource_networking.go index 8ceab22..cee551b 100644 --- a/internal/provider/resource_networking.go +++ b/internal/provider/resource_networking.go @@ -4,6 +4,8 @@ import ( "context" "fmt" + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" "github.com/hashicorp/terraform-plugin-framework/types" @@ -92,6 +94,43 @@ type mountModel struct { ServiceID types.String `tfsdk:"service_id" dokploy:"serviceId,create"` } +// mountConfigValidators enforce the type/field pairing that Dokploy itself +// does not. +// +// Dokploy's `generateVolumeMounts` renders a mount as +// `{Source: mount.volumeName || "", Target: mount.mountPath}`. A `volume` +// mount whose volumeName is null therefore reaches Docker with an empty +// source, which Docker treats as an *anonymous* volume: a fresh one is created +// on every deploy and the previous one is left orphaned, so the data silently +// never survives a redeploy. `mounts.create` accepts the mount regardless, so +// nothing surfaces until the data is already gone. +// +// The same shape applies to `bind` (hostPath) and `file` (filePath). +func mountConfigValidators() []resource.ConfigValidator { + return []resource.ConfigValidator{ + &requiredWhen{ + discriminator: path.Root("type"), + value: "volume", + attribute: path.Root("volume_name"), + rationale: "Dokploy passes an unset `volume_name` to Docker as an empty source, which creates " + + "a new anonymous volume on every deploy. The data written to the previous volume is " + + "orphaned and never reused, so the mount silently does not persist anything.", + }, + &requiredWhen{ + discriminator: path.Root("type"), + value: "bind", + attribute: path.Root("host_path"), + rationale: "A bind mount with no host path has nothing to bind to.", + }, + &requiredWhen{ + discriminator: path.Root("type"), + value: "file", + attribute: path.Root("file_path"), + rationale: "Dokploy writes `content` to `file_path` inside the service's files directory.", + }, + } +} + func mountResource() ResourceSpec { return ResourceSpec{ Name: "mount", @@ -100,11 +139,18 @@ func mountResource() ResourceSpec { UpdateProc: "mounts.update", DeleteProc: "mounts.remove", NewModel: func() any { return &mountModel{} }, + + ConfigValidators: mountConfigValidators(), + Schema: schema.Schema{ MarkdownDescription: "A volume, bind mount, or config file attached to a Dokploy service.\n\n" + "* `type = \"volume\"` — a named Docker volume; set `volume_name`.\n" + "* `type = \"bind\"` — a path on the host; set `host_path`.\n" + - "* `type = \"file\"` — a file rendered from `content`; set `file_path`.", + "* `type = \"file\"` — a file rendered from `content`; set `file_path`.\n\n" + + "~> **A `volume` mount must set `volume_name`.** Dokploy hands an unset name to Docker as an " + + "empty source, which creates a fresh anonymous volume on every deploy and orphans the " + + "previous one — the data never survives a redeploy. The provider rejects that combination " + + "at plan time.", Attributes: map[string]schema.Attribute{ "id": computedID("Unique mount identifier."), "type": enumString("The kind of mount to create.", mountTypes, true), diff --git a/internal/provider/validators.go b/internal/provider/validators.go new file mode 100644 index 0000000..7491e19 --- /dev/null +++ b/internal/provider/validators.go @@ -0,0 +1,78 @@ +package provider + +import ( + "context" + "fmt" + + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +// requiredWhen declares that `attribute` must hold a non-empty value whenever +// `discriminator` equals `value`, and must be absent otherwise. +// +// Terraform's schema language cannot express "required, but only for this +// variant", and Dokploy's Zod schemas accept every combination -- so without a +// provider-side check a nonsensical resource is created without complaint. The +// `dokploy_mount` case is the reason this exists: see mountConfigValidators. +type requiredWhen struct { + discriminator path.Path + value string + attribute path.Path + // rationale explains the consequence of getting it wrong, so the error + // tells the practitioner why rather than only what. + rationale string +} + +var _ resource.ConfigValidator = &requiredWhen{} + +func (v *requiredWhen) Description(ctx context.Context) string { + return v.MarkdownDescription(ctx) +} + +func (v *requiredWhen) MarkdownDescription(_ context.Context) string { + return fmt.Sprintf("`%s` is required when `%s` is `%s`, and must not be set otherwise.", + v.attribute, v.discriminator, v.value) +} + +func (v *requiredWhen) ValidateResource( + ctx context.Context, + req resource.ValidateConfigRequest, + resp *resource.ValidateConfigResponse, +) { + var discriminator types.String + resp.Diagnostics.Append(req.Config.GetAttribute(ctx, v.discriminator, &discriminator)...) + if resp.Diagnostics.HasError() || discriminator.IsNull() || discriminator.IsUnknown() { + return + } + + var attribute types.String + resp.Diagnostics.Append(req.Config.GetAttribute(ctx, v.attribute, &attribute)...) + if resp.Diagnostics.HasError() || attribute.IsUnknown() { + // An unknown value cannot be checked at plan time; it is resolved + // during apply and Dokploy validates it there. + return + } + + matches := discriminator.ValueString() == v.value + empty := attribute.IsNull() || attribute.ValueString() == "" + + switch { + case matches && empty: + detail := fmt.Sprintf("`%s` must be set to a non-empty value when `%s` is `%s`.", + v.attribute, v.discriminator, v.value) + if v.rationale != "" { + detail += "\n\n" + v.rationale + } + resp.Diagnostics.AddAttributeError(v.attribute, + fmt.Sprintf("Missing %s", v.attribute), detail) + + case !matches && !empty: + resp.Diagnostics.AddAttributeError(v.attribute, + fmt.Sprintf("Unexpected %s", v.attribute), + fmt.Sprintf("`%s` only applies when `%s` is `%s`, but it is `%s`. "+ + "Dokploy ignores the value, so leaving it set hides a mistake.", + v.attribute, v.discriminator, v.value, discriminator.ValueString())) + } +}