fix: add Environment cfg source and remove --create-id (#4018)

Description --- - adds `__` separator to `Environment` config source - removes `--create-id` cli arg from base node Motivation and Context --- Environment variables with previous formatting work. `--create-id` for base node is mostly redundant, a new one can be created silently by passing in `--init` or `--non-interactive` How Has This Been Tested? --- Manually using `TARI_DIBBLER__BASE_NODE__IDENTITY_FILE=...`
tari-project · Apr 12, 2022 · e59e657 · e59e657
1 parent 328867e
commit e59e657
Show file tree

Hide file tree

Showing 16 changed files with 29 additions and 51 deletions.
diff --git a/README.md b/README.md
@@ -242,7 +242,7 @@ start Tor service `~/tari/applications/tari_base_node/osx/start_tor` (on Mac),
 
 To run from any directory of your choice, where the executable is visible in the path (first time use):
 
-    tari_base_node --init --create-id
+    tari_base_node --init 
     tari_base_node
 
     tari_console_wallet --init
@@ -264,7 +264,7 @@ Consecutive runs:
 Alternatively, you can run the Tari applications from your source directory using `cargo`, and just omit the `--release`
 flag if you want to run in debug mode (first time use):
 
-    cargo run --bin tari_base_node --release --  --init --create-id
+    cargo run --bin tari_base_node --release --  --init 
     cargo run --bin tari_base_node --release
 
     cargo run --bin tari_merge_mining_proxy --release

diff --git a/applications/launchpad/docker_rig/quick_start.sh b/applications/launchpad/docker_rig/quick_start.sh
@@ -52,7 +52,7 @@ export TARI_NETWORK=$NETWORK
 
 if [[ $SETUP == 1 ]]; then
   echo "Creating identity files and default config file"
-  docker compose run --rm base_node --create-id --init
+  docker compose run --rm base_node --init
 fi
 
 if [[ $START_TOR == 1 ]]; then

diff --git a/applications/tari_app_utilities/src/identity_management.rs b/applications/tari_app_utilities/src/identity_management.rs
@@ -36,13 +36,14 @@ pub const LOG_TARGET: &str = "tari_application";
 
 const REQUIRED_IDENTITY_PERMS: u32 = 0o100600;
 
-/// Loads the node identity, or creates a new one if the --create-id flag was specified
+/// Loads the node identity, or creates a new one if create_id is true
 ///
 /// ## Parameters
-/// `identity_file` - Reference to file path
-/// `public_address` - Network address of the base node
-/// `create_id` - Whether an identity needs to be created or not
-/// `peer_features` - Enables features of the base node
+/// - `identity_file` - Reference to file path
+/// - `public_address` - Network address of the base node
+/// - `create_id` - Only applies if the identity_file does not exist or is malformed. If true, a new identity will be
+/// created, otherwise the user will be prompted to create a new ID
+/// - `peer_features` - Enables features of the base node
 ///
 /// # Return
 /// A NodeIdentity wrapped in an atomic reference counter on success, the exit code indicating the reason on failure
@@ -64,7 +65,7 @@ pub fn setup_node_identity<P: AsRef<Path>>(
             ExitCode::ConfigError,
             &format!(
                 "{path} has incorrect permissions. You can update the identity file with the correct permissions \
-                 using 'chmod 600 {path}', or delete the identity file and re-run the node with the --create-id flag.",
+                 using 'chmod 600 {path}', or delete the identity file and a new one will be created on next start",
                 path = identity_file.as_ref().to_string_lossy()
             ),
         )),
