CLOUDSTACK-6628:[Automation] Create PF rulw API failing with error

minchen07 · minchen07 · commit daadc45122b9 · 2014-05-09T18:48:08.000-07:00
"database id can only provided by VO objects".
diff --git a/api/src/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java b/api/src/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java
@@ -19,8 +19,11 @@
 import java.util.List;
 
 import org.apache.log4j.Logger;
+import org.bouncycastle.util.IPAddress;
 
 import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.acl.SecurityChecker.AccessType;
+import org.apache.cloudstack.api.ACL;
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.ApiCommandJobType;
 import org.apache.cloudstack.api.ApiConstants;
@@ -48,7 +51,7 @@
 import com.cloud.vm.VirtualMachine;
 
 @APICommand(name = "createPortForwardingRule", description = "Creates a port forwarding rule", responseObject = FirewallRuleResponse.class, entityType = {FirewallRule.class,
-        VirtualMachine.class},
+        VirtualMachine.class, IPAddress.class},
         requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
 public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd implements PortForwardingRule {
     public static final Logger s_logger = Logger.getLogger(CreatePortForwardingRuleCmd.class.getName());
@@ -59,6 +62,7 @@ public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd implements P
     // ////////////// API parameters /////////////////////
     // ///////////////////////////////////////////////////
 
+    @ACL(accessType = AccessType.OperateEntry)
     @Parameter(name = ApiConstants.IP_ADDRESS_ID,
                type = CommandType.UUID,
                entityType = IPAddressResponse.class,
@@ -96,6 +100,7 @@ public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd implements P
                description = "the ending port of port forwarding rule's private port range")
     private Integer publicEndPort;
 
+    @ACL(accessType = AccessType.OperateEntry)
     @Parameter(name = ApiConstants.VIRTUAL_MACHINE_ID,
                type = CommandType.UUID,
                entityType = UserVmResponse.class,
diff --git a/server/src/com/cloud/network/rules/RulesManagerImpl.java b/server/src/com/cloud/network/rules/RulesManagerImpl.java
@@ -195,6 +195,7 @@ public void checkRuleAndUserVm(FirewallRule rule, UserVm userVm, Account caller)
             throw new InvalidParameterValueException("Invalid user vm: " + userVm.getId());
         }
 
+        // This same owner check is actually not needed, since multiple entities OperateEntry trick guarantee that
         if (rule.getAccountId() != userVm.getAccountId()) {
             throw new InvalidParameterValueException("New rule " + rule + " and vm id=" + userVm.getId() + " belong to different accounts");
         }
@@ -267,8 +268,8 @@ public PortForwardingRule createPortForwardingRule(final PortForwardingRule rule
             if (vm == null) {
                 throw new InvalidParameterValueException("Unable to create port forwarding rule on address " + ipAddress + ", invalid virtual machine id specified (" +
                     vmId + ").");
-            } else {
-                checkRuleAndUserVm(rule, vm, caller);
+            } else if (vm.getState() == VirtualMachine.State.Destroyed || vm.getState() == VirtualMachine.State.Expunging) {
+                throw new InvalidParameterValueException("Invalid user vm: " + vm.getId());
             }
 
             // Verify that vm has nic in the network

Original file line number	Diff line number	Diff line change
`@@ -195,6 +195,7 @@ public void checkRuleAndUserVm(FirewallRule rule, UserVm userVm, Account caller)`
`195`	`195`	`throw new InvalidParameterValueException("Invalid user vm: " + userVm.getId());`
`196`	`196`	`}`
`197`	`197`
	`198`	`+ // This same owner check is actually not needed, since multiple entities OperateEntry trick guarantee that`
`198`	`199`	`if (rule.getAccountId() != userVm.getAccountId()) {`
`199`	`200`	`throw new InvalidParameterValueException("New rule " + rule + " and vm id=" + userVm.getId() + " belong to different accounts");`
`200`	`201`	`}`
`@@ -267,8 +268,8 @@ public PortForwardingRule createPortForwardingRule(final PortForwardingRule rule`
`267`	`268`	`if (vm == null) {`
`268`	`269`	`throw new InvalidParameterValueException("Unable to create port forwarding rule on address " + ipAddress + ", invalid virtual machine id specified (" +`
`269`	`270`	`vmId + ").");`
`270`		`- } else {`
`271`		`- checkRuleAndUserVm(rule, vm, caller);`
	`271`	`+ } else if (vm.getState() == VirtualMachine.State.Destroyed \|\| vm.getState() == VirtualMachine.State.Expunging) {`
	`272`	`+ throw new InvalidParameterValueException("Invalid user vm: " + vm.getId());`
`272`	`273`	`}`
`273`	`274`
`274`	`275`	`// Verify that vm has nic in the network`