Skip to content

Commit 11f090a

Browse files
authored
Support Hash-Based Routing (#4746)
* Add loadbalancing option hash, and hash_header/hash_balance. Add validations * Add basic options validation to manifest_routes_update_message * Add basic tests for manifest route updates * Simplify validations, improve error messages, adjust tests * Add validation for hash balance being a float * Have consistent validations in manifest_routes_updates_message and route_options_message * Minor refactoring * Add check for hash_balance value being 0 or >=1.1 * Transform hash_balance to string in the route model before saving * Validate the route merged with the route update from the message. * Attempt to forward errors in route_update validation to the client * Add route options changes to app event * Add new hash options to mnifest route * Add new hash options to route properties presenter * Move additional validation to route model * Move final options cleanup to route model * Proper error validation and forwarding * Forward proper error message on route updates * Add some logging to find the manifest issue * Add more logging * Logging in app_manifest_message * Test a few things * Test a few things pt 2 * Test a few things pt 3 * Test a few things pt 4 * Test a few things pt 5 * Test a few things pt 6 * Cleanup logging * Use symbolized_keys in route_update * WIP * Add more validations and tests * Add more tests for route_update * Add more tests * Some test fixes and cleanup * Use blank? instead of nil? for loadbalancing check * Fix rubocop issues * Fix rubocop spec issues * Refactor to resolve rubocop complexity findings * Remove test file * Move all route option message tests from validator spec to new spec file * Introduce stub config in validator spec to fix execution of standalone spec test * Fix typos and validation logic * Fix validation function name inconsistencies * Fix remaining issues found in review * revert change to validators spec
1 parent 9c9a9d5 commit 11f090a

23 files changed

+2339
-107
lines changed

app/actions/manifest_route_update.rb

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ def update(app_guid, message, user_audit_info)
5252
)
5353
end
5454
end
55-
rescue Sequel::ValidationFailed, RouteCreate::Error => e
55+
rescue Sequel::ValidationFailed, RouteCreate::Error, RouteUpdate::Error => e
5656
raise InvalidRoute.new(e.message)
5757
end
5858

@@ -94,9 +94,7 @@ def find_or_create_valid_route(app, manifest_route, user_audit_info)
9494
)
9595
elsif !route.available_in_space?(app.space)
9696
raise InvalidRoute.new('Routes cannot be mapped to destinations in different spaces')
97-
elsif manifest_route[:options] && route[:options] != manifest_route[:options]
98-
# remove nil values from options
99-
manifest_route[:options] = manifest_route[:options].compact
97+
elsif manifest_route[:options]
10098
message = RouteUpdateMessage.new({
10199
'options' => manifest_route[:options]
102100
})

app/actions/route_create.rb

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,22 +68,34 @@ def route_resource_manager
6868
end
6969

7070
def validation_error!(error, host, path, port, space, domain)
71-
error!("Invalid domain. Domain '#{domain.name}' is not available in organization '#{space.organization.name}'.") if error.errors.on(:domain)&.include?(:invalid_relation)
71+
validation_error_domain!(error, space, domain)
72+
validation_error_quota!(error, space)
73+
validation_error_route!(error)
74+
validation_error_routing_api!(error)
75+
validation_error_host!(error, host, domain)
76+
validation_error_path!(error, host, path, domain)
77+
validation_error_port!(error, host, port, domain)
7278

73-
error!("Routes quota exceeded for space '#{space.name}'.") if error.errors.on(:space)&.include?(:total_routes_exceeded)
79+
error!(error.message)
80+
end
7481

75-
error!("Reserved route ports quota exceeded for space '#{space.name}'.") if error.errors.on(:space)&.include?(:total_reserved_route_ports_exceeded)
82+
def validation_error_domain!(error, space, domain)
83+
return unless error.errors.on(:domain)&.include?(:invalid_relation)
7684

77-
error!("Routes quota exceeded for organization '#{space.organization.name}'.") if error.errors.on(:organization)&.include?(:total_routes_exceeded)
85+
error!("Invalid domain. Domain '#{domain.name}' is not available in organization '#{space.organization.name}'.")
86+
end
7887

