From f634f7e325fad403f3ffa5916f06fb880588736c Mon Sep 17 00:00:00 2001
From: Mark Lee <malept@users.noreply.github.com>
Date: Tue, 1 Oct 2024 09:26:48 -0700
Subject: [PATCH] fix(docker): standardize fetching the push/pull registries
 from the box config (#822)

* refactor: consistent variable naming conventions
---
 shell/docker-build.sh |  9 +++---
 shell/lib/docker.sh   | 70 ++++++++++++++++++++++++++++++++-----------
 2 files changed, 56 insertions(+), 23 deletions(-)

diff --git a/shell/docker-build.sh b/shell/docker-build.sh
index a6b6cc53..4053f9f7 100755
--- a/shell/docker-build.sh
+++ b/shell/docker-build.sh
@@ -14,6 +14,9 @@ source "${LIB_DIR}/box.sh"
 # shellcheck source=./lib/logging.sh
 source "${LIB_DIR}/logging.sh"
 
+# shellcheck source=./lib/docker.sh
+source "${LIB_DIR}/docker.sh"
+
 appName="$(get_app_name)"
 
 if [[ -z $DOCKERFILE ]]; then
@@ -26,11 +29,7 @@ warn "If you run into credential issues, ensure that your key is in your SSH age
 
 tags=()
 
-imageRegistries="${DOCKER_PUSH_REGISTRIES:-$(get_box_array 'docker.imagePushRegistries')}"
-if [[ -z $imageRegistries ]]; then
-  # Fall back to the old box field
-  imageRegistries="$(get_box_field 'devenv.imageRegistry')"
-fi
+imageRegistries="$(get_docker_push_registries)"
 
 for imageRegistry in $imageRegistries; do
   tags+=("--tag" "$imageRegistry/$appName")
diff --git a/shell/lib/docker.sh b/shell/lib/docker.sh
index 0e3d312c..730df2c6 100644
--- a/shell/lib/docker.sh
+++ b/shell/lib/docker.sh
@@ -47,11 +47,43 @@ get_image_field() {
   fi
 }
 
+# Returns a space-separated list of image registries to push to.
+get_docker_push_registries() {
+  local imageRegistries
+  imageRegistries="${BOX_DOCKER_PUSH_IMAGE_REGISTRIES:-$(get_box_array 'docker.imagePushRegistries')}"
+
+  if [[ -z $imageRegistries ]]; then
+    # Fall back to the old box field
+    imageRegistries="$(get_box_field 'devenv.imageRegistry')"
+  fi
+
+  echo "$imageRegistries"
+}
+
+# Returns the registry to pull images from. This is determined by either
+# the environment variable BOX_DOCKER_PULL_IMAGE_REGISTRY or one of the
+# following box fields:
+# * docker.imagePullRegistry
+# * devenv.imageRegistry
+get_docker_pull_registry() {
+  if [[ -n $BOX_DOCKER_PULL_IMAGE_REGISTRY ]]; then
+    echo "$BOX_DOCKER_PULL_IMAGE_REGISTRY"
+  else
+    local pullRegistry
+    pullRegistry="$(get_box_field 'docker.imagePullRegistry')"
+    if [[ -n $pullRegistry ]]; then
+      pullRegistry="$(get_box_field 'devenv.imageRegistry')"
+    fi
+
+    echo "$pullRegistry"
+  fi
+}
+
 # Generates Docker CLI arguments for building an image.
 #
-# docker_buildx_args(app_name, version, image, dockerfile[, arch]) -> arg string
+# docker_buildx_args(appName, version, image, dockerfile[, arch]) -> arg string
 docker_buildx_args() {
-  local app_name="$1"
+  local appName="$1"
   local version="$2"
   local image="$3"
   local dockerfile="$4"
@@ -112,9 +144,11 @@ docker_buildx_args() {
   if [[ -n $CIRCLE_TAG ]]; then
     tags+=("$image")
     if [[ -n $arch ]]; then
-      local remote_image_name
-      remote_image_name="$(determine_remote_image_name "$app_name" "$(get_box_field 'devenv.imageRegistry')" "$image")"
-      tags+=("$remote_image_name:latest-$arch" "$remote_image_name:$version-$arch")
+      local remoteImageName
+      local pullRegistry
+      pullRegistry="$(get_docker_pull_registry)"
+      remoteImageName="$(determine_remote_image_name "$appName" "$pullRegistry" "$image")"
+      tags+=("$remoteImageName:latest-$arch" "$remoteImageName:$version-$arch")
     fi
   fi
   for tag in "${tags[@]}"; do
@@ -129,7 +163,7 @@ docker_buildx_args() {
   buildContext="$(get_image_field "$image" "buildContext")"
   if [[ -z $buildContext ]]; then
     buildContext="."
-    if [[ $app_name != "$image" ]]; then
+    if [[ $appName != "$image" ]]; then
       buildContext="$(get_repo_directory)/deployments/$image"
     fi
   fi
@@ -141,29 +175,29 @@ docker_buildx_args() {
 # Where to push the image. This can be overridden in the manifest
 # with the field .pushTo. If not set, we'll use the imageRegistry
 # from the box configuration and the name of the image in devenv.yaml
-# as the repository. If this is not the main image (app_name), we'll
-# append the app_name to the repository to keep the images isolated
+# as the repository. If this is not the main image (appName), we'll
+# append the appName to the repository to keep the images isolated
 # to this repository.
 #
-# determine_remote_image_name(app_name, image_registry, image) -> remote_image_name
+# determine_remote_image_name(appName, imageRegistry, image) -> remoteImageName
 determine_remote_image_name() {
-  local app_name="$1"
-  local image_registry="$2"
+  local appName="$1"
+  local imageRegistry="$2"
   local image="$3"
-  local remote_image_name
+  local remoteImageName
 
-  remote_image_name=$(get_image_field "$image" "pushTo")
-  if [[ -z $remote_image_name ]]; then
-    remote_image_name="$image_registry/$image"
+  remoteImageName=$(get_image_field "$image" "pushTo")
+  if [[ -z $remoteImageName ]]; then
+    remoteImageName="$imageRegistry/$image"
 
     # If we're not the main image, then we should prefix the image name with the
     # app name, so that we can easily identify the image's source.
-    if [[ $image != "$app_name" ]]; then
-      remote_image_name="$image_registry/$app_name/$image"
+    if [[ $image != "$appName" ]]; then
+      remoteImageName="$imageRegistry/$appName/$image"
     fi
   fi
 
-  echo "$remote_image_name"
+  echo "$remoteImageName"
 }
 
 # run_docker is a wrapper for the docker command, but it prints out the