From 9daa97e270b9c1123a1e01daeda16f452df02a6e Mon Sep 17 00:00:00 2001 From: Desmond <60381871+goal86sg@users.noreply.github.com> Date: Mon, 24 Aug 2026 23:01:35 +0800 Subject: [PATCH] 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> --- .../cluster/KubernetesClusterManagerImpl.java | 2 +- ...KubernetesClusterResourceModifierActionWorker.java | 4 ++-- .../actionworkers/KubernetesClusterStartWorker.java | 2 +- .../cluster/KubernetesClusterManagerImplTest.java | 10 ++++++++++ ...rnetesClusterResourceModifierActionWorkerTest.java | 11 +++++++++++ 5 files changed, 25 insertions(+), 4 deletions(-) diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java index d1264a90fdb5..7e0af62105af 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java @@ -611,7 +611,7 @@ protected void validateVpcTier(Network network) { if (Network.State.Allocated.equals(network.getState())) { // Allocated networks won't have IP and rules return; } - if (network.getNetworkACLId() == NetworkACL.DEFAULT_DENY) { + if (Objects.equals(network.getNetworkACLId(), NetworkACL.DEFAULT_DENY)) { throw new InvalidParameterValueException(String.format("Network ID: %s can not be used for Kubernetes cluster as it uses default deny ACL", network.getUuid())); } } diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java index a1b2294bcd4d..edd697224b43 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java @@ -752,7 +752,7 @@ protected void setupKubernetesClusterIsolatedNetworkRules(IpAddress publicIp, Ne } protected void createVpcTierAclRules(Network network) throws ManagementServerException { - if (network.getNetworkACLId() == NetworkACL.DEFAULT_ALLOW) { + if (Objects.equals(network.getNetworkACLId(), NetworkACL.DEFAULT_ALLOW)) { return; } // ACL rule for API access for control node VMs @@ -781,7 +781,7 @@ protected void createVpcTierAclRules(Network network) throws ManagementServerExc } protected void removeVpcTierAclRules(Network network) throws ManagementServerException { - if (network.getNetworkACLId() == NetworkACL.DEFAULT_ALLOW) { + if (network.getNetworkACLId() == null || Objects.equals(network.getNetworkACLId(), NetworkACL.DEFAULT_ALLOW)) { return; } // ACL rule for API access for control node VMs diff --git a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java index 308fc07223de..940957a9edac 100644 --- a/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java +++ b/plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java @@ -646,7 +646,7 @@ protected void setupKubernetesEtcdNetworkRules(List etcdVms, Network net try { if (Objects.isNull(network.getVpcId())) { provisionFirewallRules(publicIp, owner, etcdStartPort, etcdStartPort); - } else if (network.getNetworkACLId() != NetworkACL.DEFAULT_ALLOW) { + } else if (!Objects.equals(network.getNetworkACLId(), NetworkACL.DEFAULT_ALLOW)) { try { provisionVpcTierAllowPortACLRule(network, ETCD_NODE_CLIENT_REQUEST_PORT, ETCD_NODE_CLIENT_REQUEST_PORT); if (logger.isInfoEnabled()) { diff --git a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java index 1fab5420c3c3..60b77c4b66d5 100644 --- a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java +++ b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java @@ -145,6 +145,16 @@ public void testValidateVpcTierValid() { kubernetesClusterManager.validateVpcTier(network); } + @Test + public void testValidateVpcTierNullAclId() { + // A VPC tier with no attached ACL is a valid state (aclid is optional on createNetwork). + // Validation must not NPE by unboxing the nullable Long against the primitive long DEFAULT_DENY. See GH-13761. + Network network = Mockito.mock(Network.class); + Mockito.when(network.getState()).thenReturn(Network.State.Implemented); + Mockito.when(network.getNetworkACLId()).thenReturn(null); + kubernetesClusterManager.validateVpcTier(network); + } + @Test public void validateIsolatedNetworkIpRulesNoRules() { long ipId = 1L; diff --git a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorkerTest.java b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorkerTest.java index c220a3468afb..99743658e44e 100644 --- a/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorkerTest.java +++ b/plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorkerTest.java @@ -23,6 +23,7 @@ import com.cloud.kubernetes.cluster.dao.KubernetesClusterDetailsDao; import com.cloud.kubernetes.cluster.dao.KubernetesClusterVmMapDao; import com.cloud.kubernetes.version.dao.KubernetesSupportedVersionDao; +import com.cloud.network.Network; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -135,4 +136,14 @@ public void getKubernetesClusterNodeNamePrefixTestNormalizedPrefixShouldNotStart Mockito.when(kubernetesClusterMock.getName()).thenReturn(originalPrefix); Assert.assertEquals(expectedPrefix, kubernetesClusterResourceModifierActionWorker.getKubernetesClusterNodeNamePrefix()); } + + @Test + public void removeVpcTierAclRulesNullAclIdIsNoOp() throws Exception { + // Deleting a cluster from a VPC tier that still has no ACL attached must be a no-op, + // not an unboxing NullPointerException. See GH-13761. + Network network = Mockito.mock(Network.class); + Mockito.when(network.getNetworkACLId()).thenReturn(null); + kubernetesClusterResourceModifierActionWorker.removeVpcTierAclRules(network); + // Reaching here without an exception is the regression assertion. + } }