Skip to content

Commit

Permalink
APPS-1494 Add xdr dc name validation (#216)
Browse files Browse the repository at this point in the history
  • Loading branch information
filkeith authored Feb 2, 2025
1 parent fbb8277 commit 87e1c5b
Show file tree
Hide file tree
Showing 5 changed files with 374 additions and 5 deletions.
8 changes: 8 additions & 0 deletions cmd/internal/app/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,14 @@ func validateBackupXDRParams(params *models.BackupXDR) error {
return fmt.Errorf("backup xdr file limit can't be less than 1")
}

if params.InfoRetryIntervalMilliseconds < 0 {
return fmt.Errorf("backup xdr info retry interval can't be negative")
}

if params.InfoRetriesMultiplier < 0 {
return fmt.Errorf("backup xdr info retries multiplier can't be negative")
}

return nil
}

Expand Down
293 changes: 293 additions & 0 deletions cmd/internal/app/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,26 @@ func Test_validateBackupXDRParams(t *testing.T) {
},
wantErr: "backup xdr file limit can't be less than 1",
},
{
name: "negative info retry interval",
params: &models.BackupXDR{
MaxConnections: 1,
ParallelWrite: 1,
FileLimit: 1,
InfoRetryIntervalMilliseconds: -1,
},
wantErr: "backup xdr info retry interval can't be negative",
},
{
name: "negative info retries multiplier",
params: &models.BackupXDR{
MaxConnections: 1,
ParallelWrite: 1,
FileLimit: 1,
InfoRetriesMultiplier: -1,
},
wantErr: "backup xdr info retries multiplier can't be negative",
},
}

for _, tt := range tests {
Expand All @@ -774,3 +794,276 @@ func Test_validateBackupXDRParams(t *testing.T) {
})
}
}

func TestValidateBackup(t *testing.T) {
tests := []struct {
name string
params *ASBackupParams
wantErr bool
errMsg string
}{
{
name: "Valid backup configuration with output file",
params: &ASBackupParams{
BackupParams: &models.Backup{
OutputFile: "backup.asb",
},
CommonParams: &models.Common{
Namespace: "test",
},
},
wantErr: false,
},
{
name: "Valid backup configuration with directory",
params: &ASBackupParams{
BackupParams: &models.Backup{},
CommonParams: &models.Common{
Directory: "backup-dir",
Namespace: "test",
},
},
wantErr: false,
},
{
name: "Valid backup configuration with estimate",
params: &ASBackupParams{
BackupParams: &models.Backup{
Estimate: true,
EstimateSamples: 100,
},
CommonParams: &models.Common{
Namespace: "test",
},
},
wantErr: false,
},
{
name: "Valid backup configuration with BackupXDR",
params: &ASBackupParams{
BackupXDRParams: &models.BackupXDR{
MaxConnections: 10,
FileLimit: 1000,
},
},
wantErr: false,
},
{
name: "Missing output file and directory",
params: &ASBackupParams{
BackupParams: &models.Backup{},
CommonParams: &models.Common{
Namespace: "test",
},
},
wantErr: true,
errMsg: "output file or directory required",
},
{
name: "Invalid backup params - both output file and directory",
params: &ASBackupParams{
BackupParams: &models.Backup{
OutputFile: "backup.asb",
},
CommonParams: &models.Common{
Directory: "backup-dir",
Namespace: "test",
},
},
wantErr: true,
errMsg: "only one of output-file and directory may be configured at the same time",
},
{
name: "Invalid common params - missing namespace",
params: &ASBackupParams{
BackupParams: &models.Backup{
OutputFile: "backup.asb",
},
CommonParams: &models.Common{},
},
wantErr: true,
errMsg: "namespace is required",
},
{
name: "Invalid BackupXDR params",
params: &ASBackupParams{
BackupXDRParams: &models.BackupXDR{
MaxConnections: 0, // Invalid: must be >= 1
},
},
wantErr: true,
errMsg: "backup xdr max connections can't be less than 1",
},
{
name: "Multiple cloud storage providers configured",
params: &ASBackupParams{
BackupParams: &models.Backup{
OutputFile: "backup.asb",
},
CommonParams: &models.Common{
Namespace: "test",
},
AwsS3: &models.AwsS3{
Region: "us-west-2",
},
GcpStorage: &models.GcpStorage{
BucketName: "my-bucket",
},
},
wantErr: true,
errMsg: "only one cloud provider can be configured",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := validateBackup(tt.params)
if tt.wantErr {
assert.Error(t, err)
if tt.errMsg != "" {
assert.Equal(t, tt.errMsg, err.Error())
}
} else {
assert.NoError(t, err)
}
})
}
}

