CKS: handle VPC tiers without attached ACLs - #13762
Open
Dogface2k wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.