Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propagate env vars $PATH and $HOME for input_type: external #864

Merged

Conversation

simu
Copy link
Contributor

@simu simu commented Oct 11, 2022

Summary

For compile objects with input_type: external, the external command is executed with subprocess.run() with keyword parameter env. By providing this keyword parameter, the command will not inherit Kapitan's os.environ, as documented in
https://docs.python.org/3/library/subprocess.html:

If env is not None, it must be a mapping that defines the environment variables for the new process; these are used instead of the default behavior of inheriting the current process’ environment. It is passed directly to Popen.

This commit ensures that we pass $PATH and $HOME from Kapitan's os.environ to the external command unless those environment variables were explicitly set in the compile object's env_vars.

Proposed Changes

  • Propagate environment variables $PATH and $HOME to external command for input_type: external unless explicitly set in the compile object's env_vars

@Moep90
Copy link
Contributor

Moep90 commented Oct 22, 2022

@simu would you please mind to change the docs as well?
See: https://github.com/kapicorp/kapitan/blob/master/docs/compile.md#external-alphaexperimental

@ramaro
Copy link
Member

ramaro commented Oct 24, 2022

@simu agree with @Moep90 ! Once we get the doc updated, this is good to merge 👍🏼

For compile objects with `input_type: external`, the external command is
executed with `subprocess.run()` with keyword parameter `env`. By
providing this keyword parameter, the command will not inherit Kapitan's
`os.environ`, as documented in
https://docs.python.org/3/library/subprocess.html:

> If env is not None, it must be a mapping that defines the environment
> variables for the new process; these are used instead of the default
> behavior of inheriting the current process’ environment. It is passed
> directly to Popen.

This commit ensures that we pass `$PATH` and `$HOME` from Kapitan's
`os.environ` to the external command unless those environment variables
were explicitly set in the compile object's `env_vars`.
@simu simu force-pushed the feat/input-type-external-propagate-path-home branch from 3c56def to fd1b073 Compare October 25, 2022 07:50
@ramaro
Copy link
Member

ramaro commented Oct 25, 2022

Perfect, thank you @simu ! Merging.

@ramaro ramaro merged commit aa2acf9 into kapicorp:master Oct 25, 2022
@simu simu deleted the feat/input-type-external-propagate-path-home branch October 25, 2022 10:37
simu added a commit to projectsyn/commodore that referenced this pull request Jan 27, 2023
This is necessary to work around a Kapitan bug we introduced in
kapicorp/kapitan#864. I've submitted a fix
upstream: kapicorp/kapitan#931.
simu added a commit to projectsyn/commodore that referenced this pull request Jan 27, 2023
This is necessary to work around a Kapitan bug we introduced in
kapicorp/kapitan#864. I've submitted a fix
upstream: kapicorp/kapitan#931.
simu added a commit to projectsyn/commodore that referenced this pull request Jan 27, 2023
This is necessary to work around a Kapitan bug we introduced in
kapicorp/kapitan#864. I've submitted a fix
upstream: kapicorp/kapitan#931.
simu added a commit to projectsyn/commodore that referenced this pull request Jan 27, 2023
This is necessary to work around a Kapitan bug we introduced in
kapicorp/kapitan#864. I've submitted a fix
upstream: kapicorp/kapitan#931.
simu added a commit to projectsyn/commodore that referenced this pull request Mar 1, 2023
This is necessary to work around a Kapitan bug we introduced in
kapicorp/kapitan#864. I've submitted a fix
upstream: kapicorp/kapitan#931.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants