Adjust behaviour to new decisions:

options default: {}
API:
 options is not nullable
 specific option is additive
 specific option is nullable
 empty hash does not change anything (additive)
 get empty options -> {}
manifest:
 options and specific option is not nullable

Author: a18e

app/actions/route_update.rb

@@ -3,12 +3,10 @@ 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?
+          route.options = if route.options.nil?
                             message.options
                           else
-                            route.options.merge(message.options)
+                            route.options.symbolize_keys.merge(message.options).compact
                           end
         end
 

app/messages/route_options_message.rb

@@ -6,7 +6,7 @@ class RouteOptionsMessage < BaseMessage
     VALID_ROUTE_OPTIONS = %i[lb_algo].freeze
     VALID_LOADBALANCING_ALGORITHMS = %w[round-robin least-connections].freeze
 
-    register_allowed_keys %i[lb_algo]
+    register_allowed_keys VALID_ROUTE_OPTIONS
     validates_with NoAdditionalKeysValidator
     validates :lb_algo,
               inclusion: { in: VALID_LOADBALANCING_ALGORITHMS, message: "'%<value>s' is not a supported load-balancing algorithm" },

app/messages/validators.rb

@@ -242,6 +242,12 @@ def validate(record)
 
   class OptionsValidator < ActiveModel::Validator
     def validate(record)
+      # If the options key is provided, it must not be nil
+      if record.respond_to?(:options) && record.options.nil?
+        record.errors.add(:options, message: "'options' must not be explicitly set to null")
+        return
+      end
+
       # Empty option hashes are allowed, so we skip further validation
       record.options.blank? && return
 

db/migrations/20241105000000_add_options_to_route.rb

@@ -2,7 +2,7 @@
   up do
     alter_table(:routes) do
       # rubocop:disable Migration/IncludeStringSize
-      add_column :options, String, text: true, default: nil
+      add_column :options, String, text: true, default: '{}'
       # rubocop:enable Migration/IncludeStringSize
     end
   end

lib/cloud_controller/app_manifest/manifest_route.rb

@@ -28,6 +28,7 @@ def self.parse(route, options=nil)
     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
 

spec/unit/actions/route_update_spec.rb

@@ -36,9 +36,6 @@ module VCAP::CloudController
         metadata: {
           labels: new_labels,
           annotations: new_annotations
-        },
-        options: {
-          lb_algo: 'round-robin'
         }
       }
     end
@@ -136,98 +133,94 @@ module VCAP::CloudController
         end
       end
     end
+
     describe '#update options' do
-      context 'when the route has no existing metadata' do
-        context 'when no metadata is specified' do
+      context 'when the route has no existing options' do
+        context 'when no options are specified' do
           let(:body) do
             {}
           end
 
-          it 'adds no metadata' do
+          it 'adds no options' do
             expect(message).to be_valid
             subject.update(route:, message:)
             route.reload
-            expect(route.labels.size).to eq(0)
-            expect(route.annotations.size).to eq(0)
+            expect(route.options).to eq({})
           end
         end
 
-        context 'when metadata is specified' do
-          it 'updates the route metadata' do
+        context 'when an option is specified' do
+          let(:body) do
+            {
+              options: {
+                lb_algo: 'round-robin'
+              }
+            }
+          end
+
+          it 'adds the route option' do
             expect(message).to be_valid
             subject.update(route:, message:)
 
             route.reload
-            expect(route).to have_labels(
-              { prefix: 'doordash.com', key_name: 'potato', value: 'mashed' },
-              { prefix: nil, key_name: 'fruit', value: 'strawberries' },
-              { prefix: nil, key_name: 'cuisine', value: 'thai' }
-            )
-            expect(route).to have_annotations(
-              { key_name: 'potato', value: 'idaho' }
-            )
             expect(route[:options]).to eq('{"lb_algo":"round-robin"}')
           end
         end
       end
 
-      context 'when the route has existing metadata' do
+      context 'when the route has existing options' do
         before do
-          VCAP::CloudController::LabelsUpdate.update(route, old_labels, VCAP::CloudController::RouteLabelModel)
-          VCAP::CloudController::AnnotationsUpdate.update(route, old_annotations, VCAP::CloudController::RouteAnnotationModel)
           route[:options] = '{"lb_algo": "round-robin"}'
         end
 
-        context 'when no metadata is specified' do
+        context 'when no options are specified' do
           let(:body) do
             {}
           end
 
-          it 'adds no metadata' do
+          it 'modifies nothing' do
             expect(message).to be_valid
             subject.update(route:, message:)
             route.reload
