Skip to content

Commit 68dbdef

Browse files
richardcaserichardcase
committed
wip: classic elb fix for TLS issues
Signed-off-by: Richard Case <[email protected]>
1 parent ac3cc61 commit 68dbdef

File tree

13 files changed

+510
-32
lines changed

13 files changed

+510
-32
lines changed

‎api/v1beta2/awscluster_webhook.go

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@ import (
3333
"sigs.k8s.io/cluster-api/util/annotations"
3434
)
3535

36+
const (
37+
warningClassicELB = "%s load balancer is using a classic elb which is deprecated, please consider using nlb instead"
38+
warningHealthCheckProtocolNotSet = "healthcheck protocol is not set, the default value has changed from SSL to TCP"
39+
)
40+
3641
// log is for logging in this package.
3742
var _ = ctrl.Log.WithName("awscluster-resource")
3843

@@ -53,15 +58,23 @@ var (
5358
// ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
5459
func (r *AWSCluster) ValidateCreate() (admission.Warnings, error) {
5560
var allErrs field.ErrorList
61+
var allWarnings admission.Warnings
5662

5763
allErrs = append(allErrs, r.Spec.Bastion.Validate()...)
5864
allErrs = append(allErrs, r.validateSSHKeyName()...)
5965
allErrs = append(allErrs, r.Spec.AdditionalTags.Validate()...)
6066
allErrs = append(allErrs, r.Spec.S3Bucket.Validate()...)
6167
allErrs = append(allErrs, r.validateNetwork()...)
62-
allErrs = append(allErrs, r.validateControlPlaneLBs()...)
6368

64-
return nil, aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs)
69+
warnings, errs := r.validateControlPlaneLBs()
70+
if len(errs) > 0 {
71+
allErrs = append(allErrs, errs...)
72+
}
73+
if len(warnings) > 0 {
74+
allWarnings = append(allWarnings, warnings...)
75+
}
76+
77+
return allWarnings, aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs)
6578
}
6679

6780
// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
@@ -72,6 +85,7 @@ func (r *AWSCluster) ValidateDelete() (admission.Warnings, error) {
7285
// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
7386
func (r *AWSCluster) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
7487
var allErrs field.ErrorList
88+
var allWarnings admission.Warnings
7589

7690
allErrs = append(allErrs, r.validateGCTasksAnnotation()...)
7791

@@ -139,7 +153,23 @@ func (r *AWSCluster) ValidateUpdate(old runtime.Object) (admission.Warnings, err
139153
allErrs = append(allErrs, r.Spec.AdditionalTags.Validate()...)
140154
allErrs = append(allErrs, r.Spec.S3Bucket.Validate()...)
141155

142-
return nil, aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs)
156+
if r.Spec.ControlPlaneLoadBalancer != nil {
157+
if r.Spec.ControlPlaneLoadBalancer.LoadBalancerType == LoadBalancerTypeClassic {
158+
allWarnings = append(allWarnings, fmt.Sprintf(warningClassicELB, "primary control plane"))
159+
}
160+
}
161+
162+
if r.Spec.SecondaryControlPlaneLoadBalancer != nil {
163+
if r.Spec.SecondaryControlPlaneLoadBalancer.LoadBalancerType == LoadBalancerTypeClassic {
164+
allWarnings = append(allWarnings, fmt.Sprintf(warningClassicELB, "secondary control plane"))
165+
}
166+
}
167+
168+
if r.Spec.ControlPlaneLoadBalancer == nil || r.Spec.ControlPlaneLoadBalancer.HealthCheckProtocol == nil {
169+
allWarnings = append(allWarnings, fmt.Sprintf("%s. Existing load balancers will be updates", warningHealthCheckProtocolNotSet))
170+
}
171+
172+
return allWarnings, aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs)
143173
}
144174

145175
func (r *AWSCluster) validateControlPlaneLoadBalancerUpdate(oldlb, newlb *AWSLoadBalancerSpec) field.ErrorList {
@@ -180,16 +210,6 @@ func (r *AWSCluster) validateControlPlaneLoadBalancerUpdate(oldlb, newlb *AWSLoa
180210
newlb.Name, "field is immutable"),
181211
)
182212
}
183-
184-
// Block the update for Protocol :
185-
// - if it was not set in old spec but added in new spec
186-
// - if it was set in old spec but changed in new spec
187-
if !cmp.Equal(newlb.HealthCheckProtocol, oldlb.HealthCheckProtocol) {
188-
allErrs = append(allErrs,
189-
field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "healthCheckProtocol"),
190-
newlb.HealthCheckProtocol, "field is immutable once set"),
191-
)
192-
}
193213
}
194214

195215
return allErrs
@@ -301,8 +321,21 @@ func (r *AWSCluster) validateNetwork() field.ErrorList {
301321
return allErrs
302322
}
303323

