From 9fc98f3c6e7701c85a9bebc23f4305de6f50b16a Mon Sep 17 00:00:00 2001 From: William Yardley Date: Sun, 19 May 2024 16:56:02 -0700 Subject: [PATCH] Add support for policy definition consumer-timeout As noted in #970, `consumer-timeout` is another setting which has to be set as an unquoted integer in the passthrough to RabbitMQ Rather than add more copy-paste, update the type code to use a constant that's a list of values that need to be converted to integers in a generic way. Fixes #970 --- lib/puppet/type/rabbitmq_policy.rb | 64 ++++++++----------- spec/unit/puppet/type/rabbitmq_policy_spec.rb | 27 ++++++-- 2 files changed, 45 insertions(+), 46 deletions(-) diff --git a/lib/puppet/type/rabbitmq_policy.rb b/lib/puppet/type/rabbitmq_policy.rb index 75e96f9b3..0bf11ad49 100644 --- a/lib/puppet/type/rabbitmq_policy.rb +++ b/lib/puppet/type/rabbitmq_policy.rb @@ -1,5 +1,19 @@ # frozen_string_literal: true +# See below; these are variables that we want to auto-convert to integer +# values +CONVERT_TO_INT_VARS = %w[ + consumer-timeout + delivery-limit + expires + ha-sync-batch-size + initial-cluster-size + max-length + max-length-bytes + message-ttl + shards-per-node +].freeze + Puppet::Type.newtype(:rabbitmq_policy) do desc <<~DESC Type for managing rabbitmq policies @@ -97,50 +111,22 @@ def validate_definition(definition) ha_params = definition['ha-params'] raise ArgumentError, "Invalid ha-params '#{ha_params}' for ha-mode 'exactly'" unless ha_params.to_i.to_s == ha_params end - if definition.key? 'expires' - expires_val = definition['expires'] - raise ArgumentError, "Invalid expires value '#{expires_val}'" unless expires_val.to_i.to_s == expires_val - end - if definition.key? 'message-ttl' - message_ttl_val = definition['message-ttl'] - raise ArgumentError, "Invalid message-ttl value '#{message_ttl_val}'" unless message_ttl_val.to_i.to_s == message_ttl_val - end - if definition.key? 'max-length' - max_length_val = definition['max-length'] - raise ArgumentError, "Invalid max-length value '#{max_length_val}'" unless max_length_val.to_i.to_s == max_length_val - end - if definition.key? 'max-length-bytes' - max_length_bytes_val = definition['max-length-bytes'] - raise ArgumentError, "Invalid max-length-bytes value '#{max_length_bytes_val}'" unless max_length_bytes_val.to_i.to_s == max_length_bytes_val - end - if definition.key? 'shards-per-node' - shards_per_node_val = definition['shards-per-node'] - raise ArgumentError, "Invalid shards-per-node value '#{shards_per_node_val}'" unless shards_per_node_val.to_i.to_s == shards_per_node_val - end - if definition.key? 'ha-sync-batch-size' - ha_sync_batch_size_val = definition['ha-sync-batch-size'] - raise ArgumentError, "Invalid ha-sync-batch-size value '#{ha_sync_batch_size_val}'" unless ha_sync_batch_size_val.to_i.to_s == ha_sync_batch_size_val - end - if definition.key? 'delivery-limit' - delivery_limit_val = definition['delivery-limit'] - raise ArgumentError, "Invalid delivery-limit value '#{delivery_limit_val}'" unless delivery_limit_val.to_i.to_s == delivery_limit_val - end - if definition.key? 'initial-cluster-size' # rubocop:disable Style/GuardClause - initial_cluster_size_val = definition['initial-cluster-size'] - raise ArgumentError, "Invalid initial-cluster-size value '#{initial_cluster_size_val}'" unless initial_cluster_size_val.to_i.to_s == initial_cluster_size_val + + # Since this pattern is repeated, use a constant to track all the types + # where we need to convert a string value to an unquoted integer explicitly + definition.each do |k, v| + raise ArgumentError, "Invalid #{k} value '#{v}'" if CONVERT_TO_INT_VARS.include?(k) && v.to_i.to_s != v end end def munge_definition(definition) definition['ha-params'] = definition['ha-params'].to_i if definition['ha-mode'] == 'exactly' - definition['expires'] = definition['expires'].to_i if definition.key? 'expires' - definition['message-ttl'] = definition['message-ttl'].to_i if definition.key? 'message-ttl' - definition['max-length'] = definition['max-length'].to_i if definition.key? 'max-length' - definition['max-length-bytes'] = definition['max-length-bytes'].to_i if definition.key? 'max-length-bytes' - definition['shards-per-node'] = definition['shards-per-node'].to_i if definition.key? 'shards-per-node' - definition['ha-sync-batch-size'] = definition['ha-sync-batch-size'].to_i if definition.key? 'ha-sync-batch-size' - definition['delivery-limit'] = definition['delivery-limit'].to_i if definition.key? 'delivery-limit' - definition['initial-cluster-size'] = definition['initial-cluster-size'].to_i if definition.key? 'initial-cluster-size' + + # Again, use a list of types to convert vs. hard-coding each one + definition.each do |k, v| + definition[k] = v.to_i if CONVERT_TO_INT_VARS.include? k + end + definition end end diff --git a/spec/unit/puppet/type/rabbitmq_policy_spec.rb b/spec/unit/puppet/type/rabbitmq_policy_spec.rb index e799abe36..109170d01 100644 --- a/spec/unit/puppet/type/rabbitmq_policy_spec.rb +++ b/spec/unit/puppet/type/rabbitmq_policy_spec.rb @@ -91,7 +91,7 @@ end end - it 'accepts and convert ha-params for ha-mode exactly' do + it 'accepts and converts ha-params for ha-mode exactly' do definition = { 'ha-mode' => 'exactly', 'ha-params' => '2' } policy[:definition] = definition expect(policy[:definition]['ha-params']).to eq(2) @@ -104,7 +104,20 @@ end.to raise_error(Puppet::Error, %r{Invalid ha-params.*nonnumeric.*exactly}) end - it 'accepts and convert the expires value' do + it 'accepts and converts the consumer-timeout value' do + definition = { 'consumer-timeout' => '86400000' } + policy[:definition] = definition + expect(policy[:definition]['consumer-timeout']).to eq(86_400_000) + end + + it 'does not accept non-numeric consumer-timeout value' do + definition = { 'consumer-timeout' => 'bogus' } + expect do + policy[:definition] = definition + end.to raise_error(Puppet::Error, %r{Invalid consumer-timeout value.*bogus}) + end + + it 'accepts and converts the expires value' do definition = { 'expires' => '1800000' } policy[:definition] = definition expect(policy[:definition]['expires']).to eq(1_800_000) @@ -117,7 +130,7 @@ end.to raise_error(Puppet::Error, %r{Invalid expires value.*future}) end - it 'accepts and convert the message-ttl value' do + it 'accepts and converts the message-ttl value' do definition = { 'message-ttl' => '1800000' } policy[:definition] = definition expect(policy[:definition]['message-ttl']).to eq(1_800_000) @@ -130,7 +143,7 @@ end.to raise_error(Puppet::Error, %r{Invalid message-ttl value.*future}) end - it 'accepts and convert the max-length value' do + it 'accepts and converts the max-length value' do definition = { 'max-length' => '1800000' } policy[:definition] = definition expect(policy[:definition]['max-length']).to eq(1_800_000) @@ -143,7 +156,7 @@ end.to raise_error(Puppet::Error, %r{Invalid max-length value.*future}) end - it 'accepts and convert the max-length-bytes value' do + it 'accepts and converts the max-length-bytes value' do definition = { 'max-length-bytes' => '1800000' } policy[:definition] = definition expect(policy[:definition]['max-length-bytes']).to eq(1_800_000) @@ -156,7 +169,7 @@ end.to raise_error(Puppet::Error, %r{Invalid max-length-bytes value.*future}) end - it 'accepts and convert the shards-per-node value' do + it 'accepts and converts the shards-per-node value' do definition = { 'shards-per-node' => '1800000' } policy[:definition] = definition expect(policy[:definition]['shards-per-node']).to eq(1_800_000) @@ -169,7 +182,7 @@ end.to raise_error(Puppet::Error, %r{Invalid shards-per-node value.*future}) end - it 'accepts and convert the ha-sync-batch-size value' do + it 'accepts and converts the ha-sync-batch-size value' do definition = { 'ha-sync-batch-size' => '1800000' } policy[:definition] = definition expect(policy[:definition]['ha-sync-batch-size']).to eq(1_800_000)