Skip to content

Commit

Permalink
feat!: rename impersonation options to use run-as nomenclature
Browse files Browse the repository at this point in the history
Summary:
 The Worker Agent refers to the feature of running a job/session as a
user separate from the os-user that under which the agent is running as
the impersonation feature. This 'impersonation' nomenclature, though
accurate as to the mechanics that are implemented, is not consistent
with the naming used in the service. We wish to make the nomenclature
consistent between the agent & service to reduce the probability of
confusion.

Solution:
 We change:
1. Configuration file's "[os] impersonation" option to "[os] jobs_run_as_agent_user".
   Impersonation had a default of true whereas the new option has a default of true;
   this is equivalent functionality.
   a. (BREAKING CHANGE) Providing the "impersonation" option in a
      configuration file is now an error.
2. (BREAKIN CHANGE) The environment variable to set whether
   "impersonation" is used was renamed from DEADLINE_WORKER_IMPERSONATION to
   DEADLINE_WORKER_JOBS_RUN_AS_AGENT_USER, and its value is inverted.
   "true" now means that we runs jobs as the agent user whereas it meant
   to run jobs as the Queue's jobsRunAsUser before.
3. The --jobs-run-as-agent-user CLI option was added, and the
   --no-impersonation option has been deprecated to be removed in a
   future release. The two options are mutually exclusive.

Signed-off-by: Daniel Neilson <53624638+ddneilson@users.noreply.github.com>
  • Loading branch information
ddneilson committed Nov 10, 2023
1 parent bf99c21 commit 8ae1752
Show file tree
Hide file tree
Showing 22 changed files with 190 additions and 128 deletions.
2 changes: 1 addition & 1 deletion scripts/submit_jobs/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@
3. `hatch shell`
4. `source .deployed_resources.sh`
5. `cd scripts/submit_jobs`
6. `deadline bundle submit --farm-id $FARM_ID --queue-id $QUEUE_ID --submit-as-json --yes -p DataDir=asset_example asset_example`
6. `deadline bundle submit --farm-id $FARM_ID --queue-id $QUEUE_ID --yes -p DataDir=asset_example asset_example`
4 changes: 3 additions & 1 deletion src/deadline_worker_agent/aws_credentials/aws_configs.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ def __init__(self, os_user: Optional[SessionUser]) -> None:
super().__init__()

if os_user is not None and not isinstance(os_user, PosixSessionUser):
raise NotImplementedError("Only posix user impersonation is currently implemented.")
raise NotImplementedError(
"jobsRunAsUser is currently only implemented for posix systems."
)

self._config_path = self._get_path(os_user=os_user.user if os_user is not None else "")
self._config_parser = ConfigParser()
Expand Down
23 changes: 12 additions & 11 deletions src/deadline_worker_agent/installer/worker.toml.example
Original file line number Diff line number Diff line change
Expand Up @@ -148,24 +148,25 @@

[os]

# Amazon Deadline Cloud may specify an OS user to impersonate when running session actions. By setting
# "impersonation" to false, the OS user is ignored and the session actions are run as the same user
# that the worker agent process runs as.
# Amazon Deadline Cloud may specify an OS user to run a Job's session actions as. By setting
# "jobs_run_as_agent_user" to true, the specified OS user is ignored and the session actions
# are instead run as the same user that the worker agent process runs as.
#
# This value is overridden when the DEADLINE_WORKER_IMPERSONATION environment variable is set using
# one of the following case-insensitive values:
# This value is overridden when the DEADLINE_WORKER_JOBS_RUN_AS_AGENT_USER environment variable
# is set using one of the following case-insensitive values:
#
# '0', 'off', 'f', 'false', 'n', 'no', '1', 'on', 't', 'true', 'y', 'yes'.
#
# or if the --no-impersonation command-line flag is specified.
# or if the --jobs-run-as-agent-user command-line flag is specified.
#
# To turn off OS user impersonation, uncomment the line below
# Uncomment the line below to run all session actions as the same user as the worker agent:
#
# impersonation = false
# jobs_run_as_agent_user = true

