Skip to content

Commit

Permalink
feat: allow port_grpc to be specified in [server] stanza (#4728)
Browse files Browse the repository at this point in the history 
Prior to this commit, `port_grpc` could not be added to the [server]
stanza. Instead of validating gRPC IP/Port/Protocol information in
ServerHandler, validate grpc port info in GRPCServer constructor. This
should not break backwards compatibility.

gRPC-related config info must be in a section (stanza) called
[port_gprc].

* Close #4015 - That was an alternate solution. It was decided that with
  relaxed validation, it is not necessary to rename port_grpc.
* Fix #4557
  • Loading branch information
@ckeshava
ckeshava authored Feb 7, 2024
1 parent 1e96a1d commit 6d3c21e
Show file tree
Hide file tree
Showing 8 changed files with 73 additions and 56 deletions.
7 changes: 4 additions & 3 deletions src/ripple/app/main/GRPCServer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <ripple/resource/Fees.h>

#include <ripple/beast/net/IPAddressConversion.h>
#include <ripple/core/ConfigSections.h>

namespace ripple {

Expand Down Expand Up @@ -427,9 +428,9 @@ GRPCServerImpl::GRPCServerImpl(Application& app)
: app_(app), journal_(app_.journal("gRPC Server"))
{
// if present, get endpoint from config
if (app_.config().exists("port_grpc"))
if (app_.config().exists(SECTION_PORT_GRPC))
{
const auto& section = app_.config().section("port_grpc");
Section const& section = app_.config().section(SECTION_PORT_GRPC);

auto const optIp = section.get("ip");
if (!optIp)
Expand Down Expand Up @@ -659,7 +660,7 @@ GRPCServerImpl::setupListeners()
secureGatewayIPs_));
}
return requests;
};
}

bool
GRPCServerImpl::start()
Expand Down
4 changes: 2 additions & 2 deletions src/ripple/app/misc/NetworkOPs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2719,9 +2719,9 @@ NetworkOPsImp::getServerInfo(bool human, bool admin, bool counters)
}
}

