CKS: handle VPC tiers without attached ACLs - #13762
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes CKS (Kubernetes Service) lifecycle failures when a VPC tier has no network ACL attached by making all Network.getNetworkACLId() comparisons null-safe, allowing the existing “lazy custom ACL creation on first rule” path to work as intended.
Changes:
- Replaced primitive ACL ID comparisons with
Objects.equals(...)/ null checks to avoidLongauto-unboxing NPEs in validation, provisioning, and cleanup paths. - Updated etcd network-rule provisioning logic to treat a null ACL the same as “not default-allow” (i.e., proceed to provision tier ACL rules).
- Added regression tests covering validation, rule provisioning, etcd setup, and cleanup behavior when
networkACLIdis null.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java | Makes VPC-tier validation reject only explicit default-deny ACLs using a null-safe comparison. |
| plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterStartWorker.java | Prevents NPE during etcd rule setup by using a null-safe default-allow ACL check. |
| plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java | Prevents NPEs and preserves semantics by making VPC-tier ACL rule create/remove logic null-safe. |
| plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImplTest.java | Adds a regression test ensuring VPC-tier validation accepts a null ACL ID without throwing. |
| plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorkerTest.java | Adds regression tests ensuring provisioning/cleanup/etcd ACL behavior is correct when ACL ID is null. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 4.22 #13762 +/- ##
=========================================
Coverage 17.69% 17.69%
- Complexity 15833 15836 +3
=========================================
Files 5925 5925
Lines 533534 533535 +1
Branches 65273 65274 +1
=========================================
+ Hits 94421 94433 +12
+ Misses 428434 428416 -18
- Partials 10679 10686 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18741 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
Description
This PR prevents CKS lifecycle operations from failing when a VPC tier does not yet have a network ACL attached.
A nullable ACL reference is an existing supported state:
createNetworkaccepts a VPC tier withoutaclid, andNetworkACLServiceImpl.createAclListIfNeededcreates and attaches a custom ACL when a rule is first added using the tier'snetworkId. CKS currently comparesNetwork.getNetworkACLId()with primitive default-ACL constants before reaching that service path, which auto-unboxes a nullLongand throwsNullPointerException.The CKS comparisons are now null-safe while preserving the existing semantics:
This does not change VPC tier creation, API behavior, schema nullability, UI behavior, or Network ACL defaults.
Fixes: #13761
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
Not applicable.
How Has This Been Tested?
The failure was reproduced on Apache CloudStack 4.22.1.0 in an NSX-backed VMware environment through the public API, using a supported NSX VPC offering and CKS tier offering. Creating the tier without the optional
aclidleft its ACL reference unset. CKS then provisioned the cluster VMs and failed with the reportedLong.longValue()NPE. Because create never recorded the cluster endpoint, a later start failed endpoint validation; delete independently reached the second null-unboxing NPE in ACL cleanup.Negative-control verification was performed with the new regression tests present but the production fix absent. These errors are expected and prove that the tests reach the unsafe comparisons:
The actual patched
4.22PR branch was tested with Java 17 and is all green:The Maven reactor built all 33 required modules successfully, including the Kubernetes Service plugin, with zero Checkstyle violations.
How did you try to break this feature and the system with this change?
The regression coverage exercises all nullable comparisons changed by this PR:
The existing 104 Network ACL service tests also pass, including coverage of the supported null-ACL tier path and lazy custom-ACL attachment. No generic exception handling, database default, API fallback, or retry was introduced.