Validate user credendials before trying to bootstrap. #209
Conversation
bootstrap.py
Outdated
| """ | ||
| sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) | ||
| sock.settimeout(2) | ||
| if sock.connect_ex((host, 8443)) == 0: |
There was a problem hiding this comment.
the if and else parts have identical code, so how about:
for port in [8443, 443]:
if sock.connect_ex((host, port)) == 0:
sock.close()
return str(port)
return None
bootstrap.py
Outdated
| so we need to disable SSL verification since it's now enabled | ||
| per default in RHEL7.4+. | ||
| """ | ||
| port = guess_api_port(options.foreman_fqdn) |
There was a problem hiding this comment.
instead of doing this here as a special case, why not using API_PORT = guess_api_port() in https://github.com/bergsjoh/katello-client-bootstrap/blob/d1a85da6d7cc9fc4c78bf56c1e325912a7cbca46/bootstrap.py#L851?
and then that should probably be moved after the argument parsing, as otherwise foreman_fqdn won't be defined yet.
bootstrap.py
Outdated
| sys.exit(2) | ||
|
|
||
|
|
||
| def guess_api_port(host): |
There was a problem hiding this comment.
the function returns an int, this will break the string concatenation above.
|
This looks good to me now. I did not run the code yet, though. Could you please rebase on the latest master changes? |
…'t have API_PORT setup globally yet.
|
Rebased. |
| @@ -1013,6 +1061,8 @@ def exec_service(service, command, failonerror=True): | |||
| # > Clean the environment from LD_... variables | |||
| clean_environment() | |||
|
|
|||
| validate_login() | |||
There was a problem hiding this comment.
Can you add a comment like # > Validate credentials, this will ease the generation of the flow as described in https://github.com/Katello/katello-client-bootstrap/blob/master/CONTRIBUTING.md#developer-and-contributor-notes
|
after discussion with @bergsjoh on IRC: this does not work as intended, as Katello/Satellite also has 8443 open (Candlepin). Going to have to hit |
|
Whoops, almost a year. Sorry I got busy about the time we realized that the first approach didn't work. Haven't looked into the alternate of using ping or so still. If you want to finish it up, feel free @beav. |
|
Please go ahead @sideangleside. Thx. |
|
@sideangleside when I last talked with @bergsjoh about this, we ended up in a dark corner: the credential verification has to happen before we install the So you'd need to do a Good luck :) |
| @@ -804,6 +836,20 @@ def exec_service(service, command, failonerror=True): | |||
| exec_failok("/sbin/service %s %s" % (service, command)) | |||
|
|
|||
|
|
|||
| def guess_api_port(host): | |||
There was a problem hiding this comment.
I think I'd go with the following version:
def guess_api_port(hostname):
"""Helper function to get the API port by probing"""
for port in [443, 8443]:
url = 'https://' + hostname + ':' + str(port) + '/katello/api/status'
try:
call_api(url, no_verify_ssl=True, silent=True, safe=True)
return str(port)
except: # noqa: E722, pylint:disable=bare-except
pass
return "443"the /katello/api/status endpoint doesn't require authentication and returns a JSON.
We just need to make call_api not to call sys.exit(1) if safe=True
Silently validate user credentials before throwing a an exception and print userfriendly errors msgs.