From 4d7ed3f6924aeb7b0dc6de8dee2991543212cd2d Mon Sep 17 00:00:00 2001
From: humanoid2050 <humanoid2050@katana>
Date: Fri, 31 Mar 2023 15:29:22 -0400
Subject: [PATCH] refactor docker script arg parsing and fix merge bugs

---
 tools/docker/build.sh            | 24 ++++--------------------
 tools/docker/deploy_images.sh    | 18 +++++-------------
 tools/docker/deploy_manifests.sh | 16 +++-------------
 tools/docker/lib/common          | 21 +++++++++++++++++++--
 tools/docker/test.sh             | 15 +--------------
 5 files changed, 32 insertions(+), 62 deletions(-)

diff --git a/tools/docker/build.sh b/tools/docker/build.sh
index 337fd6da9..9118d5b87 100755
--- a/tools/docker/build.sh
+++ b/tools/docker/build.sh
@@ -15,19 +15,7 @@ set -euxo pipefail
 
 source "$(realpath $(dirname ${BASH_SOURCE[0]}))/lib/common"
 
-#used by docker buildx bake, so mark export
-export TAG_VER="$1"
-if [ -z "$TAG_VER" ]; then
-    echo "We cannot tag Docker images with an empty string!" >&2
-    exit 1
-fi
-ARCH_LIST="$2"
-if [ -z "$ARCH_LIST" ]; then
-    echo "Architectures must be specified!" >&2
-    exit 1
-fi
-
-export REGISTRY_SPEC="${DOCKER_HUB_ORG}/"
+ParseArgs $@
 
 #jump to root, matching popd handed by Cleanup on EXIT via trap
 pushd "${REPO_ROOT}"
@@ -36,6 +24,7 @@ pushd "${REPO_ROOT}"
 trap Cleanup EXIT
 # Create the builder
 CreateBuilder
