redhat-qe-security / SCAutolib

Library for automation of smart card testing
GNU General Public License v3.0
3 stars 10 forks source link

Add default sudo cmd and rule on server #34

Closed x00Pavel closed 2 years ago

x00Pavel commented 2 years ago

By default, IPA client can't run any sudo command, so adding default sudo rule and sudo command for testing.

x00Pavel commented 2 years ago

@mahavrila maybe you don't fully understand what is the point of this check. after the script is copied to the file from ssh stdout, then this file is opened to check if something was actually written there. but ok, will try to rephrase it to make the commit more clear.

don't fully understand the opportunity regarding docstring. can you explain how do mean?

what do you mean by the 'log' function? we don't have any 'log' function in the code, only python logger that generates logs.

mahavrila commented 2 years ago

@x00Pavel I do understand script, but would not be able to guess it from commit message. 'Add check for correct getting' implies we would check some process while only result is actually checked. We actually do not check "correct getting" but presence of non-empty file. It's not very important, but if I search in commits in the future, I will not know what to think.

'log' function is used in src/env/ipa-install-server.sh and imported from /src/env/logs.sh and I pointed this out because linters usually warn about this.

Formatting of docstring of function install_ipaclient is kind of weird and I thought that we can fix also docstring of the function. This, however, can also be done separately.

As I said, non of this is critical.

x00Pavel commented 2 years ago

@mahavrila ok, now I get it. Will rephrase commit message on squashing.

Regarding the log function, I didn't meet any problem with it, but will move these functions to the file, because in this case log.sh is redundant because we have only one bash script which includes.

Improving docstring I would leave for another PR.

x00Pavel commented 2 years ago

@mahavrila actually there is no need to rephrase this commit because it would be squashed to one commit