Skip to content
Permalink
Browse files

(PUP-5659) Validate all relationships

Before this, only relationships formed with the relationship
operations <- -> <~ and ~> were validated. This validation checkes that
the referenced resource exists in the catalog and reports an error if
not.

The identical relationship formed via use of the meta parameters after,
require, subscribe, and notify were not validated. This led to that
errors in catalogs were not caught until the catalog was applied on the
agent.

This commit makes use of the recently added catalog validators and adds
validatio of all relationships.

When making this change the existing code in the relationship validator
needed refactoring. It was then detected that it used an array.find and
a predicate lambda to see if a variable was one out of 3 possible
symbols. That was measured to be 6x slower that the fastest (hash
lookup= on Ruby 1.9.3. The relationship checking can be called thousands
of times and that statement alone could add several seconds to a catalog
compilation.

Note that the validation runs after the catalog is finalized. That means
that it includes all operations that could have modified the catalog.
(Overrides, queries, relationships, defaults, etc.)

Also note that this means that relationships formed with the arrow
operators are checked twice. This because there is otherwise no
information that points to that source location in case of an error. It
was measured that a mechanism that would avoid doing it twice was just
as costly as doing the check a second time. It may be possible to
instead add additioal location information, but that is a greater
change, that also uses more memory.
  • Loading branch information
hlindberg committed Feb 3, 2016
1 parent c85a51d commit 676f0b7a2d78b2727c991f44e1c23f6df8e5233e
@@ -15,9 +15,27 @@ def validate

private

# A hash lookup is 6x avg times faster than find among 3 values.
CAPABILITY_ACCEPTED_METAPARAMS = {:require => true, :consume => true, :export => true}.freeze

def validate_relationship(param)
unless [:require, :consume, :export].find {|pname| pname == param.name }
raise CatalogValidationError.new("'#{param.name}' is not a valid relationship to a capability", param.file, param.line) if has_capability?(param.value)
# when relationship is to a capability
if has_capability?(param.value)
if ! CAPABILITY_ACCEPTED_METAPARAMS[param.name]
raise CatalogValidationError.new(
"'#{param.name}' is not a valid relationship to a capability",
param.file, param.line)
end
else
# all other relationships requires the referenced resource to exist
refs = param.value.is_a?(Array) ? param.value : [param.value]
refs.each do |r|
unless catalog.resource(r.to_s)
raise CatalogValidationError.new(
"Could not find resource '#{r.to_s}' in parameter '#{param.name.to_s}'",
param.file, param.line)
end
end
end
end

@@ -1160,6 +1160,7 @@ class baz {
end

it "should not favor local scope, (with class not included in topscope)" do
expect {
catalog = compile_to_catalog(<<-PP)
class experiment {
class baz {
@@ -1172,9 +1173,7 @@ class baz {
include experiment
include experiment::baz
PP

expect(catalog).to have_resource("Notify[x]").with_parameter(:require, be_resource("Class[Baz]"))
expect(catalog).to have_resource("Notify[y]").with_parameter(:require, be_resource("Class[Experiment::Baz]"))
}.to raise_error(/Could not find resource 'Class\[Baz\]' in parameter 'require'/)
end
end

@@ -1243,6 +1242,21 @@ class foo::bar::baz {
end
end

describe "relationships to non existing resources" do
[ 'before',
'subscribe',
'notify',
'require'].each do |meta_param|
it "are reported as an error when formed via meta parameter #{meta_param}" do
expect {
compile_to_catalog(<<-PP)
notify{ x : #{meta_param} => Notify[tooth_fairy] }
PP
}.to raise_error(/Could not find resource 'Notify\[tooth_fairy\]' in parameter '#{meta_param}'/)
end
end
end

describe "relationships can be formed" do
def extract_name(ref)
ref.sub(/File\[(\w+)\]/, '\1')

0 comments on commit 676f0b7

Please sign in to comment.
You can’t perform that action at this time.