Skip to content

CKS: handle VPC tiers without attached ACLs - #13762

Open
Dogface2k wants to merge 1 commit into
apache:4.22from
Dogface2k:agent/cks-null-vpc-tier-acl
Open

CKS: handle VPC tiers without attached ACLs#13762
Dogface2k wants to merge 1 commit into
apache:4.22from
Dogface2k:agent/cks-null-vpc-tier-acl

Conversation

@Dogface2k

@Dogface2k Dogface2k commented Jul 31, 2026

Copy link
Copy Markdown

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: createNetwork accepts a VPC tier without aclid, and NetworkACLServiceImpl.createAclListIfNeeded creates and attaches a custom ACL when a rule is first added using the tier's networkId. CKS currently compares Network.getNetworkACLId() with primitive default-ACL constants before reaching that service path, which auto-unboxes a null Long and throws NullPointerException.

The CKS comparisons are now null-safe while preserving the existing semantics:

  • validation rejects only an explicitly attached default-deny ACL;
  • create and etcd provisioning continue into the existing ACL auto-creation path when no ACL is attached;
  • cleanup treats a missing ACL as having no ACL rules to remove;
  • default-allow and custom ACL behavior is unchanged.

This does not change VPC tier creation, API behavior, schema nullability, UI behavior, or Network ACL defaults.

Fixes: #13761

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • Build/CI
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

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 aclid left its ACL reference unset. CKS then provisioned the cluster VMs and failed with the reported Long.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:

Current 4.22 target branch:
Tests run: 42, Failures: 0, Errors: 4, Skipped: 0
All four errors: the expected Long.longValue() null auto-unboxing NPE
Other 38 focused tests: passed

Current main at 4f117071c9397b1e4714c8fb61c384883c872842:
Tests run: 60, Failures: 0, Errors: 4, Skipped: 0
All four errors: the expected Long.longValue() null auto-unboxing NPE
Other 56 focused tests: passed

The actual patched 4.22 PR branch was tested with Java 17 and is all green:

Focused CKS tests:
Tests run: 42, Failures: 0, Errors: 0, Skipped: 0

Full Kubernetes Service module:
Tests run: 84, Failures: 0, Errors: 0, Skipped: 0

NetworkACLServiceImplTest:
Tests run: 104, Failures: 0, Errors: 0, Skipped: 0

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:

  • an implemented VPC tier with no ACL passes CKS validation without unboxing;
  • cluster API ACL rule provisioning proceeds when the ACL is initially absent;
  • etcd ACL rule provisioning proceeds when the ACL is initially absent;
  • cleanup on a tier that still has no ACL performs no deletion;
  • a default-allow ACL remains a no-op during provisioning;
  • a custom ACL still removes both CKS rules during cleanup.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

1 participant