Skip to content

CKS: handle VPC tier with no attached network ACL - #13961

Open
goal86sg wants to merge 1 commit into
apache:mainfrom
goal86sg:cks-null-acl-fix-13761
Open

CKS: handle VPC tier with no attached network ACL#13961
goal86sg wants to merge 1 commit into
apache:mainfrom
goal86sg:cks-null-acl-fix-13761

Conversation

@goal86sg

Copy link
Copy Markdown

Description

Fixes #13761

A VPC tier without an attached network 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 / NetworkACL.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.

Root cause

NetworkACL.DEFAULT_ALLOW (=2) and NetworkACL.DEFAULT_DENY (=1) are primitive long constants (api/.../vpc/NetworkACL.java). So an expression like network.getNetworkACLId() == NetworkACL.DEFAULT_ALLOW auto-unboxes the nullable Long and throws NullPointerException: Cannot invoke "java.lang.Long.longValue()" when the tier has no ACL attached.

Fix

Make the four comparisons null-safe with Objects.equals (value comparison, no unboxing):

# Class Method Before After
1 KubernetesClusterManagerImpl validateVpcTier == DEFAULT_DENY Objects.equals(..., DEFAULT_DENY) — null is a valid state, not rejected
2 KubernetesClusterResourceModifierActionWorker createVpcTierAclRules == DEFAULT_ALLOW (early return) Objects.equals(..., DEFAULT_ALLOW) — null falls through to provisioning
3 KubernetesClusterResourceModifierActionWorker removeVpcTierAclRules == DEFAULT_ALLOW (early return) null || Objects.equals(..., DEFAULT_ALLOW) — no ACL ⇒ no-op on delete
4 KubernetesClusterStartWorker setupKubernetesEtcdNetworkRules != DEFAULT_ALLOW !Objects.equals(..., DEFAULT_ALLOW) — null falls through to provisioning

This lets CKS reach the existing NetworkACLService auto-create path (NetworkACLServiceImpl.createAclListIfNeeded), which creates and attaches a custom ACL when a rule is added with networkid and no aclid — exactly the behavior the issue expects. The downstream paths are already null-safe (NetworkACLItemDaoImpl.listByACL(null) returns an empty list), so only the four comparisons needed changing.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • New regression unit tests, each reproducing the NPE on main before this change and passing after:
    • KubernetesClusterManagerImplTest#testValidateVpcTierNullAclId — null ACL is accepted (no NPE / no rejection).
    • KubernetesClusterResourceModifierActionWorkerTest#removeVpcTierAclRulesNullAclIdIsNoOp — delete with no ACL is a no-op (no NPE).
  • Ran the kubernetes-service plugin unit-test module locally (JDK 17): all tests pass — KubernetesClusterManagerImplTest 48/48 and KubernetesClusterResourceModifierActionWorkerTest 8/8 — including the two new regression tests.
  • The two remaining nullable-ACL paths (createVpcTierAclRules, setupKubernetesEtcdNetworkRules) use the identical null-safe Objects.equals pattern and are exercised by the reporter's regression suite referenced in CKS lifecycle fails on VPC tier without an attached network ACL #13761; the existing CKS Marvin/integration tests cover the broader create/start/delete lifecycle.

Checklist

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • I have signed off my commits (DCO).
  • I have added tests that prove my fix is effective.

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 apache#13761

Signed-off-by: Desmond <60381871+goal86sg@users.noreply.github.com>
@goal86sg
goal86sg force-pushed the cks-null-acl-fix-13761 branch from 727a784 to 9daa97e Compare August 24, 2026 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CKS lifecycle fails on VPC tier without an attached network ACL

1 participant