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

Proxy config affecting change events #1244

Merged
merged 3 commits into from
Sep 28, 2019
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
15 changes: 15 additions & 0 deletions app/events/proxy_configs/affecting_object_changed_event.rb
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions app/lib/event_store/repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
44 changes: 44 additions & 0 deletions app/lib/proxy_config_affecting_changes.rb
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

So you can use previously_changed? https://github.com/3scale/porta/blob/d21f8d5e7a6dc740db6652e0604dcf0b639edb52/lib/previously_changed.rb

Which will be introduced in rails 5.0

return if changes_attributes.include?('created_at') || (changes_attributes & PROXY_CONFIG_AFFECTING_ATTRIBUTES).empty?
Copy link
Contributor

Choose a reason for hiding this comment

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

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
2 changes: 2 additions & 0 deletions app/models/backend_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions app/models/proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -20,6 +21,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/
Expand Down Expand Up @@ -515,6 +519,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?
Copy link
Contributor

Choose a reason for hiding this comment

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

Using this method (probably in the view) creates records right?
If yes, can we avoid that somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, if we have a proxy config, then we can also have an affecting change history I think. It also saves us from having to migrate data.

Copy link
Contributor

Choose a reason for hiding this comment

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

fair enough

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
Expand Down
5 changes: 5 additions & 0 deletions app/models/proxy_config_affecting_change.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# frozen_string_literal: true

class ProxyConfigAffectingChange < ApplicationRecord
belongs_to :proxy
end
2 changes: 2 additions & 0 deletions app/models/proxy_rule.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
10 changes: 10 additions & 0 deletions app/subscribers/proxy_config_event_subscriber.rb
Original file line number Diff line number Diff line change
@@ -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
10 changes: 10 additions & 0 deletions app/workers/proxy_config_affecting_change_worker.rb
Original file line number Diff line number Diff line change
@@ -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
15 changes: 15 additions & 0 deletions db/migrate/20190925152107_create_proxy_config_affecting_changes.rb
Original file line number Diff line number Diff line change
@@ -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
10 changes: 9 additions & 1 deletion db/oracle_schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
10 changes: 9 additions & 1 deletion db/postgres_schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
10 changes: 9 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions test/events/proxy_configs/affecting_object_changed_event_test.rb
Original file line number Diff line number Diff line change
@@ -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
15 changes: 15 additions & 0 deletions test/subscribers/proxy_config_event_subscriber_test.rb
Original file line number Diff line number Diff line change
@@ -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
22 changes: 22 additions & 0 deletions test/unit/proxy_rule_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
42 changes: 42 additions & 0 deletions test/unit/proxy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -606,4 +606,46 @@ 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

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
23 changes: 23 additions & 0 deletions test/workers/proxy_config_affecting_change_worker_test.rb
Original file line number Diff line number Diff line change
@@ -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