Fix postgres_conf ability to test parameters that have a dot in them

[aaronlippold]

Fixes #1671

Signed-off-by: Aaron Lippold lippold@gmail.com

[aaronlippold]

@arlimus or @chris-rock or @adamleff please review

[chris-rock]

@aaronlippold Thank for this addition. Let's add some unit tests, so that we can ensure this stays in our scope in future

[aaronlippold]

@chris-rock we don't need no stinking unit tests ... lol. Ok working on them now :)

[chris-rock]

In this case i think it is very useful especially since the postgres config includes those values!

[aaronlippold]

@chris-rock yes, I was trying to make a joke, badly it seems :)

[chris-rock]

haha, all good

[adamleff]

Do we really need to bring in ObjectTraverser for this change? That mixin is great for config files which are multiple-levels deep, and postgres.conf is single level (which just happens to have keys that can have dots in them, which confuse things a bit). No multidimensional hashes here.

We should be able to address this need by changing the method_missing method to:

    def method_missing(*name)
      if name.is_a?(Array) && name.first == :[]
        # user passed in parameter using: its(['foo.bar'])
        # index 0 will be :[], and index 1 will be the actual key they're looking for
        name = name[1]
      end

      param = params[name.to_s]
      return nil if param.nil?
      # extract first value if we have only one value in array
      return param[0] if param.length == 1
      param
    end

method_missing just needs to take a splat instead of a single parameter, and then we need to pluck the second one off the array.

[aaronlippold]

@adamleff if there is a simpler solution totally open to it. I just used that approach because in the original issue you had suggested it.

#1671

The correct syntax here is similar to the Elasticsearch YAML inquiry you and I spoke about today:

its(['pgaudit.log_level']) { should cmp 'log' }
However, this resource isn't currently written to support that. It needs to be extended to use ObjectTraverser like the JsonConfig class and have the method_missing method updated accordingly given that there can be dots in the keys.

Flagging as a bug.

I will update the code as you requested above and push again.

[adamleff]

Yup, and I'm not perfect and sometimes have better ideas as time goes on :) I'm fine with ObjectTraverser here, but after that discussion, I went through and checked more Postgres documentation and examples and there is no hierarchy in the config... it's flat. So we don't need ObjectTraverser here. That's the only point I'm trying to make, both in an effort to educate, but also to keep resources as simple as necessary to do the right functionality.

I'll defer to @chris-rock or @arlimus if they'd like to use ObjectTraverser here or take a simpler approach.

[aaronlippold]

@adamleff Ok. Still having a few issues with that method. I am getting a nil return on the complex params.

Can I shoot you a copy of the updates in slack and perhaps I just made a small goof but I agree, if we don't need to pull in an entire separate lib for a simple extraction then I am all for it.

[aaronlippold]

@chris-rock requested changes added.

[adamleff]

Looks fine except for the test file. You've got my approval if we eliminate the redundant describe in the unit test.

[adamleff]

Let's fold the two describe blocks into one as they are redundant.

describe 'Inspec::Resources::Postgres' do
  it 'verify postgresql.conf config parsing of a simple key value' do
    # etc...
  end

  it 'verify postgresql.conf config parsing of a complex key value' do
    # second test here
  end
end

[aaronlippold]

@adamleff changes pushed. Thanks.

[chris-rock]

Thank you @aaronlippold for this improvement