Skip to content

Commit

Permalink
chore(api)!: rename jobsRunAs to jobRunAsUser
Browse files Browse the repository at this point in the history
Signed-off-by: Morgan Epp <[email protected]>
  • Loading branch information
epmog committed Sep 25, 2023
1 parent 2b89ad7 commit 15d0a13
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 42 deletions.
8 changes: 6 additions & 2 deletions src/deadline_worker_agent/api_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ class PosixUser(TypedDict):
"""The posix group name associated with session file ownership"""


class JobsRunAs(TypedDict):
class JobRunAsUser(TypedDict):
posix: PosixUser
# TODO: windows support

Expand All @@ -253,7 +253,11 @@ class JobDetailsData(JobDetailsIdentifierFields):
jobAttachmentSettings: NotRequired[JobAttachmentQueueSettings]
"""The queue's job attachment settings"""

jobsRunAs: NotRequired[JobsRunAs | None]
# TODO: remove once service no longer sends this
jobsRunAs: NotRequired[JobRunAsUser | None]
"""The queue's info on how to run the job processes (ie. posix user/group)"""

jobRunAsUser: NotRequired[JobRunAsUser | None]
"""The queue's info on how to run the job processes (ie. posix user/group)"""

logGroupName: str
Expand Down
7 changes: 7 additions & 0 deletions src/deadline_worker_agent/boto/shim.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,14 @@ def batch_get_job_entity(
"rootPrefix": "my-queue",
},
"logGroupName": "/aws/deadline/queue-abc",
# TODO: remove once service no longer sends this
"jobsRunAs": {
"posix": {
"user": "",
"group": "",
}
},
"jobRunAsUser": {
"posix": {
"user": "job-user",
"group": "job-group",
Expand Down
4 changes: 2 additions & 2 deletions src/deadline_worker_agent/scheduler/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -704,11 +704,11 @@ def _create_new_sessions(
logger.debug(f"[{new_session_id}] Assigned actions")

os_user: Optional[SessionUser] = None
if job_details.jobs_run_as and not self._impersonation.inactive:
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.jobs_run_as.posix
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
Expand Down
53 changes: 36 additions & 17 deletions src/deadline_worker_agent/sessions/job_entities/job_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
IntParameter,
JobDetailsData,
JobAttachmentQueueSettings as JobAttachmentSettingsBoto,
JobsRunAs as JobsRunAsModel,
JobRunAsUser as JobRunAsUserModel,
PathMappingRule,
PathParameter,
StringParameter,
Expand Down Expand Up @@ -89,31 +89,31 @@ def path_mapping_api_model_to_openjd(
return rules


def jobs_runs_as_api_model_to_worker_agent(
jobs_run_as_data: JobsRunAsModel | None,
) -> JobsRunAs | None:
"""Converts the 'JobsRunAs' api model to the 'JobsRunAs' dataclass
def job_run_as_user_api_model_to_worker_agent(
job_run_as_user_data: JobRunAsUserModel | None,
) -> JobRunAsUser | None:
"""Converts the 'JobRunAsUser' api model to the 'JobRunAsUser' dataclass
expected by the Worker Agent.
"""
jobs_run_as: JobsRunAs | None = None
if not jobs_run_as_data:
job_run_as_user: JobRunAsUser | None = None
if not job_run_as_user_data:
return None

if os.name == "posix":
jobs_run_as_posix = jobs_run_as_data.get("posix", {})
user = jobs_run_as_posix.get("user", "")
group = jobs_run_as_posix.get("group", "")
job_run_as_user_posix = job_run_as_user_data.get("posix", {})
user = job_run_as_user_posix.get("user", "")
group = job_run_as_user_posix.get("group", "")
if not (user and group):
return None

jobs_run_as = JobsRunAs(
job_run_as_user = JobRunAsUser(
posix=PosixSessionUser(user=user, group=group),
)
else:
# TODO: windows support
raise NotImplementedError(f"{os.name} is not supported")

return jobs_run_as
return job_run_as_user


@dataclass(frozen=True)
Expand All @@ -135,7 +135,7 @@ def from_boto(cls, data: JobAttachmentSettingsBoto) -> JobAttachmentSettings:


@dataclass(frozen=True)
class JobsRunAs:
class JobRunAsUser:
posix: PosixSessionUser
# TODO: windows support

Expand All @@ -159,7 +159,7 @@ class JobDetails:
parameters: list[Parameter] = field(default_factory=list)
"""The job's parameters"""

jobs_run_as: JobsRunAs | None = None
job_run_as_user: JobRunAsUser | None = None
"""The user associated with the job's Amazon Deadline Cloud queue"""

path_mapping_rules: list[OPENJDPathMappingRule] = field(default_factory=list)
Expand Down Expand Up @@ -194,8 +194,10 @@ def from_boto(cls, job_details_data: JobDetailsData) -> JobDetails:
if job_attachment_settings_boto := job_details_data.get("jobAttachmentSettings", None):
job_attachment_settings = JobAttachmentSettings.from_boto(job_attachment_settings_boto)

jobs_run_as_data = job_details_data.get("jobsRunAs", None)
jobs_run_as: JobsRunAs | None = jobs_runs_as_api_model_to_worker_agent(jobs_run_as_data)
job_run_as_user_data = job_details_data.get("jobRunAsUser", None)
job_run_as_user: JobRunAsUser | None = job_run_as_user_api_model_to_worker_agent(
job_run_as_user_data
)

# Note: Record the empty string as a None as well.
queue_role_arn: str | None = (
Expand All @@ -213,7 +215,7 @@ def from_boto(cls, job_details_data: JobDetailsData) -> JobDetails:
parameters=job_parameters,
schema_version=schema_version,
log_group_name=job_details_data["logGroupName"],
jobs_run_as=jobs_run_as,
job_run_as_user=job_run_as_user,
path_mapping_rules=path_mapping_rules,
job_attachment_settings=job_attachment_settings,
queue_role_arn=queue_role_arn,
Expand Down Expand Up @@ -257,6 +259,7 @@ def validate_entity_data(cls, entity_data: dict[str, Any]) -> JobDetailsData:
expected_type=list,
required=False,
),
# TODO: remove jobsRunAs once service no longer responds with jobsRunAs
Field(
key="jobsRunAs",
expected_type=dict,
Expand All @@ -273,6 +276,22 @@ def validate_entity_data(cls, entity_data: dict[str, Any]) -> JobDetailsData:
),
),
),
Field(
key="jobRunAsUser",
expected_type=dict,
required=False,
fields=(
Field(
key="posix",
expected_type=dict,
required=False,
fields=(
Field(key="user", expected_type=str, required=True),
Field(key="group", expected_type=str, required=True),
),
),
),
),
Field(
key="jobAttachmentSettings",
expected_type=dict,
Expand Down
10 changes: 5 additions & 5 deletions test/unit/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from deadline_worker_agent.sessions.job_entities.job_details import (
JobAttachmentSettings,
JobDetails,
JobsRunAs,
JobRunAsUser,
)
from deadline_worker_agent.sessions.job_entities.job_attachment_details import (
JobAttachmentDetails,
Expand Down Expand Up @@ -238,12 +238,12 @@ def job_parameters() -> list[Parameter]:


@pytest.fixture
def jobs_run_as() -> JobsRunAs | None:
def job_run_as_user() -> JobRunAsUser | None:
"""The OS user/group associated with the job's queue"""
# TODO: windows support
if os.name != "posix":
raise NotImplementedError(f"{os.name} is not supported")
return JobsRunAs(posix=PosixSessionUser(user="job-user", group="job-user"))
return JobRunAsUser(posix=PosixSessionUser(user="job-user", group="job-user"))


@pytest.fixture
Expand All @@ -257,14 +257,14 @@ def job_details(
queue_job_attachment_settings: JobAttachmentSettings,
job_parameters: list[Parameter],
log_group_name: str,
jobs_run_as: JobsRunAs | None,
job_run_as_user: JobRunAsUser | None,
path_mapping_rules: list[PathMappingRule],
schema_version: SchemaVersion,
) -> JobDetails:
return JobDetails(
job_attachment_settings=queue_job_attachment_settings,
parameters=job_parameters,
jobs_run_as=jobs_run_as,
job_run_as_user=job_run_as_user,
path_mapping_rules=path_mapping_rules,
log_group_name=log_group_name,
schema_version=schema_version,
Expand Down
69 changes: 53 additions & 16 deletions test/unit/sessions/test_job_entities.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
JobDetails as JobDetailsBoto,
JobDetailsData,
JobDetailsIdentifier,
JobsRunAs,
JobRunAsUser,
PathMappingRule,
StepDetails as StepDetailsBoto,
StepDetailsData,
Expand Down Expand Up @@ -175,16 +175,16 @@ def test_has_path_mapping_rules(
assert job_details.path_mapping_rules not in (None, [])
assert len(job_details.path_mapping_rules) == len(path_mapping_rules)

def test_jobs_run_as(self) -> None:
"""Ensures that if we receive a jobs_run_as field in the response,
def test_job_run_as_user(self) -> None:
"""Ensures that if we receive a job_run_as_user field in the response,
that the created entity has a (Posix) SessionUser created with the
proper values"""
# GIVEN
expected_user = "job-user"
expected_group = "job-group"
entity_data: JobDetailsData = {
"jobId": "job-123",
"jobsRunAs": {
"jobRunAsUser": {
"posix": {
"user": expected_user,
"group": expected_group,
Expand All @@ -198,13 +198,50 @@ def test_jobs_run_as(self) -> None:
entity_obj = JobDetails.from_boto(entity_data)

# THEN
assert entity_obj.jobs_run_as is not None
assert isinstance(entity_obj.jobs_run_as.posix, PosixSessionUser)
assert entity_obj.jobs_run_as.posix.user == expected_user
assert entity_obj.jobs_run_as.posix.group == expected_group
assert entity_obj.job_run_as_user is not None
assert isinstance(entity_obj.job_run_as_user.posix, PosixSessionUser)
assert entity_obj.job_run_as_user.posix.user == expected_user
assert entity_obj.job_run_as_user.posix.group == expected_group

# TODO: remove once service no longer sends jobs_run_as
def test_old_jobs_run_as_existence(self) -> None:
"""Ensures that if we receive a job_run_as_user field in the response,
that the created entity has a (Posix) SessionUser created with the
proper values"""
# GIVEN
expected_user = "job-user"
expected_group = "job-group"
api_response: dict = {
"jobId": "job-123",
"jobsRunAs": {
"posix": {
"user": "",
"group": "",
},
},
"jobRunAsUser": {
"posix": {
"user": expected_user,
"group": expected_group,
},
},
"logGroupName": "TEST",
"schemaVersion": SchemaVersion.v2023_09.value,
}

# WHEN
job_details_data = JobDetails.validate_entity_data(api_response)
entity_obj = JobDetails.from_boto(job_details_data)

# THEN
assert not hasattr(entity_obj, "jobs_run_as")
assert entity_obj.job_run_as_user is not None
assert isinstance(entity_obj.job_run_as_user.posix, PosixSessionUser)
assert entity_obj.job_run_as_user.posix.user == expected_user
assert entity_obj.job_run_as_user.posix.group == expected_group

@pytest.mark.parametrize(
("jobs_run_as_data"),
("job_run_as_user_data"),
(
pytest.param(
{
Expand Down Expand Up @@ -237,13 +274,13 @@ def test_jobs_run_as(self) -> None:
pytest.param({}, id="no posix"),
),
)
def test_jobs_run_empty_values(self, jobs_run_as_data: JobsRunAs | None) -> None:
"""Ensures that if we are missing values in the jobs_run_as fields
def test_job_run_as_user_empty_values(self, job_run_as_user_data: JobRunAsUser | None) -> None:
"""Ensures that if we are missing values in the job_run_as_user fields
that created entity does not have it set (ie. old queues)"""
# GIVEN
entity_data: JobDetailsData = {
"jobId": "job-123",
"jobsRunAs": jobs_run_as_data,
"jobRunAsUser": job_run_as_user_data,
"logGroupName": "TEST",
"schemaVersion": SchemaVersion.v2023_09.value,
}
Expand All @@ -252,10 +289,10 @@ def test_jobs_run_empty_values(self, jobs_run_as_data: JobsRunAs | None) -> None
entity_obj = JobDetails.from_boto(entity_data)

# THEN
assert entity_obj.jobs_run_as is None
assert entity_obj.job_run_as_user is None

def test_jobs_run_as_not_provided(self) -> None:
"""Ensures that if we somehow don't receive a jobs_run_as field
def test_job_run_as_user_not_provided(self) -> None:
"""Ensures that if we somehow don't receive a job_run_as_user field
that the created entity does not have it set (shouldn't happen)"""
# GIVEN
entity_data: JobDetailsData = {
Expand All @@ -268,7 +305,7 @@ def test_jobs_run_as_not_provided(self) -> None:
entity_obj = JobDetails.from_boto(entity_data)

# THEN
assert entity_obj.jobs_run_as is None
assert entity_obj.job_run_as_user is None


class TestDetails:
Expand Down

0 comments on commit 15d0a13

Please sign in to comment.