Skip to content

Commit d72629d

Browse files
adamruzickaclaude
andcommitted
Fixes #XXXXX - Track and surface unrecognized Smart Proxy features
When a Smart Proxy reports features that Foreman doesn't recognize (e.g. a plugin installed on the proxy but not on the server), those features were silently dropped. Now they are persisted and surfaced to the user via the API, UI, and server logs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent dc65170 commit d72629d

7 files changed

Lines changed: 92 additions & 3 deletions

File tree

app/models/smart_proxy.rb

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ class SmartProxy < ApplicationRecord
66
include Taxonomix
77
include Parameterizable::ByIdName
88

9+
serialize :unrecognized_features, Array
10+
911
validates_lengths_from_database
1012
before_destroy EnsureNotUsedBy.new(:hosts, :hostgroups, :subnets, :domains, [:puppet_ca_hosts, :hosts], [:puppet_ca_hostgroups, :hostgroups], :realms)
1113
# TODO check if there is a way to look into the tftp_id too
@@ -168,10 +170,16 @@ def associate_features
168170
end
169171

170172
feature_name_map = Feature.name_map
171-
valid_features = reply.select { |feature, options| feature_name_map.key?(feature) }
173+
valid_features, unknown_features = reply.partition { |feature, _options| feature_name_map.key?(feature) }
174+
valid_features = valid_features.to_h
175+
self.unrecognized_features = unknown_features.map(&:first)
172176

173177
if valid_features.any?
174178
SmartProxyFeature.import_features(self, valid_features)
179+
if unrecognized_features.any?
180+
logger.warn("Proxy #{name} has features not recognized by Foreman: #{unrecognized_features.to_sentence}. "\
181+
"If these features come from a Smart Proxy plugin, make sure Foreman has the plugin installed too.")
182+
end
175183
else
176184
smart_proxy_features.clear
177185
if reply.any?

app/views/api/v2/smart_proxies/main.json.rabl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ object @smart_proxy
22

33
extends "api/v2/smart_proxies/base"
44

5-
attributes :created_at, :updated_at, :hosts_count
5+
attributes :created_at, :updated_at, :hosts_count, :unrecognized_features
66

77
child :smart_proxy_features => :features do
88
attributes :capabilities

app/views/smart_proxies/index.html.erb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,12 @@
2020
<td class="nbsp ellipsis"><%= link_to_if_authorized proxy.name, {:action => :show, :id => proxy} %></td>
2121
<td class="hidden-sm hidden-xs nbsp ellipsis"><%= generate_links_for(proxy.locations).html_safe %></td>
2222
<td class="hidden-sm hidden-xs nbsp ellipsis"><%= generate_links_for(proxy.organizations).html_safe %></td>
23-
<td class="ellipsis"><%= proxy.features.map(&:name).sort.to_sentence %></td>
23+
<td class="ellipsis">
24+
<%= proxy.features.map(&:name).sort.to_sentence %>
25+
<% if proxy.unrecognized_features.present? %>
26+
<span class="pficon pficon-warning-triangle-o" title="<%= _('This proxy has features not recognized by Foreman: %s') % proxy.unrecognized_features.to_sentence %>"></span>
27+
<% end %>
28+
</td>
2429
<td>
2530
<span class="proxy-show-status"><%= spinner %></span>
2631
</td>

app/views/smart_proxies/show.html.erb

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,22 @@
6464
<% end %>
6565
</div>
6666

67+
<% if @smart_proxy.unrecognized_features.present? %>
68+
<div class="row">
69+
<div class="col-md-4">
70+
<strong><span class="pficon pficon-warning-triangle-o"></span> <%= _('Unrecognized features') %></strong>
71+
</div>
72+
<div class="col-md-8">
73+
<p><%= _('The following features are reported by this Smart Proxy but are not recognized by Foreman. The corresponding Foreman plugins may not be installed.') %></p>
74+
<ul>
75+
<% @smart_proxy.unrecognized_features.each do |feature| %>
76+
<li><%= h(feature) %></li>
77+
<% end %>
78+
</ul>
79+
</div>
80+
</div>
81+
<% end %>
82+
6783
<div class="row">
6884
<div class="col-md-4"></div>
6985
<div class="col-md-8">
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
class AddUnrecognizedFeaturesToSmartProxies < ActiveRecord::Migration[7.0]
2+
def change
3+
add_column :smart_proxies, :unrecognized_features, :text
4+
end
5+
end

