SL generic attribute keys, even nested, are breaking my node search

[in4mer]

Environment Information

Please include the following:

• version of cookbook: 1.2.23
• version of chef: 12.19.36
• operating system: linux
• resources that are affected by issue:
  All of my cookbooks that use search() functionality.

Expected Behavior

knife search node name:mongo\* -i should return nodes matching that pattern, and not nodes whose names do not match that pattern

Actual Behavior

All of my nodes that have the sumlogic-collector cookbook loaded are also returning in those search results.

Steps to Reproduce

Define a source that looks for a name of mongod.log on a node whose name does not match ^mongo.*
Do a knife search on node for name:mongo*
Non-matching node will appear

Important Factoids

Fundamentally, this is a chef-server search functionality bug due to flattened attribute trees indexed to provide wildcard search capabilities. They're making noises that sound like "wontfix" even though it's a pretty bad design, so I would sure appreciate it if someone would take a few minutes to switch things like sources and name to sumosources or sumoname or something like that.

Yes I realize this is potentially a breaking change, but now I'm having to write select{|n| ...} statements for all my node name searches because node_default['sumologic']['sources']['name'] = 'mongod.log' is breaking my existing search calls.

References

Are there any other GitHub issues (open or closed) or Pull Requests that should be linked here? For example:

• not that I can find

[majormoses]

I do not currently use sumologic (as I recently switched employers and they do not use it) but continue to maintain the cookbook when I have time so apologies for the delayed response.

Thanks for reporting, I am familiar with the issue you speak of but I have not encountered it for several years because I architected the issue away in my last few organizations so I am unsure on chefs current stance. If I had to guess it would be like you said and unlikely to want to change. I need to think about this as this would most likely be a breaking change and it currently defaults to nil so the number of people affected by this issue would be very small while the number of people it may break will be significantly higher[1] . I agree that we should make the change the question is how and why. This is clearly a no win scenario so we need to weigh the costs and benefits we get for making the change. I think you have two paths forward while we decide on how/when we look at a cookbook change:

• do not use the recipes and rely on the LWRPs as this will sidestep the issue entirely and is probably the best approach anyways
• modify your searches to use other attributes such as roles as they will less likely have namespace conflicts or use custom attributes

[1]: need to vet

They're making noises that sound like "wontfix" even though it's a pretty bad design

Are there any other GitHub issues (open or closed) or Pull Requests that should be linked here? For example:

• not that I can find

Can you elaborate on this? Do you have a github issue, slack/irc transcript, blog post, etc? Like I said I have not had to deal with this issue in years so even reading an old issue would be helpful to refresh my memory on the nuances of it.

The only way I can think of making this backwards compatible is to leave the old namespace intact and introduce a new attribute which could be used and would also default to nil. Then we could modify the logic to be something like this:

if !node['sumologic']['collector_name'].nil?
  # use node['sumologic']['collector_name']
elsif !node['sumologic']['name'].nil?`
  # log deprecation and warn that in the next major release the attribute will not be honored anymore
  # use node['sumologic']['name']
else
  # use node['hostname']
end

I have not looked at the code yet (as github search blows and I don't currently have it local) and wanted to respond and gather some information before deciding what to do.

Assuming that something like the above will work I think this is the best of both worlds. It fixes a real problem, avoids breaking people in the short term, warns them about future changes, and gives us a clear upgrade/deprecation path.