-            expect(route).to have_labels(
-              { prefix: nil, key_name: 'fruit', value: 'peach' },
-              { prefix: nil, key_name: 'clothing', value: 'blouse' }
-            )
-            expect(route).to have_annotations(
-              { key_name: 'potato', value: 'celandine' },
-              { key_name: 'beet', value: 'formanova' }
-            )
             expect(route.options).to include({ 'lb_algo' => 'round-robin' })
           end
         end
 
-        context 'when metadata is specified' do
+        context 'when an option is specified' do
           let(:body) do
             {
-              metadata: {
-                labels: new_labels.merge(fruit: nil, newstuff: 'here'),
-                annotations: new_annotations.merge(beet: nil, asparagus: 'crunchy')
+              options: {
+                lb_algo: 'least-connections'
               }
             }
           end
 
-          it 'updates some, deletes nils, leaves unspecified fields alone' do
+          it 'updates the option' do
             expect(message).to be_valid
             subject.update(route:, message:)
             route.reload
 
-            expect(route).to have_labels(
-              { prefix: 'doordash.com', key_name: 'potato', value: 'mashed' },
-              { prefix: nil, key_name: 'clothing', value: 'blouse' },
-              { prefix: nil, key_name: 'newstuff', value: 'here' },
-              { prefix: nil, key_name: 'cuisine', value: 'thai' }
-            )
-            expect(route).to have_annotations(
-              { key_name: 'potato', value: 'idaho' },
-              { key_name: 'asparagus', value: 'crunchy' }
-            )
+            expect(route.options).to include({ 'lb_algo' => 'least-connections' })
+          end
+        end
+
+        context 'when the option value is set to null' do
+          let(:body) do
+            {
+              options: {
+                lb_algo: nil
+              }
+            }
+          end
+
+          it 'removes this option' do
+            expect(message).to be_valid
+            subject.update(route:, message:)
+            route.reload
+            expect(route.options).to eq({})
           end
         end
       end
     end
-
   end
 end

spec/unit/lib/cloud_controller/app_manifest/manifest_route_spec.rb

@@ -117,6 +117,16 @@ module VCAP::CloudController
           end
         end
 
+        context 'when options is set to null' do
+          let(:route) { 'http://example.com' }
+
+          it 'is invalid' do
+            options = nil
+            manifest_route = ManifestRoute.parse(route, options)
+            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' } }
@@ -140,7 +150,7 @@ module VCAP::CloudController
                                       ],
                                       port: nil,
                                       path: '/path',
-                                      options: nil
+                                      options: {}
                                     })
       end
 
@@ -153,7 +163,7 @@ module VCAP::CloudController
                                       ],
                                       port: nil,
                                       path: '/path',
-                                      options: nil
+                                      options: {}
                                     })
       end
 
@@ -167,7 +177,7 @@ module VCAP::CloudController
                                       ],
                                       port: nil,
                                       path: '/path',
-                                      options: nil
+                                      options: {}
                                     })
       end
 
@@ -181,7 +191,7 @@ module VCAP::CloudController
                                       ],
                                       port: 1234,
                                       path: '',
-                                      options: nil
+                                      options: {}
                                     })
       end
 

spec/unit/messages/manifest_routes_update_message_spec.rb

@@ -225,6 +225,22 @@ module VCAP::CloudController
         end
       end
 
+      context 'when a route contains route-options: null' do
+        let(:body) do
+          { 'routes' =>
+              [
+                { 'route' => 'existing.example.com',
+                  'options' => nil }
+              ] }
+        end
+
+        it 'returns true' do
+          msg = ManifestRoutesUpdateMessage.new(body)
+
+          expect(msg.valid?).to be(false)
+        end
+      end
+
       context 'when a route contains invalid route options' do
         let(:body) do
           { 'routes' =>
@@ -241,6 +257,24 @@ module VCAP::CloudController
         end
       end
 
+      context 'when a route contains a route option key: null' do
+        let(:body) do
+          { 'routes' =>
+              [
+                { 'route' => 'existing.example.com',
+                  'options' => {
+                    'loadbalancing-algorithm' => nil
+                  } }
+              ] }
+        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' =>

spec/unit/messages/route_update_message_spec.rb

@@ -28,9 +28,9 @@ module VCAP::CloudController
         expect(message).to be_valid
       end
 
-      it 'accepts options: nil to unset options' do
+      it 'does not accept options: nil' do
         message = RouteUpdateMessage.new(params.merge(options: nil))
-        expect(message).to be_valid
+        expect(message).not_to be_valid
       end
 
       it 'accepts lb_algo: nil to unset load-balancing algorithm' do