diff --git a/plugin/securityGroup/src/main/java/org/zstack/network/securitygroup/SecurityGroupApiInterceptor.java b/plugin/securityGroup/src/main/java/org/zstack/network/securitygroup/SecurityGroupApiInterceptor.java index 135ad3886aa..bd994e6d069 100755 --- a/plugin/securityGroup/src/main/java/org/zstack/network/securitygroup/SecurityGroupApiInterceptor.java +++ b/plugin/securityGroup/src/main/java/org/zstack/network/securitygroup/SecurityGroupApiInterceptor.java @@ -343,13 +343,8 @@ private void validate(APISetVmNicSecurityGroupMsg msg) { if (!aoMap.isEmpty()) { Integer[] priorities = aoMap.keySet().toArray(new Integer[aoMap.size()]); Arrays.sort(priorities); - if (priorities[0] != 1) { - throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SECURITYGROUP_10022, "could no set vm nic security group, because invalid priority, priority expects to start at 1, but [%d]", priorities[0])); - } - for (int i = 0; i < priorities.length - 1; i++) { - if (priorities[i] + 1 != priorities[i + 1]) { - throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SECURITYGROUP_10023, "could no set vm nic security group, because invalid priority, priority[%d] and priority[%d] expected to be consecutive", priorities[i], priorities[i + 1])); - } + if (priorities[0] < 1) { + throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SECURITYGROUP_10022, "could no set vm nic security group, because invalid priority, priority expects to be positive, but [%d]", priorities[0])); } } @@ -390,13 +385,8 @@ private void validate(APISetVmNicSecurityGroupMsg msg) { if (!adminIntegers.isEmpty()) { Integer[] priorities = adminIntegers.toArray(new Integer[adminIntegers.size()]); Arrays.sort(priorities); - if (priorities[0] != 1) { - throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SECURITYGROUP_10024, "could no set vm nic security group, because admin security group priority[%d] must be higher than users", priorities[0])); - } - for (int i = 0; i < priorities.length - 1; i++) { - if (priorities[i] + 1 != priorities[i + 1]) { - throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SECURITYGROUP_10025, "could no set vm nic security group, because admin security group priority[%d] must be higher than users", priorities[i + 1])); - } + if (priorities[0] < 1) { + throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SECURITYGROUP_10024, "could no set vm nic security group, because admin security group priority[%d] must be positive", priorities[0])); } } } @@ -498,8 +488,9 @@ private void validate(APIUpdateSecurityGroupRulePriorityMsg msg) { rvos.stream().filter(rvo -> rvo.getUuid().equals(ao.getRuleUuid())).findFirst().orElseThrow(() -> new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SECURITYGROUP_10041, "could not update security group rule priority, because rule[uuid:%s] not in security group[uuid:%s]", ao.getRuleUuid(), msg.getSecurityGroupUuid()))); - rvos.stream().filter(rvo -> rvo.getPriority() == ao.getPriority()).findFirst().orElseThrow(() -> - new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SECURITYGROUP_10042, "could not update security group rule priority, because priority[%d] not in security group[uuid:%s]", ao.getPriority(), msg.getSecurityGroupUuid()))); + if (ao.getPriority() < 1) { + throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SECURITYGROUP_10042, "could not update security group rule priority, because priority[%d] must be positive", ao.getPriority())); + } } List uuidList = new ArrayList<>(priorityMap.values()); @@ -534,8 +525,8 @@ private void validate(APIChangeSecurityGroupRuleMsg msg) { if (count.intValue() > SecurityGroupGlobalConfig.SECURITY_GROUP_RULES_NUM_LIMIT.value(Integer.class)) { throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SECURITYGROUP_10047, "could not change security group rule, because security group %s rules number[%d] is out of max limit[%d]", vo.getType(), count.intValue(), SecurityGroupGlobalConfig.SECURITY_GROUP_RULES_NUM_LIMIT.value(Integer.class))); } - if (msg.getPriority() > count.intValue()) { - throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SECURITYGROUP_10048, "could not change security group rule, because the maximum priority of %s rule is [%d]", vo.getType().toString(), count.intValue())); + if (msg.getPriority() > SecurityGroupGlobalConfig.SECURITY_GROUP_RULES_NUM_LIMIT.value(Integer.class)) { + throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SECURITYGROUP_10048, "could not change security group rule, because priority[%d] exceeds the maximum limit[%d]", msg.getPriority(), SecurityGroupGlobalConfig.SECURITY_GROUP_RULES_NUM_LIMIT.value(Integer.class))); } if (msg.getPriority() < 0) { msg.setPriority(SecurityGroupConstant.LOWEST_RULE_PRIORITY); @@ -1198,11 +1189,8 @@ private void validate(APIAddSecurityGroupRuleMsg msg) { throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SECURITYGROUP_10119, "could not add security group rule, because security group %s rules number[%d] is out of max limit[%d]", SecurityGroupRuleType.Egress, (egressRuleCount + toCreateEgressRuleCount), SecurityGroupGlobalConfig.SECURITY_GROUP_RULES_NUM_LIMIT.value(Integer.class))); } - if (msg.getPriority() > (ingressRuleCount + 1) && toCreateIngressRuleCount > 0) { - throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SECURITYGROUP_10120, "could not add security group rule, because priority[%d] must be consecutive, the ingress rule maximum priority is [%d]", msg.getPriority(), ingressRuleCount)); - } - if (msg.getPriority() > (egressRuleCount + 1) && toCreateEgressRuleCount > 0) { - throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SECURITYGROUP_10121, "could not add security group rule, because priority[%d] must be consecutive, the egress rule maximum priority is [%d]", msg.getPriority(), egressRuleCount)); + if (msg.getPriority() > SecurityGroupGlobalConfig.SECURITY_GROUP_RULES_NUM_LIMIT.value(Integer.class)) { + throw new ApiMessageInterceptionException(argerr(ORG_ZSTACK_NETWORK_SECURITYGROUP_10120, "could not add security group rule, because priority[%d] exceeds the maximum limit[%d]", msg.getPriority(), SecurityGroupGlobalConfig.SECURITY_GROUP_RULES_NUM_LIMIT.value(Integer.class))); } } diff --git a/test/src/test/groovy/org/zstack/test/integration/networkservice/provider/flat/securitygroup/AddSecurityGroupRuleOptimizedCase.groovy b/test/src/test/groovy/org/zstack/test/integration/networkservice/provider/flat/securitygroup/AddSecurityGroupRuleOptimizedCase.groovy index ed9a2736c5f..d0f71d027b2 100644 --- a/test/src/test/groovy/org/zstack/test/integration/networkservice/provider/flat/securitygroup/AddSecurityGroupRuleOptimizedCase.groovy +++ b/test/src/test/groovy/org/zstack/test/integration/networkservice/provider/flat/securitygroup/AddSecurityGroupRuleOptimizedCase.groovy @@ -591,13 +591,13 @@ class AddSecurityGroupRuleOptimizedCase extends SubCase { rule_13.protocol = "ALL" rule_13.startPort = -1 rule_13.endPort = -1 - expect(AssertionError) { - addSecurityGroupRule { - securityGroupUuid = sg3.uuid - rules = [rule_13] - priority = 13 - } + // ZSTAC-79067: non-consecutive priorities are now allowed + sg3 = addSecurityGroupRule { + securityGroupUuid = sg3.uuid + rules = [rule_13] + priority = 13 } + assert sg3.rules.find { it.priority == 13 } != null SecurityGroupRuleAO rule_12 = new SecurityGroupRuleAO() rule_12.dstIpRange = "2.2.2.2-2.2.2.10" @@ -609,12 +609,11 @@ class AddSecurityGroupRuleOptimizedCase extends SubCase { ingressRule.protocol = "TCP" ingressRule.dstPortRange = "12-13" - expect(AssertionError) { - addSecurityGroupRule { - securityGroupUuid = sg3.uuid - rules = [rule_12, ingressRule] - priority = 12 - } + // ZSTAC-79067: mixed ingress+egress add at non-consecutive priority is now allowed + sg3 = addSecurityGroupRule { + securityGroupUuid = sg3.uuid + rules = [rule_12, ingressRule] + priority = 12 } } diff --git a/test/src/test/groovy/org/zstack/test/integration/networkservice/provider/flat/securitygroup/ChangeSecurityGroupRuleCase.groovy b/test/src/test/groovy/org/zstack/test/integration/networkservice/provider/flat/securitygroup/ChangeSecurityGroupRuleCase.groovy index ad29787ef1e..60b59424257 100644 --- a/test/src/test/groovy/org/zstack/test/integration/networkservice/provider/flat/securitygroup/ChangeSecurityGroupRuleCase.groovy +++ b/test/src/test/groovy/org/zstack/test/integration/networkservice/provider/flat/securitygroup/ChangeSecurityGroupRuleCase.groovy @@ -231,19 +231,8 @@ class ChangeSecurityGroupRuleCase extends SubCase { assert sg3 != null SecurityGroupRuleInventory rule_1 = sg3.rules.find { it.type == "Ingress" && it.priority == 1 && it.ipVersion == 4 } - expect(AssertionError) { - changeSecurityGroupRule { - uuid = rule_1.uuid - priority = 6 - } - } - - expect(AssertionError) { - changeSecurityGroupRule { - uuid = rule_1.uuid - priority = 7 - } - } + // ZSTAC-79067: priorities beyond current rule count are now allowed + // (removed consecutive priority restriction) } void testChangeRuleDuplicate() { @@ -307,12 +296,8 @@ class ChangeSecurityGroupRuleCase extends SubCase { } } - expect(AssertionError) { - changeSecurityGroupRule { - uuid = rule1.uuid - priority = 3 - } - } + // ZSTAC-79067: priority beyond rule count is now allowed + // (removed consecutive restriction, only check against global limit) expect(AssertionError) { changeSecurityGroupRule {