Merge ~ubuntu-docker-images/ubuntu-docker-images/+git/utils:refactor-libs into ~ubuntu-docker-images/ubuntu-docker-images/+git/utils:master

[Sergio Durigan Junior]

I've been meaning to do this for a while now, and since I'm working expanding these scripts and implement a way to print a manifest file from a registry, I decided to finally take some time to refactor a bit.

The idea behind this MP is to turn the existing standalone code we're carrying inside the scripts into "libraries" that can be sourced. This makes it easier to share code between the scripts, share credentials, and also simplify the validation of the arguments (now, only one validation will happen, performed by the scripts that's been called by the user).

I've tested this and it seems to be working OK, but I'd appreciate if another pair(s) of eyes could take a look.

[Bryce Harrington (bryce)]

I like it, this is a good step in the right direction. It's not only cleaner, but probably safer since there won't be need to pass credentials from process to subprocess.

The logic itself looks good; this mostly is just moving the existing (already-reviewed) code.

As part of moving this code to be used more broadly, as polish I'd also suggest this may be a good time to tighten up input variable checks, and perhaps think about code docs too. And a few other items along those lines down below - all just suggestions, pick and choose as you wish.

review: Approve

[Sergio Durigan Junior (sergiodj)]

Thanks for the review, Bryce!

I agree with everything you've said. I have a few comments/follow-ups.

[Bryce Harrington (bryce)]

Looks great.

If there are no actual subprocesses that require AUTH_TOKEN, it might be safer to not export it (all the sourced lib files will still have access to it even if not exported).

review: Approve

[Bryce Harrington (bryce)]

Oh one other thought, I'm curious about the distinction between helpers/ and lib/. I would think all these sourceable files could live together in one subdir, but maybe you have a plan in mind?

[Bryce Harrington (bryce)]

Oh another note, rather than calling the file list.sh (which may sound like utilities for manipulating bash arrays or other lists of things), could call it images.sh since that describes the main objects we're working with here?

[Sergio Durigan Junior (sergiodj)]

On Monday, April 12 2021, Bryce Harrington wrote:

> Oh one other thought, I'm curious about the distinction between
> helpers/ and lib/. I would think all these sourceable files could
> live together in one subdir, but maybe you have a plan in mind?

Hm, not really. I think it just felt more natural to put these
functions into a "lib/" dir because "helper/" seems like something that
will help the script prepare itself for the real job that it's supposed
to do. I'll leave as is for now, but we can revisit this later.

--
Sergio
GPG key ID: E92F D0B3 6B14 F1F4 D8E0 EB2F 106D A1C8 C3CB BF14

[Sergio Durigan Junior (sergiodj)]

On Monday, April 12 2021, Bryce Harrington wrote:

> Oh another note, rather than calling the file list.sh (which may sound
> like utilities for manipulating bash arrays or other lists of things),
> could call it images.sh since that describes the main objects we're
> working with here?

OK, good point, I'll rename the file.

--
Sergio
GPG key ID: E92F D0B3 6B14 F1F4 D8E0 EB2F 106D A1C8 C3CB BF14

diff --git a/helpers/validate-args.sh b/helpers/validate-args.sh
index 551c6f7..84ccd6b 100644
--- a/helpers/validate-args.sh
+++ b/helpers/validate-args.sh
@@ -7,6 +7,8 @@ export progname progdir
 
 # shellcheck disable=SC1090
 . "${progdir}/helpers/log.sh"
