Regex detection for Puppet configuration

Question

I've got a PR to add a feature to a Ruby application (Puppet) to allow Regex detection.

Right now, the method only does comparison on exact string matching:

defaultfor :operatingsystem => :fedora, :operatingsystemmajrelease => ['22', '23', '24']

This is annoying, as we have to add in new values everytime a new Fedora release comes out, for example.

I've extended it to match on a Regex:

defaultfor :operatingsystem => :fedora, :operatingsystemmajrelease => /^2[2-9]$/

The old method looks like this:

def self.fact_match(fact, values)
  values = [values] unless values.is_a? Array
  values.map! { |v| v.to_s.downcase.intern }

  if fval = Facter.value(fact).to_s and fval != ""
    fval = fval.to_s.downcase.intern

    values.include?(fval)
  else
    false
  end
end

This is my current proof-of-concept code, which will match both regex and strings, but it feels off.

def self.fact_match(fact, values)

  values = [values] unless values.is_a? Array

  values.map! do |v|
    if v.is_a? Regexp
      v
    else
      v.to_s.downcase.intern
    end
  end

  if fval = Facter.value(fact).to_s and fval != ""
    fval = fval.to_s.downcase.intern
    if values.any? {|v| v.is_a? Regexp }
      regex_match = Regexp.union(values)
      fval =~ regex_match
    else
      values.include?(fval)
    end
  else
    false
  end
end

I feel like there's a much easier way of detecting if it's a Regex or not.

Answer 1

You're working too hard, trying to distinguish strings from regular expressions. Rather, you should take advantage of the fact that the === operator for a RegExp and the === operator for a String both express the idea of a "match", which is what you want here.

I don't understand why you need to #intern all the strings.

I don't see why the fval = Facter.value(fact).to_s assignment appears inside an if condition. The result of the assignment would only be false if Facter.value(fact).to_s is false or nil, which can never happen. (Even nil.to_s results in an empty string.)

To treat values as an array regardless of whether a scalar or an array was passed in, you can use the [*values] idiom.

def self.fact_match(fact, values)
  fact_val = Facter.value(fact).to_s.downcase
  fact_val != "" and [*values].any? { |v| v === fact_val }
end

In addition to supporting literal case-insensitive matches and regular expression matches, also consider supporting Ranges.