-
Notifications
You must be signed in to change notification settings - Fork 28
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
Messaging Hostname Validation #254
Messaging Hostname Validation #254
Conversation
say("Checking if #{host} is resolvable ... ") | ||
begin | ||
ip_address = Resolv.getaddress(host) | ||
if ip_address == "127.0.0.1" || ip_address == "::1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably good enough, technically a link local address is a whole range of addrs 127.0.0.1/8
/ ::1/128
We could use IPAddr to do the subnet check, e.g.IPAddr.new("127.0.0.1/8").include?(IPAddr.new("127.0.1.2"))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if ip_address == "127.0.0.1" || ip_address == "::1" | |
if IPAddr.new("127.0.0.1/8").include?(ip_address) || IPAddr.new("::1/128").include?(ip_address) |
begin | ||
ip_address = Resolv.getaddress(host) | ||
if ip_address == "127.0.0.1" || ip_address == "::1" | ||
say("Failed.\nThe hostname must not resolve to 127.0.0.1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
say("Failed.\nThe hostname must not resolve to 127.0.0.1") | |
say("Failed.\nThe hostname must not resolve to a link-local address") |
9273bf6
to
5a3774c
Compare
updated specs, removing WIP |
5a3774c
to
a0c54e3
Compare
Checked commit nasark@a0c54e3 with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.51.0, and yamllint |
@@ -71,6 +72,11 @@ def ask_for_hostname(prompt, default = nil, validate = HOSTNAME_REGEXP, error_te | |||
just_ask(prompt, default, validate, error_text, &block) | |||
end | |||
|
|||
def ask_for_messaging_hostname(prompt, default = nil, error_text = "a valid Messaging Hostname (not an IP or localhost)", &block) | |||
validation = ->(h) { h =~ MESSAGING_HOSTNAME_REGEXP && h !~ IP_REGEXP } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm I wonder if instead of mostly duplicating the hostname regex we just say h =~ HOSTNAME_REGEXP && h !~ IP_REGEXP && h != "localhost"
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a bad idea, the only difference I see is that the regex detects instances of localhost
anywhere in the string. i.e. a hostname that has .localhost
at the end (or anywhere), another example is the infamous localhost.localadmin
Added - Add a common method for asking for a password [#251] - Add messaging hostname validation [#254] - Indicate that messaging persistent disk is optional [#256] - Add messaging password validation [#255] Changed - Deprecate message-server-use-ipaddr option from cli [#257] Fixed - Add ca-cert to messaging client installed_files [#258]
@miq-bot assign @agrare
@miq-bot add_reviewer @agrare
@miq-bot add_label enhancement