Alon Bar-Lev has posted comments on this change.
Change subject: packaging: setup: added FQDN validation to engine-setup-2
......................................................................
Patch Set 8: (10 inline comments)
Thanks!
Few more comments...
....................................................
File packaging/setup/plugins/ovirt-engine-setup/config/protocols.py
Line 166: resolved = True
Line 167: return resolved
Line 168:
Line 169: def _dig_reverse_lookup(self, addr):
Line 170: addresses = set()
names ?
Line 171: args = [
Line 172: self.command.get('dig'),
Line 173: '-x',
Line 174: addr,
Line 190: #get set of IPs
Line 191: ipAddresses = self._getConfiguredIps()
Line 192: #resolve fqdn
Line 193: try:
Line 194: resolvedAddresses = set([
Please debug the result so we can compare it to dig output.
Line 195: address[0] for __, __, __, __, address in
Line 196: socket.getaddrinfo(
Line 197: fqdn,
Line 198: None
Line 206: )
Line 207: )
Line 208:
Line 209: prettyString = ' '.join(
Line 210: [str(addr) for addr in resolvedAddresses]
Why do you need str here?
Line 211: )
Line 212:
Line 213: #compare found IP with list of local IPs and match.
Line 214: if not resolvedAddresses.issubset(ipAddresses):
Line 213: #compare found IP with list of local IPs and match.
Line 214: if not resolvedAddresses.issubset(ipAddresses):
Line 215: raise RuntimeError(
Line 216: _(
Line 217: "The following address(es): {addresses} can't be
mapped "
The following addresses resolved from xxx - so it will be clear where did we
get these ips...
Line 218: 'to non loopback devices on this host'
Line 219: ).format(
Line 220: addresses=prettyString
Line 221: )
Line 233: else:
Line 234: #reverse resolved IP and compare with given fqdn
Line 235: revResolved = False
Line 236: for address in resolvedAddresses:
Line 237: addressSet = self._dig_reverse_lookup(address)
reverseNames?
1. This is a name not address, right?
2. The fact that it is set is not really important...
Line 238: if len(addressSet) > 0:
Line 239: for reResolvedAddress in addressSet:
Line 240: if reResolvedAddress.lower() == fqdn.lower():
Line 241: revResolved = True
Line 234: #reverse resolved IP and compare with given fqdn
Line 235: revResolved = False
Line 236: for address in resolvedAddresses:
Line 237: addressSet = self._dig_reverse_lookup(address)
Line 238: if len(addressSet) > 0:
if reverseNames:
Line 239: for reResolvedAddress in addressSet:
Line 240: if reResolvedAddress.lower() == fqdn.lower():
Line 241: revResolved = True
Line 242: else:
Line 235: revResolved = False
Line 236: for address in resolvedAddresses:
Line 237: addressSet = self._dig_reverse_lookup(address)
Line 238: if len(addressSet) > 0:
Line 239: for reResolvedAddress in addressSet:
for name in reverseNames?
If you perform the for, you do not need the above condition...
Line 240: if reResolvedAddress.lower() == fqdn.lower():
Line 241: revResolved = True
Line 242: else:
Line 243: self.logger.warning(
Line 236: for address in resolvedAddresses:
Line 237: addressSet = self._dig_reverse_lookup(address)
Line 238: if len(addressSet) > 0:
Line 239: for reResolvedAddress in addressSet:
Line 240: if reResolvedAddress.lower() == fqdn.lower():
?
revResolved = reResolvedAddress.lower() == fqdn.lower()
if revResolved:
break
Line 241: revResolved = True
Line 242: else:
Line 243: self.logger.warning(
Line 244: _(
Line 239: for reResolvedAddress in addressSet:
Line 240: if reResolvedAddress.lower() == fqdn.lower():
Line 241: revResolved = True
Line 242: else:
Line 243: self.logger.warning(
Move this outside of loop
Line 244: _(
Line 245: '{address} did not
reverse-resolve '
Line 246: 'into {fqdn}'
Line 247: ).format(
Line 249: fqdn=fqdn,
Line 250: )
Line 251: )
Line 252: if not revResolved:
Line 253: raise RuntimeError(
Hmmm... you fail if no resolve, so why warn on specific address above? maybe it
should be debug... but in debug log we do have the output of dig.
Line 254: _(
Line 255: 'The following addresses: {addresses} did not
reverse'
Line 256: 'resolve into {fqdn}'
Line 257: ).format(
--
To view, visit http://gerrit.ovirt.org/14699
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9764f32ba5e30062532bf11c67756677333c44f
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Alex Lourie <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Ofer Schreiber <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Yedidyah Bar David <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
