runfinch / finch

The Finch CLI is an open source client for container development
https://www.runfinch.com
Apache License 2.0
3.48k stars 87 forks source link

Unable to use the "--env" option on finch container run #827

Open alecthegeek opened 4 months ago

alecthegeek commented 4 months ago

Describe the bug Cannot use --env option

Steps to reproduce

finch container run -it --rm --env="E=v" busybox env

Expected behavior

PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
TERM=xterm
HOME=/root
E=v

Screenshots or logs

Actual result

FATA[0000] unknown shorthand flag: 'e' in -e

Additional context

Note: Updated to Finch 1.1.2 -- error still present

As a test I ran nerdctl (installed via Rancher Desktop) and it worked as expected

$ nerdctl container run -it --rm --env="E=v" busybox env
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
E=v
TERM=xterm
HOME=/root

To help debug the issue as quickly as possible, we recommend generating a support bundle with finch support-bundle generate and attaching it to this issue. This packages all Finch-related configs and logs into one file. finch-support-20240222101339.zip

mharwani commented 4 months ago

Thanks for the issue. The command works properly when you use finch run, but fails with finch container run:

$ finch run -it --rm --env="E=v" busybox env
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
E=v
TERM=xterm
HOME=/root
$ finch container run -it --rm --env="E=v" busybox env
FATA[0000] unknown shorthand flag: 'e' in -e            
FATA[0000] exit status 1

The --debug option shows improper ordering of nerdctl arguments, where -e argument is passed before the run command:

$ finch --debug container run -it --rm --env="E=v" busybox env
DEBU[0000] Creating limactl command: ARGUMENTS: [shell finch sudo nerdctl container -e E=v run -it --rm busybox env], LIMA_HOME: /Applications/Finch/lima/data
...
mharwani commented 4 months ago

In the following code:

limaArgs = append(limaArgs, append([]string{nerdctlCmdName}, strings.Fields(cmdName)...)...)

var finalArgs []string
for key, val := range envVars {
    finalArgs = append(finalArgs, "-e", fmt.Sprintf("%s=%s", key, val))
}
finalArgs = append(finalArgs, nerdctlArgs...)
// Add -E to sudo command in order to preserve existing environment variables, more info:
// https://stackoverflow.com/questions/8633461/how-to-keep-environment-variables-when-using-sudo/8633575#8633575
limaArgs = append(limaArgs, finalArgs...)

The environment variables (such as -e E=v) are appended first, followed by rest of the arguments. For most cases, such as finch run, this should be fine because run is used as the command appended before any arguments. However, for cases like finch container run, run is treated as another argument and appended after the environment variables.

alecthegeek commented 3 months ago

Tested release 1.1.3

The original bug report is now fixed.

However as explained in the PR the following does not work

finch container run -it --rm -e "E=v" busybox echo -e "hello\tbye"