Skip to content

Commit b8da502

Browse files
morrison12morrison12
authored
operations/files.block: correct behaviour when markers/block not found and no line provided
1 parent 04468c2 commit b8da502

11 files changed

+103
-40
lines changed

‎src/pyinfra/operations/files.py‎

Lines changed: 60 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1739,6 +1739,9 @@ def block(
17391739
will either be prepended to the file (if both ``before`` and ``after``
17401740
are ``True``) or appended to the file (if both are ``False``).
17411741
1742+
If the file is created, it is created with the default umask; otherwise the umask is preserved
1743+
as is the owner.
1744+
17421745
Removal ignores ``content`` and ``line``
17431746
17441747
Preventing shell expansion works by wrapping the content in '`' before passing to `awk`.
@@ -1797,32 +1800,46 @@ def block(
17971800
mark_1 = (marker or MARKER_DEFAULT).format(mark=begin or MARKER_BEGIN_DEFAULT)
17981801
mark_2 = (marker or MARKER_DEFAULT).format(mark=end or MARKER_END_DEFAULT)
17991802

1803+
current = host.get_fact(Block, path=path, marker=marker, begin=begin, end=end)
1804+
cmd = None
1805+
18001806
# standard awk doesn't have an "in-place edit" option so we write to a tempfile and
18011807
# if edits were successful move to dest i.e. we do: <out_prep> ... do some work ... <real_out>
18021808
q_path = QuoteString(path)
1803-
out_prep = StringCommand('OUT="$(TMPDIR=/tmp mktemp -t pyinfra.XXXXXX)" && ')
1804-
if backup:
1809+
mode_get = (
1810+
""
1811+
if current is None
1812+
else (
1813+
'MODE="$(stat -c %a',
1814+
q_path,
1815+
"2>/dev/null || stat -f %Lp",
1816+
q_path,
1817+
'2>/dev/null)" &&',
1818+
)
1819+
)
1820+
out_prep = StringCommand(
1821+
'OUT="$(TMPDIR=/tmp mktemp -t pyinfra.XXXXXX)" && ',
1822+
*mode_get,
1823+
'OWNER="$(stat -c "%u:%g"',
1824+
q_path,
1825+
'2>/dev/null || stat -f "%u:%g"',
1826+
q_path,
1827+
'2>/dev/null || echo $(id -un):$(id -gn))" &&',
1828+
)
1829+
1830+
mode_change = "" if current is None else ' && chmod "$MODE"'
1831+
real_out = StringCommand(
1832+
' && mv "$OUT"', q_path, ' && chown "$OWNER"', q_path, mode_change, q_path
1833+
)
1834+
1835+
if backup and (current is not None): # can't back up something that doesn't exist
18051836
out_prep = StringCommand(
18061837
"cp",
18071838
q_path,
18081839
QuoteString(f"{path}.{get_timestamp()}"),
18091840
"&&",
18101841
out_prep,
18111842
)
1812-
real_out = StringCommand(
1813-
"chmod $(stat -c %a",
1814-
q_path,
1815-
"2>/dev/null || stat -f %Lp",
1816-
q_path,
1817-
") $OUT && ",
1818-
'(chown $(stat -c "%u:%g"',
1819-
q_path,
1820-
"2>/dev/null || ",
1821-
'stat -f "%u:%g"',
1822-
q_path,
1823-
'2>/dev/null ) $OUT) && mv "$OUT"',
1824-
q_path,
1825-
)
18261843

18271844
current = host.get_fact(Block, path=path, marker=marker, begin=begin, end=end)
18281845
# None means file didn't exist, empty list means marker was not found
@@ -1842,25 +1859,36 @@ def block(
18421859
if isinstance(content, str):
18431860
# convert string to list of lines
18441861
content = content.split("\n")
1845-
if try_prevent_shell_expansion and any("'" in line for line in content):
1846-
logger.warning("content contains single quotes, shell expansion prevention may fail")
18471862

18481863
the_block = "\n".join([mark_1, *content, mark_2])
1864+
if try_prevent_shell_expansion:
1865+
the_block = f"'{the_block}'"
1866+
if any("'" in line for line in content):
1867+
logger.warning(
1868+
"content contains single quotes, shell expansion prevention may fail"
1869+
)
1870+
else:
1871+
the_block = f'"{the_block}"'
18491872

18501873
if (current is None) or ((current == []) and (before == after)):
1851-
# a) no file or b) file but no markers and we're adding at start or end. Both use 'cat'
1852-
redirect = ">" if (current is None) else ">>"
1853-
stdin = "- " if ((current == []) and before) else ""
1854-
# here = hex(random.randint(0, 2147483647))
1874+
# a) no file or b) file but no markers and we're adding at start or end.
1875+
# here = hex(random.randint(0, 2147483647)) # not used as not testable
18551876
here = "PYINFRAHERE"
1877+
original = q_path if current is not None else QuoteString("/dev/null")
18561878
cmd = StringCommand(
1857-
f"cat {stdin}{redirect}",
1858-
q_path,
1859-
f"<<{here}" if not try_prevent_shell_expansion else f"<<'{here}'",
1860-
f"\n{the_block}\n{here}",
1879+
out_prep,
1880+
"(",
1881+
"awk '{{print}}'",
1882+
original if not before else " - ",
1883+
original if before else " - ",
1884+
'> "$OUT"',
1885+
f"<<{here}\n{the_block[1:-1]}\n{here}\n",
1886+
")",
1887+
real_out,
18611888
)
18621889
elif current == []: # markers not found and have a pattern to match (not start or end)
1863-
assert isinstance(line, str)
1890+
if not isinstance(line, str):
1891+
raise OperationTypeError("'line' must be a regex or a string")
18641892
regex = adjust_regex(line, escape_regex_characters)
18651893
print_before = "{ print }" if before else ""
18661894
print_after = "{ print }" if after else ""
@@ -1873,13 +1901,13 @@ def block(
18731901
out_prep,
18741902
prog,
18751903
q_path,
1876-
f'"{the_block}"' if not try_prevent_shell_expansion else f"'{the_block}'",
1877-
"> $OUT &&",
1904+
the_block,
1905+
'> "$OUT"',
18781906
real_out,
18791907
)
18801908
else:
18811909
if (len(current) != len(content)) or (
1882-
not all(lines[0] == lines[1] for lines in zip(content, current))
1910+
not all(lines[0] == lines[1] for lines in zip(content, current, strict=True))
18831911
): # marked_block found but text is different
18841912
prog = (
18851913
'awk \'BEGIN {{f=1; x=ARGV[2]; ARGV[2]=""}}'
@@ -1894,7 +1922,7 @@ def block(
18941922
if not try_prevent_shell_expansion
18951923
else "'" + "\n".join(content) + "'"
18961924
),
1897-
"> $OUT &&",
1925+
'> "$OUT"',
18981926
real_out,
18991927
)
19001928
else:
@@ -1911,4 +1939,4 @@ def block(
19111939
host.noop("no remove required: markers not found")
19121940
else:
19131941
cmd = StringCommand(f"awk '/{mark_1}/,/{mark_2}/ {{next}} 1'")
1914-
yield StringCommand(out_prep, cmd, q_path, "> $OUT &&", real_out)
1942+
yield StringCommand(out_prep, cmd, q_path, "> $OUT", real_out)

‎tests/operations/files.block/add_existing_block_different_content.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,6 @@
1212
}
1313
},
1414
"commands": [
15-
"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 || stat -f \"%u:%g\" /home/someone/something 2>/dev/null ) $OUT) && mv \"$OUT\" /home/someone/something"
15+
"OUT=\"$(TMPDIR=/tmp mktemp -t pyinfra.XXXXXX)\" && MODE=\"$(stat -c %a /home/someone/something 2>/dev/null || stat -f %Lp /home/someone/something 2>/dev/null)\" && OWNER=\"$(stat -c \"%u:%g\" /home/someone/something 2>/dev/null || stat -f \"%u:%g\" /home/someone/something 2>/dev/null || echo $(id -un):$(id -gn))\" && 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\" && mv \"$OUT\" /home/someone/something && chown \"$OWNER\" /home/someone/something && chmod \"$MODE\" /home/someone/something"
1616
]
1717
}

‎tests/operations/files.block/add_existing_block_different_content_and_backup.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,6 @@
1313
}
1414
},
1515
"commands": [
16-
"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 || stat -f \"%u:%g\" /home/someone/something 2>/dev/null ) $OUT) && mv \"$OUT\" /home/someone/something"
16+
"cp /home/someone/something /home/someone/something.a-timestamp && OUT=\"$(TMPDIR=/tmp mktemp -t pyinfra.XXXXXX)\" && MODE=\"$(stat -c %a /home/someone/something 2>/dev/null || stat -f %Lp /home/someone/something 2>/dev/null)\" && OWNER=\"$(stat -c \"%u:%g\" /home/someone/something 2>/dev/null || stat -f \"%u:%g\" /home/someone/something 2>/dev/null || echo $(id -un):$(id -gn))\" && 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\" && mv \"$OUT\" /home/someone/something && chown \"$OWNER\" /home/someone/something && chmod \"$MODE\" /home/someone/something"
1717
]
1818
}

