Skip to content

Commit e0596e6

Browse files
AlexanderYastrebovThilina Madumal
AlexanderYastrebov
authored and
Thilina Madumal
committed
worker: fail on missing certificates (#747)
Controller manages loadbalancer lifecycle based on available certificates. Prevent controller from deleting loadbalancers when there are no certificates found e.g. due to a bug like #725 or a tag filter misconfiguration (#658). Signed-off-by: Alexander Yastrebov <[email protected]>
1 parent a57f023 commit e0596e6

File tree

4 files changed

+192
-67
lines changed

4 files changed

+192
-67
lines changed

‎aws/acm.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func (p *acmCertificateProvider) GetCertificates(ctx context.Context) ([]*certs.
3333
if err != nil {
3434
return nil, err
3535
}
36-
result := make([]*certs.CertificateSummary, 0)
36+
result := make([]*certs.CertificateSummary, 0, len(acmSummaries))
3737
for _, o := range acmSummaries {
3838
summary, err := getCertificateSummaryFromACM(ctx, p.api, o.CertificateArn)
3939
if err != nil {

‎testdata/no_certificates/input/k8s/ing.yaml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
apiVersion: networking.k8s.io/v1
2+
kind: Ingress
3+
metadata:
4+
name: no_certificates
5+
spec:
6+
rules:
7+
- host: foo.bar.org
8+
http:
9+
paths:
10+
- backend:
11+
service:
12+
name: foo-bar-service
13+
port:
14+
name: main-port
15+
path: /
16+
pathType: ImplementationSpecific

‎worker.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,10 @@ func doWork(
318318
return problems.Add("failed to get certificates: %w", err)
319319
}
320320

321+
if len(certificateSummaries) == 0 {
322+
return problems.Add("no certificates found")
323+
}
324+
321325
cwAlarms, err := getCloudWatchAlarms(kubeAdapter, cwAlarmConfigMapLocation)
322326
if err != nil {
323327
return problems.Add("failed to retrieve cloudwatch alarm configuration: %w", err)

‎worker_test.go

Lines changed: 171 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"crypto/x509"
66
"encoding/json"
7+
"fmt"
78
"io"
89
"net/http/httptest"
910
"os"
@@ -50,15 +51,20 @@ func TestResourceConversionOneToOne(tt *testing.T) {
5051
}
5152

5253
for _, scenario := range []struct {
53-
name string
54+
name string
55+
typeLB string
56+
// mock
5457
responsesEC2 fake.EC2Outputs
5558
responsesASG fake.ASGOutputs
5659
responsesELBv2 fake.ELBv2Outputs
5760
responsesCF fake.CFOutputs
58-
typeLB string
61+
certsProvider certs.CertificatesProvider
62+
// expect
63+
problems []string
5964
}{
6065
{
61-
name: "ingress_alb",
66+
name: "ingress_alb",
67+
typeLB: aws.LoadBalancerTypeApplication,
6268
responsesEC2: fake.EC2Outputs{DescribeInstances: fake.R(fake.MockDescribeInstancesOutput(
6369
fake.TestInstance{
6470
Id: "i0",
@@ -106,10 +112,11 @@ func TestResourceConversionOneToOne(tt *testing.T) {
106112
DescribeStacks: fake.R(fake.MockDescribeStacksOutput(nil), nil),
107113
CreateStack: fake.R(fake.MockCSOutput("42"), nil),
108114
},
109-
typeLB: aws.LoadBalancerTypeApplication,
115+
certsProvider: certsProvider,
110116
},
111117
{
112-
name: "ingress_nlb",
118+
name: "ingress_nlb",
119+
typeLB: aws.LoadBalancerTypeNetwork,
113120
responsesEC2: fake.EC2Outputs{DescribeInstances: fake.R(fake.MockDescribeInstancesOutput(
114121
fake.TestInstance{
115122
Id: "i0",
@@ -157,9 +164,10 @@ func TestResourceConversionOneToOne(tt *testing.T) {
157164
DescribeStacks: fake.R(fake.MockDescribeStacksOutput(nil), nil),
158165
CreateStack: fake.R(fake.MockCSOutput("42"), nil),
159166
},
160-
typeLB: aws.LoadBalancerTypeNetwork,
167+
certsProvider: certsProvider,
161168
}, {
162-
name: "rg_alb",
169+
name: "rg_alb",
170+
typeLB: aws.LoadBalancerTypeApplication,
163171
responsesEC2: fake.EC2Outputs{DescribeInstances: fake.R(fake.MockDescribeInstancesOutput(
164172
fake.TestInstance{
165173
Id: "i0",
@@ -207,9 +215,10 @@ func TestResourceConversionOneToOne(tt *testing.T) {
207215
DescribeStacks: fake.R(fake.MockDescribeStacksOutput(nil), nil),
208216
CreateStack: fake.R(fake.MockCSOutput("42"), nil),
209217
},
210-
typeLB: aws.LoadBalancerTypeApplication,
218+
certsProvider: certsProvider,
211219
}, {
212-
name: "rg_nlb",
220+
name: "rg_nlb",
221+
typeLB: aws.LoadBalancerTypeNetwork,
213222
responsesEC2: fake.EC2Outputs{DescribeInstances: fake.R(fake.MockDescribeInstancesOutput(
214223
fake.TestInstance{
215224
Id: "i0",
@@ -257,9 +266,10 @@ func TestResourceConversionOneToOne(tt *testing.T) {
257266
DescribeStacks: fake.R(fake.MockDescribeStacksOutput(nil), nil),
258267
CreateStack: fake.R(fake.MockCSOutput("42"), nil),
259268
},
260-
typeLB: aws.LoadBalancerTypeNetwork,
269+
certsProvider: certsProvider,
261270
}, {
262-
name: "ing_shared_rg_notshared_alb",
271+
name: "ing_shared_rg_notshared_alb",
272+
typeLB: aws.LoadBalancerTypeApplication,
263273
responsesEC2: fake.EC2Outputs{DescribeInstances: fake.R(fake.MockDescribeInstancesOutput(
264274
fake.TestInstance{
265275
Id: "i0",
@@ -307,9 +317,10 @@ func TestResourceConversionOneToOne(tt *testing.T) {
307317
DescribeStacks: fake.R(fake.MockDescribeStacksOutput(nil), nil),
308318
CreateStack: fake.R(fake.MockCSOutput("42"), nil),
309319
},
310-
typeLB: aws.LoadBalancerTypeApplication,
320+
certsProvider: certsProvider,
311321
}, {
312-
name: "ingress_rg_shared_alb",
322+
name: "ingress_rg_shared_alb",
323+
typeLB: aws.LoadBalancerTypeApplication,
313324
responsesEC2: fake.EC2Outputs{DescribeInstances: fake.R(fake.MockDescribeInstancesOutput(
314325
fake.TestInstance{
315326
Id: "i0",
@@ -357,9 +368,10 @@ func TestResourceConversionOneToOne(tt *testing.T) {
357368
DescribeStacks: fake.R(fake.MockDescribeStacksOutput(nil), nil),
358369
CreateStack: fake.R(fake.MockCSOutput("42"), nil),
359370
},
360-
typeLB: aws.LoadBalancerTypeApplication,
371+
certsProvider: certsProvider,
361372
}, {
362-
name: "ingress_rg_shared_nlb",
373+
name: "ingress_rg_shared_nlb",
374+
typeLB: aws.LoadBalancerTypeNetwork,
363375
responsesEC2: fake.EC2Outputs{DescribeInstances: fake.R(fake.MockDescribeInstancesOutput(
364376
fake.TestInstance{
365377
Id: "i0",
@@ -407,59 +419,95 @@ func TestResourceConversionOneToOne(tt *testing.T) {
407419
DescribeStacks: fake.R(fake.MockDescribeStacksOutput(nil), nil),
408420
CreateStack: fake.R(fake.MockCSOutput("42"), nil),
409421
},
410-
typeLB: aws.LoadBalancerTypeNetwork,
422+
certsProvider: certsProvider,
423+
},
424+
{
425+
name: "no_certificates",
426+
typeLB: aws.LoadBalancerTypeApplication,
427+
responsesEC2: fake.EC2Outputs{DescribeInstances: fake.R(fake.MockDescribeInstancesOutput(
428+
fake.TestInstance{
429+
Id: "i0",
430+
Tags: fake.Tags{"aws:autoscaling:groupName": "asg1", clusterIDTagPrefix + clusterID: "owned"},
431+
PrivateIp: "1.2.3.3",
432+
VpcId: vpcID,
433+
State: running,
434+
}), nil),
435+
DescribeSecurityGroups: fake.R(fake.MockDescribeSecurityGroupsOutput(map[string]string{"id": securityGroupID}), nil),
436+
DescribeSubnets: fake.R(fake.MockDescribeSubnetsOutput(
437+
fake.TestSubnet{Id: "foo1", Name: "bar1", Az: "baz1", Tags: map[string]string{"kubernetes.io/role/elb": ""}}), nil),
438+
DescribeRouteTables: fake.R(fake.MockDescribeRouteTableOutput(
439+
fake.TestRouteTable{SubnetID: "foo1", GatewayIds: []string{"igw-foo1"}},
440+
fake.TestRouteTable{SubnetID: "mismatch", GatewayIds: []string{"igw-foo2"}, Main: true},
441+
), nil),
442+
},
443+
responsesASG: fake.ASGOutputs{
444+
DescribeAutoScalingGroups: fake.R(fake.MockDescribeAutoScalingGroupOutput(map[string]fake.ASGtags{"asg1": {
445+
clusterIDTagPrefix + clusterID: "owned",
446+
}}), nil),
447+
DescribeLoadBalancerTargetGroups: fake.R(&autoscaling.DescribeLoadBalancerTargetGroupsOutput{
448+
LoadBalancerTargetGroups: []autoScalingTypes.LoadBalancerTargetGroupState{},
449+
}, nil),
450+
AttachLoadBalancerTargetGroups: fake.R(nil, nil),
451+
},
452+
responsesELBv2: fake.ELBv2Outputs{
453+
DescribeTargetGroups: fake.R(fake.MockDescribeTargetGroupsOutput(), nil),
454+
DescribeTags: fake.R(nil, nil),
455+
},
456+
responsesCF: fake.CFOutputs{
457+
DescribeStacks: fake.R(fake.MockDescribeStacksOutput(nil), nil),
458+
CreateStack: fake.R(fake.MockCSOutput("42"), nil),
459+
},
460+
certsProvider: &certsfake.CertificateProvider{
461+
Summaries: nil,
462+
Error: fmt.Errorf("something went wrong"),
463+
},
464+
problems: []string{"failed to get certificates: something went wrong"},
465+
},
466+
{
467+
name: "no_certificates",
468+
typeLB: aws.LoadBalancerTypeApplication,
469+
responsesEC2: fake.EC2Outputs{DescribeInstances: fake.R(fake.MockDescribeInstancesOutput(
470+
fake.TestInstance{
471+
Id: "i0",
472+
Tags: fake.Tags{"aws:autoscaling:groupName": "asg1", clusterIDTagPrefix + clusterID: "owned"},
473+
PrivateIp: "1.2.3.3",
474+
VpcId: vpcID,
475+
State: running,
476+
}), nil),
477+
DescribeSecurityGroups: fake.R(fake.MockDescribeSecurityGroupsOutput(map[string]string{"id": securityGroupID}), nil),
478+
DescribeSubnets: fake.R(fake.MockDescribeSubnetsOutput(
479+
fake.TestSubnet{Id: "foo1", Name: "bar1", Az: "baz1", Tags: map[string]string{"kubernetes.io/role/elb": ""}}), nil),
480+
DescribeRouteTables: fake.R(fake.MockDescribeRouteTableOutput(
481+
fake.TestRouteTable{SubnetID: "foo1", GatewayIds: []string{"igw-foo1"}},
482+
fake.TestRouteTable{SubnetID: "mismatch", GatewayIds: []string{"igw-foo2"}, Main: true},
483+
), nil),
484+
},
485+
responsesASG: fake.ASGOutputs{
486+
DescribeAutoScalingGroups: fake.R(fake.MockDescribeAutoScalingGroupOutput(map[string]fake.ASGtags{"asg1": {
487+
clusterIDTagPrefix + clusterID: "owned",
488+
}}), nil),
489+
DescribeLoadBalancerTargetGroups: fake.R(&autoscaling.DescribeLoadBalancerTargetGroupsOutput{
490+
LoadBalancerTargetGroups: []autoScalingTypes.LoadBalancerTargetGroupState{},
491+
}, nil),
492+
AttachLoadBalancerTargetGroups: fake.R(nil, nil),
493+
},
494+
responsesELBv2: fake.ELBv2Outputs{
495+
DescribeTargetGroups: fake.R(fake.MockDescribeTargetGroupsOutput(), nil),
496+
DescribeTags: fake.R(nil, nil),
497+
},
498+
responsesCF: fake.CFOutputs{
499+
DescribeStacks: fake.R(fake.MockDescribeStacksOutput(nil), nil),
500+
CreateStack: fake.R(fake.MockCSOutput("42"), nil),
501+
},
502+
certsProvider: &certsfake.CertificateProvider{
503+
Summaries: []*certs.CertificateSummary{},
504+
Error: nil,
505+
},
506+
problems: []string{"no certificates found"},
411507
},
412508
} {
413509
tt.Run(scenario.name, func(t *testing.T) {
414510
ctx := context.Background()
415-
readFile := func(fileName string) []byte {
416-
b, err := os.ReadFile("./testdata/" + scenario.name + "/output/" + fileName)
417-
if err != nil {
418-
t.Fatal(err)
419-
}
420-
421-
return b
422-
}
423-
424-
var templates []string
425-
templateFiles, err := os.ReadDir("./testdata/" + scenario.name + "/output/templates/")
426-
if err != nil {
427-
t.Fatal(err)
428-
}
429-
430-
for _, file := range templateFiles {
431-
templates = append(templates, string(readFile("templates/"+file.Name())))
432-
}
433-
434-
paramFiles, err := os.ReadDir("./testdata/" + scenario.name + "/output/params/")
435-
if err != nil {
436-
t.Fatal(err)
437-
}
438-
439-
var params [][]cfTypes.Parameter
440-
for _, file := range paramFiles {
441-
var content []cfTypes.Parameter
442-
err := json.Unmarshal(readFile("params/"+file.Name()), &content)
443-
if err != nil {
444-
t.Fatal(err)
445-
}
446-
params = append(params, content)
447-
}
448-
449-
tagFiles, err := os.ReadDir("./testdata/" + scenario.name + "/output/tags/")
450-
if err != nil {
451-
t.Fatal(err)
452-
}
453-
454-
var tags [][]cfTypes.Tag
455-
for _, file := range tagFiles {
456-
var content []cfTypes.Tag
457-
err := json.Unmarshal(readFile("tags/"+file.Name()), &content)
458-
if err != nil {
459-
t.Fatal(err)
460-
}
461-
tags = append(tags, content)
462-
}
463511

464512
clientEC2 := &fake.EC2Client{Outputs: scenario.responsesEC2}
465513
clientASG := &fake.ASGClient{Outputs: scenario.responsesASG}
@@ -524,9 +572,66 @@ func TestResourceConversionOneToOne(tt *testing.T) {
524572
}
525573

526574
log.SetLevel(log.DebugLevel)
527-
problems := doWork(ctx, certsProvider, 10, time.Hour, a, k, "")
575+
problems := doWork(ctx, scenario.certsProvider, 10, time.Hour, a, k, "")
576+
577+
if scenario.problems != nil {
578+
assert.Len(t, problems.Errors(), len(scenario.problems))
579+
for i, problem := range problems.Errors() {
580+
assert.EqualError(t, problem, scenario.problems[i])
581+
}
582+
return
583+
}
584+
528585
if len(problems.Errors()) > 0 {
529-
t.Error(problems.Errors())
586+
t.Fatalf("expected no problems, got %v", problems.Errors())
587+
}
588+
589+
readFile := func(fileName string) []byte {
590+
b, err := os.ReadFile("./testdata/" + scenario.name + "/output/" + fileName)
591+
if err != nil {
592+
t.Fatal(err)
593+
}
594+
return b
595+
}
596+
597+
var templates []string
598+
templateFiles, err := os.ReadDir("./testdata/" + scenario.name + "/output/templates/")
599+
if err != nil {
600+
t.Fatal(err)
601+
}
602+
603+
for _, file := range templateFiles {
604+
templates = append(templates, string(readFile("templates/"+file.Name())))
605+
}
606+
607+
paramFiles, err := os.ReadDir("./testdata/" + scenario.name + "/output/params/")
608+
if err != nil {
609+
t.Fatal(err)
610+
}
611+
612+
var params [][]cfTypes.Parameter
613+
for _, file := range paramFiles {
614+
var content []cfTypes.Parameter
615+
err := json.Unmarshal(readFile("params/"+file.Name()), &content)
616+
if err != nil {
617+
t.Fatal(err)
618+
}
619+
params = append(params, content)
620+
}
621+
622+
tagFiles, err := os.ReadDir("./testdata/" + scenario.name + "/output/tags/")
623+
if err != nil {
624+
t.Fatal(err)
625+
}
626+
627+
var tags [][]cfTypes.Tag
628+
for _, file := range tagFiles {
629+
var content []cfTypes.Tag
630+
err := json.Unmarshal(readFile("tags/"+file.Name()), &content)
631+
if err != nil {
632+
t.Fatal(err)
633+
}
634+
tags = append(tags, content)
530635
}
531636

532637
assert.Equal(t, len(clientCF.GetTagCreationHistory()), len(tags))

0 commit comments

Comments
 (0)