Skip to content

Commit 9d54abd

Browse files
committed
Fix route options serialization to preserve explicitly removed values
1 parent e07777b commit 9d54abd

4 files changed

Lines changed: 24 additions & 7 deletions

File tree

app/models/runtime/route.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,9 @@ def options_with_serialization=(opts)
7474
cleaned_opts = remove_hash_options_for_non_hash_loadbalancing(opts)
7575
rounded_opts = round_hash_balance_to_one_decimal(cleaned_opts)
7676
normalized_opts = normalize_hash_balance_to_string(rounded_opts)
77-
# Remove nil values after all processing
78-
normalized_opts = normalized_opts.compact if normalized_opts.is_a?(Hash)
77+
# Convert nil values to empty strings to signal explicit removal to downstream consumers (e.g. gorouter),
78+
# so they can distinguish "option was explicitly removed" from "option was never set".
79+
normalized_opts = normalized_opts.transform_values { |v| v.nil? ? '' : v } if normalized_opts.is_a?(Hash)
7980
self.options_without_serialization = Oj.dump(normalized_opts)
8081
end
8182

spec/unit/actions/manifest_route_update_spec.rb

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -650,8 +650,7 @@ module VCAP::CloudController
650650
ManifestRouteUpdate.update(app.guid, message, user_audit_info)
651651

652652
route.reload
653-
expect(route.options).to eq({})
654-
expect(route.options).not_to have_key('loadbalancing')
653+
expect(route.options).to eq({ 'loadbalancing' => '' })
655654
expect(route.options).not_to have_key('hash_header')
656655
expect(route.options).not_to have_key('hash_balance')
657656
end

spec/unit/actions/route_update_spec.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ module VCAP::CloudController
209209
expect(message).to be_valid
210210
subject.update(route:, message:)
211211
route.reload
212-
expect(route.options).to eq({})
212+
expect(route.options).to eq({ 'loadbalancing' => '' })
213213
end
214214

215215
it 'notifies the backend' do
@@ -299,7 +299,7 @@ module VCAP::CloudController
299299
subject.update(route:, message:)
300300
route.reload
301301
expect(route.options).to include({ 'loadbalancing' => 'hash', 'hash_header' => 'foobar' })
302-
expect(route.options).not_to have_key('hash_balance')
302+
expect(route.options['hash_balance']).to eq('')
303303
end
304304

305305
it 'notifies the backend' do
@@ -502,7 +502,7 @@ module VCAP::CloudController
502502
expect(message).to be_valid
503503
subject.update(route:, message:)
504504
route.reload
505-
expect(route.options).to eq({})
505+
expect(route.options).to eq({ 'loadbalancing' => '' })
506506
end
507507

508508
it 'notifies the backend' do

spec/unit/models/runtime/route_spec.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1346,6 +1346,23 @@ module VCAP::CloudController
13461346
end
13471347
end
13481348

1349+
context 'when setting an option value to nil' do
1350+
it 'serializes nil values as empty strings to signal explicit removal' do
1351+
route = Route.make(
1352+
host: 'test-route',
1353+
domain: domain,
1354+
space: space,
1355+
options: { loadbalancing: 'round-robin' }
1356+
)
1357+
1358+
route.update(options: { loadbalancing: nil })
1359+
route.reload
1360+
1361+
parsed_options = Oj.load(route.options_without_serialization)
1362+
expect(parsed_options['loadbalancing']).to eq('')
1363+
end
1364+
end
1365+
13491366
context 'when using string keys instead of symbols' do
13501367
it 'still removes hash options for non-hash loadbalancing' do
13511368
route = Route.make(

0 commit comments

Comments
 (0)