+InstallMultiarchSupport
 
 
 BuildAndCacheByArch() {
@@ -43,7 +32,7 @@ BuildAndCacheByArch() {
     docker buildx build --target certbot --builder certbot_builder \
         --platform $(arch2platform $TAG_ARCH) \
         -f "${WORK_DIR}/Dockerfile" \
-        -t "${REGISTRY_SPEC}certbot:${TAG_ARCH}-${TAG_VER}" \
+        -t "${DOCKER_HUB_ORG}/certbot:${TAG_ARCH}-${TAG_VER}" \
         --load \
         .
     for plugin in "${CERTBOT_PLUGINS[@]}"; do
@@ -51,7 +40,7 @@ BuildAndCacheByArch() {
             --platform $(arch2platform $TAG_ARCH) \
             --build-context plugin-src="${REPO_ROOT}/certbot-${plugin}" \
             -f "${WORK_DIR}/Dockerfile" \
-            -t "${REGISTRY_SPEC}${plugin}:${TAG_ARCH}-${TAG_VER}" \
+            -t "${DOCKER_HUB_ORG}/${plugin}:${TAG_ARCH}-${TAG_VER}" \
             --load \
             .
     done
@@ -62,11 +51,6 @@ BuildAndCacheByArch() {
 # of such a build. See the branch buildx-bake and
 # https://github.com/certbot/certbot/issues/9587.
 
-# split arch list into an array for per-arch image building and saving
-IFS_OLD="$IFS"
-IFS=","
-read -ra REQUESTED_ARCH_ARRAY <<< $(InterpretArchRequest "$ARCH_LIST")
-IFS="$IFS_OLD"
 for ARCH in "${REQUESTED_ARCH_ARRAY[@]}"; do
     BuildAndCacheByArch $ARCH
 done    
diff --git a/tools/docker/deploy_images.sh b/tools/docker/deploy_images.sh
index cf66bfcb3..9d934f161 100755
--- a/tools/docker/deploy_images.sh
+++ b/tools/docker/deploy_images.sh
@@ -17,12 +17,7 @@ set -euxo pipefail
 
 source "$(realpath $(dirname ${BASH_SOURCE[0]}))/lib/common"
 
-TAG_VER="$1"
-if [ -z "$TAG_VER" ]; then
-    echo "We cannot tag Docker images with an empty string!" >&2
-    exit 1
-fi
-REQUESTED_ARCH_LIST=$(InterpretArchRequest "$2")
+ParseArgs $@
 
 #jump to root, matching popd handed by Cleanup on EXIT via trap
 pushd "${REPO_ROOT}"
@@ -35,21 +30,18 @@ REGISTRY_SPEC="${DOCKER_HUB_ORG}/"
 DeployImage() {
     IMAGE_NAME=$1
     TAG_ARCH=$2
-    TAG_VER=$3
     docker push "${REGISTRY_SPEC}${IMAGE_NAME}:${TAG_ARCH}-${TAG_VER}"
     if [[ "${TAG_VER}" =~ ^v([2-9]|[1-9][0-9]+)\.[0-9]+\.[0-9]+$ ]]; then
         docker tag "${REGISTRY_SPEC}${IMAGE_NAME}:${TAG_ARCH}-${TAG_VER}" "${REGISTRY_SPEC}${IMAGE_NAME}:${TAG_ARCH}-latest"
+        docker push "${REGISTRY_SPEC}${IMAGE_NAME}:${TAG_ARCH}-latest"
     fi
 }
 
-IFS_OLD="$IFS"
-IFS=","
-read -ra REQUESTED_ARCH_ARRAY <<< $(InterpretArchRequest "$2")
-IFS="$IFS_OLD"
+
 for TAG_ARCH in "${REQUESTED_ARCH_ARRAY[@]}"; do
-    DeployImage certbot $TAG_ARCH $TAG_VER
+    DeployImage certbot $TAG_ARCH
     for PLUGIN in "${CERTBOT_PLUGINS[@]}"; do
-        DeployImage $PLUGIN $TAG_ARCH $TAG_VER
+        DeployImage $PLUGIN $TAG_ARCH
     done
 done
 
diff --git a/tools/docker/deploy_manifests.sh b/tools/docker/deploy_manifests.sh
index 4d98f1a17..2cd7ddf4c 100755
--- a/tools/docker/deploy_manifests.sh
+++ b/tools/docker/deploy_manifests.sh
@@ -18,12 +18,7 @@ set -euxo pipefail
 
 source "$(realpath $(dirname ${BASH_SOURCE[0]}))/lib/common"
 
-TAG_VER="$1"
-if [ -z "$TAG_VER" ]; then
-    echo "We cannot tag Docker images with an empty string!" >&2
-    exit 1
-fi
-REQUESTED_ARCH_LIST=$(InterpretArchRequest "$2")
+ParseArgs $@
 
 #jump to root, matching popd handed by Cleanup on EXIT via trap
 pushd "${REPO_ROOT}"
@@ -35,11 +30,6 @@ REGISTRY_SPEC="${DOCKER_HUB_ORG}/"
 
 DeployManifest() {
     IMAGE_NAME=$1
-    local IFS=","
-    read -ra REQUESTED_ARCH_ARRAY <<< ${REQUESTED_ARCH_LIST}
-    TAG_VER=$3
-
-    IFS=" "
     
     SRC_IMAGES=""
     for TAG_ARCH in "${REQUESTED_ARCH_ARRAY[@]}"; do
@@ -52,9 +42,9 @@ DeployManifest() {
     fi
 }
 
-DeployManifest certbot ${REQUESTED_ARCH_LIST} $TAG_VER
+DeployManifest certbot
 for PLUGIN in "${CERTBOT_PLUGINS[@]}"; do
-    DeployManifest $PLUGIN ${REQUESTED_ARCH_LIST} $TAG_VER
+    DeployManifest $PLUGIN
 done
 
 
diff --git a/tools/docker/lib/common b/tools/docker/lib/common
index 4ba463be3..d36c5640e 100644
--- a/tools/docker/lib/common
+++ b/tools/docker/lib/common
@@ -32,8 +32,6 @@ export CERTBOT_PLUGINS=(
 export WORK_DIR="$(realpath $(dirname ${BASH_SOURCE[0]})/..)"
 # REPO_ROOT is two levels above that
 export REPO_ROOT="$(realpath ${WORK_DIR}/../..)"
-# location where docker cache should be created (may be anywhere accessble on the host filesystem)
-export DOCKER_CACHE="${REPO_ROOT}/.docker_cache"
 
 # Converts input architecture identifier to the platform specification
 # understood by `docker build buildx --platform <specification>`.
@@ -82,6 +80,24 @@ InterpretArchRequest() {
     echo "$USER_INPUT"
 }
 
+ParseArgs() {
+
+    export TAG_VER="$1"
+    if [ -z "$TAG_VER" ]; then
+        echo "We cannot tag Docker images with an empty string!" >&2
+        exit 1
+    fi
+    ARCH_LIST="$2"
+    if [ -z "$ARCH_LIST" ]; then
+        echo "Architectures must be specified!" >&2
+        exit 1
+    fi
+    # split arch list into an array for per-arch image building and saving
+    local IFS=","
+    read -ra REQUESTED_ARCH_ARRAY <<< $(InterpretArchRequest "$ARCH_LIST")
+    export REQUESTED_ARCH_ARRAY
+}
+
 
 # Function for use with trap in the primary scripts to remove the 
 # docker builder and restore the original directory
@@ -90,6 +106,7 @@ Cleanup() {
     popd
 }
 
+# add binfmt tools to the docker environment, with integration into the new builder instance
 InstallMultiarchSupport() {
     docker run --privileged --rm tonistiigi/binfmt --install all
 }
diff --git a/tools/docker/test.sh b/tools/docker/test.sh
index 6daeefd2a..cf30f87fb 100755
--- a/tools/docker/test.sh
+++ b/tools/docker/test.sh
@@ -14,20 +14,7 @@ set -euxo pipefail
 
 source "$(realpath $(dirname ${BASH_SOURCE[0]}))/lib/common"
 
-TAG_VER="$1"
-if [ -z "$TAG_VER" ]; then
-    echo "We cannot tag Docker images with an empty string!" >&2
-    exit 1
-fi
-if [ -z "$2" ]; then
-    echo "Architectures must be specified!" >&2
-    exit 1
-fi
-IFS_OLD="$IFS"
-IFS=","
-read -ra REQUESTED_ARCH_ARRAY <<< $(InterpretArchRequest "$2")
-IFS="$IFS_OLD"
-
+ParseArgs $@
 
 #jump to root, matching popd handed by Cleanup on EXIT via trap
 pushd "${REPO_ROOT}"