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.

Sign up for GitHub

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

Jump to bottom

feat!: rename impersonation options to use run-as nomenclature #92

Merged
merged 2 commits into from
Nov 10, 2023
Merged

feat!: rename impersonation options to use run-as nomenclature #92

merged 2 commits into from
Nov 10, 2023

Conversation

ddneilson
Copy link
Contributor

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

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.

What was the solution? (How)

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.

What is the impact of this change?

Clearer association between agent options and the service options.

How was this change tested?

I updated unit tests, and added a new one for new functionality. I also ran the agent in our docker testing container to check that jobs run correctly, and to verify the CLI exclusivity logic:

+ deadline-worker-agent --allow-instance-profile --posix-job-user jobuser:sharedgroup --no-shutdown --jobs-run-as-agent-user --no-impersonation --farm-id farm-4aed3bf9537845b597d899ebe97ed6a5 --fleet-id fleet-55a55f151bc64516a28502fb5e4a58c7
ERROR: Only one of --no-impersonation or --jobs-run-as-agent-user may be supplied.

+ deadline-worker-agent --allow-instance-profile --posix-job-user jobuser:sharedgroup --no-shutdown --jobs-run-as-agent-user --farm-id farm-4aed3bf9537845b597d899ebe97ed6a5 --fleet-id fleet-55a55f151bc64516a28502fb5e4a58c7
ERROR: Cannot specify a POSIX job user when the option to running Jobs as the agent user is enabled.

Was this change documented?

The CLI & configuration files are self-documenting.

Is this a breaking change?

Yes.

@ddneilson ddneilson requested a review from a team as a code owner November 10, 2023 20:30
@ddneilson
feat!: rename impersonation options to use run-as nomenclature
8ae1752 
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>
jusiskin
jusiskin approved these changes Nov 10, 2023
View reviewed changes
Copy link
Contributor

@jusiskin jusiskin left a comment

Choose a reason for hiding this comment

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

Great job navigating this change and providing a helpful migration path. Just a few nits but I'll approve regardless.

src/deadline_worker_agent/startup/config.py Outdated Show resolved Hide resolved
test/unit/aws/deadline/test_update_worker.py Outdated Show resolved Hide resolved
test/unit/aws/deadline/test_delete_worker.py Outdated Show resolved Hide resolved
test/unit/aws/deadline/test_create_worker.py Outdated Show resolved Hide resolved
test/unit/conftest.py Outdated Show resolved Hide resolved
ddneilson
ddneilson commented Nov 10, 2023
View reviewed changes
test/unit/conftest.py Outdated Show resolved Hide resolved
@ddneilson @jusiskin
Apply suggestions from code review
f27ec2c 
Signed-off-by: Daniel Neilson <53624638+ddneilson@users.noreply.github.com>

Co-authored-by: Josh Usiskin <56369778+jusiskin@users.noreply.github.com>
@ddneilson ddneilson merged commit 5165a4d into mainline Nov 10, 2023
9 checks passed
@ddneilson ddneilson deleted the ddneilson/17257 branch November 10, 2023 21:26
gahyusuh pushed a commit that referenced this pull request Nov 17, 2023
@ddneilson @jusiskin @gahyusuh
feat!: rename impersonation options to use run-as nomenclature (#92)
0d5b996 
* feat!: rename impersonation options to use run-as nomenclature

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>

* Apply suggestions from code review

Signed-off-by: Daniel Neilson <53624638+ddneilson@users.noreply.github.com>

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

---------

Signed-off-by: Daniel Neilson <53624638+ddneilson@users.noreply.github.com>
Co-authored-by: Josh Usiskin <56369778+jusiskin@users.noreply.github.com>
@jusiskin jusiskin mentioned this pull request Dec 12, 2023
Merged
gmchale79 pushed a commit that referenced this pull request Feb 12, 2024
@ddneilson @jusiskin @gmchale79
feat!: rename impersonation options to use run-as nomenclature (#92)
ccae501 
* feat!: rename impersonation options to use run-as nomenclature

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>

* Apply suggestions from code review

Signed-off-by: Daniel Neilson <53624638+ddneilson@users.noreply.github.com>

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

---------

Signed-off-by: Daniel Neilson <53624638+ddneilson@users.noreply.github.com>
Co-authored-by: Josh Usiskin <56369778+jusiskin@users.noreply.github.com>
@client-software-ci client-software-ci mentioned this pull request Feb 22, 2024
Closed
gmchale79 pushed a commit that referenced this pull request Mar 11, 2024
@ddneilson @jusiskin @gmchale79
feat!: rename impersonation options to use run-as nomenclature (#92)
bb8dfef 
* feat!: rename impersonation options to use run-as nomenclature

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>

* Apply suggestions from code review

Signed-off-by: Daniel Neilson <53624638+ddneilson@users.noreply.github.com>

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

---------

Signed-off-by: Daniel Neilson <53624638+ddneilson@users.noreply.github.com>
Co-authored-by: Josh Usiskin <56369778+jusiskin@users.noreply.github.com>
Signed-off-by: Graeme McHale <gmchale@amazon.com>
jusiskin pushed a commit to jusiskin/deadline-cloud-worker-agent that referenced this pull request Sep 4, 2024
@client-software-ci
chore(release): 0.6.1 (aws-deadline#92)
d2210bc 
Signed-off-by: client-software-ci <129794699+client-software-ci@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmchale79 gmchale79 gmchale79 approved these changes

@jusiskin jusiskin jusiskin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ddneilson @gmchale79 @jusiskin