‎tests/operations/files.block/add_existing_block_different_content_multiple_lines.json‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
{
2+
"require_platform": ["Darwin", "Linux"],
3+
"args": ["/home/someone/something"],
4+
"kwargs": {
5+
"content": ["should be this", "and this", "and even this"],
6+
"line": "before this",
7+
"before": true
8+
},
9+
"facts": {
10+
"files.Block": {
11+
"begin=None, end=None, marker=None, path=/home/someone/something": ["previously was something else"]
12+
}
13+
},
14+
"commands": [
15+
"OUT=\"$(TMPDIR=/tmp mktemp -t pyinfra.XXXXXX)\" && MODE=\"$(stat -c %a /home/someone/something 2>/dev/null || stat -f %Lp /home/someone/something 2>/dev/null)\" && OWNER=\"$(stat -c \"%u:%g\" /home/someone/something 2>/dev/null || stat -f \"%u:%g\" /home/someone/something 2>/dev/null || echo $(id -un):$(id -gn))\" && 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\nand this\nand even this\" > \"$OUT\" && mv \"$OUT\" /home/someone/something && chown \"$OWNER\" /home/someone/something && chmod \"$MODE\" /home/someone/something"
16+
]
17+
}

‎tests/operations/files.block/add_no_existing_block_and_no_line.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,6 @@
1212
}
1313
},
1414
"commands": [
15-
"cat > /home/someone/something <<PYINFRAHERE \n# BEGIN PYINFRA BLOCK\nplease add this\n# END PYINFRA BLOCK\nPYINFRAHERE"
15+
"OUT=\"$(TMPDIR=/tmp mktemp -t pyinfra.XXXXXX)\" && OWNER=\"$(stat -c \"%u:%g\" /home/someone/something 2>/dev/null || stat -f \"%u:%g\" /home/someone/something 2>/dev/null || echo $(id -un):$(id -gn))\" && ( awk '{{print}}' - /dev/null > \"$OUT\" <<PYINFRAHERE\n# BEGIN PYINFRA BLOCK\nplease add this\n# END PYINFRA BLOCK\nPYINFRAHERE\n ) && mv \"$OUT\" /home/someone/something && chown \"$OWNER\" /home/someone/something /home/someone/something"
1616
]
1717
}

