From 22bf742a33c12b4d546a2b127ad35e3d09ab4353 Mon Sep 17 00:00:00 2001
From: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
Date: Fri, 16 May 2025 09:45:34 +1200
Subject: [PATCH] Apply suggestions from code review

---
 target/scripts/startup/variables-stack.sh       | 17 ++++++++++-------
 .../env_vars_from_files.bats                    |  4 ++--
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/target/scripts/startup/variables-stack.sh b/target/scripts/startup/variables-stack.sh
index 10f1e329..d2f3c788 100644
--- a/target/scripts/startup/variables-stack.sh
+++ b/target/scripts/startup/variables-stack.sh
@@ -249,26 +249,29 @@ function __environment_variables_export() {
 # This function sets any environment variable with a value from a referenced file
 # when an equivalent ENV with a `__FILE` suffix exists with a valid file path as the value.
 function __environment_variables_from_files() {
+  # Iterate through all ENV found with a `__FILE` suffix:
   while read -r ENV_WITH_FILE_REF; do
-    # Store the ENV name without the `__FILE` suffix:
-    local TARGET_ENV_NAME="${ENV_WITH_FILE_REF/__FILE/}"
-    local -n TARGET_ENV="${ENV_WITH_FILE_REF/__FILE/}"
     # Store the value of the `__FILE` ENV:
     local FILE_PATH="${!ENV_WITH_FILE_REF}"
+    # Store the ENV name without the `__FILE` suffix:
+    local TARGET_ENV_NAME="${ENV_WITH_FILE_REF/__FILE/}"
+    # Assign a value representing a variable name,
+    # `-n` will alias `TARGET_ENV` so that it is treated as if it were the referenced variable:
+    local -n TARGET_ENV="${TARGET_ENV_NAME}"
 
-    # Skip sourcing from `__FILE` if ENV is already set:
+    # Skip if the target ENV is already set:
     if [[ -v TARGET_ENV ]]; then
-      _log 'warn' "ENV value will not be sourced from '${ENV_WITH_FILE_REF}' since '${TARGET_ENV_NAME}' is already set"
+      _log 'warn' "Ignoring '${TARGET_ENV_NAME}' since '${ENV_WITH_FILE_REF}' is also set"
       continue
     fi
 
-    # Otherwise, retrieve the value from a file and set
-    # the ENV, or fail if the reference is invalid:
+    # Skip if the file path provided is invalid:
     if [[ ! -f ${FILE_PATH} ]]; then
       _log 'warn' "File defined for secret '${TARGET_ENV_NAME}' with path '${FILE_PATH}' does not exist"
       continue
     fi
 
+    # Read the value from a file and assign it to the intended ENV:
     _log 'info' "Getting secret '${TARGET_ENV_NAME}' from '${FILE_PATH}'"
     TARGET_ENV="$(< "${FILE_PATH}")"
   done < <(env | grep -Po '^.+?__FILE')
diff --git a/test/tests/parallel/set3/container_configuration/env_vars_from_files.bats b/test/tests/parallel/set3/container_configuration/env_vars_from_files.bats
index b28e2f4c..340450e3 100644
--- a/test/tests/parallel/set3/container_configuration/env_vars_from_files.bats
+++ b/test/tests/parallel/set3/container_configuration/env_vars_from_files.bats
@@ -45,7 +45,7 @@ function setup_file() {
   CONTAINER_NAME=${CONTAINER3_NAME}
   _init_with_defaults
   local CUSTOM_SETUP_ARGUMENTS=(
-    --env TEST__FILE="${FILEPATH_INVALID}"
+    --env ENABLE_POP3__FILE="${FILEPATH_INVALID}"
   )
   _common_container_setup 'CUSTOM_SETUP_ARGUMENTS'
 }
@@ -85,5 +85,5 @@ function teardown_file() {
   # Relevant log content only available via docker logs:
   run docker logs "${CONTAINER_NAME}"
   assert_success
-  assert_line --partial "File defined for secret 'TEST' with path '${FILEPATH_INVALID}' does not exist"
+  assert_line --partial "File defined for secret 'ENABLE_POP3' with path '${FILEPATH_INVALID}' does not exist"
 }