cloudstack-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CLOUDSTACK-10314) Add Text-Field to each ACL Rule
Date Tue, 13 Mar 2018 14:08:01 GMT

    [ https://issues.apache.org/jira/browse/CLOUDSTACK-10314?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16396980#comment-16396980 ] 

ASF GitHub Bot commented on CLOUDSTACK-10314:
---------------------------------------------

rafaelweingartner closed pull request #2475: [CLOUDSTACK-10314] Add Text-Field to each ACL Rule
URL: https://github.com/apache/cloudstack/pull/2475
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/api/src/main/java/com/cloud/network/vpc/NetworkACLItem.java b/api/src/main/java/com/cloud/network/vpc/NetworkACLItem.java
index 75153fd7c5a..eeca375c46f 100644
--- a/api/src/main/java/com/cloud/network/vpc/NetworkACLItem.java
+++ b/api/src/main/java/com/cloud/network/vpc/NetworkACLItem.java
@@ -24,6 +24,7 @@
 
 public interface NetworkACLItem extends InternalIdentity, Identity, Displayable {
 
+    @Override
     String getUuid();
 
     Action getAction();
@@ -51,7 +52,7 @@
     Integer getSourcePortStart();
 
     /**
-     * @return last port of the source prot range.  If this is null, that means only one port is mapped.
+     * @return last port of the source port range.  If this is null, that means only one port is mapped.
      */
     Integer getSourcePortEnd();
 
@@ -70,12 +71,10 @@
 
     List<String> getSourceCidrList();
 
-    /**
-     * @return
-     */
     TrafficType getTrafficType();
 
     @Override
     boolean isDisplay();
 
+    String getReason();
 }
diff --git a/api/src/main/java/com/cloud/network/vpc/NetworkACLService.java b/api/src/main/java/com/cloud/network/vpc/NetworkACLService.java
index f08fff5425d..dd7c862d46b 100644
--- a/api/src/main/java/com/cloud/network/vpc/NetworkACLService.java
+++ b/api/src/main/java/com/cloud/network/vpc/NetworkACLService.java
@@ -21,6 +21,7 @@
 import org.apache.cloudstack.api.command.user.network.CreateNetworkACLCmd;
 import org.apache.cloudstack.api.command.user.network.ListNetworkACLListsCmd;
 import org.apache.cloudstack.api.command.user.network.ListNetworkACLsCmd;
+import org.apache.cloudstack.api.command.user.network.UpdateNetworkACLItemCmd;
 
 import com.cloud.exception.ResourceUnavailableException;
 import com.cloud.utils.Pair;
@@ -28,106 +29,61 @@
 public interface NetworkACLService {
     /**
      * Creates Network ACL for the specified VPC
-     * @param name
-     * @param description
-     * @param vpcId
-     * @param forDisplay TODO
-     * @return
      */
     NetworkACL createNetworkACL(String name, String description, long vpcId, Boolean forDisplay);
 
     /**
      * Get Network ACL with specified Id
-     * @param id
-     * @return
      */
     NetworkACL getNetworkACL(long id);
 
     /**
      * List NetworkACLs by Id/Name/Network or Vpc it belongs to
-     * @param cmd
-     * @return
      */
     Pair<List<? extends NetworkACL>, Integer> listNetworkACLs(ListNetworkACLListsCmd cmd);
 
     /**
      * Delete specified network ACL. Deletion fails if the list is not empty
-     * @param id
-     * @return
      */
     boolean deleteNetworkACL(long id);
 
     /**
      * Associates ACL with specified Network
-     * @param aclId
-     * @param networkId
-     * @return
-     * @throws ResourceUnavailableException
      */
     boolean replaceNetworkACL(long aclId, long networkId) throws ResourceUnavailableException;
 
     /**
      * Applied ACL to associated networks
-     * @param aclId
-     * @return
-     * @throws ResourceUnavailableException
      */
     boolean applyNetworkACL(long aclId) throws ResourceUnavailableException;
 
     /**
      * Creates a Network ACL Item within an ACL and applies the ACL to associated networks
-     * @param createNetworkACLCmd
-     * @return
      */
     NetworkACLItem createNetworkACLItem(CreateNetworkACLCmd aclItemCmd);
 
     /**
      * Return ACL item with specified Id
-     * @param ruleId
-     * @return
      */
     NetworkACLItem getNetworkACLItem(long ruleId);
 
     /**
      * Lists Network ACL Items by Id, Network, ACLId, Traffic Type, protocol
-     * @param listNetworkACLsCmd
-     * @return
      */
     Pair<List<? extends NetworkACLItem>, Integer> listNetworkACLItems(ListNetworkACLsCmd cmd);
 
     /**
      * Revoke ACL Item with specified Id
-     * @param ruleId
-     * @return
      */
     boolean revokeNetworkACLItem(long ruleId);
 
     /**
      * Updates existing aclItem applies to associated networks
-     * @param id
-     * @param protocol
-     * @param sourceCidrList
-     * @param trafficType
-     * @param action
-     * @param number
-     * @param sourcePortStart
-     * @param sourcePortEnd
-     * @param icmpCode
-     * @param icmpType
-     * @param newUUID TODO
-     * @param forDisplay TODO
-     * @return
-     * @throws ResourceUnavailableException
      */
-    NetworkACLItem updateNetworkACLItem(Long id, String protocol, List<String> sourceCidrList, NetworkACLItem.TrafficType trafficType, String action, Integer number,
-            Integer sourcePortStart, Integer sourcePortEnd, Integer icmpCode, Integer icmpType, String newUUID, Boolean forDisplay) throws ResourceUnavailableException;
+    NetworkACLItem updateNetworkACLItem(UpdateNetworkACLItemCmd updateNetworkACLItemCmd) throws ResourceUnavailableException;
 
     /**
      * Associates ACL with specified Network
-     * @param aclId
-     * @param privateGatewayId
-     * @return
-     * @throws ResourceUnavailableException
      */
     boolean replaceNetworkACLonPrivateGw(long aclId, long privateGatewayId) throws ResourceUnavailableException;
 
diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
index d9090b89217..ac9406fee99 100644
--- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
+++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
@@ -452,6 +452,7 @@
     public static final String SUPPORTED_SERVICES = "supportedservices";
     public static final String NSP_ID = "nspid";
     public static final String ACL_TYPE = "acltype";
+    public static final String ACL_REASON = "reason";
     public static final String SUBDOMAIN_ACCESS = "subdomainaccess";
     public static final String LOAD_BALANCER_DEVICE_ID = "lbdeviceid";
     public static final String LOAD_BALANCER_DEVICE_NAME = "lbdevicename";
diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkACLCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkACLCmd.java
index 4b6a836f32a..00c81a7b05b 100644
--- a/api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkACLCmd.java
+++ b/api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkACLCmd.java
@@ -40,11 +40,7 @@
 import com.cloud.user.Account;
 import com.cloud.utils.net.NetUtils;
 
-@APICommand(name = "createNetworkACL",
-            description = "Creates a ACL rule in the given network (the network has to belong to VPC)",
-            responseObject = NetworkACLItemResponse.class,
-            requestHasSensitiveInfo = false,
-            responseHasSensitiveInfo = false)
+@APICommand(name = "createNetworkACL", description = "Creates a ACL rule in the given network (the network has to belong to VPC)", responseObject = NetworkACLItemResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
 public class CreateNetworkACLCmd extends BaseAsyncCreateCmd {
     public static final Logger s_logger = Logger.getLogger(CreateNetworkACLCmd.class.getName());
 
@@ -54,10 +50,7 @@
     // ////////////// API parameters /////////////////////
     // ///////////////////////////////////////////////////
 
-    @Parameter(name = ApiConstants.PROTOCOL,
-               type = CommandType.STRING,
-               required = true,
-               description = "the protocol for the ACL rule. Valid values are TCP/UDP/ICMP/ALL or valid protocol number")
+    @Parameter(name = ApiConstants.PROTOCOL, type = CommandType.STRING, required = true, description = "the protocol for the ACL rule. Valid values are TCP/UDP/ICMP/ALL or valid protocol number")
     private String protocol;
 
     @Parameter(name = ApiConstants.START_PORT, type = CommandType.INTEGER, description = "the starting port of ACL")
@@ -75,20 +68,13 @@
     @Parameter(name = ApiConstants.ICMP_CODE, type = CommandType.INTEGER, description = "error code for this ICMP message")
     private Integer icmpCode;
 
-    @Parameter(name = ApiConstants.NETWORK_ID,
-               type = CommandType.UUID,
-               entityType = NetworkResponse.class,
-               description = "The network of the VM the ACL will be created for")
+    @Parameter(name = ApiConstants.NETWORK_ID, type = CommandType.UUID, entityType = NetworkResponse.class, description = "The network of the VM the ACL will be created for")
     private Long networkId;
 
-    @Parameter(name = ApiConstants.ACL_ID,
-               type = CommandType.UUID,
-               entityType = NetworkACLResponse.class,
-               description = "The network of the VM the ACL will be created for")
+    @Parameter(name = ApiConstants.ACL_ID, type = CommandType.UUID, entityType = NetworkACLResponse.class, description = "The network of the VM the ACL will be created for")
     private Long aclId;
 
-    @Parameter(name = ApiConstants.TRAFFIC_TYPE, type = CommandType.STRING, description = "the traffic type for the ACL,"
-        + "can be ingress or egress, defaulted to ingress if not specified")
+    @Parameter(name = ApiConstants.TRAFFIC_TYPE, type = CommandType.STRING, description = "the traffic type for the ACL," + "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")
@@ -97,16 +83,16 @@
     @Parameter(name = ApiConstants.ACTION, type = CommandType.STRING, description = "scl entry action, allow or deny")
     private String action;
 
-    @Parameter(name = ApiConstants.FOR_DISPLAY, type = CommandType.BOOLEAN, description = "an optional field, whether to the display the rule to the end user or not", since = "4.4", authorized = {RoleType.Admin})
+    @Parameter(name = ApiConstants.FOR_DISPLAY, type = CommandType.BOOLEAN, description = "an optional field, whether to the display the rule to the end user or not", since = "4.4", authorized = {
+            RoleType.Admin})
     private Boolean display;
 
+    @Parameter(name = ApiConstants.ACL_REASON, type = CommandType.STRING, description = "A description indicating why the ACL rule is required.")
+    private String reason;
+
     // ///////////////////////////////////////////////////
     // ///////////////// Accessors ///////////////////////
     // ///////////////////////////////////////////////////
-    @Deprecated
-    public Boolean getDisplay() {
-        return display;
-    }
 
     @Override
     public boolean isDisplay() {
@@ -227,6 +213,10 @@ public Long getACLId() {
         return aclId;
     }
 
+    public String getReason() {
+        return reason;
+    }
+
     @Override
     public void create() {
         NetworkACLItem result = _networkACLService.createNetworkACLItem(this);
@@ -257,5 +247,4 @@ public void execute() throws ResourceUnavailableException {
             }
         }
     }
-
-}
+}
\ No newline at end of file
diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/network/UpdateNetworkACLItemCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/network/UpdateNetworkACLItemCmd.java
index acc2ae86420..1530239fd12 100644
--- a/api/src/main/java/org/apache/cloudstack/api/command/user/network/UpdateNetworkACLItemCmd.java
+++ b/api/src/main/java/org/apache/cloudstack/api/command/user/network/UpdateNetworkACLItemCmd.java
@@ -21,10 +21,8 @@
 import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.ApiConstants;
-import org.apache.cloudstack.api.ApiErrorCode;
 import org.apache.cloudstack.api.BaseAsyncCustomIdCmd;
 import org.apache.cloudstack.api.Parameter;
-import org.apache.cloudstack.api.ServerApiException;
 import org.apache.cloudstack.api.response.NetworkACLItemResponse;
 import org.apache.cloudstack.context.CallContext;
 import org.apache.log4j.Logger;
@@ -34,8 +32,7 @@
 import com.cloud.network.vpc.NetworkACLItem;
 import com.cloud.user.Account;
 
-@APICommand(name = "updateNetworkACLItem", description = "Updates ACL item with specified ID", responseObject = NetworkACLItemResponse.class,
-        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+@APICommand(name = "updateNetworkACLItem", description = "Updates ACL item with specified ID", responseObject = NetworkACLItemResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
 public class UpdateNetworkACLItemCmd extends BaseAsyncCustomIdCmd {
     public static final Logger s_logger = Logger.getLogger(UpdateNetworkACLItemCmd.class.getName());
 
@@ -45,16 +42,10 @@
     // ////////////// API parameters /////////////////////
     // ///////////////////////////////////////////////////
 
-    @Parameter(name = ApiConstants.ID,
-               type = CommandType.UUID,
-               entityType = NetworkACLItemResponse.class,
-               required = true,
-               description = "the ID of the network ACL item")
+    @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = NetworkACLItemResponse.class, required = true, description = "the ID of the network ACL item")
     private Long id;
 
-    @Parameter(name = ApiConstants.PROTOCOL,
-               type = CommandType.STRING,
-               description = "the protocol for the ACL rule. Valid values are TCP/UDP/ICMP/ALL or valid protocol number")
+    @Parameter(name = ApiConstants.PROTOCOL, type = CommandType.STRING, description = "the protocol for the ACL rule. Valid values are TCP/UDP/ICMP/ALL or valid protocol number")
     private String protocol;
 
     @Parameter(name = ApiConstants.START_PORT, type = CommandType.INTEGER, description = "the starting port of ACL")
@@ -72,8 +63,7 @@
     @Parameter(name = ApiConstants.ICMP_CODE, type = CommandType.INTEGER, description = "error code for this ICMP message")
     private Integer icmpCode;
 
-    @Parameter(name = ApiConstants.TRAFFIC_TYPE, type = CommandType.STRING, description = "the traffic type for the ACL,"
-        + "can be Ingress or Egress, defaulted to Ingress if not specified")
+    @Parameter(name = ApiConstants.TRAFFIC_TYPE, type = CommandType.STRING, description = "the traffic type for the ACL, 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")
@@ -82,9 +72,13 @@
     @Parameter(name = ApiConstants.ACTION, type = CommandType.STRING, description = "scl entry action, allow or deny")
     private String action;
 
-    @Parameter(name = ApiConstants.FOR_DISPLAY, type = CommandType.BOOLEAN, description = "an optional field, whether to the display the rule to the end user or not", since = "4.4", authorized = {RoleType.Admin})
+    @Parameter(name = ApiConstants.FOR_DISPLAY, type = CommandType.BOOLEAN, description = "an optional field, whether to the display the rule to the end user or not", since = "4.4", authorized = {
+            RoleType.Admin})
     private Boolean display;
 
+    @Parameter(name = ApiConstants.ACL_REASON, type = CommandType.STRING, description = "A description indicating why the ACL rule is required.")
+    private String reason;
+
     // ///////////////////////////////////////////////////
     // ///////////////// Accessors ///////////////////////
     // ///////////////////////////////////////////////////
@@ -105,8 +99,9 @@ public Long getId() {
     public String getProtocol() {
         if (protocol != null) {
             return protocol.trim();
-        } else
+        } else {
             return null;
+        }
     }
 
     public List<String> getSourceCidrList() {
@@ -173,15 +168,14 @@ public Integer getIcmpType() {
         return icmpType;
     }
 
+    public String getReason() {
+        return reason;
+    }
+
     @Override
     public void execute() throws ResourceUnavailableException {
         CallContext.current().setEventDetails("Rule Id: " + getId());
-        NetworkACLItem aclItem =
-            _networkACLService.updateNetworkACLItem(getId(), getProtocol(), getSourceCidrList(), getTrafficType(), getAction(), getNumber(), getSourcePortStart(),
-                getSourcePortEnd(), getIcmpCode(), getIcmpType(), this.getCustomId(), this.isDisplay());
-        if (aclItem == null) {
-            throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to update network ACL item");
-        }
+        NetworkACLItem aclItem = _networkACLService.updateNetworkACLItem(this);
         NetworkACLItemResponse aclResponse = _responseGenerator.createNetworkACLItemResponse(aclItem);
         setResponseObject(aclResponse);
         aclResponse.setResponseName(getCommandName());
diff --git a/api/src/main/java/org/apache/cloudstack/api/response/NetworkACLItemResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/NetworkACLItemResponse.java
index 01012957978..97f5042bc9f 100644
--- a/api/src/main/java/org/apache/cloudstack/api/response/NetworkACLItemResponse.java
+++ b/api/src/main/java/org/apache/cloudstack/api/response/NetworkACLItemResponse.java
@@ -85,6 +85,10 @@
     @Param(description = "is rule for display to the regular user", since = "4.4", authorized = {RoleType.Admin})
     private Boolean forDisplay;
 
+    @SerializedName(ApiConstants.ACL_REASON)
+    @Param(description = "an explanation on why this ACL rule is being applied", since = "4.12")
+    private String reason;
+
     public void setId(String id) {
         this.id = id;
     }
@@ -140,4 +144,12 @@ public void setAction(String action) {
     public void setForDisplay(Boolean forDisplay) {
         this.forDisplay = forDisplay;
     }
+
+    public void setReason(String reason) {
+        this.reason = reason;
+    }
+
+    public String getReason() {
+        return reason;
+    }
 }
diff --git a/engine/components-api/src/main/java/com/cloud/network/vpc/NetworkACLManager.java b/engine/components-api/src/main/java/com/cloud/network/vpc/NetworkACLManager.java
index 0377c5f74d8..4200ea8c601 100644
--- a/engine/components-api/src/main/java/com/cloud/network/vpc/NetworkACLManager.java
+++ b/engine/components-api/src/main/java/com/cloud/network/vpc/NetworkACLManager.java
@@ -25,128 +25,67 @@
 
     /**
      * Creates Network ACL for the specified VPC
-     * @param name
-     * @param description
-     * @param vpcId
-     * @param forDisplay TODO
-     * @return
      */
     NetworkACL createNetworkACL(String name, String description, long vpcId, Boolean forDisplay);
 
     /**
      * Fetches Network ACL with specified Id
-     * @param id
-     * @return
      */
     NetworkACL getNetworkACL(long id);
 
     /**
      * Applies the items in the ACL to all associated networks
-     * @param aclId
-     * @return
-     * @throws ResourceUnavailableException
      */
     boolean applyNetworkACL(long aclId) throws ResourceUnavailableException;
 
     /**
      * Deletes the specified Network ACL
-     * @param id
-     * @return
      */
     boolean deleteNetworkACL(NetworkACL acl);
 
     /**
-     * Associates acl with a network and applies the ACLItems
-     * @param acl
-     * @param network
-     * @return
+     * Associates ACL with a network and applies the ACLItems
      */
     boolean replaceNetworkACL(NetworkACL acl, NetworkVO network) throws ResourceUnavailableException;
 
     /**
      * Creates a Network ACL Item within an ACL and applies it to associated networks
-     * @param sourcePortStart
-     * @param sourcePortEnd
-     * @param protocol
-     * @param sourceCidrList
-     * @param icmpCode
-     * @param icmpType
-     * @param trafficType
-     * @param aclId
-     * @param action
-     * @param number
-     * @param forDisplay TODO
-     * @return
      */
-    NetworkACLItem createNetworkACLItem(Integer sourcePortStart, Integer sourcePortEnd, String protocol, List<String> sourceCidrList, Integer icmpCode, Integer icmpType,
-        NetworkACLItem.TrafficType trafficType, Long aclId, String action, Integer number, Boolean forDisplay);
+    NetworkACLItem createNetworkACLItem(NetworkACLItemVO networkACLItemVO);
 
     /**
      * Returns Network ACL Item with specified Id
-     * @param ruleId
-     * @return
      */
     NetworkACLItem getNetworkACLItem(long ruleId);
 
     /**
      * Revoke ACL Item and apply changes
-     * @param ruleId
-     * @return
      */
     boolean revokeNetworkACLItem(long ruleId);
 
     /**
      * Revoke ACL Items for network and remove them in back-end. Db is not updated
-     * @param networkId
-     * @param userId
-     * @param caller
-     * @return
-     * @throws ResourceUnavailableException
      */
     boolean revokeACLItemsForNetwork(long networkId) throws ResourceUnavailableException;
 
     /**
      * List network ACL items by network
-     * @param guestNtwkId
-     * @return
      */
     List<NetworkACLItemVO> listNetworkACLItems(long guestNtwkId);
 
     /**
-     * Applies asscociated ACL to specified network
-     * @param networkId
-     * @return
-     * @throws ResourceUnavailableException
+     * Applies associated ACL to specified network
      */
     boolean applyACLToNetwork(long networkId) throws ResourceUnavailableException;
 
     /**
      * Updates and existing network ACL Item
-     * @param id
-     * @param protocol
-     * @param sourceCidrList
-     * @param trafficType
-     * @param action
-     * @param number
-     * @param sourcePortStart
-     * @param sourcePortEnd
-     * @param icmpCode
-     * @param icmpType
-     * @param customId TODO
-     * @param forDisplay TODO
-     * @return
-     * @throws ResourceUnavailableException
      */
-    NetworkACLItem updateNetworkACLItem(Long id, String protocol, List<String> sourceCidrList, NetworkACLItem.TrafficType trafficType, String action, Integer number,
-        Integer sourcePortStart, Integer sourcePortEnd, Integer icmpCode, Integer icmpType, String customId, Boolean forDisplay) throws ResourceUnavailableException;
+    NetworkACLItem updateNetworkACLItem(NetworkACLItemVO networkACLItemVO) throws ResourceUnavailableException;
 
     /**
-     * Associates acl with a network and applies the ACLItems
-     * @param acl
-     * @param gateway
-     * @return
+     * Associates ACL with a network and applies the ACLItems
      */
-
     boolean replaceNetworkACLForPrivateGw(NetworkACL acl, PrivateGateway gateway) throws ResourceUnavailableException;
 
     boolean revokeACLItemsForPrivateGw(PrivateGateway gateway) throws ResourceUnavailableException;
diff --git a/engine/schema/src/main/java/com/cloud/network/vpc/NetworkACLItemVO.java b/engine/schema/src/main/java/com/cloud/network/vpc/NetworkACLItemVO.java
index 6eb9cb0e991..f28b3125a09 100644
--- a/engine/schema/src/main/java/com/cloud/network/vpc/NetworkACLItemVO.java
+++ b/engine/schema/src/main/java/com/cloud/network/vpc/NetworkACLItemVO.java
@@ -33,15 +33,13 @@
 import javax.persistence.Transient;
 
 import com.cloud.utils.db.GenericDao;
+import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.net.NetUtils;
 
 @Entity
 @Table(name = "network_acl_item")
-public class NetworkACLItemVO implements NetworkACLItem {
+public class NetworkACLItemVO implements NetworkACLItem, Cloneable {
 
-    /**
-     *
-     */
     private static final long serialVersionUID = 2790623532888742060L;
 
     @Id
@@ -97,12 +95,15 @@
     @Column(name = "display", updatable = true, nullable = false)
     protected boolean display = true;
 
+    @Column(name = "reason", length = 2500)
+    private String reason;
+
     public NetworkACLItemVO() {
         uuid = UUID.randomUUID().toString();
     }
 
-    public NetworkACLItemVO(Integer portStart, Integer portEnd, String protocol, long aclId, List<String> sourceCidrs, Integer icmpCode, Integer icmpType,
-            TrafficType trafficType, Action action, int number) {
+    public NetworkACLItemVO(Integer portStart, Integer portEnd, String protocol, long aclId, List<String> sourceCidrs, Integer icmpCode, Integer icmpType, TrafficType trafficType, Action action,
+            int number, String reason) {
         sourcePortStart = portStart;
         sourcePortEnd = portEnd;
         this.protocol = protocol;
@@ -115,6 +116,7 @@ public NetworkACLItemVO(Integer portStart, Integer portEnd, String protocol, lon
         this.trafficType = trafficType;
         this.action = action;
         this.number = number;
+        this.reason = reason;
     }
 
     public void setSourceCidrList(List<String> sourceCidrs) {
@@ -225,8 +227,8 @@ public void setTrafficType(TrafficType trafficType) {
 
     public void setSourceCidrs(String sourceCidrs) {
         List<String> srcCidrs = new LinkedList<String>();
-        StringTokenizer st = new StringTokenizer(sourceCidrs,",;");
-        while(st.hasMoreTokens()) {
+        StringTokenizer st = new StringTokenizer(sourceCidrs, ",;");
+        while (st.hasMoreTokens()) {
             srcCidrs.add(st.nextToken());
         }
         this.sourceCidrs = srcCidrs;
@@ -252,4 +254,22 @@ public void setDisplay(boolean display) {
     public boolean isDisplay() {
         return display;
     }
+
+    @Override
+    public String getReason() {
+        return reason;
+    }
+
+    public void setReason(String reason) {
+        this.reason = reason;
+    }
+
+    @Override
+    protected NetworkACLItemVO clone() {
+        try {
+            return (NetworkACLItemVO)super.clone();
+        } catch (CloneNotSupportedException e) {
+            throw new CloudRuntimeException(e);
+        }
+    }
 }
diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41100to41200.sql b/engine/schema/src/main/resources/META-INF/db/schema-41100to41200.sql
index b6fd45f94a4..5667c5b3dab 100644
--- a/engine/schema/src/main/resources/META-INF/db/schema-41100to41200.sql
+++ b/engine/schema/src/main/resources/META-INF/db/schema-41100to41200.sql
@@ -17,4 +17,7 @@
 
 --;
 -- Schema upgrade from 4.11.0.0 to 4.12.0.0
---;
\ No newline at end of file
+--;
+
+-- [CLOUDSTACK-10314] Add reason column to ACL rule table
+ALTER TABLE `cloud`.`network_acl_item` ADD COLUMN `reason` VARCHAR(2500) AFTER `display`;
\ No newline at end of file
diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java
index 88da6342b81..bd9b543c58f 100644
--- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java
+++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java
@@ -2363,7 +2363,7 @@ public NetworkACLItemResponse createNetworkACLItemResponse(NetworkACLItem aclIte
             CollectionUtils.addIgnoreNull(tagResponses, tagResponse);
         }
         response.setTags(tagResponses);
-
+        response.setReason(aclItem.getReason());
         response.setObjectName("networkacl");
         return response;
     }
diff --git a/server/src/main/java/com/cloud/network/vpc/NetworkACLManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/NetworkACLManagerImpl.java
index c64a36b7c9f..d5f31d62753 100644
--- a/server/src/main/java/com/cloud/network/vpc/NetworkACLManagerImpl.java
+++ b/server/src/main/java/com/cloud/network/vpc/NetworkACLManagerImpl.java
@@ -21,7 +21,11 @@
 
 import javax.inject.Inject;
 
-import com.cloud.configuration.ConfigurationManager;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.framework.messagebus.MessageBus;
+import org.apache.cloudstack.framework.messagebus.PublishScope;
+import org.apache.log4j.Logger;
+
 import com.cloud.event.ActionEvent;
 import com.cloud.event.EventTypes;
 import com.cloud.exception.InvalidParameterValueException;
@@ -37,8 +41,6 @@
 import com.cloud.network.vpc.dao.NetworkACLDao;
 import com.cloud.network.vpc.dao.VpcGatewayDao;
 import com.cloud.offering.NetworkOffering;
-import com.cloud.tags.dao.ResourceTagDao;
-import com.cloud.user.AccountManager;
 import com.cloud.utils.component.ManagerBase;
 import com.cloud.utils.db.DB;
 import com.cloud.utils.db.EntityManager;
@@ -47,43 +49,31 @@
 import com.cloud.utils.db.TransactionStatus;
 import com.cloud.utils.exception.CloudRuntimeException;
 
-import org.apache.cloudstack.context.CallContext;
-import org.apache.cloudstack.framework.messagebus.MessageBus;
-import org.apache.cloudstack.framework.messagebus.PublishScope;
-import org.apache.log4j.Logger;
-
 public class NetworkACLManagerImpl extends ManagerBase implements NetworkACLManager {
     private static final Logger s_logger = Logger.getLogger(NetworkACLManagerImpl.class);
 
     @Inject
-    AccountManager _accountMgr;
-    @Inject
-    NetworkModel _networkMgr;
-    @Inject
-    VpcManager _vpcMgr;
-    @Inject
-    ResourceTagDao _resourceTagDao;
+    private NetworkModel _networkMgr;
     @Inject
-    NetworkACLDao _networkACLDao;
+    private NetworkACLDao _networkACLDao;
     @Inject
-    NetworkACLItemDao _networkACLItemDao;
-    List<NetworkACLServiceProvider> _networkAclElements;
+    private NetworkACLItemDao _networkACLItemDao;
     @Inject
-    NetworkModel _networkModel;
+    private NetworkModel _networkModel;
     @Inject
-    NetworkDao _networkDao;
+    private NetworkDao _networkDao;
     @Inject
-    VpcGatewayDao _vpcGatewayDao;
+    private VpcGatewayDao _vpcGatewayDao;
     @Inject
-    NetworkModel _ntwkModel;
+    private NetworkModel _ntwkModel;
     @Inject
-    ConfigurationManager _configMgr;
+    private EntityManager _entityMgr;
     @Inject
-    EntityManager _entityMgr;
+    private VpcService _vpcSvc;
     @Inject
-    VpcService _vpcSvc;
-    @Inject
-    MessageBus _messageBus;
+    private MessageBus _messageBus;
+
+    private List<NetworkACLServiceProvider> _networkAclElements;
 
     @Override
     public NetworkACL createNetworkACL(final String name, final String description, final long vpcId, final Boolean forDisplay) {
@@ -224,40 +214,21 @@ public boolean replaceNetworkACL(final NetworkACL acl, final NetworkVO network)
         return false;
     }
 
-    @Override
     @DB
+    @Override
     @ActionEvent(eventType = EventTypes.EVENT_NETWORK_ACL_ITEM_CREATE, eventDescription = "creating network ACL Item", create = true)
-    public NetworkACLItem createNetworkACLItem(final Integer portStart, final Integer portEnd, final String protocol, final List<String> sourceCidrList, final Integer icmpCode,
-            final Integer icmpType, final NetworkACLItem.TrafficType trafficType, final Long aclId, final String action, Integer number, final Boolean forDisplay) {
-        // If number is null, set it to currentMax + 1 (for backward compatibility)
-        if (number == null) {
-            number = _networkACLItemDao.getMaxNumberByACL(aclId) + 1;
-        }
-
-        final Integer numberFinal = number;
-        final NetworkACLItemVO newRule = Transaction.execute(new TransactionCallback<NetworkACLItemVO>() {
+    public NetworkACLItem createNetworkACLItem(NetworkACLItemVO networkACLItemVO) {
+        NetworkACLItemVO newRule = Transaction.execute(new TransactionCallback<NetworkACLItemVO>() {
             @Override
             public NetworkACLItemVO doInTransaction(final TransactionStatus status) {
-                NetworkACLItem.Action ruleAction = NetworkACLItem.Action.Allow;
-                if ("deny".equalsIgnoreCase(action)) {
-                    ruleAction = NetworkACLItem.Action.Deny;
-                }
+                NetworkACLItemVO networkACLItemVOFromDatabase = _networkACLItemDao.persist(networkACLItemVO);
 
-                NetworkACLItemVO newRule =
-                        new NetworkACLItemVO(portStart, portEnd, protocol.toLowerCase(), aclId, sourceCidrList, icmpCode, icmpType, trafficType, ruleAction, numberFinal);
-
-                if (forDisplay != null) {
-                    newRule.setDisplay(forDisplay);
-                }
-
-                newRule = _networkACLItemDao.persist(newRule);
-
-                if (!_networkACLItemDao.setStateToAdd(newRule)) {
-                    throw new CloudRuntimeException("Unable to update the state to add for " + newRule);
+                if (!_networkACLItemDao.setStateToAdd(networkACLItemVOFromDatabase)) {
+                    throw new CloudRuntimeException("Unable to update the state to add for " + networkACLItemVOFromDatabase);
                 }
-                CallContext.current().setEventDetails("ACL Item Id: " + newRule.getId());
+                CallContext.current().setEventDetails("ACL Item Id: " + networkACLItemVOFromDatabase.getId());
 
-                return newRule;
+                return networkACLItemVOFromDatabase;
             }
         });
 
@@ -265,7 +236,7 @@ public NetworkACLItemVO doInTransaction(final TransactionStatus status) {
     }
 
     @Override
-    public NetworkACLItem getNetworkACLItem(final long ruleId) {
+    public NetworkACLItem getNetworkACLItem(long ruleId) {
         return _networkACLItemDao.findById(ruleId);
     }
 
@@ -372,7 +343,6 @@ public boolean revokeACLItemsForPrivateGw(final PrivateGateway gateway) throws R
     }
 
     private void removeRule(final NetworkACLItem rule) {
-        //remove the rule
         _networkACLItemDao.remove(rule.getId());
     }
 
@@ -384,19 +354,14 @@ public boolean applyACLToPrivateGw(final PrivateGateway gateway) throws Resource
     }
 
     private boolean applyACLToPrivateGw(final PrivateGateway gateway, final List<? extends NetworkACLItem> rules) throws ResourceUnavailableException {
-        List<VpcProvider> vpcElements = null;
-        vpcElements = new ArrayList<VpcProvider>();
+        List<VpcProvider> vpcElements = new ArrayList<VpcProvider>();
         vpcElements.add((VpcProvider)_ntwkModel.getElementImplementingProvider(Network.Provider.VPCVirtualRouter.getName()));
 
-        if (vpcElements == null) {
-            throw new CloudRuntimeException("Failed to initialize vpc elements");
-        }
-
-        try{
+        try {
             for (final VpcProvider provider : vpcElements) {
                 return provider.applyACLItemsToPrivateGw(gateway, rules);
             }
-        } catch(final Exception ex) {
+        } catch (final Exception ex) {
             s_logger.debug("Failed to apply acl to private gateway " + gateway);
         }
         return false;
@@ -412,68 +377,24 @@ public boolean applyACLToNetwork(final long networkId) throws ResourceUnavailabl
         return applyACLItemsToNetwork(networkId, rules);
     }
 
+    /**
+     * Updates and applies the network ACL rule ({@link NetworkACLItemVO}).
+     * We will first try to update the ACL rule in the database using {@link NetworkACLItemDao#update(Long, NetworkACLItemVO)}. If it does not work, a {@link CloudRuntimeException} is thrown.
+     * If we manage to update the ACL rule in the database, we proceed to apply it using {@link #applyNetworkACL(long)}. If this does not work we throw a {@link CloudRuntimeException}.
+     * If all is working we return the {@link NetworkACLItemVO} given as parameter. We wil set the state of the rule to {@link com.cloud.network.vpc.NetworkACLItem.State#Add}.
+     */
     @Override
-    public NetworkACLItem updateNetworkACLItem(final Long id, final String protocol, final List<String> sourceCidrList, final NetworkACLItem.TrafficType trafficType, final String action,
-            final Integer number, final Integer sourcePortStart, final Integer sourcePortEnd, final Integer icmpCode, final Integer icmpType, final String customId, final Boolean forDisplay) throws ResourceUnavailableException {
-        final NetworkACLItemVO aclItem = _networkACLItemDao.findById(id);
-        aclItem.setState(State.Add);
-
-        if (protocol != null) {
-            aclItem.setProtocol(protocol);
-        }
-
-        if (sourceCidrList != null) {
-            aclItem.setSourceCidrList(sourceCidrList);
-        }
-
-        if (trafficType != null) {
-            aclItem.setTrafficType(trafficType);
-        }
-
-        if (action != null) {
-            NetworkACLItem.Action ruleAction = NetworkACLItem.Action.Allow;
-            if ("deny".equalsIgnoreCase(action)) {
-                ruleAction = NetworkACLItem.Action.Deny;
-            }
-            aclItem.setAction(ruleAction);
-        }
-
-        if (number != null) {
-            aclItem.setNumber(number);
-        }
-
-        if (sourcePortStart != null) {
-            aclItem.setSourcePortStart(sourcePortStart);
-        }
-
-        if (sourcePortEnd != null) {
-            aclItem.setSourcePortEnd(sourcePortEnd);
-        }
-
-        if (icmpCode != null) {
-            aclItem.setIcmpCode(icmpCode);
-        }
-
-        if (icmpType != null) {
-            aclItem.setIcmpType(icmpType);
-        }
-
-        if (customId != null) {
-            aclItem.setUuid(customId);
-        }
-
-        if (forDisplay != null) {
-            aclItem.setDisplay(forDisplay);
-        }
+    public NetworkACLItem updateNetworkACLItem(NetworkACLItemVO networkACLItemVO) throws ResourceUnavailableException {
+        networkACLItemVO.setState(State.Add);
 
-        if (_networkACLItemDao.update(id, aclItem)) {
-            if (applyNetworkACL(aclItem.getAclId())) {
-                return aclItem;
+        if (_networkACLItemDao.update(networkACLItemVO.getId(), networkACLItemVO)) {
+            if (applyNetworkACL(networkACLItemVO.getAclId())) {
+                return networkACLItemVO;
             } else {
-                throw new CloudRuntimeException("Failed to apply Network ACL Item: " + aclItem.getUuid());
+                throw new CloudRuntimeException("Failed to apply Network ACL rule: " + networkACLItemVO.getUuid());
             }
         }
-        return null;
+        throw new CloudRuntimeException(String.format("Network ACL rule [id=%s] acl rule list [id=%s] could not be updated.", networkACLItemVO.getUuid(), networkACLItemVO.getAclId()));
     }
 
     public boolean applyACLItemsToNetwork(final long networkId, final List<NetworkACLItemVO> rules) throws ResourceUnavailableException {
diff --git a/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java b/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java
index 1743f5c322e..bfa842807f9 100644
--- a/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java
+++ b/server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java
@@ -27,7 +27,9 @@
 import org.apache.cloudstack.api.command.user.network.CreateNetworkACLCmd;
 import org.apache.cloudstack.api.command.user.network.ListNetworkACLListsCmd;
 import org.apache.cloudstack.api.command.user.network.ListNetworkACLsCmd;
+import org.apache.cloudstack.api.command.user.network.UpdateNetworkACLItemCmd;
 import org.apache.cloudstack.context.CallContext;
+import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
@@ -41,6 +43,8 @@
 import com.cloud.network.Networks;
 import com.cloud.network.dao.NetworkDao;
 import com.cloud.network.dao.NetworkVO;
+import com.cloud.network.vpc.NetworkACLItem.Action;
+import com.cloud.network.vpc.NetworkACLItem.TrafficType;
 import com.cloud.network.vpc.dao.NetworkACLDao;
 import com.cloud.network.vpc.dao.VpcDao;
 import com.cloud.network.vpc.dao.VpcGatewayDao;
@@ -67,31 +71,29 @@
     private static final Logger s_logger = Logger.getLogger(NetworkACLServiceImpl.class);
 
     @Inject
-    AccountManager _accountMgr;
+    private AccountManager _accountMgr;
     @Inject
-    NetworkModel _networkMgr;
+    private ResourceTagDao _resourceTagDao;
     @Inject
-    ResourceTagDao _resourceTagDao;
+    private NetworkACLDao _networkACLDao;
     @Inject
-    NetworkACLDao _networkACLDao;
+    private NetworkACLItemDao _networkACLItemDao;
     @Inject
-    NetworkACLItemDao _networkACLItemDao;
+    private NetworkModel networkModel;
     @Inject
-    NetworkModel _networkModel;
+    private NetworkDao _networkDao;
     @Inject
-    NetworkDao _networkDao;
+    private NetworkACLManager _networkAclMgr;
     @Inject
-    NetworkACLManager _networkAclMgr;
+    private VpcGatewayDao _vpcGatewayDao;
     @Inject
-    VpcGatewayDao _vpcGatewayDao;
+    private EntityManager _entityMgr;
     @Inject
-    VpcManager _vpcMgr;
+    private VpcDao _vpcDao;
     @Inject
-    EntityManager _entityMgr;
-    @Inject
-    VpcDao _vpcDao;
-    @Inject
-    VpcService _vpcSvc;
+    private VpcService _vpcSvc;
+
+    private String supportedProtocolsForAclRules = "tcp,udp,icmp,all";
 
     @Override
     public NetworkACL createNetworkACL(final String name, final String description, final long vpcId, final Boolean forDisplay) {
@@ -133,7 +135,7 @@ public NetworkACL getNetworkACL(final long id) {
             sb.join("networkJoin", network, sb.entity().getId(), network.entity().getNetworkACLId(), JoinBuilder.JoinType.INNER);
         }
 
-        final SearchCriteria<NetworkACLVO> sc = sb.create();
+        SearchCriteria<NetworkACLVO> sc = sb.create();
 
         if (keyword != null) {
             final SearchCriteria<NetworkACLVO> ssc = _networkACLDao.createSearchCriteria();
@@ -146,7 +148,7 @@ public NetworkACL getNetworkACL(final long id) {
             sc.setParameters("display", display);
         }
 
-        if(id != null){
+        if (id != null) {
             sc.setParameters("id", id);
         }
 
@@ -173,10 +175,8 @@ public NetworkACL getNetworkACL(final long id) {
             final String accountName = cmd.getAccountName();
             final Long projectId = cmd.getProjectId();
             final boolean listAll = cmd.listAll();
-            final Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<Long, Boolean,
-                    ListProjectResourcesCriteria>(domainId, isRecursive, null);
-            _accountMgr.buildACLSearchParameters(caller, id, accountName, projectId, permittedAccounts, domainIdRecursiveListProject,
-                    listAll, false);
+            final Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<Long, Boolean, ListProjectResourcesCriteria>(domainId, isRecursive, null);
+            _accountMgr.buildACLSearchParameters(caller, id, accountName, projectId, permittedAccounts, domainIdRecursiveListProject, listAll, false);
             domainId = domainIdRecursiveListProject.first();
             isRecursive = domainIdRecursiveListProject.second();
             final ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third();
@@ -199,7 +199,7 @@ public NetworkACL getNetworkACL(final long id) {
         }
 
         final Filter filter = new Filter(NetworkACLVO.class, "id", false, null, null);
-        final Pair<List<NetworkACLVO>, Integer> acls =  _networkACLDao.searchAndCount(sc, filter);
+        final Pair<List<NetworkACLVO>, Integer> acls = _networkACLDao.searchAndCount(sc, filter);
         return new Pair<List<? extends NetworkACL>, Integer>(acls.first(), acls.second());
     }
 
@@ -261,7 +261,7 @@ public boolean replaceNetworkACLonPrivateGw(final long aclId, final long private
         final PrivateGateway privateGateway = _vpcSvc.getVpcPrivateGateway(gateway.getId());
         _accountMgr.checkAccess(caller, null, true, privateGateway);
 
-        return  _networkAclMgr.replaceNetworkACLForPrivateGw(acl, privateGateway);
+        return _networkAclMgr.replaceNetworkACLForPrivateGw(acl, privateGateway);
 
     }
 
@@ -304,164 +304,342 @@ public boolean replaceNetworkACL(final long aclId, final long networkId) throws
         return _networkAclMgr.replaceNetworkACL(acl, network);
     }
 
+    /**
+     * Creates and persists a network ACL rule. The network ACL rule is persisted as a {@link NetworkACLItemVO}.
+     * If no ACL list ID is informed, we will create one. To check these details, please refer to {@link #createAclListIfNeeded(CreateNetworkACLCmd)}.
+     * All of the attributes will be validated accordingly using the following methods:
+     * <ul>
+     * <li> {@link #validateAclRuleNumber(CreateNetworkACLCmd, Long, NetworkACL)} to validate the provided ACL rule number;
+     * <li> {@link #validateNetworkAclList(Long, NetworkACL)} to check if the user has access to the informed ACL list ID and respective VPC;
+     * <li> {@link #validateAndCreateNetworkAclRuleAction(String)} to validate the ACL rule action;
+     * <li> {@link #validateNetworkACLItem(NetworkACLItemVO)} to validate general configurations relating to protocol, ports, and ICMP codes and types.
+     * </ul>
+     *
+     * Moreover, if not ACL rule number is provided we generate one based on the last ACL number used. We will increment +1 in the last ACL rule number used. After all of the validation the ACL rule is persisted using the method {@link NetworkACLManagerImpl#createNetworkACLItem(NetworkACLItemVO)}.
+     */
     @Override
-    public NetworkACLItem createNetworkACLItem(final CreateNetworkACLCmd aclItemCmd) {
-        final Account caller = CallContext.current().getCallingAccount();
-        Long aclId = aclItemCmd.getACLId();
-        if (aclId == null) {
-            //ACL id is not specified. Get the ACL details from network
-            if (aclItemCmd.getNetworkId() == null) {
-                throw new InvalidParameterValueException("Cannot create Network ACL Item. ACL Id or network Id is required");
-            }
-            final Network network = _networkMgr.getNetwork(aclItemCmd.getNetworkId());
-            if (network.getVpcId() == null) {
-                throw new InvalidParameterValueException("Network: " + network.getUuid() + " does not belong to VPC");
-            }
-            aclId = network.getNetworkACLId();
-
-            if (aclId == null) {
-                //Network is not associated with any ACL. Create a new ACL and add aclItem in it for backward compatibility
-                s_logger.debug("Network " + network.getId() + " is not associated with any ACL. Creating an ACL before adding acl item");
+    public NetworkACLItem createNetworkACLItem(CreateNetworkACLCmd createNetworkACLCmd) {
+        Long aclId = createAclListIfNeeded(createNetworkACLCmd);
+
+        Integer sourcePortStart = createNetworkACLCmd.getSourcePortStart();
+        Integer sourcePortEnd = createNetworkACLCmd.getSourcePortEnd();
+        String protocol = createNetworkACLCmd.getProtocol();
+        List<String> sourceCidrList = createNetworkACLCmd.getSourceCidrList();
+        Integer icmpCode = createNetworkACLCmd.getIcmpCode();
+        Integer icmpType = createNetworkACLCmd.getIcmpType();
+        TrafficType trafficType = createNetworkACLCmd.getTrafficType();
+        String reason = createNetworkACLCmd.getReason();
+        String action = createNetworkACLCmd.getAction();
+
+        NetworkACL acl = _networkAclMgr.getNetworkACL(aclId);
+
+        validateNetworkAcl(acl);
+        validateAclRuleNumber(createNetworkACLCmd, acl);
+
+        NetworkACLItem.Action ruleAction = validateAndCreateNetworkAclRuleAction(action);
+        Integer number = createNetworkACLCmd.getNumber();
+        if (number == null) {
+            number = _networkACLItemDao.getMaxNumberByACL(aclId) + 1;
+        }
+        NetworkACLItemVO networkACLItemVO = new NetworkACLItemVO(sourcePortStart, sourcePortEnd, protocol, aclId, sourceCidrList, icmpCode, icmpType, trafficType, ruleAction, number, reason);
+        networkACLItemVO.setDisplay(createNetworkACLCmd.isDisplay());
+
+        validateNetworkACLItem(networkACLItemVO);
+        return _networkAclMgr.createNetworkACLItem(networkACLItemVO);
+    }
 
-                //verify that ACLProvider is supported by network offering
-                if (!_networkModel.areServicesSupportedByNetworkOffering(network.getNetworkOfferingId(), Network.Service.NetworkACL)) {
-                    throw new InvalidParameterValueException("Network Offering does not support NetworkACL service");
-                }
+    /**
+     *  We first validate the given ACL action as a string using {@link #validateNetworkAclRuleAction(String)}.
+     *  Afterwards, we convert this ACL to an object of {@link NetworkACLItem.Action}.
+     *  If the action as String matches the word 'deny' (ignoring case), we return an instance of {@link NetworkACLItem.Action#Deny}.
+     *  Otherwise, we return {@link NetworkACLItem.Action#Allow}.
+     */
+    protected NetworkACLItem.Action validateAndCreateNetworkAclRuleAction(String action) {
+        validateNetworkAclRuleAction(action);
+        NetworkACLItem.Action ruleAction = NetworkACLItem.Action.Allow;
+        if ("deny".equalsIgnoreCase(action)) {
+            ruleAction = NetworkACLItem.Action.Deny;
+        }
+        return ruleAction;
+    }
 
-                final Vpc vpc = _entityMgr.findById(Vpc.class, network.getVpcId());
-                if (vpc == null) {
-                    throw new InvalidParameterValueException("Unable to find Vpc associated with the Network");
-                }
+    /**
+     * Validates the network ACL rule action given as a {@link String}.
+     * If the parameter is null, we do not perform any validations. Otherwise, we check if the parameter is equal to 'Allow' or 'Deny' (ignoring the case).
+     * If the parameter is an invalid action, we throw an {@link InvalidParameterValueException}.
+     */
+    protected void validateNetworkAclRuleAction(String action) {
+        if (action != null) {
+            if (!("Allow".equalsIgnoreCase(action) || "Deny".equalsIgnoreCase(action))) {
+                throw new InvalidParameterValueException(String.format("Invalid action [%s]. Permitted actions are Allow and Deny", action));
+            }
+        }
+    }
 
-                //Create new ACL
-                final String aclName = "VPC_" + vpc.getName() + "_Tier_" + network.getName() + "_ACL_" + network.getUuid();
-                final String description = "ACL for " + aclName;
-                final NetworkACL acl = _networkAclMgr.createNetworkACL(aclName, description, network.getVpcId(), aclItemCmd.getDisplay());
-                if (acl == null) {
-                    throw new CloudRuntimeException("Error while create ACL before adding ACL Item for network " + network.getId());
-                }
-                s_logger.debug("Created ACL: " + aclName + " for network " + network.getId());
-                aclId = acl.getId();
-                //Apply acl to network
-                try {
-                    if (!_networkAclMgr.replaceNetworkACL(acl, (NetworkVO)network)) {
-                        throw new CloudRuntimeException("Unable to apply auto created ACL to network " + network.getId());
-                    }
-                    s_logger.debug("Created ACL is applied to network " + network.getId());
-                } catch (final ResourceUnavailableException e) {
-                    throw new CloudRuntimeException("Unable to apply auto created ACL to network " + network.getId(), e);
-                }
+    /**
+     * Validates the ACL rule number field. If the field is null, then we do not have anything to check here.
+     * If the number is not null, we perform the following checks:
+     * <ul>
+     *  <li>If number is less than one, than we throw an {@link InvalidParameterValueException};
+     *  <li>if there is already an ACL configured with the given number for the network, we also throw an {@link InvalidParameterValueException}. The check is performed using {@link NetworkACLItemDao#findByAclAndNumber(long, int)} method.
+     * </ul>
+     *
+     * At the end, if not exception is thrown, the number of the ACL rule is valid.
+     */
+    protected void validateAclRuleNumber(CreateNetworkACLCmd createNetworkAclCmd, NetworkACL acl) {
+        Integer number = createNetworkAclCmd.getNumber();
+        if (number != null) {
+            if (number < 1) {
+                throw new InvalidParameterValueException(String.format("Invalid number [%d]. Number cannot be < 1", number));
+            }
+            if (_networkACLItemDao.findByAclAndNumber(acl.getId(), createNetworkAclCmd.getNumber()) != null) {
+                throw new InvalidParameterValueException("ACL item with number " + number + " already exists in ACL: " + acl.getUuid());
             }
         }
+    }
 
-        final NetworkACL acl = _networkAclMgr.getNetworkACL(aclId);
+    /**
+     * Validates a given {@link NetworkACL}. The validations are the following:
+     * <ul>
+     *  <li>If the parameter is null, we return an  {@link InvalidParameterValueException};
+     *  <li>Default ACLs {@link NetworkACL#DEFAULT_ALLOW} and {@link NetworkACL#DEFAULT_DENY} cannot be modified. Therefore, if any of them is provided we throw a {@link InvalidParameterValueException};
+     *  <li>If the network does not have a VPC, we will throw an {@link InvalidParameterValueException}.
+     * </ul>
+     *
+     * After all validations, we check if the user has access to the given network ACL using {@link AccountManager#checkAccess(Account, org.apache.cloudstack.acl.SecurityChecker.AccessType, boolean, org.apache.cloudstack.acl.ControlledEntity...)}.
+     */
+    protected void validateNetworkAcl(NetworkACL acl) {
         if (acl == null) {
-            throw new InvalidParameterValueException("Unable to find specified ACL");
+            throw new InvalidParameterValueException("Unable to find specified ACL.");
         }
 
-        if (aclId == NetworkACL.DEFAULT_DENY || aclId == NetworkACL.DEFAULT_ALLOW) {
+        if (acl.getId() == NetworkACL.DEFAULT_DENY || acl.getId() == NetworkACL.DEFAULT_ALLOW) {
             throw new InvalidParameterValueException("Default ACL cannot be modified");
         }
 
-        final Vpc vpc = _entityMgr.findById(Vpc.class, acl.getVpcId());
+        Vpc vpc = _entityMgr.findById(Vpc.class, acl.getVpcId());
         if (vpc == null) {
-            throw new InvalidParameterValueException("Unable to find Vpc associated with the NetworkACL");
+            throw new InvalidParameterValueException(String.format("Unable to find Vpc associated with the NetworkACL [%s]", acl.getUuid()));
         }
+        Account caller = CallContext.current().getCallingAccount();
         _accountMgr.checkAccess(caller, null, true, vpc);
+    }
 
-        //Ensure that number is unique within the ACL
-        if (aclItemCmd.getNumber() != null) {
-            if (_networkACLItemDao.findByAclAndNumber(aclId, aclItemCmd.getNumber()) != null) {
-                throw new InvalidParameterValueException("ACL item with number " + aclItemCmd.getNumber() + " already exists in ACL: " + acl.getUuid());
-            }
+    /**
+     * This methods will simply return the ACL rule list ID if it has been provided by the parameter 'createNetworkACLCmd'.
+     * If no ACL rule List ID has been provided the method behave as follows:
+     * <ul>
+     *  <li> If it has not been provided either, we will throw an {@link InvalidParameterValueException};
+     *  <li> if the network ID has been provided, we will check if the network has a VPC; if it does not have, we will throw an {@link InvalidParameterValueException};
+     *  <ul>
+     *      <li> If the VPC already has an ACL rule list, we will return it;
+     *      <li> otherwise, we will create one using {@link #createAclListForNetworkAndReturnAclListId(CreateNetworkACLCmd, Network)} method. This behavior is a legacy thing that has been maintained so far.
+     *  </ul>
+     * </ul>
+     *
+     * @return The network ACL list ID
+     */
+    protected Long createAclListIfNeeded(CreateNetworkACLCmd createNetworkACLCmd) {
+        Long aclId = createNetworkACLCmd.getACLId();
+        if (aclId != null) {
+            return aclId;
+        }
+        if (createNetworkACLCmd.getNetworkId() == null) {
+            throw new InvalidParameterValueException("Cannot create Network ACL Item. ACL Id or network Id is required");
+        }
+        Network network = networkModel.getNetwork(createNetworkACLCmd.getNetworkId());
+        if (network.getVpcId() == null) {
+            throw new InvalidParameterValueException("Network: " + network.getUuid() + " does not belong to VPC");
+        }
+        aclId = network.getNetworkACLId();
+
+        if (aclId == null) {
+            aclId = createAclListForNetworkAndReturnAclListId(createNetworkACLCmd, network);
         }
+        return aclId;
+    }
 
-        validateNetworkACLItem(aclItemCmd.getSourcePortStart(), aclItemCmd.getSourcePortEnd(), aclItemCmd.getSourceCidrList(), aclItemCmd.getProtocol(),
-                aclItemCmd.getIcmpCode(), aclItemCmd.getIcmpType(), aclItemCmd.getAction(), aclItemCmd.getNumber());
+    /**
+     * This method will created a network ACL for the provided network. This method will behave as follows:
+     * <ul>
+     *  <li> If the network offering does not support ACLs ( {@link NetworkModel#areServicesSupportedByNetworkOffering(long, com.cloud.network.Network.Service...)} ), then it throws an {@link InvalidParameterValueException};
+     *  <li> If the network does not have any VPC, it throws an {@link InvalidParameterValueException};
+     *  <li> If everything is OK so far, we try to create the ACL using {@link NetworkACLManagerImpl#createNetworkACL(String, String, long, Boolean)} method.
+     *  <ul>
+     *      <li> If the ACL is not created we throw a {@link CloudRuntimeException};
+     *      <li> otherwise, the workflow continues.
+     *  </ul>
+     *  <li> With the ACL in our hands, we try to apply it. If it does not work we throw a {@link CloudRuntimeException}.
+     * </ul>
+     *
+     * @return the Id of the network ACL that is created.
+     */
+    protected Long createAclListForNetworkAndReturnAclListId(CreateNetworkACLCmd aclItemCmd, Network network) {
+        s_logger.debug("Network " + network.getId() + " is not associated with any ACL. Creating an ACL before adding acl item");
+
+        if (!networkModel.areServicesSupportedByNetworkOffering(network.getNetworkOfferingId(), Network.Service.NetworkACL)) {
+            throw new InvalidParameterValueException("Network Offering does not support NetworkACL service");
+        }
+
+        Vpc vpc = _entityMgr.findById(Vpc.class, network.getVpcId());
+        if (vpc == null) {
+            throw new InvalidParameterValueException("Unable to find Vpc associated with the Network");
+        }
 
-        return _networkAclMgr.createNetworkACLItem(aclItemCmd.getSourcePortStart(), aclItemCmd.getSourcePortEnd(), aclItemCmd.getProtocol(),
-                aclItemCmd.getSourceCidrList(), aclItemCmd.getIcmpCode(), aclItemCmd.getIcmpType(), aclItemCmd.getTrafficType(), aclId, aclItemCmd.getAction(),
-                aclItemCmd.getNumber(), aclItemCmd.getDisplay());
+        String aclName = "VPC_" + vpc.getName() + "_Tier_" + network.getName() + "_ACL_" + network.getUuid();
+        String description = "ACL for " + aclName;
+        NetworkACL acl = _networkAclMgr.createNetworkACL(aclName, description, network.getVpcId(), aclItemCmd.isDisplay());
+        if (acl == null) {
+            throw new CloudRuntimeException("Error while create ACL before adding ACL Item for network " + network.getId());
+        }
+        s_logger.debug("Created ACL: " + aclName + " for network " + network.getId());
+        Long aclId = acl.getId();
+        //Apply acl to network
+        try {
+            if (!_networkAclMgr.replaceNetworkACL(acl, (NetworkVO)network)) {
+                throw new CloudRuntimeException("Unable to apply auto created ACL to network " + network.getId());
+            }
+            s_logger.debug("Created ACL is applied to network " + network.getId());
+        } catch (ResourceUnavailableException e) {
+            throw new CloudRuntimeException("Unable to apply auto created ACL to network " + network.getId(), e);
+        }
+        return aclId;
     }
 
-    private void validateNetworkACLItem(final Integer portStart, final Integer portEnd, final List<String> sourceCidrList, final String protocol, final Integer icmpCode, final Integer icmpType,
-            final String action, final Integer number) {
+    /**
+     *  Performs all of the validations for the {@link NetworkACLItem}.
+     *  First we validate the sources start and end ports using {@link #validateSourceStartAndEndPorts(NetworkACLItemVO)};
+     *  then, we validate the source CIDR list using {@link #validateSourceCidrList(NetworkACLItemVO)};
+     *  afterwards, it is validated the protocol entered in the {@link NetworkACLItemVO} using {@link #validateProtocol(NetworkACLItemVO)}.
+     */
+    protected void validateNetworkACLItem(NetworkACLItemVO networkACLItemVO) {
+        validateSourceStartAndEndPorts(networkACLItemVO);
+        validateSourceCidrList(networkACLItemVO);
+        validateProtocol(networkACLItemVO);
+    }
 
-        if (portStart != null && !NetUtils.isValidPort(portStart)) {
-            throw new InvalidParameterValueException("publicPort is an invalid value: " + portStart);
+    /**
+     * Validated ICMP type and code of {@link NetworkACLItemVO}. The behavior of this method is the following:
+     * <ul>
+     *  <li>If no ICMP type is provided, we do not perform validations;
+     *  <li>If the ICMP type is not '-1', we validate it using {@link NetUtils#validateIcmpType(long)};
+     *  <li>If the ICMP code is null, we do not perform validations;
+     *  <li>If the ICMP code is not '-1', we validate it using {@link NetUtils#validateIcmpCode(long)};
+     * </ul>
+     * Failing to meet the above conditions, we throw an {@link InvalidParameterValueException}.
+     */
+    protected void validateIcmpTypeAndCode(NetworkACLItemVO networkACLItemVO) {
+        Integer icmpType = networkACLItemVO.getIcmpType();
+        Integer icmpCode = networkACLItemVO.getIcmpCode();
+        if (icmpType == null) {
+            return;
+        }
+        if (icmpType.longValue() != -1 && !NetUtils.validateIcmpType(icmpType.longValue())) {
+            throw new InvalidParameterValueException(String.format("Invalid icmp type [%d]. It should belong to [0-255] range", icmpType));
+        }
+        if (icmpCode != null) {
+            if (icmpCode.longValue() != -1 && !NetUtils.validateIcmpCode(icmpCode.longValue())) {
+                throw new InvalidParameterValueException(String.format("Invalid icmp code [%d]. It should belong to [0-15] range and can be defined when icmpType belongs to [0-40] range", icmpCode));
+            }
         }
-        if (portEnd != null && !NetUtils.isValidPort(portEnd)) {
-            throw new InvalidParameterValueException("Public port range is an invalid value: " + portEnd);
+    }
+
+    /**
+     *   Validates the {@link NetworkACLItemVO} protocol. If the protocol is blank, we do not execute any validations. Otherwise, we perform the following checks:
+     *   <ul>
+     *      <li>If it is a numeric value, the protocol must be bigger or equal to 0 and smaller or equal to 255;
+     *      <li>if it is a {@link String}, it must be one of the following: {@link #supportedProtocolsForAclRules};
+     *   </ul>
+     *    Whenever the conditions enumerated above are not met, we throw an {@link InvalidParameterValueException}.
+     *
+     *    If the parameter passes the protocol type validations, we check the following:
+     *    <ul>
+     *      <li>If it is not an ICMP type protocol, it cannot have any value in {@link NetworkACLItemVO#getIcmpCode()} and {@link NetworkACLItemVO#getIcmpType()};
+     *      <li>If it is an ICMP type protocol, it cannot have any value in {@link NetworkACLItemVO#getSourcePortStart()} and {@link NetworkACLItemVO#getSourcePortEnd()}.
+     *    </ul>
+     *    Failing to meet the above conditions, we throw an {@link InvalidParameterValueException}.
+     *
+     *    The last check is performed via {@link #validateIcmpTypeAndCode(NetworkACLItemVO)} method.
+     */
+    protected void validateProtocol(NetworkACLItemVO networkACLItemVO) {
+        String protocol = networkACLItemVO.getProtocol();
+        if (StringUtils.isBlank(protocol)) {
+            return;
+        }
+        if (StringUtils.isNumeric(protocol)) {
+            int protoNumber = Integer.parseInt(protocol);
+            if (protoNumber < 0 || protoNumber > 255) {
+                throw new InvalidParameterValueException("Invalid protocol number: " + protoNumber);
+            }
+        } else {
+            if (!supportedProtocolsForAclRules.contains(protocol.toLowerCase())) {
+                throw new InvalidParameterValueException(String.format("Invalid protocol [%s]. Expected one of: [%s]", protocol, supportedProtocolsForAclRules));
+            }
         }
 
-        // start port can't be bigger than end port
-        if (portStart != null && portEnd != null && portStart > portEnd) {
-            throw new InvalidParameterValueException("Start port can't be bigger than end port");
+        Integer icmpCode = networkACLItemVO.getIcmpCode();
+        Integer icmpType = networkACLItemVO.getIcmpType();
+        // icmp code and icmp type can't be passed in for any other protocol rather than icmp
+        boolean isIcmpProtocol = protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO);
+        if (!isIcmpProtocol && (icmpCode != null || icmpType != null)) {
+            throw new InvalidParameterValueException("Can specify icmpCode and icmpType for ICMP protocol only");
         }
 
-        // start port and end port must be null for protocol = 'all'
-        if ((portStart != null || portEnd != null) && protocol != null && protocol.equalsIgnoreCase("all")) {
-            throw new InvalidParameterValueException("start port and end port must be null if protocol = 'all'");
+        Integer sourcePortStart = networkACLItemVO.getSourcePortStart();
+        Integer sourcePortEnd = networkACLItemVO.getSourcePortEnd();
+        if (isIcmpProtocol && (sourcePortStart != null || sourcePortEnd != null)) {
+            throw new InvalidParameterValueException("Can't specify start/end port when protocol is ICMP");
         }
 
-        if (sourceCidrList != null) {
-            for (final String cidr : sourceCidrList) {
+        validateIcmpTypeAndCode(networkACLItemVO);
+    }
+
+    /**
+     *  Validates all of the CIDRs in the {@link NetworkACLItemVO#getSourceCidrList()}.
+     *  If the list is empty we do not execute any validation. Otherwise, all of the CIDRs are validated using {@link NetUtils#isValidIp4Cidr(String)}.
+     */
+    protected void validateSourceCidrList(NetworkACLItemVO networkACLItemVO) {
+        List<String> sourceCidrList = networkACLItemVO.getSourceCidrList();
+        if (CollectionUtils.isNotEmpty(sourceCidrList)) {
+            for (String cidr : sourceCidrList) {
                 if (!NetUtils.isValidIp4Cidr(cidr)) {
                     throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Source cidrs formatting error " + cidr);
                 }
             }
         }
+    }
 
-        //Validate Protocol
-        if (protocol != null) {
-            //Check if protocol is a number
-            if (StringUtils.isNumeric(protocol)) {
-                final int protoNumber = Integer.parseInt(protocol);
-                if (protoNumber < 0 || protoNumber > 255) {
-                    throw new InvalidParameterValueException("Invalid protocol number: " + protoNumber);
-                }
-            } else {
-                //Protocol is not number
-                //Check for valid protocol strings
-                final String supportedProtocols = "tcp,udp,icmp,all";
-                if (!supportedProtocols.contains(protocol.toLowerCase())) {
-                    throw new InvalidParameterValueException("Invalid protocol: " + protocol);
-                }
-            }
-
-            // icmp code and icmp type can't be passed in for any other protocol rather than icmp
-            if (!protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO) && (icmpCode != null || icmpType != null)) {
-                throw new InvalidParameterValueException("Can specify icmpCode and icmpType for ICMP protocol only");
-            }
-
-            if (protocol.equalsIgnoreCase(NetUtils.ICMP_PROTO) && (portStart != null || portEnd != null)) {
-                throw new InvalidParameterValueException("Can't specify start/end port when protocol is ICMP");
-            }
+    /**
+     * Validates the source start and end ports for the given network ACL rule.
+     * If both ports (start and end) are null, we do not execute validations. Otherwise, we check the following:
+     * <ul>
+     *  <li> Check if start port is valid using {@link NetUtils#isValidPort(int)};
+     *  <li> Check if end port is valid using {@link NetUtils#isValidPort(int)};
+     *  <li> Check if start port is bigger than end port;
+     *  <li> Check if start and end ports were used with protocol 'all'
+     *  </ul>
+     *  All of the above cases will generate an {@link InvalidParameterValueException}.
+     */
+    protected void validateSourceStartAndEndPorts(NetworkACLItemVO networkACLItemVO) {
+        Integer sourcePortStart = networkACLItemVO.getSourcePortStart();
+        Integer sourcePortEnd = networkACLItemVO.getSourcePortEnd();
+        if (sourcePortStart == null && sourcePortEnd == null) {
+            return;
         }
 
-        //validate icmp code and type
-        if (icmpType != null) {
-            if (icmpType.longValue() != -1 && !NetUtils.validateIcmpType(icmpType.longValue())) {
-                throw new InvalidParameterValueException("Invalid icmp type; should belong to [0-255] range");
-            }
-            if (icmpCode != null) {
-                if (icmpCode.longValue() != -1 && !NetUtils.validateIcmpCode(icmpCode.longValue())) {
-                    throw new InvalidParameterValueException("Invalid icmp code; should belong to [0-15] range and can"
-                            + " be defined when icmpType belongs to [0-40] range");
-                }
-            }
+        if (!NetUtils.isValidPort(sourcePortStart)) {
+            throw new InvalidParameterValueException("Start public port is an invalid value: " + sourcePortStart);
         }
 
-        //Check ofr valid action Allow/Deny
-        if (action != null) {
-            if (!("Allow".equalsIgnoreCase(action) || "Deny".equalsIgnoreCase(action))) {
-                throw new InvalidParameterValueException("Invalid action. Allowed actions are Allow and Deny");
-            }
+        if (!NetUtils.isValidPort(sourcePortEnd)) {
+            throw new InvalidParameterValueException("End public port is an invalid value: " + sourcePortEnd);
         }
-
-        //Check for valid number
-        if (number != null && number < 1) {
-            throw new InvalidParameterValueException("Invalid number. Number cannot be < 1");
+        if (sourcePortStart > sourcePortEnd) {
+            throw new InvalidParameterValueException(String.format("Start port can't be bigger than end port [startport=%d,endport=%d]", sourcePortStart, sourcePortEnd));
+        }
+        String protocol = networkACLItemVO.getProtocol();
+        if ("all".equalsIgnoreCase(protocol)) {
+            throw new InvalidParameterValueException("start port and end port must be null if protocol = 'all'");
         }
     }
 
@@ -524,7 +702,7 @@ public boolean applyNetworkACL(final long aclId) throws ResourceUnavailableExcep
         if (networkId != null) {
             final Network network = _networkDao.findById(networkId);
             aclId = network.getNetworkACLId();
-            if( aclId == null){
+            if (aclId == null) {
                 // No aclId associated with the network.
                 //Return empty list
                 return new Pair(new ArrayList<NetworkACLItem>(), 0);
@@ -549,7 +727,6 @@ public boolean applyNetworkACL(final long aclId) throws ResourceUnavailableExcep
         } else {
             //ToDo: Add accountId to network_acl_item table for permission check
 
-
             // aclId is not specified
             // List permitted VPCs and filter aclItems
             final List<Long> permittedAccounts = new ArrayList<Long>();
@@ -558,10 +735,8 @@ public boolean applyNetworkACL(final long aclId) throws ResourceUnavailableExcep
             final String accountName = cmd.getAccountName();
             final Long projectId = cmd.getProjectId();
             final boolean listAll = cmd.listAll();
-            final Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<Long, Boolean,
-                    ListProjectResourcesCriteria>(domainId, isRecursive, null);
-            _accountMgr.buildACLSearchParameters(caller, id, accountName, projectId, permittedAccounts, domainIdRecursiveListProject,
-                    listAll, false);
+            final Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<Long, Boolean, ListProjectResourcesCriteria>(domainId, isRecursive, null);
+            _accountMgr.buildACLSearchParameters(caller, id, accountName, projectId, permittedAccounts, domainIdRecursiveListProject, listAll, false);
             domainId = domainIdRecursiveListProject.first();
             isRecursive = domainIdRecursiveListProject.second();
             final ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third();
@@ -599,7 +774,7 @@ public boolean applyNetworkACL(final long aclId) throws ResourceUnavailableExcep
 
         final Pair<List<NetworkACLItemVO>, Integer> result = _networkACLItemDao.searchAndCount(sc, filter);
         final List<NetworkACLItemVO> aclItemVOs = result.first();
-        for (final NetworkACLItemVO item: aclItemVOs) {
+        for (final NetworkACLItemVO item : aclItemVOs) {
             _networkACLItemDao.loadCidrs(item);
         }
         return new Pair<List<? extends NetworkACLItem>, Integer>(aclItemVOs, result.second());
@@ -609,12 +784,12 @@ public boolean applyNetworkACL(final long aclId) throws ResourceUnavailableExcep
     @ActionEvent(eventType = EventTypes.EVENT_NETWORK_ACL_ITEM_DELETE, eventDescription = "Deleting Network ACL Item", async = true)
     public boolean revokeNetworkACLItem(final long ruleId) {
         final NetworkACLItemVO aclItem = _networkACLItemDao.findById(ruleId);
-        if(aclItem != null){
+        if (aclItem != null) {
             final NetworkACL acl = _networkAclMgr.getNetworkACL(aclItem.getAclId());
 
             final Vpc vpc = _entityMgr.findById(Vpc.class, acl.getVpcId());
 
-            if(aclItem.getAclId() == NetworkACL.DEFAULT_ALLOW || aclItem.getAclId() == NetworkACL.DEFAULT_DENY){
+            if (aclItem.getAclId() == NetworkACL.DEFAULT_ALLOW || aclItem.getAclId() == NetworkACL.DEFAULT_DENY) {
                 throw new InvalidParameterValueException("ACL Items in default ACL cannot be deleted");
             }
 
@@ -626,38 +801,105 @@ public boolean revokeNetworkACLItem(final long ruleId) {
         return _networkAclMgr.revokeNetworkACLItem(ruleId);
     }
 
+    /**
+     * Updates a network ACL with the given values found in the {@link UpdateNetworkACLItemCmd} parameter.
+     * First we will validate the network ACL rule provided in the command using {@link #validateNetworkAclRuleIdAndRetrieveIt(UpdateNetworkACLItemCmd)}.
+     * Then, we validate the ACL itself using {@link #validateNetworkAcl(NetworkACL)}. If all of the validation is ok, we do the following.
+     * <ul>
+     *  <li>Transfer new data to {@link NetworkACLItemVO} that is intended to be updated;
+     *  <li>Validate the ACL rule being updated using {@link #validateNetworkACLItem(NetworkACLItemVO)}.
+     * </ul>
+     *
+     * After the validations and updating the POJO we execute the update in the database using {@link NetworkACLManagerImpl#updateNetworkACLItem(NetworkACLItemVO)}.
+     *
+     */
     @Override
-    public NetworkACLItem updateNetworkACLItem(final Long id, final String protocol, final List<String> sourceCidrList, final NetworkACLItem.TrafficType trafficType, final String action,
-            final Integer number, final Integer sourcePortStart, final Integer sourcePortEnd, final Integer icmpCode, final Integer icmpType, final String newUUID, final Boolean forDisplay) throws ResourceUnavailableException {
-        final NetworkACLItemVO aclItem = _networkACLItemDao.findById(id);
-        if (aclItem == null) {
-            throw new InvalidParameterValueException("Unable to find ACL Item cannot be found");
-        }
-
-        if (aclItem.getAclId() == NetworkACL.DEFAULT_ALLOW || aclItem.getAclId() == NetworkACL.DEFAULT_DENY) {
-            throw new InvalidParameterValueException("Default ACL Items cannot be updated");
-        }
-
-        final NetworkACL acl = _networkAclMgr.getNetworkACL(aclItem.getAclId());
-
-        final Vpc vpc = _entityMgr.findById(Vpc.class, acl.getVpcId());
+    public NetworkACLItem updateNetworkACLItem(UpdateNetworkACLItemCmd updateNetworkACLItemCmd) throws ResourceUnavailableException {
+        NetworkACLItemVO networkACLItemVo = validateNetworkAclRuleIdAndRetrieveIt(updateNetworkACLItemCmd);
 
-        final Account caller = CallContext.current().getCallingAccount();
+        NetworkACL acl = _networkAclMgr.getNetworkACL(networkACLItemVo.getAclId());
+        validateNetworkAcl(acl);
 
-        _accountMgr.checkAccess(caller, null, true, vpc);
+        transferDataToNetworkAclRulePojo(updateNetworkACLItemCmd, networkACLItemVo, acl);
+        validateNetworkACLItem(networkACLItemVo);
+        return _networkAclMgr.updateNetworkACLItem(networkACLItemVo);
+    }
 
+    /**
+     *  We transfer the update information form {@link UpdateNetworkACLItemCmd} to the {@link NetworkACLItemVO} POJO passed as parameter.
+     *  There is one validation performed here, which is regarding the number of the ACL. We will check if there is already an ACL rule with that number, and if this is the case an {@link InvalidParameterValueException} is thrown.
+     *  All of the parameters in {@link UpdateNetworkACLItemCmd} that are not null will be set to their corresponding fields in {@link NetworkACLItemVO}.
+     *
+     *  We use {@link #validateAndCreateNetworkAclRuleAction(String)} when converting an action as {@link String} to its Enum corresponding value.
+     */
+    protected void transferDataToNetworkAclRulePojo(UpdateNetworkACLItemCmd updateNetworkACLItemCmd, NetworkACLItemVO networkACLItemVo, NetworkACL acl) {
+        Integer number = updateNetworkACLItemCmd.getNumber();
         if (number != null) {
-            //Check if ACL Item with specified number already exists
-            final NetworkACLItemVO aclNumber = _networkACLItemDao.findByAclAndNumber(acl.getId(), number);
-            if (aclNumber != null && aclNumber.getId() != id) {
+            NetworkACLItemVO aclNumber = _networkACLItemDao.findByAclAndNumber(acl.getId(), number);
+            if (aclNumber != null && aclNumber.getId() != networkACLItemVo.getId()) {
                 throw new InvalidParameterValueException("ACL item with number " + number + " already exists in ACL: " + acl.getUuid());
             }
+            networkACLItemVo.setNumber(number);
         }
 
-        validateNetworkACLItem(sourcePortStart == null ? aclItem.getSourcePortStart() : sourcePortStart, sourcePortEnd == null ? aclItem.getSourcePortEnd()
-                : sourcePortEnd, sourceCidrList, protocol, icmpCode, icmpType == null ? aclItem.getIcmpType() : icmpType, action, number);
+        Integer sourcePortStart = updateNetworkACLItemCmd.getSourcePortStart();
+        if (sourcePortStart != null) {
+            networkACLItemVo.setSourcePortStart(sourcePortStart);
+        }
+        Integer sourcePortEnd = updateNetworkACLItemCmd.getSourcePortEnd();
+        if (sourcePortEnd != null) {
+            networkACLItemVo.setSourcePortEnd(sourcePortEnd);
+        }
+        List<String> sourceCidrList = updateNetworkACLItemCmd.getSourceCidrList();
+        if (CollectionUtils.isNotEmpty(sourceCidrList)) {
+            networkACLItemVo.setSourceCidrList(sourceCidrList);
+        }
+        String protocol = updateNetworkACLItemCmd.getProtocol();
+        if (StringUtils.isNotBlank(protocol)) {
+            networkACLItemVo.setProtocol(protocol);
+        }
+        Integer icmpCode = updateNetworkACLItemCmd.getIcmpCode();
+        if (icmpCode != null) {
+            networkACLItemVo.setIcmpCode(icmpCode);
+        }
+        Integer icmpType = updateNetworkACLItemCmd.getIcmpType();
+        if (icmpType != null) {
+            networkACLItemVo.setIcmpType(icmpType);
+        }
+        String action = updateNetworkACLItemCmd.getAction();
+        if (StringUtils.isNotBlank(action)) {
+            Action aclRuleAction = validateAndCreateNetworkAclRuleAction(action);
+            networkACLItemVo.setAction(aclRuleAction);
+        }
+        TrafficType trafficType = updateNetworkACLItemCmd.getTrafficType();
+        if (trafficType != null) {
+            networkACLItemVo.setTrafficType(trafficType);
+        }
+        String customId = updateNetworkACLItemCmd.getCustomId();
+        if (StringUtils.isNotBlank(customId)) {
+            networkACLItemVo.setUuid(customId);
+        }
+        boolean display = updateNetworkACLItemCmd.isDisplay();
+        if (display != networkACLItemVo.isDisplay()) {
+            networkACLItemVo.setDisplay(display);
+        }
+        String reason = updateNetworkACLItemCmd.getReason();
+        if (StringUtils.isNotBlank(reason)) {
+            networkACLItemVo.setReason(reason);
+        }
+    }
 
-        return _networkAclMgr.updateNetworkACLItem(id, protocol, sourceCidrList, trafficType, action, number, sourcePortStart, sourcePortEnd, icmpCode, icmpType, newUUID, forDisplay);
+    /**
+     * We validate the network ACL rule ID provided. If not ACL rule is found with the given Id an {@link InvalidParameterValueException} is thrown.
+     * If an ACL rule is found, we return the clone of the rule to avoid messing up with CGlib enhanced objects that might be linked to database entries.
+     */
+    protected NetworkACLItemVO validateNetworkAclRuleIdAndRetrieveIt(UpdateNetworkACLItemCmd updateNetworkACLItemCmd) {
+        Long id = updateNetworkACLItemCmd.getId();
+        NetworkACLItemVO networkACLItemVoFromDatabase = _networkACLItemDao.findById(id);
+        if (networkACLItemVoFromDatabase == null) {
+            throw new InvalidParameterValueException(String.format("Unable to find ACL rule with ID [%s]", id));
+        }
+        return networkACLItemVoFromDatabase.clone();
     }
 
     @Override
diff --git a/server/src/test/java/com/cloud/network/vpc/NetworkACLManagerImplTest.java b/server/src/test/java/com/cloud/network/vpc/NetworkACLManagerImplTest.java
new file mode 100644
index 00000000000..1d7cdc14e43
--- /dev/null
+++ b/server/src/test/java/com/cloud/network/vpc/NetworkACLManagerImplTest.java
@@ -0,0 +1,81 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package com.cloud.network.vpc;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.network.vpc.NetworkACLItem.State;
+import com.cloud.utils.exception.CloudRuntimeException;
+
+@RunWith(MockitoJUnitRunner.class)
+public class NetworkACLManagerImplTest {
+
+    @Spy
+    @InjectMocks
+    private NetworkACLManagerImpl networkACLManagerImpl;
+    @Mock
+    private NetworkACLItemDao networkACLItemDaoMock;
+
+    @Test(expected = CloudRuntimeException.class)
+    public void updateNetworkACLItemTestUpdateDoesNotWork() throws ResourceUnavailableException {
+        NetworkACLItemVO networkACLItemVOMock = new NetworkACLItemVO();
+        networkACLItemVOMock.id = 1L;
+
+        Mockito.doReturn(false).when(networkACLItemDaoMock).update(1L, networkACLItemVOMock);
+
+        networkACLManagerImpl.updateNetworkACLItem(networkACLItemVOMock);
+    }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void updateNetworkACLItemTestUpdateWorksButApplyDoesNotWork() throws ResourceUnavailableException {
+        NetworkACLItemVO networkACLItemVOMock = new NetworkACLItemVO();
+        networkACLItemVOMock.id = 1L;
+        networkACLItemVOMock.aclId = 2L;
+
+        Mockito.doReturn(true).when(networkACLItemDaoMock).update(1L, networkACLItemVOMock);
+        Mockito.doReturn(false).when(networkACLManagerImpl).applyNetworkACL(2L);
+
+        networkACLManagerImpl.updateNetworkACLItem(networkACLItemVOMock);
+    }
+
+    @Test
+    public void updateNetworkACLItemTestHappyDay() throws ResourceUnavailableException {
+        NetworkACLItemVO networkACLItemVOMock = new NetworkACLItemVO();
+        networkACLItemVOMock.id = 1L;
+        networkACLItemVOMock.aclId = 2L;
+
+        Mockito.doReturn(true).when(networkACLItemDaoMock).update(1L, networkACLItemVOMock);
+        Mockito.doReturn(true).when(networkACLManagerImpl).applyNetworkACL(2L);
+
+        NetworkACLItem returnedNetworkAclItem = networkACLManagerImpl.updateNetworkACLItem(networkACLItemVOMock);
+
+        Mockito.verify(networkACLItemDaoMock).update(1L, networkACLItemVOMock);
+        Mockito.verify(networkACLManagerImpl).applyNetworkACL(2L);
+
+        Assert.assertEquals(networkACLItemVOMock, returnedNetworkAclItem);
+        Assert.assertEquals(State.Add, returnedNetworkAclItem.getState());
+    }
+}
diff --git a/server/src/test/java/com/cloud/network/vpc/NetworkACLServiceImplTest.java b/server/src/test/java/com/cloud/network/vpc/NetworkACLServiceImplTest.java
new file mode 100644
index 00000000000..ee7a474bd19
--- /dev/null
+++ b/server/src/test/java/com/cloud/network/vpc/NetworkACLServiceImplTest.java
@@ -0,0 +1,774 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package com.cloud.network.vpc;
+
+import java.util.ArrayList;
+
+import org.apache.cloudstack.acl.SecurityChecker.AccessType;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.user.network.CreateNetworkACLCmd;
+import org.apache.cloudstack.api.command.user.network.UpdateNetworkACLItemCmd;
+import org.apache.cloudstack.context.CallContext;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InOrder;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.network.Network;
+import com.cloud.network.NetworkModel;
+import com.cloud.network.dao.NetworkVO;
+import com.cloud.network.vpc.NetworkACLItem.Action;
+import com.cloud.network.vpc.NetworkACLItem.TrafficType;
+import com.cloud.user.Account;
+import com.cloud.user.AccountManager;
+import com.cloud.utils.db.EntityManager;
+import com.cloud.utils.exception.CloudRuntimeException;
+
+@RunWith(PowerMockRunner.class)
+public class NetworkACLServiceImplTest {
+
+    @Spy
+    @InjectMocks
+    private NetworkACLServiceImpl networkAclServiceImpl = new NetworkACLServiceImpl();
+    @Mock
+    private NetworkModel networkModelMock;
+    @Mock
+    private NetworkACLManager networkAclManager;
+    @Mock
+    private NetworkACLItemDao networkAclItemDaoMock;
+    @Mock
+    private EntityManager EntityManagerMock;
+    @Mock
+    private AccountManager AccountManager;
+
+    @Mock
+    private CreateNetworkACLCmd createNetworkAclCmdMock;
+    @Mock
+    private UpdateNetworkACLItemCmd updateNetworkACLItemCmdMock;
+    @Mock
+    private Network networkMock;
+    @Mock
+    private NetworkACL networkAclMock;
+    @Mock
+    private NetworkACLItemVO networkAclItemVoMock;
+
+    private Long networkAclMockId = 1L;
+    private Long networkOfferingMockId = 2L;
+    private Long networkMockVpcMockId = 3L;
+
+    @Before
+    public void befoteTest() {
+        Mockito.when(createNetworkAclCmdMock.getNetworkId()).thenReturn(1L);
+        Mockito.when(createNetworkAclCmdMock.getProtocol()).thenReturn("tcp");
+
+        Mockito.when(networkMock.getNetworkOfferingId()).thenReturn(networkOfferingMockId);
+        Mockito.when(networkMock.getVpcId()).thenReturn(networkMockVpcMockId);
+    }
+
+    @Test
+    public void createNetworkACLItemTestAclNumberNull() {
+        createNetworkACLItemTestForNumberAndExecuteTest(null);
+    }
+
+    @Test
+    public void createNetworkACLItemTestAclNumberNotNull() {
+        createNetworkACLItemTestForNumberAndExecuteTest(10);
+    }
+
+    private void createNetworkACLItemTestForNumberAndExecuteTest(Integer number) {
+        Mockito.when(createNetworkAclCmdMock.getNumber()).thenReturn(number);
+
+        Mockito.doReturn(networkAclMockId).when(networkAclServiceImpl).createAclListIfNeeded(createNetworkAclCmdMock);
+        Mockito.when(networkAclManager.getNetworkACL(networkAclMockId)).thenReturn(networkAclMock);
+
+        Mockito.doNothing().when(networkAclServiceImpl).validateAclRuleNumber(createNetworkAclCmdMock, networkAclMock);
+        Mockito.doNothing().when(networkAclServiceImpl).validateNetworkAcl(networkAclMock);
+
+        Mockito.doReturn(Action.Allow).when(networkAclServiceImpl).validateAndCreateNetworkAclRuleAction(Mockito.anyString());
+        Mockito.when(networkAclItemDaoMock.getMaxNumberByACL(networkAclMockId)).thenReturn(5);
+
+        Mockito.doNothing().when(networkAclServiceImpl).validateNetworkACLItem(Mockito.any(NetworkACLItemVO.class));
+        Mockito.when(networkAclManager.createNetworkACLItem(Mockito.any(NetworkACLItemVO.class))).thenAnswer(new Answer<NetworkACLItemVO>() {
+            @Override
+            public NetworkACLItemVO answer(InvocationOnMock invocation) throws Throwable {
+                return (NetworkACLItemVO)invocation.getArguments()[0];
+            }
+        });
+
+        NetworkACLItem netowkrAclRuleCreated = networkAclServiceImpl.createNetworkACLItem(createNetworkAclCmdMock);
+
+        Assert.assertEquals(number == null ? 6 : number, netowkrAclRuleCreated.getNumber());
+
+        InOrder inOrder = Mockito.inOrder(networkAclManager, networkAclServiceImpl, networkAclItemDaoMock);
+        inOrder.verify(networkAclServiceImpl).createAclListIfNeeded(createNetworkAclCmdMock);
+        inOrder.verify(networkAclManager).getNetworkACL(networkAclMockId);
+        inOrder.verify(networkAclServiceImpl).validateNetworkAcl(networkAclMock);
+        inOrder.verify(networkAclServiceImpl).validateAclRuleNumber(createNetworkAclCmdMock, networkAclMock);
+        inOrder.verify(networkAclServiceImpl).validateAndCreateNetworkAclRuleAction(Mockito.anyString());
+        inOrder.verify(networkAclItemDaoMock, Mockito.times(number == null ? 1 : 0)).getMaxNumberByACL(networkAclMockId);
+        inOrder.verify(networkAclServiceImpl).validateNetworkACLItem(Mockito.any(NetworkACLItemVO.class));
+        inOrder.verify(networkAclManager).createNetworkACLItem(Mockito.any(NetworkACLItemVO.class));
+    }
+
+    @Test
+    public void createAclListIfNeededTestAclRuleListIdNotNull() {
+        Long expectedAclListId = 1L;
+        Mockito.when(createNetworkAclCmdMock.getACLId()).thenReturn(expectedAclListId);
+
+        Long returnetAclListId = networkAclServiceImpl.createAclListIfNeeded(createNetworkAclCmdMock);
+
+        Assert.assertEquals(expectedAclListId, returnetAclListId);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void createAclListIfNeededTestAclRuleListIdNullAndNetworkDoesNotHaveVpc() {
+        Mockito.when(createNetworkAclCmdMock.getACLId()).thenReturn(null);
+
+        long networkId = 1L;
+        Mockito.when(createNetworkAclCmdMock.getNetworkId()).thenReturn(networkId);
+
+        Network networkMock = Mockito.mock(Network.class);
+        Mockito.when(networkMock.getVpcId()).thenReturn(null);
+
+        Mockito.doReturn(networkMock).when(networkModelMock).getNetwork(networkId);
+
+        networkAclServiceImpl.createAclListIfNeeded(createNetworkAclCmdMock);
+    }
+
+    @Test
+    public void createAclListIfNeededTestAclRuleListIdNullAndNetworkWithVpcAndNotAclListYet() {
+        Mockito.when(createNetworkAclCmdMock.getACLId()).thenReturn(null);
+
+        long networkId = 1L;
+        Mockito.when(createNetworkAclCmdMock.getNetworkId()).thenReturn(networkId);
+
+        Network networkMock = Mockito.mock(Network.class);
+        Mockito.when(networkMock.getVpcId()).thenReturn(12L);
+        Mockito.when(networkMock.getNetworkACLId()).thenReturn(null);
+
+        Mockito.doReturn(networkMock).when(networkModelMock).getNetwork(networkId);
+
+        Long expectedAclListId = 15L;
+        Mockito.doReturn(expectedAclListId).when(networkAclServiceImpl).createAclListForNetworkAndReturnAclListId(createNetworkAclCmdMock, networkMock);
+
+        Long aclIdReturned = networkAclServiceImpl.createAclListIfNeeded(createNetworkAclCmdMock);
+
+        Assert.assertEquals(expectedAclListId, aclIdReturned);
+        Mockito.verify(networkAclServiceImpl).createAclListForNetworkAndReturnAclListId(createNetworkAclCmdMock, networkMock);
+    }
+
+    @Test
+    public void createAclListIfNeededTestAclRuleListIdNullAndNetworkWithVpcAndAclListAlreadyCreated() {
+        Mockito.when(createNetworkAclCmdMock.getACLId()).thenReturn(null);
+
+        long networkId = 1L;
+        Mockito.when(createNetworkAclCmdMock.getNetworkId()).thenReturn(networkId);
+        Network networkMock = Mockito.mock(Network.class);
+        ;
+        Mockito.when(networkMock.getVpcId()).thenReturn(12L);
+        Long expectedAclListId = 15L;
+        Mockito.when(networkMock.getNetworkACLId()).thenReturn(expectedAclListId);
+
+        Mockito.doReturn(networkMock).when(networkModelMock).getNetwork(networkId);
+
+        Mockito.doReturn(16L).when(networkAclServiceImpl).createAclListForNetworkAndReturnAclListId(createNetworkAclCmdMock, networkMock);
+
+        Long aclIdReturned = networkAclServiceImpl.createAclListIfNeeded(createNetworkAclCmdMock);
+
+        Assert.assertEquals(expectedAclListId, aclIdReturned);
+        Mockito.verify(networkAclServiceImpl, Mockito.times(0)).createAclListForNetworkAndReturnAclListId(createNetworkAclCmdMock, networkMock);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void createAclListForNetworkAndReturnAclListIdTestServicesNotSupportedByNetworkOffering() {
+        Mockito.doReturn(false).when(networkModelMock).areServicesSupportedByNetworkOffering(networkOfferingMockId, Network.Service.NetworkACL);
+        networkAclServiceImpl.createAclListForNetworkAndReturnAclListId(createNetworkAclCmdMock, networkMock);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void createAclListForNetworkAndReturnAclListIdTestServicesSupportedByNetworkOfferingButVpcNotFound() {
+        Mockito.doReturn(true).when(networkModelMock).areServicesSupportedByNetworkOffering(networkOfferingMockId, Network.Service.NetworkACL);
+        Mockito.doReturn(null).when(EntityManagerMock).findById(Vpc.class, networkMockVpcMockId);
+
+        networkAclServiceImpl.createAclListForNetworkAndReturnAclListId(createNetworkAclCmdMock, networkMock);
+    }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void createAclListForNetworkAndReturnAclListIdTestCreateNetworkAclReturnsNull() {
+        Mockito.doReturn(true).when(networkModelMock).areServicesSupportedByNetworkOffering(networkOfferingMockId, Network.Service.NetworkACL);
+        Mockito.doReturn(Mockito.mock(Vpc.class)).when(EntityManagerMock).findById(Vpc.class, networkMockVpcMockId);
+        Mockito.doReturn(null).when(networkAclManager).createNetworkACL(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong(), Mockito.anyBoolean());
+
+        networkAclServiceImpl.createAclListForNetworkAndReturnAclListId(createNetworkAclCmdMock, networkMock);
+    }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void createAclListForNetworkAndReturnAclListIdTestAclNetworkIsCreatedButNotApplied() throws ResourceUnavailableException {
+        Mockito.doReturn(true).when(networkModelMock).areServicesSupportedByNetworkOffering(networkOfferingMockId, Network.Service.NetworkACL);
+        Mockito.doReturn(Mockito.mock(Vpc.class)).when(EntityManagerMock).findById(Vpc.class, networkMockVpcMockId);
+        Mockito.doReturn(Mockito.mock(NetworkACL.class)).when(networkAclManager).createNetworkACL(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong(), Mockito.anyBoolean());
+        Mockito.doReturn(false).when(networkAclManager).replaceNetworkACL(Mockito.any(NetworkACL.class), Mockito.any(NetworkVO.class));
+
+        NetworkVO networkVoMock = new NetworkVO();
+        networkVoMock.setNetworkOfferingId(networkOfferingMockId);
+        networkVoMock.setVpcId(networkMockVpcMockId);
+
+        networkAclServiceImpl.createAclListForNetworkAndReturnAclListId(createNetworkAclCmdMock, networkVoMock);
+    }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void createAclListForNetworkAndReturnAclListIdTestAclNetworkIsCreatedButNotAppliedWithException() throws ResourceUnavailableException {
+        Mockito.doReturn(true).when(networkModelMock).areServicesSupportedByNetworkOffering(networkOfferingMockId, Network.Service.NetworkACL);
+        Mockito.doReturn(Mockito.mock(Vpc.class)).when(EntityManagerMock).findById(Vpc.class, networkMockVpcMockId);
+        Mockito.doReturn(Mockito.mock(NetworkACL.class)).when(networkAclManager).createNetworkACL(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong(), Mockito.anyBoolean());
+
+        Mockito.doThrow(ResourceUnavailableException.class).when(networkAclManager).replaceNetworkACL(Mockito.any(NetworkACL.class), Mockito.any(NetworkVO.class));
+
+        NetworkVO networkVoMock = new NetworkVO();
+        networkVoMock.setNetworkOfferingId(networkOfferingMockId);
+        networkVoMock.setVpcId(networkMockVpcMockId);
+
+        networkAclServiceImpl.createAclListForNetworkAndReturnAclListId(createNetworkAclCmdMock, networkVoMock);
+    }
+
+    @Test
+    public void createAclListForNetworkAndReturnAclListIdTestAclIsCreatedAndAppliedWithSuccess() throws ResourceUnavailableException {
+        Mockito.doReturn(true).when(networkModelMock).areServicesSupportedByNetworkOffering(networkOfferingMockId, Network.Service.NetworkACL);
+        Mockito.doReturn(Mockito.mock(Vpc.class)).when(EntityManagerMock).findById(Vpc.class, networkMockVpcMockId);
+
+        NetworkACL networkAclMock = Mockito.mock(NetworkACL.class);
+        Long expectedNetworkAclId = 5L;
+        Mockito.when(networkAclMock.getId()).thenReturn(expectedNetworkAclId);
+        Mockito.doReturn(networkAclMock).when(networkAclManager).createNetworkACL(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong(), Mockito.anyBoolean());
+
+        Mockito.doReturn(true).when(networkAclManager).replaceNetworkACL(Mockito.any(NetworkACL.class), Mockito.any(NetworkVO.class));
+
+        NetworkVO networkVoMock = new NetworkVO();
+        networkVoMock.setNetworkOfferingId(networkOfferingMockId);
+        networkVoMock.setVpcId(networkMockVpcMockId);
+
+        Long networkAclIdReceived = networkAclServiceImpl.createAclListForNetworkAndReturnAclListId(createNetworkAclCmdMock, networkVoMock);
+
+        Assert.assertEquals(expectedNetworkAclId, networkAclIdReceived);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void validateAclRuleNumberTestNumberLessThanOne() {
+        Mockito.when(createNetworkAclCmdMock.getNumber()).thenReturn(0);
+        networkAclServiceImpl.validateAclRuleNumber(createNetworkAclCmdMock, networkAclMock);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void validateAclRuleNumberTestNumberNegative() {
+        Mockito.when(createNetworkAclCmdMock.getNumber()).thenReturn(-1);
+        networkAclServiceImpl.validateAclRuleNumber(createNetworkAclCmdMock, networkAclMock);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void validateAclRuleNumberTestNumberInOtherAcl() {
+        Mockito.when(createNetworkAclCmdMock.getNumber()).thenReturn(1);
+        Mockito.doReturn(Mockito.mock(NetworkACLItemVO.class)).when(networkAclItemDaoMock).findByAclAndNumber(Mockito.anyLong(), Mockito.anyInt());
+
+        networkAclServiceImpl.validateAclRuleNumber(createNetworkAclCmdMock, networkAclMock);
+
+        Mockito.verify(networkAclItemDaoMock).findByAclAndNumber(Mockito.anyLong(), Mockito.anyInt());
+    }
+
+    @Test
+    public void validateAclRuleNumberTestHappyDay() {
+        Mockito.when(createNetworkAclCmdMock.getNumber()).thenReturn(1);
+        Mockito.doReturn(null).when(networkAclItemDaoMock).findByAclAndNumber(Mockito.anyLong(), Mockito.anyInt());
+
+        networkAclServiceImpl.validateAclRuleNumber(createNetworkAclCmdMock, networkAclMock);
+        Mockito.verify(networkAclItemDaoMock).findByAclAndNumber(Mockito.anyLong(), Mockito.anyInt());
+    }
+
+    @Test
+    public void validateAclRuleNumberTestNumberNull() {
+        Mockito.when(createNetworkAclCmdMock.getNumber()).thenReturn(null);
+        Mockito.doReturn(null).when(networkAclItemDaoMock).findByAclAndNumber(Mockito.anyLong(), Mockito.anyInt());
+
+        networkAclServiceImpl.validateAclRuleNumber(createNetworkAclCmdMock, networkAclMock);
+        Mockito.verify(networkAclItemDaoMock, Mockito.times(0)).findByAclAndNumber(Mockito.anyLong(), Mockito.anyInt());
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void validateNetworkAclTestAclNull() {
+        networkAclServiceImpl.validateNetworkAcl(null);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void validateNetworkAclTestAclDefaulAllow() {
+        Mockito.when(networkAclMock.getId()).thenReturn(2L);
+        networkAclServiceImpl.validateNetworkAcl(networkAclMock);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void validateNetworkAclTestAclDefaulDeny() {
+        Mockito.when(networkAclMock.getId()).thenReturn(1L);
+        networkAclServiceImpl.validateNetworkAcl(networkAclMock);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void validateNetworkAclTestAclNotDefaulWithoutVpc() {
+        Mockito.when(networkAclMock.getId()).thenReturn(3L);
+        Mockito.doReturn(null).when(EntityManagerMock).findById(Vpc.class, networkMockVpcMockId);
+        ;
+
+        networkAclServiceImpl.validateNetworkAcl(networkAclMock);
+    }
+
+    @Test
+    @PrepareForTest(CallContext.class)
+    public void validateNetworkAclTestAclNotDefaulWithVpc() {
+        CallContext callContextMock = Mockito.mock(CallContext.class);
+        Mockito.doReturn(Mockito.mock(Account.class)).when(callContextMock).getCallingAccount();
+
+        PowerMockito.mockStatic(CallContext.class);
+        PowerMockito.when(CallContext.current()).thenReturn(callContextMock);
+
+        Mockito.when(networkAclMock.getId()).thenReturn(3L);
+        Mockito.when(networkAclMock.getVpcId()).thenReturn(networkMockVpcMockId);
+
+        Mockito.doReturn(Mockito.mock(Vpc.class)).when(EntityManagerMock).findById(Vpc.class, networkMockVpcMockId);
+        Mockito.doNothing().when(AccountManager).checkAccess(Mockito.any(Account.class), Mockito.isNull(AccessType.class), Mockito.eq(true), Mockito.any(Vpc.class));
+
+        networkAclServiceImpl.validateNetworkAcl(networkAclMock);
+
+        Mockito.verify(EntityManagerMock).findById(Vpc.class, networkMockVpcMockId);
+        Mockito.verify(AccountManager).checkAccess(Mockito.any(Account.class), Mockito.isNull(AccessType.class), Mockito.eq(true), Mockito.any(Vpc.class));
+
+        PowerMockito.verifyStatic();
+        CallContext.current();
+
+    }
+
+    @Test
+    public void validateAndCreateNetworkAclRuleActionTestActionNull() {
+        NetworkACLItem.Action receivedAction = networkAclServiceImpl.validateAndCreateNetworkAclRuleAction(null);
+
+        Assert.assertEquals(NetworkACLItem.Action.Allow, receivedAction);
+
+        Mockito.verify(networkAclServiceImpl).validateNetworkAclRuleAction(null);
+    }
+
+    @Test
+    public void validateAndCreateNetworkAclRuleActionTestActionAllow() {
+        NetworkACLItem.Action receivedAction = networkAclServiceImpl.validateAndCreateNetworkAclRuleAction("allow");
+
+        Assert.assertEquals(NetworkACLItem.Action.Allow, receivedAction);
+        Mockito.verify(networkAclServiceImpl).validateNetworkAclRuleAction("allow");
+    }
+
+    @Test
+    public void validateAndCreateNetworkAclRuleActionTestActionDeny() {
+        NetworkACLItem.Action receivedAction = networkAclServiceImpl.validateAndCreateNetworkAclRuleAction("deny");
+
+        Assert.assertEquals(NetworkACLItem.Action.Deny, receivedAction);
+        Mockito.verify(networkAclServiceImpl).validateNetworkAclRuleAction("deny");
+    }
+
+    @Test
+    public void validateNetworkAclRuleActionTestActionNull() {
+        networkAclServiceImpl.validateNetworkAclRuleAction(null);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void validateNetworkAclRuleActionTestInvalidAction() {
+        networkAclServiceImpl.validateNetworkAclRuleAction("Invalid");
+    }
+
+    @Test
+    public void validateNetworkAclRuleActionTestValidActions() {
+        networkAclServiceImpl.validateNetworkAclRuleAction("deny");
+        networkAclServiceImpl.validateNetworkAclRuleAction("allow");
+    }
+
+    @Test
+    public void validateNetworkACLItemTest() {
+        Mockito.doNothing().when(networkAclServiceImpl).validateSourceStartAndEndPorts(networkAclItemVoMock);
+        Mockito.doNothing().when(networkAclServiceImpl).validateSourceCidrList(networkAclItemVoMock);
+        Mockito.doNothing().when(networkAclServiceImpl).validateProtocol(networkAclItemVoMock);
+
+        networkAclServiceImpl.validateNetworkACLItem(networkAclItemVoMock);
+
+        InOrder inOrder = Mockito.inOrder(networkAclServiceImpl);
+        inOrder.verify(networkAclServiceImpl).validateSourceStartAndEndPorts(networkAclItemVoMock);
+        inOrder.verify(networkAclServiceImpl).validateSourceCidrList(networkAclItemVoMock);
+        inOrder.verify(networkAclServiceImpl).validateProtocol(networkAclItemVoMock);
+    }
+
+    @Test
+    public void validateSourceStartAndEndPortsTestBothPortsNull() {
+        networkAclServiceImpl.validateSourceStartAndEndPorts(networkAclItemVoMock);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void validateSourceStartAndEndPortsTestStartPorInvalid() {
+        Mockito.when(networkAclItemVoMock.getSourcePortStart()).thenReturn(65536);
+
+        networkAclServiceImpl.validateSourceStartAndEndPorts(networkAclItemVoMock);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void validateSourceStartAndEndPortsTestStartPorValidButEndPortInvalid() {
+        Mockito.when(networkAclItemVoMock.getSourcePortStart()).thenReturn(65535);
+        Mockito.when(networkAclItemVoMock.getSourcePortEnd()).thenReturn(65536);
+
+        networkAclServiceImpl.validateSourceStartAndEndPorts(networkAclItemVoMock);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void validateSourceStartAndEndPortsTestStartPortBiggerThanEndPort() {
+        Mockito.when(networkAclItemVoMock.getSourcePortStart()).thenReturn(65535);
+        Mockito.when(networkAclItemVoMock.getSourcePortEnd()).thenReturn(2);
+
+        networkAclServiceImpl.validateSourceStartAndEndPorts(networkAclItemVoMock);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void validateSourceStartAndEndPortsTestPortsWithAllProtocol() {
+        Mockito.when(networkAclItemVoMock.getSourcePortStart()).thenReturn(1);
+        Mockito.when(networkAclItemVoMock.getSourcePortEnd()).thenReturn(2);
+        Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn("all");
+
+        networkAclServiceImpl.validateSourceStartAndEndPorts(networkAclItemVoMock);
+    }
+
+    @Test
+    public void validateSourceStartAndEndPortsTestPortsWithTcpProtocol() {
+        Mockito.when(networkAclItemVoMock.getSourcePortStart()).thenReturn(1);
+        Mockito.when(networkAclItemVoMock.getSourcePortEnd()).thenReturn(2);
+        Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn("tcp");
+
+        networkAclServiceImpl.validateSourceStartAndEndPorts(networkAclItemVoMock);
+    }
+
+    @Test
+    public void validateSourceCidrListTestEmptySourceCirdList() {
+        Mockito.when(networkAclItemVoMock.getSourceCidrList()).thenReturn(new ArrayList<>());
+        networkAclServiceImpl.validateSourceCidrList(networkAclItemVoMock);
+    }
+
+    @Test(expected = ServerApiException.class)
+    public void validateSourceCidrListTestInvalidCidrs() {
+        ArrayList<String> cidrsInvalid = new ArrayList<>();
+        cidrsInvalid.add("256.0.0.0./32");
+
+        Mockito.when(networkAclItemVoMock.getSourceCidrList()).thenReturn(cidrsInvalid);
+        networkAclServiceImpl.validateSourceCidrList(networkAclItemVoMock);
+    }
+
+    @Test
+    public void validateSourceCidrListTestValidCidrs() {
+        ArrayList<String> cidrsInvalid = new ArrayList<>();
+        cidrsInvalid.add("192.168.12.0/24");
+
+        Mockito.when(networkAclItemVoMock.getSourceCidrList()).thenReturn(cidrsInvalid);
+        networkAclServiceImpl.validateSourceCidrList(networkAclItemVoMock);
+    }
+
+    @Test
+    public void validateProtocolTestProtocolIsNullOrBlank() {
+        Mockito.doNothing().when(networkAclServiceImpl).validateIcmpTypeAndCode(networkAclItemVoMock);
+
+        Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn(null);
+        networkAclServiceImpl.validateProtocol(networkAclItemVoMock);
+
+        Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn("");
+        networkAclServiceImpl.validateProtocol(networkAclItemVoMock);
+
+        Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn("    ");
+        networkAclServiceImpl.validateProtocol(networkAclItemVoMock);
+
+        Mockito.verify(networkAclServiceImpl, Mockito.times(0)).validateIcmpTypeAndCode(networkAclItemVoMock);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void validateProtocolTestProtocolIsNumericValueLessThanZero() {
+        Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn("-1");
+        networkAclServiceImpl.validateProtocol(networkAclItemVoMock);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void validateProtocolTestProtocolIsNumericValueMoreThan255() {
+        Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn("256");
+        networkAclServiceImpl.validateProtocol(networkAclItemVoMock);
+    }
+
+    @Test
+    public void validateProtocolTestProtocolIsNumericValidValue() {
+        Mockito.doNothing().when(networkAclServiceImpl).validateIcmpTypeAndCode(networkAclItemVoMock);
+
+        Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn("255");
+        Mockito.when(networkAclItemVoMock.getIcmpCode()).thenReturn(null);
+        Mockito.when(networkAclItemVoMock.getIcmpType()).thenReturn(null);
+
+        networkAclServiceImpl.validateProtocol(networkAclItemVoMock);
+
+        Mockito.verify(networkAclServiceImpl).validateIcmpTypeAndCode(networkAclItemVoMock);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void validateProtocolTestProtocolIsStringInvalid() {
+        Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn("invalid");
+        networkAclServiceImpl.validateProtocol(networkAclItemVoMock);
+    }
+
+    @Test
+    public void validateProtocolTestProtocolIsStringValid() {
+        Mockito.doNothing().when(networkAclServiceImpl).validateIcmpTypeAndCode(networkAclItemVoMock);
+
+        Mockito.when(networkAclItemVoMock.getIcmpCode()).thenReturn(null);
+        Mockito.when(networkAclItemVoMock.getIcmpType()).thenReturn(null);
+
+        Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn("tcp");
+        networkAclServiceImpl.validateProtocol(networkAclItemVoMock);
+
+        Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn("all");
+        networkAclServiceImpl.validateProtocol(networkAclItemVoMock);
+
+        Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn("udp");
+        networkAclServiceImpl.validateProtocol(networkAclItemVoMock);
+
+        Mockito.verify(networkAclServiceImpl, Mockito.times(3)).validateIcmpTypeAndCode(networkAclItemVoMock);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void validateProtocolTestProtocolNotIcmpWithIcmpConfigurations() {
+        Mockito.when(networkAclItemVoMock.getIcmpCode()).thenReturn(1);
+        Mockito.when(networkAclItemVoMock.getIcmpType()).thenReturn(1);
+
+        Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn("tcp");
+        networkAclServiceImpl.validateProtocol(networkAclItemVoMock);
+    }
+
+    @Test
+    public void validateProtocolTestProtocolNotIcmpWithSourcePorts() {
+        Mockito.doNothing().when(networkAclServiceImpl).validateIcmpTypeAndCode(networkAclItemVoMock);
+
+        Mockito.when(networkAclItemVoMock.getIcmpCode()).thenReturn(null);
+        Mockito.when(networkAclItemVoMock.getIcmpType()).thenReturn(null);
+
+        Mockito.when(networkAclItemVoMock.getSourcePortStart()).thenReturn(1);
+        Mockito.when(networkAclItemVoMock.getSourcePortEnd()).thenReturn(1);
+
+        Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn("tcp");
+        networkAclServiceImpl.validateProtocol(networkAclItemVoMock);
+
+        Mockito.verify(networkAclServiceImpl).validateIcmpTypeAndCode(networkAclItemVoMock);
+    }
+
+    @Test
+    public void validateProtocolTestProtocolIcmpWithIcmpConfigurations() {
+        Mockito.when(networkAclItemVoMock.getIcmpCode()).thenReturn(1);
+        Mockito.when(networkAclItemVoMock.getIcmpType()).thenReturn(1);
+
+        Mockito.when(networkAclItemVoMock.getSourcePortStart()).thenReturn(null);
+        Mockito.when(networkAclItemVoMock.getSourcePortEnd()).thenReturn(null);
+
+        Mockito.doNothing().when(networkAclServiceImpl).validateIcmpTypeAndCode(networkAclItemVoMock);
+
+        Mockito.when(networkAclItemVoMock.getProtocol()).thenReturn("icmp");
+        networkAclServiceImpl.validateProtocol(networkAclItemVoMock);
+
+        Mockito.verify(networkAclServiceImpl).validateIcmpTypeAndCode(networkAclItemVoMock);
+    }
+
+    @Test
+    public void validateIcmpTypeAndCodeTestIcmpTypeNull() {
+        Mockito.when(networkAclItemVoMock.getIcmpType()).thenReturn(null);
+
+        networkAclServiceImpl.validateIcmpTypeAndCode(networkAclItemVoMock);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void validateIcmpTypeAndCodeTestIcmpTypeInvalid() {
+        Mockito.when(networkAclItemVoMock.getIcmpType()).thenReturn(256);
+
+        networkAclServiceImpl.validateIcmpTypeAndCode(networkAclItemVoMock);
+    }
+
+    @Test
+    public void validateIcmpTypeAndCodeTestIcmpTypeNegativeOne() {
+        Mockito.when(networkAclItemVoMock.getIcmpType()).thenReturn(-1);
+        Mockito.when(networkAclItemVoMock.getIcmpCode()).thenReturn(null);
+
+        networkAclServiceImpl.validateIcmpTypeAndCode(networkAclItemVoMock);
+    }
+
+    @Test
+    public void validateIcmpTypeAndCodeTestIcmpTypeNegativeOneAndIcmpCodeNegativeOne() {
+        Mockito.when(networkAclItemVoMock.getIcmpType()).thenReturn(-1);
+        Mockito.when(networkAclItemVoMock.getIcmpCode()).thenReturn(-1);
+
+        networkAclServiceImpl.validateIcmpTypeAndCode(networkAclItemVoMock);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void validateIcmpTypeAndCodeTestIcmpTypeValidAndIcmpCodeInvalid() {
+        Mockito.when(networkAclItemVoMock.getIcmpType()).thenReturn(255);
+        Mockito.when(networkAclItemVoMock.getIcmpCode()).thenReturn(16);
+
+        networkAclServiceImpl.validateIcmpTypeAndCode(networkAclItemVoMock);
+    }
+
+    @Test
+    public void validateIcmpTypeAndCodeTestIcmpTypeValidAndIcmpCodeValid() {
+        Mockito.when(networkAclItemVoMock.getIcmpType()).thenReturn(255);
+        Mockito.when(networkAclItemVoMock.getIcmpCode()).thenReturn(1);
+
+        networkAclServiceImpl.validateIcmpTypeAndCode(networkAclItemVoMock);
+    }
+
+    @Test
+    public void updateNetworkACLItemTest() throws ResourceUnavailableException {
+        Mockito.when(networkAclItemVoMock.getAclId()).thenReturn(networkAclMockId);
+        Mockito.doReturn(networkAclItemVoMock).when(networkAclServiceImpl).validateNetworkAclRuleIdAndRetrieveIt(updateNetworkACLItemCmdMock);
+        Mockito.doReturn(networkAclMock).when(networkAclManager).getNetworkACL(networkAclMockId);
+        Mockito.doNothing().when(networkAclServiceImpl).validateNetworkAcl(Mockito.eq(networkAclMock));
+        Mockito.doNothing().when(networkAclServiceImpl).transferDataToNetworkAclRulePojo(Mockito.eq(updateNetworkACLItemCmdMock), Mockito.eq(networkAclItemVoMock), Mockito.eq(networkAclMock));
+        Mockito.doNothing().when(networkAclServiceImpl).validateNetworkACLItem(networkAclItemVoMock);
+        Mockito.doReturn(networkAclItemVoMock).when(networkAclManager).updateNetworkACLItem(networkAclItemVoMock);
+
+        networkAclServiceImpl.updateNetworkACLItem(updateNetworkACLItemCmdMock);
+
+        InOrder inOrder = Mockito.inOrder(networkAclServiceImpl, networkAclManager);
+        inOrder.verify(networkAclServiceImpl).validateNetworkAclRuleIdAndRetrieveIt(updateNetworkACLItemCmdMock);
+        inOrder.verify(networkAclManager).getNetworkACL(networkAclMockId);
+        inOrder.verify(networkAclServiceImpl).validateNetworkAcl(networkAclMock);
+        inOrder.verify(networkAclServiceImpl).transferDataToNetworkAclRulePojo(Mockito.eq(updateNetworkACLItemCmdMock), Mockito.eq(networkAclItemVoMock), Mockito.eq(networkAclMock));
+        inOrder.verify(networkAclServiceImpl).validateNetworkACLItem(networkAclItemVoMock);
+        inOrder.verify(networkAclManager).updateNetworkACLItem(networkAclItemVoMock);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void validateNetworkAclRuleIdAndRetrieveItTestNetworkAclNotFound() {
+        Mockito.doReturn(null).when(networkAclItemDaoMock).findById(Mockito.anyLong());
+
+        networkAclServiceImpl.validateNetworkAclRuleIdAndRetrieveIt(updateNetworkACLItemCmdMock);
+    }
+
+    @Test
+    public void validateNetworkAclRuleIdAndRetrieveItTestNetworkAclFound() {
+        Mockito.doReturn(networkAclItemVoMock).when(networkAclItemDaoMock).findById(Mockito.anyLong());
+
+        NetworkACLItemVO returnedNetworkAclItemVo = networkAclServiceImpl.validateNetworkAclRuleIdAndRetrieveIt(updateNetworkACLItemCmdMock);
+
+        Assert.assertNotEquals(networkAclItemVoMock, returnedNetworkAclItemVo);
+        Mockito.verify(networkAclItemVoMock).clone();
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void transferDataToNetworkAclRulePojoTestNumberOfAcltoBeUpdatedAlreadyInUse() {
+        int aclNumberToUpdate = 1;
+        Mockito.when(updateNetworkACLItemCmdMock.getNumber()).thenReturn(aclNumberToUpdate);
+        Mockito.when(networkAclMock.getId()).thenReturn(networkAclMockId);
+        Mockito.when(networkAclItemVoMock.getId()).thenReturn(100L);
+        NetworkACLItemVO otherNetworkAclItemVoMock = Mockito.mock(NetworkACLItemVO.class);
+        Mockito.when(otherNetworkAclItemVoMock.getId()).thenReturn(101L);
+        Mockito.doReturn(otherNetworkAclItemVoMock).when(networkAclItemDaoMock).findByAclAndNumber(networkAclMockId, aclNumberToUpdate);
+
+        networkAclServiceImpl.transferDataToNetworkAclRulePojo(updateNetworkACLItemCmdMock, networkAclItemVoMock, networkAclMock);
+    }
+
+    @Test
+    public void transferDataToNetworkAclRulePojoTestAllValuesNull() {
+        Mockito.when(updateNetworkACLItemCmdMock.getNumber()).thenReturn(null);
+        Mockito.when(updateNetworkACLItemCmdMock.getSourcePortStart()).thenReturn(null);
+        Mockito.when(updateNetworkACLItemCmdMock.getSourcePortEnd()).thenReturn(null);
+        Mockito.when(updateNetworkACLItemCmdMock.getSourceCidrList()).thenReturn(null);
+        Mockito.when(updateNetworkACLItemCmdMock.getProtocol()).thenReturn(null);
+        Mockito.when(updateNetworkACLItemCmdMock.getIcmpCode()).thenReturn(null);
+        Mockito.when(updateNetworkACLItemCmdMock.getIcmpType()).thenReturn(null);
+        Mockito.when(updateNetworkACLItemCmdMock.getAction()).thenReturn(null);
+        Mockito.when(updateNetworkACLItemCmdMock.getTrafficType()).thenReturn(null);
+        Mockito.when(updateNetworkACLItemCmdMock.getCustomId()).thenReturn(null);
+        Mockito.when(updateNetworkACLItemCmdMock.getReason()).thenReturn(null);
+
+        Mockito.when(updateNetworkACLItemCmdMock.isDisplay()).thenReturn(false);
+        Mockito.when(networkAclItemVoMock.isDisplay()).thenReturn(false);
+
+        networkAclServiceImpl.transferDataToNetworkAclRulePojo(updateNetworkACLItemCmdMock, networkAclItemVoMock, networkAclMock);
+
+        Mockito.verify(networkAclItemVoMock, Mockito.times(0)).setNumber(Mockito.anyInt());
+        Mockito.verify(networkAclItemVoMock, Mockito.times(0)).setSourcePortStart(Mockito.anyInt());
+        Mockito.verify(networkAclItemVoMock, Mockito.times(0)).setSourcePortEnd(Mockito.anyInt());
+        Mockito.verify(networkAclItemVoMock, Mockito.times(0)).setSourceCidrList(Mockito.anyListOf(String.class));
+        Mockito.verify(networkAclItemVoMock, Mockito.times(0)).setProtocol(Mockito.anyString());
+        Mockito.verify(networkAclItemVoMock, Mockito.times(0)).setIcmpCode(Mockito.anyInt());
+        Mockito.verify(networkAclItemVoMock, Mockito.times(0)).setIcmpType(Mockito.anyInt());
+        Mockito.verify(networkAclItemVoMock, Mockito.times(0)).setAction(Mockito.any(Action.class));
+        Mockito.verify(networkAclItemVoMock, Mockito.times(0)).setTrafficType(Mockito.any(TrafficType.class));
+        Mockito.verify(networkAclItemVoMock, Mockito.times(0)).setUuid(Mockito.anyString());
+        Mockito.verify(networkAclItemVoMock, Mockito.times(0)).setReason(Mockito.anyString());
+        Mockito.verify(networkAclItemVoMock, Mockito.times(0)).setDisplay(Mockito.anyBoolean());
+        Mockito.verify(networkAclServiceImpl, Mockito.times(0)).validateAndCreateNetworkAclRuleAction(Mockito.anyString());
+    }
+
+    @Test
+    public void transferDataToNetworkAclRulePojoTestAllValuesWithUpdateData() {
+        Mockito.when(updateNetworkACLItemCmdMock.getNumber()).thenReturn(1);
+        Mockito.when(updateNetworkACLItemCmdMock.getSourcePortStart()).thenReturn(23);
+        Mockito.when(updateNetworkACLItemCmdMock.getSourcePortEnd()).thenReturn(24);
+
+        ArrayList<String> cidrsList = new ArrayList<>();
+        cidrsList.add("192.168.6.0/24");
+        Mockito.when(updateNetworkACLItemCmdMock.getSourceCidrList()).thenReturn(cidrsList);
+
+        Mockito.when(updateNetworkACLItemCmdMock.getProtocol()).thenReturn("all");
+        Mockito.when(updateNetworkACLItemCmdMock.getIcmpCode()).thenReturn(5);
+        Mockito.when(updateNetworkACLItemCmdMock.getIcmpType()).thenReturn(6);
+        Mockito.when(updateNetworkACLItemCmdMock.getAction()).thenReturn("deny");
+        Mockito.when(updateNetworkACLItemCmdMock.getTrafficType()).thenReturn(TrafficType.Egress);
+        Mockito.when(updateNetworkACLItemCmdMock.getCustomId()).thenReturn("customUuid");
+        Mockito.when(updateNetworkACLItemCmdMock.getReason()).thenReturn("reason");
+
+        Mockito.when(updateNetworkACLItemCmdMock.isDisplay()).thenReturn(true);
+        Mockito.when(networkAclItemVoMock.isDisplay()).thenReturn(false);
+
+        networkAclServiceImpl.transferDataToNetworkAclRulePojo(updateNetworkACLItemCmdMock, networkAclItemVoMock, networkAclMock);
+
+        Mockito.verify(networkAclItemVoMock).setNumber(1);
+        Mockito.verify(networkAclItemVoMock).setSourcePortStart(23);
+        Mockito.verify(networkAclItemVoMock).setSourcePortEnd(24);
+        Mockito.verify(networkAclItemVoMock).setSourceCidrList(cidrsList);
+        Mockito.verify(networkAclItemVoMock).setProtocol("all");
+        Mockito.verify(networkAclItemVoMock).setIcmpCode(5);
+        Mockito.verify(networkAclItemVoMock).setIcmpType(6);
+        Mockito.verify(networkAclItemVoMock).setAction(Action.Deny);
+        Mockito.verify(networkAclItemVoMock).setTrafficType(TrafficType.Egress);
+        Mockito.verify(networkAclItemVoMock).setUuid("customUuid");
+        Mockito.verify(networkAclItemVoMock).setReason("reason");
+        Mockito.verify(networkAclItemVoMock).setDisplay(true);
+        Mockito.verify(networkAclServiceImpl).validateAndCreateNetworkAclRuleAction("deny");
+    }
+
+}
diff --git a/server/src/test/java/com/cloud/vpc/NetworkACLManagerTest.java b/server/src/test/java/com/cloud/vpc/NetworkACLManagerTest.java
index 9daf551e9ec..ca1ddc62b52 100644
--- a/server/src/test/java/com/cloud/vpc/NetworkACLManagerTest.java
+++ b/server/src/test/java/com/cloud/vpc/NetworkACLManagerTest.java
@@ -22,8 +22,6 @@
 
 import javax.inject.Inject;
 
-import junit.framework.TestCase;
-
 import org.apache.cloudstack.context.CallContext;
 import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
 import org.apache.cloudstack.framework.messagebus.MessageBus;
@@ -74,6 +72,8 @@
 import com.cloud.utils.component.ComponentContext;
 import com.cloud.utils.db.EntityManager;
 
+import junit.framework.TestCase;
+
 @RunWith(SpringJUnit4ClassRunner.class)
 @ContextConfiguration(loader = AnnotationConfigContextLoader.class)
 public class NetworkACLManagerTest extends TestCase {
@@ -135,8 +135,7 @@ public void testCreateACL() throws Exception {
     public void testApplyACL() throws Exception {
         final NetworkVO network = Mockito.mock(NetworkVO.class);
         Mockito.when(_networkDao.findById(Matchers.anyLong())).thenReturn(network);
-        Mockito.when(_networkModel.isProviderSupportServiceInNetwork(Matchers.anyLong(), Matchers.any(Network.Service.class), Matchers.any(Network.Provider.class)))
-        .thenReturn(true);
+        Mockito.when(_networkModel.isProviderSupportServiceInNetwork(Matchers.anyLong(), Matchers.any(Network.Service.class), Matchers.any(Network.Provider.class))).thenReturn(true);
         Mockito.when(_networkAclElements.get(0).applyNetworkACLs(Matchers.any(Network.class), Matchers.anyList())).thenReturn(true);
         assertTrue(_aclMgr.applyACLToNetwork(1L));
     }
@@ -162,14 +161,10 @@ public void driveTestApplyNetworkACL(final boolean result, final boolean applyNe
         final NetworkVO network = Mockito.mock(NetworkVO.class);
         final List<NetworkVO> networks = new ArrayList<NetworkVO>();
         networks.add(network);
-        Mockito.when(_networkDao.listByAclId(Matchers.anyLong()))
-        .thenReturn(networks);
+        Mockito.when(_networkDao.listByAclId(Matchers.anyLong())).thenReturn(networks);
         Mockito.when(_networkDao.findById(Matchers.anyLong())).thenReturn(network);
-        Mockito.when(_networkModel.isProviderSupportServiceInNetwork(Matchers.anyLong(),
-                Matchers.any(Network.Service.class), Matchers.any(Network.Provider.class)))
-                .thenReturn(true);
-        Mockito.when(_networkAclElements.get(0).applyNetworkACLs(Matchers.any(Network.class),
-                Matchers.anyList())).thenReturn(applyNetworkACLs);
+        Mockito.when(_networkModel.isProviderSupportServiceInNetwork(Matchers.anyLong(), Matchers.any(Network.Service.class), Matchers.any(Network.Provider.class))).thenReturn(true);
+        Mockito.when(_networkAclElements.get(0).applyNetworkACLs(Matchers.any(Network.class), Matchers.anyList())).thenReturn(applyNetworkACLs);
 
         // Make sure it applies ACL to private gateway
         final List<VpcGatewayVO> vpcGateways = new ArrayList<VpcGatewayVO>();
@@ -177,8 +172,7 @@ public void driveTestApplyNetworkACL(final boolean result, final boolean applyNe
         final PrivateGateway privateGateway = Mockito.mock(PrivateGateway.class);
         Mockito.when(_vpcSvc.getVpcPrivateGateway(Mockito.anyLong())).thenReturn(privateGateway);
         vpcGateways.add(vpcGateway);
-        Mockito.when(_vpcGatewayDao.listByAclIdAndType(aclId, VpcGateway.Type.Private))
-        .thenReturn(vpcGateways);
+        Mockito.when(_vpcGatewayDao.listByAclIdAndType(aclId, VpcGateway.Type.Private)).thenReturn(vpcGateways);
 
         // Create 4 rules to test all 4 scenarios: only revoke should
         // be deleted, only add should update
@@ -203,8 +197,7 @@ public void driveTestApplyNetworkACL(final boolean result, final boolean applyNe
         Mockito.when(rule2Add.getId()).thenReturn(addId);
         Mockito.when(_networkACLItemDao.findById(addId)).thenReturn(rule2Add);
 
-        Mockito.when(_networkACLItemDao.listByACL(aclId))
-        .thenReturn(rules);
+        Mockito.when(_networkACLItemDao.listByACL(aclId)).thenReturn(rules);
         // Mock methods to avoid
         Mockito.doReturn(applyACLToPrivateGw).when(aclManager).applyACLToPrivateGw(privateGateway);
 
@@ -218,20 +211,12 @@ public void driveTestApplyNetworkACL(final boolean result, final boolean applyNe
         Mockito.verify(_networkACLItemDao, Mockito.times(timesProcessingDone)).update(addId, rule2Add);
     }
 
-
     @Test
     public void testRevokeACLItem() throws Exception {
         Mockito.when(_networkACLItemDao.findById(Matchers.anyLong())).thenReturn(aclItem);
         assertTrue(_aclMgr.revokeNetworkACLItem(1L));
     }
 
-    @Test
-    public void testUpdateACLItem() throws Exception {
-        Mockito.when(_networkACLItemDao.findById(Matchers.anyLong())).thenReturn(aclItem);
-        Mockito.when(_networkACLItemDao.update(Matchers.anyLong(), Matchers.any(NetworkACLItemVO.class))).thenReturn(true);
-        assertNotNull(_aclMgr.updateNetworkACLItem(1L, "UDP", null, NetworkACLItem.TrafficType.Ingress, "Deny", 10, 22, 32, null, null, null, true));
-    }
-
     @Test
     public void deleteNonEmptyACL() throws Exception {
         final List<NetworkACLItemVO> aclItems = new ArrayList<NetworkACLItemVO>();
@@ -251,8 +236,8 @@ public void deleteNonEmptyACL() throws Exception {
     }
 
     @Configuration
-    @ComponentScan(basePackageClasses = {NetworkACLManagerImpl.class}, includeFilters = {@ComponentScan.Filter(value = NetworkACLTestConfiguration.Library.class,
-    type = FilterType.CUSTOM)}, useDefaultFilters = false)
+    @ComponentScan(basePackageClasses = {NetworkACLManagerImpl.class}, includeFilters = {
+            @ComponentScan.Filter(value = NetworkACLTestConfiguration.Library.class, type = FilterType.CUSTOM)}, useDefaultFilters = false)
     public static class NetworkACLTestConfiguration extends SpringUtils.CloudStackTestConfiguration {
 
         @Bean
diff --git a/server/src/test/java/com/cloud/vpc/NetworkACLServiceTest.java b/server/src/test/java/com/cloud/vpc/NetworkACLServiceTest.java
index 1909a4f500b..35a94a2914b 100644
--- a/server/src/test/java/com/cloud/vpc/NetworkACLServiceTest.java
+++ b/server/src/test/java/com/cloud/vpc/NetworkACLServiceTest.java
@@ -20,14 +20,9 @@
 
 import javax.inject.Inject;
 
-import com.cloud.user.User;
-import junit.framework.TestCase;
-
-import org.apache.cloudstack.api.command.user.network.CreateNetworkACLCmd;
 import org.apache.cloudstack.context.CallContext;
 import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
 import org.apache.cloudstack.test.utils.SpringUtils;
-import org.apache.log4j.Logger;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -48,7 +43,6 @@
 import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.network.NetworkModel;
 import com.cloud.network.dao.NetworkDao;
-import com.cloud.network.vpc.NetworkACLItem;
 import com.cloud.network.vpc.NetworkACLItemDao;
 import com.cloud.network.vpc.NetworkACLItemVO;
 import com.cloud.network.vpc.NetworkACLManager;
@@ -66,39 +60,31 @@
 import com.cloud.user.Account;
 import com.cloud.user.AccountManager;
 import com.cloud.user.AccountVO;
+import com.cloud.user.User;
 import com.cloud.user.UserVO;
 import com.cloud.utils.component.ComponentContext;
 import com.cloud.utils.db.EntityManager;
 
+import junit.framework.TestCase;
+
 @RunWith(SpringJUnit4ClassRunner.class)
 @ContextConfiguration(loader = AnnotationConfigContextLoader.class)
 public class NetworkACLServiceTest extends TestCase {
-    @Inject
-    NetworkACLService _aclService;
 
     @Inject
-    AccountManager _accountMgr;
-    @Inject
-    VpcManager _vpcMgr;
-    @Inject
-    NetworkACLManager _networkAclMgr;
+    private NetworkACLService _aclService;
     @Inject
-    NetworkACLDao _networkACLDao;
+    private NetworkACLManager _networkAclMgr;
     @Inject
-    NetworkACLItemDao _networkACLItemDao;
+    private NetworkACLDao _networkACLDao;
     @Inject
-    EntityManager _entityMgr;
+    private NetworkACLItemDao _networkACLItemDao;
     @Inject
-    VpcDao _vpcDao;
-    @Inject
-    VpcService _vpcSrv;
+    private EntityManager _entityMgr;
 
-    private CreateNetworkACLCmd createACLItemCmd;
     private NetworkACLVO acl;
     private NetworkACLItemVO aclItem;
 
-    private static final Logger s_logger = Logger.getLogger(NetworkACLServiceTest.class);
-
     @Override
     @Before
     public void setUp() {
@@ -107,43 +93,6 @@ public void setUp() {
         UserVO user = new UserVO(1, "testuser", "password", "firstname", "lastName", "email", "timezone", UUID.randomUUID().toString(), User.Source.UNKNOWN);
 
         CallContext.register(user, account);
-
-        createACLItemCmd = new CreateNetworkACLCmd() {
-            @Override
-            public Long getACLId() {
-                return 3L;
-            }
-
-            @Override
-            public Integer getNumber() {
-                return 1;
-            }
-
-            @Override
-            public String getProtocol() {
-                return "TCP";
-            }
-        };
-
-        acl = new NetworkACLVO() {
-            @Override
-            public Long getVpcId() {
-                return 1L;
-            }
-
-            @Override
-            public long getId() {
-                return 1L;
-            }
-
-        };
-
-        aclItem = new NetworkACLItemVO() {
-            @Override
-            public long getAclId() {
-                return 4L;
-            }
-        };
     }
 
     @Override
@@ -152,13 +101,6 @@ public void tearDown() {
         CallContext.unregister();
     }
 
-    @Test
-    public void testCreateACL() throws Exception {
-        Mockito.when(_entityMgr.findById(Matchers.eq(Vpc.class), Matchers.anyLong())).thenReturn(new VpcVO());
-        Mockito.when(_networkAclMgr.createNetworkACL("acl_new", "acl desc", 1L, true)).thenReturn(acl);
-        assertNotNull(_aclService.createNetworkACL("acl_new", "acl desc", 1L, true));
-    }
-
     @Test(expected = InvalidParameterValueException.class)
     public void testDeleteDefaultACL() throws Exception {
         Mockito.when(_networkACLDao.findById(Matchers.anyLong())).thenReturn(acl);
@@ -166,26 +108,6 @@ public void testDeleteDefaultACL() throws Exception {
         _aclService.deleteNetworkACL(1L);
     }
 
-    @Test
-    public void testCreateACLItem() throws Exception {
-        Mockito.when(_entityMgr.findById(Matchers.eq(Vpc.class), Matchers.anyLong())).thenReturn(new VpcVO());
-        Mockito.when(_networkAclMgr.getNetworkACL(Matchers.anyLong())).thenReturn(acl);
-        Mockito.when(
-            _networkAclMgr.createNetworkACLItem(Matchers.anyInt(), Matchers.anyInt(), Matchers.anyString(), Matchers.anyList(), Matchers.anyInt(), Matchers.anyInt(),
-                        Matchers.any(NetworkACLItem.TrafficType.class), Matchers.anyLong(), Matchers.anyString(), Matchers.anyInt(), Matchers.anyBoolean())).thenReturn(
-                new NetworkACLItemVO());
-        Mockito.when(_networkACLItemDao.findByAclAndNumber(Matchers.anyLong(), Matchers.anyInt())).thenReturn(null);
-        assertNotNull(_aclService.createNetworkACLItem(createACLItemCmd));
-    }
-
-    @Test(expected = InvalidParameterValueException.class)
-    public void testCreateACLItemDuplicateNumber() throws Exception {
-        Mockito.when(_entityMgr.findById(Matchers.eq(Vpc.class), Matchers.anyLong())).thenReturn(new VpcVO());
-        Mockito.when(_networkAclMgr.getNetworkACL(Matchers.anyLong())).thenReturn(acl);
-        Mockito.when(_networkACLItemDao.findByAclAndNumber(Matchers.anyLong(), Matchers.anyInt())).thenReturn(new NetworkACLItemVO());
-        _aclService.createNetworkACLItem(createACLItemCmd);
-    }
-
     @Test
     public void testDeleteACLItem() throws Exception {
         Mockito.when(_networkACLItemDao.findById(Matchers.anyLong())).thenReturn(aclItem);
@@ -196,8 +118,8 @@ public void testDeleteACLItem() throws Exception {
     }
 
     @Configuration
-    @ComponentScan(basePackageClasses = {NetworkACLServiceImpl.class}, includeFilters = {@ComponentScan.Filter(value = NetworkACLTestConfiguration.Library.class,
-                                                                                                               type = FilterType.CUSTOM)}, useDefaultFilters = false)
+    @ComponentScan(basePackageClasses = {NetworkACLServiceImpl.class}, includeFilters = {
+            @ComponentScan.Filter(value = NetworkACLTestConfiguration.Library.class, type = FilterType.CUSTOM)}, useDefaultFilters = false)
     public static class NetworkACLTestConfiguration extends SpringUtils.CloudStackTestConfiguration {
 
         @Bean
diff --git a/ui/css/cloudstack3.css b/ui/css/cloudstack3.css
index 9b4e7638001..2acabdbfc79 100644
--- a/ui/css/cloudstack3.css
+++ b/ui/css/cloudstack3.css
@@ -8453,11 +8453,32 @@ div#details-tab-aclRules div.multi-edit table tr td.protocolnumber {
     max-width: 60px !important;
 }
 
+div#details-tab-aclRules div.multi-edit table tr th.traffictype,
+div#details-tab-aclRules div.multi-edit table tr td.traffictype {
+    width: 60px !important;
+    min-width: 60px !important;
+    max-width: 60px !important;
+}
+
+div#details-tab-aclRules div.multi-edit table tr th.reason,
+div#details-tab-aclRules div.multi-edit table tr td.reason {
+    width: 60px !important;
+    min-width: 60px !important;
+    max-width: 60px !important;
+}
+
+div#details-tab-aclRules div.multi-edit table tr th.icmptype, div#details-tab-aclRules div.multi-edit table tr td.icmptype,
+div#details-tab-aclRules div.multi-edit table tr th.icmpcode, div#details-tab-aclRules div.multi-edit table tr td.icmpcode {
+    width: 60px !important;
+    min-width: 60px !important;
+    max-width: 60px !important;
+}
+
 div#details-tab-aclRules div.multi-edit table tr th.startport, div#details-tab-aclRules div.multi-edit table tr td.startport,
 div#details-tab-aclRules div.multi-edit table tr th.endport, div#details-tab-aclRules div.multi-edit table tr td.endport {
-    width: 70px !important;
-    min-width: 70px !important;
-    max-width: 70px !important;
+    width: 60px !important;
+    min-width: 60px !important;
+    max-width: 60px !important;
 }
 
 div#details-tab-aclRules td.cidrlist span {
diff --git a/ui/l10n/ar.js b/ui/l10n/ar.js
index 9c10afcea00..f6927437017 100644
--- a/ui/l10n/ar.js
+++ b/ui/l10n/ar.js
@@ -104,6 +104,8 @@ var dictionary = {
     "label.acl.list.rules": "ACL List Rules",
     "label.acl.name": "ACL Name",
     "label.acl.replaced": "ACL replaced",
+    "label.acl.reason": "Reason",
+    "label.acl.reason.description": "Enter the reason behind an ACL rule.",
     "label.acquire.new.ip": "Acquire New IP",
     "label.acquire.new.secondary.ip": "Acquire new secondary IP",
     "label.action": "Action",
diff --git a/ui/l10n/ca.js b/ui/l10n/ca.js
index fe85a1938be..276b3e51bfc 100644
--- a/ui/l10n/ca.js
+++ b/ui/l10n/ca.js
@@ -104,6 +104,8 @@ var dictionary = {
     "label.acl.list.rules": "ACL List Rules",
     "label.acl.name": "ACL Name",
     "label.acl.replaced": "ACL replaced",
+    "label.acl.reason": "Reason",
+    "label.acl.reason.description": "Enter the reason behind an ACL rule.",
     "label.acquire.new.ip": "Acquire New IP",
     "label.acquire.new.secondary.ip": "Acquire new secondary IP",
     "label.action": "Action",
diff --git a/ui/l10n/de_DE.js b/ui/l10n/de_DE.js
index 62e00560a69..76374f63459 100644
--- a/ui/l10n/de_DE.js
+++ b/ui/l10n/de_DE.js
@@ -104,6 +104,8 @@ var dictionary = {
     "label.acl.list.rules": "ACL-Listenregeln",
     "label.acl.name": "ACL-Name",
     "label.acl.replaced": "ACL ersetzt",
+    "label.acl.reason": "Reason",
+    "label.acl.reason.description": "Enter the reason behind an ACL rule.",
     "label.acquire.new.ip": "Neue IP erwerben",
     "label.acquire.new.secondary.ip": "Neue sekundäre IP erwerben",
     "label.action": "Aktion",
diff --git a/ui/l10n/en.js b/ui/l10n/en.js
index ef7c91e290a..6d994303f1a 100644
--- a/ui/l10n/en.js
+++ b/ui/l10n/en.js
@@ -105,6 +105,8 @@ var dictionary = {
 "label.acl.list.rules":"ACL List Rules",
 "label.acl.name":"ACL Name",
 "label.acl.replaced":"ACL replaced",
+"label.acl.reason": "Reason",
+"label.acl.reason.description": "Enter the reason behind an ACL rule.",
 "label.acquire.new.ip":"Acquire New IP",
 "label.acquire.new.secondary.ip":"Acquire new secondary IP",
 "label.action":"Action",
diff --git a/ui/l10n/es.js b/ui/l10n/es.js
index 5a0982bee27..37f8425ac05 100644
--- a/ui/l10n/es.js
+++ b/ui/l10n/es.js
@@ -104,6 +104,8 @@ var dictionary = {
     "label.acl.list.rules": "Lista de Reglas ACL",
     "label.acl.name": "Nombre de ACL",
     "label.acl.replaced": "ACL reemplazada",
+    "label.acl.reason": "Reason",
+    "label.acl.reason.description": "Enter the reason behind an ACL rule.",
     "label.acquire.new.ip": "Adquirir nueva IP",
     "label.acquire.new.secondary.ip": "Adquirir nueva IP secundaria",
     "label.action": "Acción",
diff --git a/ui/l10n/fr_FR.js b/ui/l10n/fr_FR.js
index 0f81e9ccbee..2d03b64dcaa 100644
--- a/ui/l10n/fr_FR.js
+++ b/ui/l10n/fr_FR.js
@@ -104,6 +104,8 @@ var dictionary = {
     "label.acl.list.rules": "Liste règles ACL",
     "label.acl.name": "Nom ACL",
     "label.acl.replaced": "ACL remplacée",
+    "label.acl.reason": "Reason",
+    "label.acl.reason.description": "Enter the reason behind an ACL rule.",
     "label.acquire.new.ip": "Acquérir nouvelle adr. IP",
     "label.acquire.new.secondary.ip": "Acquérir nouvelle IP secondaire",
     "label.action": "Action",
diff --git a/ui/l10n/hu.js b/ui/l10n/hu.js
index 105b3ce845b..f4d20e5be1c 100644
--- a/ui/l10n/hu.js
+++ b/ui/l10n/hu.js
@@ -104,6 +104,8 @@ var dictionary = {
     "label.acl.list.rules": "ACL List Rules",
     "label.acl.name": "ACL név",
     "label.acl.replaced": "ACL lehelyettesítve",
+    "label.acl.reason": "Reason",
+    "label.acl.reason.description": "Enter the reason behind an ACL rule.",
     "label.acquire.new.ip": "Új IP cím beszerzése",
     "label.acquire.new.secondary.ip": "Új másodlagos IP cím beszerzése",
     "label.action": "Művelet",
diff --git a/ui/l10n/it_IT.js b/ui/l10n/it_IT.js
index 0e7e17a4db9..bcc3a0fa661 100644
--- a/ui/l10n/it_IT.js
+++ b/ui/l10n/it_IT.js
@@ -104,6 +104,8 @@ var dictionary = {
     "label.acl.list.rules": "ACL List Rules",
     "label.acl.name": "ACL Name",
     "label.acl.replaced": "ACL replaced",
+    "label.acl.reason": "Reason",
+    "label.acl.reason.description": "Enter the reason behind an ACL rule.",
     "label.acquire.new.ip": "Acquisizione nuovo indirizzo IP",
     "label.acquire.new.secondary.ip": "Acquisizione nuovo IP secondario",
     "label.action": "Action",
diff --git a/ui/l10n/ja_JP.js b/ui/l10n/ja_JP.js
index c0e8abdcc0a..efa937a4856 100644
--- a/ui/l10n/ja_JP.js
+++ b/ui/l10n/ja_JP.js
@@ -104,6 +104,8 @@ var dictionary = {
     "label.acl.list.rules": "ACL ルールのリスト",
     "label.acl.name": "ACL 名",
     "label.acl.replaced": "ACL が置き換えられました",
+    "label.acl.reason": "Reason",
+    "label.acl.reason.description": "Enter the reason behind an ACL rule.",
     "label.acquire.new.ip": "新しい IP アドレスの取得",
     "label.acquire.new.secondary.ip": "セカンダリ IP アドレスの取得",
     "label.action": "操作",
diff --git a/ui/l10n/ko_KR.js b/ui/l10n/ko_KR.js
index d8ffb72ef24..de2d6ba9c98 100644
--- a/ui/l10n/ko_KR.js
+++ b/ui/l10n/ko_KR.js
@@ -104,6 +104,8 @@ var dictionary = {
     "label.acl.list.rules": "ACL List Rules",
     "label.acl.name": "ACL Name",
     "label.acl.replaced": "ACL replaced",
+    "label.acl.reason": "Reason",
+    "label.acl.reason.description": "Enter the reason behind an ACL rule.",
     "label.acquire.new.ip": "새로운 IP 주소 취득",
     "label.acquire.new.secondary.ip": "새로운 두번째 IP 주소 취득",
     "label.action": "Action",
diff --git a/ui/l10n/nb_NO.js b/ui/l10n/nb_NO.js
index 0725037de7e..57fda63e7e1 100644
--- a/ui/l10n/nb_NO.js
+++ b/ui/l10n/nb_NO.js
@@ -104,6 +104,8 @@ var dictionary = {
     "label.acl.list.rules": "ACL Liste Regler",
     "label.acl.name": "ACL Navn",
     "label.acl.replaced": "ACL erstattet",
+    "label.acl.reason": "Reason",
+    "label.acl.reason.description": "Enter the reason behind an ACL rule.",
     "label.acquire.new.ip": "Tilegne ny IP",
     "label.acquire.new.secondary.ip": "Tilegne ny sekundær IP",
     "label.action": "Handling",
diff --git a/ui/l10n/nl_NL.js b/ui/l10n/nl_NL.js
index 9bba48c3bf0..4bf253dd074 100644
--- a/ui/l10n/nl_NL.js
+++ b/ui/l10n/nl_NL.js
@@ -104,6 +104,8 @@ var dictionary = {
     "label.acl.list.rules": "ACL lijst regels",
     "label.acl.name": "ACL naam",
     "label.acl.replaced": "ACL vervangen",
+    "label.acl.reason": "Reason",
+    "label.acl.reason.description": "Enter the reason behind an ACL rule.",
     "label.acquire.new.ip": "Bemachtig nieuw IP",
     "label.acquire.new.secondary.ip": "Verkrijg nieuw secundair IP",
     "label.action": "Actie",
diff --git a/ui/l10n/pl.js b/ui/l10n/pl.js
index b829dc3a9b5..7b7a89b316a 100644
--- a/ui/l10n/pl.js
+++ b/ui/l10n/pl.js
@@ -104,6 +104,8 @@ var dictionary = {
     "label.acl.list.rules": "ACL List Rules",
     "label.acl.name": "ACL Name",
     "label.acl.replaced": "ACL replaced",
+    "label.acl.reason": "Reason",
+    "label.acl.reason.description": "Enter the reason behind an ACL rule.",
     "label.acquire.new.ip": "Acquire New IP",
     "label.acquire.new.secondary.ip": "Acquire new secondary IP",
     "label.action": "Action",
diff --git a/ui/l10n/pt_BR.js b/ui/l10n/pt_BR.js
index 590dd794b76..45de2067fa7 100644
--- a/ui/l10n/pt_BR.js
+++ b/ui/l10n/pt_BR.js
@@ -104,6 +104,8 @@ var dictionary = {
     "label.acl.list.rules": "Lista de regas de ACL",
     "label.acl.name": "Nome da ACL",
     "label.acl.replaced": "ACL trocado",
+    "label.acl.reason": "Motivo",
+    "label.acl.reason.description": "Motivo para se utilizar a regra.",
     "label.acquire.new.ip": "Adquirir novo IP",
     "label.acquire.new.secondary.ip": "Adquira um novo IP secundário",
     "label.action": "Ação",
diff --git a/ui/l10n/ru_RU.js b/ui/l10n/ru_RU.js
index d676df59d4a..0321f710cee 100644
--- a/ui/l10n/ru_RU.js
+++ b/ui/l10n/ru_RU.js
@@ -104,6 +104,8 @@ var dictionary = {
     "label.acl.list.rules": "ACL List Rules",
     "label.acl.name": "ACL Name",
     "label.acl.replaced": "ACL replaced",
+    "label.acl.reason": "Reason",
+    "label.acl.reason.description": "Enter the reason behind an ACL rule.",
     "label.acquire.new.ip": "Получить новый IP",
     "label.acquire.new.secondary.ip": "Запросить дополнительный IP-адрес",
     "label.action": "Действия",
diff --git a/ui/l10n/zh_CN.js b/ui/l10n/zh_CN.js
index 4d0762d9d96..1d617d6f630 100644
--- a/ui/l10n/zh_CN.js
+++ b/ui/l10n/zh_CN.js
@@ -104,6 +104,8 @@ var dictionary = {
     "label.acl.list.rules": "ACL列表策略",
     "label.acl.name": "ACL 名称",
     "label.acl.replaced": "ACL 已替换",
+    "label.acl.reason": "Reason",
+    "label.acl.reason.description": "Enter the reason behind an ACL rule.",
     "label.acquire.new.ip": "获取新 IP",
     "label.acquire.new.secondary.ip": "获取新二级 IP",
     "label.action": "操作",
diff --git a/ui/scripts/vpc.js b/ui/scripts/vpc.js
index 9dc8323e7cd..e5cb8bb9f88 100644
--- a/ui/scripts/vpc.js
+++ b/ui/scripts/vpc.js
@@ -429,7 +429,14 @@
                         }]
                     });
                 }
-            }
+            },
+            'reason': {
+                edit: true,
+                label: 'label.acl.reason',
+                desc: 'label.acl.reason.description',
+                isEditable: true,
+                isTextarea: true
+           }
     };
     
     var aclRuleFieldsForMultiEdit = {
@@ -594,7 +601,8 @@
                         number: args.data.number,
                         protocol: args.data.protocol,
                         traffictype: args.data.traffictype,
-                        action: args.data.action
+                        action: args.data.action,
+                        reason: args.data.reason
                     };
 
                     if (data.protocol === 'tcp' || data.protocol === 'udp') {
@@ -620,6 +628,7 @@
                     $.ajax({
                         url: createURL('updateNetworkACLItem'),
                         data: data,
+                        type: "POST",
                         success: function(json) {
                             args.response.success({
                                 _custom: {


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


> Add Text-Field to each ACL Rule 
> --------------------------------
>
>                 Key: CLOUDSTACK-10314
>                 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-10314
>             Project: CloudStack
>          Issue Type: Improvement
>      Security Level: Public(Anyone can view this level - this is the default.) 
>            Reporter: Rafael Weingärtner
>            Assignee: Rafael Weingärtner
>            Priority: Minor
>
> It might be interesting to have a text field (e.g. CHAR-256) added to each ACL rule, which allows to enter a "reason" for each FW Rule created. This is valuable for customer documentation, as well as best practice for an evidence towards auditing the system.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message