[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

Author: rafaelweingartner

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

@@ -78,4 +78,5 @@ enum Action {
     @Override
     boolean isDisplay();
 
+    String getReason();
 }

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

@@ -21,6 +21,7 @@
 import org.apache.cloudstack.api.command.user.network.CreateNetworkACLCmd;
 import org.apache.cloudstack.api.command.user.network.ListNetworkACLListsCmd;
 import org.apache.cloudstack.api.command.user.network.ListNetworkACLsCmd;
+import org.apache.cloudstack.api.command.user.network.UpdateNetworkACLItemCmd;
 
 import com.cloud.exception.ResourceUnavailableException;
 import com.cloud.utils.Pair;
@@ -119,8 +120,7 @@ public interface NetworkACLService {
      * @return
      * @throws ResourceUnavailableException
      */
-    NetworkACLItem updateNetworkACLItem(Long id, String protocol, List<String> sourceCidrList, NetworkACLItem.TrafficType trafficType, String action, Integer number,
-            Integer sourcePortStart, Integer sourcePortEnd, Integer icmpCode, Integer icmpType, String newUUID, Boolean forDisplay) throws ResourceUnavailableException;
+    NetworkACLItem updateNetworkACLItem(UpdateNetworkACLItemCmd updateNetworkACLItemCmd) throws ResourceUnavailableException;
 
     /**
      * Associates ACL with specified Network

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

@@ -100,13 +100,12 @@ public class CreateNetworkACLCmd extends BaseAsyncCreateCmd {
     @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 +226,10 @@ public Long getACLId() {
         return aclId;
     }
 
+    public String getReason() {
+        return reason;
+    }
+
     @Override
     public void create() {
         NetworkACLItem result = _networkACLService.createNetworkACLItem(this);

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

@@ -21,10 +21,8 @@
 import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.ApiConstants;
-import org.apache.cloudstack.api.ApiErrorCode;
 import org.apache.cloudstack.api.BaseAsyncCustomIdCmd;
 import org.apache.cloudstack.api.Parameter;
-import org.apache.cloudstack.api.ServerApiException;
 import org.apache.cloudstack.api.response.NetworkACLItemResponse;
 import org.apache.cloudstack.context.CallContext;
 import org.apache.log4j.Logger;
@@ -85,6 +83,9 @@ public class UpdateNetworkACLItemCmd extends BaseAsyncCustomIdCmd {
     @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 ///////////////////////
     // ///////////////////////////////////////////////////
@@ -173,15 +174,14 @@ public Integer getIcmpType() {
         return icmpType;
     }
 
+    public String getReason() {
+        return reason;
+    }
+
     @Override
     public void execute() throws ResourceUnavailableException {
         CallContext.current().setEventDetails("Rule Id: " + getId());
-        NetworkACLItem aclItem =
-            _networkACLService.updateNetworkACLItem(getId(), getProtocol(), getSourceCidrList(), getTrafficType(), getAction(), getNumber(), getSourcePortStart(),
-                getSourcePortEnd(), getIcmpCode(), getIcmpType(), this.getCustomId(), this.isDisplay());
-        if (aclItem == null) {
-            throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to update network ACL item");
-        }
+        NetworkACLItem aclItem = _networkACLService.updateNetworkACLItem(this);
         NetworkACLItemResponse aclResponse = _responseGenerator.createNetworkACLItemResponse(aclItem);
         setResponseObject(aclResponse);
         aclResponse.setResponseName(getCommandName());

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 void setAction(String action) {
     public void setForDisplay(Boolean forDisplay) {
         this.forDisplay = forDisplay;
     }
+
+    public void setReason(String reason) {
+        this.reason = reason;
+    }
+
+    public String getReason() {
+        return reason;
+    }
 }

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

@@ -78,8 +78,7 @@ public interface NetworkACLManager {
      * @param forDisplay TODO
      * @return
      */
-    NetworkACLItem createNetworkACLItem(Integer sourcePortStart, Integer sourcePortEnd, String protocol, List<String> sourceCidrList, Integer icmpCode, Integer icmpType,
-        NetworkACLItem.TrafficType trafficType, Long aclId, String action, Integer number, Boolean forDisplay);
+    NetworkACLItem createNetworkACLItem(NetworkACLItemVO networkACLItemVO);
 
     /**
      * Returns Network ACL Item with specified Id
@@ -137,8 +136,7 @@ NetworkACLItem createNetworkACLItem(Integer sourcePortStart, Integer sourcePortE
      * @return
      * @throws ResourceUnavailableException
      */
-    NetworkACLItem updateNetworkACLItem(Long id, String protocol, List<String> sourceCidrList, NetworkACLItem.TrafficType trafficType, String action, Integer number,
-        Integer sourcePortStart, Integer sourcePortEnd, Integer icmpCode, Integer icmpType, String customId, Boolean forDisplay) throws ResourceUnavailableException;
+    NetworkACLItem updateNetworkACLItem(NetworkACLItemVO networkACLItemVO) throws ResourceUnavailableException;
 
     /**
      * Associates acl with a network and applies the ACLItems

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

@@ -33,11 +33,12 @@
 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 {
 
     /**
      *
@@ -97,12 +98,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,
-            TrafficType trafficType, Action action, int number) {
+            TrafficType trafficType, Action action, int number, String reason) {
         sourcePortStart = portStart;
         sourcePortEnd = portEnd;
         this.protocol = protocol;
@@ -115,6 +119,7 @@ public NetworkACLItemVO(Integer portStart, Integer portEnd, String protocol, lon
         this.trafficType = trafficType;
         this.action = action;
         this.number = number;
+        this.reason = reason;
     }
 
     public void setSourceCidrList(List<String> sourceCidrs) {
@@ -252,4 +257,22 @@ public void setDisplay(boolean display) {
     public boolean isDisplay() {
         return display;
     }
+
+    @Override
+    public String getReason() {
+        return reason;
+    }
+
+    public void setReason(String reason) {
+        this.reason = reason;
+    }
+
+    @Override
+    protected NetworkACLItemVO clone(){
+        try {
+            return (NetworkACLItemVO)super.clone();
+        } catch (CloneNotSupportedException e) {
+           throw new CloudRuntimeException(e);
+        }
+    }
 }

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 NetworkACLItemResponse createNetworkACLItemResponse(NetworkACLItem aclIte
             CollectionUtils.addIgnoreNull(tagResponses, tagResponse);
         }
         response.setTags(tagResponses);
-
+        response.setReason(aclItem.getReason());
         response.setObjectName("networkacl");
         return response;
     }