+# shellcheck disable=SC1090
+. "${progdir}/lib/image.sh"
 
 validate_var ()
 {
@@ -71,10 +73,8 @@ validate_image ()
 
     # shellcheck disable=SC2153
     # Fill the ALL_IMAGES array.
-    readarray -t ALL_IMAGES < <("${progdir}/list-all-images.sh" \
-				    -r "$REGISTRY" \
-				    -n "$NAMESPACE" \
-				    -a "$AUTH_TOKEN")
+    readarray -t ALL_IMAGES < <(print_list_of_images \
+				    "${REGISTRY}" "${NAMESPACE}")
 
     if ! grep -Fwq "$image" <<< "${ALL_IMAGES[@]}"; then
 	error "Invalid image name '$image'."
diff --git a/lib/image.sh b/lib/image.sh
new file mode 100644
index 0000000..a719c41
--- /dev/null
+++ b/lib/image.sh
@@ -0,0 +1,73 @@
+#!/bin/bash
+
+# This script is intended to be sourced.
+
+## List images ##
+
+_print_list_of_images_docker ()
+{
+    [ -n "${NAMESPACE}" ] || return 1
+    [ -n "${DOCKERHUB_URL}" ] || return 1
+    [ -n "${AUTH_TOKEN}" ] || return 1
+
+    for image in $(curl -s \
+			-H "Authorization: Bearer ${AUTH_TOKEN}" \
+			"${DOCKERHUB_URL}/v2/repositories/${NAMESPACE}/?page_size=10000" \
+		       | jq -r '.results|.[]|.name'); do
+	echo "$image"
+    done
+}
+
+_print_list_of_images_aws ()
+{
+    aws --region us-east-1 ecr-public describe-repositories \
+	| jq -r '.repositories[].repositoryName'
+}
+
+print_list_of_images ()
+{
+    [ -n "${REGISTRY}" ] || return 1
+
+    _print_list_of_images_"${REGISTRY}"
+}
+
+## List tags ##
+
+_print_tags_for_image_docker ()
+{
+    local image="$1"
+
+    [ -n "${NAMESPACE}" ] || return 1
+    [ -n "${AUTH_TOKEN}" ] || return 1
+    [ -n "${DOCKERHUB_URL}" ] || return 1
+    [ -n "${image}" ] || return 1
+
+    for tag in $(curl -s \
+		      -H "Authorization: JWT ${AUTH_TOKEN}" \
+		      "${DOCKERHUB_URL}/v2/repositories/${NAMESPACE}/${image}/tags/?page_size=10000" \
+		     | jq -r '.results|.[]|.name'); do
+	echo "${tag}"
+    done
+}
+
+_print_tags_for_image_aws ()
+{
+    local image="$1"
+
+    [ -n "${image}" ] || return 1
+
+    for tag in $(aws --region us-east-1 ecr-public describe-image-tags --repository-name "${image}" \
+		     | jq -r '.imageTagDetails[].imageTag'); do
+	echo "${tag}"
+    done
+}
+
+print_tags_for_image ()
+{
+    local image="$1"
+
+    [ -n "${REGISTRY}" ] || return 1
+    [ -n "${image}" ] || return 1
+
+    _print_tags_for_image_"${REGISTRY}" "${image}"
+}
diff --git a/lib/tag.sh b/lib/tag.sh
new file mode 100644
index 0000000..2679401
--- /dev/null
+++ b/lib/tag.sh
@@ -0,0 +1,51 @@
+#!/bin/bash
+
+# This script is intended to be sourced.
+
+## Tag image ##
+
+tag_image ()
+{
+    local image="$1"
+    local source_tag="$2"
+    local tag="$3"
+
+    [ -n "${REGISTRY}" ] || return 1
+    [ -n "${NAMESPACE}" ] || return 1
+    [ -n "${image}" ] || return 1
+    [ -n "${source_tag}" ] || return 1
+    [ -n "${tag}" ] || return 1
+    [ -n "${REGISTRY_AUTH_TOKEN}" ] || return 1
+    [ -n "${REGISTRY_URL}" ] || return 1
+
+    info "Tagging ${NAMESPACE}/${image}:${source_tag} as ${NAMESPACE}/${image}:${tag} (on ${REGISTRY})"
+
+    local tmpfile
+    local ret
+
+    tmpfile=$(mktemp)
+    trap 'rm -f ${tmpfile}' 0 INT QUIT ABRT PIPE TERM
+
+    ret=$(curl -s -H "Authorization: Bearer ${REGISTRY_AUTH_TOKEN}" \
+	       -H "Accept: application/vnd.docker.distribution.manifest.list.v2+json" \
+	       "${REGISTRY_URL}/v2/${NAMESPACE}/${image}/manifests/${source_tag}" \
+	       -s -o "${tmpfile}" -w "%{http_code}" || true)
+
+    if [ $ret -ne 200 ]; then
+	error "Unable to obtain manifest file for image (return code $ret)"
+    fi
+
+    # We use "|| true" here because we want the command to complete
+    # even if it fails.  We check for the HTTP response code below.
+    ret=$(curl -X PUT -H "Authorization: Bearer ${REGISTRY_AUTH_TOKEN}" \
+	       -H "Content-Type: application/vnd.docker.distribution.manifest.list.v2+json" \
+	       "${REGISTRY_URL}/v2/${NAMESPACE}/${image}/manifests/${tag}" \
+	       -d "@${tmpfile}" \
+	       -s -o /dev/null -w "%{http_code}" || true)
+
+    rm "$tmpfile"
+
+    if [ $ret -ne 201 ]; then
+	error "Image tagging failed with status code $ret"
+    fi
+}
diff --git a/list-all-images.sh b/list-all-images.sh
index c82b50c..63493a4 100755
--- a/list-all-images.sh
+++ b/list-all-images.sh
@@ -11,6 +11,8 @@ progdir=$(dirname "$(readlink -f "${0}")")
 . "${progdir}/helpers/log.sh"
 # shellcheck disable=SC1090
 . "${progdir}/helpers/registry-login.sh"
+# shellcheck disable=SC1090
+. "${progdir}/lib/image.sh"
 
 # Obtain the list of images for a certain repository.
 
@@ -62,27 +64,6 @@ validate_args ()
     do_login
 }
 
