Skip to content

Commit

Permalink
chore(api)!: remove old jobsRunAs (#44)
Browse files Browse the repository at this point in the history
* chore(api)!: remove old jobsRunAs

Signed-off-by: Morgan Epp <60796713+epmog@users.noreply.github.com>

* chore: Make JobRunAsUser required

Signed-off-by: Evan Spearman <evans@amazon.com>

* test: add missing required fields to input validation tests

Signed-off-by: Josh Usiskin <56369778+jusiskin@users.noreply.github.com>

---------

Signed-off-by: Morgan Epp <60796713+epmog@users.noreply.github.com>
Signed-off-by: Evan Spearman <evans@amazon.com>
Signed-off-by: Josh Usiskin <56369778+jusiskin@users.noreply.github.com>
Co-authored-by: Evan Spearman <evans@amazon.com>
Co-authored-by: Josh Usiskin <56369778+jusiskin@users.noreply.github.com>
  • Loading branch information
3 people authored Jan 4, 2024
1 parent f2b3893 commit fab7d53
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 99 deletions.
6 changes: 1 addition & 5 deletions src/deadline_worker_agent/api_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,11 +249,7 @@ class JobDetailsData(JobDetailsIdentifierFields):
jobAttachmentSettings: NotRequired[JobAttachmentQueueSettings]
"""The queue's job attachment settings"""

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

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

logGroupName: str
Expand Down
7 changes: 0 additions & 7 deletions src/deadline_worker_agent/boto/shim.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,13 +254,6 @@ 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",
Expand Down
25 changes: 8 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 @@ -85,21 +85,15 @@ def path_mapping_api_model_to_openjd(


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

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 @@ -148,15 +142,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 @@ -189,11 +183,8 @@ 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)

# Use jobRunAsUser if it exists, otherwise jobsRunAs if it exists, otherwise None.
job_run_as_user_data = job_details_data.get(
"jobRunAsUser", job_details_data.get("jobsRunAs", None)
)
job_run_as_user: JobRunAsUser | None = job_run_as_user_api_model_to_worker_agent(
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_data
)

Expand Down Expand Up @@ -260,7 +251,7 @@ def validate_entity_data(cls, entity_data: dict[str, Any]) -> JobDetailsData:
Field(
key="jobRunAsUser",
expected_type=dict,
required=False,
required=True,
fields=(
Field(
key="posix",
Expand Down
2 changes: 1 addition & 1 deletion test/unit/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ def job_details(
queue_job_attachment_settings: JobAttachmentSettings,
job_parameters: list[Parameter],
log_group_name: str,
job_run_as_user: JobRunAsUser | None,
job_run_as_user: JobRunAsUser,
path_mapping_rules: list[PathMappingRule],
schema_version: SchemaVersion,
) -> JobDetails:
Expand Down
30 changes: 30 additions & 0 deletions test/unit/sessions/job_entities/test_job_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@
"jobId": "job-0000",
"logGroupName": "/aws/deadline/queue-0000",
"schemaVersion": "jobtemplate-0000-00",
"jobRunAsUser": {
"posix": {
"user": "user1",
"group": "group1",
},
},
},
id="only required fields",
),
Expand All @@ -30,6 +36,12 @@
"path": "param2value",
},
},
"jobRunAsUser": {
"posix": {
"user": "user1",
"group": "group1",
},
},
},
id="valid parameters",
),
Expand All @@ -39,6 +51,12 @@
"logGroupName": "/aws/deadline/queue-0000",
"schemaVersion": "jobtemplate-0000-00",
"pathMappingRules": [],
"jobRunAsUser": {
"posix": {
"user": "user1",
"group": "group1",
},
},
},
id="valid pathMappingRules - empty list",
),
Expand All @@ -54,6 +72,12 @@
"destinationPath": "/destination/path",
},
],
"jobRunAsUser": {
"posix": {
"user": "user1",
"group": "group1",
},
},
},
id="valid pathMappingRules",
),
Expand Down Expand Up @@ -89,6 +113,12 @@
"s3BucketName": "mybucket",
"rootPrefix": "myprefix",
},
"jobRunAsUser": {
"posix": {
"user": "user1",
"group": "group1",
},
},
},
id="valid jobAttachmentSettings",
),
Expand Down
99 changes: 30 additions & 69 deletions test/unit/sessions/job_entities/test_job_entities.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
JobDetails as JobDetailsBoto,
JobDetailsData,
JobDetailsIdentifier,
JobRunAsUser,
PathMappingRule,
StepDetails as StepDetailsBoto,
StepDetailsData,
Expand All @@ -45,6 +44,7 @@
JobEntities,
StepDetails,
)
from deadline_worker_agent.sessions.job_entities.job_details import JobRunAsUser
import deadline_worker_agent.sessions.job_entities.job_entities as job_entities_mod