func TestValidateRestore(t *testing.T) {
tests := []struct {
name string
params *ASRestoreParams
wantErr bool
errMsg string
}{
{
name: "Valid restore configuration with input file",
params: &ASRestoreParams{
RestoreParams: &models.Restore{
InputFile: "backup.asb",
Mode: models.RestoreModeASB,
},
CommonParams: &models.Common{
Namespace: "test",
},
},
wantErr: false,
},
{
name: "Valid restore configuration with directory",
params: &ASRestoreParams{
RestoreParams: &models.Restore{
Mode: models.RestoreModeASB,
},
CommonParams: &models.Common{
Directory: "restore-dir",
Namespace: "test",
},
},
wantErr: false,
},
{
name: "Valid restore configuration with directory list",
params: &ASRestoreParams{
RestoreParams: &models.Restore{
DirectoryList: "dir1,dir2",
ParentDirectory: "parent",
Mode: models.RestoreModeASB,
},
CommonParams: &models.Common{
Namespace: "test",
},
},
wantErr: false,
},
{
name: "Invalid restore mode",
params: &ASRestoreParams{
RestoreParams: &models.Restore{
InputFile: "backup.asb",
Mode: "invalid-mode",
},
CommonParams: &models.Common{
Namespace: "test",
},
},
wantErr: true,
errMsg: "invalid restore mode: invalid-mode",
},
{
name: "Missing input source",
params: &ASRestoreParams{
RestoreParams: &models.Restore{
Mode: models.RestoreModeASB,
},
CommonParams: &models.Common{
Namespace: "test",
},
},
wantErr: true,
errMsg: "input file or directory required",
},
{
name: "Invalid restore params - both input file and directory",
params: &ASRestoreParams{
RestoreParams: &models.Restore{
InputFile: "backup.asb",
Mode: models.RestoreModeASB,
},
CommonParams: &models.Common{
Directory: "restore-dir",
Namespace: "test",
},
},
wantErr: true,
errMsg: "only one of directory and input-file may be configured at the same time",
},
{
name: "Invalid common params - missing namespace",
params: &ASRestoreParams{
RestoreParams: &models.Restore{
InputFile: "backup.asb",
Mode: models.RestoreModeASB,
},
CommonParams: &models.Common{},
},
wantErr: true,
errMsg: "namespace is required",
},
{
name: "Multiple cloud storage providers configured",
params: &ASRestoreParams{
RestoreParams: &models.Restore{
InputFile: "backup.asb",
Mode: models.RestoreModeASB,
},
CommonParams: &models.Common{
Namespace: "test",
},
AwsS3: &models.AwsS3{
Region: "us-west-2",
},
AzureBlob: &models.AzureBlob{
ContainerName: "my-container",
},
},
wantErr: true,
errMsg: "only one cloud provider can be configured",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := validateRestore(tt.params)
if tt.wantErr {
assert.Error(t, err)
if tt.errMsg != "" {
assert.Equal(t, tt.errMsg, err.Error())
}
} else {
assert.NoError(t, err)
}
})
}
}
4 changes: 3 additions & 1 deletion cmd/internal/flags/backup_xdr.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ func (f *BackupXDR) NewFlagSet() *pflag.FlagSet {
"If not set the default value is automatically calculated and appears as the number of CPUs on your machine.")
flagSet.StringVar(&f.DC, "dc",
"dc",
"DC that will be created on source instance for xdr backup.")
"DC that will be created on source instance for xdr backup.\n"+
"DC name can include only Latin lowercase and uppercase letters with no diacritical marks (a-z, A-Z),\n"+
"digits 0-9, underscores (_), hyphens (-), and dollar signs ($). Max length is 31 bytes.")
flagSet.StringVar(&f.LocalAddress, "local-address",
"127.0.0.1",
"Local IP address on which XDR server listens on.")
Expand Down
19 changes: 15 additions & 4 deletions config_backup_xdr.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,15 @@ package backup
import (
"crypto/tls"
"fmt"
"regexp"
"strconv"

a "github.com/aerospike/aerospike-client-go/v8"
"github.com/aerospike/backup-go/models"
)

var expDCName = regexp.MustCompile(`^[a-zA-Z0-9_\-$]+$`)

// ConfigBackupXDR contains configuration for the xdr backup operation.
type ConfigBackupXDR struct {
// InfoPolicy applies to Aerospike Info requests made during backup and
Expand Down Expand Up @@ -92,14 +95,22 @@ func (c *ConfigBackupXDR) validate() error {
return fmt.Errorf("filelimit value must not be negative, got %d", c.FileLimit)
}

if c.ParallelWrite < MinParallel || c.ParallelWrite > MaxParallel {
return fmt.Errorf("parallel write must be between 1 and 1024, got %d", c.ParallelWrite)
}

if c.DC == "" {
return fmt.Errorf("dc name must not be empty")
}

if len(c.DC) > 31 {
return fmt.Errorf("dc name must be less than 32 characters")
}

if !expDCName.MatchString(c.DC) {
return fmt.Errorf("dc name must match ^[a-zA-Z0-9_\\-$]+$")
}

if c.ParallelWrite < MinParallel || c.ParallelWrite > MaxParallel {
return fmt.Errorf("parallel write must be between 1 and 1024, got %d", c.ParallelWrite)
}

if c.LocalAddress == "" {
return fmt.Errorf("local address must not be empty")
}
Expand Down
Loading

0 comments on commit 87e1c5b

Please sign in to comment.