SSH-key configuration utilities #176
Conversation
|
For this psutil (pip package) and libselinux-python (yum package) is required, but for rhel names are different python-psutil and libselinux i am going to raise bug for this. @TimothyAsir @shtripat @r0h4n @nnDarshan @brainfunked |
|
@TimothyAsir @shtripat @nnDarshan @r0h4n @brainfunked please review this |
tendrl/commons/utils/SSHUtil.py
Outdated
|
|
||
|
|
||
| def sshd_status(): | ||
| cmd = cmd_utils.Command("cat /var/run/sshd.pid") |
There was a problem hiding this comment.
pid file is not guaranteed to exist. Furthermore, locations change. Since we're relying on systemd, a much more reliable method would be to use systemctl show sshd.service and grab the value of MainPID.
There was a problem hiding this comment.
@brainfunked problem is we can't use pip symbol in ansible command module, so grep is not possible
tendrl/commons/utils/generate_ssh.py
Outdated
| from tendrl.commons.event import Event | ||
| from tendrl.commons.message import Message | ||
|
|
||
| ANSIBLE_MODULE_PATH = "core/system/user.py" |
There was a problem hiding this comment.
Does this module guarantee that the SSH key will be generated if the user already exists?
There was a problem hiding this comment.
@brainfunked yes i tried, if the user is exist it will create only ssh-key, if the user is not exist it will create user with ssh-key
tendrl-bug-id: Tendrl/specifications#142 Signed-off-by: GowthamShanmugam <gshanmug@redhat.com>
|
@brainfunked Changes done |
Signed-off-by: GowthamShanmugam <gshanmug@redhat.com>
|
@brainfunked done all changes |
tendrl/commons/utils/ssh_util.py
Outdated
| ) | ||
| return None, err | ||
|
|
||
| def find_pid(out): |
There was a problem hiding this comment.
This method should be private and not accessible to anyone using the util.
There was a problem hiding this comment.
@brainfunked ok
tendrl/commons/utils/ssh_util.py
Outdated
| Message( | ||
| priority="error", | ||
| publisher="commons", | ||
| payload={"message": "sshd service is not run"} |
There was a problem hiding this comment.
@brainfunked done
tendrl/commons/utils/ssh_util.py
Outdated
| payload={"message": "sshd service is not run"} | ||
| ) | ||
| ) | ||
| return sshd, "sshd service is not run" |
There was a problem hiding this comment.
No returns in between. Write pre-determined status values in a constant and use one of them in the dict you return at the end of the method. You can short circuit in between if the checks further down don't make any sense, but return must always be a dict with the same format each time.
There was a problem hiding this comment.
@brainfunked ok
tendrl/commons/utils/ssh_util.py
Outdated
| if result == []: | ||
| Event( | ||
| Message( | ||
| priority="error", |
There was a problem hiding this comment.
@brainfunked ok
tendrl/commons/utils/ssh_util.py
Outdated
| from tendrl.commons.utils import cmd_utils | ||
|
|
||
|
|
||
| def sshd_status(): |
There was a problem hiding this comment.
This util must always return a specific data structure, which would seem to be a dict with keys "name", "port" and "status". This must always be the return value. If there are any errors, the values should probably be Null or empty, whatever is the appropriate python convention. In none of the error conditions are you supposed to return directly with just a string. Use log messages to indicate errors.
There was a problem hiding this comment.
@brainfunked i will change as per your suggestion
tendrl/commons/utils/ssh_util.py
Outdated
| if pid == 0: | ||
| Event( | ||
| Message( | ||
| priority="error", |
There was a problem hiding this comment.
@brainfunked done
tendrl/commons/utils/ssh_util.py
Outdated
| payload={"message": "Unable to find port number"} | ||
| ) | ||
| ) | ||
| return sshd, "Unable to find port number" |
There was a problem hiding this comment.
Again, no return in between.
There was a problem hiding this comment.
@brainfunked ok
| ANSIBLE_MODULE_PATH = "core/system/user.py" | ||
|
|
||
|
|
||
| class GenerateSSH(object): |
There was a problem hiding this comment.
Update the class names to match the file names.
There was a problem hiding this comment.
@brainfunked ok
| ANSIBLE_MODULE_PATH = "core/system/authorized_key.py" | ||
|
|
||
|
|
||
| class AuthorizeKey(object): |
There was a problem hiding this comment.
Determine the output data structure for this utility. Make sure that all return values use the same data structure. Write error messages into logs at warning priority.
There was a problem hiding this comment.
@brainfunked ok
Note: Authorize_key module works when selinux is disabled Signed-off-by: GowthamShanmugam <gshanmug@redhat.com>
|
@shtripat @brainfunked @r0h4n @nnDarshan @nthomas-redhat please review |
Signed-off-by: GowthamShanmugam <gshanmug@redhat.com>
tendrl-bug-id: Tendrl/specifications#141
tendrl-spec: ssh_configuration
Signed-off-by: GowthamShanmugam gshanmug@redhat.com