test/controllers/api/v2/smart_proxies_controller_test.rb

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,31 @@ class Api::V2::SmartProxiesControllerTest < ActionController::TestCase
202202
# assert_response :unprocessable_entity
203203
# end
204204

205+
test "should include unrecognized_features in show response" do
206+
features_with_unknown = Hash[Feature.name_map.keys.collect { |f| [f, {'state' => 'running'}] }]
207+
features_with_unknown['unknown_plugin'] = {'state' => 'running'}
208+
ProxyAPI::V2::Features.any_instance.stubs(:features).returns(features_with_unknown)
209+
proxy = smart_proxies(:one)
210+
proxy.save!
211+
get :show, params: { :id => proxy.to_param }
212+
assert_response :success
213+
show_response = ActiveSupport::JSON.decode(@response.body)
214+
assert_equal ['unknown_plugin'], show_response['unrecognized_features']
215+
end
216+
217+
test "should include unrecognized_features in index response" do
218+
features_with_unknown = Hash[Feature.name_map.keys.collect { |f| [f, {'state' => 'running'}] }]
219+
features_with_unknown['unknown_plugin'] = {'state' => 'running'}
220+
ProxyAPI::V2::Features.any_instance.stubs(:features).returns(features_with_unknown)
221+
proxy = smart_proxies(:one)
222+
proxy.save!
223+
get :index
224+
assert_response :success
225+
index_response = ActiveSupport::JSON.decode(@response.body)
226+
proxy_response = index_response['results'].find { |p| p['id'] == proxy.id }
227+
assert_equal ['unknown_plugin'], proxy_response['unrecognized_features']
228+
end
229+
205230
test "should refresh smart proxy features" do
206231
proxy = smart_proxies(:one)
207232
post :refresh, params: { :id => proxy }

test/models/smart_proxy_test.rb

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,36 @@ class SmartProxyTest < ActiveSupport::TestCase
115115
assert_equal(error_message, proxy.errors[:base].first)
116116
end
117117

118+
test "should store unrecognized features when proxy has mix of known and unknown" do
119+
ProxyAPI::V2::Features.any_instance.stubs(:features).returns(
120+
'tftp' => {'state' => 'running'},
121+
'unknown_plugin' => {'state' => 'running'},
122+
'another_unknown' => {'state' => 'running'}
123+
)
124+
proxy = FactoryBot.build(:smart_proxy)
125+
assert proxy.save
126+
proxy.reload
127+
assert_include(proxy.features, features(:tftp))
128+
assert_equal %w[unknown_plugin another_unknown].sort, proxy.unrecognized_features.sort
129+
end
130+
131+
test "should clear unrecognized features when all proxy features are known" do
132+
ProxyAPI::V2::Features.any_instance.stubs(:features).returns(
133+
'tftp' => {'state' => 'running'}
134+
)
135+
proxy = FactoryBot.build(:smart_proxy)
136+
assert proxy.save
137+
proxy.reload
138+
assert_empty proxy.unrecognized_features
139+
end
140+
141+
test "should populate unrecognized features even when no valid features exist" do
142+
proxy = SmartProxy.new(:name => 'Proxy', :url => 'https://some.where.net:8443')
143+
ProxyAPI::V2::Features.any_instance.stubs(:features).returns({'unknown_feature' => {'state' => 'running'}})
144+
refute proxy.save
145+
assert_equal ['unknown_feature'], proxy.unrecognized_features
146+
end
147+
118148
test "can import and access capabilities and settings" do
119149
ProxyAPI::V2::Features.any_instance.stubs(:features).returns(:tftp => {:settings => {:foo => :bar}, :capabilities => ['FOO'], :state => 'running'})
120150
proxy = FactoryBot.build(:smart_proxy)

0 commit comments

Comments
 (0)