Skip to content

Commit

Permalink
Fix coredns failing during custom networking tests (#2844)
Browse files Browse the repository at this point in the history 
Co-authored-by: Joseph Chen <chenjoez@amazon.com>
  • Loading branch information
@jchen6585
jchen6585 and Joseph Chen authored Mar 13, 2024
1 parent f2c3f73 commit 4f42b6f
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 49 deletions.
100 changes: 64 additions & 36 deletions test/framework/resources/aws/services/ec2.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ type EC2 interface {
DescribeInstance(instanceID string) (*ec2.Instance, error)
DescribeVPC(vpcID string) (*ec2.DescribeVpcsOutput, error)
DescribeNetworkInterface(interfaceIDs []string) (*ec2.DescribeNetworkInterfacesOutput, error)
AuthorizeSecurityGroupIngress(groupID string, protocol string, fromPort int, toPort int, cidrIP string) error
RevokeSecurityGroupIngress(groupID string, protocol string, fromPort int, toPort int, cidrIP string) error
AuthorizeSecurityGroupIngress(groupID string, protocol string, fromPort int, toPort int, cidrIP string, sourceSG bool) error
RevokeSecurityGroupIngress(groupID string, protocol string, fromPort int, toPort int, cidrIP string, sourceSG bool) error
AuthorizeSecurityGroupEgress(groupID string, protocol string, fromPort int, toPort int, cidrIP string) error
RevokeSecurityGroupEgress(groupID string, protocol string, fromPort int, toPort int, cidrIP string) error
AssociateVPCCIDRBlock(vpcId string, cidrBlock string) (*ec2.AssociateVpcCidrBlockOutput, error)
Expand Down Expand Up @@ -96,30 +96,44 @@ func (d *defaultEC2) DescribeInstance(instanceID string) (*ec2.Instance, error)
return describeInstanceOutput.Reservations[0].Instances[0], nil
}

func (d *defaultEC2) AuthorizeSecurityGroupIngress(groupID string, protocol string, fromPort int, toPort int, cidrIP string) error {
func (d *defaultEC2) AuthorizeSecurityGroupIngress(groupID string, protocol string, fromPort int, toPort int, cidrIP string, sourceSG bool) error {
var ipv4Ranges []*ec2.IpRange
var ipv6Ranges []*ec2.Ipv6Range
if strings.Contains(cidrIP, ":") {
ipv6Ranges = []*ec2.Ipv6Range{
{
CidrIpv6: aws.String(cidrIP),
},
var ipPermissions *ec2.IpPermission
if !sourceSG {
if strings.Contains(cidrIP, ":") {
ipv6Ranges = []*ec2.Ipv6Range{
{
CidrIpv6: aws.String(cidrIP),
},
}
} else {
ipv4Ranges = []*ec2.IpRange{
{
CidrIp: aws.String(cidrIP),
},
}
}

ipPermissions = &ec2.IpPermission{
FromPort: aws.Int64(int64(fromPort)),
ToPort: aws.Int64(int64(toPort)),
IpProtocol: aws.String(protocol),
IpRanges: ipv4Ranges,
Ipv6Ranges: ipv6Ranges,
}
} else {
ipv4Ranges = []*ec2.IpRange{
{
CidrIp: aws.String(cidrIP),
ipPermissions = &ec2.IpPermission{
FromPort: aws.Int64(int64(fromPort)),
ToPort: aws.Int64(int64(toPort)),
IpProtocol: aws.String(protocol),
UserIdGroupPairs: []*ec2.UserIdGroupPair{
{
GroupId: aws.String(cidrIP),
},
},
}
}

ipPermissions := &ec2.IpPermission{
FromPort: aws.Int64(int64(fromPort)),
ToPort: aws.Int64(int64(toPort)),
IpProtocol: aws.String(protocol),
IpRanges: ipv4Ranges,
Ipv6Ranges: ipv6Ranges,
}
authorizeSecurityGroupIngressInput := &ec2.AuthorizeSecurityGroupIngressInput{
GroupId: aws.String(groupID),
IpPermissions: []*ec2.IpPermission{ipPermissions},
Expand All @@ -128,30 +142,44 @@ func (d *defaultEC2) AuthorizeSecurityGroupIngress(groupID string, protocol stri
return err
}

func (d *defaultEC2) RevokeSecurityGroupIngress(groupID string, protocol string, fromPort int, toPort int, cidrIP string) error {
func (d *defaultEC2) RevokeSecurityGroupIngress(groupID string, protocol string, fromPort int, toPort int, cidrIP string, sourceSG bool) error {
var ipv4Ranges []*ec2.IpRange
var ipv6Ranges []*ec2.Ipv6Range
if strings.Contains(cidrIP, ":") {
ipv6Ranges = []*ec2.Ipv6Range{
{
CidrIpv6: aws.String(cidrIP),
},
var ipPermissions *ec2.IpPermission
if !sourceSG {
if strings.Contains(cidrIP, ":") {
ipv6Ranges = []*ec2.Ipv6Range{
{
CidrIpv6: aws.String(cidrIP),
},
}
} else {
ipv4Ranges = []*ec2.IpRange{
{
CidrIp: aws.String(cidrIP),
},
}
}

ipPermissions = &ec2.IpPermission{
FromPort: aws.Int64(int64(fromPort)),
ToPort: aws.Int64(int64(toPort)),
IpProtocol: aws.String(protocol),
IpRanges: ipv4Ranges,
Ipv6Ranges: ipv6Ranges,
}
} else {
ipv4Ranges = []*ec2.IpRange{
{
CidrIp: aws.String(cidrIP),
ipPermissions = &ec2.IpPermission{
FromPort: aws.Int64(int64(fromPort)),
ToPort: aws.Int64(int64(toPort)),
IpProtocol: aws.String(protocol),
UserIdGroupPairs: []*ec2.UserIdGroupPair{
{
GroupId: aws.String(cidrIP),
},
},
}
}

ipPermissions := &ec2.IpPermission{
FromPort: aws.Int64(int64(fromPort)),
ToPort: aws.Int64(int64(toPort)),
IpProtocol: aws.String(protocol),
IpRanges: ipv4Ranges,
Ipv6Ranges: ipv6Ranges,
}
revokeSecurityGroupIngressInput := &ec2.RevokeSecurityGroupIngressInput{
GroupId: aws.String(groupID),
IpPermissions: []*ec2.IpPermission{ipPermissions},
Expand Down
4 changes: 2 additions & 2 deletions test/integration/cni/pod_traffic_across_az_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ var _ = Describe("[STATIC_CANARY] test pod networking", FlakeAttempts(retries),
JustBeforeEach(func() {
By("authorizing security group ingress on instance security group")
err = f.CloudServices.EC2().
AuthorizeSecurityGroupIngress(instanceSecurityGroupID, protocol, serverPort, serverPort, "0.0.0.0/0")
AuthorizeSecurityGroupIngress(instanceSecurityGroupID, protocol, serverPort, serverPort, "0.0.0.0/0", false)
Expect(err).ToNot(HaveOccurred())

By("authorizing security group egress on instance security group")
Expand Down Expand Up @@ -140,7 +140,7 @@ var _ = Describe("[STATIC_CANARY] test pod networking", FlakeAttempts(retries),
JustAfterEach(func() {
By("revoking security group ingress on instance security group")
err = f.CloudServices.EC2().
RevokeSecurityGroupIngress(instanceSecurityGroupID, protocol, serverPort, serverPort, "0.0.0.0/0")
RevokeSecurityGroupIngress(instanceSecurityGroupID, protocol, serverPort, serverPort, "0.0.0.0/0", false)
Expect(err).ToNot(HaveOccurred())

By("revoking security group egress on instance security group")
Expand Down
4 changes: 2 additions & 2 deletions test/integration/cni/pod_traffic_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ var _ = Describe("test pod networking", func() {
JustBeforeEach(func() {
By("authorizing security group ingress on instance security group")
err = f.CloudServices.EC2().
AuthorizeSecurityGroupIngress(instanceSecurityGroupID, protocol, serverPort, serverPort, "0.0.0.0/0")
AuthorizeSecurityGroupIngress(instanceSecurityGroupID, protocol, serverPort, serverPort, "0.0.0.0/0", false)
Expect(err).ToNot(HaveOccurred())

By("authorizing security group egress on instance security group")
Expand Down Expand Up @@ -139,7 +139,7 @@ var _ = Describe("test pod networking", func() {
JustAfterEach(func() {
By("revoking security group ingress on instance security group")
err = f.CloudServices.EC2().
RevokeSecurityGroupIngress(instanceSecurityGroupID, protocol, serverPort, serverPort, "0.0.0.0/0")
RevokeSecurityGroupIngress(instanceSecurityGroupID, protocol, serverPort, serverPort, "0.0.0.0/0", false)
Expect(err).ToNot(HaveOccurred())

By("revoking security group egress on instance security group")
Expand Down
61 changes: 59 additions & 2 deletions test/integration/custom-networking/custom_networking_suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,17 @@ import (
"net"
"testing"

"github.com/apparentlymart/go-cidr/cidr"
"github.com/aws/amazon-vpc-cni-k8s/pkg/apis/crd/v1alpha1"
"github.com/aws/amazon-vpc-cni-k8s/test/framework"
awsUtils "github.com/aws/amazon-vpc-cni-k8s/test/framework/resources/aws/utils"
"github.com/aws/amazon-vpc-cni-k8s/test/framework/resources/k8s/manifest"
k8sUtils "github.com/aws/amazon-vpc-cni-k8s/test/framework/resources/k8s/utils"
"github.com/aws/amazon-vpc-cni-k8s/test/framework/utils"
"github.com/aws/amazon-vpc-cni-k8s/test/integration/common"
"github.com/prometheus/client_golang/prometheus"
corev1 "k8s.io/api/core/v1"

"github.com/apparentlymart/go-cidr/cidr"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
Expand All @@ -51,6 +53,9 @@ var (
customNetworkingSGID string
customNetworkingSGOpenPort = 8080
customNetworkingSubnetIDList []string
corednsSGOpenPort = 53
primaryENISGID string
primaryENISGList []string
// List of ENIConfig per Availability Zone
eniConfigList []*v1alpha1.ENIConfig
eniConfigBuilderList []*manifest.ENIConfigBuilder
Expand Down Expand Up @@ -85,7 +90,53 @@ var _ = BeforeSuite(func() {
f.CloudServices.EC2().AuthorizeSecurityGroupEgress(customNetworkingSGID, "TCP",
customNetworkingSGOpenPort, customNetworkingSGOpenPort, "0.0.0.0/0")
f.CloudServices.EC2().AuthorizeSecurityGroupIngress(customNetworkingSGID, "TCP",
customNetworkingSGOpenPort, customNetworkingSGOpenPort, "0.0.0.0/0")
customNetworkingSGOpenPort, customNetworkingSGOpenPort, "0.0.0.0/0", false)
f.CloudServices.EC2().AuthorizeSecurityGroupEgress(customNetworkingSGID, "UDP",
corednsSGOpenPort, corednsSGOpenPort, "0.0.0.0/0")
f.CloudServices.EC2().AuthorizeSecurityGroupIngress(customNetworkingSGID, "UDP",
corednsSGOpenPort, corednsSGOpenPort, "0.0.0.0/0", false)
f.CloudServices.EC2().AuthorizeSecurityGroupEgress(customNetworkingSGID, "TCP",
corednsSGOpenPort, corednsSGOpenPort, "0.0.0.0/0")
f.CloudServices.EC2().AuthorizeSecurityGroupIngress(customNetworkingSGID, "TCP",
corednsSGOpenPort, corednsSGOpenPort, "0.0.0.0/0", false)

By("Adding custom networking security group ingress rule from primary eni")
nodeList, err := f.K8sResourceManagers.NodeManager().GetNodes(f.Options.NgNameLabelKey,
f.Options.NgNameLabelVal)
Expect(err).ToNot(HaveOccurred())

var primaryNode *corev1.Node
for _, n := range nodeList.Items {
if len(n.Spec.Taints) == 0 {
primaryNode = &n
break
}
}
Expect(primaryNode).To(Not(BeNil()), "expected to find a non-tainted node")

instanceID := k8sUtils.GetInstanceIDFromNode(*primaryNode)
primaryInstance, err := f.CloudServices.EC2().DescribeInstance(instanceID)
Expect(err).ToNot(HaveOccurred())

instance, err := f.CloudServices.EC2().DescribeInstance(*primaryInstance.InstanceId)
Expect(err).ToNot(HaveOccurred())

var primaryENIID string
for _, nwInterface := range instance.NetworkInterfaces {
primaryENI := common.IsPrimaryENI(nwInterface, instance.PrivateIpAddress)
if primaryENI {
primaryENIID = *nwInterface.NetworkInterfaceId
break
}
}

eniOutput, err := f.CloudServices.EC2().DescribeNetworkInterface([]string{primaryENIID})
Expect(err).ToNot(HaveOccurred())
for _, sg := range eniOutput.NetworkInterfaces[0].Groups {
primaryENISGList = append(primaryENISGList, *sg.GroupId)
f.CloudServices.EC2().AuthorizeSecurityGroupIngress(*sg.GroupId, "-1",
-1, -1, customNetworkingSGID, true)
}

By("associating cidr range to the VPC")
association, err := f.CloudServices.EC2().AssociateVPCCIDRBlock(f.Options.AWSVPCID, cidrRange.String())
Expand Down Expand Up @@ -160,6 +211,12 @@ var _ = AfterSuite(func() {
By("terminating instances")
errs.Append(awsUtils.TerminateInstances(f))

By("Removing custom networking security group ingress rule from primary eni")
for _, sg := range primaryENISGList {
f.CloudServices.EC2().RevokeSecurityGroupIngress(sg, "-1",
-1, -1, customNetworkingSGID, true)
}

By("deleting security group")
errs.Append(f.CloudServices.EC2().DeleteSecurityGroup(customNetworkingSGID))

Expand Down
6 changes: 3 additions & 3 deletions test/integration/pod-eni/security_group_per_pod_suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,11 @@ var _ = BeforeSuite(func() {
By("authorizing egress and ingress on security group for client-server communication")
if isIPv4Cluster {
f.CloudServices.EC2().AuthorizeSecurityGroupEgress(securityGroupId, "tcp", openPort, openPort, v4Zero)
f.CloudServices.EC2().AuthorizeSecurityGroupIngress(securityGroupId, "tcp", openPort, openPort, v4Zero)
f.CloudServices.EC2().AuthorizeSecurityGroupIngress(securityGroupId, "tcp", openPort, openPort, v4Zero, false)
} else {
f.CloudServices.EC2().AuthorizeSecurityGroupEgress(securityGroupId, "tcp", openPort, openPort, v6Zero)
f.CloudServices.EC2().AuthorizeSecurityGroupIngress(securityGroupId, "tcp", openPort, openPort, v6Zero)
f.CloudServices.EC2().AuthorizeSecurityGroupIngress(securityGroupId, "icmpv6", -1, -1, v6Zero)
f.CloudServices.EC2().AuthorizeSecurityGroupIngress(securityGroupId, "tcp", openPort, openPort, v6Zero, false)
f.CloudServices.EC2().AuthorizeSecurityGroupIngress(securityGroupId, "icmpv6", -1, -1, v6Zero, false)
}

By("getting branch ENI limits")
Expand Down
8 changes: 4 additions & 4 deletions test/integration/pod-eni/security_group_per_pod_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,10 @@ var _ = Describe("Security Group for Pods Test", func() {
// 8080: metric-pod listener port
By("Adding an additional Ingress Rule on NodeSecurityGroupID to allow client-to-metric traffic")
if isIPv4Cluster {
err := f.CloudServices.EC2().AuthorizeSecurityGroupIngress(clusterSGID, "TCP", metricsPort, metricsPort, v4Zero)
err := f.CloudServices.EC2().AuthorizeSecurityGroupIngress(clusterSGID, "TCP", metricsPort, metricsPort, v4Zero, false)
Expect(err).ToNot(HaveOccurred())
} else {
err := f.CloudServices.EC2().AuthorizeSecurityGroupIngress(clusterSGID, "TCP", metricsPort, metricsPort, v6Zero)
err := f.CloudServices.EC2().AuthorizeSecurityGroupIngress(clusterSGID, "TCP", metricsPort, metricsPort, v6Zero, false)
Expect(err).ToNot(HaveOccurred())
}
})
Expand Down Expand Up @@ -160,10 +160,10 @@ var _ = Describe("Security Group for Pods Test", func() {
// Revoke the Ingress rule for traffic from client pods added to Node Security Group
By("Revoking the additional Ingress rule added to allow client-to-metric traffic")
if isIPv4Cluster {
err := f.CloudServices.EC2().RevokeSecurityGroupIngress(clusterSGID, "TCP", metricsPort, metricsPort, v4Zero)
err := f.CloudServices.EC2().RevokeSecurityGroupIngress(clusterSGID, "TCP", metricsPort, metricsPort, v4Zero, false)
Expect(err).ToNot(HaveOccurred())
} else {
err := f.CloudServices.EC2().RevokeSecurityGroupIngress(clusterSGID, "TCP", metricsPort, metricsPort, v6Zero)
err := f.CloudServices.EC2().RevokeSecurityGroupIngress(clusterSGID, "TCP", metricsPort, metricsPort, v6Zero, false)
Expect(err).ToNot(HaveOccurred())
}
})
Expand Down

0 comments on commit 4f42b6f

Please sign in to comment.