FS#27783 - add ipv6 hostname along with ipv4

Attached to Project: Release Engineering
Opened by Stefan Wilkens (stefanwilkens) - Saturday, 31 December 2011, 14:46 GMT
Last edited by Gerardo Exequiel Pozzi (djgera) - Monday, 26 November 2012, 04:42 GMT
Task Type Feature Request
Category AIF
Status Closed
Assigned To Dieter Plaetinck (Dieter_be)
Architecture All
Severity Very Low
Priority Normal
Reported Version 2011.08.19
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 1
Private No

Details

if a user changes HOSTNAME in /etc/rc.conf during installation, AIF will add this hostname to /etc/hosts during installation.

AIF only does this for ipv4, it does not for ipv6.

relevant code starts on line 99 in http://projects.archlinux.org/aif.git/tree/src/core/libs/lib-ui-interactive.sh.

I suggest we also add the user configured hostname for ipv6.
This task depends upon

Closed by  Gerardo Exequiel Pozzi (djgera)
Monday, 26 November 2012, 04:42 GMT
Reason for closing:  Deferred
Comment by Stefan Wilkens (stefanwilkens) - Thursday, 05 January 2012, 01:18 GMT
I matched the coding style, this should do for ipv6 what it did for ipv4.
Comment by Dieter Plaetinck (Dieter_be) - Thursday, 05 January 2012, 09:23 GMT
thanks, looks good (I am aware that the ipv4 code could be prettier though), but what could be up with this?:

$ git am 0001-add-HOSTNAME-to-ipv6.patch
Patch format detection failed.

btw, this is against the experimental branch, right?
Comment by Dieter Plaetinck (Dieter_be) - Thursday, 05 January 2012, 09:28 GMT
never mind, that was because it was not a real commit (with commit message etc), i'll apply and commit it, thanks.
Comment by Dieter Plaetinck (Dieter_be) - Thursday, 05 January 2012, 10:06 GMT
i cleaned up the code a bit and then created a commit from your patch, let me know how this looks:
https://github.com/Dieterbe/aif/commit/68b2c17d1c4205e39c0dd36856cfeb56050911ac
https://github.com/Dieterbe/aif/commit/51d3d8e1ec7c1d73814de504bd43e3fb5b497dce

now this just needs testing.
Comment by Stefan Wilkens (stefanwilkens) - Thursday, 05 January 2012, 10:15 GMT
looks like the top of the commit got lost, odd. the file attached is against master from here: http://projects.archlinux.org/aif.git (it was after making that diff that I was told to do a pull against your github)

either way, after pulling your develop and starting to make a diff against that tree I noticed that you just merged it so all is well.

I did think that you could do away with the second if statement and merge the sed line in with the ipv4 loop, but that would break if a user had managed to edit /etc/hosts before adding a different HOSTNAME to /etc/rc.conf. This takes up a few more lines, but is the secure option.
Comment by Dieter Plaetinck (Dieter_be) - Thursday, 05 January 2012, 10:29 GMT
yes, i wouldn't want to introduce even more obscurity and buggyness just to save a few lines.
in fact, this code is already a bit flakey, as the comment above that part explains.
Comment by Stefan Wilkens (stefanwilkens) - Thursday, 05 January 2012, 11:03 GMT
I don't see a decent way of solving the additional (invalid) hostnames that would pop up in /etc/hosts due to successive edits of rc.conf unless you're willing to take in more code. We could check against the current $HOSTNAME in rc.conf and the 'localhost' string in /etc/hosts and filter out anything else, or perhaps simply strip off anything after 'localhost' and add the current HOSTNAME by default. A few options, I'll see if I can come up with something elegant.

You might also consider sanity checking the $HOSTNAME value for spaces, this current code would cause $HOSTNAME="foo bar" to be added as such. giving us 2 hostnames in /etc/hosts we don't want. Or just mention that HOSTNAME can't have a space in the comment block above the value in rc.conf and tell users to RTFM ;)

the clean up looks fine, certainly better
Comment by Dieter Plaetinck (Dieter_be) - Thursday, 05 January 2012, 11:13 GMT
the problem is, the user should always be allowed to manually add hostnames ("aliases") in /etc/hosts, even behind AIF's back.
so if we remove anything from /etc/hosts, we should be sure he/she doesn't want it anymore. I guess checking the HOSTNAME before and after the user edits rc.conf through AIF, adding the new one, but also removing the old one, is sane behavior. (the user may want to keep the previous one as additional alias, but in that case he'll just need to re-add it to /etc/hosts manually, this seems an unlikely scenario anyway, in nearly all cases if the user changes the hostname in rc.conf he'll want the old entry to be replaced with the new one in /etc/hosts)

about the spaces, meh that should be common sense... although a patch that introduces a "spaces? WTF are you doing?" error would be accepted if it's elegant, i guess. but then were do you draw the line.. then we might start sanity checking _everything_
Comment by Stefan Wilkens (stefanwilkens) - Thursday, 05 January 2012, 11:46 GMT
Some thoughts on that:
Forcing a user to follow AIF's steps in a sane way seems to clash with allowing them to patch things up behind the scenes? Is it sane to build an installer that allows for user edits in the background, wouldn't user edits potentially break the installer completely? Just thinking out loud here.

I agree that 99.9% of cases would be a user intending to set one HOSTNAME in addition to 'localhost', but either changing his mind about the name chosen or noticing a typo. But we can't really stick it to the 0.1%, also considering that Arch doesn't dump user config as a policy.

Switching out the previous $HOSTNAME with the new $HOSTNAME is sane and would not touch any other user-set config in /etc/hosts, but it would have to avoid the removal of 'localhost'. Quite a bit of code but it would prevent hostname issues on the network.

So perhaps this solution:
1. always maintain 'localhost'
2. by default, switch the previously set $HOSTNAME with the newly set $HOSTNAME except if this means switching out 'localhost'. in that case add.
3. users can add additional host names through AIF's /etc/hosts option or behind the scenes, this config would not be touched if a user skips back to editing rc.conf after having edited /etc/hosts.

as for the sanity checking, we could also check for max string length and other crap. I only suggested to sanity check for spaces because this would break the networking so obviously. A one-liner check against a list of illegal characters wouldn't add much to the code but I see your point where we should then evaluate all the possible input for sanity.

Anyway, I think we've went off-topic enough here. I'll consider more patches after reading through the rest of the code and hopefully finding some elegant solutions. If not, rainy day well spent :)

cheers
Comment by Dieter Plaetinck (Dieter_be) - Thursday, 05 January 2012, 12:03 GMT
It seems the code for the "sane way" of removing can be much more elegant then you think (and then I previously thought),
anyway see  FS#24121  for the correct ticket and the patch i just made.

let's keep this ticket for the "ipv6 hostname must be added"
Comment by Tom Gundersen (tomegun) - Saturday, 10 March 2012, 11:40 GMT
A possibly simpler solution is to install nss-myhostname (currently in extra, maintained by yours truly, could be moved to core if you want it) by default and postfix 'myhostname' to 'hosts:' in nsswitch.conf. This means that no entries for the hostname needs to be entered into /etc/hosts, and they will still be resolved correctly.

See: http://0pointer.de/lennart/projects/nss-myhostname/

Loading...