From 2a25b6d4c251500b77c340ceb7be45f69c9a15f5 Mon Sep 17 00:00:00 2001 From: Alexander Fisher Date: Tue, 11 Feb 2025 21:21:13 +0000 Subject: [PATCH] Fix `has_ip_address` and `has_ip_network` functions The functions were using the old legacy function API. This meant that when they called `has_interface_with`, they were calling the legacy implementation of that function and not the v4 API version introduced in bc218f01497da494a499a4ab5d5fadb0ac278a37. Only in the v4 API implementation was the modern `networking` structured fact being used. The old `parser/functions/has_interface_with.rb` version still used legacy facts that are now not included in Puppet 8 by default. In this commit, we replace the `has_ip_address` and `has_ip_network` functions with namespaced Puppet language functions, (these functions are simple enough to not need ruby). Non-namespaced versions are added (but marked as deprecated) in `lib/puppet/functions`. The old implementations are removed completely. This is _almost_ certainly not going to be a breaking change for anyone. (Only other legacy functions which in turn call these functions could be affected). Fixes #1447 --- REFERENCE.md | 109 +++++++++--------- functions/has_ip_address.pp | 10 ++ functions/has_ip_network.pp | 10 ++ lib/puppet/functions/has_ip_address.rb | 14 +++ lib/puppet/functions/has_ip_network.rb | 14 +++ .../parser/functions/has_interface_with.rb | 68 ----------- lib/puppet/parser/functions/has_ip_address.rb | 27 ----- lib/puppet/parser/functions/has_ip_network.rb | 27 ----- spec/functions/has_ip_address_spec.rb | 21 ++-- spec/functions/has_ip_network_spec.rb | 19 ++- 10 files changed, 131 insertions(+), 188 deletions(-) create mode 100644 functions/has_ip_address.pp create mode 100644 functions/has_ip_network.pp create mode 100644 lib/puppet/functions/has_ip_address.rb create mode 100644 lib/puppet/functions/has_ip_network.rb delete mode 100644 lib/puppet/parser/functions/has_interface_with.rb delete mode 100644 lib/puppet/parser/functions/has_ip_address.rb delete mode 100644 lib/puppet/parser/functions/has_ip_network.rb diff --git a/REFERENCE.md b/REFERENCE.md index 63169ab67..a472419bc 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -63,9 +63,8 @@ environment. * [`grep`](#grep): This function searches through an array and returns any elements that match the provided regular expression. * [`has_interface_with`](#has_interface_with): DEPRECATED. Use the namespaced function [`stdlib::has_interface_with`](#stdlibhas_interface_with) instead. -* [`has_interface_with`](#has_interface_with): Returns boolean based on kind and value. -* [`has_ip_address`](#has_ip_address): Returns true if the client has the requested IP address on some interface. -* [`has_ip_network`](#has_ip_network): Returns true if the client has an IP address within the requested network. +* [`has_ip_address`](#has_ip_address): DEPRECATED. Use the namespaced function [`stdlib::has_ip_address`](#stdlibhas_ip_address) instead. +* [`has_ip_network`](#has_ip_network): DEPRECATED. Use the namespaced function [`stdlib::has_ip_network`](#stdlibhas_ip_network) instead. * [`intersection`](#intersection): This function returns an array of the intersection of two. * [`is_a`](#is_a): Boolean check to determine whether a variable is of a given data type. This is equivalent to the `=~` type checks. @@ -126,6 +125,8 @@ optional seed for repeatable randomness. * [`stdlib::fqdn_rotate`](#stdlib--fqdn_rotate): Rotates an array or string a random number of times, combining the `fqdn` fact and an optional seed for repeatable randomness. * [`stdlib::has_function`](#stdlib--has_function): Returns whether the Puppet runtime has access to a given function. * [`stdlib::has_interface_with`](#stdlib--has_interface_with): Returns boolean based on network interfaces present and their attribute values. +* [`stdlib::has_ip_address`](#stdlib--has_ip_address): Returns true if the client has the requested IPv4 address on some interface. +* [`stdlib::has_ip_network`](#stdlib--has_ip_network): Returns true if the client has the requested IPv4 network on some interface. * [`stdlib::ip_in_range`](#stdlib--ip_in_range): Returns true if the ipaddress is within the given CIDRs * [`stdlib::merge`](#stdlib--merge): Merges two or more hashes together or hashes resulting from iteration, and returns the resulting hash. @@ -2052,75 +2053,41 @@ Data type: `Any` -### `has_interface_with` - -Type: Ruby 3.x API - -Valid kinds are `macaddress`, `netmask`, `ipaddress` and `network`. - -#### Examples - -##### **Usage** - -```puppet -has_interface_with("macaddress", "x:x:x:x:x:x") # Returns `false` -has_interface_with("ipaddress", "127.0.0.1") # Returns `true` -``` - -##### If no "kind" is given, then the presence of the interface is checked: - -```puppet -has_interface_with("lo") # Returns `true` -``` +### `has_ip_address` -#### `has_interface_with()` +Type: Ruby 4.x API -Valid kinds are `macaddress`, `netmask`, `ipaddress` and `network`. +DEPRECATED. Use the namespaced function [`stdlib::has_ip_address`](#stdlibhas_ip_address) instead. -Returns: `Any` boolean values `true` or `false` +#### `has_ip_address(Any *$args)` -##### Examples +The has_ip_address function. -###### **Usage** - -```puppet -has_interface_with("macaddress", "x:x:x:x:x:x") # Returns `false` -has_interface_with("ipaddress", "127.0.0.1") # Returns `true` -``` +Returns: `Any` -###### If no "kind" is given, then the presence of the interface is checked: +##### `*args` -```puppet -has_interface_with("lo") # Returns `true` -``` +Data type: `Any` -### `has_ip_address` -Type: Ruby 3.x API -This function iterates through the 'interfaces' fact and checks the -'ipaddress_IFACE' facts, performing a simple string comparison. +### `has_ip_network` -#### `has_ip_address()` +Type: Ruby 4.x API -This function iterates through the 'interfaces' fact and checks the -'ipaddress_IFACE' facts, performing a simple string comparison. +DEPRECATED. Use the namespaced function [`stdlib::has_ip_network`](#stdlibhas_ip_network) instead. -Returns: `Boolean` `true` or `false` +#### `has_ip_network(Any *$args)` -### `has_ip_network` +The has_ip_network function. -Type: Ruby 3.x API +Returns: `Any` -This function iterates through the 'interfaces' fact and checks the -'network_IFACE' facts, performing a simple string comparision. +##### `*args` -#### `has_ip_network()` +Data type: `Any` -This function iterates through the 'interfaces' fact and checks the -'network_IFACE' facts, performing a simple string comparision. -Returns: `Any` Boolean value, `true` if the client has an IP address within the requested network. ### `intersection` @@ -3575,6 +3542,42 @@ Data type: `String[1]` The value of the attribute +### `stdlib::has_ip_address` + +Type: Puppet Language + +Returns true if the client has the requested IPv4 address on some interface. + +#### `stdlib::has_ip_address(Stdlib::IP::Address::V4::Nosubnet $ip_address)` + +The stdlib::has_ip_address function. + +Returns: `Boolean` Returns `true` if the requested IP address exists on any interface. + +##### `ip_address` + +Data type: `Stdlib::IP::Address::V4::Nosubnet` + +The IPv4 address you want to check the existence of + +### `stdlib::has_ip_network` + +Type: Puppet Language + +Returns true if the client has the requested IPv4 network on some interface. + +#### `stdlib::has_ip_network(Stdlib::IP::Address::V4::Nosubnet $ip_network)` + +The stdlib::has_ip_network function. + +Returns: `Boolean` Returns `true` if the requested IP network exists on any interface. + +##### `ip_network` + +Data type: `Stdlib::IP::Address::V4::Nosubnet` + +The IPv4 network you want to check the existence of + ### `stdlib::ip_in_range` Type: Ruby 4.x API diff --git a/functions/has_ip_address.pp b/functions/has_ip_address.pp new file mode 100644 index 000000000..d8d7e6a79 --- /dev/null +++ b/functions/has_ip_address.pp @@ -0,0 +1,10 @@ +# @summary Returns true if the client has the requested IPv4 address on some interface. +# +# @param ip_address +# The IPv4 address you want to check the existence of +# @return [Boolean] Returns `true` if the requested IP address exists on any interface. +function stdlib::has_ip_address( + Stdlib::IP::Address::V4::Nosubnet $ip_address, +) >> Boolean { + stdlib::has_interface_with('ipaddress', $ip_address) +} diff --git a/functions/has_ip_network.pp b/functions/has_ip_network.pp new file mode 100644 index 000000000..b6b9cb0f8 --- /dev/null +++ b/functions/has_ip_network.pp @@ -0,0 +1,10 @@ +# @summary Returns true if the client has the requested IPv4 network on some interface. +# +# @param ip_network +# The IPv4 network you want to check the existence of +# @return [Boolean] Returns `true` if the requested IP network exists on any interface. +function stdlib::has_ip_network( + Stdlib::IP::Address::V4::Nosubnet $ip_network, +) >> Boolean { + stdlib::has_interface_with('network', $ip_network) +} diff --git a/lib/puppet/functions/has_ip_address.rb b/lib/puppet/functions/has_ip_address.rb new file mode 100644 index 000000000..392d5c550 --- /dev/null +++ b/lib/puppet/functions/has_ip_address.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +# THIS FILE WAS GENERATED BY `rake regenerate_unamespaced_shims` + +# @summary DEPRECATED. Use the namespaced function [`stdlib::has_ip_address`](#stdlibhas_ip_address) instead. +Puppet::Functions.create_function(:has_ip_address) do + dispatch :deprecation_gen do + repeated_param 'Any', :args + end + def deprecation_gen(*args) + call_function('deprecation', 'has_ip_address', 'This function is deprecated, please use stdlib::has_ip_address instead.', false) + call_function('stdlib::has_ip_address', *args) + end +end diff --git a/lib/puppet/functions/has_ip_network.rb b/lib/puppet/functions/has_ip_network.rb new file mode 100644 index 000000000..b0219b671 --- /dev/null +++ b/lib/puppet/functions/has_ip_network.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +# THIS FILE WAS GENERATED BY `rake regenerate_unamespaced_shims` + +# @summary DEPRECATED. Use the namespaced function [`stdlib::has_ip_network`](#stdlibhas_ip_network) instead. +Puppet::Functions.create_function(:has_ip_network) do + dispatch :deprecation_gen do + repeated_param 'Any', :args + end + def deprecation_gen(*args) + call_function('deprecation', 'has_ip_network', 'This function is deprecated, please use stdlib::has_ip_network instead.', false) + call_function('stdlib::has_ip_network', *args) + end +end diff --git a/lib/puppet/parser/functions/has_interface_with.rb b/lib/puppet/parser/functions/has_interface_with.rb deleted file mode 100644 index b78029d08..000000000 --- a/lib/puppet/parser/functions/has_interface_with.rb +++ /dev/null @@ -1,68 +0,0 @@ -# frozen_string_literal: true - -# -# has_interface_with -# -module Puppet::Parser::Functions - newfunction(:has_interface_with, type: :rvalue, doc: <<-DOC - @summary - Returns boolean based on kind and value. - - @return - boolean values `true` or `false` - - Valid kinds are `macaddress`, `netmask`, `ipaddress` and `network`. - - @example **Usage** - has_interface_with("macaddress", "x:x:x:x:x:x") # Returns `false` - has_interface_with("ipaddress", "127.0.0.1") # Returns `true` - - @example If no "kind" is given, then the presence of the interface is checked: - has_interface_with("lo") # Returns `true` - DOC - ) do |args| - raise(Puppet::ParseError, "has_interface_with(): Wrong number of arguments given (#{args.size} for 1 or 2)") if args.empty? || args.size > 2 - - interfaces = lookupvar('interfaces') - - # If we do not have any interfaces, then there are no requested attributes - return false if interfaces == :undefined || interfaces.nil? - - interfaces = interfaces.split(',') - - return interfaces.member?(args[0]) if args.size == 1 - - kind, value = args - - # Bug with 3.7.1 - 3.7.3 when using future parser throws :undefined_variable - # https://tickets.puppetlabs.com/browse/PUP-3597 - factval = nil - begin - catch :undefined_variable do - factval = lookupvar(kind) - end - rescue Puppet::ParseError - end - return true if factval == value - - result = false - interfaces.each do |iface| - iface.downcase! - factval = nil - begin - # Bug with 3.7.1 - 3.7.3 when using future parser throws :undefined_variable - # https://tickets.puppetlabs.com/browse/PUP-3597 - catch :undefined_variable do - factval = lookupvar("#{kind}_#{iface}") - end - rescue Puppet::ParseError - end - if value == factval - result = true - break - end - end - - result - end -end diff --git a/lib/puppet/parser/functions/has_ip_address.rb b/lib/puppet/parser/functions/has_ip_address.rb deleted file mode 100644 index 763813c22..000000000 --- a/lib/puppet/parser/functions/has_ip_address.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -# -# has_ip_address -# -module Puppet::Parser::Functions - newfunction(:has_ip_address, type: :rvalue, doc: <<-DOC - @summary - Returns true if the client has the requested IP address on some interface. - - @return [Boolean] - `true` or `false` - - This function iterates through the 'interfaces' fact and checks the - 'ipaddress_IFACE' facts, performing a simple string comparison. - DOC - ) do |args| - raise(Puppet::ParseError, "has_ip_address(): Wrong number of arguments given (#{args.size} for 1)") if args.size != 1 - - Puppet::Parser::Functions.autoloader.load(:has_interface_with) \ - unless Puppet::Parser::Functions.autoloader.loaded?(:has_interface_with) - - function_has_interface_with(['ipaddress', args[0]]) - end -end - -# vim:sts=2 sw=2 diff --git a/lib/puppet/parser/functions/has_ip_network.rb b/lib/puppet/parser/functions/has_ip_network.rb deleted file mode 100644 index b556d97c0..000000000 --- a/lib/puppet/parser/functions/has_ip_network.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -# -# has_ip_network -# -module Puppet::Parser::Functions - newfunction(:has_ip_network, type: :rvalue, doc: <<-DOC - @summary - Returns true if the client has an IP address within the requested network. - - @return - Boolean value, `true` if the client has an IP address within the requested network. - - This function iterates through the 'interfaces' fact and checks the - 'network_IFACE' facts, performing a simple string comparision. - DOC - ) do |args| - raise(Puppet::ParseError, "has_ip_network(): Wrong number of arguments given (#{args.size} for 1)") if args.size != 1 - - Puppet::Parser::Functions.autoloader.load(:has_interface_with) \ - unless Puppet::Parser::Functions.autoloader.loaded?(:has_interface_with) - - function_has_interface_with(['network', args[0]]) - end -end - -# vim:sts=2 sw=2 diff --git a/spec/functions/has_ip_address_spec.rb b/spec/functions/has_ip_address_spec.rb index 5b787d693..7256e7518 100644 --- a/spec/functions/has_ip_address_spec.rb +++ b/spec/functions/has_ip_address_spec.rb @@ -4,22 +4,29 @@ describe 'has_ip_address' do it { is_expected.not_to be_nil } - 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('one', 'two').and_raise_error(Puppet::ParseError, %r{wrong number of arguments}i) } + it { is_expected.to run.with_params.and_raise_error(ArgumentError, %r{'stdlib::has_ip_address' expects 1 argument, got none}) } + it { is_expected.to run.with_params('one', 'two').and_raise_error(ArgumentError, %r{'stdlib::has_ip_address' expects 1 argument, got 2}) } context 'when on Linux Systems' do let(:facts) do { - interfaces: 'eth0,lo', - ipaddress: '10.0.0.1', - ipaddress_lo: '127.0.0.1', - ipaddress_eth0: '10.0.0.1' + networking: { + 'interfaces' => { + 'eth0' => { + 'ip' => '10.0.0.1', + }, + 'lo' => { + 'ip' => '127.0.0.1', + }, + }, + 'ip' => '10.0.0.1', + }, } end it { is_expected.to run.with_params('127.0.0.1').and_return(true) } it { is_expected.to run.with_params('10.0.0.1').and_return(true) } it { is_expected.to run.with_params('8.8.8.8').and_return(false) } - it { is_expected.to run.with_params('invalid').and_return(false) } + it { is_expected.to run.with_params('invalid').and_raise_error(ArgumentError, %r{parameter 'ip_address' expects a match}) } end end diff --git a/spec/functions/has_ip_network_spec.rb b/spec/functions/has_ip_network_spec.rb index 22564bb71..c8352272e 100644 --- a/spec/functions/has_ip_network_spec.rb +++ b/spec/functions/has_ip_network_spec.rb @@ -4,21 +4,28 @@ describe 'has_ip_network' do it { is_expected.not_to be_nil } - 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('one', 'two').and_raise_error(Puppet::ParseError, %r{wrong number of arguments}i) } + it { is_expected.to run.with_params.and_raise_error(ArgumentError, %r{'stdlib::has_ip_network' expects 1 argument, got none}) } + it { is_expected.to run.with_params('one', 'two').and_raise_error(ArgumentError, %r{'stdlib::has_ip_network' expects 1 argument, got 2}) } context 'when on Linux Systems' do let(:facts) do { - interfaces: 'eth0,lo', - network_lo: '127.0.0.0', - network_eth0: '10.0.0.0' + networking: { + 'interfaces' => { + 'eth0' => { + 'network' => '10.0.0.0', + }, + 'lo' => { + 'network' => '127.0.0.0', + }, + }, + }, } end it { is_expected.to run.with_params('127.0.0.0').and_return(true) } it { is_expected.to run.with_params('10.0.0.0').and_return(true) } it { is_expected.to run.with_params('8.8.8.0').and_return(false) } - it { is_expected.to run.with_params('invalid').and_return(false) } + it { is_expected.to run.with_params('invalid').and_raise_error(ArgumentError, %r{parameter 'ip_network' expects a match}) } end end