vmbackup: Prevent save backups to the same folder where TSDB data is (#2547)

* {vmbackup, vmbackup/snapshot}: validate snapshot name

* vmbackup/snapshot: added another checks

* backup/actions: added check that we ignore backup_complete.ignore file

* vmbackup: moved snapshot to lib directory

* lib/snapshot: added functions description

* lib/snapshot: fixed typo

* vmbackup: code cleanup

* wip

* vmbackup: Prevent save backups to the same folder where TSDB data is

* Apply suggestions from code review

* wip

* wip

* wip

Co-authored-by: Aliaksandr Valialkin <valyala@victoriametrics.com>

app/vmbackup/README.md

@@ -7,7 +7,7 @@ Supported storage systems for backups:
 * [GCS](https://cloud.google.com/storage/). Example: `gs://<bucket>/<path/to/backup>`
 * [S3](https://aws.amazon.com/s3/). Example: `s3://<bucket>/<path/to/backup>`
 * Any S3-compatible storage such as [MinIO](https://github.com/minio/minio), [Ceph](https://docs.ceph.com/en/pacific/radosgw/s3/) or [Swift](https://platform.swiftstack.com/docs/admin/middleware/s3_middleware.html). See [these docs](#advanced-usage) for details.
-* Local filesystem. Example: `fs://</absolute/path/to/backup>`
+* Local filesystem. Example: `fs://</absolute/path/to/backup>`. Note that `vmbackup` prevents from storing the backup into the directory pointed by `-storageDataPath` command-line flag, since this directory should be managed solely by VictoriaMetrics or `vmstorage`.
 
 `vmbackup` supports incremental and full backups. Incremental backups are created automatically if the destination path already contains data from the previous backup.
 Full backups can be sped up with `-origin` pointing to an already existing backup on the same remote storage. In this case `vmbackup` makes server-side copy for the shared

app/vmbackup/main.go

@@ -4,6 +4,7 @@ import (
 	"flag"
 	"fmt"
 	"os"
+	"path/filepath"
 	"strings"
 	"time"
 
@@ -155,9 +156,28 @@ func newDstFS() (common.RemoteFS, error) {
 	if err != nil {
 		return nil, fmt.Errorf("cannot parse `-dst`=%q: %w", *dst, err)
 	}
+	if hasFilepathPrefix(*dst, *storageDataPath) {
+		return nil, fmt.Errorf("-dst=%q can not point to the directory with VictoriaMetrics data (aka -storageDataPath=%q)", *dst, *storageDataPath)
+	}
 	return fs, nil
 }
 
+func hasFilepathPrefix(path, prefix string) bool {
+	if !strings.HasPrefix(path, "fs://") {
+		return false
+	}
+	path = path[len("fs://"):]
+	pathAbs, err := filepath.Abs(path)
+	if err != nil {
+		return false
+	}
+	prefixAbs, err := filepath.Abs(prefix)
+	if err != nil {
+		return false
+	}
+	return strings.HasPrefix(pathAbs, prefixAbs)
+}
+
 func newOriginFS() (common.OriginFS, error) {
 	if len(*origin) == 0 {
 		return &fsnil.FS{}, nil

app/vmbackup/main_test.go

@@ -0,0 +1,29 @@
+package main
+
+import (
+	"path/filepath"
+	"testing"
+)
+
+func TestHasFilepathPrefix(t *testing.T) {
+	f := func(dst, storageDataPath string, resultExpected bool) {
+		t.Helper()
+		result := hasFilepathPrefix(dst, storageDataPath)
+		if result != resultExpected {
+			t.Errorf("unexpected hasFilepathPrefix(%q, %q); got: %v; want: %v", dst, storageDataPath, result, resultExpected)
+		}
+	}
+	pwd, err := filepath.Abs("")
+	if err != nil {
+		t.Fatalf("cannot determine working directory: %s", err)
+	}
+	f("s3://foo/bar", "foo", false)
+	f("fs://"+pwd+"/foo", "foo", true)
+	f("fs://"+pwd+"/foo", "foo/bar", false)
+	f("fs://"+pwd+"/foo/bar", "foo", true)
+	f("fs://"+pwd+"/foo", "bar", false)
+	f("fs://"+pwd+"/foo", pwd+"/foo", true)
+	f("fs://"+pwd+"/foo", pwd+"/foo/bar", false)
+	f("fs://"+pwd+"/foo/bar", pwd+"/foo", true)
+	f("fs://"+pwd+"/foo", pwd+"/bar", false)
+}

docs/CHANGELOG.md

@@ -23,6 +23,7 @@ The following tip changes can be tested by building VictoriaMetrics components f
 * BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): properly show the graph when clicking on `Prometheus` link in Grafana graph editor. Previously the graph wasn't shown until clicking on the `Graph` tab. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2402#issuecomment-1115830648).
 * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): rename `vmagent_remote_write_rate_limit_reached_total` metric to `vmagent_remotewrite_rate_limit_reached_total`, so its name is consistent with the rest of `vmagent_remotewrite_` metrics.
 * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): rename `promscrape_stale_samples_created_total` metric to `vm_promscrape_stale_samples_created_total`, so its name is consistent with the rest of `vm_promscrape_` metrics.
+* BUGFIX: [vmbackup](https://docs.victoriametrics.com/vmbackup.html): disallow writing backups to `-storageDataPath` directory, since this directory is managed solely by VictoriaMetrics or `vmstorage`. Other apps shouldn't write into this directory. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2503).
 
 
 ## [v1.77.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.77.0)

docs/vmbackup.md

@@ -11,7 +11,7 @@ Supported storage systems for backups:
 * [GCS](https://cloud.google.com/storage/). Example: `gs://<bucket>/<path/to/backup>`
 * [S3](https://aws.amazon.com/s3/). Example: `s3://<bucket>/<path/to/backup>`
 * Any S3-compatible storage such as [MinIO](https://github.com/minio/minio), [Ceph](https://docs.ceph.com/en/pacific/radosgw/s3/) or [Swift](https://platform.swiftstack.com/docs/admin/middleware/s3_middleware.html). See [these docs](#advanced-usage) for details.
-* Local filesystem. Example: `fs://</absolute/path/to/backup>`
+* Local filesystem. Example: `fs://</absolute/path/to/backup>`. Note that `vmbackup` prevents from storing the backup into the directory pointed by `-storageDataPath` command-line flag, since this directory should be managed solely by VictoriaMetrics or `vmstorage`.
 
 `vmbackup` supports incremental and full backups. Incremental backups are created automatically if the destination path already contains data from the previous backup.
 Full backups can be sped up with `-origin` pointing to an already existing backup on the same remote storage. In this case `vmbackup` makes server-side copy for the shared