88+
def validation_error_quota!(error, space)
89+
error!("Routes quota exceeded for space '#{space.name}'.") if error.errors.on(:space)&.include?(:total_routes_exceeded)
90+
error!("Reserved route ports quota exceeded for space '#{space.name}'.") if error.errors.on(:space)&.include?(:total_reserved_route_ports_exceeded)
91+
error!("Routes quota exceeded for organization '#{space.organization.name}'.") if error.errors.on(:organization)&.include?(:total_routes_exceeded)
7992
error!("Reserved route ports quota exceeded for organization '#{space.organization.name}'.") if error.errors.on(:organization)&.include?(:total_reserved_route_ports_exceeded)
93+
end
8094

81-
validation_error_routing_api!(error)
82-
validation_error_host!(error, host, domain)
83-
validation_error_path!(error, host, path, domain)
84-
validation_error_port!(error, host, port, domain)
95+
def validation_error_route!(error)
96+
return unless error.errors.on(:route)&.include?(:hash_header_missing)
8597

86-
error!(error.message)
98+
error!('Hash header must be present when loadbalancing is set to hash.')
8799
end
88100

89101
def validation_error_routing_api!(error)

app/actions/route_update.rb

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
module VCAP::CloudController
22
class RouteUpdate
3+
class Error < StandardError
4+
end
5+
36
def update(route:, message:)
47
Route.db.transaction do
5-
route.options = route.options.symbolize_keys.merge(message.options).compact if message.requested?(:options)
8+
route.options = route.options.symbolize_keys.merge(message.options) if message.requested?(:options)
69
route.save
710
MetadataUpdate.update(route, message)
811
end
@@ -13,6 +16,18 @@ def update(route:, message:)
1316
end
1417
end
1518
route
19+
rescue Sequel::ValidationFailed => e
20+
validation_error!(e)
21+
end
22+
23+
private
24+
25+
def validation_error!(error)
26+
# Handle hash_header validation error for hash loadbalancing
27+
raise Error.new('Hash header must be present when loadbalancing is set to hash.') if error.errors.on(:route)&.include?(:hash_header_missing)
28+
29+
# Fallback for any other validation errors
30+
raise Error.new(error.message)
1631
end
1732
end
1833
end

app/controllers/v3/routes_controller.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ def update
108108
VCAP::CloudController::RouteUpdate.new.update(route:, message:)
109109

110110
render status: :ok, json: Presenters::V3::RoutePresenter.new(route)
111+
rescue RouteUpdate::Error => e
112+
unprocessable!(e.message)
111113
end
112114

113115
def destroy

app/messages/manifest_routes_update_message.rb

Lines changed: 71 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ def contains_non_route_hash_values?(routes)
2828
validate :route_protocols_are_valid, if: proc { |record| record.requested?(:routes) }
2929
validate :route_options_are_valid, if: proc { |record| record.requested?(:routes) }
3030
validate :loadbalancings_are_valid, if: proc { |record| record.requested?(:routes) }
31+
validate :hash_options_are_valid, if: proc { |record| record.requested?(:routes) }
3132
validate :no_route_is_boolean
3233
validate :default_route_is_boolean
3334
validate :random_route_is_boolean
@@ -58,10 +59,10 @@ def route_options_are_valid
5859
end
5960