-print_list_of_images_docker ()
-{
-    for image in $(curl -s \
-			-H "Authorization: Bearer ${AUTH_TOKEN}" \
-			"${DOCKERHUB_URL}/v2/repositories/${NAMESPACE}/?page_size=10000" \
-		       | jq -r '.results|.[]|.name'); do
-	echo "$image"
-    done
-}
-
-print_list_of_images_aws ()
-{
-    aws --region us-east-1 ecr-public describe-repositories \
-	| jq -r '.repositories[].repositoryName'
-}
-
-print_list_of_images ()
-{
-    print_list_of_images_"${REGISTRY}"
-}
-
 if [ $# -lt 2 ]; then
     error "You need to provide at least the registry (-r) and the namespace (-n) arguments."
     usage
@@ -92,17 +73,18 @@ fi
 while [ -n "$1" ]; do
     case "$1" in
 	"-a"|"--auth-token")
+	    # shellcheck disable=SC2034
 	    AUTH_TOKEN="$2"
 	    shift 2
 	    ;;
 
 	"-r"|"--registry")
-	    REGISTRY="$2"
+	    export REGISTRY="$2"
 	    shift 2
 	    ;;
 
 	"-n"|"--namespace")
-	    NAMESPACE="$2"
+	    export NAMESPACE="$2"
 	    shift 2
 	    ;;
 
diff --git a/list-tags-for-image.sh b/list-tags-for-image.sh
index 305c9be..67380f8 100755
--- a/list-tags-for-image.sh
+++ b/list-tags-for-image.sh
@@ -11,6 +11,8 @@ progdir=$(dirname "$(readlink -f "${0}")")
 . "${progdir}/helpers/log.sh"
 # shellcheck disable=SC1090
 . "${progdir}/helpers/registry-login.sh"
+# shellcheck disable=SC1090
+. "${progdir}/lib/image.sh"
 
 # Obtain the list of tags for a certain image.
 
@@ -67,29 +69,6 @@ validate_args ()
     validate_image "$IMAGE"
 }
 
-print_tags_for_image_docker ()
-{
-    for tag in $(curl -s \
-		      -H "Authorization: JWT ${AUTH_TOKEN}" \
-		      "${DOCKERHUB_URL}/v2/repositories/${NAMESPACE}/${IMAGE}/tags/?page_size=10000" \
-		     | jq -r '.results|.[]|.name'); do
-	echo "${tag}"
-    done
-}
-
-print_tags_for_image_aws ()
-{
-    for tag in $(aws --region us-east-1 ecr-public describe-image-tags --repository-name "${IMAGE}" \
-		     | jq -r '.imageTagDetails[].imageTag'); do
-	echo "${tag}"
-    done
-}
-
-print_tags_for_image ()
-{
-    print_tags_for_image_"${REGISTRY}"
-}
-
 if [ $# -lt 2 ]; then
     error "You need to provide at least the registry (-r), the namespace (-n) and the image (-i) arguments."
     usage
@@ -99,22 +78,23 @@ fi
 while [ -n "$1" ]; do
     case "$1" in
 	"-a"|"--auth-token")
