From a3431fb147a55d44a751465715907d485a5b55dd Mon Sep 17 00:00:00 2001 From: Ducem Barr <46427745+ducembarr@users.noreply.github.com> Date: Mon, 7 Jan 2019 05:00:52 -0500 Subject: [PATCH] feat: Configuration option to include foreign API endpoints in the owner API (#2305) * Allow foreign API over owner API port * Tests for owner_api_include_foreign config option --- config/src/comments.rs | 9 +++++ doc/api/wallet_owner_api.md | 5 +++ servers/tests/framework.rs | 66 +++++++++++++++++++++++++++++++------ servers/tests/wallet.rs | 48 +++++++++++++++++++++++++++ src/bin/cmd/wallet_args.rs | 4 +-- wallet/src/command.rs | 2 ++ wallet/src/controller.rs | 12 ++++++- wallet/src/types.rs | 3 ++ 8 files changed, 136 insertions(+), 13 deletions(-) diff --git a/config/src/comments.rs b/config/src/comments.rs index f8b49aff1..0d96053ba 100644 --- a/config/src/comments.rs +++ b/config/src/comments.rs @@ -390,6 +390,15 @@ fn comments() -> HashMap { "node_api_secret_path".to_string(), " #location of the node api secret for basic auth on the Grin API +" + .to_string(), + ); + retval.insert( + "owner_api_include_foreign".to_string(), + " +#include the foreign API endpoints on the same port as the owner +#API. Useful for networking environments like AWS ECS that make +#it difficult to access multiple ports on a single service. " .to_string(), ); diff --git a/doc/api/wallet_owner_api.md b/doc/api/wallet_owner_api.md index 36e67b8a7..9f2e7d9bb 100644 --- a/doc/api/wallet_owner_api.md +++ b/doc/api/wallet_owner_api.md @@ -13,6 +13,7 @@ 1. [POST Cancel Tx](#post-cancel-tx) 1. [POST Post Tx](#post-post-tx) 1. [POST Issue Burn Tx](#post-issue-burn-tx) + 1. [Adding Foreign API Endpoints](#add-foreign-api-endpoints) ## Wallet Owner Endpoint @@ -681,3 +682,7 @@ Issue a burn TX. } }); ``` + +### Add Foreign API Endpoints + +For some environments it may be convenient to have the [wallet foreign API](wallet_foreign_api.md) available on the same port as the owner API. You can do so by setting the `owner_api_include_foreign` configuration setting to `true`. diff --git a/servers/tests/framework.rs b/servers/tests/framework.rs index 79988f728..8f9117407 100644 --- a/servers/tests/framework.rs +++ b/servers/tests/framework.rs @@ -67,6 +67,12 @@ pub struct LocalServerContainerConfig { // Port the wallet server is running on pub wallet_port: u16, + // Port the wallet owner API is running on + pub owner_port: u16, + + // Whether to include the foreign API endpoints in the owner API + pub owner_api_include_foreign: bool, + // Whether we're going to mine pub start_miner: bool, @@ -104,6 +110,8 @@ impl Default for LocalServerContainerConfig { api_server_port: 13413, p2p_server_port: 13414, wallet_port: 13415, + owner_port: 13420, + owner_api_include_foreign: false, seed_addr: String::from(""), is_seeding: false, start_miner: false, @@ -161,6 +169,7 @@ impl LocalServerContainer { wallet_config.api_listen_port = config.wallet_port; wallet_config.check_node_api_http_addr = config.wallet_validating_node_url.clone(); + wallet_config.owner_api_include_foreign = Some(config.owner_api_include_foreign); wallet_config.data_file_dir = working_dir.clone(); Ok(LocalServerContainer { config: config, @@ -237,10 +246,10 @@ impl LocalServerContainer { s } - /// Starts a wallet daemon to receive and returns the - /// listening server url - - pub fn run_wallet(&mut self, _duration_in_mills: u64) { + /// Make a wallet for use in test endpoints (run_wallet and run_owner). + fn make_wallet_for_tests( + &mut self, + ) -> Arc>> { // URL on which to start the wallet listener (i.e. api server) let _url = format!("{}:{}", self.config.base_addr, self.config.wallet_port); @@ -260,6 +269,7 @@ impl LocalServerContainer { self.wallet_config.check_node_api_http_addr = self.config.wallet_validating_node_url.clone(); self.wallet_config.data_file_dir = self.working_dir.clone(); + self.wallet_config.owner_api_include_foreign = Some(self.config.owner_api_include_foreign); let _ = fs::create_dir_all(self.wallet_config.clone().data_file_dir); let r = wallet::WalletSeed::init_file(&self.wallet_config, 32, ""); @@ -278,19 +288,45 @@ impl LocalServerContainer { ) }); - wallet::controller::foreign_listener( - Arc::new(Mutex::new(wallet)), - &self.wallet_config.api_listen_addr(), + Arc::new(Mutex::new(wallet)) + } + + /// Starts a wallet daemon to receive + pub fn run_wallet(&mut self, _duration_in_mills: u64) { + let wallet = self.make_wallet_for_tests(); + + wallet::controller::foreign_listener(wallet, &self.wallet_config.api_listen_addr(), None) + .unwrap_or_else(|e| { + panic!( + "Error creating wallet listener: {:?} Config: {:?}", + e, self.wallet_config + ) + }); + + self.wallet_is_running = true; + } + + /// Starts a wallet owner daemon + pub fn run_owner(&mut self) { + let wallet = self.make_wallet_for_tests(); + + // WalletConfig doesn't allow changing the owner API path, so we build + // the path ourselves + let owner_listen_addr = format!("127.0.0.1:{}", self.config.owner_port); + + wallet::controller::owner_listener( + wallet, + &owner_listen_addr, None, + None, + self.wallet_config.owner_api_include_foreign.clone(), ) .unwrap_or_else(|e| { panic!( - "Error creating wallet listener: {:?} Config: {:?}", + "Error creating wallet owner listener: {:?} Config: {:?}", e, self.wallet_config ) }); - - self.wallet_is_running = true; } pub fn get_wallet_seed(config: &WalletConfig) -> wallet::WalletSeed { @@ -403,6 +439,10 @@ pub struct LocalServerContainerPoolConfig { // pub base_wallet_port: u16, + // Base wallet owner port for this server + // + pub base_owner_port: u16, + // How long the servers in the pool are going to run pub run_length_in_seconds: u64, } @@ -417,6 +457,7 @@ impl Default for LocalServerContainerPoolConfig { base_p2p_port: 10000, base_api_port: 11000, base_wallet_port: 12000, + base_owner_port: 13000, run_length_in_seconds: 30, } } @@ -439,6 +480,8 @@ pub struct LocalServerContainerPool { next_wallet_port: u16, + next_owner_port: u16, + // keep track of whether a seed exists, and pause a bit if so is_seeding: bool, } @@ -449,6 +492,7 @@ impl LocalServerContainerPool { next_api_port: config.base_api_port, next_p2p_port: config.base_p2p_port, next_wallet_port: config.base_wallet_port, + next_owner_port: config.base_owner_port, config: config, server_containers: Vec::new(), is_seeding: false, @@ -466,6 +510,7 @@ impl LocalServerContainerPool { server_config.p2p_server_port = self.next_p2p_port; server_config.api_server_port = self.next_api_port; server_config.wallet_port = self.next_wallet_port; + server_config.owner_port = self.next_owner_port; server_config.name = String::from(format!( "{}/{}-{}", @@ -481,6 +526,7 @@ impl LocalServerContainerPool { self.next_p2p_port += 1; self.next_api_port += 1; self.next_wallet_port += 1; + self.next_owner_port += 1; if server_config.is_seeding { self.is_seeding = true; diff --git a/servers/tests/wallet.rs b/servers/tests/wallet.rs index c831c1b66..8dab9557a 100644 --- a/servers/tests/wallet.rs +++ b/servers/tests/wallet.rs @@ -21,6 +21,7 @@ use self::util::Mutex; use crate::framework::{LocalServerContainer, LocalServerContainerConfig}; use grin_core as core; use grin_util as util; +use grin_wallet as wallet; use std::sync::Arc; use std::{thread, time}; @@ -152,3 +153,50 @@ fn basic_wallet_transactions() { LocalServerContainer::get_wallet_info(&coinbase_wallet_config, &coinbase_seed); println!("Coinbase wallet info final: {:?}", coinbase_info); } + +/// Tests the owner_api_include_foreign configuration option. +#[test] +fn wallet_config_owner_api_include_foreign() { + // Test setup + let test_name_dir = "test_servers"; + core::global::set_mining_mode(core::global::ChainTypes::AutomatedTesting); + framework::clean_all_output(test_name_dir); + let mut log_config = util::LoggingConfig::default(); + log_config.stdout_log_level = util::LogLevel::Info; + util::init_test_logger(); + + // This is just used for testing whether the API endpoint exists + // so we have nonsense values + let block_fees = wallet::BlockFees { + fees: 1, + height: 2, + key_id: None, + }; + + let mut base_config = LocalServerContainerConfig::default(); + base_config.name = String::from("test_owner_api_include_foreign"); + base_config.start_wallet = true; + + // Start up the wallet owner API with the default config, and make sure + // we get an error when trying to hit the coinbase endpoint + let mut default_config = base_config.clone(); + default_config.owner_port = 20005; + let _ = thread::spawn(move || { + let mut default_owner = LocalServerContainer::new(default_config).unwrap(); + default_owner.run_owner(); + }); + thread::sleep(time::Duration::from_millis(1000)); + assert!(wallet::create_coinbase("http://127.0.0.1:20005", &block_fees).is_err()); + + // Start up the wallet owner API with the owner_api_include_foreign setting set, + // and confirm that we can hit the endpoint + let mut foreign_config = base_config.clone(); + foreign_config.owner_port = 20006; + foreign_config.owner_api_include_foreign = true; + let _ = thread::spawn(move || { + let mut owner_with_foreign = LocalServerContainer::new(foreign_config).unwrap(); + owner_with_foreign.run_owner(); + }); + thread::sleep(time::Duration::from_millis(1000)); + assert!(wallet::create_coinbase("http://127.0.0.1:20006", &block_fees).is_ok()); +} diff --git a/src/bin/cmd/wallet_args.rs b/src/bin/cmd/wallet_args.rs index 7228578bc..3bd8d24f1 100644 --- a/src/bin/cmd/wallet_args.rs +++ b/src/bin/cmd/wallet_args.rs @@ -494,9 +494,9 @@ pub fn wallet_command( ("owner_api", Some(_)) => { let mut g = global_wallet_args.clone(); g.tls_conf = None; - command::owner_api(inst_wallet(), &g) + command::owner_api(inst_wallet(), &wallet_config, &g) } - ("web", Some(_)) => command::owner_api(inst_wallet(), &global_wallet_args), + ("web", Some(_)) => command::owner_api(inst_wallet(), &wallet_config, &global_wallet_args), ("account", Some(args)) => { let a = arg_parse!(parse_account_args(&args)); command::account(inst_wallet(), a) diff --git a/wallet/src/command.rs b/wallet/src/command.rs index 3b9dbf9ac..25c65fb6d 100644 --- a/wallet/src/command.rs +++ b/wallet/src/command.rs @@ -127,6 +127,7 @@ pub fn listen(config: &WalletConfig, args: &ListenArgs, g_args: &GlobalArgs) -> pub fn owner_api( wallet: Arc>>, + config: &WalletConfig, g_args: &GlobalArgs, ) -> Result<(), Error> { let res = controller::owner_listener( @@ -134,6 +135,7 @@ pub fn owner_api( "127.0.0.1:13420", g_args.node_api_secret.clone(), g_args.tls_conf.clone(), + config.owner_api_include_foreign.clone(), ); if let Err(e) = res { return Err(ErrorKind::LibWallet(e.kind(), e.cause_string()).into()); diff --git a/wallet/src/controller.rs b/wallet/src/controller.rs index 7bfa79530..1277e7450 100644 --- a/wallet/src/controller.rs +++ b/wallet/src/controller.rs @@ -74,6 +74,7 @@ pub fn owner_listener( addr: &str, api_secret: Option, tls_config: Option, + owner_api_include_foreign: Option, ) -> Result<(), Error> where T: WalletBackend + Send + Sync + 'static, @@ -81,7 +82,7 @@ where C: NodeClient + 'static, K: Keychain + 'static, { - let api_handler = OwnerAPIHandler::new(wallet); + let api_handler = OwnerAPIHandler::new(wallet.clone()); let mut router = Router::new(); if api_secret.is_some() { @@ -95,6 +96,15 @@ where .add_route("/v1/wallet/owner/**", Arc::new(api_handler)) .map_err(|_| ErrorKind::GenericError("Router failed to add route".to_string()))?; + // If so configured, add the foreign API to the same port + if owner_api_include_foreign.unwrap_or(false) { + info!("Starting HTTP Foreign API on Owner server at {}.", addr); + let foreign_api_handler = ForeignAPIHandler::new(wallet.clone()); + router + .add_route("/v1/wallet/foreign/**", Arc::new(foreign_api_handler)) + .map_err(|_| ErrorKind::GenericError("Router failed to add route".to_string()))?; + } + let mut apis = ApiServer::new(); info!("Starting HTTP Owner API server at {}.", addr); let socket_addr: SocketAddr = addr.parse().expect("unable to parse socket address"); diff --git a/wallet/src/types.rs b/wallet/src/types.rs index 351317daa..7cb5a78c1 100644 --- a/wallet/src/types.rs +++ b/wallet/src/types.rs @@ -48,6 +48,8 @@ pub struct WalletConfig { // The api address of a running server node against which transaction inputs // will be checked during send pub check_node_api_http_addr: String, + // Whether to include foreign API endpoints on the Owner API + pub owner_api_include_foreign: Option, // The directory in which wallet files are stored pub data_file_dir: String, /// TLS certificate file @@ -70,6 +72,7 @@ impl Default for WalletConfig { api_secret_path: Some(".api_secret".to_string()), node_api_secret_path: Some(".api_secret".to_string()), check_node_api_http_addr: "http://127.0.0.1:3413".to_string(), + owner_api_include_foreign: Some(false), data_file_dir: ".".to_string(), tls_certificate_file: None, tls_certificate_key: None,