vishesh92 commented on code in PR #12706:
URL: https://github.com/apache/cloudstack/pull/12706#discussion_r2871845201
##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/KubernetesClusterManagerImpl.java:
##########
@@ -2713,9 +2713,8 @@ private void createNetworkOfferingForKubernetes(String
offeringName, String offe
defaultKubernetesServiceNetworkOfferingProviders.put(Service.UserData,
provider);
if (forVpc) {
defaultKubernetesServiceNetworkOfferingProviders.put(Service.NetworkACL, forNsx
? Network.Provider.Nsx : provider);
- } else {
-
defaultKubernetesServiceNetworkOfferingProviders.put(Service.Firewall, forNsx ?
Network.Provider.Nsx : provider);
}
+ defaultKubernetesServiceNetworkOfferingProviders.put(Service.Firewall,
Network.Provider.VPCVirtualRouter);
Review Comment:
```suggestion
defaultKubernetesServiceNetworkOfferingProviders.put(Service.Firewall, forNsx ?
Network.Provider.Nsx : provider);
```
##########
engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42210to42300.java:
##########
@@ -42,4 +50,126 @@ public InputStream[] getPrepareScripts() {
return new InputStream[] {script};
}
+
+ @Override
+ public void performDataMigration(Connection conn) {
+ updateNetworkDefaultOfferingsForVPCWithFirewallService(conn);
+ updateVpcOfferingsWithFirewallService(conn);
+ }
+
+ private void
updateNetworkDefaultOfferingsForVPCWithFirewallService(Connection conn) {
Review Comment:
I am not sure if we should update the existing service offerings. IMO, it
would be better to add new service offerings. This will change the behavior of
a network after a network restart with cleanup.
##########
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java:
##########
@@ -439,7 +442,10 @@ protected void executeViewScripts() {
Connection conn = txn.getConnection();
for (String filePath : filesPathUnderViewsDirectory) {
- LOGGER.debug("Executing VIEW script [{}].", filePath);
+ LOGGER.debug(String.format("Executing VIEW script [%s].",
filePath));
Review Comment:
```suggestion
LOGGER.debug("Executing VIEW script [{}].", filePath);
```
##########
server/src/main/java/com/cloud/server/ConfigurationServerImpl.java:
##########
@@ -1186,6 +1188,7 @@ public void
doInTransactionWithoutResult(TransactionStatus status) {
internalLbOffProviders.put(Service.Gateway,
Provider.VPCVirtualRouter);
internalLbOffProviders.put(Service.Lb, Provider.InternalLbVm);
internalLbOffProviders.put(Service.SourceNat,
Provider.VPCVirtualRouter);
+ internalLbOffProviders.put(Service.Firewall,
Provider.VPCVirtualRouter);
Review Comment:
I am not sure about this. But if the LB is internal, the firewall will work
only on public traffic for VPC Virtual router. Is this change required here?
##########
systemvm/debian/opt/cloud/bin/configure.py:
##########
@@ -705,8 +705,8 @@ def process(self):
for item in self.dbag:
if item == "id":
- continue
- if self.config.is_vpc():
+ continue
+ if self.config.is_vpc() and not ("purpose" in self.dbag[item] and
self.dbag[item]["purpose"] == "Firewall"):
Review Comment:
```suggestion
continue
if self.config.is_vpc() and not ("purpose" in self.dbag[item]
and self.dbag[item]["purpose"] == "Firewall"):
```
##########
ui/src/views/offering/AddVpcOffering.vue:
##########
@@ -525,6 +525,12 @@ export default {
{ name: 'ConfigDrive' }
]
})
+ services.push({
+ name: 'Firewall',
+ provider: [
+ { name: 'VpcVirtualRouter' }
Review Comment:
I am not sure if we can add providers (NSX, etc.) other than
`VpcVirtualRouter` here.
##########
systemvm/debian/opt/cloud/bin/configure.py:
##########
@@ -705,8 +705,8 @@ def process(self):
for item in self.dbag:
if item == "id":
- continue
- if self.config.is_vpc():
+ continue
+ if self.config.is_vpc() and not ("purpose" in self.dbag[item] and
self.dbag[item]["purpose"] == "Firewall"):
Review Comment:
I am not exactly sure about this change.
##########
systemvm/debian/opt/cloud/bin/cs/CsAddress.py:
##########
@@ -632,6 +632,18 @@ def fw_vpcrouter(self):
(self.address['network'],
self.address['network'])])
if self.get_type() in ["public"]:
+ # Add PREROUTING firewall chain jump for public IP
+ self.fw.append(["mangle", "front",
Review Comment:
We seem to be adding this for all the VPC Offerings which might not have
firewall as a supported service.
##########
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java:
##########
@@ -327,7 +327,10 @@ protected void executeProcedureScripts() {
Connection conn = txn.getConnection();
for (String filePath : filesPathUnderViewsDirectory) {
- LOGGER.debug("Executing PROCEDURE script [{}].", filePath);
+ LOGGER.debug(String.format("Executing PROCEDURE script [%s].",
filePath));
Review Comment:
```suggestion
LOGGER.debug("Executing PROCEDURE script [{}].", filePath);
```
--
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]
