Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

process grpc header flag for exit #8334

Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions validator/accounts/BUILD.bazel
Expand Up @@ -54,6 +54,7 @@ go_library(
"@com_github_urfave_cli_v2//:go_default_library",
"@com_github_wealdtech_go_eth2_wallet_encryptor_keystorev4//:go_default_library",
"@org_golang_google_grpc//:go_default_library",
"@org_golang_google_grpc//metadata:go_default_library",
],
)

Expand Down Expand Up @@ -98,5 +99,6 @@ go_test(
"@com_github_sirupsen_logrus//hooks/test:go_default_library",
"@com_github_urfave_cli_v2//:go_default_library",
"@com_github_wealdtech_go_eth2_wallet_encryptor_keystorev4//:go_default_library",
"@org_golang_google_grpc//metadata:go_default_library",
],
)
12 changes: 11 additions & 1 deletion validator/accounts/accounts_exit.go
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/prysmaticlabs/prysm/validator/keymanager"
"github.com/urfave/cli/v2"
"google.golang.org/grpc"
"google.golang.org/grpc/metadata"
)

type performExitCfg struct {
Expand Down Expand Up @@ -183,13 +184,22 @@ func prepareClients(cliCtx *cli.Context) (*ethpb.BeaconNodeValidatorClient, *eth
if dialOpts == nil {
return nil, nil, errors.New("failed to construct dial options")
}
for _, hdr := range strings.Split(cliCtx.String(flags.GrpcHeadersFlag.Name), ",") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. To accept and merge this PR, we are going to need a regression test. Do you mind adding one for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@djoyahoy has added the test and the validator tests pass:

Target //validator:go_default_test up-to-date:
  bazel-bin/validator/go_default_test_/go_default_test
INFO: Elapsed time: 8.898s, Critical Path: 8.30s
INFO: 46 processes: 6 internal, 40 darwin-sandbox.
INFO: Build completed successfully, 46 total actions
//validator:go_default_test                                              PASSED in 1.2s
  WARNING: //validator:go_default_test: Test execution time (1.2s excluding execution overhead) outside of range for MODERATE tests. Consider setting timeout="short" or size="small".
INFO: Build completed successfully, 46 total actions

We are however, seeing a failure in the end to end tests that I can't pin down:

> bazel test //endtoend:go_default_test --define=ssz=minimal
...
=== CONT  TestEndToEnd_MinimalConfig
    helpers.go:126: Ending time: 2021-01-26 16:55:28.701779 +0000 UTC m=+472.829059328
--- FAIL: TestEndToEnd_MinimalConfig (472.79s)
...
    --- FAIL: TestEndToEnd_MinimalConfig/validators_participating_epoch_5 (0.00s)
...
FAIL
================================================================================
Target //endtoend:go_default_test up-to-date:
  bazel-bin/endtoend/go_default_test_/go_default_test
INFO: Elapsed time: 493.339s, Critical Path: 487.68s
INFO: 65 processes: 4 internal, 61 darwin-sandbox.
INFO: Build completed, 1 test FAILED, 65 total actions
//endtoend:go_default_test                                               FAILED in 1 out of 2 in 475.8s
  Stats over 2 runs: max = 475.8s, min = 295.1s, avg = 385.4s, dev = 90.4s
  /private/var/tmp/_bazel_tusharshah/3f45103db17b7158700e6ae8b56c98c1/execroot/prysm/bazel-out/darwin-fastbuild/testlogs/endtoend/go_default_test/shard_1_of_2/test.log

INFO: Build completed, 1 test FAILED, 65 total actions

if hdr != "" {
ss := strings.Split(hdr, "=")
if len(ss) < 2 {
log.Warnf("Incorrect gRPC header flag format. Skipping %v", ss[0])
continue
}
cliCtx.Context = metadata.AppendToOutgoingContext(cliCtx.Context, ss[0], strings.Join(ss[1:], "="))
}
}
conn, err := grpc.DialContext(cliCtx.Context, cliCtx.String(flags.BeaconRPCProviderFlag.Name), dialOpts...)
if err != nil {
return nil, nil, errors.Wrapf(err, "could not dial endpoint %s", flags.BeaconRPCProviderFlag.Name)
}
validatorClient := ethpb.NewBeaconNodeValidatorClient(conn)
nodeClient := ethpb.NewNodeClient(conn)

return &validatorClient, &nodeClient, nil
}

