[v23 regression] `docker build --build-arg =TEST_STRING` no longer fails

[AkihiroSuda]

Description

DOCKER_BUILDKIT=1 docker build --build-arg =TEST_STRING no longer fails since v23

v20.10.24

$ DOCKER_BUILDKIT=0 docker build --build-arg =TEST_STRING .
invalid argument "=TEST_STRING" for "--build-arg" flag: invalid environment variable: =TEST_STRING
See 'docker build --help'.

$ DOCKER_BUILDKIT=1 docker build --build-arg =TEST_STRING .
invalid argument "=TEST_STRING" for "--build-arg" flag: invalid environment variable: =TEST_STRING
See 'docker build --help'.

v23.0.6, v24.0.5

$ DOCKER_BUILDKIT=0 docker build --build-arg =TEST_STRING .
DEPRECATED: The legacy builder is deprecated and will be removed in a future release.
            BuildKit is currently disabled; enable it by removing the DOCKER_BUILDKIT=0
            environment-variable.

invalid argument "=TEST_STRING" for "--build-arg" flag: invalid environment variable: =TEST_STRING
See 'docker build --help'.

$ DOCKER_BUILDKIT=1 docker build --build-arg =TEST_STRING .
[+] Building 0.0s (3/3) FINISHED                                                                  docker:default
...
(no error)

Reproduce

DOCKER_BUILDKIT=1 docker build --build-arg =TEST_STRING

Expected behavior

Should fail with an error like invalid argument "=TEST_STRING" for "--build-arg" flag: invalid environment variable: =TEST_STRING

docker version

Client:
 Version:           24.0.5
 API version:       1.43
 Go version:        go1.20.6
 Git commit:        ced0996
 Built:             Fri Jul 21 20:34:32 2023
 OS/Arch:           linux/amd64
 Context:           default

Server: Docker Engine - Community
 Engine:
  Version:          24.0.5
  API version:      1.43 (minimum version 1.12)
  Go version:       go1.20.6
  Git commit:       a61e2b4
  Built:            Fri Jul 21 20:35:56 2023
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          v1.7.1
  GitCommit:        1677a17964311325ed1c31e2c0a3589ce6d5c30d
 runc:
  Version:          1.1.8
  GitCommit:        v1.1.8-0-g82f18fe
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

docker info

Client:
 Version:    24.0.5
 Context:    default
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc.)
    Version:  v0.11.2
    Path:     /usr/local/libexec/docker/cli-plugins/docker-buildx
  compose: Docker Compose (Docker Inc.)
    Version:  v2.20.2
    Path:     /usr/local/libexec/docker/cli-plugins/docker-compose

Server:
 Containers: 0
  Running: 0
  Paused: 0
  Stopped: 0
 Images: 1
 Server Version: 24.0.5
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Using metacopy: false
  Native Overlay Diff: true
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Cgroup Version: 2
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 1677a17964311325ed1c31e2c0a3589ce6d5c30d
 runc version: v1.1.8-0-g82f18fe
 init version: de40ad0
 Security Options:
  apparmor
  seccomp
   Profile: builtin
  cgroupns
 Kernel Version: 6.2.0-26-generic
 Operating System: Alpine Linux v3.18 (containerized)
 OSType: linux
 Architecture: x86_64
 CPUs: 4
 Total Memory: 15.57GiB
 Name: cdba30409c70
 ID: 5a603912-b731-4b97-aad1-91e31bdcb2cd
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 Experimental: false
 Insecure Registries:
  127.0.0.0/8
 Live Restore Enabled: false
 Product License: Community Engine

Additional Info

No response

[thaJeztah]

This looks to be that buildx may not be using this ValidateEnv function

// ValidateEnv validates an environment variable and returns it.
// If no value is specified, it obtains its value from the current environment
//
// As on ParseEnvFile and related to #16585, environment variable names
// are not validated, and it's up to the application inside the container
// to validate them or not.
//
// The only validation here is to check if name is empty, per #25099
func ValidateEnv(val string) (string, error) {
	k, _, hasValue := strings.Cut(val, "=")
	if k == "" {
		return "", errors.New("invalid environment variable: " + val)
	}
	if hasValue {
		// val contains a "=" (but value may be an empty string)
		return val, nil
	}
	if envVal, ok := os.LookupEnv(k); ok {
		return k + "=" + envVal, nil
	}
	return val, nil
}

Let me move this to the buildx repo, but perhaps we need to look if that function should be moved elsewhere to make it easier to use.