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

[PVT#181020089] Minor followup deployment improvements #2234

Merged
merged 20 commits into from May 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 1 addition & 7 deletions .github/workflows/asset_compilation.yml
Expand Up @@ -115,15 +115,9 @@ jobs:
bundle config set --local without 'production staging development'
bundle install --jobs 10 --retry 3

- name: 'App setup'
env:
SECRETS_FILE_SECRET_ARN: ${{ secrets.SECRETS_FILE_SECRET_ARN }}
run: |
# cp config/database.yml.ci config/database.yml
SECRET_ARN=$SECRETS_FILE_SECRET_ARN bundle exec bin/download_secrets.rb > config/deploy/docker/assets/secret.deploy.values.yml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is happening within command_args.rb now.

- name: Generate checksum and compile
env:
SECRETS_YML_SECRET_ARN: ${{ secrets.SECRETS_YML_SECRET_ARN }}
DATABASE_ADAPTER: postgresql
DATABASE_HOST: postgres
DATABASE_PASS: postgres
Expand Down
10 changes: 1 addition & 9 deletions bin/asset_checksum
Expand Up @@ -10,14 +10,6 @@ if [ "$ASSETS_PREFIX" = "" ]; then
export ASSETS_PREFIX=qa-warehouse-staging-ecs
fi

# Get client secrets, store in separate secrets file for hashing later.
if [ "$SECRET_ARN" != "" ]; then
bin/download_secrets.rb > .env.secret
else
echo 'No $SECRET_ARN!'
exit 1
fi

# Pull down our client theme files.
ASSETS_BUCKET_NAME=openpath-ecs-assets UPDATE_ONLY=false ./bin/sync_app_assets.rb > /dev/null 2>&1

Expand All @@ -33,4 +25,4 @@ tar -cf - --sort=name `# Force sorting` \
--no-acls `# No ACLs` \
--mode="go-rwx,u-rw" `# Normalize permissions` \
--exclude=.keep,*.rbc,**.orig,*.swp `# Exclude a few files we know we don't care about` \
app/assets .env.secret | md5sum | cut -d ' ' -f 1
app/assets | md5sum | cut -d ' ' -f 1
1 change: 0 additions & 1 deletion bin/download_secrets.rb

This file was deleted.

4 changes: 4 additions & 0 deletions bin/download_secrets.rb
@@ -0,0 +1,4 @@
#!/usr/bin/env ruby
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved this file to bin/ (instead of symlinking) to simplify pathing logic in require_relative.

require_relative '../config/deploy/docker/lib/aws_sdk_helpers'

puts AwsSdkHelpers::Helpers.get_secret(ENV.fetch('SECRET_ARN'))
26 changes: 23 additions & 3 deletions config/brakeman.ignore
@@ -1,5 +1,25 @@
{
"ignored_warnings": [
{
"warning_type": "Command Injection",
"warning_code": 14,
"fingerprint": "023c724508b4c341ebb8aae1c407e91e113c05bccffe0dca58702f276fbe1dd0",
"check_name": "Execute",
"message": "Possible command injection",
"file": "config/deploy/docker/lib/command_args.rb",
"line": 34,
"link": "https://brakemanscanner.org/docs/warning_types/command_injection/",
"code": "`diff #{path} #{tmppath}`",
"render_path": null,
"location": {
"type": "method",
"class": "CommandArgs",
"method": "initialize"
},
"user_input": "path",
"confidence": "Medium",
"note": ""
},
{
"warning_type": "File Access",
"warning_code": 16,
Expand Down Expand Up @@ -254,7 +274,7 @@
{
"type": "template",
"name": "homeless_summary_report/warehouse_reports/reports/show",
"line": 25,
"line": 27,
"file": "drivers/homeless_summary_report/app/views/homeless_summary_report/warehouse_reports/reports/show.haml",
"rendered": {
"name": "homeless_summary_report/warehouse_reports/reports/_counts_table",
Expand Down Expand Up @@ -1821,7 +1841,7 @@
"check_name": "SQL",
"message": "Possible SQL injection",
"file": "drivers/homeless_summary_report/app/models/homeless_summary_report/report.rb",
"line": 1035,
"line": 986,
"link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
"code": "clients.send(variant).send(\"spm_#{field}\").average(\"spm_#{field}\")",
"render_path": null,
Expand Down Expand Up @@ -2564,6 +2584,6 @@
"note": ""
}
],
"updated": "2022-05-14 15:14:48 +0000",
"updated": "2022-05-28 00:26:04 +0000",
"brakeman_version": "5.0.4"
}
Expand Up @@ -14,7 +14,6 @@ WORKDIR /app
COPY Gemfile Gemfile.lock Rakefile config.ru package.json ./
COPY bin ./bin
COPY public ./public
COPY config/deploy/docker/assets/download_secrets.rb ./bin
COPY config/deploy/docker/lib ./bin
COPY config/deploy/docker/assets/deploy_tasks.open-path-warehouse.sh ./bin/deploy_tasks.sh

Expand Down
16 changes: 0 additions & 16 deletions config/deploy/docker/assets/download_secrets.rb

This file was deleted.

6 changes: 3 additions & 3 deletions config/deploy/docker/assets/entrypoint.sh
Expand Up @@ -30,13 +30,13 @@ echo 'Setting Timezone'
cp /usr/share/zoneinfo/$TIMEZONE /app/etc-localtime
echo $TIMEZONE > /etc/timezone

echo 'Syncing the assets from s3...'
echo 'Syncing the client assets from s3...'
T1=`date +%s`
./bin/sync_app_assets.rb
T2=`date +%s`
echo "...sync_app_assets 1 took $(expr $T2 - $T1) seconds"

echo 'Pulling down assets from S3...'
echo 'Clobbering assets...'
T1=`date +%s`
bundle exec rake assets:clobber && mkdir -p ./public/assets
T2=`date +%s`
Expand All @@ -48,8 +48,8 @@ T2=`date +%s`
echo "...checksumming took $(expr $T2 - $T1) seconds"
echo "Using ASSET_CHECKSUM [${ASSET_CHECKSUM}]"

echo 'Pulling down the compiled assets...' # Using ASSETS_PREFIX from .env and ASSET_CHECKSUM from above.
cd ./public/assets
# Pull down the compiled assets. Using ASSETS_PREFIX from .env and ASSET_CHECKSUM from above.
bundle exec ../../bin/wait_for_compiled_assets.rb
T1=`date +%s`
ASSETS_PREFIX="${ASSETS_PREFIX}/${ASSET_CHECKSUM}" ASSETS_BUCKET_NAME=openpath-precompiled-assets UPDATE_ONLY=true ../../bin/sync_app_assets.rb
Expand Down
16 changes: 6 additions & 10 deletions config/deploy/docker/lib/asset_compiler.rb
Expand Up @@ -30,16 +30,6 @@ def self.compiled_assets_s3_path(target_group_name, checksum)
end

def run!
time_me name: 'Secrets download' do
system(`SECRET_ARN=#{@secret_arn.shellescape} bin/download_secrets.rb > .env`)
end

Dotenv.load('.env')

time_me name: 'Clobberin\'' do
system('source .env; rake assets:clobber') # TODO: don't call out to rake like this, it's inefficient
end

checksum = '<nochecksum>'
time_me name: 'Checksumming' do
checksum = `SECRET_ARN=#{@secret_arn.shellescape} ASSETS_PREFIX=#{@target_group_name.shellescape} bin/asset_checksum`.split(' ')[-1]
Expand All @@ -57,6 +47,12 @@ def run!
return
end

time_me name: 'Secrets download' do
system(`SECRET_ARN=#{@secret_arn.shellescape} bin/download_secrets.rb > .env`)
end

Dotenv.load('.env')

time_me name: 'Compiling assets' do
system('source .env; rake --quiet assets:precompile > /dev/null 2>&1') # TODO: don't call out to rake like this, it's inefficient
end
Expand Down
11 changes: 11 additions & 0 deletions config/deploy/docker/lib/aws_sdk_helpers.rb
Expand Up @@ -111,5 +111,16 @@ def _spot_capacity_provider_name
def _on_demand_capacity_provider_name
@_on_demand_capacity_provider_name ||= AwsSdkHelpers::Helpers.get_capacity_provider_name('OnDemand')
end

def self.get_secret(secret_arn)
resp = AwsSdkHelpers::ClientMethods.secretsmanager.get_secret_value(
secret_id: secret_arn,
)

return resp.to_h[:secret_string]
rescue Aws::Errors::MissingCredentialsError
warn 'Need credentials to sync secrets (or run on a server/container with a role)'
puts 'ERROR=secretsyncfailed'
end
end
end
39 changes: 38 additions & 1 deletion config/deploy/docker/lib/command_args.rb
@@ -1,13 +1,50 @@
# frozen_string_literal: true

require 'yaml'
require 'dotenv'

class CommandArgs
attr_accessor :deployments

def initialize
Dotenv.load('.env', '.env.local')

path = Pathname.new(__FILE__).join('..', '..', 'assets', 'secret.deploy.values.yml')
config = YAML.load_file(path)

if File.exist?(path)
local_config = YAML.load_file(path)
local_config = nil if local_config.empty?
else
local_config = nil
end

remote_config_text = AwsSdkHelpers::Helpers.get_secret(ENV['SECRETS_YML_SECRET_ARN'])
remote_config = YAML.safe_load(remote_config_text, [Symbol], aliases: true)

if !local_config.nil? && local_config != remote_config
puts 'Local secrets.yml differs from remote config, would you like to pull down the remote version? This will overwrite your local file. [y/N]'
unsure = $stdin.readline
if unsure.chomp.downcase.match?(/y(es)?/)
File.write(path, remote_config_text)
config = remote_config
else
tmppath = "/tmp/remote-secrets-#{Time.now.to_i}.yml"
File.write(tmppath, remote_config_text)
puts "Okay. Remote config saved to #{tmppath} for your convenience. (Local is at #{path})"
pp `diff #{path} #{tmppath}`
puts 'continue anyway? [y/N]'
unsure = $stdin.readline
exit unless unsure.chomp.downcase.match?(/y(es)?/)
config = local_config
puts "[WARN] ❗ Using local config: #{path}"
end
elsif remote_config.nil?
config = local_config
puts "[WARN] ❗ Remote secrets.yml not found, using local: #{path}"
else
config = remote_config
end

defaults = config['_global_defaults'] || {}
config.each_key do |key|
config.delete(key) if key.match?(/^_/)
Expand Down
30 changes: 30 additions & 0 deletions docs/asset_precompilation.md
@@ -0,0 +1,30 @@
## Asset Checksumming

`bin/asset_checksum`

In order to make it easy to tell if the assets need to be recompiled or if the cached versions can be used, we generate a checksum based on the content of `app/assets` (including client theme files). The goal is for the checksum to be guaranteed to change if a change is made to the source assets that will alter the compiled output, *and* guaranteed *not* to change otherwise.
Copy link
Contributor

Choose a reason for hiding this comment

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

Double checking "client theme files" are the files we store on S3 and pull down in prior to checksumming right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the ones in the openpath-ecs-assets S3 bucket. We should have a better nomenclature to differentiate those... thoughts?

I can also put the name of the bucket here, but it would be good to have an easier way of referring to the two different things: (a) client theme files/assets (openpath-ecs-assets) and (b) client compiled asset (cache?) (openpath-precompiled-assets).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I took a stab at a glossary at the top. Feel free to adjust.


*Note: Because the checksum when stored in "cache" (S3) will be namespaced to client/environment, it is not necessary to hash any of the client ENV secrets (CLIENT or RAILS_ENV would have been the only ones affecting compiled output).*

## The GitHub Actions Workflow

`.github/workflows/asset_compilation.yml`

This workflow iterates through every client (for both environments). It uses a [matrix](https://docs.github.com/en/actions/using-jobs/using-a-matrix-for-your-jobs) to iterate over a list of anonymized identifiers (`gha_staging_load_1`, `gha_production_load_3`), which are used to preserve client anonymity in an open source code base. Each identifier is passed to `bin/compile_assets.rb`.

## The Asset Compiler

`bin/compile_assets.rb`
`config/deploy/docker/lib/asset_compiler.rb`

`bin/compile_assets.rb` is analogous to `bin/deploy.rb`. It uses `config/deploy/docker/lib/command_args.rb` to pull down the secrets.yml file from AWS Secrets Manager (the credentials are stored in the GitHub [repository secrets](https://docs.github.com/en/actions/security-guides/encrypted-secrets) and are not loaded for non-internal workflow runs). It takes the anonymized identifier passed by the workflow and finds it in the list of client group identifiers at the bottom of the secrets.yml file, where it is mapped to real client identifying information. Currently each anonymized "group" has only 1 client, in order to maximize speed of the parallel asset compilation. The client information is passed to `AssetCompiler.run!`

`config/deploy/docker/lib/asset_compiler.rb` is analogous to `config/deploy/docker/lib/deployer.rb`. It starts by performing an asset_checksum and checking if that checksum has been stored in S3 yet. If it has, we don't need to do anything else, since the compiled output of the source assets would match the stored compiled output. If the checksum hasn't been stored, then there has been a change to the assets. In this case we pull down the client ENV secrets to bootstrap the client environment, and run asset precompilation via rake. These assets are then uploaded to S3 under the client, environment, and checksum.

## The Deployed Containers

`config/deploy/docker/assets/entrypoint.sh`

When a container spins up, the Docker entrypoint script generates an asset_checksum and pulls down the assets from S3. If the assets don't exist yet, the script will wait (`bin/wait_for_compiled_assets.rb`) and check again every 60 seconds. Note that this will happen in the deploy container before any of the application containers are spun up, meaning that you should be able to catch missing assets before the deploy goes through.

**NOTE:** If you make a change in the remote client theme files, you will need to ensure that the GitHub Actions workflow runs at least once to pick up that change. Otherwise the checksum generated in the entrypoint won't match the last stored checksum and the waiting will never finish.
andyzito marked this conversation as resolved.
Show resolved Hide resolved