Skip to content

Commit bdbba6a

Browse files
committed
(MODULES-5651) Do not append infinitely
https://tickets.puppetlabs.com/browse/MODULES-5003 gave rise to #788 and #794 which caused different behavior based on whether the line value was matched by the match regex or not. This resolves the behavior by adding a new parameter to control whether multiple matches should universally be replaced by the line value, or whether matches should be left alone when the line exists elsewhere in the file. This problem only affects modifying multiple lines when the line already exists. I also revised many of the tests and some of the documentation where wrong or ambiguous.
1 parent 2339ea8 commit bdbba6a

File tree

4 files changed

+543
-270
lines changed

4 files changed

+543
-270
lines changed

README.md

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ Values: String.
219219

220220
##### `match`
221221

222-
Specifies a regular expression to compare against existing lines in the file; if a match is found, it is replaced rather than adding a new line. A regex comparison is performed against the line value, and if it does not match, an exception is raised.
222+
Specifies a regular expression to compare against existing lines in the file; if a match is found, it is replaced rather than adding a new line.
223223

224224
Values: String containing a regex.
225225

@@ -236,7 +236,7 @@ Default value: `false`.
236236

237237
##### `multiple`
238238

239-
Specifies whether `match` and `after` can change multiple lines. If set to `false`, an exception is raised if more than one line matches.
239+
Specifies whether `match` and `after` can change multiple lines. If set to `false`, allows file\_line to replace only one line and raises an error if more than one will be replaced. If set to `true`, allows file\_line to replace one or more lines.
240240

241241
Values: `true`, `false`.
242242

@@ -261,12 +261,20 @@ Value: String specifying an absolute path to the file.
261261

262262
##### `replace`
263263

264-
Specifies whether the resource overwrites an existing line that matches the `match` parameter. If set to `false` and a line is found matching the `match` parameter, the line is not placed in the file.
264+
Specifies whether the resource overwrites an existing line that matches the `match` parameter when `line` does not otherwise exist.
265+
266+
If set to `false` and a line is found matching the `match` parameter, the line is not placed in the file.
265267

266268
Boolean.
267269

268270
Default value: `true`.
269271

272+
##### `replace_all_matches_not_matching_line`
273+
274+
Replace all lines matched by `match` parameter, even if `line` already exists in the file.
275+
276+
Default value: `false`.
277+
270278
### Data types
271279

272280
#### `Stdlib::Absolutepath`

lib/puppet/provider/file_line/ruby.rb

Lines changed: 68 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,66 @@ def exists?
1212
found = lines_count > 0
1313
else
1414
match_count = count_matches(new_match_regex)
15-
if resource[:append_on_no_match].to_s == 'false'
16-
found = true
17-
elsif resource[:replace].to_s == 'true'
18-
found = lines_count > 0 && lines_count == match_count
15+
# conditions: match, line, append, replace, replace_all_matches_not_matching_line
16+
# values int int t/f t/f t/f
17+
# cases:
18+
# - false replace, true replace_all_matches_not_matching_line => error (handled in type)
19+
# - false multiple, true replace_all_matches_not_matching_line => error (handled in type)
20+
#
21+
#
22+
#
23+
# - zero match matches, zero line matches, and false append => leave it
24+
# - zero match matches, zero line matches, and true append => append
25+
# - zero match matches, any line matches => leave it
26+
# - any match matches, zero line matches, and false replace => leave it
27+
# - any match matches, zero line matches, and true replace => replace
28+
# - any match matches, true replace_all_matches_not_matching_line => replace
29+
# - any match matches, false replace_all_matches_not_matching_line => go with old behavior
30+
if resource[:ensure] == :present
31+
if match_count == 0
32+
if lines_count == 0
33+
if resource[:append_on_no_match].to_s == 'false'
34+
found = true # lies
35+
else
36+
found = false
37+
end
38+
else
39+
found = true
40+
end
41+
else
42+
if resource[:replace_all_matches_not_matching_line].to_s == 'true'
43+
# there are matches, and replace_all_matches_not_matching_line is true so we want to replace all matches that don't match line... at this point we only get matches if they DONT match line because count_matches takes this into account and will only tell us if it found matches OTHER than line
44+
found = false # unknown lies if the match matches line, otherwise lies
45+
else
46+
if lines_count == 0
47+
if resource[:replace].to_s == 'false'
48+
found = true
49+
else
50+
found = false
51+
end
52+
else
53+
found = true
54+
end
55+
end
56+
end
1957
else
20-
found = match_count > 0
58+
if match_count == 0
59+
if lines_count == 0
60+
found = false
61+
else
62+
found = true
63+
end
64+
else
65+
if lines_count == 0
66+
if resource[:match_for_absence].to_s == 'true'
67+
found = true
68+
else
69+
found = false
70+
end
71+
else
72+
found = true
73+
end
74+
end
2175
end
2276
end
2377
found
@@ -68,7 +122,13 @@ def new_match_regex
68122
end
69123

70124
def count_matches(regex)
71-
lines.select{ |line| line.match(regex) }.size
125+
lines.select do |line|
126+
if resource[:replace_all_matches_not_matching_line].to_s == 'true'
127+
line.match(regex) unless line.chomp == resource[:line]
128+
else
129+
line.match(regex)
130+
end
131+
end.size
72132
end
73133

74134
def handle_create_with_match()
@@ -140,8 +200,9 @@ def handle_destroy_line
140200
end
141201

142202
def handle_append_line
203+
local_lines = lines
143204
File.open(resource[:path],'w') do |fh|
144-
lines.each do |line|
205+
local_lines.each do |line|
145206
fh.puts(line)
146207
end
147208
fh.puts(resource[:line])

lib/puppet/type/file_line.rb

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,13 @@ def retrieve
151151
defaultto true
152152
end
153153

154+
newparam(:replace_all_matches_not_matching_line) do
155+
desc 'Configures the behavior of replacing all lines in a file which match the `match` parameter regular expression, regardless of whether the specified line is already present in the file.'
156+
157+
newvalues(true, false)
158+
defaultto false
159+
end
160+
154161
newparam(:encoding) do
155162
desc 'For files that are not UTF-8 encoded, specify encoding such as iso-8859-1'
156163
defaultto 'UTF-8'
@@ -168,6 +175,12 @@ def retrieve
168175
end
169176

170177
validate do
178+
if self[:replace_all_matches_not_matching_line].to_s == 'true' and self[:multiple].to_s == 'false'
179+
raise(Puppet::Error, "multiple must be true when replace_all_matches_not_matching_line is true")
180+
end
181+
if self[:replace_all_matches_not_matching_line].to_s == 'true' and self[:replace].to_s == 'false'
182+
raise(Puppet::Error, "replace must be true when replace_all_matches_not_matching_line is true")
183+
end
171184
unless self[:line]
172185
unless (self[:ensure].to_s == 'absent') and (self[:match_for_absence].to_s == 'true') and self[:match]
173186
raise(Puppet::Error, "line is a required attribute")

0 commit comments

Comments
 (0)