diff --git a/app/actions/manifest_route_update.rb b/app/actions/manifest_route_update.rb index d5fef841e5d..50830da16d3 100644 --- a/app/actions/manifest_route_update.rb +++ b/app/actions/manifest_route_update.rb @@ -1,5 +1,6 @@ require 'cloud_controller/app_manifest/manifest_route' require 'actions/route_create' +require 'actions/route_update' module VCAP::CloudController class ManifestRouteUpdate @@ -81,7 +82,8 @@ def find_or_create_valid_route(app, manifest_route, user_audit_info) message = RouteCreateMessage.new({ 'host' => host, 'path' => manifest_route[:path], - 'port' => manifest_route[:port] + 'port' => manifest_route[:port], + 'options' => manifest_route[:options] }) route = RouteCreate.new(user_audit_info).create( @@ -90,6 +92,12 @@ def find_or_create_valid_route(app, manifest_route, user_audit_info) domain: existing_domain, manifest_triggered: true ) + elsif route[:options] != manifest_route[:options] + message = RouteUpdateMessage.new({ + 'options' => manifest_route[:options] + }) + route = RouteUpdate.new.update(route:, message:) + elsif route.space.guid != app.space_guid raise InvalidRoute.new('Routes cannot be mapped to destinations in different spaces') end diff --git a/app/actions/route_create.rb b/app/actions/route_create.rb index 21d08fd8fdd..bac1c489739 100644 --- a/app/actions/route_create.rb +++ b/app/actions/route_create.rb @@ -15,7 +15,8 @@ def create(message:, space:, domain:, manifest_triggered: false) path: message.path || '', port: port(message, domain), space: space, - domain: domain + domain: domain, + options: message.options ) Route.db.transaction do diff --git a/app/actions/route_update.rb b/app/actions/route_update.rb index d5254a824b7..91a1821a4a2 100644 --- a/app/actions/route_update.rb +++ b/app/actions/route_update.rb @@ -2,6 +2,16 @@ module VCAP::CloudController class RouteUpdate def update(route:, message:) Route.db.transaction do + if message.requested?(:options) + route.options = if message.options.nil? + nil + elsif route.options.nil? + message.options + else + route.options.merge(message.options) + end + end + route.save MetadataUpdate.update(route, message) end diff --git a/app/messages/manifest_routes_update_message.rb b/app/messages/manifest_routes_update_message.rb index 74fa2a31e8d..00923bee92d 100644 --- a/app/messages/manifest_routes_update_message.rb +++ b/app/messages/manifest_routes_update_message.rb @@ -1,4 +1,5 @@ require 'messages/base_message' +require 'messages/route_options_message' require 'cloud_controller/app_manifest/manifest_route' module VCAP::CloudController @@ -7,9 +8,20 @@ class ManifestRoutesUpdateMessage < BaseMessage class ManifestRoutesYAMLValidator < ActiveModel::Validator def validate(record) - return unless is_not_array?(record.routes) || contains_non_route_hash_values?(record.routes) + if is_not_array?(record.routes) || contains_non_route_hash_values?(record.routes) + record.errors.add(:routes, message: 'must be a list of route objects') + return + end - record.errors.add(:routes, message: 'must be a list of route objects') + if contains_invalid_route_options?(record.routes) + record.errors.add(:routes, message: 'contains invalid route options') + return + end + + return unless contains_invalid_lb_algo?(record.routes) + + record.errors.add(:routes, message: 'contains an invalid loadbalancing-algorithm option') + nil end def is_not_array?(routes) @@ -19,6 +31,26 @@ def is_not_array?(routes) def contains_non_route_hash_values?(routes) routes.any? { |r| !(r.is_a?(Hash) && r[:route].present?) } end + + def contains_invalid_route_options?(routes) + routes.any? do |r| + next unless r[:options] + + return true unless r[:options].is_a?(Hash) + + return false if r[:options].empty? + + return r[:options].keys.all? { |key| RouteOptionsMessage::VALID_MANIFEST_ROUTE_OPTIONS.exclude?(key) } + end + end + + def contains_invalid_lb_algo?(routes) + routes.any? do |r| + next unless r[:options] && r[:options][:'loadbalancing-algorithm'] + + return true if r[:options][:'loadbalancing-algorithm'] && RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.exclude?(r[:options][:'loadbalancing-algorithm']) + end + end end validates_with NoAdditionalKeysValidator @@ -32,10 +64,12 @@ def contains_non_route_hash_values?(routes) def manifest_route_mappings @manifest_route_mappings ||= routes.map do |route| - { - route: ManifestRoute.parse(route[:route]), + r = { + route: ManifestRoute.parse(route[:route], route[:options]), protocol: route[:protocol] } + r[:options] = route[:options] unless route[:options].nil? + r end end diff --git a/app/messages/route_create_message.rb b/app/messages/route_create_message.rb index 1eeac2c2e46..12a1eda8581 100644 --- a/app/messages/route_create_message.rb +++ b/app/messages/route_create_message.rb @@ -1,4 +1,5 @@ require 'messages/metadata_base_message' +require 'messages/route_options_message' module VCAP::CloudController class RouteCreateMessage < MetadataBaseMessage @@ -10,6 +11,7 @@ class RouteCreateMessage < MetadataBaseMessage path port relationships + options ] validates :host, @@ -56,6 +58,7 @@ class RouteCreateMessage < MetadataBaseMessage validates_with NoAdditionalKeysValidator validates_with RelationshipValidator + validates_with OptionsValidator delegate :space_guid, to: :relationships_message delegate :domain_guid, to: :relationships_message @@ -65,6 +68,10 @@ def relationships_message @relationships_message ||= Relationships.new(relationships&.deep_symbolize_keys) end + def options_message + @options_message ||= RouteOptionsMessage.new(options&.deep_symbolize_keys) + end + def wildcard? host == '*' end diff --git a/app/messages/route_options_message.rb b/app/messages/route_options_message.rb new file mode 100644 index 00000000000..614eccba7f7 --- /dev/null +++ b/app/messages/route_options_message.rb @@ -0,0 +1,16 @@ +require 'messages/metadata_base_message' + +module VCAP::CloudController + class RouteOptionsMessage < BaseMessage + VALID_MANIFEST_ROUTE_OPTIONS = %i[loadbalancing-algorithm].freeze + VALID_ROUTE_OPTIONS = %i[lb_algo].freeze + VALID_LOADBALANCING_ALGORITHMS = %w[round-robin least-connections].freeze + + register_allowed_keys %i[lb_algo] + validates_with NoAdditionalKeysValidator + validates :lb_algo, + inclusion: { in: VALID_LOADBALANCING_ALGORITHMS, message: "'%s' is not a supported load-balancing algorithm" }, + presence: true, + allow_nil: true + end +end diff --git a/app/messages/route_update_message.rb b/app/messages/route_update_message.rb index 84365bdb3d2..3e567404690 100644 --- a/app/messages/route_update_message.rb +++ b/app/messages/route_update_message.rb @@ -1,8 +1,19 @@ require 'messages/metadata_base_message' +require 'messages/route_options_message' module VCAP::CloudController class RouteUpdateMessage < MetadataBaseMessage - register_allowed_keys [] + register_allowed_keys %i[options] + + def self.options_requested? + @options_requested ||= proc { |a| a.requested?(:options) } + end + + def options_message + @options_message ||= RouteOptionsMessage.new(options&.deep_symbolize_keys) + end + + validates_with OptionsValidator, if: options_requested? validates_with NoAdditionalKeysValidator end diff --git a/app/messages/validators.rb b/app/messages/validators.rb index c5555856c33..c9baf7e83a2 100644 --- a/app/messages/validators.rb +++ b/app/messages/validators.rb @@ -240,6 +240,26 @@ def validate(record) end end + class OptionsValidator < ActiveModel::Validator + def validate(record) + # Empty option hashes are allowed, so we skip further validation + record.options.blank? && return + + unless record.options.is_a?(Hash) + record.errors.add(:options, message: "'options' is not a valid object") + return + end + + opt = record.options_message + + return if opt.valid? + + opt.errors.full_messages.each do |message| + record.errors.add(:options, message:) + end + end + end + class ToOneRelationshipValidator < ActiveModel::EachValidator def validate_each(record, attribute, relationship) if has_correct_structure?(relationship) diff --git a/app/models/runtime/route.rb b/app/models/runtime/route.rb index 3f10c89dd67..7ec7a0b2264 100644 --- a/app/models/runtime/route.rb +++ b/app/models/runtime/route.rb @@ -40,8 +40,8 @@ class OutOfVIPException < CloudController::Errors::InvalidRelation; end add_association_dependencies route_mappings: :destroy - export_attributes :host, :path, :domain_guid, :space_guid, :service_instance_guid, :port - import_attributes :host, :path, :domain_guid, :space_guid, :app_guids, :port + export_attributes :host, :path, :domain_guid, :space_guid, :service_instance_guid, :port, :options + import_attributes :host, :path, :domain_guid, :space_guid, :app_guids, :port, :options add_association_dependencies labels: :destroy add_association_dependencies annotations: :destroy @@ -71,6 +71,23 @@ def as_summary_json } end + def options_with_serialization=(opts) + self.options_without_serialization = Oj.dump(opts) + end + + alias_method :options_without_serialization=, :options= + alias_method :options=, :options_with_serialization= + + def options_with_serialization + string = options_without_serialization + return nil if string.blank? + + Oj.load(string) + end + + alias_method :options_without_serialization, :options + alias_method :options, :options_with_serialization + alias_method :old_path, :path def path old_path.nil? ? '' : old_path diff --git a/app/presenters/v3/app_manifest_presenters/route_properties_presenter.rb b/app/presenters/v3/app_manifest_presenters/route_properties_presenter.rb index d3f43f40d31..f4cef6ab901 100644 --- a/app/presenters/v3/app_manifest_presenters/route_properties_presenter.rb +++ b/app/presenters/v3/app_manifest_presenters/route_properties_presenter.rb @@ -5,10 +5,17 @@ module AppManifestPresenters class RoutePropertiesPresenter def to_hash(route_mappings:, app:, **_) route_hashes = route_mappings.map do |route_mapping| - { + route_hash = { route: route_mapping.route.uri, protocol: route_mapping.protocol } + + if route_mapping.route.options + route_hash[:options] = {} + route_hash[:options][:'loadbalancing-algorithm'] = route_mapping.route.options[:lb_algo] if route_mapping.route.options[:lb_algo] + end + + route_hash end { routes: alphabetize(route_hashes).presence } diff --git a/app/presenters/v3/route_presenter.rb b/app/presenters/v3/route_presenter.rb index 9715e3c09c9..c090fafae5b 100644 --- a/app/presenters/v3/route_presenter.rb +++ b/app/presenters/v3/route_presenter.rb @@ -48,6 +48,7 @@ def to_hash }, links: build_links } + hash.merge!(options: route.options) unless route.options.nil? @decorators.reduce(hash) { |memo, d| d.decorate(memo, [route]) } end diff --git a/db/migrations/20241105000000_add_options_to_route.rb b/db/migrations/20241105000000_add_options_to_route.rb new file mode 100644 index 00000000000..94b75933ced --- /dev/null +++ b/db/migrations/20241105000000_add_options_to_route.rb @@ -0,0 +1,14 @@ +Sequel.migration do + up do + alter_table(:routes) do + # rubocop:disable Migration/IncludeStringSize + add_column :options, String, text: true, default: nil + # rubocop:enable Migration/IncludeStringSize + end + end + down do + alter_table(:routes) do + drop_column :options + end + end +end diff --git a/lib/cloud_controller/app_manifest/manifest_route.rb b/lib/cloud_controller/app_manifest/manifest_route.rb index 74ad1493e72..6a3cab14c90 100644 --- a/lib/cloud_controller/app_manifest/manifest_route.rb +++ b/lib/cloud_controller/app_manifest/manifest_route.rb @@ -6,8 +6,8 @@ class ManifestRoute SUPPORTED_TCP_SCHEMES = %w[tcp unspecified].freeze WILDCARD_HOST = '*'.freeze - def self.parse(string) - parsed_uri = Addressable::URI.heuristic_parse(string, scheme: 'unspecified') + def self.parse(route, options=nil) + parsed_uri = Addressable::URI.heuristic_parse(route, scheme: 'unspecified') attrs = if parsed_uri.nil? {} @@ -15,11 +15,22 @@ def self.parse(string) parsed_uri.to_hash end - attrs[:full_route] = string + attrs[:full_route] = route + + if options + attrs[:options] = {} + attrs[:options][:lb_algo] = options[:'loadbalancing-algorithm'] if options[:'loadbalancing-algorithm'] + end + ManifestRoute.new(attrs) end def valid? + if @attrs[:options] && !@attrs[:options].empty? + return false if @attrs[:options].keys.any? { |key| RouteOptionsMessage::VALID_ROUTE_OPTIONS.exclude?(key) } + return false if @attrs[:options][:lb_algo] && RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.exclude?(@attrs[:options][:lb_algo]) + end + return false if @attrs[:host].blank? return SUPPORTED_TCP_SCHEMES.include?(@attrs[:scheme]) if @attrs[:port] @@ -43,7 +54,8 @@ def to_hash { candidate_host_domain_pairs: pairs, port: @attrs[:port], - path: @attrs[:path] + path: @attrs[:path], + options: @attrs[:options] } end diff --git a/lib/cloud_controller/diego/app_recipe_builder.rb b/lib/cloud_controller/diego/app_recipe_builder.rb index 46ce619baa6..f206e901349 100644 --- a/lib/cloud_controller/diego/app_recipe_builder.rb +++ b/lib/cloud_controller/diego/app_recipe_builder.rb @@ -151,13 +151,15 @@ def health_check_timeout_in_seconds def generate_routes(info) http_routes = (info['http_routes'] || []).map do |i| - { + http_route = { hostnames: [i['hostname']], port: i['port'], route_service_url: i['route_service_url'], isolation_segment: IsolationSegmentSelector.for_space(process.space), protocol: i['protocol'] } + http_route[:options] = i['options'] if i['options'] + http_route end { diff --git a/lib/cloud_controller/diego/protocol/routing_info.rb b/lib/cloud_controller/diego/protocol/routing_info.rb index 278f587f347..e85c061a4fd 100644 --- a/lib/cloud_controller/diego/protocol/routing_info.rb +++ b/lib/cloud_controller/diego/protocol/routing_info.rb @@ -43,6 +43,7 @@ def http_info(process_eager) info['router_group_guid'] = r.domain.router_group_guid if r.domain.is_a?(SharedDomain) && !r.domain.router_group_guid.nil? info['port'] = get_port_to_use(route_mapping) info['protocol'] = route_mapping.protocol + info['options'] = r.options if r.options info end end diff --git a/lib/cloud_controller/route_validator.rb b/lib/cloud_controller/route_validator.rb index 978fda7c02d..637a883ab38 100644 --- a/lib/cloud_controller/route_validator.rb +++ b/lib/cloud_controller/route_validator.rb @@ -4,6 +4,7 @@ class ValidationError < StandardError; end class DomainInvalid < ValidationError; end class RouteInvalid < ValidationError; end class RoutePortTaken < ValidationError; end + class InvalidRouteOptions < ValidationError; end attr_reader :route diff --git a/spec/request/space_manifests_spec.rb b/spec/request/space_manifests_spec.rb index 74b77103bd0..d3f21eaa922 100644 --- a/spec/request/space_manifests_spec.rb +++ b/spec/request/space_manifests_spec.rb @@ -624,6 +624,136 @@ end end + describe 'route options' do + context 'when an empty route options hash is provided' do + let(:yml_manifest) do + { + 'applications' => [ + { + 'name' => app1_model.name, + 'routes' => [ + { 'route' => "https://#{route.host}.#{route.domain.name}", + 'options' => {} } + ] + } + ] + }.to_yaml + end + + it 'applies the manifest' do + post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header) + + expect(last_response.status).to eq(202) + end + end + + context 'when an invalid route option is provided' do + let(:yml_manifest) do + { + 'applications' => [ + { + 'name' => app1_model.name, + 'routes' => [ + { 'route' => "https://#{route.host}.#{route.domain.name}", + 'options' => { + 'doesnt-exist' => 'doesnt-exist' + } } + ] + } + ] + }.to_yaml + end + + it 'returns a 422' do + post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header) + + expect(last_response).to have_status_code(422) + expect(last_response).to have_error_message('Routes contains invalid route options') + end + end + + context 'loadbalancing-algorithm' do + context 'when the loadbalancing-algorithm is not supported' do + let(:yml_manifest) do + { + 'applications' => [ + { + 'name' => app1_model.name, + 'routes' => [ + { 'route' => "https://#{route.host}.#{route.domain.name}", + 'options' => { + 'loadbalancing-algorithm' => 'unsupported-lb-algorithm' + } } + ] + } + ] + }.to_yaml + end + + it 'returns a 422' do + post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header) + + expect(last_response).to have_status_code(422) + expect(last_response).to have_error_message('Routes contains an invalid loadbalancing-algorithm option') + end + end + + context 'when the loadbalancing-algorithm is supported' do + let(:yml_manifest) do + { + 'applications' => [ + { 'name' => app1_model.name, + 'routes' => [ + { 'route' => "https://round-robin-app.#{shared_domain.name}", + 'options' => { + 'loadbalancing-algorithm' => 'round-robin' + } } + ] } + ] + }.to_yaml + end + + it 'adds and updates the loadbalancing-algorithm' do + post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header) + + expect(last_response.status).to eq(202) + job_guid = VCAP::CloudController::PollableJobModel.last.guid + + Delayed::Worker.new.work_off + expect(VCAP::CloudController::PollableJobModel.find(guid: job_guid)).to be_complete, VCAP::CloudController::PollableJobModel.find(guid: job_guid).cf_api_error + + app1_model.reload + expect(app1_model.routes.first.options).to eq({ 'lb_algo' => 'round-robin' }) + + ### update the loadbalancing-algorithm from the route + + yml_manifest = { + 'applications' => [ + { 'name' => app1_model.name, + 'routes' => [ + { 'route' => "https://round-robin-app.#{shared_domain.name}", + 'options' => { + 'loadbalancing-algorithm' => 'least-connections' + } } + ] } + ] + }.to_yaml + + post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_header) + + expect(last_response.status).to eq(202) + job_guid = VCAP::CloudController::PollableJobModel.last.guid + + Delayed::Worker.new.work_off + expect(VCAP::CloudController::PollableJobModel.find(guid: job_guid)).to be_complete, VCAP::CloudController::PollableJobModel.find(guid: job_guid).cf_api_error + + app1_model.reload + expect(app1_model.routes.first.options).to eq({ 'lb_algo' => 'least-connections' }) + end + end + end + end + describe 'audit events' do let!(:process1) { nil } diff --git a/spec/support/legacy_api_dsl.rb b/spec/support/legacy_api_dsl.rb index c5c9c888291..54ab3deb48c 100644 --- a/spec/support/legacy_api_dsl.rb +++ b/spec/support/legacy_api_dsl.rb @@ -4,7 +4,7 @@ module LegacyApiDsl extend ActiveSupport::Concern def validate_response(model, json, expected_values: {}, ignored_attributes: [], expected_attributes: nil) - ignored_attributes.push :guid + ignored_attributes.push :guid, :options (expected_attributes || expected_attributes_for_model(model)).each do |expected_attribute| # refactor: pass exclusions, and figure out which are valid to not be there next if ignored_attributes.include? expected_attribute diff --git a/spec/unit/lib/cloud_controller/app_manifest/manifest_route_spec.rb b/spec/unit/lib/cloud_controller/app_manifest/manifest_route_spec.rb index 29d4005a06d..380d75855a3 100644 --- a/spec/unit/lib/cloud_controller/app_manifest/manifest_route_spec.rb +++ b/spec/unit/lib/cloud_controller/app_manifest/manifest_route_spec.rb @@ -58,6 +58,16 @@ module VCAP::CloudController expect(manifest_route.valid?).to be(true) end end + + context 'when there is a valid loadbalancing-algorithm' do + let(:route) { 'http://example.com' } + let(:options) { { 'loadbalancing-algorithm' => 'round-robin' } } + + it 'is valid' do + manifest_route = ManifestRoute.parse(route, options) + expect(manifest_route.valid?).to be(true) + end + end end describe 'invalid routes' do @@ -106,6 +116,16 @@ module VCAP::CloudController expect(manifest_route.valid?).to be(false) end end + + context 'when there is an invalid loadbalancing-algorithm' do + let(:route) { 'http://example.com' } + let(:options) { { 'loadbalancing-algorithm': 'invalid' } } + + it 'is invalid' do + manifest_route = ManifestRoute.parse(route, options) + expect(manifest_route.valid?).to be(false) + end + end end end @@ -119,7 +139,8 @@ module VCAP::CloudController { host: 'host', domain: 'sub.some-domain.com' } ], port: nil, - path: '/path' + path: '/path', + options: nil }) end @@ -131,7 +152,8 @@ module VCAP::CloudController { host: '*', domain: 'sub.some-domain.com' } ], port: nil, - path: '/path' + path: '/path', + options: nil }) end @@ -144,7 +166,8 @@ module VCAP::CloudController { host: 'potato', domain: 'sub.some-domain.com' } ], port: nil, - path: '/path' + path: '/path', + options: nil }) end @@ -157,7 +180,22 @@ module VCAP::CloudController { host: 'potato', domain: 'sub.some-domain.com' } ], port: 1234, - path: '' + path: '', + options: nil + }) + end + + it 'parses a route with a loadbalancing-algorithm into route components' do + route = ManifestRoute.parse('http://potato.sub.some-domain.com', { 'loadbalancing-algorithm': 'round-robin' }) + + expect(route.to_hash).to eq({ + candidate_host_domain_pairs: [ + { host: '', domain: 'potato.sub.some-domain.com' }, + { host: 'potato', domain: 'sub.some-domain.com' } + ], + port: nil, + path: '', + options: { lb_algo: 'round-robin' } }) end end diff --git a/spec/unit/messages/manifest_routes_update_message_spec.rb b/spec/unit/messages/manifest_routes_update_message_spec.rb index 2f894a81c71..e370b3126d0 100644 --- a/spec/unit/messages/manifest_routes_update_message_spec.rb +++ b/spec/unit/messages/manifest_routes_update_message_spec.rb @@ -208,6 +208,56 @@ module VCAP::CloudController expect(msg.errors.full_messages).to include("Route protocol must be 'http1', 'http2' or 'tcp'.") end end + + context 'when a route contains empty route options' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => {} } + ] } + end + + it 'returns true' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(true) + end + end + + context 'when a route contains invalid route options' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { 'invalid' => 'invalid' } } + ] } + end + + it 'returns true' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(false) + end + end + + context 'when a route contains a valid loadbalancing-algorithm' do + let(:body) do + { 'routes' => + [ + { 'route' => 'existing.example.com', + 'options' => { + 'loadbalancing-algorithm' => 'round-robin' + } } + ] } + end + + it 'returns true' do + msg = ManifestRoutesUpdateMessage.new(body) + + expect(msg.valid?).to be(true) + end + end end end end diff --git a/spec/unit/messages/route_create_message_spec.rb b/spec/unit/messages/route_create_message_spec.rb index 66bb5c6fb26..d64eb61a456 100644 --- a/spec/unit/messages/route_create_message_spec.rb +++ b/spec/unit/messages/route_create_message_spec.rb @@ -19,6 +19,9 @@ module VCAP::CloudController metadata: { labels: { potato: 'yam' }, annotations: { style: 'mashed' } + }, + options: { + lb_algo: 'round-robin' } } end @@ -354,6 +357,111 @@ module VCAP::CloudController end end end + + context 'options' do + context 'when not provided' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + } + } + end + + it 'is valid' do + expect(subject).to be_valid + end + end + + context 'when empty' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + }, + options: {} + } + end + + it 'is valid' do + expect(subject).to be_valid + end + end + + context 'when options has invalid key' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + }, + options: { foobar: 'baz' } + } + end + + it 'is not valid' do + expect(subject).not_to be_valid + expect(subject.errors[:options]).to include("Unknown field(s): 'foobar'") + end + end + + context 'when lb_algo has value round-robin' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + }, + options: { lb_algo: 'round-robin' } + } + end + + it 'is valid' do + expect(subject).to be_valid + end + + context 'when lb_algo has value least-connections' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + }, + options: { lb_algo: 'least-connections' } + } + end + + it 'is valid' do + expect(subject).to be_valid + end + end + + context 'when lb_algo has invalid value' do + let(:params) do + { + host: 'some-host', + relationships: { + space: { data: { guid: 'space-guid' } }, + domain: { data: { guid: 'domain-guid' } } + }, + options: { lb_algo: 'random' } + } + end + + it 'is not valid' do + expect(subject).not_to be_valid + expect(subject.errors[:options]).to include("Lb algo 'random' is not a supported load-balancing algorithm") + end + end + end + end end describe 'accessor methods' do diff --git a/spec/unit/messages/route_update_message_spec.rb b/spec/unit/messages/route_update_message_spec.rb index f3a9c5a66f3..18e92f49ac6 100644 --- a/spec/unit/messages/route_update_message_spec.rb +++ b/spec/unit/messages/route_update_message_spec.rb @@ -18,11 +18,43 @@ module VCAP::CloudController expect(message).to be_valid end + it 'accepts options params with round-robin load-balancing algorithm' do + message = RouteUpdateMessage.new(params.merge(options: { lb_algo: 'round-robin' })) + expect(message).to be_valid + end + + it 'accepts options params with least-connections load-balancing algorithm' do + message = RouteUpdateMessage.new(params.merge(options: { lb_algo: 'least-connections' })) + expect(message).to be_valid + end + + it 'accepts options: nil to unset options' do + message = RouteUpdateMessage.new(params.merge(options: nil)) + expect(message).to be_valid + end + + it 'accepts lb_algo: nil to unset load-balancing algorithm' do + message = RouteUpdateMessage.new(params.merge(options: { lb_algo: nil })) + expect(message).to be_valid + end + it 'does not accept any other params' do message = RouteUpdateMessage.new(params.merge(unexpected: 'unexpected_value')) expect(message).not_to be_valid expect(message.errors.full_messages[0]).to include("Unknown field(s): 'unexpected'") end + + it 'does not accept unknown load-balancing algorithm' do + message = RouteUpdateMessage.new(params.merge(options: { lb_algo: 'cheesecake' })) + expect(message).not_to be_valid + expect(message.errors.full_messages[0]).to include("Options Lb algo 'cheesecake' is not a supported load-balancing algorithm") + end + + it 'does not accept unknown option' do + message = RouteUpdateMessage.new(params.merge(options: { gorgonzola: 'gouda' })) + expect(message).not_to be_valid + expect(message.errors.full_messages[0]).to include("Options Unknown field(s): 'gorgonzola'") + end end end end diff --git a/spec/unit/messages/validators_spec.rb b/spec/unit/messages/validators_spec.rb index 303a5f823c0..093f2638aeb 100644 --- a/spec/unit/messages/validators_spec.rb +++ b/spec/unit/messages/validators_spec.rb @@ -529,6 +529,51 @@ class Relationships < VCAP::CloudController::BaseMessage end end + describe 'OptionsValidator' do + class OptionsMessage < VCAP::CloudController::BaseMessage + register_allowed_keys [:options] + + def options_message + VCAP::CloudController::RouteOptionsMessage.new(options&.deep_symbolize_keys) + end + + validates_with OptionsValidator + end + + it 'successfully validates round-robin load-balancing algorithm' do + message = OptionsMessage.new({ options: { lb_algo: 'round-robin' } }) + expect(message).to be_valid + end + + it 'successfully validates least-connections load-balancing algorithm' do + message = OptionsMessage.new({ options: { lb_algo: 'least-connections' } }) + expect(message).to be_valid + end + + it 'successfully validates empty options' do + message = OptionsMessage.new({ options: {} }) + expect(message).to be_valid + end + + it 'adds invalid object error message when options is not an object' do + message = OptionsMessage.new({ options: 'cheesecake' }) + expect(message).not_to be_valid + expect(message.errors_on(:options)).to include('\'options\' is not a valid object') + end + + it 'adds invalid load balancer error message to the base class' do + message = OptionsMessage.new({ options: { lb_algo: 'donuts' } }) + expect(message).not_to be_valid + expect(message.errors_on(:options)).to include('Lb algo \'donuts\' is not a supported load-balancing algorithm') + end + + it 'adds invalid field error message to the base class' do + message = OptionsMessage.new({ options: { cookies: 'round-robin' } }) + expect(message).not_to be_valid + expect(message.errors_on(:options)).to include('Unknown field(s): \'cookies\'') + end + end + describe 'ToOneRelationshipValidator' do let(:to_one_class) do Class.new(fake_class) do diff --git a/spec/unit/models/runtime/route_spec.rb b/spec/unit/models/runtime/route_spec.rb index 50e493ee45d..173d4cf1234 100644 --- a/spec/unit/models/runtime/route_spec.rb +++ b/spec/unit/models/runtime/route_spec.rb @@ -1049,8 +1049,8 @@ module VCAP::CloudController end describe 'Serialization' do - it { is_expected.to export_attributes :host, :domain_guid, :space_guid, :path, :service_instance_guid, :port } - it { is_expected.to import_attributes :host, :domain_guid, :space_guid, :app_guids, :path, :port } + it { is_expected.to export_attributes :host, :domain_guid, :space_guid, :path, :service_instance_guid, :port, :options } + it { is_expected.to import_attributes :host, :domain_guid, :space_guid, :app_guids, :path, :port, :options } end describe 'instance methods' do diff --git a/spec/unit/presenters/v3/route_presenter_spec.rb b/spec/unit/presenters/v3/route_presenter_spec.rb index c7c91dceea1..313a6b6f9f8 100644 --- a/spec/unit/presenters/v3/route_presenter_spec.rb +++ b/spec/unit/presenters/v3/route_presenter_spec.rb @@ -5,6 +5,7 @@ module VCAP::CloudController::Presenters::V3 RSpec.describe RoutePresenter do let!(:app) { VCAP::CloudController::AppModel.make } let(:space) { VCAP::CloudController::Space.make } + let(:options) { { lb_algo: 'round-robin' } } let(:org) { space.organization } let(:route_host) { 'host' } let(:path) { '/path' } @@ -20,7 +21,8 @@ module VCAP::CloudController::Presenters::V3 host: route_host, path: path, space: space, - domain: domain + domain: domain, + options: options ) end @@ -70,6 +72,7 @@ module VCAP::CloudController::Presenters::V3 expect(subject[:updated_at]).to be_a(Time) expect(subject[:host]).to eq(route_host) expect(subject[:path]).to eq(path) + expect(subject[:options]).to eq('lb_algo' => 'round-robin') expect(subject[:url]).to eq("#{route.host}.#{domain.name}#{route.path}") expected_destinations = [ @@ -125,6 +128,21 @@ module VCAP::CloudController::Presenters::V3 end end + context 'when options is empty' do + let(:route) do + VCAP::CloudController::Route.make( + host: 'foobar', + path: path, + space: space, + domain: domain + ) + end + + it 'does not output options' do + expect(subject[:options]).to be_nil + end + end + context 'when there are decorators' do let(:banana_decorator) do Class.new do