diff --git a/app/errors/fastly_config.rb b/app/errors/fastly_config.rb new file mode 100644 index 000000000..f88176055 --- /dev/null +++ b/app/errors/fastly_config.rb @@ -0,0 +1,19 @@ +module FastlyConfig + module Errors + class Error < StandardError + end + + class InvalidConfigsFormat < Error + def initialize(msg = "configs: must be an Array of Strings") + super(msg) + end + end + + class InvalidConfig < Error + def initialize(invalid_config, valid_configs) + msg = "Invalid Fastly config - #{invalid_config}. Only #{valid_configs.join(', ')} are valid." + super(msg) + end + end + end +end diff --git a/app/services/fastly_config/base.rb b/app/services/fastly_config/base.rb new file mode 100644 index 000000000..de410e0ee --- /dev/null +++ b/app/services/fastly_config/base.rb @@ -0,0 +1,33 @@ +module FastlyConfig + class Base + attr_reader :fastly, :active_version, :updated_files + + def initialize(fastly, active_version) + @fastly = fastly + @active_version = active_version + @updated_files = get_updated_files + end + + def update_needed? + updated_files.present? + end + + def update(new_version) + updated_files.each { |filename| upsert_config(new_version, filename) } + end + + private + + def get_updated_files(files: self.class::FASTLY_FILES) + Dir.glob(files).filter_map { |filename| filename if file_updated?(filename) } + end + + def file_updated? + raise SubclassResponsibility, "Fastly configs must implement their own file_updated? method" + end + + def upsert_config + raise SubclassResponsibility, "Fastly configs must implement their own upsert_config method" + end + end +end diff --git a/app/services/fastly_config/snippets.rb b/app/services/fastly_config/snippets.rb new file mode 100644 index 000000000..6d9ba023b --- /dev/null +++ b/app/services/fastly_config/snippets.rb @@ -0,0 +1,62 @@ +module FastlyConfig + class Snippets < Base + FASTLY_FILES = Rails.root.join("config/fastly/snippets/*.vcl").freeze + + private + + def file_updated?(filename) + snippet_name = File.basename(filename, ".vcl").humanize + snippet = find_snippet(fastly, active_version, snippet_name) + + return true if snippet.nil? + + new_snippet_content = File.read(filename) + new_snippet_content != snippet.content + end + + def upsert_config(new_version, filename) + snippet_name = File.basename(filename, ".vcl").humanize + snippet = find_snippet(fastly, new_version, snippet_name) + new_snippet_content = File.read(filename) + + if snippet + snippet.content = new_snippet_content + snippet.save! + log_to_datadog("update", snippet, new_version) + else + snippet_options = { + content: new_snippet_content, + dynamic: 0, + name: snippet_name, + service_id: ApplicationConfig["FASTLY_SERVICE_ID"], + type: "init", + version: new_version.number + } + + snippet = fastly.create(Fastly::Snippet, snippet_options) + log_to_datadog("create", snippet, new_version) + end + + true + end + + def find_snippet(fastly, new_version, snippet_name) + fastly.get_snippet(ApplicationConfig["FASTLY_SERVICE_ID"], new_version.number, snippet_name) + rescue Fastly::Error => e + error_message = JSON.parse(e.message) + raise e unless error_message["msg"] == "Record not found" + + nil + end + + def log_to_datadog(update_type, snippet, new_version) + tags = [ + "snippet_update_type:#{update_type}", + "snippet_name:#{snippet.name}", + "new_version:#{new_version.number}", + ] + + DatadogStatsClient.increment("fastly.snippets", tags: tags) + end + end +end diff --git a/app/services/fastly_config/update.rb b/app/services/fastly_config/update.rb new file mode 100644 index 000000000..64f7720ee --- /dev/null +++ b/app/services/fastly_config/update.rb @@ -0,0 +1,65 @@ +module FastlyConfig + # Handles updates to our Fastly configurations + class Update + FASTLY_CONFIGS = %w[Snippets].freeze + + def self.call + new.run + end + + attr_reader :fastly, :service + + def initialize + @fastly = Fastly.new(api_key: ApplicationConfig["FASTLY_API_KEY"]) + @service = fastly.get_service(ApplicationConfig["FASTLY_SERVICE_ID"]) + end + + def run(configs: FASTLY_CONFIGS) + validate_configs(configs) + + active_version = get_active_version(service) + config_handlers = configs.map { |config| "FastlyConfig::#{config}".constantize.new(fastly, active_version) } + configs_updated = config_handlers.any?(&:update_needed?) + + return unless configs_updated + + new_version = active_version.clone + config_handlers.each { |config_handler| config_handler.update(new_version) } + new_version.activate! + log_to_datadog(configs, new_version) + Rails.logger.info("Fastly updated to version #{new_version.number}.") + rescue Fastly::Error => e + error_msg = JSON.parse(e.message) + raise e unless unauthorized_error?(error_msg) && Rails.env.development? + + nil + end + + private + + def get_active_version(service) + service.versions.reverse_each.detect(&:active?) + end + + def log_to_datadog(configs, new_version) + tags = [ + "new_version:#{new_version.number}", + "configs_updated:#{configs.join(', ')}", + ] + + DatadogStatsClient.increment("fastly.update", tags: tags) + end + + def validate_configs(configs) + raise FastlyConfig::Errors::InvalidConfigsFormat unless configs.is_a? Array + + configs.each do |config| + raise FastlyConfig::Errors::InvalidConfig.new(config, FASTLY_CONFIGS) unless FASTLY_CONFIGS.include? config + end + end + + def unauthorized_error?(error_msg) + error_msg["msg"] == "Provided credentials are missing or invalid" + end + end +end diff --git a/app/services/fastly_vcl/safe_params.rb b/app/services/fastly_vcl/safe_params.rb deleted file mode 100644 index 365c830fa..000000000 --- a/app/services/fastly_vcl/safe_params.rb +++ /dev/null @@ -1,67 +0,0 @@ -module FastlyVCL - # Handles updates to our VCL snippet on Fastly that lists safe params - class SafeParams - VCL_DELIMITER_START = "^(".freeze - VCL_DELIMITER_END = ")$".freeze - SNIPPET_NAME = ApplicationConfig["FASTLY_SAFE_PARAMS_SNIPPET_NAME"].freeze - FILE_PARAMS = YAML.load_file("config/fastly/safe_params.yml").freeze - - class << self - def update - fastly = Fastly.new(api_key: ApplicationConfig["FASTLY_API_KEY"]) - service = fastly.get_service(ApplicationConfig["FASTLY_SERVICE_ID"]) - latest_version = service.version - - snippet = fastly.get_snippet(ApplicationConfig["FASTLY_SERVICE_ID"], - latest_version.number, - SNIPPET_NAME) - - current_params = params_to_array(snippet.content) - - return unless params_updated?(current_params) - - new_version = latest_version.clone - new_snippet = fastly.get_snippet(ApplicationConfig["FASTLY_SERVICE_ID"], - new_version.number, - SNIPPET_NAME) - new_snippet.content = build_content(FILE_PARAMS, new_snippet.content) - new_snippet.save! - - new_version.activate! - log_params_diff_to_datadaog(current_params, new_version) - Rails.logger.info("Fastly updated to version #{new_version.number}.") - end - - private - - def params_updated?(current_params) - (current_params - FILE_PARAMS).any? || (FILE_PARAMS - current_params).any? - end - - def params_to_array(snippet_content) - snippet_suffix = snippet_content.split(VCL_DELIMITER_START).last - fastly_params = snippet_suffix.split(VCL_DELIMITER_END).first - - fastly_params.split("|") - end - - def build_content(new_params, snippet_content) - new_params = new_params.join("|") - snippet_prefix = snippet_content.split(VCL_DELIMITER_START).first - snippet_suffix = snippet_content.split(VCL_DELIMITER_END).last - - "#{snippet_prefix}#{VCL_DELIMITER_START}#{new_params}#{VCL_DELIMITER_END}#{snippet_suffix}" - end - - def log_params_diff_to_datadaog(current_params, new_version) - tags = [ - "added_params:#{(FILE_PARAMS - current_params).join(', ')}", - "removed_params:#{(current_params - FILE_PARAMS).join(', ')}", - "new_version:#{new_version.number}", - ] - - DatadogStatsClient.increment("fastly.safelist", tags: tags) - end - end - end -end diff --git a/config/fastly/safe_params.yml b/config/fastly/safe_params.yml deleted file mode 100644 index a9b0596fe..000000000 --- a/config/fastly/safe_params.yml +++ /dev/null @@ -1,57 +0,0 @@ ---- -- a_id -- args -- article_id -- article_ids -- articles -- asc -- callback_url -- category -- chat_channel_id -- client_id -- code -- collection_id -- commentable_id -- commentable_type -- confirmation_token -- created_at -- end -- filter -- followable_id -- followable_type -- fork_id -- i -- key -- message_offset -- name -- oauth_token -- oauth_verifier -- offset -- org_id -- organization_id -- p -- page -- per_page -- prefill -- preview -- purchaser -- reactable_ids -- redirect_uri -- reported_url -- reporter_username -- response_type -- scope -- search -- signature -- sort -- start -- state -- status -- tag -- tag_list -- top -- type_of -- url -- username -- ut -- verb diff --git a/config/fastly/snippets/safe_params_list.vcl b/config/fastly/snippets/safe_params_list.vcl new file mode 100644 index 000000000..41c6c3dbd --- /dev/null +++ b/config/fastly/snippets/safe_params_list.vcl @@ -0,0 +1,7 @@ +import querystring; +sub vcl_recv { + # return this URL with only the parameters that match this regular expression + if (req.url !~ "/internal/" && req.url !~ "/search/") { + set req.url = querystring.regfilter_except(req.url, "^(a_id|args|article_id|article_ids|articles|asc|callback_url|category|chat_channel_id|client_id|code|collection_id|commentable_id|commentable_type|confirmation_token|created_at|end|filter|followable_id|followable_type|fork_id|i|key|message_offset|name|oauth_token|oauth_verifier|offset|org_id|organization_id|p|page|per_page|prefill|preview|purchaser|reactable_ids|redirect_uri|reported_url|reporter_username|response_type|scope|search|signature|sort|start|state|status|tag|tag_list|top|type_of|url|username|ut|verb)$"); + } +} diff --git a/docs/backend/fastly.md b/docs/backend/fastly.md index 5a808edc1..2ce922e84 100644 --- a/docs/backend/fastly.md +++ b/docs/backend/fastly.md @@ -13,7 +13,20 @@ If you want to learn more about we use Fastly, check out this [@benhalpern](https://dev.to/ben), gave at RailsConf 2018 talking about how we made our app so fast it went viral. -## Adding query string parameters to a safe list +Fastly offers many different configurations (conditions, +[snippets](https://docs.fastly.com/vcl/vcl-snippets/about-vcl-snippets/), etc.). + +## Snippets + +Snippets, in general, are _"short blocks of VCL logic that can be included +directly in your service configurations. They're ideal for adding small sections +of code when you don't need more complex, specialized configurations that +sometimes require custom VCL."_ + +You can learn more about VCL snippets on +[Fastly's website](https://docs.fastly.com/vcl/vcl-snippets/about-vcl-snippets/) + +### Adding query string parameters to a safe list In the context of contributing, here's what you need to know about Fastly. In order for our servers to receive any sort of query string parameters in a @@ -28,27 +41,23 @@ to see examples of this. Previously this was a manual process done by an internal team member. Now we do it programmatically using the Fastly -[gem](https://github.com/fastly/fastly-ruby) as of -[this pr](https://github.com/thepracticaldev/dev.to/pull/7279). +[gem](https://github.com/fastly/fastly-ruby). -## How it works +### How it works -We created a new file, `config/fastly/safe_params.yml`, to house all of the safe -params in Fastly. +We created a new file, `config/fastly/safe_params_list.vcl`, to house all of the +safe params in Fastly. If you need to update this list, simply update this file. It's as easy as that! +This is a VCL file and you'll see a little Regex checking for a list of safe +params. _Fastly is not setup for development._ -## In production +### In production -If you have Fastly configured in Production and have a similar custom VCL script -to list safe query string params, make sure you've set the -`FASTLY_SAFE_PARAMS_SNIPPET_NAME` ENV variable with the name of the VCL snippet -you have configured in Fastly. +If you have Fastly configured in Production, all Fastly configs are updated +automatically when a production deploy goes out. -Safe params on Fastly are updated automatically when a production deploy goes -out unless this key is not set (i.e. you don't have a similar custom VCL setup). - -We do this by executing `bin/rails fastly:update_safe_params` in our +We do this by executing `bin/rails fastly:update_configs` in our `release-tasks.sh` script. diff --git a/lib/tasks/fastly.rake b/lib/tasks/fastly.rake index f987f9377..5bbccb75d 100644 --- a/lib/tasks/fastly.rake +++ b/lib/tasks/fastly.rake @@ -1,10 +1,9 @@ namespace :fastly do - desc "Update VCL for safe params on Fastly" - task update_safe_params: :environment do + desc "Update Fastly configs" + task update_configs: :environment do fastly_credentials = %w[ FASTLY_API_KEY FASTLY_SERVICE_ID - FASTLY_SAFE_PARAMS_SNIPPET_NAME ] if fastly_credentials.any? { |cred| ApplicationConfig[cred].blank? } @@ -15,6 +14,6 @@ namespace :fastly do next end - FastlyVCL::SafeParams.update + FastlyConfig::Update.call end end diff --git a/release-tasks.sh b/release-tasks.sh index 635500e48..e95c9599b 100755 --- a/release-tasks.sh +++ b/release-tasks.sh @@ -17,7 +17,7 @@ set -Eex # runs migration for Postgres, setups/updates Elasticsearch # and boots the app to check there are no errors STATEMENT_TIMEOUT=180000 bundle exec rails db:migrate -bundle exec rake fastly:update_safe_params +bundle exec rake fastly:update_configs bundle exec rake search:setup bundle exec rake data_updates:enqueue_data_update_worker bundle exec rails runner "puts 'app load success'" diff --git a/spec/services/fastly_config/snippets_spec.rb b/spec/services/fastly_config/snippets_spec.rb new file mode 100644 index 000000000..0b95b0f0d --- /dev/null +++ b/spec/services/fastly_config/snippets_spec.rb @@ -0,0 +1,53 @@ +require "rails_helper" + +RSpec.describe FastlyConfig::Snippets, type: :service do + let(:fastly) { instance_double Fastly } + let(:fastly_version) { instance_double Fastly::Version } + let(:fastly_snippet) { instance_double Fastly::Snippet } + let(:snippets_config) { described_class.new(fastly, fastly_version) } + + it "determines if an update is needed" do + allow(fastly_version).to receive(:number).and_return(1) + allow(fastly).to receive(:get_snippet).and_return(fastly_snippet) + allow(fastly_snippet).to receive(:content).and_return("some VCL") + expect(snippets_config.update_needed?).to be true + end + + describe "upsert_config" do + it "creates a new snippet if one isn't found" do + allow(fastly_version).to receive(:number).and_return(1) + allow(fastly).to receive(:get_snippet).and_return(nil) + allow(fastly).to receive(:create).and_return(fastly_snippet) + allow(fastly_snippet).to receive(:name).and_return("test") + snippets_config.update(fastly_version) + expect(fastly).to have_received(:create) + end + + it "updates a snippet if one is found" do + allow(fastly_version).to receive(:number).and_return(1) + allow(fastly).to receive(:get_snippet).and_return(fastly_snippet) + allow(fastly_snippet).to receive(:content).and_return("test") + allow(fastly_snippet).to receive(:content=).and_return("test") + allow(fastly_snippet).to receive(:save!).and_return(fastly_snippet) + allow(fastly_snippet).to receive(:name).and_return("test") + snippets_config.update(fastly_version) + expect(fastly_snippet).to have_received(:save!) + end + + it "logs success messages" do + allow(DatadogStatsClient).to receive(:increment) + allow(fastly_version).to receive(:number).and_return(1) + allow(fastly).to receive(:get_snippet).and_return(fastly_snippet) + allow(fastly_snippet).to receive(:content).and_return("test") + allow(fastly_snippet).to receive(:content=).and_return("test") + allow(fastly_snippet).to receive(:save!).and_return(fastly_snippet) + allow(fastly_snippet).to receive(:name).and_return("test") + + snippets_config.update(fastly_version) + + tags = hash_including(tags: array_including("snippet_update_type:update", "snippet_name:test", "new_version:#{fastly_version.number}")) + + expect(DatadogStatsClient).to have_received(:increment).with("fastly.snippets", tags) + end + end +end diff --git a/spec/services/fastly_config/update_spec.rb b/spec/services/fastly_config/update_spec.rb new file mode 100644 index 000000000..8c78efc46 --- /dev/null +++ b/spec/services/fastly_config/update_spec.rb @@ -0,0 +1,72 @@ +require "rails_helper" + +RSpec.describe FastlyConfig::Update, type: :service do + let(:fastly) { instance_double(Fastly) } + let(:fastly_service) { instance_double(Fastly::Service) } + let(:fastly_version) { instance_double(Fastly::Version) } + let(:fastly_snippet) { instance_double(Fastly::Snippet) } + let(:fastly_updater) { described_class.new } + + # Fastly isn't setup for test or development environments so we have to stub + # quite a bit here to simulate Fastly working ¯\_(ツ)_/¯ + before do + allow(Fastly).to receive(:new).and_return(fastly) + allow(fastly).to receive(:get_service).and_return(fastly_service) + allow(described_class).to receive(:new).and_return(fastly_updater) + allow(fastly_updater).to receive(:get_active_version).and_return(fastly_version) + allow(fastly_updater).to receive(:fastly).and_return(fastly) + allow(fastly_updater).to receive(:service).and_return(fastly_service) + allow(fastly_version).to receive(:number).and_return(1) + allow(fastly).to receive(:get_snippet).and_return(fastly_snippet) + allow(fastly_version).to receive(:clone).and_return(fastly_version) + allow(fastly_version).to receive(:number).and_return(99) + allow(fastly_version).to receive(:activate!).and_return(true) + end + + describe "::run" do + it "raises an error for incorrectly formatted configs" do + expect { fastly_updater.run(configs: "Not an Array") }.to raise_error(FastlyConfig::Errors::InvalidConfigsFormat) + end + + it "raises an error for invalid configs" do + expect { fastly_updater.run(configs: ["Invalid config"]) }.to raise_error(FastlyConfig::Errors::InvalidConfig) + end + + it "doesn't update if the params haven't changed" do + stub_const("#{described_class}::FASTLY_CONFIGS", ["Snippets"]) + snippet_handler = instance_double FastlyConfig::Snippets + allow(FastlyConfig::Snippets).to receive(:new).and_return(snippet_handler) + allow(snippet_handler).to receive(:update_needed?).and_return(false) + described_class.call + expect(fastly_version).not_to have_received(:clone) + end + + it "updates Fastly if new updates are found" do + stub_const("#{described_class}::FASTLY_CONFIGS", ["Snippets"]) + snippet_handler = instance_double FastlyConfig::Snippets + allow(FastlyConfig::Snippets).to receive(:new).and_return(snippet_handler) + allow(snippet_handler).to receive(:update_needed?).and_return(true) + allow(snippet_handler).to receive(:update).and_return(true) + described_class.call + expect(fastly_version).to have_received(:activate!) + end + + it "logs success messages" do + allow(Rails.logger).to receive(:info) + allow(DatadogStatsClient).to receive(:increment) + + stub_const("#{described_class}::FASTLY_CONFIGS", ["Snippets"]) + snippet_handler = instance_double FastlyConfig::Snippets + allow(FastlyConfig::Snippets).to receive(:new).and_return(snippet_handler) + allow(snippet_handler).to receive(:update_needed?).and_return(true) + allow(snippet_handler).to receive(:update).and_return(true) + + described_class.call + expect(Rails.logger).to have_received(:info) + + tags = hash_including(tags: array_including("new_version:#{fastly_version.number}", "configs_updated:Snippets")) + + expect(DatadogStatsClient).to have_received(:increment).with("fastly.update", tags) + end + end +end diff --git a/spec/services/fastly_vcl/safe_params_spec.rb b/spec/services/fastly_vcl/safe_params_spec.rb deleted file mode 100644 index cc7a6dde5..000000000 --- a/spec/services/fastly_vcl/safe_params_spec.rb +++ /dev/null @@ -1,75 +0,0 @@ -require "rails_helper" - -RSpec.describe FastlyVCL::SafeParams, type: :service do - let(:fastly) { instance_double(Fastly) } - let(:fastly_service) { instance_double(Fastly::Service) } - let(:fastly_version) { instance_double(Fastly::Version) } - let(:fastly_snippet) { instance_double(Fastly::Snippet) } - let(:file_params) { YAML.load_file("config/fastly/safe_params.yml") } - let(:snippet_content) do - "#{described_class::VCL_DELIMITER_START}#{file_params.join('|')}#{described_class::VCL_DELIMITER_END}" - end - - # Fastly isn't setup for test or development environments so we have to stub - # quite a bit here to simulate Fastly working ¯\_(ツ)_/¯ - before do - allow(Fastly).to receive(:new).and_return(fastly) - allow(fastly).to receive(:get_service).and_return(fastly_service) - allow(fastly_service).to receive(:version).and_return(fastly_version) - allow(fastly_version).to receive(:number).and_return(1) - allow(fastly).to receive(:get_snippet).and_return(fastly_snippet) - allow(fastly_snippet).to receive(:content).and_return(snippet_content) - allow(fastly_version).to receive(:clone).and_return(fastly_version) - allow(fastly_version).to receive(:number).and_return(99) - allow(described_class).to receive(:build_content).and_return(snippet_content) - allow(fastly_snippet).to receive(:content).and_return(fastly_snippet) - allow(described_class).to receive(:params_to_array).and_return(file_params) - allow(fastly_snippet).to receive(:content=).and_return(snippet_content) - allow(fastly_snippet).to receive(:save!).and_return(true) - allow(fastly_version).to receive(:activate!).and_return(true) - end - - describe "::update" do - it "doesn't update if the params haven't changed" do - described_class.update - expect(fastly_version).not_to have_received(:clone) - end - - it "updates Fastly if new params are added" do - new_params = file_params + ["new_param"] - stub_const("#{described_class}::FILE_PARAMS", new_params) - described_class.update - expect(fastly_version).to have_received(:activate!) - end - - it "updates Fastly if params are removed" do - new_params = file_params - [file_params.last] - stub_const("#{described_class}::FILE_PARAMS", new_params) - described_class.update - expect(fastly_version).to have_received(:activate!) - end - - it "overwrites updates made directly in Fastly" do - new_fastly_params = file_params + ["new_fastly_param"] - allow(described_class).to receive(:params_to_array).and_return(new_fastly_params) - described_class.update - expect(fastly_version).to have_received(:activate!) - end - - it "logs success messages" do - allow(Rails.logger).to receive(:info) - allow(DatadogStatsClient).to receive(:increment) - - old_param = file_params.last - new_params = file_params + ["new_param"] - [old_param] - stub_const("#{described_class}::FILE_PARAMS", new_params) - described_class.update - - expect(Rails.logger).to have_received(:info) - - tags = hash_including(tags: array_including("added_params:new_param", "removed_params:#{old_param}", "new_version:#{fastly_version.number}")) - - expect(DatadogStatsClient).to have_received(:increment).with("fastly.safelist", tags) - end - end -end diff --git a/spec/tasks/fastly_spec.rb b/spec/tasks/fastly_spec.rb index 26566a11b..c0726be3e 100644 --- a/spec/tasks/fastly_spec.rb +++ b/spec/tasks/fastly_spec.rb @@ -6,16 +6,16 @@ RSpec.describe "Fastly tasks", type: :task do PracticalDeveloper::Application.load_tasks end - describe "#update_safe_params" do + describe "#update_configs" do it "doesn't run if Fastly isn't configured" do - %w[FASTLY_API_KEY FASTLY_SERVICE_ID FASTLY_SAFE_PARAMS_SNIPPET_NAME].each do |var| + %w[FASTLY_API_KEY FASTLY_SERVICE_ID].each do |var| allow(ApplicationConfig).to receive(:[]).with(var).and_return(nil) end - allow(FastlyVCL::SafeParams).to receive(:update) + allow(FastlyConfig::Update).to receive(:call) - Rake::Task["fastly:update_safe_params"].invoke + Rake::Task["fastly:update_configs"].invoke - expect(FastlyVCL::SafeParams).not_to have_received(:update) + expect(FastlyConfig::Update).not_to have_received(:call) end end end