Skip to content

Commit 12a57d3

Browse files
authored
Merge pull request #4390 from zac-nixon/main
fix waf name resolution
2 parents 51df227 + f3425d4 commit 12a57d3

File tree

6 files changed

+130
-46
lines changed

6 files changed

+130
-46
lines changed

docs/guide/ingress/annotations.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,6 +1055,9 @@ Load balancer capacity unit reservation can be configured via following annotati
10551055
10561056
- <a name="wafv2-acl-name">`alb.ingress.kubernetes.io/wafv2-acl-name`</a> specifies Name of the Amazon WAFv2 web ACL.
10571057
1058+
!!!note ""
1059+
The controller role must allow access to `wafv2:ListWebACLs`
1060+
10581061
!!!warning ""
10591062
Only Regional WAFv2 is supported.
10601063

pkg/aws/services/wafv2.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ type WAFv2 interface {
1010
AssociateWebACLWithContext(context.Context, *wafv2.AssociateWebACLInput) (*wafv2.AssociateWebACLOutput, error)
1111
DisassociateWebACLWithContext(ctx context.Context, req *wafv2.DisassociateWebACLInput) (*wafv2.DisassociateWebACLOutput, error)
1212
GetWebACLForResourceWithContext(ctx context.Context, req *wafv2.GetWebACLForResourceInput) (*wafv2.GetWebACLForResourceOutput, error)
13-
GetWebACLWithContext(ctx context.Context, req *wafv2.GetWebACLInput) (*wafv2.GetWebACLOutput, error)
13+
ListWebACLsWithContext(ctx context.Context, req *wafv2.ListWebACLsInput) (*wafv2.ListWebACLsOutput, error)
1414
}
1515

1616
// NewWAFv2 constructs new WAFv2 implementation.
@@ -48,10 +48,10 @@ func (c *wafv2Client) GetWebACLForResourceWithContext(ctx context.Context, req *
4848
return client.GetWebACLForResource(ctx, req)
4949
}
5050

