Moti Asayag has uploaded a new change for review.
Change subject: engine: Use HostValidator in AddVdsCommand
......................................................................
engine: Use HostValidator in AddVdsCommand
For the sake of DRY, AddVdsCommand will use the
HostValidator for covering its basic validations.
Change-Id: Iad171cbeca252d72417e59be01e1cd6955a07933
Signed-off-by: Moti Asayag <masa...@redhat.com>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
M
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVdsCommandTest.java
2 files changed, 57 insertions(+), 76 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/65/36165/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
index 34335bd..51991c8 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
@@ -20,6 +20,7 @@
import org.ovirt.engine.core.bll.utils.EngineSSHClient;
import org.ovirt.engine.core.bll.utils.GlusterUtil;
import org.ovirt.engine.core.bll.utils.PermissionSubject;
+import org.ovirt.engine.core.bll.validator.HostValidator;
import org.ovirt.engine.core.common.AuditLogType;
import org.ovirt.engine.core.common.FeatureSupported;
import org.ovirt.engine.core.common.VdcObjectType;
@@ -29,10 +30,7 @@
import org.ovirt.engine.core.common.action.VdcActionType;
import org.ovirt.engine.core.common.action.VdcReturnValueBase;
import org.ovirt.engine.core.common.action.VdsActionParameters;
-import
org.ovirt.engine.core.common.action.VdsOperationActionParameters.AuthenticationMethod;
-import
org.ovirt.engine.core.common.businessentities.BusinessEntitiesDefinitions;
import org.ovirt.engine.core.common.businessentities.Provider;
-import org.ovirt.engine.core.common.businessentities.StoragePool;
import org.ovirt.engine.core.common.businessentities.VDS;
import org.ovirt.engine.core.common.businessentities.VDSGroup;
import org.ovirt.engine.core.common.businessentities.VDSStatus;
@@ -47,7 +45,6 @@
import org.ovirt.engine.core.common.errors.VdcBllMessages;
import org.ovirt.engine.core.common.job.Step;
import org.ovirt.engine.core.common.job.StepEnum;
-import org.ovirt.engine.core.common.utils.ValidationUtils;
import org.ovirt.engine.core.common.validation.group.CreateEntity;
import org.ovirt.engine.core.common.validation.group.PowerManagementCheck;
import org.ovirt.engine.core.compat.Guid;
@@ -56,7 +53,6 @@
import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogableBase;
import org.ovirt.engine.core.dal.job.ExecutionMessageDirector;
import org.ovirt.engine.core.dao.gluster.GlusterDBUtils;
-import org.ovirt.engine.core.utils.crypt.EngineEncryptionUtils;
import org.ovirt.engine.core.utils.threadpool.ThreadPoolUtil;
import org.ovirt.engine.core.utils.transaction.TransactionMethod;
import org.ovirt.engine.core.utils.transaction.TransactionSupport;
@@ -325,62 +321,43 @@
// Check if this is a valid cluster
boolean returnValue = validateVdsGroup();
if (returnValue) {
- VDS vds = getParameters().getvds();
- String vdsName = vds.getName();
- String hostName = vds.getHostName();
- int maxVdsNameLength = Config.<Integer>
getValue(ConfigValues.MaxVdsNameLength);
- // check that vds name is not null or empty
- if (vdsName == null || vdsName.isEmpty()) {
- returnValue =
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NAME_MAY_NOT_BE_EMPTY);
- // check that VDS name is not too long
- } else if (vdsName.length() > maxVdsNameLength) {
- returnValue =
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NAME_LENGTH_IS_TOO_LONG);
- // check that VDS hostname does not contain special characters.
- } else if (!ValidationUtils.validHostname(hostName)) {
- returnValue =
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_INVALID_VDS_HOSTNAME);
- } else if (getVdsDAO().getByName(vdsName) != null) {
- returnValue =
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NAME_ALREADY_USED);
- } else if (getVdsDAO().getAllForHostname(hostName).size() != 0) {
- returnValue =
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VDS_WITH_SAME_HOST_EXIST);
- } else if (!ValidationUtils.validatePort(vds.getSshPort())) {
- returnValue =
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VDS_WITH_INVALID_SSH_PORT);
- } else if ((StringUtils.isBlank(vds.getSshUsername())) ||
- (vds.getSshUsername().length() >
BusinessEntitiesDefinitions.USER_LOGIN_NAME_SIZE)) {
- returnValue =
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VDS_WITH_INVALID_SSH_USERNAME);
- } else {
- returnValue = returnValue &&
validateSingleHostAttachedToLocalStorage();
+ HostValidator validator = getHostValidator();
+ returnValue = validate(validator.nameNotEmpty())
+ && validate(validator.nameLengthIsLegal())
+ && validate(validator.hostNameIsValid())
+ && validate(validator.nameNotUsed())
+ && validate(validator.hostNameNotUsed())
+ && validate(validator.portIsValid())
+ && validate(validator.sshUserNameNotEmpty())
+ && validate(validator.validateSingleHostAttachedToLocalStorage())
+ && validate(validator.securityKeysExists())
+ &&
validate(validator.passwordNotEmpty(getParameters().getAddPending(),
+ getParameters().getAuthMethod(),
+ getParameters().getPassword()));
+ }
- if (Config.<Boolean>
getValue(ConfigValues.EncryptHostCommunication)
- && !EngineEncryptionUtils.haveKey()) {
- returnValue =
failCanDoAction(VdcBllMessages.VDS_TRY_CREATE_SECURE_CERTIFICATE_NOT_FOUND);
- } else if (!getParameters().getAddPending()
- && (getParameters().getAuthMethod() ==
AuthenticationMethod.Password)
- && StringUtils.isEmpty(getParameters().getPassword()))
{
- // We block vds installations if it's not a RHEV-H and
password is empty
- // Note that this may override local host SSH policy. See
BZ#688718.
- returnValue =
failCanDoAction(VdcBllMessages.VDS_CANNOT_INSTALL_EMPTY_PASSWORD);
- } else if (!isPowerManagementLegal()) {
- returnValue = false;
- } else {
- returnValue = returnValue && canConnect(vds);
- }
+ if (!(returnValue && isPowerManagementLegal() &&
canConnect(getParameters().getvds()))) {
+ return false;
+ }
+
+ if (getParameters().getNetworkProviderId() != null
+ &&
!validateNetworkProviderProperties(getParameters().getNetworkProviderId(),
+ getParameters().getNetworkMappings())) {
+ return false;
+ }
+
+ if (isGlusterSupportEnabled() && clusterHasServers()) {
+ VDS upServer = getClusterUtils().getUpServer(getVdsGroupId());
+ if (upServer == null) {
+ return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NO_GLUSTER_HOST_TO_PEER_PROBE);
}
}
- if (returnValue && getParameters().getNetworkProviderId() != null) {
- returnValue =
validateNetworkProviderProperties(getParameters().getNetworkProviderId(),
- getParameters().getNetworkMappings());
- }
+ return true;
+ }
- if (returnValue && isGlusterSupportEnabled()) {
- if (clusterHasServers()) {
- VDS upServer = getClusterUtils().getUpServer(getVdsGroupId());
- if (upServer == null) {
- returnValue =
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NO_GLUSTER_HOST_TO_PEER_PROBE);
- }
- }
- }
- return returnValue;
+ protected HostValidator getHostValidator() {
+ return new HostValidator(getDbFacade(), getParameters().getvds());
}
protected boolean isPowerManagementLegal() {
@@ -538,23 +515,6 @@
protected GlusterUtil getGlusterUtil() {
return GlusterUtil.getInstance();
- }
-
- protected boolean validateSingleHostAttachedToLocalStorage() {
- boolean retrunValue = true;
- StoragePool storagePool =
DbFacade.getInstance().getStoragePoolDao().getForVdsGroup(
- getParameters().getVdsStaticData().getVdsGroupId());
-
- if (storagePool != null && storagePool.isLocal()) {
- if (!DbFacade.getInstance()
- .getVdsStaticDao()
-
.getAllForVdsGroup(getParameters().getVdsStaticData().getVdsGroupId())
- .isEmpty()) {
-
addCanDoActionMessage(VdcBllMessages.VDS_CANNOT_ADD_MORE_THEN_ONE_HOST_TO_LOCAL_STORAGE);
- retrunValue = false;
- }
- }
- return retrunValue;
}
@Override
diff --git
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVdsCommandTest.java
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVdsCommandTest.java
index 3b874c1..b0f46e2 100644
---
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVdsCommandTest.java
+++
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/AddVdsCommandTest.java
@@ -6,6 +6,7 @@
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.doCallRealMethod;
import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.when;
import java.util.Collections;
@@ -15,12 +16,13 @@
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
-import org.mockito.Mockito;
import org.mockito.runners.MockitoJUnitRunner;
import org.ovirt.engine.core.bll.utils.ClusterUtils;
import org.ovirt.engine.core.bll.utils.EngineSSHClient;
import org.ovirt.engine.core.bll.utils.GlusterUtil;
+import org.ovirt.engine.core.bll.validator.HostValidator;
import org.ovirt.engine.core.common.action.AddVdsActionParameters;
+import
org.ovirt.engine.core.common.action.VdsOperationActionParameters.AuthenticationMethod;
import org.ovirt.engine.core.common.businessentities.VDS;
import org.ovirt.engine.core.common.config.ConfigValues;
import org.ovirt.engine.core.common.errors.VdcBllMessages;
@@ -54,6 +56,8 @@
private EngineSSHClient sshClient;
@Mock
private Logger log;
+ @Mock
+ private HostValidator validator;
@ClassRule
public static MockConfigRule configRule =
@@ -83,8 +87,9 @@
}
private void setupCommonMock(boolean glusterEnabled) throws Exception {
+ mockHostValidator();
when(commandMock.canDoAction()).thenCallRealMethod();
-
when(commandMock.canConnect(Mockito.any(VDS.class))).thenCallRealMethod();
+ when(commandMock.canConnect(any(VDS.class))).thenCallRealMethod();
when(commandMock.getParameters()).thenReturn(parameters);
when(commandMock.isGlusterSupportEnabled()).thenReturn(glusterEnabled);
@@ -94,7 +99,6 @@
when(vdsDaoMock.get(vdsId)).thenReturn(null);
when(commandMock.getVdsDAO()).thenReturn(vdsDaoMock);
when(commandMock.validateVdsGroup()).thenReturn(true);
-
when(commandMock.validateSingleHostAttachedToLocalStorage()).thenReturn(true);
when(commandMock.isPowerManagementLegal()).thenReturn(true);
when(commandMock.getSSHClient()).thenReturn(sshClient);
@@ -102,6 +106,23 @@
doNothing().when(sshClient).authenticate();
}
+ private void mockHostValidator() {
+ when(validator.nameNotEmpty()).thenReturn(ValidationResult.VALID);
+ doReturn(ValidationResult.VALID).when(validator).nameLengthIsLegal();
+ doReturn(ValidationResult.VALID).when(validator).hostNameIsValid();
+ doReturn(ValidationResult.VALID).when(validator).nameNotUsed();
+ doReturn(ValidationResult.VALID).when(validator).hostNameNotUsed();
+ doReturn(ValidationResult.VALID).when(validator).portIsValid();
+
when(validator.sshUserNameNotEmpty()).thenReturn(ValidationResult.VALID);
+
doReturn(ValidationResult.VALID).when(validator).validateSingleHostAttachedToLocalStorage();
+ doReturn(ValidationResult.VALID).when(validator).securityKeysExists();
+ when(validator.passwordNotEmpty(any(Boolean.class),
+ any(AuthenticationMethod.class),
+ any(String.class))).thenReturn(ValidationResult.VALID);
+ when(commandMock.getHostValidator()).thenReturn(validator);
+
when(commandMock.validate(any(ValidationResult.class))).thenCallRealMethod();
+ }
+
private void setupVirtMock() throws Exception {
setupCommonMock(false);
}
--
To view, visit http://gerrit.ovirt.org/36165
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iad171cbeca252d72417e59be01e1cd6955a07933
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Moti Asayag <masa...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
