Merge pull request #52334 from waynew/51879-fix-binary-pillar-return-…

…error

51879 fix binary pillar return error

.gitignore

@@ -2,7 +2,7 @@
 /src
 *.py[co]
 pkg/arch/*.tar.xz
-*.sw[pon]
+*.sw[a-p]
 doc/_build
 dist
 MANIFEST
@@ -84,6 +84,9 @@ tests/unit/templates/roots
 # Pycharm
 .idea
 
+# VS Code
+.vscode
+
 # Ignore the log directory created by tests
 /logs
 tests/integration/cloud/providers/logs

doc/topics/pillar/index.rst

@@ -747,6 +747,34 @@ done:
 .. _`minion config file`: https://github.com/saltstack/salt/tree/develop/doc/ref/configuration/minion.rst
 .. _`master config template`: https://github.com/saltstack/salt/tree/develop/conf/master
 
+Binary Data in the Pillar
+=========================
+
+Salt has partial support for binary pillar data.
+
+.. note::
+
+   There are some situations (such as salt-ssh) where only text (ASCII or
+   Unicode) is allowed.
+
+The simplest way to embed binary data in your pillar is to make use of YAML's
+built-in binary data type, which requires base64 encoded data.
+
+.. code-block:: yaml
+
+    salt_pic: !!binary
+        iVBORw0KGgoAAAANSUhEUgAAAAoAAAAKCAMAAAC67D+PAAAABGdBTUEAALGPC/xhBQAAACBjSFJNAA
+
+Then you can use it as a ``contents_pillar`` in a state:
+
+.. code-block:: yaml
+
+    /tmp/salt.png:
+      file.managed:
+        - contents_pillar: salt_pic
+
+It is also possible to add ASCII-armored encrypted data to pillars, as
+mentioned in the Pillar Encryption section.
 
 Master Config in Pillar
 =======================

salt/renderers/gpg.py

@@ -228,7 +228,11 @@
 log = logging.getLogger(__name__)
 
 GPG_CIPHERTEXT = re.compile(
-    r'-----BEGIN PGP MESSAGE-----.*?-----END PGP MESSAGE-----', re.DOTALL)
+    salt.utils.stringutils.to_bytes(
+        r'-----BEGIN PGP MESSAGE-----.*?-----END PGP MESSAGE-----'
+    ),
+    re.DOTALL,
+)
 
 
 def _get_gpg_exec():
@@ -281,36 +285,40 @@ def _decrypt_ciphertext(cipher):
     proc = Popen(cmd, stdin=PIPE, stdout=PIPE, stderr=PIPE, shell=False)
     decrypted_data, decrypt_error = proc.communicate(input=cipher)
     if not decrypted_data:
-        try:
-            cipher = salt.utils.stringutils.to_unicode(cipher)
-        except UnicodeDecodeError:
-            # decrypted data contains undecodable binary data
-            pass
         log.warning(
-            'Could not decrypt cipher %s, received: %s',
+            'Could not decrypt cipher %r, received: %r',
             cipher,
             decrypt_error
         )
         return cipher
     else:
-        try:
-            decrypted_data = salt.utils.stringutils.to_unicode(decrypted_data)
-        except UnicodeDecodeError:
-            # decrypted data contains undecodable binary data
-            pass
         return decrypted_data
 
 
 def _decrypt_ciphertexts(cipher, translate_newlines=False):
+    to_bytes = salt.utils.stringutils.to_bytes
+    cipher = to_bytes(cipher)
     if translate_newlines:
-        cipher = cipher.replace(r'\n', '\n')
-    ret, num = GPG_CIPHERTEXT.subn(lambda m: _decrypt_ciphertext(m.group()), cipher)
+        cipher = cipher.replace(to_bytes(r'\n'), to_bytes('\n'))
+
+    def replace(match):
+        result = to_bytes(_decrypt_ciphertext(match.group()))
+        return result
+
+    ret, num = GPG_CIPHERTEXT.subn(replace, to_bytes(cipher))
     if num > 0:
         # Remove trailing newlines. Without if crypted value initially specified as a YAML multiline
         # it will conain unexpected trailing newline.
-        return ret.rstrip('\n')
+        ret = ret.rstrip(b'\n')
     else:
-        return cipher
+        ret = cipher
+
+    try:
+        ret = salt.utils.stringutils.to_unicode(ret)
+    except UnicodeDecodeError:
+        # decrypted data contains some sort of binary data - not our problem
+        pass
+    return ret
 
 
 def _decrypt_object(obj, translate_newlines=False):

salt/states/file.py

@@ -2615,11 +2615,8 @@ def managed(name,
                 'to True to allow the managed file to be empty.'
                 .format(contents_id)
             )
-        if isinstance(use_contents, six.binary_type) and b'\0' in use_contents:
-            contents = use_contents
-        elif isinstance(use_contents, six.text_type) and str('\0') in use_contents:
-            contents = use_contents
-        else:
+
+        try:
             validated_contents = _validate_str_list(use_contents)
             if not validated_contents:
                 return _error(
@@ -2634,6 +2631,17 @@ def managed(name,
                     contents += line.rstrip('\n').rstrip('\r') + os.linesep
             if contents_newline and not contents.endswith(os.linesep):
                 contents += os.linesep
+        except UnicodeDecodeError:
+            # Either something terrible happened, or we have binary data.
+            if template:
+                return _error(
+                    ret,
+                    'Contents specified by contents/contents_pillar/'
+                    'contents_grains appears to be binary data, and'
+                    ' as will not be able to be treated as a Jinja'
+                    ' template.'
+                )
+            contents = use_contents
         if template:
             contents = __salt__['file.apply_template_on_contents'](
                 contents,

tests/unit/renderers/test_gpg.py

@@ -3,6 +3,8 @@
 # Import Python Libs
 from __future__ import absolute_import, print_function, unicode_literals
 
+from textwrap import dedent
+
 # Import Salt Testing libs
 from tests.support.mixins import LoaderModuleMockMixin
 from tests.support.unit import skipIf, TestCase
@@ -100,3 +102,69 @@ def test_render(self):
             with patch('salt.renderers.gpg._get_key_dir', MagicMock(return_value=key_dir)):
                 with patch('salt.renderers.gpg._decrypt_object', MagicMock(return_value=secret)):
                     self.assertEqual(gpg.render(crypted), secret)
+
+    def test_multi_render(self):
+        key_dir = '/etc/salt/gpgkeys'
+        secret = 'Use more salt.'
+        expected = '\n'.join([secret]*3)
+        crypted = dedent('''\
+            -----BEGIN PGP MESSAGE-----
+            !@#$%^&*()_+
+            -----END PGP MESSAGE-----
+            -----BEGIN PGP MESSAGE-----
+            !@#$%^&*()_+
+            -----END PGP MESSAGE-----
+            -----BEGIN PGP MESSAGE-----
+            !@#$%^&*()_+
+            -----END PGP MESSAGE-----
+        ''')
+
+        with patch('salt.renderers.gpg._get_gpg_exec', MagicMock(return_value=True)):
+            with patch('salt.renderers.gpg._get_key_dir', MagicMock(return_value=key_dir)):
+                with patch('salt.renderers.gpg._decrypt_ciphertext', MagicMock(return_value=secret)):
+                    self.assertEqual(gpg.render(crypted), expected)
+
+    def test_render_with_binary_data_should_return_binary_data(self):
+        key_dir = '/etc/salt/gpgkeys'
+        secret = b'Use\x8b more\x8b salt.'
+        expected = b'\n'.join([secret]*3)
+        crypted = dedent('''\
+            -----BEGIN PGP MESSAGE-----
+            !@#$%^&*()_+
+            -----END PGP MESSAGE-----
+            -----BEGIN PGP MESSAGE-----
+            !@#$%^&*()_+
+            -----END PGP MESSAGE-----
+            -----BEGIN PGP MESSAGE-----
+            !@#$%^&*()_+
+            -----END PGP MESSAGE-----
+        ''')
+
+        with patch('salt.renderers.gpg._get_gpg_exec', MagicMock(return_value=True)):
+            with patch('salt.renderers.gpg._get_key_dir', MagicMock(return_value=key_dir)):
+                with patch('salt.renderers.gpg._decrypt_ciphertext', MagicMock(return_value=secret)):
+                    self.assertEqual(gpg.render(crypted), expected)
+
+    def test_render_with_translate_newlines_should_translate_newlines(self):
+        key_dir = '/etc/salt/gpgkeys'
+        secret = b'Use\x8b more\x8b salt.'
+        expected = b'\n\n'.join([secret]*3)
+        crypted = dedent('''\
+            -----BEGIN PGP MESSAGE-----
+            !@#$%^&*()_+
+            -----END PGP MESSAGE-----\\n
+            -----BEGIN PGP MESSAGE-----
+            !@#$%^&*()_+
+            -----END PGP MESSAGE-----\\n
+            -----BEGIN PGP MESSAGE-----
+            !@#$%^&*()_+
+            -----END PGP MESSAGE-----
+        ''')
+
+        with patch('salt.renderers.gpg._get_gpg_exec', MagicMock(return_value=True)):
+            with patch('salt.renderers.gpg._get_key_dir', MagicMock(return_value=key_dir)):
+                with patch('salt.renderers.gpg._decrypt_ciphertext', MagicMock(return_value=secret)):
+                    self.assertEqual(
+                        gpg.render(crypted, translate_newlines=True),
+                        expected,
+                    )

tests/unit/states/test_file.py

@@ -604,6 +604,26 @@ def test_missing(self):
 
     # 'managed' function tests: 1
 
+    def test_file_managed_should_fall_back_to_binary(self):
+        expected_contents = b'\x8b'
+        filename = '/tmp/blarg'
+        mock_manage = MagicMock(return_value={'fnord': 'fnords'})
+        with patch('salt.states.file._load_accumulators',
+                   MagicMock(return_value=([], []))):
+            with patch.dict(filestate.__salt__,
+                            {
+                             'file.get_managed': MagicMock(return_value=['', '', '']),
+                             'file.source_list': MagicMock(return_value=['', '']),
+                             'file.manage_file': mock_manage,
+                             'pillar.get': MagicMock(return_value=expected_contents),
+                            }):
+                ret = filestate.managed(
+                    filename,
+                    contents_pillar='fnord',
+                )
+                actual_contents = mock_manage.call_args[0][14]
+                self.assertEqual(actual_contents, expected_contents)
+
     def test_managed(self):
         '''
         Test to manage a given file, this function allows for a file to be