ipa-server-install, ipa-server-upgrade fixes #460
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| # The SafeConfigParser class has been renamed to ConfigParser in Py3 | ||
| from configparser import ConfigParser as SafeConfigParser | ||
| else: | ||
| from ConfigParser import SafeConfigParser | ||
| # pylint: enable=import-error |
There was a problem hiding this comment.
Are the pylint pragmas still needed?
There was a problem hiding this comment.
Yes, still needed for pylint-3
| @@ -283,8 +283,7 @@ def __setup_replica_keys(self): | |||
| while True: | |||
| # check if key with this ID exist in softHSM | |||
| # id is 16 Bytes long | |||
| key_id = "".join(chr(random.randint(0, 255)) | |||
| for _ in range(0, 16)) | |||
| key_id = bytes(random.randint(0, 255) for _ in range(0, 16)) | |||
There was a problem hiding this comment.
$ python2 -c 'import random; print(repr(bytes(random.randint(0, 255) for _ in range(0, 16))))'
'<generator object <genexpr> at 0x7f872517f730>'
$ python3 -c 'import random; print(repr(bytes(random.randint(0, 255) for _ in range(0, 16))))'
b'\x11\xd9\xd9\x9c\x98U\xb8w\x17\x8ar\xd6\xc6\x91\xe8\xb4'
MartinBasti
changed the title
stlaz
reviewed
May 29, 2017
| """ | ||
| return struct.pack( | ||
| "B" * key_id_len, # key_id must be bytes | ||
| *(random.randint(0, 255) for _ in range(key_id_len)) |
There was a problem hiding this comment.
You should use ipautil.ipa_generate_password() here instead.
There was a problem hiding this comment.
It is not password, but ID
There was a problem hiding this comment.
Ah, nvm, saw key which triggered a reaction. LGTM then, need to test it.
|
|
stlaz
reviewed
May 31, 2017
ipaserver/install/installutils.py
Outdated
| @@ -744,10 +749,9 @@ def read_replica_info(dir_path, rconfig): | |||
|
|
|||
| rconfig is a ReplicaConfig object | |||
| """ | |||
| filename = dir_path + "/realm_info" | |||
| fd = open(filename) | |||
| filename = os.path.join(dir_path, "/realm_info") | |||
There was a problem hiding this comment.
One shall not os.path.join() with / (causes ipa-replica-install on DL0 to fail)
DeprecationWarning: The SafeConfigParser class has been renamed to ConfigParser in Python 3.2. This alias will be removed in future versions. Use ConfigParser directly instead. https://fedorahosted.org/freeipa/ticket/4985
ConfigParser.readfd() is deprecated in py3, we can use .read() which is compatible with py2 https://fedorahosted.org/freeipa/ticket/4985
Python LDAP requires bytes https://fedorahosted.org/freeipa/ticket/4985
softhsm works with bytes, so key_id must be byte otherwise we get errors from bytes and string comparison https://fedorahosted.org/freeipa/ticket/4985
Functions mix unicode and bytes, use only bytes. https://fedorahosted.org/freeipa/ticket/4985
str() was called on bytes https://fedorahosted.org/freeipa/ticket/4985
with py3 urlopen used internally with pyldap doesn't work with raw filepaths without specifying "file://" prefix. This works on both py2/py3 https://fedorahosted.org/freeipa/ticket/4985
Py3 doesn't support ordering with None value https://fedorahosted.org/freeipa/ticket/4985
|
ACK, I did a sanity testing, if something's wrong, some kind of our CI will hopefully tell us. |
|
master:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ipa-server-install --setup-dns now work without BytesWarnings under python3, ipa-server-upgrade should work on IPA side but there are issues on pyldap side.