From 8f3ccee7a9bda2d57b9132461d53d0b87facdf56 Mon Sep 17 00:00:00 2001 From: stone-w4tch3r <100294019+stone-w4tch3r@users.noreply.github.com> Date: Sat, 11 Nov 2023 17:29:48 +0500 Subject: [PATCH 1/5] changes --- pyinfra/operations/files.py | 60 +++++++++++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 13 deletions(-) diff --git a/pyinfra/operations/files.py b/pyinfra/operations/files.py index 041d6e3da..ac6a7a086 100644 --- a/pyinfra/operations/files.py +++ b/pyinfra/operations/files.py @@ -1614,6 +1614,7 @@ def block( line=None, backup=False, escape_regex_characters=False, + try_prevent_shell_expansion=False, before=False, after=False, marker=None, @@ -1631,6 +1632,7 @@ def block( + line: regex before or after which the content should be added if it doesn't exist. + backup: whether to backup the file (see ``files.line``). Default False. + escape_regex_characters: whether to escape regex characters from the matching line + + try_prevent_shell_expansion: tries to prevent shell expanding by values like `$` + marker: the base string used to mark the text. Default is ``# {mark} PYINFRA BLOCK`` + begin: the value for ``{mark}`` in the marker before the content. Default is ``BEGIN`` + end: the value for ``{mark}`` in the marker after the content. Default is ``END`` @@ -1647,12 +1649,15 @@ def block( Removal ignores ``content`` and ``line`` + Preventing shell expansion works by wrapping the content in '`' before passing to `awk`. + WARNING: This will break if the content contains raw single quotes. + **Examples:** .. code:: python # add entry to /etc/host - files.marked_block( + files.block( name="add IP address for red server", path="/etc/hosts", content="10.0.0.1 mars-one", @@ -1661,7 +1666,7 @@ def block( ) # have two entries in /etc/host - files.marked_block( + files.block( name="add IP address for red server", path="/etc/hosts", content="10.0.0.1 mars-one\\n10.0.0.2 mars-two", @@ -1670,14 +1675,14 @@ def block( ) # remove marked entry from /etc/hosts - files.marked_block( + files.block( name="remove all 10.* addresses from /etc/hosts", path="/etc/hosts", present=False ) # add out of date warning to web page - files.marked_block( + files.block( name="add out of date warning to web page", path="/var/www/html/something.html", content= "

Warning: this page is out of date.

", @@ -1685,6 +1690,14 @@ def block( after=True marker="", ) + + # put complex alias into .zshrc + files.block( + path="/home/user/.zshrc", + content="eval $(thefuck -a)", + try_prevent_shell_expansion=True, + marker="## {mark} ALIASES ##" + ) """ logger.warning("The `files.block` operation is currently in beta!") @@ -1729,8 +1742,13 @@ def block( raise ValueError("only one of 'before' or 'after' used when 'line` is specified") elif before != after: raise ValueError("'line' must be supplied or 'before' and 'after' must be equal") + if isinstance(content, str): + # convert string to list of lines + content = content.split("\n") + if try_prevent_shell_expansion and any("'" in line for line in content): + logger.warning("content contains single quotes, shell expansion prevention may fail") - the_block = "\n".join([mark_1, content, mark_2]) + the_block = "\n".join([mark_1, *content, mark_2]) if (current is None) or ((current == []) and (before == after)): # a) no file or b) file but no markers and we're adding at start or end. Both use 'cat' @@ -1738,7 +1756,14 @@ def block( stdin = "- " if ((current == []) and before) else "" # here = hex(random.randint(0, 2147483647)) here = "PYINFRAHERE" - cmd = StringCommand(f"cat {stdin}{redirect}", q_path, f"<<{here}\n{the_block}\n{here}") + cmd = StringCommand( + f"cat {stdin}{redirect}", + q_path, + f"<<{here}" \ + if not try_prevent_shell_expansion \ + else f"<<'{here}'", + f"\n{the_block}\n{here}" + ) elif current == []: # markers not found and have a pattern to match (not start or end) regex = adjust_regex(line, escape_regex_characters) print_before = "{ print }" if before else "" @@ -1748,21 +1773,30 @@ def block( f"{print_after} f!=1 && /{regex}/ {{ print x; f=1}} " f"END {{if (f==0) print ARGV[2] }} {print_before}'" ) - cmd = StringCommand(out_prep, prog, q_path, f'$"{the_block}"', "> $OUT &&", real_out) + cmd = StringCommand( + out_prep, + prog, q_path, + f'"{the_block}"' \ + if not try_prevent_shell_expansion \ + else f"'{the_block}'", + "> $OUT &&", + real_out + ) else: - pieces = content.split("\n") - if (len(current) != len(pieces)) or ( - not all(lines[0] == lines[1] for lines in zip(pieces, current)) + if (len(current) != len(content)) or ( + not all(lines[0] == lines[1] for lines in zip(content, current)) ): # marked_block found but text is different prog = ( 'awk \'BEGIN {{f=1; x=ARGV[2]; ARGV[2]=""}}' - f"/{mark_1}/ {{print; print x; f=0}} /{mark_2}/ {{print; f=1}} f'" + f"/{mark_1}/ {{print; print x; f=0}} /{mark_2}/ {{print; f=1; next}} f'" ) cmd = StringCommand( out_prep, prog, q_path, - '$"' + content + '"', + '"' + "\n".join(content) + '"' \ + if not try_prevent_shell_expansion \ + else "'" + "\n".join(content) + "'", "> $OUT &&", real_out, ) @@ -1774,7 +1808,7 @@ def block( host.create_fact( Block, kwargs={"path": path, "marker": marker, "begin": begin, "end": end}, - data=content.split("\n"), + data=content, ) else: # remove the marked_block if content: From 130759c659a61a8299c94d21437fcc391a1561f7 Mon Sep 17 00:00:00 2001 From: stone-w4tch3r <100294019+stone-w4tch3r@users.noreply.github.com> Date: Sat, 11 Nov 2023 17:43:18 +0500 Subject: [PATCH 2/5] flake8 run --- pyinfra/operations/files.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/pyinfra/operations/files.py b/pyinfra/operations/files.py index ac6a7a086..3c7f16de5 100644 --- a/pyinfra/operations/files.py +++ b/pyinfra/operations/files.py @@ -1759,9 +1759,9 @@ def block( cmd = StringCommand( f"cat {stdin}{redirect}", q_path, - f"<<{here}" \ - if not try_prevent_shell_expansion \ - else f"<<'{here}'", + f"<<{here}" + if not try_prevent_shell_expansion + else f"<<'{here}'", f"\n{the_block}\n{here}" ) elif current == []: # markers not found and have a pattern to match (not start or end) @@ -1776,9 +1776,9 @@ def block( cmd = StringCommand( out_prep, prog, q_path, - f'"{the_block}"' \ - if not try_prevent_shell_expansion \ - else f"'{the_block}'", + f'"{the_block}"' + if not try_prevent_shell_expansion + else f"'{the_block}'", "> $OUT &&", real_out ) @@ -1794,9 +1794,9 @@ def block( out_prep, prog, q_path, - '"' + "\n".join(content) + '"' \ - if not try_prevent_shell_expansion \ - else "'" + "\n".join(content) + "'", + '"' + "\n".join(content) + '"' + if not try_prevent_shell_expansion + else "'" + "\n".join(content) + "'", "> $OUT &&", real_out, ) From 936fe826971aaa8b230f4ce480f685497fd9165c Mon Sep 17 00:00:00 2001 From: Nick Mills-Barrett Date: Sat, 13 Jan 2024 10:21:42 +0000 Subject: [PATCH 3/5] Run black on `files.py` --- pyinfra/operations/files.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/pyinfra/operations/files.py b/pyinfra/operations/files.py index 3c7f16de5..570874639 100644 --- a/pyinfra/operations/files.py +++ b/pyinfra/operations/files.py @@ -1759,10 +1759,8 @@ def block( cmd = StringCommand( f"cat {stdin}{redirect}", q_path, - f"<<{here}" - if not try_prevent_shell_expansion - else f"<<'{here}'", - f"\n{the_block}\n{here}" + f"<<{here}" if not try_prevent_shell_expansion else f"<<'{here}'", + f"\n{the_block}\n{here}", ) elif current == []: # markers not found and have a pattern to match (not start or end) regex = adjust_regex(line, escape_regex_characters) @@ -1775,16 +1773,15 @@ def block( ) cmd = StringCommand( out_prep, - prog, q_path, - f'"{the_block}"' - if not try_prevent_shell_expansion - else f"'{the_block}'", + prog, + q_path, + f'"{the_block}"' if not try_prevent_shell_expansion else f"'{the_block}'", "> $OUT &&", - real_out + real_out, ) else: if (len(current) != len(content)) or ( - not all(lines[0] == lines[1] for lines in zip(content, current)) + not all(lines[0] == lines[1] for lines in zip(content, current)) ): # marked_block found but text is different prog = ( 'awk \'BEGIN {{f=1; x=ARGV[2]; ARGV[2]=""}}' From 913379a983be8cd2e54b3a4bdc1c158e8f9c6cad Mon Sep 17 00:00:00 2001 From: Nick Mills-Barrett Date: Sat, 13 Jan 2024 10:34:25 +0000 Subject: [PATCH 4/5] Use `OperationValueError` in `files.block` --- pyinfra/operations/files.py | 11 ++++++++--- .../add_line_given_but_before_eq_after.json | 2 +- .../files.block/add_no_content_provided.json | 2 +- .../add_no_line_given_but_before_ne_after.json | 2 +- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/pyinfra/operations/files.py b/pyinfra/operations/files.py index 570874639..469e48d6f 100644 --- a/pyinfra/operations/files.py +++ b/pyinfra/operations/files.py @@ -19,6 +19,7 @@ FileUploadCommand, OperationError, OperationTypeError, + OperationValueError, QuoteString, RsyncCommand, StringCommand, @@ -1736,12 +1737,16 @@ def block( cmd = None if present: if not content: - raise ValueError("'content' must be supplied when 'present' == True") + raise OperationValueError("'content' must be supplied when 'present' == True") if line: if before == after: - raise ValueError("only one of 'before' or 'after' used when 'line` is specified") + raise OperationValueError( + "only one of 'before' or 'after' used when 'line` is specified" + ) elif before != after: - raise ValueError("'line' must be supplied or 'before' and 'after' must be equal") + raise OperationValueError( + "'line' must be supplied or 'before' and 'after' must be equal" + ) if isinstance(content, str): # convert string to list of lines content = content.split("\n") diff --git a/tests/operations/files.block/add_line_given_but_before_eq_after.json b/tests/operations/files.block/add_line_given_but_before_eq_after.json index 7f3e678ef..901e0a3c4 100644 --- a/tests/operations/files.block/add_line_given_but_before_eq_after.json +++ b/tests/operations/files.block/add_line_given_but_before_eq_after.json @@ -13,7 +13,7 @@ } }, "exception": { - "names": ["ValueError"], + "names": ["OperationValueError"], "message": "only one of 'before' or 'after' used when 'line` is specified" } } diff --git a/tests/operations/files.block/add_no_content_provided.json b/tests/operations/files.block/add_no_content_provided.json index 8dd61ebca..fc50d36f9 100644 --- a/tests/operations/files.block/add_no_content_provided.json +++ b/tests/operations/files.block/add_no_content_provided.json @@ -7,7 +7,7 @@ } }, "exception": { - "names": ["ValueError"], + "names": ["OperationValueError"], "message": "'content' must be supplied when 'present' == True" } } diff --git a/tests/operations/files.block/add_no_line_given_but_before_ne_after.json b/tests/operations/files.block/add_no_line_given_but_before_ne_after.json index d7c7034b4..57e4f335d 100644 --- a/tests/operations/files.block/add_no_line_given_but_before_ne_after.json +++ b/tests/operations/files.block/add_no_line_given_but_before_ne_after.json @@ -12,7 +12,7 @@ } }, "exception": { - "names": ["ValueError"], + "names": ["OperationValueError"], "message": "'line' must be supplied or 'before' and 'after' must be equal" } } From 372222340eebba4596d83f96fe1d5b93e9411a58 Mon Sep 17 00:00:00 2001 From: Nick Mills-Barrett Date: Sat, 13 Jan 2024 10:35:17 +0000 Subject: [PATCH 5/5] Update `files.block` tests with fixes --- .../files.block/add_existing_block_different_content.json | 2 +- .../add_existing_block_different_content_and_backup.json | 2 +- .../files.block/add_no_existing_block_and_no_line.json | 2 +- .../files.block/add_no_existing_block_line_provided.json | 2 +- .../add_no_existing_block_line_provided_escape_regex.json | 2 +- tests/operations/files.block/add_no_existing_file.json | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/operations/files.block/add_existing_block_different_content.json b/tests/operations/files.block/add_existing_block_different_content.json index 30a359ab1..3c29332d6 100644 --- a/tests/operations/files.block/add_existing_block_different_content.json +++ b/tests/operations/files.block/add_existing_block_different_content.json @@ -12,6 +12,6 @@ } }, "commands": [ - "OUT=\"$(TMPDIR=/tmp mktemp -t pyinfra.XXXXXX)\" && awk 'BEGIN {{f=1; x=ARGV[2]; ARGV[2]=\"\"}}/# BEGIN PYINFRA BLOCK/ {print; print x; f=0} /# END PYINFRA BLOCK/ {print; f=1} f' /home/someone/something $\"should be this\" > $OUT && chmod $(stat -c %a /home/someone/something 2>/dev/null || stat -f %Lp /home/someone/something ) $OUT && (chown $(stat -c \"%U:%G\" /home/someone/something 2>/dev/null) $OUT || chown -n $(stat -f \"%u:%g\" /home/someone/something ) $OUT) && mv \"$OUT\" /home/someone/something" + "OUT=\"$(TMPDIR=/tmp mktemp -t pyinfra.XXXXXX)\" && awk 'BEGIN {{f=1; x=ARGV[2]; ARGV[2]=\"\"}}/# BEGIN PYINFRA BLOCK/ {print; print x; f=0} /# END PYINFRA BLOCK/ {print; f=1; next} f' /home/someone/something \"should be this\" > $OUT && chmod $(stat -c %a /home/someone/something 2>/dev/null || stat -f %Lp /home/someone/something ) $OUT && (chown $(stat -c \"%U:%G\" /home/someone/something 2>/dev/null) $OUT || chown -n $(stat -f \"%u:%g\" /home/someone/something ) $OUT) && mv \"$OUT\" /home/someone/something" ] } diff --git a/tests/operations/files.block/add_existing_block_different_content_and_backup.json b/tests/operations/files.block/add_existing_block_different_content_and_backup.json index 6f93b9a59..a871a98cd 100644 --- a/tests/operations/files.block/add_existing_block_different_content_and_backup.json +++ b/tests/operations/files.block/add_existing_block_different_content_and_backup.json @@ -13,6 +13,6 @@ } }, "commands": [ - "cp /home/someone/something /home/someone/something.a-timestamp && OUT=\"$(TMPDIR=/tmp mktemp -t pyinfra.XXXXXX)\" && awk 'BEGIN {{f=1; x=ARGV[2]; ARGV[2]=\"\"}}/# BEGIN PYINFRA BLOCK/ {print; print x; f=0} /# END PYINFRA BLOCK/ {print; f=1} f' /home/someone/something $\"should be this\" > $OUT && chmod $(stat -c %a /home/someone/something 2>/dev/null || stat -f %Lp /home/someone/something ) $OUT && (chown $(stat -c \"%U:%G\" /home/someone/something 2>/dev/null) $OUT || chown -n $(stat -f \"%u:%g\" /home/someone/something ) $OUT) && mv \"$OUT\" /home/someone/something" + "cp /home/someone/something /home/someone/something.a-timestamp && OUT=\"$(TMPDIR=/tmp mktemp -t pyinfra.XXXXXX)\" && awk 'BEGIN {{f=1; x=ARGV[2]; ARGV[2]=\"\"}}/# BEGIN PYINFRA BLOCK/ {print; print x; f=0} /# END PYINFRA BLOCK/ {print; f=1; next} f' /home/someone/something \"should be this\" > $OUT && chmod $(stat -c %a /home/someone/something 2>/dev/null || stat -f %Lp /home/someone/something ) $OUT && (chown $(stat -c \"%U:%G\" /home/someone/something 2>/dev/null) $OUT || chown -n $(stat -f \"%u:%g\" /home/someone/something ) $OUT) && mv \"$OUT\" /home/someone/something" ] } diff --git a/tests/operations/files.block/add_no_existing_block_and_no_line.json b/tests/operations/files.block/add_no_existing_block_and_no_line.json index 8e7094bbf..c32a5f6b9 100644 --- a/tests/operations/files.block/add_no_existing_block_and_no_line.json +++ b/tests/operations/files.block/add_no_existing_block_and_no_line.json @@ -12,6 +12,6 @@ } }, "commands": [ - "cat > /home/someone/something < /home/someone/something < $OUT && chmod $(stat -c %a /home/someone/something 2>/dev/null || stat -f %Lp /home/someone/something ) $OUT && (chown $(stat -c \"%U:%G\" /home/someone/something 2>/dev/null) $OUT || chown -n $(stat -f \"%u:%g\" /home/someone/something ) $OUT) && mv \"$OUT\" /home/someone/something" + "OUT=\"$(TMPDIR=/tmp mktemp -t pyinfra.XXXXXX)\" && awk 'BEGIN {x=ARGV[2]; ARGV[2]=\"\"} f!=1 && /^.*before this.*$/ { print x; f=1} END {if (f==0) print ARGV[2] } { print }' /home/someone/something \"# BEGIN PYINFRA BLOCK\nplease add this\n# END PYINFRA BLOCK\" > $OUT && chmod $(stat -c %a /home/someone/something 2>/dev/null || stat -f %Lp /home/someone/something ) $OUT && (chown $(stat -c \"%U:%G\" /home/someone/something 2>/dev/null) $OUT || chown -n $(stat -f \"%u:%g\" /home/someone/something ) $OUT) && mv \"$OUT\" /home/someone/something" ] } diff --git a/tests/operations/files.block/add_no_existing_block_line_provided_escape_regex.json b/tests/operations/files.block/add_no_existing_block_line_provided_escape_regex.json index f447d5dfc..cd9b1adae 100644 --- a/tests/operations/files.block/add_no_existing_block_line_provided_escape_regex.json +++ b/tests/operations/files.block/add_no_existing_block_line_provided_escape_regex.json @@ -13,6 +13,6 @@ } }, "commands": [ - "OUT=\"$(TMPDIR=/tmp mktemp -t pyinfra.XXXXXX)\" && awk 'BEGIN {x=ARGV[2]; ARGV[2]=\"\"} f!=1 && /^.*before this \\*.*$/ { print x; f=1} END {if (f==0) print ARGV[2] } { print }' /home/someone/something $\"# BEGIN PYINFRA BLOCK\nplease add this\n# END PYINFRA BLOCK\" > $OUT && chmod $(stat -c %a /home/someone/something 2>/dev/null || stat -f %Lp /home/someone/something ) $OUT && (chown $(stat -c \"%U:%G\" /home/someone/something 2>/dev/null) $OUT || chown -n $(stat -f \"%u:%g\" /home/someone/something ) $OUT) && mv \"$OUT\" /home/someone/something" + "OUT=\"$(TMPDIR=/tmp mktemp -t pyinfra.XXXXXX)\" && awk 'BEGIN {x=ARGV[2]; ARGV[2]=\"\"} f!=1 && /^.*before this \\*.*$/ { print x; f=1} END {if (f==0) print ARGV[2] } { print }' /home/someone/something \"# BEGIN PYINFRA BLOCK\nplease add this\n# END PYINFRA BLOCK\" > $OUT && chmod $(stat -c %a /home/someone/something 2>/dev/null || stat -f %Lp /home/someone/something ) $OUT && (chown $(stat -c \"%U:%G\" /home/someone/something 2>/dev/null) $OUT || chown -n $(stat -f \"%u:%g\" /home/someone/something ) $OUT) && mv \"$OUT\" /home/someone/something" ] } diff --git a/tests/operations/files.block/add_no_existing_file.json b/tests/operations/files.block/add_no_existing_file.json index cd2505808..358c1ddca 100644 --- a/tests/operations/files.block/add_no_existing_file.json +++ b/tests/operations/files.block/add_no_existing_file.json @@ -12,6 +12,6 @@ } }, "commands": [ - "cat > /home/someone/something < /home/someone/something <