From 9635f97d587cc0c47490198fe0225daea6853585 Mon Sep 17 00:00:00 2001 From: Jos Houtman Date: Wed, 6 Sep 2017 10:45:34 +0200 Subject: [PATCH 1/6] (#359) Datacenter support to consul_key_value This includes tests for the provider that mock the consul API. --- CONTRIBUTORS | 1 + Gemfile | 1 + Gemfile.lock | 11 +- .../provider/consul_key_value/default.rb | 38 ++- lib/puppet/type/consul_key_value.rb | 8 + spec/spec_helper.rb | 3 + .../puppet/provider/consul_key_value_spec.rb | 313 ++++++++++++++++++ tests/init.pp | 13 + 8 files changed, 371 insertions(+), 17 deletions(-) create mode 100644 spec/unit/puppet/provider/consul_key_value_spec.rb diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 3735c32f..0e715381 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -4,3 +4,4 @@ Simon Croome Dan Tehranian Vik Bhatti Justin King +Jos Houtman diff --git a/Gemfile b/Gemfile index 6ba98c6a..2b055d3b 100644 --- a/Gemfile +++ b/Gemfile @@ -27,4 +27,5 @@ group :test do gem "puppetlabs_spec_helper" gem "hiera" gem "hiera-puppet-helper" + gem "webmock", "~> 2.3.0" end diff --git a/Gemfile.lock b/Gemfile.lock index 53f9be32..b441a2cb 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -50,6 +50,8 @@ GEM specinfra (~> 2) builder (3.2.2) coderay (1.1.1) + crack (0.4.3) + safe_yaml (~> 1.0.0) deep_merge (1.1.1) diff-lcs (1.2.5) docker-api (1.31.0) @@ -177,6 +179,7 @@ GEM multi_json (~> 1.11) os (~> 0.9) signet (~> 0.7) + hashdiff (0.3.6) hiera (1.3.4) json_pure hiera-puppet-helper (1.0.1) @@ -267,6 +270,7 @@ GEM rspec rspec-support (3.1.2) rsync (1.0.9) + safe_yaml (1.0.4) serverspec (2.36.1) multi_json rspec (~> 3.0) @@ -292,6 +296,10 @@ GEM unf_ext unf_ext (0.0.7.2) vagrant-wrapper (2.0.3) + webmock (2.3.2) + addressable (>= 2.3.6) + crack (>= 0.3.2) + hashdiff PLATFORMS ruby @@ -316,6 +324,7 @@ DEPENDENCIES rspec-puppet serverspec vagrant-wrapper + webmock (~> 2.3.0) BUNDLED WITH - 1.12.5 + 1.15.4 diff --git a/lib/puppet/provider/consul_key_value/default.rb b/lib/puppet/provider/consul_key_value/default.rb index e1822441..a5b23a17 100644 --- a/lib/puppet/provider/consul_key_value/default.rb +++ b/lib/puppet/provider/consul_key_value/default.rb @@ -15,8 +15,9 @@ def self.prefetch(resources) protocol = resource[:protocol] token = resource[:acl_api_token] tries = resource[:api_tries] + datacenter = resource[:datacenter] - found_key_values = list_resources(token, port, hostname, protocol, tries).select do |key_value| + found_key_values = list_resources(token, port, hostname, protocol, tries, datacenter).select do |key_value| key_value[:name] == name end @@ -31,19 +32,17 @@ def self.prefetch(resources) end end - def self.list_resources(acl_api_token, port, hostname, protocol, tries) + def self.list_resources(acl_api_token, port, hostname, protocol, tries, datacenter) @key_values ||= {} - if @key_values[ "#{acl_api_token}#{port}#{hostname}#{protocol}#{tries}" ] - return @key_values[ "#{acl_api_token}#{port}#{hostname}#{protocol}#{tries}" ] + if @key_values[ "#{acl_api_token}#{port}#{hostname}#{protocol}#{tries}#{datacenter}" ] + return @key_values[ "#{acl_api_token}#{port}#{hostname}#{protocol}#{tries}#{datacenter}" ] end # this might be configurable by searching /etc/consul.d # but would break for anyone using nonstandard paths - uri = URI("#{protocol}://#{hostname}:#{port}/v1/kv/?recurse") - http = Net::HTTP.new(uri.host, uri.port) + consul_url = "#{protocol}://#{hostname}:#{port}/v1/kv/?dc=#{datacenter}&recurse&token=#{acl_api_token}" - path=uri.request_uri + "&token=#{acl_api_token}" - req = Net::HTTP::Get.new(path) + uri = URI(consul_url) res = nil # retry Consul API query for ACLs, in case Consul has just started @@ -52,7 +51,7 @@ def self.list_resources(acl_api_token, port, hostname, protocol, tries) Puppet.debug("retrying Consul API query in #{i} seconds") sleep i end - res = http.request(req) + res = Net::HTTP.get_response(uri) break if res.code == '200' end @@ -63,7 +62,7 @@ def self.list_resources(acl_api_token, port, hostname, protocol, tries) elsif res.code == '404' return [] else - Puppet.warning("Cannot retrieve key_values: invalid return code #{res.code} uri: #{path} body: #{req.body}") + Puppet.warning("Cannot retrieve key_values: invalid return code #{res.code} uri: #{uri.request_uri}") return {} end @@ -76,15 +75,20 @@ def self.list_resources(acl_api_token, port, hostname, protocol, tries) :protocol => protocol, } end - @key_values[ "#{acl_api_token}#{port}#{hostname}#{protocol}#{tries}" ] = nkey_values + @key_values[ "#{acl_api_token}#{port}#{hostname}#{protocol}#{tries}#{datacenter}" ] = nkey_values nkey_values end + # Reset the state of the provider between tests. + def self.reset() + @key_values = {} + end + def get_path(name) - uri = URI("#{@resource[:protocol]}://#{@resource[:hostname]}:#{@resource[:port]}/v1/kv/#{name}") + uri = URI("#{@resource[:protocol]}://#{@resource[:hostname]}:#{@resource[:port]}/v1/kv/#{name}?dc=#{@resource[:datacenter]}&token=#{@resource[:acl_api_token]}") http = Net::HTTP.new(uri.host, uri.port) acl_api_token = @resource[:acl_api_token] - return uri.request_uri + "?token=#{acl_api_token}", http + return uri.request_uri, http end def create_or_update_key_value(name, value, flags) @@ -106,9 +110,9 @@ def delete_key_value(name) end end - def get_resource(name, port, hostname, protocol, tries) + def get_resource(name, port, hostname, protocol, tries, datacenter) acl_api_token = @resource[:acl_api_token] - resources = self.class.list_resources(acl_api_token, port, hostname, protocol, tries).select do |res| + resources = self.class.list_resources(acl_api_token, port, hostname, protocol, tries, datacenter).select do |res| res[:name] == name end # if the user creates multiple with the same name this will do odd things @@ -140,7 +144,9 @@ def flush hostname = @resource[:hostname] protocol = @resource[:protocol] tries = @resource[:api_tries] - key_value = self.get_resource(name, port, hostname, protocol, tries) + datacenter = @resource[:datacenter] + key_value = self.get_resource(name, port, hostname, protocol, tries, datacenter) + if key_value if @property_flush[:ensure] == :absent delete_key_value(name) diff --git a/lib/puppet/type/consul_key_value.rb b/lib/puppet/type/consul_key_value.rb index 6884c3de..ba3959ca 100644 --- a/lib/puppet/type/consul_key_value.rb +++ b/lib/puppet/type/consul_key_value.rb @@ -35,6 +35,14 @@ defaultto '' end + newparam(:datacenter) do + desc 'Name of the datacenter to query. If unspecified, the query will default to the datacenter of the Consul agent at the HTTP address.' + validate do |value| + raise ArgumentError, "Datacenter must be a string" if not value.is_a?(String) + end + defaultto '' + end + newparam(:protocol) do desc 'consul protocol' newvalues(:http, :https) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 0cfbc95e..9e54c579 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -2,6 +2,9 @@ require 'hiera-puppet-helper/rspec' require 'hiera' require 'puppet/indirector/hiera' +require 'webmock/rspec' + +WebMock.disable_net_connect!() # config hiera to work with let(:hiera_data) def hiera_stub diff --git a/spec/unit/puppet/provider/consul_key_value_spec.rb b/spec/unit/puppet/provider/consul_key_value_spec.rb new file mode 100644 index 00000000..e02bf7e8 --- /dev/null +++ b/spec/unit/puppet/provider/consul_key_value_spec.rb @@ -0,0 +1,313 @@ +require 'spec_helper' +require 'json' + +describe Puppet::Type.type(:consul_key_value).provider(:default) do + let(:resource) { Puppet::Type.type(:consul_key_value).new( + { + :name => "sample/key", + :value => 'sampleValue', + :acl_api_token => 'sampleToken', + :datacenter => 'dc1', + } + )} + + let(:resources) { { 'sample/key' => resource } } + + describe '.list_resources' do + context "when the first two responses are unexpected" do + it 'should retry 3 times' do + kv_content = [ + {"LockIndex" => 0, + "Key" => "sample/key", + "Flags" => 0, + "Value" => "RGlmZmVyZW50IHZhbHVl", #Different value + "CreateIndex" => 1350503, + "ModifyIndex" => 1350503} + ] + + stub_request(:get, "http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken"). + with(:headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_return(:status => 400, :body => "", :headers => {}).times(2).then. + to_return(:status => 200, :body => JSON.dump(kv_content), :headers => {}) + + described_class.reset + described_class.prefetch( resources ) + expect(resource.provider.ensure).to eql(:present) + end + end + + context "when the first three responses are unexpected" do + it 'should silently fail to prefetch' do + stub_request(:get, "http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken"). + with(:headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_return(:status => 400, :body => "", :headers => {}) + + described_class.reset + described_class.prefetch( resources ) + expect(resource.provider.ensure).to eql(:absent) + end + end + + context "when a timeout is received" do + it 'should not handle the timeout' do + stub_request(:get, "http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken"). + with(:headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_timeout + + described_class.reset + # expect(described_class.prefetch( resources )).to raise_error + expect{ described_class.prefetch (resources) }.to raise_error(Timeout::Error, "execution expired") + end + end + + context "when resources use different datacenters" do + it 'should handle fetching properly' do + #dc1 allready contains a key-value + #dc2 has an empty key-value store. + # + #That means the providers should reflect this, unless the caching is corrupt. + + res_dc1 = Puppet::Type.type(:consul_key_value).new( + { + :name => "sample/keydc1", + :value => 'sampleValue', + :acl_api_token => 'sampleToken', + :datacenter => 'dc1', + } + ) + + res_dc2 = Puppet::Type.type(:consul_key_value).new( + { + :name => "sample/keydc2", + :value => 'sampleValue', + :acl_api_token => 'sampleToken', + :datacenter => 'dc2', + } + ) + + resources = { 'sample/keydc1' => res_dc1, 'sample/keydc2' => res_dc2 } + + kv_content = [ + {"LockIndex" => 0, + "Key" => "sample/keydc1", + "Flags" => 0, + "Value" => "RGlmZmVyZW50IHZhbHVl", #Different value + "CreateIndex" => 1350503, + "ModifyIndex" => 1350503} + ] + + stub_request(:get, "http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken"). + with(:headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_return(:status => 200, :body => JSON.dump(kv_content), :headers => {}) + + stub_request(:get, "http://localhost:8500/v1/kv/?dc=dc2&recurse&token=sampleToken"). + with(:headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_return(:status => 404, :body => "", :headers => {}) + + described_class.reset + described_class.prefetch(resources) + expect(res_dc1.provider.exists?).to eql(true) + expect(res_dc2.provider.exists?).to eql(false) + end + end + end + + describe '#exists?' do + context "when resource does not exists" do + it 'should return false' do + stub_request(:get, "http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken"). + with(:headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_return(:status => 404, :body => "", :headers => {}) + + described_class.reset + described_class.prefetch( resources ) + expect(resource.provider.exists?).to eql(false) + end + end + + context "when resource exists" do + it 'it should return true' do + kv_content = [ + {"LockIndex" => 0, + "Key" => "sample/key", + "Flags" => 0, + "Value" => "RGlmZmVyZW50IHZhbHVl", #Different value + "CreateIndex" => 1350503, + "ModifyIndex" => 1350503} + ] + + stub_request(:get, "http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken"). + with(:headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_return(:status => 200, :body => JSON.dump(kv_content), :headers => {}) + + described_class.reset + described_class.prefetch( resources ) + expect(resource.provider.exists?).to eql(true) + end + end + end + + + describe '#create' do + context "when key does not exist" do + it "should write to consul" do + kv_content = [ + {"LockIndex" => 0, + "Key" => "sample/key-different-key", + "Flags" => 0, + "Value" => "RGlmZmVyZW50IHZhbHVl", #Different value + "CreateIndex" => 1350503, + "ModifyIndex" => 1350503} + ] + + stub_request(:get, "http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken"). + with(:headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_return(:status => 200, :body => JSON.dump(kv_content), :headers => {}) + + stub_request(:put, "http://localhost:8500/v1/kv/sample/key?dc=dc1&flags=0&token=sampleToken"). + with(:body => "sampleValue", + :headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_return(:status => 200, :body => "", :headers => {}) + + described_class.reset + described_class.prefetch( resources ) + resource.provider.create + resource.provider.flush + end + end + + context "when key does exist, with different value" do + it "it should write to consul" do + kv_content = [ + {"LockIndex" => 0, + "Key" => "sample/key", + "Flags" => 0, + "Value" => "RGlmZmVyZW50IHZhbHVl", #Different value + "CreateIndex" => 1350503, + "ModifyIndex" => 1350503} + ] + + stub_request(:get, "http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken"). + with(:headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_return(:status => 200, :body => JSON.dump(kv_content), :headers => {}) + + stub_request(:put, "http://localhost:8500/v1/kv/sample/key?dc=dc1&flags=0&token=sampleToken"). + with(:body => "sampleValue", + :headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_return(:status => 200, :body => "", :headers => {}) + + described_class.reset + described_class.prefetch( resources ) + resource.provider.create + resource.provider.flush + end + end + + context "when key does exist, with same value" do + it "it should write to consul" do + kv_content = [ + {"LockIndex" => 0, + "Key" => "sample/key", + "Flags" => 0, + "Value" => "c2FtcGxlVmFsdWU=", #sampleValue + "CreateIndex" => 1350503, + "ModifyIndex" => 1350503} + ] + + stub_request(:get, "http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken"). + with(:headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_return(:status => 200, :body => JSON.dump(kv_content), :headers => {}) + + stub_request(:put, "http://localhost:8500/v1/kv/sample/key?dc=dc1&flags=0&token=sampleToken"). + with(:body => "sampleValue", + :headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_return(:status => 200, :body => "", :headers => {}) + + described_class.prefetch( resources ) + resource.provider.create + resource.provider.flush + end + end + + context "when consul returns an error" do + it "should raise Puppet::Error on failed create" do + kv_content = [ + {"LockIndex" => 0, + "Key" => "sample/key", + "Flags" => 0, + "Value" => "c2FtcGxlVmFsdWU=", #sampleValue + "CreateIndex" => 1350503, + "ModifyIndex" => 1350503} + ] + + stub_request(:get, "http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken"). + with(:headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_return(:status => 200, :body => JSON.dump(kv_content), :headers => {}) + + stub_request(:put, "http://localhost:8500/v1/kv/sample/key?dc=dc1&flags=0&token=sampleToken"). + with(:body => "sampleValue", + :headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_return(:status => 400, :body => "", :headers => {}) + + described_class.prefetch( resources ) + resource.provider.create + expect { resource.provider.flush }.to raise_error(Puppet::Error, /Session sample\/key create\/update: invalid return code 400 uri:/) + end + end + end + + describe '#destroy' do + context "when key exists" do + it 'should delete key' do + kv_content = [ + {"LockIndex" => 0, + "Key" => "sample/key", + "Flags" => 0, + "Value" => "RGlmZmVyZW50IHZhbHVl", #Different value + "CreateIndex" => 1350503, + "ModifyIndex" => 1350503} + ] + + stub_request(:get, "http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken"). + with(:headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_return(:status => 200, :body => JSON.dump(kv_content), :headers => {}) + + stub_request(:delete, "http://localhost:8500/v1/kv/sample/key?dc=dc1&token=sampleToken"). + with(:headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_return(:status => 200, :body => "", :headers => {}) + + described_class.reset + described_class.prefetch( resources ) + resource.provider.destroy + resource.provider.flush + end + end + + context "when key exists, but consul returns an error" do + it 'should raise error on failed delete' do + kv_content = [ + {"LockIndex" => 0, + "Key" => "sample/key", + "Flags" => 0, + "Value" => "RGlmZmVyZW50IHZhbHVl", #Different value + "CreateIndex" => 1350503, + "ModifyIndex" => 1350503} + ] + + stub_request(:get, "http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken"). + with(:headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_return(:status => 200, :body => JSON.dump(kv_content), :headers => {}) + + stub_request(:delete, "http://localhost:8500/v1/kv/sample/key?dc=dc1&token=sampleToken"). + with(:headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_return(:status => 400, :body => "", :headers => {}) + + described_class.reset + described_class.prefetch( resources ) + resource.provider.destroy + + expect { resource.provider.flush }.to raise_error(Puppet::Error, /Session sample\/key delete: invalid return code 400 uri:/) + end + end + end +end diff --git a/tests/init.pp b/tests/init.pp index 4bc45ffd..5146397a 100644 --- a/tests/init.pp +++ b/tests/init.pp @@ -10,3 +10,16 @@ # http://docs.puppetlabs.com/guides/tests_smoke.html # include consul + +# allows for a quick and dirty test of the consul_key_value with consul. +# You can execute consul in docker using: +# > docker run -d -p 8500:8500 --name=dev-consul consul +# use this to find the hostname +# > docker exec -t dev-consul consul members +# node default { +# consul_key_value{'sample/key': +# ensure => 'absent', +# value => 'testValue', +# datacenter => 'dc1' +# } +# } From 845c3691748c5b538d024978aa6bbdad33451f63 Mon Sep 17 00:00:00 2001 From: Jos Houtman Date: Fri, 8 Sep 2017 19:46:26 +0200 Subject: [PATCH 2/6] Use --full-index during bundle install $ bundle install --without development --path=${BUNDLE_PATH:-vendor/bundle} Fetching gem metadata from https://rubygems.org/...Retrying dependency api due to error (2/3): Bundler::HTTPError Net::HTTPClientError: Too many gems! (use --full-index instead) Retrying dependency api due to error (3/3): Bundler::HTTPError Net::HTTPClientError: Too many gems! (use --full-index instead) --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 340b55d2..c335f22f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,6 @@ --- language: ruby -bundler_args: --without development +bundler_args: --without development --full-index before_install: rm Gemfile.lock || true sudo: false cache: bundler From 5431360952446dc977e0066a0e9f228c6873535b Mon Sep 17 00:00:00 2001 From: Jos Houtman Date: Sat, 9 Sep 2017 15:03:19 +0200 Subject: [PATCH 3/6] Pin public_suffix and upgrade puppet. public_suffix is pinned to a version that supports ruby 1.9.3. puppet is updated to 3.8.4 since that is the minimal version for the tests. --- Gemfile | 4 +- Gemfile.lock | 134 ++++++++++++++++++++++++++------------------------- 2 files changed, 71 insertions(+), 67 deletions(-) diff --git a/Gemfile b/Gemfile index 2b055d3b..545e4801 100644 --- a/Gemfile +++ b/Gemfile @@ -16,7 +16,7 @@ group :test do gem "json_pure", '~> 1.8.3' gem "rake" - gem "puppet", ENV['PUPPET_VERSION'] || '~> 3.7.0' + gem "puppet", ENV['PUPPET_VERSION'] || '~> 3.8.4' gem "puppet-lint" gem "rspec" @@ -28,4 +28,6 @@ group :test do gem "hiera" gem "hiera-puppet-helper" gem "webmock", "~> 2.3.0" + #pin for 1.9.3 compatability for now + gem "public_suffix", '~> 1.4.6' end diff --git a/Gemfile.lock b/Gemfile.lock index b441a2cb..1f67fd14 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,12 +1,12 @@ GEM remote: https://rubygems.org/ specs: - CFPropertyList (2.3.1) + CFPropertyList (2.3.5) addressable (2.4.0) - aws-sdk-v1 (1.66.0) + aws-sdk-v1 (1.67.0) json (~> 1.4) - nokogiri (>= 1.4.4) - beaker (2.51.0) + nokogiri (~> 1) + beaker (2.52.0) aws-sdk-v1 (~> 1.57) beaker-answers (~> 0.0) beaker-hiera (~> 0.0) @@ -26,42 +26,43 @@ GEM net-scp (~> 1.2) net-ssh (~> 2.9) open_uri_redirections (~> 0.2.1) + public_suffix (< 1.5.0) rbvmomi (~> 1.8, < 1.9.0) rsync (~> 1.0.9) stringify-hash (~> 0.0) unf (~> 0.1) - beaker-answers (0.11.0) + beaker-answers (0.17.0) hocon (~> 1.0) require_all (~> 1.3.2) stringify-hash (~> 0.0.0) beaker-hiera (0.1.1) stringify-hash (~> 0.0.0) - beaker-hostgenerator (0.7.3) + beaker-hostgenerator (1.1.0) deep_merge (~> 1.0) stringify-hash (~> 0.0.0) - beaker-pe (0.12.0) + beaker-pe (0.12.2) stringify-hash (~> 0.0.0) - beaker-puppet_install_helper (0.4.4) - beaker (~> 2.0) + beaker-puppet_install_helper (0.7.1) + beaker (>= 2.0) beaker-rspec (5.6.0) beaker (~> 2.0) rspec serverspec (~> 2) specinfra (~> 2) - builder (3.2.2) - coderay (1.1.1) + builder (3.2.3) + coderay (1.1.2) crack (0.4.3) safe_yaml (~> 1.0.0) deep_merge (1.1.1) - diff-lcs (1.2.5) - docker-api (1.31.0) + diff-lcs (1.3) + docker-api (1.33.6) excon (>= 0.38.0) json - domain_name (0.5.20160826) + domain_name (0.5.20170404) unf (>= 0.0.5, < 1.0.0) - excon (0.52.0) - facter (2.4.6) - faraday (0.9.2) + excon (0.59.0) + facter (2.5.1) + faraday (0.13.1) multipart-post (>= 1.2, < 3) fission (0.5.0) CFPropertyList (~> 2.2) @@ -92,18 +93,18 @@ GEM fog-atmos (0.1.0) fog-core fog-xml - fog-aws (0.12.0) + fog-aws (1.4.1) fog-core (~> 1.38) fog-json (~> 1.0) fog-xml (~> 0.1) ipaddress (~> 0.8) - fog-brightbox (0.11.0) + fog-brightbox (0.13.0) fog-core (~> 1.22) fog-json inflecto (~> 0.0.2) - fog-core (1.42.0) + fog-core (1.45.0) builder - excon (~> 0.49) + excon (~> 0.58) formatador (~> 0.2) fog-dynect (0.0.3) fog-core @@ -119,13 +120,13 @@ GEM fog-json (1.0.2) fog-core (~> 1.0) multi_json (~> 1.10) - fog-local (0.3.0) + fog-local (0.4.0) fog-core (~> 1.27) fog-powerdns (0.1.1) fog-core (~> 1.27) fog-json (~> 1.0) fog-xml (~> 0.1) - fog-profitbricks (2.0.1) + fog-profitbricks (4.0.0) fog-core (~> 1.42) fog-json (~> 1.0) fog-radosgw (0.0.5) @@ -157,9 +158,9 @@ GEM fog-voxel (0.1.0) fog-core fog-xml - fog-xml (0.1.2) + fog-xml (0.1.3) fog-core - nokogiri (~> 1.5, >= 1.5.11) + nokogiri (>= 1.5.11, < 2.0.0) formatador (0.2.5) google-api-client (0.9.4) addressable (~> 2.3) @@ -171,8 +172,8 @@ GEM representable (~> 2.3.0) retriable (~> 2.0) thor (~> 0.19) - googleauth (0.5.1) - faraday (~> 0.9) + googleauth (0.5.3) + faraday (~> 0.12) jwt (~> 1.4) logging (~> 2.0) memoist (~> 0.12) @@ -183,63 +184,62 @@ GEM hiera (1.3.4) json_pure hiera-puppet-helper (1.0.1) - hocon (1.1.2) - http-cookie (1.0.2) + hocon (1.2.5) + http-cookie (1.0.3) domain_name (~> 0.5) - httpclient (2.8.2.4) + httpclient (2.8.3) hurley (0.2) - in-parallel (0.1.15) + in-parallel (0.1.17) inflecto (0.0.2) inifile (2.0.2) ipaddress (0.8.3) - json (1.8.3) - json_pure (1.8.3) + json (1.8.6) + json_pure (1.8.6) jwt (1.5.6) little-plugger (1.1.4) - logging (2.1.0) + logging (2.2.2) little-plugger (~> 1.1) multi_json (~> 1.10) - memoist (0.15.0) + memoist (0.16.0) metaclass (0.0.4) method_source (0.8.2) mime-types (2.99.3) mini_portile2 (2.1.0) - minitest (5.9.0) - mocha (1.1.0) + minitest (5.10.3) + mocha (1.3.0) metaclass (~> 0.0.1) - multi_json (1.12.1) + multi_json (1.12.2) multipart-post (2.0.0) net-scp (1.2.1) net-ssh (>= 2.6.5) net-ssh (2.9.4) net-telnet (0.1.1) netrc (0.11.0) - nokogiri (1.6.8) + nokogiri (1.6.8.1) mini_portile2 (~> 2.1.0) - pkg-config (~> 1.1.7) open_uri_redirections (0.2.1) os (0.9.6) - pkg-config (1.1.7) pry (0.10.4) coderay (~> 1.1.0) method_source (~> 0.8.1) slop (~> 3.4) - puppet (3.7.5) + public_suffix (1.4.6) + puppet (3.8.7) facter (> 1.6, < 3) hiera (~> 1.0) json_pure puppet-blacksmith (3.4.0) puppet (>= 2.7.16) rest-client (~> 1.8.0) - puppet-lint (2.0.2) - puppet-syntax (2.1.0) + puppet-lint (2.3.0) + puppet-syntax (2.4.1) rake - puppetlabs_spec_helper (1.2.2) + puppetlabs_spec_helper (2.3.2) mocha (~> 1.0) puppet-lint (~> 2.0) puppet-syntax (~> 2.0) rspec-puppet (~> 2.0) - rake (10.5.0) + rake (12.0.0) rbvmomi (1.8.2) builder nokogiri (>= 1.4.1) @@ -252,49 +252,50 @@ GEM mime-types (>= 1.16, < 3.0) netrc (~> 0.7) retriable (2.1.0) - rspec (3.1.0) - rspec-core (~> 3.1.0) - rspec-expectations (~> 3.1.0) - rspec-mocks (~> 3.1.0) - rspec-core (3.1.7) - rspec-support (~> 3.1.0) - rspec-expectations (3.1.2) + rspec (3.6.0) + rspec-core (~> 3.6.0) + rspec-expectations (~> 3.6.0) + rspec-mocks (~> 3.6.0) + rspec-core (3.6.0) + rspec-support (~> 3.6.0) + rspec-expectations (3.6.0) diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.1.0) + rspec-support (~> 3.6.0) rspec-its (1.2.0) rspec-core (>= 3.0.0) rspec-expectations (>= 3.0.0) - rspec-mocks (3.1.3) - rspec-support (~> 3.1.0) - rspec-puppet (2.4.0) + rspec-mocks (3.6.0) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.6.0) + rspec-puppet (2.6.8) rspec - rspec-support (3.1.2) + rspec-support (3.6.0) rsync (1.0.9) safe_yaml (1.0.4) - serverspec (2.36.1) + serverspec (2.40.0) multi_json rspec (~> 3.0) rspec-its - specinfra (~> 2.53) - sfl (2.2) + specinfra (~> 2.68) + sfl (2.3) signet (0.7.3) addressable (~> 2.3) faraday (~> 0.9) jwt (~> 1.5) multi_json (~> 1.10) slop (3.6.0) - specinfra (2.63.1) + specinfra (2.71.2) net-scp - net-ssh (>= 2.7, < 4.0) + net-ssh (>= 2.7, < 5.0) net-telnet sfl stringify-hash (0.0.2) - thor (0.19.1) + thor (0.20.0) trollop (2.1.2) uber (0.0.15) unf (0.1.4) unf_ext - unf_ext (0.0.7.2) + unf_ext (0.0.7.4) vagrant-wrapper (2.0.3) webmock (2.3.2) addressable (>= 2.3.6) @@ -313,7 +314,8 @@ DEPENDENCIES json (~> 1.8.3) json_pure (~> 1.8.3) pry - puppet (~> 3.7.0) + public_suffix (~> 1.4.6) + puppet (~> 3.8.4) puppet-blacksmith puppet-lint puppet-syntax From c04770c4239fff6ac4ae65c640c4021bcc530d39 Mon Sep 17 00:00:00 2001 From: Jos Houtman Date: Tue, 12 Sep 2017 15:09:16 +0200 Subject: [PATCH 4/6] Do not update kv store unnecessarily Test if the value in the keyvalue store is allready what we want it to be and skip the update. Value and flags are both tested. --- .../provider/consul_key_value/default.rb | 17 +++++-- .../puppet/provider/consul_key_value_spec.rb | 44 +++++++++++++++++-- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/lib/puppet/provider/consul_key_value/default.rb b/lib/puppet/provider/consul_key_value/default.rb index a5b23a17..e286f5ca 100644 --- a/lib/puppet/provider/consul_key_value/default.rb +++ b/lib/puppet/provider/consul_key_value/default.rb @@ -147,16 +147,25 @@ def flush datacenter = @resource[:datacenter] key_value = self.get_resource(name, port, hostname, protocol, tries, datacenter) - if key_value - if @property_flush[:ensure] == :absent + if @property_flush[:ensure] == :absent + if key_value + #key actually exists in the kv, delete it. delete_key_value(name) return end + elsif @property_flush[:ensure] == :present + if key_value + if key_value[:value] == value and key_value[:flags] == flags + # the key exists in the kv and has the right value and flag. + # return without updating the key. + return + end + end create_or_update_key_value(name, value, flags) - else - create_or_update_key_value(name, value, flags) + raise(Puppet::Error,"ensure attribute is set to unexpected value: #{@property_flush[:ensure]}") end + @property_hash.clear end end diff --git a/spec/unit/puppet/provider/consul_key_value_spec.rb b/spec/unit/puppet/provider/consul_key_value_spec.rb index e02bf7e8..d829cb2e 100644 --- a/spec/unit/puppet/provider/consul_key_value_spec.rb +++ b/spec/unit/puppet/provider/consul_key_value_spec.rb @@ -203,26 +203,61 @@ end end - context "when key does exist, with same value" do + context "when key does exist, with different flag" do it "it should write to consul" do kv_content = [ {"LockIndex" => 0, "Key" => "sample/key", - "Flags" => 0, + "Flags" => 1, "Value" => "c2FtcGxlVmFsdWU=", #sampleValue "CreateIndex" => 1350503, "ModifyIndex" => 1350503} ] + resource = Puppet::Type.type(:consul_key_value).new( + { + :name => "sample/key", + :value => 'sampleValue', + :flags => 2, + :acl_api_token => 'sampleToken', + :datacenter => 'dc1', + } + ) + resources = { 'sample/key' => resource } + stub_request(:get, "http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken"). with(:headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). to_return(:status => 200, :body => JSON.dump(kv_content), :headers => {}) - stub_request(:put, "http://localhost:8500/v1/kv/sample/key?dc=dc1&flags=0&token=sampleToken"). + stub_request(:put, "http://localhost:8500/v1/kv/sample/key?dc=dc1&flags=2&token=sampleToken"). with(:body => "sampleValue", :headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). to_return(:status => 200, :body => "", :headers => {}) + described_class.reset + described_class.prefetch( resources ) + resource.provider.create + resource.provider.flush + end + end + + + context "when key does exist, with same value and flag" do + it "it should not write to consul" do + kv_content = [ + {"LockIndex" => 0, + "Key" => "sample/key", + "Flags" => 0, + "Value" => "c2FtcGxlVmFsdWU=", #sampleValue + "CreateIndex" => 1350503, + "ModifyIndex" => 1350503} + ] + + stub_request(:get, "http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken"). + with(:headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). + to_return(:status => 200, :body => JSON.dump(kv_content), :headers => {}) + + described_class.reset described_class.prefetch( resources ) resource.provider.create resource.provider.flush @@ -233,7 +268,7 @@ it "should raise Puppet::Error on failed create" do kv_content = [ {"LockIndex" => 0, - "Key" => "sample/key", + "Key" => "sample/different-key", "Flags" => 0, "Value" => "c2FtcGxlVmFsdWU=", #sampleValue "CreateIndex" => 1350503, @@ -249,6 +284,7 @@ :headers => {'Accept'=>'*/*', 'User-Agent'=>'Ruby'}). to_return(:status => 400, :body => "", :headers => {}) + described_class.reset described_class.prefetch( resources ) resource.provider.create expect { resource.provider.flush }.to raise_error(Puppet::Error, /Session sample\/key create\/update: invalid return code 400 uri:/) From 2a7037b27f800d1d6a81058039873ca53d6c6ac4 Mon Sep 17 00:00:00 2001 From: Jos Houtman Date: Tue, 12 Sep 2017 15:32:32 +0200 Subject: [PATCH 5/6] Updated explanation about of consul_key_value type --- README.md | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 0229e40f..1b05ca8c 100644 --- a/README.md +++ b/README.md @@ -245,13 +245,31 @@ consul_prepared_query { 'consul': ```puppet consul_key_value { 'key/path': - ensure => 'present', - value => 'myvaluestring', - flags => 12345, + ensure => 'present', + value => 'myvaluestring', + flags => 12345, + datacenter => 'dc1' } ``` -This provider allows you to manage key/value pairs. +This provider allows you to manage key/value pairs. It tries to be smart in two ways: + +1. It caches the data accessible from the kv store with the specified acl token. +2. It does not update the key if the value & flag are allready correct. + +Allowed parameters for this type and their default values: + +Name | Default value | Comments +------------ | ------------- | ------ +name | value of :title | Name of the key/value object. Path in key/value store. +flags | 0 | an opaque unsigned integer that can be attached to each entry. Clients can choose to use this however makes sense for their application. +value | | value of this key. Must be specified. +acl\_api_token | '' | Token for accessing the ACL API +datacenter | '' | Name of the datacenter to query. If unspecified, the query will default to the datacenter of the Consul agent at the HTTP address. +protocol | http | consul protocol: http or https. +port | 8500 | consul port +hostname | localhost | consul hostname +api_tries | 3 | number of tries when contacting the Consul REST API. Timeouts are not retried because a timeout allready takes long. ## Limitations From fb083661418eb37b0045d735be8e5b35ff3a063c Mon Sep 17 00:00:00 2001 From: Jos Houtman Date: Tue, 3 Oct 2017 06:23:41 +0200 Subject: [PATCH 6/6] Changes to documentation of the consul_key_value type. --- README.md | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 1b05ca8c..a525d3cf 100644 --- a/README.md +++ b/README.md @@ -243,6 +243,8 @@ consul_prepared_query { 'consul': ## Key/Value Objects +Example: + ```puppet consul_key_value { 'key/path': ensure => 'present', @@ -255,21 +257,23 @@ consul_key_value { 'key/path': This provider allows you to manage key/value pairs. It tries to be smart in two ways: 1. It caches the data accessible from the kv store with the specified acl token. -2. It does not update the key if the value & flag are allready correct. - -Allowed parameters for this type and their default values: - -Name | Default value | Comments ------------- | ------------- | ------ -name | value of :title | Name of the key/value object. Path in key/value store. -flags | 0 | an opaque unsigned integer that can be attached to each entry. Clients can choose to use this however makes sense for their application. -value | | value of this key. Must be specified. -acl\_api_token | '' | Token for accessing the ACL API -datacenter | '' | Name of the datacenter to query. If unspecified, the query will default to the datacenter of the Consul agent at the HTTP address. -protocol | http | consul protocol: http or https. -port | 8500 | consul port -hostname | localhost | consul hostname -api_tries | 3 | number of tries when contacting the Consul REST API. Timeouts are not retried because a timeout allready takes long. +2. It does not update the key if the value & flag are already correct. + + +These parameters are mandatory when using `consul_key_value`: + +* `name` Name of the key/value object. Path in key/value store. +* `value` value of the key. + +The optional parameters only need to be specified if you require changes from default behaviour. + +* `flags` {Integer} an opaque unsigned integer that can be attached to each entry. Clients can choose to use this however makes sense for their application. Default is `0`. +* `acl\_api_token` {String} Token for accessing the ACL API. Default is `''`. +* `datacenter` {String} Use the key/value store in specified datacenter. If `''` (default) it will use the datacenter of the Consul agent at the HTTP address. +* `protocol` {String} protocol to use. Either `'http'` (default) or `'https'`. +* `port` {Integer} consul port. Defaults to `8500`. +* `hostname` {String} consul hostname. Defaults to `'localhost'`. +* `api_tries` {Integer} number of tries when contacting the Consul REST API. Timeouts are not retried because a timeout already takes long. Defaults to `3`. ## Limitations