Added env-file flag to docker exec by BrianWieder · Pull Request #2602 · docker/cli

BrianWieder · 2020-06-26T03:39:48Z

Signed-off-by: Brian Wieder [email protected]

This is my first PR for an open source project and I am a college student starting my journey learning GoLang, so I am open to any and all feedback regarding my code, testing, PR, or anything else!

- What I did
Added a flag to bring in environment variables from a file into docker exec.
Closes #1681

- How I did it
Added a flag to the exec command, and if the value is not the empty string, use the existing logic to parse an env file to add them to the slice that contains the environment variables.

- How to verify it
Run docker exec with --env-file flag with a file containing environment variables and print/use any of the variables inside the file in the command.

Ex:
docker exec --env-file .env 123abc sh -c 'echo $ONE`

where .env comprises of:
ONE=1

- Description for the changelog

Added flag in the docker exec command for parsing a file for environment variables and adding them into the environment for the exec.

- A picture of a cute animal (not mandatory but encouraged)

codecov-commenter · 2020-06-26T03:47:07Z

Codecov Report

Merging #2602 into master will increase coverage by 0.00%.
The diff coverage is 91.30%.

@@           Coverage Diff           @@
##           master    #2602   +/-   ##
=======================================
  Coverage   58.16%   58.17%           
=======================================
  Files         295      295           
  Lines       21174    21182    +8     
=======================================
+ Hits        12316    12322    +6     
- Misses       7955     7956    +1     
- Partials      903      904    +1

thaJeztah

Thanks! I left comments inline. It became quite some comments, so I also pushed it to a branch if that helps; https://github.com/docker/cli/compare/master..thaJeztah:changes_for_2602?expand=1

Let me know if you want me to open a pull-request against your branch

BrianWieder · 2020-06-28T23:59:32Z

@thaJeztah Thank you for the comments and branch with your feedback, I really appreciate them!

I made the changes myself on my branch to reinforce the feedback. I am new to open source, so I apologize if I should've taken a PR against my branch with your changes instead.

I would be happy to make any changes that I may have missed or any further suggestions you may have!

thaJeztah

Thanks!

I made the changes myself on my branch to reinforce the feedback. I am new to open source, so I apologize if I should've taken a PR against my branch with your changes instead.

That's perfect; no need to pull my changes in as a separate commit 👍

I left some more comments on your PR. Also, perhaps you could update the bash and zsh completion scripts to add the new flag?

For zsh:

Add this line below the -e=,--env= line;

cli/contrib/completion/zsh/_docker

Lines 752 to 753 in 87db438

    
           "($help)*"{-e=,--env=}"[Set environment variables]:environment variable: " \ 
        
           "($help -i --interactive)"{-i,--interactive}"[Keep stdin open even if not attached]" \

diff --git a/contrib/completion/zsh/_docker b/contrib/completion/zsh/_docker
index 835c8d459..c11342cee 100644
--- a/contrib/completion/zsh/_docker
+++ b/contrib/completion/zsh/_docker
@@ -750,6 +750,7 @@ __docker_container_subcommand() {
                 $opts_attach_exec_run_start \
                 "($help -d --detach)"{-d,--detach}"[Detached mode: leave the container running in the background]" \
                 "($help)*"{-e=,--env=}"[Set environment variables]:environment variable: " \
+                "($help)*--env-file=[Read environment variables from a file]:environment file:_files" \
                 "($help -i --interactive)"{-i,--interactive}"[Keep stdin open even if not attached]" \
                 "($help)--privileged[Give extended Linux capabilities to the command]" \
                 "($help -t --tty)"{-t,--tty}"[Allocate a pseudo-tty]" \

For bash:

Add this "case" to the _docker_container_exec function

		--env-file)
			_filedir
			return
			;;

Update this line to add the env-file option;

cli/contrib/completion/bash/docker

Line 1618 in 87db438