‎tests/operations/files.block/add_no_existing_block_and_non_str_line_given.json‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
{
2+
"require_platform": ["Darwin", "Linux"],
3+
"args": ["/home/someone/something"],
4+
"kwargs": {
5+
"content": "please add this",
6+
"after": true,
7+
"line": 12
8+
},
9+
"facts": {
10+
"files.Block": {
11+
"begin=None, end=None, marker=None, path=/home/someone/something": []
12+
}
13+
},
14+
"exception": {
15+
"names": ["OperationTypeError"],
16+
"message": "'line' must be a regex or a string"
17+
}
18+
}

‎tests/operations/files.block/add_no_existing_block_line_provided.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,6 @@
1212
}
1313
},
1414
"commands": [
15-
"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 x } { 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 || stat -f \"%u:%g\" /home/someone/something 2>/dev/null ) $OUT) && mv \"$OUT\" /home/someone/something"
15+
"OUT=\"$(TMPDIR=/tmp mktemp -t pyinfra.XXXXXX)\" && MODE=\"$(stat -c %a /home/someone/something 2>/dev/null || stat -f %Lp /home/someone/something 2>/dev/null)\" && OWNER=\"$(stat -c \"%u:%g\" /home/someone/something 2>/dev/null || stat -f \"%u:%g\" /home/someone/something 2>/dev/null || echo $(id -un):$(id -gn))\" && awk 'BEGIN {x=ARGV[2]; ARGV[2]=\"\"} f!=1 && /^.*before this.*$/ { print x; f=1} END {if (f==0) print x } { print }' /home/someone/something \"# BEGIN PYINFRA BLOCK\nplease add this\n# END PYINFRA BLOCK\" > \"$OUT\" && mv \"$OUT\" /home/someone/something && chown \"$OWNER\" /home/someone/something && chmod \"$MODE\" /home/someone/something"
1616
]
1717
}

