Bug 6144 - People often forget to set agent_hostname
Summary: People often forget to set agent_hostname
Status: CLOSED FIXED
Alias: None
Product: ThinLinc
Classification: Unclassified
Component: Server Installer (show other bugs)
Version: trunk
Hardware: PC Unknown
: P2 Normal
Target Milestone: 4.18.0
Assignee: Emelie
URL:
Keywords: adaha_tester, linma_tester, relnotes
: 4228 7975 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-01-19 13:57 CET by Samuel Mannehed
Modified: 2024-12-12 17:26 CET (History)
10 users (show)

See Also:
Acceptance Criteria:
MUST * The admin must be informed that agent_hostname needs to be set if ThinLinc is to be accessible from the outside, if NAT is enabled. SHOULD * ThinLinc should offer a way to set agent_hostname during the setup. - This should be available both in GUI and text-mode. - Unattended setup (with answer file) should support it. * ThinLinc setup should be easy to navigate using only a keyboard (TAB, arrow keys, etc.) COULD * The admin could ideally not be pestered if agent_hostname is already set. * Leading and trailing whitespace could be stripped from the string that the admin inputs. * The hostname could be validated in ThinLinc setup to assist the admin to set a sane value. * ThinLinc setup could suggest a probable value for agent_hostname.


Attachments
External connections page with the "big box" selection for IP address (132.68 KB, image/png)
2024-12-03 17:45 CET, Linn
Details
External connections page with the "small box" selection for IP address (130.23 KB, image/png)
2024-12-03 17:46 CET, Linn
Details

Description Samuel Mannehed cendio 2017-01-19 13:57:23 CET
One solution could be to offer to set this in tl-setup when choosing agent.
Comment 1 Samuel Mannehed cendio 2017-01-24 13:21:35 CET
Also see bug 2561.
Comment 2 Samuel Mannehed cendio 2021-11-23 13:04:56 CET
One consequence of this is that clients can't reach the agent from outside the LAN.
Comment 3 Pierre Ossman cendio 2022-06-07 16:31:03 CEST
*** Bug 4228 has been marked as a duplicate of this bug. ***
Comment 4 Pierre Ossman cendio 2022-08-15 11:22:51 CEST
*** Bug 7975 has been marked as a duplicate of this bug. ***
Comment 8 Pierre Ossman cendio 2024-02-06 16:24:38 CET
Ideas/goals of what we'd like to improve here:

The basics:

 * Inform the user about the issue, using a new page in tl-setup

 * Allow the user to configure agent_hostname directly from tl-setup

Rudimentary suggestions:

 * The machine's current ip address (just the internet facing one, or all?)

 * The machine's hostname

Rudimentary verification of suggestions, warning users if something seems unlikely to work:

 * Check if the IP address is in a public or private range

 * Check if the hostname looks like a fqdn