COMPREPLY=( $( compgen -W "--detach -d --detach-keys --env -e --help --interactive -i --privileged -t --tty -u --user --workdir -w" -- "$cur" ) )

			COMPREPLY=( $( compgen -W "--detach -d --detach-keys --env -e --env-file --help --interactive -i --privileged -t --tty -u --user --workdir -w" -- "$cur" ) )

Diff:

diff --git a/contrib/completion/bash/docker b/contrib/completion/bash/docker
index 6a5298319..f86263ec7 100644
--- a/contrib/completion/bash/docker
+++ b/contrib/completion/bash/docker
@@ -1604,6 +1604,10 @@ _docker_container_exec() {
                        __docker_nospace
                        return
                        ;;
+               --env-file)
+                       _filedir
+                       return
+                       ;;
                --user|-u)
                        __docker_complete_user_group
                        return
@@ -1615,7 +1619,7 @@ _docker_container_exec() {

        case "$cur" in
                -*)
-                       COMPREPLY=( $( compgen -W "--detach -d --detach-keys --env -e --help --interactive -i --privileged -t --tty -u --user --workdir -w" -- "$cur" ) )
+                       COMPREPLY=( $( compgen -W "--detach -d --detach-keys --env -e --env-file --help --interactive -i --privileged -t --tty -u --user --workdir -w" -- "$cur" ) )
                        ;;
                *)
                        __docker_complete_containers_running

Signed-off-by: Brian Wieder <[email protected]>

BrianWieder · 2020-06-29T22:35:10Z

@thaJeztah Thanks for the comments! Made some more changes according to your feedback.

albers

Completion LGTM, thanks!

thaJeztah

LGTM, thank you!

silvin-lubecki

LGTM, thank you @BrianWieder for this first PR 👏

BrianWieder requested a review from thaJeztah as a code owner June 26, 2020 03:39

thaJeztah added impact/changelog status/2-code-review labels Jun 26, 2020

thaJeztah mentioned this pull request Jun 26, 2020

ReadKVEnvStrings/ReadKVStrings return nil if empty, and add tests #2603

Merged

thaJeztah requested changes Jun 26, 2020

View reviewed changes

BrianWieder force-pushed the 1681-docker-exec-env-file branch 3 times, most recently from 27d2820 to a7223af Compare June 28, 2020 23:57

thaJeztah requested changes Jun 29, 2020

View reviewed changes

Comment thread cli/command/container/exec.go Outdated

Comment thread docs/reference/commandline/exec.md Outdated

Comment thread cli/command/container/exec_test.go Outdated

Comment thread cli/command/container/exec_test.go Outdated

Comment thread cli/command/container/exec_test.go Outdated

Added env-file flag to docker exec

a6cfbd2

Signed-off-by: Brian Wieder <[email protected]>

BrianWieder force-pushed the 1681-docker-exec-env-file branch from a7223af to a6cfbd2 Compare June 29, 2020 22:32

BrianWieder requested a review from albers as a code owner June 29, 2020 22:32

albers approved these changes Jun 30, 2020

View reviewed changes

thaJeztah approved these changes Jun 30, 2020

View reviewed changes

thaJeztah added the docs/revisit label Jun 30, 2020

silvin-lubecki approved these changes Jun 30, 2020

View reviewed changes

silvin-lubecki merged commit 1c6dd42 into docker:master Jun 30, 2020

thaJeztah added this to the 20.03.0 milestone Jun 30, 2020

coderabbitai Bot mentioned this pull request Jul 4, 2026

tx9 Go CLI: full v1 command surface on the Docker agent/executor runtime davis7dotsh/tx9#13

Merged

	"($help)*"{-e=,--env=}"[Set environment variables]:environment variable: " \
	"($help -i --interactive)"{-i,--interactive}"[Keep stdin open even if not attached]" \

Uh oh!

Conversation

BrianWieder commented Jun 26, 2020

Uh oh!

codecov-commenter commented Jun 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BrianWieder commented Jun 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BrianWieder commented Jun 29, 2020

Uh oh!

albers left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

silvin-lubecki left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-commenter commented Jun 26, 2020 •

edited

Loading

BrianWieder commented Jun 28, 2020 •

edited

Loading