fix indents so we don't overwrite the main settings.yml

[jrafanie]

This should fix the following errors on manageiq master:

  1) EmsEvent.event_groups returns a list of groups
     Failure/Error: expect(described_class.event_groups[:addition][:critical]).to include('CloneTaskEvent')
       expected ["aggregate.addhost.end", "aggregate.create.end", "aggregate.removehost.end", "aggregate.updateprop.e..."AWS_EC2_Instance_DELETE", "TerminateInstances", "RunInstances", "virtualMachines_write_EndRequest"] to include "CloneTaskEvent"
     # ./spec/models/ems_event_spec.rb:212:in `block (3 levels) in <top (required)>'
  2) EmsEvent.event_groups returns the provider event if configured
     Failure/Error: expect(described_class.event_groups[:addition][:critical]).to include('CloneTaskEvent')
       expected ["aggregate.addhost.end", "aggregate.create.end", "aggregate.removehost.end", "aggregate.updateprop.e...TerminateInstances", "RunInstances", "virtualMachines_write_EndRequest", "SomeSpecialProviderEvent"] to include "CloneTaskEvent"
     # ./spec/models/ems_event_spec.rb:232:in `block (3 levels) in <top (required)>'
Finished in 8 minutes 36 seconds (files took 1 minute 2.99 seconds to load)
4177 examples, 2 failures, 19 pending
Failed examples:
rspec ./spec/models/ems_event_spec.rb:205 # EmsEvent.event_groups returns a list of groups
rspec ./spec/models/ems_event_spec.rb:216 # EmsEvent.event_groups returns the provider event if configured

[jrafanie]

cc @tzumainn

[bdunne]

👍 LGTM

[miq-bot]

Checked commit jrafanie@f52a80f with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
0 files checked, 0 offenses detected
Everything looks fine. 🍪

[jrafanie]

@durandom Please review... the way the provider plugin code merges in provider settings should probably not allow plugins to touch other plugins or the main settings.yml like this did.

[aufi]

Thanks for the fix

[durandom]

@jrafanie I've discussed this with @Fryguy in ManageIQ/manageiq-providers-amazon#138 and ManageIQ/manageiq#13942

I dont see a reasonable approach for that until we have either scoped all plugin related settings under one plugin key (then this would be the gatekeeper) or have real content based validation of the config file / structure.

Maybe this is a use case for re-arch too? When moving to a distributed config storage? Does e.g. etcd support config schema validation?

[Fryguy]

We can actually do config validation with the config gem... It's a relatively new feature we could leverage.

[durandom]

@Fryguy yeah, I experienced this the hard way when moving the authenticator settings to aws. But afaik we only do this for a small amount of the settings... Maybe we should expand that on some crucial settings that potentially break the app

[Fryguy]

Can we just go through all of the plugins and ensure they have the event_handling indented like this PR?

[jrafanie]

@durandom @Fryguy It's an easy mistake since yaml is so unforgiving but it might be worthwhile to only let providers merge in their subhash as opposed to the root of the hash.

[Fryguy]

to only let providers merge in their subhash as opposed to the root of the hash.

💯 This would be great. I think to @durandom's point though, we have to refactor some things to make that a reality. I feel we should just prioritize the effort for that though.

[durandom]

merge in their subhash

Thats hard to define. We could start with the section we know and check that when loading settings.
However this mixes validation and merging of the config...
I'll explore both approaches
Added item to ManageIQ/manageiq#14840