You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by koushik-das <gi...@git.apache.org> on 2015/11/02 10:39:44 UTC
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
GitHub user koushik-das opened a pull request:
https://github.com/apache/cloudstack/pull/1021
CLOUDSTACK-8485: listAPIs are taking too long to return results
- Removed regex. based search/replace of sensitive data on API response introduced as part of commit b0c6d4734724358df97b6fa4d8c5beb0f447745e
- Added new response serializer to skip sensitive data from getting logged based on annotation present in resposne object fields
- Added annotation (@LogLevel(Log4jLevel.Off)) to sensitive response object fields
Ran the following tests on simulator:
test_vm_life_cycle.py
Test advanced zone virtual router ... === TestName: test_advZoneVirtualRouter | Status : SUCCESS ===
ok
Test Deploy Virtual Machine ... === TestName: test_deploy_vm | Status : SUCCESS ===
ok
Test Multiple Deploy Virtual Machine ... === TestName: test_deploy_vm_multiple | Status : SUCCESS ===
ok
Test Stop Virtual Machine ... === TestName: test_01_stop_vm | Status : SUCCESS ===
ok
Test Start Virtual Machine ... === TestName: test_02_start_vm | Status : SUCCESS ===
ok
Test Reboot Virtual Machine ... === TestName: test_03_reboot_vm | Status : SUCCESS ===
ok
Test destroy Virtual Machine ... === TestName: test_06_destroy_vm | Status : SUCCESS ===
ok
Test recover Virtual Machine ... === TestName: test_07_restore_vm | Status : SUCCESS ===
ok
Test migrate VM ... === TestName: test_08_migrate_vm | Status : SUCCESS ===
ok
Test destroy(expunge) Virtual Machine ... === TestName: test_09_expunge_vm | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 10 tests in 306.429s
OK
test_volumes.py
Download a Volume attached to a VM ... === TestName: test_03_download_attached_volume | Status : SUCCESS ===
ok
Delete a Volume attached to a VM ... === TestName: test_04_delete_attached_volume | Status : SUCCESS ===
ok
Detach a Volume attached to a VM ... === TestName: test_05_detach_volume | Status : SUCCESS ===
ok
Delete a Volume unattached to an VM ... === TestName: test_09_delete_detached_volume | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 4 tests in 184.132s
OK
test_network.py
Test for delete account ... === TestName: test_delete_account | Status : SUCCESS ===
ok
Test for Associate/Disassociate public IP address for admin account ... === TestName: test_public_ip_admin_account | Status : SUCCESS ===
ok
Test for Associate/Disassociate public IP address for user account ... === TestName: test_public_ip_user_account | Status : SUCCESS ===
ok
Test for release public IP address ... === TestName: test_releaseIP | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 4 tests in 783.726s
OK
test_routers.py
Test router internal advanced zone ... SKIP: Marvin configuration has no host credentials to check router services
Test restart network ... === TestName: test_03_restart_network_cleanup | Status : SUCCESS ===
ok
Test router basic setup ... === TestName: test_05_router_basic | Status : SUCCESS ===
ok
Test router advanced setup ... === TestName: test_06_router_advanced | Status : SUCCESS ===
ok
Test stop router ... === TestName: test_07_stop_router | Status : SUCCESS ===
ok
Test start router ... === TestName: test_08_start_router | Status : SUCCESS ===
ok
Test reboot router ... === TestName: test_09_reboot_router | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 7 tests in 42.958s
OK (SKIP=1)
test_global_settings.py
test update configuration setting at zone level scope ... === TestName: test_UpdateConfigParamWithScope | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 1 test in 0.127s
OK
test_resource_detail.py
Test volume detail ... === TestName: test_01_updatevolumedetail | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 1 test in 11.492s
OK
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/koushik-das/cloudstack CLOUDSTACK-8485
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1021.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1021
----
commit a32c91f290f3d7300c4a0d40455f28cf99f4e769
Author: Koushik Das <ko...@apache.org>
Date: 2015-11-02T09:29:00Z
CLOUDSTACK-8485: listAPIs are taking too long to return results
- Removed regex. based search/replace of sensitive data on API response introduced as part of commit b0c6d4734724358df97b6fa4d8c5beb0f447745e
- Added new response serializer to skip sensitive data from getting logged based on annotation present in resposne object fields
- Added annotation (@LogLevel(Log4jLevel.Off)) to sensitive response object fields
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-158336847
@DaanHoogland did you get a chance to test it?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-158617403
[1021.network.results.txt](https://github.com/apache/cloudstack/files/40781/1021.network.results.txt)
[1021.vpc.results.txt](https://github.com/apache/cloudstack/files/40782/1021.vpc.results.txt)
all tests passed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1021#discussion_r43891649
--- Diff: api/src/org/apache/cloudstack/api/response/SSHKeyPairResponse.java ---
@@ -40,6 +42,7 @@
@SerializedName("fingerprint")
@Param(description = "Fingerprint of the public key")
+ @LogLevel(Log4jLevel.Off)
--- End diff --
why is fingerprint sensitive?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1021#discussion_r43996309
--- Diff: api/src/org/apache/cloudstack/api/response/UserVmResponse.java ---
@@ -256,6 +259,7 @@
@SerializedName(ApiConstants.SSH_KEYPAIR)
@Param(description = "ssh key-pair")
+ @LogLevel(Log4jLevel.Off)
--- End diff --
thought its the actual data, will change
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-152996985
registerUserKeys log based on this change
2015-11-02 17:15:19,061 INFO [a.c.c.a.ApiServer] (2001334745@qtp-678426242-0:ctx-608cad64 ctx-6e00b6fc) (logid:bfb12e9d) (userId=2 accountId=2 sessionId=9g6dq5sjxlk7137t68hfkux8) 0:0:0:0:0:0:0:1 -- GET command=registerUserKeys&response=json&id=e2fdf8e4-73be-11e5-8882-249684d59f9c&_=1446464718702 200 {"registeruserkeysresponse":{"userkeys":{}}}
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1021
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-154030678
@DaanHoogland That's right. I had already mentioned about the agent commands in one of my previous comments
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-154035894
@DaanHoogland If you look at ApiResponseGsonHelper.java in this PR there are GSon builders
private static final GsonBuilder s_gBuilder;
private static final GsonBuilder s_gLogBuilder;
Similarly for agent commands, if you look at GsonHelper.java there are 2 of them
protected static final Gson s_gson;
protected static final Gson s_gogger;
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-154971789
@DaanHoogland Updated based on our discussion last week
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-154990742
great @koushik-das I'll rerun the tests.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1021#discussion_r43892103
--- Diff: api/src/org/apache/cloudstack/api/response/SslCertResponse.java ---
@@ -38,6 +40,7 @@
@SerializedName(ApiConstants.CERTIFICATE)
@Param(description = "certificate")
+ @LogLevel(Log4jLevel.Off)
--- End diff --
certificates are not sensitive data. They are meant to be public.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-158366425
@DaanHoogland I have force pushed again. Hopefully a good jenkins slave will pick this up :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1021#discussion_r43997043
--- Diff: server/src/com/cloud/api/ApiResponseGsonHelper.java ---
@@ -71,4 +83,19 @@ public boolean shouldSkipField(FieldAttributes f) {
return false;
}
}
+
+ private static class LogExclusionStrategy extends ApiResponseExclusionStrategy implements ExclusionStrategy {
--- End diff --
The existing LoggingExlusionStrategy is for agent commands/workjob etc. But it won't work for API responses. If you look at the code, exclusion strategy for API response log depends on ApiResponseExclusionStrategy as well for exclusion based on roles - user/admin.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-158401181
also running the regressions suites on it now
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-158374179
code reviewed. LGTM awaiting tests. it is running on one of the INFRA-10703 hosts but I know @abayer did some work on that issue...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153690547
@DaanHoogland I think LogLevel is a better/generic name to suppress a field from getting logged. Currently only sensitive field is getting annotated with it. Going forward some non-sensitive field may also be annotated if someone wants to prevent it from getting logged. Also since LogLevel is already used in agent commands, having an uniform approach throughout the code is always better and creates less confusion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1021#discussion_r43996820
--- Diff: server/src/com/cloud/api/ApiResponseGsonHelper.java ---
@@ -27,30 +29,40 @@
import com.google.gson.GsonBuilder;
/**
- * The ApiResonseGsonHelper is different from ApiGsonHelper - it registeres one more adapter for String type required for api response encoding
+ * The ApiResonseGsonHelper is different from ApiGsonHelper - it registers one more adapter for String type required for api response encoding
--- End diff --
I have to read more on the gson changes you are referring to. But I think we shouldn't mix multiple things with this. The primary purpose of this PR is to fix the perf. issue in list API responses.
There may be some rework if gson is upgraded but it is better to do that when gson upgrade work is done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153682474
@bhaisaab no we shouldn't remove those both are used. I didn't understand the {, <, >, } part.
The @LogLevel is used in the gson serialization and we can abuse it in the API but your comment makes sense. In the meanwhile that doesn't invalidate Koushik's change.
let's make the right decision
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153695015
@bhaisaab Agree that the existing JSON and XML serialization can be improved but it is better to do it as a separate PR. For JSON, the standard Gson library is used. For XML we may have to check for a ASF compatible library that solves the purpose.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1021#discussion_r43899212
--- Diff: server/src/com/cloud/api/ApiResponseGsonHelper.java ---
@@ -71,4 +83,19 @@ public boolean shouldSkipField(FieldAttributes f) {
return false;
}
}
+
+ private static class LogExclusionStrategy extends ApiResponseExclusionStrategy implements ExclusionStrategy {
--- End diff --
LoggingExclusionStrategy allready exists. It shouldn't be needed to implement again.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1021#discussion_r43996031
--- Diff: api/src/org/apache/cloudstack/api/response/SSHKeyPairResponse.java ---
@@ -40,6 +42,7 @@
@SerializedName("fingerprint")
@Param(description = "Fingerprint of the public key")
+ @LogLevel(Log4jLevel.Off)
--- End diff --
will change
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153686114
@DaanHoogland @bhaisaab requestHasSensitiveInfo/responseHasSensitiveInfo can be removed once this PR is accepted. I don't see any other use of these parameters.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-158901685
LGTM based on these tests (may not have tested the actual change):
```
nosetests --with-marvin --marvin-config=${marvinCfg} -s -a tags=advanced,required_hardware=true \
component/test_vpc_redundant.py \
component/test_routers_iptables_default_policy.py \
component/test_routers_network_ops.py \
component/test_vpc_router_nics.py \
smoke/test_loadbalance.py \
smoke/test_internal_lb.py \
smoke/test_ssvm.py \
smoke/test_network.py
```
Result:
```
Test iptables default INPUT/FORWARD policies on VPC router ... === TestName: test_01_single_VPC_iptables_policies | Status : SUCCESS ===
ok
Test redundant router internals ... === TestName: test_01_isolate_network_FW_PF_default_routes_egress_true | Status : SUCCESS ===
ok
Test redundant router internals ... === TestName: test_02_isolate_network_FW_PF_default_routes_egress_false | Status : SUCCESS ===
ok
Test redundant router internals ... === TestName: test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | Status : SUCCESS ===
ok
Test redundant router internals ... === TestName: test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | Status : SUCCESS ===
ok
Create a VPC with two networks with one VM in each network and test nics after destroy ... === TestName: test_01_VPC_nics_after_destroy | Status : SUCCESS ===
ok
Create a VPC with two networks with one VM in each network and test default routes ... === TestName: test_02_VPC_default_routes | Status : SUCCESS ===
ok
Check the password file in the Router VM ... === TestName: test_isolate_network_password_server | Status : SUCCESS ===
ok
Check that the /etc/dhcphosts.txt doesn't contain duplicate IPs ... === TestName: test_router_dhcphosts | Status : SUCCESS ===
ok
Test to create Load balancing rule with source NAT ... === TestName: test_01_create_lb_rule_src_nat | Status : SUCCESS ===
ok
Test to create Load balancing rule with non source NAT ... === TestName: test_02_create_lb_rule_non_nat | Status : SUCCESS ===
ok
Test for assign & removing load balancing rule ... === TestName: test_assign_and_removal_lb | Status : SUCCESS ===
ok
Test to verify access to loadbalancer haproxy admin stats page ... === TestName: test02_internallb_haproxy_stats_on_all_interfaces | Status : SUCCESS ===
ok
Test create, assign, remove of an Internal LB with roundrobin http traffic to 3 vm's ... === TestName: test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | Status : SUCCESS ===
ok
Test SSVM Internals ... === TestName: test_03_ssvm_internals | Status : SUCCESS ===
ok
Test CPVM Internals ... === TestName: test_04_cpvm_internals | Status : SUCCESS ===
ok
Test stop SSVM ... === TestName: test_05_stop_ssvm | Status : SUCCESS ===
ok
Test stop CPVM ... === TestName: test_06_stop_cpvm | Status : SUCCESS ===
ok
Test reboot SSVM ... === TestName: test_07_reboot_ssvm | Status : SUCCESS ===
ok
Test reboot CPVM ... === TestName: test_08_reboot_cpvm | Status : SUCCESS ===
ok
Test destroy SSVM ... === TestName: test_09_destroy_ssvm | Status : SUCCESS ===
ok
Test destroy CPVM ... === TestName: test_10_destroy_cpvm | Status : SUCCESS ===
ok
Test Remote Access VPN in VPC ... === TestName: test_vpc_remote_access_vpn | Status : SUCCESS ===
ok
Test VPN in VPC ... === TestName: test_vpc_site2site_vpn | Status : SUCCESS ===
ok
Test for port forwarding on source NAT ... === TestName: test_01_port_fwd_on_src_nat | Status : SUCCESS ===
ok
Test for port forwarding on non source NAT ... === TestName: test_02_port_fwd_on_non_src_nat | Status : SUCCESS ===
ok
Test for reboot router ... === TestName: test_reboot_router | Status : SUCCESS ===
ok
Test for Router rules for network rules on acquired public IP ... === TestName: test_network_rules_acquired_public_ip_1_static_nat_rule | Status : SUCCESS ===
ok
Test for Router rules for network rules on acquired public IP ... === TestName: test_network_rules_acquired_public_ip_2_nat_rule | Status : SUCCESS ===
ok
Test for Router rules for network rules on acquired public IP ... === TestName: test_network_rules_acquired_public_ip_3_Load_Balancer_Rule | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 33 tests in 17014.931s
OK
```
And:
```
nosetests --with-marvin --marvin-config=${marvinCfg} -s -a tags=advanced,required_hardware=false \
smoke/test_routers.py \
smoke/test_network_acl.py \
smoke/test_privategw_acl.py \
smoke/test_reset_vm_on_reboot.py \
smoke/test_vm_life_cycle.py \
smoke/test_vpc_vpn.py \
smoke/test_service_offerings.py \
component/test_vpc_offerings.py \
component/test_vpc_routers.py
```
Result:
```
Test router internal advanced zone ... === TestName: test_02_router_internal_adv | Status : SUCCESS ===
ok
Test restart network ... === TestName: test_03_restart_network_cleanup | Status : SUCCESS ===
ok
Test router basic setup ... === TestName: test_05_router_basic | Status : SUCCESS ===
ok
Test router advanced setup ... === TestName: test_06_router_advanced | Status : SUCCESS ===
ok
Test stop router ... === TestName: test_07_stop_router | Status : SUCCESS ===
ok
Test start router ... === TestName: test_08_start_router | Status : SUCCESS ===
ok
Test reboot router ... === TestName: test_09_reboot_router | Status : SUCCESS ===
ok
test_privategw_acl (integration.smoke.test_privategw_acl.TestPrivateGwACL) ... === TestName: test_privategw_acl | Status : SUCCESS ===
ok
Test reset virtual machine on reboot ... === TestName: test_01_reset_vm_on_reboot | Status : SUCCESS ===
ok
Test advanced zone virtual router ... === TestName: test_advZoneVirtualRouter | Status : SUCCESS ===
ok
Test Deploy Virtual Machine ... === TestName: test_deploy_vm | Status : SUCCESS ===
ok
Test Multiple Deploy Virtual Machine ... === TestName: test_deploy_vm_multiple | Status : SUCCESS ===
ok
Test Stop Virtual Machine ... === TestName: test_01_stop_vm | Status : SUCCESS ===
ok
Test Start Virtual Machine ... === TestName: test_02_start_vm | Status : SUCCESS ===
ok
Test Reboot Virtual Machine ... === TestName: test_03_reboot_vm | Status : SUCCESS ===
ok
Test destroy Virtual Machine ... === TestName: test_06_destroy_vm | Status : SUCCESS ===
ok
Test recover Virtual Machine ... === TestName: test_07_restore_vm | Status : SUCCESS ===
ok
Test migrate VM ... === TestName: test_08_migrate_vm | Status : SUCCESS ===
ok
Test destroy(expunge) Virtual Machine ... === TestName: test_09_expunge_vm | Status : SUCCESS ===
ok
Test to create service offering ... === TestName: test_01_create_service_offering | Status : SUCCESS ===
ok
Test to update existing service offering ... === TestName: test_02_edit_service_offering | Status : SUCCESS ===
ok
Test to delete service offering ... === TestName: test_03_delete_service_offering | Status : SUCCESS ===
ok
Test for delete account ... === TestName: test_delete_account | Status : SUCCESS ===
ok
Test for Associate/Disassociate public IP address for admin account ... === TestName: test_public_ip_admin_account | Status : SUCCESS ===
ok
Test for Associate/Disassociate public IP address for user account ... === TestName: test_public_ip_user_account | Status : SUCCESS ===
ok
Test for release public IP address ... === TestName: test_releaseIP | Status : SUCCESS ===
ok
Test create VPC offering ... === TestName: test_01_create_vpc_offering | Status : SUCCESS ===
ok
Test VPC offering without load balancing service ... === TestName: test_03_vpc_off_without_lb | Status : SUCCESS ===
ok
Test VPC offering without static NAT service ... === TestName: test_04_vpc_off_without_static_nat | Status : SUCCESS ===
ok
Test VPC offering without port forwarding service ... === TestName: test_05_vpc_off_without_pf | Status : SUCCESS ===
ok
Test VPC offering with invalid services ... === TestName: test_06_vpc_off_invalid_services | Status : SUCCESS ===
ok
Test update VPC offering ... === TestName: test_07_update_vpc_off | Status : SUCCESS ===
ok
Test list VPC offering ... === TestName: test_08_list_vpc_off | Status : SUCCESS ===
test_09_create_redundant_vpc_offering (integration.component.test_vpc_offerings.TestVPCOffering) ... === TestName: test_09_create_redundant_vpc_offering | Status : SUCCESS ===
ok
Test start/stop of router after addition of one guest network ... === TestName: test_01_start_stop_router_after_addition_of_one_guest_network | Status : SUCCESS ===
ok
Test reboot of router after addition of one guest network ... === TestName: test_02_reboot_router_after_addition_of_one_guest_network | Status : SUCCESS ===
ok
Test to change service offering of router after addition of one guest network ... === TestName: test_04_chg_srv_off_router_after_addition_of_one_guest_network | Status : SUCCESS ===
ok
Test destroy of router after addition of one guest network ... === TestName: test_05_destroy_router_after_addition_of_one_guest_network | Status : SUCCESS ===
ok
Test to stop and start router after creation of VPC ... === TestName: test_01_stop_start_router_after_creating_vpc | Status : SUCCESS ===
ok
Test to reboot the router after creating a VPC ... === TestName: test_02_reboot_router_after_creating_vpc | Status : SUCCESS ===
ok
Tests to change service offering of the Router after ... === TestName: test_04_change_service_offerring_vpc | Status : SUCCESS ===
ok
Test to destroy the router after creating a VPC ... === TestName: test_05_destroy_router_after_creating_vpc | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 42 tests in 8513.577s
OK
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-154027199
@koushik-das I don't think you are right, command serialisation uses LogLevel as well. Look at com.cloud.agent.transport.RequestTest and tell me if you agree.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-154062799
I am not fine with this change but must be honest and report regressions tests from the bubble: they all passed.
```
Test router internal advanced zone ... === TestName: test_02_router_internal_adv | Status : SUCCESS ===
ok
Test restart network ... === TestName: test_03_restart_network_cleanup | Status : SUCCESS ===
ok
Test router basic setup ... === TestName: test_05_router_basic | Status : SUCCESS ===
ok
Test router advanced setup ... === TestName: test_06_router_advanced | Status : SUCCESS ===
ok
Test stop router ... === TestName: test_07_stop_router | Status : SUCCESS ===
ok
Test start router ... === TestName: test_08_start_router | Status : SUCCESS ===
ok
Test reboot router ... === TestName: test_09_reboot_router | Status : SUCCESS ===
ok
test_privategw_acl (integration.smoke.test_privategw_acl.TestPrivateGwACL) ... === TestName: test_privategw_acl | Status : SUCCESS ===
ok
Test reset virtual machine on reboot ... === TestName: test_01_reset_vm_on_reboot | Status : SUCCESS ===
ok
Test advanced zone virtual router ... === TestName: test_advZoneVirtualRouter | Status : SUCCESS ===
ok
Test Deploy Virtual Machine ... === TestName: test_deploy_vm | Status : SUCCESS ===
ok
Test Multiple Deploy Virtual Machine ... === TestName: test_deploy_vm_multiple | Status : SUCCESS ===
ok
Test Stop Virtual Machine ... === TestName: test_01_stop_vm | Status : SUCCESS ===
ok
Test Start Virtual Machine ... === TestName: test_02_start_vm | Status : SUCCESS ===
ok
Test Reboot Virtual Machine ... === TestName: test_03_reboot_vm | Status : SUCCESS ===
ok
Test destroy Virtual Machine ... === TestName: test_06_destroy_vm | Status : SUCCESS ===
ok
Test recover Virtual Machine ... === TestName: test_07_restore_vm | Status : SUCCESS ===
ok
Test migrate VM ... === TestName: test_08_migrate_vm | Status : SUCCESS ===
ok
Test destroy(expunge) Virtual Machine ... === TestName: test_09_expunge_vm | Status : SUCCESS ===
ok
Test to create service offering ... === TestName: test_01_create_service_offering | Status : SUCCESS ===
ok
Test to update existing service offering ... === TestName: test_02_edit_service_offering | Status : SUCCESS ===
ok
Test to delete service offering ... === TestName: test_03_delete_service_offering | Status : SUCCESS ===
ok
Test for delete account ... === TestName: test_delete_account | Status : SUCCESS ===
ok
Test for Associate/Disassociate public IP address for admin account ... === TestName: test_public_ip_admin_account | Status : SUCCESS ===
ok
Test for Associate/Disassociate public IP address for user account ... === TestName: test_public_ip_user_account | Status : SUCCESS ===
ok
Test for release public IP address ... === TestName: test_releaseIP | Status : SUCCESS ===
ok
Test create VPC offering ... === TestName: test_01_create_vpc_offering | Status : SUCCESS ===
ok
Test VPC offering without load balancing service ... === TestName: test_03_vpc_off_without_lb | Status : SUCCESS ===
ok
Test VPC offering without static NAT service ... === TestName: test_04_vpc_off_without_static_nat | Status : SUCCESS ===
ok
Test VPC offering without port forwarding service ... === TestName: test_05_vpc_off_without_pf | Status : SUCCESS ===
ok
Test VPC offering with invalid services ... === TestName: test_06_vpc_off_invalid_services | Status : SUCCESS ===
ok
Test update VPC offering ... === TestName: test_07_update_vpc_off | Status : SUCCESS ===
ok
Test list VPC offering ... === TestName: test_08_list_vpc_off | Status : SUCCESS ===
ok
test_09_create_redundant_vpc_offering (integration.component.test_vpc_offerings.TestVPCOffering) ... === TestName: test_09_create_redundant_vpc_offering | Status : SUCCESS ===
ok
Test start/stop of router after addition of one guest network ... === TestName: test_01_start_stop_router_after_addition_of_one_guest_network | Status : SUCCESS ===
ok
Test reboot of router after addition of one guest network ... === TestName: test_02_reboot_router_after_addition_of_one_guest_network | Status : SUCCESS ===
ok
Test to change service offering of router after addition of one guest network ... === TestName: test_04_chg_srv_off_router_after_addition_of_one_guest_network | Status : SUCCESS ===
ok
Test destroy of router after addition of one guest network ... === TestName: test_05_destroy_router_after_addition_of_one_guest_network | Status : SUCCESS ===
ok
Test to stop and start router after creation of VPC ... === TestName: test_01_stop_start_router_after_creating_vpc | Status : SUCCESS ===
ok
Test to reboot the router after creating a VPC ... === TestName: test_02_reboot_router_after_creating_vpc | Status : SUCCESS ===
ok
Tests to change service offering of the Router after ... === TestName: test_04_change_service_offerring_vpc | Status : SUCCESS ===
ok
Test to destroy the router after creating a VPC ... === TestName: test_05_destroy_router_after_creating_vpc | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 42 tests in 8451.564s
OK
```
and
```
Create a redundant VPC with two networks with two VMs in each network ... === TestName: test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | Status : SUCCESS ===
ok
Create a redundant VPC with two networks with two VMs in each network and check default routes ... === TestName: test_02_redundant_VPC_default_routes | Status : SUCCESS ===
ok
Test iptables default INPUT/FORWARD policy on RouterVM ... === TestName: test_02_routervm_iptables_policies | Status : SUCCESS ===
ok
Test iptables default INPUT/FORWARD policies on VPC router ... === TestName: test_01_single_VPC_iptables_policies | Status : SUCCESS ===
ok
Stop existing router, add a PF rule and check we can access the VM ... === TestName: test_isolate_network_FW_PF_default_routes | Status : SUCCESS ===
ok
Test redundant router internals ... === TestName: test_RVR_Network_FW_PF_SSH_default_routes | Status : SUCCESS ===
ok
Create a VPC with two networks with one VM in each network and test nics after destroy ... === TestName: test_01_VPC_nics_after_destroy | Status : SUCCESS ===
ok
Create a VPC with two networks with one VM in each network and test default routes ... === TestName: test_02_VPC_default_routes | Status : SUCCESS ===
ok
Check the password file in the Router VM ... === TestName: test_isolate_network_password_server | Status : SUCCESS ===
ok
Check that the /etc/dhcphosts.txt doesn't contain duplicate IPs ... === TestName: test_router_dhcphosts | Status : SUCCESS ===
ok
Test to create Load balancing rule with source NAT ... === TestName: test_01_create_lb_rule_src_nat | Status : SUCCESS ===
ok
Test to create Load balancing rule with non source NAT ... === TestName: test_02_create_lb_rule_non_nat | Status : SUCCESS ===
ok
Test for assign & removing load balancing rule ... === TestName: test_assign_and_removal_lb | Status : SUCCESS ===
ok
Test to verify access to loadbalancer haproxy admin stats page ... === TestName: test02_internallb_haproxy_stats_on_all_interfaces | Status : SUCCESS ===
ok
Test create, assign, remove of an Internal LB with roundrobin http traffic to 3 vm's ... === TestName: test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | Status : SUCCESS ===
ok
Test SSVM Internals ... === TestName: test_03_ssvm_internals | Status : SUCCESS ===
ok
Test CPVM Internals ... === TestName: test_04_cpvm_internals | Status : SUCCESS ===
ok
Test stop SSVM ... === TestName: test_05_stop_ssvm | Status : SUCCESS ===
ok
Test stop CPVM ... === TestName: test_06_stop_cpvm | Status : SUCCESS ===
ok
Test reboot SSVM ... === TestName: test_07_reboot_ssvm | Status : SUCCESS ===
ok
Test reboot CPVM ... === TestName: test_08_reboot_cpvm | Status : SUCCESS ===
ok
Test destroy SSVM ... === TestName: test_09_destroy_ssvm | Status : SUCCESS ===
ok
Test destroy CPVM ... === TestName: test_10_destroy_cpvm | Status : SUCCESS ===
ok
Test for port forwarding on source NAT ... === TestName: test_01_port_fwd_on_src_nat | Status : SUCCESS ===
ok
Test for port forwarding on non source NAT ... === TestName: test_02_port_fwd_on_non_src_nat | Status : SUCCESS ===
ok
Test for reboot router ... === TestName: test_reboot_router | Status : SUCCESS ===
ok
Test for Router rules for network rules on acquired public IP ... === TestName: test_network_rules_acquired_public_ip_1_static_nat_rule | Status : SUCCESS ===
ok
Test for Router rules for network rules on acquired public IP ... === TestName: test_network_rules_acquired_public_ip_2_nat_rule | Status : SUCCESS ===
ok
Test for Router rules for network rules on acquired public IP ... === TestName: test_network_rules_acquired_public_ip_3_Load_Balancer_Rule | Status : SUCCESS ===
ok
----------------------------------------------------------------------
Ran 29 tests in 13087.859s
OK
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1021#discussion_r44210897
--- Diff: server/src/com/cloud/api/ApiResponseGsonHelper.java ---
@@ -71,4 +83,19 @@ public boolean shouldSkipField(FieldAttributes f) {
return false;
}
}
+
+ private static class LogExclusionStrategy extends ApiResponseExclusionStrategy implements ExclusionStrategy {
+ public boolean shouldSkipClass(Class<?> arg0) {
+ return false;
+ }
+
+ public boolean shouldSkipField(FieldAttributes f) {
+ boolean skip = super.shouldSkipField(f);
+ if (!skip) {
+ LogLevel logLevel = f.getAnnotation(LogLevel.class);
--- End diff --
suggest:
```
Param sensitivity = f.getAnnotation(Param.class);
skip = (sensitivity != null && sensitivity.isSensitive());
```
extra changes are needed fro this on the response classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1021#discussion_r43996196
--- Diff: api/src/org/apache/cloudstack/api/response/SslCertResponse.java ---
@@ -38,6 +40,7 @@
@SerializedName(ApiConstants.CERTIFICATE)
@Param(description = "certificate")
+ @LogLevel(Log4jLevel.Off)
--- End diff --
I suppressed cert. related data from getting logged. But you are right they are not sensitive.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-158903497
Saw the changes, LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153755733
@koushik-das On second thought LogLevel can not be used I think: It is used for json serialization and one might have to serialize a password to be send to an agent while it may not be logged at the same time. We do need a separate annotation. this is not going to fly and hide sensitive data at the same time.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1021#discussion_r43897730
--- Diff: api/src/org/apache/cloudstack/api/response/SslCertResponse.java ---
@@ -62,10 +65,12 @@
@SerializedName(ApiConstants.CERTIFICATE_CHAIN)
@Param(description = "certificate chain")
+ @LogLevel(Log4jLevel.Off)
--- End diff --
cert chain is not sensitive data.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153003710
@koushik-das I've seen it before and the actual error message is
```
#
# A fatal error has been detected by the Java Runtime Environment:
#
SUREFIRE-859: # SIGSEGV (0xb) at pc=0xada0752c, pid=22300, tid=4136188736
#
# JRE version: 7.0_25-b15
# Java VM: Java HotSpot(TM) Server VM (23.25-b01 mixed mode linux-x86 )
# Problematic frame:
# C [libnet.so+0x352c] _init+0x620
#
SUREFIRE-859: # Failed to write core dump. Core dumps have been disabled. To enable core dumping, try "ulimit -c unlimited" before starting Java again
#
# An error report file with more information is saved as:
# /home/jenkins/jenkins-slave/workspace/cloudstack-pull-analysis/server/hs_err_pid22300.log
#
SUREFIRE-859: # If you would like to submit a bug report, please visit:
# http://bugreport.sun.com/bugreport/crash.jsp
# The crash happened outside the Java Virtual Machine in native code.
# See problematic frame for where to report the bug.
#
```
I think java needs updating on the slave (or we need to reconfigure the job to use a better version)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-154032129
@koushik-das you said 'LogLevel is used in the latter one only.'. That is not true, I think. see com.cloud.agent.transport.RequestTest
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153681687
@DaanHoogland we can simply use @LogLevel I simply shared that there is a slight chance on confusion in future, and do we then remove the requestHasSensitiveInfo/responseHasSensitiveInfo from Param/Apis throughout the codebase?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-158347545
@koushik-das sorry, no. I have not much test infra and run a lot of tests. Could you force push your changes, please. I'll review your last changes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153001614
Jenkins build failed with the following error. I saw some other PRs with the same failure. So looks like some random issue.
[ERROR] SLF4J: Class path contains multiple SLF4J bindings.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153690595
@DaanHoogland if you look at the changes or the code that takes in Object (Response object) and serializes it; we create the json or xml response string by manually adding the delimiters ({, }, <, >, : characters etc). I was wondering if there is a way to avoid that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153677208
@bhaisaab can you explain ? I like your comment about the annotation not being descriptive on the sensitivity of the field. So for the best solution we would have to write another annotation for that. It would be tchnical similar to the @LogLevel and the paramter of the @APICommand. thoughts?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-152971700
I have tried to look for the existing API response objects based on some keyword search for sensitive fields. Please let me know if some response objects are left out.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1021#discussion_r44210903
--- Diff: server/src/com/cloud/api/ApiResponseGsonHelper.java ---
@@ -71,4 +83,19 @@ public boolean shouldSkipField(FieldAttributes f) {
return false;
}
}
+
+ private static class LogExclusionStrategy extends ApiResponseExclusionStrategy implements ExclusionStrategy {
+ public boolean shouldSkipClass(Class<?> arg0) {
+ return false;
+ }
+
+ public boolean shouldSkipField(FieldAttributes f) {
+ boolean skip = super.shouldSkipField(f);
--- End diff --
maybe peformance can be improved by handling thsi first and super afterwards (it is more code)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1021#discussion_r43898186
--- Diff: api/src/org/apache/cloudstack/api/response/UserVmResponse.java ---
@@ -256,6 +259,7 @@
@SerializedName(ApiConstants.SSH_KEYPAIR)
@Param(description = "ssh key-pair")
+ @LogLevel(Log4jLevel.Off)
--- End diff --
why is the keypair name sensitive?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153749695
```
2015-11-04 14:23:51,076 DEBUG [c.c.v.VmWorkJobHandlerProxy] (Work-Job-Executor-34:ctx-0d2ee513 job-104/job-105 ctx-0d5c6252) Done executing VM work job: com.cloud.vm.VmWorkStart{"dcId
":1,"podId":1,"clusterId":1,"hostId":2,"rawParams":{"VmPassword":"<snip/>"},"userId":2,"accountId":2,"vmId":13,"handlerName":"VirtualMachineManagerImpl"}
```
And some other examples in there. I am looking further. @koushik-das this is going to take a lot of work to get right, right? Like this it is not done I think.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153783852
I think we should add isSensitive() to @Parameter and use that.
Also we should base this on gson 2+ and not on the version we are presently using.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1021#discussion_r45461347
--- Diff: server/src/com/cloud/api/response/ApiResponseSerializer.java ---
@@ -127,53 +151,74 @@ public static String toJSONSerializedString(ResponseObject result) {
} else {
sb.append("{}");
}
+ String logStr = logBuilder.toJson(result);
+ if (logStr != null && !logStr.isEmpty()) {
+ logStr = unescape(logStr);
+ if (result instanceof AsyncJobResponse || result instanceof CreateCmdResponse || result instanceof AuthenticationCmdResponse) {
+ log.append(logStr);
+ } else {
+ log.append("{\"").append(result.getObjectName()).append("\":").append(logStr).append("}");
+ }
+ } else {
+ log.append("{}");
+ }
}
sb.append("}");
+ log.append("}");
return sb.toString();
}
return null;
}
- private static String toXMLSerializedString(ResponseObject result) {
- StringBuilder sb = new StringBuilder();
- sb.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>");
- sb.append("<").append(result.getResponseName()).append(" cloud-stack-version=\"").append(ApiDBUtils.getVersion()).append("\">");
+ private static String toXMLSerializedString(ResponseObject result, StringBuilder log) {
+ if (result != null && log != null) {
+ StringBuilder sb = new StringBuilder();
+ sb.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>");
+ sb.append("<").append(result.getResponseName()).append(" cloud-stack-version=\"").append(ApiDBUtils.getVersion()).append("\">");
+ log.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>");
+ log.append("<").append(result.getResponseName()).append(" cloud-stack-version=\"").append(ApiDBUtils.getVersion()).append("\">");
- if (result instanceof ListResponse) {
- Integer count = ((ListResponse)result).getCount();
+ if (result instanceof ListResponse) {
+ Integer count = ((ListResponse)result).getCount();
- if (count != null && count != 0) {
- sb.append("<").append(ApiConstants.COUNT).append(">").append(((ListResponse)result).getCount()).append("</").append(ApiConstants.COUNT).append(">");
- }
- List<? extends ResponseObject> responses = ((ListResponse)result).getResponses();
- if ((responses != null) && !responses.isEmpty()) {
- for (ResponseObject obj : responses) {
- serializeResponseObjXML(sb, obj);
+ if (count != null && count != 0) {
+ sb.append("<").append(ApiConstants.COUNT).append(">").append(((ListResponse)result).getCount()).append("</").append(ApiConstants.COUNT).append(">");
+ log.append("<").append(ApiConstants.COUNT).append(">").append(((ListResponse)result).getCount()).append("</").append(ApiConstants.COUNT).append(">");
+ }
+ List<? extends ResponseObject> responses = ((ListResponse)result).getResponses();
+ if ((responses != null) && !responses.isEmpty()) {
+ for (ResponseObject obj : responses) {
+ serializeResponseObjXML(sb, log, obj);
+ }
}
- }
- } else {
- if (result instanceof CreateCmdResponse || result instanceof AsyncJobResponse || result instanceof AuthenticationCmdResponse) {
- serializeResponseObjFieldsXML(sb, result);
} else {
- serializeResponseObjXML(sb, result);
+ if (result instanceof CreateCmdResponse || result instanceof AsyncJobResponse || result instanceof AuthenticationCmdResponse) {
+ serializeResponseObjFieldsXML(sb, log, result);
+ } else {
+ serializeResponseObjXML(sb, log, result);
+ }
}
- }
- sb.append("</").append(result.getResponseName()).append(">");
- return sb.toString();
+ sb.append("</").append(result.getResponseName()).append(">");
+ log.append("</").append(result.getResponseName()).append(">");
--- End diff --
@koushik-das I see this happening in a few places. I'm fine with it but from a performance point of view it might not be optimal.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153327157
@bhaisaab What are your reasons for saying that @LogLevel(Log4jLevel.Off) is inappropriate?
By sensitive param/annotation are you referring to the existing @APICommand annotation params requestHasSensitiveInfo/responseHasSensitiveInfo? The problem with this approach is that it doesn't tell about the specific field which is sensitive and so the entire response needs to be searched for sensitive data by means of regex which is very bad in terms of performance.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153667726
@koushik-das I think it's inappropriate as we're not marking those fields to note that they have hold sensitive information; but anyway that works
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153337620
@DaanHoogland @bhaisaab I also couldn't find any annotation on API fields for this. So ended up using the LogLevel which is also used in the agent commands.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153309944
Usage of @LogLevel(Log4jLevel.Off) seems inappropriate to avoid logging sensitive information, why not still check the sensitive param/annotation to log or not log data?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-154025365
@DaanHoogland About the issue you pointed out in com.cloud.vm.VmWorkStart serialization. Nice find but these are already existing issues. They surely needs fixing but I feel a separate effort is better. What do you say?
LogLevel can be safely used. If you look at the code there are 2 different GSON serializers - one for serialising the command that needs to be transmitted over the wire, and another is for logging purposes only. LogLevel is used in the latter one only.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153704575
@koushik-das makes sense so we have a policy that for any sensitive field LogLevel(LogLevel.OFF) be used. We should add that as a comment to the enum to document it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1021#discussion_r43898594
--- Diff: server/src/com/cloud/api/ApiResponseGsonHelper.java ---
@@ -27,30 +29,40 @@
import com.google.gson.GsonBuilder;
/**
- * The ApiResonseGsonHelper is different from ApiGsonHelper - it registeres one more adapter for String type required for api response encoding
+ * The ApiResonseGsonHelper is different from ApiGsonHelper - it registers one more adapter for String type required for api response encoding
--- End diff --
We should upgrade gson before further expanding on this feature of it. Exlusion strategies have somewhat changed in version 2 in the sense that exclusion is cahced inside gson and a change in loglevel is not endorsed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1021#discussion_r43897789
--- Diff: api/src/org/apache/cloudstack/api/response/SslCertResponse.java ---
@@ -62,10 +65,12 @@
@SerializedName(ApiConstants.CERTIFICATE_CHAIN)
@Param(description = "certificate chain")
+ @LogLevel(Log4jLevel.Off)
private String certchain;
@SerializedName(ApiConstants.CERTIFICATE_FINGERPRINT)
@Param(description = "certificate fingerprint")
+ @LogLevel(Log4jLevel.Off)
--- End diff --
fingerprint is not sensitive (though it doesn't need to logged either)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153330895
@bhaisaab @koushik-das Isn't there an annotation on field level for APIs as well? A change was accepted for that. I am looking for it and will update this comment.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153686801
@koushik-das You don't agree that the name LogLevel obfuscates that it is used to hide sensitive data?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153668149
Also, it is possible to avoid serializing the objects by hand (like adding/appending {, >, <, } etc by hand?). Probably use any available library, and if that's not possible write unit tests to do some crud like comparisons?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-153338874
The field level annotation was discussed as a prefered solution but in the end a regexp on field names was used. I am fine with re-using the LogLevel annotation. It does what is needed and the name suggests this use.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8485: listAPIs are taking too ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1021#issuecomment-158617888
@bhaisaab @jlk want to review give a second lgtm?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---