Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions lib/puppet/resource/type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -206,11 +206,17 @@ def instantiate_resource(scope, resource)

def name
if type == :node && name_is_regex?
# Normalize lookarround regex patthern
# Normalize lookaround regex pattern to a unique, lookup-safe lowercase string.
# The LOOKAROUND_OPERATORS tokens are uppercased to keep them distinct from ordinary
# regex characters, but the final result MUST be downcased so it matches the key
# produced by munge_name() (which calls String#downcase) inside TypeCollection.
# Without this, a regex such as /^thing-(foo|bar)(?:-test)?-(\d+)/ produces a
# mixed-case synthetic key that is stored correctly but can never be found again
# via the downcased lookup path, causing "Cannot find definition Node" at runtime.
internal_name = @name.source.downcase.gsub(/\(\?[^)]*\)/) do |str|
str.gsub(/./) { |ch| LOOKAROUND_OPERATORS[ch] || ch }
end
"__node_regexp__#{internal_name.gsub(/[^-\w:.]/, '').sub(/^\.+/, '')}"
"__node_regexp__#{internal_name.gsub(/[^-\w:.]/, '').sub(/^\.+/, '')}".downcase
else
@name
end
Expand Down
59 changes: 58 additions & 1 deletion spec/integration/parser/node_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class foo {
end.not_to raise_error
end

it 'does not raise an error with 2 regex node names are the same due to lookarround pattern' do
it 'does not raise an error with 2 regex node names are the same due to lookaround pattern' do
expect do
compile_to_catalog(<<-MANIFEST, Puppet::Node.new("async"))
node /(?<!a)sync/ { }
Expand All @@ -103,6 +103,63 @@ class foo {
end.to raise_error(Puppet::Error, /Node '__node_regexp__a.c' is already defined/)
end

# Regression test for https://github.com/OpenVoxProject/openvox/issues/14
# A regex with non-capturing group syntax (?:...) or other lookaround constructs
# caused uppercase characters to appear in the synthetic node key, which is stored
# in the TypeCollection hash. Lookups always downcase the key via munge_name(), so
# the node could never be found again, producing "Cannot find definition Node".
it 'matches a node whose regex contains a non-capturing group' do
catalog = compile_to_catalog(<<-MANIFEST, Puppet::Node.new("thing-foo-42"))
node /^thing-(foo|bar)(?:-test)?-(\\d+)$/ { notify { matched: } }
node default { notify { unmatched: } }
MANIFEST

expect(catalog).not_to have_resource('Notify[unmatched]')
expect(catalog).to have_resource('Notify[matched]')
end

it 'matches a node whose regex contains a non-capturing group for the optional suffix' do
catalog = compile_to_catalog(<<-MANIFEST, Puppet::Node.new("thing-bar-test-7"))
node /^thing-(foo|bar)(?:-test)?-(\\d+)$/ { notify { matched: } }
node default { notify { unmatched: } }
MANIFEST

expect(catalog).not_to have_resource('Notify[unmatched]')
expect(catalog).to have_resource('Notify[matched]')
end

it 'does not match a node when the non-capturing-group regex prefix does not match' do
catalog = compile_to_catalog(<<-MANIFEST, Puppet::Node.new("thing-baz-7"))
node /^thing-(foo|bar)(?:-test)?-(\\d+)$/ { notify { matched: } }
node default { notify { unmatched: } }
MANIFEST

expect(catalog).not_to have_resource('Notify[matched]')
expect(catalog).to have_resource('Notify[unmatched]')
end

it 'matches a node whose regex contains a negative lookahead' do
# /^(db01)\.(?!hosts).*$/ matches db01.<anything-except-hosts...>
# e.g. db01.example.com matches, db01.hosts.example.com does not
catalog = compile_to_catalog(<<-MANIFEST, Puppet::Node.new("db01.example.com"))
node /^(db01)\.(?!hosts).*$/ { notify { matched: } }
node default { notify { unmatched: } }
MANIFEST

expect(catalog).not_to have_resource('Notify[unmatched]')
expect(catalog).to have_resource('Notify[matched]')
end

it 'does not match a node excluded by a negative lookahead' do
catalog = compile_to_catalog(<<-MANIFEST, Puppet::Node.new("db01.hosts.example.com"))
node /^(db01)\.(?!hosts).*$/ { notify { matched: } }
node default { notify { unmatched: } }
MANIFEST

expect(catalog).not_to have_resource('Notify[matched]')
expect(catalog).to have_resource('Notify[unmatched]')
end

it 'provides captures from the regex in the node body' do
catalog = compile_to_catalog(<<-MANIFEST, Puppet::Node.new("nodename"))
node /(.*)/ { notify { "$1": } }
Expand Down
15 changes: 15 additions & 0 deletions spec/unit/resource/type_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,21 @@
expect(Puppet::Resource::Type.new(:node, /W/).name).to eq("__node_regexp__w")
end

# Regression coverage for issue #14: lookaround syntax must be encoded into a
# lowercase synthetic name so node lookups can find the stored regex node again.
it "should normalize lookaround syntax into a lowercase synthetic name" do
expect(Puppet::Resource::Type.new(:node, /(?<!a)sync/).name).to eq("__node_regexp__lpqultexarpsync")
end

it "should keep lookaround names distinct from otherwise similar regexes" do
lookaround = Puppet::Resource::Type.new(:node, /(?<!a)sync/).name
plain = Puppet::Resource::Type.new(:node, /async/).name

expect(lookaround).not_to eq(plain)
expect(lookaround).to eq(lookaround.downcase)
expect(plain).to eq(plain.downcase)
end

it "should remove non-alpha characters when returning the name as a string" do
expect(Puppet::Resource::Type.new(:node, /w*w/).name).not_to include("*")
end
Expand Down
Loading