Skip to content

Commit

Permalink
fix: crash if queue has no user defined (#127)
Browse files Browse the repository at this point in the history
The fix in fab7d53
caused a regression whereby the agent would crash if it dequeues a job
from a Queue that has an empty posix user/group defined. The crash was
due to the QueueBoto3Session being passed a PosixSessionUser with empty
user/group -- it expects to be passed None in this case.

When converting the boto3 payload in to a JobDetails object I added back the
case that checks for an empty string user/group, and handle those properly again.

Signed-off-by: Daniel Neilson <53624638+ddneilson@users.noreply.github.com>
  • Loading branch information
ddneilson authored Jan 5, 2024
1 parent 844b5e9 commit 3b62715
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 6 deletions.
15 changes: 10 additions & 5 deletions src/deadline_worker_agent/sessions/job_entities/job_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,16 @@ def path_mapping_api_model_to_openjd(

def job_run_as_user_api_model_to_worker_agent(
job_run_as_user_data: JobRunAsUserModel,
) -> JobRunAsUser:
) -> JobRunAsUser | None:
"""Converts the 'JobRunAsUser' api model to the 'JobRunAsUser' dataclass
expected by the Worker Agent.
"""
if os.name == "posix":
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

job_run_as_user = JobRunAsUser(
posix=PosixSessionUser(user=user, group=group),
Expand Down Expand Up @@ -128,6 +130,9 @@ class JobRunAsUser:
posix: PosixSessionUser
# TODO: windows support

def __eq__(self, other: Any) -> bool:
return self.posix.user == other.posix.user and self.posix.group == other.posix.group


@dataclass(frozen=True)
class JobDetails:
Expand All @@ -142,15 +147,15 @@ class JobDetails:
schema_version: SchemaVersion
"""The Open Job Description schema version"""

job_run_as_user: JobRunAsUser
"""The user associated with the job's Amazon Deadline Cloud queue"""

job_attachment_settings: JobAttachmentSettings | None = None
"""The job attachment settings of the job's queue"""

parameters: list[Parameter] = field(default_factory=list)
"""The job's parameters"""

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)
"""The path mapping rules for the job"""

Expand Down Expand Up @@ -184,7 +189,7 @@ def from_boto(cls, job_details_data: JobDetailsData) -> JobDetails:
job_attachment_settings = JobAttachmentSettings.from_boto(job_attachment_settings_boto)

job_run_as_user_data = job_details_data["jobRunAsUser"]
job_run_as_user: JobRunAsUser = job_run_as_user_api_model_to_worker_agent(
job_run_as_user: JobRunAsUser | None = job_run_as_user_api_model_to_worker_agent(
job_run_as_user_data
)

Expand Down
68 changes: 67 additions & 1 deletion test/unit/sessions/job_entities/test_job_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
from typing import Any
import pytest

from deadline_worker_agent.sessions.job_entities.job_details import JobDetails
from deadline_worker_agent.sessions.job_entities.job_details import JobDetails, JobRunAsUser
from deadline_worker_agent.api_models import JobDetailsData
from openjd.model import SchemaVersion
from openjd.sessions import PosixSessionUser


@pytest.mark.parametrize(
Expand All @@ -23,6 +26,20 @@
},
id="only required fields",
),
pytest.param(
{
"jobId": "job-0000",
"logGroupName": "/aws/deadline/queue-0000",
"schemaVersion": "jobtemplate-0000-00",
"jobRunAsUser": {
"posix": {
"user": "",
"group": "",
},
},
},
id="only required fields, empty user",
),
pytest.param(
{
"jobId": "job-0000",
Expand Down Expand Up @@ -164,6 +181,55 @@ def test_input_validation_success(data: dict[str, Any]) -> None:
JobDetails.validate_entity_data(entity_data=data)


@pytest.mark.parametrize(
"data, expected",
[
pytest.param(
{
"jobId": "job-0000",
"logGroupName": "/aws/deadline/queue-0000",
"schemaVersion": "jobtemplate-2023-09",
"jobRunAsUser": {
"posix": {
"user": "user1",
"group": "group1",
},
},
},
JobDetails(
log_group_name="/aws/deadline/queue-0000",
schema_version=SchemaVersion.v2023_09,
job_run_as_user=JobRunAsUser(posix=PosixSessionUser(user="user1", group="group1")),
),
id="only required fields",
),
pytest.param(
{
"jobId": "job-0000",
"logGroupName": "/aws/deadline/queue-0000",
"schemaVersion": "jobtemplate-2023-09",
"jobRunAsUser": {
"posix": {
"user": "",
"group": "",
},
},
},
JobDetails(
log_group_name="/aws/deadline/queue-0000",
schema_version=SchemaVersion.v2023_09,
),
id="required with empty user/group",
),
],
)
def test_convert_job_user_from_boto(data: JobDetailsData, expected: JobDetails) -> None:
# WHEN
job_details = JobDetails.from_boto(data)
# THEN
assert job_details == expected


@pytest.mark.parametrize(
"data",
[
Expand Down
2 changes: 2 additions & 0 deletions test/unit/sessions/job_entities/test_job_entities.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ def test_job_details(self, deadline_client: MagicMock, job_id: str):
posix=PosixSessionUser(user="job-user", group="job-group")
),
)
assert expected_details.job_run_as_user is not None # For type checker
deadline_client.batch_get_job_entity.return_value = response
job_entities = JobEntities(
farm_id="farm-id",
Expand All @@ -252,6 +253,7 @@ def test_job_details(self, deadline_client: MagicMock, job_id: str):
# THEN
assert details.log_group_name == expected_details.log_group_name
assert details.schema_version == expected_details.schema_version
assert details.job_run_as_user is not None
assert details.job_run_as_user.posix.user == expected_details.job_run_as_user.posix.user
assert details.job_run_as_user.posix.group == expected_details.job_run_as_user.posix.group
assert details.job_attachment_settings == expected_details.job_attachment_settings
Expand Down

0 comments on commit 3b62715

Please sign in to comment.