From c2afcdec5280c53b8c6c117516521cd864b216c9 Mon Sep 17 00:00:00 2001 From: alena Date: Tue, 17 May 2011 14:37:28 -0700 Subject: [PATCH] bug 9873: always add default security group to the SG list when deploy vm in 1) Basic zone 2) Advance zone using SG enabled network status 9873: resolved fixed Following fixes were made as a part of the checkin: * When deploy user vm and SG doesn't exist in the DB, create it automatically. * SecurityGroup enabled use vm start: if map to default group is not present in security_group_vm_map table, create one. * Added "name" (securityGroupName) parameter back to deleteSecurityGroup/authorizeSecurityGroupIngress/deployVm. Mutually exclusive with security group id parameter. Conflicts: api/src/com/cloud/api/commands/AuthorizeSecurityGroupIngressCmd.java api/src/com/cloud/api/commands/DeleteSecurityGroupCmd.java api/src/com/cloud/api/commands/DeployVMCmd.java server/src/com/cloud/api/ApiDBUtils.java server/src/com/cloud/vm/UserVmManagerImpl.java --- api/src/com/cloud/api/ApiConstants.java | 1 + api/src/com/cloud/api/ResponseGenerator.java | 2 + .../AuthorizeSecurityGroupIngressCmd.java | 62 ++++-- .../api/commands/CreateSecurityGroupCmd.java | 6 +- .../api/commands/DeleteSecurityGroupCmd.java | 195 ++++++++++-------- .../com/cloud/api/commands/DeployVMCmd.java | 39 +++- scripts/vm/hypervisor/xenserver/vmops | 2 +- server/src/com/cloud/api/ApiDBUtils.java | 4 + .../src/com/cloud/api/ApiResponseHelper.java | 10 + .../com/cloud/network/NetworkManagerImpl.java | 2 +- .../security/SecurityGroupManager.java | 8 +- .../security/SecurityGroupManagerImpl.java | 26 +++ .../src/com/cloud/vm/UserVmManagerImpl.java | 113 ++++++++-- server/src/com/cloud/vm/dao/NicDao.java | 4 +- server/src/com/cloud/vm/dao/NicDaoImpl.java | 7 - 15 files changed, 336 insertions(+), 145 deletions(-) diff --git a/api/src/com/cloud/api/ApiConstants.java b/api/src/com/cloud/api/ApiConstants.java index a125606e630..6faa8879713 100755 --- a/api/src/com/cloud/api/ApiConstants.java +++ b/api/src/com/cloud/api/ApiConstants.java @@ -140,6 +140,7 @@ public class ApiConstants { public static final String SCOPE = "scope"; public static final String SECRET_KEY = "secretkey"; public static final String SECURITY_GROUP_IDS = "securitygroupids"; + public static final String SECURITY_GROUP_NAMES = "securitygroupnames"; public static final String SECURITY_GROUP_NAME = "securitygroupname"; public static final String SECURITY_GROUP_ID = "securitygroupid"; public static final String SECURITY_GROUP_EANBLED = "securitygroupenabled"; diff --git a/api/src/com/cloud/api/ResponseGenerator.java b/api/src/com/cloud/api/ResponseGenerator.java index 0d4372cec09..730b85415d8 100644 --- a/api/src/com/cloud/api/ResponseGenerator.java +++ b/api/src/com/cloud/api/ResponseGenerator.java @@ -204,5 +204,7 @@ public interface ResponseGenerator { UserResponse createUserResponse(User user); AccountResponse createUserAccountResponse(UserAccount user); + + Long getSecurityGroupId(String groupName, long accountId); } diff --git a/api/src/com/cloud/api/commands/AuthorizeSecurityGroupIngressCmd.java b/api/src/com/cloud/api/commands/AuthorizeSecurityGroupIngressCmd.java index 1dda17d260b..fe1f9ee1b55 100644 --- a/api/src/com/cloud/api/commands/AuthorizeSecurityGroupIngressCmd.java +++ b/api/src/com/cloud/api/commands/AuthorizeSecurityGroupIngressCmd.java @@ -36,9 +36,10 @@ import com.cloud.api.response.IngressRuleResponse; import com.cloud.api.response.SecurityGroupResponse; import com.cloud.async.AsyncJob; import com.cloud.event.EventTypes; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.network.security.IngressRule; -import com.cloud.network.security.SecurityGroup; import com.cloud.user.Account; +import com.cloud.user.UserContext; import com.cloud.utils.StringUtils; @Implementation(responseObject = IngressRuleResponse.class, description = "Authorizes a particular ingress rule for this security group") @@ -67,20 +68,33 @@ public class AuthorizeSecurityGroupIngressCmd extends BaseAsyncCmd { @Parameter(name = ApiConstants.ICMP_CODE, type = CommandType.INTEGER, description = "error code for this icmp message") private Integer icmpCode; - @Parameter(name = ApiConstants.SECURITY_GROUP_ID, type = CommandType.LONG, required = true, description = "The ID of the security group") - private Long securityGroupId; - - @Parameter(name = ApiConstants.CIDR_LIST, type = CommandType.LIST, collectionType = CommandType.STRING, description = "the cidr list associated") - private List cidrList; + @Parameter(name=ApiConstants.CIDR_LIST, type=CommandType.LIST, collectionType=CommandType.STRING, description="the cidr list associated") + private List cidrList; @Parameter(name = ApiConstants.USER_SECURITY_GROUP_LIST, type = CommandType.MAP, description = "user to security group mapping") private Map userSecurityGroupList; + + @Parameter(name=ApiConstants.DOMAIN_ID, type=CommandType.LONG, description="an optional domainId for the security group. If the account parameter is used, domainId must also be used.") + private Long domainId; + + @Parameter(name=ApiConstants.ACCOUNT, type=CommandType.STRING, description="an optional account for the virtual machine. Must be used with domainId.") + private String accountName; + + @Parameter(name=ApiConstants.SECURITY_GROUP_ID, type=CommandType.LONG, description="The ID of the security group. Mutually exclusive with securityGroupName parameter") + private Long securityGroupId; + + @Parameter(name=ApiConstants.SECURITY_GROUP_NAME, type=CommandType.STRING, description="The name of the security group. Mutually exclusive with securityGroupName parameter") + private String securityGroupName; - // /////////////////////////////////////////////////// - // ///////////////// Accessors /////////////////////// - // /////////////////////////////////////////////////// + ///////////////////////////////////////////////////// + /////////////////// Accessors /////////////////////// + ///////////////////////////////////////////////////// - public List getCidrList() { + public String getAccountName() { + return accountName; + } + + public List getCidrList() { return cidrList; } @@ -97,6 +111,17 @@ public class AuthorizeSecurityGroupIngressCmd extends BaseAsyncCmd { } public Long getSecurityGroupId() { + if (securityGroupId != null && securityGroupName != null) { + throw new InvalidParameterValueException("securityGroupId and securityGroupName parameters are mutually exclusive"); + } + + if (securityGroupName != null) { + securityGroupId = _responseGenerator.getSecurityGroupId(securityGroupName, getEntityOwnerId()); + if (securityGroupId == null) { + throw new InvalidParameterValueException("Unable to find security group " + securityGroupName + " for account id=" + getEntityOwnerId()); + } + } + return securityGroupId; } @@ -130,12 +155,19 @@ public class AuthorizeSecurityGroupIngressCmd extends BaseAsyncCmd { @Override public long getEntityOwnerId() { - SecurityGroup group = _entityMgr.findById(SecurityGroup.class, getSecurityGroupId()); - if (group != null) { - return group.getAccountId(); + Account account = UserContext.current().getCaller(); + if ((account == null) || isAdmin(account.getType())) { + if ((domainId != null) && (accountName != null)) { + Account userAccount = _responseGenerator.findAccountByNameDomain(accountName, domainId); + if (userAccount != null) { + return userAccount.getId(); + } else { + throw new InvalidParameterValueException("Unable to find account by name " + accountName + " in domain " + domainId); + } + } } - - return Account.ACCOUNT_ID_SYSTEM; // no account info given, parent this command to SYSTEM so ERROR events are tracked + + return account.getId(); } @Override diff --git a/api/src/com/cloud/api/commands/CreateSecurityGroupCmd.java b/api/src/com/cloud/api/commands/CreateSecurityGroupCmd.java index 0cfd95f6146..b5e7f1247d6 100644 --- a/api/src/com/cloud/api/commands/CreateSecurityGroupCmd.java +++ b/api/src/com/cloud/api/commands/CreateSecurityGroupCmd.java @@ -41,13 +41,13 @@ public class CreateSecurityGroupCmd extends BaseCmd { @Parameter(name=ApiConstants.ACCOUNT, type=CommandType.STRING, description="an optional account for the security group. Must be used with domainId.") private String accountName; + + @Parameter(name=ApiConstants.DOMAIN_ID, type=CommandType.LONG, description="an optional domainId for the security group. If the account parameter is used, domainId must also be used.") + private Long domainId; @Parameter(name=ApiConstants.DESCRIPTION, type=CommandType.STRING, description="the description of the security group") private String description; - @Parameter(name=ApiConstants.DOMAIN_ID, type=CommandType.LONG, description="an optional domainId for the security group. If the account parameter is used, domainId must also be used.") - private Long domainId; - @Parameter(name=ApiConstants.NAME, type=CommandType.STRING, required=true, description="name of the security group") private String securityGroupName; diff --git a/api/src/com/cloud/api/commands/DeleteSecurityGroupCmd.java b/api/src/com/cloud/api/commands/DeleteSecurityGroupCmd.java index ff03a4a5300..b654fa8e511 100644 --- a/api/src/com/cloud/api/commands/DeleteSecurityGroupCmd.java +++ b/api/src/com/cloud/api/commands/DeleteSecurityGroupCmd.java @@ -1,87 +1,108 @@ -/** - * Copyright (C) 2010 Cloud.com, Inc. All rights reserved. - * - * This software is licensed under the GNU General Public License v3 or later. - * - * It is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or any later version. - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - * - */ - -package com.cloud.api.commands; - -import org.apache.log4j.Logger; - -import com.cloud.api.ApiConstants; -import com.cloud.api.BaseCmd; -import com.cloud.api.Implementation; -import com.cloud.api.Parameter; -import com.cloud.api.ServerApiException; -import com.cloud.api.response.SuccessResponse; -import com.cloud.exception.ResourceInUseException; -import com.cloud.network.security.SecurityGroup; -import com.cloud.user.Account; - -@Implementation(description = "Deletes security group", responseObject = SuccessResponse.class) -public class DeleteSecurityGroupCmd extends BaseCmd { - public static final Logger s_logger = Logger.getLogger(DeleteSecurityGroupCmd.class.getName()); - private static final String s_name = "deletesecuritygroupresponse"; - - // /////////////////////////////////////////////////// - // ////////////// API parameters ///////////////////// - // /////////////////////////////////////////////////// - - @Parameter(name = ApiConstants.ID, type = CommandType.LONG, required = true, description = "The ID of the security group") - private Long id; - - // /////////////////////////////////////////////////// - // ///////////////// Accessors /////////////////////// - // /////////////////////////////////////////////////// - - public Long getId() { - return id; - } - - // /////////////////////////////////////////////////// - // ///////////// API Implementation/////////////////// - // /////////////////////////////////////////////////// - - @Override - public String getCommandName() { - return s_name; - } - - @Override - public long getEntityOwnerId() { - SecurityGroup group = _entityMgr.findById(SecurityGroup.class, getId()); - if (group != null) { - return group.getAccountId(); - } - - return Account.ACCOUNT_ID_SYSTEM; // no account info given, parent this command to SYSTEM so ERROR events are tracked - } - - @Override - public void execute() { - try { - boolean result = _securityGroupService.deleteSecurityGroup(this); - if (result) { - SuccessResponse response = new SuccessResponse(getCommandName()); - this.setResponseObject(response); - } else { - throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to delete security group"); - } - } catch (ResourceInUseException ex) { - s_logger.warn("Exception: ", ex); - throw new ServerApiException(BaseCmd.RESOURCE_IN_USE_ERROR, ex.getMessage()); - } - } -} +package com.cloud.api.commands; + +import org.apache.log4j.Logger; + +import com.cloud.api.ApiConstants; +import com.cloud.api.BaseCmd; +import com.cloud.api.Implementation; +import com.cloud.api.Parameter; +import com.cloud.api.ServerApiException; +import com.cloud.api.response.SuccessResponse; +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.exception.ResourceInUseException; +import com.cloud.user.Account; +import com.cloud.user.UserContext; + +@Implementation(description="Deletes security group", responseObject=SuccessResponse.class) +public class DeleteSecurityGroupCmd extends BaseCmd { + public static final Logger s_logger = Logger.getLogger(DeleteSecurityGroupCmd.class.getName()); + private static final String s_name = "deletesecuritygroupresponse"; + + ///////////////////////////////////////////////////// + //////////////// API parameters ///////////////////// + ///////////////////////////////////////////////////// + + @Parameter(name=ApiConstants.ACCOUNT, type=CommandType.STRING, description="the account of the security group. Must be specified with domain ID") + private String accountName; + + @Parameter(name=ApiConstants.DOMAIN_ID, type=CommandType.LONG, description="the domain ID of account owning the security group") + private Long domainId; + + @Parameter(name=ApiConstants.ID, type=CommandType.LONG, description="The ID of the security group. Mutually exclusive with name parameter") + private Long id; + + @Parameter(name=ApiConstants.NAME, type=CommandType.STRING, description="The ID of the security group. Mutually exclusive with id parameter") + private String name; + + + ///////////////////////////////////////////////////// + /////////////////// Accessors /////////////////////// + ///////////////////////////////////////////////////// + + public String getAccountName() { + return accountName; + } + + public Long getDomainId() { + return domainId; + } + + public Long getId() { + if (id != null && name != null) { + throw new InvalidParameterValueException("name and id parameters are mutually exclusive"); + } + + if (name != null) { + id = _responseGenerator.getSecurityGroupId(name, getEntityOwnerId()); + if (id == null) { + throw new InvalidParameterValueException("Unable to find security group by name " + name + " for the account id=" + getEntityOwnerId()); + } + } + + return id; + } + + + + ///////////////////////////////////////////////////// + /////////////// API Implementation/////////////////// + ///////////////////////////////////////////////////// + + @Override + public String getCommandName() { + return s_name; + } + + @Override + public long getEntityOwnerId() { + Account account = UserContext.current().getCaller(); + if ((account == null) || isAdmin(account.getType())) { + if ((domainId != null) && (accountName != null)) { + Account userAccount = _responseGenerator.findAccountByNameDomain(accountName, domainId); + if (userAccount != null) { + return userAccount.getId(); + } else { + throw new InvalidParameterValueException("Unable to find account by name " + accountName + " in domain " + domainId); + } + } + } + + return account.getId(); + } + + @Override + public void execute(){ + try{ + boolean result = _securityGroupService.deleteSecurityGroup(this); + if (result) { + SuccessResponse response = new SuccessResponse(getCommandName()); + this.setResponseObject(response); + } else { + throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to delete security group"); + } + } catch (ResourceInUseException ex) { + s_logger.warn("Exception: ", ex); + throw new ServerApiException(BaseCmd.RESOURCE_IN_USE_ERROR, ex.getMessage()); + } + } +} \ No newline at end of file diff --git a/api/src/com/cloud/api/commands/DeployVMCmd.java b/api/src/com/cloud/api/commands/DeployVMCmd.java index c9a8a8169e6..5df5a24f73f 100644 --- a/api/src/com/cloud/api/commands/DeployVMCmd.java +++ b/api/src/com/cloud/api/commands/DeployVMCmd.java @@ -18,6 +18,7 @@ package com.cloud.api.commands; +import java.util.ArrayList; import java.util.List; import org.apache.log4j.Logger; @@ -81,9 +82,6 @@ public class DeployVMCmd extends BaseAsyncCreateCmd { @Parameter(name=ApiConstants.NETWORK_IDS, type=CommandType.LIST, collectionType=CommandType.LONG, description="list of network ids used by virtual machine") private List networkIds; - @Parameter(name=ApiConstants.SECURITY_GROUP_IDS, type=CommandType.LIST, collectionType=CommandType.LONG, description="comma separated list of security groups id that going to be applied to the virtual machine. Should be passed only when vm is created from a zone with Basic Network support") - private List securityGroupIdList; - //DataDisk information @Parameter(name=ApiConstants.DISK_OFFERING_ID, type=CommandType.LONG, description="the ID of the disk offering for the virtual machine. If the template is of ISO format, the diskOfferingId is for the root disk volume. Otherwise this parameter is used to indicate the offering for the data disk volume. If the templateId parameter passed is from a Template object, the diskOfferingId refers to a DATA Disk Volume created. If the templateId parameter passed is from an ISO object, the diskOfferingId refers to a ROOT Disk Volume created.") private Long diskOfferingId; @@ -105,6 +103,12 @@ public class DeployVMCmd extends BaseAsyncCreateCmd { @Parameter(name=ApiConstants.HOST_ID, type=CommandType.LONG, description="destination Host ID to deploy the VM to - parameter available for root admin only") private Long hostId; + + @Parameter(name=ApiConstants.SECURITY_GROUP_IDS, type=CommandType.LIST, collectionType=CommandType.LONG, description="comma separated list of security groups id that going to be applied to the virtual machine. Should be passed only when vm is created from a zone with Basic Network support. Mutually exclusive with securitygroupnames parameter") + private List securityGroupIdList; + + @Parameter(name=ApiConstants.SECURITY_GROUP_NAMES, type=CommandType.LIST, collectionType=CommandType.STRING, description="comma separated list of security groups names that going to be applied to the virtual machine. Should be passed only when vm is created from a zone with Basic Network support. Mutually exclusive with securitygroupids parameter") + private List securityGroupNameList; ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// @@ -141,7 +145,24 @@ public class DeployVMCmd extends BaseAsyncCreateCmd { } public List getSecurityGroupIdList() { - return securityGroupIdList; + if (securityGroupIdList != null && securityGroupIdList != null) { + throw new InvalidParameterValueException("securitygroupids parameter is mutually exclusive with securitygroupnames parameter"); + } + + //transform group names to ids here + if (securityGroupNameList != null) { + securityGroupIdList = new ArrayList(); + for (String groupName : securityGroupNameList) { + Long groupId = _responseGenerator.getSecurityGroupId(groupName, getEntityOwnerId()); + if (groupId == null) { + throw new InvalidParameterValueException("Unable to find group by name " + groupName + " for account " + getEntityOwnerId()); + } else { + securityGroupIdList.add(groupId); + } + } + } + + return securityGroupIdList; } public Long getServiceOfferingId() { @@ -201,15 +222,13 @@ public class DeployVMCmd extends BaseAsyncCreateCmd { Account userAccount = _responseGenerator.findAccountByNameDomain(accountName, domainId); if (userAccount != null) { return userAccount.getId(); + } else { + throw new InvalidParameterValueException("Unable to find account by name " + getAccountName() + " in domain " + getDomainId()); } } } - - if (account != null) { - return account.getId(); - } - - return Account.ACCOUNT_ID_SYSTEM; // no account info given, parent this command to SYSTEM so ERROR events are tracked + + return account.getId(); } @Override diff --git a/scripts/vm/hypervisor/xenserver/vmops b/scripts/vm/hypervisor/xenserver/vmops index 0b19967584a..ba5767cf1a2 100755 --- a/scripts/vm/hypervisor/xenserver/vmops +++ b/scripts/vm/hypervisor/xenserver/vmops @@ -654,7 +654,7 @@ def default_network_rules(session, args): return 'false' for v in vifs: - default_ebtables_rules(vm_chain, v, vm_ip, vm_mac) + default_ebtables_rules(vmchain, v, vm_ip, vm_mac) if write_rule_log_for_vm(vm_name, vm_id, vm_ip, domid, '_initial_', '-1') == False: util.SMlog("Failed to log default network rules, ignoring") diff --git a/server/src/com/cloud/api/ApiDBUtils.java b/server/src/com/cloud/api/ApiDBUtils.java index 87ff3ed0cfc..73bd4e2cc82 100755 --- a/server/src/com/cloud/api/ApiDBUtils.java +++ b/server/src/com/cloud/api/ApiDBUtils.java @@ -581,6 +581,10 @@ public class ApiDBUtils { float cpuOverprovisioningFactor = NumbersUtil.parseFloat(opFactor, 1); return cpuOverprovisioningFactor; } + + public static SecurityGroup getSecurityGroup(String groupName, long ownerId) { + return _securityGroupMgr.getSecurityGroup(groupName, ownerId); + } public static ConsoleProxyVO findConsoleProxy(long id) { return _consoleProxyDao.findById(id); diff --git a/server/src/com/cloud/api/ApiResponseHelper.java b/server/src/com/cloud/api/ApiResponseHelper.java index e4697ce0851..463a1843d58 100755 --- a/server/src/com/cloud/api/ApiResponseHelper.java +++ b/server/src/com/cloud/api/ApiResponseHelper.java @@ -2485,4 +2485,14 @@ public class ApiResponseHelper implements ResponseGenerator { response.setObjectName("network"); return response; } + + @Override + public Long getSecurityGroupId (String groupName, long accountId) { + SecurityGroup sg = ApiDBUtils.getSecurityGroup(groupName, accountId); + if (sg == null) { + return null; + } else { + return sg.getId(); + } + } } diff --git a/server/src/com/cloud/network/NetworkManagerImpl.java b/server/src/com/cloud/network/NetworkManagerImpl.java index 25ab9b54de9..56b50d44580 100755 --- a/server/src/com/cloud/network/NetworkManagerImpl.java +++ b/server/src/com/cloud/network/NetworkManagerImpl.java @@ -1446,7 +1446,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag @Override public void expungeNics(VirtualMachineProfile vm) { - List nics = _nicDao.listIncludingRemovedBy(vm.getId()); + List nics = _nicDao.listByVmIdIncludingRemoved(vm.getId()); for (NicVO nic : nics) { _nicDao.expunge(nic.getId()); } diff --git a/server/src/com/cloud/network/security/SecurityGroupManager.java b/server/src/com/cloud/network/security/SecurityGroupManager.java index e9bbb280d38..fefda1eed1a 100644 --- a/server/src/com/cloud/network/security/SecurityGroupManager.java +++ b/server/src/com/cloud/network/security/SecurityGroupManager.java @@ -34,7 +34,7 @@ public interface SecurityGroupManager { public SecurityGroupVO createSecurityGroup(String name, String description, Long domainId, Long accountId, String accountName); - public SecurityGroupVO createDefaultSecurityGroup( Long accountId); + public SecurityGroupVO createDefaultSecurityGroup(Long accountId); public boolean addInstanceToGroups(Long userVmId, List groups); @@ -47,4 +47,10 @@ public interface SecurityGroupManager { public List getSecurityGroupsForVm(long vmId); public boolean isVmSecurityGroupEnabled(Long vmId); + + SecurityGroup getDefaultSecurityGroup(long accountId); + + SecurityGroup getSecurityGroup(String name, long accountId); + + boolean isVmMappedToDefaultSecurityGroup(long vmId); } diff --git a/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java b/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java index db46e531980..50e6a7314cb 100755 --- a/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java +++ b/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java @@ -1168,4 +1168,30 @@ public class SecurityGroupManagerImpl implements SecurityGroupManager, SecurityG } return false; } + + @Override + public SecurityGroupVO getDefaultSecurityGroup(long accountId) { + return _securityGroupDao.findByAccountAndName(accountId, DEFAULT_GROUP_NAME); + } + + @Override + public SecurityGroup getSecurityGroup(String name, long accountId) { + return _securityGroupDao.findByAccountAndName(accountId, name); + } + + @Override + public boolean isVmMappedToDefaultSecurityGroup(long vmId) { + UserVmVO vm = _userVmMgr.getVirtualMachine(vmId); + SecurityGroup defaultGroup = getDefaultSecurityGroup(vm.getAccountId()); + if (defaultGroup == null) { + s_logger.warn("Unable to find default security group for account id=" + vm.getAccountId()); + return false; + } + SecurityGroupVMMapVO map = _securityGroupVMMapDao.findByVmIdGroupId(vmId, defaultGroup.getId()); + if (map == null) { + return false; + } else { + return true; + } + } } diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java index 5023907545d..e686ee38a82 100755 --- a/server/src/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/com/cloud/vm/UserVmManagerImpl.java @@ -126,6 +126,7 @@ import com.cloud.network.dao.NetworkDao; import com.cloud.network.lb.LoadBalancingRulesManager; import com.cloud.network.router.VirtualNetworkApplianceManager; import com.cloud.network.rules.RulesManager; +import com.cloud.network.security.SecurityGroup; import com.cloud.network.security.SecurityGroupManager; import com.cloud.network.vpn.PasswordResetElement; import com.cloud.offering.NetworkOffering; @@ -209,6 +210,7 @@ import com.cloud.vm.dao.UserVmDetailsDao; @Local(value = { UserVmManager.class, UserVmService.class }) public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager { private static final Logger s_logger = Logger.getLogger(UserVmManagerImpl.class); + private static final int ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_COOPERATION = 3; // 3 seconds @Inject @@ -292,7 +294,7 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager @Inject protected VMTemplateHostDao _vmTemplateHostDao; @Inject - protected SecurityGroupManager _networkGroupMgr; + protected SecurityGroupManager _securityGroupMgr; @Inject protected ServiceOfferingDao _serviceOfferingDao; @Inject @@ -1216,11 +1218,10 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager private boolean cleanupVmResources(long vmId) { boolean success = true; - - // Remove vm from security groups - _networkGroupMgr.removeInstanceFromGroups(vmId); - - // Remove vm from instance group + //Remove vm from security groups + _securityGroupMgr.removeInstanceFromGroups(vmId); + + //Remove vm from instance group removeInstanceFromInstanceGroup(vmId); // cleanup port forwarding rules @@ -2010,9 +2011,37 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager } else { networkList.add(_networkDao.findById(defaultNetwork.getId())); } - - return createVirtualMachine(zone, serviceOffering, template, hostName, displayName, owner, diskOfferingId, diskSize, networkList, securityGroupIdList, group, userData, sshKeyPair, hypervisor, - caller); + + if (securityGroupIdList == null) { + securityGroupIdList = new ArrayList(); + } + + SecurityGroup defaultGroup = _securityGroupMgr.getDefaultSecurityGroup(owner.getId()); + if (defaultGroup != null) { + //check if security group id list already contains Default security group, and if not - add it + boolean defaultGroupPresent = false; + for (Long securityGroupId : securityGroupIdList) { + if (securityGroupId.longValue() == defaultGroup.getId()) { + defaultGroupPresent = true; + break; + } + } + + if (!defaultGroupPresent) { + securityGroupIdList.add(defaultGroup.getId()); + } + + } else { + //create default security group for the account + if (s_logger.isDebugEnabled()) { + s_logger.debug("Couldn't find default security group for the account " + owner + " so creating a new one"); + } + defaultGroup = _securityGroupMgr.createSecurityGroup(SecurityGroupManager.DEFAULT_GROUP_NAME, SecurityGroupManager.DEFAULT_GROUP_DESCRIPTION, owner.getDomainId(), owner.getId(), owner.getAccountName()); + securityGroupIdList.add(defaultGroup.getId()); + } + + return createVirtualMachine(zone, serviceOffering, template, hostName, displayName, owner, diskOfferingId, + diskSize, networkList, securityGroupIdList, group, userData, sshKeyPair, hypervisor, caller); } @Override @@ -2023,8 +2052,9 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager Account caller = UserContext.current().getCaller(); List networkList = new ArrayList(); - - // Verify that caller can perform actions in behalf of vm owner + boolean isSecurityGroupEnabledNetworkUsed = false; + + //Verify that caller can perform actions in behalf of vm owner _accountMgr.checkAccess(caller, owner); // If no network is specified, find system security group enabled network @@ -2053,7 +2083,8 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager } networkList.add(network); - + isSecurityGroupEnabledNetworkUsed = true; + } else { // Verify that all the networks are Direct/Guest/AccountSpecific; can't create combination of SG enabled network and // regular networks @@ -2084,9 +2115,40 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager networkList.add(network); } } - - return createVirtualMachine(zone, serviceOffering, template, hostName, displayName, owner, diskOfferingId, diskSize, networkList, securityGroupIdList, group, userData, sshKeyPair, hypervisor, - caller); + + // if network is security group enabled, and default security group is not present in the list of groups specified, add it automatically + if (isSecurityGroupEnabledNetworkUsed) { + if (securityGroupIdList == null) { + securityGroupIdList = new ArrayList(); + } + + SecurityGroup defaultGroup = _securityGroupMgr.getDefaultSecurityGroup(owner.getId()); + if (defaultGroup != null) { + //check if security group id list already contains Default security group, and if not - add it + boolean defaultGroupPresent = false; + for (Long securityGroupId : securityGroupIdList) { + if (securityGroupId.longValue() == defaultGroup.getId()) { + defaultGroupPresent = true; + break; + } + } + + if (!defaultGroupPresent) { + securityGroupIdList.add(defaultGroup.getId()); + } + + } else { + //create default security group for the account + if (s_logger.isDebugEnabled()) { + s_logger.debug("Couldn't find default security group for the account " + owner + " so creating a new one"); + } + defaultGroup = _securityGroupMgr.createSecurityGroup(SecurityGroupManager.DEFAULT_GROUP_NAME, SecurityGroupManager.DEFAULT_GROUP_DESCRIPTION, owner.getDomainId(), owner.getId(), owner.getAccountName()); + securityGroupIdList.add(defaultGroup.getId()); + } + } + + return createVirtualMachine(zone, serviceOffering, template, hostName, displayName, owner, diskOfferingId, + diskSize, networkList, securityGroupIdList, group, userData, sshKeyPair, hypervisor, caller); } @Override @@ -2403,8 +2465,9 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager throw new CloudRuntimeException("Unable to assign Vm to the group " + group); } - _networkGroupMgr.addInstanceToGroups(vm.getId(), securityGroupIdList); - + + _securityGroupMgr.addInstanceToGroups(vm.getId(), securityGroupIdList); + return vm; } @@ -2720,6 +2783,22 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager userId = accountAndUserValidation(vmId, account, userId, vm); UserVO user = _userDao.findById(userId); + + //check if vm is security group enabled + if (_securityGroupMgr.isVmSecurityGroupEnabled(vmId) && !_securityGroupMgr.isVmMappedToDefaultSecurityGroup(vmId)) { + //if vm is not mapped to security group, create a mapping + if (s_logger.isDebugEnabled()) { + s_logger.debug("Vm " + vm + " is security group enabled, but not mapped to default security group; creating the mapping automatically"); + } + + SecurityGroup defaultSecurityGroup = _securityGroupMgr.getDefaultSecurityGroup(vm.getAccountId()); + if (defaultSecurityGroup != null) { + List groupList = new ArrayList(); + groupList.add(defaultSecurityGroup.getId()); + _securityGroupMgr.addInstanceToGroups(vmId, groupList); + } + } + return _itMgr.start(vm, null, user, account); } diff --git a/server/src/com/cloud/vm/dao/NicDao.java b/server/src/com/cloud/vm/dao/NicDao.java index 22706ad71be..e2408d08bfb 100644 --- a/server/src/com/cloud/vm/dao/NicDao.java +++ b/server/src/com/cloud/vm/dao/NicDao.java @@ -30,10 +30,8 @@ import com.cloud.vm.VirtualMachine; public interface NicDao extends GenericDao { List listByVmId(long instanceId); - List listByVmIdIncludingRemoved(long instanceId); - List listIpAddressInNetwork(long networkConfigId); - List listIncludingRemovedBy(long instanceId); + List listByVmIdIncludingRemoved(long instanceId); List listByNetworkId(long networkId); diff --git a/server/src/com/cloud/vm/dao/NicDaoImpl.java b/server/src/com/cloud/vm/dao/NicDaoImpl.java index 002a7dad7ab..a8c89b5cadf 100644 --- a/server/src/com/cloud/vm/dao/NicDaoImpl.java +++ b/server/src/com/cloud/vm/dao/NicDaoImpl.java @@ -77,13 +77,6 @@ public class NicDaoImpl extends GenericDaoBase implements NicDao { } - @Override - public List listIncludingRemovedBy(long instanceId) { - SearchCriteria sc = AllFieldsSearch.create(); - sc.setParameters("instance", instanceId); - return listIncludingRemovedBy(sc); - } - @Override public List listIpAddressInNetwork(long networkId) { SearchCriteria sc = IpSearch.create();