Skip to content

Commit

Permalink
Require BackupDisabledDataDir only if PFSEnabled is true
Browse files Browse the repository at this point in the history
  • Loading branch information
@dshulyak
dshulyak committed Jan 22, 2019
1 parent 0e48a02 commit 1a365a5
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 100 deletions.
28 changes: 5 additions & 23 deletions lib/library_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ func TestValidateNodeConfig(t *testing.T) {
"EnableMailServer": true,
"DataDir": "/tmp",
"MailServerPassword": "status-offline-inbox"
},
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
}`,
Callback: noErrorsCallback,
Expand All @@ -69,9 +66,6 @@ func TestValidateNodeConfig(t *testing.T) {
"NoDiscovery": true,
"WhisperConfig": {
"Enabled": false
},
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
}`,
Callback: func(t *testing.T, resp APIDetailedResponse) {
Expand All @@ -97,9 +91,7 @@ func TestValidateNodeConfig(t *testing.T) {
}`,
Callback: func(t *testing.T, resp APIDetailedResponse) {
require.False(t, resp.Status)
require.Equal(t, 1, len(resp.FieldErrors))
require.Equal(t, "NodeConfig.ShhextConfig.BackupDisabledDataDir", resp.FieldErrors[0].Parameter)
require.Contains(t, resp.Message, "validation: validation failed")
require.Contains(t, resp.Message, "validation: field BackupDisabledDataDir is required if PFSEnabled is true")
},
},
{
Expand All @@ -110,9 +102,6 @@ func TestValidateNodeConfig(t *testing.T) {
"NoDiscovery": true,
"WhisperConfig": {
"Enabled": false
},
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
}`,
Callback: func(t *testing.T, resp APIDetailedResponse) {
Expand All @@ -132,9 +121,6 @@ func TestValidateNodeConfig(t *testing.T) {
"WhisperConfig": {
"Enabled": true,
"EnableMailServer": true
},
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
}`,
Callback: func(t *testing.T, resp APIDetailedResponse) {
Expand All @@ -153,9 +139,6 @@ func TestValidateNodeConfig(t *testing.T) {
"WhisperConfig": {
"Enabled": true,
"EnableMailServer": false
},
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
}`,
Callback: noErrorsCallback,
Expand All @@ -165,15 +148,14 @@ func TestValidateNodeConfig(t *testing.T) {
Config: `{}`,
Callback: func(t *testing.T, resp APIDetailedResponse) {
required := map[string]string{
"NodeConfig.NetworkID": "required",
"NodeConfig.DataDir": "required",
"NodeConfig.ShhextConfig.BackupDisabledDataDir": "required",
"NodeConfig.KeyStoreDir": "required",
"NodeConfig.NetworkID": "required",
"NodeConfig.DataDir": "required",
"NodeConfig.KeyStoreDir": "required",
}

require.False(t, resp.Status)
require.Contains(t, resp.Message, "validation: validation failed")
require.Equal(t, 4, len(resp.FieldErrors))
require.Equal(t, 3, len(resp.FieldErrors))

for _, err := range resp.FieldErrors {
require.Contains(t, required, err.Parameter, err.Error())
Expand Down
15 changes: 11 additions & 4 deletions params/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package params

import (
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"net/url"
Expand Down Expand Up @@ -283,7 +284,7 @@ type NodeConfig struct {
WhisperConfig WhisperConfig `json:"WhisperConfig," validate:"structonly"`

// ShhextConfig keeps configuration for service running under shhext namespace.
ShhextConfig ShhextConfig `json:"ShhextConfig," validate:"required"`
ShhextConfig ShhextConfig `json:"ShhextConfig," validate:"structonly"`

// SwarmConfig extra configuration for Swarm and ENS
SwarmConfig SwarmConfig `json:"SwarmConfig," validate:"structonly"`
Expand All @@ -302,12 +303,12 @@ type NodeConfig struct {

// ShhextConfig defines options used by shhext service.
type ShhextConfig struct {
PFSEnabled bool
// BackupDisabledDataDir is the file system folder the node should use for any data storage needs that it doesn't want backed up.
BackupDisabledDataDir string `validate:"required"`
BackupDisabledDataDir string
// InstallationId id of the current installation
InstallationID string
DebugAPIEnabled bool
PFSEnabled bool
// MailServerConfirmations should be true if client wants to receive confirmatons only from a selected mail servers.
MailServerConfirmations bool
// EnableConnectionManager turns on management of the mail server connections if true.
Expand All @@ -322,7 +323,13 @@ type ShhextConfig struct {

// Validate validates the ShhextConfig struct and returns an error if inconsistent values are found
func (c *ShhextConfig) Validate(validate *validator.Validate) error {
return validate.Struct(c)
if err := validate.Struct(c); err != nil {
return err
}
if c.PFSEnabled && len(c.BackupDisabledDataDir) == 0 {
return errors.New("field BackupDisabledDataDir is required if PFSEnabled is true")
}
return nil
}

// Option is an additional setting when creating a NodeConfig
Expand Down
104 changes: 31 additions & 73 deletions params/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,7 @@ func TestNewConfigFromJSON(t *testing.T) {
"NetworkId": 3,
"DataDir": "` + tmpDir + `",
"KeyStoreDir": "` + tmpDir + `",
"NoDiscovery": true,
"ShhextConfig": {
"BackupDisabledDataDir": "` + tmpDir + `"
}
"NoDiscovery": true
}`
c, err := params.NewConfigFromJSON(json)
require.NoError(t, err)
Expand Down Expand Up @@ -107,10 +104,7 @@ func TestNodeConfigValidate(t *testing.T) {
"DataDir": "/tmp/data",
"BackupDisabledDataDir": "/tmp/data",
"KeyStoreDir": "/tmp/data",
"NoDiscovery": true,
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
"NoDiscovery": true
}`,
},
{
Expand All @@ -127,23 +121,18 @@ func TestNodeConfigValidate(t *testing.T) {
Name: "Validate all required fields",
Config: `{}`,
FieldErrors: map[string]string{
"NetworkID": "required",
"DataDir": "required",
"BackupDisabledDataDir": "required",
"KeyStoreDir": "required",
"NetworkID": "required",
"DataDir": "required",
"KeyStoreDir": "required",
},
},
{
Name: "Validate that Name does not contain slash",
Config: `{
"NetworkId": 1,
"DataDir": "/some/dir",
"BackupDisabledDataDir": "/some/dir",
"KeyStoreDir": "/some/dir",
"Name": "invalid/name",
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
"Name": "invalid/name"
}`,
FieldErrors: map[string]string{
"Name": "excludes",
Expand All @@ -154,13 +143,9 @@ func TestNodeConfigValidate(t *testing.T) {
Config: `{
"NetworkId": 1,
"DataDir": "/some/dir",
"BackupDisabledDataDir": "/some/dir",
"KeyStoreDir": "/some/dir",
"NoDiscovery": true,
"NodeKey": "foo",
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
"NodeKey": "foo"
}`,
Error: "NodeKey is invalid",
},
Expand All @@ -169,15 +154,11 @@ func TestNodeConfigValidate(t *testing.T) {
Config: `{
"NetworkId": 1,
"DataDir": "/some/dir",
"BackupDisabledDataDir": "/some/dir",
"KeyStoreDir": "/some/dir",
"NoDiscovery": true,
"UpstreamConfig": {
"Enabled": true,
"URL": "[bad.url]"
},
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
}`,
Error: "'[bad.url]' is invalid",
Expand All @@ -187,15 +168,11 @@ func TestNodeConfigValidate(t *testing.T) {
Config: `{
"NetworkId": 1,
"DataDir": "/some/dir",
"BackupDisabledDataDir": "/some/dir",
"KeyStoreDir": "/some/dir",
"NoDiscovery": true,
"UpstreamConfig": {
"Enabled": false,
"URL": "[bad.url]"
},
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
}`,
},
Expand All @@ -204,15 +181,11 @@ func TestNodeConfigValidate(t *testing.T) {
Config: `{
"NetworkId": 1,
"DataDir": "/some/dir",
"BackupDisabledDataDir": "/some/dir",
"KeyStoreDir": "/some/dir",
"NoDiscovery": true,
"UpstreamConfig": {
"Enabled": true,
"URL": "` + params.MainnetEthereumNetworkURL + `"
},
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
}`,
},
Expand All @@ -221,12 +194,8 @@ func TestNodeConfigValidate(t *testing.T) {
Config: `{
"NetworkId": 1,
"DataDir": "/some/dir",
"BackupDisabledDataDir": "/some/dir",
"KeyStoreDir": "/some/dir",
"NoDiscovery": false,
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
"NoDiscovery": false
}`,
Error: "NoDiscovery is false, but ClusterConfig.BootNodes is empty",
},
Expand All @@ -235,13 +204,9 @@ func TestNodeConfigValidate(t *testing.T) {
Config: `{
"NetworkId": 1,
"DataDir": "/some/dir",
"BackupDisabledDataDir": "/some/dir",
"KeyStoreDir": "/some/dir",
"NoDiscovery": true,
"Rendezvous": true,
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
"Rendezvous": true
}`,
Error: "Rendezvous is enabled, but ClusterConfig.RendezvousNodes is empty",
},
Expand All @@ -255,9 +220,6 @@ func TestNodeConfigValidate(t *testing.T) {
"Rendezvous": false,
"ClusterConfig": {
"RendezvousNodes": ["a"]
},
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
}`,
Error: "Rendezvous is disabled, but ClusterConfig.RendezvousNodes is not empty",
Expand All @@ -273,9 +235,6 @@ func TestNodeConfigValidate(t *testing.T) {
"Enabled": true,
"EnableMailServer": true,
"MailserverPassword": "foo"
},
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
}`,
Error: "WhisperConfig.DataDir must be specified when WhisperConfig.EnableMailServer is true",
Expand All @@ -292,9 +251,6 @@ func TestNodeConfigValidate(t *testing.T) {
"EnableMailServer": true,
"DataDir": "/other/dir",
"MailserverPassword": "foo"
},
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
}`,
Error: "WhisperConfig.DataDir must start with DataDir fragment",
Expand All @@ -311,9 +267,6 @@ func TestNodeConfigValidate(t *testing.T) {
"EnableMailServer": true,
"DataDir": "/some/dir",
"MailserverPassword": "foo"
},
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
}`,
CheckFunc: func(t *testing.T, config *params.NodeConfig) {
Expand All @@ -331,9 +284,6 @@ func TestNodeConfigValidate(t *testing.T) {
"Enabled": true,
"EnableMailServer": true,
"DataDir": "/some/dir"
},
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
}`,
Error: "WhisperConfig.MailServerPassword or WhisperConfig.MailServerAsymKey must be specified when WhisperConfig.EnableMailServer is true",
Expand All @@ -350,9 +300,6 @@ func TestNodeConfigValidate(t *testing.T) {
"EnableMailServer": true,
"DataDir": "/some/dir",
"MailServerAsymKey": "06c365919f1fc8e13ff79a84f1dd14b7e45b869aa5fc0e34940481ee20d32f90"
},
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
}`,
CheckFunc: func(t *testing.T, config *params.NodeConfig) {
Expand All @@ -371,9 +318,6 @@ func TestNodeConfigValidate(t *testing.T) {
"EnableMailServer": true,
"DataDir": "/foo",
"MailServerAsymKey": "bar"
},
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
}`,
Error: "WhisperConfig.MailServerAsymKey is invalid",
Expand Down Expand Up @@ -401,10 +345,7 @@ func TestNodeConfigValidate(t *testing.T) {
Config: `{
"NetworkId": 1,
"DataDir": "/some/dir",
"KeyStoreDir": "/some/dir",
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
"KeyStoreDir": "/some/dir"
}`,
CheckFunc: func(t *testing.T, config *params.NodeConfig) {
require.Equal(t, []string{"localhost"}, config.HTTPVirtualHosts)
Expand All @@ -418,16 +359,33 @@ func TestNodeConfigValidate(t *testing.T) {
"DataDir": "/some/dir",
"KeyStoreDir": "/some/dir",
"HTTPVirtualHosts": ["my.domain.com"],
"HTTPCors": ["http://my.domain.com:8080"],
"ShhextConfig": {
"BackupDisabledDataDir": "/tmp"
}
"HTTPCors": ["http://my.domain.com:8080"]
}`,
CheckFunc: func(t *testing.T, config *params.NodeConfig) {
require.Equal(t, []string{"my.domain.com"}, config.HTTPVirtualHosts)
require.Equal(t, []string{"http://my.domain.com:8080"}, config.HTTPCors)
},
},
{
Name: "ShhextConfig is not required",
Config: `{
"NetworkId": 1,
"DataDir": "/some/dir",
"KeyStoreDir": "/some/dir"
}`,
},
{
Name: "BackupDisabledDataDir must be set if PFSEnabled is true",
Config: `{
"NetworkId": 1,
"DataDir": "/some/dir",
"KeyStoreDir": "/some/dir",
"ShhextConfig": {
"PFSEnabled": true
}
}`,
Error: "field BackupDisabledDataDir is required if PFSEnabled is true",
},
}

for _, tc := range testCases {
Expand Down

0 comments on commit 1a365a5

Please sign in to comment.