+	    # shellcheck disable=SC2034
 	    AUTH_TOKEN="$2"
 	    shift 2
 	    ;;
 
 	"-r"|"--registry")
-	    REGISTRY="$2"
+	    export REGISTRY="$2"
 	    shift 2
 	    ;;
 
 	"-n"|"--namespace")
-	    NAMESPACE="$2"
+	    export NAMESPACE="$2"
 	    shift 2
 	    ;;
 
 	"-i"|"--image")
-	    IMAGE="$2"
+	    export IMAGE="$2"
 	    shift 2
 	    ;;
 
@@ -138,4 +118,4 @@ done
 
 validate_args
 
-print_tags_for_image
+print_tags_for_image "${IMAGE}"
diff --git a/multi-arch-tagger.sh b/multi-arch-tagger.sh
index a493fa2..2b63ee5 100755
--- a/multi-arch-tagger.sh
+++ b/multi-arch-tagger.sh
@@ -11,6 +11,8 @@ progdir=$(dirname "$(readlink -f "${0}")")
 . "${progdir}/helpers/log.sh"
 # shellcheck disable=SC1090
 . "${progdir}/helpers/registry-login.sh"
+# shellcheck disable=SC1090
+. "${progdir}/lib/image.sh"
 
 # Tag all architectures of an image.
 
@@ -76,37 +78,6 @@ Arguments:
 EOF
 }
 
-do_tag_image ()
-{
-    info "Tagging ${NAMESPACE}/${IMAGE}:${SOURCE_TAG} as ${NAMESPACE}/${IMAGE}:${TAG} (on ${REGISTRY})"
-
-    local tmpfile
-    local ret
-
-    tmpfile=$(mktemp)
-    trap 'rm -f ${tmpfile}' 0 INT QUIT ABRT PIPE TERM
-
-    curl -s -H "Authorization: Bearer ${REGISTRY_AUTH_TOKEN}" \
-	 -H "Accept: application/vnd.docker.distribution.manifest.list.v2+json" \
-	 "${REGISTRY_URL}/v2/${NAMESPACE}/${IMAGE}/manifests/${SOURCE_TAG}" \
-	 > "$tmpfile"
-
-    # We use "|| true" here because we want the command to complete
-    # even if it fails.  We check for the HTTP response code below.
-    ret=$(curl -X PUT -H "Authorization: Bearer ${REGISTRY_AUTH_TOKEN}" \
-	       -H "Content-Type: application/vnd.docker.distribution.manifest.list.v2+json" \
-	       "${REGISTRY_URL}/v2/${NAMESPACE}/${IMAGE}/manifests/${TAG}" \
-	       -d "@${tmpfile}" \
-	       -s -o /dev/null -w "%{http_code}" || true)
-
-    rm "$tmpfile"
-
-    if [ "$ret" -ne 201 ]; then
-	echo "ERROR: Image tagging failed with status code $ret"
-	exit 1
-    fi
-}
-
 do_registry_login ()
 {
     _login_"${REGISTRY}"_registry1 "$IMAGE" "$REGISTRY_USERNAME" "$REGISTRY_PASSWORD"
@@ -158,32 +129,33 @@ fi
 while [ -n "$1" ]; do
     case "$1" in
 	"-a"|"--auth-token")
-	    export AUTH_TOKEN="$2"
+	    # shellcheck disable=SC2034
+	    AUTH_TOKEN="$2"
 	    shift 2
 	    ;;
 
 	"-r"|"--registry")
-	    REGISTRY="$2"
+	    export REGISTRY="$2"
 	    shift 2
 	    ;;
 
 	"-n"|"--namespace")
-	    NAMESPACE="$2"
+	    export NAMESPACE="$2"
 	    shift 2
 	    ;;
 
 	"-i"|"--image")
-	    IMAGE="$2"
+	    export IMAGE="$2"
 	    shift 2
 	    ;;
 
 	"-s"|"--source-tag")
