Skip to content

Commit

Permalink
✨ Add support for additionalControlPlaneIngressRule on AWSManaged… (#…
Browse files Browse the repository at this point in the history 
…4783)

* feat: add support for additionalControlPlaneIngressRule on AWSManagedControlPlane

* refactor: simplify ingressrules
  • Loading branch information
@fad3t
fad3t authored Oct 25, 2024
1 parent fd0ad38 commit f3c178f
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 24 deletions.
2 changes: 1 addition & 1 deletion pkg/cloud/scope/managedcontrolplane.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ func (s *ManagedControlPlaneScope) Partition() string {

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

// UnstructuredControlPlane returns the unstructured object for the control plane, if any.
Expand Down
7 changes: 3 additions & 4 deletions pkg/cloud/services/securitygroup/securitygroups.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -682,12 +682,11 @@ func (s *Service) getSecurityGroupIngressRules(role infrav1.SecurityGroupRole) (
}
return append(cniRules, rules...), nil
case infrav1.SecurityGroupEKSNodeAdditional:
ingressRules := s.scope.AdditionalControlPlaneIngressRules()
if s.scope.Bastion().Enabled {
return infrav1.IngressRules{
s.defaultSSHIngressRule(s.scope.SecurityGroups()[infrav1.SecurityGroupBastion].ID),
}, nil
ingressRules = append(ingressRules, s.defaultSSHIngressRule(s.scope.SecurityGroups()[infrav1.SecurityGroupBastion].ID))
}
return infrav1.IngressRules{}, nil
return ingressRules, nil
case infrav1.SecurityGroupAPIServerLB:
kubeletRules := s.getIngressRulesToAllowKubeletToAccessTheControlPlaneLB()
customIngressRules, err := s.processIngressRulesSGs(s.getControlPlaneLBIngressRules())
Expand Down
145 changes: 126 additions & 19 deletions pkg/cloud/services/securitygroup/securitygroups_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package securitygroup

import (
"context"
"reflect"
"strings"
"testing"

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

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

testCases := []struct {
name string
networkSpec infrav1.NetworkSpec
networkStatus infrav1.NetworkStatus
expectedAdditionalIngresRule infrav1.IngressRule
wantErr bool
name string
networkSpec infrav1.NetworkSpec
networkStatus infrav1.NetworkStatus
expectedAdditionalIngressRule infrav1.IngressRule
wantErr bool
}{
{
name: "default control plane security group is used",
Expand All @@ -1220,7 +1222,7 @@ func TestAdditionalControlPlaneSecurityGroup(t *testing.T) {
},
},
},
expectedAdditionalIngresRule: infrav1.IngressRule{
expectedAdditionalIngressRule: infrav1.IngressRule{
Description: "test",
Protocol: infrav1.SecurityGroupProtocolTCP,
FromPort: 9345,
Expand Down Expand Up @@ -1251,7 +1253,7 @@ func TestAdditionalControlPlaneSecurityGroup(t *testing.T) {
},
},
},
expectedAdditionalIngresRule: infrav1.IngressRule{
expectedAdditionalIngressRule: infrav1.IngressRule{
Description: "test",
Protocol: infrav1.SecurityGroupProtocolTCP,
FromPort: 9345,
Expand Down Expand Up @@ -1282,7 +1284,7 @@ func TestAdditionalControlPlaneSecurityGroup(t *testing.T) {
},
},
},
expectedAdditionalIngresRule: infrav1.IngressRule{
expectedAdditionalIngressRule: infrav1.IngressRule{
Description: "test",
Protocol: infrav1.SecurityGroupProtocolTCP,
FromPort: 9345,
Expand Down Expand Up @@ -1314,7 +1316,7 @@ func TestAdditionalControlPlaneSecurityGroup(t *testing.T) {
},
},
},
expectedAdditionalIngresRule: infrav1.IngressRule{
expectedAdditionalIngressRule: infrav1.IngressRule{
Description: "test",
Protocol: infrav1.SecurityGroupProtocolTCP,
FromPort: 9345,
Expand Down Expand Up @@ -1345,7 +1347,7 @@ func TestAdditionalControlPlaneSecurityGroup(t *testing.T) {
},
},
},
expectedAdditionalIngresRule: infrav1.IngressRule{
expectedAdditionalIngressRule: infrav1.IngressRule{
Description: "test",
Protocol: infrav1.SecurityGroupProtocolTCP,
FromPort: 9345,
Expand Down Expand Up @@ -1376,7 +1378,7 @@ func TestAdditionalControlPlaneSecurityGroup(t *testing.T) {
},
NatGatewaysIPs: []string{"test-ip"},
},
expectedAdditionalIngresRule: infrav1.IngressRule{
expectedAdditionalIngressRule: infrav1.IngressRule{
Description: "test",
Protocol: infrav1.SecurityGroupProtocolTCP,
CidrBlocks: []string{"test-ip/32"},
Expand Down Expand Up @@ -1437,20 +1439,125 @@ func TestAdditionalControlPlaneSecurityGroup(t *testing.T) {
}
found = true

if r.Protocol != tc.expectedAdditionalIngresRule.Protocol {
t.Fatalf("Expected protocol %s, got %s", tc.expectedAdditionalIngresRule.Protocol, r.Protocol)
if r.Protocol != tc.expectedAdditionalIngressRule.Protocol {
t.Fatalf("Expected protocol %s, got %s", tc.expectedAdditionalIngressRule.Protocol, r.Protocol)
}

if r.FromPort != tc.expectedAdditionalIngresRule.FromPort {
t.Fatalf("Expected from port %d, got %d", tc.expectedAdditionalIngresRule.FromPort, r.FromPort)
if r.FromPort != tc.expectedAdditionalIngressRule.FromPort {
t.Fatalf("Expected from port %d, got %d", tc.expectedAdditionalIngressRule.FromPort, r.FromPort)
}

if r.ToPort != tc.expectedAdditionalIngresRule.ToPort {
t.Fatalf("Expected to port %d, got %d", tc.expectedAdditionalIngresRule.ToPort, r.ToPort)
if r.ToPort != tc.expectedAdditionalIngressRule.ToPort {
t.Fatalf("Expected to port %d, got %d", tc.expectedAdditionalIngressRule.ToPort, r.ToPort)
}

if !sets.New(tc.expectedAdditionalIngresRule.SourceSecurityGroupIDs...).Equal(sets.New(r.SourceSecurityGroupIDs...)) {
t.Fatalf("Expected source security group IDs %v, got %v", tc.expectedAdditionalIngresRule.SourceSecurityGroupIDs, r.SourceSecurityGroupIDs)
if !sets.New[string](tc.expectedAdditionalIngressRule.SourceSecurityGroupIDs...).Equal(sets.New[string](tc.expectedAdditionalIngressRule.SourceSecurityGroupIDs...)) {
t.Fatalf("Expected source security group IDs %v, got %v", tc.expectedAdditionalIngressRule.SourceSecurityGroupIDs, r.SourceSecurityGroupIDs)
}
}

if !found {
t.Fatal("Additional ingress rule was not found")
}
})
}
}

func TestAdditionalManagedControlPlaneSecurityGroup(t *testing.T) {
scheme := runtime.NewScheme()
_ = ekscontrolplanev1.AddToScheme(scheme)

testCases := []struct {
name string
networkSpec infrav1.NetworkSpec
expectedAdditionalIngressRule infrav1.IngressRule
}{
{
name: "default control plane security group is used",
networkSpec: infrav1.NetworkSpec{
AdditionalControlPlaneIngressRules: []infrav1.IngressRule{
{
Description: "test",
Protocol: infrav1.SecurityGroupProtocolTCP,
FromPort: 9345,
ToPort: 9345,
},
},
},
expectedAdditionalIngressRule: infrav1.IngressRule{
Description: "test",
Protocol: infrav1.SecurityGroupProtocolTCP,
FromPort: 9345,
ToPort: 9345,
SourceSecurityGroupIDs: []string{"cp-sg-id"},
},
},
{
name: "don't set source security groups if cidr blocks are set",
networkSpec: infrav1.NetworkSpec{
AdditionalControlPlaneIngressRules: []infrav1.IngressRule{
{
Description: "test",
Protocol: infrav1.SecurityGroupProtocolTCP,
FromPort: 9345,
ToPort: 9345,
CidrBlocks: []string{"test-cidr-block"},
},
},
},
expectedAdditionalIngressRule: infrav1.IngressRule{
Description: "test",
Protocol: infrav1.SecurityGroupProtocolTCP,
FromPort: 9345,
ToPort: 9345,
CidrBlocks: []string{"test-cidr-block"},
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
cs, err := scope.NewManagedControlPlaneScope(scope.ManagedControlPlaneScopeParams{
Client: fake.NewClientBuilder().WithScheme(scheme).Build(),
Cluster: &clusterv1.Cluster{
ObjectMeta: metav1.ObjectMeta{Name: "test-cluster"},
},
ControlPlane: &ekscontrolplanev1.AWSManagedControlPlane{
Spec: ekscontrolplanev1.AWSManagedControlPlaneSpec{
NetworkSpec: tc.networkSpec,
},
Status: ekscontrolplanev1.AWSManagedControlPlaneStatus{
Network: infrav1.NetworkStatus{
SecurityGroups: map[infrav1.SecurityGroupRole]infrav1.SecurityGroup{
infrav1.SecurityGroupControlPlane: {
ID: "cp-sg-id",
},
infrav1.SecurityGroupNode: {
ID: "node-sg-id",
},
},
},
},
},
})
if err != nil {
t.Fatalf("Failed to create test context: %v", err)
}

s := NewService(cs, testSecurityGroupRoles)
rules, err := s.getSecurityGroupIngressRules(infrav1.SecurityGroupControlPlane)
if err != nil {
t.Fatalf("Failed to lookup controlplane security group ingress rules: %v", err)
}

found := false
for _, r := range rules {
if r.Description == "test" {
found = true

if !reflect.DeepEqual(r, tc.expectedAdditionalIngressRule) {
t.Fatalf("Expected ingress rule %#v, got %#v", tc.expectedAdditionalIngressRule, r)
}
}
}

Expand Down

0 comments on commit f3c178f

Please sign in to comment.