Inconsistent handling of empty env vars

[ndeloof]

Description

docker run -e FOO will remove var FOO set by Docker image if no such var exists in user environment. But FOO (without =) in an env_file passed by --env-file doesn't. Seems this is expected behaviour reading env_file according to https://github.com/docker/cli/blob/master/opts/envfile_test.go#L161-L163

Steps to reproduce the issue:

1. Create Dockerfile :

   FROM alpine
   ENV FOO=BAR

2. build image from if docker build -t bug .
3. execute docker run -e FOO bug env: FOO is removed from container environment
4. Create foo.env text file with single line FOO
5. execute docker run --env_file foo.env bug env

Describe the results you received:

➜  docker run --env-file foo.env  tutu env
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
HOSTNAME=66a4ecf874af
FOO=BAR
HOME=/root

Describe the results you expected:

FOO removed from container environment

➜  docker run --env-file foo.env  tutu env
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
HOSTNAME=66a4ecf874af
HOME=/root

Output of docker version:

Client: Docker Engine - Community
 Version:           19.03.3
 API version:       1.40
 Go version:        go1.12.10
 Git commit:        a872fc2f86
 Built:             Tue Oct  8 01:00:44 2019
 OS/Arch:           linux/amd64
 Experimental:      true

Server: Docker Engine - Community
 Engine:
  Version:          19.03.3
  API version:      1.40 (minimum version 1.12)
  Go version:       go1.12.10
  Git commit:       a872fc2f86
  Built:            Tue Oct  8 00:59:17 2019
  OS/Arch:          linux/amd64
  Experimental:     true
 containerd:
  Version:          1.2.6
  GitCommit:        894b81a4b802e4eb2a91d1ce216b8817763c29fb
 runc:
  Version:          1.0.0-rc8
  GitCommit:        425e105d5a03fabd737a126ad93d62a9eeede87f
 docker-init:
  Version:          0.18.0
  GitCommit:        fec3683

Output of docker info:

Client:
 Debug Mode: false
 Plugins:
  app: Docker App (Docker Inc., v0.9.0-zeta1-230-gc1e55c1bc4-dirty)
  compose: Docker Compose (Docker Inc., 2.0.0-developer-preview-early-prealpha)
  hub: Docker Hub (Docker Inc., 0.0.0-developer-preview-early-pre-alpha)
  buildx: Build with BuildKit (Docker Inc., v0.3.1-tp-docker)

Server:
 Containers: 6
  Running: 0
  Paused: 0
  Stopped: 6
 Images: 259
 Server Version: 19.03.3
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Native Overlay Diff: true
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 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: runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 894b81a4b802e4eb2a91d1ce216b8817763c29fb
 runc version: 425e105d5a03fabd737a126ad93d62a9eeede87f
 init version: fec3683
 Security Options:
  apparmor
  seccomp
   Profile: default
 Kernel Version: 5.3.0-24-generic
 Operating System: Ubuntu 19.10
 OSType: linux
 Architecture: x86_64
 CPUs: 8
 Total Memory: 31.09GiB
 Name: dock
 ID: J5J2:BP5I:DYBW:B77U:TJMW:ZWDI:BO62:UMO4:FCLO:C5BM:XT6M:2EAY
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 Username: ndeloof
 Registry: https://index.docker.io/v1/
 Labels:
 Experimental: true
 Insecure Registries:
  127.0.0.0/8
 Live Restore Enabled: false

WARNING: No swap limit support

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

[ndeloof]

cc @thaJeztah

[ndeloof]

Tested with various CLI versions

behaviour changed between 18.09.0 (dump FOO=BAR, env from image not overridden) and 18.06.3 (dump FOO=, env overridden but not removed :-\ )

[ndeloof]

Change was introduced by https://github.com/docker/cli/commit/96c026eb301f4f9d2cb224d0ee1a2312b7f5b60c cf https://github.com/docker/for-linux/issues/284

Before this, VAR without a value which didn't map to an existing env variable resulted in VAR= added to parsed lines and override in container environment with empty string.

After change, such a VAR is just ignored, still being inconsistent with -e

[ktomk]

At the time of change I was looking into parsing dot env files (--env-files), not specifically env names (-e, --env). From what I remember by the code changes, it was not my understanding that it should have any side-effects on -e, --env as the changes were in the part of parsing env-files.

However there might have been a similar error in -e, --env parsing which got unnoticed that time as I didn't focus on that part and/or I was not aware it would have been useful to keep the import line. I'm pretty sure I didn't think about that.

For your change request: It's perhaps better then to first format the line as string and then append it (so the append operation is not two times within the the two if / else branches) so that it becomes more clear that a line has to be added always - present/set or not (unset).

I was not aware at the time of change that imports should be kept. That I remember for sure.