feat: Allow providing agent user credentials as a secrets manager secret#187
Conversation
Signed-off-by: Church Hindley <59745380+hindleym@users.noreply.github.com>
Signed-off-by: Church Hindley <59745380+hindleym@users.noreply.github.com>
jusiskin
left a comment
jusiskin
left a comment
There was a problem hiding this comment.
Just one suggestion to improve the secret handling.
|
|
||
| password = None | ||
| if config.windows_user_secret: | ||
| user_secret = self.get_windows_user_secret(secret_id=config.windows_user_secret) |
There was a problem hiding this comment.
Right now, the test runner is the one fetching the value of the secret, but then putting the secret in the contents of the SSM run command.
To avoid sending the password in the SSM command, we could instead have the worker fetch the secret value and construct the argument to --password. It would require granting the instance profile permissions to retrieve the secret value (secretsmanager:GetSecretValue).
What do you think?
|
One more suggestion is that the PR title and commit titles all have the I know in the worker agent repository we use that prefix when changing tests, but in this repository, the changes are functional since this package is providing functionality for building tests for Deadline Cloud. In the case of this PR, I think we want to switch to something like: What do you think? |
Signed-off-by: Church Hindley <59745380+hindleym@users.noreply.github.com>
hindleym
changed the title
hindleym
changed the title
…configure worker command Signed-off-by: Church Hindley <59745380+hindleym@users.noreply.github.com>
|
jusiskin
left a comment
jusiskin
left a comment
There was a problem hiding this comment.
LGTM. Just a small suggestion to make this new method a private API so that consumers of deadline-cloud-test-fixtures don't depend on it directly.
This is not critical to merge, but it helps keep this an implementation detail and not something we need to maintain.
leongdl
left a comment
leongdl
left a comment
There was a problem hiding this comment.
Please add _ as requested by Josh.
3 participants
What was the problem/requirement? (What/Why)
What was the solution? (How)
What is the impact of this change?
How was this change tested?
Was this change documented?
Is this a breaking change?
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.