Katello / katello-packaging

[DEPRECATED] Packaging for Katello
7 stars 33 forks source link

Fixes #20799 - add valid fqdn check for kchange-hostname #516

Closed johnpmitsch closed 7 years ago

theforeman-bot commented 7 years ago

Issues: #20799

johnpmitsch commented 7 years ago

To test

spin up a katello 3.4 production box and run:

yum install -y git vim
cd ~
git clone https://github.com/Katello/katello-packaging
cd katello-packaging
git remote add fork https://github.com/johnpmitsch/katello-packaging
git fetch fork
git checkout fork/invalid_hostname
cd ~
ln -s katello-packaging/katello/katello-change-hostname katello-change-hostname
chmod a+x katello-change-hostname 

then try a variety of invalid hostnames, i.e shouldntwork.$$. Make sure they don't work and you see the correct error message and the script exits. Here is how to use the command: katello-change-hostname -u admin -p changeme -yd HOSTNAME

Then make sure a a real hostname works using the above command.

Please test any edge cases you can think of. Ping me with any questions

jturel commented 7 years ago

I think this is working as you intended it to. Here are my observations:

irb(main):011:0> hostname_regex === 'example.com' => true

irb(main):012:0> hostname_regex === 'example.COM' => false

irb(main):013:0> hostname_regex === 'localhost' => false

Would you expect the last 2 to work? Also, do you think we should provide more guidelines around what constitutes a valid hostname upon failure? Not sure what our usual level of hand-holding is.

johnpmitsch commented 7 years ago

@jturel The regex follows (for the most part) these specifications which assumes

TLD is at least 2 characters and only a-z we want at least 1 level above TLD

so that would exclude localhost and example.COM

I'll change the error message to say a fully qualified domain name is required, which is more accurate for the user

johnpmitsch commented 7 years ago

@jturel updated the error message