Once those things are in place, we could attempt more complex systems that are technically uncertain, or require external services:

 * A reverse DNS lookup of the machine's IP address, hopefully getting something that is more universally useful

 * Assuming tl-setup was started via ssh, can we get anything from ssh that tells how the machine is reached externally?

 * Contact some service on the internet and ask what our external address is

 * Query the infrastructure we're on/in, e.g. some AWS API if we're a VM, UPnP, ...

 * Look up candidate host names in DNS and check if the result is in a public or private range

 * Do the above DNS lookup with an external DNS (e.g. Google's) to get an external perspective

 * Is the hostname using a valid TLD?

 * Request an external service to attempt a connection to our candidate address

 * Ask the user to run some command from a client to attempt a connection to our candidate address
Comment 9 Pierre Ossman cendio 2024-03-21 10:04:33 CET
(In reply to Pierre Ossman from comment #8)
> 
>  * Query the infrastructure we're on/in, e.g. some AWS API if we're a VM,
> UPnP, ...
> 

Getting information from AWS seems to be trivial at least:

https://stackoverflow.com/questions/38679346/get-public-ip-address-on-current-ec2-instance
Comment 48 Emelie cendio 2024-11-26 09:21:05 CET
A new page "External connections" was added to tl-setup. The admin will be presented with three choices, none of which are selected by default. The options are "ip", "hostname" and "manual". Depending on the choice, the configuration variable 'agent_hostname' will be set. For unattended tl-setup, two new variables were added to the answers file.

The text in tl-setup avoids being too technical, by avoiding terms like 'NAT' and 'clusters'. We believe that we were able to be clear and simple by keeping the context within the current machine.

In Web Administation, 'agent_hostname' could be set already, but this section was modified to be more similar to tl-setup with updated text and options.

This was tested on Fedora 41.

Left to do:
* Open bugs for remaining work on the subject
* Update the TAG
Comment 52 Emelie cendio 2024-11-27 10:35:47 CET
Found an issue on RHEL9:

> 2024-11-27 10:28:44,766: Traceback (most recent call last):
> 2024-11-27 10:28:44,766:   File "/opt/thinlinc/sbin/../libexec/tl-setup.py", line 329, in <module>
> 2024-11-27 10:28:44,766:     oOoO00 ( )
> 2024-11-27 10:28:44,766:   File "/opt/thinlinc/sbin/../libexec/tl-setup.py", line 285, in oOoO00
> 2024-11-27 10:28:44,766:     O0 = ooOo00oOo0Ooo . run ( )
> 2024-11-27 10:28:44,766:   File "/opt/thinlinc/modules/thinlinc/wizard.py", line 186, in run
> 2024-11-27 10:28:44,766:     return self . _run_gui ( )
> 2024-11-27 10:28:44,766:   File "/opt/thinlinc/modules/thinlinc/wizard.py", line 220, in _run_gui
> 2024-11-27 10:28:44,766:     iiiiII11II ( ooo )
> 2024-11-27 10:28:44,766:   File "/opt/thinlinc/modules/thinlinc/tlsetup/external_connections.py", line 178, in I11I1iI
> 2024-11-27 10:28:44,766:     IiI11II11II11 . load_from_data (
> 2024-11-27 10:28:44,766: TypeError: Item 0: Must be number, not str
Comment 53 Emelie cendio 2024-11-27 10:37:27 CET
When not running tl-setup for the first time, we usually suggest continuing using the previous answer. This is not how the external connections page behaves right now, you are always forced to choose.
Comment 57 Emelie cendio 2024-11-27 10:50:27 CET
The issues in comment 53 and comment 54 are fixed!
Comment 58 Emelie cendio 2024-11-27 10:53:52 CET
> MUST
> * The admin must be informed that agent_hostname needs to be set if ThinLinc is to be accessible from the outside, if NAT is enabled.
Yes, a new page was added that explains this need and links to the TAG.

> SHOULD
> * ThinLinc should offer a way to set agent_hostname during the setup.
>   - This should be available both in GUI and text-mode.
>   - Unattended setup (with answer file) should support it.
Yes!

> COULD
> * The admin could ideally not be pestered if agent_hostname is already set.
No, we always show the new page in case the admin wants to change the hostname.
> * Leading and trailing whitespace could be stripped from the string that the admin inputs.
Yes
> * The hostname could be validated in ThinLinc setup to assist the admin to set a sane value.
No, this was split to bug 8459
> * ThinLinc setup could suggest a probable value for agent_hostname.
No, this was split to bug 8460
Comment 63 Samuel Mannehed cendio 2024-11-28 10:43:47 CET
A few minor changes were committed today and yesterday. They do not affect the main function in any meaningful way.

Regarding r41410, an acceptance criteria was added for keyboard navigation:
> * ThinLinc setup should be easy to navigate using only a keyboard (TAB, arrow keys, etc.)
Yes, it is now.
Comment 64 Linn cendio 2024-11-29 16:50:15 CET
Looked through the commits and things looked fine.

The majority of the changes are limited to tl-setup. Files installer/tlinstaller and tlmisc/profiles/modules/thinlinc/profiles/chooser.py are also affected, but only through commit r41408 which does a small restruction for function linkify_text(). These files should be tested implicitly when doing the larger testing for tl-setup, so no special tests for these files are needed.

When choosing the "IP adress" option, agent_hostname is set to "", which means this option works the same way as the deafult behaviour before starting this bug. When choosing option "Hostname", the hostname is retrieved the same way regardless of platform (by socket.getfqdn()), so no platform-specific testing is needed there.
Comment 65 Tobias cendio 2024-12-03 08:37:49 CET
Getting deprecration warning on RHEL8:

/opt/thinlinc/modules/thinlinc/tlsetup/external_connections.py:242: DeprecationWarning: Gtk.Button.set_focus_on_click is deprecated
  iiII . set_focus_on_click ( False )

Seems like the docs [1] are suggesting Gtk.Widget.set_focus_on_click().

[1] https://docs.gtk.org/gtk3/method.Button.set_focus_on_click.html
Comment 66 Pierre Ossman cendio 2024-12-03 10:30:11 CET
Found an issue with the order of the new module:

It is run before migrating configuration. Which means it might be reacting to incorrect hiveconf settings.

It needs to be moved to a later position.
Comment 67 Pierre Ossman cendio 2024-12-03 10:32:16 CET
The keyboard behaviour of tlwebadm is also a bit surprising. If you simply tab between elements, then it will modify your settings.

The issue is that it modifies the radio buttons just because the input box gets focus. This is very surprising as I would expect tab:ing to be a read-only operation.

The issue does not happen in tl-setup as the input box does not seem to be a tab stop.
Comment 76 Linn cendio 2024-12-03 14:55:11 CET
To summarise, the following things have been found since this bug was set to resolved:

 1) A deprecation warning is seen on RHEL 8 (comment 65).

 2) Module "External connections" is placed before migrating the conf (comment 66)
   
 3) In Web Admin, tabbing to certain elements caused that setting to be selected
    (comment 67).
   - However, we probably want to change selection to "Manually selected" when the
     user types inside the input field.

 4) Move handling for _setup_never_completed() to shared code

 5) Don't show agent_hostname from previous terminal input

