Require BackupDisabledDataDir only if PFSEnabled is true

lib/library_test.go

@@ -46,9 +46,6 @@ func TestValidateNodeConfig(t *testing.T) {
 					"EnableMailServer": true,
 					"DataDir": "/tmp",
 					"MailServerPassword": "status-offline-inbox"
-				},
-				"ShhextConfig": {
-					"BackupDisabledDataDir": "/tmp"
 				}
 			}`,
 			Callback: noErrorsCallback,
@@ -69,9 +66,6 @@ func TestValidateNodeConfig(t *testing.T) {
 				"NoDiscovery": true,
 				"WhisperConfig": {
 					"Enabled": false
-				},
-				"ShhextConfig": {
-					"BackupDisabledDataDir": "/tmp"
 				}
 			}`,
 			Callback: func(t *testing.T, resp APIDetailedResponse) {
@@ -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.Contains(t, resp.Message, "validation: field BackupDisabledDataDir is required if PFSEnabled is true")
-				require.Equal(t, "NodeConfig.ShhextConfig.BackupDisabledDataDir", resp.FieldErrors[0].Parameter)
-				require.Contains(t, resp.Message, "validation: validation failed")
 			},
 		},
 		{
@@ -110,9 +102,6 @@ func TestValidateNodeConfig(t *testing.T) {
 				"NoDiscovery": true,
 				"WhisperConfig": {
 					"Enabled": false
-				},
-				"ShhextConfig": {
-					"BackupDisabledDataDir": "/tmp"
 				}
 			}`,
 			Callback: func(t *testing.T, resp APIDetailedResponse) {
@@ -132,9 +121,6 @@ func TestValidateNodeConfig(t *testing.T) {
 				"WhisperConfig": {
 					"Enabled": true,
 					"EnableMailServer": true
-				},
-				"ShhextConfig": {
-					"BackupDisabledDataDir": "/tmp"
 				}
 			}`,
 			Callback: func(t *testing.T, resp APIDetailedResponse) {
@@ -153,9 +139,6 @@ func TestValidateNodeConfig(t *testing.T) {
 				"WhisperConfig": {
 					"Enabled": true,
 					"EnableMailServer": false
-				},
-				"ShhextConfig": {
-					"BackupDisabledDataDir": "/tmp"
 				}
 			}`,
 			Callback: noErrorsCallback,
@@ -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.NetworkID":   "required",
-					"NodeConfig.DataDir":                            "required",
+					"NodeConfig.DataDir":     "required",
-					"NodeConfig.ShhextConfig.BackupDisabledDataDir": "required",
+					"NodeConfig.KeyStoreDir": "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())

params/config.go

@@ -2,6 +2,7 @@ package params
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net/url"
@@ -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"`
@@ -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.
@@ -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

params/config_test.go

@@ -60,10 +60,7 @@ func TestNewConfigFromJSON(t *testing.T) {
 		"NetworkId": 3,
 		"DataDir": "` + tmpDir + `",
 		"KeyStoreDir": "` + tmpDir + `",
-		"NoDiscovery": true,
+		"NoDiscovery": true
-		"ShhextConfig": {
-			"BackupDisabledDataDir": "` + tmpDir + `"
-		}
 	}`
 	c, err := params.NewConfigFromJSON(json)
 	require.NoError(t, err)
@@ -107,10 +104,7 @@ func TestNodeConfigValidate(t *testing.T) {
 				"DataDir": "/tmp/data",
 				"BackupDisabledDataDir": "/tmp/data",
 				"KeyStoreDir": "/tmp/data",
-				"NoDiscovery": true,
+				"NoDiscovery": true
-				"ShhextConfig": {
-					"BackupDisabledDataDir": "/tmp"
-				}
 			}`,
 		},
 		{
@@ -127,10 +121,9 @@ func TestNodeConfigValidate(t *testing.T) {
 			Name:   "Validate all required fields",
 			Config: `{}`,
 			FieldErrors: map[string]string{
-				"NetworkID":             "required",
+				"NetworkID":   "required",
-				"DataDir":               "required",
+				"DataDir":     "required",
-				"BackupDisabledDataDir": "required",
+				"KeyStoreDir": "required",
-				"KeyStoreDir":           "required",
 			},
 		},
 		{
@@ -138,12 +131,8 @@ func TestNodeConfigValidate(t *testing.T) {
 			Config: `{
 				"NetworkId": 1,
 				"DataDir": "/some/dir",
-				"BackupDisabledDataDir": "/some/dir",
 				"KeyStoreDir": "/some/dir",
-				"Name": "invalid/name",
+				"Name": "invalid/name"
-				"ShhextConfig": {
-					"BackupDisabledDataDir": "/tmp"
-				}
 			}`,
 			FieldErrors: map[string]string{
 				"Name": "excludes",
@@ -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",
+				"NodeKey": "foo"
-				"ShhextConfig": {
-					"BackupDisabledDataDir": "/tmp"
-				}
 			}`,
 			Error: "NodeKey is invalid",
 		},
@@ -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",
@@ -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"
 				}
 			}`,
 		},
@@ -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"
 				}
 			}`,
 		},
@@ -221,12 +194,8 @@ func TestNodeConfigValidate(t *testing.T) {
 			Config: `{
 				"NetworkId": 1,
 				"DataDir": "/some/dir",
-				"BackupDisabledDataDir": "/some/dir",
 				"KeyStoreDir": "/some/dir",
-				"NoDiscovery": false,
+				"NoDiscovery": false
-				"ShhextConfig": {
-					"BackupDisabledDataDir": "/tmp"
-				}
 			}`,
 			Error: "NoDiscovery is false, but ClusterConfig.BootNodes is empty",
 		},
@@ -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,
+				"Rendezvous": true
-				"ShhextConfig": {
-					"BackupDisabledDataDir": "/tmp"
-				}
 			}`,
 			Error: "Rendezvous is enabled, but ClusterConfig.RendezvousNodes is empty",
 		},
@@ -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",
@@ -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",
@@ -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",
@@ -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) {
@@ -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",
@@ -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) {
@@ -371,9 +318,6 @@ func TestNodeConfigValidate(t *testing.T) {
 					"EnableMailServer": true,
 					"DataDir": "/foo",
 					"MailServerAsymKey": "bar"
-				},
-				"ShhextConfig": {
-					"BackupDisabledDataDir": "/tmp"
 				}
 			}`,
 			Error: "WhisperConfig.MailServerAsymKey is invalid",
@@ -401,10 +345,7 @@ func TestNodeConfigValidate(t *testing.T) {
 			Config: `{
 				"NetworkId": 1,
 				"DataDir": "/some/dir",
-				"KeyStoreDir": "/some/dir",
+				"KeyStoreDir": "/some/dir"
-				"ShhextConfig": {
-					"BackupDisabledDataDir": "/tmp"
-				}
 			}`,
 			CheckFunc: func(t *testing.T, config *params.NodeConfig) {
 				require.Equal(t, []string{"localhost"}, config.HTTPVirtualHosts)
@@ -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"],
+				"HTTPCors": ["http://my.domain.com:8080"]
-				"ShhextConfig": {
-					"BackupDisabledDataDir": "/tmp"
-				}
 			}`,
 			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 {