From 45d5f942a37eecfa7856200376b8cf8a35e071de Mon Sep 17 00:00:00 2001
From: Aditya R <arajan@redhat.com>
Date: Wed, 2 Nov 2022 14:34:26 +0530
Subject: [PATCH] compat,build: handle docker's preconfigured cacheTo,cacheFrom

Docker's newer clients popuates `cacheFrom` and `cacheTo` parameter
by default as empty array for all commands but buildah's design of
distributed cache expects this to be a repo not image hence parse
only the first populated repo and igore if empty array.

Signed-off-by: Aditya R <arajan@redhat.com>
---
 pkg/api/handlers/compat/images_build.go | 43 ++++++++++++++++++++-----
 test/apiv2/10-images.at                 |  7 ++++
 2 files changed, 42 insertions(+), 8 deletions(-)

cf. https://github.com/containers/podman/pull/16380#issuecomment-1299918607

NB: this affects the podman release 4.3.0: building anything using the docker
client and targetting podman as a server is broken, even when not using any
cache-to/cache-from flag.



diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go
index d4c8c0743a4..577dfcb26f6 100644
--- a/pkg/api/handlers/compat/images_build.go
+++ b/pkg/api/handlers/compat/images_build.go
@@ -399,20 +399,47 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
+	// Docker's newer clients popuates `cacheFrom` and `cacheTo` parameter
+	// by default as empty array for all commands but buildah's design of
+	// distributed cache expects this to be a repo not image hence parse
+	// only the first populated repo and igore if empty array.
+	// Read more here: https://github.com/containers/podman/issues/15928
+	// TODO: Remove this when buildah's API is extended.
+	compatIgnoreForcedCacheOptions := func(queryStr string) string {
+		query := queryStr
+		if strings.HasPrefix(query, "[") {
+			query = ""
+			var arr []string
+			parseErr := json.Unmarshal([]byte(query), &arr)
+			if parseErr != nil {
+				if len(arr) > 0 {
+					query = arr[0]
+				}
+			}
+		}
+		return query
+	}
+
 	var cacheFrom reference.Named
 	if _, found := r.URL.Query()["cachefrom"]; found {
-		cacheFrom, err = parse.RepoNameToNamedReference(query.CacheFrom)
-		if err != nil {
-			utils.BadRequest(w, "cacheFrom", query.CacheFrom, err)
-			return
+		cacheFromQuery := compatIgnoreForcedCacheOptions(query.CacheFrom)
+		if cacheFromQuery != "" {
+			cacheFrom, err = parse.RepoNameToNamedReference(cacheFromQuery)
+			if err != nil {
+				utils.BadRequest(w, "cacheFrom", cacheFromQuery, err)
+				return
+			}
 		}
 	}
 	var cacheTo reference.Named
 	if _, found := r.URL.Query()["cacheto"]; found {
-		cacheTo, err = parse.RepoNameToNamedReference(query.CacheTo)
-		if err != nil {
-			utils.BadRequest(w, "cacheto", query.CacheTo, err)
-			return
+		cacheToQuery := compatIgnoreForcedCacheOptions(query.CacheTo)
+		if cacheToQuery != "" {
+			cacheTo, err = parse.RepoNameToNamedReference(cacheToQuery)
+			if err != nil {
+				utils.BadRequest(w, "cacheto", cacheToQuery, err)
+				return
+			}
 		}
 	}
 	var cacheTTL time.Duration
diff --git a/test/apiv2/10-images.at b/test/apiv2/10-images.at
index 3ffc6f73856..19096280689 100644
--- a/test/apiv2/10-images.at
+++ b/test/apiv2/10-images.at
@@ -189,6 +189,13 @@ tar --format=posix -C $TMPD -cvf ${CONTAINERFILE_TAR} containerfile &> /dev/null
 t POST "libpod/build?dockerfile=containerfile" $CONTAINERFILE_TAR 200 \
   .stream~"STEP 1/1: FROM $IMAGE"
 
+# Newer Docker client sets empty cacheFrom for every build command even if it is not used,
+# following commit makes sure we test such use-case. See https://github.com/containers/podman/pull/16380
+#TODO: This test should be extended when buildah's cache-from and cache-to functionally supports
+# multiple remote-repos
+t POST "libpod/build?dockerfile=containerfile&cachefrom=[]" $CONTAINERFILE_TAR 200 \
+  .stream~"STEP 1/1: FROM $IMAGE"
+
 # With -q, all we should get is image ID. Test both libpod & compat endpoints.
 t POST "libpod/build?dockerfile=containerfile&q=true" $CONTAINERFILE_TAR 200 \
   .stream~'^[0-9a-f]\{64\}$'