Please review pull request #794: Bug/2.7.x/8174 incorrect warning about deprecated scoping opened by (zaphod42)
Description:
Ed Brannin reported that the previous fix for bug #8174 didn't work with defined resource types. This pull request adds tests for the the case of using define and cherry picks two commits that occurred on the master branch which solve the issue.
The two cherry picked commits stop the topscope from being re-assigned after the nodes are evaluated.
- Opened: Wed May 16 20:58:15 UTC 2012
- Based on: puppetlabs:2.7.x (97962ec22daf7680d6738e458629d64de08c5f89)
- Requested merge: zaphod42:bug/2.7.x/8174-incorrect-warning-about-deprecated-scoping (c6e006edc54bf6e022e2207ed2d000c70350a179)
Diff follows:
diff --git a/acceptance/tests/language/node_overrides_topscope_when_using_enc.rb b/acceptance/tests/language/node_overrides_topscope_when_using_enc.rb
new file mode 100644
index 0000000..28c99a0
--- /dev/null
+++ b/acceptance/tests/language/node_overrides_topscope_when_using_enc.rb
@@ -0,0 +1,67 @@
+test_name "ENC still allows a node to override a topscope var"
+
+testdir = master.tmpdir('scoping_deprecation')
+
+create_remote_file(master, "#{testdir}/puppet.conf", <<END)
+[main]
+node_terminus = exec
+external_nodes = "#{testdir}/enc"
+manifest = "#{testdir}/site.pp"
+modulepath = "#{testdir}/modules"
+END
+
+on master, "mkdir -p #{testdir}/modules/a/manifests"
+
+create_remote_file(master, "#{testdir}/enc", <<-PP)
+#!/usr/bin/env sh
+
+cat <<END
+---
+classes:
+ a
+parameters:
+ enc_var: "Set from ENC."
+END
+exit 0
+PP
+
+agent_names = agents.map { |agent| "'#{agent.to_s}'" }.join(', ')
+create_remote_file(master, "#{testdir}/site.pp", <<-PP)
+$top_scope = "set from site.pp"
+node default {
+ $enc_var = "ENC overridden in default node."
+}
+
+node #{agent_names} inherits default {
+ $top_scope = "top_scope overridden in agent node."
+}
+PP
+create_remote_file(master, "#{testdir}/modules/a/manifests/init.pp", <<-PP)
+class a {
+ notify { "from enc": message => $enc_var }
+ notify { "from site.pp": message => $top_scope }
+}
+PP
+
+on master, "chown -R root:puppet #{testdir}"
+on master, "chmod -R g+rwX #{testdir}"
+on master, "chmod -R a+x #{testdir}/enc"
+
+assert_log_on_master_contains = lambda do |string|
+ on master, "grep '#{string}' #{testdir}/log"
+end
+
+assert_log_on_master_does_not_contain = lambda do |string|
+ on master, "grep -v '#{string}' #{testdir}/log"
+end
+
+with_master_running_on(master, "--config #{testdir}/puppet.conf --debug --verbose --daemonize --dns_alt_names=\"puppet,$(hostname -s),$(hostname -f)\" --autosign true") do
+ agents.each do |agent|
+ run_agent_on(agent, "--no-daemonize --onetime --verbose --server #{master}")
+
+ assert_match("top_scope overridden in agent node.", stdout)
+ assert_match("ENC overridden in default node.", stdout)
+ end
+end
+
+on master, "rm -rf #{testdir}"
diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
index 01accea..392091e 100644
--- a/lib/puppet/parser/compiler.rb
+++ b/lib/puppet/parser/compiler.rb
@@ -27,7 +27,7 @@ def self.compile(node)
raise Puppet::Error, "#{detail} on node #{node.name}"
end
- attr_reader :node, :facts, :collections, :catalog, :node_scope, :resources, :relationships
+ attr_reader :node, :facts, :collections, :catalog, :resources, :relationships, :topscope
# Add a collection to the global list.
def add_collection(coll)
@@ -127,7 +127,7 @@ def environment
# Evaluate all of the classes specified by the node.
def evaluate_node_classes
- evaluate_classes(@node.classes, topscope)
+ evaluate_classes(@node.classes, @node_scope || topscope)
end
# Evaluate each specified class in turn. If there are any classes we can't
@@ -201,12 +201,6 @@ def resource_overrides(resource)
@resource_overrides[resource.ref]
end
- # The top scope is usually the top-level scope, but if we're using AST nodes,
- # then it is instead the node's scope.
- def topscope
- node_scope || @topscope
- end
-
private
# If ast nodes are enabled, then see if we can find and evaluate one.
@@ -229,8 +223,6 @@ def evaluate_ast_node
resource.evaluate
- # Now set the node scope appropriately, so that :topscope can
- # behave differently.
@node_scope = topscope.class_scope(astnode)
end
diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb
index 1405f26..6d91bed 100644
--- a/lib/puppet/parser/scope.rb
+++ b/lib/puppet/parser/scope.rb
@@ -241,6 +241,7 @@ def twoscope_lookupvar(name, options = {})
# Save the originating scope for the request
options[:origin] = self unless options[:origin]
table = ephemeral?(name) ? @ephemeral.last : @symtable
+
if name =~ /^(.*)::(.+)$/
begin
qualified_scope($1).twoscope_lookupvar($2, options.merge({:origin => nil}))
@@ -250,7 +251,7 @@ def twoscope_lookupvar(name, options = {})
:undefined
end
# If the value is present and either we are top/node scope or originating scope...
- elsif (ephemeral_include?(name) or table.include?(name)) and (compiler and self == compiler.topscope or (self.resource and self.resource.type == "Node") or self == options[:origin])
+ elsif (ephemeral_include?(name) or table.include?(name)) and (compiler and self == compiler.topscope or (resource and resource.type == "Node") or self == options[:origin])
table[name]
elsif resource and resource.type == "Class" and parent_type = resource.resource_type.parent
class_scope(parent_type).twoscope_lookupvar(name,options.merge({:origin => nil}))
diff --git a/spec/unit/parser/compiler_spec.rb b/spec/unit/parser/compiler_spec.rb
index 2478d27..c45fcc9 100755
--- a/spec/unit/parser/compiler_spec.rb
+++ b/spec/unit/parser/compiler_spec.rb
@@ -741,23 +741,6 @@ def add_resource(name, parent = nil)
@compiler.send(:evaluate_ast_node)
end
-
- it "should set the node's scope as the top scope" do
- node_resource = stub 'node resource', :ref => "Node[c]", :evaluate => nil, :type => "node"
- node_class = stub 'node', :name => "c", :ensure_in_catalog => node_resource
-
- @compiler.known_resource_types.stubs(:node).with("c").returns(node_class)
-
- # The #evaluate method normally does this.
- scope = stub 'scope', :source => "mysource"
- @compiler.topscope.expects(:class_scope).with(node_class).returns(scope)
- node_resource.stubs(:evaluate)
- @compiler.stubs :create_settings_scope
-
- @compiler.compile
-
- @compiler.topscope.should equal(scope)
- end
end
describe "when managing resource overrides" do
diff --git a/spec/unit/parser/scope_spec.rb b/spec/unit/parser/scope_spec.rb
index f9020e7..370c362 100755
--- a/spec/unit/parser/scope_spec.rb
+++ b/spec/unit/parser/scope_spec.rb
@@ -280,6 +280,22 @@ class baz {
end
end
+ it "finds values in its included scope for a defined type (DEPRECATED)" do
+ expect_the_message_to_be('foo_msg') do <<-MANIFEST
+ node default {
+ include foo
+ }
+ class foo {
+ $var = "foo_msg"
+ bar { "testing": }
+ }
+ define bar() {
+ notify { 'something': message => $var, }
+ }
+ MANIFEST
+ end
+ end
+
it "recognizes a dynamically scoped boolean (DEPRECATED)" do
expect_the_message_to_be(true) do <<-MANIFEST
node default {
@@ -530,6 +546,33 @@ class foo {
MANIFEST
end
end
+
+ it "finds top scope variables referenced inside a defined type" do
+ expect_the_message_to_be('top_msg') do <<-MANIFEST
+ $var = "top_msg"
+ node default {
+ foo { "testing": }
+ }
+ define foo() {
+ notify { 'something': message => $var, }
+ }
+ MANIFEST
+ end
+ end
+
+ it "finds node scope variables referenced inside a defined type" do
+ expect_the_message_to_be('node_msg') do <<-MANIFEST
+ $var = "top_msg"
+ node default {
+ $var = "node_msg"
+ foo { "testing": }
+ }
+ define foo() {
+ notify { 'something': message => $var, }
+ }
+ MANIFEST
+ end
+ end
end
end
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
