fixes #36160 - Redefine append domain names setting

This PR aims to unify the format of host names stored in the database and the way they are displayed.
With this change, the name of the host is always going to be stored with the domain name appended.
The setting formerly named `append_domain_name_for_hosts` is now renamed to `display_fqdn_for_hosts`
because it will only impact how the names are displayed from now.
This means dashboards and breadcrumbs are going to display the whole FQDN if you choose to.

app/assets/javascripts/host_edit_interfaces.js

@@ -391,9 +391,6 @@ $(document).on('change', '.virtual', function() {
 function construct_host_name() {
   var host_name_el = $('#host_name')
   var host_name = host_name_el.val();
-  if (host_name_el.data('appendDomainNameForHosts') === false) {
-    return host_name;
-  }
   var domain_name = primary_nic_form()
     .find('.interface_domain option:selected')
     .text();

app/helpers/application_helper.rb

@@ -411,6 +411,7 @@ def ui_settings
       destroyVmOnHostDelete: Setting['destroy_vm_on_host_delete'],
       labFeatures: Setting[:lab_features],
       safeMode: Setting[:safemode_render],
+      displayFqdnForHosts: Setting[:display_fqdn_for_hosts],
     }
   end
 

app/models/host/base.rb

@@ -115,6 +115,7 @@ def dup
       :domain=, :domain_id=, :domain_name=, :to => :primary_interface
 
     attr_writer :updated_virtuals
+
     def updated_virtuals
       @updated_virtuals ||= []
     end
@@ -351,6 +352,14 @@ def render_template(template:, **params)
       template.render(host: self, **params)
     end
 
+    def to_label
+      if Setting[:display_fqdn_for_hosts]
+        name
+      else
+        name.split('.')[0]
+      end
+    end
+
     private
 
     def parse_ip_address(address, ignore_link_local: true)

app/registries/foreman/settings/general.rb

@@ -51,11 +51,11 @@
       description: N_("Whether or not to show a menu to access experimental lab features (requires reload of page)"),
       default: false,
       full_name: N_('Show Experimental Labs'))
-    setting('append_domain_name_for_hosts',
+    setting('display_fqdn_for_hosts',
       type: :boolean,
-      description: N_('Foreman will append domain names when new hosts are provisioned'),
+      description: N_('Foreman will display domain names appended to hostnames.'),
       default: true,
-      full_name: N_('Append domain names to the host'))
+      full_name: N_('Display FQDN for hosts'))
     setting('outofsync_interval',
       type: :integer,
       description: N_('Duration in minutes after servers are classed as out of sync. ' \

app/services/name_synchronizer.rb

@@ -25,6 +25,6 @@ def sync_required?
   private
 
   def interface_name
-    Setting[:append_domain_name_for_hosts] ? @interface.name : @interface.shortname
+    @interface.name
   end
 end

app/views/api/v2/hosts/base.json.rabl

@@ -1,3 +1,7 @@
 object @host
 
 attributes :name, :id
+
+node :displayname do |host|
+  host.to_label
+end

app/views/dashboard/_hosts_in_build_mode_widget_host_list.html.erb

@@ -8,7 +8,7 @@
   <tbody>
     <% hosts.each do |host| %>
       <tr>
-        <td class="ellipsis"><%= host_build_status_icon(host) %> <%= link_to host.name, current_host_details_path(host) %></td>
+        <td class="ellipsis"><%= host_build_status_icon(host) %> <%= link_to host, current_host_details_path(host) %></td>
         <td class="hidden-tablet hidden-xs ellipsis"><%= host.owner %></td>
         <td class="hidden-tablet hidden-xs ellipsis"><%= build_duration(host) %></td>
         <td class="hidden-tablet hidden-xs ellipsis"><%= date_time_relative(host.token&.expires)%></td>

app/views/dashboard/_new_hosts_widget_host_list.html.erb

@@ -9,7 +9,7 @@
   <tbody>
     <% hosts.each do |host| %>
       <tr>
-        <td class="ellipsis"><%= link_to host.name, current_host_details_path(host) %></td>
+        <td class="ellipsis"><%= link_to host, current_host_details_path(host) %></td>
         <td class="hidden-tablet hidden-xs ellipsis"><%= safe_join([icon(host.operatingsystem, :size => '16x16'), host.operatingsystem.to_label]) if host.operatingsystem.present? %></td>
         <td class="hidden-tablet hidden-xs ellipsis"><%= host.owner %></td>
         <td class="hidden-tablet hidden-xs ellipsis"><%= date_time_relative(host.created_at) %></td>

app/views/hosts/_form.html.erb

@@ -32,7 +32,7 @@
         <%= text_f f, :name, :size => "col-md-4", :value => name_field(@host),
             :input_group_btn => randomize_mac_link,
             :help_inline => _("This value is used also as the host's primary interface name."),
-            :data => { 'append_domain_name_for_hosts' => Setting[:append_domain_name_for_hosts] }
+            :data => {}
         %>
 
         <% if show_organization_tab?  %>

db/migrate/20230414091257_rename_append_domain_setting.rb

@@ -0,0 +1,9 @@
+class RenameAppendDomainSetting < ActiveRecord::Migration[6.1]
+  def up
+    Setting.find_by(name: 'append_domain_name_for_hosts').update_attribute(:name, 'display_fqdn_for_hosts')
+  end
+
+  def down
+    Setting.find_by(name: 'display_fqdn_for_hosts').update_attribute(:name, 'append_domain_name_for_hosts')
+  end
+end

db/migrate/20230418075940_assing_fqdn_to_host_name.rb

@@ -0,0 +1,11 @@
+class AssingFqdnToHostName < ActiveRecord::Migration[6.1]
+  def up
+    Host.all.each do |host|
+      if !host.domain.nil? && !host.name.nil?
+        unless host.name.ends_with? host.domain.name
+          host.update(name: host.fqdn)
+        end
+      end
+    end
+  end
+end

test/controllers/api/v2/settings_controller_test.rb

@@ -8,16 +8,16 @@ def setup
     end
 
     test "should get all settings through index" do
-      Setting['append_domain_name_for_hosts'] = false
+      Setting['display_fqdn_for_hosts'] = false
       get :index, params: { per_page: 'all' }
       assert_response :success
       settings = ActiveSupport::JSON.decode(@response.body)['results']
       assert_equal Foreman.settings.count, settings.count
       foreman_url = settings.detect { |s| s['name'] == 'foreman_url' }
       assert_equal Setting['foreman_url'], foreman_url['value']
       assert_equal Foreman.settings.find('foreman_url').default, foreman_url['default']
-      append_domain_name_for_hosts = settings.detect { |s| s['name'] == 'append_domain_name_for_hosts' }
-      assert_equal false, append_domain_name_for_hosts['value']
+      display_fqdn_for_hosts = settings.detect { |s| s['name'] == 'display_fqdn_for_hosts' }
+      assert_equal false, display_fqdn_for_hosts['value']
     end
 
     test "should get index with organization and location params" do
@@ -68,8 +68,8 @@ def setup
     end
 
     test "properly show overriden false value" do
-      Setting['append_domain_name_for_hosts'] = value = false
-      get :show, params: { :id => 'append_domain_name_for_hosts' }
+      Setting['display_fqdn_for_hosts'] = value = false
+      get :show, params: { :id => 'display_fqdn_for_hosts' }
       assert_response :success
       show_response = ActiveSupport::JSON.decode(@response.body)
       assert_equal value, show_response['value']

test/integration/host_js_test.rb

@@ -173,8 +173,8 @@ class HostJSTest < IntegrationTestWithJavascript
         find('h5', :text => /newhost2.*/) # wait for the new host details page
       end
 
-      test "redirects correctly with append_domain_name_for_hosts turned off" do
-        Setting['append_domain_name_for_hosts'] = false
+      test "redirects correctly with display_fqdn_for_hosts turned off" do
+        Setting['display_fqdn_for_hosts'] = false
         compute_resource = FactoryBot.create(:compute_resource, :libvirt)
         os = FactoryBot.create(:ubuntu14_10, :with_associations)
         Nic::Managed.any_instance.stubs(:dns_conflict_detected?).returns(true)

test/models/lookup_value_test.rb

@@ -56,19 +56,6 @@ def valid_attrs2
     end
   end
 
-  test "can create lookup value if match fqdn= does match existing host" do
-    as_admin do
-      Setting[:append_domain_name_for_hosts] = false
-      domain = FactoryBot.create(:domain)
-      host = FactoryBot.create(:host, interfaces: [FactoryBot.build(:nic_managed, identifier: 'fqdn_test', primary: true, domain: domain)])
-      attrs = { :match => "fqdn=#{host.primary_interface.fqdn}", :value => "123", :lookup_key_id => lookup_key.id }
-      refute_match /#{domain.name}/, host.name, "#{host.name} shouldn't be FQDN"
-      assert_difference('LookupValue.count') do
-        LookupValue.create!(attrs)
-      end
-    end
-  end
-
   test "can create lookup value if user has matching hostgroup " do
     attrs = valid_attrs2 # create key outside as_user
     as_user :one do

test/unit/name_synchronizer_test.rb

@@ -27,7 +27,7 @@ def setup
 
   context "synchronizer build from host on shortnames" do
     before do
-      Setting[:append_domain_name_for_hosts] = false
+      Setting[:display_fqdn_for_hosts] = false
     end
     test "#sync_required? detects difference between names" do
       refute_equal @host.name, @host.primary_interface.shortname

webpack/assets/javascripts/react_app/components/HostDetails/index.js

@@ -43,6 +43,7 @@ import BreadcrumbBar from '../BreadcrumbBar';
 import { foremanUrl } from '../../common/helpers';
 import { CardExpansionContextWrapper } from './CardExpansionContext';
 import Head from '../Head';
+import { useForemanSettings } from '../../Root/Context/ForemanContext';
 
 const HostDetails = ({
   match: {
@@ -51,6 +52,7 @@ const HostDetails = ({
   location: { hash },
   history,
 }) => {
+  const { displayFqdnForHosts } = useForemanSettings();
   const { response, status } = useAPI(
     'get',
     `/api/hosts/${id}?show_hidden_parameters=true`,
@@ -112,7 +114,11 @@ const HostDetails = ({
               }}
               breadcrumbItems={[
                 { caption: __('Hosts'), url: foremanUrl('/hosts') },
-                { caption: response.name },
+                {
+                  caption: displayFqdnForHosts
+                    ? response.name
+                    : response.name?.replace(`.${response.domain_name}`, ''),
+                },
               ]}
             />
           )}
@@ -136,7 +142,12 @@ const HostDetails = ({
                           headingLevel="h5"
                           size="2xl"
                         >
-                          {response.name}
+                          {displayFqdnForHosts
+                            ? response.name
+                            : response.name?.replace(
+                                `.${response.domain_name}`,
+                                ''
+                              )}
                         </Title>
                       )}
                     </SkeletonLoader>

webpack/assets/javascripts/react_app/components/SettingRecords/__tests__/SettingRecords.fixtures.js

@@ -58,7 +58,7 @@ export const settings = [
     updatedAt: '2018-01-22 14:03:38 +0100',
     readonly: false,
     id: 177,
-    name: 'append_domain_name_for_hosts',
+    name: 'display_fqdn_for_hosts',
     fullName: 'Append domain names to the host',
     selectValues: null,
     value: true,
@@ -250,7 +250,7 @@ export const withHashSelection = settings.find(
   item => item.name === 'global_PXELinux'
 );
 export const boolSetting = settings.find(
-  item => item.name === 'append_domain_name_for_hosts'
+  item => item.name === 'display_fqdn_for_hosts'
 );
 export const arraySetting = settings.find(
   item => item.name === 'http_proxy_except_list'

webpack/assets/javascripts/react_app/components/SettingRecords/__tests__/__snapshots__/SettingRecordsReducer.test.js.snap

@@ -30,7 +30,7 @@ Object {
         "encrypted": false,
         "fullName": "Append domain names to the host",
         "id": 177,
-        "name": "append_domain_name_for_hosts",
+        "name": "display_fqdn_for_hosts",
         "readonly": false,
         "selectValues": null,
         "settingsType": "boolean",
@@ -296,7 +296,7 @@ Object {
         "encrypted": false,
         "fullName": "Append domain names to the host",
         "id": 177,
-        "name": "append_domain_name_for_hosts",
+        "name": "display_fqdn_for_hosts",
         "readonly": false,
         "selectValues": null,
         "settingsType": "boolean",

webpack/assets/javascripts/react_app/components/SettingRecords/__tests__/__snapshots__/SettingRecordsSelectors.test.js.snap

@@ -47,7 +47,7 @@ Object {
       "encrypted": false,
       "fullName": "Append domain names to the host",
       "id": 177,
-      "name": "append_domain_name_for_hosts",
+      "name": "display_fqdn_for_hosts",
       "readonly": false,
       "selectValues": null,
       "settingsType": "boolean",
@@ -299,7 +299,7 @@ Array [
     "encrypted": false,
     "fullName": "Append domain names to the host",
     "id": 177,
-    "name": "append_domain_name_for_hosts",
+    "name": "display_fqdn_for_hosts",
     "readonly": false,
     "selectValues": null,
     "settingsType": "boolean",

webpack/assets/javascripts/react_app/components/SettingsTable/__tests__/__snapshots__/SettingsTable.test.js.snap

@@ -88,7 +88,7 @@ exports[`SettingsTable should render 1`] = `
         "encrypted": false,
         "fullName": "Append domain names to the host",
         "id": 177,
-        "name": "append_domain_name_for_hosts",
+        "name": "display_fqdn_for_hosts",
         "readonly": false,
         "selectValues": null,
         "settingsType": "boolean",