[GitHub] blueorangutan commented on issue #2514: [CLOUDSTACK-10346] Problem with NAT configuration and VMs not accessing each other via public IPs
blueorangutan commented on issue #2514: [CLOUDSTACK-10346] Problem with NAT configuration and VMs not accessing each other via public IPs URL: https://github.com/apache/cloudstack/pull/2514#issuecomment-382146230 Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1939 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2514: [CLOUDSTACK-10346] Problem with NAT configuration and VMs not accessing each other via public IPs
blueorangutan commented on issue #2514: [CLOUDSTACK-10346] Problem with NAT configuration and VMs not accessing each other via public IPs URL: https://github.com/apache/cloudstack/pull/2514#issuecomment-382134856 @rafaelweingartner a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update
rafaelweingartner commented on issue #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update URL: https://github.com/apache/cloudstack/pull/2574#issuecomment-382061698 @nitin-maharana thank you for the review ;) Thanks! This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] nitin-maharana commented on issue #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update
nitin-maharana commented on issue #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update URL: https://github.com/apache/cloudstack/pull/2574#issuecomment-382061346 Thanks, @rafaelweingartner for making the changes, I will review rest of the changes by tomorrow or day after. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2577: Prevent NPE if guest OS mapping is missing while prioritizing hosts
blueorangutan commented on issue #2577: Prevent NPE if guest OS mapping is missing while prioritizing hosts URL: https://github.com/apache/cloudstack/pull/2577#issuecomment-382055913 Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1938 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] khos2ow commented on a change in pull request #2577: Prevent NPE if guest OS mapping is missing while prioritizing hosts
khos2ow commented on a change in pull request #2577: Prevent NPE if guest OS mapping is missing while prioritizing hosts URL: https://github.com/apache/cloudstack/pull/2577#discussion_r182139455 ## File path: server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java ## @@ -417,6 +419,10 @@ public boolean isVirtualMachineUpgradable(VirtualMachine vm, ServiceOffering off // Determine the guest OS category of the template String templateGuestOSCategory = getTemplateGuestOSCategory(template); +if (Strings.isNullOrEmpty(templateGuestOSCategory)) { Review comment: Technically `templateGuestOSCategory == null` would be enough, and I cannot think of a way `getTemplateGuestOSCategory` returning empty string unless the database record integrity has been changed, that's why I used `Strings.isNullOrEmpty` rather rather than a simple `== null` check. (which might be slightly overkill at the same time!) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2577: Prevent NPE if guest OS mapping is missing while prioritizing hosts
rafaelweingartner commented on a change in pull request #2577: Prevent NPE if guest OS mapping is missing while prioritizing hosts URL: https://github.com/apache/cloudstack/pull/2577#discussion_r182134818 ## File path: server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java ## @@ -417,6 +419,10 @@ public boolean isVirtualMachineUpgradable(VirtualMachine vm, ServiceOffering off // Determine the guest OS category of the template String templateGuestOSCategory = getTemplateGuestOSCategory(template); +if (Strings.isNullOrEmpty(templateGuestOSCategory)) { Review comment: if you return a `null` value in `getTemplateGuestOSCategory` method, why not simply check `templateGuestOSCategory == null`? Is it possible to have an empty or blank `guestOSCategory.name`? BTW: `Strings.isNullOrEmpty` will return `false` for blanks. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2577: Prevent NPE if guest OS mapping is missing while prioritizing hosts
blueorangutan commented on issue #2577: Prevent NPE if guest OS mapping is missing while prioritizing hosts URL: https://github.com/apache/cloudstack/pull/2577#issuecomment-382046711 @khos2ow a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] khos2ow opened a new pull request #2577: Prevent NPE if guest OS mapping is missing while prioritizing hosts
khos2ow opened a new pull request #2577: Prevent NPE if guest OS mapping is missing while prioritizing hosts URL: https://github.com/apache/cloudstack/pull/2577 ## Description When starting an existing VM and trying to prioritize hosts, if Guest OS mapping has been removed from that host, there will be a NullPointerException which prevents the VM to be started successfully. ## Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [x] Bug fix (non-breaking change which fixes an issue) - [ ] Enhancement (improves an existing feature and functionality) - [ ] Cleanup (Code refactoring and cleanup, that may add test cases) ## GitHub Issue/PRs ## Screenshots (if appropriate): ## How Has This Been Tested? ## Checklist: - [ ] I have read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document. - [ ] My code follows the code style of this project. - [ ] My change requires a change to the documentation. - [ ] I have updated the documentation accordingly. Testing - [ ] I have added tests to cover my changes. - [ ] All relevant new and existing integration tests have passed. - [ ] A full integration testsuite with all test that can run on my environment has passed. @blueorangutan package This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2573: Cloudstack 10356
rafaelweingartner commented on a change in pull request #2573: Cloudstack 10356 URL: https://github.com/apache/cloudstack/pull/2573#discussion_r182091684 ## File path: server/src/main/java/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java ## @@ -740,14 +740,14 @@ public boolean startRemoteAccessVpn(final RemoteAccessVpn vpn, final VirtualRout throw new AgentUnavailableException("Unable to send commands to virtual router ", router.getHostId(), e); } Answer answer = cmds.getAnswer("users"); -if (!answer.getResult()) { +if (answer != null && !answer.getResult()) { Review comment: Sorry, I did not understand your question. Are you asking if I would suggest to use BooleanUtils here as well? Please, go ahead and do that. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] lujiefsi commented on a change in pull request #2573: Cloudstack 10356
lujiefsi commented on a change in pull request #2573: Cloudstack 10356 URL: https://github.com/apache/cloudstack/pull/2573#discussion_r182090707 ## File path: server/src/main/java/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java ## @@ -740,14 +740,14 @@ public boolean startRemoteAccessVpn(final RemoteAccessVpn vpn, final VirtualRout throw new AgentUnavailableException("Unable to send commands to virtual router ", router.getHostId(), e); } Answer answer = cmds.getAnswer("users"); -if (!answer.getResult()) { +if (answer != null && !answer.getResult()) { Review comment: hum, here is not change as @rafaelweingartner suggest This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
blueorangutan commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381998091 @rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rhtyd commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381997838 @blueorangutan test This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2524: [CLOUDSTACK-9261] Upgrate jQuery-UI to 1.11 (JQuery UI 1.8.4 prone to XSS)
rafaelweingartner commented on issue #2524: [CLOUDSTACK-9261] Upgrate jQuery-UI to 1.11 (JQuery UI 1.8.4 prone to XSS) URL: https://github.com/apache/cloudstack/pull/2524#issuecomment-381990262 Thanks @GabrielBrascher! I fixed both of the issues you raised. What do you guys think? Should I change the non-minified version of Jquery-Ui to the minified one? Others (@khos2ow, @rhtyd, @DaanHoogland and so on), could you also help here? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
blueorangutan commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381988851 Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1937 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
blueorangutan commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381979907 @rafaelweingartner a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rafaelweingartner commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381978823 @blueorangutan package. @rhtyd I think everything is fine now. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update
rafaelweingartner commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182051964 ## File path: server/src/main/java/com/cloud/user/AccountManagerImpl.java ## @@ -1153,157 +1152,246 @@ public UserVO createUser(String userName, String password, String firstName, Str @Override @ActionEvent(eventType = EventTypes.EVENT_USER_CREATE, eventDescription = "creating User") -public UserVO createUser(String userName, String password, String firstName, String lastName, String email, String timeZone, String accountName, Long domainId, -String userUUID) { +public UserVO createUser(String userName, String password, String firstName, String lastName, String email, String timeZone, String accountName, Long domainId, String userUUID) { return createUser(userName, password, firstName, lastName, email, timeZone, accountName, domainId, userUUID, User.Source.UNKNOWN); } @Override -@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = "updating User") -public UserAccount updateUser(Long userId, String firstName, String lastName, String email, String userName, String password, String apiKey, String secretKey, -String timeZone) { -// Input validation -UserVO user = _userDao.getUser(userId); +@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = "Updating User") +public UserAccount updateUser(UpdateUserCmd updateUserCmd) { +UserVO user = retrieveAndValidateUser(updateUserCmd); +s_logger.debug("Updating user with Id: " + user.getUuid()); -if (user == null) { -throw new InvalidParameterValueException("unable to find user by id"); -} +validateAndUpdatApiAndSecretKeyIfNeeded(updateUserCmd, user); +Account account = retrieveAndValidateAccount(user); -if ((apiKey == null && secretKey != null) || (apiKey != null && secretKey == null)) { -throw new InvalidParameterValueException("Please provide an userApiKey/userSecretKey pair"); -} +validateAndUpdateFirstNameIfNeeded(updateUserCmd, user); +validateAndUpdateLastNameIfNeeded(updateUserCmd, user); +validateAndUpdateUsernameIfNeeded(updateUserCmd, user, account); -// If the account is an admin type, return an error. We do not allow this -Account account = _accountDao.findById(user.getAccountId()); -if (account == null) { -throw new InvalidParameterValueException("unable to find user account " + user.getAccountId()); +validateUserPasswordAndUpdateIfNeeded(updateUserCmd.getPassword(), user, updateUserCmd.getOldPassword()); +String email = updateUserCmd.getEmail(); +if (StringUtils.isNotBlank(email)) { +user.setEmail(email); } - -// don't allow updating project account -if (account.getType() == Account.ACCOUNT_TYPE_PROJECT) { -throw new InvalidParameterValueException("unable to find user by id"); +String timezone = updateUserCmd.getTimezone(); +if (StringUtils.isNotBlank(timezone)) { +user.setTimezone(timezone); +} +_userDao.update(user.getId(), user); +return _userAccountDao.findById(user.getId()); +} + +/** + * Updates the password in the user POJO if needed. If no password is provided, then the password is not updated. + * The following validations are executed if 'password' is not null. Admins (root admins or domain admins) can execute password updates without entering the old password. + * + * If 'password' is blank, we throw an {@link InvalidParameterValueException}; + * If old password is not provided and user is not an Admin, we throw an {@link InvalidParameterValueException}; + * If a normal user is calling this method, we use {@link #validateOldPassword(UserVO, String)} to check if the provided old password matches the database one; + * + * + * If all checks pass, we encode the given password with the most preferable password mechanism given in {@link #_userPasswordEncoders}. + */ +protected void validateUserPasswordAndUpdateIfNeeded(String password, UserVO user, String oldPassword) { +if (password == null) { +s_logger.trace("No new password to update for user: " + user.getUuid()); +return; } - -// don't allow updating system account -if (account.getId() == Account.ACCOUNT_ID_SYSTEM) { -throw new PermissionDeniedException("user id : " + userId + " is system account, update is not allowed"); +if (StringUtils.isBlank(password)) { +throw new InvalidParameterValueException("Password cannot be empty or blank."); +} +Account cal
[GitHub] rafaelweingartner commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update
rafaelweingartner commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182051870 ## File path: ui/scripts/accounts.js ## @@ -1476,6 +1476,14 @@ form: { title: 'label.action.change.password', fields: { +oldPassword: { +label: 'label.old.password', +isPassword: true, +validation: { +required: !(isAdmin() || isDomainAdmin()) Review comment: I thought about that, but I was under the impression that the field would still be validated by the javascript framework we have. I could not find a `hide` parameter. Therefore, I am hiding it manually. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update
rafaelweingartner commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182051594 ## File path: server/src/main/java/com/cloud/user/AccountManagerImpl.java ## @@ -1153,157 +1152,246 @@ public UserVO createUser(String userName, String password, String firstName, Str @Override @ActionEvent(eventType = EventTypes.EVENT_USER_CREATE, eventDescription = "creating User") -public UserVO createUser(String userName, String password, String firstName, String lastName, String email, String timeZone, String accountName, Long domainId, -String userUUID) { +public UserVO createUser(String userName, String password, String firstName, String lastName, String email, String timeZone, String accountName, Long domainId, String userUUID) { return createUser(userName, password, firstName, lastName, email, timeZone, accountName, domainId, userUUID, User.Source.UNKNOWN); } @Override -@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = "updating User") -public UserAccount updateUser(Long userId, String firstName, String lastName, String email, String userName, String password, String apiKey, String secretKey, -String timeZone) { -// Input validation -UserVO user = _userDao.getUser(userId); +@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = "Updating User") +public UserAccount updateUser(UpdateUserCmd updateUserCmd) { +UserVO user = retrieveAndValidateUser(updateUserCmd); +s_logger.debug("Updating user with Id: " + user.getUuid()); -if (user == null) { -throw new InvalidParameterValueException("unable to find user by id"); -} +validateAndUpdatApiAndSecretKeyIfNeeded(updateUserCmd, user); +Account account = retrieveAndValidateAccount(user); -if ((apiKey == null && secretKey != null) || (apiKey != null && secretKey == null)) { -throw new InvalidParameterValueException("Please provide an userApiKey/userSecretKey pair"); -} +validateAndUpdateFirstNameIfNeeded(updateUserCmd, user); +validateAndUpdateLastNameIfNeeded(updateUserCmd, user); +validateAndUpdateUsernameIfNeeded(updateUserCmd, user, account); -// If the account is an admin type, return an error. We do not allow this -Account account = _accountDao.findById(user.getAccountId()); -if (account == null) { -throw new InvalidParameterValueException("unable to find user account " + user.getAccountId()); +validateUserPasswordAndUpdateIfNeeded(updateUserCmd.getPassword(), user, updateUserCmd.getOldPassword()); +String email = updateUserCmd.getEmail(); +if (StringUtils.isNotBlank(email)) { +user.setEmail(email); } - -// don't allow updating project account -if (account.getType() == Account.ACCOUNT_TYPE_PROJECT) { -throw new InvalidParameterValueException("unable to find user by id"); +String timezone = updateUserCmd.getTimezone(); +if (StringUtils.isNotBlank(timezone)) { +user.setTimezone(timezone); +} +_userDao.update(user.getId(), user); +return _userAccountDao.findById(user.getId()); +} + +/** + * Updates the password in the user POJO if needed. If no password is provided, then the password is not updated. + * The following validations are executed if 'password' is not null. Admins (root admins or domain admins) can execute password updates without entering the old password. + * + * If 'password' is blank, we throw an {@link InvalidParameterValueException}; + * If old password is not provided and user is not an Admin, we throw an {@link InvalidParameterValueException}; + * If a normal user is calling this method, we use {@link #validateOldPassword(UserVO, String)} to check if the provided old password matches the database one; + * + * + * If all checks pass, we encode the given password with the most preferable password mechanism given in {@link #_userPasswordEncoders}. + */ +protected void validateUserPasswordAndUpdateIfNeeded(String password, UserVO user, String oldPassword) { Review comment: done This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update
nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182041837 ## File path: server/src/main/java/com/cloud/user/AccountManagerImpl.java ## @@ -1153,157 +1152,246 @@ public UserVO createUser(String userName, String password, String firstName, Str @Override @ActionEvent(eventType = EventTypes.EVENT_USER_CREATE, eventDescription = "creating User") -public UserVO createUser(String userName, String password, String firstName, String lastName, String email, String timeZone, String accountName, Long domainId, -String userUUID) { +public UserVO createUser(String userName, String password, String firstName, String lastName, String email, String timeZone, String accountName, Long domainId, String userUUID) { return createUser(userName, password, firstName, lastName, email, timeZone, accountName, domainId, userUUID, User.Source.UNKNOWN); } @Override -@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = "updating User") -public UserAccount updateUser(Long userId, String firstName, String lastName, String email, String userName, String password, String apiKey, String secretKey, -String timeZone) { -// Input validation -UserVO user = _userDao.getUser(userId); +@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = "Updating User") +public UserAccount updateUser(UpdateUserCmd updateUserCmd) { +UserVO user = retrieveAndValidateUser(updateUserCmd); +s_logger.debug("Updating user with Id: " + user.getUuid()); -if (user == null) { -throw new InvalidParameterValueException("unable to find user by id"); -} +validateAndUpdatApiAndSecretKeyIfNeeded(updateUserCmd, user); +Account account = retrieveAndValidateAccount(user); -if ((apiKey == null && secretKey != null) || (apiKey != null && secretKey == null)) { -throw new InvalidParameterValueException("Please provide an userApiKey/userSecretKey pair"); -} +validateAndUpdateFirstNameIfNeeded(updateUserCmd, user); +validateAndUpdateLastNameIfNeeded(updateUserCmd, user); +validateAndUpdateUsernameIfNeeded(updateUserCmd, user, account); -// If the account is an admin type, return an error. We do not allow this -Account account = _accountDao.findById(user.getAccountId()); -if (account == null) { -throw new InvalidParameterValueException("unable to find user account " + user.getAccountId()); +validateUserPasswordAndUpdateIfNeeded(updateUserCmd.getPassword(), user, updateUserCmd.getOldPassword()); +String email = updateUserCmd.getEmail(); +if (StringUtils.isNotBlank(email)) { +user.setEmail(email); } - -// don't allow updating project account -if (account.getType() == Account.ACCOUNT_TYPE_PROJECT) { -throw new InvalidParameterValueException("unable to find user by id"); +String timezone = updateUserCmd.getTimezone(); +if (StringUtils.isNotBlank(timezone)) { +user.setTimezone(timezone); +} +_userDao.update(user.getId(), user); +return _userAccountDao.findById(user.getId()); +} + +/** + * Updates the password in the user POJO if needed. If no password is provided, then the password is not updated. + * The following validations are executed if 'password' is not null. Admins (root admins or domain admins) can execute password updates without entering the old password. + * + * If 'password' is blank, we throw an {@link InvalidParameterValueException}; + * If old password is not provided and user is not an Admin, we throw an {@link InvalidParameterValueException}; + * If a normal user is calling this method, we use {@link #validateOldPassword(UserVO, String)} to check if the provided old password matches the database one; + * + * + * If all checks pass, we encode the given password with the most preferable password mechanism given in {@link #_userPasswordEncoders}. + */ +protected void validateUserPasswordAndUpdateIfNeeded(String password, UserVO user, String oldPassword) { +if (password == null) { +s_logger.trace("No new password to update for user: " + user.getUuid()); +return; } - -// don't allow updating system account -if (account.getId() == Account.ACCOUNT_ID_SYSTEM) { -throw new PermissionDeniedException("user id : " + userId + " is system account, update is not allowed"); +if (StringUtils.isBlank(password)) { +throw new InvalidParameterValueException("Password cannot be empty or blank."); +} +Account calling
[GitHub] nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update
nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182039920 ## File path: ui/scripts/accounts.js ## @@ -1476,6 +1476,14 @@ form: { title: 'label.action.change.password', fields: { +oldPassword: { +label: 'label.old.password', +isPassword: true, +validation: { +required: !(isAdmin() || isDomainAdmin()) Review comment: As we are hiding this field for admin and domain admin. I think we can assign the value true for the required parameter. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update
nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182038769 ## File path: server/src/main/java/com/cloud/user/AccountManagerImpl.java ## @@ -1153,157 +1152,246 @@ public UserVO createUser(String userName, String password, String firstName, Str @Override @ActionEvent(eventType = EventTypes.EVENT_USER_CREATE, eventDescription = "creating User") -public UserVO createUser(String userName, String password, String firstName, String lastName, String email, String timeZone, String accountName, Long domainId, -String userUUID) { +public UserVO createUser(String userName, String password, String firstName, String lastName, String email, String timeZone, String accountName, Long domainId, String userUUID) { return createUser(userName, password, firstName, lastName, email, timeZone, accountName, domainId, userUUID, User.Source.UNKNOWN); } @Override -@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = "updating User") -public UserAccount updateUser(Long userId, String firstName, String lastName, String email, String userName, String password, String apiKey, String secretKey, -String timeZone) { -// Input validation -UserVO user = _userDao.getUser(userId); +@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = "Updating User") +public UserAccount updateUser(UpdateUserCmd updateUserCmd) { +UserVO user = retrieveAndValidateUser(updateUserCmd); +s_logger.debug("Updating user with Id: " + user.getUuid()); -if (user == null) { -throw new InvalidParameterValueException("unable to find user by id"); -} +validateAndUpdatApiAndSecretKeyIfNeeded(updateUserCmd, user); +Account account = retrieveAndValidateAccount(user); -if ((apiKey == null && secretKey != null) || (apiKey != null && secretKey == null)) { -throw new InvalidParameterValueException("Please provide an userApiKey/userSecretKey pair"); -} +validateAndUpdateFirstNameIfNeeded(updateUserCmd, user); +validateAndUpdateLastNameIfNeeded(updateUserCmd, user); +validateAndUpdateUsernameIfNeeded(updateUserCmd, user, account); -// If the account is an admin type, return an error. We do not allow this -Account account = _accountDao.findById(user.getAccountId()); -if (account == null) { -throw new InvalidParameterValueException("unable to find user account " + user.getAccountId()); +validateUserPasswordAndUpdateIfNeeded(updateUserCmd.getPassword(), user, updateUserCmd.getOldPassword()); +String email = updateUserCmd.getEmail(); +if (StringUtils.isNotBlank(email)) { +user.setEmail(email); } - -// don't allow updating project account -if (account.getType() == Account.ACCOUNT_TYPE_PROJECT) { -throw new InvalidParameterValueException("unable to find user by id"); +String timezone = updateUserCmd.getTimezone(); +if (StringUtils.isNotBlank(timezone)) { +user.setTimezone(timezone); +} +_userDao.update(user.getId(), user); +return _userAccountDao.findById(user.getId()); +} + +/** + * Updates the password in the user POJO if needed. If no password is provided, then the password is not updated. + * The following validations are executed if 'password' is not null. Admins (root admins or domain admins) can execute password updates without entering the old password. + * + * If 'password' is blank, we throw an {@link InvalidParameterValueException}; + * If old password is not provided and user is not an Admin, we throw an {@link InvalidParameterValueException}; + * If a normal user is calling this method, we use {@link #validateOldPassword(UserVO, String)} to check if the provided old password matches the database one; + * + * + * If all checks pass, we encode the given password with the most preferable password mechanism given in {@link #_userPasswordEncoders}. + */ +protected void validateUserPasswordAndUpdateIfNeeded(String password, UserVO user, String oldPassword) { Review comment: I mean the first parameter (password -> newPassword) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rhtyd commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381955944 let me kick tests @blueorangutan package This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rhtyd commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381956264 Nevermind, @rafaelweingartner please kick packaging job with BO once you're able to re-fix the styles issues. Sorry for the additional trouble on porting to 4.11. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rafaelweingartner commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#discussion_r182038641 ## File path: tools/travis/before_install.sh ## @@ -97,10 +99,11 @@ echo " echo -e "\nInstalling some python packages: " pip install --user --upgrade pip +pip uninstall pylint for ((i=0;i<$RETRY_COUNT;i++)) do - pip install --user --upgrade lxml paramiko nose texttable ipmisim pyopenssl mock flask netaddr pylint pep8 > /tmp/piplog + pip install --user --upgrade lxml paramiko texttable ipmisim pyopenssl mock flask netaddr nose pylint pycodestyle six astroid Markdown > /tmp/piplog Review comment: That is what I expected to happen, but it seems it did not work. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rhtyd commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381955944 let me kick tests @blueorangutan package This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rafaelweingartner commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381955657 Well, now, the error in Travis are related to the code style. I will apply the code style fixes too then. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#discussion_r182038138 ## File path: tools/travis/before_install.sh ## @@ -97,10 +99,11 @@ echo " echo -e "\nInstalling some python packages: " pip install --user --upgrade pip +pip uninstall pylint for ((i=0;i<$RETRY_COUNT;i++)) do - pip install --user --upgrade lxml paramiko nose texttable ipmisim pyopenssl mock flask netaddr pylint pep8 > /tmp/piplog + pip install --user --upgrade lxml paramiko texttable ipmisim pyopenssl mock flask netaddr nose pylint pycodestyle six astroid Markdown > /tmp/piplog Review comment: If dependencies are already installed, it should not cause an issue. The `--upgrade` should upgrade an installed package and its dependencies, but nonethless let's see how the Travis run goes. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update
rafaelweingartner commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182035843 ## File path: server/src/main/java/com/cloud/user/AccountManagerImpl.java ## @@ -1153,157 +1152,246 @@ public UserVO createUser(String userName, String password, String firstName, Str @Override @ActionEvent(eventType = EventTypes.EVENT_USER_CREATE, eventDescription = "creating User") -public UserVO createUser(String userName, String password, String firstName, String lastName, String email, String timeZone, String accountName, Long domainId, -String userUUID) { +public UserVO createUser(String userName, String password, String firstName, String lastName, String email, String timeZone, String accountName, Long domainId, String userUUID) { return createUser(userName, password, firstName, lastName, email, timeZone, accountName, domainId, userUUID, User.Source.UNKNOWN); } @Override -@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = "updating User") -public UserAccount updateUser(Long userId, String firstName, String lastName, String email, String userName, String password, String apiKey, String secretKey, -String timeZone) { -// Input validation -UserVO user = _userDao.getUser(userId); +@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = "Updating User") +public UserAccount updateUser(UpdateUserCmd updateUserCmd) { +UserVO user = retrieveAndValidateUser(updateUserCmd); +s_logger.debug("Updating user with Id: " + user.getUuid()); -if (user == null) { -throw new InvalidParameterValueException("unable to find user by id"); -} +validateAndUpdatApiAndSecretKeyIfNeeded(updateUserCmd, user); +Account account = retrieveAndValidateAccount(user); -if ((apiKey == null && secretKey != null) || (apiKey != null && secretKey == null)) { -throw new InvalidParameterValueException("Please provide an userApiKey/userSecretKey pair"); -} +validateAndUpdateFirstNameIfNeeded(updateUserCmd, user); +validateAndUpdateLastNameIfNeeded(updateUserCmd, user); +validateAndUpdateUsernameIfNeeded(updateUserCmd, user, account); -// If the account is an admin type, return an error. We do not allow this -Account account = _accountDao.findById(user.getAccountId()); -if (account == null) { -throw new InvalidParameterValueException("unable to find user account " + user.getAccountId()); +validateUserPasswordAndUpdateIfNeeded(updateUserCmd.getPassword(), user, updateUserCmd.getOldPassword()); +String email = updateUserCmd.getEmail(); +if (StringUtils.isNotBlank(email)) { +user.setEmail(email); } - -// don't allow updating project account -if (account.getType() == Account.ACCOUNT_TYPE_PROJECT) { -throw new InvalidParameterValueException("unable to find user by id"); +String timezone = updateUserCmd.getTimezone(); +if (StringUtils.isNotBlank(timezone)) { +user.setTimezone(timezone); +} +_userDao.update(user.getId(), user); +return _userAccountDao.findById(user.getId()); +} + +/** + * Updates the password in the user POJO if needed. If no password is provided, then the password is not updated. + * The following validations are executed if 'password' is not null. Admins (root admins or domain admins) can execute password updates without entering the old password. + * + * If 'password' is blank, we throw an {@link InvalidParameterValueException}; + * If old password is not provided and user is not an Admin, we throw an {@link InvalidParameterValueException}; + * If a normal user is calling this method, we use {@link #validateOldPassword(UserVO, String)} to check if the provided old password matches the database one; + * + * + * If all checks pass, we encode the given password with the most preferable password mechanism given in {@link #_userPasswordEncoders}. + */ +protected void validateUserPasswordAndUpdateIfNeeded(String password, UserVO user, String oldPassword) { Review comment: Do you mean `validateUserNewPasswordAndUpdateIfNeeded`? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update
rafaelweingartner commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182035508 ## File path: api/src/main/java/com/cloud/user/AccountService.java ## @@ -23,53 +23,28 @@ import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.command.admin.user.GetUserKeysCmd; import org.apache.cloudstack.api.command.admin.user.RegisterCmd; +import org.apache.cloudstack.api.command.admin.user.UpdateUserCmd; import com.cloud.domain.Domain; import com.cloud.exception.PermissionDeniedException; import com.cloud.offering.DiskOffering; import com.cloud.offering.ServiceOffering; - public interface AccountService { /** * Creates a new user and account, stores the password as is so encrypted passwords are recommended. - * Review comment: Well, I do like Java documentation; however, in this case, the use of `@param` does not bring benefits. I mean, if each one of the parameters was fully described it would be ok, but that is not what happened here. That is why I removed them. I am sorry Nitin, but I would rather not document those parameters. Instead, we should get rid of these methods with a lot of parameters. My changes affect only the `updateUser` method (I already refactored that whole method). Afterwards, I executed a cleanup in the classes that I had modified some code. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rafaelweingartner commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381950233 @rhtyd rebased against 4.11. I also added only the dependencies upgrade. Let's see what happens. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rafaelweingartner commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#discussion_r18205 ## File path: tools/travis/before_install.sh ## @@ -97,10 +99,11 @@ echo " echo -e "\nInstalling some python packages: " pip install --user --upgrade pip +pip uninstall pylint for ((i=0;i<$RETRY_COUNT;i++)) do - pip install --user --upgrade lxml paramiko nose texttable ipmisim pyopenssl mock flask netaddr pylint pep8 > /tmp/piplog + pip install --user --upgrade lxml paramiko texttable ipmisim pyopenssl mock flask netaddr nose pylint pycodestyle six astroid Markdown > /tmp/piplog Review comment: I think the problems happens when the dependencies are already installed. Then, it seems that for some cases `pip` does not upgrade them if you do not ask to. Again, I am not a Python guy, so that that hypothesis with a grain of salt. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rhtyd commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381948034 I'll kick test once the PR is re-targeted for 4.11 branch @rafaelweingartner This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#discussion_r182032036 ## File path: tools/travis/before_install.sh ## @@ -97,10 +99,11 @@ echo " echo -e "\nInstalling some python packages: " pip install --user --upgrade pip +pip uninstall pylint for ((i=0;i<$RETRY_COUNT;i++)) do - pip install --user --upgrade lxml paramiko nose texttable ipmisim pyopenssl mock flask netaddr pylint pep8 > /tmp/piplog + pip install --user --upgrade lxml paramiko texttable ipmisim pyopenssl mock flask netaddr nose pylint pycodestyle six astroid Markdown > /tmp/piplog Review comment: Another note, the new release of pip (v10) might deprecate few things -- I know python 2.6.x is not well supported now. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#discussion_r182031926 ## File path: tools/travis/before_install.sh ## @@ -97,10 +99,11 @@ echo " echo -e "\nInstalling some python packages: " pip install --user --upgrade pip +pip uninstall pylint for ((i=0;i<$RETRY_COUNT;i++)) do - pip install --user --upgrade lxml paramiko nose texttable ipmisim pyopenssl mock flask netaddr pylint pep8 > /tmp/piplog + pip install --user --upgrade lxml paramiko texttable ipmisim pyopenssl mock flask netaddr nose pylint pycodestyle six astroid Markdown > /tmp/piplog Review comment: @rafaelweingartner ideally, pip install should install additional dependencies we should not need to install dependencies for pylint manually. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#discussion_r182031799 ## File path: tools/travis/before_install.sh ## @@ -97,10 +99,11 @@ echo " echo -e "\nInstalling some python packages: " pip install --user --upgrade pip +pip uninstall pylint Review comment: Thanks, np I was just curious why we were doing it that way. (love the expression of the smiley btw) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rafaelweingartner commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381945912 @rhtyd Locally my tests were also passing :(... Actually, locally I received those problems regarding code formatting. That is why I fixed them too. However, even after fixing all of them, Travis would not work. Then, I started digging deeper in the dependencies and looking for incompatibilities (this is a nightmare!). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update
nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182030109 ## File path: server/src/main/java/com/cloud/user/AccountManagerImpl.java ## @@ -1153,157 +1152,246 @@ public UserVO createUser(String userName, String password, String firstName, Str @Override @ActionEvent(eventType = EventTypes.EVENT_USER_CREATE, eventDescription = "creating User") -public UserVO createUser(String userName, String password, String firstName, String lastName, String email, String timeZone, String accountName, Long domainId, -String userUUID) { +public UserVO createUser(String userName, String password, String firstName, String lastName, String email, String timeZone, String accountName, Long domainId, String userUUID) { return createUser(userName, password, firstName, lastName, email, timeZone, accountName, domainId, userUUID, User.Source.UNKNOWN); } @Override -@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = "updating User") -public UserAccount updateUser(Long userId, String firstName, String lastName, String email, String userName, String password, String apiKey, String secretKey, -String timeZone) { -// Input validation -UserVO user = _userDao.getUser(userId); +@ActionEvent(eventType = EventTypes.EVENT_USER_UPDATE, eventDescription = "Updating User") +public UserAccount updateUser(UpdateUserCmd updateUserCmd) { +UserVO user = retrieveAndValidateUser(updateUserCmd); +s_logger.debug("Updating user with Id: " + user.getUuid()); -if (user == null) { -throw new InvalidParameterValueException("unable to find user by id"); -} +validateAndUpdatApiAndSecretKeyIfNeeded(updateUserCmd, user); +Account account = retrieveAndValidateAccount(user); -if ((apiKey == null && secretKey != null) || (apiKey != null && secretKey == null)) { -throw new InvalidParameterValueException("Please provide an userApiKey/userSecretKey pair"); -} +validateAndUpdateFirstNameIfNeeded(updateUserCmd, user); +validateAndUpdateLastNameIfNeeded(updateUserCmd, user); +validateAndUpdateUsernameIfNeeded(updateUserCmd, user, account); -// If the account is an admin type, return an error. We do not allow this -Account account = _accountDao.findById(user.getAccountId()); -if (account == null) { -throw new InvalidParameterValueException("unable to find user account " + user.getAccountId()); +validateUserPasswordAndUpdateIfNeeded(updateUserCmd.getPassword(), user, updateUserCmd.getOldPassword()); +String email = updateUserCmd.getEmail(); +if (StringUtils.isNotBlank(email)) { +user.setEmail(email); } - -// don't allow updating project account -if (account.getType() == Account.ACCOUNT_TYPE_PROJECT) { -throw new InvalidParameterValueException("unable to find user by id"); +String timezone = updateUserCmd.getTimezone(); +if (StringUtils.isNotBlank(timezone)) { +user.setTimezone(timezone); +} +_userDao.update(user.getId(), user); +return _userAccountDao.findById(user.getId()); +} + +/** + * Updates the password in the user POJO if needed. If no password is provided, then the password is not updated. + * The following validations are executed if 'password' is not null. Admins (root admins or domain admins) can execute password updates without entering the old password. + * + * If 'password' is blank, we throw an {@link InvalidParameterValueException}; + * If old password is not provided and user is not an Admin, we throw an {@link InvalidParameterValueException}; + * If a normal user is calling this method, we use {@link #validateOldPassword(UserVO, String)} to check if the provided old password matches the database one; + * + * + * If all checks pass, we encode the given password with the most preferable password mechanism given in {@link #_userPasswordEncoders}. + */ +protected void validateUserPasswordAndUpdateIfNeeded(String password, UserVO user, String oldPassword) { Review comment: I think newPassword name would be more meaningful. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rafaelweingartner commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#discussion_r182029928 ## File path: tools/travis/before_install.sh ## @@ -97,10 +99,11 @@ echo " echo -e "\nInstalling some python packages: " pip install --user --upgrade pip +pip uninstall pylint for ((i=0;i<$RETRY_COUNT;i++)) do - pip install --user --upgrade lxml paramiko nose texttable ipmisim pyopenssl mock flask netaddr pylint pep8 > /tmp/piplog + pip install --user --upgrade lxml paramiko texttable ipmisim pyopenssl mock flask netaddr nose pylint pycodestyle six astroid Markdown > /tmp/piplog Review comment: `Markdown` I added because I was getting a message saying that it is not installed, but some of these dependencies might require it. That is why I installed. `six and astroid` seems to be used by `pylint` and `nose`. And I think that is the problem we were having. There is an issue regarding these dependencies versions, which are not automatically upgraded when you install some library the relies on them. `¯\_(ツ)_/¯` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rafaelweingartner commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#discussion_r182029173 ## File path: tools/travis/before_install.sh ## @@ -97,10 +99,11 @@ echo " echo -e "\nInstalling some python packages: " pip install --user --upgrade pip +pip uninstall pylint Review comment: ¯\_(ツ)_/¯... I really do not understand this. Most of the results I found on Google were telling me to remove and install again the library. I guess when people do not know what they are doing, they do this things. Well, I will remove and see what happens. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update
nitin-maharana commented on a change in pull request #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update URL: https://github.com/apache/cloudstack/pull/2574#discussion_r182028887 ## File path: api/src/main/java/com/cloud/user/AccountService.java ## @@ -23,53 +23,28 @@ import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.command.admin.user.GetUserKeysCmd; import org.apache.cloudstack.api.command.admin.user.RegisterCmd; +import org.apache.cloudstack.api.command.admin.user.UpdateUserCmd; import com.cloud.domain.Domain; import com.cloud.exception.PermissionDeniedException; import com.cloud.offering.DiskOffering; import com.cloud.offering.ServiceOffering; - public interface AccountService { /** * Creates a new user and account, stores the password as is so encrypted passwords are recommended. - * Review comment: @rafaelweingartner, Thanks for the fix. I think it's better to keep the comments, would be helpful in the documentation. Would be great if you describe a bit about each parameter. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rafaelweingartner commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rafaelweingartner commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#discussion_r182028493 ## File path: systemvm/test/runtests.sh ## @@ -21,9 +21,9 @@ export PYTHONPATH="../debian/opt/cloud/bin/" export PYTHONDONTWRITEBYTECODE=False -echo "Running pep8 to check systemvm/python code for errors" -pep8 --max-line-length=179 *py -pep8 --max-line-length=179 --exclude=monitorServices.py,baremetal-vr.py,passwd_server_ip.py `find ../debian -name \*.py` +echo "Running pycodestyle to check systemvm/python code for errors" +pycodestyle --max-line-length=179 *py Review comment: I was getting a message that `pep8` is deprecated and is being replaced by `pycodestyle`, that is why I replaced. It was one of my commits trying to solve the problem (or at least change some error message). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update
blueorangutan commented on issue #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update URL: https://github.com/apache/cloudstack/pull/2574#issuecomment-381895005 @borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
blueorangutan commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381894866 @borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2505: WIP CLOUDSTACK-10333: Secure Live VM Migration for KVM
blueorangutan commented on issue #2505: WIP CLOUDSTACK-10333: Secure Live VM Migration for KVM URL: https://github.com/apache/cloudstack/pull/2505#issuecomment-381894661 @borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] borisstoyanov commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
borisstoyanov commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381894577 @blueorangutan test This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2550: debian: Use only `-l` for libvirtd default file on debian
blueorangutan commented on issue #2550: debian: Use only `-l` for libvirtd default file on debian URL: https://github.com/apache/cloudstack/pull/2550#issuecomment-381894539 @borisstoyanov a Trillian-Jenkins test job (ubuntu mgmt + kvm-ubuntu) has been kicked to run smoke tests This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] borisstoyanov commented on issue #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update
borisstoyanov commented on issue #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update URL: https://github.com/apache/cloudstack/pull/2574#issuecomment-381894511 @blueorangutan test This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2376: [4.11] Smoketest Health Check
blueorangutan commented on issue #2376: [4.11] Smoketest Health Check URL: https://github.com/apache/cloudstack/pull/2376#issuecomment-381894276 @borisstoyanov a Trillian-Jenkins matrix job (centos6 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] borisstoyanov commented on issue #2550: debian: Use only `-l` for libvirtd default file on debian
borisstoyanov commented on issue #2550: debian: Use only `-l` for libvirtd default file on debian URL: https://github.com/apache/cloudstack/pull/2550#issuecomment-381894393 @blueorangutan test ubuntu kvm-ubuntu This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] borisstoyanov commented on issue #2505: WIP CLOUDSTACK-10333: Secure Live VM Migration for KVM
borisstoyanov commented on issue #2505: WIP CLOUDSTACK-10333: Secure Live VM Migration for KVM URL: https://github.com/apache/cloudstack/pull/2505#issuecomment-381894193 @blueorangutan test This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] borisstoyanov commented on issue #2376: [4.11] Smoketest Health Check
borisstoyanov commented on issue #2376: [4.11] Smoketest Health Check URL: https://github.com/apache/cloudstack/pull/2376#issuecomment-381893956 @blueorangutan test matrix This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update
blueorangutan commented on issue #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update URL: https://github.com/apache/cloudstack/pull/2574#issuecomment-381882865 @borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] borisstoyanov commented on issue #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update
borisstoyanov commented on issue #2574: [CLOUDSTACK-5235] ask users old password when they are executing a password update URL: https://github.com/apache/cloudstack/pull/2574#issuecomment-381882481 @blueorangutan test This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
blueorangutan commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381882211 @borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] borisstoyanov commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
borisstoyanov commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381882063 @blueorangutan test This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rhtyd commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381879043 Thanks @rafaelweingartner I see the issue is Travis related, thanks for fixing. I've left some remarks. Also, the Travis failure affects 4.11 branch too, can you rebase and edit this PR against 4.11? Locally unable to reproduce the issue; On 4.11, I get this: ``` > bash -x runtests.sh + export PYTHONPATH=../debian/opt/cloud/bin/ + PYTHONPATH=../debian/opt/cloud/bin/ + export PYTHONDONTWRITEBYTECODE=False + PYTHONDONTWRITEBYTECODE=False + echo 'Running pep8 to check systemvm/python code for errors' Running pep8 to check systemvm/python code for errors + pep8 --max-line-length=179 TestCsAddress.py TestCsApp.py TestCsCmdLine.py TestCsConfig.py TestCsDatabag.py TestCsDhcp.py TestCsFile.py TestCsGuestNetwork.py TestCsHelper.py TestCsInterface.py TestCsNetfilter.py TestCsProcess.py TestCsRedundant.py TestCsRoute.py TestCsRule.py ++ find ../debian -name '*.py' + pep8 --max-line-length=179 --exclude=monitorServices.py,baremetal-vr.py,passwd_server_ip.py ../debian/opt/cloud/bin/cs_loadbalancer.py ../debian/opt/cloud/bin/configure.py ../debian/opt/cloud/bin/master.py ../debian/opt/cloud/bin/cs_ip.py ../debian/opt/cloud/bin/update_config.py ../debian/opt/cloud/bin/cs_guestnetwork.py ../debian/opt/cloud/bin/passwd_server_ip.py ../debian/opt/cloud/bin/cs_staticroutes.py ../debian/opt/cloud/bin/cs_dhcp.py ../debian/opt/cloud/bin/cs_vpnusers.py ../debian/opt/cloud/bin/set_redundant.py ../debian/opt/cloud/bin/cs_remoteaccessvpn.py ../debian/opt/cloud/bin/baremetal-vr.py ../debian/opt/cloud/bin/cs_monitorservice.py ../debian/opt/cloud/bin/cs_site2sitevpn.py ../debian/opt/cloud/bin/merge.py ../debian/opt/cloud/bin/cs_vmp.py ../debian/opt/cloud/bin/cs_forwardingrules.py ../debian/opt/cloud/bin/cs_vmdata.py ../debian/opt/cloud/bin/vmdata.py ../debian/opt/cloud/bin/cs/CsStaticRoutes.py ../debian/opt/cloud/bin/cs/CsFile.py ../debian/opt/cloud/bin/cs/CsDhcp.py ../debian/opt/cloud/bin/cs/CsDatabag.py ../debian/opt/cloud/bin/cs/CsProcess.py ../debian/opt/cloud/bin/cs/CsRoute.py ../debian/opt/cloud/bin/cs/CsAddress.py ../debian/opt/cloud/bin/cs/CsMonitor.py ../debian/opt/cloud/bin/cs/CsLoadBalancer.py ../debian/opt/cloud/bin/cs/__init__.py ../debian/opt/cloud/bin/cs/CsNetfilter.py ../debian/opt/cloud/bin/cs/CsGuestNetwork.py ../debian/opt/cloud/bin/cs/CsConfig.py ../debian/opt/cloud/bin/cs/CsRule.py ../debian/opt/cloud/bin/cs/CsRedundant.py ../debian/opt/cloud/bin/cs/CsApp.py ../debian/opt/cloud/bin/cs/CsHelper.py ../debian/opt/cloud/bin/line_edit.py ../debian/opt/cloud/bin/cs_cmdline.py ../debian/opt/cloud/bin/cs_firewallrules.py ../debian/opt/cloud/bin/cs_network_acl.py ../debian/root/monitorServices.py + '[' 0 -gt 0 ']' + echo 'Running pylint to check systemvm/python code for errors' Running pylint to check systemvm/python code for errors + pylint --disable=R,C,W TestCsAddress.py TestCsApp.py TestCsCmdLine.py TestCsConfig.py TestCsDatabag.py TestCsDhcp.py TestCsFile.py TestCsGuestNetwork.py TestCsHelper.py TestCsInterface.py TestCsNetfilter.py TestCsProcess.py TestCsRedundant.py TestCsRoute.py TestCsRule.py No config file found, using default configuration Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00) ++ find ../debian -name '*.py' + pylint --disable=R,C,W ../debian/opt/cloud/bin/cs_loadbalancer.py ../debian/opt/cloud/bin/configure.py ../debian/opt/cloud/bin/master.py ../debian/opt/cloud/bin/cs_ip.py ../debian/opt/cloud/bin/update_config.py ../debian/opt/cloud/bin/cs_guestnetwork.py ../debian/opt/cloud/bin/passwd_server_ip.py ../debian/opt/cloud/bin/cs_staticroutes.py ../debian/opt/cloud/bin/cs_dhcp.py ../debian/opt/cloud/bin/cs_vpnusers.py ../debian/opt/cloud/bin/set_redundant.py ../debian/opt/cloud/bin/cs_remoteaccessvpn.py ../debian/opt/cloud/bin/baremetal-vr.py ../debian/opt/cloud/bin/cs_monitorservice.py ../debian/opt/cloud/bin/cs_site2sitevpn.py ../debian/opt/cloud/bin/merge.py ../debian/opt/cloud/bin/cs_vmp.py ../debian/opt/cloud/bin/cs_forwardingrules.py ../debian/opt/cloud/bin/cs_vmdata.py ../debian/opt/cloud/bin/vmdata.py ../debian/opt/cloud/bin/cs/CsStaticRoutes.py ../debian/opt/cloud/bin/cs/CsFile.py ../debian/opt/cloud/bin/cs/CsDhcp.py ../debian/opt/cloud/bin/cs/CsDatabag.py ../debian/opt/cloud/bin/cs/CsProcess.py ../debian/opt/cloud/bin/cs/CsRoute.py ../debian/opt/cloud/bin/cs/CsAddress.py ../debian/opt/cloud/bin/cs/CsMonitor.py ../debian/opt/cloud/bin/cs/CsLoadBalancer.py ../debian/opt/cloud/bin/cs/__init__.py ../debian/opt/cloud/bin/cs/CsNetfilter.py ../debian/opt/cloud/bin/cs/CsGuestNetwork.py ../debian/opt/cloud/bin/cs/CsConfig.py ../debian/opt/cloud/bin/cs/CsRule.py ../debian/opt/cloud/bin/cs/CsRedundant.py ..
[cloudstack] branch master updated (c43c69a -> b940a89)
This is an automated email from the ASF dual-hosted git repository. rohit pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/cloudstack.git. from c43c69a Merge release branch 4.11 to master add 392f62d consoleproxy: use consoleproxy.domain for non-ssl enable env (#2562) new b940a89 Merge branch '4.11' The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: core/src/main/java/com/cloud/info/ConsoleProxyInfo.java | 5 + .../main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java| 5 ++--- 2 files changed, 7 insertions(+), 3 deletions(-) -- To stop receiving notification emails like this one, please contact ro...@apache.org.
[cloudstack] 01/01: Merge branch '4.11'
This is an automated email from the ASF dual-hosted git repository. rohit pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/cloudstack.git commit b940a892f76c32b761dc7a8eec0f9a3197c5ba30 Merge: c43c69a 392f62d Author: Rohit Yadav AuthorDate: Tue Apr 17 12:58:39 2018 +0530 Merge branch '4.11' core/src/main/java/com/cloud/info/ConsoleProxyInfo.java | 5 + .../main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java| 5 ++--- 2 files changed, 7 insertions(+), 3 deletions(-) -- To stop receiving notification emails like this one, please contact ro...@apache.org.
[GitHub] MartinEmrich commented on issue #2561: cloud-early-config detects unknown hypervisor type "xen-domU"
MartinEmrich commented on issue #2561: cloud-early-config detects unknown hypervisor type "xen-domU" URL: https://github.com/apache/cloudstack/issues/2561#issuecomment-381661246 I wiped everything and started over: Install 4.9.2.0 with some test VMs, Upgrade to 4.11.0-SNAPSHOT with the systemvm template from @rhtyd. I registered the System VM template * as Debian 7 (4.9.2 does not offer something newer) * with no checkboxes checked except "HVM" (the default selection). After clicking "Update", the new systemVMs this time come up as paravirtualized (despite the checkbox "HVM"). The initialization took more than half an hour... Only the next day they are ready. As pointed out on the cloudstack-users list, I had to "clean up" the network after adding the egress rule to allow traffic to flow, but that's another story. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#discussion_r181974407 ## File path: tools/travis/before_install.sh ## @@ -97,10 +99,11 @@ echo " echo -e "\nInstalling some python packages: " pip install --user --upgrade pip +pip uninstall pylint for ((i=0;i<$RETRY_COUNT;i++)) do - pip install --user --upgrade lxml paramiko nose texttable ipmisim pyopenssl mock flask netaddr pylint pep8 > /tmp/piplog + pip install --user --upgrade lxml paramiko texttable ipmisim pyopenssl mock flask netaddr nose pylint pycodestyle six astroid Markdown > /tmp/piplog Review comment: @rafaelweingartner are we using new pkgs - `six astroid Markdown`, where/how? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#discussion_r181974273 ## File path: tools/travis/before_install.sh ## @@ -97,10 +99,11 @@ echo " echo -e "\nInstalling some python packages: " pip install --user --upgrade pip +pip uninstall pylint Review comment: @rafaelweingartner Why uninstall here? I see `pylint` is installed again by pip at line 106. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
rhtyd commented on a change in pull request #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#discussion_r181974168 ## File path: systemvm/test/runtests.sh ## @@ -21,9 +21,9 @@ export PYTHONPATH="../debian/opt/cloud/bin/" export PYTHONDONTWRITEBYTECODE=False -echo "Running pep8 to check systemvm/python code for errors" -pep8 --max-line-length=179 *py -pep8 --max-line-length=179 --exclude=monitorServices.py,baremetal-vr.py,passwd_server_ip.py `find ../debian -name \*.py` +echo "Running pycodestyle to check systemvm/python code for errors" +pycodestyle --max-line-length=179 *py Review comment: @rafaelweingartner Why not retain using pep8? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] blueorangutan commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh"
blueorangutan commented on issue #2576: Fix Python code checkstyle execute by "systemvm\test\runtests.sh" URL: https://github.com/apache/cloudstack/pull/2576#issuecomment-381874376 Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1936 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services