[CLOUDSTACK-10314] Add Text-Field to each ACL Rule (#2475)

* [CLOUDSTACK-10314] Add Text-Field to each ACL Rule

It is 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

* Formatting to make check style happy and code clean ups

api/src/main/java/com/cloud/network/vpc/NetworkACLItem.java

@@ -24,6 +24,7 @@ import org.apache.cloudstack.api.InternalIdentity;
 
 public interface NetworkACLItem extends InternalIdentity, Identity, Displayable {
 
+    @Override
     String getUuid();
 
     Action getAction();
@@ -51,7 +52,7 @@ public interface NetworkACLItem extends InternalIdentity, Identity, Displayable
     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 @@ public interface NetworkACLItem extends InternalIdentity, Identity, Displayable
 
     List<String> getSourceCidrList();
 
-    /**
-     * @return
-     */
     TrafficType getTrafficType();
 
     @Override
     boolean isDisplay();
 
+    String getReason();
 }

api/src/main/java/com/cloud/network/vpc/NetworkACLService.java

@@ -21,6 +21,7 @@ import java.util.List;
 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 @@ import com.cloud.utils.Pair;
 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,
+    NetworkACLItem updateNetworkACLItem(UpdateNetworkACLItemCmd updateNetworkACLItemCmd) throws ResourceUnavailableException;
-            Integer sourcePortStart, Integer sourcePortEnd, Integer icmpCode, Integer icmpType, String newUUID, Boolean forDisplay) throws ResourceUnavailableException;
 
     /**
      * Associates ACL with specified Network
-     * @param aclId
-     * @param privateGatewayId
-     * @return
-     * @throws ResourceUnavailableException
      */
     boolean replaceNetworkACLonPrivateGw(long aclId, long privateGatewayId) throws ResourceUnavailableException;
 

api/src/main/java/org/apache/cloudstack/api/ApiConstants.java

