Change in vdsm[master]: xmlrpc: Hide fenceNode password in the log

Francesco Romani has posted comments on this change.

Change subject: xmlrpc: Hide fenceNode password in the log
......................................................................


Patch Set 4:

(2 comments)

https://gerrit.ovirt.org/#/c/58833/4/lib/vdsm/rpc/bindingxmlrpc.py
File lib/vdsm/rpc/bindingxmlrpc.py:

Line 1197: def wrapApiMethod(f):
Line 1198:     def wrapper(*args, **kwargs):
Line 1199:         try:
Line 1200:             logLevel = logging.DEBUG
Line 1201:             suppress_args = f.__name__ in ('fenceNode',)
>  why to check inclusion in a tuple when it is a single element?
not sure we need this for more verbs, in this case we could have the soil prepared
here.
Line 1202:             suppress_response = f.__name__ in ('getAllVmStats',)
Line 1203: 
Line 1204:             # TODO: This password protection code is fragile and ugly.
Password
Line 1205:             # protection should be done in the wrapped methods, and logging


Line 1207: 
Line 1208:             displayArgs = args
Line 1209:             if suppress_args:
Line 1210:                 displayArgs = '(suppressed)'
Line 1211:             elif f.__name__ == 'vmDesktopLogin':
>  like here. And it would be more consistent too ;-) 
True, but
this code is going to be killed ASAP.
In 3.6.x this code will survive longer, but not sure worth a resubmit for this. I
personally don't mind.
Line 1212:                 if 'password' in kwargs:
Line 1213:                     raise TypeError("Got an unexpected keyword argument:
"
Line 1214:                                     "'password'")
Line 1215:                 if len(args) > 3:


-- 
To view, visit https://gerrit.ovirt.org/58833
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I53318ed8ce425f042b5f783af779b540c1dfd7b5
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <toni+ovirt(a)midokura.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com&gt;
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com&gt;
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes