Skip to content

Commit

Permalink
ir: Improve feedback on consensus misconfiguration
Browse files Browse the repository at this point in the history
Inner Ring app requires either `morph.endpoints` or `morph.consensus` to
be configured. Previously, when user forgot to set both sections, the
app responded with:
```
invalid blockchain configuration: missing root section 'morph.consensus'
```

This could confuse the app user: maybe he wants the remote consensus,
just set FS chain RPC endpoints incorrectly, why local consensus is
required then?

From now failure will be:
```
invalid consensus configuration: either 'morph.endpoints' or
'morph.consensus' must be configured
```

Refs #2650.

Signed-off-by: Leonard Lyubich <[email protected]>
  • Loading branch information
cthulhu-rider committed Dec 20, 2023
1 parent b2ef403 commit 581787f
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 16 deletions.
12 changes: 9 additions & 3 deletions pkg/innerring/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,15 @@ func isAutoDeploymentMode(cfg *viper.Viper) (bool, error) {
}

// checks if Inner Ring app is configured to be launched in local consensus
// mode.
func isLocalConsensusMode(cfg *viper.Viper) bool {
return !cfg.IsSet("morph.endpoints")
// mode. Returns error if neither NeoFS chain RPC endpoints nor local consensus
// is configured.
func isLocalConsensusMode(cfg *viper.Viper) (bool, error) {
endpointsUnset := !cfg.IsSet("morph.endpoints")
if endpointsUnset && !cfg.IsSet("morph.consensus") {
return false, fmt.Errorf("either 'morph.endpoints' or 'morph.consensus' must be configured")
}

return endpointsUnset, nil
}

func parseBlockchainConfig(v *viper.Viper, _logger *zap.Logger) (c blockchain.Config, err error) {
Expand Down
80 changes: 68 additions & 12 deletions pkg/innerring/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,17 +345,64 @@ func TestIsLocalConsensusMode(t *testing.T) {
v.SetEnvPrefix("neofs_ir")
v.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))

const envKey = "NEOFS_IR_MORPH_ENDPOINTS"

err := os.Unsetenv(envKey)
require.NoError(t, err)

require.True(t, isLocalConsensusMode(v))

err = os.Setenv(envKey, "any string")
require.NoError(t, err)
const envKeyEndpoints = "NEOFS_IR_MORPH_ENDPOINTS"
const envKeyConsensus = "NEOFS_IR_MORPH_CONSENSUS"
var err error

for _, tc := range []struct {
setEndpoints bool
setConsensus bool
expected uint8 // 0:false, 1:true, 2:error
}{
{
setEndpoints: true,
setConsensus: true,
expected: 0,
},
{
setEndpoints: true,
setConsensus: false,
expected: 0,
},
{
setEndpoints: false,
setConsensus: true,
expected: 1,
},
{
setEndpoints: false,
setConsensus: false,
expected: 2,
},
} {
if tc.setEndpoints {
err = os.Setenv(envKeyEndpoints, "any")
} else {
err = os.Unsetenv(envKeyEndpoints)
}
require.NoError(t, err, tc)

require.False(t, isLocalConsensusMode(v))
if tc.setConsensus {
err = os.Setenv(envKeyConsensus, "any")
} else {
err = os.Unsetenv(envKeyConsensus)
}
require.NoError(t, err, tc)

res, err := isLocalConsensusMode(v)
switch tc.expected {
default:
t.Fatalf("unexpected result value %v", tc.expected)
case 0:
require.NoError(t, err, tc)
require.False(t, res, tc)
case 1:
require.NoError(t, err, tc)
require.True(t, res, tc)
case 2:
require.Error(t, err, tc)
}
}
})

t.Run("YAML", func(t *testing.T) {
Expand All @@ -368,11 +415,20 @@ morph:
`))
require.NoError(t, err)

require.False(t, isLocalConsensusMode(v))
res, err := isLocalConsensusMode(v)
require.NoError(t, err)
require.False(t, res)

resetConfig(t, v, "morph.endpoints")

require.True(t, isLocalConsensusMode(v))
_, err = isLocalConsensusMode(v)
require.Error(t, err)

v.Set("morph.consensus", "any")

res, err = isLocalConsensusMode(v)
require.NoError(t, err)
require.True(t, res)
})
}

Expand Down
6 changes: 5 additions & 1 deletion pkg/innerring/innerring.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,11 @@ func New(ctx context.Context, log *zap.Logger, cfg *viper.Viper, errChan chan<-
return nil, err
}

isLocalConsensus := isLocalConsensusMode(cfg)
isLocalConsensus, err := isLocalConsensusMode(cfg)
if err != nil {
return nil, fmt.Errorf("invalid consensus configuration: %w", err)
}

if isLocalConsensus {
if singleAcc == nil {
return nil, fmt.Errorf("missing account with label '%s' in wallet '%s'", singleAccLabel, walletPass)
Expand Down

0 comments on commit 581787f

Please sign in to comment.