Skip to content

Commit bf61ba0

Browse files
sandraromanchenkosandraromanchenko
authored
Merge branch 'dev' into main
2 parents cd59a62 + 51e704a commit bf61ba0

5 files changed

Lines changed: 232 additions & 11 deletions

File tree

‎pbm/backup/physical.go‎

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -511,16 +511,32 @@ func (b *Backup) uploadPhysical(
511511
l log.LogEvent,
512512
) error {
513513
l.Info("uploading data")
514-
dataFiles, err := uploadFiles(ctx, data, bcp.Name+"/"+rsMeta.Name, dbpath,
515-
b.typ == defs.IncrementalBackup, stg, bcp.Compression, bcp.CompressionLevel, l)
514+
dataFiles, err := uploadFiles(
515+
ctx,
516+
data,
517+
bcp.Name+"/"+rsMeta.Name,
518+
dbpath,
519+
b.typ == defs.IncrementalBackup,
520+
stg,
521+
bcp.Compression,
522+
bcp.CompressionLevel,
523+
)
516524
if err != nil {
517525
return errors.Wrap(err, "upload data files")
518526
}
519527
l.Info("uploading data done")
520528

521529
l.Info("uploading journals")
522-
ju, err := uploadFiles(ctx, jrnls, bcp.Name+"/"+rsMeta.Name, dbpath,
523-
false, stg, bcp.Compression, bcp.CompressionLevel, l)
530+
ju, err := uploadFiles(
531+
ctx,
532+
jrnls,
533+
bcp.Name+"/"+rsMeta.Name,
534+
dbpath,
535+
false,
536+
stg,
537+
bcp.Compression,
538+
bcp.CompressionLevel,
539+
)
524540
if err != nil {
525541
return errors.Wrap(err, "upload journal files")
526542
}
@@ -533,7 +549,12 @@ func (b *Backup) uploadPhysical(
533549
sizeUncompressed := int64(0)
534550
for _, f := range filelist {
535551
size += f.StgSize
536-
sizeUncompressed += f.Size
552+
if f.StgSize != 0 {
553+
// maintain uncompressed size just for the files that have a disk footprint,
554+
// backup meta might contains files which are unchanged from the previous
555+
// inc/base backup
556+
sizeUncompressed += f.Size
557+
}
537558
}
538559

539560
filelistPath := path.Join(bcp.Name, rsMeta.Name, FilelistName)
@@ -633,7 +654,6 @@ func uploadFiles(
633654
stg storage.Storage,
634655
comprT compress.CompressionType,
635656
comprL *int,
636-
l log.LogEvent,
637657
) ([]File, error) {
638658
if len(files) == 0 {
639659
return nil, nil
@@ -675,7 +695,7 @@ func uploadFiles(
675695
continue
676696
}
677697

678-
fw, err := writeFile(ctx, wfile, path.Join(subdir, trim(wfile.Name)), stg, comprT, comprL, l)
698+
fw, err := writeFile(ctx, wfile, path.Join(subdir, trim(wfile.Name)), stg, comprT, comprL)
679699
if err != nil {
680700
return data, errors.Wrapf(err, "upload file `%s`", wfile.Name)
681701
}
@@ -690,7 +710,7 @@ func uploadFiles(
690710
return data, nil
691711
}
692712

693-
f, err := writeFile(ctx, wfile, path.Join(subdir, trim(wfile.Name)), stg, comprT, comprL, l)
713+
f, err := writeFile(ctx, wfile, path.Join(subdir, trim(wfile.Name)), stg, comprT, comprL)
694714
if err != nil {
695715
return data, errors.Wrapf(err, "upload file `%s`", wfile.Name)
696716
}
@@ -708,7 +728,6 @@ func writeFile(
708728
stg storage.Storage,
709729
compression compress.CompressionType,
710730
compressLevel *int,
711-
l log.LogEvent,
712731
) (*File, error) {
713732
fstat, err := os.Stat(src.Name)
714733
if err != nil {
@@ -739,7 +758,7 @@ func writeFile(
739758

740759
return &File{
741760
Name: src.Name,
742-
Size: fstat.Size(),
761+
Size: sz,
743762
Fmode: fstat.Mode(),
744763
StgSize: finf.Size,
745764
Off: src.Off,

‎pbm/config/config.go‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,7 @@ func SetConfig(ctx context.Context, m connect.Client, cfg *Config) error {
542542
if err := cfg.Storage.Cast(); err != nil {
543543
return errors.Wrap(err, "cast storage")
544544
}
545+
sanitizeStoragePaths(&cfg.Storage)
545546

546547
if cfg.Storage.Type == storage.S3 {
547548
// call the function for notification purpose.
@@ -613,6 +614,10 @@ func SetConfigVar(ctx context.Context, m connect.Client, key, val string) error
613614
return errors.Wrapf(err, "casting value of %s", key)
614615
}
615616

617+
if isStoragePathKey(key) {
618+
v = storage.TrimSlashes(v.(string))
619+
}
620+
616621
// TODO: how to be with special case options like pitr.enabled
617622
switch key {
618623
case "pitr.enabled":
@@ -635,6 +640,50 @@ func SetConfigVar(ctx context.Context, m connect.Client, key, val string) error
635640
return errors.Wrap(err, "write to db")
636641
}
637642

643+
// sanitizeStoragePaths trims leading/trailing slashes from bucket, container,
644+
// and prefix values in the storage config. Extra slashes in these values cause
645+
// S3-compatible API signature errors or backup discovery failures.
646+
func sanitizeStoragePaths(s *StorageConf) {
647+
switch s.Type {
648+
case storage.S3:
649+
if s.S3 != nil {
650+
s.S3.Bucket = storage.TrimSlashes(s.S3.Bucket)
651+
s.S3.Prefix = storage.TrimSlashes(s.S3.Prefix)
652+
}
653+
case storage.Minio:
654+
if s.Minio != nil {
655+
s.Minio.Bucket = storage.TrimSlashes(s.Minio.Bucket)
656+
s.Minio.Prefix = storage.TrimSlashes(s.Minio.Prefix)
657+
}
658+
case storage.GCS:
659+
if s.GCS != nil {
660+
s.GCS.Bucket = storage.TrimSlashes(s.GCS.Bucket)
661+
s.GCS.Prefix = storage.TrimSlashes(s.GCS.Prefix)
662+
}
663+
case storage.Azure:
664+
if s.Azure != nil {
665+
s.Azure.Container = storage.TrimSlashes(s.Azure.Container)
666+
s.Azure.Prefix = storage.TrimSlashes(s.Azure.Prefix)
667+
}
668+
case storage.OSS:
669+
if s.OSS != nil {
670+
s.OSS.Bucket = storage.TrimSlashes(s.OSS.Bucket)
671+
s.OSS.Prefix = storage.TrimSlashes(s.OSS.Prefix)
672+
}
673+
}
674+
}
675+
676+
// isStoragePathKey reports whether the config key refers to a storage
677+
// bucket, container, or prefix field that should have slashes trimmed.
678+
func isStoragePathKey(key string) bool {
679+
if !strings.HasPrefix(key, "storage.") {
680+
return false
681+
}
682+
return strings.HasSuffix(key, ".bucket") ||
683+
strings.HasSuffix(key, ".container") ||
684+
strings.HasSuffix(key, ".prefix")
685+
}
686+
638687
func confSetPITR(ctx context.Context, m connect.Client, value bool) error {
639688
ct, err := topo.GetClusterTime(ctx, m)
640689
if err != nil {

‎pbm/config/config_test.go‎

Lines changed: 144 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ import (
99
"time"
1010

1111
"github.com/google/go-cmp/cmp"
12-
"github.com/percona/percona-backup-mongodb/pbm/storage/oss"
12+
"github.com/stretchr/testify/assert"
13+
"github.com/stretchr/testify/require"
1314
"github.com/testcontainers/testcontainers-go"
1415
"github.com/testcontainers/testcontainers-go/modules/mongodb"
1516
"go.mongodb.org/mongo-driver/mongo"
@@ -22,6 +23,7 @@ import (
2223
"github.com/percona/percona-backup-mongodb/pbm/storage/fs"
2324
"github.com/percona/percona-backup-mongodb/pbm/storage/gcs"
2425
"github.com/percona/percona-backup-mongodb/pbm/storage/mio"
26+
"github.com/percona/percona-backup-mongodb/pbm/storage/oss"
2527
"github.com/percona/percona-backup-mongodb/pbm/storage/s3"
2628
)
2729

@@ -458,6 +460,147 @@ func TestConfig(t *testing.T) {
458460
})
459461
}
460462

463+
func TestSanitizeStoragePaths(t *testing.T) {
464+
tests := []struct {
465+
name string
466+
conf StorageConf
467+
wantBucket string
468+
wantPrefix string
469+
}{
470+
{
471+
"s3 trailing slash on bucket",
472+
StorageConf{Type: storage.S3, S3: &s3.Config{Bucket: "bcp/", Prefix: "data"}},
473+
"bcp", "data",
474+
},
475+
{
476+
"s3 leading slash on prefix",
477+
StorageConf{Type: storage.S3, S3: &s3.Config{Bucket: "bcp", Prefix: "/data/pbm"}},
478+
"bcp", "data/pbm",
479+
},
480+
{
481+
"s3 both slashes",
482+
StorageConf{Type: storage.S3, S3: &s3.Config{Bucket: "bcp/", Prefix: "/data/"}},
483+
"bcp", "data",
484+
},
485+
{
486+
"s3 multiple slashes",
487+
StorageConf{Type: storage.S3, S3: &s3.Config{Bucket: "///bcp///", Prefix: "///data///"}},
488+
"bcp", "data",
489+
},
490+
{
491+
"s3 clean values",
492+
StorageConf{Type: storage.S3, S3: &s3.Config{Bucket: "bcp", Prefix: "data"}},
493+
"bcp", "data",
494+
},
495+
{
496+
"minio trailing slash",
497+
StorageConf{Type: storage.Minio, Minio: &mio.Config{Bucket: "bcp/", Prefix: "/pfx/"}},
498+
"bcp", "pfx",
499+
},
500+
{
501+
"gcs leading slash on prefix",
502+
StorageConf{Type: storage.GCS, GCS: &gcs.Config{Bucket: "bcp/", Prefix: "/pfx"}},
503+
"bcp", "pfx",
504+
},
505+
{
506+
"oss trailing slash",
507+
StorageConf{Type: storage.OSS, OSS: &oss.Config{Bucket: "bcp/", Prefix: "/pfx/"}},
508+
"bcp", "pfx",
509+
},
510+
}
511+
512+
for _, tt := range tests {
513+
t.Run(tt.name, func(t *testing.T) {
514+
sanitizeStoragePaths(&tt.conf)
515+
switch tt.conf.Type {
516+
case storage.S3:
517+
assert.Equal(t, tt.wantBucket, tt.conf.S3.Bucket)
518+
assert.Equal(t, tt.wantPrefix, tt.conf.S3.Prefix)
519+
case storage.Minio:
520+
assert.Equal(t, tt.wantBucket, tt.conf.Minio.Bucket)
521+
assert.Equal(t, tt.wantPrefix, tt.conf.Minio.Prefix)
522+
case storage.GCS:
523+
assert.Equal(t, tt.wantBucket, tt.conf.GCS.Bucket)
524+
assert.Equal(t, tt.wantPrefix, tt.conf.GCS.Prefix)
525+
case storage.OSS:
526+
assert.Equal(t, tt.wantBucket, tt.conf.OSS.Bucket)
527+
assert.Equal(t, tt.wantPrefix, tt.conf.OSS.Prefix)
528+
}
529+
})
530+
}
531+
}
532+
533+
func TestSanitizeStoragePathsAzure(t *testing.T) {
534+
conf := StorageConf{Type: storage.Azure, Azure: &azure.Config{Container: "cnt/", Prefix: "/pfx/"}}
535+
sanitizeStoragePaths(&conf)
536+
assert.Equal(t, "cnt", conf.Azure.Container)
537+
assert.Equal(t, "pfx", conf.Azure.Prefix)
538+
}
539+
540+
func TestIsStoragePathKey(t *testing.T) {
541+
// Storage keys that should match.
542+
for _, key := range []string{
543+
"storage.s3.bucket",
544+
"storage.s3.prefix",
545+
"storage.minio.bucket",
546+
"storage.minio.prefix",
547+
"storage.gcs.bucket",
548+
"storage.gcs.prefix",
549+
"storage.azure.container",
550+
"storage.azure.prefix",
551+
"storage.oss.bucket",
552+
"storage.oss.prefix",
553+
} {
554+
assert.True(t, isStoragePathKey(key), "expected true for %q", key)
555+
}
556+
557+
// Non-storage keys must not match.
558+
for _, key := range []string{
559+
"pitr.enabled",
560+
"pitr.compression",
561+
"storage.s3.region",
562+
"storage.s3.debugLogLevels",
563+
"storage.filesystem.path",
564+
"bucket",
565+
"prefix",
566+
} {
567+
assert.False(t, isStoragePathKey(key), "expected false for %q", key)
568+
}
569+
}
570+
571+
func TestSetConfigVarTrimsSlashes(t *testing.T) {
572+
ctx := context.Background()
573+
574+
// Set up initial config so SetConfigVar works
575+
emptyCfg := &Config{
576+
Storage: StorageConf{Type: storage.S3, S3: &s3.Config{Bucket: "init"}},
577+
}
578+
err := SetConfig(ctx, connClient, emptyCfg)
579+
require.NoError(t, err)
580+
581+
tests := []struct {
582+
key string
583+
val string
584+
wantVal string
585+
}{
586+
{"storage.s3.bucket", "bcp/", "bcp"},
587+
{"storage.s3.prefix", "/data/pbm/", "data/pbm"},
588+
{"storage.s3.bucket", "///bcp///", "bcp"},
589+
{"storage.s3.prefix", "clean", "clean"},
590+
}
591+
592+
for _, tt := range tests {
593+
t.Run(tt.key+"="+tt.val, func(t *testing.T) {
594+
err := SetConfigVar(ctx, connClient, tt.key, tt.val)
595+
require.NoError(t, err)
596+
597+
got, err := GetConfigVar(ctx, connClient, tt.key)
598+
require.NoError(t, err)
599+
assert.Equal(t, tt.wantVal, got)
600+
})
601+
}
602+
}
603+
461604
func boolPtr(b bool) *bool {
462605
return &b
463606
}

‎pbm/config/profile.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ func AddProfile(ctx context.Context, m connect.Client, profile *Config) error {
6464
if err := profile.Storage.Cast(); err != nil {
6565
return errors.Wrap(err, "cast storage")
6666
}
67+
sanitizeStoragePaths(&profile.Storage)
6768

6869
if profile.Storage.Type == storage.S3 {
6970
// call the function for notification purpose.

‎pbm/storage/storage.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"context"
66
"fmt"
77
"io"
8+
"strings"
89

910
"github.com/percona/percona-backup-mongodb/pbm/compress"
1011
"github.com/percona/percona-backup-mongodb/pbm/defs"
@@ -393,3 +394,11 @@ func (s MaskedString) MarshalYAML() (any, error) {
393394
}
394395
return "***", nil
395396
}
397+
398+
// TrimSlashes removes leading and trailing '/' characters from a storage path
399+
// component (bucket, prefix, container). This prevents issues with S3-compatible
400+
// APIs where extra slashes in bucket names or prefixes cause signature errors
401+
// or object discovery failures.
402+
func TrimSlashes(s string) string {
403+
return strings.Trim(s, "/")
404+
}

0 commit comments

Comments
 (0)