Merge pull request containers#16380 from flouthoc/compat-build-fix

compat,build: handle `docker's` preconfigured `cacheTo`,`cacheFrom`
edsantiago · Nov 15, 2022 · 6c37cf3 · 6c37cf3
2 parents b73c0f2 + c7073b5
commit 6c37cf3
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 8 deletions.
diff --git 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
@@ -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\}$'