galaxy - preserve symlinks on build/install (#69959) (#69994)

* galaxy - preserve symlinks on build/install (#69959)

* galaxy - preserve symlinks on build/install

* Handle directory symlinks

* py2 compat change

* Updated changelog fragment

(cherry picked from commit d30fc6c0b359f631130b0e979d9a78a7b3747d48)

* Fix integration test

* ansible-galaxy - fix collection installation with trailing slashes (#70016)

If we fail to find a member when extracting a directory, try adding a trailing
slash to the member name. In certain cases, the member in the tarfile will
contain a trailing slash but the file name in FILES.json will never contain
the trailing slash.

If unable to find the member, handle the KeyError and print a nicer error.

Also check if a directory exists before creating it since it may have been
extracted from the archive.

Fixes #70009

* Add unit tests
* Use loop for trying to get members

(cherry picked from commit d45cb01b845d78779877325f03f2d1ab74e1f6b4)

Co-authored-by: Sam Doran <sdoran@redhat.com>

7 files changed, 368 insertions, 60 deletions

diff --git a/changelogs/fragments/galaxy-symlinks.yaml b/changelogs/fragments/galaxy-symlinks.yaml
new file mode 100644
index 0000000000..7c05b93874
--- /dev/null
+++ b/changelogs/fragments/galaxy-symlinks.yaml
@@ -0,0 +1,2 @@
+bugfixes:
+- ansible-galaxy - Preserve symlinks when building and installing a collection
diff --git a/lib/ansible/galaxy/collection.py b/lib/ansible/galaxy/collection.py
index 352940ac2f..9320d26fe7 100644
--- a/lib/ansible/galaxy/collection.py
+++ b/lib/ansible/galaxy/collection.py
@@ -4,6 +4,7 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+import errno
 import fnmatch
 import json
 import operator
@@ -171,7 +172,7 @@ class CollectionRequirement:
         try:
             with tarfile.open(self.b_path, mode='r') as collection_tar:
                 files_member_obj = collection_tar.getmember('FILES.json')
-                with _tarfile_extract(collection_tar, files_member_obj) as files_obj:
+                with _tarfile_extract(collection_tar, files_member_obj) as (dummy, files_obj):
                     files = json.loads(to_text(files_obj.read(), errors='surrogate_or_strict'))
 
                 _extract_tar_file(collection_tar, 'MANIFEST.json', b_collection_path, b_temp_path)
@@ -185,8 +186,10 @@ class CollectionRequirement:
                     if file_info['ftype'] == 'file':
                         _extract_tar_file(collection_tar, file_name, b_collection_path, b_temp_path,
                                           expected_hash=file_info['chksum_sha256'])
+
                     else:
-                        os.makedirs(os.path.join(b_collection_path, to_bytes(file_name, errors='surrogate_or_strict')))
+                        _extract_tar_dir(collection_tar, file_name, b_collection_path)
+
         except Exception:
             # Ensure we don't leave the dir behind in case of a failure.
             shutil.rmtree(b_collection_path)
@@ -262,7 +265,7 @@ class CollectionRequirement:
                     raise AnsibleError("Collection at '%s' does not contain the required file %s."
                                        % (to_native(b_path), n_member_name))
 
-                with _tarfile_extract(collection_tar, member) as member_obj:
+                with _tarfile_extract(collection_tar, member) as (dummy, member_obj):
                     try:
                         info[property_name] = json.loads(to_text(member_obj.read(), errors='surrogate_or_strict'))
                     except ValueError:
@@ -494,7 +497,7 @@ def _tempdir():
 @contextmanager
 def _tarfile_extract(tar, member):
     tar_obj = tar.extractfile(member)
-    yield tar_obj
+    yield member, tar_obj
     tar_obj.close()
 
 
@@ -663,7 +666,7 @@ def _build_files_manifest(b_collection_path, namespace, name):
                 if os.path.islink(b_abs_path):
                     b_link_target = os.path.realpath(b_abs_path)
 
-                    if not b_link_target.startswith(b_top_level_dir):
+                    if not _is_child_path(b_link_target, b_top_level_dir):
                         display.warning("Skipping '%s' as it is a symbolic link to a directory outside the collection"
                                         % to_text(b_abs_path))
                         continue
@@ -674,7 +677,8 @@ def _build_files_manifest(b_collection_path, namespace, name):
 
                 manifest['files'].append(manifest_entry)
 
-                _walk(b_abs_path, b_top_level_dir)
+                if not os.path.islink(b_abs_path):
+                    _walk(b_abs_path, b_top_level_dir)
             else:
                 if b_item == b'galaxy.yml':
                     continue
@@ -683,6 +687,8 @@ def _build_files_manifest(b_collection_path, namespace, name):
                     display.vvv("Skipping '%s' for collection build" % to_text(b_abs_path))
                     continue
 
+                # Handling of file symlinks occur in _build_collection_tar, the manifest for a symlink is the same for
+                # a normal file.
                 manifest_entry = entry_template.copy()
                 manifest_entry['name'] = rel_path
                 manifest_entry['ftype'] = 'file'
@@ -756,12 +762,28 @@ def _build_collection_tar(b_collection_path, b_tar_path, collection_manifest, fi
                 b_src_path = os.path.join(b_collection_path, to_bytes(filename, errors='surrogate_or_strict'))
 
                 def reset_stat(tarinfo):
-                    existing_is_exec = tarinfo.mode & stat.S_IXUSR
-                    tarinfo.mode = 0o0755 if existing_is_exec or tarinfo.isdir() else 0o0644
+                    if tarinfo.type != tarfile.SYMTYPE:
+                        existing_is_exec = tarinfo.mode & stat.S_IXUSR
+                        tarinfo.mode = 0o0755 if existing_is_exec or tarinfo.isdir() else 0o0644
                     tarinfo.uid = tarinfo.gid = 0
                     tarinfo.uname = tarinfo.gname = ''
+
                     return tarinfo
 
+                if os.path.islink(b_src_path):
+                    b_link_target = os.path.realpath(b_src_path)
+                    if _is_child_path(b_link_target, b_collection_path):
+                        b_rel_path = os.path.relpath(b_link_target, start=os.path.dirname(b_src_path))
+
+                        tar_info = tarfile.TarInfo(filename)
+                        tar_info.type = tarfile.SYMTYPE
+                        tar_info.linkname = to_native(b_rel_path, errors='surrogate_or_strict')
+                        tar_info = reset_stat(tar_info)
+                        tar_file.addfile(tarinfo=tar_info)
+
+                        continue
+
+                # Dealing with a normal file, just add it by name.
                 tar_file.add(os.path.realpath(b_src_path), arcname=filename, recursive=False, filter=reset_stat)
 
         shutil.copy(b_tar_filepath, b_tar_path)
@@ -910,26 +932,57 @@ def _download_file(url, b_path, expected_hash, validate_certs, headers=None):
     return b_file_path
 
 
+def _extract_tar_dir(tar, dirname, b_dest):
+    """ Extracts a directory from a collection tar. """
+    member_names = [to_native(dirname, errors='surrogate_or_strict')]
+
+    # Create list of members with and without trailing separator
+    if not member_names[-1].endswith(os.path.sep):
+        member_names.append(member_names[-1] + os.path.sep)
+
+    # Try all of the member names and stop on the first one that are able to successfully get
+    for member in member_names:
+        try:
+            tar_member = tar.getmember(member)
+        except KeyError:
+            continue
+        break
+    else:
+        # If we still can't find the member, raise a nice error.
+        raise AnsibleError("Unable to extract '%s' from collection" % to_native(member, errors='surrogate_or_strict'))
+
+    b_dir_path = os.path.join(b_dest, to_bytes(dirname, errors='surrogate_or_strict'))
+
+    b_parent_path = os.path.dirname(b_dir_path)
+    try:
+        os.makedirs(b_parent_path, mode=0o0755)
+    except OSError as e:
+        if e.errno != errno.EEXIST:
+            raise
+
+    if tar_member.type == tarfile.SYMTYPE:
+        b_link_path = to_bytes(tar_member.linkname, errors='surrogate_or_strict')
+        if not _is_child_path(b_link_path, b_dest, link_name=b_dir_path):
+            raise AnsibleError("Cannot extract symlink '%s' in collection: path points to location outside of "
+                               "collection '%s'" % (to_native(dirname), b_link_path))
+
+        os.symlink(b_link_path, b_dir_path)
+
+    else:
+        if not os.path.isdir(b_dir_path):
+            os.mkdir(b_dir_path, 0o0755)
+
+
 def _extract_tar_file(tar, filename, b_dest, b_temp_path, expected_hash=None):
+    """ Extracts a file from a collection tar. """
     n_filename = to_native(filename, errors='surrogate_or_strict')
-    try:
-        member = tar.getmember(n_filename)
-    except KeyError:
-        raise AnsibleError("Collection tar at '%s' does not contain the expected file '%s'." % (to_native(tar.name),
-                                                                                                n_filename))
-
-    with tempfile.NamedTemporaryFile(dir=b_temp_path, delete=False) as tmpfile_obj:
-        bufsize = 65536
-        sha256_digest = sha256()
-        with _tarfile_extract(tar, member) as tar_obj:
-            data = tar_obj.read(bufsize)
-            while data:
-                tmpfile_obj.write(data)
-                tmpfile_obj.flush()
-                sha256_digest.update(data)
-                data = tar_obj.read(bufsize)
-
-        actual_hash = sha256_digest.hexdigest()
+    with _get_tar_file_member(tar, filename) as (tar_member, tar_obj):
+        if tar_member.type == tarfile.SYMTYPE:
+            actual_hash = _consume_file(tar_obj)
+
+        else:
+            with tempfile.NamedTemporaryFile(dir=b_temp_path, delete=False) as tmpfile_obj:
+                actual_hash = _consume_file(tar_obj, tmpfile_obj)
 
         if expected_hash and actual_hash != expected_hash:
             raise AnsibleError("Checksum mismatch for '%s' inside collection at '%s'"
@@ -937,7 +990,7 @@ def _extract_tar_file(tar, filename, b_dest, b_temp_path, expected_hash=None):
 
         b_dest_filepath = os.path.abspath(os.path.join(b_dest, to_bytes(filename, errors='surrogate_or_strict')))
         b_parent_dir = os.path.dirname(b_dest_filepath)
-        if b_parent_dir != b_dest and not b_parent_dir.startswith(b_dest + to_bytes(os.path.sep)):
+        if not _is_child_path(b_parent_dir, b_dest):
             raise AnsibleError("Cannot extract tar entry '%s' as it will be placed outside the collection directory"
                                % to_native(filename, errors='surrogate_or_strict'))
 
@@ -946,11 +999,60 @@ def _extract_tar_file(tar, filename, b_dest, b_temp_path, expected_hash=None):
             # makes sure we create the parent directory even if it wasn't set in the metadata.
             os.makedirs(b_parent_dir, mode=0o0755)
 
-        shutil.move(to_bytes(tmpfile_obj.name, errors='surrogate_or_strict'), b_dest_filepath)
+        if tar_member.type == tarfile.SYMTYPE:
+            b_link_path = to_bytes(tar_member.linkname, errors='surrogate_or_strict')
+            if not _is_child_path(b_link_path, b_dest, link_name=b_dest_filepath):
+                raise AnsibleError("Cannot extract symlink '%s' in collection: path points to location outside of "
+                                   "collection '%s'" % (to_native(filename), b_link_path))
+
+            os.symlink(b_link_path, b_dest_filepath)
+
+        else:
+            shutil.move(to_bytes(tmpfile_obj.name, errors='surrogate_or_strict'), b_dest_filepath)
+
+            # Default to rw-r--r-- and only add execute if the tar file has execute.
+            tar_member = tar.getmember(to_native(filename, errors='surrogate_or_strict'))
+            new_mode = 0o644
+            if stat.S_IMODE(tar_member.mode) & stat.S_IXUSR:
+                new_mode |= 0o0111
+
+            os.chmod(b_dest_filepath, new_mode)
+
+
+def _get_tar_file_member(tar, filename):
+    n_filename = to_native(filename, errors='surrogate_or_strict')
+    try:
+        member = tar.getmember(n_filename)
+    except KeyError:
+        raise AnsibleError("Collection tar at '%s' does not contain the expected file '%s'." % (
+            to_native(tar.name),
+            n_filename))
+
+    return _tarfile_extract(tar, member)
 
-        # Default to rw-r--r-- and only add execute if the tar file has execute.
-        new_mode = 0o644
-        if stat.S_IMODE(member.mode) & stat.S_IXUSR:
-            new_mode |= 0o0111
 
-        os.chmod(b_dest_filepath, new_mode)
+def _is_child_path(path, parent_path, link_name=None):
+    """ Checks that path is a path within the parent_path specified. """
+    b_path = to_bytes(path, errors='surrogate_or_strict')
+
+    if link_name and not os.path.isabs(b_path):
+        # If link_name is specified, path is the source of the link and we need to resolve the absolute path.
+        b_link_dir = os.path.dirname(to_bytes(link_name, errors='surrogate_or_strict'))
+        b_path = os.path.abspath(os.path.join(b_link_dir, b_path))
+
+    b_parent_path = to_bytes(parent_path, errors='surrogate_or_strict')
+    return b_path == b_parent_path or b_path.startswith(b_parent_path + to_bytes(os.path.sep))
+
+
+def _consume_file(read_from, write_to=None):
+    bufsize = 65536
+    sha256_digest = sha256()
+    data = read_from.read(bufsize)
+    while data:
+        if write_to is not None:
+            write_to.write(data)
+            write_to.flush()
+        sha256_digest.update(data)
+        data = read_from.read(bufsize)
+
+    return sha256_digest.hexdigest()
diff --git a/test/integration/targets/ansible-galaxy-collection/library/setup_collections.py b/test/integration/targets/ansible-galaxy-collection/library/setup_collections.py
index 9129ca768f..a27ec2cd1f 100644
--- a/test/integration/targets/ansible-galaxy-collection/library/setup_collections.py
+++ b/test/integration/targets/ansible-galaxy-collection/library/setup_collections.py
@@ -78,6 +78,7 @@ RETURN = '''
 '''
 
 import os
+import tempfile
 import yaml
 
 from ansible.module_utils.basic import AnsibleModule
@@ -97,6 +98,7 @@ def run_module():
                 name=dict(type='str', required=True),
                 version=dict(type='str', default='1.0.0'),
                 dependencies=dict(type='dict', default={}),
+                use_symlink=dict(type='bool', default=False),
             ),
         ),
     )
@@ -128,9 +130,26 @@ def run_module():
         with open(os.path.join(b_collection_dir, b'galaxy.yml'), mode='wb') as fd:
             fd.write(to_bytes(yaml.safe_dump(galaxy_meta), errors='surrogate_or_strict'))
 
-        release_filename = '%s-%s-%s.tar.gz' % (collection['namespace'], collection['name'], collection['version'])
-        collection_path = os.path.join(collection_dir, release_filename)
-        module.run_command(['ansible-galaxy', 'collection', 'build'], cwd=collection_dir)
+        with tempfile.NamedTemporaryFile(mode='wb') as temp_fd:
+            temp_fd.write(b"data")
+
+            if collection['use_symlink']:
+                os.mkdir(os.path.join(b_collection_dir, b'docs'))
+                os.mkdir(os.path.join(b_collection_dir, b'plugins'))
+                b_target_file = b'RE\xc3\x85DM\xc3\x88.md'
+                with open(os.path.join(b_collection_dir, b_target_file), mode='wb') as fd:
+                    fd.write(b'data')
+
+                os.symlink(b_target_file, os.path.join(b_collection_dir, b_target_file + b'-link'))
+                os.symlink(temp_fd.name, os.path.join(b_collection_dir, b_target_file + b'-outside-link'))
+                os.symlink(os.path.join(b'..', b_target_file), os.path.join(b_collection_dir, b'docs', b_target_file))
+                os.symlink(os.path.join(b_collection_dir, b_target_file),
+                           os.path.join(b_collection_dir, b'plugins', b_target_file))
+                os.symlink(b'docs', os.path.join(b_collection_dir, b'docs-link'))
+
+            release_filename = '%s-%s-%s.tar.gz' % (collection['namespace'], collection['name'], collection['version'])
+            collection_path = os.path.join(collection_dir, release_filename)
+            module.run_command(['ansible-galaxy', 'collection', 'build'], cwd=collection_dir)
 
         # To save on time, skip the import wait until the last collection is being uploaded.
         publish_args = ['ansible-galaxy', 'collection', 'publish', collection_path, '--server',
diff --git a/test/integration/targets/ansible-galaxy-collection/tasks/install.yml b/test/integration/targets/ansible-galaxy-collection/tasks/install.yml
index 3ed1a48d0d..3a8e215d57 100644
--- a/test/integration/targets/ansible-galaxy-collection/tasks/install.yml
+++ b/test/integration/targets/ansible-galaxy-collection/tasks/install.yml
@@ -199,3 +199,44 @@
   file:
     path: '{{ galaxy_dir }}/ansible_collections'
     state: absent
+
+- name: install collection with symlink - {{ test_name }}
+  command: ansible-galaxy collection install symlink.symlink -s '{{ test_server }}'
+  environment:
+    ANSIBLE_COLLECTIONS_PATHS: '{{ galaxy_dir }}/ansible_collections'
+  register: install_symlink
+
+- find:
+    paths: '{{ galaxy_dir }}/ansible_collections/symlink/symlink'
+    recurse: yes
+    file_type: any
+
+- name: get result of install collection with symlink - {{ test_name }}
+  stat:
+    path: '{{ galaxy_dir }}/ansible_collections/symlink/symlink/{{ path }}'
+  register: install_symlink_actual
+  loop_control:
+    loop_var: path
+  loop:
+  - REÅDMÈ.md-link
+  - docs/REÅDMÈ.md
+  - plugins/REÅDMÈ.md
+  - REÅDMÈ.md-outside-link
+  - docs-link
+  - docs-link/REÅDMÈ.md
+
+- name: assert install collection with symlink - {{ test_name }}
+  assert:
+    that:
+    - '"Installing ''symlink.symlink:1.0.0'' to" in install_symlink.stdout'
+    - install_symlink_actual.results[0].stat.islnk
+    - install_symlink_actual.results[0].stat.lnk_target == 'REÅDMÈ.md'
+    - install_symlink_actual.results[1].stat.islnk
+    - install_symlink_actual.results[1].stat.lnk_target == '../REÅDMÈ.md'
+    - install_symlink_actual.results[2].stat.islnk
+    - install_symlink_actual.results[2].stat.lnk_target == '../REÅDMÈ.md'
+    - install_symlink_actual.results[3].stat.isreg
+    - install_symlink_actual.results[4].stat.islnk
+    - install_symlink_actual.results[4].stat.lnk_target == 'docs'
+    - install_symlink_actual.results[5].stat.islnk
+    - install_symlink_actual.results[5].stat.lnk_target == '../REÅDMÈ.md'
diff --git a/test/integration/targets/ansible-galaxy-collection/tasks/main.yml b/test/integration/targets/ansible-galaxy-collection/tasks/main.yml
index 6dc2ab1c01..00517d948f 100644
--- a/test/integration/targets/ansible-galaxy-collection/tasks/main.yml
+++ b/test/integration/targets/ansible-galaxy-collection/tasks/main.yml
@@ -138,6 +138,11 @@
     - namespace: fail_dep2
       name: name
 
+    # Symlink tests
+    - namespace: symlink
+      name: symlink
+      use_symlink: yes
+
 - name: run ansible-galaxy collection install tests for {{ test_name }}
   include_tasks: install.yml
   vars:
diff --git a/test/units/cli/galaxy/test_collection_extract_tar.py b/test/units/cli/galaxy/test_collection_extract_tar.py
new file mode 100644
index 0000000000..526442cc9d
--- /dev/null
+++ b/test/units/cli/galaxy/test_collection_extract_tar.py
@@ -0,0 +1,61 @@
+# -*- coding: utf-8 -*-
+# Copyright (c) 2020 Ansible Project
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from __future__ import absolute_import, division, print_function
+__metaclass__ = type
+
+import pytest
+
+from ansible.errors import AnsibleError
+from ansible.galaxy.collection import _extract_tar_dir
+
+
+@pytest.fixture
+def fake_tar_obj(mocker):
+    m_tarfile = mocker.Mock()
+    m_tarfile.type = mocker.Mock(return_value=b'99')
+    m_tarfile.SYMTYPE = mocker.Mock(return_value=b'22')
+
+    return m_tarfile
+
+
+def test_extract_tar_member_trailing_sep(mocker):
+    m_tarfile = mocker.Mock()
+    m_tarfile.getmember = mocker.Mock(side_effect=KeyError)
+
+    with pytest.raises(AnsibleError, match='Unable to extract'):
+        _extract_tar_dir(m_tarfile, '/some/dir/', b'/some/dest')
+
+    assert m_tarfile.getmember.call_count == 1
+
+
+def test_extract_tar_member_no_trailing_sep(mocker):
+    m_tarfile = mocker.Mock()
+    m_tarfile.getmember = mocker.Mock(side_effect=KeyError)
+
+    with pytest.raises(AnsibleError, match='Unable to extract'):
+        _extract_tar_dir(m_tarfile, '/some/dir', b'/some/dest')
+
+    assert m_tarfile.getmember.call_count == 2
+
+
+def test_extract_tar_dir_exists(mocker, fake_tar_obj):
+    mocker.patch('os.makedirs', return_value=None)
+    m_makedir = mocker.patch('os.mkdir', return_value=None)
+    mocker.patch('os.path.isdir', return_value=True)
+
+    _extract_tar_dir(fake_tar_obj, '/some/dir', b'/some/dest')
+
+    assert not m_makedir.called
+
+
+def test_extract_tar_dir_does_not_exist(mocker, fake_tar_obj):
+    mocker.patch('os.makedirs', return_value=None)
+    m_makedir = mocker.patch('os.mkdir', return_value=None)
+    mocker.patch('os.path.isdir', return_value=False)
+
+    _extract_tar_dir(fake_tar_obj, '/some/dir', b'/some/dest')
+
+    assert m_makedir.called
+    assert m_makedir.call_args[0] == (b'/some/dir', 0o0755)
diff --git a/test/units/galaxy/test_collection.py b/test/units/galaxy/test_collection.py
index 9072fff5c1..16a0b322fb 100644
--- a/test/units/galaxy/test_collection.py
+++ b/test/units/galaxy/test_collection.py
@@ -15,13 +15,14 @@ import uuid
 
 from hashlib import sha256
 from io import BytesIO
-from units.compat.mock import MagicMock
+from units.compat.mock import MagicMock, mock_open, patch
 
 from ansible import context
 from ansible.cli.galaxy import GalaxyCLI
 from ansible.errors import AnsibleError
 from ansible.galaxy import api, collection, token
 from ansible.module_utils._text import to_bytes, to_native, to_text
+from ansible.module_utils.six.moves import builtins
 from ansible.utils import context_objects as co
 from ansible.utils.display import Display
 from ansible.utils.hashing import secure_hash_s
@@ -114,6 +115,60 @@ def galaxy_server():
     return galaxy_api
 
 
+@pytest.fixture()
+def manifest_template():
+    def get_manifest_info(namespace='ansible_namespace', name='collection', version='0.1.0'):
+        return {
+            "collection_info": {
+                "namespace": namespace,
+                "name": name,
+                "version": version,
+                "authors": [
+                    "shertel"
+                ],
+                "readme": "README.md",
+                "tags": [
+                    "test",
+                    "collection"
+                ],
+                "description": "Test",
+                "license": [
+                    "MIT"
+                ],
+                "license_file": None,
+                "dependencies": {},
+                "repository": "https://github.com/{0}/{1}".format(namespace, name),
+                "documentation": None,
+                "homepage": None,
+                "issues": None
+            },
+            "file_manifest_file": {
+                "name": "FILES.json",
+                "ftype": "file",
+                "chksum_type": "sha256",
+                "chksum_sha256": "files_manifest_checksum",
+                "format": 1
+            },
+            "format": 1
+        }
+
+    return get_manifest_info
+
+
+@pytest.fixture()
+def manifest_info(manifest_template):
+    return manifest_template()
+
+
+@pytest.fixture()
+def manifest(manifest_info):
+    b_data = to_bytes(json.dumps(manifest_info))
+
+    with patch.object(builtins, 'open', mock_open(read_data=b_data)) as m:
+        with open('MANIFEST.json', mode='rb') as fake_file:
+            yield fake_file, sha256(b_data).hexdigest()
+
+
 def test_build_collection_no_galaxy_yaml():
     fake_path = u'/fake/ÅÑŚÌβŁÈ/path'
     expected = to_native("The collection galaxy.yml path '%s/galaxy.yml' does not exist." % fake_path)
@@ -337,14 +392,9 @@ def test_build_copy_symlink_target_inside_collection(collection_input):
     actual = collection._build_files_manifest(to_bytes(input_dir), 'namespace', 'collection')
 
     linked_entries = [e for e in actual['files'] if e['name'].startswith('playbooks/roles/linked')]
-    assert len(linked_entries) == 3
+    assert len(linked_entries) == 1
     assert linked_entries[0]['name'] == 'playbooks/roles/linked'
     assert linked_entries[0]['ftype'] == 'dir'
-    assert linked_entries[1]['name'] == 'playbooks/roles/linked/tasks'
-    assert linked_entries[1]['ftype'] == 'dir'
-    assert linked_entries[2]['name'] == 'playbooks/roles/linked/tasks/main.yml'
-    assert linked_entries[2]['ftype'] == 'file'
-    assert linked_entries[2]['chksum_sha256'] == '9c97a1633c51796999284c62236b8d5462903664640079b80c37bf50080fcbc3'
 
 
 def test_build_with_symlink_inside_collection(collection_input):
@@ -372,25 +422,15 @@ def test_build_with_symlink_inside_collection(collection_input):
     with tarfile.open(output_artifact, mode='r') as actual:
         members = actual.getmembers()
 
-        linked_members = [m for m in members if m.path.startswith('playbooks/roles/linked/tasks')]
-        assert len(linked_members) == 2
-        assert linked_members[0].name == 'playbooks/roles/linked/tasks'
-        assert linked_members[0].isdir()
-
-        assert linked_members[1].name == 'playbooks/roles/linked/tasks/main.yml'
-        assert linked_members[1].isreg()
-
-        linked_task = actual.extractfile(linked_members[1].name)
-        actual_task = secure_hash_s(linked_task.read())
-        linked_task.close()
+        linked_folder = next(m for m in members if m.path == 'playbooks/roles/linked')
+        assert linked_folder.type == tarfile.SYMTYPE
+        assert linked_folder.linkname == '../../roles/linked'
 
-        assert actual_task == 'f4dcc52576b6c2cd8ac2832c52493881c4e54226'
+        linked_file = next(m for m in members if m.path == 'docs/README.md')
+        assert linked_file.type == tarfile.SYMTYPE
+        assert linked_file.linkname == '../README.md'
 
-        linked_file = [m for m in members if m.path == 'docs/README.md']
-        assert len(linked_file) == 1
-        assert linked_file[0].isreg()
-
-        linked_file_obj = actual.extractfile(linked_file[0].name)
+        linked_file_obj = actual.extractfile(linked_file.name)
         actual_file = secure_hash_s(linked_file_obj.read())
         linked_file_obj.close()
 
@@ -585,3 +625,41 @@ def test_extract_tar_file_outside_dir(tmp_path_factory):
     with tarfile.open(tar_file, 'r') as tfile:
         with pytest.raises(AnsibleError, match=expected):
             collection._extract_tar_file(tfile, tar_filename, os.path.join(temp_dir, to_bytes(filename)), temp_dir)
+
+
+def test_consume_file(manifest):
+
+    manifest_file, checksum = manifest
+    assert checksum == collection._consume_file(manifest_file)
+
+
+def test_consume_file_and_write_contents(manifest, manifest_info):
+
+    manifest_file, checksum = manifest
+
+    write_to = BytesIO()
+    actual_hash = collection._consume_file(manifest_file, write_to)
+
+    write_to.seek(0)
+    assert to_bytes(json.dumps(manifest_info)) == write_to.read()
+    assert actual_hash == checksum
+
+
+def test_get_tar_file_member(tmp_tarfile):
+
+    temp_dir, tfile, filename, checksum = tmp_tarfile
+
+    with collection._get_tar_file_member(tfile, filename) as (tar_file_member, tar_file_obj):
+        assert isinstance(tar_file_member, tarfile.TarInfo)
+        assert isinstance(tar_file_obj, tarfile.ExFileObject)
+
+
+def test_get_nonexistent_tar_file_member(tmp_tarfile):
+    temp_dir, tfile, filename, checksum = tmp_tarfile
+
+    file_does_not_exist = filename + 'nonexistent'
+
+    with pytest.raises(AnsibleError) as err:
+        collection._get_tar_file_member(tfile, file_does_not_exist)
+
+    assert to_text(err.value.message) == "Collection tar at '%s' does not contain the expected file '%s'." % (to_text(tfile.name), file_does_not_exist)