dockerfile parameter broken in the /v3.1.2/libpod/build api call

[mwhahaha]

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind bug

Description

#10550 has changed the way the dockerfile parameter is handled by the api. Previously the expectation was that dockerfile is a string parameter, however the patch seems to change the expectations to json which fails when passed in a simple string.

This can be seen over in the podman-py CI which attempts to use the image build comand via the api.

containers/podman-py#110

This works with the shipped 3.2.0 version, however the latest version in git (3.3.0-dev) is broken.

Steps to reproduce the issue:

1) build from source
2) create containerfile
[root@aschultz-fc34 fedora]# cat containerfile
FROM quay.io/libpod/alpine_labels:latest

3) tar containerfile
[root@aschultz-fc34 fedora]# tar cvf c.tar containerfile
containerfile
4) issue build command
[root@aschultz-fc34 fedora]# curl -XPOST --data-binary @<(cat c.tar) -H "Content-type: application/x-tar" --unix-socket /run/podman/podman.sock http://localhost/v3.1.2/libpod/build?dockerfile=containerfile

Describe the results you received:

[root@aschultz-fc34 fedora]# cat containerfile
FROM quay.io/libpod/alpine_labels:latest

[root@aschultz-fc34 fedora]# tar cvf c.tar containerfile
containerfile
[root@aschultz-fc34 fedora]# curl -XPOST --data-binary @<(cat c.tar) -H "Content-type: application/x-tar" --unix-socket /run/podman/podman.sock http://localhost/v3.1.2/libpod/build?dockerfile=containerfile
{"stream":"STEP 1: FROM quay.io/libpod/alpine_labels:latest\n"}
{"stream":"STEP 2: COMMIT\n"}
{"stream":"--\u003e 4fab981df73\n"}
{"stream":"4fab981df737963d83b548a3a6f4e7b43f718eb88567ba5ec7921f636a93d7bc\n"}
[root@aschultz-fc34 fedora]# podman --version
podman version 3.2.0
[root@aschultz-fc34 fedora]# curl -XPOST --data-binary @<(cat c.tar) -H "Content-type: application/x-tar" --unix-socket /run/podman/podman.sock http://localhost/v3.1.2/libpod/build?dockerfile=containerfile
{"cause":"invalid character 'c' looking for beginning of value","message":"failed to parse query parameter 'dockerfile': \"containerfile\": invalid character 'c' looking for beginning of value","response":400}
[root@aschultz-fc34 fedora]# podman --version
podman version 3.3.0-dev

Describe the results you expected:

[root@aschultz-fc34 fedora]# cat containerfile
FROM quay.io/libpod/alpine_labels:latest

[root@aschultz-fc34 fedora]# tar cvf c.tar containerfile
containerfile
[root@aschultz-fc34 fedora]# curl -XPOST --data-binary @<(cat c.tar) -H "Content-type: application/x-tar" --unix-socket /run/podman/podman.sock http://localhost/v3.1.2/libpod/build?dockerfile=containerfile
{"stream":"STEP 1: FROM quay.io/libpod/alpine_labels:latest\n"}
{"stream":"STEP 2: COMMIT\n"}
{"stream":"--\u003e 4fab981df73\n"}
{"stream":"4fab981df737963d83b548a3a6f4e7b43f718eb88567ba5ec7921f636a93d7bc\n"}

Additional information you deem important (e.g. issue happens only occasionally):

Output of podman version:

podman version 3.3.0-dev

Output of podman info --debug:

