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

@@ -390,6 +390,15 @@ fn comments() -> HashMap<String, String> {
 		"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(),
 	);

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`.

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<Mutex<LMDBBackend<HTTPNodeClient, keychain::ExtKeychain>>> {
 		// 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;

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());
+}

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)

wallet/src/command.rs

@@ -127,6 +127,7 @@ pub fn listen(config: &WalletConfig, args: &ListenArgs, g_args: &GlobalArgs) ->
 
 pub fn owner_api(
 	wallet: Arc<Mutex<WalletInst<impl NodeClient + 'static, keychain::ExtKeychain>>>,
+	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());

wallet/src/controller.rs

@@ -74,6 +74,7 @@ pub fn owner_listener<T: ?Sized, C, K>(
 	addr: &str,
 	api_secret: Option<String>,
 	tls_config: Option<TLSConfig>,
+	owner_api_include_foreign: Option<bool>,
 ) -> Result<(), Error>
 where
 	T: WalletBackend<C, K> + 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");

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<bool>,
 	// 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,