Update manifest diff to only display meaningful changes

[#173802711]

Co-authored-by: Jenna Goldstrich <jgoldstrich@pivotal.io>
Co-authored-by: Mona Mohebbi <mmohebbi@pivotal.io>
Co-authored-by: Teal Stannard <tstannard@pivotal.io>
Co-authored-by: Nick Webb <nwebb@pivotal.io>

Author: 4 people

app/actions/space_diff_manifest.rb

@@ -3,10 +3,62 @@
 
 module VCAP::CloudController
   class SpaceDiffManifest
+    def self.filter_manifest_app_hash(manifest_app_hash)
+      if manifest_app_hash.key? 'sidecars'
+        manifest_app_hash['sidecars'] = manifest_app_hash['sidecars'].map do |hash|
+          hash.slice(
+            'name',
+            'command',
+            'process_types',
+            'memory'
+          )
+        end
+        manifest_app_hash = manifest_app_hash.except('sidecars') if manifest_app_hash['sidecars'] == [{}]
+      end
+      if manifest_app_hash.key? 'processes'
+        manifest_app_hash['processes'] = manifest_app_hash['processes'].map do |hash|
+          hash.slice(
+            'type',
+            'command',
+            'disk_quota',
+            'health-check-http-endpoint',
+            'health-check-invocation-timeout',
+            'health-check-type',
+            'instances',
+            'memory',
+            'timeout'
+          )
+        end
+        manifest_app_hash = manifest_app_hash.except('processes') if manifest_app_hash['processes'] == [{}]
+      end
+
+      if manifest_app_hash.key? 'services'
+        manifest_app_hash['services'] = manifest_app_hash['services'].map do |hash|
+          hash.slice(
+            'name',
+            'parameters'
+          )
+        end
+        manifest_app_hash = manifest_app_hash.except('services') if manifest_app_hash['services'] == [{}]
+      end
+
+      if manifest_app_hash.key? 'metadata'
+        manifest_app_hash['metadata'] = manifest_app_hash['metadata'].map do |hash|
+          hash.slice(
+            'labels',
+            'annotations'
+          )
+        end
+        manifest_app_hash = manifest_app_hash.except('metadata') if manifest_app_hash['metadata'] == [{}]
+      end
+      manifest_app_hash
+    end
+
     def self.generate_diff(app_manifests, space)
       json_diff = []
-
+      recognized_top_level_keys = NamedAppManifestMessage.allowed_keys.map(&:to_s)
       app_manifests.each_with_index do |manifest_app_hash, index|
+        manifest_app_hash = SpaceDiffManifest.filter_manifest_app_hash(manifest_app_hash)
         existing_app = space.app_models.find { |app| app.name == manifest_app_hash['name'] }
 
         if existing_app.nil?
@@ -23,13 +75,16 @@ def self.generate_diff(app_manifests, space)
         end
 
         manifest_app_hash.each do |key, value|
+          next unless recognized_top_level_keys.include?(key)
+
           existing_value = existing_app_hash[key]
 
           key_diffs = JsonDiff.diff(
             existing_value,
             value,
             include_was: true,
           )
+
           key_diffs.each do |diff|
             diff['path'] = "/applications/#{index}/#{key}" + diff['path']
 

spec/request/space_manifests_spec.rb

@@ -538,7 +538,6 @@
       let(:diff_json) do
         {
           diff: a_collection_containing_exactly(
-            { op: 'add', path: '/applications/0/new-key', value: 'hoh' },
             { op: 'replace', path: '/applications/0/stack', was: process1.stack.name, value: 'big brother' },
           )
         }

spec/unit/actions/space_diff_manifest_spec.rb

@@ -49,15 +49,25 @@ module VCAP::CloudController
         end
       end
 
+      context 'when there is an unrecognized top-level field' do
+        before do
+          default_manifest['applications'][0]['foo'] = 'bar'
+        end
+
+        it 'returns an empty diff' do
+          expect(subject).to eq([])
+        end
+      end
+
       context 'when there are changes in the manifest' do
         before do
-          default_manifest['applications'][0]['new-key'] = 'hoh'
+          default_manifest['applications'][0]['random_route'] = true
           default_manifest['applications'][0]['stack'] = 'big brother'
         end
 
         it 'returns the correct diff' do
           expect(subject).to match_array([
-            { 'op' => 'add', 'path' => '/applications/0/new-key', 'value' => 'hoh' },
+            { 'op' => 'add', 'path' => '/applications/0/random_route', 'value' => true },
             { 'op' => 'replace', 'path' => '/applications/0/stack', 'was' => process1.stack.name, 'value' => 'big brother' },
           ])
         end
@@ -75,7 +85,7 @@ module VCAP::CloudController
         end
       end
 
-      context 'when there is a difference in a nested array' do
+      context 'when there is a change inside of a nested array' do
         before do
           default_manifest['applications'][0]['sidecars'] = [
             { 'name' => 'sidecar1', 'command' => 'bundle exec rake lol', 'process_types' => ['web', 'worker'] }
@@ -95,6 +105,32 @@ module VCAP::CloudController
         end
       end
 
+      context 'when there is an unrecognized field in a nested hash' do
+        before do
+          default_manifest['applications'][0]['processes'][0]['foo'] = 'bar'
+          default_manifest['applications'][0]['services'] = [
+            {
+              'foo' => 'bar'
+            }
+          ]
+
+          default_manifest['applications'][0]['metadata'] = [
+            {
+              'foo' => 'bar'
+            }
+          ]
+          default_manifest['applications'][0]['sidecars'] = [
+            {
+              'foo' => 'bar'
+            }
+          ]
+        end
+
+        it 'returns an empty diff' do
+          expect(subject).to eq([])
+        end
+      end
+
       context 'when there is a new app' do
         let(:app_manifests) do
           [