CLOUDSTACK-763: Fixed source CIDR and apply ACL items
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/33220630 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/33220630 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/33220630 Branch: refs/heads/master Commit: 33220630679e3707134a1f4228c1dc1727ced100 Parents: e2449cf Author: Kishan Kavala <kis...@cloud.com> Authored: Wed Apr 24 18:07:22 2013 +0530 Committer: Kishan Kavala <kis...@cloud.com> Committed: Mon May 13 12:03:38 2013 +0530 ---------------------------------------------------------------------- api/src/com/cloud/agent/api/to/NetworkACLTO.java | 6 +- .../cloud/network/firewall/NetworkACLService.java | 2 +- api/src/com/cloud/network/vpc/NetworkACLItem.java | 7 -- .../org/apache/cloudstack/api/ApiConstants.java | 1 + .../command/user/network/CreateNetworkACLCmd.java | 33 +++---- .../command/user/network/DeleteNetworkACLCmd.java | 30 ++---- .../api/response/NetworkACLItemResponse.java | 4 +- .../src/com/cloud/network/NetworkManagerImpl.java | 2 +- .../com/cloud/network/vpc/NetworkACLItemDao.java | 5 - .../com/cloud/network/vpc/NetworkACLItemVO.java | 70 +++++++------ .../com/cloud/network/vpc/NetworkACLManager.java | 7 +- .../cloud/network/vpc/NetworkACLManagerImpl.java | 82 +++++++++++---- .../network/vpc/dao/NetworkACLItemDaoImpl.java | 41 +------- setup/db/db/schema-410to420.sql | 4 +- 14 files changed, 142 insertions(+), 152 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/33220630/api/src/com/cloud/agent/api/to/NetworkACLTO.java ---------------------------------------------------------------------- diff --git a/api/src/com/cloud/agent/api/to/NetworkACLTO.java b/api/src/com/cloud/agent/api/to/NetworkACLTO.java index 48de40c..bd91f77 100644 --- a/api/src/com/cloud/agent/api/to/NetworkACLTO.java +++ b/api/src/com/cloud/agent/api/to/NetworkACLTO.java @@ -72,10 +72,10 @@ public class NetworkACLTO implements InternalIdentity { this.icmpCode = icmpCode; this.trafficType = trafficType; - if(allow){ - this.action = "ACCEPT"; - } else { + if(!allow){ this.action = "DROP"; + } else { + this.action = "ACCEPT"; } this.number = number; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/33220630/api/src/com/cloud/network/firewall/NetworkACLService.java ---------------------------------------------------------------------- diff --git a/api/src/com/cloud/network/firewall/NetworkACLService.java b/api/src/com/cloud/network/firewall/NetworkACLService.java index ae46d83..779e54e 100644 --- a/api/src/com/cloud/network/firewall/NetworkACLService.java +++ b/api/src/com/cloud/network/firewall/NetworkACLService.java @@ -33,7 +33,7 @@ import com.cloud.utils.Pair; public interface NetworkACLService { NetworkACLItem getNetworkACLItem(long ruleId); - boolean applyNetworkACLtoNetworks(long aclId, Account caller) throws ResourceUnavailableException; + boolean applyNetworkACL(long aclId, Account caller) throws ResourceUnavailableException; /** * @param createNetworkACLCmd http://git-wip-us.apache.org/repos/asf/cloudstack/blob/33220630/api/src/com/cloud/network/vpc/NetworkACLItem.java ---------------------------------------------------------------------- diff --git a/api/src/com/cloud/network/vpc/NetworkACLItem.java b/api/src/com/cloud/network/vpc/NetworkACLItem.java index 9cce187..308696c 100644 --- a/api/src/com/cloud/network/vpc/NetworkACLItem.java +++ b/api/src/com/cloud/network/vpc/NetworkACLItem.java @@ -30,11 +30,6 @@ public interface NetworkACLItem extends InternalIdentity { int getNumber(); - enum NetworkACLType { - System, // The pre-defined rules created by admin, in the system wide - User // the rules created by user, to a specific ip - } - enum State { Staged, // Rule been created but has never got through network rule conflict detection. Rules in this state can not be sent to network elements. Add, // Add means the rule has been created and has gone through network rule conflict detection. @@ -77,8 +72,6 @@ public interface NetworkACLItem extends InternalIdentity { List<String> getSourceCidrList(); - NetworkACLType getType(); - /** * @return */ http://git-wip-us.apache.org/repos/asf/cloudstack/blob/33220630/api/src/org/apache/cloudstack/api/ApiConstants.java ---------------------------------------------------------------------- diff --git a/api/src/org/apache/cloudstack/api/ApiConstants.java b/api/src/org/apache/cloudstack/api/ApiConstants.java index 170af2e..849376b 100755 --- a/api/src/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/org/apache/cloudstack/api/ApiConstants.java @@ -491,6 +491,7 @@ public class ApiConstants { public static final String ASA_INSIDE_PORT_PROFILE = "insideportprofile"; public static final String AFFINITY_GROUP_ID = "affinitygroupid"; public static final String ACL_ID = "aclid"; + public static final String NUMBER = "number"; public enum HostDetails { all, capacity, events, stats, min; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/33220630/api/src/org/apache/cloudstack/api/command/user/network/CreateNetworkACLCmd.java ---------------------------------------------------------------------- diff --git a/api/src/org/apache/cloudstack/api/command/user/network/CreateNetworkACLCmd.java b/api/src/org/apache/cloudstack/api/command/user/network/CreateNetworkACLCmd.java index b6c0eb6..b806a59 100644 --- a/api/src/org/apache/cloudstack/api/command/user/network/CreateNetworkACLCmd.java +++ b/api/src/org/apache/cloudstack/api/command/user/network/CreateNetworkACLCmd.java @@ -21,7 +21,6 @@ import java.util.List; import com.cloud.network.vpc.NetworkACL; import com.cloud.network.vpc.NetworkACLItem; -import com.cloud.network.vpc.NetworkACLItem.NetworkACLType; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; @@ -88,14 +87,16 @@ public class CreateNetworkACLCmd extends BaseAsyncCreateCmd { "can be Ingress or Egress, defaulted to Ingress if not specified") private String trafficType; + @Parameter(name=ApiConstants.NUMBER, type=CommandType.INTEGER, description="The network of the vm the ACL will be created for") + private Integer number; + + @Parameter(name=ApiConstants.ACTION, type=CommandType.STRING, description="scl entry action, allow or deny") + private String action; + // /////////////////////////////////////////////////// // ///////////////// Accessors /////////////////////// // /////////////////////////////////////////////////// - public Long getIpAddressId() { - return null; - } - public String getProtocol() { return protocol.trim(); } @@ -155,8 +156,12 @@ public class CreateNetworkACLCmd extends BaseAsyncCreateCmd { return s_name; } - public void setSourceCidrList(List<String> cidrs){ - cidrlist = cidrs; + public String getAction() { + return action; + } + + public Integer getNumber() { + return number; } @Override @@ -166,7 +171,7 @@ public class CreateNetworkACLCmd extends BaseAsyncCreateCmd { NetworkACLItem rule = _networkACLService.getNetworkACLItem(getEntityId()); try { UserContext.current().setEventDetails("Rule Id: " + getEntityId()); - success = _networkACLService.applyNetworkACLtoNetworks(rule.getACLId(), callerContext.getCaller()); + success = _networkACLService.applyNetworkACL(rule.getACLId(), callerContext.getCaller()); // State is different after the rule is applied, so get new object here NetworkACLItemResponse aclResponse = new NetworkACLItemResponse(); @@ -183,10 +188,6 @@ public class CreateNetworkACLCmd extends BaseAsyncCreateCmd { } } - public Long getSourceIpAddressId() { - return null; - } - public Integer getSourcePortStart() { if (publicStartPort != null) { return publicStartPort.intValue(); @@ -206,10 +207,6 @@ public class CreateNetworkACLCmd extends BaseAsyncCreateCmd { return null; } - public NetworkACLItem.State getState() { - throw new UnsupportedOperationException("Should never call me to find the state"); - } - public Long getNetworkId() { return networkId; } @@ -296,10 +293,6 @@ public class CreateNetworkACLCmd extends BaseAsyncCreateCmd { return null; } - public NetworkACLType getType() { - return NetworkACLType.User; - } - @Override public AsyncJob.Type getInstanceType() { return AsyncJob.Type.FirewallRule; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/33220630/api/src/org/apache/cloudstack/api/command/user/network/DeleteNetworkACLCmd.java ---------------------------------------------------------------------- diff --git a/api/src/org/apache/cloudstack/api/command/user/network/DeleteNetworkACLCmd.java b/api/src/org/apache/cloudstack/api/command/user/network/DeleteNetworkACLCmd.java index 2f88230..faf4630 100644 --- a/api/src/org/apache/cloudstack/api/command/user/network/DeleteNetworkACLCmd.java +++ b/api/src/org/apache/cloudstack/api/command/user/network/DeleteNetworkACLCmd.java @@ -25,6 +25,7 @@ import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.response.AccountResponse; import org.apache.cloudstack.api.response.FirewallRuleResponse; +import org.apache.cloudstack.api.response.NetworkACLItemResponse; import org.apache.cloudstack.api.response.SuccessResponse; import org.apache.log4j.Logger; @@ -44,7 +45,7 @@ public class DeleteNetworkACLCmd extends BaseAsyncCmd { //////////////// API parameters ///////////////////// ///////////////////////////////////////////////////// - @Parameter(name=ApiConstants.ID, type=CommandType.UUID, entityType = FirewallRuleResponse.class, + @Parameter(name=ApiConstants.ID, type=CommandType.UUID, entityType = NetworkACLItemResponse.class, required=true, description="the ID of the network ACL") private Long id; @@ -70,7 +71,7 @@ public class DeleteNetworkACLCmd extends BaseAsyncCmd { @Override public String getEventType() { - return EventTypes.EVENT_FIREWALL_CLOSE; + return EventTypes.EVENT_NETWORK_ACL_ITEM_DELETE; } @Override @@ -80,15 +81,19 @@ public class DeleteNetworkACLCmd extends BaseAsyncCmd { @Override public long getEntityOwnerId() { - if (ownerId == null) { + return 2L; +/* if (ownerId == null) { NetworkACLItem rule = _networkACLService.getNetworkACLItem(id); if (rule == null) { throw new InvalidParameterValueException("Unable to find network ACL by id=" + id); } else { - //ownerId = rule.getAccountId(); + + NetworkACL acl = _networkACLService + rule.getACLId(); + } } - return ownerId; + return ownerId;*/ } @Override @@ -104,20 +109,5 @@ public class DeleteNetworkACLCmd extends BaseAsyncCmd { } } - - @Override - public String getSyncObjType() { - return BaseAsyncCmd.networkSyncObject; - } - - @Override - public Long getSyncObjId() { - return _firewallService.getFirewallRule(id).getNetworkId(); - } - - @Override - public AsyncJob.Type getInstanceType() { - return AsyncJob.Type.FirewallRule; - } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/33220630/api/src/org/apache/cloudstack/api/response/NetworkACLItemResponse.java ---------------------------------------------------------------------- diff --git a/api/src/org/apache/cloudstack/api/response/NetworkACLItemResponse.java b/api/src/org/apache/cloudstack/api/response/NetworkACLItemResponse.java index d40acbf..177c42b 100644 --- a/api/src/org/apache/cloudstack/api/response/NetworkACLItemResponse.java +++ b/api/src/org/apache/cloudstack/api/response/NetworkACLItemResponse.java @@ -18,13 +18,15 @@ package org.apache.cloudstack.api.response; import java.util.List; +import com.cloud.network.vpc.NetworkACLItem; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseResponse; import com.cloud.serializer.Param; import com.google.gson.annotations.SerializedName; +import org.apache.cloudstack.api.EntityReference; -@SuppressWarnings("unused") +@EntityReference(value = NetworkACLItem.class) public class NetworkACLItemResponse extends BaseResponse { @SerializedName(ApiConstants.ID) @Param(description="the ID of the ACL Item") private String id; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/33220630/server/src/com/cloud/network/NetworkManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/NetworkManagerImpl.java b/server/src/com/cloud/network/NetworkManagerImpl.java index c6fb6b8..6667771 100755 --- a/server/src/com/cloud/network/NetworkManagerImpl.java +++ b/server/src/com/cloud/network/NetworkManagerImpl.java @@ -2686,7 +2686,7 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L } //apply network ACLs - if (!_networkACLMgr.applyNetworkACL(networkId, caller)) { + if (!_networkACLMgr.applyACLToNetwork(networkId, caller)) { s_logger.warn("Failed to reapply network ACLs as a part of of network id=" + networkId + " restart"); success = false; } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/33220630/server/src/com/cloud/network/vpc/NetworkACLItemDao.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/vpc/NetworkACLItemDao.java b/server/src/com/cloud/network/vpc/NetworkACLItemDao.java index 739aa8c..d24f082 100644 --- a/server/src/com/cloud/network/vpc/NetworkACLItemDao.java +++ b/server/src/com/cloud/network/vpc/NetworkACLItemDao.java @@ -31,14 +31,9 @@ public interface NetworkACLItemDao extends GenericDao<NetworkACLItemVO, Long> { boolean revoke(NetworkACLItemVO rule); - boolean releasePorts(long ipAddressId, String protocol, int[] ports); - List<NetworkACLItemVO> listByACL(long aclId); - List<NetworkACLItemVO> listSystemRules(); - List<NetworkACLItemVO> listByACLTrafficTypeAndNotRevoked(long aclId, NetworkACLItemVO.TrafficType trafficType); List<NetworkACLItemVO> listByACLTrafficType(long aclId, NetworkACLItemVO.TrafficType trafficType); - void loadSourceCidrs(NetworkACLItemVO rule); } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/33220630/server/src/com/cloud/network/vpc/NetworkACLItemVO.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/vpc/NetworkACLItemVO.java b/server/src/com/cloud/network/vpc/NetworkACLItemVO.java index 9b24631..5df97a9 100644 --- a/server/src/com/cloud/network/vpc/NetworkACLItemVO.java +++ b/server/src/com/cloud/network/vpc/NetworkACLItemVO.java @@ -21,9 +21,7 @@ import com.cloud.utils.db.GenericDao; import com.cloud.utils.net.NetUtils; import javax.persistence.*; -import java.util.Date; -import java.util.List; -import java.util.UUID; +import java.util.*; @Entity @Table(name="network_acl_item") @@ -59,31 +57,50 @@ public class NetworkACLItemVO implements NetworkACLItem { @Column(name="icmp_type") Integer icmpType; - @Column(name="type") - @Enumerated(value=EnumType.STRING) - NetworkACLType type; - @Column(name="traffic_type") @Enumerated(value=EnumType.STRING) TrafficType trafficType; - - // This is a delayed load value. If the value is null, - // then this field has not been loaded yet. - // Call firewallrules dao to load it. - @Transient - List<String> sourceCidrs; + @Column(name="cidr") + String sourceCidrs; @Column(name="uuid") String uuid; + @Column(name="number") + int number; + + @Column(name="action") + @Enumerated(value=EnumType.STRING) + Action action; + public void setSourceCidrList(List<String> sourceCidrs) { - this.sourceCidrs=sourceCidrs; + if(sourceCidrs == null){ + this.sourceCidrs = null; + } else { + StringBuilder sb = new StringBuilder(); + for(String cidr : sourceCidrs){ + if(sb.length() != 0){ + sb.append(","); + } + sb.append(cidr); + } + this.sourceCidrs=sb.toString(); + } } @Override public List<String> getSourceCidrList() { - return sourceCidrs; + if(sourceCidrs == null || sourceCidrs.isEmpty()){ + return null; + } else { + List<String> cidrList = new ArrayList<String>(); + String[] cidrs = sourceCidrs.split(","); + for(String cidr : cidrs){ + cidrList.add(cidr); + } + return cidrList; + } } @Override @@ -120,10 +137,6 @@ public class NetworkACLItemVO implements NetworkACLItem { return ACLId; } - @Override - public NetworkACLType getType() { - return type; - } public Date getCreated() { return created; } @@ -134,7 +147,7 @@ public class NetworkACLItemVO implements NetworkACLItem { public NetworkACLItemVO(Integer portStart, Integer portEnd, String protocol, long aclId, List<String> sourceCidrs, Integer icmpCode, - Integer icmpType, TrafficType trafficType) { + Integer icmpType, TrafficType trafficType, Action action, int number) { this.sourcePortStart = portStart; this.sourcePortEnd = portEnd; this.protocol = protocol; @@ -142,15 +155,16 @@ public class NetworkACLItemVO implements NetworkACLItem { this.state = State.Staged; this.icmpCode = icmpCode; this.icmpType = icmpType; - this.sourceCidrs = sourceCidrs; + setSourceCidrList(sourceCidrs); this.uuid = UUID.randomUUID().toString(); - this.type = NetworkACLType.User; this.trafficType = trafficType; + this.action = action; + this.number = number; } - public NetworkACLItemVO(int port, String protocol, long aclId, List<String> sourceCidrs, Integer icmpCode, Integer icmpType) { - this(port, port, protocol, aclId, sourceCidrs, icmpCode, icmpType, null); + public NetworkACLItemVO(int port, String protocol, long aclId, List<String> sourceCidrs, Integer icmpCode, Integer icmpType, Action action, int number) { + this(port, port, protocol, aclId, sourceCidrs, icmpCode, icmpType, null, action, number); } @Override @@ -175,22 +189,18 @@ public class NetworkACLItemVO implements NetworkACLItem { @Override public Action getAction() { - return null; //To change body of implemented methods use File | Settings | File Templates. + return action; } @Override public int getNumber() { - return 0; //To change body of implemented methods use File | Settings | File Templates. + return number; } public void setUuid(String uuid) { this.uuid = uuid; } - public void setType(NetworkACLType type) { - this.type = type; - } - @Override public TrafficType getTrafficType() { return trafficType; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/33220630/server/src/com/cloud/network/vpc/NetworkACLManager.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/vpc/NetworkACLManager.java b/server/src/com/cloud/network/vpc/NetworkACLManager.java index dba91b2..3be15fa 100644 --- a/server/src/com/cloud/network/vpc/NetworkACLManager.java +++ b/server/src/com/cloud/network/vpc/NetworkACLManager.java @@ -39,8 +39,9 @@ public interface NetworkACLManager extends NetworkACLService{ List<NetworkACLItemVO> listNetworkACLItems(long guestNtwkId); - boolean applyNetworkACL(long networkId, Account caller) throws ResourceUnavailableException; + boolean applyNetworkACL(long aclId, Account caller) throws ResourceUnavailableException; - @DB - void revokeRule(NetworkACLItemVO rule, Account caller, long userId, boolean needUsageEvent); + void removeRule(NetworkACLItem rule); + + boolean applyACLToNetwork(long networkId, Account caller) throws ResourceUnavailableException; } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/33220630/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java b/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java index a345cc6..8c6cf35 100644 --- a/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java +++ b/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java @@ -111,13 +111,16 @@ public class NetworkACLManagerImpl extends ManagerBase implements NetworkACLMana for (NetworkACLItemVO aclItem : aclItems) { // Mark all Network ACLs rules as Revoke, but don't revoke them yet - we have to revoke all rules for ip, no // need to send them one by one - revokeNetworkACLItem(aclItem.getId(), false, caller, Account.ACCOUNT_ID_SYSTEM); + //revokeNetworkACLItem(aclItem.getId(), false, caller, Account.ACCOUNT_ID_SYSTEM); + if (aclItem.getState() == State.Add || aclItem.getState() == State.Active) { + aclItem.setState(State.Revoke); + } } //List<NetworkACLItemVO> ACLsToRevoke = _networkACLItemDao.listByNetwork(networkId); // now send everything to the backend - boolean success = applyNetworkACL(network.getNetworkACLId(), caller); + boolean success = applyACLItemsToNetwork(network.getId(), aclItems, caller); if (s_logger.isDebugEnabled()) { s_logger.debug("Successfully released Network ACLs for network id=" + networkId + " and # of rules now = " @@ -139,22 +142,45 @@ public class NetworkACLManagerImpl extends ManagerBase implements NetworkACLMana } @Override - public boolean applyNetworkACLtoNetworks(long aclId, Account caller) throws ResourceUnavailableException { + public boolean applyNetworkACL(long aclId, Account caller) throws ResourceUnavailableException { boolean handled = false; List<NetworkACLItemVO> rules = _networkACLItemDao.listByACL(aclId); //Find all networks using this ACL List<NetworkVO> networks = _networkDao.listByAclId(aclId); for(NetworkVO network : networks){ - applyNetworkACL(network.getId(), caller); + //Failure case?? + handled = applyACLItemsToNetwork(network.getId(), rules, caller); + } + if(handled){ + for (NetworkACLItem rule : rules) { + if (rule.getState() == NetworkACLItem.State.Revoke) { + removeRule(rule); + } else if (rule.getState() == NetworkACLItem.State.Add) { + NetworkACLItemVO ruleVO = _networkACLItemDao.findById(rule.getId()); + ruleVO.setState(NetworkACLItem.State.Active); + _networkACLItemDao.update(ruleVO.getId(), ruleVO); + } + } } return handled; } @Override - public boolean applyNetworkACL(long networkId, Account caller) throws ResourceUnavailableException { + public void removeRule(NetworkACLItem rule) { + //remove the rule + _networkACLItemDao.remove(rule.getId()); + } + + @Override + public boolean applyACLToNetwork(long networkId, Account caller) throws ResourceUnavailableException { Network network = _networkDao.findById(networkId); - boolean handled = false; List<NetworkACLItemVO> rules = _networkACLItemDao.listByACL(network.getNetworkACLId()); + return applyACLItemsToNetwork(networkId, rules, caller); + } + + public boolean applyACLItemsToNetwork(long networkId, List<NetworkACLItemVO> rules, Account caller) throws ResourceUnavailableException { + Network network = _networkDao.findById(networkId); + boolean handled = false; for (NetworkACLServiceProvider element: _networkAclElements) { Network.Provider provider = element.getProvider(); boolean isAclProvider = _networkModel.isProviderSupportServiceInNetwork(network.getId(), Service.NetworkACL, provider); @@ -170,19 +196,16 @@ public class NetworkACLManagerImpl extends ManagerBase implements NetworkACLMana @Override public NetworkACLItem createNetworkACLItem(CreateNetworkACLCmd aclItemCmd) throws NetworkRuleConflictException { - if (aclItemCmd.getSourceCidrList() == null) { - //_networkACLItemDao.loadSourceCidrs(aclItemCmd); - } return createNetworkACLItem(UserContext.current().getCaller(), aclItemCmd.getSourcePortStart(), aclItemCmd.getSourcePortEnd(), aclItemCmd.getProtocol(), aclItemCmd.getSourceCidrList(), aclItemCmd.getIcmpCode(), - aclItemCmd.getIcmpType(), null, aclItemCmd.getType(), aclItemCmd.getNetworkId(), aclItemCmd.getTrafficType(), aclItemCmd.getACLId()); + aclItemCmd.getIcmpType(), aclItemCmd.getNetworkId(), aclItemCmd.getTrafficType(), aclItemCmd.getACLId(), aclItemCmd.getAction(), aclItemCmd.getNumber()); } @DB @ActionEvent(eventType = EventTypes.EVENT_NETWORK_ACL_ITEM_CREATE, eventDescription = "creating network ACL Item", create = true) protected NetworkACLItem createNetworkACLItem(Account caller, Integer portStart, Integer portEnd, String protocol, List<String> sourceCidrList, - Integer icmpCode, Integer icmpType, Long relatedRuleId, NetworkACLItem.NetworkACLType type, - Long networkId, NetworkACLItem.TrafficType trafficType, Long aclId) throws NetworkRuleConflictException { + Integer icmpCode, Integer icmpType, Long networkId, NetworkACLItem.TrafficType trafficType, Long aclId, + String action, Integer number) throws NetworkRuleConflictException { if(aclId == null){ Network network = _networkMgr.getNetwork(networkId); @@ -229,19 +252,22 @@ public class NetworkACLManagerImpl extends ManagerBase implements NetworkACLMana } } + NetworkACLItem.Action ruleAction = NetworkACLItem.Action.Allow; + if("deny".equals(action)){ + ruleAction = NetworkACLItem.Action.Deny; + } + // If number is null, set it to currentMax + 1 validateNetworkACLItem(caller, portStart, portEnd, protocol); Transaction txn = Transaction.currentTxn(); txn.start(); - NetworkACLItemVO newRule = new NetworkACLItemVO(portStart, portEnd, protocol.toLowerCase(), aclId, sourceCidrList, icmpCode, icmpType, trafficType); - newRule.setType(type); + + NetworkACLItemVO newRule = new NetworkACLItemVO(portStart, portEnd, protocol.toLowerCase(), aclId, sourceCidrList, icmpCode, icmpType, trafficType, ruleAction, number); newRule = _networkACLItemDao.persist(newRule); - if (type == NetworkACLItem.NetworkACLType.User) { //ToDo: Is this required now with?? //detectNetworkACLConflict(newRule); - } if (!_networkACLItemDao.setStateToAdd(newRule)) { throw new CloudRuntimeException("Unable to update the state to add for " + newRule); @@ -292,7 +318,8 @@ public class NetworkACLManagerImpl extends ManagerBase implements NetworkACLMana if (apply) { try { - success = applyNetworkACL(rule.getACLId(), caller); + applyNetworkACL(rule.getACLId(), caller); + success = true; } catch (ResourceUnavailableException e) { e.printStackTrace(); //To change body of catch statement use File | Settings | File Templates. } @@ -327,7 +354,7 @@ public class NetworkACLManagerImpl extends ManagerBase implements NetworkACLMana // _accountMgr.buildACLSearchBuilder(sb, domainId, isRecursive, permittedAccounts, listProjectResourcesCriteria); sb.and("id", sb.entity().getId(), Op.EQ); - //sb.and("networkId", sb.entity().getNetworkId(), Op.EQ); + sb.and("aclId", sb.entity().getACLId(), Op.EQ); sb.and("trafficType", sb.entity().getTrafficType(), Op.EQ); if (tags != null && !tags.isEmpty()) { @@ -350,7 +377,8 @@ public class NetworkACLManagerImpl extends ManagerBase implements NetworkACLMana } if (networkId != null) { - sc.setParameters("networkId", networkId); + Network network = _networkDao.findById(networkId); + sc.setParameters("aclId", network.getNetworkACLId()); } if (trafficType != null) { @@ -400,13 +428,25 @@ public class NetworkACLManagerImpl extends ManagerBase implements NetworkACLMana @Override public boolean replaceNetworkACL(long aclId, long networkId) { NetworkVO network = _networkDao.findById(networkId); + if(network == null){ + throw new InvalidParameterValueException("Unable to find Network: " +networkId); + } + NetworkACL acl = _networkACLDao.findById(aclId); + if(acl == null){ + throw new InvalidParameterValueException("Unable to find NetworkACL: " +aclId); + } + if(network.getVpcId() == null){ + throw new InvalidParameterValueException("Network does not belong to VPC: " +networkId); + } + if(network.getVpcId() != acl.getVpcId()){ + throw new InvalidParameterValueException("Network: "+networkId+" and ACL: "+aclId+" do not belong to the same VPC"); + } network.setNetworkACLId(aclId); return _networkDao.update(networkId, network); } - @Override @DB - public void revokeRule(NetworkACLItemVO rule, Account caller, long userId, boolean needUsageEvent) { + private void revokeRule(NetworkACLItemVO rule, Account caller, long userId, boolean needUsageEvent) { if (caller != null) { //_accountMgr.checkAccess(caller, null, true, rule); } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/33220630/server/src/com/cloud/network/vpc/dao/NetworkACLItemDaoImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/vpc/dao/NetworkACLItemDaoImpl.java b/server/src/com/cloud/network/vpc/dao/NetworkACLItemDaoImpl.java index 69810e0..98f5d6f 100644 --- a/server/src/com/cloud/network/vpc/dao/NetworkACLItemDaoImpl.java +++ b/server/src/com/cloud/network/vpc/dao/NetworkACLItemDaoImpl.java @@ -16,28 +16,18 @@ // under the License. package com.cloud.network.vpc.dao; -import com.cloud.network.dao.FirewallRulesCidrsDao; -import com.cloud.network.dao.IPAddressDao; -import com.cloud.network.dao.IPAddressVO; import com.cloud.network.vpc.NetworkACLItem; import com.cloud.network.vpc.NetworkACLItem.State; import com.cloud.network.vpc.NetworkACLItemDao; import com.cloud.network.vpc.NetworkACLItemVO; -import com.cloud.server.ResourceTag.TaggedResourceType; -import com.cloud.tags.dao.ResourceTagDao; import com.cloud.utils.db.DB; import com.cloud.utils.db.GenericDaoBase; -import com.cloud.utils.db.GenericSearchBuilder; -import com.cloud.utils.db.JoinBuilder; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; -import com.cloud.utils.db.SearchCriteria.Func; import com.cloud.utils.db.SearchCriteria.Op; -import com.cloud.utils.db.Transaction; import org.springframework.stereotype.Component; import javax.ejb.Local; -import javax.inject.Inject; import java.util.List; @Component @@ -48,13 +38,6 @@ public class NetworkACLItemDaoImpl extends GenericDaoBase<NetworkACLItemVO, Long protected final SearchBuilder<NetworkACLItemVO> AllFieldsSearch; protected final SearchBuilder<NetworkACLItemVO> NotRevokedSearch; protected final SearchBuilder<NetworkACLItemVO> ReleaseSearch; - protected SearchBuilder<NetworkACLItemVO> VmSearch; - protected final SearchBuilder<NetworkACLItemVO> SystemRuleSearch; - protected final GenericSearchBuilder<NetworkACLItemVO, Long> RulesByIpCount; - - @Inject protected FirewallRulesCidrsDao _firewallRulesCidrsDao; - @Inject ResourceTagDao _tagsDao; - @Inject IPAddressDao _ipDao; protected NetworkACLItemDaoImpl() { super(); @@ -81,13 +64,6 @@ public class NetworkACLItemDaoImpl extends GenericDaoBase<NetworkACLItemVO, Long ReleaseSearch.and("ports", ReleaseSearch.entity().getSourcePortStart(), Op.IN); ReleaseSearch.done(); - SystemRuleSearch = createSearchBuilder(); - SystemRuleSearch.and("type", SystemRuleSearch.entity().getType(), Op.EQ); - SystemRuleSearch.done(); - - RulesByIpCount = createSearchBuilder(Long.class); - RulesByIpCount.select(null, Func.COUNT, RulesByIpCount.entity().getId()); - RulesByIpCount.done(); } @@ -109,12 +85,8 @@ public class NetworkACLItemDaoImpl extends GenericDaoBase<NetworkACLItemVO, Long @Override public boolean revoke(NetworkACLItemVO rule) { - return false; //To change body of implemented methods use File | Settings | File Templates. - } - - @Override - public boolean releasePorts(long ipAddressId, String protocol, int[] ports) { - return false; //To change body of implemented methods use File | Settings | File Templates. + rule.setState(State.Revoke); + return update(rule.getId(), rule); } @Override @@ -126,11 +98,6 @@ public class NetworkACLItemDaoImpl extends GenericDaoBase<NetworkACLItemVO, Long } @Override - public List<NetworkACLItemVO> listSystemRules() { - return null; //To change body of implemented methods use File | Settings | File Templates. - } - - @Override public List<NetworkACLItemVO> listByACLTrafficTypeAndNotRevoked(long aclId, NetworkACLItem.TrafficType trafficType) { return null; //To change body of implemented methods use File | Settings | File Templates. } @@ -140,8 +107,4 @@ public class NetworkACLItemDaoImpl extends GenericDaoBase<NetworkACLItemVO, Long return null; //To change body of implemented methods use File | Settings | File Templates. } - @Override - public void loadSourceCidrs(NetworkACLItemVO rule) { - //To change body of implemented methods use File | Settings | File Templates. - } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/33220630/setup/db/db/schema-410to420.sql ---------------------------------------------------------------------- diff --git a/setup/db/db/schema-410to420.sql b/setup/db/db/schema-410to420.sql index 6c8a3dd..31ac75d 100644 --- a/setup/db/db/schema-410to420.sql +++ b/setup/db/db/schema-410to420.sql @@ -1199,8 +1199,10 @@ CREATE TABLE `cloud`.`network_acl_item` ( `created` datetime COMMENT 'Date created', `icmp_code` int(10) COMMENT 'The ICMP code (if protocol=ICMP). A value of -1 means all codes for the given ICMP type.', `icmp_type` int(10) COMMENT 'The ICMP type (if protocol=ICMP). A value of -1 means all types.', - `type` varchar(10) NOT NULL DEFAULT 'USER', `traffic_type` char(32) COMMENT 'the traffic type of the rule, can be Ingress or Egress', + `cidr` varchar(255) COMMENT 'comma seperated cidr list', + `number` int(10) NOT NULL COMMENT 'priority number of the acl item', + `action` varchar(10) NOT NULL COMMENT 'rule action, allow or deny', PRIMARY KEY (`id`), CONSTRAINT `fk_network_acl_item__acl_id` FOREIGN KEY(`acl_id`) REFERENCES `network_acl`(`id`) ON DELETE CASCADE, CONSTRAINT `uc_network_acl_item__uuid` UNIQUE (`uuid`)