-	    SOURCE_TAG="$2"
+	    export SOURCE_TAG="$2"
 	    shift 2
 	    ;;
 
 	"-t"|"--tag")
-	    TAG="$2"
+	    export TAG="$2"
 	    shift 2
 	    ;;
 
@@ -217,4 +189,4 @@ done
 
 validate_args
 
-do_tag_image
+tag_image "${IMAGE}" "${SOURCE_TAG}" "${TAG}"
diff --git a/tag-images.sh b/tag-images.sh
index d669d86..589fa0e 100755
--- a/tag-images.sh
+++ b/tag-images.sh
@@ -11,17 +11,21 @@ progdir=$(dirname "$(readlink -f "${0}")")
 . "${progdir}/helpers/log.sh"
 # shellcheck disable=SC1090
 . "${progdir}/helpers/registry-login.sh"
+# shellcheck disable=SC1090
+. "${progdir}/lib/image.sh"
+# shellcheck disable=SC1090
+. "${progdir}/lib/tag.sh"
 
 # Tag images from a repository.
 
 # The list of images to tag.
-IMAGES=()
+export IMAGES=()
 
 # The list of all available images.
-ALL_IMAGES=()
+export ALL_IMAGES=()
 
 # List of images to be excluded from the the processing.
-EXCLUDE_IMAGES=()
+export EXCLUDE_IMAGES=()
 
 # Usage.
 usage ()
@@ -107,11 +111,7 @@ do_tag_base_ubuntu_image ()
 
     local -a IMAGE_TAGS EDGE_TAGS
 
-    readarray -t IMAGE_TAGS < <("${progdir}/list-tags-for-image.sh" \
-				    -a "$AUTH_TOKEN" \
-				    -r "$REGISTRY" \
-				    -n "$NAMESPACE" \
-				    -i "$image")
+    readarray -t IMAGE_TAGS < <(print_tags_for_image "${image}")
 
     readarray -t EDGE_TAGS < <(printf '%s\n' "${IMAGE_TAGS[@]}" \
 				   | grep -E '^[[:alpha:]]+-[[:digit:]]+\.[[:digit:]]+_edge$')
@@ -134,44 +134,29 @@ do_tag_base_ubuntu_image ()
 	    # Tag each suffix.
 	    if [ -n "$FORCE" ] || ! grep -Fwq "${IMG_RELEASEVER}_${tagsuffix}" <<< "${IMAGE_TAGS[@]}"; then
 		info "Invoking multi-arch tagger for ${NAMESPACE}/${image}:${IMG_RELEASEVER}_${tagsuffix} (source tag: ${EDGE_TAG_PREFIX}_edge) (on ${REGISTRY})"
-		"${progdir}/multi-arch-tagger.sh" \
-		    -a "$AUTH_TOKEN" \
-		    -r "$REGISTRY" \
-		    -n "$NAMESPACE" \
-		    -i "$image" \
-		    -s "${EDGE_TAG_PREFIX}_edge" \
-		    -t "${IMG_RELEASEVER}_${tagsuffix}" \
-		    -u "$USERNAME" \
-		    -p "$PASSWORD"
+		tag_image \
+		    "${image}" \
+		    "${EDGE_TAG_PREFIX}_edge" \
+		    "${IMG_RELEASEVER}_${tagsuffix}"
 	    fi
 	done
 
 	# Tag the DISTRONAME.
 	if [ -n "$FORCE" ] || ! grep -Fwq "${IMG_DISTRONAME}" <<< "${IMAGE_TAGS[@]}"; then
 	    info "Invoking multi-arch tagger for ${NAMESPACE}/${image}:${IMG_DISTRONAME} (source tag: ${EDGE_TAG_PREFIX}_edge) (on ${REGISTRY})"
