Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #36160 - Redefine append domain names setting #9613

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions app/assets/javascripts/host_edit_interfaces.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,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

Expand Down
9 changes: 9 additions & 0 deletions app/models/host/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ def dup
:domain=, :domain_id=, :domain_name=, :to => :primary_interface

attr_writer :updated_virtuals

Dyrkon marked this conversation as resolved.
Show resolved Hide resolved
def updated_virtuals
@updated_virtuals ||= []
end
Expand Down Expand Up @@ -351,6 +352,14 @@ def render_template(template:, **params)
template.render(host: self, **params)
end

def to_label
Dyrkon marked this conversation as resolved.
Show resolved Hide resolved
if Setting[:display_fqdn_for_hosts]
name
else
name.split('.')[0]
end
end

private

def parse_ip_address(address, ignore_link_local: true)
Expand Down
6 changes: 3 additions & 3 deletions app/registries/foreman/settings/general.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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_('Display names of hosts as FQDNs. If disabled, only display names of hosts as 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. ' \
Expand Down
2 changes: 1 addition & 1 deletion app/services/name_synchronizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 4 additions & 0 deletions app/views/api/v2/hosts/base.json.rabl
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
object @host

attributes :name, :id

stejskalleos marked this conversation as resolved.
Show resolved Hide resolved
node :display_name do |host|
host.to_label
end
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion app/views/dashboard/_new_hosts_widget_host_list.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion app/views/hosts/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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? %>
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20230414091257_rename_append_domain_setting.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class RenameAppendDomainSetting < ActiveRecord::Migration[6.1]
def change
Setting.find_by(name: 'append_domain_name_for_hosts')&.update_attribute(:name, 'display_fqdn_for_hosts')
end
end
9 changes: 9 additions & 0 deletions db/migrate/20230418075940_assign_fqdn_to_host_name.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class AssignFqdnToHostName < ActiveRecord::Migration[6.1]
def up
Host.find_in_batches(batch_size: 1000) do |hosts|
hosts.each do |host|
host.update_attribute(:name, host.fqdn)
end
end
end
end
10 changes: 5 additions & 5 deletions test/controllers/api/v2/settings_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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']
Expand Down
4 changes: 2 additions & 2 deletions test/integration/host_js_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
13 changes: 0 additions & 13 deletions test/models/lookup_value_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/unit/name_synchronizer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ import Slot from '../../../../../common/Slot';
import SkeletonLoader from '../../../../../common/SkeletonLoader';
import DefaultLoaderEmptyState from '../../../../DetailsCard/DefaultLoaderEmptyState';
import { STATUS } from '../../../../../../constants';
import { useForemanSettings } from '../../../../../../Root/Context/ForemanContext';

const SystemPropertiesCard = ({ status, hostDetails }) => {
const { displayFqdnForHosts } = useForemanSettings();
const {
name,
model_name: model,
Expand Down Expand Up @@ -48,7 +50,7 @@ const SystemPropertiesCard = ({ status, hostDetails }) => {
hoverTip={__('Copy to clipboard')}
clickTip={__('Copied to clipboard')}
>
{name}
{displayFqdnForHosts ? name : name?.replace(`.${domain}`, '')}
</ClipboardCopy>
)}
</SkeletonLoader>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -51,6 +52,7 @@ const HostDetails = ({
location: { hash },
history,
}) => {
const { displayFqdnForHosts } = useForemanSettings();
const { response, status } = useAPI(
'get',
`/api/hosts/${id}?show_hidden_parameters=true`,
Expand Down Expand Up @@ -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}`, ''),
},
]}
/>
)}
Expand All @@ -136,7 +142,12 @@ const HostDetails = ({
headingLevel="h5"
size="2xl"
>
{response.name}
{displayFqdnForHosts
? response.name
: response.name?.replace(
`.${response.domain_name}`,
''
)}
</Title>
)}
</SkeletonLoader>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down