Merge pull request #49033 from terminalmage/issue48996

Fix file.get_diff for remote files

salt/modules/file.py

@@ -4989,7 +4989,7 @@ def get_diff(file1,
         )
 
     args = []
-    for filename in files:
+    for filename in paths:
         try:
             with salt.utils.files.fopen(filename, 'rb') as fp_:
                 args.append(fp_.readlines())
@@ -5007,12 +5007,12 @@ def get_diff(file1,
         elif not show_changes:
             ret = '<show_changes=False>'
         else:
-            bdiff = _binary_replace(*files)
+            bdiff = _binary_replace(*paths)  # pylint: disable=no-value-for-parameter
             if bdiff:
                 ret = bdiff
             else:
                 if show_filenames:
-                    args.extend(files)
+                    args.extend(paths)
                 ret = __utils__['stringutils.get_diff'](*args)
         return ret
     return ''

tests/unit/modules/test_file.py

@@ -910,6 +910,17 @@ class FileModuleTestCase(TestCase, LoaderModuleMockMixin):
             baz
             яйца
             ''')
+        diff_result = textwrap.dedent('''\
+            --- text1
+            +++ text2
+            @@ -1,4 +1,4 @@
+             foo
+             bar
+             baz
+            -спам
+            +яйца
+            ''')
+
         # The below two variables are 8 bytes of data pulled from /dev/urandom
         binary1 = b'\xd4\xb2\xa6W\xc6\x8e\xf5\x0f'
         binary2 = b',\x13\x04\xa5\xb0\x12\xdf%'
@@ -940,7 +951,7 @@ class FileModuleTestCase(TestCase, LoaderModuleMockMixin):
         # pylint: enable=no-self-argument
 
         fopen = MagicMock(side_effect=lambda x, *args, **kwargs: MockFopen(x))
-        cache_file = MagicMock(side_effect=lambda x, *args, **kwargs: x)
+        cache_file = MagicMock(side_effect=lambda x, *args, **kwargs: x.split('/')[-1])
 
         # Mocks for __utils__['files.is_text']
         mock_text_text = MagicMock(side_effect=[True, True])
@@ -960,19 +971,18 @@ class FileModuleTestCase(TestCase, LoaderModuleMockMixin):
 
                 # Non-identical files
                 ret = filemod.get_diff('text1', 'text2')
-                self.assertEqual(
+                self.assertEqual(ret, diff_result)
-                    ret,
+
-                    textwrap.dedent('''\
+                # Repeat the above test with remote file paths. The expectation
-                        --- text1
+                # is that the cp.cache_file mock will ensure that we are not
-                        +++ text2
+                # trying to do an fopen on the salt:// URL, but rather the
-                        @@ -1,4 +1,4 @@
+                # "cached" file path we've mocked.
-                         foo
+                with patch.object(filemod, '_binary_replace',
-                         bar
+                                  MagicMock(return_value='')):
-                         baz
+                    ret = filemod.get_diff('salt://text1', 'salt://text1')
-                        -спам
+                    self.assertEqual(ret, '')
-                        +яйца
+                    ret = filemod.get_diff('salt://text1', 'salt://text2')
-                        ''')
+                    self.assertEqual(ret, diff_result)
-                )
 
             # Test diffing two binary files
             with patch.dict(filemod.__utils__, {'files.is_text': mock_bin_bin}):