-	    "${progdir}/multi-arch-tagger.sh" \
-		-a "$AUTH_TOKEN" \
-		-r "$REGISTRY" \
-		-n "$NAMESPACE" \
-		-i "$image" \
-		-s "${EDGE_TAG_PREFIX}_edge" \
-		-t "${IMG_DISTRONAME}" \
-		-u "$USERNAME" \
-		-p "$PASSWORD"
+	    tag_image \
+		"${image}" \
+		"${EDGE_TAG_PREFIX}_edge" \
+		"${IMG_DISTRONAME}"
 	fi
 
 	# Tag the RELEASEVER
 	if [ -n "$FORCE" ] || ! grep -Fwq "${IMG_RELEASEVER}" <<< "${IMAGE_TAGS[@]}"; then
 	    info "Invoking multi-arch tagger for ${NAMESPACE}/${image}:${IMG_RELEASEVER} (source tag: ${EDGE_TAG_PREFIX}_edge) (on ${REGISTRY})"
-	    "${progdir}/multi-arch-tagger.sh" \
-		-a "$AUTH_TOKEN" \
-		-r "$REGISTRY" \
-		-n "$NAMESPACE" \
-		-i "$image" \
-		-s "${EDGE_TAG_PREFIX}_edge" \
-		-t "${IMG_RELEASEVER}" \
-		-u "$USERNAME" \
-		-p "$PASSWORD"
+	    tag_image \
+		"${image}" \
+		"${EDGE_TAG_PREFIX}_edge" \
+		"${IMG_RELEASEVER}"
 	fi
     done
 }
@@ -195,11 +180,7 @@ do_tag_images ()
 
 	local -a IMAGE_TAGS EDGE_TAGS
 
-	readarray -t IMAGE_TAGS < <("${progdir}/list-tags-for-image.sh" \
-					-a "$AUTH_TOKEN" \
-					-r "$REGISTRY" \
-					-n "$NAMESPACE" \
-					-i "$image")
+	readarray -t IMAGE_TAGS < <(print_tags_for_image "${image}")
 
 	# Get the list of all M.N-XX.YY_edge tags, and sort them
 	# according to the release numbers.  In the end, we will have
@@ -234,15 +215,10 @@ do_tag_images ()
 		    # corresponding 'M.N-XX.YY_beta' tag associated with
 		    # the '_edge' tag, so we need to create it.
 		    info "Invoking multi-arch tagger for ${NAMESPACE}/${image}:${suptag} (source tag: ${EDGE_TAG_PREFIX}_edge) (on ${REGISTRY})"
-		    "${progdir}/multi-arch-tagger.sh" \
-			-a "$AUTH_TOKEN" \
-			-r "$REGISTRY" \
-			-n "$NAMESPACE" \
-			-i "$image" \
-			-s "${EDGE_TAG_PREFIX}_edge" \
-			-t "${suptag}" \
-			-u "$USERNAME" \
-			-p "$PASSWORD"
+		    tag_image \
+			"${image}" \
+			"${EDGE_TAG_PREFIX}_edge" \
+			"${suptag}"
 		fi
 	    done
 	    tagidx=$((++tagidx))
@@ -268,10 +244,7 @@ validate_args ()
     do_login
 
     # Fill the ALL_IMAGES array.
-    readarray -t ALL_IMAGES < <("${progdir}/list-all-images.sh" \
-					  -r "$REGISTRY" \
-					  -n "$NAMESPACE" \
-					  -a "$AUTH_TOKEN")
+    readarray -t ALL_IMAGES < <(print_list_of_images)
 
     if [ "${#IMAGES[@]}" -eq 0 ]; then
 	# The user hasn't provided any image names, so just use all of
@@ -310,6 +283,8 @@ validate_args ()
 	readarray -t IMAGES <<< "${NEW_IMAGES[@]}"
 	unset NEW_IMAGES
     fi
+
+    _login_"${REGISTRY}"_registry1 "${IMAGES[@]}" "$USERNAME" "$PASSWORD"
 }
 
 if [ $# -lt 2 ]; then
@@ -321,17 +296,17 @@ fi
 while [ -n "$1" ]; do
     case "$1" in
 	"-r"|"--registry")
-	    REGISTRY="$2"
+	    export REGISTRY="$2"
 	    shift 2
 	    ;;
 
 	"-n"|"--namespace")
-	    NAMESPACE="$2"
+	    export NAMESPACE="$2"
 	    shift 2
 	    ;;
 
 	"-f"|"--force")
-	    FORCE=1
+	    export FORCE=1
 	    shift
 	    ;;