From 90f8f25d044fe44103cc59c0658a18c6baa708f4 Mon Sep 17 00:00:00 2001 From: Guilherme Cassolato Date: Wed, 25 Sep 2019 18:45:51 +0200 Subject: [PATCH 1/3] New model ProxyConfigAffectingChange schema for psql and oracle --- app/models/proxy_config_affecting_change.rb | 5 +++++ ...52107_create_proxy_config_affecting_changes.rb | 15 +++++++++++++++ db/oracle_schema.rb | 10 +++++++++- db/postgres_schema.rb | 10 +++++++++- db/schema.rb | 10 +++++++++- 5 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 app/models/proxy_config_affecting_change.rb create mode 100644 db/migrate/20190925152107_create_proxy_config_affecting_changes.rb diff --git a/app/models/proxy_config_affecting_change.rb b/app/models/proxy_config_affecting_change.rb new file mode 100644 index 0000000000..23bdd0420d --- /dev/null +++ b/app/models/proxy_config_affecting_change.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class ProxyConfigAffectingChange < ApplicationRecord + belongs_to :proxy +end diff --git a/db/migrate/20190925152107_create_proxy_config_affecting_changes.rb b/db/migrate/20190925152107_create_proxy_config_affecting_changes.rb new file mode 100644 index 0000000000..3a205b6944 --- /dev/null +++ b/db/migrate/20190925152107_create_proxy_config_affecting_changes.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class CreateProxyConfigAffectingChanges < ActiveRecord::Migration + disable_ddl_transaction! if System::Database.postgres? + + def change + create_table :proxy_config_affecting_changes do |t| + t.references :proxy, null: false + t.timestamps null: false + end + + index_options = System::Database.postgres? ? { algorithm: :concurrently } : {} + add_index :proxy_config_affecting_changes, :proxy_id, index_options.merge(unique: true) + end +end diff --git a/db/oracle_schema.rb b/db/oracle_schema.rb index ba372a5d8b..2dddf27f48 100644 --- a/db/oracle_schema.rb +++ b/db/oracle_schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20190925133159) do +ActiveRecord::Schema.define(version: 20190925152107) do create_table "access_tokens", force: :cascade do |t| t.integer "owner_id", precision: 38, null: false @@ -1133,6 +1133,14 @@ add_index "proxies", ["service_id"], name: "index_proxies_on_service_id" add_index "proxies", ["staging_domain", "production_domain"], name: "index_proxies_on_staging_domain_and_production_domain" + create_table "proxy_config_affecting_changes", force: :cascade do |t| + t.integer "proxy_id", precision: 38, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "proxy_config_affecting_changes", ["proxy_id"], name: "index_proxy_config_affecting_changes_on_proxy_id", unique: true + create_table "proxy_configs", force: :cascade do |t| t.integer "proxy_id", precision: 38, null: false t.integer "user_id", precision: 38 diff --git a/db/postgres_schema.rb b/db/postgres_schema.rb index 85161506f8..b99cc5c971 100644 --- a/db/postgres_schema.rb +++ b/db/postgres_schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20190925133159) do +ActiveRecord::Schema.define(version: 20190925152107) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1134,6 +1134,14 @@ add_index "proxies", ["service_id"], name: "index_proxies_on_service_id", using: :btree add_index "proxies", ["staging_domain", "production_domain"], name: "index_proxies_on_staging_domain_and_production_domain", using: :btree + create_table "proxy_config_affecting_changes", force: :cascade do |t| + t.integer "proxy_id", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "proxy_config_affecting_changes", ["proxy_id"], name: "index_proxy_config_affecting_changes_on_proxy_id", unique: true, using: :btree + create_table "proxy_configs", force: :cascade do |t| t.integer "proxy_id", limit: 8, null: false t.integer "user_id", limit: 8 diff --git a/db/schema.rb b/db/schema.rb index 380669d3a6..585b0ade89 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20190925133159) do +ActiveRecord::Schema.define(version: 20190925152107) do create_table "access_tokens", force: :cascade do |t| t.integer "owner_id", limit: 8, null: false @@ -1135,6 +1135,14 @@ add_index "proxies", ["service_id"], name: "index_proxies_on_service_id", using: :btree add_index "proxies", ["staging_domain", "production_domain"], name: "index_proxies_on_staging_domain_and_production_domain", using: :btree + create_table "proxy_config_affecting_changes", force: :cascade do |t| + t.integer "proxy_id", limit: 4, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "proxy_config_affecting_changes", ["proxy_id"], name: "index_proxy_config_affecting_changes_on_proxy_id", unique: true, using: :btree + create_table "proxy_configs", force: :cascade do |t| t.integer "proxy_id", limit: 8, null: false t.integer "user_id", limit: 8 From 11f00d04cd862cc7a9d27f87fdbe4434719933e3 Mon Sep 17 00:00:00 2001 From: Guilherme Cassolato Date: Wed, 25 Sep 2019 19:06:09 +0200 Subject: [PATCH 2/3] Proxy config affecting object changed event --- .../affecting_object_changed_event.rb | 15 +++++++++++ app/lib/event_store/repository.rb | 1 + app/models/proxy.rb | 16 ++++++++++++ .../proxy_config_event_subscriber.rb | 10 +++++++ .../proxy_config_affecting_change_worker.rb | 10 +++++++ .../affecting_object_changed_event_test.rb | 15 +++++++++++ .../proxy_config_event_subscriber_test.rb | 15 +++++++++++ test/unit/proxy_test.rb | 26 +++++++++++++++++++ ...oxy_config_affecting_change_worker_test.rb | 23 ++++++++++++++++ 9 files changed, 131 insertions(+) create mode 100644 app/events/proxy_configs/affecting_object_changed_event.rb create mode 100644 app/subscribers/proxy_config_event_subscriber.rb create mode 100644 app/workers/proxy_config_affecting_change_worker.rb create mode 100644 test/events/proxy_configs/affecting_object_changed_event_test.rb create mode 100644 test/subscribers/proxy_config_event_subscriber_test.rb create mode 100644 test/workers/proxy_config_affecting_change_worker_test.rb diff --git a/app/events/proxy_configs/affecting_object_changed_event.rb b/app/events/proxy_configs/affecting_object_changed_event.rb new file mode 100644 index 0000000000..0429ba81c3 --- /dev/null +++ b/app/events/proxy_configs/affecting_object_changed_event.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class ProxyConfigs::AffectingObjectChangedEvent < ServiceRelatedEvent + def self.create(proxy, object) + new( + proxy_id: proxy.id, + object_id: object.id, + object_type: object.class.name, + metadata: { + service_id: proxy.service_id, + provider_id: proxy.account.id + } + ) + end +end diff --git a/app/lib/event_store/repository.rb b/app/lib/event_store/repository.rb index 487314d370..a5d62114df 100644 --- a/app/lib/event_store/repository.rb +++ b/app/lib/event_store/repository.rb @@ -164,6 +164,7 @@ def initialize(repository = self.class.repository, event_broker = EventBroker.ne subscribe_event(UserEventSubscriber.new, Users::UserDeletedEvent) subscribe_event(ServiceDeletionSubscriber.new, Services::ServiceScheduledForDeletionEvent) subscribe_event(ServiceDeletedSubscriber.new, Services::ServiceDeletedEvent) + subscribe_event(ProxyConfigEventSubscriber.new, ProxyConfigs::AffectingObjectChangedEvent) subscribe_event(ZyncSubscriber.new, ZyncEvent) end diff --git a/app/models/proxy.rb b/app/models/proxy.rb index 236b02587d..1281c5ebc1 100644 --- a/app/models/proxy.rb +++ b/app/models/proxy.rb @@ -20,6 +20,9 @@ class Proxy < ApplicationRecord validates :error_status_no_match, :error_status_auth_missing, :error_status_auth_failed, :error_status_limits_exceeded, presence: true + has_one :proxy_config_affecting_change, dependent: :delete + private :proxy_config_affecting_change + uri_pattern = URI::DEFAULT_PARSER.pattern URI_OPTIONAL_PORT = /\Ahttps?:\/\/[a-zA-Z0-9._-]*(:\d+)?\Z/ @@ -515,6 +518,19 @@ def service_mesh_integration? Service::DeploymentOption.service_mesh.include?(deployment_option) end + def find_or_create_proxy_config_affecting_change + proxy_config_affecting_change || create_proxy_config_affecting_change + end + alias affecting_change_history find_or_create_proxy_config_affecting_change + private :find_or_create_proxy_config_affecting_change + + def pending_affecting_changes? + return unless apicast_configuration_driven? + config = proxy_configs.sandbox.newest_first.first + return false unless config + config.created_at < affecting_change_history.updated_at + end + protected class PolicyConfig diff --git a/app/subscribers/proxy_config_event_subscriber.rb b/app/subscribers/proxy_config_event_subscriber.rb new file mode 100644 index 0000000000..ed313dcdce --- /dev/null +++ b/app/subscribers/proxy_config_event_subscriber.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class ProxyConfigEventSubscriber + def call(event) + case event + when ProxyConfigs::AffectingObjectChangedEvent then ProxyConfigAffectingChangeWorker.perform_later(event.event_id) + else raise "Unknown event type #{event.class}" + end + end +end diff --git a/app/workers/proxy_config_affecting_change_worker.rb b/app/workers/proxy_config_affecting_change_worker.rb new file mode 100644 index 0000000000..bccfe51442 --- /dev/null +++ b/app/workers/proxy_config_affecting_change_worker.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class ProxyConfigAffectingChangeWorker < ActiveJob::Base # rubocop:disable Rails/ApplicationJob + def perform(event_id) + event = EventStore::Repository.find_event!(event_id) + proxy = Proxy.find(event.proxy_id) + proxy.affecting_change_history.touch + rescue ActiveRecord::RecordNotFound + end +end diff --git a/test/events/proxy_configs/affecting_object_changed_event_test.rb b/test/events/proxy_configs/affecting_object_changed_event_test.rb new file mode 100644 index 0000000000..1e6eab5577 --- /dev/null +++ b/test/events/proxy_configs/affecting_object_changed_event_test.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'test_helper' + +class ProxyConfigs::AffectingObjectChangedEventTest < ActiveSupport::TestCase + test 'create' do + proxy = FactoryBot.create(:proxy) + proxy_rule = FactoryBot.build_stubbed(:proxy_rule, proxy: proxy) + event = ProxyConfigs::AffectingObjectChangedEvent.create(proxy, proxy_rule) + + assert_equal proxy.id, event.proxy_id + assert_equal proxy_rule.id, event.object_id + assert_equal 'ProxyRule', event.object_type + end +end diff --git a/test/subscribers/proxy_config_event_subscriber_test.rb b/test/subscribers/proxy_config_event_subscriber_test.rb new file mode 100644 index 0000000000..7754a3ab9a --- /dev/null +++ b/test/subscribers/proxy_config_event_subscriber_test.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'test_helper' + +class ProxyConfigEventSubscriberTest < ActiveSupport::TestCase + test 'create' do + proxy = FactoryBot.create(:proxy) + proxy_rule = FactoryBot.build_stubbed(:proxy_rule, proxy: proxy) + event = ProxyConfigs::AffectingObjectChangedEvent.create(proxy, proxy_rule) + + ProxyConfigAffectingChangeWorker.expects(:perform_later).with(event.event_id) + + ProxyConfigEventSubscriber.new.call(event) + end +end diff --git a/test/unit/proxy_test.rb b/test/unit/proxy_test.rb index 8f5e19bd2b..ca9e1eadad 100644 --- a/test/unit/proxy_test.rb +++ b/test/unit/proxy_test.rb @@ -606,4 +606,30 @@ def test_set_correct_endpoints def analytics ThreeScale::Analytics::UserTracking.any_instance end + + test 'affecting change' do + refute ProxyConfigAffectingChange.find_by(proxy_id: @proxy.id) + @proxy.affecting_change_history + assert ProxyConfigAffectingChange.find_by(proxy_id: @proxy.id) + end + + test '#pending_affecting_changes?' do + proxy = FactoryBot.create(:simple_proxy, api_backend: nil) + proxy.affecting_change_history.touch + + # no existing config for staging (sandbox) + refute proxy.pending_affecting_changes? + + Timecop.travel(1.second.from_now) do + FactoryBot.create(:proxy_config, proxy: proxy, environment: :sandbox) + + # latest config is ahead of affecting change record + refute proxy.pending_affecting_changes? + + proxy.affecting_change_history.touch + + # latest config is behind of affecting change record + assert proxy.pending_affecting_changes? + end + end end diff --git a/test/workers/proxy_config_affecting_change_worker_test.rb b/test/workers/proxy_config_affecting_change_worker_test.rb new file mode 100644 index 0000000000..3fab1e192a --- /dev/null +++ b/test/workers/proxy_config_affecting_change_worker_test.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'test_helper' + +class ProxyConfigAffectingChangeWorkerTest < ActiveSupport::TestCase + setup do + @worker = ProxyConfigAffectingChangeWorker.new + EventStore::Repository.stubs(raise_errors: true) + end + + attr_reader :worker + + test '#perform' do + proxy = FactoryBot.create(:proxy) + event = ProxyConfigs::AffectingObjectChangedEvent.create_and_publish!(proxy, mock(id: 123)) + + affecting_change_history = mock + Proxy.any_instance.expects(:create_proxy_config_affecting_change).returns(affecting_change_history) + affecting_change_history.expects(:touch) + + worker.perform(event.event_id) + end +end From bf69759a462af5fbe8a9dc082cfed9227645dc7b Mon Sep 17 00:00:00 2001 From: Guilherme Cassolato Date: Thu, 26 Sep 2019 14:32:50 +0200 Subject: [PATCH 3/3] Trigger proxy config affecting change events for proxy rules and policies configs --- app/lib/proxy_config_affecting_changes.rb | 44 +++++++++++++++++++++++ app/models/backend_api.rb | 2 ++ app/models/proxy.rb | 1 + app/models/proxy_rule.rb | 2 ++ test/unit/proxy_rule_test.rb | 22 ++++++++++++ test/unit/proxy_test.rb | 16 +++++++++ 6 files changed, 87 insertions(+) create mode 100644 app/lib/proxy_config_affecting_changes.rb diff --git a/app/lib/proxy_config_affecting_changes.rb b/app/lib/proxy_config_affecting_changes.rb new file mode 100644 index 0000000000..ad84b64e36 --- /dev/null +++ b/app/lib/proxy_config_affecting_changes.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module ProxyConfigAffectingChanges + module ProxyRuleExtension + extend ActiveSupport::Concern + + included do + include ProxyConfigAffectingChanges + + after_commit :issue_proxy_affecting_change_events + + def issue_proxy_affecting_change_events + case owner + when Proxy + issue_proxy_affecting_change_event(owner) + when BackendApi + owner.proxies.each(&method(:issue_proxy_affecting_change_event)) + end + end + end + end + + module ProxyExtension + extend ActiveSupport::Concern + + PROXY_CONFIG_AFFECTING_ATTRIBUTES = %w[policies_config].freeze # TODO: add more attributes here + + included do + include ProxyConfigAffectingChanges + + after_commit :issue_proxy_affecting_change_events, on: :update + + def issue_proxy_affecting_change_events + changes_attributes = previous_changes.keys + return if changes_attributes.include?('created_at') || (changes_attributes & PROXY_CONFIG_AFFECTING_ATTRIBUTES).empty? + issue_proxy_affecting_change_event(self) + end + end + end + + def issue_proxy_affecting_change_event(proxy) + ProxyConfigs::AffectingObjectChangedEvent.create_and_publish!(proxy, self) + end +end diff --git a/app/models/backend_api.rb b/app/models/backend_api.rb index b10a1b1fdb..0e748ed867 100644 --- a/app/models/backend_api.rb +++ b/app/models/backend_api.rb @@ -11,6 +11,8 @@ class BackendApi < ApplicationRecord has_many :backend_api_configs, inverse_of: :backend_api, dependent: :destroy has_many :services, through: :backend_api_configs + has_many :proxies, through: :services + belongs_to :account, inverse_of: :backend_apis delegate :provider_can_use?, to: :account, allow_nil: true diff --git a/app/models/proxy.rb b/app/models/proxy.rb index 1281c5ebc1..2894f19cbe 100644 --- a/app/models/proxy.rb +++ b/app/models/proxy.rb @@ -6,6 +6,7 @@ class Proxy < ApplicationRecord include BackendApiLogic::ProxyExtension prepend BackendApiLogic::RoutingPolicy include GatewaySettings::ProxyExtension + include ProxyConfigAffectingChanges::ProxyExtension DEFAULT_POLICY = { 'name' => 'apicast', 'humanName' => 'APIcast policy', 'description' => 'Main functionality of APIcast.', 'configuration' => {}, 'version' => 'builtin', 'enabled' => true, 'removable' => false, 'id' => 'apicast-policy' }.freeze diff --git a/app/models/proxy_rule.rb b/app/models/proxy_rule.rb index 139a815b76..41dbb62240 100644 --- a/app/models/proxy_rule.rb +++ b/app/models/proxy_rule.rb @@ -11,6 +11,8 @@ class ProxyRule < ApplicationRecord belongs_to :owner, polymorphic: true # FIXME: we should touch the owner here, but it will raise ActiveRecord::StaleObjectError belongs_to :metric + include ProxyConfigAffectingChanges::ProxyRuleExtension + validates :http_method, :pattern, :owner_id, :owner_type, :metric_id, presence: true validates :owner_type, length: { maximum: 255 } validates :delta, numericality: { :only_integer => true, :greater_than => 0 } diff --git a/test/unit/proxy_rule_test.rb b/test/unit/proxy_rule_test.rb index 4d37288567..b5b80915fb 100644 --- a/test/unit/proxy_rule_test.rb +++ b/test/unit/proxy_rule_test.rb @@ -156,4 +156,26 @@ class ProxyRuleTest < ActiveSupport::TestCase assert_equal backend_api, backend_proxy_rule.owner assert backend_proxy_rule.valid? end + + class ProxyConfigAffectingChangesTest < ActiveSupport::TestCase + disable_transactional_fixtures! + + test 'proxy config affecting changes of object owner by proxy' do + proxy = FactoryBot.create(:proxy) + proxy_rule = FactoryBot.build(:proxy_rule, owner: proxy) + ProxyConfigs::AffectingObjectChangedEvent.expects(:create_and_publish!).with(proxy, proxy_rule).twice + proxy_rule.save! + proxy_rule.update_attributes(pattern: '/new-pattern') + end + + test 'proxy config affecting changes of object owner by backend_api' do + backend_api = FactoryBot.create(:backend_api) + products = FactoryBot.create_list(:simple_service, 2) + proxy_rule = FactoryBot.build(:proxy_rule, owner: backend_api) + products.each { |product| product.backend_api_configs.create(backend_api: backend_api) } + products.each { |product| ProxyConfigs::AffectingObjectChangedEvent.expects(:create_and_publish!).with(product.proxy, proxy_rule).twice } + proxy_rule.save! + proxy_rule.update_attributes(pattern: '/new-pattern') + end + end end diff --git a/test/unit/proxy_test.rb b/test/unit/proxy_test.rb index ca9e1eadad..0a24c19d0f 100644 --- a/test/unit/proxy_test.rb +++ b/test/unit/proxy_test.rb @@ -632,4 +632,20 @@ def analytics assert proxy.pending_affecting_changes? end end + + class ProxyConfigAffectingChangesTest < ActiveSupport::TestCase + disable_transactional_fixtures! + + test 'proxy config affecting changes' do + proxy = FactoryBot.build(:proxy) + + ProxyConfigs::AffectingObjectChangedEvent.expects(:create_and_publish!).with(proxy, instance_of(ProxyRule)) + ProxyConfigs::AffectingObjectChangedEvent.expects(:create_and_publish!).with(proxy, proxy) + proxy.save! # it should not trigger the event + proxy.update_attributes(policies_config: [{ name: '1', version: 'b', configuration: {} }]) + + ProxyConfigs::AffectingObjectChangedEvent.expects(:create_and_publish!).with(proxy, proxy).never + proxy.update_attributes(deployed_at: Time.utc(2019, 9, 26, 12, 20)) + end + end end