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

feat(deadline): configure identity registration settings for deadline clients #576

Conversation

jusiskin
Copy link
Contributor

@jusiskin jusiskin commented Sep 20, 2021

Summary

This change makes RFDK automatically configure Deadline Secrets Management identity registration settings based on the RFDK constructs that are present in a CDK application.

The changes are broken out by construct below:

DeploymentInstance (new)

This construct is an extraction/generalization of the CDK/CloudFormation pattern used by the Repository construct. It deploys an EC2 Auto-Scaling Group with a size of one. The Auto-Scaling Group is configured to require one CloudFormation success signal which is only sent after the user-data completes without error. The user data can optionally (by default enabled) self-terminate the instance by scaling down its own auto-scaling group to zero.

For now, this construct is being kept internal to RFDK and not exported. It was designed with the possibility of making it a public construct in the future.

SecretsManagementIdentityRegistration (new)

The SecretsManagementIdentityRegistration construct is also introduced in this PR. It is responsible for configuring the user data (on a DeploymentInstance) to configure the identity registration settings. It adds IAM permissions and user-data to perform the following steps:

  1. download and install the Deadline Client
  2. configure Deadline to make a direct connection to the Deadline Repository
  3. run a python script that
    1. fetches CIDR ranges for subnets involved
    2. fetches existing identity registration settings
    3. creates/updates/deletse identity registration settings so that they match what's desired

The main API of this construct is the .addSubnetIdentityRegistrationSetting(...) method which a caller can use to specify the subnets of the client, the desired secrets management role and registration status.

RenderQueue (modified)

This PR introduces a new configureSecretsManagementAutoRegistration(...) method to IRenderQueue. When called on the RenderQueue, it will lazily create DeploymentInstance and SecretsManagementIdentityRegistration constructs and delegate the work to the SecretsManagementIdentityRegistration construct. It also accepts a dependent prop which is used to enforce a CloudFormation dependency on a dependent resource of the caller's choosing.

The CloudWatch logs for the DeploymentInstance are sent to the /renderfarm/ConfigureRepository log group with the intention that the behavior of the deployment instance can be augmented in the future.

WorkerInstanceFleet / ConfigureSpotEventPlugin / UsageBasedLicensing (modified)

The RFDK constructs that deploy Deadline Clients were modified to call the configureSecretsManagementAutoRegistration(...) method

IVersion / VersionQuery / ThinkboxDockerRecipes.version (modified)

The IVersion interface and classes that implement it were modified to return the path to the Deadline Client installer location. This was required in order for the SecretsManagementIdentityRegistration construct to be able to install the Deadline Client on instance deployed by the DeploymentInstance.

Testing

Unit tests were updated to reflect all of the changes

Automatic identity registration of Deadline Clients was end-to-end tested by deploying the All-In-AWS-Infrastructure-Basic TypeScript example app with Thinkbox Usage-Based Licensing (UBL). Verified that:

  • Client identities were properly being assigned with the "Client" role and statuses by checking in a connected Deadline Monitor
  • Logs in the renderfarm/ConfigureRepository log group showed correct behavior
  • Clients were able to access Deadline-managed secrets. Thinkbox UBL requires the "Client" role to be assigned properly. Renders were successful.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@jusiskin jusiskin added the contribution/core This is a PR that came from AWS. label Sep 21, 2021
@jusiskin jusiskin force-pushed the sm_worker_auto_registration branch 5 times, most recently from f58da2e to 8c4292d Compare September 27, 2021 17:43
@jusiskin jusiskin force-pushed the sm_worker_auto_registration branch from 8c4292d to 26cd4e1 Compare September 30, 2021 22:08
@jusiskin jusiskin marked this pull request as ready for review October 1, 2021 16:32
@jusiskin jusiskin changed the title feat(deadline): configure identity registration settings for Deadline… feat(deadline): configure identity registration settings for deadline clients Oct 1, 2021
@jusiskin jusiskin force-pushed the sm_worker_auto_registration branch from 26cd4e1 to 0cc7f99 Compare October 1, 2021 16:59
Copy link
Contributor

@horsmand horsmand left a comment

Choose a reason for hiding this comment

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

Overall this looks really good, none of my comments should require changing too much in the overall architecture of this. I think the deployment instance could be a really powerful tool.

packages/aws-rfdk/lib/core/lib/deployment-instance.ts Outdated Show resolved Hide resolved
packages/aws-rfdk/lib/core/lib/deployment-instance.ts Outdated Show resolved Hide resolved
packages/aws-rfdk/lib/core/lib/deployment-instance.ts Outdated Show resolved Hide resolved
packages/aws-rfdk/lib/deadline/lib/render-queue.ts Outdated Show resolved Hide resolved
packages/aws-rfdk/lib/deadline/test/repository.test.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@kozlove-aws kozlove-aws 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. Just a couple moments that I want clarify.

Copy link
Contributor

@jericht jericht left a comment

Choose a reason for hiding this comment

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

Looks great! Just have a few minor comments

kozlove-aws
kozlove-aws previously approved these changes Oct 6, 2021
jusiskin and others added 9 commits October 7, 2021 17:23
Co-authored-by: David Horsman <56004724+horsmand@users.noreply.github.com>
- reorder DeploymentInstance props
- document internal use of RenderQueue.configureSecretsManagementAutoRegistration
- move SecretsManagementIdentityRegistration construct ID to constant
- Add TSDoc header for SecretsManagementIdentityRegistration construct
- Factor out constant for MacOS Deadline path file
- Remove artifacts from FileSecret in configure_identity_registration_settings.py
- fixes to encoding, logging and error-handling of deadline path on MacOS
- TSDoc improvements
- Use MachineImage.latestAmazonLinux
- Remove unnecessary test parameterization
- Add code comments
- Fix typos/punctuation/redundant syntax
jericht
jericht previously approved these changes Oct 7, 2021
Copy link
Contributor

@jericht jericht left a comment

Choose a reason for hiding this comment

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

LGTM!

I left a few minor comments, but it's mostly tiny formatting/organization in the unit tests, so not holding back the PR for it.

horsmand
horsmand previously approved these changes Oct 8, 2021
Copy link
Contributor

@horsmand horsmand left a comment

Choose a reason for hiding this comment

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

I'm in the same boat as Jericho. This looks good as-is. I'm going to give my approval but leave it for Josh to look over the comments and decide whether to fix them or merge.

- flatten jest test heirarchies
- fix typos
- clarify test name
@jusiskin jusiskin dismissed stale reviews from horsmand and jericht via 5565a47 October 8, 2021 14:31
@horsmand horsmand merged commit b9082b2 into aws:feature_enable_secret_manager Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants