Unset xtrace while logging into acr via podman to prevent exposing the acr login secret

Reference: https://portal.microsofticm.com/imp/v5/incidents/details/752908291/summary

Add unset xtrace while configuring tls certificates
    * To be extra safe and ensure no certificate details are logged via shell tracing.

Add shell check comments, Update function descriptions
    * Add shellcheck comments for warnings that can be safely ignored.
    * Improve function readability by formatting them all in the same style.

Address shellcheck warnings in bootstrap scripts
    * Several warnings were unaddressed. Some of them were disabled as they can be safely ignored.
    * Others were refactored as the warnings were legitimate.

Author: s-fairchild

pkg/deploy/assets/gateway-production.json

@@ -147,8 +147,11 @@ RPIMAGE='$rpimage'"
     reboot_vm
 }
 
+# export AZURE_CLOUD_NAME="${AZURECLOUDNAME:?"Failed to carry over variables"}"
 export AZURE_CLOUD_NAME="${AZURECLOUDNAME:?"Failed to carry over variables"}"
 
+# util="util.sh"
+#
 # util.sh does not exist when deployed to VMSS via VMSS extensions
 # Provides shellcheck definitions
 util="util.sh"

pkg/deploy/assets/rp-production.json

@@ -8,10 +8,12 @@ main() {
     # transaction attempt retry time in seconds
     # shellcheck disable=SC2034
     local -ri retry_wait_time=30
-    local -ri pkg_retry_count=60
 
     create_required_dirs
+
     configure_sshd
+
+    local -ri pkg_retry_count=60
     configure_rpm_repos retry_wait_time \
                         "$pkg_retry_count"
 
@@ -53,10 +55,16 @@ main() {
     # shellcheck disable=SC2153 disable=SC2034
     local -r mdmimage="${RPIMAGE%%/*}/${MDMIMAGE#*/}"
     local -r rpimage="$RPIMAGE"
+
+    # shellcheck disable=SC2034
     local -r miseimage="${RPIMAGE%%/*}/${MISEIMAGE#*/}"
+
+    # shellcheck disable=SC2034
     local -r otelimage="$OTELIMAGE"
+
     # shellcheck disable=SC2034
     local -r fluentbit_image="$FLUENTBITIMAGE"
+
     # shellcheck disable=SC2034
     local -rA aro_images=(
         ["mdm"]="mdmimage"
@@ -218,11 +226,15 @@ MSI_RP_ENDPOINT='$MSIRPENDPOINT'
     reboot_vm
 }
 
+# export AZURE_CLOUD_NAME="${AZURECLOUDNAME:?"Failed to carry over variables"}"
+#
 # This variable is used by az-cli
 # It's assumed that if this variable hasn't been carried over, that others are also not present, so we fail early by returning an error
 # This was mostly helpful when testing on a development VM, but is still applicable
 export AZURE_CLOUD_NAME="${AZURECLOUDNAME:?"Failed to carry over variables"}"
 
+# util="util.sh"
+#
 # util.sh does not exist when deployed to VMSS via VMSS extensions
 # Provides shellcheck definitions
 util="util.sh"

pkg/deploy/generator/scripts/gatewayVMSS.sh

@@ -1,49 +1,111 @@
 #!/bin/bash
 # Internal Functions and Constants
 
-# empty_str - constant; used by functions for optional nameref string arguements
-# empty_str=""
+# declare -r empty_str=""
+#
+# empty_str - constant
+#   * used by functions for optional nameref string arguments
 # shellcheck disable=SC2034
 declare -r empty_str=""
 
-# role_gateway - constant; Is used to determine which VMSS is being bootstrapped
+# declare -r role_gateway="gateway"
+#
 # this should be referenced by scripts sourcing this file
 # role_gateway="gateway"
 declare -r role_gateway="gateway"
-# role_rp - constant; Is used to determine which VMSS is being bootstrapped
+
+# declare -r role_rp="rp"
+#
 # this should be referenced by scripts sourcing this file
 # role_rp="rp"
 declare -r role_rp="rp"
-# role_devproxy - constant; Is used to determine which VMSS is being bootstrapped
-# role_devproxy="devproxy"
+
+# declare -r role_devproxy="devproxy"
+#
+# role_devproxy - constant
+#   * Is used to determine which VMSS is being bootstrapped
 declare -r role_devproxy="devproxy"
-# us_gov_cloud - constant; Is the name of AZURECLOUDNAME for US government cloud
-# us_gov_cloud="AzureUSGovernment"
+
+# declare -r us_gov_cloud="AzureUSGovernment"
+#
+# us_gov_cloud - constant
+#   * Is the name of AZURECLOUDNAME for US government cloud
 declare -r us_gov_cloud="AzureUSGovernment"
 
-# log is a wrapper for echo that includes the function name
-# Args
-# 1) msg - string
-# 2) stack_level - int; optional, defaults to the function at the bottom of the call stack
+# declare -i XTRACE_SET=1
+#
+# constant value signifying xtrace shell value is/should be set
+declare -ir XTRACE_SET=1
+
+# declare -i XTRACE_UNSET=0
+#
+# constant value signifying xtrace shell value is/should be unset
+declare -ir XTRACE_UNSET=0
+
+# xtrace_is_set()
+#
+# Check if xtrace shell option is enabled/disabled
+#   * Returns XTRACE_SET value if set
+#   * Returns XTRACE_UNSET value if unset
+xtrace_is_set() {
+    if [[ $- =~ "x" ]]; then
+        echo XTRACE_SET
+    fi
+    
+    echo XTRACE_UNSET
+}
+
+# xtrace_toggle()
+#
+# set/unset xtrace shell option
+# args:
+#   1) string - nameref
+#       * Must be XTRACE_SET or XTRACE_UNSET
+xtrace_toggle() {
+    if ! [[ $1 =~ ("XTRACE_SET"|"XTRACE_UNSET") ]]; then
+        log "\$1 invalid; \$1 must be XTRACE_SET or XTRACE_UNSET. \$1: $1"
+        return 1
+    fi
+
+    if (( $1 == XTRACE_SET )); then
+        set -x 
+    elif
+        (( $1 == XTRACE_UNSET )); then
+        set +x
+    fi
+}
+
+# log()
+#
+# Wrapper for echo that includes the function name
+# args:
+#   1) msg - string
+#   2) stack_level - int
+#       * optional
+#       * defaults to the function at the bottom of the call stack
 log() {
     local -r msg="${1:-"log message is empty"}"
     local -r stack_level="${2:-1}"
     echo "${FUNCNAME[${stack_level}]}: ${msg}"
 }
 
-# abort is a wrapper for log that exits with an error code
+# abort()
+#
+# Wrapper for log that exits with an error code
 abort() {
     local -ri origin_stacklevel=2
     log "${1}" "$origin_stacklevel"
     log "Exiting"
     exit 1
 }
 
-# write_file
-# Args
-# 1) filename - string
-# 2) file_contents - string
-# 3) clobber - boolean; optional - defaults to false
+# write_file()
+#
+# args:
+#   1) filename - string
+#   2) file_contents - string
+#   3) clobber - boolean
+#       * Optional; defaults to false
 write_file() {
     local -n filename="$1"
     local -n file_contents="$2"
@@ -58,11 +120,16 @@ write_file() {
     fi
 }
 
-# retry Adding retry logic to yum commands in order to avoid stalling out on resource locks
+# retry()
+#
+# Add retry logic to commands in order to avoid stalling out on resource locks
 # args:
-# 1) cmd_retry - nameref, array; Command and arguement(s) to retry
-# 2) wait_time - nameref, integer; Time to wait before retrying command
-# 3) retries - integer, optional; Ammount of times to retry command, defaults to 5
+#   1) cmd_retry - nameref, array
+#       * Command and argument(s) to retry
+#   2) wait_time - nameref, integer
+#       * Time to wait before retrying command
+#   3) retries - integer, optional
+#       * Amount of times to retry command, defaults to 5
 retry() {
     local -n cmd_retry="$1"
     local -n wait_time="$2"
@@ -81,9 +148,11 @@ retry() {
     abort "${cmd_retry[*]} failed after #$retries attempts"
 }
 
-# verify_role
+# verify_role()
+#
 # args:
-# 1) test_role - nameref; role being verified
+#   1) test_role - nameref
+#       * role being verified
 verify_role() {
     local -n test_role="$1"
 
@@ -95,11 +164,15 @@ verify_role() {
     fi
 }
 
-# get_keyvault_suffix
+# get_keyvault_suffix()
+#
 # args:
-# 1) rl - nameref, string; role to get short role for
-# 2) kv_suffix - nameref, string; short role will be assigned to this nameref
-# 3) sec_prefix - nameref, string; keyvault certificate prefix will be assigned to this nameref
+#   1) rl - nameref, string
+#       * role to get short role for
+#   2) kv_suffix - nameref, string
+#       * short role will be assigned to this nameref
+#   3) sec_prefix - nameref, string
+#       * keyvault certificate prefix will be assigned to this nameref
 get_keyvault_suffix() {
     local -n rl="$1"
     local -n kv_suffix="$2"
@@ -118,14 +191,16 @@ get_keyvault_suffix() {
             sec_prefix="$role_rp"
             ;;
         *)
-            abort "unkown role $rl"
+            abort "unknown role $rl"
             ;;
     esac
 }
 
+# reboot_vm()
+#
 # reboot_vm restores calls shutdown -r in a subshell
-# Reboots should scheduled after all VM extensions have had time to complete
-# Reference: https://learn.microsoft.com/en-us/azure/virtual-machines/extensions/custom-script-linux#tips
+#   * Reboots should scheduled after all VM extensions have had time to complete
+#   * Reference: https://learn.microsoft.com/en-us/azure/virtual-machines/extensions/custom-script-linux#tips
 reboot_vm() {
     log "starting"
 

pkg/deploy/generator/scripts/rpVMSS.sh

@@ -1,6 +1,7 @@
 #!/bin/bash
 # Repository and package management related functions
 
+# configure_repo_mariner_extended()
 configure_repo_mariner_extended() {
     local -r extended_repo_config="https://packages.microsoft.com/cbl-mariner/2.0/prod/extended/x86_64/config.repo"
     curl -sSL "$extended_repo_config" -o /etc/yum.repos.d/mariner-extended.repo
@@ -19,21 +20,28 @@ configure_repo_mariner_extended() {
 }
 
 # configure_rpm_repos
+#
 # New repositories should be added in their own functions, and called here
 # args:
-# 1) wait_time - nameref, integer; Time to wait before retrying command
-# 2) retries - integer, optional; Amount of times to retry command, defaults to 5
+#   1) wait_time - nameref, integer
+#       * Time to wait before retrying command
+#   2) retries - integer, optional
+#       * Amount of times to retry command, defaults to 5
 configure_rpm_repos() {
     log "starting"
 
     configure_repo_mariner_extended "$1" "${2:-1}"
 }
 
 # dnf_install_pkgs
+#
 # args:
-# 1) pkgs - nameref, string array; Packages to be installed
-# 2) wait_time - nameref, integer; Time to wait before retrying command
-# 3) retries - integer, optional; Amount of times to retry command, defaults to 5
+#   1) pkgs - nameref, string array
+#       * Packages to be installed
+#   2) wait_time - nameref, integer
+#       * Time to wait before retrying command
+#   3) retries - integer, optional
+#       * Amount of times to retry command, defaults to 5
 dnf_install_pkgs() {
     local -n pkgs="$1"
     log "starting"
@@ -55,11 +63,15 @@ dnf_install_pkgs() {
 
 
 # dnf_update_pkgs
+#
 # args:
-# 1) excludes - nameref, string array, optional; Packages to exclude from updating
-#       Each index must be prefixed with -x 
-# 2) wait_time - nameref, integer; Time to wait before retrying command
-# 3) retries - integer, optional; Ammount of times to retry command, defaults to 5
+#   1) excludes - nameref, string array, optional
+#       * Packages to exclude from updating
+#       * Each index must be prefixed with -x 
+#   2) wait_time - nameref, integer
+#       * Time to wait before retrying command
+#   3) retries - integer, optional
+#       * Amount of times to retry command, defaults to 5
 dnf_update_pkgs() {
     local -n excludes="${1:-empty_str}"
     log "starting"
@@ -87,9 +99,12 @@ dnf_update_pkgs() {
 }
 
 # rpm_import_keys
+#
 # args:
-# 1) keys - nameref, string array; rpm keys to be imported
-# 2) wait_time - nameref, integer; Time to wait before retrying command
+#   1) keys - nameref, string array
+#       * rpm keys to be imported
+#   2) wait_time - nameref, integer
+#       * Time to wait before retrying command
 rpm_import_keys() {
     local -n keys="$1"
     log "starting"
@@ -112,6 +127,8 @@ rpm_import_keys() {
     done
 }
 
+# util_common="util-common.sh"
+#
 # util-common.sh does not exist when deployed to VMSS via VMSS extensions
 # Provides shellcheck definitions
 util_common="util-common.sh"