Use AWS secret for chef config overrides #36366

wjordan · 2020-08-20T00:37:11Z

Use AWS secret for chef config overrides

Overview

This PR adds a Secrets Manager secret to the Code.org application CloudFormation stack. The secret is a JSON string containing stack-wide Chef cookbook attributes. The value contained in this secret is passed as an argument to chef-client whenever our system invokes Chef, whether as part of its bootstrapping process or through a CI build.

Background

This AWS secret providing chef-config attributes will help transition existing cookbooks comprising our application's provisioning process from hosted to local-mode Chef. Specifically, the Chef Environment Attributes and Role Attributes currently stored in Hosted Chef Server will be manually merged and stored in this secret, then eventually deleted from the Hosted Chef resources.

This is the last part of our Chef provisioning depending on state stored in the Hosted Chef Server repo.

Implementation details

The SecretsManager::Secret resource's logical id is ChefConfig, and the secret name is CfnStack/[StackName]/chef.
To ensure the secret can be passed as an argument to chef-client whenever our system runs Chef, a new shell script is written to Cdo::CloudFormation::CdoApp::CHEF_CLIENT_BIN (a constant equal to '/usr/local/bin/chef-client') which fetches the secret (via aws secretsmanager get-secret-value) and passes it to chef-client (via the --json-attributes option). Existing calls to /opt/chef/bin/chef-client in our CI build scripts have been updated to call this helper script instead.
Our bootstrap also sets a number of Chef attributes through a first-boot.json file it creates. In order to ensure these first-boot attributes are applied in addition to the secret attributes, a separate call to chef-client with an empty runlist is now done first to persist the first-boot attributes to the newly-created Chef node.

Test Story

Deployed a new adhoc instance and verified it provisions correctly
Modified a Chef attribute within the created secret on the adhoc stack
Manually ran an additional CI build on the adhoc instance and verified it ran correctly and used the Chef attribute from the updated secret

Migration Plan

Merge this PR which will update the bootstrap scripts and create an empty secret in each application-stack (each infrastructure-managed environment)
- Verify deploy without issue
Copy attributes from Hosted Chef resources to secret (merging Environment-specific attributes with the baseline role attributes for each stack/environment)
- Verify deploy without issue
Remove attributes from Hosted Chef resources
- Verify deploy without issue

wjordan · 2020-08-21T16:04:58Z

Realized that we'll need to update these attributes before both calls to chef-client in ci.rake as well:

code-dot-org/lib/rake/ci.rake

Line 36 in c1e8b15

RakeUtils.sudo '/opt/chef/bin/chef-client --chef-license accept-silent'

code-dot-org/lib/rake/ci.rake

Line 121 in c1e8b15

command = 'sudo /opt/chef/bin/chef-client --chef-license accept-silent'

This could make things quite a bit messier and harder to maintain- I might spend a bit more time rethinking the approach to see if there are any simpler alternatives.

Add a Secrets Manager secret to CloudFormation stack, containing stack-wide Chef cookbook attributes. Pass attributes as argument to chef-client in bootstrap and CI.

sureshc · 2020-08-27T18:58:18Z

aws/cloudformation/cloud_formation_stack.yml.erb

+    Properties:
+      Description: !Sub "Custom Chef attributes for ${AWS::StackName} CloudFormation stack"
+      Name: !Sub "CfnStack/${AWS::StackName}/chef"
+      SecretString: '{}'


Might be useful to mention that we manage the values stored in this Secret manually (outside of this CloudFormation template).

done in 4155489.

uponthesun

Just checking that I understand the migration plan - with these changes, chef attributes will be set both from Secrets Manager (via --json-attributes) and from the Hosted Chef server. In the middle step, where they'll be set in both places, the values from Secrets will override the ones from Hosted Chef (though it won't really matter because the values should be identical in both places.) Is this right?

uponthesun · 2020-08-27T19:45:40Z

aws/cloudformation/bootstrap_chef_stack.sh.erb

 sudo -u ubuntu bash -c "aws s3 cp <%=bootstrap_script_path%> - | sudo bash -s -- $OPTIONS"
 [ $? -eq 0 ] && STATUS=SUCCESS || STATUS=<%=daemon ? 'SUCCESS' : 'FAILURE'%>

+# Bash script that invokes chef-client with fetched config attributes and provided run-list.
+CHEF_CLIENT_BIN=<%=CHEF_CLIENT_BIN%>


