Skip to content

Commit

Permalink
fix: Removing overridden AWS_CONFIG_FILE path and base environment va…
Browse files Browse the repository at this point in the history
…riables from deadline_vfs POpen launch env and using -E option to persist environment through sudo (#247)

Signed-off-by: Brian Axelson <baxelson@amazon.com>
  • Loading branch information
baxeaz authored Mar 26, 2024
1 parent 88dd57e commit 4a7be81
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 6 deletions.
6 changes: 2 additions & 4 deletions src/deadline/job_attachments/vfs.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ def build_launch_command(self, mount_point: Union[os.PathLike, str]) -> str:
executable = VFSProcessManager.find_vfs_launch_script()

command = (
f"sudo -u {self._os_user}"
f"sudo -E -u {self._os_user}"
f" {executable} {mount_point} -f --clienttype=deadline"
f" --bucket={self._asset_bucket}"
f" --manifest={self._manifest_path}"
Expand Down Expand Up @@ -403,12 +403,10 @@ def get_launch_environ(self) -> dict:
Get the environment variables we'll pass to the launch command.
:returns: dictionary of default environment variables with VFS changes applied
"""
my_env = {**os.environ, **self._os_env_vars}
my_env = {**self._os_env_vars}
my_env["PATH"] = f"{VFSProcessManager.find_vfs_link_dir()}{os.pathsep}{os.environ['PATH']}"
my_env["LD_LIBRARY_PATH"] = VFSProcessManager.get_library_path() # type: ignore[assignment]

my_env["AWS_CONFIG_FILE"] = str(Path(f"~{self._os_user}/.aws/config").expanduser())

return my_env

def set_manifest_owner(self) -> None:
Expand Down
44 changes: 42 additions & 2 deletions test/unit/deadline_job_attachments/test_vfs.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def test_build_launch_command(
test_executable = os.environ[DEADLINE_VFS_ENV_VAR] + DEADLINE_VFS_EXECUTABLE_SCRIPT

expected_launch_command = (
f"sudo -u {test_os_user}"
f"sudo -E -u {test_os_user}"
f" {test_executable} {local_root} -f --clienttype=deadline"
f" --bucket={self.s3_settings.s3BucketName}"
f" --manifest={manifest_path}"
Expand Down Expand Up @@ -132,7 +132,7 @@ def test_build_launch_command(
VFSProcessManager.launch_script_path = None

expected_launch_command = (
f"sudo -u {test_os_user}"
f"sudo -E -u {test_os_user}"
f" {test_executable} {local_root} -f --clienttype=deadline"
f" --bucket={self.s3_settings.s3BucketName}"
f" --manifest={manifest_path}"
Expand Down Expand Up @@ -762,3 +762,43 @@ def test_manifest_group_set(
mock_chown.assert_called_with(manifest_path, group=test_os_group)

mock_chmod.assert_called_with(manifest_path, DEADLINE_MANIFEST_GROUP_READ_PERMS)

def test_launch_environment_has_expected_settings(
self,
tmp_path: Path,
):
# Test to verify when retrieving the launch environment it does not contain os.environ variables (Unless passed in),
# it DOES contain the VFSProcessManager's environment variables, and aws configuration variables aren't modified
session_dir: str = str(tmp_path)
test_mount: str = f"{session_dir}/test_mount"
manifest_path1: str = f"{session_dir}/manifests/some_manifest.json"
os.environ[DEADLINE_VFS_ENV_VAR] = str((Path(__file__) / "deadline_vfs").resolve())

provided_vars = {
"VFS_ENV_VAR": "test-vfs-env-var",
"AWS_PROFILE": "test-profile",
"AWS_CONFIG_FILE": "test-config",
"AWS_SHARED_CREDENTIALS_FILE": "test-credentials",
}
# Create process managers
process_manager: VFSProcessManager = VFSProcessManager(
asset_bucket=self.s3_settings.s3BucketName,
region=os.environ["AWS_DEFAULT_REGION"],
manifest_path=manifest_path1,
mount_point=test_mount,
os_user="test-user",
os_env_vars=provided_vars,
)

# Provided environment variables are passed through
with patch(
f"{deadline.__package__}.job_attachments.vfs.VFSProcessManager.find_vfs",
return_value="/test/directory/path",
):
launch_env = process_manager.get_launch_environ()

for key, value in provided_vars.items():
assert launch_env.get(key) == value

# Base environment variables are not passed through
assert not launch_env.get(DEADLINE_VFS_ENV_VAR)

0 comments on commit 4a7be81

Please sign in to comment.