diff --git a/lib/puppet/provider/consul_policy/default.rb b/lib/puppet/provider/consul_policy/default.rb index de38ddad..bf0827e1 100644 --- a/lib/puppet/provider/consul_policy/default.rb +++ b/lib/puppet/provider/consul_policy/default.rb @@ -35,7 +35,7 @@ def self.prefetch(resources) existing_policy = nil end - resource.provider = new({}, @client, rules_encoded, existing_policy) + resource.provider = new({}, @client, rules_encoded, existing_policy, resource) end end @@ -60,17 +60,28 @@ def self.list_policies(acl_api_token, hostname, port, protocol, tries) @all_policies end - def initialize(messages, client = nil, rules_encoded = '', existing_policy = nil) + def initialize(messages, client = nil, rules_encoded = '', existing_policy = nil, resource = nil) super(messages) @property_flush = {} @client = client @rules_encoded = rules_encoded @existing_policy = existing_policy + + if existing_policy + @property_hash = { + :id => existing_policy.id, + :description => existing_policy.description + } + + if rules_encoded == existing_policy.rules + @property_hash[:rules] = resource[:rules] + end + end end def exists? - @property_hash[:ensure] = :present + @existing_policy end def create diff --git a/lib/puppet/provider/consul_token/default.rb b/lib/puppet/provider/consul_token/default.rb index 4686c493..0f47072d 100644 --- a/lib/puppet/provider/consul_token/default.rb +++ b/lib/puppet/provider/consul_token/default.rb @@ -14,7 +14,7 @@ def self.prefetch(resources) tokens = list_tokens(resource[:acl_api_token], resource[:hostname], resource[:port], resource[:protocol], resource[:api_tries]) token = tokens.select{|token| token.accessor_id == resource[:accessor_id]} - resource.provider = new({}, @client, token.any? ? token.first : nil) + resource.provider = new({}, @client, token.any? ? token.first : nil, resource) end end @@ -29,16 +29,31 @@ def self.list_tokens(acl_api_token, hostname, port, protocol, tries) @token_collection end - def initialize(messages, client = nil, existing_token = nil) + def initialize(messages, client = nil, existing_token = nil, resource = nil) super(messages) @property_flush = {} @client = client @existing_token = existing_token + + if resource + @property_hash = { + :secret_id => resource[:secret_id], + } + end + + if existing_token + @property_hash[:accessor_id] = existing_token.accessor_id + + if existing_token.is_policy_list_equal(resource[:policies_by_id], resource[:policies_by_name]) + @property_hash[:policies_by_id] = resource[:policies_by_id] + @property_hash[:policies_by_name] = resource[:policies_by_name] + end + end end def exists? - @property_hash[:ensure] = :present + @existing_token end def create diff --git a/lib/puppet/type/consul_policy.rb b/lib/puppet/type/consul_policy.rb index 0a260011..07642e52 100644 --- a/lib/puppet/type/consul_policy.rb +++ b/lib/puppet/type/consul_policy.rb @@ -21,15 +21,27 @@ defaultto '' end - newparam(:description) do + newproperty(:description) do desc 'Description of the policy' validate do |value| raise ArgumentError, "Description must be a string" if not value.is_a?(String) end end - newparam(:rules) do + newproperty(:rules, :array_matching => :all) do desc 'List of ACL rules for this policy' + validate do |value| + raise ArgumentError, "Policy rule must be a hash" unless value.is_a?(Hash) + + raise ArgumentError, "Policy rule needs to specify a resource" unless value.key?('resource') + raise ArgumentError, "Policy rule needs to specify a segment" unless value.key?('segment') + raise ArgumentError, "Policy rule needs to specify a disposition" unless value.key?('disposition') + + raise ArgumentError, "Policy rule resource must be a string" unless value['resource'].is_a?(String) + raise ArgumentError, "Policy rule segment must be a string" unless value['segment'].is_a?(String) + raise ArgumentError, "Policy rule disposition must be a string" unless value['disposition'].is_a?(String) + end + defaultto [] end diff --git a/lib/puppet/type/consul_token.rb b/lib/puppet/type/consul_token.rb index aeb36882..de066be3 100644 --- a/lib/puppet/type/consul_token.rb +++ b/lib/puppet/type/consul_token.rb @@ -28,19 +28,19 @@ defaultto '' end - newparam(:policies_by_name) do + newproperty(:policies_by_name, :array_matching => :all) do desc 'List of policy names assigned to the token' validate do |value| - raise ArgumentError, "Policy name list must be an array" if not value.is_a?(Array) + raise ArgumentError, "Policy name list must be an array of strings" if not value.is_a?(String) end defaultto [] end - newparam(:policies_by_id) do + newproperty(:policies_by_id, :array_matching => :all) do desc 'List of policy IDs assigned to the token' validate do |value| - raise ArgumentError, "Policy ID list must be an array" if not value.is_a?(Array) + raise ArgumentError, "Policy ID list must be an array of strings" if not value.is_a?(String) end defaultto [] diff --git a/spec/acceptance/class_spec.rb b/spec/acceptance/class_spec.rb index 6e9eb224..06b0fef1 100644 --- a/spec/acceptance/class_spec.rb +++ b/spec/acceptance/class_spec.rb @@ -188,6 +188,11 @@ class { 'consul': 'test_token_xyz' => { 'accessor_id' => '7c4e3f11-786d-44e6-ac1d-b99546a1ccbd', 'policies_by_name' => ['test_policy_abc'] + }, + 'test_token_absent' => { + 'accessor_id' => '10381ad3-2837-43a6-b1ea-e27b7d53a749', + 'policies_by_name' => ['test_policy_abc'], + 'ensure' => 'absent' } }, policies => { @@ -198,6 +203,13 @@ class { 'consul': {'resource' => 'key', 'segment' => 'test_key', 'disposition' => 'write'}, {'resource' => 'node_prefix', 'segment' => '', 'disposition' => 'deny'}, ], + }, + 'test_policy_absent' => { + 'description' => "This policy should not exists", + 'rules' => [ + {'resource' => 'service_prefix', 'segment' => 'test_segment', 'disposition' => 'read'} + ], + 'ensure' => 'absent' } } } @@ -205,7 +217,7 @@ class { 'consul': # Run it twice to test for idempotency apply_manifest(pp, catch_failures: true) - apply_manifest(pp, catch_changes: false) + apply_manifest(pp, catch_changes: true) end describe file('/opt/consul') do diff --git a/spec/unit/puppet/type/consul_policy_spec.rb b/spec/unit/puppet/type/consul_policy_spec.rb index 32a92a33..b678e4a6 100644 --- a/spec/unit/puppet/type/consul_policy_spec.rb +++ b/spec/unit/puppet/type/consul_policy_spec.rb @@ -4,7 +4,7 @@ it 'should fail if no name is provided' do expect do - Puppet::Type.type(:consul_policy).new(:rules => {}) + Puppet::Type.type(:consul_policy).new(:id => {}, :rules => {}) end.to raise_error(Puppet::Error, /Title or name must be provided/) end @@ -20,17 +20,127 @@ end.to raise_error(Puppet::Error, /Description must be a string/) end + it 'should fail if rules is not a hash' do + expect do + Puppet::Type.type(:consul_policy).new( + :name => 'testing', + :id => '39c75e12-7f43-0a40-dfba-9aa3fcda08d4', + :description => 'test description', + :rules => 'abc' + ) + end.to raise_error(Puppet::Error, /Policy rule must be a hash/) + end + + it 'should fail if rule resource is missing' do + expect do + Puppet::Type.type(:consul_policy).new( + :name => 'testing', + :id => '39c75e12-7f43-0a40-dfba-9aa3fcda08d4', + :description => 'test description', + :rules => [ + { + 'segment' => 'test_service', + 'disposition' => 'read' + } + ] + ) + end.to raise_error(Puppet::Error, /Policy rule needs to specify a resource/) + end + + it 'should fail if rule segment is missing' do + expect do + Puppet::Type.type(:consul_policy).new( + :name => 'testing', + :id => '39c75e12-7f43-0a40-dfba-9aa3fcda08d4', + :description => 'test description', + :rules => [ + { + 'resource' => 'service_prefix', + 'disposition' => 'read' + } + ] + ) + end.to raise_error(Puppet::Error, /Policy rule needs to specify a segment/) + end + + it 'should fail if rule disposition is missing' do + expect do + Puppet::Type.type(:consul_policy).new( + :name => 'testing', + :id => '39c75e12-7f43-0a40-dfba-9aa3fcda08d4', + :description => 'test description', + :rules => [ + { + 'resource' => 'service_prefix', + 'segment' => 'test_service', + } + ] + ) + end.to raise_error(Puppet::Error, /Policy rule needs to specify a disposition/) + end + + it 'should fail if rule resource is not a string' do + expect do + Puppet::Type.type(:consul_policy).new( + :name => 'testing', + :id => '39c75e12-7f43-0a40-dfba-9aa3fcda08d4', + :description => 'test description', + :rules => [ + { + 'resource' => [], + 'segment' => 'test_service', + 'disposition' => 'read' + } + ] + ) + end.to raise_error(Puppet::Error, /Policy rule resource must be a string/) + end + + it 'should fail if rule segment is not a string' do + expect do + Puppet::Type.type(:consul_policy).new( + :name => 'testing', + :id => '39c75e12-7f43-0a40-dfba-9aa3fcda08d4', + :description => 'test description', + :rules => [ + { + 'resource' => 'key_prefix', + 'segment' => [], + 'disposition' => 'read' + } + ] + ) + end.to raise_error(Puppet::Error, /Policy rule segment must be a string/) + end + + it 'should fail if rule disposition is not a string' do + expect do + Puppet::Type.type(:consul_policy).new( + :name => 'testing', + :id => '39c75e12-7f43-0a40-dfba-9aa3fcda08d4', + :description => 'test description', + :rules => [ + { + 'resource' => 'key_prefix', + 'segment' => 'test_service', + 'disposition' => [] + } + ] + ) + end.to raise_error(Puppet::Error, /Policy rule disposition must be a string/) + end + context 'with name defined' do rules = [ { - :resource => 'service_prefix', - :segment => 'test_service', - :disposition => 'read' + 'resource' => 'service_prefix', + 'segment' => 'test_service', + 'disposition' => 'read' }, { - :resource => 'key_prefix', - :segment => 'key', - :disposition => 'write' + 'resource' => 'key_prefix', + 'segment' => 'key', + 'disposition' => 'write' } ] diff --git a/spec/unit/puppet/type/consul_token_spec.rb b/spec/unit/puppet/type/consul_token_spec.rb index 8cdb471b..4eef76f3 100644 --- a/spec/unit/puppet/type/consul_token_spec.rb +++ b/spec/unit/puppet/type/consul_token_spec.rb @@ -22,14 +22,14 @@ it 'should fail if policy name list is not an array' do expect do - Puppet::Type.type(:consul_token).new(:name => 'foo', :policies_by_name => 'abc') - end.to raise_error(Puppet::Error, /Policy name list must be an array/) + Puppet::Type.type(:consul_token).new(:name => 'foo', :policies_by_name => [[]]) + end.to raise_error(Puppet::Error, /Policy name list must be an array of strings/) end it 'should fail if policy ID list is not an array' do expect do - Puppet::Type.type(:consul_token).new(:name => 'foo', :policies_by_id => 'abc') - end.to raise_error(Puppet::Error, /Policy ID list must be an array/) + Puppet::Type.type(:consul_token).new(:name => 'foo', :policies_by_id => [[]]) + end.to raise_error(Puppet::Error, /Policy ID list must be an array of strings/) end context 'with name defined' do diff --git a/types/policystruct.pp b/types/policystruct.pp index c8bfe62d..14a6994a 100644 --- a/types/policystruct.pp +++ b/types/policystruct.pp @@ -1,5 +1,6 @@ type Consul::PolicyStruct = Struct[{ id => Optional[String[1]], + ensure => Optional[Enum['present', 'absent']], description => Optional[String[0]], rules => Optional[Array[Struct[{ resource => String[1], diff --git a/types/tokenstruct.pp b/types/tokenstruct.pp index c0dd3e16..bc706f9e 100644 --- a/types/tokenstruct.pp +++ b/types/tokenstruct.pp @@ -1,5 +1,6 @@ type Consul::TokenStruct = Struct[{ accessor_id => String[1], + ensure => Optional[Enum['present', 'absent']], secret_id => Optional[String[1]], policies_by_name => Optional[Array[String]], policies_by_id => Optional[Array[String]],