-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16385/
-----------------------------------------------------------
(Updated Feb. 18, 2014, 11:41 a.m.)
Review request for cloudstack and daan Hoogland.
Changes
-------
Daan,
I have created a pilot patch based on our discussion. In this I have
modified existing APICommand annotation to add extra flags. Let me know if this
looks okay or something needs to change. If this is fine I can start making
changes to the rest of the code.
Thanks,
Mandar
Bugs: CLOUDSTACK-4406
https://issues.apache.org/jira/browse/CLOUDSTACK-4406
Repository: cloudstack-git
Description
-------
JIRA 4406 expects removal of cleanString() call for performance
improvements. This is called when building audit trail for command responses
and used for removing sensitive data (passwords, secret keys) from the log
buffer. All the API responses do not carry such sensitive information so
pattern matching done by cleanString against all API response strings can be
costly.
I propose following for a solution:
* Modify BaseCmd class to add flags that will store cmd/response sensitivity
* At init these flags will be set to false indicating no cmd req/resp carries
sensitive data
* any child api cmd class that will carry sensitive data in the req/resp should
set the respective flags
* before calling any logging function the flag should be checked and
cleanString should be called only for cmds with flags set
Pro: This approach will scale well as new cmds get added and no additional
changes should be required.
Con: Big change upfront as it will touch all API cmd classes that carry
sensitive information along with BaseCmd class.
NOTE: changes should be simple and straightforward though spread across
multiple classes.
Diffs (updated)
-----
api/src/org/apache/cloudstack/api/APICommand.java 5587a48
api/src/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java
c5a2d1a
api/src/org/apache/cloudstack/api/command/admin/account/DeleteAccountCmd.java
7c1b206
api/src/org/apache/cloudstack/api/command/admin/account/DisableAccountCmd.java
6fdbefe
api/src/org/apache/cloudstack/api/command/admin/account/EnableAccountCmd.java
59d6acd
api/src/org/apache/cloudstack/api/command/user/account/ListAccountsCmd.java
bc93d21
server/src/com/cloud/api/ApiServer.java d715db6
Diff: https://reviews.apache.org/r/16385/diff/
Testing
-------
Using CloudMonkey following commands have been tested to make sure secret
key/password is stripped from the response
list users
list accounts
list virtualmachines
create user
update user
create sshkeypair
Thanks,
Mandar Barve
