* 4.9:
CLOUDSTACK-9470: Fix for SshHelper - test_network_acl was failing on Vmware due to a bug in sshExecute, in which value returned was null and there was still stdout to consume. This fix addresses this problem, consuming stdout peoperly to return expected value in sshExecute
Often, patch and security releases do not require schema migrations or
data migrations. However, if an empty upgrade class and associated
scripts are not defined, the upgrade process will break. With this
change, if a release does not have an upgrade, a noop DbUpgrade is added
to the upgrade path. This approach allows the upgrade to proceed and
for the database to properly reflect the installed version. This change
should make the release process simpler as RMs no longer need to
rememeber to create this boilerplate code when starting a new release.
Beginning with the 4.8.2.0 and 4.9.1.0 releases, the project will
formally adopt a four (4) position release number to properly accomodate
rekeases that contain only CVE fixes. The DatabaseUpgradeChecker and
Version classes made assumptions that they would always parse and
compare three (3) position version numbers. This change adds the
CloudStackVersion value object that supports both three (3) and four (4)
version numbers. It encapsulates version comparsion logic, as well as,
the rules to allow three (3) and four (4) to interoperate.
* Modifies DatabaseUpgradeChecker to handle derive an upgrade path for
a version that was not explicitly specified. It determines the
releases the first release before it with database migrations and uses
that list as the basis for the list for version being calculated. A
noop upgrade is then added to the list which causes no schema changes
or data migrations, but will update the database to the version.
* Adds unit tests for the upgrade path calculation logic in
DatabaseUpgradeChecker
* Removes dummy upgrade logic for the 4.8.2.0 introduced in previous
versions of this patch
* Introduces the CloudStackVersion value object which parses and
compares three (3) and four (4) position version numbers. This class
is intended to replace com.cloud.maint.Version.
* Adds the junit-dataprovider dependency -- allowing test data to be
concisely generated separately from the execution of a test case.
Used extensively in the CloudStackVersionTest.
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
This reduced the Nio loop selector wait time, this way the selector will
check frequently (as much as 100ms per iteration) and handle any pending
connection/tasks. This would make reconnections very quick at the expense of
some CPU usage.
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
CLOUDSTACK-9348: NioConnection improvementsReopened PR with squashed changes for a re-review and testing after https://github.com/apache/cloudstack/pull/1493 and sub-sequent PRs got reverted
* pr/1549:
CLOUDSTACK-9348: NioConnection improvements
Signed-off-by: Will Stevens <williamstevens@gmail.com>
Taking fast and efficient volume snapshots with XenServer (and your storage provider)A XenServer storage repository (SR) and virtual disk image (VDI) each have UUIDs that are immutable.
This poses a problem for SAN snapshots, if you intend on mounting the underlying snapshot SR alongside the source SR (duplicate UUIDs).
VMware has a solution for this called re-signaturing (so, in other words, the snapshot UUIDs can be changed).
This PR only deals with the CloudStack side of things, but it works in concert with a new XenServer storage manager created by CloudOps (this storage manager enables re-signaturing of XenServer SR and VDI UUIDs).
I have written Marvin integration tests to go along with this, but cannot yet check those into the CloudStack repo as they rely on SolidFire hardware.
If anyone would like to see these integration tests, please let me know.
JIRA ticket: https://issues.apache.org/jira/browse/CLOUDSTACK-9281
Here's a video I made that shows this feature in action:
https://www.youtube.com/watch?v=YQ3pBeL-WaA&list=PLqOXKM0Bt13DFnQnwUx8ZtJzoyDV0Uuye&index=13
* pr/1403:
Faster logic to see if a cluster supports resigning
Support for backend snapshots with XenServer
Signed-off-by: Will Stevens <williamstevens@gmail.com>
- Unit test to demonstrate denial of service attack
The NioConnection uses blocking handlers for various events such as connect,
accept, read, write. In case a client connects NioServer (used by
agent mgr to service agents on port 8250) but fails to participate in SSL
handshake or just sits idle, this would block the main IO/selector loop in
NioConnection. Such a client could be either malicious or aggresive.
This unit test demonstrates such a malicious client that can perform a
denial-of-service attack on NioServer that blocks it to serve any other client.
- Use non-blocking SSL handshake
- Uses non-blocking socket config in NioClient and NioServer/NioConnection
- Scalable connectivity from agents and peer clustered-management server
- Removes blocking ssl handshake code with a non-blocking code
- Protects from denial-of-service issues that can degrade mgmt server responsiveness
due to an aggressive/malicious client
- Uses separate executor services for handling ssl handshakes
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
This reverts commit 7ce0e10fbcd949375e43535aae168421ecdaa562, reversing
changes made to 29ba71f2db3a3b7dcdad3a7c1f83725cffd56261.
This was reverted because it seemed to be related to an issue
when doing a DeployDC, causing an `addHost` error.
This reverts commit 9f970f28b18534dffe33196ead60ea861f501fa9, reversing
changes made to 6d0c92be7257afcf5f1a9f4453e3470e0083125d.
This was reverted because it seemed to be related to an issue
when doing a DeployDC, causing an `addHost` error.
This reverts commit f88cb880974fa56866492c437af291e40bd1a4f6, reversing
changes made to 688522ecd41d5e2bf09467aa236deb50f7826982.
This was reverted because it seemed to be related to an issue
when doing a DeployDC, causing an `addHost` error.
This reverts commit 540d9572fd491db3ce182d26636fc74ada4e171c.
This was reverted because it seemed to be related to an issue
when doing a DeployDC, causing an `addHost` error.
- Fixes oobm integration test to skip if known ipmitool bug is hit
- Fixes ProcessTest unit test case to use sleep
- Removes redundant unit test that covers code in ProcessTest
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
- Reverts ea2286 that introduced a wakeup on each connection loop run.
- In SSL handshake code removes delegated tasks to be run in separate threads.
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
Remove classes with no referencesI used UCDetector (http://www.ucdetector.org/) as a plugin for Eclipse. With this tool, I discovered a lot of code without any reference (variables, methods and classes).
Following the work that was done at [https://github.com/apache/cloudstack/pull/1448]; this pull request had the goal of removing some of these classes. To check if I wasn't missing anything I searched for any file that could reference some of those classes. As I haven't found any way of these classes being used, they were removed. Note that some of them I found other references, but references such as commented lines or tests, nothing that could indicate their use (as XML files configuring beans or another class instantiating an object with "new").
Waiting for tests. Please tell me if I am missing something.
Removed Classes:
- org.apache.cloudstack.framework.jobs.JobCancellationException (**Note:** removed
variable JobCancellationException in com.cloud.utils.SerialVersionUID)
- org.apache.cloudstack.ldap.NoSuchLdapUserException (**Note:** removed test file
/cloud-plugin-user-authenticator-ldap/test/groovy/org/apache/cloudstack/ldap/NoSuchLdapUserExceptionSpec.groovy)
- com.cloud.agent.api.storage.CreateVolumeOVAAnswer
- com.cloud.exception.MissingParameterValueException
- org.apache.cloudstack.api.response.StatusResponse
- org.apache.cloudstack.api.response.VolumeDetailResponse
- org.apache.cloudstack.api.response.UpgradeVmResponse
- org.apache.cloudstack.api.response.AddIpToVmNicResponse
- org.apache.cloudstack.api.response.TemplateZoneResponse (**Note:** at
org.apache.cloudstack.api.response.TemplateResponse, there is this
comment "To avoid breaking backwards compatibility, we still treat a
template at different zones as different templates, so not embedding
template_zone information in this TemplateZoneResponse set. `private
Set<TemplateZoneResponse> zones;`" but right now it is not used)
- org.apache.cloudstack.api.response.NicDetailResponse
* pr/1453:
Removed classes with no reference
Signed-off-by: Will Stevens <williamstevens@gmail.com>
Disable NioTestHistorically NioTest has caused issue in CI environment and several developer machines due to network requirements which could be disabled by firewall or security enforcers such as selinux. This disables the test once again using a historic commit 881a6e1
Signed-off-by: Rohit Yadav <rohit.yada
To build and just run this test: mvn clean install -pl utils -Dtest=NioTest
* pr/1538:
utils: Disable NioTest
Signed-off-by: Will Stevens <williamstevens@gmail.com>
Disable the NioTest, it is far to dependent on the network configuration of the
machine running the test. Cherry-picked change from a historic commit where
this test was disabled.
(cherry picked from commit 881a6e164ece8e34154555b71e97c9f1763674f5)
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
Support access to a host’s out-of-band management interface (e.g. IPMI, iLO,
DRAC, etc.) to manage host power operations (on/off etc.) and querying current
power state in CloudStack.
Given the wide range of out-of-band management interfaces such as iLO and iDRA,
the service implementation allows for development of separate drivers as plugins.
This feature comes with a ipmitool based driver that uses the
ipmitool (http://linux.die.net/man/1/ipmitool) to communicate with any
out-of-band management interface that support IPMI 2.0.
This feature allows following common use-cases:
- Restarting stalled/failed hosts
- Powering off under-utilised hosts
- Powering on hosts for provisioning or to increase capacity
- Allowing system administrators to see the current power state of the host
For testing this feature `ipmisim` can be used:
https://pypi.python.org/pypi/ipmisim
FS:
https://cwiki.apache.org/confluence/display/CLOUDSTACK/Out-of-band+Management+for+CloudStack
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
This feature allows root administrators to define new roles and associate API
permissions to them.
A limited form of role-based access control for the CloudStack management server
API is provided through a properties file, commands.properties, embedded in the
WAR distribution. Therefore, customizing API permissions requires unpacking the
distribution and modifying this file consistently on all servers. The old system
also does not permit the specification of additional roles.
FS:
https://cwiki.apache.org/confluence/display/CLOUDSTACK/Dynamic+Role+Based+API+Access+Checker+for+CloudStack
DB-Backed Dynamic Role Based API Access Checker for CloudStack brings following
changes, features and use-cases:
- Moves the API access definitions from commands.properties to the mgmt server DB
- Allows defining custom roles (such as a read-only ROOT admin) beyond the
current set of four (4) roles
- All roles will resolve to one of the four known roles types (Admin, Resource
Admin, Domain Admin and User) which maintains this association by requiring
all new defined roles to specify a role type.
- Allows changes to roles and API permissions per role at runtime including additions or
removal of roles and/or modifications of permissions, without the need
of restarting management server(s)
Upgrade/installation notes:
- The feature will be enabled by default for new installations, existing
deployments will continue to use the older static role based api access checker
with an option to enable this feature
- During fresh installation or upgrade, the upgrade paths will add four default
roles based on the four default role types
- For ease of migration, at the time of upgrade commands.properties will be used
to add existing set of permissions to the default roles. cloud.account
will have a new role_id column which will be populated based on default roles
as well
Dynamic-roles migration tool: scripts/util/migrate-dynamicroles.py
- Allows admins to migrate to the dynamic role based checker at a future date
- Performs a harder one-way migrate and update
- Migrates rules from existing commands.properties file into db and deprecates it
- Enables an internal hidden switch to enable dynamic role based checker feature
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
- Reduces SSL handshake timeout to 15s, previously this was only 10s in
commit debfcdef788ce0d51be06db0ef10f6815f9b563b
- Adds an aggresive explicit wakeup to save the Nio main IO loop/handler from
getting blocked
- Fix NioTest to fail/succeed in about 60s, previously this was 300s
- Due to aggresive wakeup usage, NioTest should complete in less than 5s on most
systems. On virtualized environment this may slightly increase due to thread,
CPU burst/scheduling delays.
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
CLOUDSTACK-9348: Use non-blocking SSL handshake in NioConnection/Link- Uses non-blocking socket config in NioClient and NioServer/NioConnection
- Scalable connectivity from agents and peer clustered-management server
- Removes blocking ssl handshake code with a non-blocking code
- Protects from denial-of-service issues that can degrade mgmt server responsiveness
due to an aggressive/malicious client
- Uses separate executor services for handling connect/accept events
Changes are covered the NioTest so I did not write a new test, advise how we can improve this. Further, I tried to invest time on writing a benchmark test to reproduce a degraded server but could not write it deterministic-ally (sometimes fails/passes but not always). Review, CI testing and feedback requested /cc @swill @jburwell @DaanHoogland @wido @remibergsma @rafaelweingartner @GabrielBrascher
* pr/1493:
CLOUDSTACK-9348: Use non-blocking SSL handshake
CLOUDSTACK-9348: Unit test to demonstrate denial of service attack
Signed-off-by: Will Stevens <williamstevens@gmail.com>
- Uses non-blocking socket config in NioClient and NioServer/NioConnection
- Scalable connectivity from agents and peer clustered-management server
- Removes blocking ssl handshake code with a non-blocking code
- Protects from denial-of-service issues that can degrade mgmt server responsiveness
due to an aggressive/malicious client
- Uses separate executor services for handling ssl handshakes
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
The NioConnection uses blocking handlers for various events such as connect,
accept, read, write. In case a client connects NioServer (used by
agent mgr to service agents on port 8250) but fails to participate in SSL
handshake or just sits idle, this would block the main IO/selector loop in
NioConnection. Such a client could be either malicious or aggresive.
This unit test demonstrates such a malicious client that can perform a
denial-of-service attack on NioServer that blocks it to serve any other client.
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
4.9 mvn version safeupgradeonlyUpgrades maven dependencies versions that can be safely upgraded without breaking console-proxy/crypto usage.
Bisected changes from: https://github.com/apache/cloudstack/pull/1397
cc @swill @DaanHoogland
* pr/1510:
maven: fix dependency version support by JDK7
further maven dependency updates from Daan
framework/quota: fix checkstyle issue
maven: Upgrade dependency versions
Signed-off-by: Will Stevens <williamstevens@gmail.com>
CLOUDSTACK-9352: Test fails in Widows as the file separator "/" is different from "\"**Problem:**
File separator in windows ("\") is different from the expected in the test ("/"); thus, the test *com.cloud.utils.SwiftUtilTest.testSplitSwiftPath()* will fail in Windows systems.
The problem is that the input of the test is "*container/object*" but the tested method uses the *File.separator* (that depends from the OS), in windows systems the tested method (*com.cloud.utils.SwiftUtil.splitSwiftPath(String)*) looks for a "\", as the string does not contain "\" it returns an empty string and consequently results in a test failure.
**Solution:**
Create a string `String input = "container" + File.separator + "object";`, with that the test will validate the tested method verifying if the method splits the string around matches of the given regular expression (in this case *File.separator*).
*JIRA link: https://issues.apache.org/jira/browse/CLOUDSTACK-9352*
* pr/1498:
Test fails in Widows as the file separator "/" is different from "\"
Signed-off-by: Will Stevens <williamstevens@gmail.com>
Updated most dependencies to latest minor releases, EXCEPT:
- Gson 2.x
- Major spring framework version
- Servlet version
- Embedded jetty version
- Mockito version (beta)
- Mysql lib minor version upgrade (breaks mysql-ha plugin)
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
This closes#561
CLOUDSTACK-8611:Handle SSH if server "forget" to send exit statusContinuing the work started by @likitha, I did not cherry-picked the
commit (b9181c689e0e7b5f1e28c81d73710196dfabd0ba) from PR <https://github.com/apache/cloudstack/pull/561> due to the fact that the path of that SshHelper class was different of the current SshHelper; that is because the fact that by cherry-picking it would seem that I had changed all the class as the code is from another file.
I made some changes from the cherry-picked commit adding @wilderrodrigues suggestions (create simple methods to have reusable code, make unit tests and create the `WAITING_OPEN_SSH_SESSION` variable to manipulate with the delay of 1000 milliseconds).
Also, I tried to simplify the logic by assuming that ....
if ((conditions & ChannelCondition.EXIT_STATUS) != 0) {
if ((conditions & (ChannelCondition.STDOUT_DATA | ChannelCondition.STDERR_DATA)) == 0) {
break;
}
}
... is the same as `((conditions & ChannelCondition.EXIT_STATUS) != 0) && ((conditions & (ChannelCondition.STDOUT_DATA | ChannelCondition.STDERR_DATA)) == 0)`. This expression has the following results according to each possible condition.
|Condition|Value|result
|-----------------|-------|------|
TIMEOUT | 0000001|false
CLOSED | 0000010 |false
STDERR_DATA | 0000100 | false
STDERR_DATA | 0001000 | false
EOF | 0010000 | false
EXIT_STATUS | 0100000 | **true**
EXIT_SIGNAL | 1000000 | false
After testing all the possibilities we can note that the condition of `(conditions & ChannelCondition.EXIT_STATUS) != 0` is sufficient; thus, the simplified "if" conditional can be:
`if ((conditions & ChannelCondition.EXIT_STATUS) != 0) {
break;
}`
This proposed work can be explained by quoting @likitha:
>CheckS2SVpnConnectionsCommand execution involves executing a script (checkbatchs2svpn.sh) in the virtual router. Once CS has opened a session to a virtual router and executed a script in the router, it waits indefinitely till the session either times out or the exit status of the remote process is available. But it is possible that an EOF is reached by the process in the router and the router never set the exit status.
>References -
>1. Some servers never send the exit status, or occasionally "forget" to do so (http://grepcode.com/file/repo1.maven.org/maven2/org.jvnet.hudson/trilead-ssh2/build212-hudson-1/com/trilead/ssh2/ChannelCondition.java).
>2. Get the exit code/status from the remote command - if available. Be careful - not all server implementations return this value - (http://grepcode.com/file/repo1.maven.org/maven2/org.jvnet.hudson/trilead-ssh2/build212-hudson-1/com/trilead/ssh2/Session.java#Session.waitForCondition%28int%2Clong%29).
* pr/1459:
Handle SSH if server "forget" to send exit status
Signed-off-by: Will Stevens <williamstevens@gmail.com>
File separator in windows is different from linux (the expected in the
test); thus, the test
*com.cloud.utils.SwiftUtilTest.testSplitSwiftPath()* will fail in
windows. The problem is that the input of the test is
*"container/object"* but the tested method uses the *File.separator*
(that depends from the OS), in the windows the tested method
(*com.cloud.utils.SwiftUtil.splitSwiftPath(String)*) looks for a "\" in
windows systems, resulting in an empty string and consequently a failure
in the test.
Some solutions:
- the simple way is to create a string `String input = "container" +
File.separator + "object";`, thus independent of the OS, the test will
succeed.
- a tricky solution is to mock the final static variable
*File.separator* and return "/".
I picked the easy way.
Continued the work started by https://github.com/likitha
commit (b9181c6) from PR #561.
CS waits indefinitely for CheckS2SVpnConnectionsComm and to return.
While remote executing commands through ssh, handle channel condition of
EOF because we wait for the the condition.
The SshHelper of the PR #561 is of another path from the
current master, its path was
https://github.com/likitha/cloudstack/commits/CLOUDSTACK-8611/utils/src/com/cloud/utils/ssh/SshHelper.java;
thus, although this commit brings changes from PR #561, I did not
cherry-picked to keep the master file, otherwise it would look that I
had changed all the file.
by me.
- Improve disk chain usage while attaching, migrating disks
- Gets root disk controller based diskDeviceBusName from volume's chain info
- Refactor and move VirtualMachineDiskInfo to cloud-utils
- Allows mixing of scsi controller types
- Fixes a NPE case with map passed as null, for example in case of detach volume
command
- Use a osdefault translator that allow use of recent os types added (enums of
which) are not available in the sdk
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
Fixed Profiler's unit tests bugs.### **Problem:**
The TestProfiler class was using Java Thread methods to test the
Profiler's functionality. That was causing the tests to fail sometimes
since the JVM's thread priority could be low on some OS.
### **Fix:**
Using PowerMockito to mock the System calls, the threads could be
removed. This makes the tests considerably faster, OS independent and
still guarantees the correct implementation of the Profiler class.
The changes on the Profiler's class was only to shorten the class's line
size by not assigning the return value to a variable returning it
straight out.
* pr/1445:
Fixed changes to match code conventions
Fixed Profiler's unit tests bugs.
Signed-off-by: Will Stevens <williamstevens@gmail.com>
- org.apache.cloudstack.framework.jobs.JobCancellationException (removed
variable JobCancellationException in com.cloud.utils.SerialVersionUID)
- org.apache.cloudstack.ldap.NoSuchLdapUserException (removed test file
/cloud-plugin-user-authenticator-ldap/test/groovy/org/apache/cloudstack/ldap/NoSuchLdapUserExceptionSpec.groovy)
- com.cloud.agent.api.storage.CreateVolumeOVAAnswer
- com.cloud.exception.MissingParameterValueException
- org.apache.cloudstack.api.response.StatusResponse
- org.apache.cloudstack.api.response.VolumeDetailResponse
- org.apache.cloudstack.api.response.UpgradeVmResponse
- org.apache.cloudstack.api.response.AddIpToVmNicResponse
- org.apache.cloudstack.api.response.TemplateZoneResponse (at
org.apache.cloudstack.api.response.TemplateResponse, there is this
comment "To avoid breaking backwards compatibility, we still treat a
template at different zones as different templates, so not embedding
template_zone information in this TemplateZoneResponse set. `private
Set<TemplateZoneResponse> zones;`" but right now it is not used)
- org.apache.cloudstack.api.response.NicDetailResponse
Add ability to download templates in SwiftThis PR adds the ability to download templates when using Swift as a secondary storage. Uses the "temp_url" feature of Swift so that tempates can be downloaded without authenticaiton.
* pr/1332:
Add ability to download templates in Swift
Signed-off-by: Will Stevens <williamstevens@gmail.com>
