Enable scoped_search for monitoring_proxy

[ochnerd]

We want to be able to easily search Hosts without any monitoring_proxy set

[ochnerd]

Hi @dgoetz , could you take a look at this PR? It is a feature we would like to have.

[dgoetz]

Two questions on the implementation: Is it possible to limit the suggestions to only smart proxies which have the feature monitoring? Is it possible to have a test for this to prevent breaking it in the future?

A note on the feature itself, for perhaps adding documentation in the future: I can search for hosts with a specific smart proxy set (=), a specific smart proxy not set (!=), same for matching (~) and not matching (!~) and lists (^ / !^), and I can negate the search (! before the query). Last one would be the solution for "Hosts without any monitoring_proxy set".

[ekohl]

As noted in Matrix: I don't see any relationship defined. Normally you include BelongsToProxies and then use belongs_to_proxy on the model. Foreman's subnet has examples.

[ochnerd]

@ekohl so you mean add belongs_to_proxy :monitoring in https://github.com/theforeman/foreman_monitoring/blob/master/app/models/monitoring_result.rb?

[ekohl]

Oh, I now see where it's added. I missed that at first:

foreman_monitoring/lib/foreman_monitoring/engine.rb

Lines 69 to 78 in d1c0bfa

	monitoring_proxy_options = {
	:feature => 'Monitoring',
	:label => N_('Monitoring Proxy'),
	:description => N_('Monitoring Proxy to use to manage monitoring of this host'),
	:api_description => N_('ID of Monitoring Proxy to use to manage monitoring of this host')
	}
	# add monitoring smart proxy to hosts and hostgroups
	smart_proxy_for Host::Managed, :monitoring_proxy, monitoring_proxy_options
	smart_proxy_for Hostgroup, :monitoring_proxy, monitoring_proxy_options

That may be sufficient.

Two questions on the implementation: Is it possible to limit the suggestions to only smart proxies which have the feature monitoring? Is it possible to have a test for this to prevent breaking it in the future?

See my above comments: I'd expect it to already respect what smart_proxy_for does. If it doesn't then that would be the proper fix.

[ochnerd]

So the search for monitoring_proxy is working in the Hosts View but it suggests every Smart Proxy not just the one with the feature monitoring.

[ekohl]

So it's not filtering on the feature. I suspect because that's added as a validation which scoped_search ignores:
https://github.com/theforeman/foreman/blob/0d9d5fddf0f326b7d882f90d84e8061445fe0b23/app/models/concerns/belongs_to_proxies.rb#L17-L21

I wonder if you can somehow make belongs_to aware of the constraint. Looking at https://api.rubyonrails.org/v7.0.2/classes/ActiveRecord/Associations/ClassMethods.html#method-i-belongs_to you can pass where constraints. If you solve that in Foreman itself then all plugins should benefit from it.

[ochnerd]

Hmm I build a sql query which allows filtering by feature select smart_proxies.name from smart_proxies INNER JOIN smart_proxy_features ON smart_proxy_features.smart_proxy_id = smart_proxies.id INNER JOIN features ON features.id = smart_proxy_features.feature_id where features.name = 'Monitoring'; I have seen that it is possible to use join in contraints.
Maybe something like this would also be possible.

[jeremylenz]

I wonder if you can somehow make belongs_to aware of the constraint. Looking at https://api.rubyonrails.org/v7.0.2/classes/ActiveRecord/Associations/ClassMethods.html#method-i-belongs_to you can pass where constraints. If you solve that in Foreman itself then all plugins should benefit from it.

We recently did something like this in Katello and it works well: https://github.com/Katello/katello/blob/df7ea596fc2b655c3c61a90293aac478d9804bb2/app/models/katello/content_view.rb#L45

[ochnerd]

I tried it like that

   def register_smart_proxy(name, options)
      self.registered_smart_proxies = registered_smart_proxies.merge(name => options)
      has_many :smart_proxy_features, :dependent => :destroy
      has_many :features, :through => :smart_proxy_features
      belongs_to name, -> { where( :features  => { :name => options[:feature]}).joins(:features)}, :class_name => 'SmartProxy'
      validates name, :proxy_features => { :feature => options[:feature], :required => options[:required] }
   end

but got the same. So it suggests all Proxies.

[ekohl]

      has_many :smart_proxy_features, :dependent => :destroy
      has_many :features, :through => :smart_proxy_features

I don't think these should be needed.

      belongs_to name, -> { where( :features  => { :name => options[:feature]}).joins(:features)}, :class_name => 'SmartProxy'

This looks alright. My theory is that scoped_search doesn't look at the scope when searching.

When digging into this I found that it's really the complete_value_from_db method that gives the options. That calls completer_scope and in my testing I found it doesn't respect the scope from belongs_to. These are the methods:

https://github.com/wvanbergen/scoped_search/blob/3c431dff922c57a06989ddde999c71e44430bf33/lib/scoped_search/auto_complete_builder.rb#L208-L225

It may be tricky to apply the scope. I found you can call Subnet.reflect_on_association('dhcp') to get the reflection for it. That stores the scope.

ekohl/scoped_search@9567990 is where I tried to reproduce this in tests. However, the complete_for doesn't show any results so I'm missing something.

However, this is where I need to end my research. If you want to solve this it's probably within scoped_search or how we call scoped_search.

On a related note: I think can reuse the with_features scope

belongs_to name, -> { with_features(options[:feature]) }, :class_name => 'SmartProxy'

[ochnerd]

@ekohl thanks for your help. I'm going to research a little bit after my vacation. Maybe I could try to use scoped_search with external_search_ method https://github.com/wvanbergen/scoped_search/wiki/Search-definition#external-search-methods.
I'm going to look into it.

[dgoetz]

@ochnerd If you find out it is not fixable, I am not against merging the current state as it is already an improvement!

So thanks already to everyone for investigating!

[ekohl]

I agree with @dgoetz: this looks like a fundamental problem that we already have in Foreman with other proxies like DHCP or DNS. If we can resolve it in Foreman itself then this fix should be good already.

[ochnerd]

I got to experiment a little bit with ext_method but haven't been successfull. But I found am PR which is Foreman releated in scoped_search wvanbergen/scoped_search#221 which is about enhanced_filtering theforeman/foreman#10197.

@dgoetz I think we should merge this and when I have spare time i look into the scoping issues.

[dgoetz]

Any other feature planned for inclusion or should I do a new release?

[ochnerd]

We currently have no other features planned. Please create a new release. Thank you!