Add allow-env and block-env option for limactl shell#4483
Add allow-env and block-env option for limactl shell#4483vax-r wants to merge 1 commit intolima-vm:masterfrom
Conversation
| shellCmd.Flags().Bool("reconnect", false, "Reconnect to the SSH session") | ||
| shellCmd.Flags().Bool("preserve-env", false, "Propagate environment variables to the shell") | ||
| shellCmd.Flags().Bool("start", false, "Start the instance if it is not already running") | ||
| shellCmd.Flags().String("allow-env", "", "Comma-separated list of environment variable patterns to allow when --preserve-env is set (overrides LIMA_SHELLENV_ALLOW)") |
There was a problem hiding this comment.
vax-r
force-pushed
the
shell_option
branch
2 times, most recently
from
f662f04 to
124a66f
Compare
Implement allow-env and block-env option for limactl shell command, which is equivalent of setting environment variable "LIMA_SHELLENV_ALLOW" and "LIMA_SHELLENV_BLOCK". Except that "allow-env" and "block-env" are treated as StringSlice rather than pure string in these options. Related issue: lima-vm#4263 Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
|
@AkihiroSuda Btw, the test starting WSL2 seems to fail, I've read the log I don't think it's triggered by this commit change? |
| shellCmd.Flags().Bool("reconnect", false, "Reconnect to the SSH session") | ||
| shellCmd.Flags().Bool("preserve-env", false, "Propagate environment variables to the shell") | ||
| shellCmd.Flags().Bool("start", false, "Start the instance if it is not already running") | ||
| shellCmd.Flags().StringSlice("allow-env", []string{}, "Comma-separated list of environment variable patterns to allow when --preserve-env is set (overrides LIMA_SHELLENV_ALLOW)") |
There was a problem hiding this comment.
#4263 says:
They would (just for this command) append the pattern to the respective lists and automatically enable
--preserve-env.
These options are convenience methods to make it easier to append custom patterns for just a single command (otherwise you would change the env variables if you want the changes to persist for longer). So it makes no sense to require the user to also specify --preserve-env as well.
| shellCmd.Flags().StringSlice("allow-env", []string{}, "Comma-separated list of environment variable patterns to allow when --preserve-env is set (overrides LIMA_SHELLENV_ALLOW)") | |
| shellCmd.Flags().StringSlice("allow-env", []string{}, "Comma-separated list of additional environment variable patterns to allow (implies --preserve-env)") |
| shellCmd.Flags().Bool("preserve-env", false, "Propagate environment variables to the shell") | ||
| shellCmd.Flags().Bool("start", false, "Start the instance if it is not already running") | ||
| shellCmd.Flags().StringSlice("allow-env", []string{}, "Comma-separated list of environment variable patterns to allow when --preserve-env is set (overrides LIMA_SHELLENV_ALLOW)") | ||
| shellCmd.Flags().StringSlice("block-env", []string{}, "Comma-separated list of environment variable patterns to allow when --preserve-env is set (overrides LIMA_SHELLENV_BLOCK)") |
There was a problem hiding this comment.
Same as the line above, but the description here has a cut&pasto: needs to say to block instead of to allow.
| if len(patterns) == 0 { | ||
| blockEnv := os.Getenv("LIMA_SHELLENV_BLOCK") |
There was a problem hiding this comment.
You are replacing the environment variable with the content of the --block-env option. #4263 specifies that the option has to be appended to the env variable list. In addition, it says it has to be appended to the default block list if the env variable is empty.
Same thing applies to the allow list (with the exception that there is no default allow list).
Please re-read the issue, and implement the functionality as specified. Or start a discussion in the issue if you think the requested semantics are wrong!
|
ping @vax-r Could you check the comments from @jandubois ? |
|
@jandubois , @AkihiroSuda - |
Of course; this is open source. You work on it when you have the time. You might miss the 2.1 release, but there will always be a 2.2 after that... 😄 |
|
It is too late now to make it into 2.1 |
3 participants
Summary
Implement allow-env and block-env option for limactl shell command, which is equivalent of setting
environment variable "LIMA_SHELLENV_ALLOW" and
"LIMA_SHELLENV_BLOCK".
Related issue
#4263