Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ignore job-entity run-as value when agent has a job user override #346

Conversation

AWS-Samuel
Copy link
Contributor

@AWS-Samuel AWS-Samuel commented Jul 4, 2024

What was the problem/requirement? (What/Why)

The worker agent was validating the jobRunAsUser field in the BatchGetJobEntity API response, even when the worker agent was configured with a job user override.

This would lead to issues if a queue was configured with only the opposite system type in the jobRunAsUser. For example, a linux worker with the posix_job_user configuration receives a job, which has a jobRunAsUser value of:

{
    "runAs": "QUEUE_CONFIGURED_USER",
    "windows": {
        "user": "windows-job-user",
        "passwordArn": "<password arn>"
    }
}

Then the worker agent would throw an error here because there is no posix runAs field available, even though it shouldn't matter due to the override.

What was the solution? (How)

Ignore the jobRunAsUser field if the worker agent has a job user override

What is the impact of this change?

The erroneous behavior above no longer occurs.

How was this change tested?

Unit tests added.

Tested on a CMF worker with and without this change.

Confirmed that the worker with this change had a successful job run, while the worker without this change encountered the Expected "jobRunAs" -> "posix" to exist when "jobRunAs" -> "runAs" is "QUEUE_CONFIGURED_USER" but it was not present error

Was this change documented?

No

Is this a breaking change?

No


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@AWS-Samuel AWS-Samuel force-pushed the fix_queue_job_run_as_user_validation branch from 4b51fa0 to 50fd3f2 Compare July 5, 2024 19:00
@AWS-Samuel AWS-Samuel marked this pull request as ready for review July 5, 2024 19:01
@AWS-Samuel AWS-Samuel requested a review from a team as a code owner July 5, 2024 19:01
Comment on lines +327 to +331
# WHEN
job_details_data = JobDetails.validate_entity_data(
api_response, job_user_override=job_run_as_user_overrides
)
entity_obj = JobDetails.from_boto(job_details_data)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: shouldn't this test belong in test/unit/sessions/job_entities/test_job_details.py instaed, since we're testing behaviour of the JobDetails class?

Marked as a nit because I see this is not precedent as the test above has the same problem. 😞

Signed-off-by: Samuel Anderson <119458760+AWS-Samuel@users.noreply.github.com>
@AWS-Samuel AWS-Samuel force-pushed the fix_queue_job_run_as_user_validation branch from 50fd3f2 to 325828c Compare July 5, 2024 19:13
@AWS-Samuel AWS-Samuel enabled auto-merge (squash) July 5, 2024 19:15
@AWS-Samuel AWS-Samuel disabled auto-merge July 5, 2024 19:17
@AWS-Samuel AWS-Samuel merged commit c0dd3b3 into aws-deadline:mainline Jul 5, 2024
12 checks passed
AWS-Samuel added a commit to AWS-Samuel/deadline-cloud-worker-agent that referenced this pull request Jul 5, 2024
…a job user override (aws-deadline#346)"

This reverts commit c0dd3b3.

Signed-off-by: Samuel Anderson <119458760+AWS-Samuel@users.noreply.github.com>
@AWS-Samuel AWS-Samuel deleted the fix_queue_job_run_as_user_validation branch August 12, 2024 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants