Merge pull request #9003 from AriaXLi/pup-11689/strict_mode_default

(PUP-11689) Set strict_variables to true to enable strict mode by default

Author: AriaXLi

lib/puppet/defaults.rb

@@ -2210,7 +2210,7 @@ def self.initialize_default_settings!(settings)
     EOT
     },
   :strict_variables => {
-    :default => false,
+    :default => true,
     :type => :boolean,
     :desc => <<-'EOT'
       Causes an evaluation error when referencing unknown variables. (This does not affect

spec/integration/application/agent_spec.rb

@@ -993,13 +993,11 @@ def mounts
         agent.command_line.args << '--test'
         expect {
           agent.run
-        }.to exit_with(2)
+        }.to exit_with(1)
           .and output(
-            match(/defined 'message' as 'legacy '/)
-              .and match(/defined 'message' as 'custom a custom value'/)
-              .and match(/defined 'message' as 'external an external value'/)
-            ).to_stdout
-          .and output(/Warning: Unknown variable: 'osfamily'/).to_stderr
+            match(/Error: Evaluation Error: Unknown variable: 'osfamily'/)
+              .and match(/Error: Could not retrieve catalog from remote server: Error 500 on SERVER:/)
+          ).to_stderr
       end
     end
   end

spec/integration/parser/compiler_spec.rb

@@ -693,8 +693,19 @@ class a { $_a = 10}
         end.to raise_error(/Illegal variable name/)
       end
 
-      it 'a missing variable as default value becomes undef' do
-        # strict variables not on
+      it 'a missing variable as default causes an evaluation error' do
+        # when strict variables not on
+        expect {
+          compile_to_catalog(<<-MANIFEST)
+          class a ($b=$x) { notify {test: message=>"yes ${undef == $b}" } }
+            include a
+          MANIFEST
+        }.to raise_error(/Evaluation Error: Unknown variable: 'x'/)
+      end
+
+      it 'a missing variable as default value becomes undef when strict_variables is off' do
+        # strict variables on by default for 8.x
+        Puppet[:strict_variables] = false
         catalog = compile_to_catalog(<<-MANIFEST)
         class a ($b=$x) { notify {test: message=>"yes ${undef == $b}" } }
           include a

spec/integration/parser/conditionals_spec.rb

@@ -68,6 +68,8 @@
     end
 
     it "evaluates undefined variables as false" do
+      # strict_variables is off so behavior this test is trying to check isn't stubbed out
+      Puppet[:strict_variables] = false
       catalog = compile_to_catalog(<<-CODE)
       if $undef_var {
       } else {

spec/integration/parser/scope_spec.rb

@@ -276,6 +276,8 @@ class a inherits parent { }
 
     ['a:.b', '::a::b'].each do |ref|
       it "does not resolve a qualified name on the form #{ref} against top scope" do
+        # strict_variables is off so behavior this test is trying to check isn't stubbed out
+        Puppet[:strict_variables] = false
         expect_the_message_not_to_be("from topscope") do <<-"MANIFEST"
               class c {
                 notify { 'something': message => "$#{ref}" }
@@ -297,6 +299,8 @@ class a inherits parent { }
 
     ['a:.b', '::a::b'].each do |ref|
       it "does not resolve a qualified name on the form #{ref} against node scope" do
+        # strict_variables is off so behavior this test is trying to check isn't stubbed out
+        Puppet[:strict_variables] = false
         expect_the_message_not_to_be("from node") do <<-MANIFEST
               class c {
                 notify { 'something': message => "$a::b" }
@@ -626,8 +630,9 @@ class bar {
       end
     end
 
-    it "finds nil when the only set variable is in the dynamic scope" do
-      expect_the_message_to_be(nil) do <<-MANIFEST
+    it "raises an evaluation error when the only set variable is in the dynamic scope" do
+      expect {
+        compile_to_catalog(<<-MANIFEST)
             node default {
               include baz
             }
@@ -641,7 +646,7 @@ class baz {
               include bar
             }
         MANIFEST
-      end
+      }.to raise_error(/Evaluation Error: Unknown variable: 'var'./)
     end
 
     it "ignores the value in the dynamic scope for a defined type" do

spec/unit/application/lookup_spec.rb

@@ -673,6 +673,8 @@ def run_lookup(lookup)
       let(:node) { Puppet::Node.new("testnode", :facts => facts, :environment => 'puppet_func_provider') }
 
       it "works OK in the absense of '--compile'" do
+        # strict_variables is off so behavior this test is trying to check isn't stubbed out
+        Puppet[:strict_variables] = false
         lookup.options[:node] = node
         allow(lookup.command_line).to receive(:args).and_return(['c'])
         lookup.options[:render_as] = :s

spec/unit/functions/epp_spec.rb

@@ -18,7 +18,12 @@
       expect(eval_template("all your base <%= $what::is %> to us")).to eq("all your base are belong to us")
     end
 
-    it "get nil accessing a variable that does not exist" do
+    it "gets error accessing a variable that does not exist" do
+      expect { eval_template("<%= $kryptonite == undef %>")}.to raise_error(/Evaluation Error: Unknown variable: 'kryptonite'./)
+    end
+
+    it "get nil accessing a variable that does not exist when strict_variables is off" do
+      Puppet[:strict_variables] = false
       expect(eval_template("<%= $kryptonite == undef %>")).to eq("true")
     end
 

spec/unit/functions/inline_epp_spec.rb

@@ -16,7 +16,12 @@
       expect(eval_template("all your base <%= $what %> to us")).to eq("all your base are belong to us")
     end
 
-    it "get nil accessing a variable that does not exist" do
+    it "gets error accessing a variable that does not exist" do
+      expect { eval_template("<%= $kryptonite == undef %>")}.to raise_error(/Evaluation Error: Unknown variable: 'kryptonite'./)
+    end
+
+    it "get nil accessing a variable that does not exist when strict_variables is off" do
+      Puppet[:strict_variables] = false
       expect(eval_template("<%= $kryptonite == undef %>")).to eq("true")
     end
 

spec/unit/functions/return_spec.rb

@@ -19,6 +19,8 @@
     end
 
     it 'with undef value as function result when not given an argument' do
+      # strict_variables is off so behavior this test is trying to check isn't stubbed out
+      Puppet[:strict_variables] = false
       expect(compile_to_catalog(<<-CODE)).to have_resource('Notify[xy]')
           function please_return() {
             [1,2,3].map |$x| { if $x == 1 { return() } 200 }

spec/unit/indirector/catalog/compiler_spec.rb

@@ -514,9 +514,10 @@ def a_request_that_contains(facts)
 
     it "should not add 'pe_serverversion' when FOSS" do
       allow(Puppet::Node.indirection).to receive(:find).with(node_name, anything).and_return(node)
-      catalog = compiler.find(request)
-
-      expect(catalog.resource('notify', 'PE Version: 2019.2.0')).to be_nil
+      expect {
+        catalog = compiler.find(request)
+        catalog.resource('notify', 'PE Version: 2019.2.0') 
+      }.to raise_error(/Evaluation Error: Unknown variable: 'pe_serverversion'./)
     end
 
     it "should add 'pe_serverversion' when PE" do