Skip to content

Commit 0befcf3

Browse files
joshcooperjoshcooper
committed
(PUP-7582) Error on virtual and exported classes
Previously puppet would warn or error if it encountered a virtual class (indicated by `@class`) or an exported class (indicated by `@@class`) based on the `Puppet[:strict]` setting. By default, puppet would warn, yet include resources from the virtual class in the catalog. So the resources in the virtual class weren't virtual! Also note, it wasn't possible to turn off the warning. This commit makes puppet always error on virtual and exported classes. The validator_factory_4_0.rb validates static resource expressions, while runtime3_support validates `create_resources` calls, which can be used to create resources at runtime, so we need to prevent things like: create_resources('@Class', {}) The integration tests are removed since we already test for validation errors in unit tests.
1 parent b3fe419 commit 0befcf3

File tree

5 files changed

+16
-72
lines changed

5 files changed

+16
-72
lines changed

‎lib/puppet/pops/evaluator/runtime3_support.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,7 @@ def initialize
505505
# Store config issues, ignore or warning
506506
p[Issues::RT_NO_STORECONFIGS_EXPORT] = Puppet[:storeconfigs] ? :ignore : :warning
507507
p[Issues::RT_NO_STORECONFIGS] = Puppet[:storeconfigs] ? :ignore : :warning
508-
p[Issues::CLASS_NOT_VIRTUALIZABLE] = Puppet[:strict] == :off ? :warning : Puppet[:strict]
508+
p[Issues::CLASS_NOT_VIRTUALIZABLE] = :error
509509
p[Issues::NUMERIC_COERCION] = Puppet[:strict] == :off ? :ignore : Puppet[:strict]
510510
p[Issues::SERIALIZATION_DEFAULT_CONVERTED_TO_STRING] = Puppet[:strict] == :off ? :warning : Puppet[:strict]
511511
p[Issues::SERIALIZATION_UNKNOWN_CONVERTED_TO_STRING] = Puppet[:strict] == :off ? :warning : Puppet[:strict]

‎lib/puppet/pops/validation/validator_factory_4_0.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ def severity_producer
3535
p[Issues::DUPLICATE_KEY] = Puppet[:strict] == :off ? :ignore : Puppet[:strict]
3636
p[Issues::NAME_WITH_HYPHEN] = :error
3737
p[Issues::EMPTY_RESOURCE_SPECIALIZATION] = :ignore
38-
p[Issues::CLASS_NOT_VIRTUALIZABLE] = Puppet[:strict] == :off ? :warning : Puppet[:strict]
38+
p[Issues::CLASS_NOT_VIRTUALIZABLE] = :error
3939
p
4040
end
4141
end

‎spec/integration/parser/catalog_spec.rb

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -48,44 +48,6 @@ class { "third": }
4848
expect(resources_in(agent_catalog)).
4949
to include_in_order(*resources_in_declaration_order)
5050
end
51-
52-
it "does not contain unrealized, virtual resources" do
53-
virtual_resources = ["Unrealized[unreal]", "Class[Unreal]"]
54-
55-
master_catalog, agent_catalog = master_and_agent_catalogs_for(<<-EOM)
56-
class unreal { }
57-
define unrealized() { }
58-
59-
class real {
60-
@unrealized { "unreal": }
61-
@class { "unreal": }
62-
}
63-
64-
include real
65-
EOM
66-
67-
expect(resources_in(master_catalog)).to_not include(*virtual_resources)
68-
expect(resources_in(agent_catalog)).to_not include(*virtual_resources)
69-
end
70-
71-
it "does not contain unrealized, exported resources" do
72-
exported_resources = ["Unrealized[unreal]", "Class[Unreal]"]
73-
74-
master_catalog, agent_catalog = master_and_agent_catalogs_for(<<-EOM)
75-
class unreal { }
76-
define unrealized() { }
77-
78-
class real {
79-
@@unrealized { "unreal": }
80-
@@class { "unreal": }
81-
}
82-
83-
include real
84-
EOM
85-
86-
expect(resources_in(master_catalog)).to_not include(*exported_resources)
87-
expect(resources_in(agent_catalog)).to_not include(*exported_resources)
88-
end
8951
end
9052
end
9153