‎tests/operations/files.block/add_no_existing_block_line_provided_escape_regex.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,6 @@
1313
}
1414
},
1515
"commands": [
16-
"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 x } { 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 || stat -f \"%u:%g\" /home/someone/something 2>/dev/null ) $OUT) && mv \"$OUT\" /home/someone/something"
16+
"OUT=\"$(TMPDIR=/tmp mktemp -t pyinfra.XXXXXX)\" && MODE=\"$(stat -c %a /home/someone/something 2>/dev/null || stat -f %Lp /home/someone/something 2>/dev/null)\" && OWNER=\"$(stat -c \"%u:%g\" /home/someone/something 2>/dev/null || stat -f \"%u:%g\" /home/someone/something 2>/dev/null || echo $(id -un):$(id -gn))\" && awk 'BEGIN {x=ARGV[2]; ARGV[2]=\"\"} f!=1 && /^.*before this \\*.*$/ { print x; f=1} END {if (f==0) print x } { print }' /home/someone/something \"# BEGIN PYINFRA BLOCK\nplease add this\n# END PYINFRA BLOCK\" > \"$OUT\" && mv \"$OUT\" /home/someone/something && chown \"$OWNER\" /home/someone/something && chmod \"$MODE\" /home/someone/something"
1717
]
1818
}

‎tests/operations/files.block/add_no_existing_file.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,6 @@
1212
}
1313
},
1414
"commands": [
15-
"cat > /home/someone/something <<PYINFRAHERE \n# BEGIN PYINFRA BLOCK\nplease add this\n# END PYINFRA BLOCK\nPYINFRAHERE"
15+
"OUT=\"$(TMPDIR=/tmp mktemp -t pyinfra.XXXXXX)\" && OWNER=\"$(stat -c \"%u:%g\" /home/someone/something 2>/dev/null || stat -f \"%u:%g\" /home/someone/something 2>/dev/null || echo $(id -un):$(id -gn))\" && ( awk '{{print}}' /dev/null - > \"$OUT\" <<PYINFRAHERE\n# BEGIN PYINFRA BLOCK\nplease add this\n# END PYINFRA BLOCK\nPYINFRAHERE\n ) && mv \"$OUT\" /home/someone/something && chown \"$OWNER\" /home/someone/something /home/someone/something"
1616
]
1717
}

‎tests/operations/files.block/remove_but_content_not_none.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@
1111
}
1212
},
1313
"commands": [
14-
"OUT=\"$(TMPDIR=/tmp mktemp -t pyinfra.XXXXXX)\" && awk '/# BEGIN PYINFRA BLOCK/,/# END PYINFRA BLOCK/ {next} 1' /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 || stat -f \"%u:%g\" /home/someone/something 2>/dev/null ) $OUT) && mv \"$OUT\" /home/someone/something"
14+
"OUT=\"$(TMPDIR=/tmp mktemp -t pyinfra.XXXXXX)\" && MODE=\"$(stat -c %a /home/someone/something 2>/dev/null || stat -f %Lp /home/someone/something 2>/dev/null)\" && OWNER=\"$(stat -c \"%u:%g\" /home/someone/something 2>/dev/null || stat -f \"%u:%g\" /home/someone/something 2>/dev/null || echo $(id -un):$(id -gn))\" && awk '/# BEGIN PYINFRA BLOCK/,/# END PYINFRA BLOCK/ {next} 1' /home/someone/something > $OUT && mv \"$OUT\" /home/someone/something && chown \"$OWNER\" /home/someone/something && chmod \"$MODE\" /home/someone/something"
1515
]
1616
}

0 commit comments

Comments
 (0)