Skip to content

Commit f3c178f

Browse files
fad3tfad3t
authored
✨ Add support for additionalControlPlaneIngressRule on AWSManaged… (#4783)
* feat: add support for additionalControlPlaneIngressRule on AWSManagedControlPlane * refactor: simplify ingressrules
1 parent fd0ad38 commit f3c178f

File tree

3 files changed

+130
-24
lines changed

3 files changed

+130
-24
lines changed

‎pkg/cloud/scope/managedcontrolplane.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ func (s *ManagedControlPlaneScope) Partition() string {
472472

473473
// AdditionalControlPlaneIngressRules returns the additional ingress rules for the control plane security group.
474474
func (s *ManagedControlPlaneScope) AdditionalControlPlaneIngressRules() []infrav1.IngressRule {
475-
return nil
475+
return s.ControlPlane.Spec.NetworkSpec.DeepCopy().AdditionalControlPlaneIngressRules
476476
}
477477

478478
// UnstructuredControlPlane returns the unstructured object for the control plane, if any.

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -682,12 +682,11 @@ func (s *Service) getSecurityGroupIngressRules(role infrav1.SecurityGroupRole) (
682682
}
683683
return append(cniRules, rules...), nil
684684
case infrav1.SecurityGroupEKSNodeAdditional:
685+
ingressRules := s.scope.AdditionalControlPlaneIngressRules()
685686
if s.scope.Bastion().Enabled {
686-
return infrav1.IngressRules{
687-
s.defaultSSHIngressRule(s.scope.SecurityGroups()[infrav1.SecurityGroupBastion].ID),
688-
}, nil
687+
ingressRules = append(ingressRules, s.defaultSSHIngressRule(s.scope.SecurityGroups()[infrav1.SecurityGroupBastion].ID))
689688
}
690-
return infrav1.IngressRules{}, nil
689+
return ingressRules, nil
691690
case infrav1.SecurityGroupAPIServerLB:
692691
kubeletRules := s.getIngressRulesToAllowKubeletToAccessTheControlPlaneLB()
693692
customIngressRules, err := s.processIngressRulesSGs(s.getControlPlaneLBIngressRules())

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

Lines changed: 126 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package securitygroup
1818

1919
import (
2020
"context"
21+
"reflect"
2122
"strings"
2223
"testing"
2324

@@ -34,6 +35,7 @@ import (
3435
"sigs.k8s.io/controller-runtime/pkg/client/fake"
3536

3637
infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
38+
ekscontrolplanev1 "sigs.k8s.io/cluster-api-provider-aws/v2/controlplane/eks/api/v1beta2"
3739
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/awserrors"
3840
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/filter"
3941
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope"
@@ -1192,11 +1194,11 @@ func TestAdditionalControlPlaneSecurityGroup(t *testing.T) {
11921194
_ = infrav1.AddToScheme(scheme)
11931195

11941196
testCases := []struct {
1195-
name string
1196-
networkSpec infrav1.NetworkSpec
1197-
networkStatus infrav1.NetworkStatus
1198-
expectedAdditionalIngresRule infrav1.IngressRule
1199-
wantErr bool
1197+
name string
1198+
networkSpec infrav1.NetworkSpec
1199+
networkStatus infrav1.NetworkStatus
1200+
expectedAdditionalIngressRule infrav1.IngressRule
1201+
wantErr bool
12001202
}{
12011203
{
12021204
name: "default control plane security group is used",
@@ -1220,7 +1222,7 @@ func TestAdditionalControlPlaneSecurityGroup(t *testing.T) {
12201222
},
12211223
},
12221224
},
1223-
expectedAdditionalIngresRule: infrav1.IngressRule{
1225+
expectedAdditionalIngressRule: infrav1.IngressRule{
12241226
Description: "test",
12251227
Protocol: infrav1.SecurityGroupProtocolTCP,
12261228
FromPort: 9345,
@@ -1251,7 +1253,7 @@ func TestAdditionalControlPlaneSecurityGroup(t *testing.T) {
12511253
},
12521254
},
12531255
},
1254-
expectedAdditionalIngresRule: infrav1.IngressRule{
1256+
expectedAdditionalIngressRule: infrav1.IngressRule{
12551257
Description: "test",
12561258
Protocol: infrav1.SecurityGroupProtocolTCP,
12571259
FromPort: 9345,
@@ -1282,7 +1284,7 @@ func TestAdditionalControlPlaneSecurityGroup(t *testing.T) {
12821284
},
12831285
},
12841286
},
1285-
expectedAdditionalIngresRule: infrav1.IngressRule{
1287+
expectedAdditionalIngressRule: infrav1.IngressRule{
12861288
Description: "test",
12871289
Protocol: infrav1.SecurityGroupProtocolTCP,
12881290
FromPort: 9345,
@@ -1314,7 +1316,7 @@ func TestAdditionalControlPlaneSecurityGroup(t *testing.T) {
13141316
},
13151317
},
13161318
},
1317-
expectedAdditionalIngresRule: infrav1.IngressRule{
1319+
expectedAdditionalIngressRule: infrav1.IngressRule{
13181320
Description: "test",
13191321
Protocol: infrav1.SecurityGroupProtocolTCP,
13201322
FromPort: 9345,
@@ -1345,7 +1347,7 @@ func TestAdditionalControlPlaneSecurityGroup(t *testing.T) {
13451347
},
13461348
},
13471349
},
1348-
expectedAdditionalIngresRule: infrav1.IngressRule{
1350+
expectedAdditionalIngressRule: infrav1.IngressRule{
13491351
Description: "test",
13501352
Protocol: infrav1.SecurityGroupProtocolTCP,
13511353
FromPort: 9345,
@@ -1376,7 +1378,7 @@ func TestAdditionalControlPlaneSecurityGroup(t *testing.T) {
13761378
},
13771379
NatGatewaysIPs: []string{"test-ip"},
13781380
},
1379-
expectedAdditionalIngresRule: infrav1.IngressRule{
1381+
expectedAdditionalIngressRule: infrav1.IngressRule{
13801382
Description: "test",
13811383
Protocol: infrav1.SecurityGroupProtocolTCP,
13821384
CidrBlocks: []string{"test-ip/32"},
@@ -1437,20 +1439,125 @@ func TestAdditionalControlPlaneSecurityGroup(t *testing.T) {
14371439
}
14381440
found = true
14391441

1440-
if r.Protocol != tc.expectedAdditionalIngresRule.Protocol {
1441-
t.Fatalf("Expected protocol %s, got %s", tc.expectedAdditionalIngresRule.Protocol, r.Protocol)
1442+
if r.Protocol != tc.expectedAdditionalIngressRule.Protocol {
1443+
t.Fatalf("Expected protocol %s, got %s", tc.expectedAdditionalIngressRule.Protocol, r.Protocol)
14421444
}
14431445

1444-
if r.FromPort != tc.expectedAdditionalIngresRule.FromPort {
1445-
t.Fatalf("Expected from port %d, got %d", tc.expectedAdditionalIngresRule.FromPort, r.FromPort)
1446+
if r.FromPort != tc.expectedAdditionalIngressRule.FromPort {
1447+
t.Fatalf("Expected from port %d, got %d", tc.expectedAdditionalIngressRule.FromPort, r.FromPort)
14461448
}
14471449

1448-
if r.ToPort != tc.expectedAdditionalIngresRule.ToPort {
1449-
t.Fatalf("Expected to port %d, got %d", tc.expectedAdditionalIngresRule.ToPort, r.ToPort)
1450+
if r.ToPort != tc.expectedAdditionalIngressRule.ToPort {
1451+
t.Fatalf("Expected to port %d, got %d", tc.expectedAdditionalIngressRule.ToPort, r.ToPort)
14501452
}
14511453

1452-
if !sets.New(tc.expectedAdditionalIngresRule.SourceSecurityGroupIDs...).Equal(sets.New(r.SourceSecurityGroupIDs...)) {
1453-
t.Fatalf("Expected source security group IDs %v, got %v", tc.expectedAdditionalIngresRule.SourceSecurityGroupIDs, r.SourceSecurityGroupIDs)
1454+
if !sets.New[string](tc.expectedAdditionalIngressRule.SourceSecurityGroupIDs...).Equal(sets.New[string](tc.expectedAdditionalIngressRule.SourceSecurityGroupIDs...)) {
1455+
t.Fatalf("Expected source security group IDs %v, got %v", tc.expectedAdditionalIngressRule.SourceSecurityGroupIDs, r.SourceSecurityGroupIDs)
1456+
}
1457+
}
1458+
1459+
if !found {
1460+
t.Fatal("Additional ingress rule was not found")
1461+
}
1462+
})
1463+
}
1464+
}
1465+
1466+
func TestAdditionalManagedControlPlaneSecurityGroup(t *testing.T) {
1467+
scheme := runtime.NewScheme()
1468+
_ = ekscontrolplanev1.AddToScheme(scheme)
1469+
1470+
testCases := []struct {
1471+
name string
1472+
networkSpec infrav1.NetworkSpec
1473+
expectedAdditionalIngressRule infrav1.IngressRule
1474+
}{
1475+
{
1476+
name: "default control plane security group is used",
1477+
networkSpec: infrav1.NetworkSpec{
1478+
AdditionalControlPlaneIngressRules: []infrav1.IngressRule{
1479+
{
1480+
Description: "test",
1481+
Protocol: infrav1.SecurityGroupProtocolTCP,
1482+
FromPort: 9345,
1483+
ToPort: 9345,
1484+
},
1485+
},
1486+
},
1487+
expectedAdditionalIngressRule: infrav1.IngressRule{
1488+
Description: "test",
1489+
Protocol: infrav1.SecurityGroupProtocolTCP,
1490+
FromPort: 9345,
1491+
ToPort: 9345,
1492+
SourceSecurityGroupIDs: []string{"cp-sg-id"},
1493+
},
1494+
},
1495+
{
1496+
name: "don't set source security groups if cidr blocks are set",
1497+
networkSpec: infrav1.NetworkSpec{
1498+
AdditionalControlPlaneIngressRules: []infrav1.IngressRule{
1499+
{
1500+
Description: "test",
1501+
Protocol: infrav1.SecurityGroupProtocolTCP,
1502+
FromPort: 9345,
1503+
ToPort: 9345,
1504+
CidrBlocks: []string{"test-cidr-block"},
1505+
},
1506+
},
1507+
},
1508+
expectedAdditionalIngressRule: infrav1.IngressRule{
1509+
Description: "test",
1510+
Protocol: infrav1.SecurityGroupProtocolTCP,
1511+
FromPort: 9345,
1512+
ToPort: 9345,
1513+
CidrBlocks: []string{"test-cidr-block"},
1514+
},
1515+
},
1516+
}
1517+
1518+
for _, tc := range testCases {
1519+
t.Run(tc.name, func(t *testing.T) {
1520+
cs, err := scope.NewManagedControlPlaneScope(scope.ManagedControlPlaneScopeParams{
1521+
Client: fake.NewClientBuilder().WithScheme(scheme).Build(),
1522+
Cluster: &clusterv1.Cluster{
1523+
ObjectMeta: metav1.ObjectMeta{Name: "test-cluster"},
1524+
},
1525+
ControlPlane: &ekscontrolplanev1.AWSManagedControlPlane{
1526+
Spec: ekscontrolplanev1.AWSManagedControlPlaneSpec{
1527+
NetworkSpec: tc.networkSpec,
1528+
},
1529+
Status: ekscontrolplanev1.AWSManagedControlPlaneStatus{
1530+
Network: infrav1.NetworkStatus{
1531+
SecurityGroups: map[infrav1.SecurityGroupRole]infrav1.SecurityGroup{
1532+
infrav1.SecurityGroupControlPlane: {
1533+
ID: "cp-sg-id",
1534+
},
1535+
infrav1.SecurityGroupNode: {
1536+
ID: "node-sg-id",
1537+
},
1538+
},
1539+
},
1540+
},
1541+
},
1542+
})
1543+
if err != nil {
1544+
t.Fatalf("Failed to create test context: %v", err)
1545+
}
1546+
1547+
s := NewService(cs, testSecurityGroupRoles)
1548+
rules, err := s.getSecurityGroupIngressRules(infrav1.SecurityGroupControlPlane)
1549+
if err != nil {
1550+
t.Fatalf("Failed to lookup controlplane security group ingress rules: %v", err)
1551+
}
1552+
1553+
found := false
1554+
for _, r := range rules {
1555+
if r.Description == "test" {
1556+
found = true
1557+
1558+
if !reflect.DeepEqual(r, tc.expectedAdditionalIngressRule) {
1559+
t.Fatalf("Expected ingress rule %#v, got %#v", tc.expectedAdditionalIngressRule, r)
1560+
}
14541561
}
14551562
}
14561563

0 commit comments

Comments
 (0)