‎spec/unit/parser/functions/create_resources_spec.rb

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -257,31 +257,13 @@ class bar($one) {
257257
expect(catalog.resource(:class, "bar")).not_to be_nil
258258
end
259259

260-
[:off, :warning].each do | strictness |
261-
it "should warn if strict = #{strictness} and class is exported" do
262-
Puppet[:strict] = strictness
263-
collect_notices('class test{} create_resources("@@class", {test => {}})')
264-
expect(warnings).to include(/Classes are not virtualizable/)
265-
end
266-
end
267-
268-
it 'should error if strict = error and class is exported' do
269-
Puppet[:strict] = :error
260+
it 'should error if class is exported' do
270261
expect{
271262
compile_to_catalog('class test{} create_resources("@@class", {test => {}})')
272263
}.to raise_error(/Classes are not virtualizable/)
273264
end
274265

275-
[:off, :warning].each do | strictness |
276-
it "should warn if strict = #{strictness} and class is virtual" do
277-
Puppet[:strict] = strictness
278-
collect_notices('class test{} create_resources("@class", {test => {}})')
279-
expect(warnings).to include(/Classes are not virtualizable/)
280-
end
281-
end
282-
283-
it 'should error if strict = error and class is virtual' do
284-
Puppet[:strict] = :error
266+
it 'should error if class is virtual' do
285267
expect{
286268
compile_to_catalog('class test{} create_resources("@class", {test => {}})')
287269
}.to raise_error(/Classes are not virtualizable/)

‎spec/unit/pops/validator/validator_spec.rb

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -179,17 +179,17 @@ def with_environment(environment, env_params = {})
179179
expect(acceptor).to have_issue(Puppet::Pops::Issues::DUPLICATE_KEY)
180180
end
181181

182-
it 'produces a warning for virtual class resource' do
182+
it 'produces an error for virtual class resource' do
183183
acceptor = validate(parse('@class { test: }'))
184-
expect(acceptor.warning_count).to eql(1)
185-
expect(acceptor.error_count).to eql(0)
184+
expect(acceptor.warning_count).to eql(0)
185+
expect(acceptor.error_count).to eql(1)
186186
expect(acceptor).to have_issue(Puppet::Pops::Issues::CLASS_NOT_VIRTUALIZABLE)
187187
end
188188

189-
it 'produces a warning for exported class resource' do
189+
it 'produces an error for exported class resource' do
190190
acceptor = validate(parse('@@class { test: }'))
191-
expect(acceptor.warning_count).to eql(1)
192-
expect(acceptor.error_count).to eql(0)
191+
expect(acceptor.warning_count).to eql(0)
192+
expect(acceptor.error_count).to eql(1)
193193
expect(acceptor).to have_issue(Puppet::Pops::Issues::CLASS_NOT_VIRTUALIZABLE)
194194
end
195195

@@ -310,17 +310,17 @@ def with_environment(environment, env_params = {})
310310
expect(acceptor).to have_issue(Puppet::Pops::Issues::DUPLICATE_DEFAULT)
311311
end
312312

313-
it 'produces a warning for virtual class resource' do
313+
it 'produces an error for virtual class resource' do
314314
acceptor = validate(parse('@class { test: }'))
315-
expect(acceptor.warning_count).to eql(1)
316-
expect(acceptor.error_count).to eql(0)
315+
expect(acceptor.warning_count).to eql(0)
316+
expect(acceptor.error_count).to eql(1)
317317
expect(acceptor).to have_issue(Puppet::Pops::Issues::CLASS_NOT_VIRTUALIZABLE)
318318
end
319319

320-
it 'produces a warning for exported class resource' do
320+
it 'produces an error for exported class resource' do
321321
acceptor = validate(parse('@@class { test: }'))
322-
expect(acceptor.warning_count).to eql(1)
323-
expect(acceptor.error_count).to eql(0)
322+
expect(acceptor.warning_count).to eql(0)
323+
expect(acceptor.error_count).to eql(1)
324324
expect(acceptor).to have_issue(Puppet::Pops::Issues::CLASS_NOT_VIRTUALIZABLE)
325325
end
326326
end

0 commit comments

Comments
 (0)