From 88cf3320bbabdc56b814e4f1d968e3b0a1199ed1 Mon Sep 17 00:00:00 2001 From: jotaen4tinypilot <83721279+jotaen4tinypilot@users.noreply.github.com> Date: Wed, 17 Jan 2024 14:58:13 +0100 Subject: [PATCH] Delegate to unified strip-marker-sections script (#1722) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves https://github.com/tiny-pilot/tinypilot/issues/1710. Stacked on https://github.com/tiny-pilot/tinypilot/pull/1720. Based on the [simplifications in `unset-static-ip`](https://github.com/tiny-pilot/tinypilot/pull/1719), this PR removes the code redundancies in the `unset-static-ip` and `change-hostname` privileged scripts, and calls the new, unified `strip-marker-sections` script instead. ## Notes - `change-hostname`: - Naively speaking, it would seem simpler to *append* the new entry to the file. I remembered, however, that the prepending was done on purpose, because in `/etc/hosts` the first matching entry takes precedence. I captured that as a comment therefore. - I slightly reordered the code so that all `/etc/hosts`-related logic is close together now (i.e., clearing the markers, then rewriting the file). - `unset-static-ip` - ~I couldn’t figure out why we have been using `tee` previously to write to the config file, because we discarded `tee`’s stdout output by redirecting to `/dev/null`. I’m also not sure why we used `sudo` for invoking `tee` – [we already execute the script with `root` privileges](https://github.com/tiny-pilot/tinypilot-pro/blob/cfb99f88a3039d587a9108d427e7ecfa81b7d190/app/static_ip.py#L128). It’s a bit odd, because I cannot imagine we did that without good reason~ 🤔 (We [had been using `sudo tee` to avoid permission issues](https://github.com/tiny-pilot/tinypilot/pull/1722#issuecomment-1892704458) when writing the file.) Review on CodeApprove --------- Co-authored-by: Jan Heuermann --- .../scripts/change-hostname | 37 +++++-------------- .../scripts/unset-static-ip | 30 +-------------- 2 files changed, 12 insertions(+), 55 deletions(-) diff --git a/debian-pkg/opt/tinypilot-privileged/scripts/change-hostname b/debian-pkg/opt/tinypilot-privileged/scripts/change-hostname index a7fdd4008..5ccb8255f 100755 --- a/debian-pkg/opt/tinypilot-privileged/scripts/change-hostname +++ b/debian-pkg/opt/tinypilot-privileged/scripts/change-hostname @@ -56,36 +56,19 @@ if ! [[ "${NEW_HOSTNAME}" =~ $VALID_HOSTNAME_PATTERN ]] \ exit 1 fi -# Retrieve original `/etc/hosts`. -# - Read file line by line, make sure to preserve all whitespace. -# - Remove all marker sections. -etc_hosts_original=() -is_in_marker_section='false' -while IFS= read -r line; do - if "${is_in_marker_section}" && [[ "${line}" == "${MARKER_END}" ]]; then - is_in_marker_section='false' - continue - fi - if "${is_in_marker_section}" || [[ "${line}" == "${MARKER_START}" ]]; then - is_in_marker_section='true' - continue - fi - etc_hosts_original+=("${line}") -done < /etc/hosts +# Remove any existing marker sections from the `/etc/hosts` file. +"${SCRIPT_DIR}/strip-marker-sections" /etc/hosts -if "${is_in_marker_section}"; then - echo 'Unclosed marker section' >&2 - exit 1 -fi +# Populate new entry to `/etc/hosts`. +# Note that the first matching entry takes precedence, which is why we prepend +# our new entry. +OLD_ETC_HOSTS="$( /etc/hosts # Populate new hostname to `/etc/hostname`. # Note that the value must be newline-terminated, see: # https://manpages.debian.org/stretch/systemd/hostname.5.en.html printf "%s\n" "${NEW_HOSTNAME}" > /etc/hostname - -# Populate corresponding entry to `etc/hosts`. -OLD_ETC_HOSTS=$(printf "%s\n" "${etc_hosts_original[@]}") -readonly OLD_ETC_HOSTS -printf "%s\n127.0.1.1 %s\n%s\n%s\n" \ - "${MARKER_START}" "${NEW_HOSTNAME}" "${MARKER_END}" "${OLD_ETC_HOSTS}" \ - > /etc/hosts diff --git a/debian-pkg/opt/tinypilot-privileged/scripts/unset-static-ip b/debian-pkg/opt/tinypilot-privileged/scripts/unset-static-ip index a7dce0a2d..b83f02bce 100755 --- a/debian-pkg/opt/tinypilot-privileged/scripts/unset-static-ip +++ b/debian-pkg/opt/tinypilot-privileged/scripts/unset-static-ip @@ -17,7 +17,6 @@ EOF SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" readonly SCRIPT_DIR -readonly CONFIG_FILE="/etc/dhcpcd.conf" # shellcheck source=lib/markers.sh . "${SCRIPT_DIR}/lib/markers.sh" @@ -59,33 +58,8 @@ fi # Exit on unset variable set -u -# Remove any existing auto-generated configuration as populated by the -# set-static-ip script. -dhcpcd_original=() -is_in_marker_section='false' -while IFS= read -r line; do - if "${is_in_marker_section}" && [[ "${line}" == "${MARKER_END}" ]]; then - is_in_marker_section='false' - continue - fi - if "${is_in_marker_section}" || [[ "${line}" == "${MARKER_START}" ]]; then - is_in_marker_section='true' - continue - fi - dhcpcd_original+=("${line}") -done < "${CONFIG_FILE}" - -if "${is_in_marker_section}"; then - echo 'Unclosed marker section' >&2 - exit 1 -fi - -# Convert array of lines to a single string. -OLD_CONFIG_FILE=$(printf "%s\n" "${dhcpcd_original[@]}") -readonly OLD_CONFIG_FILE - -# Write original file contents back to the file. -echo "${OLD_CONFIG_FILE}" | sudo tee "${CONFIG_FILE}" > /dev/null +# Remove any existing marker sections from the config file. +"${SCRIPT_DIR}/strip-marker-sections" /etc/dhcpcd.conf # Changes require a reboot so prompt the user. if [[ "${QUIET}" == "false" ]] ; then