Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update install.func: Install figlet on all new LXC #1350

Closed
wants to merge 4 commits into from

Conversation

michelroegl-brunner
Copy link
Member

🛠️ Note:
We are meticulous about merging code into the main branch, so please understand that pull requests not meeting the project's standards may be rejected. It's never personal!
🎮 Note for game-related scripts: These have a lower likelihood of being merged.


✍️ Description

Installs Figlet on all new LXC for Header generation.


🛠️ Type of Change

Please check the relevant options:

  • Bug fix (non-breaking change that resolves an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change unexpectedly)
  • New script (a fully functional and thoroughly tested script or set of scripts)

✅ Prerequisites

The following steps must be completed for the pull request to be considered:

  • Self-review performed (I have reviewed my code to ensure it follows established patterns and conventions.)
  • Testing performed (I have thoroughly tested my changes and verified expected functionality.)
  • Documentation updated (I have updated any relevant documentation)

📋 Additional Information (optional)

Provide any extra context or screenshots about the feature or fix here.

@michelroegl-brunner michelroegl-brunner self-assigned this Jan 9, 2025
@michelroegl-brunner michelroegl-brunner requested a review from a team as a code owner January 9, 2025 13:38
@github-actions github-actions bot added the high risk A change that can affect many scripts label Jan 9, 2025
Copy link
Contributor

github-actions bot commented Jan 9, 2025

Script formatting

❌ We found issues in the formatting of the following changed files:

diff misc/install.func.orig misc/install.func
--- misc/install.func.orig
+++ misc/install.func
@@ -19,7 +19,7 @@
   BOLD=$(echo "\033[1m")
   HOLD=" "
   TAB="  "
-  
+
   # System
   RETRY_NUM=10
   RETRY_EVERY=3
