Skip to content

Commit 727a784

Browse files
committed
CKS: handle VPC tier with no attached network ACL
A VPC tier without an attached ACL is a valid, supported state (aclid is optional on createNetwork; CloudStack stopped assigning a default-deny ACL unconditionally in CLOUDSTACK-2809). However, four CKS lifecycle sites compared the nullable Long returned by Network.getNetworkACLId() against the primitive long constants NetworkACL.DEFAULT_ALLOW / DEFAULT_DENY, auto-unboxing it and throwing NullPointerException when the tier had no ACL attached. This broke CKS cluster create/start/delete on a legitimate VPC tier configuration and left clusters stuck in Starting. Make the four comparisons null-safe with Objects.equals so that: - validateVpcTier accepts a null ACL (a valid state) instead of NPE-ing; - createVpcTierAclRules and setupKubernetesEtcdNetworkRules reach the existing NetworkACLService auto-create path (NetworkACLServiceImpl.createAclListIfNeeded) that creates and attaches a custom ACL when a rule is added with networkid and no aclid; - removeVpcTierAclRules treats a missing ACL as a no-op on delete. Adds regression unit tests: - KubernetesClusterManagerImplTest#testValidateVpcTierNullAclId - KubernetesClusterResourceModifierActionWorkerTest#removeVpcTierAclRulesNullAclIdIsNoOp Fixes #13761 Signed-off-by: Desmond <60381871+goal86sg@users.noreply.github.com>
1 parent 8902c32 commit 727a784

5 files changed

Lines changed: 25 additions & 4 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
@@ -611,7 +611,7 @@ protected void validateVpcTier(Network network) {
611611
if (Network.State.Allocated.equals(network.getState())) { // Allocated networks won't have IP and rules
612612
return;
613613
}
614-
if (network.getNetworkACLId() == NetworkACL.DEFAULT_DENY) {
614+
if (Objects.equals(network.getNetworkACLId(), NetworkACL.DEFAULT_DENY)) {
615615
throw new InvalidParameterValueException(String.format("Network ID: %s can not be used for Kubernetes cluster as it uses default deny ACL", network.getUuid()));
616616
}
617617
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -752,7 +752,7 @@ protected void setupKubernetesClusterIsolatedNetworkRules(IpAddress publicIp, Ne
752752
}
753753

754754
protected void createVpcTierAclRules(Network network) throws ManagementServerException {
755-
if (network.getNetworkACLId() == NetworkACL.DEFAULT_ALLOW) {
755+
if (Objects.equals(network.getNetworkACLId(), NetworkACL.DEFAULT_ALLOW)) {
756756
return;
757757
}
758758
// ACL rule for API access for control node VMs
@@ -781,7 +781,7 @@ protected void createVpcTierAclRules(Network network) throws ManagementServerExc
781781
}
782782

783783
protected void removeVpcTierAclRules(Network network) throws ManagementServerException {
784-
if (network.getNetworkACLId() == NetworkACL.DEFAULT_ALLOW) {
784+
if (network.getNetworkACLId() == null || Objects.equals(network.getNetworkACLId(), NetworkACL.DEFAULT_ALLOW)) {
785785
return;
786786
}
787787
// 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: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,16 @@ public void testValidateVpcTierValid() {
145145
kubernetesClusterManager.validateVpcTier(network);
146146
}
147147

148+
@Test
149+
public void testValidateVpcTierNullAclId() {
150+
// A VPC tier with no attached ACL is a valid state (aclid is optional on createNetwork).
151+
// Validation must not NPE by unboxing the nullable Long against the primitive long DEFAULT_DENY. See GH-13761.
152+
Network network = Mockito.mock(Network.class);
153+
Mockito.when(network.getState()).thenReturn(Network.State.Implemented);
154+
Mockito.when(network.getNetworkACLId()).thenReturn(null);
155+
kubernetesClusterManager.validateVpcTier(network);
156+
}
157+
148158
@Test
149159
public void validateIsolatedNetworkIpRulesNoRules() {
150160
long ipId = 1L;

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.cloud.kubernetes.cluster.dao.KubernetesClusterDetailsDao;
2424
import com.cloud.kubernetes.cluster.dao.KubernetesClusterVmMapDao;
2525
import com.cloud.kubernetes.version.dao.KubernetesSupportedVersionDao;
26+
import com.cloud.network.Network;
2627
import org.junit.Assert;
2728
import org.junit.Before;
2829
import org.junit.Test;
@@ -135,4 +136,14 @@ public void getKubernetesClusterNodeNamePrefixTestNormalizedPrefixShouldNotStart
135136
Mockito.when(kubernetesClusterMock.getName()).thenReturn(originalPrefix);
136137
Assert.assertEquals(expectedPrefix, kubernetesClusterResourceModifierActionWorker.getKubernetesClusterNodeNamePrefix());
137138
}
139+
140+
@Test
141+
public void removeVpcTierAclRulesNullAclIdIsNoOp() throws Exception {
142+
// Deleting a cluster from a VPC tier that still has no ACL attached must be a no-op,
143+
// not an unboxing NullPointerException. See GH-13761.
144+
Network network = Mockito.mock(Network.class);
145+
Mockito.when(network.getNetworkACLId()).thenReturn(null);
146+
kubernetesClusterResourceModifierActionWorker.removeVpcTierAclRules(network);
147+
// Reaching here without an exception is the regression assertion.
148+
}
138149
}

0 commit comments

Comments
 (0)