Skip to content

Commit b2d1d3d

Browse files
r4f4r4f4
committed
🌱: securitygroup: add unit test against unnecessary revokes
This test checks that target ingress rules that have already been authorized are not revoked and then authorized again.
1 parent ce5aaf7 commit b2d1d3d

File tree

1 file changed

+289
-0
lines changed

1 file changed

+289
-0
lines changed

‎pkg/cloud/services/securitygroup/securitygroups_test.go‎

Lines changed: 289 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -827,6 +827,295 @@ func TestReconcileSecurityGroups(t *testing.T) {
827827
Return(&ec2.AuthorizeSecurityGroupIngressOutput{}, nil).AnyTimes()
828828
},
829829
},
830+
{
831+
name: "authorized target ingress rules are not revoked",
832+
awsCluster: func(acl infrav1.AWSCluster) infrav1.AWSCluster {
833+
return acl
834+
},
835+
input: &infrav1.NetworkSpec{
836+
VPC: infrav1.VPCSpec{
837+
ID: "vpc-securitygroups",
838+
InternetGatewayID: aws.String("igw-01"),
839+
Tags: infrav1.Tags{
840+
infrav1.ClusterTagKey("test-cluster"): "owned",
841+
},
842+
EmptyRoutesDefaultVPCSecurityGroup: true,
843+
},
844+
Subnets: infrav1.Subnets{
845+
infrav1.SubnetSpec{
846+
ID: "subnet-securitygroups-private",
847+
IsPublic: false,
848+
AvailabilityZone: "us-east-1a",
849+
},
850+
infrav1.SubnetSpec{
851+
ID: "subnet-securitygroups-public",
852+
IsPublic: true,
853+
NatGatewayID: aws.String("nat-01"),
854+
AvailabilityZone: "us-east-1a",
855+
},
856+
},
857+
},
858+
expect: func(m *mocks.MockEC2APIMockRecorder) {
859+
m.DescribeSecurityGroupsWithContext(context.TODO(), &ec2.DescribeSecurityGroupsInput{
860+
Filters: []*ec2.Filter{
861+
filter.EC2.VPC("vpc-securitygroups"),
862+
filter.EC2.SecurityGroupName("default"),
863+
},
864+
}).
865+
Return(&ec2.DescribeSecurityGroupsOutput{
866+
SecurityGroups: []*ec2.SecurityGroup{
867+
{
868+
Description: aws.String("default VPC security group"),
869+
GroupName: aws.String("default"),
870+
GroupId: aws.String("sg-default"),
871+
},
872+
},
873+
}, nil)
874+
875+
m.RevokeSecurityGroupIngressWithContext(context.TODO(), gomock.Eq(&ec2.RevokeSecurityGroupIngressInput{
876+
GroupId: aws.String("sg-default"),
877+
IpPermissions: []*ec2.IpPermission{
878+
{
879+
IpProtocol: aws.String("-1"),
880+
UserIdGroupPairs: []*ec2.UserIdGroupPair{
881+
{
882+
GroupId: aws.String("sg-default"),
883+
},
884+
},
885+
},
886+
},
887+
})).Times(1)
888+
889+
m.RevokeSecurityGroupEgressWithContext(context.TODO(), gomock.AssignableToTypeOf(&ec2.RevokeSecurityGroupEgressInput{
890+
GroupId: aws.String("sg-default"),
891+
}))
892+
893+
securityGroupBastion := &ec2.SecurityGroup{
894+
Description: aws.String("Kubernetes cluster test-cluster: bastion"),
895+
GroupName: aws.String("test-cluster-bastion"),
896+
GroupId: aws.String("sg-bastion"),
897+
Tags: []*ec2.Tag{
898+
{
899+
Key: aws.String("Name"),
900+
Value: aws.String("test-cluster-bastion"),
901+
}, {
902+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
903+
Value: aws.String("owned"),
904+
}, {
905+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
906+
Value: aws.String("bastion"),
907+
},
908+
},
909+
}
910+
911+
securityGroupLB := &ec2.SecurityGroup{
912+
Description: aws.String("Kubernetes cluster test-cluster: lb"),
913+
GroupName: aws.String("test-cluster-lb"),
914+
GroupId: aws.String("sg-lb"),
915+
Tags: []*ec2.Tag{
916+
{
917+
Key: aws.String("Name"),
918+
Value: aws.String("test-cluster-lb"),
919+
}, {
920+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
921+
Value: aws.String("owned"),
922+
}, {
923+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
924+
Value: aws.String("lb"),
925+
}, {
926+
Key: aws.String("kubernetes.io/cluster/test-cluster"),
927+
Value: aws.String("owned"),
928+
},
929+
},
930+
}
931+
932+
securityGroupAPIServerLB := &ec2.SecurityGroup{
933+
Description: aws.String("Kubernetes cluster test-cluster: apiserver-lb"),
934+
GroupName: aws.String("test-cluster-apiserver-lb"),
935+
GroupId: aws.String("sg-apiserver-lb"),
936+
Tags: []*ec2.Tag{
937+
{
938+
Key: aws.String("Name"),
939+
Value: aws.String("test-cluster-apiserver-lb"),
940+
}, {
941+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
942+
Value: aws.String("owned"),
943+
}, {
944+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
945+
Value: aws.String("apiserver-lb"),
946+
},
947+
},
948+
IpPermissions: []*ec2.IpPermission{
949+
{
950+
FromPort: aws.Int64(6443),
951+
IpProtocol: aws.String("tcp"),
952+
IpRanges: []*ec2.IpRange{
953+
{
954+
CidrIp: aws.String("0.0.0.0/0"),
955+
Description: aws.String("Kubernetes API"),
956+
},
957+
},
958+
ToPort: aws.Int64(6443),
959+
},
960+
// Extra rule to be revoked
961+
{
962+
FromPort: aws.Int64(22),
963+
IpProtocol: aws.String("tcp"),
964+
ToPort: aws.Int64(22),
965+
IpRanges: []*ec2.IpRange{
966+
{
967+
CidrIp: aws.String("0.0.0.0/0"),
968+
Description: aws.String("SSH"),
969+
},
970+
},
971+
},
972+
},
973+
}
974+
975+
securityGroupControl := &ec2.SecurityGroup{
976+
Description: aws.String("Kubernetes cluster test-cluster: controlplane"),
977+
GroupName: aws.String("test-cluster-controlplane"),
978+
GroupId: aws.String("sg-control"),
979+
Tags: []*ec2.Tag{
980+
{
981+
Key: aws.String("Name"),
982+
Value: aws.String("test-cluster-controlplane"),
983+
}, {
984+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
985+
Value: aws.String("owned"),
986+
}, {
987+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
988+
Value: aws.String("controlplane"),
989+
},
990+
},
991+
IpPermissions: []*ec2.IpPermission{
992+
{
993+
FromPort: aws.Int64(6443),
994+
IpProtocol: aws.String("tcp"),
995+
ToPort: aws.Int64(6443),
996+
UserIdGroupPairs: []*ec2.UserIdGroupPair{
997+
{
998+
Description: aws.String("Kubernetes API"),
999+
GroupId: aws.String("sg-apiserver-lb"),
1000+
}, {
1001+
Description: aws.String("Kubernetes API"),
1002+
GroupId: aws.String("sg-control"),
1003+
}, {
1004+
Description: aws.String("Kubernetes API"),
1005+
GroupId: aws.String("sg-node"),
1006+
},
1007+
},
1008+
},
1009+
{
1010+
FromPort: aws.Int64(2379),
1011+
IpProtocol: aws.String("tcp"),
1012+
ToPort: aws.Int64(2379),
1013+
UserIdGroupPairs: []*ec2.UserIdGroupPair{
1014+
{
1015+
Description: aws.String("etcd"),
1016+
GroupId: aws.String("sg-control"),
1017+
},
1018+
},
1019+
},
1020+
{
1021+
FromPort: aws.Int64(2380),
1022+
IpProtocol: aws.String("tcp"),
1023+
ToPort: aws.Int64(2380),
1024+
UserIdGroupPairs: []*ec2.UserIdGroupPair{
1025+
{
1026+
Description: aws.String("etcd peer"),
1027+
GroupId: aws.String("sg-control"),
1028+
},
1029+
},
1030+
},
1031+
},
1032+
}
1033+
1034+
securityGroupNode := &ec2.SecurityGroup{
1035+
Description: aws.String("Kubernetes cluster test-cluster: node"),
1036+
GroupName: aws.String("test-cluster-node"),
1037+
GroupId: aws.String("sg-node"),
1038+
Tags: []*ec2.Tag{
1039+
{
1040+
Key: aws.String("Name"),
1041+
Value: aws.String("test-cluster-node"),
1042+
}, {
1043+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
1044+
Value: aws.String("owned"),
1045+
}, {
1046+
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
1047+
Value: aws.String("node"),
1048+
},
1049+
},
1050+
IpPermissions: []*ec2.IpPermission{
1051+
{
1052+
FromPort: aws.Int64(30000),
1053+
ToPort: aws.Int64(32767),
1054+
IpProtocol: aws.String("tcp"),
1055+
IpRanges: []*ec2.IpRange{
1056+
{
1057+
CidrIp: aws.String("0.0.0.0/0"),
1058+
Description: aws.String("Node Port Services"),
1059+
},
1060+
},
1061+
}, {
1062+
FromPort: aws.Int64(10250),
1063+
IpProtocol: aws.String("tcp"),
1064+
ToPort: aws.Int64(10250),
1065+
UserIdGroupPairs: []*ec2.UserIdGroupPair{
1066+
{
1067+
Description: aws.String("Kubelet API"),
1068+
GroupId: aws.String("sg-control"),
1069+
}, {
1070+
Description: aws.String("Kubelet API"),
1071+
GroupId: aws.String("sg-node"),
1072+
},
1073+
},
1074+
},
1075+
},
1076+
}
1077+
1078+
m.DescribeSecurityGroupsWithContext(context.TODO(), gomock.AssignableToTypeOf(&ec2.DescribeSecurityGroupsInput{})).
1079+
Return(&ec2.DescribeSecurityGroupsOutput{
1080+
SecurityGroups: []*ec2.SecurityGroup{
1081+
securityGroupBastion,
1082+
securityGroupLB,
1083+
securityGroupAPIServerLB,
1084+
securityGroupControl,
1085+
securityGroupNode,
1086+
},
1087+
}, nil)
1088+
1089+
m.RevokeSecurityGroupIngressWithContext(context.TODO(), gomock.Eq(&ec2.RevokeSecurityGroupIngressInput{
1090+
GroupId: aws.String("sg-apiserver-lb"),
1091+
IpPermissions: []*ec2.IpPermission{
1092+
{
1093+
FromPort: aws.Int64(22),
1094+
ToPort: aws.Int64(22),
1095+
IpProtocol: aws.String("tcp"),
1096+
IpRanges: []*ec2.IpRange{
1097+
{
1098+
CidrIp: aws.String("0.0.0.0/0"),
1099+
Description: aws.String("SSH"),
1100+
},
1101+
},
1102+
},
1103+
},
1104+
})).Times(1)
1105+
1106+
m.AuthorizeSecurityGroupIngressWithContext(context.TODO(), gomock.AssignableToTypeOf(&ec2.AuthorizeSecurityGroupIngressInput{
1107+
GroupId: aws.String("sg-bastion"),
1108+
IpPermissions: []*ec2.IpPermission{
1109+
{
1110+
ToPort: aws.Int64(22),
1111+
FromPort: aws.Int64(22),
1112+
IpProtocol: aws.String("tcp"),
1113+
},
1114+
},
1115+
})).
1116+
Return(&ec2.AuthorizeSecurityGroupIngressOutput{}, nil).AnyTimes()
1117+
},
1118+
},
8301119
}
8311120

8321121
for _, tc := range testCases {

0 commit comments

Comments
 (0)