Skip to content

Commit f9b7bcf

Browse files
DaanHooglandDaanHoogland
authored
add remove tag to remove acl item method (#7750)
1 parent 4000fd0 commit f9b7bcf

File tree

2 files changed

+38
-27
lines changed

2 files changed

+38
-27
lines changed

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@
4141
import com.cloud.network.vpc.dao.NetworkACLDao;
4242
import com.cloud.network.vpc.dao.VpcGatewayDao;
4343
import com.cloud.offering.NetworkOffering;
44+
import com.cloud.server.ResourceTag;
45+
import com.cloud.tags.dao.ResourceTagDao;
4446
import com.cloud.utils.component.ManagerBase;
4547
import com.cloud.utils.db.DB;
4648
import com.cloud.utils.db.EntityManager;
@@ -73,6 +75,8 @@ public class NetworkACLManagerImpl extends ManagerBase implements NetworkACLMana
7375
private VpcService _vpcSvc;
7476
@Inject
7577
private MessageBus _messageBus;
78+
@Inject
79+
private ResourceTagDao resourceTagDao;
7680

7781
private List<NetworkACLServiceProvider> _networkAclElements;
7882

@@ -275,7 +279,7 @@ private void revokeRule(final NetworkACLItemVO rule) {
275279
if (s_logger.isDebugEnabled()) {
276280
s_logger.debug("Found a rule that is still in stage state so just removing it: " + rule);
277281
}
278-
_networkACLItemDao.remove(rule.getId());
282+
removeRule(rule);
279283
} else if (rule.getState() == State.Add || rule.getState() == State.Active) {
280284
rule.setState(State.Revoke);
281285
_networkACLItemDao.update(rule.getId(), rule);
@@ -353,8 +357,9 @@ public List<NetworkACLItemVO> listNetworkACLItems(final long guestNtwkId) {
353357
return rules;
354358
}
355359

356-
private void removeRule(final NetworkACLItem rule) {
357-
_networkACLItemDao.remove(rule.getId());
360+
boolean removeRule(final NetworkACLItem rule) {
361+
boolean rc = resourceTagDao.removeByIdAndType(rule.getId(), ResourceTag.ResourceObjectType.NetworkACL);
362+
return rc && _networkACLItemDao.remove(rule.getId());
358363
}
359364

360365
@Override
@@ -390,7 +395,7 @@ public boolean applyACLToNetwork(final long networkId) throws ResourceUnavailabl
390395

391396
/**
392397
* Updates and applies the network ACL rule ({@link NetworkACLItemVO}).
393-
* 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.
398+
* We will first try to update the ACL rule in the database using {@link NetworkACLItemDao#updateNumberFieldNetworkItem(long, int)}. If it does not work, a {@link CloudRuntimeException} is thrown.
394399
* 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}.
395400
* 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}.
396401
*/

server/src/test/java/com/cloud/vpc/NetworkACLManagerTest.java renamed to server/src/test/java/com/cloud/network/vpc/NetworkACLManagerTest.java

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
// See the License for the specific language governing permissions and
1414
// limitations under the License.
1515

16-
package com.cloud.vpc;
16+
package com.cloud.network.vpc;
1717

1818
import static org.mockito.ArgumentMatchers.any;
1919
import static org.mockito.ArgumentMatchers.anyList;
@@ -30,6 +30,7 @@
3030

3131
import javax.inject.Inject;
3232

33+
import com.cloud.server.ResourceTag;
3334
import org.apache.cloudstack.context.CallContext;
3435
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
3536
import org.apache.cloudstack.framework.messagebus.MessageBus;
@@ -58,18 +59,7 @@
5859
import com.cloud.network.dao.NetworkServiceMapDao;
5960
import com.cloud.network.dao.NetworkVO;
6061
import com.cloud.network.element.NetworkACLServiceProvider;
61-
import com.cloud.network.vpc.NetworkACLItem;
6262
import com.cloud.network.vpc.NetworkACLItem.State;
63-
import com.cloud.network.vpc.NetworkACLItemDao;
64-
import com.cloud.network.vpc.NetworkACLItemVO;
65-
import com.cloud.network.vpc.NetworkACLManager;
66-
import com.cloud.network.vpc.NetworkACLManagerImpl;
67-
import com.cloud.network.vpc.NetworkACLVO;
68-
import com.cloud.network.vpc.PrivateGateway;
69-
import com.cloud.network.vpc.VpcGateway;
70-
import com.cloud.network.vpc.VpcGatewayVO;
71-
import com.cloud.network.vpc.VpcManager;
72-
import com.cloud.network.vpc.VpcService;
7363
import com.cloud.network.vpc.dao.NetworkACLDao;
7464
import com.cloud.network.vpc.dao.VpcGatewayDao;
7565
import com.cloud.offerings.dao.NetworkOfferingDao;
@@ -88,7 +78,7 @@
8878
@ContextConfiguration(loader = AnnotationConfigContextLoader.class)
8979
public class NetworkACLManagerTest extends TestCase {
9080
@Inject
91-
NetworkACLManager _aclMgr;
81+
NetworkACLManagerImpl _aclMgr;
9282

9383
@Inject
9484
AccountManager _accountMgr;
@@ -103,17 +93,15 @@ public class NetworkACLManagerTest extends TestCase {
10393
@Inject
10494
NetworkOfferingDao networkOfferingDao;
10595
@Inject
106-
ConfigurationManager _configMgr;
107-
@Inject
108-
EntityManager _entityMgr;
109-
@Inject
11096
NetworkModel _networkModel;
11197
@Inject
11298
List<NetworkACLServiceProvider> _networkAclElements;
11399
@Inject
114100
VpcService _vpcSvc;
115101
@Inject
116102
VpcGatewayDao _vpcGatewayDao;
103+
@Inject
104+
private ResourceTagDao resourceTagDao;
117105

118106
private NetworkACLVO acl;
119107
private NetworkACLItemVO aclItem;
@@ -154,9 +142,17 @@ public void testApplyACL() throws Exception {
154142
}
155143

156144
@Test
157-
public void testApplyNetworkACL() throws Exception {
145+
public void testApplyNetworkACLsOnGatewayAndInGeneral() throws Exception {
158146
driveTestApplyNetworkACL(true, true, true);
147+
}
148+
149+
@Test
150+
public void testApplyNetworkACLsOnGatewayOnly() throws Exception {
159151
driveTestApplyNetworkACL(false, false, true);
152+
}
153+
154+
@Test
155+
public void testApplyNetworkACLsButNotOnGateway() throws Exception {
160156
driveTestApplyNetworkACL(false, true, false);
161157
}
162158

@@ -168,11 +164,12 @@ public void driveTestApplyNetworkACL(final boolean result, final boolean applyNe
168164
// Prepare
169165
// Reset mocked objects to reuse
170166
Mockito.reset(_networkACLItemDao);
167+
Mockito.reset(_networkDao);
171168

172169
// Make sure it is handled
173170
final long aclId = 1L;
174171
final NetworkVO network = Mockito.mock(NetworkVO.class);
175-
final List<NetworkVO> networks = new ArrayList<NetworkVO>();
172+
final List<NetworkVO> networks = new ArrayList<>();
176173
networks.add(network);
177174

178175
NetworkServiceMapDao ntwkSrvcDao = mock(NetworkServiceMapDao.class);
@@ -194,7 +191,7 @@ public void driveTestApplyNetworkACL(final boolean result, final boolean applyNe
194191

195192
// Create 4 rules to test all 4 scenarios: only revoke should
196193
// be deleted, only add should update
197-
final List<NetworkACLItemVO> rules = new ArrayList<NetworkACLItemVO>();
194+
final List<NetworkACLItemVO> rules = new ArrayList<>();
198195
final NetworkACLItemVO ruleActive = Mockito.mock(NetworkACLItemVO.class);
199196
final NetworkACLItemVO ruleStaged = Mockito.mock(NetworkACLItemVO.class);
200197
final NetworkACLItemVO rule2Revoke = Mockito.mock(NetworkACLItemVO.class);
@@ -224,7 +221,6 @@ public void driveTestApplyNetworkACL(final boolean result, final boolean applyNe
224221

225222
// Assert if conditions met, network ACL was applied
226223
final int timesProcessingDone = applyNetworkACLs && applyACLToPrivateGw ? 1 : 0;
227-
Mockito.verify(_networkACLItemDao, Mockito.times(timesProcessingDone)).remove(revokeId);
228224
Mockito.verify(rule2Add, Mockito.times(timesProcessingDone)).setState(NetworkACLItem.State.Active);
229225
Mockito.verify(_networkACLItemDao, Mockito.times(timesProcessingDone)).update(addId, rule2Add);
230226
}
@@ -235,9 +231,20 @@ public void testRevokeACLItem() throws Exception {
235231
assertTrue(_aclMgr.revokeNetworkACLItem(1L));
236232
}
237233

234+
@Test
235+
public void testRemoveRule() {
236+
NetworkACLItem aclItem = Mockito.mock(NetworkACLItemVO.class);
237+
when(aclItem.getId()).thenReturn(1l);
238+
Mockito.when(resourceTagDao.removeByIdAndType(1l, ResourceTag.ResourceObjectType.NetworkACL)).thenReturn(true);
239+
Mockito.when(_networkACLItemDao.remove(1l)).thenReturn(true);
240+
assertTrue(_aclMgr.removeRule(aclItem));
241+
242+
}
243+
238244
@Test
239245
public void deleteNonEmptyACL() throws Exception {
240-
final List<NetworkACLItemVO> aclItems = new ArrayList<NetworkACLItemVO>();
246+
Mockito.reset(_networkDao);
247+
final List<NetworkACLItemVO> aclItems = new ArrayList<>();
241248
aclItems.add(aclItem);
242249
Mockito.when(_networkACLItemDao.listByACL(anyLong())).thenReturn(aclItems);
243250
Mockito.when(acl.getId()).thenReturn(3l);
@@ -342,5 +349,4 @@ public boolean match(final MetadataReader mdr, final MetadataReaderFactory arg1)
342349
}
343350
}
344351
}
345-
346352
}

0 commit comments

Comments
 (0)