Expand Down
27 changes: 27 additions & 0 deletions validator/accounts/accounts_exit_test.go
Expand Up @@ -10,6 +10,8 @@ import (
"github.com/gogo/protobuf/types"
"github.com/golang/mock/gomock"
ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
"google.golang.org/grpc/metadata"

twoshark marked this conversation as resolved.
Show resolved Hide resolved
"github.com/prysmaticlabs/prysm/shared/mock"
"github.com/prysmaticlabs/prysm/shared/testutil/assert"
"github.com/prysmaticlabs/prysm/shared/testutil/require"
Expand Down Expand Up @@ -123,3 +125,28 @@ func TestPrepareWallet_EmptyWalletReturnsError(t *testing.T) {
_, _, err = prepareWallet(cliCtx)
assert.ErrorContains(t, "wallet is empty", err)
}

func TestPrepareClients_AddsGRPCHeaders(t *testing.T) {
imported.ResetCaches()
walletDir, _, passwordFilePath := setupWalletAndPasswordsDir(t)
cliCtx := setupWalletCtx(t, &testWalletConfig{
walletDir: walletDir,
keymanagerKind: keymanager.Imported,
walletPasswordFile: passwordFilePath,
accountPasswordFile: passwordFilePath,
grpcHeaders: "Authorization=Basic some-token,Some-Other-Header=some-value",
})
_, err := CreateWalletWithKeymanager(cliCtx.Context, &CreateWalletConfig{
WalletCfg: &wallet.Config{
WalletDir: walletDir,
KeymanagerKind: keymanager.Imported,
WalletPassword: password,
},
})
require.NoError(t, err)
_, _, err = prepareClients(cliCtx)
require.NoError(t, err)
md, _ := metadata.FromOutgoingContext(cliCtx.Context)
assert.Equal(t, "Basic some-token", md.Get("Authorization")[0])
assert.Equal(t, "some-value", md.Get("Some-Other-Header")[0])
}
3 changes: 3 additions & 0 deletions validator/accounts/wallet_create_test.go
Expand Up @@ -50,6 +50,7 @@ type testWalletConfig struct {
walletPasswordFile string
accountPasswordFile string
privateKeyFile string
grpcHeaders string
skipDepositConfirm bool
numAccounts int64
keymanagerKind keymanager.Kind
Expand All @@ -76,6 +77,7 @@ func setupWalletCtx(
set.Int64(flags.NumAccountsFlag.Name, cfg.numAccounts, "")
set.Bool(flags.SkipDepositConfirmationFlag.Name, cfg.skipDepositConfirm, "")
set.Bool(flags.SkipMnemonic25thWordCheckFlag.Name, true, "")
set.String(flags.GrpcHeadersFlag.Name, cfg.grpcHeaders, "")

if cfg.privateKeyFile != "" {
set.String(flags.ImportPrivateKeyFileFlag.Name, cfg.privateKeyFile, "")
Expand All @@ -96,6 +98,7 @@ func setupWalletCtx(
assert.NoError(tb, set.Set(flags.AccountPasswordFileFlag.Name, cfg.accountPasswordFile))
assert.NoError(tb, set.Set(flags.NumAccountsFlag.Name, strconv.Itoa(int(cfg.numAccounts))))
assert.NoError(tb, set.Set(flags.SkipDepositConfirmationFlag.Name, strconv.FormatBool(cfg.skipDepositConfirm)))
assert.NoError(tb, set.Set(flags.GrpcHeadersFlag.Name, cfg.grpcHeaders))
return cli.NewContext(&app, set, nil)
}

Expand Down