Dogface2k commented on code in PR #13762:
URL: https://github.com/apache/cloudstack/pull/13762#discussion_r3980741641
##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java:
##########
@@ -717,7 +717,7 @@ protected void
setupKubernetesClusterIsolatedNetworkRules(IpAddress publicIp, Ne
}
protected void createVpcTierAclRules(Network network) throws
ManagementServerException {
- if (network.getNetworkACLId() == NetworkACL.DEFAULT_ALLOW) {
+ if (Objects.equals(network.getNetworkACLId(),
NetworkACL.DEFAULT_ALLOW)) {
Review Comment:
@sureshanaparti Thanks, I see why the two guards look inconsistent. The
difference is intentional, creation needs to reach the existing ACL-service
path when the tier has no ACL yet, whereas cleanup has nothing to revoke if the
tier still has no ACL.
provisionVpcTierAllowPortACLRule() supplies the network ID to
createNetworkACLItem(). On 4.22, createAclListIfNeeded() reloads the network
and creates and attaches an ACL when needed, subject to the existing access and
NetworkACL-offering checks. Objects.equals() already makes this comparison
null-safe. Returning on null here would skip the API and SSH ACL-rule
provisioning instead of reaching that path.
The destroy worker reads the network again before tier-rule cleanup. When it
calls removeVpcTierAclRules(), the null check is against that reloaded record,
not the original value from creation. The regression tests check the
create/remove distinction at the provisioning-helper boundary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