@@ -452,6 +452,7 @@ public class ApiConstants {
     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";

api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkACLCmd.java

@@ -40,11 +40,7 @@ import com.cloud.network.vpc.NetworkACLItem;
 import com.cloud.user.Account;
 import com.cloud.utils.net.NetUtils;
 
-@APICommand(name = "createNetworkACL",
+@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)
-            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 @@ public class CreateNetworkACLCmd extends BaseAsyncCreateCmd {
     // ////////////// API parameters /////////////////////
     // ///////////////////////////////////////////////////
 
-    @Parameter(name = ApiConstants.PROTOCOL,
+    @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")
-               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 @@ public class CreateNetworkACLCmd extends BaseAsyncCreateCmd {
     @Parameter(name = ApiConstants.ICMP_CODE, type = CommandType.INTEGER, description = "error code for this ICMP message")
     private Integer icmpCode;
 
-    @Parameter(name = ApiConstants.NETWORK_ID,
+    @Parameter(name = ApiConstants.NETWORK_ID, type = CommandType.UUID, entityType = NetworkResponse.class, description = "The network of the VM the ACL will be created for")
-               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,
+    @Parameter(name = ApiConstants.ACL_ID, type = CommandType.UUID, entityType = NetworkACLResponse.class, description = "The network of the VM the ACL will be created for")
-               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,"
+    @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")
-        + "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 @@ public class CreateNetworkACLCmd extends BaseAsyncCreateCmd {
     @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 class CreateNetworkACLCmd extends BaseAsyncCreateCmd {
         return aclId;
     }
 
+    public String getReason() {
+        return reason;
+    }
+
     @Override
     public void create() {
         NetworkACLItem result = _networkACLService.createNetworkACLItem(this);
@@ -257,5 +247,4 @@ public class CreateNetworkACLCmd extends BaseAsyncCreateCmd {
             }
         }
     }
-
+}
-}

api/src/main/java/org/apache/cloudstack/api/command/user/network/UpdateNetworkACLItemCmd.java

@@ -21,10 +21,8 @@ import java.util.List;
 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.exception.ResourceUnavailableException;
 import com.cloud.network.vpc.NetworkACLItem;
 import com.cloud.user.Account;
 
-@APICommand(name = "updateNetworkACLItem", description = "Updates ACL item with specified ID", responseObject = NetworkACLItemResponse.class,
+@APICommand(name = "updateNetworkACLItem", description = "Updates ACL item with specified ID", responseObject = NetworkACLItemResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
-        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
 public class UpdateNetworkACLItemCmd extends BaseAsyncCustomIdCmd {
     public static final Logger s_logger = Logger.getLogger(UpdateNetworkACLItemCmd.class.getName());
 
@@ -45,16 +42,10 @@ public class UpdateNetworkACLItemCmd extends BaseAsyncCustomIdCmd {
     // ////////////// API parameters /////////////////////
     // ///////////////////////////////////////////////////
 
-    @Parameter(name = ApiConstants.ID,
+    @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = NetworkACLItemResponse.class, required = true, description = "the ID of the network ACL item")
-               type = CommandType.UUID,
-               entityType = NetworkACLItemResponse.class,
-               required = true,
-               description = "the ID of the network ACL item")
     private Long id;
 
-    @Parameter(name = ApiConstants.PROTOCOL,
+    @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")
-               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 @@ public class UpdateNetworkACLItemCmd extends BaseAsyncCustomIdCmd {
     @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,"
+    @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")
-        + "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 @@ public class UpdateNetworkACLItemCmd extends BaseAsyncCustomIdCmd {
     @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 class UpdateNetworkACLItemCmd extends BaseAsyncCustomIdCmd {
     public String getProtocol() {
         if (protocol != null) {
             return protocol.trim();
-        } else
+        } else {
             return null;
+        }
     }
 
     public List<String> getSourceCidrList() {
@@ -173,15 +168,14 @@ public class UpdateNetworkACLItemCmd extends BaseAsyncCustomIdCmd {
         return icmpType;
     }
 
+    public String getReason() {
+        return reason;
+    }
+
     @Override
     public void execute() throws ResourceUnavailableException {
         CallContext.current().setEventDetails("Rule Id: " + getId());
-        NetworkACLItem aclItem =
+        NetworkACLItem aclItem = _networkACLService.updateNetworkACLItem(this);
-            _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");
-        }
         NetworkACLItemResponse aclResponse = _responseGenerator.createNetworkACLItemResponse(aclItem);
         setResponseObject(aclResponse);
         aclResponse.setResponseName(getCommandName());

api/src/main/java/org/apache/cloudstack/api/response/NetworkACLItemResponse.java

@@ -85,6 +85,10 @@ public class NetworkACLItemResponse extends BaseResponse {
     @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 class NetworkACLItemResponse extends BaseResponse {
     public void setForDisplay(Boolean forDisplay) {
         this.forDisplay = forDisplay;
     }
+
+    public void setReason(String reason) {
+        this.reason = reason;
+    }
+
+    public String getReason() {
+        return reason;
+    }
 }

engine/components-api/src/main/java/com/cloud/network/vpc/NetworkACLManager.java

@@ -25,128 +25,67 @@ public interface NetworkACLManager {
 
     /**
      * 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
+     * Associates ACL with a network and applies the ACLItems
-     * @param acl
-     * @param network
-     * @return
      */
     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 createNetworkACLItem(NetworkACLItemVO networkACLItemVO);
-        NetworkACLItem.TrafficType trafficType, Long aclId, String action, Integer number, Boolean forDisplay);
 
     /**
      * 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
+     * Applies associated ACL to specified network
-     * @param networkId
-     * @return
-     * @throws ResourceUnavailableException
      */
     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,
+    NetworkACLItem updateNetworkACLItem(NetworkACLItemVO networkACLItemVO) throws ResourceUnavailableException;
-        Integer sourcePortStart, Integer sourcePortEnd, Integer icmpCode, Integer icmpType, String customId, Boolean forDisplay) throws ResourceUnavailableException;
 
     /**
-     * Associates acl with a network and applies the ACLItems
+     * Associates ACL with a network and applies the ACLItems
-     * @param acl
-     * @param gateway
-     * @return
      */
-
     boolean replaceNetworkACLForPrivateGw(NetworkACL acl, PrivateGateway gateway) throws ResourceUnavailableException;
 
     boolean revokeACLItemsForPrivateGw(PrivateGateway gateway) throws ResourceUnavailableException;

engine/schema/src/main/java/com/cloud/network/vpc/NetworkACLItemVO.java

@@ -33,15 +33,13 @@ import javax.persistence.Table;
 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 @@ public class NetworkACLItemVO implements NetworkACLItem {
     @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,
+    public NetworkACLItemVO(Integer portStart, Integer portEnd, String protocol, long aclId, List<String> sourceCidrs, Integer icmpCode, Integer icmpType, TrafficType trafficType, Action action,
-            TrafficType trafficType, Action action, int number) {
+            int number, String reason) {
         sourcePortStart = portStart;
         sourcePortEnd = portEnd;
         this.protocol = protocol;
@@ -115,6 +116,7 @@ public class NetworkACLItemVO implements NetworkACLItem {
         this.trafficType = trafficType;
         this.action = action;
         this.number = number;
+        this.reason = reason;
     }
 
     public void setSourceCidrList(List<String> sourceCidrs) {
@@ -225,8 +227,8 @@ public class NetworkACLItemVO implements NetworkACLItem {
 
     public void setSourceCidrs(String sourceCidrs) {
         List<String> srcCidrs = new LinkedList<String>();
-        StringTokenizer st = new StringTokenizer(sourceCidrs,",;");
+        StringTokenizer st = new StringTokenizer(sourceCidrs, ",;");
-        while(st.hasMoreTokens()) {
+        while (st.hasMoreTokens()) {
             srcCidrs.add(st.nextToken());
         }
         this.sourceCidrs = srcCidrs;
@@ -252,4 +254,22 @@ public class NetworkACLItemVO implements NetworkACLItem {
     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);
+        }
+    }
 }

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
---;
+--;
+
+-- [CLOUDSTACK-10314] Add reason column to ACL rule table
+ALTER TABLE `cloud`.`network_acl_item` ADD COLUMN `reason` VARCHAR(2500) AFTER `display`;

server/src/main/java/com/cloud/api/ApiResponseHelper.java

@@ -2363,7 +2363,7 @@ public class ApiResponseHelper implements ResponseGenerator {
             CollectionUtils.addIgnoreNull(tagResponses, tagResponse);
         }
         response.setTags(tagResponses);
-
+        response.setReason(aclItem.getReason());
         response.setObjectName("networkacl");
         return response;
     }

server/src/main/java/com/cloud/network/vpc/NetworkACLManagerImpl.java

@@ -21,7 +21,11 @@ import java.util.List;
 
 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.NetworkACLItem.State;
 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.TransactionCallback;
 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;
+    private NetworkModel _networkMgr;
     @Inject
-    NetworkModel _networkMgr;
+    private NetworkACLDao _networkACLDao;
     @Inject
-    VpcManager _vpcMgr;
+    private NetworkACLItemDao _networkACLItemDao;
     @Inject
-    ResourceTagDao _resourceTagDao;
+    private NetworkModel _networkModel;
     @Inject
-    NetworkACLDao _networkACLDao;
+    private NetworkDao _networkDao;
     @Inject
-    NetworkACLItemDao _networkACLItemDao;
+    private VpcGatewayDao _vpcGatewayDao;
-    List<NetworkACLServiceProvider> _networkAclElements;
     @Inject
-    NetworkModel _networkModel;
+    private NetworkModel _ntwkModel;
     @Inject
-    NetworkDao _networkDao;
+    private EntityManager _entityMgr;
     @Inject
-    VpcGatewayDao _vpcGatewayDao;
+    private VpcService _vpcSvc;
     @Inject
-    NetworkModel _ntwkModel;
+    private MessageBus _messageBus;
-    @Inject
+
-    ConfigurationManager _configMgr;
+    private List<NetworkACLServiceProvider> _networkAclElements;
-    @Inject
-    EntityManager _entityMgr;
-    @Inject
-    VpcService _vpcSvc;
-    @Inject
-    MessageBus _messageBus;
 
     @Override
     public NetworkACL createNetworkACL(final String name, final String description, final long vpcId, final Boolean forDisplay) {
@@ -224,40 +214,21 @@ public class NetworkACLManagerImpl extends ManagerBase implements NetworkACLMana
         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,
+    public NetworkACLItem createNetworkACLItem(NetworkACLItemVO networkACLItemVO) {
-            final Integer icmpType, final NetworkACLItem.TrafficType trafficType, final Long aclId, final String action, Integer number, final Boolean forDisplay) {
+        NetworkACLItemVO newRule = Transaction.execute(new TransactionCallback<NetworkACLItemVO>() {
-        // 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>() {
             @Override
             public NetworkACLItemVO doInTransaction(final TransactionStatus status) {
-                NetworkACLItem.Action ruleAction = NetworkACLItem.Action.Allow;
+                NetworkACLItemVO networkACLItemVOFromDatabase = _networkACLItemDao.persist(networkACLItemVO);
-                if ("deny".equalsIgnoreCase(action)) {
+
-                    ruleAction = NetworkACLItem.Action.Deny;
+                if (!_networkACLItemDao.setStateToAdd(networkACLItemVOFromDatabase)) {
+                    throw new CloudRuntimeException("Unable to update the state to add for " + networkACLItemVOFromDatabase);
                 }
+                CallContext.current().setEventDetails("ACL Item Id: " + networkACLItemVOFromDatabase.getId());
 
-                NetworkACLItemVO newRule =
+                return networkACLItemVOFromDatabase;
-                        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);
-                }
-                CallContext.current().setEventDetails("ACL Item Id: " + newRule.getId());
-
-                return newRule;
             }
         });
 
@@ -265,7 +236,7 @@ public class NetworkACLManagerImpl extends ManagerBase implements NetworkACLMana
     }
 
     @Override
-    public NetworkACLItem getNetworkACLItem(final long ruleId) {
+    public NetworkACLItem getNetworkACLItem(long ruleId) {
         return _networkACLItemDao.findById(ruleId);
     }
 
@@ -372,7 +343,6 @@ public class NetworkACLManagerImpl extends ManagerBase implements NetworkACLMana
     }
 
     private void removeRule(final NetworkACLItem rule) {
-        //remove the rule
         _networkACLItemDao.remove(rule.getId());
     }
 
@@ -384,19 +354,14 @@ public class NetworkACLManagerImpl extends ManagerBase implements NetworkACLMana
     }
 
     private boolean applyACLToPrivateGw(final PrivateGateway gateway, final List<? extends NetworkACLItem> rules) throws ResourceUnavailableException {
-        List<VpcProvider> vpcElements = null;
+        List<VpcProvider> vpcElements = new ArrayList<VpcProvider>();
-        vpcElements = new ArrayList<VpcProvider>();
         vpcElements.add((VpcProvider)_ntwkModel.getElementImplementingProvider(Network.Provider.VPCVirtualRouter.getName()));
 
-        if (vpcElements == null) {
+        try {
-            throw new CloudRuntimeException("Failed to initialize vpc elements");
-        }
-
-        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 class NetworkACLManagerImpl extends ManagerBase implements NetworkACLMana
         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,
+    public NetworkACLItem updateNetworkACLItem(NetworkACLItemVO networkACLItemVO) throws ResourceUnavailableException {
-            final Integer number, final Integer sourcePortStart, final Integer sourcePortEnd, final Integer icmpCode, final Integer icmpType, final String customId, final Boolean forDisplay) throws ResourceUnavailableException {
+        networkACLItemVO.setState(State.Add);
-        final NetworkACLItemVO aclItem = _networkACLItemDao.findById(id);
-        aclItem.setState(State.Add);
 
-        if (protocol != null) {
+        if (_networkACLItemDao.update(networkACLItemVO.getId(), networkACLItemVO)) {
-            aclItem.setProtocol(protocol);
+            if (applyNetworkACL(networkACLItemVO.getAclId())) {
-        }
+                return networkACLItemVO;
-
-        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);
-        }
-
-        if (_networkACLItemDao.update(id, aclItem)) {
-            if (applyNetworkACL(aclItem.getAclId())) {
-                return aclItem;
             } 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 {

server/src/main/java/com/cloud/network/vpc/NetworkACLServiceImpl.java

@@ -27,7 +27,9 @@ import org.apache.cloudstack.api.ServerApiException;
 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.NetworkModel;
 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 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ
     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;
+    private VpcService _vpcSvc;
-    @Inject
+
-    VpcDao _vpcDao;
+    private String supportedProtocolsForAclRules = "tcp,udp,icmp,all";
-    @Inject
-    VpcService _vpcSvc;
 
     @Override
     public NetworkACL createNetworkACL(final String name, final String description, final long vpcId, final Boolean forDisplay) {
@@ -133,7 +135,7 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ
             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 class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ
             sc.setParameters("display", display);
         }
 
-        if(id != null){
+        if (id != null) {
             sc.setParameters("id", id);
         }
 
@@ -173,10 +175,8 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ
             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,
+            final Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<Long, Boolean, ListProjectResourcesCriteria>(domainId, isRecursive, null);
-                    ListProjectResourcesCriteria>(domainId, isRecursive, null);
+            _accountMgr.buildACLSearchParameters(caller, id, accountName, projectId, permittedAccounts, domainIdRecursiveListProject, listAll, false);
-            _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 class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ
         }
 
         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 class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ
         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 class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ
         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) {
+    public NetworkACLItem createNetworkACLItem(CreateNetworkACLCmd createNetworkACLCmd) {
-        final Account caller = CallContext.current().getCallingAccount();
+        Long aclId = createAclListIfNeeded(createNetworkACLCmd);
-        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) {
+        Integer sourcePortStart = createNetworkACLCmd.getSourcePortStart();
-                //Network is not associated with any ACL. Create a new ACL and add aclItem in it for backward compatibility
+        Integer sourcePortEnd = createNetworkACLCmd.getSourcePortEnd();
-                s_logger.debug("Network " + network.getId() + " is not associated with any ACL. Creating an ACL before adding acl item");
+        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();
 
-                //verify that ACLProvider is supported by network offering
+        NetworkACL acl = _networkAclMgr.getNetworkACL(aclId);
-                if (!_networkModel.areServicesSupportedByNetworkOffering(network.getNetworkOfferingId(), Network.Service.NetworkACL)) {
-                    throw new InvalidParameterValueException("Network Offering does not support NetworkACL service");
-                }
 
-                final Vpc vpc = _entityMgr.findById(Vpc.class, network.getVpcId());
+        validateNetworkAcl(acl);
-                if (vpc == null) {
+        validateAclRuleNumber(createNetworkACLCmd, acl);
-                    throw new InvalidParameterValueException("Unable to find Vpc associated with the Network");
-                }
 
-                //Create new ACL
+        NetworkACLItem.Action ruleAction = validateAndCreateNetworkAclRuleAction(action);
-                final String aclName = "VPC_" + vpc.getName() + "_Tier_" + network.getName() + "_ACL_" + network.getUuid();
+        Integer number = createNetworkACLCmd.getNumber();
-                final String description = "ACL for " + aclName;
+        if (number == null) {
-                final NetworkACL acl = _networkAclMgr.createNetworkACL(aclName, description, network.getVpcId(), aclItemCmd.getDisplay());
+            number = _networkACLItemDao.getMaxNumberByACL(aclId) + 1;
-                if (acl == null) {
+        }
-                    throw new CloudRuntimeException("Error while create ACL before adding ACL Item for network " + network.getId());
+        NetworkACLItemVO networkACLItemVO = new NetworkACLItemVO(sourcePortStart, sourcePortEnd, protocol, aclId, sourceCidrList, icmpCode, icmpType, trafficType, ruleAction, number, reason);
-                }
+        networkACLItemVO.setDisplay(createNetworkACLCmd.isDisplay());
-                s_logger.debug("Created ACL: " + aclName + " for network " + network.getId());
+
-                aclId = acl.getId();
+        validateNetworkACLItem(networkACLItemVO);
-                //Apply acl to network
+        return _networkAclMgr.createNetworkACLItem(networkACLItemVO);
-                try {
+    }
-                    if (!_networkAclMgr.replaceNetworkACL(acl, (NetworkVO)network)) {
+
-                        throw new CloudRuntimeException("Unable to apply auto created ACL to network " + network.getId());
+    /**
-                    }
+     *  We first validate the given ACL action as a string using {@link #validateNetworkAclRuleAction(String)}.
-                    s_logger.debug("Created ACL is applied to network " + network.getId());
+     *  Afterwards, we convert this ACL to an object of {@link NetworkACLItem.Action}.
-                } catch (final ResourceUnavailableException e) {
+     *  If the action as String matches the word 'deny' (ignoring case), we return an instance of {@link NetworkACLItem.Action#Deny}.
-                    throw new CloudRuntimeException("Unable to apply auto created ACL to network " + network.getId(), e);
+     *  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;
+    }
+
+    /**
+     * 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));
             }
         }
+    }
 
-        final NetworkACL acl = _networkAclMgr.getNetworkACL(aclId);
+    /**
+     * 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());
+            }
+        }
+    }
+
+    /**
+     * 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) {
+     * This methods will simply return the ACL rule list ID if it has been provided by the parameter 'createNetworkACLCmd'.
-            if (_networkACLItemDao.findByAclAndNumber(aclId, aclItemCmd.getNumber()) != null) {
+     * If no ACL rule List ID has been provided the method behave as follows:
-                throw new InvalidParameterValueException("ACL item with number " + aclItemCmd.getNumber() + " already exists in ACL: " + acl.getUuid());
+     * <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;
+    }
+
+    /**
+     * 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");
+        }
+
+        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;
+    }
+
+    /**
+     *  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);
+    }
+
+    /**
+     * 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));
+            }
+        }
+    }
+
+    /**
+     *   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));
             }
         }
 
-        validateNetworkACLItem(aclItemCmd.getSourcePortStart(), aclItemCmd.getSourcePortEnd(), aclItemCmd.getSourceCidrList(), aclItemCmd.getProtocol(),
+        Integer icmpCode = networkACLItemVO.getIcmpCode();
-                aclItemCmd.getIcmpCode(), aclItemCmd.getIcmpType(), aclItemCmd.getAction(), aclItemCmd.getNumber());
+        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");
+        }
 
-        return _networkAclMgr.createNetworkACLItem(aclItemCmd.getSourcePortStart(), aclItemCmd.getSourcePortEnd(), aclItemCmd.getProtocol(),
+        Integer sourcePortStart = networkACLItemVO.getSourcePortStart();
-                aclItemCmd.getSourceCidrList(), aclItemCmd.getIcmpCode(), aclItemCmd.getIcmpType(), aclItemCmd.getTrafficType(), aclId, aclItemCmd.getAction(),
+        Integer sourcePortEnd = networkACLItemVO.getSourcePortEnd();
-                aclItemCmd.getNumber(), aclItemCmd.getDisplay());
+        if (isIcmpProtocol && (sourcePortStart != null || sourcePortEnd != null)) {
+            throw new InvalidParameterValueException("Can't specify start/end port when protocol is ICMP");
+        }
+
+        validateIcmpTypeAndCode(networkACLItemVO);
     }
 
-    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) {
+     *  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)}.
-        if (portStart != null && !NetUtils.isValidPort(portStart)) {
+     */
-            throw new InvalidParameterValueException("publicPort is an invalid value: " + portStart);
+    protected void validateSourceCidrList(NetworkACLItemVO networkACLItemVO) {
-        }
+        List<String> sourceCidrList = networkACLItemVO.getSourceCidrList();
-        if (portEnd != null && !NetUtils.isValidPort(portEnd)) {
+        if (CollectionUtils.isNotEmpty(sourceCidrList)) {
-            throw new InvalidParameterValueException("Public port range is an invalid value: " + portEnd);
+            for (String cidr : sourceCidrList) {
-        }
-
-        // 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");
-        }
-
-        // 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'");
-        }
-
-        if (sourceCidrList != null) {
-            for (final String cidr : sourceCidrList) {
                 if (!NetUtils.isValidIp4Cidr(cidr)) {
                     throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Source cidrs formatting error " + cidr);
                 }
             }
         }
+    }
 
-        //Validate Protocol
+    /**
-        if (protocol != null) {
+     * Validates the source start and end ports for the given network ACL rule.
-            //Check if protocol is a number
+     * If both ports (start and end) are null, we do not execute validations. Otherwise, we check the following:
-            if (StringUtils.isNumeric(protocol)) {
+     * <ul>
-                final int protoNumber = Integer.parseInt(protocol);
+     *  <li> Check if start port is valid using {@link NetUtils#isValidPort(int)};
-                if (protoNumber < 0 || protoNumber > 255) {
+     *  <li> Check if end port is valid using {@link NetUtils#isValidPort(int)};
-                    throw new InvalidParameterValueException("Invalid protocol number: " + protoNumber);
+     *  <li> Check if start port is bigger than end port;
-                }
+     *  <li> Check if start and end ports were used with protocol 'all'
-            } else {
+     *  </ul>
-                //Protocol is not number
+     *  All of the above cases will generate an {@link InvalidParameterValueException}.
-                //Check for valid protocol strings
+     */
-                final String supportedProtocols = "tcp,udp,icmp,all";
+    protected void validateSourceStartAndEndPorts(NetworkACLItemVO networkACLItemVO) {
-                if (!supportedProtocols.contains(protocol.toLowerCase())) {
+        Integer sourcePortStart = networkACLItemVO.getSourcePortStart();
-                    throw new InvalidParameterValueException("Invalid protocol: " + protocol);
+        Integer sourcePortEnd = networkACLItemVO.getSourcePortEnd();
-                }
+        if (sourcePortStart == null && sourcePortEnd == null) {
-            }
+            return;
-
-            // 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");
-            }
         }
 
-        //validate icmp code and type
+        if (!NetUtils.isValidPort(sourcePortStart)) {
-        if (icmpType != null) {
+            throw new InvalidParameterValueException("Start public port is an invalid value: " + sourcePortStart);
-            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");
-                }
-            }
         }
 
-        //Check ofr valid action Allow/Deny
+        if (!NetUtils.isValidPort(sourcePortEnd)) {
-        if (action != null) {
+            throw new InvalidParameterValueException("End public port is an invalid value: " + sourcePortEnd);
-            if (!("Allow".equalsIgnoreCase(action) || "Deny".equalsIgnoreCase(action))) {
-                throw new InvalidParameterValueException("Invalid action. Allowed actions are Allow and Deny");
-            }
         }
-
+        if (sourcePortStart > sourcePortEnd) {
-        //Check for valid number
+            throw new InvalidParameterValueException(String.format("Start port can't be bigger than end port [startport=%d,endport=%d]", sourcePortStart, sourcePortEnd));
-        if (number != null && number < 1) {
+        }
-            throw new InvalidParameterValueException("Invalid number. Number cannot be < 1");
+        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 class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ
         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 class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ
         } 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 class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ
             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,
+            final Ternary<Long, Boolean, ListProjectResourcesCriteria> domainIdRecursiveListProject = new Ternary<Long, Boolean, ListProjectResourcesCriteria>(domainId, isRecursive, null);
-                    ListProjectResourcesCriteria>(domainId, isRecursive, null);
+            _accountMgr.buildACLSearchParameters(caller, id, accountName, projectId, permittedAccounts, domainIdRecursiveListProject, listAll, false);
-            _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 class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ
 
         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 class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ
     @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 class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ
         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,
+    public NetworkACLItem updateNetworkACLItem(UpdateNetworkACLItemCmd updateNetworkACLItemCmd) throws ResourceUnavailableException {
-            final Integer number, final Integer sourcePortStart, final Integer sourcePortEnd, final Integer icmpCode, final Integer icmpType, final String newUUID, final Boolean forDisplay) throws ResourceUnavailableException {
+        NetworkACLItemVO networkACLItemVo = validateNetworkAclRuleIdAndRetrieveIt(updateNetworkACLItemCmd);
-        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) {
+        NetworkACL acl = _networkAclMgr.getNetworkACL(networkACLItemVo.getAclId());
-            throw new InvalidParameterValueException("Default ACL Items cannot be updated");
+        validateNetworkAcl(acl);
-        }
 
-        final NetworkACL acl = _networkAclMgr.getNetworkACL(aclItem.getAclId());
+        transferDataToNetworkAclRulePojo(updateNetworkACLItemCmd, networkACLItemVo, acl);
-
+        validateNetworkACLItem(networkACLItemVo);
-        final Vpc vpc = _entityMgr.findById(Vpc.class, acl.getVpcId());
+        return _networkAclMgr.updateNetworkACLItem(networkACLItemVo);
-
+    }
-        final Account caller = CallContext.current().getCallingAccount();
-
-        _accountMgr.checkAccess(caller, null, true, vpc);
 
+    /**
+     *  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
+            NetworkACLItemVO aclNumber = _networkACLItemDao.findByAclAndNumber(acl.getId(), number);
-            final NetworkACLItemVO aclNumber = _networkACLItemDao.findByAclAndNumber(acl.getId(), number);
+            if (aclNumber != null && aclNumber.getId() != networkACLItemVo.getId()) {
-            if (aclNumber != null && aclNumber.getId() != id) {
                 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()
+        Integer sourcePortStart = updateNetworkACLItemCmd.getSourcePortStart();
-                : sourcePortEnd, sourceCidrList, protocol, icmpCode, icmpType == null ? aclItem.getIcmpType() : icmpType, action, number);
+        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

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());
+    }
+}

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");
+    }
+
+}

server/src/test/java/com/cloud/vpc/NetworkACLManagerTest.java

@@ -22,8 +22,6 @@ import java.util.UUID;
 
 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.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 NetworkACLManagerTest extends TestCase {
@@ -135,8 +135,7 @@ public class NetworkACLManagerTest extends TestCase {
     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)))
+        Mockito.when(_networkModel.isProviderSupportServiceInNetwork(Matchers.anyLong(), Matchers.any(Network.Service.class), Matchers.any(Network.Provider.class))).thenReturn(true);
-        .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 class NetworkACLManagerTest extends TestCase {
         final NetworkVO network = Mockito.mock(NetworkVO.class);
         final List<NetworkVO> networks = new ArrayList<NetworkVO>();
         networks.add(network);
-        Mockito.when(_networkDao.listByAclId(Matchers.anyLong()))
+        Mockito.when(_networkDao.listByAclId(Matchers.anyLong())).thenReturn(networks);
-        .thenReturn(networks);
         Mockito.when(_networkDao.findById(Matchers.anyLong())).thenReturn(network);
-        Mockito.when(_networkModel.isProviderSupportServiceInNetwork(Matchers.anyLong(),
+        Mockito.when(_networkModel.isProviderSupportServiceInNetwork(Matchers.anyLong(), Matchers.any(Network.Service.class), Matchers.any(Network.Provider.class))).thenReturn(true);
-                Matchers.any(Network.Service.class), Matchers.any(Network.Provider.class)))
+        Mockito.when(_networkAclElements.get(0).applyNetworkACLs(Matchers.any(Network.class), Matchers.anyList())).thenReturn(applyNetworkACLs);
-                .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 class NetworkACLManagerTest extends TestCase {
         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))
+        Mockito.when(_vpcGatewayDao.listByAclIdAndType(aclId, VpcGateway.Type.Private)).thenReturn(vpcGateways);
-        .thenReturn(vpcGateways);
 
         // Create 4 rules to test all 4 scenarios: only revoke should
         // be deleted, only add should update
@@ -203,8 +197,7 @@ public class NetworkACLManagerTest extends TestCase {
         Mockito.when(rule2Add.getId()).thenReturn(addId);
         Mockito.when(_networkACLItemDao.findById(addId)).thenReturn(rule2Add);
 
-        Mockito.when(_networkACLItemDao.listByACL(aclId))
+        Mockito.when(_networkACLItemDao.listByACL(aclId)).thenReturn(rules);
-        .thenReturn(rules);
         // Mock methods to avoid
         Mockito.doReturn(applyACLToPrivateGw).when(aclManager).applyACLToPrivateGw(privateGateway);
 
@@ -218,20 +211,12 @@ public class NetworkACLManagerTest extends TestCase {
         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 class NetworkACLManagerTest extends TestCase {
     }
 
     @Configuration
-    @ComponentScan(basePackageClasses = {NetworkACLManagerImpl.class}, includeFilters = {@ComponentScan.Filter(value = NetworkACLTestConfiguration.Library.class,
+    @ComponentScan(basePackageClasses = {NetworkACLManagerImpl.class}, includeFilters = {
-    type = FilterType.CUSTOM)}, useDefaultFilters = false)
+            @ComponentScan.Filter(value = NetworkACLTestConfiguration.Library.class, type = FilterType.CUSTOM)}, useDefaultFilters = false)
     public static class NetworkACLTestConfiguration extends SpringUtils.CloudStackTestConfiguration {
 
         @Bean

server/src/test/java/com/cloud/vpc/NetworkACLServiceTest.java

@@ -20,14 +20,9 @@ import java.util.UUID;
 
 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 org.springframework.test.context.support.AnnotationConfigContextLoader;
 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.tags.dao.ResourceTagDao;
 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;
+    private NetworkACLService _aclService;
     @Inject
-    VpcManager _vpcMgr;
+    private NetworkACLManager _networkAclMgr;
     @Inject
-    NetworkACLManager _networkAclMgr;
+    private NetworkACLDao _networkACLDao;
     @Inject
-    NetworkACLDao _networkACLDao;
+    private NetworkACLItemDao _networkACLItemDao;
     @Inject
-    NetworkACLItemDao _networkACLItemDao;
+    private EntityManager _entityMgr;
-    @Inject
-    EntityManager _entityMgr;
-    @Inject
-    VpcDao _vpcDao;
-    @Inject
-    VpcService _vpcSrv;
 
-    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 class NetworkACLServiceTest extends TestCase {
         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 class NetworkACLServiceTest extends TestCase {
         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 class NetworkACLServiceTest extends TestCase {
         _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 class NetworkACLServiceTest extends TestCase {
     }
 
     @Configuration
-    @ComponentScan(basePackageClasses = {NetworkACLServiceImpl.class}, includeFilters = {@ComponentScan.Filter(value = NetworkACLTestConfiguration.Library.class,
+    @ComponentScan(basePackageClasses = {NetworkACLServiceImpl.class}, includeFilters = {
-                                                                                                               type = FilterType.CUSTOM)}, useDefaultFilters = false)
+            @ComponentScan.Filter(value = NetworkACLTestConfiguration.Library.class, type = FilterType.CUSTOM)}, useDefaultFilters = false)
     public static class NetworkACLTestConfiguration extends SpringUtils.CloudStackTestConfiguration {
 
         @Bean

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;
+    width: 60px !important;
-    min-width: 70px !important;
+    min-width: 60px !important;
-    max-width: 70px !important;
+    max-width: 60px !important;
 }
 
 div#details-tab-aclRules td.cidrlist span {

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",

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",

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",

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",

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",

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",

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",

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",

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": "操作",

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",

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",

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",

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",

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",

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": "Действия",

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": "操作",

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: {