if (app_.config().exists("port_grpc"))
if (app_.config().exists(SECTION_PORT_GRPC))
{
auto const& grpcSection = app_.config().section("port_grpc");
auto const& grpcSection = app_.config().section(SECTION_PORT_GRPC);
auto const optPort = grpcSection.get("port");
if (optPort && grpcSection.get("ip"))
{
Expand Down
13 changes: 7 additions & 6 deletions src/ripple/core/ConfigSections.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ struct ConfigSection
// VFALCO TODO Rename and replace these macros with variables.
#define SECTION_AMENDMENTS "amendments"
#define SECTION_AMENDMENT_MAJORITY_TIME "amendment_majority_time"
#define SECTION_BETA_RPC_API "beta_rpc_api"
#define SECTION_CLUSTER_NODES "cluster_nodes"
#define SECTION_COMPRESSION "compression"
#define SECTION_DEBUG_LOGFILE "debug_logfile"
Expand All @@ -57,10 +58,13 @@ struct ConfigSection
#define SECTION_FETCH_DEPTH "fetch_depth"
#define SECTION_HISTORICAL_SHARD_PATHS "historical_shard_paths"
#define SECTION_INSIGHT "insight"
#define SECTION_IO_WORKERS "io_workers"
#define SECTION_IPS "ips"
#define SECTION_IPS_FIXED "ips_fixed"
#define SECTION_LEDGER_HISTORY "ledger_history"
#define SECTION_LEDGER_REPLAY "ledger_replay"
#define SECTION_MAX_TRANSACTIONS "max_transactions"
#define SECTION_NETWORK_ID "network_id"
#define SECTION_NETWORK_QUORUM "network_quorum"
#define SECTION_NODE_SEED "node_seed"
#define SECTION_NODE_SIZE "node_size"
Expand All @@ -73,6 +77,8 @@ struct ConfigSection
#define SECTION_PEERS_MAX "peers_max"
#define SECTION_PEERS_IN_MAX "peers_in_max"
#define SECTION_PEERS_OUT_MAX "peers_out_max"
#define SECTION_PORT_GRPC "port_grpc"
#define SECTION_PREFETCH_WORKERS "prefetch_workers"
#define SECTION_REDUCE_RELAY "reduce_relay"
#define SECTION_RELATIONAL_DB "relational_db"
#define SECTION_RELAY_PROPOSALS "relay_proposals"
Expand All @@ -84,6 +90,7 @@ struct ConfigSection
#define SECTION_SSL_VERIFY_FILE "ssl_verify_file"
#define SECTION_SSL_VERIFY_DIR "ssl_verify_dir"
#define SECTION_SERVER_DOMAIN "server_domain"
#define SECTION_SWEEP_INTERVAL "sweep_interval"
#define SECTION_VALIDATORS_FILE "validators_file"
#define SECTION_VALIDATION_SEED "validation_seed"
#define SECTION_VALIDATOR_KEYS "validator_keys"
Expand All @@ -94,12 +101,6 @@ struct ConfigSection
#define SECTION_VALIDATOR_TOKEN "validator_token"
#define SECTION_VETO_AMENDMENTS "veto_amendments"
#define SECTION_WORKERS "workers"
#define SECTION_IO_WORKERS "io_workers"
#define SECTION_PREFETCH_WORKERS "prefetch_workers"
#define SECTION_LEDGER_REPLAY "ledger_replay"
#define SECTION_BETA_RPC_API "beta_rpc_api"
#define SECTION_SWEEP_INTERVAL "sweep_interval"
#define SECTION_NETWORK_ID "network_id"

} // namespace ripple

Expand Down
9 changes: 7 additions & 2 deletions src/ripple/rpc/impl/ServerHandler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <ripple/basics/make_SSLContext.h>
#include <ripple/beast/net/IPAddressConversion.h>
#include <ripple/beast/rfc2616.h>
#include <ripple/core/ConfigSections.h>
#include <ripple/core/JobQueue.h>
#include <ripple/json/json_reader.h>
#include <ripple/json/to_string.h>
Expand All @@ -46,9 +47,7 @@
#include <boost/algorithm/string.hpp>
#include <boost/beast/http/fields.hpp>
#include <boost/beast/http/string_body.hpp>
#include <boost/type_traits.hpp>
#include <algorithm>
#include <mutex>
#include <stdexcept>

namespace ripple {
Expand Down Expand Up @@ -1149,6 +1148,12 @@ parse_Ports(Config const& config, std::ostream& log)
log << "Missing section: [" << name << "]";
Throw<std::exception>();
}

// grpc ports are parsed by GRPCServer class. Do not validate
// grpc port information in this file.
if (name == SECTION_PORT_GRPC)
continue;

ParsedPort parsed = common;
parsed.name = name;
parse_Port(parsed, config[name], log);
Expand Down
5 changes: 5 additions & 0 deletions src/test/jtx/envconfig.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@
namespace ripple {
namespace test {

// frequently used macros defined here for convinience.
#define PORT_WS "port_ws"
#define PORT_RPC "port_rpc"
#define PORT_PEER "port_peer"

extern std::atomic<bool> envUseIPv4;

inline const char*
Expand Down
67 changes: 33 additions & 34 deletions src/test/jtx/impl/envconfig.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include <test/jtx/envconfig.h>

#include <ripple/core/ConfigSections.h>
#include <test/jtx/Env.h>
#include <test/jtx/amount.h>

namespace ripple {
Expand Down Expand Up @@ -57,22 +56,22 @@ setupConfigForUnitTests(Config& cfg)
cfg.deprecatedClearSection(ConfigSection::importNodeDatabase());
cfg.legacy("database_path", "");
cfg.setupControl(true, true, true);
cfg["server"].append("port_peer");
cfg["port_peer"].set("ip", getEnvLocalhostAddr());
cfg["port_peer"].set("port", port_peer);
cfg["port_peer"].set("protocol", "peer");

cfg["server"].append("port_rpc");
cfg["port_rpc"].set("ip", getEnvLocalhostAddr());
cfg["port_rpc"].set("admin", getEnvLocalhostAddr());
cfg["port_rpc"].set("port", port_rpc);
cfg["port_rpc"].set("protocol", "http,ws2");

cfg["server"].append("port_ws");
cfg["port_ws"].set("ip", getEnvLocalhostAddr());
cfg["port_ws"].set("admin", getEnvLocalhostAddr());
cfg["port_ws"].set("port", port_ws);
cfg["port_ws"].set("protocol", "ws");
cfg["server"].append(PORT_PEER);
cfg[PORT_PEER].set("ip", getEnvLocalhostAddr());
cfg[PORT_PEER].set("port", port_peer);
cfg[PORT_PEER].set("protocol", "peer");

cfg["server"].append(PORT_RPC);
cfg[PORT_RPC].set("ip", getEnvLocalhostAddr());
cfg[PORT_RPC].set("admin", getEnvLocalhostAddr());
cfg[PORT_RPC].set("port", port_rpc);
cfg[PORT_RPC].set("protocol", "http,ws2");

cfg["server"].append(PORT_WS);
cfg[PORT_WS].set("ip", getEnvLocalhostAddr());
cfg[PORT_WS].set("admin", getEnvLocalhostAddr());
cfg[PORT_WS].set("port", port_ws);
cfg[PORT_WS].set("protocol", "ws");
cfg.SSL_VERIFY = false;
}

Expand All @@ -81,35 +80,35 @@ namespace jtx {
std::unique_ptr<Config>
no_admin(std::unique_ptr<Config> cfg)
{
(*cfg)["port_rpc"].set("admin", "");
(*cfg)["port_ws"].set("admin", "");
(*cfg)[PORT_RPC].set("admin", "");
(*cfg)[PORT_WS].set("admin", "");
return cfg;
}

std::unique_ptr<Config>
secure_gateway(std::unique_ptr<Config> cfg)
{
(*cfg)["port_rpc"].set("admin", "");
(*cfg)["port_ws"].set("admin", "");
(*cfg)["port_rpc"].set("secure_gateway", getEnvLocalhostAddr());
(*cfg)[PORT_RPC].set("admin", "");
(*cfg)[PORT_WS].set("admin", "");
(*cfg)[PORT_RPC].set("secure_gateway", getEnvLocalhostAddr());
return cfg;
}

std::unique_ptr<Config>
admin_localnet(std::unique_ptr<Config> cfg)
{
(*cfg)["port_rpc"].set("admin", "127.0.0.0/8");
(*cfg)["port_ws"].set("admin", "127.0.0.0/8");
(*cfg)[PORT_RPC].set("admin", "127.0.0.0/8");
(*cfg)[PORT_WS].set("admin", "127.0.0.0/8");
return cfg;
}

std::unique_ptr<Config>
secure_gateway_localnet(std::unique_ptr<Config> cfg)
{
(*cfg)["port_rpc"].set("admin", "");
(*cfg)["port_ws"].set("admin", "");
(*cfg)["port_rpc"].set("secure_gateway", "127.0.0.0/8");
(*cfg)["port_ws"].set("secure_gateway", "127.0.0.0/8");
(*cfg)[PORT_RPC].set("admin", "");
(*cfg)[PORT_WS].set("admin", "");
(*cfg)[PORT_RPC].set("secure_gateway", "127.0.0.0/8");
(*cfg)[PORT_WS].set("secure_gateway", "127.0.0.0/8");
return cfg;
}

Expand All @@ -127,7 +126,7 @@ validator(std::unique_ptr<Config> cfg, std::string const& seed)
std::unique_ptr<Config>
port_increment(std::unique_ptr<Config> cfg, int increment)
{
for (auto const sectionName : {"port_peer", "port_rpc", "port_ws"})
for (auto const sectionName : {PORT_PEER, PORT_RPC, PORT_WS})
{
Section& s = (*cfg)[sectionName];
auto const port = s.get<std::int32_t>("port");
Expand All @@ -143,8 +142,8 @@ std::unique_ptr<Config>
addGrpcConfig(std::unique_ptr<Config> cfg)
{
std::string port_grpc = std::to_string(port_base + 3);
(*cfg)["port_grpc"].set("ip", getEnvLocalhostAddr());
(*cfg)["port_grpc"].set("port", port_grpc);
(*cfg)[SECTION_PORT_GRPC].set("ip", getEnvLocalhostAddr());
(*cfg)[SECTION_PORT_GRPC].set("port", port_grpc);
return cfg;
}

Expand All @@ -154,9 +153,9 @@ addGrpcConfigWithSecureGateway(
std::string const& secureGateway)
{
std::string port_grpc = std::to_string(port_base + 3);
(*cfg)["port_grpc"].set("ip", getEnvLocalhostAddr());
(*cfg)["port_grpc"].set("port", port_grpc);
(*cfg)["port_grpc"].set("secure_gateway", secureGateway);
(*cfg)[SECTION_PORT_GRPC].set("ip", getEnvLocalhostAddr());
(*cfg)[SECTION_PORT_GRPC].set("port", port_grpc);
(*cfg)[SECTION_PORT_GRPC].set("secure_gateway", secureGateway);
return cfg;
}

Expand Down
21 changes: 13 additions & 8 deletions src/test/rpc/ReportingETL_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <ripple/beast/unit_test.h>
#include <ripple/rpc/impl/Tuning.h>

#include <ripple/core/ConfigSections.h>
#include <test/jtx.h>
#include <test/jtx/Env.h>
#include <test/jtx/envconfig.h>
Expand Down Expand Up @@ -56,7 +57,8 @@ class ReportingETL_test : public beast::unit_test::suite
testcase("GetLedger");
using namespace test::jtx;
std::unique_ptr<Config> config = envconfig(addGrpcConfig);
std::string grpcPort = *(*config)["port_grpc"].get<std::string>("port");
std::string grpcPort =
*(*config)[SECTION_PORT_GRPC].get<std::string>("port");
Env env(*this, std::move(config));

env.close();
Expand Down Expand Up @@ -498,7 +500,8 @@ class ReportingETL_test : public beast::unit_test::suite
testcase("GetLedgerData");
using namespace test::jtx;
std::unique_ptr<Config> config = envconfig(addGrpcConfig);
std::string grpcPort = *(*config)["port_grpc"].get<std::string>("port");
std::string grpcPort =
*(*config)[SECTION_PORT_GRPC].get<std::string>("port");
Env env(*this, std::move(config));
auto grpcLedgerData = [&grpcPort](
auto sequence, std::string marker = "") {
Expand Down Expand Up @@ -620,7 +623,8 @@ class ReportingETL_test : public beast::unit_test::suite
testcase("GetLedgerDiff");
using namespace test::jtx;
std::unique_ptr<Config> config = envconfig(addGrpcConfig);
std::string grpcPort = *(*config)["port_grpc"].get<std::string>("port");
std::string grpcPort =
*(*config)[SECTION_PORT_GRPC].get<std::string>("port");
Env env(*this, std::move(config));

auto grpcLedgerDiff = [&grpcPort](
Expand Down Expand Up @@ -735,7 +739,8 @@ class ReportingETL_test : public beast::unit_test::suite
testcase("GetLedgerDiff");
using namespace test::jtx;
std::unique_ptr<Config> config = envconfig(addGrpcConfig);
std::string grpcPort = *(*config)["port_grpc"].get<std::string>("port");
std::string grpcPort =
*(*config)[SECTION_PORT_GRPC].get<std::string>("port");
Env env(*this, std::move(config));

auto grpcLedgerEntry = [&grpcPort](auto sequence, auto key) {
Expand Down Expand Up @@ -895,7 +900,7 @@ class ReportingETL_test : public beast::unit_test::suite
std::unique_ptr<Config> config = envconfig(
addGrpcConfigWithSecureGateway, getEnvLocalhostAddr());
std::string grpcPort =
*(*config)["port_grpc"].get<std::string>("port");
*(*config)[SECTION_PORT_GRPC].get<std::string>("port");
Env env(*this, std::move(config));

env.close();
Expand Down Expand Up @@ -955,7 +960,7 @@ class ReportingETL_test : public beast::unit_test::suite
std::unique_ptr<Config> config =
envconfig(addGrpcConfigWithSecureGateway, secureGatewayIp);
std::string grpcPort =
*(*config)["port_grpc"].get<std::string>("port");
*(*config)[SECTION_PORT_GRPC].get<std::string>("port");
Env env(*this, std::move(config));

env.close();
Expand Down Expand Up @@ -1008,7 +1013,7 @@ class ReportingETL_test : public beast::unit_test::suite
std::unique_ptr<Config> config = envconfig(
addGrpcConfigWithSecureGateway, getEnvLocalhostAddr());
std::string grpcPort =
*(*config)["port_grpc"].get<std::string>("port");
*(*config)[SECTION_PORT_GRPC].get<std::string>("port");
Env env(*this, std::move(config));

env.close();
Expand Down Expand Up @@ -1065,7 +1070,7 @@ class ReportingETL_test : public beast::unit_test::suite
std::unique_ptr<Config> config =
envconfig(addGrpcConfigWithSecureGateway, secureGatewayIp);
std::string grpcPort =
*(*config)["port_grpc"].get<std::string>("port");
*(*config)[SECTION_PORT_GRPC].get<std::string>("port");
Env env(*this, std::move(config));

env.close();
Expand Down
3 changes: 2 additions & 1 deletion src/test/rpc/ServerInfo_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include <ripple/app/misc/NetworkOPs.h>
#include <ripple/beast/unit_test.h>
#include <ripple/core/ConfigSections.h>
#include <ripple/protocol/jss.h>
#include <test/jtx.h>

Expand Down Expand Up @@ -107,7 +108,7 @@ admin = 127.0.0.1
auto const rpc_port =
(*config)["port_rpc"].get<unsigned int>("port");
auto const grpc_port =
(*config)["port_grpc"].get<unsigned int>("port");
(*config)[SECTION_PORT_GRPC].get<unsigned int>("port");
auto const ws_port = (*config)["port_ws"].get<unsigned int>("port");
BEAST_EXPECT(grpc_port);
BEAST_EXPECT(rpc_port);
Expand Down

0 comments on commit 6d3c21e

Please sign in to comment.