Tal Nisan has uploaded a new change for review. Change subject: core: Added validation on managed custom mount option ......................................................................
core: Added validation on managed custom mount option When adding a new NFS/Posix storage server connection, if the custom mount options contains a managed option (i.e. property that the engine is managing directly), the command will fail with a CDA Change-Id: Id90cea934e0a9c10c5df435f564530b8658a64bb Bug-Url: https://bugzilla.redhat.com/1129597 Signed-off-by: Tal Nisan <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java M frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties M frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties 7 files changed, 77 insertions(+), 8 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/02/32202/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java index fa130b7..94fe4b0 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommand.java @@ -1,10 +1,14 @@ package org.ovirt.engine.core.bll.storage; +import org.ovirt.engine.core.bll.ValidationResult; import org.ovirt.engine.core.bll.context.CommandContext; +import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.InternalCommandAttribute; @@ -23,6 +27,7 @@ import org.ovirt.engine.core.common.validation.NfsMountPointConstraint; import org.ovirt.engine.core.common.validation.group.CreateEntity; import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.vdsbroker.xmlrpc.XmlRpcStringUtils; @InternalCommandAttribute public class AddStorageServerConnectionCommand<T extends StorageServerConnectionParametersBase> extends @@ -79,22 +84,24 @@ return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_ID_NOT_EMPTY); } - if (paramConnection.getstorage_type() == StorageType.NFS - && !new NfsMountPointConstraint().isValid(paramConnection.getconnection(), null)) { + StorageType storageType = paramConnection.getstorage_type(); + + if (storageType == StorageType.NFS && !new NfsMountPointConstraint().isValid(paramConnection.getconnection(), null)) { return failCanDoAction(VdcBllMessages.VALIDATION_STORAGE_CONNECTION_INVALID); } - if (paramConnection.getstorage_type() == StorageType.POSIXFS - && (StringUtils.isEmpty(paramConnection.getVfsType()))) { + if (storageType == StorageType.POSIXFS && (StringUtils.isEmpty(paramConnection.getVfsType()))) { return failCanDoAction(VdcBllMessages.VALIDATION_STORAGE_CONNECTION_EMPTY_VFSTYPE); } - if (paramConnection.getstorage_type() == StorageType.ISCSI - && StringUtils.isEmpty(paramConnection.getiqn())) { + if ((storageType == StorageType.POSIXFS || storageType == StorageType.NFS) && !validate(validateMountOptions())) { + return false; + } + + if (storageType == StorageType.ISCSI && StringUtils.isEmpty(paramConnection.getiqn())) { return failCanDoAction(VdcBllMessages.VALIDATION_STORAGE_CONNECTION_EMPTY_IQN); } - if (paramConnection.getstorage_type() == StorageType.ISCSI - && !isValidStorageConnectionPort(paramConnection.getport())) { + if (storageType == StorageType.ISCSI && !isValidStorageConnectionPort(paramConnection.getport())) { return failCanDoAction(VdcBllMessages.VALIDATION_STORAGE_CONNECTION_INVALID_PORT); } @@ -121,6 +128,35 @@ return true; } + private static final List<String> NFS_MANAGED_OPTIONS = Arrays.asList("timeout", "retrans", "vfs_type", "protocol_version", "nfsvers", "vers"); + private static final List<String> POSIX_MANAGED_OPTIONS = Arrays.asList("vfs_type"); + + private ValidationResult validateMountOptions() { + + //failCanDoAction() + String mountOptions = getConnection().getMountOptions(); + if (StringUtils.isBlank(mountOptions)) { + return ValidationResult.VALID; + } + + List<String> disallowedOptions = + getConnection().getstorage_type() == StorageType.POSIXFS ? POSIX_MANAGED_OPTIONS : NFS_MANAGED_OPTIONS; + Map<String, String> optionsMap = XmlRpcStringUtils.string2Map(mountOptions); + Set<String> optionsKeys = new HashSet<>(); + for (String optionName : optionsMap.keySet()) { + optionsKeys.add(optionName.toLowerCase()); + } + + optionsKeys.retainAll(disallowedOptions); + + if (!optionsKeys.isEmpty()) { + addCanDoActionMessageVariable("invalidOptions", StringUtils.join(optionsKeys, ", ")); + return new ValidationResult(VdcBllMessages.VALIDATION_STORAGE_CONNECTION_MOUNT_OPTIONS_CONTAINS_MANAGED_PROPERTY); + } + + return ValidationResult.VALID; + } + @Override protected List<Class<?>> getValidationGroups() { addValidationGroup(CreateEntity.class); diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java index 36eacf0..5f4f90c 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AddStorageServerConnectionCommandTest.java @@ -319,4 +319,30 @@ boolean isExists = command.isConnWithSameDetailsExists(newLocalConnection, newLocalConnectionStoragePoolId); assertFalse(isExists); } + + @Test + public void testPosixConnectionWithInvalidMountOptions() { + StorageServerConnections newPosixConnection = + createPosixConnection("multipass.my.domain.tlv.company.com:/export/allstorage/data1", + StorageType.NFS, + "nfs", + "timeo=30, vfs_type=nfs"); + parameters.setStorageServerConnection(newPosixConnection); + parameters.setVdsId(Guid.Empty); + doReturn(false).when(command).isConnWithSameDetailsExists(newPosixConnection, null); + CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, VdcBllMessages.VALIDATION_STORAGE_CONNECTION_MOUNT_OPTIONS_CONTAINS_MANAGED_PROPERTY); + } + + @Test + public void testNfsConnectionWithInvalidMountOptions() { + StorageServerConnections newPosixConnection = + createPosixConnection("multipass.my.domain.tlv.company.com:/export/allstorage/data1", + StorageType.NFS, + "nfs", + "timeo=30, nfsvers=4"); + parameters.setStorageServerConnection(newPosixConnection); + parameters.setVdsId(Guid.Empty); + doReturn(false).when(command).isConnWithSameDetailsExists(newPosixConnection, null); + CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, VdcBllMessages.VALIDATION_STORAGE_CONNECTION_MOUNT_OPTIONS_CONTAINS_MANAGED_PROPERTY); + } } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java index fac5612..72dd182 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java @@ -826,6 +826,7 @@ VALIDATION_STORAGE_CONNECTION_INVALID(ErrorType.BAD_PARAMETERS), VALIDATION_STORAGE_CONNECTION_INVALID_PORT(ErrorType.BAD_PARAMETERS), VALIDATION_STORAGE_CONNECTION_EMPTY_VFSTYPE(ErrorType.BAD_PARAMETERS), + VALIDATION_STORAGE_CONNECTION_MOUNT_OPTIONS_CONTAINS_MANAGED_PROPERTY(ErrorType.BAD_PARAMETERS), VALIDATION_STORAGE_CONNECTION_EMPTY_IQN(ErrorType.BAD_PARAMETERS), VALIDATION_STORAGE_CONNECTION_EMPTY_CONNECTION(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_ACTION_IS_SUPPORTED_ONLY_FOR_ISCSI_DOMAINS(ErrorType.BAD_PARAMETERS), diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties index 5235cc4..5404dee 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -779,6 +779,7 @@ VALIDATION_STORAGE_CONNECTION_INVALID=Mount path is illegal, please use [IP:/path or FQDN:/path] convention. VALIDATION_STORAGE_CONNECTION_INVALID_PORT=Invalid value for port, should be an integer greater than 0. VALIDATION_STORAGE_CONNECTION_EMPTY_VFSTYPE=VFS type cannot be empty. +VALIDATION_STORAGE_CONNECTION_MOUNT_OPTIONS_CONTAINS_MANAGED_PROPERTY=Cannot ${action} ${type}. Custom mount options contain the following managed options: ${invalidOptions}. VALIDATION_STORAGE_CONNECTION_EMPTY_IQN=Target details cannot be empty. VALIDATION_STORAGE_CONNECTION_EMPTY_CONNECTION=${fieldName} field cannot be empty. VALIDATION_STORAGE_CONNECTION_NFS_RETRANS=NFS Retransmissions should be between 0 and 32767 diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java index 9f2f749..20393ae 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java @@ -2762,6 +2762,9 @@ @DefaultStringValue("VFS type cannot be empty") String VALIDATION_STORAGE_CONNECTION_EMPTY_VFSTYPE(); + @DefaultStringValue("Cannot ${action} ${type}. Custom mount options contain the following managed options: ${invalidOptions}.") + String VALIDATION_STORAGE_CONNECTION_MOUNT_OPTIONS_CONTAINS_MANAGED_PROPERTY(); + @DefaultStringValue("Target details cannot be empty.") String VALIDATION_STORAGE_CONNECTION_EMPTY_IQN(); diff --git a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 888e556..4dfc6f1 100644 --- a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -738,6 +738,7 @@ VALIDATION_STORAGE_CONNECTION_INVALID=Mount path is illegal, please use [IP:/path or FQDN:/path] convention. VALIDATION_STORAGE_CONNECTION_INVALID_PORT=Invalid value for port, should be an integer greater than 0. VALIDATION_STORAGE_CONNECTION_EMPTY_VFSTYPE=VFS type cannot be empty. +VALIDATION_STORAGE_CONNECTION_MOUNT_OPTIONS_CONTAINS_MANAGED_PROPERTY=Cannot ${action} ${type}. Custom mount options contain the following managed options: ${invalidOptions}. VALIDATION_STORAGE_CONNECTION_EMPTY_IQN=Target details cannot be empty. VALIDATION_STORAGE_CONNECTION_EMPTY_CONNECTION=${fieldName} field cannot be empty. VALIDATION_STORAGE_CONNECTION_NFS_RETRANS=NFS Retransmissions should be between 0 and 32767 diff --git a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index da25c07..bab1ea2 100644 --- a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -778,6 +778,7 @@ STORAGE_OPERATION_FAILED_SPM_NETWORK_PROBLEMS=Storage related operations can't be performed while the Storage Pool Manager is down.\nPlease make sure the Storage Pool Manager is up and running, and check network connectivity. VALIDATION_STORAGE_CONNECTION_INVALID=Mount path is illegal, please use [IP:/path or FQDN:/path] convention. VALIDATION_STORAGE_CONNECTION_EMPTY_VFSTYPE=VFS type cannot be empty. +VALIDATION_STORAGE_CONNECTION_MOUNT_OPTIONS_CONTAINS_MANAGED_PROPERTY=Cannot ${action} ${type}. Custom mount options contain the following managed options: ${invalidOptions}. VALIDATION_STORAGE_CONNECTION_EMPTY_IQN=Target details cannot be empty. VALIDATION_STORAGE_CONNECTION_EMPTY_CONNECTION=${fieldName} field cannot be empty. VALIDATION_STORAGE_CONNECTION_NFS_RETRANS=NFS Retransmissions should be between 0 and 32767 -- To view, visit http://gerrit.ovirt.org/32202 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id90cea934e0a9c10c5df435f564530b8658a64bb Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tal Nisan <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
