Merge pull request #39762 from terminalmage/issue39751

Fix regression in file.get_managed

salt/modules/file.py

@@ -3768,13 +3768,13 @@ def get_managed(
         if cached_dest and (source_hash or skip_verify):
             htype = source_sum.get('hash_type', 'sha256')
             cached_sum = get_hash(cached_dest, form=htype)
-            if cached_sum != source_sum['hsum']:
-                cache_refetch = True
-            elif skip_verify:
+            if skip_verify:
                 # prev: if skip_verify or cached_sum == source_sum['hsum']:
                 # but `cached_sum == source_sum['hsum']` is elliptical as prev if
                 sfn = cached_dest
                 source_sum = {'hsum': cached_sum, 'hash_type': htype}
+            elif cached_sum != source_sum['hsum']:
+                cache_refetch = True
 
         # If we didn't have the template or remote file, let's get it
         # Similarly when the file has been updated and the cache has to be refreshed

salt/states/archive.py

@@ -897,17 +897,6 @@ def extracted(name,
                 ret['comment'] = '\n'.join([str(x) for x in file_result])
             return ret
 
-        # Get actual state result. The state.single return is a single-element
-        # dictionary with the state's unique ID at the top level, and its value
-        # being the state's return dictionary. next(iter(dict_name)) will give
-        # us the value of the first key, so
-        # file_result[next(iter(file_result))] will give us the results of the
-        # state.single we just ran.
-        try:
-            file_result = file_result[next(iter(file_result))]
-        except AttributeError:
-            pass
-
         try:
             if not file_result['result']:
                 log.debug('failed to download {0}'.format(source_match))

tests/integration/states/file.py

@@ -9,15 +9,22 @@ from __future__ import absolute_import
 from distutils.version import LooseVersion
 import errno
 import glob
+import logging
 import os
 import re
 import sys
 import shutil
+import socket
 import stat
 import tempfile
 import textwrap
+import threading
+import tornado.ioloop
+import tornado.web
 import filecmp
 
+log = logging.getLogger(__name__)
+
 # Import 3rd-party libs
 from salt.ext.six.moves import range  # pylint: disable=import-error,redefined-builtin
 
@@ -2392,6 +2399,104 @@ class FileTest(integration.ModuleCase, integration.SaltReturnAssertsMixIn):
             if check_file:
                 self.run_function('file.remove', [file])
 
+
+PORT = 9999
+FILE_SOURCE = 'http://localhost:{0}/grail/scene33'.format(PORT)
+FILE_HASH = 'd2feb3beb323c79fc7a0f44f1408b4a3'
+STATE_DIR = os.path.join(integration.FILES, 'file', 'base')
+
+
+class RemoteFileTest(integration.ModuleCase, integration.SaltReturnAssertsMixIn):
+    '''
+    Uses a local tornado webserver to test http(s) file.managed states with and
+    without skip_verify
+    '''
+    @classmethod
+    def webserver(cls):
+        '''
+        method to start tornado static web app
+        '''
+        application = tornado.web.Application([
+            (r'/(.*)', tornado.web.StaticFileHandler, {'path': STATE_DIR})
+        ])
+        application.listen(PORT)
+        tornado.ioloop.IOLoop.instance().start()
+
+    @classmethod
+    def setUpClass(cls):
+        '''
+        start tornado app on thread and wait until it is running
+        '''
+        cls.server_thread = threading.Thread(target=cls.webserver)
+        cls.server_thread.daemon = True
+        cls.server_thread.start()
+        # check if tornado app is up
+        port_closed = True
+        while port_closed:
+            sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+            result = sock.connect_ex(('127.0.0.1', PORT))
+            if result == 0:
+                port_closed = False
+
+    @classmethod
+    def tearDownClass(cls):
+        tornado.ioloop.IOLoop.instance().stop()
+        cls.server_thread.join()
+
+    def setUp(self):
+        fd_, self.name = tempfile.mkstemp(dir=integration.TMP)
+        try:
+            os.close(fd_)
+        except OSError as exc:
+            if exc.errno != errno.EBADF:
+                raise exc
+        # Remove the file that mkstemp just created so that the states can test
+        # creating a new file instead of a diff from a zero-length file.
+        self.tearDown()
+
+    def tearDown(self):
+        try:
+            os.remove(self.name)
+        except OSError as exc:
+            if exc.errno != errno.ENOENT:
+                raise exc
+
+    def test_file_managed_http_source_no_hash(self):
+        '''
+        Test a remote file with no hash
+        '''
+        ret = self.run_state('file.managed',
+                             name=self.name,
+                             source=FILE_SOURCE,
+                             skip_verify=False)
+        log.debug('ret = %s', ret)
+        # This should fail because no hash was provided
+        self.assertSaltFalseReturn(ret)
+
+    def test_file_managed_http_source(self):
+        '''
+        Test a remote file with no hash
+        '''
+        ret = self.run_state('file.managed',
+                             name=self.name,
+                             source=FILE_SOURCE,
+                             source_hash=FILE_HASH,
+                             skip_verify=False)
+        log.debug('ret = %s', ret)
+        self.assertSaltTrueReturn(ret)
+
+    def test_file_managed_http_source_skip_verify(self):
+        '''
+        Test a remote file using skip_verify
+        '''
+        ret = self.run_state('file.managed',
+                             name=self.name,
+                             source=FILE_SOURCE,
+                             skip_verify=True)
+        log.debug('ret = %s', ret)
+        self.assertSaltTrueReturn(ret)
+
+
 if __name__ == '__main__':
     from integration import run_tests
     run_tests(FileTest)