Guess in addition to the command level flag that we have Parameter walk
will need to be done only for the already identified sensitive responses
as discussed on the thread so this may be fine.
Thanks,
Mandar
On Mon, Mar 10, 2014 at 9:34 AM, Mandar Barve mandar.ba...@sungard.comwrote:
We
We surely need a way to make this generic since cleanString looks for
specific keywords to filter. I will take a look at this. Using @Parameter
may have its own limitations like running through the entire list of
parameters per API before deciding which ones to exclude. But let me take a
look.
I
On Fri, Mar 7, 2014 at 7:31 PM, Alena Prokharchyk
alena.prokharc...@citrix.com wrote:
And the defaults should be false,
I don't agree, The true case does nothing if no fields are recognized
as sensitive, but it the flase case skips sensitive data containing
log messages. The only consquence of
Daan, if the default comes as true for the command, I assume that the user
won¹t see the command logged at all? Unless he overrides it.
I assume sensitive=³true² means not ³analyze the command² but rather
³don¹t log the command². That doesn¹t seem right to me.
True would seem right to me if the
Alena, I can see I am not being clear because what you say is the
sensible way and apart from the parameter level exactly what happens.
The parameter thing is an enhancement that we can make on top of this.
At the moment it only obfuscate a set of parameters with a fixed set
of names. We will
Ok, got it, somehow missed the “hardcoded” parameters part. In this case
true is fine as the parameter in @ApiCommand just triggers the validation.
We only have to fix one part - instead of hardcoding the parameter(s) to
hide, we have to come up with the new parameter in @Parameter to trigger
the
no problem, glad we agree.
On Fri, Mar 7, 2014 at 8:38 PM, Alena Prokharchyk
alena.prokharc...@citrix.com wrote:
Ok, got it, somehow missed the hardcoded parameters part. In this case
true is fine as the parameter in @ApiCommand just triggers the validation.
We only have to fix one part -
And here is the Jira ticket:
https://issues.apache.org/jira/browse/CLOUDSTACK-6213
Add new field to API @Parameter indicating if the param should be skipped
from logs”
-Alena.
On 3/7/14, 1:47 PM, Daan Hoogland daan.hoogl...@gmail.com wrote:
no problem, glad we agree.
On Fri, Mar 7, 2014 at
Mandar, you want to take it?
On Fri, Mar 7, 2014 at 11:12 PM, Alena Prokharchyk
alena.prokharc...@citrix.com wrote:
And here is the Jira ticket:
https://issues.apache.org/jira/browse/CLOUDSTACK-6213
Add new field to API @Parameter indicating if the param should be skipped
from logs
Mandar, I¹ve ran into this checkin submitted by you:
b0c6d4734724358df97b6fa4d8c5beb0f447745e
- Updated APICommand annotation to add new flags that indicate if API
request or response carry sensitive info
And have a couple of comments on that.
1) I don¹t see the parameter being checked
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16385/#review35475
---
Ship it!
b0c6d4734724358df97b6fa4d8c5beb0f447745e
- daan Hoogland
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16385/#review35477
---
Ship it!
b0c6d4734724358df97b6fa4d8c5beb0f447745e
- daan Hoogland
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16385/
---
(Updated Feb. 24, 2014, 2:37 p.m.)
Review request for cloudstack and daan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16385/
---
(Updated Feb. 24, 2014, 2:38 p.m.)
Review request for cloudstack and daan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16385/#review35276
---
I'll download and money test. please adjust the description to
---
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
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16385/#review34691
---
I like. the good default is used (unlike what the description of
You are right in your analysis but about the methods you are drawing
the wrong conclusion.
We want each class to have its own values, not each command object.
Any BlaCmd should have exactly the same values so it makes sense to
make them static on the class object.
About the variables; I see how
I think I get what you are saying. We should be using annotation per API
class declaring sensitivity at class level. Using static methods returning
predefined values is similar to this and annotation looks more elegant as
what Nitin had suggested. At run time we will need to load this annotation
Daan,
I am still failing to understand the use of static vars and setter
methods. If we do that then those vars will essentially become class vars
and not instance vars. Don't we want each API class to have a diff instance
var telling us if its sensitive or not? Am I missing something?
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16385/#review34036
---
Mandar, this does not apply to master. It seems note serious but
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16385/#review34037
---
Mandar, this does not apply to master. It seems note serious but
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16385/#review34038
---
H Mandar, some little issues applying, but mostly: the methods for
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16385/
---
(Updated Feb. 7, 2014, 10:30 a.m.)
Review request for cloudstack and daan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16385/
---
(Updated Feb. 7, 2014, 10:30 a.m.)
Review request for cloudstack and daan
Daan,
I have been busy with few other things. Will need to get back to this
and update, hopefully before EoW.
Thanks,
Mandar
On Fri, Jan 31, 2014 at 3:03 PM, daan Hoogland daan.hoogl...@gmail.comwrote:
---
This is an automatically
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16385/#review33306
---
Mandar, still feel like picking this up. As Nithin didn't reply
Daan,
I don't get the idea behind making the methods static. Making
getter/setters static will lead to instance vars losing their meaning and
we need each instance to let us know its sensitivity. I assume you are not
suggesting changing the abstract method into static. Can you please
explain?
I mean to have the setting of the vars and the vars them selves be
static , not the retrieving method.
On Thu, Jan 9, 2014 at 1:52 PM, Mandar Barve mandar.ba...@sungard.com wrote:
Daan,
I don't get the idea behind making the methods static. Making
getter/setters static will lead to
On Dec. 23, 2013, 5:58 p.m., Nitin Mehta wrote:
api/src/org/apache/cloudstack/api/BaseListTemplateOrIsoPermissionsCmd.java,
line 53
https://reviews.apache.org/r/16385/diff/1/?file=400860#file400860line53
You shouldn't have to override for every cmd. By default its false and
Nithin, I think your last points are valid but should not stop Mandar's
change. Except for making the booleans static I think further improvements
are for the next version and we should apply Mandar's version.
On Thu, Jan 2, 2014 at 12:39 PM, Mandar Barve mandar.ba...@sungard.comwrote:
This
On Dec. 23, 2013, 5:58 p.m., Nitin Mehta wrote:
api/src/org/apache/cloudstack/api/BaseListTemplateOrIsoPermissionsCmd.java,
line 53
https://reviews.apache.org/r/16385/diff/1/?file=400860#file400860line53
You shouldn't have to override for every cmd. By default its false and
On Dec. 23, 2013, 5:58 p.m., Nitin Mehta wrote:
api/src/org/apache/cloudstack/api/BaseCmd.java, line 415
https://reviews.apache.org/r/16385/diff/1/?file=400859#file400859line415
Can you please create names which are more intuitive such as
cmdRequestContainsSensitiveInfo and also
On Dec. 19, 2013, 3:58 p.m., daan Hoogland wrote:
api/src/org/apache/cloudstack/api/BaseCmd.java, line 427
https://reviews.apache.org/r/16385/diff/1/?file=400859#file400859line427
please make sure a clear and extensive javadoc is present on why and
how this abstract method should
H Mandar,
why not just put
/**
* cmdHandlesCriticalData method must be implemented for all APIs. This
method declares if it handles requests and/or responses that carry
sensitive data such as passwords, secret keys.
* Method implementation should call cmdReqIsCritical and/or
Sounds good. I will post updated patch.
Thanks,
Mandar
On Mon, Dec 23, 2013 at 8:14 PM, Daan Hoogland daan.hoogl...@gmail.comwrote:
H Mandar,
why not just put
/**
* cmdHandlesCriticalData method must be implemented for all APIs.
This method declares if it handles requests
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16385/#review30828
---
api/src/org/apache/cloudstack/api/BaseCmd.java
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16385/
---
(Updated Dec. 23, 2013, 6:11 p.m.)
Review request for cloudstack and daan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16385/
---
(Updated Dec. 23, 2013, 6:13 p.m.)
Review request for cloudstack and daan
On Dec. 23, 2013, 5:58 p.m., Nitin Mehta wrote:
api/src/org/apache/cloudstack/api/BaseCmd.java, line 415
https://reviews.apache.org/r/16385/diff/1/?file=400859#file400859line415
Can you please create names which are more intuitive such as
cmdRequestContainsSensitiveInfo and also
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16385/
---
Review request for cloudstack and daan Hoogland.
Bugs: CLOUDSTACK-4406
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16385/#review30699
---
api/src/org/apache/cloudstack/api/BaseCmd.java
42 matches
Mail list logo
