From 8b7104e76f35613ec972abfbe4caf8206b28a72c Mon Sep 17 00:00:00 2001 From: Alexander Fisher Date: Sat, 9 Mar 2019 10:17:37 +0000 Subject: [PATCH] Convert `redisget` to `redis::get` API v4 function Since we'll be dropping puppet 3 support in the next release, we should move away from using legacy functions that suffer from environment isolation issues. See https://puppet.com/docs/puppet/4.10/functions_legacy.html The newer function API also has built in support for validating function arguments, declaring some parameters as optional etc. This allows us to remove a lot of our own code that was performing this validation before. Fixes #291 --- README.md | 23 ++----- REFERENCE.md | 61 +++++++++++++++++++ lib/puppet/functions/redis/get.rb | 32 ++++++++++ lib/puppet/parser/functions/redisget.rb | 44 ------------- .../suites/default/redisget_spec.rb | 24 ++++---- .../{redisget_spec.rb => redis/get_spec.rb} | 14 ++--- types/redisurl.pp | 1 + 7 files changed, 117 insertions(+), 82 deletions(-) create mode 100644 REFERENCE.md create mode 100644 lib/puppet/functions/redis/get.rb delete mode 100644 lib/puppet/parser/functions/redisget.rb rename spec/functions/{redisget_spec.rb => redis/get_spec.rb} (88%) create mode 100644 types/redisurl.pp diff --git a/README.md b/README.md index 29b1bd8d..eca48416 100644 --- a/README.md +++ b/README.md @@ -104,28 +104,13 @@ class { '::redis::sentinel': } ``` -## `redisget()` function - -`redisget()` takes two or three arguments that are strings. The first is the key -to be looked up, the second is the URL to the Redis service and the -optional third argument is a default value to use if the key is not -found or connection to the Redis service cannot be made. - -Example of basic usage. - -```puppet -$version = redisget('version.myapp', 'redis://redis.example.com:6379') -``` - -Example with default value specified. This is useful to allow for cached -data in case Redis is not available. - -```puppet -$version = redisget('version.myapp', 'redis://redis.example.com:6379', $::myapp_version) -``` +## `redis::get()` function +This function is used to get data from redis. You must have the 'redis' gem installed on your puppet master. +Functions are documented in [REFERENCE.md](REFERENCE.md) + ## Unit testing Plain RSpec: diff --git a/REFERENCE.md b/REFERENCE.md new file mode 100644 index 00000000..d796bf4c --- /dev/null +++ b/REFERENCE.md @@ -0,0 +1,61 @@ +# Reference + + +## Table of Contents + +**Functions** + +* [`redis::get`](#redisget): Returns the value of the key being looked up or `undef` if the key does not exist. Takes two arguments with an optional third. The first bein + +## Functions + +### redis::get + +Type: Ruby 4.x API + +Returns the value of the key being looked up or `undef` if the key does not +exist. Takes two arguments with an optional third. The first being a string +value of the key to be looked up, the second is the URL to the Redis service +and the third optional argument is a default value to be used if the lookup +fails. + +example usage +``` +$version = redis::get('version.myapp', 'redis://redis.example.com:6379') +$version_with_default = redis::get('version.myapp', 'redis://redis.example.com:6379', $::myapp_version) +``` + +#### `redis::get(String[1] $key, Redis::RedisUrl $url, Optional[String] $default)` + +Returns the value of the key being looked up or `undef` if the key does not +exist. Takes two arguments with an optional third. The first being a string +value of the key to be looked up, the second is the URL to the Redis service +and the third optional argument is a default value to be used if the lookup +fails. + +example usage +``` +$version = redis::get('version.myapp', 'redis://redis.example.com:6379') +$version_with_default = redis::get('version.myapp', 'redis://redis.example.com:6379', $::myapp_version) +``` + +Returns: `Optional[String]` Returns the value of the key from Redis + +##### `key` + +Data type: `String[1]` + +The key to look up in redis + +##### `url` + +Data type: `Redis::RedisUrl` + +The endpoint of the Redis instance + +##### `default` + +Data type: `Optional[String]` + +The value to return if the key is not found or the connection to Redis fails + diff --git a/lib/puppet/functions/redis/get.rb b/lib/puppet/functions/redis/get.rb new file mode 100644 index 00000000..ad6d484c --- /dev/null +++ b/lib/puppet/functions/redis/get.rb @@ -0,0 +1,32 @@ +require 'redis' +# Returns the value of the key being looked up or `undef` if the key does not +# exist. Takes two arguments with an optional third. The first being a string +# value of the key to be looked up, the second is the URL to the Redis service +# and the third optional argument is a default value to be used if the lookup +# fails. +# +# example usage +# ``` +# $version = redis::get('version.myapp', 'redis://redis.example.com:6379') +# $version_with_default = redis::get('version.myapp', 'redis://redis.example.com:6379', $::myapp_version) +# ``` +Puppet::Functions.create_function(:'redis::get') do + # @param key The key to look up in redis + # @param url The endpoint of the Redis instance + # @param default The value to return if the key is not found or the connection to Redis fails + # @return Returns the value of the key from Redis + dispatch :get do + param 'String[1]', :key + param 'Redis::RedisUrl', :url + optional_param 'String', :default + return_type 'Optional[String]' + end + + def get(key, url, default = nil) + Redis.new(url: url).get(key) || default + rescue Redis::CannotConnectError, SocketError => e + raise Puppet::Error, "connection to redis server failed - #{e}" unless default + Puppet.debug "Connection to redis failed with #{e} - Returning default value of #{default}" + default + end +end diff --git a/lib/puppet/parser/functions/redisget.rb b/lib/puppet/parser/functions/redisget.rb deleted file mode 100644 index d799ae97..00000000 --- a/lib/puppet/parser/functions/redisget.rb +++ /dev/null @@ -1,44 +0,0 @@ -require 'redis' - -module Puppet::Parser::Functions - newfunction(:redisget, type: :rvalue, doc: <<-DOC -Returns the value of the key being looked up or nil if the key does not -exist. Takes two arguments with an optional third. The first being a string -value of the key to be looked up, the second is the URL to the Redis service -and the third optional argument is a default value to be used if the lookup -fails. -@param redis_key [String] The key to look up in redis. -@param redis_uri [String] The endpoint of the Redis instance. -@param default_value [String] The value to return if the key is not found or - the connection to Redis fails -@return [String] The value of the key from redis -@return [String] An empty string eg. `''` -@example Calling the function. - $version = redisget('version.myapp', 'redis://redis.example.com:6379') -@example Calling the function with a default if failure occurs - $version = redisget('version.myapp', 'redis://redis.example.com:6379', $::myapp_version) -DOC - ) do |args| - - raise(Puppet::ParseError, "redisget(): Wrong number of arguments given (#{args.size} for 2 or 3)") if args.size != 2 && args.size != 3 - - key = args[0] - url = args[1] - - if args.size == 3 - default = args[2] - raise(Puppet::ParseError, "redisget(): Wrong argument type given (#{default.class} for String) for arg3 (default)") if default.is_a?(String) == false - end - - raise(Puppet::ParseError, "redisget(): Wrong argument type given (#{key.class} for String) for arg1 (key)") if key.is_a?(String) == false - raise(Puppet::ParseError, "redisget(): Wrong argument type given (#{url.class} for String) for arg2 (url)") if url.is_a?(String) == false - - begin - Redis.new(url: url).get(key) || default - rescue Redis::CannotConnectError, SocketError => e - raise Puppet::Error, "connection to redis server failed - #{e}" unless default - debug "Connection to redis failed with #{e} - Returning default value of #{default}" - default - end - end -end diff --git a/spec/acceptance/suites/default/redisget_spec.rb b/spec/acceptance/suites/default/redisget_spec.rb index af124967..7bcf7739 100644 --- a/spec/acceptance/suites/default/redisget_spec.rb +++ b/spec/acceptance/suites/default/redisget_spec.rb @@ -1,14 +1,14 @@ # rubocop:disable RSpec/MultipleExpectations require 'spec_helper_acceptance' -describe 'redisget() function' do +describe 'redis::get() function' do it 'runs successfully' do pp = <<-EOS Exec { path => [ '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin', ] } - class { '::redis': + class { 'redis': manage_repo => true, } @@ -33,9 +33,9 @@ class { '::redis': end end - it 'returns a value from MyKey with the redisget() function' do + it 'returns a value from MyKey with the redis::get() function' do pp = <<-EOS - $mykey = redisget('mykey', 'redis://127.0.0.1:6379') + $mykey = redis::get('mykey', 'redis://127.0.0.1:6379') notify{"mykey value: ${mykey}":} EOS @@ -46,9 +46,9 @@ class { '::redis': end end - it 'returns a value from valid MyKey with the redisget() function while specifying a default' do + it 'returns a value from valid MyKey with the redis::get() function while specifying a default' do pp = <<-EOS - $mykey = redisget('mykey', 'redis://127.0.0.1:6379', 'default_value') + $mykey = redis::get('mykey', 'redis://127.0.0.1:6379', 'default_value') notify{"mykey value: ${mykey}":} EOS @@ -59,9 +59,9 @@ class { '::redis': end end - it 'returns an empty string when value not present with redisget() function' do + it 'returns an empty string when value not present with redis::get() function' do pp = <<-EOS - $foo_key = redisget('foo', 'redis://127.0.0.1:6379') + $foo_key = redis::get('foo', 'redis://127.0.0.1:6379') if empty($foo_key){ notify{"foo_key value was empty string":} @@ -74,9 +74,9 @@ class { '::redis': end end - it 'returns the specified default value when key not present with redisget() function' do + it 'returns the specified default value when key not present with redis::get() function' do pp = <<-EOS - $foo_key = redisget('foo', 'redis://127.0.0.1:6379', 'default_value') + $foo_key = redis::get('foo', 'redis://127.0.0.1:6379', 'default_value') notify { $foo_key: } EOS @@ -90,7 +90,7 @@ class { '::redis': it 'returns the specified default value when connection to redis server fails' do pp = <<-EOS # Bogus port for redis server - $foo_key = redisget('foo', 'redis://127.0.0.1:12345', 'default_value') + $foo_key = redis::get('foo', 'redis://127.0.0.1:12345', 'default_value') notify { $foo_key: } EOS @@ -104,7 +104,7 @@ class { '::redis': it 'returns an error when specifying a non connectable redis server' do pp = <<-EOS # Bogus port for redis server - $foo_key = redisget('foo', 'redis://127.0.0.1:12345') + $foo_key = redis::get('foo', 'redis://127.0.0.1:12345') notify { $foo_key: } EOS diff --git a/spec/functions/redisget_spec.rb b/spec/functions/redis/get_spec.rb similarity index 88% rename from spec/functions/redisget_spec.rb rename to spec/functions/redis/get_spec.rb index cf38770c..c774ba5d 100644 --- a/spec/functions/redisget_spec.rb +++ b/spec/functions/redis/get_spec.rb @@ -6,7 +6,7 @@ LOCAL_BROKEN_URL = 'redis://localhost:1234'.freeze REMOTE_BROKEN_URL = 'redis://redis.example.com:1234'.freeze -describe 'redisget' do +describe 'redis::get' do context 'should error if connection to remote redis server cannot be made and no default is specified' do it { is_expected.to run.with_params('nonexistent_key', REMOTE_BROKEN_URL).and_raise_error(Puppet::Error, %r{connection to redis server failed - Error connecting to Redis on redis.example.com:1234 \(SocketError\)}) } end @@ -59,15 +59,15 @@ describe 'with incorrect arguments' do context 'with no argument specified' do - it { is_expected.to run.with_params.and_raise_error(Puppet::ParseError, %r{wrong number of arguments}i) } + it { is_expected.to run.with_params.and_raise_error(ArgumentError) } end context 'with only one argument specified' do - it { is_expected.to run.with_params('some_key').and_raise_error(Puppet::ParseError, %r{wrong number of arguments}i) } + it { is_expected.to run.with_params('some_key').and_raise_error(ArgumentError) } end context 'with more than three arguments specified' do - it { is_expected.to run.with_params('way', 'too', 'many', 'args').and_raise_error(Puppet::ParseError, %r{wrong number of arguments}i) } + it { is_expected.to run.with_params('way', 'too', 'many', 'args').and_raise_error(ArgumentError) } end end @@ -78,15 +78,15 @@ end [{ 'ha' => 'sh' }, true, 1, %w[an array]].each do |p| context "specifing first parameter as <#{p}>" do - it { is_expected.to run.with_params(p, REDIS_URL).and_raise_error(Puppet::ParseError, %r{wrong argument type}i) } + it { is_expected.to run.with_params(p, REDIS_URL).and_raise_error(ArgumentError) } end context "specifing second parameter as <#{p}>" do - it { is_expected.to run.with_params('valid', p).and_raise_error(Puppet::ParseError, %r{wrong argument type}i) } + it { is_expected.to run.with_params('valid', p).and_raise_error(ArgumentError) } end context "specifing third parameter as <#{p}>" do - it { is_expected.to run.with_params('valid', p).and_raise_error(Puppet::ParseError, %r{wrong argument type}i) } + it { is_expected.to run.with_params('valid', p).and_raise_error(ArgumentError) } end end end diff --git a/types/redisurl.pp b/types/redisurl.pp new file mode 100644 index 00000000..9bd4ffff --- /dev/null +++ b/types/redisurl.pp @@ -0,0 +1 @@ +type Redis::RedisUrl = Pattern[/(^redis:\/\/)/]