51-
func (c *wafv2Client) GetWebACLWithContext(ctx context.Context, req *wafv2.GetWebACLInput) (*wafv2.GetWebACLOutput, error) {
51+
func (c *wafv2Client) ListWebACLsWithContext(ctx context.Context, req *wafv2.ListWebACLsInput) (*wafv2.ListWebACLsOutput, error) {
5252
client, err := c.awsClientsProvider.GetWAFv2Client(ctx, "GetWebACL")
5353
if err != nil {
5454
return nil, err
5555
}
56-
return client.GetWebACL(ctx, req)
56+
return client.ListWebACLs(ctx, req)
5757
}

pkg/aws/services/wafv2_mocks.go

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

pkg/ingress/model_build_load_balancer_addons.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,14 @@ package ingress
22

33
import (
44
"context"
5-
"sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants"
6-
75
"github.com/pkg/errors"
86
"k8s.io/apimachinery/pkg/util/sets"
97
"sigs.k8s.io/aws-load-balancer-controller/pkg/annotations"
108
"sigs.k8s.io/aws-load-balancer-controller/pkg/model/core"
119
shieldmodel "sigs.k8s.io/aws-load-balancer-controller/pkg/model/shield"
1210
wafregionalmodel "sigs.k8s.io/aws-load-balancer-controller/pkg/model/wafregional"
1311
wafv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/wafv2"
12+
"sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants"
1413
)
1514

1615
const (

pkg/ingress/model_build_load_balancer_addons_test.go

Lines changed: 89 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -847,8 +847,8 @@ func Test_defaultModelBuildTask_buildShieldProtection(t *testing.T) {
847847

848848
func Test_defaultModelBuildTask_buildWAFv2WebACLAssociationFromWAFv2Name(t *testing.T) {
849849
type getWebACLCall struct {
850-
req *wafv2sdk.GetWebACLInput
851-
resp *wafv2sdk.GetWebACLOutput
850+
req *wafv2sdk.ListWebACLsInput
851+
resp *wafv2sdk.ListWebACLsOutput
852852
err error
853853
}
854854
type fields struct {
@@ -942,12 +942,15 @@ func Test_defaultModelBuildTask_buildWAFv2WebACLAssociationFromWAFv2Name(t *test
942942
cache: map[string]string{},
943943
getWebACLCalls: []getWebACLCall{
944944
{
945-
req: &wafv2sdk.GetWebACLInput{
946-
Name: awssdk.String("web-acl-name1"),
945+
req: &wafv2sdk.ListWebACLsInput{
946+
Scope: wafv2types.ScopeRegional,
947947
},
948-
resp: &wafv2sdk.GetWebACLOutput{
949-
WebACL: &wafv2types.WebACL{
950-
ARN: awssdk.String("web-acl-arn1"),
948+
resp: &wafv2sdk.ListWebACLsOutput{
949+
WebACLs: []wafv2types.WebACLSummary{
950+
{
951+
Name: awssdk.String("web-acl-name1"),
952+
ARN: awssdk.String("web-acl-arn1"),
953+
},
951954
},
952955
},
953956
err: nil,
@@ -1041,12 +1044,15 @@ func Test_defaultModelBuildTask_buildWAFv2WebACLAssociationFromWAFv2Name(t *test
10411044
cache: map[string]string{},
10421045
getWebACLCalls: []getWebACLCall{
10431046
{
1044-
req: &wafv2sdk.GetWebACLInput{
1045-
Name: awssdk.String("web-acl-name1"),
1047+
req: &wafv2sdk.ListWebACLsInput{
1048+
Scope: wafv2types.ScopeRegional,
10461049
},
1047-
resp: &wafv2sdk.GetWebACLOutput{
1048-
WebACL: &wafv2types.WebACL{
1049-
ARN: awssdk.String("web-acl-arn1"),
1050+
resp: &wafv2sdk.ListWebACLsOutput{
1051+
WebACLs: []wafv2types.WebACLSummary{
1052+
{
1053+
Name: awssdk.String("web-acl-name1"),
1054+
ARN: awssdk.String("web-acl-arn1"),
1055+
},
10501056
},
10511057
},
10521058
err: nil,
@@ -1093,12 +1099,15 @@ func Test_defaultModelBuildTask_buildWAFv2WebACLAssociationFromWAFv2Name(t *test
10931099
cache: map[string]string{},
10941100
getWebACLCalls: []getWebACLCall{
10951101
{
1096-
req: &wafv2sdk.GetWebACLInput{
1097-
Name: awssdk.String("web-acl-name1"),
1102+
req: &wafv2sdk.ListWebACLsInput{
1103+
Scope: wafv2types.ScopeRegional,
10981104
},
1099-
resp: &wafv2sdk.GetWebACLOutput{
1100-
WebACL: &wafv2types.WebACL{
1101-
ARN: awssdk.String("web-acl-arn1"),
1105+
resp: &wafv2sdk.ListWebACLsOutput{
1106+
WebACLs: []wafv2types.WebACLSummary{
1107+
{
1108+
Name: awssdk.String("web-acl-name1"),
1109+
ARN: awssdk.String("web-acl-arn1"),
1110+
},
11021111
},
11031112
},
11041113
err: nil,
@@ -1147,12 +1156,15 @@ func Test_defaultModelBuildTask_buildWAFv2WebACLAssociationFromWAFv2Name(t *test
11471156
cache: map[string]string{},
11481157
getWebACLCalls: []getWebACLCall{
11491158
{
1150-
req: &wafv2sdk.GetWebACLInput{
1151-
Name: awssdk.String("web-acl-name1"),
1159+
req: &wafv2sdk.ListWebACLsInput{
1160+
Scope: wafv2types.ScopeRegional,
11521161
},
1153-
resp: &wafv2sdk.GetWebACLOutput{
1154-
WebACL: &wafv2types.WebACL{
1155-
ARN: awssdk.String("web-acl-arn1"),
1162+
resp: &wafv2sdk.ListWebACLsOutput{
1163+
WebACLs: []wafv2types.WebACLSummary{
1164+
{
1165+
Name: awssdk.String("web-acl-name1"),
1166+
ARN: awssdk.String("web-acl-arn1"),
1167+
},
11561168
},
11571169
},
11581170
err: nil,
@@ -1216,6 +1228,58 @@ func Test_defaultModelBuildTask_buildWAFv2WebACLAssociationFromWAFv2Name(t *test
12161228
},
12171229
wantCache: map[string]string{},
12181230
},
1231+
{
1232+
name: "name not found",
1233+
fields: fields{
1234+
ingGroup: Group{
1235+
Members: []ClassifiedIngress{
1236+
{
1237+
Ing: &networking.Ingress{
1238+
ObjectMeta: metav1.ObjectMeta{
1239+
Namespace: "awesome-ns",
1240+
Name: "awesome-ing-0",
1241+
Annotations: map[string]string{
1242+
"alb.ingress.kubernetes.io/wafv2-acl-name": "web-acl-name1",
1243+
},
1244+
},
1245+
},
1246+
IngClassConfig: ClassConfiguration{
1247+
IngClassParams: &v1beta1.IngressClassParams{
1248+
Spec: v1beta1.IngressClassParamsSpec{
1249+
WAFv2ACLName: "web-acl-name1",
1250+
},
1251+
},
1252+
},
1253+
},
1254+
},
1255+
},
1256+
},
1257+
args: args{
1258+
lbARN: core.LiteralStringToken("awesome-lb-arn"),
1259+
cache: map[string]string{},
1260+
getWebACLCalls: []getWebACLCall{
1261+
{
1262+
req: &wafv2sdk.ListWebACLsInput{
1263+
Scope: wafv2types.ScopeRegional,
1264+
},
1265+
resp: &wafv2sdk.ListWebACLsOutput{
1266+
WebACLs: []wafv2types.WebACLSummary{
1267+
{
1268+
Name: awssdk.String("some other name"),
1269+
ARN: awssdk.String("web-acl-arn1"),
1270+
},
1271+
},
1272+
},
1273+
err: nil,
1274+
},
1275+
},
1276+
},
1277+
wantErr: func(t assert.TestingT, err error, msgAndArgs ...interface{}) bool {
1278+
assert.EqualError(t, err, "couldn't find WAFv2 WebACL with name: web-acl-name1", msgAndArgs...)
1279+
return false
1280+
},
1281+
wantCache: map[string]string{},
1282+
},
12191283
}
12201284

12211285
for _, tt := range tests {
@@ -1229,7 +1293,7 @@ func Test_defaultModelBuildTask_buildWAFv2WebACLAssociationFromWAFv2Name(t *test
12291293

12301294
for _, call := range tt.args.getWebACLCalls {
12311295
wafv2Client.EXPECT().
1232-
GetWebACLWithContext(gomock.Any(), call.req).
1296+
ListWebACLsWithContext(gomock.Any(), call.req).
12331297
Return(call.resp, call.err)
12341298
}
12351299

@@ -1257,8 +1321,8 @@ func Test_defaultModelBuildTask_buildWAFv2WebACLAssociationFromWAFv2Name(t *test
12571321
for webACLName, expectedArn := range tt.wantCache {
12581322
rawCacheItem, exists := task.webACLNameToArnMapper.cache.Get(webACLName)
12591323
assert.True(t, exists)
1260-
cachedArn := rawCacheItem.(*string)
1261-
assert.Equal(t, expectedArn, *cachedArn)
1324+
cachedArn := rawCacheItem.(string)
1325+
assert.Equal(t, expectedArn, cachedArn)
12621326
}
12631327
})
12641328
}

pkg/ingress/model_builder.go

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
elbv2sdk "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2"
66
wafv2sdk "github.com/aws/aws-sdk-go-v2/service/wafv2"
7+
wafv2types "github.com/aws/aws-sdk-go-v2/service/wafv2/types"
78
"k8s.io/apimachinery/pkg/util/cache"
89
"reflect"
910
"strconv"
@@ -191,6 +192,7 @@ func (b *defaultModelBuilder) Build(ctx context.Context, ingGroup Group, metrics
191192
tgByResID: make(map[string]*elbv2model.TargetGroup),
192193
backendServices: make(map[types.NamespacedName]*corev1.Service),
193194
targetGroupNameToArnMapper: b.targetGroupNameToArnMapper,
195+
webACLNameToArnMapper: b.webACLNameToArnMapper,
194196
}
195197
if err := task.run(ctx); err != nil {
196198
return nil, nil, nil, false, nil, nil, err
@@ -560,6 +562,7 @@ func newWebACLNameToArnMapper(wafv2Client services.WAFv2, ttl time.Duration) *we
560562
wafv2Client: wafv2Client,
561563
cache: cache.NewExpiring(),
562564
cacheTTL: ttl,
565+
cacheMutex: sync.RWMutex{},
563566
}
564567
}
565568

@@ -609,15 +612,30 @@ func (w *webACLNameToArnMapper) getArnByName(ctx context.Context, webACLName str
609612
if rawCacheItem, exists := w.cache.Get(webACLName); exists {
610613
return rawCacheItem.(string), nil
611614
}
612-
req := &wafv2sdk.GetWebACLInput{
613-
Name: awssdk.String(webACLName),
614-
}
615615

616-
webACL, err := w.wafv2Client.GetWebACLWithContext(ctx, req)
617-
if err != nil {
618-
return "", err
616+
firstRun := true
617+
var next *string
618+
619+
for firstRun || next != nil {
620+
req := &wafv2sdk.ListWebACLsInput{
621+
Scope: wafv2types.ScopeRegional,
622+
NextMarker: next,
623+
}
624+
625+
output, err := w.wafv2Client.ListWebACLsWithContext(ctx, req)
626+
if err != nil {
627+
return "", err
628+
}
629+
630+
for _, o := range output.WebACLs {
631+
if o.Name != nil && *o.Name == webACLName {
632+
arn := *o.ARN
633+
w.cache.Set(webACLName, arn, w.cacheTTL)
634+
return arn, nil
635+
}
636+
}
637+
firstRun = false
638+
next = output.NextMarker
619639
}
620-
arn := webACL.WebACL.ARN
621-
w.cache.Set(webACLName, arn, w.cacheTTL)
622-
return *arn, nil
640+
return "", errors.New("Unable to find web acl named " + webACLName)
623641
}

0 commit comments

Comments
 (0)