Except for the addition on item 3) regarding the input field, all these things have been taken care of in the last rounds of commits.
Comment 78 Linn cendio 2024-12-03 17:44:12 CET
Found an issue regarding the tabbing in the External connections page. Seen on RHEL 8.

When running tl-setup the first time and all radio buttons are unselected, the first radio button cannot be selected by keyboard. If the radio button is selected by the "big box", pressing enter changes selection to a "small box" (see screenshots). If the radio button is selected by the "small box", pressing enter triggers an animation on the button, but the button does not get selected.

If I instead tab to another radio button and press enter, that entry is selected. If I then tab back to the first radio button and press enter, it is now selected correctly. Also, note that I can only tab to the "big box" after selecting another entry - the "small box" seems to only be available for a selected entry.

The page for migrating config has the same visual issue with a "big box" and a "small box", but here the first element can be chosen by pressing enter.
Comment 79 Linn cendio 2024-12-03 17:45:39 CET
Created attachment 1248 [details]
External connections page with the "big box" selection for IP address
Comment 80 Linn cendio 2024-12-03 17:46:05 CET
Created attachment 1249 [details]
External connections page with the "small box" selection for IP address
Comment 81 Tobias cendio 2024-12-04 16:34:30 CET
Tested server build #3858 on RHEL8 and Ubuntu22.04.

Concerning comment #65:

This deprecation warning is a bit strange, since Gtk.Button inherits Gtk.Widget, but calling Gtk.Button.set_focus_on_click() is deprecated. Calling the parent method works but will mar the readability, so a decision was made to silence the warning. When this Gtk.Button method is removed, the call will still be valid as the desired Gtk.Widget parent method will be called instead.

Can confirm that the warning is no longer visible. For some reason, it was never visible on Ubuntu22.04, even before the fix.
Comment 84 Linn cendio 2024-12-05 13:47:21 CET
Found an issue when running tl-setup with answer files.

When selecting manual input in the answer file (agent-hostname-choice=manual) and not specifying what the hostname should be, tl-setup did not stop to ask what the hostname should be. Instead, it just continued, resulting in the hostname keeping the same value as before, since the config file was not updated.

If the hostname was set in the answer file (e.g. manual-agent-hostname=test), tl-setup was setting the value correctly in the config file.
Comment 85 Linn cendio 2024-12-05 14:12:46 CET
Comment 64 is now fixed. The issue was that we exited too early when using an answer file, so we never got to the place where input is asked for.
Comment 87 Linn cendio 2024-12-06 15:44:05 CET
The issue with unselectable radio buttons mentioned in comment 78 has now been fixed. On the first run of tl-setup GUI, when all radio buttons are unselected, the first one now reacts as expected when selected by keyboard. Also tested clicking on the radio buttons, which works as well. 

Found one more cosmetic issues when testing regarding the focus frame default position - see bug 8475.


This means that all issues have been address. Marking as resolved.
Comment 89 Linn cendio 2024-12-06 17:32:45 CET
Found another issue with the input field for the manual radio button.