host:
  arch: amd64
  buildahVersion: 1.21.1
  cgroupControllers:
  - cpuset
  - cpu
  - io
  - memory
  - hugetlb
  - pids
  cgroupManager: cgroupfs
  cgroupVersion: v2
  conmon:
    package: conmon-2.0.27-2.fc34.x86_64
    path: /usr/bin/conmon
    version: 'conmon version 2.0.27, commit: '
  cpus: 8
  distribution:
    distribution: fedora
    version: "34"
  eventLogger: file
  hostname: aschultz-fc34
  idMappings:
    gidmap: null
    uidmap: null
  kernel: 5.12.9-300.fc34.x86_64
  linkmode: dynamic
  memFree: 13132312576
  memTotal: 16778973184
  ociRuntime:
    name: crun
    package: crun-0.20.1-1.fc34.x86_64
    path: /usr/bin/crun
    version: |-
      crun version 0.20.1
      commit: 0d42f1109fd73548f44b01b3e84d04a279e99d2e
      spec: 1.0.0
      +SYSTEMD +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +CRIU +YAJL
  os: linux
  remoteSocket:
    exists: true
    path: /run/podman/podman.sock
  security:
    apparmorEnabled: false
    capabilities: CAP_CHOWN,CAP_DAC_OVERRIDE,CAP_FOWNER,CAP_FSETID,CAP_KILL,CAP_NET_BIND_SERVICE,CAP_SETFCAP,CAP_SETGID,CAP_SETPCAP,CAP_SETUID,CAP_SYS_CHROOT
    rootless: false
    seccompEnabled: true
    seccompProfilePath: /usr/share/containers/seccomp.json
    selinuxEnabled: true
  serviceIsRemote: false
  slirp4netns:
    executable: ""
    package: ""
    version: ""
  swapFree: 0
  swapTotal: 0
  uptime: 2h 12m 18.64s (Approximately 0.08 days)
registries:
  search:
  - registry.fedoraproject.org
  - registry.access.redhat.com
  - docker.io
  - quay.io
store:
  configFile: /etc/containers/storage.conf
  containerStore:
    number: 1
    paused: 0
    running: 0
    stopped: 1
  graphDriverName: overlay
  graphOptions:
    overlay.mountopt: nodev,metacopy=on
  graphRoot: /var/lib/containers/storage
  graphStatus:
    Backing Filesystem: extfs
    Native Overlay Diff: "false"
    Supports d_type: "true"
    Using metacopy: "true"
  imageStore:
    number: 3
  runRoot: /run/containers/storage
  volumePath: /var/lib/containers/storage/volumes
version:
  APIVersion: 3.3.0-dev
  Built: 1623429334
  BuiltTime: Fri Jun 11 16:35:34 2021
  GitCommit: a634b2cd5977c60f1907733efe07b61ba36271fb-dirty
  GoVersion: go1.16.3
  OsArch: linux/amd64
  Version: 3.3.0-dev

Package info (e.g. output of rpm -q podman or apt list podman):

built from source using a634b2cd5977c60f1907733efe07b61ba36271fb

Have you tested with the latest version of Podman and have you checked the Podman Troubleshooting Guide? (https://github.com/containers/podman/blob/master/troubleshooting.md)

Yes

Additional environment details (AWS, VirtualBox, physical, etc.):

[mwhahaha]

A proposed fix would be:

diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go
index 50423fb96..9c4dd8638 100644
--- a/pkg/api/handlers/compat/images_build.go
+++ b/pkg/api/handlers/compat/images_build.go
@@ -144,8 +144,8 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
        if _, found := r.URL.Query()["dockerfile"]; found {
                var m = []string{}
                if err := json.Unmarshal([]byte(query.Dockerfile), &m); err != nil {
-                       utils.BadRequest(w, "dockerfile", query.Dockerfile, err)
-                       return
+                       // it's not json, assume just a string
+                       m = append(m, query.Dockerfile)
                }
                containerFiles = m
        } else {

[mheon]

Eeek. This one seems pretty bad.

@rhatdan @Luap99 @jwhonce I think we need to hold 3.2.1 until we have a fix for this, breaking the Docker-compat API this seriously is bad.

[rhatdan]

@mwhahaha interested in opening a PR? @mheon I did not know you were back porting all of the new fixes.

[mwhahaha]

@rhatdan #10061 is open, i'm looking at adding tests

[mheon]

@rhatdan I grab just about everything that's not a new feature or change to existing functionality and applies cleanly on the branch.