@@ -56,7 +56,7 @@
 
 # This function handles errors
 error_handler() {
-  if [ -n "$SPINNER_PID" ] && ps -p $SPINNER_PID > /dev/null; then kill $SPINNER_PID > /dev/null; fi
+  if [ -n "$SPINNER_PID" ] && ps -p $SPINNER_PID >/dev/null; then kill $SPINNER_PID >/dev/null; fi
   printf "\e[?25h"
   local exit_code="$?"
   local line_number="$1"
@@ -79,7 +79,7 @@
 
   while true; do
     printf "\r ${color}%s${CL}" "${frames[spin_i]}"
-    spin_i=$(( (spin_i + 1) % ${#frames[@]} ))
+    spin_i=$(((spin_i + 1) % ${#frames[@]}))
     sleep "$interval"
   done
 }
@@ -94,7 +94,7 @@
 
 # This function displays a success message with a green color.
 msg_ok() {
-  if [ -n "$SPINNER_PID" ] && ps -p $SPINNER_PID > /dev/null; then kill $SPINNER_PID > /dev/null; fi
+  if [ -n "$SPINNER_PID" ] && ps -p $SPINNER_PID >/dev/null; then kill $SPINNER_PID >/dev/null; fi
   printf "\e[?25h"
   local msg="$1"
   echo -e "${BFR}${CM}${GN}${msg}${CL}"
@@ -102,20 +102,20 @@
 
 # This function displays a error message with a red color.
 msg_error() {
-  if [ -n "$SPINNER_PID" ] && ps -p $SPINNER_PID > /dev/null; then kill $SPINNER_PID > /dev/null; fi
+  if [ -n "$SPINNER_PID" ] && ps -p $SPINNER_PID >/dev/null; then kill $SPINNER_PID >/dev/null; fi
   printf "\e[?25h"
   local msg="$1"
   echo -e "${BFR}${CROSS}${RD}${msg}${CL}"
 }
-install_figlet(){
-  if ! command -v figlet &> /dev/null; then
-    
+install_figlet() {
+  if ! command -v figlet &>/dev/null; then
+
     # Install necessary dependencies and figlet
     if [ -f /etc/debian_version ] || [ -f /etc/lsb-release ]; then
-      apt-get update -y &> /dev/null
-      apt-get install -y tar build-essential curl &> /dev/null
+      apt-get update -y &>/dev/null
+      apt-get install -y tar build-essential curl &>/dev/null
     elif [ -f /etc/alpine-release ]; then
-      apk add --no-cache tar build-base curl&> /dev/null
+      apk add --no-cache tar build-base curl &>/dev/null
       export TERM=xterm
     else
       return 1
@@ -176,23 +176,23 @@
   ipv4_connected=false
   ipv6_connected=false
   sleep 1
-# Check IPv4 connectivity to Google, Cloudflare & Quad9 DNS servers.
-  if ping -c 1 -W 1 1.1.1.1 &>/dev/null || ping -c 1 -W 1 8.8.8.8 &>/dev/null || ping -c 1 -W 1 9.9.9.9 &>/dev/null; then 
-    msg_ok "IPv4 Internet Connected";
+  # Check IPv4 connectivity to Google, Cloudflare & Quad9 DNS servers.
+  if ping -c 1 -W 1 1.1.1.1 &>/dev/null || ping -c 1 -W 1 8.8.8.8 &>/dev/null || ping -c 1 -W 1 9.9.9.9 &>/dev/null; then
+    msg_ok "IPv4 Internet Connected"
     ipv4_connected=true
   else
-    msg_error "IPv4 Internet Not Connected";
-  fi
-
-# Check IPv6 connectivity to Google, Cloudflare & Quad9 DNS servers.
+    msg_error "IPv4 Internet Not Connected"
+  fi
+
+  # Check IPv6 connectivity to Google, Cloudflare & Quad9 DNS servers.
   if ping6 -c 1 -W 1 2606:4700:4700::1111 &>/dev/null || ping6 -c 1 -W 1 2001:4860:4860::8888 &>/dev/null || ping6 -c 1 -W 1 2620:fe::fe &>/dev/null; then
-    msg_ok "IPv6 Internet Connected";
+    msg_ok "IPv6 Internet Connected"
     ipv6_connected=true
   else
-    msg_error "IPv6 Internet Not Connected";
-  fi
-
-# If both IPv4 and IPv6 checks fail, prompt the user
+    msg_error "IPv6 Internet Not Connected"
+  fi
+
+  # If both IPv4 and IPv6 checks fail, prompt the user
   if [[ $ipv4_connected == false && $ipv6_connected == false ]]; then
     read -r -p "No Internet detected,would you like to continue anyway? <y/N> " prompt
     if [[ "${prompt,,}" =~ ^(y|yes)$ ]]; then
@@ -222,7 +222,7 @@
   echo -n "DIRECT"
 fi
 EOF
-  chmod +x /usr/local/bin/apt-proxy-detect.sh
+    chmod +x /usr/local/bin/apt-proxy-detect.sh
   fi
   $STD apt-get update
   $STD apt-get -o Dpkg::Options::="--force-confold" -y dist-upgrade
@@ -233,10 +233,10 @@
 # This function modifies the message of the day (motd) and SSH settings
 motd_ssh() {
   # Set terminal to 256-color mode
-  grep -qxF "export TERM='xterm-256color'" /root/.bashrc || echo "export TERM='xterm-256color'" >> /root/.bashrc
+  grep -qxF "export TERM='xterm-256color'" /root/.bashrc || echo "export TERM='xterm-256color'" >>/root/.bashrc
 
   # Get the current private IP address
-  IP=$(hostname -I | awk '{print $1}')  # Private IP
+  IP=$(hostname -I | awk '{print $1}') # Private IP
 
   # Get OS information (Debian / Ubuntu)
   if [ -f "/etc/os-release" ]; then
@@ -251,13 +251,13 @@
   MOTD_FILE="/etc/motd"
   if [ -f "$MOTD_FILE" ]; then
     # Start MOTD with application info and link
-    echo -e "\n${BOLD}${APPLICATION} LXC Container${CL}" > "$MOTD_FILE"
-    echo -e "${TAB}${GATEWAY}${YW} Provided by: ${GN}community-scripts ORG ${YW}| GitHub: ${GN}https://github.com/community-scripts/ProxmoxVE${CL}\n" >> "$MOTD_FILE"
+    echo -e "\n${BOLD}${APPLICATION} LXC Container${CL}" >"$MOTD_FILE"
+    echo -e "${TAB}${GATEWAY}${YW} Provided by: ${GN}community-scripts ORG ${YW}| GitHub: ${GN}https://github.com/community-scripts/ProxmoxVE${CL}\n" >>"$MOTD_FILE"
 
     # Add system information with icons
-    echo -e "${TAB}${OS}${YW} OS: ${GN}${OS_NAME} - Version: ${OS_VERSION}${CL}" >> "$MOTD_FILE"
-    echo -e "${TAB}${HOSTNAME}${YW} Hostname: ${GN}$(hostname)${CL}" >> "$MOTD_FILE"
-    echo -e "${TAB}${INFO}${YW} IP Address: ${GN}${IP}${CL}" >> "$MOTD_FILE"
+    echo -e "${TAB}${OS}${YW} OS: ${GN}${OS_NAME} - Version: ${OS_VERSION}${CL}" >>"$MOTD_FILE"
+    echo -e "${TAB}${HOSTNAME}${YW} Hostname: ${GN}$(hostname)${CL}" >>"$MOTD_FILE"
+    echo -e "${TAB}${INFO}${YW} IP Address: ${GN}${IP}${CL}" >>"$MOTD_FILE"
   else
     echo "MotD file does not exist!" >&2
   fi

@Mellowlynx
Copy link
Contributor

And this is why I was against this way of the header.
New we need to add software to make it work...
No, just add it by hand in the script, we should not need to add "unneeded" software to any container or VM.
More user-friendly, one less maintenance burden and possible vulnerability.

@MickLesk
Copy link
Member

MickLesk commented Jan 9, 2025

And this is why I was against this way of the header. New we need to add software to make it work... No, just add it by hand in the script, we should not need to add "unneeded" software to any container or VM. More user-friendly, one less maintenance burden and possible vulnerability.

#1246

misc/install.func Outdated Show resolved Hide resolved
@MickLesk MickLesk added the maintenance Code maintenance or general upkeep of the project label Jan 9, 2025
@michelroegl-brunner michelroegl-brunner deleted the figlet_install branch January 10, 2025 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high risk A change that can affect many scripts maintenance Code maintenance or general upkeep of the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants