From df221b8a27d4161a5732fb6175e12e300927653f Mon Sep 17 00:00:00 2001 From: Rahul Jain Date: Fri, 13 Oct 2023 10:58:42 -0700 Subject: [PATCH] Support ANP with ICMP Rule (#315) With this PR, Nephe will support icmp protocol in Antrea NetworkPolicy. Its enabled for both AWS and Azure Cloud. Both ICMP-ALL and ICMP-Custom are supported. With ICMP-Custom, user can specify ICMP-Type and ICMP-Code for more granular control. Signed-off-by: Rahul Jain --- .../cloudresource/cloudresource.go | 14 +- .../plugins/aws/aws_converters.go | 140 +++++++++---- .../plugins/aws/aws_security_test.go | 24 +-- .../plugins/azure/azure_nsg_rules.go | 186 +++++++++++------- .../plugins/azure/azure_security_test.go | 20 +- pkg/controllers/networkpolicy/controller.go | 4 +- .../networkpolicy/controller_test.go | 72 ++++++- .../networkpolicy/networkpolicy.go | 18 +- pkg/controllers/networkpolicy/sync.go | 4 +- test/integration/networkpolicy_e2e_test.go | 159 ++++++++++----- test/templates/vm_anp.go | 19 +- test/upgrade/networkpolicy_e2e_test.go | 11 +- test/utils/helpers.go | 10 + test/utils/networkpolicy_helper.go | 13 +- 14 files changed, 485 insertions(+), 209 deletions(-) diff --git a/pkg/cloudprovider/cloudresource/cloudresource.go b/pkg/cloudprovider/cloudresource/cloudresource.go index 5d0deb8a..e65b6682 100644 --- a/pkg/cloudprovider/cloudresource/cloudresource.go +++ b/pkg/cloudprovider/cloudresource/cloudresource.go @@ -36,6 +36,12 @@ var ( CloudSecurityGroupVisibility bool ) +var ( + IcmpProtocol = 1 + TcpProtocol = 6 + UdpProtocol = 17 +) + // CloudResourceType specifies the type of cloud resource. type CloudResourceType string @@ -122,7 +128,7 @@ type Rule interface { // IngressRule specifies one ingress rule of cloud SecurityGroup. type IngressRule struct { - FromPort *int + FromPort *int32 FromSrcIP []*net.IPNet FromSecurityGroups []*CloudResourceID Protocol *int @@ -130,13 +136,15 @@ type IngressRule struct { Priority *float64 Action *antreacrdv1beta1.RuleAction RuleName string + IcmpType *int32 + IcmpCode *int32 } func (i *IngressRule) isRule() {} // EgressRule specifies one egress rule of cloud SecurityGroup. type EgressRule struct { - ToPort *int + ToPort *int32 ToDstIP []*net.IPNet ToSecurityGroups []*CloudResourceID Protocol *int @@ -144,6 +152,8 @@ type EgressRule struct { Priority *float64 Action *antreacrdv1beta1.RuleAction RuleName string + IcmpType *int32 + IcmpCode *int32 } func (e *EgressRule) isRule() {} diff --git a/pkg/cloudprovider/plugins/aws/aws_converters.go b/pkg/cloudprovider/plugins/aws/aws_converters.go index 617999a0..b3803cec 100644 --- a/pkg/cloudprovider/plugins/aws/aws_converters.go +++ b/pkg/cloudprovider/plugins/aws/aws_converters.go @@ -35,14 +35,28 @@ func convertToIPPermissionProtocol(protocol *int) *string { return aws.String(strconv.FormatInt(int64(*protocol), 10)) } -func convertToIPPermissionPort(port *int, protocol *int) (*int64, *int64) { +func convertToIPPermissionPort(port *int32, protocol *int, icmpType, icmpCode *int32) (*int64, *int64) { if port == nil { // For TCP and UDP, aws expects explicit start and end port numbers (for all ports case) - if protocol != nil && (*protocol == 6 || *protocol == 17) { - return aws.Int64(int64(tcpUDPPortStart)), aws.Int64(int64(tcpUDPPortEnd)) + if protocol != nil && + (*protocol == cloudresource.TcpProtocol || *protocol == cloudresource.UdpProtocol || *protocol == cloudresource.IcmpProtocol) { + if *protocol == cloudresource.IcmpProtocol { + awsIcmpType := aws.Int64(-1) + awsIcmpCode := aws.Int64(-1) + if icmpType != nil { + awsIcmpType = aws.Int64(int64(*icmpType)) + } + if icmpCode != nil { + awsIcmpCode = aws.Int64(int64(*icmpCode)) + } + return awsIcmpType, awsIcmpCode + } else { + return aws.Int64(int64(tcpUDPPortStart)), aws.Int64(int64(tcpUDPPortEnd)) + } } return nil, nil } + portVal := aws.Int64(int64(*port)) return portVal, portVal } @@ -153,7 +167,7 @@ func convertIngressToIpPermission(rules []*cloudresource.CloudRule, cloudSGNameT } idGroupPairs := buildEc2UserIDGroupPairs(rule.FromSecurityGroups, cloudSGNameToObj, &description) ipv4Ranges, ipv6Ranges := convertToEc2IpRanges(rule.FromSrcIP, len(rule.FromSecurityGroups) > 0, &description) - startPort, endPort := convertToIPPermissionPort(rule.FromPort, rule.Protocol) + startPort, endPort := convertToIPPermissionPort(rule.FromPort, rule.Protocol, rule.IcmpType, rule.IcmpCode) ipPermission := &ec2.IpPermission{ FromPort: startPort, ToPort: endPort, @@ -183,7 +197,7 @@ func convertEgressToIpPermission(rules []*cloudresource.CloudRule, cloudSGNameTo idGroupPairs := buildEc2UserIDGroupPairs(rule.ToSecurityGroups, cloudSGNameToObj, &description) ipv4Ranges, ipv6Ranges := convertToEc2IpRanges(rule.ToDstIP, len(rule.ToSecurityGroups) > 0, &description) - startPort, endPort := convertToIPPermissionPort(rule.ToPort, rule.Protocol) + startPort, endPort := convertToIPPermissionPort(rule.ToPort, rule.Protocol, rule.IcmpType, rule.IcmpCode) ipPermission := &ec2.IpPermission{ FromPort: startPort, ToPort: endPort, @@ -203,18 +217,25 @@ func convertFromIngressIpPermissionToCloudRule(sgID string, ipPermissions []*ec2 managedSGs, unmanagedSGs map[string]*ec2.SecurityGroup) []cloudresource.CloudRule { var ingressRules []cloudresource.CloudRule for _, ipPermission := range ipPermissions { + protocol := convertFromIPPermissionProtocol(*ipPermission.IpProtocol) + fromPort, toPort := convertFromIPPermissionPort(ipPermission.FromPort, ipPermission.ToPort) fromSrcIPs, descriptions := convertFromIPRange(ipPermission.IpRanges, ipPermission.Ipv6Ranges) for i, srcIP := range fromSrcIPs { - // Get cloud rule description. - desc, ok := utils.ExtractCloudDescription(descriptions[i]) + rule := &cloudresource.IngressRule{ + FromSrcIP: []*net.IPNet{srcIP}, + Protocol: protocol, + } + if protocol != nil && *protocol == cloudresource.IcmpProtocol { + rule.IcmpType = fromPort + rule.IcmpCode = toPort + } else { + rule.FromPort = fromPort + } ingressRule := cloudresource.CloudRule{ - Rule: &cloudresource.IngressRule{ - FromPort: convertFromIPPermissionPort(ipPermission.FromPort, ipPermission.ToPort), - FromSrcIP: []*net.IPNet{srcIP}, - Protocol: convertFromIPPermissionProtocol(*ipPermission.IpProtocol), - }, + Rule: rule, AppliedToGrp: sgID, } + desc, ok := utils.ExtractCloudDescription(descriptions[i]) if ok { ingressRule.NpNamespacedName = types.NamespacedName{Name: desc.Name, Namespace: desc.Namespace}.String() } @@ -223,16 +244,22 @@ func convertFromIngressIpPermissionToCloudRule(sgID string, ipPermissions []*ec2 } fromSecurityGroups, descriptions := convertFromSecurityGroupPair(ipPermission.UserIdGroupPairs, managedSGs, unmanagedSGs) for i, SecurityGroup := range fromSecurityGroups { - // Get cloud rule description. - desc, ok := utils.ExtractCloudDescription(descriptions[i]) + rule := &cloudresource.IngressRule{ + FromSecurityGroups: []*cloudresource.CloudResourceID{SecurityGroup}, + Protocol: protocol, + } + if protocol != nil && *protocol == cloudresource.IcmpProtocol { + rule.IcmpType = fromPort + rule.IcmpCode = toPort + } else { + rule.FromPort = fromPort + } ingressRule := cloudresource.CloudRule{ - Rule: &cloudresource.IngressRule{ - FromPort: convertFromIPPermissionPort(ipPermission.FromPort, ipPermission.ToPort), - FromSecurityGroups: []*cloudresource.CloudResourceID{SecurityGroup}, - Protocol: convertFromIPPermissionProtocol(*ipPermission.IpProtocol), - }, + Rule: rule, AppliedToGrp: sgID, } + // Get cloud rule description. + desc, ok := utils.ExtractCloudDescription(descriptions[i]) if ok { ingressRule.NpNamespacedName = types.NamespacedName{Name: desc.Name, Namespace: desc.Namespace}.String() } @@ -249,18 +276,26 @@ func convertFromEgressIpPermissionToCloudRule(sgID string, ipPermissions []*ec2. managedSGs, unmanagedSGs map[string]*ec2.SecurityGroup) []cloudresource.CloudRule { var egressRules []cloudresource.CloudRule for _, ipPermission := range ipPermissions { + protocol := convertFromIPPermissionProtocol(*ipPermission.IpProtocol) + fromPort, toPort := convertFromIPPermissionPort(ipPermission.FromPort, ipPermission.ToPort) toDstIPs, descriptions := convertFromIPRange(ipPermission.IpRanges, ipPermission.Ipv6Ranges) for i, dstIP := range toDstIPs { - // Get cloud rule description. - desc, ok := utils.ExtractCloudDescription(descriptions[i]) + rule := &cloudresource.EgressRule{ + ToDstIP: []*net.IPNet{dstIP}, + Protocol: protocol, + } + if protocol != nil && *protocol == cloudresource.IcmpProtocol { + rule.IcmpType = fromPort + rule.IcmpCode = toPort + } else { + rule.ToPort = fromPort + } egressRule := cloudresource.CloudRule{ - Rule: &cloudresource.EgressRule{ - ToPort: convertFromIPPermissionPort(ipPermission.FromPort, ipPermission.ToPort), - ToDstIP: []*net.IPNet{dstIP}, - Protocol: convertFromIPPermissionProtocol(*ipPermission.IpProtocol), - }, + Rule: rule, AppliedToGrp: sgID, } + // Get cloud rule description. + desc, ok := utils.ExtractCloudDescription(descriptions[i]) if ok { egressRule.NpNamespacedName = types.NamespacedName{Name: desc.Name, Namespace: desc.Namespace}.String() } @@ -269,16 +304,22 @@ func convertFromEgressIpPermissionToCloudRule(sgID string, ipPermissions []*ec2. } toSecurityGroups, descriptions := convertFromSecurityGroupPair(ipPermission.UserIdGroupPairs, managedSGs, unmanagedSGs) for i, SecurityGroup := range toSecurityGroups { - // Get cloud rule description. - desc, ok := utils.ExtractCloudDescription(descriptions[i]) + rule := &cloudresource.EgressRule{ + ToSecurityGroups: []*cloudresource.CloudResourceID{SecurityGroup}, + Protocol: protocol, + } + if protocol != nil && *protocol == cloudresource.IcmpProtocol { + rule.IcmpType = fromPort + rule.IcmpCode = toPort + } else { + rule.ToPort = fromPort + } egressRule := cloudresource.CloudRule{ - Rule: &cloudresource.EgressRule{ - ToPort: convertFromIPPermissionPort(ipPermission.FromPort, ipPermission.ToPort), - ToSecurityGroups: []*cloudresource.CloudResourceID{SecurityGroup}, - Protocol: convertFromIPPermissionProtocol(*ipPermission.IpProtocol), - }, + Rule: rule, AppliedToGrp: sgID, } + // Get cloud rule description. + desc, ok := utils.ExtractCloudDescription(descriptions[i]) if ok { egressRule.NpNamespacedName = types.NamespacedName{Name: desc.Name, Namespace: desc.Namespace}.String() } @@ -289,23 +330,38 @@ func convertFromEgressIpPermissionToCloudRule(sgID string, ipPermissions []*ec2. return egressRules } -func convertFromIPPermissionPort(startPort *int64, endPort *int64) *int { +func convertFromIPPermissionPort(startPort *int64, endPort *int64) (*int32, *int32) { if startPort == nil { - return nil + if endPort == nil { + return nil, nil + } else { + retval := int32(*endPort) + return nil, &retval + } } if endPort == nil { - retVal := int(*startPort) - return &retVal + retVal := int32(*startPort) + return &retVal, nil } if *startPort == -1 { - return nil + if *endPort == -1 { + return nil, nil + } else { + retval := int32(*endPort) + return nil, &retval + } } + if *startPort == *endPort { - retVal := int(*startPort) - return &retVal + retVal := int32(*startPort) + return &retVal, nil + } else if *startPort == 0 && *endPort == 65535 { + // (0 - 65535) tcp/udp ports returns nil + return nil, nil } - // other cases along with all (0 - 65535) tcp/udp ports returns nil - return nil + retval1 := int32(*startPort) + retval2 := int32(*endPort) + return &retval1, &retval2 } // convertFromIPPermissionPortToString helper function to convert cloud port number field to string. diff --git a/pkg/cloudprovider/plugins/aws/aws_security_test.go b/pkg/cloudprovider/plugins/aws/aws_security_test.go index 28f49da0..4bd17895 100644 --- a/pkg/cloudprovider/plugins/aws/aws_security_test.go +++ b/pkg/cloudprovider/plugins/aws/aws_security_test.go @@ -274,7 +274,7 @@ var _ = Describe("AWS Cloud Security", func() { } addRule := []*cloudresource.CloudRule{{ Rule: &cloudresource.IngressRule{ - FromPort: aws.Int(22), + FromPort: aws.Int32(22), FromSrcIP: []*net.IPNet{{ IP: net.ParseIP("2600:1f16:c77:a001:fb97:21b2:a8dc:dc60"), Mask: net.CIDRMask(128, 128)}, @@ -319,7 +319,7 @@ var _ = Describe("AWS Cloud Security", func() { } addRule := []*cloudresource.CloudRule{{ Rule: &cloudresource.IngressRule{ - FromPort: aws.Int(22), + FromPort: aws.Int32(22), FromSrcIP: []*net.IPNet{}, FromSecurityGroups: []*cloudresource.CloudResourceID{&webSgIdentifier.CloudResourceID}, Protocol: aws.Int(6), @@ -355,7 +355,7 @@ var _ = Describe("AWS Cloud Security", func() { } addRule := []*cloudresource.CloudRule{{ Rule: &cloudresource.EgressRule{ - ToPort: aws.Int(22), + ToPort: aws.Int32(22), ToDstIP: []*net.IPNet{}, ToSecurityGroups: []*cloudresource.CloudResourceID{&webSgIdentifier.CloudResourceID}, Protocol: aws.Int(6), @@ -396,7 +396,7 @@ var _ = Describe("AWS Cloud Security", func() { } addRule := []*cloudresource.CloudRule{{ Rule: &cloudresource.EgressRule{ - ToPort: aws.Int(22), + ToPort: aws.Int32(22), ToDstIP: []*net.IPNet{}, ToSecurityGroups: []*cloudresource.CloudResourceID{&webSgIdentifier.CloudResourceID}, Protocol: aws.Int(6), @@ -441,7 +441,7 @@ var _ = Describe("AWS Cloud Security", func() { addRule := []*cloudresource.CloudRule{ { Rule: &cloudresource.IngressRule{ - FromPort: aws.Int(22), + FromPort: aws.Int32(22), FromSrcIP: []*net.IPNet{{ IP: net.ParseIP("2600:1f16:c77:a001:fb97:21b2:a8dc:dc60"), Mask: net.CIDRMask(128, 128)}, @@ -450,7 +450,7 @@ var _ = Describe("AWS Cloud Security", func() { }, NpNamespacedName: testAnpNamespacedName.String(), }, { Rule: &cloudresource.IngressRule{ - FromPort: aws.Int(22), + FromPort: aws.Int32(22), FromSrcIP: []*net.IPNet{{ IP: net.ParseIP("2600:1f16:c77:a001:fb97:21b2:a8dc:dc61"), Mask: net.CIDRMask(128, 128)}, @@ -459,13 +459,13 @@ var _ = Describe("AWS Cloud Security", func() { }, NpNamespacedName: testAnpNamespacedName.String(), }, { Rule: &cloudresource.IngressRule{ - FromPort: aws.Int(22), + FromPort: aws.Int32(22), FromSecurityGroups: []*cloudresource.CloudResourceID{&webSgIdentifier1.CloudResourceID}, Protocol: aws.Int(6), }, NpNamespacedName: testAnpNamespacedName.String(), }, { Rule: &cloudresource.IngressRule{ - FromPort: aws.Int(22), + FromPort: aws.Int32(22), FromSecurityGroups: []*cloudresource.CloudResourceID{&webSgIdentifier2.CloudResourceID}, Protocol: aws.Int(6), }, NpNamespacedName: testAnpNamespacedName.String(), @@ -543,7 +543,7 @@ var _ = Describe("AWS Cloud Security", func() { addRule := []*cloudresource.CloudRule{ { Rule: &cloudresource.EgressRule{ - ToPort: aws.Int(22), + ToPort: aws.Int32(22), ToDstIP: []*net.IPNet{{ IP: net.ParseIP("2600:1f16:c77:a001:fb97:21b2:a8dc:dc60"), Mask: net.CIDRMask(128, 128)}, @@ -552,7 +552,7 @@ var _ = Describe("AWS Cloud Security", func() { }, NpNamespacedName: testAnpNamespacedName.String(), }, { Rule: &cloudresource.EgressRule{ - ToPort: aws.Int(22), + ToPort: aws.Int32(22), ToDstIP: []*net.IPNet{{ IP: net.ParseIP("2600:1f16:c77:a001:fb97:21b2:a8dc:dc61"), Mask: net.CIDRMask(128, 128)}, @@ -561,13 +561,13 @@ var _ = Describe("AWS Cloud Security", func() { }, NpNamespacedName: testAnpNamespacedName.String(), }, { Rule: &cloudresource.EgressRule{ - ToPort: aws.Int(22), + ToPort: aws.Int32(22), ToSecurityGroups: []*cloudresource.CloudResourceID{&webSgIdentifier1.CloudResourceID}, Protocol: aws.Int(6), }, NpNamespacedName: testAnpNamespacedName.String(), }, { Rule: &cloudresource.EgressRule{ - ToPort: aws.Int(22), + ToPort: aws.Int32(22), ToSecurityGroups: []*cloudresource.CloudResourceID{&webSgIdentifier2.CloudResourceID}, Protocol: aws.Int(6), }, NpNamespacedName: testAnpNamespacedName.String(), diff --git a/pkg/cloudprovider/plugins/azure/azure_nsg_rules.go b/pkg/cloudprovider/plugins/azure/azure_nsg_rules.go index c1cee485..9845e282 100644 --- a/pkg/cloudprovider/plugins/azure/azure_nsg_rules.go +++ b/pkg/cloudprovider/plugins/azure/azure_nsg_rules.go @@ -38,15 +38,15 @@ const ( ) var protoNumAzureNameMap = map[int]armnetwork.SecurityRuleProtocol{ - 1: armnetwork.SecurityRuleProtocolIcmp, - 6: armnetwork.SecurityRuleProtocolTCP, - 17: armnetwork.SecurityRuleProtocolUDP, + cloudresource.IcmpProtocol: armnetwork.SecurityRuleProtocolIcmp, + cloudresource.TcpProtocol: armnetwork.SecurityRuleProtocolTCP, + cloudresource.UdpProtocol: armnetwork.SecurityRuleProtocolUDP, } var azureProtoNameToNumMap = map[string]int{ - strings.ToLower(string(armnetwork.SecurityRuleProtocolIcmp)): 1, - strings.ToLower(string(armnetwork.SecurityRuleProtocolTCP)): 6, - strings.ToLower(string(armnetwork.SecurityRuleProtocolUDP)): 17, + strings.ToLower(string(armnetwork.SecurityRuleProtocolIcmp)): cloudresource.IcmpProtocol, + strings.ToLower(string(armnetwork.SecurityRuleProtocolTCP)): cloudresource.TcpProtocol, + strings.ToLower(string(armnetwork.SecurityRuleProtocolUDP)): cloudresource.UdpProtocol, } // isAzureRuleAttachedToAtSg check if the given Azure security rule is attached to the specified appliedTo sg. @@ -157,7 +157,7 @@ func convertIngressToNsgSecurityRules(appliedToGroupID *cloudresource.CloudResou return []*armnetwork.SecurityRule{}, err } - srcPort := convertToAzurePortRange(rule.FromPort) + srcPort, dstPort := convertToAzurePortRange(rule.FromPort, rule.IcmpType, rule.IcmpCode) action := armnetwork.SecurityRuleAccessAllow if rule.Action != nil && *rule.Action == antreacrdv1beta1.RuleActionDrop { action = armnetwork.SecurityRuleAccessDeny @@ -167,8 +167,8 @@ func convertIngressToNsgSecurityRules(appliedToGroupID *cloudresource.CloudResou srcAddrPrefix, srcAddrPrefixes := convertToAzureAddressPrefix(rule.FromSrcIP) if srcAddrPrefix != nil || srcAddrPrefixes != nil { securityRule := buildSecurityRule(rule.RuleName, protoName, armnetwork.SecurityRuleDirectionInbound, - to.StringPtr(emptyPort), srcAddrPrefix, srcAddrPrefixes, nil, - &srcPort, nil, nil, []*armnetwork.ApplicationSecurityGroup{&dstAsgObj}, &description, action) + &srcPort, srcAddrPrefix, srcAddrPrefixes, nil, + &dstPort, nil, nil, []*armnetwork.ApplicationSecurityGroup{&dstAsgObj}, &description, action) securityRules = append(securityRules, &securityRule) } } @@ -179,8 +179,8 @@ func convertIngressToNsgSecurityRules(appliedToGroupID *cloudresource.CloudResou } if len(srcApplicationSecurityGroups) != 0 { securityRule := buildSecurityRule(rule.RuleName, protoName, armnetwork.SecurityRuleDirectionInbound, - to.StringPtr(emptyPort), nil, nil, srcApplicationSecurityGroups, - &srcPort, nil, nil, []*armnetwork.ApplicationSecurityGroup{&dstAsgObj}, &description, action) + &srcPort, nil, nil, srcApplicationSecurityGroups, + &dstPort, nil, nil, []*armnetwork.ApplicationSecurityGroup{&dstAsgObj}, &description, action) securityRules = append(securityRules, &securityRule) } } @@ -208,7 +208,7 @@ func convertIngressToPeerNsgSecurityRules(appliedToGroupID *cloudresource.CloudR return []*armnetwork.SecurityRule{}, err } - srcPort := convertToAzurePortRange(rule.FromPort) + srcPort, dstPort := convertToAzurePortRange(rule.FromPort, rule.IcmpType, rule.IcmpCode) action := armnetwork.SecurityRuleAccessAllow if rule.Action != nil && *rule.Action == antreacrdv1beta1.RuleActionDrop { action = armnetwork.SecurityRuleAccessDeny @@ -232,8 +232,8 @@ func convertIngressToPeerNsgSecurityRules(appliedToGroupID *cloudresource.CloudR } if len(srcApplicationSecurityGroups) != 0 { securityRule := buildSecurityRule(rule.RuleName, protoName, armnetwork.SecurityRuleDirectionInbound, - to.StringPtr(emptyPort), nil, nil, srcApplicationSecurityGroups, - &srcPort, to.StringPtr(emptyPort), nil, nil, &description, action) + &srcPort, nil, nil, srcApplicationSecurityGroups, + &dstPort, to.StringPtr(emptyPort), nil, nil, &description, action) securityRules = append(securityRules, &securityRule) flag = 1 break @@ -242,8 +242,8 @@ func convertIngressToPeerNsgSecurityRules(appliedToGroupID *cloudresource.CloudR } if flag == 0 { securityRule := buildSecurityRule(rule.RuleName, protoName, armnetwork.SecurityRuleDirectionInbound, - to.StringPtr(emptyPort), ruleIP, nil, nil, - &srcPort, to.StringPtr(emptyPort), nil, nil, &description, + &srcPort, ruleIP, nil, nil, + &dstPort, to.StringPtr(emptyPort), nil, nil, &description, armnetwork.SecurityRuleAccessAllow) securityRules = append(securityRules, &securityRule) } @@ -279,7 +279,7 @@ func convertEgressToNsgSecurityRules(appliedToGroupID *cloudresource.CloudResour return []*armnetwork.SecurityRule{}, err } - dstPort := convertToAzurePortRange(rule.ToPort) + srcPort, dstPort := convertToAzurePortRange(rule.ToPort, rule.IcmpType, rule.IcmpCode) action := armnetwork.SecurityRuleAccessAllow if rule.Action != nil && *rule.Action == antreacrdv1beta1.RuleActionDrop { action = armnetwork.SecurityRuleAccessDeny @@ -289,7 +289,7 @@ func convertEgressToNsgSecurityRules(appliedToGroupID *cloudresource.CloudResour dstAddrPrefix, dstAddrPrefixes := convertToAzureAddressPrefix(rule.ToDstIP) if dstAddrPrefix != nil || dstAddrPrefixes != nil { securityRule := buildSecurityRule(rule.RuleName, protoName, armnetwork.SecurityRuleDirectionOutbound, - to.StringPtr(emptyPort), nil, nil, []*armnetwork.ApplicationSecurityGroup{&srcAsgObj}, + &srcPort, nil, nil, []*armnetwork.ApplicationSecurityGroup{&srcAsgObj}, &dstPort, dstAddrPrefix, dstAddrPrefixes, nil, &description, action) securityRules = append(securityRules, &securityRule) } @@ -301,7 +301,7 @@ func convertEgressToNsgSecurityRules(appliedToGroupID *cloudresource.CloudResour } if len(dstApplicationSecurityGroups) != 0 { securityRule := buildSecurityRule(rule.RuleName, protoName, armnetwork.SecurityRuleDirectionOutbound, - to.StringPtr(emptyPort), nil, nil, []*armnetwork.ApplicationSecurityGroup{&srcAsgObj}, + &srcPort, nil, nil, []*armnetwork.ApplicationSecurityGroup{&srcAsgObj}, &dstPort, nil, nil, dstApplicationSecurityGroups, &description, action) securityRules = append(securityRules, &securityRule) } @@ -330,7 +330,7 @@ func convertEgressToPeerNsgSecurityRules(appliedToGroupID *cloudresource.CloudRe return []*armnetwork.SecurityRule{}, err } - dstPort := convertToAzurePortRange(rule.ToPort) + srcPort, dstPort := convertToAzurePortRange(rule.ToPort, rule.IcmpType, rule.IcmpCode) action := armnetwork.SecurityRuleAccessAllow if rule.Action != nil && *rule.Action == antreacrdv1beta1.RuleActionDrop { action = armnetwork.SecurityRuleAccessDeny @@ -354,7 +354,7 @@ func convertEgressToPeerNsgSecurityRules(appliedToGroupID *cloudresource.CloudRe } if len(dstApplicationSecurityGroups) != 0 { securityRule := buildSecurityRule(rule.RuleName, protoName, armnetwork.SecurityRuleDirectionOutbound, - to.StringPtr(emptyPort), to.StringPtr(emptyPort), nil, nil, + &srcPort, to.StringPtr(emptyPort), nil, nil, &dstPort, nil, nil, dstApplicationSecurityGroups, &description, action) securityRules = append(securityRules, &securityRule) flag = 1 @@ -364,7 +364,7 @@ func convertEgressToPeerNsgSecurityRules(appliedToGroupID *cloudresource.CloudRe } if flag == 0 { securityRule := buildSecurityRule(rule.RuleName, protoName, armnetwork.SecurityRuleDirectionOutbound, - to.StringPtr(emptyPort), to.StringPtr(emptyPort), nil, nil, + &srcPort, to.StringPtr(emptyPort), nil, nil, &dstPort, ruleIP, nil, nil, &description, action) securityRules = append(securityRules, &securityRule) } @@ -465,11 +465,18 @@ func convertToAzureProtocolName(protoNum *int) (armnetwork.SecurityRuleProtocol, return protocolName, nil } -func convertToAzurePortRange(port *int) string { - if port == nil { - return emptyPort +func convertToAzurePortRange(port *int32, icmpType *int32, icmpCode *int32) (string, string) { + if port != nil { + return emptyPort, strconv.Itoa(int(*port)) + } + if icmpType == nil && icmpCode == nil { + return emptyPort, emptyPort + } else if icmpType != nil && icmpCode == nil { + return strconv.Itoa(int(*icmpType)), emptyPort + } else if icmpType == nil && icmpCode != nil { + return emptyPort, strconv.Itoa(int(*icmpCode)) } - return strconv.Itoa(*port) + return strconv.Itoa(int(*icmpType)), strconv.Itoa(int(*icmpCode)) } func convertToAzureAddressPrefix(ruleIPs []*net.IPNet) (*string, []*string) { @@ -568,7 +575,7 @@ func convertFromAzureIngressSecurityRuleToCloudRule(rule armnetwork.SecurityRule desc *cloudresource.CloudRuleDescription) ([]cloudresource.CloudRule, error) { ingressList := make([]cloudresource.CloudRule, 0) - port := convertFromAzurePortToNepheControllerPort(rule.Properties.DestinationPortRange) + sPort, dPort := convertFromAzurePortToNepheControllerPort(rule.Properties.SourcePortRange, rule.Properties.DestinationPortRange) srcIP := convertFromAzurePrefixesToNepheControllerIPs(rule.Properties.SourceAddressPrefix, rule.Properties.SourceAddressPrefixes) securityGroups := convertFromAzureASGsToNepheControllerSecurityGroups(rule.Properties.SourceApplicationSecurityGroups, vnetID) protoNum, err := convertFromAzureProtocolToNepheControllerProtocol(rule.Properties.Protocol) @@ -583,16 +590,24 @@ func convertFromAzureIngressSecurityRuleToCloudRule(rule armnetwork.SecurityRule priority = desc.Priority npNamespacedName = types.NamespacedName{Name: desc.Name, Namespace: desc.Namespace}.String() } + for _, ip := range srcIP { + iRule := &cloudresource.IngressRule{ + FromSrcIP: []*net.IPNet{ip}, + Protocol: protoNum, + Priority: priority, + Action: action, + RuleName: *rule.Name, + } + if iRule.Protocol != nil && *iRule.Protocol == cloudresource.IcmpProtocol { + iRule.IcmpType = sPort + iRule.IcmpCode = dPort + } else { + iRule.FromPort = dPort + } + ingressRule := cloudresource.CloudRule{ - Rule: &cloudresource.IngressRule{ - FromPort: port, - FromSrcIP: []*net.IPNet{ip}, - Protocol: protoNum, - Priority: priority, - Action: action, - RuleName: *rule.Name, - }, + Rule: iRule, AppliedToGrp: sgID, NpNamespacedName: npNamespacedName, } @@ -600,15 +615,22 @@ func convertFromAzureIngressSecurityRuleToCloudRule(rule armnetwork.SecurityRule ingressList = append(ingressList, ingressRule) } for _, sg := range securityGroups { + iRule := &cloudresource.IngressRule{ + FromSecurityGroups: []*cloudresource.CloudResourceID{sg}, + Protocol: protoNum, + Priority: priority, + Action: action, + RuleName: *rule.Name, + } + if iRule.Protocol != nil && *iRule.Protocol == cloudresource.IcmpProtocol { + iRule.IcmpType = sPort + iRule.IcmpCode = dPort + } else { + iRule.FromPort = dPort + } + ingressRule := cloudresource.CloudRule{ - Rule: &cloudresource.IngressRule{ - FromPort: port, - FromSecurityGroups: []*cloudresource.CloudResourceID{sg}, - Protocol: protoNum, - Priority: priority, - Action: action, - RuleName: *rule.Name, - }, + Rule: iRule, AppliedToGrp: sgID, NpNamespacedName: npNamespacedName, } @@ -624,7 +646,7 @@ func convertFromAzureEgressSecurityRuleToCloudRule(rule armnetwork.SecurityRule, desc *cloudresource.CloudRuleDescription) ([]cloudresource.CloudRule, error) { egressList := make([]cloudresource.CloudRule, 0) - port := convertFromAzurePortToNepheControllerPort(rule.Properties.DestinationPortRange) + sPort, dPort := convertFromAzurePortToNepheControllerPort(rule.Properties.SourcePortRange, rule.Properties.DestinationPortRange) dstIP := convertFromAzurePrefixesToNepheControllerIPs(rule.Properties.DestinationAddressPrefix, rule.Properties.DestinationAddressPrefixes) securityGroups := convertFromAzureASGsToNepheControllerSecurityGroups(rule.Properties.DestinationApplicationSecurityGroups, vnetID) protoNum, err := convertFromAzureProtocolToNepheControllerProtocol(rule.Properties.Protocol) @@ -640,15 +662,22 @@ func convertFromAzureEgressSecurityRuleToCloudRule(rule armnetwork.SecurityRule, npNamespacedName = types.NamespacedName{Name: desc.Name, Namespace: desc.Namespace}.String() } for _, ip := range dstIP { + eRule := &cloudresource.EgressRule{ + ToDstIP: []*net.IPNet{ip}, + Protocol: protoNum, + Priority: priority, + Action: action, + RuleName: *rule.Name, + } + if eRule.Protocol != nil && *eRule.Protocol == cloudresource.IcmpProtocol { + eRule.IcmpType = sPort + eRule.IcmpCode = dPort + } else { + eRule.ToPort = dPort + } + egressRule := cloudresource.CloudRule{ - Rule: &cloudresource.EgressRule{ - ToPort: port, - ToDstIP: []*net.IPNet{ip}, - Protocol: protoNum, - Priority: priority, - Action: action, - RuleName: *rule.Name, - }, + Rule: eRule, AppliedToGrp: sgID, NpNamespacedName: npNamespacedName, } @@ -656,15 +685,21 @@ func convertFromAzureEgressSecurityRuleToCloudRule(rule armnetwork.SecurityRule, egressList = append(egressList, egressRule) } for _, sg := range securityGroups { + eRule := &cloudresource.EgressRule{ + ToSecurityGroups: []*cloudresource.CloudResourceID{sg}, + Protocol: protoNum, + Priority: priority, + Action: action, + RuleName: *rule.Name, + } + if eRule.Protocol != nil && *eRule.Protocol == cloudresource.IcmpProtocol { + eRule.IcmpType = sPort + eRule.IcmpCode = dPort + } else { + eRule.ToPort = dPort + } egressRule := cloudresource.CloudRule{ - Rule: &cloudresource.EgressRule{ - ToPort: port, - ToSecurityGroups: []*cloudresource.CloudResourceID{sg}, - Protocol: protoNum, - Priority: priority, - Action: action, - RuleName: *rule.Name, - }, + Rule: eRule, AppliedToGrp: sgID, NpNamespacedName: npNamespacedName, } @@ -749,13 +784,28 @@ func convertFromAzurePrefixesToNepheControllerIPs(ipPrefix *string, ipPrefixes [ return ipNetList } -func convertFromAzurePortToNepheControllerPort(port *string) *int { - if port == nil || *port == emptyPort { - return nil - } - portNum, err := strconv.ParseInt(*port, 10, 32) - if err != nil { - return nil +func convertFromAzurePortToNepheControllerPort(sPort, dPort *string) (*int32, *int32) { + if sPort == nil || dPort == nil { + return nil, nil + } else if sPort != nil && dPort == nil { + if *sPort == emptyPort { + return nil, nil + } else { + portNum, _ := strconv.ParseInt(*sPort, 10, 32) + return to.Int32Ptr(int32(portNum)), nil + } + } else if sPort == nil && dPort != nil { + if *dPort == emptyPort { + return nil, nil + } else { + portNum, _ := strconv.ParseInt(*dPort, 10, 32) + return nil, to.Int32Ptr(int32(portNum)) + } + } else if *sPort == emptyPort && *dPort == emptyPort { + return nil, nil } - return to.IntPtr(int(portNum)) + + sPortNum, _ := strconv.ParseInt(*sPort, 10, 32) + dPortNum, _ := strconv.ParseInt(*dPort, 10, 32) + return to.Int32Ptr(int32(sPortNum)), to.Int32Ptr(int32(dPortNum)) } diff --git a/pkg/cloudprovider/plugins/azure/azure_security_test.go b/pkg/cloudprovider/plugins/azure/azure_security_test.go index 362eb4fc..270c51af 100644 --- a/pkg/cloudprovider/plugins/azure/azure_security_test.go +++ b/pkg/cloudprovider/plugins/azure/azure_security_test.go @@ -64,8 +64,8 @@ var _ = Describe("Azure Cloud Security", func() { testDestinationPortRange = "*" testPrivateIP = "0.0.0.0" testProtocol = 6 - testFromPort = 22 - testToPort = 23 + testFromPort = int32(22) + testToPort = int32(23) testCidrStr = "192.168.1.1/24" testVnet01 = "testVnet01" @@ -553,7 +553,7 @@ var _ = Describe("Azure Cloud Security", func() { SourceAddressPrefixes: []*string{to.StringPtr("2600:1f16:c77:a001:fb97:21b2:a8dc:dc60/128")}, Priority: &testPriority, SourcePortRange: &testSourcePortRange, - DestinationPortRange: to.StringPtr(strconv.Itoa(testFromPort)), + DestinationPortRange: to.StringPtr(strconv.Itoa(int(testFromPort))), Direction: &testDirection, Description: &desc, }, @@ -567,7 +567,7 @@ var _ = Describe("Azure Cloud Security", func() { SourceAddressPrefixes: []*string{to.StringPtr("2600:1f16:c77:a001:fb97:21b2:a8dc:dc61/128")}, Priority: to.Int32Ptr(testPriority + 1), SourcePortRange: &testSourcePortRange, - DestinationPortRange: to.StringPtr(strconv.Itoa(testFromPort)), + DestinationPortRange: to.StringPtr(strconv.Itoa(int(testFromPort))), Direction: &testDirection, Description: &desc, }, @@ -581,7 +581,7 @@ var _ = Describe("Azure Cloud Security", func() { DestinationApplicationSecurityGroups: []*network.ApplicationSecurityGroup{{ID: &testATAsgID}}, Priority: to.Int32Ptr(testPriority + 2), SourcePortRange: &testSourcePortRange, - DestinationPortRange: to.StringPtr(strconv.Itoa(testFromPort)), + DestinationPortRange: to.StringPtr(strconv.Itoa(int(testFromPort))), Direction: &testDirection, Description: &desc, }, @@ -674,7 +674,7 @@ var _ = Describe("Azure Cloud Security", func() { SourceApplicationSecurityGroups: []*network.ApplicationSecurityGroup{{ID: &testATAsgID}}, DestinationAddressPrefixes: []*string{to.StringPtr("2600:1f16:c77:a001:fb97:21b2:a8dc:dc60/128")}, Priority: &testPriority, - DestinationPortRange: to.StringPtr(strconv.Itoa(testToPort)), + DestinationPortRange: to.StringPtr(strconv.Itoa(int(testToPort))), SourcePortRange: &testSourcePortRange, Direction: &outbound, Description: &desc, @@ -688,7 +688,7 @@ var _ = Describe("Azure Cloud Security", func() { SourceApplicationSecurityGroups: []*network.ApplicationSecurityGroup{{ID: &testATAsgID}}, DestinationAddressPrefixes: []*string{to.StringPtr("2600:1f16:c77:a001:fb97:21b2:a8dc:dc61/128")}, Priority: to.Int32Ptr(testPriority + 1), - DestinationPortRange: to.StringPtr(strconv.Itoa(testToPort)), + DestinationPortRange: to.StringPtr(strconv.Itoa(int(testToPort))), SourcePortRange: &testSourcePortRange, Direction: &outbound, Description: &desc, @@ -702,7 +702,7 @@ var _ = Describe("Azure Cloud Security", func() { DestinationApplicationSecurityGroups: []*network.ApplicationSecurityGroup{{ID: to.StringPtr(testAGAsgID + "1")}}, SourceApplicationSecurityGroups: []*network.ApplicationSecurityGroup{{ID: &testATAsgID}}, Priority: to.Int32Ptr(testPriority + 2), - DestinationPortRange: to.StringPtr(strconv.Itoa(testToPort)), + DestinationPortRange: to.StringPtr(strconv.Itoa(int(testToPort))), SourcePortRange: &testSourcePortRange, Direction: &outbound, Description: &desc, @@ -776,7 +776,7 @@ var _ = Describe("Azure Cloud Security", func() { SourceAddressPrefixes: []*string{to.StringPtr("1.1.1.0/24")}, Priority: &testPriority, SourcePortRange: &testSourcePortRange, - DestinationPortRange: to.StringPtr(strconv.Itoa(testFromPort)), + DestinationPortRange: to.StringPtr(strconv.Itoa(int(testFromPort))), Direction: &testDirection, Description: &desc, }, @@ -854,7 +854,7 @@ var _ = Describe("Azure Cloud Security", func() { SourceAddressPrefixes: []*string{to.StringPtr("1.1.1.0/24")}, Priority: &testPriority, SourcePortRange: &testSourcePortRange, - DestinationPortRange: to.StringPtr(strconv.Itoa(testFromPort)), + DestinationPortRange: to.StringPtr(strconv.Itoa(int(testFromPort))), Direction: &outbound, Description: &desc, }, diff --git a/pkg/controllers/networkpolicy/controller.go b/pkg/controllers/networkpolicy/controller.go index d66c4bf5..54be0156 100644 --- a/pkg/controllers/networkpolicy/controller.go +++ b/pkg/controllers/networkpolicy/controller.go @@ -133,13 +133,13 @@ func (r *NetworkPolicyReconciler) isNetworkPolicySupported(anp *antreanetworking // Check for support actions. for _, rule := range anp.Rules { if rule.Action != nil && !(*rule.Action == antreacrdv1beta1.RuleActionAllow || *rule.Action == antreacrdv1beta1.RuleActionDrop) { - return fmt.Errorf("unsupported action: %v, supportted actions: %v, %v", *rule.Action, + return fmt.Errorf("unsupported action: %v, supported actions: %v, %v", *rule.Action, antreacrdv1beta1.RuleActionAllow, antreacrdv1beta1.RuleActionDrop) } // check for supported protocol. for _, s := range rule.Services { if _, ok := AntreaProtocolMap[*s.Protocol]; !ok { - return fmt.Errorf("unsupported protocol: %v, supportted protocols: %v", *s.Protocol, reflect.ValueOf(AntreaProtocolMap).MapKeys()) + return fmt.Errorf("unsupported protocol: %v, supported protocols: %v", *s.Protocol, reflect.ValueOf(AntreaProtocolMap).MapKeys()) } } } diff --git a/pkg/controllers/networkpolicy/controller_test.go b/pkg/controllers/networkpolicy/controller_test.go index b6c38481..3ac47eb8 100644 --- a/pkg/controllers/networkpolicy/controller_test.go +++ b/pkg/controllers/networkpolicy/controller_test.go @@ -264,10 +264,15 @@ var _ = Describe("NetworkPolicy", func() { Type: antreanetworking.AntreaNetworkPolicy, } protocol := antreanetworking.ProtocolTCP + icmpProtocol := antreanetworking.ProtocolICMP + icmpType := int32(8) + icmpCode := int32(0) port := &intstr.IntOrString{IntVal: 443} inRule := antreanetworking.NetworkPolicyRule{Direction: antreanetworking.DirectionIn} inRule.Services = []antreanetworking.Service{ {Port: port, Protocol: &protocol}, + {Protocol: &icmpProtocol}, + {Protocol: &icmpProtocol, ICMPType: &icmpType, ICMPCode: &icmpCode}, } _, ingressIPBlock, _ := net.ParseCIDR("5.5.5.0/24") ipInBlock := antreanetworking.IPBlock{} @@ -279,6 +284,8 @@ var _ = Describe("NetworkPolicy", func() { eRule := antreanetworking.NetworkPolicyRule{Direction: antreanetworking.DirectionOut} eRule.Services = []antreanetworking.Service{ {Port: port, Protocol: &protocol}, + {Protocol: &icmpProtocol}, + {Protocol: &icmpProtocol, ICMPType: &icmpType, ICMPCode: &icmpCode}, } _, egressIPBlock, _ := net.ParseCIDR("6.6.6.0/24") ipEgBlock := antreanetworking.IPBlock{} @@ -290,30 +297,70 @@ var _ = Describe("NetworkPolicy", func() { // rules tcp := 6 - portInt := int(port.IntVal) + icmp := 1 ingressRule = []*cloudresource.IngressRule{ { - FromPort: &portInt, + FromPort: &port.IntVal, Protocol: &tcp, FromSrcIP: []*net.IPNet{ingressIPBlock}, }, { - FromPort: &portInt, + FromPort: &port.IntVal, Protocol: &tcp, FromSecurityGroups: []*cloudresource.CloudResourceID{addrGrpIDs[addrGrps[0].Name]}, }, + { + FromSrcIP: []*net.IPNet{ingressIPBlock}, + Protocol: &icmp, + }, + { + FromSecurityGroups: []*cloudresource.CloudResourceID{addrGrpIDs[addrGrps[0].Name]}, + Protocol: &icmp, + }, + { + FromSrcIP: []*net.IPNet{ingressIPBlock}, + Protocol: &icmp, + IcmpType: &icmpType, + IcmpCode: &icmpCode, + }, + { + FromSecurityGroups: []*cloudresource.CloudResourceID{addrGrpIDs[addrGrps[0].Name]}, + Protocol: &icmp, + IcmpType: &icmpType, + IcmpCode: &icmpCode, + }, } egressRule = []*cloudresource.EgressRule{ { - ToPort: &portInt, + ToPort: &port.IntVal, Protocol: &tcp, ToDstIP: []*net.IPNet{egressIPBlock}, }, { - ToPort: &portInt, + ToPort: &port.IntVal, Protocol: &tcp, ToSecurityGroups: []*cloudresource.CloudResourceID{addrGrpIDs[addrGrps[1].Name]}, }, + { + ToDstIP: []*net.IPNet{egressIPBlock}, + Protocol: &icmp, + }, + { + ToSecurityGroups: []*cloudresource.CloudResourceID{addrGrpIDs[addrGrps[1].Name]}, + Protocol: &icmp, + }, + { + ToDstIP: []*net.IPNet{egressIPBlock}, + Protocol: &icmp, + IcmpType: &icmpType, + IcmpCode: &icmpCode, + }, + { + ToSecurityGroups: []*cloudresource.CloudResourceID{addrGrpIDs[addrGrps[1].Name]}, + Protocol: &icmp, + IcmpType: &icmpType, + IcmpCode: &icmpCode, + }, } for i := appliedToVMIdx; i < patchVMIdx; i++ { @@ -962,7 +1009,7 @@ var _ = Describe("NetworkPolicy", func() { It("Verify unsupported networkPolicy protocol", func() { anpTemp := anp inRule := antreanetworking.NetworkPolicyRule{Direction: antreanetworking.DirectionIn} - protocol := antreanetworking.ProtocolICMP + protocol := antreanetworking.ProtocolIGMP inRule.Services = []antreanetworking.Service{ {Protocol: &protocol}, } @@ -1037,6 +1084,19 @@ var _ = Describe("NetworkPolicy", func() { ingress.FromSecurityGroups = append(ingress.FromSecurityGroups, addrGrpIDs[ag.Name]) ingress.FromSrcIP = nil ingressRule = append(ingressRule, ingress) + + // Update ICMP without Type and Code. + ingress = ingressRule[2] + ingress.FromSecurityGroups = append(ingress.FromSecurityGroups, addrGrpIDs[ag.Name]) + ingress.FromSrcIP = nil + ingressRule = append(ingressRule, ingress) + + // Update ICMP with Type and Code. + ingress = ingressRule[4] + ingress.FromSecurityGroups = append(ingress.FromSecurityGroups, addrGrpIDs[ag.Name]) + ingress.FromSrcIP = nil + ingressRule = append(ingressRule, ingress) + checkNPPatchChange(appliedToGrps) event = watch.Event{Type: watch.Modified, Object: anp} err = reconciler.processNetworkPolicy(event) diff --git a/pkg/controllers/networkpolicy/networkpolicy.go b/pkg/controllers/networkpolicy/networkpolicy.go index e70cf1e3..20d3e359 100644 --- a/pkg/controllers/networkpolicy/networkpolicy.go +++ b/pkg/controllers/networkpolicy/networkpolicy.go @@ -111,6 +111,7 @@ var ( antreanetworking.ProtocolTCP: 6, antreanetworking.ProtocolUDP: 17, antreanetworking.ProtocolSCTP: 132, + antreanetworking.ProtocolICMP: 1, } ) @@ -1351,28 +1352,35 @@ func (r *networkPolicyRule) rules(rr *NetworkPolicyReconciler, anpName string, t } else { for _, s := range rule.Services { var protocol *int - var targetPort *int + var targetPort *int32 if s.Protocol != nil { if p, ok := AntreaProtocolMap[*s.Protocol]; ok { protocol = &p } } if s.Port != nil { - port := int(s.Port.IntVal) + port := s.Port.IntVal targetPort = &port } - if ingress { iRule := deepcopy.Copy(cloudRule).(*cloudresource.IngressRule) - iRule.FromPort = targetPort iRule.Protocol = protocol + if *protocol == AntreaProtocolMap[antreanetworking.ProtocolICMP] { + iRule.IcmpCode = s.ICMPCode + iRule.IcmpType = s.ICMPType + } + iRule.FromPort = targetPort iRule.RuleName = rule.Name + "-" + anpName + "-" + strconv.Itoa(rCount) rCount++ ingressList = append(ingressList, iRule) } else { eRule := deepcopy.Copy(cloudRule).(*cloudresource.EgressRule) - eRule.ToPort = targetPort eRule.Protocol = protocol + if *protocol == AntreaProtocolMap[antreanetworking.ProtocolICMP] { + eRule.IcmpCode = s.ICMPCode + eRule.IcmpType = s.ICMPType + } + eRule.ToPort = targetPort eRule.RuleName = rule.Name + "-" + anpName + "-" + strconv.Itoa(rCount) rCount++ egressList = append(egressList, eRule) diff --git a/pkg/controllers/networkpolicy/sync.go b/pkg/controllers/networkpolicy/sync.go index 2325503e..99fbf5ca 100644 --- a/pkg/controllers/networkpolicy/sync.go +++ b/pkg/controllers/networkpolicy/sync.go @@ -329,7 +329,7 @@ func countIngressRuleItems(iRule *cloudresource.IngressRule, items map[string]in if iRule.Protocol != nil { proto = *iRule.Protocol } - port := 0 + var port int32 if iRule.FromPort != nil { port = *iRule.FromPort } @@ -351,7 +351,7 @@ func countEgressRuleItems(eRule *cloudresource.EgressRule, items map[string]int, if eRule.Protocol != nil { proto = *eRule.Protocol } - port := 0 + var port int32 if eRule.ToPort != nil { port = *eRule.ToPort } diff --git a/test/integration/networkpolicy_e2e_test.go b/test/integration/networkpolicy_e2e_test.go index eb74bf24..7cbb2c16 100644 --- a/test/integration/networkpolicy_e2e_test.go +++ b/test/integration/networkpolicy_e2e_test.go @@ -29,6 +29,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" logf "sigs.k8s.io/controller-runtime/pkg/log" + cpv1beta2 "antrea.io/antrea/pkg/apis/controlplane/v1beta2" runtimev1alpha1 "antrea.io/nephe/apis/runtime/v1alpha1" cpautils "antrea.io/nephe/pkg/cloudprovider/utils" "antrea.io/nephe/pkg/labels" @@ -190,11 +191,11 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws vmKind := reflect.TypeOf(runtimev1alpha1.VirtualMachine{}).Name() anpSetupParams.AppliedTo = utils.ConfigANPApplyTo(vmKind, "", "", "", "") anpSetupParams.From = utils.ConfigANPToFrom("", "", "", "", "", - "0.0.0.0/0", "", allowedPorts, false) + "0.0.0.0/0", "", cpv1beta2.ProtocolTCP, allowedPorts, false) if denyEgress { - anpSetupParams.To = utils.ConfigANPToFrom("", "", "", "", "", "", "", nil, denyEgress) + anpSetupParams.To = utils.ConfigANPToFrom("", "", "", "", "", "", "", cpv1beta2.ProtocolTCP, nil, denyEgress) } else { - anpSetupParams.To = utils.ConfigANPToFrom("", "", "", "", "", "0.0.0.0/0", "", nil, false) + anpSetupParams.To = utils.ConfigANPToFrom("", "", "", "", "", "0.0.0.0/0", "", cpv1beta2.ProtocolTCP, nil, false) } err := utils.ConfigureK8s(kubeCtl, anpSetupParams, k8stemplates.CloudAntreaNetworkPolicy, false) @@ -301,7 +302,7 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws } } - verifyEgress := func(kind, id, srcVM string, dstIPs []string, oks []bool) { + verifyEgress := func(kind, id, srcVM string, dstIPs []string, oks []bool, ping bool) { // Applied ANP and check configuration. err := utils.ConfigureK8s(kubeCtl, anpParams, k8stemplates.CloudAntreaNetworkPolicy, false) Expect(err).ToNot(HaveOccurred()) @@ -316,11 +317,15 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws err = utils.CheckCloudResourceNetworkPolicies(kubeCtl, k8sClient, kind, namespace.Name, []string{id}, np, withAgent) Expect(err).ToNot(HaveOccurred()) - err = utils.ExecuteCurlCmds(cloudVPC, nil, []string{srcVM}, "", dstIPs, apachePort, oks, 30) + if !ping { + err = utils.ExecuteCurlCmds(cloudVPC, nil, []string{srcVM}, "", dstIPs, apachePort, oks, 30) + } else { + err = utils.ExecutePingCmds(cloudVPC, nil, []string{srcVM}, "", dstIPs, oks, 10) + } Expect(err).ToNot(HaveOccurred()) } - verifyIngress := func(kind, id, ip string, srcVMs []string, oks []bool, verifyOnly bool) { + verifyIngress := func(kind, id, ip string, srcVMs []string, oks []bool, verifyOnly bool, ping bool) { // Applied ANP and check configuration. var err error if !verifyOnly { @@ -338,7 +343,11 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws err = utils.CheckCloudResourceNetworkPolicies(kubeCtl, k8sClient, kind, namespace.Name, []string{id}, np, withAgent) Expect(err).ToNot(HaveOccurred()) - err = utils.ExecuteCurlCmds(cloudVPC, nil, srcVMs, "", []string{ip}, apachePort, oks, 30) + if !ping { + err = utils.ExecuteCurlCmds(cloudVPC, nil, srcVMs, "", []string{ip}, apachePort, oks, 30) + } else { + err = utils.ExecutePingCmds(cloudVPC, nil, srcVMs, "", []string{ip}, oks, 10) + } Expect(err).ToNot(HaveOccurred()) } @@ -358,7 +367,7 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws dstNsName := namespace.Name appliedIdx := len(ids) - 1 - anpParams.From = utils.ConfigANPToFrom(kind, "", "", "", "", "", dstNsName, []string{apachePort}, false) + anpParams.From = utils.ConfigANPToFrom(kind, "", "", "", "", "", dstNsName, cpv1beta2.ProtocolTCP, []string{apachePort}, false) By(fmt.Sprintf("Applied NetworkPolicy to %v by kind label selector", kind)) applied := make([]bool, len(ids)) @@ -424,7 +433,7 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws err := utils.ConfigureK8s(kubeCtl, groupParameters, k8stemplates.CloudAntreaGroup, false) Expect(err).ToNot(HaveOccurred()) - anpParams.From = utils.ConfigANPToFrom(kind, "", "", "", "", "", nsName, []string{apachePort}, false) + anpParams.From = utils.ConfigANPToFrom(kind, "", "", "", "", "", nsName, cpv1beta2.ProtocolTCP, []string{apachePort}, false) if rule { anpParams.RuleAppliedToGroup = &groupParameters } else { @@ -473,15 +482,15 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws oks[i] = true } anpParams.To = utils.ConfigANPToFrom(kind, "", "", "", "", "", dstNsName, - []string{apachePort}, false) - verifyEgress(kind, ids[appliedIdx], srcVM, ips[:len(ips)-1], oks) + cpv1beta2.ProtocolTCP, []string{apachePort}, false) + verifyEgress(kind, ids[appliedIdx], srcVM, ips[:len(ips)-1], oks, false) By(fmt.Sprintf("Egress NetworkPolicy on %v by name label selector", kind)) oks = make([]bool, len(ids)-1) oks[0] = true anpParams.To = utils.ConfigANPToFrom(kind, ids[0], "", "", "", "", dstNsName, - []string{apachePort}, false) - verifyEgress(kind, ids[appliedIdx], srcVM, ips[:len(ips)-1], oks) + cpv1beta2.ProtocolTCP, []string{apachePort}, false) + verifyEgress(kind, ids[appliedIdx], srcVM, ips[:len(ips)-1], oks, false) if abbreviated { return @@ -492,8 +501,8 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws oks[i] = true } anpParams.To = utils.ConfigANPToFrom(kind, "", cloudVPC.GetCRDVPCID(), "", "", "", dstNsName, - []string{apachePort}, false) - verifyEgress(kind, ids[appliedIdx], srcVM, ips[:len(ips)-1], oks) + cpv1beta2.ProtocolTCP, []string{apachePort}, false) + verifyEgress(kind, ids[appliedIdx], srcVM, ips[:len(ips)-1], oks, false) By(fmt.Sprintf("Egress NetworkPolicy on %v by tag label selector", kind)) oks = make([]bool, len(ids)-1) @@ -501,16 +510,37 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws key := "Name" if value, found := cloudVPC.GetTags()[0][key]; found { anpParams.To = utils.ConfigANPToFrom(kind, "", "", key, value, "", dstNsName, - []string{apachePort}, false) - verifyEgress(kind, ids[appliedIdx], srcVM, ips[:len(ips)-1], oks) + cpv1beta2.ProtocolTCP, []string{apachePort}, false) + verifyEgress(kind, ids[appliedIdx], srcVM, ips[:len(ips)-1], oks, false) } By(fmt.Sprintf("Egress NetworkPolicy on %v by IPBlock", kind)) oks = make([]bool, len(ids)-1) oks[1] = true anpParams.To = utils.ConfigANPToFrom("", "", "", "", "", ips[1]+"/32", dstNsName, - []string{apachePort}, false) - verifyEgress(kind, ids[appliedIdx], srcVM, ips[:len(ips)-1], oks) + cpv1beta2.ProtocolTCP, []string{apachePort}, false) + verifyEgress(kind, ids[appliedIdx], srcVM, ips[:len(ips)-1], oks, false) + } + + testEgressWithPing := func(kind string) { + var ids []string + if kind == reflect.TypeOf(runtimev1alpha1.VirtualMachine{}).Name() { + ids = cloudVPC.GetVMs() + } else { + Fail("Unsupported type") + } + setup(kind, len(ids), []string{"22", "8080"}, true) + dstNsName := namespace.Name + + appliedIdx := len(ids) - 1 + srcVM := cloudVPC.GetVMs()[appliedIdx] + anpParams.AppliedTo = utils.ConfigANPApplyTo(kind, ids[appliedIdx], "", "", "") + + By(fmt.Sprintf("Egress NetworkPolicy on %v by kind label selector", kind)) + oks := []bool{true} + anpParams.To = utils.ConfigANPToFrom("", "", "", "", "", "", dstNsName, + cpv1beta2.ProtocolICMP, []string{}, false) + verifyEgress(kind, ids[appliedIdx], srcVM, []string{"8.8.8.8"}, oks, true) } testIngress := func(kind string) { @@ -536,15 +566,15 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws oks[i] = true } anpParams.From = utils.ConfigANPToFrom(kind, "", "", "", "", "", dstNsName, - []string{apachePort}, false) - verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false) + cpv1beta2.ProtocolTCP, []string{apachePort}, false) + verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false, false) By(fmt.Sprintf("Ingress NetworkPolicy on %v by name label selector", kind)) oks = make([]bool, len(ids)-1) oks[0] = true anpParams.From = utils.ConfigANPToFrom(kind, ids[0], "", "", "", "", dstNsName, - []string{apachePort}, false) - verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false) + cpv1beta2.ProtocolTCP, []string{apachePort}, false) + verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false, false) if abbreviated { return @@ -555,8 +585,8 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws oks[i] = true } anpParams.From = utils.ConfigANPToFrom(kind, "", cloudVPC.GetCRDVPCID(), "", "", "", - dstNsName, []string{apachePort}, false) - verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false) + dstNsName, cpv1beta2.ProtocolTCP, []string{apachePort}, false) + verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false, false) By(fmt.Sprintf("Ingress NetworkPolicy on %v by tag label selector", kind)) oks = make([]bool, len(ids)-1) @@ -564,16 +594,16 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws key := "Name" if value, found := cloudVPC.GetTags()[0][key]; found { anpParams.From = utils.ConfigANPToFrom(kind, "", "", key, value, "", dstNsName, - []string{apachePort}, false) - verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false) + cpv1beta2.ProtocolTCP, []string{apachePort}, false) + verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false, false) } By(fmt.Sprintf("Ingress NetworkPolicy on %v by IPBlock", kind)) oks = make([]bool, len(ids)-1) oks[1] = true anpParams.From = utils.ConfigANPToFrom("", "", "", "", "", ips[1]+"/32", dstNsName, - []string{apachePort}, false) - verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false) + cpv1beta2.ProtocolTCP, []string{apachePort}, false) + verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false, false) } testIngressAllowAll := func(kind string) { @@ -598,8 +628,31 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws } // wildcard ipblock and port. anpParams.From = utils.ConfigANPToFrom("", "", "", "", "", "", namespace.Name, - []string{}, false) - verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false) + cpv1beta2.ProtocolTCP, []string{}, false) + verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false, false) + } + + testIngressWithPing := func(kind string) { + var ids []string + var ips []string + if kind == reflect.TypeOf(runtimev1alpha1.VirtualMachine{}).Name() { + ids = cloudVPC.GetVMs() + ips = cloudVPC.GetVMPrivateIPs() + } else { + Fail("Unsupported type") + } + setup(kind, len(ids), []string{"22"}, false) + + appliedIdx := len(ids) - 1 + srcVMs := cloudVPC.GetVMs()[:appliedIdx] + anpParams.AppliedTo = utils.ConfigANPApplyTo(kind, ids[appliedIdx], "", "", "") + oks := make([]bool, len(ids)-1) + for i := range oks { + oks[i] = true + } + anpParams.From = utils.ConfigANPToFrom("", "", "", "", "", "", namespace.Name, + cpv1beta2.ProtocolICMP, []string{}, false) + verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false, true) } testIngressDenyAll := func(kind string) { @@ -625,8 +678,8 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws // wildcard ipblock and port. anpParams.From = utils.ConfigANPToFrom("", "", "", "", "", "", namespace.Name, - []string{}, false) - verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false) + cpv1beta2.ProtocolTCP, []string{}, false) + verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false, false) By("Make default rule higher priority") defaultANPParameters.Priority = 5 @@ -635,7 +688,7 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws for i := range oks { oks[i] = false } - verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false) + verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false, false) } DescribeTable("AppliedTo", @@ -670,6 +723,14 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws reflect.TypeOf(runtimev1alpha1.VirtualMachine{}).Name()), ) + DescribeTable("Egress With Ping", + func(kind string) { + testEgressWithPing(kind) + }, + Entry(fmt.Sprintf("%s: VM In Same Namespace", focusAzure), + reflect.TypeOf(runtimev1alpha1.VirtualMachine{}).Name()), + ) + DescribeTable("Ingress", func(kind string) { testIngress(kind) @@ -696,6 +757,14 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws ) } + DescribeTable("Ingress Test WithPing", + func(kind string) { + testIngressWithPing(kind) + }, + Entry(fmt.Sprintf("%s: VM In Same Namespace", focusAzure), + reflect.TypeOf(runtimev1alpha1.VirtualMachine{}).Name()), + ) + Context("Enforce Before Import", func() { JustBeforeEach(func() { importAfterANP = true @@ -744,7 +813,7 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws oks := make([]bool, len(ids)-1) oks[0] = true anpParams.From = utils.ConfigANPToFrom(kind, ids[0], "", "", "", "", namespace.Name, - []string{apachePort}, false) + cpv1beta2.ProtocolTCP, []string{apachePort}, false) err = utils.ConfigureK8s(kubeCtl, anpParams, k8stemplates.CloudAntreaNetworkPolicy, false) Expect(err).ToNot(HaveOccurred()) err = utils.StartOrWaitDeployment(k8sClient, "nephe-controller", "nephe-system", replicas, time.Second*120) @@ -752,7 +821,7 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws // wait for aggregated api server to ready. err = utils.WaitApiServer(k8sClient, time.Second*60) Expect(err).NotTo(HaveOccurred()) - verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, true) + verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, true, false) By("Changed NetworkPolicy") replicas, err = utils.StopDeployment(k8sClient, "nephe-controller", "nephe-system", time.Second*120) @@ -762,14 +831,14 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws oks[i] = true } anpParams.From = utils.ConfigANPToFrom(kind, "", "", "", "", "", namespace.Name, - []string{apachePort}, false) + cpv1beta2.ProtocolTCP, []string{apachePort}, false) err = utils.ConfigureK8s(kubeCtl, anpParams, k8stemplates.CloudAntreaNetworkPolicy, false) Expect(err).ToNot(HaveOccurred()) err = utils.StartOrWaitDeployment(k8sClient, "nephe-controller", "nephe-system", replicas, time.Second*120) Expect(err).NotTo(HaveOccurred()) err = utils.WaitApiServer(k8sClient, time.Second*60) Expect(err).NotTo(HaveOccurred()) - verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, true) + verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, true, false) By("Stale NetworkPolicy") replicas, err = utils.StopDeployment(k8sClient, "nephe-controller", "nephe-system", time.Second*120) @@ -812,25 +881,25 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws oks[i] = true } anpParams.From = utils.ConfigANPToFrom(kind, "", "", "", "", "", namespace.Name, - []string{apachePort}, false) - verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false) + cpv1beta2.ProtocolTCP, []string{apachePort}, false) + verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false, false) err = utils.RestartOrWaitDeployment(k8sClient, "antrea-controller", "kube-system", time.Second*200, true) Expect(err).ToNot(HaveOccurred()) time.Sleep(time.Second * 30) - verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, true) + verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, true, false) By(fmt.Sprintf("Ingress NetworkPolicy on %v by name label selector while restarting Nephe controller", kind)) anpParams.AppliedTo = utils.ConfigANPApplyTo(kind, ids[appliedIdx], "", "", "") oks = make([]bool, len(ids)-1) oks[0] = true anpParams.From = utils.ConfigANPToFrom(kind, ids[0], "", "", "", "", namespace.Name, - []string{apachePort}, false) - verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false) + cpv1beta2.ProtocolTCP, []string{apachePort}, false) + verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, false, false) By("Restarting controllers now...") err = utils.RestartOrWaitDeployment(k8sClient, "nephe-controller", "nephe-system", time.Second*200, true) Expect(err).ToNot(HaveOccurred()) time.Sleep(time.Second * 30) - verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, true) + verifyIngress(kind, ids[appliedIdx], ips[appliedIdx], srcVMs, oks, true, false) }) It("Remove Stale Member", func() { @@ -840,7 +909,7 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy On Cloud Resources", focusAws setup(kind, len(ids), []string{"22"}, false) anpParams.AppliedTo = utils.ConfigANPApplyTo(kind, "", cloudVPC.GetCRDVPCID(), "", "") anpParams.From = utils.ConfigANPToFrom(kind, "", "", "", "", "", namespace.Name, - []string{apachePort}, false) + cpv1beta2.ProtocolTCP, []string{apachePort}, false) err := utils.ConfigureK8s(kubeCtl, anpParams, k8stemplates.CloudAntreaNetworkPolicy, false) Expect(err).ToNot(HaveOccurred()) diff --git a/test/templates/vm_anp.go b/test/templates/vm_anp.go index 7456a824..1078ec1a 100644 --- a/test/templates/vm_anp.go +++ b/test/templates/vm_anp.go @@ -26,11 +26,12 @@ type NamespaceParameters struct { } type ToFromParameters struct { - Entity *EntitySelectorParameters - Namespace *NamespaceParameters - IPBlock string - Ports []*PortParameters - DenyAll bool + Entity *EntitySelectorParameters + Namespace *NamespaceParameters + IPBlock string + Ports []*PortParameters + ICMPProtocol string + DenyAll bool } type PortParameters struct { @@ -127,6 +128,10 @@ spec: port: {{$port.Port}} {{ end }} {{- end }}{{/* .From.Ports */}} +{{- if .From.ICMPProtocol }} + protocols: + - icmp: {} +{{- end }}{{/* .From.ICMPProtocol */}} {{- if .RuleAppliedToGroup }} appliedTo: - group: {{ .RuleAppliedToGroup.Name }} @@ -174,6 +179,10 @@ spec: port: {{$port.Port}} {{ end }} {{- end }}{{/* .To.Ports */}} +{{- if .To.ICMPProtocol }} + protocols: + - icmp: {} +{{- end }}{{/* .To.ICMPProtocol */}} {{ end }} {{/* .To */}} ` const CloudAntreaGroup = ` diff --git a/test/upgrade/networkpolicy_e2e_test.go b/test/upgrade/networkpolicy_e2e_test.go index 3b98b295..699361b8 100644 --- a/test/upgrade/networkpolicy_e2e_test.go +++ b/test/upgrade/networkpolicy_e2e_test.go @@ -29,6 +29,7 @@ import ( v1 "k8s.io/api/core/v1" logf "sigs.k8s.io/controller-runtime/pkg/log" + controlplanev1beta2 "antrea.io/antrea/pkg/apis/controlplane/v1beta2" runtimev1alpha1 "antrea.io/nephe/apis/runtime/v1alpha1" "antrea.io/nephe/pkg/labels" "antrea.io/nephe/test/utils" @@ -164,11 +165,11 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy upgrade test", focusAws, focu vmKind := reflect.TypeOf(runtimev1alpha1.VirtualMachine{}).Name() anpSetupParams.AppliedTo = utils.ConfigANPApplyTo(vmKind, "", "", "", "") anpSetupParams.From = utils.ConfigANPToFrom("", "", "", "", "", - "0.0.0.0/0", "", allowedPorts, false) + "0.0.0.0/0", "", controlplanev1beta2.ProtocolTCP, allowedPorts, false) if denyEgress { - anpSetupParams.To = utils.ConfigANPToFrom("", "", "", "", "", "", "", nil, denyEgress) + anpSetupParams.To = utils.ConfigANPToFrom("", "", "", "", "", "", "", controlplanev1beta2.ProtocolTCP, nil, denyEgress) } else { - anpSetupParams.To = utils.ConfigANPToFrom("", "", "", "", "", "0.0.0.0/0", "", nil, false) + anpSetupParams.To = utils.ConfigANPToFrom("", "", "", "", "", "0.0.0.0/0", "", controlplanev1beta2.ProtocolTCP, nil, false) } err := utils.ConfigureK8s(kubeCtl, anpSetupParams, k8stemplates.CloudAntreaNetworkPolicy, false) @@ -252,7 +253,7 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy upgrade test", focusAws, focu dstNsName := namespace.Name appliedIdx := len(ids) - 1 - anpParams.From = utils.ConfigANPToFrom(kind, "", "", "", "", "", dstNsName, []string{apachePort}, false) + anpParams.From = utils.ConfigANPToFrom(kind, "", "", "", "", "", dstNsName, controlplanev1beta2.ProtocolTCP, []string{apachePort}, false) By(fmt.Sprintf("Applied NetworkPolicy to %v by kind label selector", kind)) applied := make([]bool, len(ids)) @@ -318,7 +319,7 @@ var _ = Describe(fmt.Sprintf("%s,%s: NetworkPolicy upgrade test", focusAws, focu srcVM := cloudVPC.GetVMs()[0] srcIP := cloudVPC.GetVMPrivateIPs()[0] dstNsName := namespace.Name - anpParams.From = utils.ConfigANPToFrom(kind, "", "", "", "", "", dstNsName, []string{apachePort}, false) + anpParams.From = utils.ConfigANPToFrom(kind, "", "", "", "", "", dstNsName, controlplanev1beta2.ProtocolTCP, []string{apachePort}, false) By(fmt.Sprintf("Applied NetworkPolicy to %v by kind label selector", kind)) applied := make([]bool, len(ids)) diff --git a/test/utils/helpers.go b/test/utils/helpers.go index c85290a9..ce609288 100644 --- a/test/utils/helpers.go +++ b/test/utils/helpers.go @@ -408,6 +408,16 @@ func ExecuteCurlCmds(vpc CloudVPC, kubctl *KubeCtl, return ExecuteCmds(vpc, kubctl, srcIDs, ns, cmds, oks, retries) } +// ExecutePingCmds executes ping on resource srcIDs in parallel, and returns error if oks mismatch. +func ExecutePingCmds(vpc CloudVPC, kubctl *KubeCtl, + srcIDs []string, ns string, destIPs []string, oks []bool, retries int) error { + cmds := make([][]string, 0, len(destIPs)) + for _, ip := range destIPs { + cmds = append(cmds, []string{"ping", "-c", "1", "-w", "1", ip}) + } + return ExecuteCmds(vpc, kubctl, srcIDs, ns, cmds, oks, retries) +} + // CheckRestart returns error if nephe controller has restarted. func CheckRestart(kubctl *KubeCtl) error { controllers := []string{"nephe-controller"} diff --git a/test/utils/networkpolicy_helper.go b/test/utils/networkpolicy_helper.go index 569f53a2..45af6932 100644 --- a/test/utils/networkpolicy_helper.go +++ b/test/utils/networkpolicy_helper.go @@ -17,8 +17,7 @@ package utils import ( "strings" - v1 "k8s.io/api/core/v1" - + cpv1beta2 "antrea.io/antrea/pkg/apis/controlplane/v1beta2" "antrea.io/nephe/pkg/labels" k8stemplates "antrea.io/nephe/test/templates" ) @@ -43,7 +42,7 @@ func ConfigANPApplyTo(kind, instanceName, vpc, tagKey, tagVal string) *k8stempla } // ConfigANPToFrom helper function to configure to and from fields in Antrea NetworkPolicy. -func ConfigANPToFrom(kind, instanceName, vpc, tagKey, tagVal, ipBlock, nsName string, ports []string, +func ConfigANPToFrom(kind, instanceName, vpc, tagKey, tagVal, ipBlock, nsName string, protocol cpv1beta2.Protocol, ports []string, denyAll bool) *k8stemplates.ToFromParameters { ret := &k8stemplates.ToFromParameters{ DenyAll: denyAll, @@ -67,8 +66,12 @@ func ConfigANPToFrom(kind, instanceName, vpc, tagKey, tagVal, ipBlock, nsName st } } - for _, p := range ports { - ret.Ports = append(ret.Ports, &k8stemplates.PortParameters{Protocol: string(v1.ProtocolTCP), Port: p}) + if protocol == cpv1beta2.ProtocolTCP { + for _, p := range ports { + ret.Ports = append(ret.Ports, &k8stemplates.PortParameters{Protocol: string(protocol), Port: p}) + } + } else if protocol == cpv1beta2.ProtocolICMP { + ret.ICMPProtocol = string(cpv1beta2.ProtocolICMP) } return ret }