Skip to content

Commit

Permalink
Remove route option validations from manifest_route
Browse files Browse the repository at this point in the history
  • Loading branch information
hoffmaen committed Dec 4, 2024
1 parent 5eff0c2 commit 7eaa6fc
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 54 deletions.
75 changes: 37 additions & 38 deletions app/messages/manifest_routes_update_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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?

Expand Down
6 changes: 0 additions & 6 deletions lib/cloud_controller/app_manifest/manifest_route.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
10 changes: 0 additions & 10 deletions spec/unit/lib/cloud_controller/app_manifest/manifest_route_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions spec/unit/messages/manifest_routes_update_message_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 7eaa6fc

Please sign in to comment.