diff --git a/app/messages/manifest_routes_update_message.rb b/app/messages/manifest_routes_update_message.rb index 8b9cfe0a39d..f8502cd8f18 100644 --- a/app/messages/manifest_routes_update_message.rb +++ b/app/messages/manifest_routes_update_message.rb @@ -8,14 +8,9 @@ class ManifestRoutesUpdateMessage < BaseMessage class ManifestRoutesYAMLValidator < ActiveModel::Validator def validate(record) - 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 + return unless is_not_array?(record.routes) || contains_non_route_hash_values?(record.routes) - contains_invalid_route_options?(record) - contains_invalid_lb_algo?(record) - nil + record.errors.add(:routes, message: 'must be a list of route objects') end def is_not_array?(routes) @@ -25,43 +20,14 @@ 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?(record) - routes = record.routes - routes.any? do |r| - next unless r[:options] - - return true unless r[:options].is_a?(Hash) - - return false if r[:options].empty? - - r[:options].each_key do |key| - RouteOptionsMessage::VALID_MANIFEST_ROUTE_OPTIONS.exclude?(key) && - record.errors.add(:base, - message: "Route '#{r[:route]}' contains invalid route option '#{key}'. \ -Valid keys: '#{RouteOptionsMessage::VALID_MANIFEST_ROUTE_OPTIONS.join(', ')}'") - end - end - end - - def contains_invalid_lb_algo?(record) - routes = record.routes - routes.each do |r| - next unless r[:options] && r[:options][:'loadbalancing-algorithm'] - - lb_algo = r[:options][:'loadbalancing-algorithm'] - RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.exclude?(lb_algo) && - record.errors.add(:base, - message: "Route '#{r[:route]}' contains invalid load-balancing algorithm '#{lb_algo}'. \ -Valid algorithms: '#{RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.join(', ')}'") - end - end end validates_with NoAdditionalKeysValidator validates_with ManifestRoutesYAMLValidator, if: proc { |record| record.requested?(:routes) } validate :routes_are_uris, if: proc { |record| record.requested?(:routes) } validate :route_protocols_are_valid, if: proc { |record| record.requested?(:routes) } + validate :route_options_are_valid, if: proc { |record| record.requested?(:routes) } + validate :lb_algos_are_valid, if: proc { |record| record.requested?(:routes) } validate :no_route_is_boolean validate :default_route_is_boolean validate :random_route_is_boolean @@ -80,6 +46,39 @@ def manifest_route_mappings private + def route_options_are_valid + return if errors[:routes].present? + + routes.any? do |r| + next unless r[:options] + + return true unless r[:options].is_a?(Hash) + + return false if r[:options].empty? + + r[:options].each_key do |key| + RouteOptionsMessage::VALID_MANIFEST_ROUTE_OPTIONS.exclude?(key) && + errors.add(:base, + message: "Route '#{r[:route]}' contains invalid route option '#{key}'. \ +Valid keys: '#{RouteOptionsMessage::VALID_MANIFEST_ROUTE_OPTIONS.join(', ')}'") + end + end + end + + def lb_algos_are_valid + return if errors[:routes].present? + + routes.each do |r| + next unless r[:options] && r[:options][:'loadbalancing-algorithm'] + + lb_algo = r[:options][:'loadbalancing-algorithm'] + RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.exclude?(lb_algo) && + errors.add(:base, + message: "Route '#{r[:route]}' contains invalid load-balancing algorithm '#{lb_algo}'. \ +Valid algorithms: '#{RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.join(', ')}'") + end + end + def routes_are_uris return if errors[:routes].present? diff --git a/lib/cloud_controller/app_manifest/manifest_route.rb b/lib/cloud_controller/app_manifest/manifest_route.rb index 9c17fc08a94..9449328819e 100644 --- a/lib/cloud_controller/app_manifest/manifest_route.rb +++ b/lib/cloud_controller/app_manifest/manifest_route.rb @@ -23,12 +23,6 @@ def self.parse(route, options=nil) end def valid? - if @attrs[:options] && !@attrs[:options].empty? - return false if @attrs[:options].keys.any? { |key| RouteOptionsMessage::VALID_ROUTE_OPTIONS.exclude?(key) } - # validation for loadbalancing algorithm - 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] 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 289eae0982a..d685a5c7746 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 @@ -116,16 +116,6 @@ 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 diff --git a/spec/unit/messages/manifest_routes_update_message_spec.rb b/spec/unit/messages/manifest_routes_update_message_spec.rb index c62f7bf567a..03135aff1c7 100644 --- a/spec/unit/messages/manifest_routes_update_message_spec.rb +++ b/spec/unit/messages/manifest_routes_update_message_spec.rb @@ -238,6 +238,7 @@ module VCAP::CloudController msg = ManifestRoutesUpdateMessage.new(body) expect(msg.valid?).to be(false) + expect(msg.errors.errors.length).to eq(1) expect(msg.errors.full_messages).to include("Route 'existing.example.com' contains invalid route option 'invalid'. Valid keys: 'loadbalancing-algorithm'") end end @@ -275,6 +276,7 @@ module VCAP::CloudController msg = ManifestRoutesUpdateMessage.new(body) expect(msg.valid?).to be(false) + expect(msg.errors.errors.length).to eq(1) expect(msg.errors.full_messages).to include("Route 'existing.example.com' contains invalid load-balancing algorithm 'sushi'. \ Valid algorithms: 'round-robin, least-connections'") end