# Amazon Deadline Cloud may specify an OS user to impersonate when running session actions. By setting
# "posix_job_user", this will override the OS user and the session actions will be run as
# this user. Requires Impersonation to be set to true.
# Amazon Deadline Cloud may specify an OS user to run a Job's session actions as. Setting
# "posix_job_user" will override the OS user and the session actions will be run as
# the user given in the value of "posix_job_user" instead. This setting is ignored
# if "jobs_run_as_agent_user" is set to true.
#
# posix_job_user is expressed as a string in the following format:
#
Expand Down
27 changes: 15 additions & 12 deletions src/deadline_worker_agent/scheduler/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
from .log import LOGGER
from .session_cleanup import SessionUserCleanupManager
from .session_queue import SessionActionQueue, SessionActionStatus
from ..startup.config import ImpersonationOverrides
from ..startup.config import JobsRunAsUserOverride
from ..utils import MappingWithCallbacks


Expand Down Expand Up @@ -154,7 +154,7 @@ class WorkerScheduler:
_action_updates_map: dict[str, SessionActionStatus]
_action_completes: list[SessionActionStatus]
_action_update_lock: RLock
_impersonation: ImpersonationOverrides
_jobs_run_as_user_override: JobsRunAsUserOverride
_boto_session: BotoSession
_worker_persistence_dir: Path
_worker_logs_dir: Path | None
Expand All @@ -172,7 +172,7 @@ def __init__(
fleet_id: str,
worker_id: str,
deadline: DeadlineClient,
impersonation: ImpersonationOverrides,
jobs_run_as_user_override: JobsRunAsUserOverride,
boto_session: BotoSession,
cleanup_session_user_processes: bool,
worker_persistence_dir: Path,
Expand Down Expand Up @@ -203,7 +203,7 @@ def __init__(
self._action_completes = []
self._action_updates_map = {}
self._action_update_lock = RLock()
self._impersonation = impersonation
self._jobs_run_as_user_override = jobs_run_as_user_override
self._shutdown_grace = None
self._boto_session = boto_session
self._queue_aws_credentials = dict[str, QueueAwsCredentials]()
Expand Down Expand Up @@ -696,14 +696,17 @@ def _create_new_sessions(
logger.debug(f"[{new_session_id}] Assigned actions")

os_user: Optional[SessionUser] = None
if job_details.job_run_as_user and not self._impersonation.inactive:
if os.name != "posix":
# TODO: windows support
raise NotImplementedError(f"{os.name} is not supported")
os_user = job_details.job_run_as_user.posix

if self._impersonation.posix_job_user is not None:
os_user = self._impersonation.posix_job_user
if not self._jobs_run_as_user_override.run_as_agent:
if (
self._jobs_run_as_user_override.posix_job_user is not None
and os.name == "posix"
):
os_user = self._jobs_run_as_user_override.posix_job_user
elif job_details.job_run_as_user:
if os.name != "posix":
# TODO: windows support
raise NotImplementedError(f"{os.name} is not supported")
os_user = job_details.job_run_as_user.posix

queue_credentials: QueueAwsCredentials | None = None
asset_sync: AssetSync | None = None
Expand Down
20 changes: 16 additions & 4 deletions src/deadline_worker_agent/startup/cli_args.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ class ParsedCommandLineArguments(Namespace):
profile: str | None = None
verbose: bool | None = None
no_shutdown: bool | None = None
impersonation: bool | None = None
# TODO - Remove no_impersonation when removing corresponding CLI option
no_impersonation: bool | None = None
jobs_run_as_agent_user: bool | None = None
posix_job_user: str | None = None
allow_instance_profile: bool | None = None
logs_dir: Path | None = None
Expand Down Expand Up @@ -61,12 +63,22 @@ def get_argument_parser() -> ArgumentParser:
const=True,
default=None,
)
# TODO - Remove the no-impersonation option after a deprecation period.
# Remove the test named test_impersonation_mutual_exclusion at the same time
parser.add_argument(
"--no-impersonation",
help="Does not use OS impersonation to run actions. WARNING: this is insecure - for development use only.",
help="(DEPRECATED: use --jobs-run-as-agent-user instead) If set, then all Jobs' session actions will run as the same user as the agent. WARNING: this is insecure - for development use only.",
action="store_const",
const=False,
dest="impersonation",
const=True,
dest="no_impersonation",
default=None,
)
parser.add_argument(
"--jobs-run-as-agent-user",
help="If set, then all Jobs' session actions will run as the same user as the agent. WARNING: this is insecure - for development use only.",
action="store_const",
const=True,
dest="jobs_run_as_agent_user",
default=None,
)
parser.add_argument(
Expand Down
40 changes: 27 additions & 13 deletions src/deadline_worker_agent/startup/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@


@dataclass(frozen=True)
class ImpersonationOverrides:
inactive: bool
"""True -> Impersonation is inactive. All jobs run as the agent process' user."""
class JobsRunAsUserOverride:
# inactive: bool
run_as_agent: bool
"""True -> All jobs run as the agent process' user."""

posix_job_user: Optional[SessionUser] = None
"""If provided, then all Jobs run by this agent will run as this user."""
"""If provided and run_as_agent is False, then all Jobs run by this agent will run as this user."""


# Default paths for the Worker persistence directory subdirectories.
Expand All @@ -51,7 +52,7 @@ class Configuration:
profile: Optional[str]
verbose: bool
no_shutdown: bool
impersonation: ImpersonationOverrides
jobs_run_as_overrides: JobsRunAsUserOverride
allow_instance_profile: bool
capabilities: Capabilities
"""Whether to use the new Worker Sessions API (UpdateWorkerSchedule)"""
Expand Down Expand Up @@ -79,7 +80,7 @@ class Configuration:
"profile",
"verbose",
"no_shutdown",
"impersonation",
"jobs_run_as_overrides",
"allow_instance_profile",
"capabilities",
"worker_persistence_dir",
Expand Down Expand Up @@ -110,8 +111,14 @@ def __init__(
settings_kwargs["verbose"] = parsed_cli_args.verbose
if parsed_cli_args.no_shutdown is not None:
settings_kwargs["no_shutdown"] = parsed_cli_args.no_shutdown
if parsed_cli_args.impersonation is not None:
settings_kwargs["impersonation"] = parsed_cli_args.impersonation
if parsed_cli_args.jobs_run_as_agent_user is not None:
if parsed_cli_args.no_impersonation is not None:
raise ConfigurationError(
"Only one of --no-impersonation or --jobs-run-as-agent-user may be supplied."
)
settings_kwargs["jobs_run_as_agent_user"] = parsed_cli_args.jobs_run_as_agent_user
elif parsed_cli_args.no_impersonation is not None:
settings_kwargs["jobs_run_as_agent_user"] = parsed_cli_args.no_impersonation
if parsed_cli_args.posix_job_user is not None:
settings_kwargs["posix_job_user"] = parsed_cli_args.posix_job_user
if parsed_cli_args.allow_instance_profile is not None:
Expand All @@ -133,12 +140,14 @@ def __init__(

if settings.posix_job_user is not None:
user, group = self._get_user_and_group_from_posix_job_user(settings.posix_job_user)
self.impersonation = ImpersonationOverrides(
inactive=not settings.impersonation,
self.jobs_run_as_overrides = JobsRunAsUserOverride(
run_as_agent=settings.jobs_run_as_agent_user,
posix_job_user=PosixSessionUser(user=user, group=group),
)
else:
self.impersonation = ImpersonationOverrides(inactive=not settings.impersonation)
self.jobs_run_as_overrides = JobsRunAsUserOverride(
run_as_agent=settings.jobs_run_as_agent_user
)

self.farm_id = settings.farm_id
self.fleet_id = settings.fleet_id
Expand Down Expand Up @@ -175,8 +184,13 @@ def _validate(self) -> None:
if not self.fleet_id:
raise ConfigurationError(f"Fleet ID must be specified, but got {repr(self.fleet_id)})")

if self.impersonation.inactive and self.impersonation.posix_job_user:
raise ConfigurationError("Cannot specify an impersonation user with impersonation off")
if (
self.jobs_run_as_overrides.run_as_agent
and self.jobs_run_as_overrides.posix_job_user is not None
):
raise ConfigurationError(
"Cannot specify a POSIX job user when the option to running Jobs as the agent user is enabled."
)

if self.host_metrics_logging_interval_seconds <= 0:
raise ConfigurationError(
Expand Down
31 changes: 22 additions & 9 deletions src/deadline_worker_agent/startup/config_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from typing import Any, Optional
import sys

from pydantic import BaseModel, BaseSettings, Field
from pydantic import BaseModel, BaseSettings, Field, ValidationError, root_validator

try:
from tomllib import load as load_toml, TOMLDecodeError
Expand Down Expand Up @@ -44,12 +44,20 @@ class LoggingConfigSection(BaseModel):


class OsConfigSection(BaseModel):
impersonation: Optional[bool] = None
jobs_run_as_agent_user: Optional[bool] = None
posix_job_user: Optional[str] = Field(
regex=r"^[a-zA-Z0-9_.][^:]{0,31}:[a-zA-Z0-9_.][^:]{0,31}$"
)
shutdown_on_stop: Optional[bool] = None

@root_validator(pre=True)
def _disallow_impersonation(cls, values: dict[str, Any]) -> dict[str, Any]:
if "impersonation" in values:
raise ValueError(
"The 'impersonation' option has been removed. Please use 'jobs_run_as_agent_user' instead."
)
return values


class ConfigFile(BaseModel):
worker: WorkerConfigSection
Expand All @@ -63,17 +71,22 @@ def load(cls, config_path: Optional[Path] = None) -> ConfigFile:
if not config_path:
config_path = cls.get_config_path()

# File must be open in binary mode for tomli to ensure the file is utf-8
with config_path.open(mode="rb") as fh:
toml_doc = load_toml(fh)

try:
return cls.parse_obj(toml_doc)
# File must be open in binary mode for tomli to ensure the file is utf-8
with config_path.open(mode="rb") as fh:
toml_doc = load_toml(fh)
except TOMLDecodeError as toml_error:
raise ConfigurationError(
f"Configuration file ({config_path}) is not valid TOML: {toml_error}"
) from toml_error

try:
return cls.parse_obj(toml_doc)
except ValidationError as pydantic_error:
raise ConfigurationError(
f"Parsing errors loading configuration file ({config_path}):\n{str(pydantic_error)}"
) from pydantic_error

@classmethod
def get_config_path(cls) -> Path:
try:
Expand Down Expand Up @@ -117,8 +130,8 @@ def as_settings(
] = self.logging.host_metrics_logging_interval_seconds
if self.os.shutdown_on_stop is not None:
output_settings["no_shutdown"] = self.os.shutdown_on_stop
if self.os.impersonation is not None:
output_settings["impersonation"] = self.os.impersonation
if self.os.jobs_run_as_agent_user is not None:
output_settings["jobs_run_as_agent_user"] = self.os.jobs_run_as_agent_user
if self.os.posix_job_user is not None:
output_settings["posix_job_user"] = self.os.posix_job_user
if self.aws.allow_ec2_instance_profile is not None:
Expand Down
2 changes: 1 addition & 1 deletion src/deadline_worker_agent/startup/entrypoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ def filter(self, record: logging.LogRecord) -> bool:
s3_client=s3_client,
logs_client=logs_client,
boto_session=session,
impersonation=config.impersonation,
jobs_run_as_user_override=config.jobs_run_as_overrides,
cleanup_session_user_processes=config.cleanup_session_user_processes,
worker_persistence_dir=config.worker_persistence_dir,
worker_logs_dir=config.worker_logs_dir if config.local_session_logs else None,
Expand Down
8 changes: 4 additions & 4 deletions src/deadline_worker_agent/startup/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ class WorkerSettings(BaseSettings):
Whether to emit more verbose logging
no_shutdown : bool
If true, then the Worker will not shut down when the service tells the worker to stop
impersonation : bool
Whether to use OS user impersonation (e.g. sudo on Linux) when running session actions
jobs_run_as_agent_user : bool
If true, then all jobs run as the same user as the agent.
posix_job_user : str
Which 'user:group' to use instead of the Queue user when turned on.
allow_instance_profile : bool
Expand Down Expand Up @@ -76,7 +76,7 @@ class WorkerSettings(BaseSettings):
profile: Optional[str] = Field(min_length=1, max_length=64, default=None)
verbose: bool = False
no_shutdown: bool = False
impersonation: bool = True
jobs_run_as_agent_user: bool = False
posix_job_user: Optional[str] = Field(
regex=r"^[a-zA-Z0-9_.][^:]{0,31}:[a-zA-Z0-9_.][^:]{0,31}$"
)
Expand All @@ -100,7 +100,7 @@ class Config:
"profile": {"env": "DEADLINE_WORKER_PROFILE"},
"verbose": {"env": "DEADLINE_WORKER_VERBOSE"},
"no_shutdown": {"env": "DEADLINE_WORKER_NO_SHUTDOWN"},
"impersonation": {"env": "DEADLINE_WORKER_IMPERSONATION"},
"jobs_run_as_agent_user": {"env": "DEADLINE_WORKER_JOBS_RUN_AS_AGENT_USER"},
"posix_job_user": {"env": "DEADLINE_WORKER_POSIX_JOB_USER"},
"allow_instance_profile": {"env": "DEADLINE_WORKER_ALLOW_INSTANCE_PROFILE"},
"capabilities": {"env": "DEADLINE_WORKER_CAPABILITIES"},
Expand Down
6 changes: 3 additions & 3 deletions src/deadline_worker_agent/worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from .metrics import HostMetricsLogger
from .scheduler import WorkerScheduler
from .sessions import Session
from .startup.config import ImpersonationOverrides
from .startup.config import JobsRunAsUserOverride
from .aws_credentials import WorkerBoto3Session, AwsCredentialsRefresher

logger = getLogger(__name__)
Expand Down Expand Up @@ -82,7 +82,7 @@ def __init__(
s3_client: boto3.client,
logs_client: boto3.client,
boto_session: WorkerBoto3Session,
impersonation: ImpersonationOverrides,
jobs_run_as_user_override: JobsRunAsUserOverride,
cleanup_session_user_processes: bool,
worker_persistence_dir: Path,
worker_logs_dir: Path | None,
Expand All @@ -101,7 +101,7 @@ def __init__(
farm_id=farm_id,
fleet_id=fleet_id,
worker_id=worker_id,
impersonation=impersonation,
jobs_run_as_user_override=jobs_run_as_user_override,
boto_session=boto_session,
cleanup_session_user_processes=cleanup_session_user_processes,
worker_persistence_dir=worker_persistence_dir,
Expand Down
2 changes: 1 addition & 1 deletion test/unit/aws/deadline/test_create_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def config(
cli_args = ParsedCommandLineArguments()
cli_args.farm_id = farm_id
cli_args.fleet_id = fleet_id
cli_args.impersonation = False
cli_args.jobs_run_as_agent_user = False
cli_args.no_shutdown = True
cli_args.profile = "profilename"
cli_args.verbose = False
Expand Down
2 changes: 1 addition & 1 deletion test/unit/aws/deadline/test_delete_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def config(
cli_args = ParsedCommandLineArguments()
cli_args.farm_id = farm_id
cli_args.fleet_id = fleet_id
cli_args.impersonation = False
cli_args.jobs_run_as_agent_user = False
cli_args.no_shutdown = True
cli_args.profile = "profilename"
cli_args.verbose = False
Expand Down
Loading

0 comments on commit 8ae1752

Please sign in to comment.