Skip to content

Commit

Permalink
(PDB-3620) Use string 'alias' as a key in the parameters hash
Browse files Browse the repository at this point in the history
In Puppet 5, Catalog#to_data_hash will only emit data types that are
safe for JSON, YAML, etc, and in particular will never emit Symbols.

When synthesizing an edge from any resource to an aliased resource:

    require => Package['my_alias']

puppetdb would fail to find the target resource in the catalog. And that
was because it was looking for the `:alias` parameter in the catalog
hash, which didn't exist.

This commit updates the add_namevar_alias and map_aliases_to_titles to
use the String form of 'alias'. It adds a unit test demonstrating the
problem, and updates other tests that relied on the previous symbol
behavior.

Note the terminus behaves different if the resource specifying the
`alias` metaparam has multiple namevars (eg Package) or not.
  • Loading branch information
joshcooper committed Jul 20, 2017
1 parent 022354d commit be63412
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 8 deletions.
5 changes: 3 additions & 2 deletions puppet/lib/puppet/indirector/catalog/puppetdb.rb
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ def add_namevar_aliases(hash, catalog)
# is both an optimization and a safeguard.
next if real_resource.key_attributes.count > 1

# Symbol is correct in this context
aliases = [real_resource[:alias]].flatten.compact

# Non-isomorphic resources aren't unique based on namevar, so we can't
Expand All @@ -259,7 +260,7 @@ def add_namevar_aliases(hash, catalog)
end
end

resource['parameters'][:alias] = aliases unless aliases.empty?
resource['parameters']['alias'] = aliases unless aliases.empty?
end
end

Expand Down Expand Up @@ -306,7 +307,7 @@ def map_aliases_to_title(hash)
profile("Map aliases to title (resource count: #{resources.count})",
[:puppetdb, :aliases, :map_to_title]) do
resources.each do |resource|
names = Array(resource['parameters'][:alias]) || []
names = Array(resource['parameters']['alias']) || []
resource_hash = {'type' => resource['type'], 'title' => resource['title']}
names.each do |name|
alias_array = [resource['type'], name]
Expand Down
32 changes: 26 additions & 6 deletions puppet/spec/unit/indirector/catalog/puppetdb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def catalog_data_hash
end

resource.should_not be_nil
resource['parameters'][:alias].should include(name)
resource['parameters']['alias'].should include(name)
end

context "with resource types that provide #title_patterns" do
Expand Down Expand Up @@ -203,8 +203,8 @@ def catalog_data_hash
# this test should cover other resource types that fall into
# this category as well.
resource.should_not be_nil
resource['parameters'][:alias].should_not be_nil
resource['parameters'][:alias].should include('/tmp/foo')
resource['parameters']['alias'].should_not be_nil
resource['parameters']['alias'].should include('/tmp/foo')
end
end
end
Expand All @@ -218,7 +218,7 @@ def catalog_data_hash
end

resource.should_not be_nil
resource['parameters'][:alias].should be_nil
resource['parameters']['alias'].should be_nil
end

describe "for resources with composite namevars" do
Expand All @@ -237,7 +237,7 @@ def catalog_data_hash
end

resource.should_not be_nil
resource['parameters'][:alias].should be_nil
resource['parameters']['alias'].should be_nil
end
end

Expand All @@ -255,7 +255,7 @@ def catalog_data_hash
end

resource.should_not be_nil
resource['parameters'][:alias].should == ['something awesome']
resource['parameters']['alias'].should == ['something awesome']
end
end
end
Expand Down Expand Up @@ -570,6 +570,26 @@ def catalog_data_hash
result['edges'].should include(edge)
end

it "should produce an edge when referencing an aliased resource that supports composite namevars" do
Puppet[:code] = <<-MANIFEST
package { 'foo':
ensure => present,
alias => 'bar'
}
notify { 'hello':
require => Package['bar']
}
MANIFEST

result = subject.munge_catalog(catalog, Time.now.utc)

edge = {'source' => {'type' => 'Package', 'title' => 'foo'},
'target' => {'type' => 'Notify', 'title' => 'hello'},
'relationship' => 'required-by'}

result['edges'].should include(edge)
end

it "should produce a reasonable error message for a missing 'before' relationship" do

error = Gem::Version.new(Puppet.version) < Gem::Version.new("5.0.0") ?
Expand Down

0 comments on commit be63412

Please sign in to comment.