@@ -75,17 +76,15 @@ pub fn setup_node_identity<P: AsRef<Path>>(
                 if !prompt {
                     error!(
                         target: LOG_TARGET,
-                        "Node identity information not found. {}. You can update the configuration file to point to a \
-                         valid node identity file, or re-run the node with the --create-id flag to create a new \
-                         identity.",
+                        "Node identity not found. {}. You can update the configuration file to point to a valid node \
+                         identity file, or re-run the node and create a new one.",
                         e
                     );
                     return Err(ExitError::new(
                         ExitCode::ConfigError,
                         &format!(
                             "Node identity information not found. {}. You can update the configuration file to point \
-                             to a valid node identity file, or re-run the node with the --create-id flag to create a \
-                             new identity.",
+                             to a valid node identity file, or re-run the node to create a new one",
                             e
                         ),
                     ));

diff --git a/applications/tari_base_node/linux/supervisord.md b/applications/tari_base_node/linux/supervisord.md
@@ -20,7 +20,7 @@ sudo cp -v /home/vagrant/src/tari/target/release/tari_base_node  /usr/local/tari
 ```
 Create your tari_base_node configs
 ```
-sudo /usr/local/tari/bin/tari_base_node --base-path /usr/local/tari --init --create-id
+sudo /usr/local/tari/bin/tari_base_node --base-path /usr/local/tari --init 
 ```
 Setup ```tari_base_node``` services in supervisord -
 ```/etc/supervisor/conf.d/tari_base_node.conf```

diff --git a/applications/tari_base_node/osx/install.sh b/applications/tari_base_node/osx/install.sh
@@ -83,7 +83,6 @@ fi
 
 # Configure Base Node
 ./tari_base_node --init
-./tari_base_node --create-id
 
 banner Running Tari Base Node
 # Run Base Node

diff --git a/applications/tari_base_node/osx/post_install.sh b/applications/tari_base_node/osx/post_install.sh
@@ -197,7 +197,7 @@ if [ ! -f "$DATA_DIR/config.toml" ]; then
 #  cp log4rs_sample_base_node.yml $DATA_DIR/log4rs_base_node.yml
 
   # Configure Base Node
-  tari_base_node --init --create-id
+  tari_base_node --init
 
   echo "Configuration complete."
 fi

diff --git a/applications/tari_base_node/setup.sh b/applications/tari_base_node/setup.sh
@@ -29,7 +29,6 @@ osascript -e "tell application \"Terminal\" to do script \"sh ${PWD}/start_tor.s
 # Configure Base Node
 cd "${INSTALL_ROOT}" || exit
 tari_base-node --init
-tari_base_node --create-id
 
 # Run Base Node
 tari_base_node
diff --git a/applications/tari_base_node/src/cli.rs b/applications/tari_base_node/src/cli.rs
@@ -31,9 +31,6 @@ const DEFAULT_NETWORK: &str = "dibbler";
 pub(crate) struct Cli {
     #[clap(flatten)]
     pub common: CommonCliArgs,
-    /// Create and save new node identity if one doesn't exist
-    #[clap(long, alias = "create_id")]
-    pub create_id: bool,
     /// Create a default configuration file if it doesn't exist
     #[clap(long)]
     pub init: bool,

diff --git a/applications/tari_base_node/src/main.rs b/applications/tari_base_node/src/main.rs
@@ -44,7 +44,7 @@
 /// ```
 ///
 /// For the first run
-/// `cargo run tari_base_node -- --create-id`
+/// `cargo run tari_base_node -- --init
 ///
 /// Subsequent runs
 /// `cargo run tari_base_node`
@@ -153,20 +153,10 @@ fn main_inner() -> Result<(), ExitError> {
     let node_identity = setup_node_identity(
         &config.base_node.identity_file,
         config.base_node.p2p.public_address.as_ref(),
-        cli.create_id || cli.non_interactive_mode,
+        cli.non_interactive_mode || cli.init,
         PeerFeatures::COMMUNICATION_NODE,
     )?;
 
-    // Exit if create_id or init arguments were run
-    if cli.create_id {
-        info!(
-            target: LOG_TARGET,
-            "Base node's node ID created at '{}'. Done.",
-            config.base_node.identity_file.as_path().to_string_lossy(),
-        );
-        return Ok(());
-    }
-
     if cli.init {
         info!(target: LOG_TARGET, "Default configuration created. Done.");
         return Ok(());

diff --git a/applications/tari_base_node/windows/runtime/source_base_node_env.bat b/applications/tari_base_node/windows/runtime/source_base_node_env.bat
@@ -61,7 +61,7 @@ if exist "%my_exe_path%\%my_exe%" (
 
 rem First time run
 if not exist "%config_path%\base_node_id.json" (
-    "%base_node%" --create-id --init --config "%config_path%\config.toml" --log_config "%config_path%\log4rs_base_node.yml" --base-path "%base_path%"
+    "%base_node%" --init --config "%config_path%\config.toml" --log_config "%config_path%\log4rs_base_node.yml" --base-path "%base_path%"
     echo.
     echo.
     echo Created "%config_path%\base_node_id.json".

diff --git a/applications/tari_validator_node/src/cli.rs b/applications/tari_validator_node/src/cli.rs
@@ -56,9 +56,6 @@ pub(crate) struct Cli {
     /// Enable tracing
     #[clap(long, aliases = &["tracing", "enable-tracing"])]
     pub tracing_enabled: bool,
-    /// Create and save new node identity if one doesn't exist
-    #[clap(long, alias = "create_id")]
-    pub create_id: bool,
     /// Supply a network (overrides existing configuration)
     #[clap(long, alias = "network", default_value = DEFAULT_NETWORK)]
     pub network: String,

diff --git a/applications/tari_validator_node/src/main.rs b/applications/tari_validator_node/src/main.rs
@@ -90,18 +90,18 @@ fn main_inner() -> Result<(), ExitError> {
 
     let config = ApplicationConfig::load_from(&cfg)?;
     let runtime = build_runtime()?;
-    runtime.block_on(run_node(&config, cli))?;
+    runtime.block_on(run_node(&config))?;
 
     Ok(())
 }
 
-async fn run_node(config: &ApplicationConfig, cli: Cli) -> Result<(), ExitError> {
+async fn run_node(config: &ApplicationConfig) -> Result<(), ExitError> {
     let shutdown = Shutdown::new();
 
     let node_identity = setup_node_identity(
         &config.validator_node.identity_file,
         config.validator_node.public_address.as_ref(),
-        cli.create_id,
+        true,
         PeerFeatures::NONE,
     )?;
     let db_factory = SqliteDbFactory::new(config.validator_node.data_dir.clone());

diff --git a/buildtools/docker/start.sh b/buildtools/docker/start.sh
@@ -11,7 +11,7 @@ sleep 15
 
 TARI_CONFIG=~/.tari/config/config.toml
 if [[ ! -f $TARI_CONFIG ]]; then
-  tari_base_node --init --create-id
+  tari_base_node --init
   # fix for docker, bind grpc to 0.0.0.0 instead of loopback
   sed -i -e 's/127.0.0.1:18142/0.0.0.0:18142/' $TARI_CONFIG
 fi

diff --git a/common/src/configuration/utils.rs b/common/src/configuration/utils.rs
@@ -36,7 +36,11 @@ pub fn load_configuration<P: AsRef<Path>>(
         .ok_or_else(|| ConfigError::new("Invalid config file path", None))?;
     let mut cfg = Config::builder()
         .add_source(config::File::with_name(filename))
-        .add_source(config::Environment::with_prefix("TARI"));
+        .add_source(
+            config::Environment::with_prefix("TARI")
+                .prefix_separator("_")
+                .separator("__"),
+        );
 
     for (key, value) in overrides {
         cfg = cfg

diff --git a/integration_tests/helpers/baseNodeProcess.js b/integration_tests/helpers/baseNodeProcess.js
@@ -49,14 +49,7 @@ class BaseNodeProcess {
         break;
       }
     } while (fs.existsSync(this.baseDir));
-    const args = [
-      "--base-path",
-      ".",
-      "--init",
-      "--create-id",
-      "--network",
-      "localnet",
-    ];
+    const args = ["--base-path", ".", "--init", "--network", "localnet"];
     const overrides = this.getOverrides();
     Object.keys(overrides).forEach((k) => {
       args.push("-p");

diff --git a/integration_tests/helpers/validatorNodeProcess.js b/integration_tests/helpers/validatorNodeProcess.js
@@ -47,7 +47,7 @@ class ValidatorNodeProcess {
         break;
       }
     } while (fs.existsSync(this.baseDir));
-    const args = ["--base-path", ".", "--init", "--create-id"];
+    const args = ["--base-path", ".", "--init"];
     if (this.logFilePath) {
       args.push("--log-config", this.logFilePath);
     }