304-
func (r *AWSCluster) validateControlPlaneLBs() field.ErrorList {
324+
func (r *AWSCluster) validateControlPlaneLBs() (admission.Warnings, field.ErrorList) {
305325
var allErrs field.ErrorList
326+
var allWarnings admission.Warnings
327+
328+
if r.Spec.ControlPlaneLoadBalancer != nil && r.Spec.ControlPlaneLoadBalancer.LoadBalancerType == LoadBalancerTypeClassic {
329+
allWarnings = append(allWarnings, fmt.Sprintf(warningClassicELB, "primary control plane"))
330+
331+
if r.Spec.ControlPlaneLoadBalancer.HealthCheckProtocol == nil {
332+
allWarnings = append(allWarnings, warningHealthCheckProtocolNotSet)
333+
}
334+
335+
if r.Spec.ControlPlaneLoadBalancer.HealthCheckProtocol != nil && *r.Spec.ControlPlaneLoadBalancer.HealthCheckProtocol != ELBProtocolSSL {
336+
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "healthcheckProtocol"), r.Spec.ControlPlaneLoadBalancer.HealthCheckProtocol, "loadbalancer is using a classic elb with SSL health check, this causes issues with ciper suites, please use TCP instead"))
337+
}
338+
}
306339

307340
// If the secondary is defined, check that the name is not empty and different from the primary.
308341
// Also, ensure that the secondary load balancer is an NLB
@@ -322,6 +355,9 @@ func (r *AWSCluster) validateControlPlaneLBs() field.ErrorList {
322355
if r.Spec.SecondaryControlPlaneLoadBalancer.LoadBalancerType != LoadBalancerTypeNLB {
323356
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "secondaryControlPlaneLoadBalancer", "loadBalancerType"), r.Spec.SecondaryControlPlaneLoadBalancer.LoadBalancerType, "secondary control plane load balancer must be a Network Load Balancer"))
324357
}
358+
if r.Spec.SecondaryControlPlaneLoadBalancer.LoadBalancerType == LoadBalancerTypeClassic {
359+
allWarnings = append(allWarnings, fmt.Sprintf(warningClassicELB, "secondary control plane"))
360+
}
325361
}
326362

327363
// Additional listeners are only supported for NLBs.
@@ -378,7 +414,7 @@ func (r *AWSCluster) validateControlPlaneLBs() field.ErrorList {
378414
}
379415
}
380416

381-
return allErrs
417+
return allWarnings, allErrs
382418
}
383419

384420
func (r *AWSCluster) validateIngressRule(rule IngressRule) field.ErrorList {

‎pkg/cloud/services/elb/loadbalancer.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,14 @@ func (s *Service) reconcileClassicLoadBalancer() error {
545545
}
546546
}
547547

548+
if !cmp.Equal(spec.HealthCheck, apiELB.HealthCheck) {
549+
s.scope.Debug("Reconciling health check for apiserver load balancer", "health-check", spec.HealthCheck)
550+
err := s.configureHealthCheck(apiELB.Name, spec.HealthCheck)
551+
if err != nil {
552+
return err
553+
}
554+
}
555+
548556
if err := s.reconcileELBTags(apiELB, spec.Tags); err != nil {
549557
return errors.Wrapf(err, "failed to reconcile tags for apiserver load balancer %q", apiELB.Name)
550558
}
@@ -587,6 +595,22 @@ func (s *Service) reconcileClassicLoadBalancer() error {
587595
return nil
588596
}
589597

