storagenode/main: map aliases for kademlia config values (#3118)

cmd/storagenode/entrypoint

@@ -11,8 +11,8 @@ RUN_PARAMS="${RUN_PARAMS:-} --identity-dir identity"
 RUN_PARAMS="${RUN_PARAMS:-} --metrics.app-suffix=-alpha"
 RUN_PARAMS="${RUN_PARAMS:-} --metrics.interval=30m"
 RUN_PARAMS="${RUN_PARAMS:-} --contact.external-address=${ADDRESS}"
-RUN_PARAMS="${RUN_PARAMS:-} --kademlia.operator.email=${EMAIL}"
+RUN_PARAMS="${RUN_PARAMS:-} --operator.email=${EMAIL}"
-RUN_PARAMS="${RUN_PARAMS:-} --kademlia.operator.wallet=${WALLET}"
+RUN_PARAMS="${RUN_PARAMS:-} --operator.wallet=${WALLET}"
 RUN_PARAMS="${RUN_PARAMS:-} --console.address=:14002"
 RUN_PARAMS="${RUN_PARAMS:-} --console.static-dir=/app"
 RUN_PARAMS="${RUN_PARAMS:-} --storage.allocated-bandwidth=${BANDWIDTH}"

cmd/storagenode/main.go

@@ -31,6 +31,16 @@ type StorageNodeFlags struct {
 	EditConf bool `default:"false" help:"open config in default editor"`
 
 	storagenode.Config
+
+	Deprecated
+}
+
+// Deprecated contains deprecated config structs
+type Deprecated struct {
+	Kademlia struct {
+		ExternalAddress string `user:"true" help:"the public address of the Kademlia node, useful for nodes behind NAT" default:""`
+		Operator        storagenode.OperatorConfig
+	}
 }
 
 var (
@@ -120,6 +130,8 @@ func cmdRun(cmd *cobra.Command, args []string) (err error) {
 	ctx, _ := process.Ctx(cmd)
 	log := zap.L()
 
+	mapDeprecatedConfigs(log)
+
 	identity, err := runCfg.Identity.Load()
 	if err != nil {
 		zap.S().Fatal(err)
@@ -291,6 +303,48 @@ func cmdDiag(cmd *cobra.Command, args []string) (err error) {
 	return nil
 }
 
+// maps deprecated config values to new values if applicable
+func mapDeprecatedConfigs(log *zap.Logger) {
+	type migration struct {
+		newValue        *string
+		newConfigString string
+		oldValue        *string
+		oldConfigString string
+	}
+	migrations := []migration{
+		{
+			newValue:        &runCfg.Contact.ExternalAddress,
+			newConfigString: "contact.external-address",
+			oldValue:        &runCfg.Kademlia.ExternalAddress,
+			oldConfigString: "kademlia.external-address",
+		},
+		{
+			newValue:        &runCfg.Operator.Wallet,
+			newConfigString: "operator.wallet",
+			oldValue:        &runCfg.Kademlia.Operator.Wallet,
+			oldConfigString: "kademlia.operator.wallet",
+		},
+		{
+			newValue:        &runCfg.Operator.Email,
+			newConfigString: "operator.email",
+			oldValue:        &runCfg.Kademlia.Operator.Email,
+			oldConfigString: "kademlia.operator.email",
+		},
+	}
+
+	for _, migration := range migrations {
+		if *migration.newValue != "" && *migration.oldValue != "" {
+			log.Sugar().Debugf("Both %s and %s are designated in your config.yaml. %s is deprecated. Using %s with the value of %v. Please update your config.",
+				migration.oldConfigString, migration.newConfigString, migration.oldConfigString, migration.newConfigString, *migration.newValue)
+		}
+		if *migration.newValue == "" && *migration.oldValue != "" {
+			*migration.newValue = *migration.oldValue
+			log.Sugar().Debugf("%s is deprecated. Please update your config file with %s.", migration.oldConfigString, migration.newConfigString)
+			log.Sugar().Debugf("Setting %s to the value of %s: %v.", migration.newConfigString, migration.oldConfigString, *migration.oldValue)
+		}
+	}
+}
+
 func main() {
 	process.Exec(rootCmd)
 }

cmd/storagenode/main_test.go

@@ -0,0 +1,88 @@
+// Copyright (C) 2019 Storj Labs, Inc.
+// See LICENSE for copying information.
+
+package main
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
+)
+
+func TestMapConfigs(t *testing.T) {
+	log := zap.L()
+
+	cases := []struct {
+		testID          string
+		newExternalAddr string
+		oldExternalAddr string
+		expectedAddr    string
+		newWallet       string
+		oldWallet       string
+		expectedWallet  string
+		newEmail        string
+		oldEmail        string
+		expectedEmail   string
+	}{
+		{testID: "new and old present, use new",
+			newExternalAddr: "newAddr",
+			oldExternalAddr: "oldAddr",
+			expectedAddr:    "newAddr",
+			newWallet:       "newWallet",
+			oldWallet:       "oldWallet",
+			expectedWallet:  "newWallet",
+			newEmail:        "newEmail",
+			oldEmail:        "oldEmail",
+			expectedEmail:   "newEmail",
+		},
+		{testID: "old present, new not, use old",
+			newExternalAddr: "",
+			oldExternalAddr: "oldAddr",
+			expectedAddr:    "oldAddr",
+			newWallet:       "",
+			oldWallet:       "oldWallet",
+			expectedWallet:  "oldWallet",
+			newEmail:        "",
+			oldEmail:        "oldEmail",
+			expectedEmail:   "oldEmail",
+		},
+		{testID: "new present, old not, use new",
+			newExternalAddr: "newAddr",
+			oldExternalAddr: "",
+			expectedAddr:    "newAddr",
+			newWallet:       "newWallet",
+			oldWallet:       "",
+			expectedWallet:  "newWallet",
+			newEmail:        "newEmail",
+			oldEmail:        "",
+			expectedEmail:   "newEmail",
+		},
+		{testID: "neither present",
+			newExternalAddr: "",
+			oldExternalAddr: "",
+			expectedAddr:    "",
+			newWallet:       "",
+			oldWallet:       "",
+			expectedWallet:  "",
+			newEmail:        "",
+			oldEmail:        "",
+			expectedEmail:   "",
+		},
+	}
+	for _, c := range cases {
+		testCase := c
+		t.Run(testCase.testID, func(t *testing.T) {
+			runCfg.Contact.ExternalAddress = testCase.newExternalAddr
+			runCfg.Kademlia.ExternalAddress = testCase.oldExternalAddr
+			runCfg.Operator.Wallet = testCase.newWallet
+			runCfg.Kademlia.Operator.Wallet = testCase.oldWallet
+			runCfg.Operator.Email = testCase.newEmail
+			runCfg.Kademlia.Operator.Email = testCase.oldEmail
+			mapDeprecatedConfigs(log)
+			require.Equal(t, testCase.expectedAddr, runCfg.Contact.ExternalAddress)
+			require.Equal(t, testCase.expectedWallet, runCfg.Operator.Wallet)
+			require.Equal(t, testCase.expectedEmail, runCfg.Operator.Email)
+		})
+	}
+}

internal/testplanet/storagenode.go

@@ -15,7 +15,6 @@ import (
 	"github.com/zeebo/errs"
 
 	"storj.io/storj/internal/memory"
-	"storj.io/storj/pkg/kademlia"
 	"storj.io/storj/pkg/peertls/extensions"
 	"storj.io/storj/pkg/peertls/tlsopts"
 	"storj.io/storj/pkg/revocation"
@@ -73,11 +72,9 @@ func (planet *Planet) newStorageNodes(count int, whitelistedSatellites storj.Nod
 					},
 				},
 			},
-			Kademlia: kademlia.Config{
+			Operator: storagenode.OperatorConfig{
-				Operator: kademlia.OperatorConfig{
+				Email:  prefix + "@mail.test",
-					Email:  prefix + "@mail.test",
+				Wallet: "0x" + strings.Repeat("00", 20),
-					Wallet: "0x" + strings.Repeat("00", 20),
-				},
 			},
 			Storage: piecestore.OldConfig{
 				Path:                   filepath.Join(storageDir, "pieces/"),

storagenode/operator.go

@@ -0,0 +1,50 @@
+// Copyright (C) 2019 Storj Labs, Inc.
+// See LICENSE for copying information.
+
+package storagenode
+
+import (
+	"fmt"
+	"regexp"
+
+	"go.uber.org/zap"
+)
+
+// OperatorConfig defines properties related to storage node operator metadata
+type OperatorConfig struct {
+	Email  string `user:"true" help:"operator email address" default:""`
+	Wallet string `user:"true" help:"operator wallet address" default:""`
+}
+
+// Verify verifies whether operator config is valid.
+func (c OperatorConfig) Verify(log *zap.Logger) error {
+	if err := isOperatorEmailValid(log, c.Email); err != nil {
+		return err
+	}
+	if err := isOperatorWalletValid(log, c.Wallet); err != nil {
+		return err
+	}
+	return nil
+}
+
+func isOperatorEmailValid(log *zap.Logger, email string) error {
+	if email == "" {
+		log.Sugar().Warn("Operator email address isn't specified.")
+	} else {
+		log.Sugar().Info("Operator email: ", email)
+	}
+	return nil
+}
+
+func isOperatorWalletValid(log *zap.Logger, wallet string) error {
+	if wallet == "" {
+		return fmt.Errorf("operator wallet address isn't specified")
+	}
+	r := regexp.MustCompile("^0x[a-fA-F0-9]{40}$")
+	if match := r.MatchString(wallet); !match {
+		return fmt.Errorf("operator wallet address isn't valid")
+	}
+
+	log.Sugar().Info("operator wallet: ", wallet)
+	return nil
+}

storagenode/peer.go

@@ -15,7 +15,6 @@ import (
 	"storj.io/storj/internal/errs2"
 	"storj.io/storj/internal/version"
 	"storj.io/storj/pkg/identity"
-	"storj.io/storj/pkg/kademlia"
 	"storj.io/storj/pkg/pb"
 	"storj.io/storj/pkg/peertls/extensions"
 	"storj.io/storj/pkg/peertls/tlsopts"
@@ -74,7 +73,7 @@ type Config struct {
 	Server server.Config
 
 	Contact  contact.Config
-	Kademlia kademlia.Config
+	Operator OperatorConfig
 
 	// TODO: flatten storage config and only keep the new one
 	Storage   piecestore.OldConfig
@@ -94,7 +93,7 @@ type Config struct {
 
 // Verify verifies whether configuration is consistent and acceptable.
 func (config *Config) Verify(log *zap.Logger) error {
-	return config.Kademlia.Verify(log)
+	return config.Operator.Verify(log)
 }
 
 // Peer is the representation of a Storage Node.
@@ -215,8 +214,8 @@ func New(log *zap.Logger, full *identity.FullIdentity, db DB, revocationDB exten
 			},
 			Type: pb.NodeType_STORAGE,
 			Operator: pb.NodeOperator{
-				Email:  config.Kademlia.Operator.Email,
+				Email:  config.Operator.Email,
-				Wallet: config.Kademlia.Operator.Wallet,
+				Wallet: config.Operator.Wallet,
 			},
 			Version: *pbVersion,
 		}
@@ -330,7 +329,7 @@ func New(log *zap.Logger, full *identity.FullIdentity, db DB, revocationDB exten
 			peer.Version,
 			config.Storage.AllocatedBandwidth,
 			config.Storage.AllocatedDiskSpace,
-			config.Kademlia.Operator.Wallet,
+			config.Operator.Wallet,
 			versionInfo,
 			peer.Storage2.Trust,
 			peer.DB.Reputation(),