6061
r[:options].each_key do |key|
61-
RouteOptionsMessage::VALID_MANIFEST_ROUTE_OPTIONS.exclude?(key) &&
62+
RouteOptionsMessage.valid_route_options.exclude?(key) &&
6263
errors.add(:base,
6364
message: "Route '#{r[:route]}' contains invalid route option '#{key}'. \
64-
Valid keys: '#{RouteOptionsMessage::VALID_MANIFEST_ROUTE_OPTIONS.join(', ')}'")
65+
Valid keys: '#{RouteOptionsMessage.valid_route_options.join(', ')}'")
6566
end
6667
end
6768
end
@@ -76,16 +77,81 @@ def loadbalancings_are_valid
7677
unless loadbalancing.is_a?(String)
7778
errors.add(:base,
7879
message: "Invalid value for 'loadbalancing' for Route '#{r[:route]}'; \
79-
Valid values are: '#{RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.join(', ')}'")
80+
Valid values are: '#{RouteOptionsMessage.valid_loadbalancing_algorithms.join(', ')}'")
8081
next
8182
end
82-
RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.exclude?(loadbalancing) &&
83+
RouteOptionsMessage.valid_loadbalancing_algorithms.exclude?(loadbalancing) &&
8384
errors.add(:base,
8485
message: "Cannot use loadbalancing value '#{loadbalancing}' for Route '#{r[:route]}'; \
85-
Valid values are: '#{RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.join(', ')}'")
86+
Valid values are: '#{RouteOptionsMessage.valid_loadbalancing_algorithms.join(', ')}'")
8687
end
8788
end
8889

90+
def hash_options_are_valid
91+
return if errors[:routes].present?
92+
93+
# Only validate hash-specific options when the feature flag is enabled
94+
# If disabled, route_options_are_valid will already report them as invalid
95+
return unless VCAP::CloudController::FeatureFlag.enabled?(:hash_based_routing)
96+
97+
# NOTE: route_options_are_valid already validates that hash_header and hash_balance
98+
# are only allowed when the feature flag is enabled (via valid_route_options).
99+
100+
routes.each do |r|
101+
next unless r.keys.include?(:options) && r[:options].is_a?(Hash)
102+
103+
validate_route_hash_options(r)
104+
end
105+
end
106+
107+
def validate_route_hash_options(route)
108+
options = route[:options]
109+
loadbalancing = options[:loadbalancing]
110+
hash_header = options[:hash_header]
111+
hash_balance = options[:hash_balance]
112+
113+
validate_route_hash_header(route, hash_header)
114+
validate_route_hash_balance(route, hash_balance)
115+
116+
validate_route_hash_options_with_loadbalancing(route, loadbalancing, hash_header, hash_balance)
117+
end
118+
119+
def validate_route_hash_header(route, hash_header)
120+
return unless hash_header.present? && (hash_header.to_s.length > 128)
121+
122+
errors.add(:base, message: "Route '#{route[:route]}': Hash header must be at most 128 characters")
123+
end
124+
125+
def validate_route_hash_balance(route, hash_balance)
126+
return if hash_balance.blank?
127+
128+
if hash_balance.to_s.length > 16
129+
errors.add(:base, message: "Route '#{route[:route]}': Hash balance must be at most 16 characters")
130+
return
131+
end
132+
133+
validate_route_hash_balance_numeric(route, hash_balance)
134+
end
135+
136+
def validate_route_hash_balance_numeric(route, hash_balance)
137+
balance_float = Float(hash_balance)
138+
# Must be either 0 or >= 1.1 and <= 10.0
139+
errors.add(:base, message: "Route '#{route[:route]}': Hash balance must be either 0 or between 1.1 and 10.0") unless balance_float == 0 || balance_float.between?(1.1, 10)
140+
rescue ArgumentError, TypeError
141+
errors.add(:base, message: "Route '#{route[:route]}': Hash balance must be a numeric value")
142+
end
143+
144+
def validate_route_hash_options_with_loadbalancing(route, loadbalancing, hash_header, hash_balance)
145+
# When loadbalancing is explicitly set to non-hash value, hash options are not allowed
146+
if hash_header.present? && loadbalancing.present? && loadbalancing != 'hash'
147+
errors.add(:base, message: "Route '#{route[:route]}': Hash header can only be set when loadbalancing is hash")
148+
end
149+
150+
return unless hash_balance.present? && loadbalancing.present? && loadbalancing != 'hash'
151+
152+
errors.add(:base, message: "Route '#{route[:route]}': Hash balance can only be set when loadbalancing is hash")
153+
end
154+
89155
def routes_are_uris
90156
return if errors[:routes].present?
91157

app/messages/route_options_message.rb

Lines changed: 78 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,85 @@
22

33
module VCAP::CloudController
44
class RouteOptionsMessage < BaseMessage
5-
VALID_MANIFEST_ROUTE_OPTIONS = %i[loadbalancing].freeze
6-
VALID_ROUTE_OPTIONS = %i[loadbalancing].freeze
7-
VALID_LOADBALANCING_ALGORITHMS = %w[round-robin least-connection].freeze
5+
# Register all possible keys upfront so attr_accessors are created
6+
register_allowed_keys %i[loadbalancing hash_header hash_balance]
7+
8+
def self.valid_route_options
9+
options = %i[loadbalancing]
10+
options += %i[hash_header hash_balance] if VCAP::CloudController::FeatureFlag.enabled?(:hash_based_routing)
11+
options.freeze
12+
end
13+
14+
def self.valid_loadbalancing_algorithms
15+
algorithms = %w[round-robin least-connection]
16+
algorithms << 'hash' if VCAP::CloudController::FeatureFlag.enabled?(:hash_based_routing)
17+
algorithms.freeze
18+
end
819

9-
register_allowed_keys VALID_ROUTE_OPTIONS
1020
validates_with NoAdditionalKeysValidator
11-
validates :loadbalancing,
12-
inclusion: { in: VALID_LOADBALANCING_ALGORITHMS, message: "must be one of '#{RouteOptionsMessage::VALID_LOADBALANCING_ALGORITHMS.join(', ')}' if present" },
13-
presence: true,
14-
allow_nil: true
21+
validate :loadbalancing_algorithm_is_valid
22+
validate :route_options_are_valid
23+
validate :hash_options_are_valid
24+
25+
def loadbalancing_algorithm_is_valid
26+
return if loadbalancing.blank?
27+
return if self.class.valid_loadbalancing_algorithms.include?(loadbalancing)
28+
29+
errors.add(:loadbalancing, "must be one of '#{self.class.valid_loadbalancing_algorithms.join(', ')}' if present")
30+
end
31+
32+
def route_options_are_valid
33+
valid_options = self.class.valid_route_options
34+
35+
# Check if any requested options are not in valid_route_options
36+
# Check needs to be done manually, as the set of allowed options may change during runtime (feature flag)
37+
invalid_keys = requested_keys.select do |key|
38+
value = public_send(key) if respond_to?(key)
39+
value.present? && valid_options.exclude?(key)
40+
end
41+
42+
errors.add(:base, "Unknown field(s): '#{invalid_keys.join("', '")}'") if invalid_keys.any?
43+
end
44+
45+
def hash_options_are_valid
46+
# Only validate hash-specific options when the feature flag is enabled
47+
# If disabled, route_options_are_valid will already report them as unknown fields
48+
return unless VCAP::CloudController::FeatureFlag.enabled?(:hash_based_routing)
49+
50+
validate_hash_header_length
51+
validate_hash_balance_value
52+
validate_hash_options_with_loadbalancing
53+
end
54+
55+
def validate_hash_header_length
56+
return unless hash_header.present? && (hash_header.to_s.length > 128)
57+
58+
errors.add(:hash_header, 'must be at most 128 characters')
59+
end
60+
61+
def validate_hash_balance_value
62+
return if hash_balance.blank?
63+
64+
if hash_balance.to_s.length > 16
65+
errors.add(:hash_balance, 'must be at most 16 characters')
66+
return
67+
end
68+
69+
validate_hash_balance_numeric
70+
end
71+
72+
def validate_hash_balance_numeric
73+
balance_float = Float(hash_balance)
74+
# Must be either 0 or >= 1.1 and <= 10.0
75+
errors.add(:hash_balance, 'must be either 0 or between 1.1 and 10.0') unless balance_float == 0 || balance_float.between?(1.1, 10)
76+
rescue ArgumentError, TypeError
77+
errors.add(:hash_balance, 'must be a numeric value')
78+
end
79+
80+
def validate_hash_options_with_loadbalancing
81+
# When loadbalancing is explicitly set to non-hash value, hash options are not allowed
82+
errors.add(:base, 'Hash header can only be set when loadbalancing is hash') if hash_header.present? && loadbalancing.present? && loadbalancing != 'hash'
83+
errors.add(:base, 'Hash balance can only be set when loadbalancing is hash') if hash_balance.present? && loadbalancing.present? && loadbalancing != 'hash'
84+
end
1585
end
1686
end

app/models/runtime/feature_flag.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ class UndefinedFeatureFlagError < StandardError
2323
service_instance_sharing: false,
2424
hide_marketplace_from_unauthenticated_users: false,
2525
resource_matching: true,
26-
route_sharing: false
26+
route_sharing: false,
27+
hash_based_routing: false
2728
}.freeze
2829

2930
ADMIN_SKIPPABLE = %i[

0 commit comments

Comments
 (0)