598+
func (s *Service) configureHealthCheck(name string, healthCheck *infrav1.ClassicELBHealthCheck) error {
599+
if _, err := s.ELBClient.ConfigureHealthCheck(&elb.ConfigureHealthCheckInput{
600+
LoadBalancerName: aws.String(name),
601+
HealthCheck: &elb.HealthCheck{
602+
Target: aws.String(healthCheck.Target),
603+
Interval: aws.Int64(int64(healthCheck.Interval.Seconds())),
604+
Timeout: aws.Int64(int64(healthCheck.Timeout.Seconds())),
605+
HealthyThreshold: aws.Int64(healthCheck.HealthyThreshold),
606+
UnhealthyThreshold: aws.Int64(healthCheck.UnhealthyThreshold),
607+
},
608+
}); err != nil {
609+
return errors.Wrapf(err, "failed to configure health check for classic load balancer: %s", name)
610+
}
611+
return nil
612+
}
613+
590614
func (s *Service) deleteAPIServerELB() error {
591615
s.scope.Debug("Deleting control plane load balancer")
592616

‎test/e2e/data/e2e_conf.yaml

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ images:
1717
- name: gcr.io/k8s-staging-cluster-api/capa-manager:e2e
1818
loadBehavior: mustLoad
1919

20-
## PLEASE KEEP THESE UP TO DATE WITH THE COMPONENTS
20+
## PLEASE KEEP THESE UP TO DATE WITH THE COMPONENTS
2121

2222
# Cluster API v1beta1 Preloads
2323
- name: quay.io/jetstack/cert-manager-cainjector:v1.15.1
@@ -123,7 +123,17 @@ providers:
123123
files:
124124
- sourcePath: "./shared/v1beta1_provider/metadata.yaml"
125125
- sourcePath: "./infrastructure-aws/capi-upgrades/v1beta1/cluster-template.yaml"
126-
- name: v2.0.99
126+
- name: "v2.7.1"
127+
value: "https://github.com/kubernetes-sigs/cluster-api-provider-aws/releases/download/v2.7.1/infrastructure-components.yaml"
128+
type: "url"
129+
contract: v1beta2
130+
replacements:
131+
- old: --metrics-addr=127.0.0.1:8080
132+
new: --metrics-addr=:8080
133+
files:
134+
- sourcePath: "./shared/v1beta2_provider/v2.7/metadata.yaml"
135+
- sourcePath: "./infrastructure-aws/withoutclusterclass/generated/cluster-template-classicelb-upgrade.yaml"
136+
- name: v9.9.99
127137
# Use manifest from source files
128138
value: ../../../config/default
129139
# Do not add contract field for v1beta1 --> v1beta2 clusterctl upgrades test to work.
@@ -223,7 +233,7 @@ intervals:
223233
default/wait-failed-machine-status: ["2m", "10s"]
224234
default/wait-infra-subnets: ["5m", "30s"]
225235
default/wait-machine-pool-nodes: ["40m", "10s"]
226-
default/wait-machine-pool-upgrade: [ "50m", "10s" ]
236+
default/wait-machine-pool-upgrade: ["50m", "10s"]
227237
default/wait-create-identity: ["1m", "10s"]
228238
default/wait-job: ["10m", "10s"]
229239
default/wait-deployment-ready: ["5m", "10s"]

‎test/e2e/data/infrastructure-aws/withoutclusterclass/kustomize_sources/classicelb-upgrade/kustomization.yaml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
apiVersion: kustomize.config.k8s.io/v1beta1
2+
kind: Kustomization
3+
4+
resources:
5+
- ../default
6+
7+
patches:
8+
- path: nocontrolplane.yaml
9+
target:
10+
group: infrastructure.cluster.x-k8s.io
11+
kind: AWSCluster
12+
version: v1beta2

‎test/e2e/data/infrastructure-aws/withoutclusterclass/kustomize_sources/classicelb-upgrade/nocontrolplane.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
- op: remove
2+
path: /spec/controlPlaneLoadBalancer

‎test/e2e/data/shared/v1beta2_provider/metadata.yaml

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,27 @@ releaseSeries:
4141
- major: 2
4242
minor: 0
4343
contract: v1beta1
44+
- major: 2
45+
minor: 1
46+
contract: v1beta1
47+
- major: 2
48+
minor: 2
49+
contract: v1beta1
50+
- major: 2
51+
minor: 3
52+
contract: v1beta1
53+
- major: 2
54+
minor: 4
55+
contract: v1beta1
56+
- major: 2
57+
minor: 5
58+
contract: v1beta1
59+
- major: 2
60+
minor: 6
61+
contract: v1beta1
62+
- major: 2
63+
minor: 7
64+
contract: v1beta1
65+
- major: 9
66+
minor: 9
67+
contract: v1beta1

‎test/e2e/data/shared/v1beta2_provider/v2.7/metadata.yaml

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
# maps release series of major.minor to cluster-api contract version
2+
# the contract version may change between minor or major versions, but *not*
3+
# between patch versions.
4+
#
5+
# update this file only when a new major or minor version is released
6+
apiVersion: clusterctl.cluster.x-k8s.io/v1alpha3
7+
releaseSeries:
8+
- major: 0
9+
minor: 4
10+
contract: v1alpha2
11+
- major: 0
12+
minor: 5
13+
contract: v1alpha3
14+
- major: 0
15+
minor: 6
16+
contract: v1alpha3
17+
- major: 0
18+
minor: 7
19+
contract: v1alpha4
20+
- major: 1
21+
minor: 0
22+
contract: v1beta1
23+
- major: 1
24+
minor: 1
25+
contract: v1beta1
26+
- major: 1
27+
minor: 2
28+
contract: v1beta1
29+
- major: 1
30+
minor: 3
31+
contract: v1beta1
32+
- major: 1
33+
minor: 4
34+
contract: v1beta1
35+
- major: 1
36+
minor: 5
37+
contract: v1beta1
38+
- major: 1
39+
minor: 6
40+
contract: v1beta1
41+
- major: 2
42+
minor: 0
43+
contract: v1beta1
44+
- major: 2
45+
minor: 1
46+
contract: v1beta1
47+
- major: 2
48+
minor: 2
49+
contract: v1beta1
50+
- major: 2
51+
minor: 3
52+
contract: v1beta1
53+
- major: 2
54+
minor: 4
55+
contract: v1beta1
56+
- major: 2
57+
minor: 5
58+
contract: v1beta1
59+
- major: 2
60+
minor: 6
61+
contract: v1beta1
62+
- major: 2
63+
minor: 7
64+
contract: v1beta1

‎test/e2e/shared/aws_helpers.go

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,14 @@ import (
2424
"fmt"
2525
"strings"
2626

27+
. "github.com/onsi/ginkgo/v2"
28+
. "github.com/onsi/gomega"
29+
2730
"github.com/aws/aws-sdk-go/aws"
2831
"github.com/aws/aws-sdk-go/aws/arn"
2932
"github.com/aws/aws-sdk-go/aws/client"
33+
"github.com/aws/aws-sdk-go/service/elb"
3034
rgapi "github.com/aws/aws-sdk-go/service/resourcegroupstaggingapi"
31-
. "github.com/onsi/ginkgo/v2"
32-
. "github.com/onsi/gomega"
3335

3436
infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
3537
)
@@ -164,3 +166,28 @@ func DescribeResourcesByTags(input DescribeResourcesByTagsInput) (*DescribeResou
164166

165167
return output, nil
166168
}
169+
170+
type CheckClassicElbHealthCheckInput struct {
171+
AWSSession client.ConfigProvider
172+
LoadBalancerName string
173+
ExpectedTarget string
174+
}
175+
176+
func CheckClassicElbHealthCheck(input CheckClassicElbHealthCheckInput) {
177+
Byf("Checking the health check for the classic load balancer %s", input.LoadBalancerName)
178+
179+
elbSvc := elb.New(input.AWSSession)
180+
181+
out, err := elbSvc.DescribeLoadBalancers(&elb.DescribeLoadBalancersInput{
182+
LoadBalancerNames: []*string{
183+
aws.String(input.LoadBalancerName),
184+
},
185+
})
186+
Expect(err).NotTo(HaveOccurred(), "failed to get list of load balancers")
187+
Expect(out.LoadBalancerDescriptions).NotTo(BeEmpty(), "no load balancers found")
188+
lb := out.LoadBalancerDescriptions[0]
189+
190+
Expect(lb.HealthCheck.Target).ToNot(BeNil(), "health check target is nil")
191+
192+
Expect(*lb.HealthCheck.Target).To(BeEquivalentTo(input.ExpectedTarget), "health check target does not match expected target")
193+
}

‎test/e2e/shared/common.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,10 +162,10 @@ func DumpMachine(ctx context.Context, e2eCtx *E2EContext, machine infrav1.AWSMac
162162
}
163163
machineLogBase := path.Join(logPath, "instances", machine.Namespace, machine.Name)
164164
metaLog := path.Join(machineLogBase, "instance.log")
165-
if err := os.MkdirAll(filepath.Dir(metaLog), 0750); err != nil {
165+
if err := os.MkdirAll(filepath.Dir(metaLog), 0o750); err != nil {
166166
fmt.Fprintf(GinkgoWriter, "Couldn't create directory for file: path=%q, err=%s\n", metaLog, err)
167167
}
168-
f, err := os.OpenFile(metaLog, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) //nolint:gosec
168+
f, err := os.OpenFile(metaLog, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644) //nolint:gosec
169169
if err != nil {
170170
return
171171
}
@@ -238,7 +238,7 @@ func ConditionalIt(conditionFn ConditionFn, text string, body func()) bool {
238238

239239
// LoadE2EConfig loads the e2econfig from the specified path.
240240
func LoadE2EConfig(configPath string) *clusterctl.E2EConfig {
241-
//TODO: This is commented out as it assumes kubeadm and errors if its not there
241+
// TODO: This is commented out as it assumes kubeadm and errors if its not there
242242
// Remove localLoadE2EConfig and use the line below when this issue is resolved:
243243
// https://github.com/kubernetes-sigs/cluster-api/issues/3983
244244
// config := clusterctl.LoadE2EConfig(context.TODO(), clusterctl.LoadE2EConfigInput{ConfigPath: configPath})
@@ -277,3 +277,7 @@ func CreateAWSClusterControllerIdentity(k8sclient crclient.Client) {
277277
}
278278
_ = k8sclient.Create(context.TODO(), controllerIdentity)
279279
}
280+
281+
func Byf(format string, a ...interface{}) {
282+
By(fmt.Sprintf(format, a...))
283+
}

0 commit comments

Comments
 (0)