Open egress firewall for UDP and ICMP too

Signed-off-by: Hans Rakers <[email protected]>

Author: hrak

docs/book/src/clustercloudstack/configuration.md

@@ -61,7 +61,8 @@ cmk list zones listall=true | jq '.zone[] | {name, id}'
 
 The network must be declared as an environment variable `CLOUDSTACK_NETWORK_NAME` and is a mandatory parameter.
 As of now, only isolated and shared networks are supported.
-If the specified network does not exist, a new isolated network will be created.
+
+If the specified network does not exist, a new isolated network will be created. The newly created network will have a default egress firewall policy that allows all TCP, UDP and ICMP traffic from the cluster to the outside world.
 
 The list of networks for the specific zone can be fetched using the cmk cli as follows :
 ```

pkg/cloud/isolated_network.go

@@ -110,12 +110,26 @@ func (c *client) CreateIsolatedNetwork(fd *infrav1.CloudStackFailureDomain, isoN
 	return c.AddCreatedByCAPCTag(ResourceTypeNetwork, isoNet.Spec.ID)
 }
 
-// OpenFirewallRules opens a CloudStack firewall for an isolated network.
+// OpenFirewallRules opens a CloudStack egress firewall for an isolated network.
 func (c *client) OpenFirewallRules(isoNet *infrav1.CloudStackIsolatedNetwork) (retErr error) {
-	p := c.cs.Firewall.NewCreateEgressFirewallRuleParams(isoNet.Spec.ID, NetworkProtocolTCP)
-	_, retErr = c.cs.Firewall.CreateEgressFirewallRule(p)
-	if retErr != nil && strings.Contains(strings.ToLower(retErr.Error()), "there is already") { // Already a firewall rule here.
-		retErr = nil
+	protocols := []string{NetworkProtocolTCP, NetworkProtocolUDP, NetworkProtocolICMP}
+	for _, proto := range protocols {
+		p := c.cs.Firewall.NewCreateEgressFirewallRuleParams(isoNet.Spec.ID, proto)
+
+		if proto == "icmp" {
+			p.SetIcmptype(-1)
+			p.SetIcmpcode(-1)
+		}
+
+		_, err := c.cs.Firewall.CreateEgressFirewallRule(p)
+		if err != nil &&
+			// Ignore errors regarding already existing fw rules for TCP/UDP
+			!strings.Contains(strings.ToLower(err.Error()), "there is already") &&
+			// Ignore errors regarding already existing fw rule for ICMP
+			!strings.Contains(strings.ToLower(err.Error()), "new rule conflicts with existing rule") {
+			retErr = multierror.Append(retErr, errors.Wrapf(
+				err, "failed creating egress firewall rule for network ID %s protocol %s", isoNet.Spec.ID, proto))
+		}
 	}
 	c.customMetrics.EvaluateErrorAndIncrementAcsReconciliationErrorCounter(retErr)
 	return retErr

pkg/cloud/isolated_network_test.go

@@ -84,10 +84,24 @@ var _ = Describe("Network", func() {
 					PublicIpAddresses: []*csapi.PublicIpAddress{{Id: dummies.PublicIPID, Ipaddress: "fakeIP"}}}, nil)
 			as.EXPECT().NewAssociateIpAddressParams().Return(&csapi.AssociateIpAddressParams{})
 			as.EXPECT().AssociateIpAddress(gomock.Any())
-			fs.EXPECT().NewCreateEgressFirewallRuleParams(dummies.ISONet1.ID, cloud.NetworkProtocolTCP).
-				Return(&csapi.CreateEgressFirewallRuleParams{})
-			fs.EXPECT().CreateEgressFirewallRule(&csapi.CreateEgressFirewallRuleParams{}).
-				Return(&csapi.CreateEgressFirewallRuleResponse{}, nil)
+			fs.EXPECT().NewCreateEgressFirewallRuleParams(dummies.ISONet1.ID, gomock.Any()).
+				DoAndReturn(func(networkid string, protocol string) *csapi.CreateEgressFirewallRuleParams {
+					p := &csapi.CreateEgressFirewallRuleParams{}
+					if protocol == "icmp" {
+						p.SetIcmptype(-1)
+						p.SetIcmpcode(-1)
+					}
+					return p
+				}).Times(3)
+
+			ruleParamsICMP := &csapi.CreateEgressFirewallRuleParams{}
+			ruleParamsICMP.SetIcmptype(-1)
+			ruleParamsICMP.SetIcmpcode(-1)
+			gomock.InOrder(
+				fs.EXPECT().CreateEgressFirewallRule(&csapi.CreateEgressFirewallRuleParams{}).
+					Return(&csapi.CreateEgressFirewallRuleResponse{}, nil).Times(2),
+				fs.EXPECT().CreateEgressFirewallRule(ruleParamsICMP).
+					Return(&csapi.CreateEgressFirewallRuleResponse{}, nil))
 
 			// Will add cluster tag once to Network and once to PublicIP.
 			createdByResponse := &csapi.ListTagsResponse{Tags: []*csapi.Tag{{Key: cloud.CreatedByCAPCTagName, Value: "1"}}}
@@ -124,10 +138,24 @@ var _ = Describe("Network", func() {
 	Context("for a closed firewall", func() {
 		It("OpenFirewallRule asks CloudStack to open the firewall", func() {
 			dummies.Zone1.Network = dummies.ISONet1
-			fs.EXPECT().NewCreateEgressFirewallRuleParams(dummies.ISONet1.ID, cloud.NetworkProtocolTCP).
-				Return(&csapi.CreateEgressFirewallRuleParams{})
-			fs.EXPECT().CreateEgressFirewallRule(&csapi.CreateEgressFirewallRuleParams{}).
-				Return(&csapi.CreateEgressFirewallRuleResponse{}, nil)
+			fs.EXPECT().NewCreateEgressFirewallRuleParams(dummies.ISONet1.ID, gomock.Any()).
+				DoAndReturn(func(networkid string, protocol string) *csapi.CreateEgressFirewallRuleParams {
+					p := &csapi.CreateEgressFirewallRuleParams{}
+					if protocol == "icmp" {
+						p.SetIcmptype(-1)
+						p.SetIcmpcode(-1)
+					}
+					return p
+				}).Times(3)
+
+			ruleParamsICMP := &csapi.CreateEgressFirewallRuleParams{}
+			ruleParamsICMP.SetIcmptype(-1)
+			ruleParamsICMP.SetIcmpcode(-1)
+			gomock.InOrder(
+				fs.EXPECT().CreateEgressFirewallRule(&csapi.CreateEgressFirewallRuleParams{}).
+					Return(&csapi.CreateEgressFirewallRuleResponse{}, nil).Times(2),
+				fs.EXPECT().CreateEgressFirewallRule(ruleParamsICMP).
+					Return(&csapi.CreateEgressFirewallRuleResponse{}, nil))
 
 			Ω(client.OpenFirewallRules(dummies.CSISONet1)).Should(Succeed())
 		})
@@ -137,10 +165,24 @@ var _ = Describe("Network", func() {
 		It("OpenFirewallRule asks CloudStack to open the firewall anyway, but doesn't fail", func() {
 			dummies.Zone1.Network = dummies.ISONet1
 
-			fs.EXPECT().NewCreateEgressFirewallRuleParams(dummies.ISONet1.ID, "tcp").
-				Return(&csapi.CreateEgressFirewallRuleParams{})
-			fs.EXPECT().CreateEgressFirewallRule(&csapi.CreateEgressFirewallRuleParams{}).
-				Return(&csapi.CreateEgressFirewallRuleResponse{}, errors.New("there is already a rule like this"))
+			fs.EXPECT().NewCreateEgressFirewallRuleParams(dummies.ISONet1.ID, gomock.Any()).
+				DoAndReturn(func(networkid string, protocol string) *csapi.CreateEgressFirewallRuleParams {
+					p := &csapi.CreateEgressFirewallRuleParams{}
+					if protocol == "icmp" {
+						p.SetIcmptype(-1)
+						p.SetIcmpcode(-1)
+					}
+					return p
+				}).Times(3)
+
+			ruleParamsICMP := &csapi.CreateEgressFirewallRuleParams{}
+			ruleParamsICMP.SetIcmptype(-1)
+			ruleParamsICMP.SetIcmpcode(-1)
+			gomock.InOrder(
+				fs.EXPECT().CreateEgressFirewallRule(&csapi.CreateEgressFirewallRuleParams{}).
+					Return(&csapi.CreateEgressFirewallRuleResponse{}, nil).Times(2),
+				fs.EXPECT().CreateEgressFirewallRule(ruleParamsICMP).
+					Return(&csapi.CreateEgressFirewallRuleResponse{}, nil))
 
 			Ω(client.OpenFirewallRules(dummies.CSISONet1)).Should(Succeed())
 		})

pkg/cloud/network.go

@@ -33,6 +33,8 @@ const (
 	NetworkTypeIsolated = "Isolated"
 	NetworkTypeShared   = "Shared"
 	NetworkProtocolTCP  = "tcp"
+	NetworkProtocolUDP  = "udp"
+	NetworkProtocolICMP = "icmp"
 )
 
 // NetworkExists checks that the network already exists based on the presence of all fields.