[CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop) (#2511)

* Add permission to 'moveNetworkAclItem' API method in default roles

Author: rafaelweingartner

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

@@ -21,13 +21,15 @@
 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.MoveNetworkAclItemCmd;
 import org.apache.cloudstack.api.command.user.network.UpdateNetworkACLItemCmd;
 import org.apache.cloudstack.api.command.user.network.UpdateNetworkACLListCmd;
 
 import com.cloud.exception.ResourceUnavailableException;
 import com.cloud.utils.Pair;
 
 public interface NetworkACLService {
+
     /**
      * Creates Network ACL for the specified VPC
      */
@@ -90,4 +92,8 @@ public interface NetworkACLService {
 
     NetworkACL updateNetworkACL(UpdateNetworkACLListCmd updateNetworkACLListCmd);
 
-}
+    /**
+     * Updates a network item ACL to a new position. This method allows users to inform between which ACLs the given ACL will be placed. Therefore, the 'number' field will be filled out by the system in the best way possible to place the ACL accordingly.
+     */
+    NetworkACLItem moveNetworkAclRuleToNewPosition(MoveNetworkAclItemCmd moveNetworkAclItemCmd);
+}

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

@@ -152,6 +152,8 @@ public class ApiConstants {
     public static final String ICMP_TYPE = "icmptype";
     public static final String ID = "id";
     public static final String IDS = "ids";
+    public static final String PREVIOUS_ACL_RULE_ID = "previousaclruleid";
+    public static final String NEXT_ACL_RULE_ID = "nextaclruleid";
     public static final String INTERNAL_DNS1 = "internaldns1";
     public static final String INTERNAL_DNS2 = "internaldns2";
     public static final String INTERVAL_TYPE = "intervaltype";

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

@@ -0,0 +1,96 @@
+// 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 org.apache.cloudstack.api.command.user.network;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseAsyncCustomIdCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.response.NetworkACLItemResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.log4j.Logger;
+
+import com.cloud.event.EventTypes;
+import com.cloud.network.vpc.NetworkACLItem;
+import com.cloud.user.Account;
+
+@APICommand(name = "moveNetworkAclItem", description = "Move an ACL rule to a position bettwen two other ACL rules of the same ACL network list", responseObject = NetworkACLItemResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+public class MoveNetworkAclItemCmd extends BaseAsyncCustomIdCmd {
+
+    public static final Logger s_logger = Logger.getLogger(MoveNetworkAclItemCmd.class.getName());
+    private static final String s_name = "moveNetworkAclItemResponse";
+
+    @Parameter(name = ApiConstants.ID, type = CommandType.STRING, required = true, description = "The ID of the network ACL rule that is being moved to a new position.")
+    private String uuidRuleBeingMoved;
+
+    @Parameter(name = ApiConstants.PREVIOUS_ACL_RULE_ID, type = CommandType.STRING, description = "The ID of the first rule that is right before the new position where the rule being moved is going to be placed. This value can be 'NULL' if the rule is being moved to the first position of the network ACL list.")
+    private String previousAclRuleUuid;
+
+    @Parameter(name = ApiConstants.NEXT_ACL_RULE_ID, type = CommandType.STRING, description = "The ID of the rule that is right after the new position where the rule being moved is going to be placed. This value can be 'NULL' if the rule is being moved to the last position of the network ACL list.")
+    private String nextAclRuleUuid;
+
+    @Override
+    public void execute() {
+        CallContext.current().setEventDetails(getEventDescription());
+
+        NetworkACLItem aclItem = _networkACLService.moveNetworkAclRuleToNewPosition(this);
+
+        NetworkACLItemResponse aclResponse = _responseGenerator.createNetworkACLItemResponse(aclItem);
+        setResponseObject(aclResponse);
+        aclResponse.setResponseName(getCommandName());
+    }
+
+    public String getUuidRuleBeingMoved() {
+        return uuidRuleBeingMoved;
+    }
+
+    public String getPreviousAclRuleUuid() {
+        return previousAclRuleUuid;
+    }
+
+    public String getNextAclRuleUuid() {
+        return nextAclRuleUuid;
+    }
+
+    @Override
+    public void checkUuid() {
+        if (this.getCustomId() != null) {
+            _uuidMgr.checkUuid(this.getCustomId(), NetworkACLItem.class);
+        }
+    }
+
+    @Override
+    public String getEventType() {
+        return EventTypes.EVENT_NETWORK_ACL_ITEM_UPDATE;
+    }
+
+    @Override
+    public String getEventDescription() {
+        return String.format("Placing network ACL item [%s] between [%s] and [%s].", uuidRuleBeingMoved, previousAclRuleUuid, nextAclRuleUuid);
+    }
+
+    @Override
+    public String getCommandName() {
+        return s_name;
+    }
+
+    @Override
+    public long getEntityOwnerId() {
+        Account caller = CallContext.current().getCallingAccount();
+        return caller.getAccountId();
+    }
+}

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

@@ -20,7 +20,7 @@
 
 import com.cloud.utils.db.GenericDao;
 
-/*
+/**
  * Data Access Object for network_acl_item table
  */
 public interface NetworkACLItemDao extends GenericDao<NetworkACLItemVO, Long> {
@@ -36,4 +36,12 @@ public interface NetworkACLItemDao extends GenericDao<NetworkACLItemVO, Long> {
     NetworkACLItemVO findByAclAndNumber(long aclId, int number);
 
     void loadCidrs(NetworkACLItemVO item);
+
+    /**
+     * Updated the network ACL item 'number' field.
+     *
+     * @param networkItemId is the ID of the network ACL rule that will have its 'number' field updated.
+     * @param newNumberValue is the new value that will be assigned to the 'number' field.
+     */
+    void updateNumberFieldNetworkItem(long networkItemId, int newNumberValue);
 }

engine/schema/src/main/java/com/cloud/network/vpc/dao/NetworkACLItemCidrsDaoImpl.java

@@ -21,7 +21,6 @@
 import java.util.ArrayList;
 import java.util.List;
 
-
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 

engine/schema/src/main/java/com/cloud/network/vpc/dao/NetworkACLItemDaoImpl.java

@@ -16,12 +16,12 @@
 // under the License.
 package com.cloud.network.vpc.dao;
 
+import java.sql.PreparedStatement;
+import java.sql.SQLException;
 import java.util.List;
 
 import javax.inject.Inject;
 
-import com.google.common.collect.Lists;
-import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 
 import com.cloud.network.vpc.NetworkACLItem.State;
@@ -35,11 +35,12 @@
 import com.cloud.utils.db.SearchCriteria;
 import com.cloud.utils.db.SearchCriteria.Op;
 import com.cloud.utils.db.TransactionLegacy;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.google.common.collect.Lists;
 
-@Component
 @DB()
+@Component
 public class NetworkACLItemDaoImpl extends GenericDaoBase<NetworkACLItemVO, Long> implements NetworkACLItemDao {
-    private static final Logger s_logger = Logger.getLogger(NetworkACLItemDaoImpl.class);
 
     protected final SearchBuilder<NetworkACLItemVO> AllFieldsSearch;
     protected final SearchBuilder<NetworkACLItemVO> NotRevokedSearch;
@@ -115,12 +116,14 @@ public boolean revoke(NetworkACLItemVO rule) {
 
     @Override
     public List<NetworkACLItemVO> listByACL(Long aclId) {
-        if (aclId == null) return Lists.newArrayList();
+        if (aclId == null) {
+            return Lists.newArrayList();
+        }
 
         SearchCriteria<NetworkACLItemVO> sc = AllFieldsSearch.create();
         sc.setParameters("aclId", aclId);
         List<NetworkACLItemVO> list = listBy(sc);
-        for(NetworkACLItemVO item :list) {
+        for (NetworkACLItemVO item : list) {
             loadCidrs(item);
         }
         return list;
@@ -140,7 +143,7 @@ public NetworkACLItemVO findByAclAndNumber(long aclId, int number) {
         sc.setParameters("aclId", aclId);
         sc.setParameters("number", number);
         NetworkACLItemVO vo = findOneBy(sc);
-        if(vo != null) {
+        if (vo != null) {
             loadCidrs(vo);
         }
         return vo;
@@ -172,4 +175,19 @@ public void loadCidrs(NetworkACLItemVO item) {
         List<String> cidrs = _networkACLItemCidrsDao.getCidrs(item.getId());
         item.setSourceCidrList(cidrs);
     }
-}
+
+    private String sqlUpdateNumberFieldNetworkItem = "UPDATE network_acl_item SET number = ? where id =?";
+
+    @Override
+    public void updateNumberFieldNetworkItem(long networkItemId, int newNumberValue) {
+        try (TransactionLegacy txn = TransactionLegacy.currentTxn();
+                PreparedStatement pstmt = txn.prepareAutoCloseStatement(sqlUpdateNumberFieldNetworkItem)) {
+            pstmt.setLong(1, newNumberValue);
+            pstmt.setLong(2, networkItemId);
+            pstmt.executeUpdate();
+            txn.commit();
+        } catch (SQLException e) {
+            throw new CloudRuntimeException(e);
+        }
+    }
+}

engine/schema/src/main/resources/META-INF/db/schema-41110to41200.sql

@@ -26,4 +26,10 @@ ALTER TABLE `cloud`.`network_acl_item` ADD COLUMN `reason` VARCHAR(2500) AFTER `
 ALTER TABLE `cloud`.`alert` ADD COLUMN `content` VARCHAR(5000);
 
 -- Fix the name of the column used to hold IPv4 range in 'vlan' table.
-ALTER TABLE `vlan` CHANGE `description` `ip4_range` varchar(255);
+ALTER TABLE `vlan` CHANGE `description` `ip4_range` varchar(255);
+
+-- [CLOUDSTACK-10344] bug when moving ACL rules (change order with drag and drop)
+-- We are only adding the permission to the default rules. Any custom rule must be configured by the root admin.
+INSERT INTO `cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`, `sort_order`) values (UUID(), 2, 'moveNetworkAclItem', 'ALLOW', 100) ON DUPLICATE KEY UPDATE rule=rule;
+INSERT INTO `cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`, `sort_order`) values (UUID(), 3, 'moveNetworkAclItem', 'ALLOW', 302) ON DUPLICATE KEY UPDATE rule=rule;
+INSERT INTO `cloud`.`role_permissions` (`uuid`, `role_id`, `rule`, `permission`, `sort_order`) values (UUID(), 4, 'moveNetworkAclItem', 'ALLOW', 260) ON DUPLICATE KEY UPDATE rule=rule;