Expand Down Expand Up @@ -146,6 +146,12 @@ def test_has_path_mapping_rules(
"jobId": job_id,
"schemaVersion": "jobtemplate-2023-09",
"logGroupName": "fake-name",
"jobRunAsUser": {
"posix": {
"user": "job-user",
"group": "job-group",
},
},
},
)
response: BatchGetJobEntityResponse = {
Expand Down Expand Up @@ -203,73 +209,6 @@ def test_job_run_as_user(self) -> None:
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(
("job_run_as_user_data"),
(
pytest.param(
{
"posix": {
"user": "",
"group": "",
}
},
id="empty user and group",
),
pytest.param(
{
"posix": {
"user": "job-user",
"group": "",
}
},
id="empty group",
),
pytest.param(
{
"posix": {
"user": "",
"group": "job-group",
}
},
id="empty user",
),
pytest.param({"posix": {}}, id="no user/group entries"),
pytest.param({}, id="no posix"),
),
)
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",
"jobRunAsUser": job_run_as_user_data,
"logGroupName": "TEST",
"schemaVersion": SchemaVersion.v2023_09.value,
}

# WHEN
entity_obj = JobDetails.from_boto(entity_data)

# THEN
assert entity_obj.job_run_as_user is None

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 = {
"jobId": "job-123",
"logGroupName": "TEST",
"schemaVersion": SchemaVersion.v2023_09.value,
}

# WHEN
entity_obj = JobDetails.from_boto(entity_data)

# THEN
assert entity_obj.job_run_as_user is None


class TestDetails:
def test_job_details(self, deadline_client: MagicMock, job_id: str):
Expand All @@ -279,6 +218,12 @@ def test_job_details(self, deadline_client: MagicMock, job_id: str):
"jobId": job_id,
"schemaVersion": "jobtemplate-2023-09",
"logGroupName": "fake-name",
"jobRunAsUser": {
"posix": {
"user": "job-user",
"group": "job-group",
},
},
},
)
response: BatchGetJobEntityResponse = {
Expand All @@ -288,6 +233,9 @@ def test_job_details(self, deadline_client: MagicMock, job_id: str):
expected_details = JobDetails(
schema_version=SchemaVersion("jobtemplate-2023-09"),
log_group_name="fake-name",
job_run_as_user=JobRunAsUser(
posix=PosixSessionUser(user="job-user", group="job-group")
),
)
deadline_client.batch_get_job_entity.return_value = response
job_entities = JobEntities(
Expand All @@ -302,7 +250,14 @@ def test_job_details(self, deadline_client: MagicMock, job_id: str):
details = job_entities.job_details()

# THEN
assert details == expected_details
assert details.log_group_name == expected_details.log_group_name
assert details.schema_version == expected_details.schema_version
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
assert details.parameters == expected_details.parameters
assert details.path_mapping_rules == expected_details.path_mapping_rules
assert details.queue_role_arn == expected_details.queue_role_arn

def test_environment_details(self, deadline_client: MagicMock, job_id: str):
# GIVEN
Expand Down Expand Up @@ -464,6 +419,12 @@ def test_cache_entities(
"jobId": job_id,
"logGroupName": "/aws/service/loggroup",
"schemaVersion": "jobtemplate-2023-09",
"jobRunAsUser": {
"posix": {
"user": "job-user",
"group": "job-group",
},
},
}
expected_environment_details: EnvironmentDetailsData = {
"jobId": job_id,
Expand Down

0 comments on commit fab7d53

Please sign in to comment.