Damans227 commented on code in PR #14165:
URL: https://github.com/apache/cloudstack/pull/14165#discussion_r4283370489
##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java:
##########
@@ -544,16 +544,23 @@ protected FirewallRule removeApiFirewallRule(final
IpAddress publicIp) {
}
protected FirewallRule removeSshFirewallRule(final IpAddress publicIp,
final long networkId) {
- FirewallRule rule = null;
+ FirewallRuleVO rule = null;
List<FirewallRuleVO> firewallRules =
firewallRulesDao.listByIpPurposeProtocolAndNotRevoked(publicIp.getId(),
FirewallRule.Purpose.Firewall, NetUtils.TCP_PROTO);
for (FirewallRuleVO firewallRule : firewallRules) {
- PortForwardingRuleVO pfRule =
portForwardingRulesDao.findByNetworkAndPorts(networkId,
firewallRule.getSourcePortStart(), firewallRule.getSourcePortEnd());
- if (Objects.equals(firewallRule.getSourcePortStart(),
CLUSTER_NODES_DEFAULT_START_SSH_PORT) || (Objects.nonNull(pfRule) &&
pfRule.getDestinationPortStart() == DEFAULT_SSH_PORT) ) {
+ if (Objects.equals(firewallRule.getSourcePortStart(),
CLUSTER_NODES_DEFAULT_START_SSH_PORT)) {
rule = firewallRule;
- firewallService.revokeIngressFwRule(firewallRule.getId(),
true);
- logger.debug("The SSH firewall rule {} with the id {} was
revoked", firewallRule.getName(), firewallRule.getId());
break;
}
+ if (rule == null) {
+ PortForwardingRuleVO pfRule =
portForwardingRulesDao.findByNetworkAndPorts(networkId,
firewallRule.getSourcePortStart(), firewallRule.getSourcePortEnd());
+ if (Objects.nonNull(pfRule) &&
pfRule.getDestinationPortStart() == DEFAULT_SSH_PORT) {
Review Comment:
if the 2222 rule was deleted by hand, can this still pick an etcd rule and
wipe the etcd forwarding again? the caller already falls back to a safe range
when nothing is found
##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java:
##########
@@ -544,16 +544,23 @@ protected FirewallRule removeApiFirewallRule(final
IpAddress publicIp) {
}
protected FirewallRule removeSshFirewallRule(final IpAddress publicIp,
final long networkId) {
- FirewallRule rule = null;
+ FirewallRuleVO rule = null;
List<FirewallRuleVO> firewallRules =
firewallRulesDao.listByIpPurposeProtocolAndNotRevoked(publicIp.getId(),
FirewallRule.Purpose.Firewall, NetUtils.TCP_PROTO);
for (FirewallRuleVO firewallRule : firewallRules) {
- PortForwardingRuleVO pfRule =
portForwardingRulesDao.findByNetworkAndPorts(networkId,
firewallRule.getSourcePortStart(), firewallRule.getSourcePortEnd());
- if (Objects.equals(firewallRule.getSourcePortStart(),
CLUSTER_NODES_DEFAULT_START_SSH_PORT) || (Objects.nonNull(pfRule) &&
pfRule.getDestinationPortStart() == DEFAULT_SSH_PORT) ) {
+ if (Objects.equals(firewallRule.getSourcePortStart(),
CLUSTER_NODES_DEFAULT_START_SSH_PORT)) {
rule = firewallRule;
- firewallService.revokeIngressFwRule(firewallRule.getId(),
true);
- logger.debug("The SSH firewall rule {} with the id {} was
revoked", firewallRule.getName(), firewallRule.getId());
break;
}
+ if (rule == null) {
+ PortForwardingRuleVO pfRule =
portForwardingRulesDao.findByNetworkAndPorts(networkId,
firewallRule.getSourcePortStart(), firewallRule.getSourcePortEnd());
+ if (Objects.nonNull(pfRule) &&
pfRule.getDestinationPortStart() == DEFAULT_SSH_PORT) {
+ rule = firewallRule;
+ }
+ }
+ }
+ if (rule != null) {
Review Comment:
on destroy in a network the user keeps, should the etcd rules from 50000 up
go too? they stay open and the next cluster with etcd nodes would clash on
those ports
--
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]
