Skip to content

keep other clusters' acl rules when deleting a cks cluster on a vpc - #14424

Open
Damans227 wants to merge 1 commit into
apache:mainfrom
Damans227:fix-cks-vpc-acl-delete
Open

Damans227 wants to merge 1 commit into
apache:mainfrom
Damans227:fix-cks-vpc-acl-delete

Conversation

@Damans227

@Damans227 Damans227 commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #14409

Deleting a CKS cluster on a VPC tier removed every allow rule for ports 6443 and 22 in the tier's ACL list, so other clusters using the same list lost their rules. The rules CKS added had nothing tying them to a cluster. Now each rule CKS adds gets the cluster's id in its reason, and delete only removes the rules with that id. For clusters made before this change, whose rules have no reason, delete removes one rule per port instead of all of them.

Test:

On a 2 host KVM lab, make a VPC with a custom ACL list cks-acl and two tiers on it, tier-a and tier-b. Create one CKS cluster on each tier, then delete the one on tier-a and list the rules on cks-acl.

Before: deleting the tier-a cluster also removes the tier-b cluster's 22 and 6443 rules. Rules before the delete:

number = 1  startport = 22
number = 2  startport = 6443
number = 3  startport = 22    reason = Kubernetes cluster 414c6f45-4536-4b20-9139-ccde183f8c2e
number = 4  startport = 6443  reason = Kubernetes cluster 414c6f45-4536-4b20-9139-ccde183f8c2e
number = 5
number = 6

After the delete only 5 and 6 are left, and cluster B (414c6f45...) has no 22 or 6443 rule.

After: only the deleted cluster's rules go. Rules before the delete:

number = 1  startport = 22    reason = Kubernetes cluster 5762bf77-0b61-4c56-85e4-d66423464b63
number = 2  startport = 6443  reason = Kubernetes cluster 5762bf77-0b61-4c56-85e4-d66423464b63
number = 3  startport = 22    reason = Kubernetes cluster 414c6f45-4536-4b20-9139-ccde183f8c2e
number = 4  startport = 6443  reason = Kubernetes cluster 414c6f45-4536-4b20-9139-ccde183f8c2e
number = 5
number = 6

After the delete:

number = 3  startport = 22    reason = Kubernetes cluster 414c6f45-4536-4b20-9139-ccde183f8c2e
number = 4  startport = 6443  reason = Kubernetes cluster 414c6f45-4536-4b20-9139-ccde183f8c2e
number = 5
number = 6

Log from the delete job:

Removed network ACL rule to open up port 6443 on null for Kubernetes cluster KubernetesCluster {"id":3,"name":"cks-a","uuid":"5762bf77-0b61-4c56-85e4-d66423464b63"}
Removed network ACL rule to open up port 22 on null for Kubernetes cluster KubernetesCluster {"id":3,"name":"cks-a","uuid":"5762bf77-0b61-4c56-85e4-d66423464b63"}

Unit tests also cover three 6443 rules in one list (cluster A, cluster B, and one older rule with no reason): deleting A removes only A's rule.

@Damans227

Copy link
Copy Markdown
Collaborator Author

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@Damans227 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

@codecov

codecov Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 20.40%. Comparing base (d3f0700) to head (f38b135).

Files with missing lines Patch % Lines
...KubernetesClusterResourceModifierActionWorker.java 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               main   #14424   +/-   ##
=========================================
  Coverage     20.39%   20.40%           
- Complexity    21283    21287    +4     
=========================================
  Files          6481     6481           
  Lines        588040   588052   +12     
  Branches      72021    72022    +1     
=========================================
+ Hits         119957   119970   +13     
+ Misses       455052   455045    -7     
- Partials      13031    13037    +6     
Flag Coverage Δ
uitests 3.70% <ø> (ø)
unittests 21.70% <92.30%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19586

@Damans227
Damans227 marked this pull request as ready for review October 16, 2026 19:40

This branch has not been deployed

No deployments
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 on VPC: deleting a cluster removes ACL rules used by other clusters

2 participants