This looks fine to me, but I was curious on your thoughts on inlining this here, rather than uploading to S3 like we do for chef-bootstrap.sh. (Why isn't chef-bootstrap.sh just part of this script, anyway? Is it just to break up this script into smaller chunks?)

I was curious on your thoughts on inlining this here, rather than uploading to S3 like we do for chef-bootstrap.sh.

The parameterized arguments ${Chefconfig} (which is a reference to the CFn resource) and $RUN_LIST (which is a reference to the run_list option passed from the stack ERB including this script) make it more complicated to render this script to S3 at compile-time like we do chef-bootstrap.sh, especially since this script is being invoked from the CI-build as well, not just from this bootstrap script.

Why isn't chef-bootstrap.sh just part of this script, anyway? Is it just to break up this script into smaller chunks?

chef-bootstrap.sh was an experiment with deploying a self-contained bash script partly to break apart the bootstrap script as you guessed, but also because I thought that its purpose was narrow/isolated enough that it could be written as a standalone script, manually deployed once to a static location and touched very rarely after that.

That experiment turned out to be challenging as soon as it became a production-build dependency: deploying any meaningful change to the script required either careful backwards-compatibility with each change, and/or synchronization across all environments to prevent breaking the production workflow. So to avoid these issues (without setting up a whole more elaborate versioning, packaging and deployment system for such a tiny thing), I made a change to deploy it to a separate path per stack. So now it's basically tied to the stack anyway, contrary to the original design. It could be in-lined at this point to reduce the overall complexity (with some additional work).

uponthesun · 2020-08-27T19:47:24Z

lib/cdo/cloud_formation/cdo_app.rb

@@ -24,6 +24,7 @@ class CdoApp < StackTemplate
    end

    # Hard-coded constants and default values.
+    CHEF_CLIENT_BIN = '/usr/local/bin/chef-client'


Can we name both the variable and the file itself something that indicates it's not just another copy of chef-client? It's clear from this PR, but confusing if you see it in isolation. Maybe something like chef-client-with-secrets?

How about chef-cdo-app, to make it clear it's invoking chef to provision cdo-app (see 60b476c).

Update this secret manually after the stack is created to customize Chef configuration. The value should never be changed in the stack itself, since it would clobber all other changes.

wjordan · 2020-08-28T16:45:28Z

(CI unit-test failure seems unrelated..)

wjordan · 2020-08-28T17:13:47Z

Just checking that I understand the migration plan - with these changes, chef attributes will be set both from Secrets Manager (via --json-attributes) and from the Hosted Chef server. In the middle step, where they'll be set in both places, the values from Secrets will override the ones from Hosted Chef (though it won't really matter because the values should be identical in both places.) Is this right?

Yes, this is correct!

Fixes constant not found error. Followup to #36366.

Partial revert of #36366. The script needs to be created on existing environments by rebooting existing instances before the path can be safely updated.

uponthesun self-requested a review August 20, 2020 03:59

wjordan force-pushed the chef_config_aws_secret branch 2 times, most recently from fbd5196 to 1999a17 Compare August 20, 2020 21:04

Use AWS secret for chef config overrides

304ae3d

Add a Secrets Manager secret to CloudFormation stack, containing stack-wide Chef cookbook attributes. Pass attributes as argument to chef-client in bootstrap and CI.

wjordan force-pushed the chef_config_aws_secret branch from 125c1b2 to 304ae3d Compare August 27, 2020 00:36

wjordan marked this pull request as ready for review August 27, 2020 00:37

wjordan requested a review from sureshc August 27, 2020 00:37

sureshc reviewed Aug 27, 2020

View reviewed changes

sureshc approved these changes Aug 27, 2020

View reviewed changes

uponthesun reviewed Aug 27, 2020

View reviewed changes

wjordan added 2 commits August 27, 2020 16:22

rename chef-client to chef-cdo-app

60b476c

Add comment to ChefConfig

4155489

Update this secret manually after the stack is created to customize Chef configuration. The value should never be changed in the stack itself, since it would clobber all other changes.

uponthesun approved these changes Aug 27, 2020

View reviewed changes

wjordan merged commit 5ec2a4d into staging Aug 28, 2020

wjordan deleted the chef_config_aws_secret branch August 28, 2020 17:14

wjordan added a commit that referenced this pull request Aug 28, 2020

Fix call to chef-cdo-app in ci.rake

8018c27

Fixes constant not found error. Followup to #36366.

wjordan mentioned this pull request Aug 28, 2020

Fix call to chef-cdo-app in ci.rake #36498

Merged

wjordan added a commit that referenced this pull request Aug 28, 2020

Call original chef-client from CI build

d1c26c9

Partial revert of #36366. The script needs to be created on existing environments by rebooting existing instances before the path can be safely updated.

This was referenced Aug 28, 2020

Call original chef-client from CI build #36500

Merged

Fix bootstrap-chef status response #36582

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use AWS secret for chef config overrides #36366

Use AWS secret for chef config overrides #36366

wjordan commented Aug 20, 2020 •

edited

Loading

wjordan commented Aug 21, 2020

sureshc Aug 27, 2020

wjordan Aug 27, 2020

uponthesun left a comment

uponthesun Aug 27, 2020

wjordan Aug 27, 2020 •

edited

Loading

uponthesun Aug 27, 2020

wjordan Aug 27, 2020

wjordan commented Aug 28, 2020

wjordan commented Aug 28, 2020

Use AWS secret for chef config overrides #36366

Use AWS secret for chef config overrides #36366

Conversation

wjordan commented Aug 20, 2020 • edited Loading

Use AWS secret for chef config overrides

Overview

Background

Implementation details

Test Story

Migration Plan

wjordan commented Aug 21, 2020

sureshc Aug 27, 2020

Choose a reason for hiding this comment

wjordan Aug 27, 2020

Choose a reason for hiding this comment

uponthesun left a comment

Choose a reason for hiding this comment

uponthesun Aug 27, 2020

Choose a reason for hiding this comment

wjordan Aug 27, 2020 • edited Loading

Choose a reason for hiding this comment

uponthesun Aug 27, 2020

Choose a reason for hiding this comment

wjordan Aug 27, 2020

Choose a reason for hiding this comment

wjordan commented Aug 28, 2020

wjordan commented Aug 28, 2020

wjordan commented Aug 20, 2020 •

edited

Loading

wjordan Aug 27, 2020 •

edited

Loading