Skip to content

Commit 644c644

Browse files
Dogface2kDogface2k
committed
CKS: handle VPC tiers without attached ACLs
1 parent 0339f31 commit 644c644

5 files changed

Lines changed: 134 additions & 6 deletions

File tree

‎plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,7 @@ protected void validateVpcTier(Network network) {
602602
if (Network.State.Allocated.equals(network.getState())) { // Allocated networks won't have IP and rules
603603
return;
604604
}
605-
if (network.getNetworkACLId() == NetworkACL.DEFAULT_DENY) {
605+
if (Objects.equals(network.getNetworkACLId(), NetworkACL.DEFAULT_DENY)) {
606606
throw new InvalidParameterValueException(String.format("Network ID: %s can not be used for Kubernetes cluster as it uses default deny ACL", network.getUuid()));
607607
}
608608
}

‎plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,7 @@ protected void setupKubernetesClusterIsolatedNetworkRules(IpAddress publicIp, Ne
717717
}
718718

719719
protected void createVpcTierAclRules(Network network) throws ManagementServerException {
720-
if (network.getNetworkACLId() == NetworkACL.DEFAULT_ALLOW) {
720+
if (Objects.equals(network.getNetworkACLId(), NetworkACL.DEFAULT_ALLOW)) {
721721
return;
722722
}
723723
// ACL rule for API access for control node VMs
@@ -746,7 +746,8 @@ protected void createVpcTierAclRules(Network network) throws ManagementServerExc
746746
}
747747

748748
protected void removeVpcTierAclRules(Network network) throws ManagementServerException {
749-
if (network.getNetworkACLId() == NetworkACL.DEFAULT_ALLOW) {
749+
Long networkAclId = network.getNetworkACLId();
750+
if (networkAclId == null || Objects.equals(networkAclId, NetworkACL.DEFAULT_ALLOW)) {
750751
return;
751752
}
752753
// ACL rule for API access for control node VMs

‎plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,7 @@ protected void setupKubernetesEtcdNetworkRules(List<UserVm> etcdVms, Network net
646646
try {
647647
if (Objects.isNull(network.getVpcId())) {
648648
provisionFirewallRules(publicIp, owner, etcdStartPort, etcdStartPort);
649-
} else if (network.getNetworkACLId() != NetworkACL.DEFAULT_ALLOW) {
649+
} else if (!Objects.equals(network.getNetworkACLId(), NetworkACL.DEFAULT_ALLOW)) {
650650
try {
651651
provisionVpcTierAllowPortACLRule(network, ETCD_NODE_CLIENT_REQUEST_PORT, ETCD_NODE_CLIENT_REQUEST_PORT);
652652
if (logger.isInfoEnabled()) {

‎plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,14 @@ public void testValidateVpcTierValid() {
130130
kubernetesClusterManager.validateVpcTier(network);
131131
}
132132

133+
@Test
134+
public void testValidateVpcTierWithoutAcl() {
135+
Network network = Mockito.mock(Network.class);
136+
Mockito.when(network.getState()).thenReturn(Network.State.Implemented);
137+
Mockito.when(network.getNetworkACLId()).thenReturn(null);
138+
kubernetesClusterManager.validateVpcTier(network);
139+
}
140+
133141
@Test
134142
public void validateIsolatedNetworkIpRulesNoRules() {
135143
long ipId = 1L;

‎plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorkerTest.java‎

Lines changed: 121 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,28 @@
1717

1818
package com.cloud.kubernetes.cluster.actionworkers;
1919

20+
import java.util.List;
21+
2022
import com.cloud.kubernetes.cluster.KubernetesCluster;
2123
import com.cloud.kubernetes.cluster.KubernetesClusterManagerImpl;
2224
import com.cloud.kubernetes.cluster.dao.KubernetesClusterDao;
2325
import com.cloud.kubernetes.cluster.dao.KubernetesClusterDetailsDao;
2426
import com.cloud.kubernetes.cluster.dao.KubernetesClusterVmMapDao;
2527
import com.cloud.kubernetes.version.dao.KubernetesSupportedVersionDao;
28+
import com.cloud.network.IpAddress;
29+
import com.cloud.network.Network;
30+
import com.cloud.network.dao.IPAddressDao;
31+
import com.cloud.network.dao.IPAddressVO;
32+
import com.cloud.network.vpc.NetworkACL;
33+
import com.cloud.user.Account;
34+
import com.cloud.uservm.UserVm;
35+
import org.apache.cloudstack.context.CallContext;
2636
import org.junit.Assert;
2737
import org.junit.Before;
2838
import org.junit.Test;
2939
import org.junit.runner.RunWith;
3040
import org.mockito.Mock;
41+
import org.mockito.MockedStatic;
3142
import org.mockito.Mockito;
3243
import org.mockito.junit.MockitoJUnitRunner;
3344

@@ -51,7 +62,52 @@ public class KubernetesClusterResourceModifierActionWorkerTest {
5162
@Mock
5263
private KubernetesCluster kubernetesClusterMock;
5364

54-
private KubernetesClusterResourceModifierActionWorker kubernetesClusterResourceModifierActionWorker;
65+
@Mock
66+
private IPAddressDao ipAddressDaoMock;
67+
68+
@Mock
69+
private Account accountMock;
70+
71+
private TestKubernetesClusterResourceModifierActionWorker kubernetesClusterResourceModifierActionWorker;
72+
73+
private static class TestKubernetesClusterResourceModifierActionWorker extends KubernetesClusterResourceModifierActionWorker {
74+
private int provisionAclRuleCalls;
75+
private int removeAclRuleCalls;
76+
77+
TestKubernetesClusterResourceModifierActionWorker(KubernetesCluster kubernetesCluster, KubernetesClusterManagerImpl clusterManager) {
78+
super(kubernetesCluster, clusterManager);
79+
}
80+
81+
@Override
82+
protected void provisionVpcTierAllowPortACLRule(Network network, int startPort, int endPort) {
83+
provisionAclRuleCalls++;
84+
}
85+
86+
@Override
87+
protected void removeVpcTierAllowPortACLRule(Network network, int startPort, int endPort) {
88+
removeAclRuleCalls++;
89+
}
90+
}
91+
92+
private static class TestKubernetesClusterStartWorker extends KubernetesClusterStartWorker {
93+
private int provisionAclRuleCalls;
94+
private int provisionPortForwardingRuleCalls;
95+
96+
TestKubernetesClusterStartWorker(KubernetesCluster kubernetesCluster, KubernetesClusterManagerImpl clusterManager) {
97+
super(kubernetesCluster, clusterManager);
98+
}
99+
100+
@Override
101+
protected void provisionVpcTierAllowPortACLRule(Network network, int startPort, int endPort) {
102+
provisionAclRuleCalls++;
103+
}
104+
105+
@Override
106+
protected void provisionPublicIpPortForwardingRule(IpAddress publicIp, Network network, Account account,
107+
long vmId, int sourcePort, int destPort) {
108+
provisionPortForwardingRuleCalls++;
109+
}
110+
}
55111

56112
@Before
57113
public void setUp() {
@@ -60,7 +116,7 @@ public void setUp() {
60116
kubernetesClusterManagerMock.kubernetesClusterDetailsDao = kubernetesClusterDetailsDaoMock;
61117
kubernetesClusterManagerMock.kubernetesClusterVmMapDao = kubernetesClusterVmMapDaoMock;
62118

63-
kubernetesClusterResourceModifierActionWorker = new KubernetesClusterResourceModifierActionWorker(kubernetesClusterMock, kubernetesClusterManagerMock);
119+
kubernetesClusterResourceModifierActionWorker = new TestKubernetesClusterResourceModifierActionWorker(kubernetesClusterMock, kubernetesClusterManagerMock);
64120
}
65121

66122
@Test
@@ -135,4 +191,67 @@ public void getKubernetesClusterNodeNamePrefixTestNormalizedPrefixShouldNotStart
135191
Mockito.when(kubernetesClusterMock.getName()).thenReturn(originalPrefix);
136192
Assert.assertEquals(expectedPrefix, kubernetesClusterResourceModifierActionWorker.getKubernetesClusterNodeNamePrefix());
137193
}
194+
195+
@Test
196+
public void createVpcTierAclRulesWithoutAclProvisionsRules() throws Exception {
197+
Network network = Mockito.mock(Network.class);
198+
Mockito.when(network.getNetworkACLId()).thenReturn(null);
199+
200+
try (MockedStatic<CallContext> ignored = Mockito.mockStatic(CallContext.class)) {
201+
kubernetesClusterResourceModifierActionWorker.createVpcTierAclRules(network);
202+
}
203+
204+
Assert.assertEquals(2, kubernetesClusterResourceModifierActionWorker.provisionAclRuleCalls);
205+
}
206+
207+
@Test
208+
public void createVpcTierAclRulesWithDefaultAllowDoesNotProvisionRules() throws Exception {
209+
Network network = Mockito.mock(Network.class);
210+
Mockito.when(network.getNetworkACLId()).thenReturn(NetworkACL.DEFAULT_ALLOW);
211+
212+
kubernetesClusterResourceModifierActionWorker.createVpcTierAclRules(network);
213+
214+
Assert.assertEquals(0, kubernetesClusterResourceModifierActionWorker.provisionAclRuleCalls);
215+
}
216+
217+
@Test
218+
public void removeVpcTierAclRulesWithoutAclDoesNotRemoveRules() throws Exception {
219+
Network network = Mockito.mock(Network.class);
220+
Mockito.when(network.getNetworkACLId()).thenReturn(null);
221+
222+
kubernetesClusterResourceModifierActionWorker.removeVpcTierAclRules(network);
223+
224+
Assert.assertEquals(0, kubernetesClusterResourceModifierActionWorker.removeAclRuleCalls);
225+
}
226+
227+
@Test
228+
public void removeVpcTierAclRulesWithCustomAclRemovesRules() throws Exception {
229+
Network network = Mockito.mock(Network.class);
230+
Mockito.when(network.getNetworkACLId()).thenReturn(3L);
231+
232+
kubernetesClusterResourceModifierActionWorker.removeVpcTierAclRules(network);
233+
234+
Assert.assertEquals(2, kubernetesClusterResourceModifierActionWorker.removeAclRuleCalls);
235+
}
236+
237+
@Test
238+
public void setupKubernetesEtcdNetworkRulesWithoutAclProvisionsAclRule() throws Exception {
239+
Network network = Mockito.mock(Network.class);
240+
Mockito.when(network.getVpcId()).thenReturn(1L);
241+
Mockito.when(network.getNetworkACLId()).thenReturn(null);
242+
UserVm etcdVm = Mockito.mock(UserVm.class);
243+
Mockito.when(etcdVm.getId()).thenReturn(1L);
244+
IPAddressVO publicIp = Mockito.mock(IPAddressVO.class);
245+
Mockito.when(ipAddressDaoMock.findByIpAndDcId(Mockito.anyLong(), Mockito.anyString())).thenReturn(publicIp);
246+
247+
TestKubernetesClusterStartWorker worker = new TestKubernetesClusterStartWorker(kubernetesClusterMock, kubernetesClusterManagerMock);
248+
worker.ipAddressDao = ipAddressDaoMock;
249+
worker.owner = accountMock;
250+
worker.publicIpAddress = "192.0.2.1";
251+
252+
worker.setupKubernetesEtcdNetworkRules(List.of(etcdVm), network);
253+
254+
Assert.assertEquals(1, worker.provisionAclRuleCalls);
255+
Assert.assertEquals(1, worker.provisionPortForwardingRuleCalls);
256+
}
138257
}

0 commit comments

Comments
 (0)