From 5e5d056025d1d5042f23024b3a48b206b41764a1 Mon Sep 17 00:00:00 2001 From: Ben Roberts Date: Thu, 13 Oct 2016 16:52:29 +0100 Subject: [PATCH] Remove required attribute validation Previously, the entropy types would require that either package, tag, or repo were provided, since that makes sense for the entropy files. Having a resource that could not provide any of these would leave a malformed entry in the files. I've had to remove that due to a deficiency in Puppet that's taken a long time to track down. While the validation works fine for resources specified in manifests, it breaks when parsing the records back in. Here's why: - The provider instances method is called to retrieve a list of all entries in the entropy files. This correctly parses the name and properties into the provider instance object - Puppet's type.rb enumerates through each of these, and tries to create a new Type insance using just the name and provider parameters from the provider instance. It then intends to iterate through the properties and add them one at a time. https://github.com/puppetlabs/puppet/blob/0c2157974a5d9523d438de2fd3b16b4ccaa3b87a/lib/puppet/type.rb#L1180 - The problem is that the top-level validation function is called when the object is first created, and at this time, none of the properties have been set, so the required properties have not been set and validation fails. The top-level validation block cannot mandate a property be set, only other types of condition. --- lib/puppet/type/entropy_keywords.rb | 2 -- lib/puppet/type/entropy_mask.rb | 2 -- lib/puppet/type/entropy_splitdebug.rb | 2 -- lib/puppet/type/entropy_splitdebug_mask.rb | 2 -- lib/puppet/type/entropy_unmask.rb | 2 -- spec/unit/type/entropy_keywords_spec.rb | 6 ------ 6 files changed, 16 deletions(-) diff --git a/lib/puppet/type/entropy_keywords.rb b/lib/puppet/type/entropy_keywords.rb index 87295e2..8842253 100644 --- a/lib/puppet/type/entropy_keywords.rb +++ b/lib/puppet/type/entropy_keywords.rb @@ -51,8 +51,6 @@ Puppet::Type.newtype(:entropy_keywords) do end validate do - raise(ArgumentError, "At least one of package or repo is required") if self[:package].nil? && self[:repo].nil? - raise(ArgumentError, "Package is required when a version is specified") if self[:package].nil? && !self[:version].nil? raise(ArgumentError, "Version is required when an operator is specified") if self[:version].nil? && !self[:operator].nil? diff --git a/lib/puppet/type/entropy_mask.rb b/lib/puppet/type/entropy_mask.rb index 2847a56..33f1d6a 100644 --- a/lib/puppet/type/entropy_mask.rb +++ b/lib/puppet/type/entropy_mask.rb @@ -50,8 +50,6 @@ Puppet::Type.newtype(:entropy_mask) do end validate do - raise(ArgumentError, "At least one of package, tag or repo is required") if self[:package].nil? && self[:tag].nil? && self[:repo].nil? - raise(ArgumentError, "Package is required when a version is specified") if self[:package].nil? && !self[:version].nil? raise(ArgumentError, "Version is required when an operator is specified") if self[:version].nil? && !self[:operator].nil? diff --git a/lib/puppet/type/entropy_splitdebug.rb b/lib/puppet/type/entropy_splitdebug.rb index bbfdb07..0176750 100644 --- a/lib/puppet/type/entropy_splitdebug.rb +++ b/lib/puppet/type/entropy_splitdebug.rb @@ -50,8 +50,6 @@ Puppet::Type.newtype(:entropy_splitdebug) do end validate do - raise(ArgumentError, "At least one of package, tag or repo is required") if self[:package].nil? && self[:tag].nil? && self[:repo].nil? - raise(ArgumentError, "Package is required when a version is specified") if self[:package].nil? && !self[:version].nil? raise(ArgumentError, "Version is required when an operator is specified") if self[:version].nil? && !self[:operator].nil? diff --git a/lib/puppet/type/entropy_splitdebug_mask.rb b/lib/puppet/type/entropy_splitdebug_mask.rb index c4b6992..1016b38 100644 --- a/lib/puppet/type/entropy_splitdebug_mask.rb +++ b/lib/puppet/type/entropy_splitdebug_mask.rb @@ -50,8 +50,6 @@ Puppet::Type.newtype(:entropy_splitdebug_mask) do end validate do - raise(ArgumentError, "At least one of package, tag or repo is required") if self[:package].nil? && self[:tag].nil? && self[:repo].nil? - raise(ArgumentError, "Package is required when a version is specified") if self[:package].nil? && !self[:version].nil? raise(ArgumentError, "Version is required when an operator is specified") if self[:version].nil? && !self[:operator].nil? diff --git a/lib/puppet/type/entropy_unmask.rb b/lib/puppet/type/entropy_unmask.rb index a5b740c..f81dcf7 100644 --- a/lib/puppet/type/entropy_unmask.rb +++ b/lib/puppet/type/entropy_unmask.rb @@ -50,8 +50,6 @@ Puppet::Type.newtype(:entropy_unmask) do end validate do - raise(ArgumentError, "At least one of package, tag or repo is required") if self[:package].nil? && self[:tag].nil? && self[:repo].nil? - raise(ArgumentError, "Package is required when a version is specified") if self[:package].nil? && !self[:version].nil? raise(ArgumentError, "Version is required when an operator is specified") if self[:version].nil? && !self[:operator].nil? diff --git a/spec/unit/type/entropy_keywords_spec.rb b/spec/unit/type/entropy_keywords_spec.rb index 57001e0..555c750 100644 --- a/spec/unit/type/entropy_keywords_spec.rb +++ b/spec/unit/type/entropy_keywords_spec.rb @@ -40,12 +40,6 @@ describe Puppet::Type.type(:entropy_keywords) do end describe "when validating required properties" do - it "should raise an error when no required attributes are passed" do - expect { - described_class.new(:name => "test") - }.to raise_error(Puppet::Error, /At least one of (.*) is required/) - end - it "should raise an error when a version is passed with no package" do expect { described_class.new(:name => "test", :repo => "test", :version => "1.2.3")