Skip to content

Commit

Permalink
remove any pipe files before zip/unzipping (#1133)
Browse files Browse the repository at this point in the history
* Write test to demonstrate job hang on pipes

* Remove any pipe files before zip/unzipping

- Prevents hangs when zipping, unzipping artifacts dir. If python
attempts to open a fifo pipe, it will block indefinitely until it can
read data from the pipe.

* move fifo check below symlink

Co-authored-by: Alan Rominger <arominge@redhat.com>
  • Loading branch information
fosterseth and AlanCoding committed Oct 28, 2022
1 parent 0b6ea94 commit 8f39752
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 0 deletions.
11 changes: 11 additions & 0 deletions ansible_runner/utils/streaming.py
Expand Up @@ -32,6 +32,11 @@ def stream_dir(source_directory, stream):
permissions |= 0xA000
zip_info.external_attr = permissions << 16
archive.writestr(zip_info, os.readlink(full_path))
elif stat.S_ISFIFO(os.stat(full_path).st_mode):
# skip any pipes, as python hangs when attempting
# to open them.
# i.e. ssh_key_data that was never cleaned up
continue
else:
archive.write(
os.path.join(dirpath, fname), arcname=os.path.join(relpath, fname)
Expand Down Expand Up @@ -81,6 +86,12 @@ def unstream_dir(stream, length, target_directory):
if os.path.exists(out_path):
if is_symlink:
os.remove(out_path)
elif stat.S_ISFIFO(os.stat(out_path).st_mode):
# remove any pipes, as python hangs when attempting
# to open them.
# i.e. ssh_key_data that was never cleaned up
os.remove(out_path)
continue
elif os.path.isdir(out_path):
# Special case, the important dirs were pre-created so don't try to chmod them
continue
Expand Down
45 changes: 45 additions & 0 deletions test/unit/utils/test_utils.py
Expand Up @@ -4,6 +4,7 @@
import os
import signal
import time
import stat

from pathlib import Path

Expand Down Expand Up @@ -113,6 +114,50 @@ def test_transmit_symlink(tmp_path, symlink_dest, check_content):
assert f.read() == 'hello world'


@pytest.mark.timeout(timeout=3)
def test_stream_dir_no_hang_on_pipe(tmp_path):
# prepare the input private_data_dir directory to zip
pdd = tmp_path / 'timeout_test'
pdd.mkdir()

with open(pdd / 'ordinary_file.txt', 'w') as f:
f.write('hello world')

# make pipe, similar to open_fifo_write
os.mkfifo(pdd / 'my_pipe', stat.S_IRUSR | stat.S_IWUSR)

# zip and stream the data into the in-memory buffer outgoing_buffer
outgoing_buffer = io.BytesIO()
outgoing_buffer.name = 'not_stdout'
stream_dir(pdd, outgoing_buffer)


@pytest.mark.timeout(timeout=3)
def test_unstream_dir_no_hang_on_pipe(tmp_path):
# prepare the input private_data_dir directory to zip
pdd = tmp_path / 'timeout_test_source_dir'
pdd.mkdir()

with open(pdd / 'ordinary_file.txt', 'w') as f:
f.write('hello world')

# zip and stream the data into the in-memory buffer outgoing_buffer
outgoing_buffer = io.BytesIO()
outgoing_buffer.name = 'not_stdout'
stream_dir(pdd, outgoing_buffer)

dest_dir = tmp_path / 'timeout_test_dest'
dest_dir.mkdir()

# We create the pipe in the same location as an archived file to trigger the bug
os.mkfifo(dest_dir / 'ordinary_file.txt', stat.S_IRUSR | stat.S_IWUSR)

outgoing_buffer.seek(0)
first_line = outgoing_buffer.readline()
size_data = json.loads(first_line.strip())
unstream_dir(outgoing_buffer, size_data['zipfile'], dest_dir)


@pytest.mark.parametrize('fperm', [
0o777,
0o666,
Expand Down

0 comments on commit 8f39752

Please sign in to comment.