Ohai merges duplicate plugins in very dangerous ways

[jaymzh]

Description

It turns out that Ohai evals every plugin in every directory... and so all methods inside of Ohai.plugin blocks are evaluated... however the collect_data blocks are not - if a second one is hit on the same platform, it will not evaluate it, it runs on the first one it hits. This means the methods from the last plugin seen win, but the collector code from the first plugin seen wins.

This means that you can into a problem where if you have two versions of the same module in different plugin paths, the first path will be run but call the methods from the second plugin.

So if you have /root/ohai_repro/test.rb with:

Ohai.plugin(:Test) do
  provides 'xtest'
  def test(a, b, c)
    Ohai::Log.warn("this is test() from test1")
  end
  collect_data do
    Ohai::Log.warn("this is collect_data from test1")
    test(1, 2, 3)
    xtest 'test1'
  end
end

So if you have /root/ohai_repro2/test.rb with:

Ohai.plugin(:Test) do
  provides 'xtest'
  def test(a, b, c)
    Ohai::Log.warn("this is test() from test2")
  end
  collect_data do
    Ohai::Log.warn("this is collect_data from test2")
    test(1, 2, 3)
    xtest 'test2'
  end
end

Your output will be the very unexpected:

[2018-07-06T10:36:27-07:00] WARN: this is collect_data from test1
[2018-07-06T10:36:27-07:00] WARN: this is test() from test2

Woah!! That's pretty weird.

Ohai Version

13.8.0

works on 12 too.

Platform Version

CentOS 7.5

[btm]

@jaymzh what's the ideal/expected behavior for you? One plugin always wins? An exception is raised for duplicate plugins?

[jaymzh]

@btm I'd expect the behavior to be the same as the collect_data blocks

Currently that means first one wins (with a warning on subsequent ones), but I don't really care if it's first one wins or last one wins, I can order my plugin list accordingly. It seems like last one should win since core plugins come first, but that would be a very significant change and not a bugfix, so we probably don't want to do that without an RFC. But making sure the whole plugin follows the same rules as collect_data (which right now is first-win) is a critical bugfix.

[lamont-granquist]

It looks like someone tried to make multiple collect_data statements for the same platform fail with an exception:

https:/chef/ohai/blob/master/lib/ohai/dsl/plugin/versionvii.rb#L111-L115

Its possible that gets raised only if it is declared in the same file, and not across two different files?

Whatever the fix, probably need to be careful about explicitly allowing different implementations for e.g. linux and solaris in different files to still work.

[jaymzh]

That error is raised - and like any other error in Ohai - is caught, printed, and then it carry's on.

So we have several of those. ;) But it shouldn't have eval'd it in the same namespace to determine that, IMO.

However, It does because that collect_data block could be for a different platform.

[lamont-granquist]

ah, so that's the problem.

we're monkeypatching the class, so last-writer-wins for methods, but the last-writer throws an exception there instead of wiring up its collect_data method.

the intent seems to be that it should throw and fail, but that's difficult with ohai's never-fail "philosophy"

[btm]

@jaymzh how are you specifying multiple ohai plugin directories in your repro case?

[jaymzh]

I put those two dirs in client.rb and then ran chef-client -o test where test::default was an empty recipe..

[jaymzh]

@btm you may want to base your work on #1221 to make your life easier :)

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.