In a regular input field, you get a text cursor when hovering (the slim one that looks like "I"). You can also click somewhere on the text to move the marker to that position.

In the input field for setting a manual agent hostname, the cursor is a regular arrow, and the marker does not move when it's clicked. However, it is possible to navigate in the input field with the keyboard.

There is a difference between different distributions for what happens when the input field is clicked. On RHEL 9, the input field loses focus when clicked - the dotted "focus frame" covers the radio button, and it does not respond to keyboard input. On Fedora 40, the input field keeps focus and keeps responding to keyboard input, but does not change the position of the marker.

We have a working input field in tl-setup on the page for entering the admin's email address. However, it only uses built-in defaults, so it doesn't give much guidance in this case.

Let's talk about this next week and see how to go forward with it.
Comment 90 Adam Halim cendio 2024-12-09 09:37:25 CET
Tested server build #3862 running tl-setup on Ubuntu 24.10 Desktop and got the following error when installing postfix with the graphical installer:
> 2024-12-09 09:29:06,951: After modifying main.cf, be sure to run 'systemctl reload postfix'.
> 2024-12-09 09:29:06,951:
> 2024-12-09 09:29:06,951: Running newaliases
> 2024-12-09 09:29:06,951: newaliases: warning: valid_hostname: numeric hostname: 10
> 2024-12-09 09:29:06,951: newaliases: fatal: file /etc/postfix/main.cf: parameter mydomain: bad parameter value: 10
> 2024-12-09 09:29:06,952: dpkg: error processing package postfix (--configure):
> 2024-12-09 09:29:06,952: installed postfix package post-installation script subprocess returned error exit status 75
> 2024-12-09 09:29:06,952: Processing triggers for ufw (0.36.2-6) ...
> 2024-12-09 09:29:06,952: Processing triggers for man-db (2.12.1-3) ...
> 2024-12-09 09:29:06,952: Processing triggers for rsyslog (8.2406.0-1ubuntu2) ...
> 2024-12-09 09:29:06,952: Errors were encountered while processing:
> 2024-12-09 09:29:06,952: postfix
Comment 91 Adam Halim cendio 2024-12-09 09:41:43 CET
(In reply to Adam Halim from comment #90)

Looks like our hostname was set to "ubuntu-24.10", and the '.' breaks stuff. Simply removing the '.' from the hostname, or from the conf, fixes the issue.
Comment 92 Linn cendio 2024-12-09 17:17:00 CET
(In reply to Linn from comment #89)
> Found another issue with the input field for the manual radio button.
We want to make sure the input field behaves like a regular Gtk.Entry, reopening.
Comment 96 Linn cendio 2024-12-12 10:37:21 CET
The issue with the input field from comment 89 has now been fixed.

We moved the label and input field to be side by side in the radio button element, instead of the input field being inside the label element. This move caused the input field to behave more like a regular input field, where click events were recieved.

To keep the behaviour of selecting the "Manual" radio button when typing or clicking inside the input field, a few event handlers had to be tweaked. Due to this, we had to move the call for setting hostname in the input field to avoid "Manual" always being selected.

Tested the following on RHEL 8 (GTK v. 3.22.30), RHEL 9 (GTK v. 3.24.31), Ubuntu 22.04 (GTK v. 3.24.33) and Ubuntu 24.10 (GTK v. 3.24.43). The tabbing and clicking were tested both on the first tl-setup run (where all radio buttons are unselected) and a run after tl-setup had completed.

 * Tab to the radio button and select it by pressing space
 * Typing in input field selectes "Manual" radio button
 * Click on the radio button to select it
 * Clicking in input field selectes "Manual" radio button
 * The corresponding vaule is set in the config file

That was the last remaining issue, marking as Resolved.
Comment 97 Linn cendio 2024-12-12 12:59:48 CET
Tested text mode install and tl-setup, as well as Web Admin with server build 3857 on Ubuntu 24.04.

tl-setup text mode
------------------
When tl-setup has not been run before, none of the options for external connections is pre-selected. The admin has to choose an option to continue with tl-setup.

The corresponding value on /vsmagent/agent_hostname is set in the config-file:
 * IP address:  agent_hostname=  (empty string)
 * Hostname:   agent_hostname=<DNS name> (e.g. domain.example.com)
 * Manually specified:  agent_hostname=<input from input field> 

When tl-setup has been completed once and tl-setup runs again, the previously selected option is pre-selected. Note that the value of agent_hostname determines which option is pre-selected. If the value is the empty string, IP is pre-selected, and if the value is the suggested DNS name, Hostname is pre-selected.

Also tested running tl-setup with an answer file. The file has two new variables, agent-hostname-choice and manual-agent-hostname. Tested all values for agent-hostname-choice (ip, hostname and manual). When manual is selected, the value from manual-agent-hostname is used if it exists. If not, tl-setup either stops to ask for it or aborts, depending on the value in missing-answer.

Web Admin
---------
On the VSM -> Agent page, the current value of agent_hostname is visible in the input box for Manual, and the corresponding radio button is chosen. The corresponding value on /vsmagent/agent_hostname is set in the configfile when saving the page, in the same way as tl-setup text mode.

The input box does not accept an empty value, as it will give the error "No agent hostname set.". All other values seem to be accepted. 

Radio buttons can be selected both with clicking and tabbing. Typing inside the input box or clicking on it selects the radio button for "Manual".

Error handling
--------------
I tried removing vsmagent.hconf. Tl-setup works when this doesn't exist, as it just creates the variable /vsmagent/agent_hostname in another hconf-file. Web Admin on the other hand crashes, since it expects the folder /vsmagent to exist. This behaviour was present before doing this bug, so it's not a regression.

I also tried setting weird values on agent_hostname, such as unicode characters or "-1". No errors occured. 


Also read the documentation and the release notes, looks good.
Comment 98 Alexander Zeijlon cendio 2024-12-12 16:35:34 CET
I have taken a look at the latest changes, more specifically, those related to tl-setup running in GTK-mode.

Testing of the changes were done on:
  RHEL 8
  RHEL 9
  Ubuntu 24.04

After the latest updates, I see consistent behavior when using both tab-key and mouse for moving between and selecting the three radio buttons in the installer section that handles external reachability.

All radio buttons are togglable.

The special case is the radio button for entering hostname manually, which has behavior for selecting the input in different situations. In most situations, the association is clear between the radio button and its input, regardless of which one of them is interacted with.

There is however still some weird behavior, but it is only visual. If you tab through the options and there is already data in the input. It gets highlighted/selected, and remains highlighted even if one of the two other radio buttons are eventually toggled. To me, this makes it look a bit ambiguous as to what will be the resulting setting.

In the end, the radio button selection governs what option is chosen. E.g. having the ip radio toggled, and then using tab to move focus to the input and then clicking enter results in the toggled button being the chosen option. And this, at least to me, is the correct behavior.
Comment 99 Linn cendio 2024-12-12 17:26:59 CET
Tested the acceptance criteria:

> MUST
>  ✅ The admin must be informed that agent_hostname needs to be set if ThinLinc is 
> to be accessible from the outside, if NAT is enabled.
Yes, NAT is not mentioned explicitly, but the text in tl-setup informs that a public address has to be used for the agent to be accessible from all clients.

> SHOULD
> ✅ ThinLinc should offer a way to set agent_hostname during the setup.
>   ✅ This should be available both in GUI and text-mode.
Yes, both the GUI and text-mode have a new section for "External connections" which enables setting agent_hostname to IP, hostname or something manually specified. The first time tl-setup is run after install, the admin have to decide which option to choose, i.e. this page can't be skipped. When tl-setup is rerun, the previously chosen option will be the pre-selected option. 

>   ✅ Unattended setup (with answer file) should support it.
Yes, tested all combinations options for the answer file - ip, hostname and manual with and without an input specified.
  
> ✅ ThinLinc setup should be easy to navigate using only a keyboard (TAB, arrow keys, etc.)
The text mode have this functionality by default, and the GUI can also be navigated using only keyboard.

> COULD
> ❌ The admin could ideally not be pestered if agent_hostname is already set.
No, the page is always showed. This was done to give the admin an opportunity to change the hostname, like other configuration set in tl-setup.

> ✅ Leading and trailing whitespace could be stripped from the string that the 
> admin inputs.
It is.

> ❌ The hostname could be validated in ThinLinc setup to assist the admin to set a sane value.
> ❌ ThinLinc setup could suggest a probable value for agent_hostname
These were split out to bug 8459 and bug 8460 respectively.

Note You need to log in before you can comment on or make changes to this bug.