Skip to content

Commit f0b6914

Browse files
committed
Added preserve client ip setting and used the correct security group
1 parent 7e87185 commit f0b6914

17 files changed

+1882
-120
lines changed

api/v1beta1/awscluster_conversion.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ func restoreControlPlaneLoadBalancer(restored, dst *infrav1.AWSLoadBalancerSpec)
6464
dst.HealthCheckProtocol = restored.HealthCheckProtocol
6565
dst.LoadBalancerType = restored.LoadBalancerType
6666
dst.DisableHostsRewrite = restored.DisableHostsRewrite
67+
dst.PreserveClientIP = restored.PreserveClientIP
6768
}
6869

6970
// ConvertFrom converts the v1beta1 AWSCluster receiver to a v1beta1 AWSCluster.

api/v1beta1/zz_generated.conversion.go

Lines changed: 4 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/v1beta2/awscluster_types.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ type AWSLoadBalancerSpec struct {
193193
// +optional
194194
Subnets []string `json:"subnets,omitempty"`
195195

196-
// HealthCheckProtocol sets the protocol type for classic ELB health check target
196+
// HealthCheckProtocol sets the protocol type for ELB health check target
197197
// default value is ElbProtocolSSL
198198
// +optional
199199
HealthCheckProtocol *ElbProtocol `json:"healthCheckProtocol,omitempty"`
@@ -210,6 +210,10 @@ type AWSLoadBalancerSpec struct {
210210
// DisableHostsRewrite disabled the hair pinning issue solution that adds the NLB's address as 127.0.0.1 to the hosts
211211
// file of each instance. This is by default, false.
212212
DisableHostsRewrite bool `json:"disableHostsRewrite,omitempty"`
213+
214+
// PreserveClientIP lets the user control if preservation of client ips must be retained or not.
215+
// If this is enabled 6443 will be opened to 0.0.0.0/0.
216+
PreserveClientIP bool `json:"preserveClientIP,omitempty"`
213217
}
214218

215219
// AWSClusterStatus defines the observed state of AWSCluster.

api/v1beta2/network_types.go

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,15 +75,30 @@ var (
7575
)
7676

7777
// TargetGroupHealthCheck defines health check settings for the target group.
78-
// TODO: Create default values for these.
7978
type TargetGroupHealthCheck struct {
80-
HealthCheckProtocol *string `json:"healthCheckProtocol"`
81-
HealthCheckPath *string `json:"healthCheckPath"`
82-
HealthCheckIntervalSeconds *int64 `json:"healthCheckIntervalSeconds"`
83-
HealthCheckTimeoutSeconds *int64 `json:"healthCheckTimeoutSeconds"`
84-
HealthyThresholdCount *int64 `json:"healthyThresholdCount"`
79+
HealthCheckProtocol *string `json:"healthCheckProtocol,omitempty"`
80+
HealthCheckPath *string `json:"healthCheckPath,omitempty"`
81+
HealthCheckPort *string `json:"healthCheckPort,omitempty"`
82+
HealthCheckIntervalSeconds *int64 `json:"healthCheckIntervalSeconds,omitempty"`
83+
HealthCheckTimeoutSeconds *int64 `json:"healthCheckTimeoutSeconds,omitempty"`
84+
HealthyThresholdCount *int64 `json:"healthyThresholdCount,omitempty"`
8585
}
8686

87+
// TargetGroupAttribute defines attribute key values for V2 Load Balancer Attributes.
88+
type TargetGroupAttribute string
89+
90+
var (
91+
TargetGroupAttributeEnablePreserveClientIP = "preserve_client_ip.enabled"
92+
)
93+
94+
// LoadBalancerAttribute defines a set of attributes for a V2 load balancer
95+
type LoadBalancerAttribute string
96+
97+
var (
98+
LoadBalancerAttributeEnableLoadBalancingCrossZone = "load_balancing.cross_zone.enabled"
99+
LoadBalancerAttributeIdleTimeTimeoutSeconds = "idle_timeout.timeout_seconds"
100+
)
101+
87102
// TargetGroupSpec specifies target group settings for a given listener.
88103
// This is created first, and the ARN is then passed to the listener.
89104
type TargetGroupSpec struct {

api/v1beta2/zz_generated.deepcopy.go

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1262,6 +1262,8 @@ spec:
12621262
type: integer
12631263
healthCheckPath:
12641264
type: string
1265+
healthCheckPort:
1266+
type: string
12651267
healthCheckProtocol:
12661268
type: string
12671269
healthCheckTimeoutSeconds:
@@ -1270,12 +1272,6 @@ spec:
12701272
healthyThresholdCount:
12711273
format: int64
12721274
type: integer
1273-
required:
1274-
- healthCheckIntervalSeconds
1275-
- healthCheckPath
1276-
- healthCheckProtocol
1277-
- healthCheckTimeoutSeconds
1278-
- healthyThresholdCount
12791275
type: object
12801276
vpcId:
12811277
type: string
@@ -2630,6 +2626,8 @@ spec:
26302626
type: integer
26312627
healthCheckPath:
26322628
type: string
2629+
healthCheckPort:
2630+
type: string
26332631
healthCheckProtocol:
26342632
type: string
26352633
healthCheckTimeoutSeconds:
@@ -2638,12 +2636,6 @@ spec:
26382636
healthyThresholdCount:
26392637
format: int64
26402638
type: integer
2641-
required:
2642-
- healthCheckIntervalSeconds
2643-
- healthCheckPath
2644-
- healthCheckProtocol
2645-
- healthCheckTimeoutSeconds
2646-
- healthyThresholdCount
26472639
type: object
26482640
vpcId:
26492641
type: string

config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -990,8 +990,8 @@ spec:
990990
file of each instance. This is by default, false.
991991
type: boolean
992992
healthCheckProtocol:
993-
description: HealthCheckProtocol sets the protocol type for classic
994-
ELB health check target default value is ElbProtocolSSL
993+
description: HealthCheckProtocol sets the protocol type for ELB
994+
health check target default value is ElbProtocolSSL
995995
type: string
996996
loadBalancerType:
997997
description: LoadBalancerType sets the type for a load balancer.
@@ -1011,6 +1011,11 @@ spec:
10111011
maxLength: 32
10121012
pattern: ^[A-Za-z0-9]([A-Za-z0-9]{0,31}|[-A-Za-z0-9]{0,30}[A-Za-z0-9])$
10131013
type: string
1014+
preserveClientIP:
1015+
description: PreserveClientIP lets the user control if preservation
1016+
of client ips must be retained or not. If this is enabled 6443
1017+
will be opened to 0.0.0.0/0.
1018+
type: boolean
10141019
scheme:
10151020
default: internet-facing
10161021
description: Scheme sets the scheme of the load balancer (defaults
@@ -1709,6 +1714,8 @@ spec:
17091714
type: integer
17101715
healthCheckPath:
17111716
type: string
1717+
healthCheckPort:
1718+
type: string
17121719
healthCheckProtocol:
17131720
type: string
17141721
healthCheckTimeoutSeconds:
@@ -1717,12 +1724,6 @@ spec:
17171724
healthyThresholdCount:
17181725
format: int64
17191726
type: integer
1720-
required:
1721-
- healthCheckIntervalSeconds
1722-
- healthCheckPath
1723-
- healthCheckProtocol
1724-
- healthCheckTimeoutSeconds
1725-
- healthyThresholdCount
17261727
type: object
17271728
vpcId:
17281729
type: string

config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -586,8 +586,7 @@ spec:
586586
type: boolean
587587
healthCheckProtocol:
588588
description: HealthCheckProtocol sets the protocol type
589-
for classic ELB health check target default value is
590-
ElbProtocolSSL
589+
for ELB health check target default value is ElbProtocolSSL
591590
type: string
592591
loadBalancerType:
593592
description: LoadBalancerType sets the type for a load
@@ -608,6 +607,11 @@ spec:
608607
maxLength: 32
609608
pattern: ^[A-Za-z0-9]([A-Za-z0-9]{0,31}|[-A-Za-z0-9]{0,30}[A-Za-z0-9])$
610609
type: string
610+
preserveClientIP:
611+
description: PreserveClientIP lets the user control if
612+
preservation of client ips must be retained or not.
613+
If this is enabled 6443 will be opened to 0.0.0.0/0.
614+
type: boolean
611615
scheme:
612616
default: internet-facing
613617
description: Scheme sets the scheme of the load balancer

controllers/awscluster_controller_test.go

Lines changed: 109 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func TestAWSClusterReconciler_IntegrationTests(t *testing.T) {
7373
elbMock := mocks.NewMockELBAPI(mockCtrl)
7474
expect := func(m *mocks.MockEC2APIMockRecorder, e *mocks.MockELBAPIMockRecorder) {
7575
mockedCreateVPCCalls(m)
76-
mockedCreateSGCalls(m)
76+
mockedCreateSGCalls(false, m)
7777
mockedCreateLBCalls(t, e)
7878
mockedDescribeInstanceCall(m)
7979
}
@@ -162,6 +162,106 @@ func TestAWSClusterReconciler_IntegrationTests(t *testing.T) {
162162
{conditionType: infrav1.SubnetsReadyCondition, status: corev1.ConditionTrue, severity: "", reason: ""},
163163
})
164164
})
165+
t.Run("Should successfully reconcile AWSCluster creation with unmanaged VPC and a network type load balancer", func(t *testing.T) {
166+
g := NewWithT(t)
167+
mockCtrl = gomock.NewController(t)
168+
ec2Mock := mocks.NewMockEC2API(mockCtrl)
169+
elbv2Mock := mocks.NewMockELBV2API(mockCtrl)
170+
171+
setup(t)
172+
controllerIdentity := createControllerIdentity(g)
173+
ns, err := testEnv.CreateNamespace(ctx, fmt.Sprintf("integ-test-%s", util.RandomString(5)))
174+
g.Expect(err).To(BeNil())
175+
176+
awsCluster := getAWSCluster("test", ns.Name)
177+
awsCluster.Spec.ControlPlaneLoadBalancer = &infrav1.AWSLoadBalancerSpec{
178+
LoadBalancerType: infrav1.LoadBalancerTypeNLB,
179+
// Overwrite here because otherwise it's longer than 32, and we'll get a hashed name.
180+
Name: aws.String("test-cluster-apiserver"),
181+
}
182+
183+
expect := func(m *mocks.MockEC2APIMockRecorder, e *mocks.MockELBV2APIMockRecorder) {
184+
mockedCreateVPCCalls(m)
185+
mockedCreateSGCalls(true, m)
186+
mockedCreateLBV2Calls(t, e)
187+
mockedDescribeInstanceCall(m)
188+
}
189+
expect(ec2Mock.EXPECT(), elbv2Mock.EXPECT())
190+
191+
g.Expect(testEnv.Create(ctx, &awsCluster)).To(Succeed())
192+
g.Eventually(func() bool {
193+
cluster := &infrav1.AWSCluster{}
194+
key := client.ObjectKey{
195+
Name: awsCluster.Name,
196+
Namespace: ns.Name,
197+
}
198+
err := testEnv.Get(ctx, key, cluster)
199+
return err == nil
200+
}, 10*time.Second).Should(Equal(true))
201+
202+
defer teardown()
203+
defer t.Cleanup(func() {
204+
g.Expect(testEnv.Cleanup(ctx, &awsCluster, controllerIdentity, ns)).To(Succeed())
205+
})
206+
207+
cs, err := getClusterScope(awsCluster)
208+
cs.Cluster.Namespace = ns.Name
209+
g.Expect(err).To(BeNil())
210+
networkSvc := network.NewService(cs)
211+
networkSvc.EC2Client = ec2Mock
212+
reconciler.networkServiceFactory = func(clusterScope scope.ClusterScope) services.NetworkInterface {
213+
return networkSvc
214+
}
215+
216+
ec2Svc := ec2Service.NewService(cs)
217+
ec2Svc.EC2Client = ec2Mock
218+
reconciler.ec2ServiceFactory = func(scope scope.EC2Scope) services.EC2Interface {
219+
return ec2Svc
220+
}
221+
testSecurityGroupRoles := []infrav1.SecurityGroupRole{
222+
infrav1.SecurityGroupBastion,
223+
infrav1.SecurityGroupAPIServerLB,
224+
infrav1.SecurityGroupLB,
225+
infrav1.SecurityGroupControlPlane,
226+
infrav1.SecurityGroupNode,
227+
}
228+
sgSvc := securitygroup.NewService(cs, testSecurityGroupRoles)
229+
sgSvc.EC2Client = ec2Mock
230+
231+
reconciler.securityGroupFactory = func(clusterScope scope.ClusterScope) services.SecurityGroupInterface {
232+
return sgSvc
233+
}
234+
elbSvc := elbService.NewService(cs)
235+
elbSvc.EC2Client = ec2Mock
236+
elbSvc.ELBV2Client = elbv2Mock
237+
238+
reconciler.elbServiceFactory = func(elbScope scope.ELBScope) services.ELBInterface {
239+
return elbSvc
240+
}
241+
cs.SetSubnets([]infrav1.SubnetSpec{
242+
{
243+
ID: "subnet-2",
244+
AvailabilityZone: "us-east-1c",
245+
IsPublic: true,
246+
CidrBlock: "10.0.11.0/24",
247+
},
248+
{
249+
ID: "subnet-1",
250+
AvailabilityZone: "us-east-1a",
251+
CidrBlock: "10.0.10.0/24",
252+
IsPublic: false,
253+
},
254+
})
255+
_, err = reconciler.reconcileNormal(cs)
256+
g.Expect(err).To(BeNil())
257+
g.Expect(cs.VPC().ID).To(Equal("vpc-exists"))
258+
expectAWSClusterConditions(g, cs.AWSCluster, []conditionAssertion{
259+
{conditionType: infrav1.ClusterSecurityGroupsReadyCondition, status: corev1.ConditionTrue, severity: "", reason: ""},
260+
{conditionType: infrav1.BastionHostReadyCondition, status: corev1.ConditionTrue, severity: "", reason: ""},
261+
{conditionType: infrav1.VpcReadyCondition, status: corev1.ConditionTrue, severity: "", reason: ""},
262+
{conditionType: infrav1.SubnetsReadyCondition, status: corev1.ConditionTrue, severity: "", reason: ""},
263+
})
264+
})
165265
t.Run("Should fail on AWSCluster reconciliation if VPC limit exceeded", func(t *testing.T) {
166266
// Assuming the max VPC limit is 2 and when two VPCs are created, the creation of 3rd VPC throws mocked error from EC2 API
167267
g := NewWithT(t)
@@ -753,7 +853,7 @@ func mockedDeleteVPCCalls(m *mocks.MockEC2APIMockRecorder) {
753853
}))
754854
}
755855

756-
func mockedCreateSGCalls(m *mocks.MockEC2APIMockRecorder) {
856+
func mockedCreateSGCalls(recordLBV2 bool, m *mocks.MockEC2APIMockRecorder) {
757857
m.DescribeSecurityGroups(gomock.Eq(&ec2.DescribeSecurityGroupsInput{
758858
Filters: []*ec2.Filter{
759859
{
@@ -913,4 +1013,11 @@ func mockedCreateSGCalls(m *mocks.MockEC2APIMockRecorder) {
9131013
})).
9141014
Return(&ec2.AuthorizeSecurityGroupIngressOutput{}, nil).
9151015
After(securityGroupNode).Times(2)
1016+
if recordLBV2 {
1017+
m.AuthorizeSecurityGroupIngress(gomock.AssignableToTypeOf(&ec2.AuthorizeSecurityGroupIngressInput{
1018+
GroupId: aws.String("sg-lb"),
1019+
})).
1020+
Return(&ec2.AuthorizeSecurityGroupIngressOutput{}, nil).
1021+
After(securityGroupNode).Times(1)
1022+
}
9161023
}

0 commit comments

Comments
 (0)