Skip to content

Commit 3226ff2

Browse files
M00nF1shM00nF1sh
committed
adding cache around wafAPI usage
1 parent 32ca4ad commit 3226ff2

File tree

4 files changed

+166
-52
lines changed

4 files changed

+166
-52
lines changed

‎internal/alb/lb/loadbalancer.go

Lines changed: 4 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ func NewController(
4545
sgAssociationController sg.AssociationController,
4646
tagsController tags.Controller) Controller {
4747
attrsController := NewAttributesController(cloud)
48+
wafController := NewWAFController(cloud)
4849

4950
return &defaultController{
5051
cloud: cloud,
@@ -55,6 +56,7 @@ func NewController(
5556
sgAssociationController: sgAssociationController,
5657
tagsController: tagsController,
5758
attrsController: attrsController,
59+
wafController: wafController,
5860
}
5961
}
6062

@@ -78,6 +80,7 @@ type defaultController struct {
7880
sgAssociationController sg.AssociationController
7981
tagsController tags.Controller
8082
attrsController AttributesController
83+
wafController WAFController
8184
}
8285

8386
var _ Controller = (*defaultController)(nil)
@@ -111,7 +114,7 @@ func (controller *defaultController) Reconcile(ctx context.Context, ingress *ext
111114
}
112115

113116
if controller.store.GetConfig().FeatureGate.Enabled(config.WAF) {
114-
if err := controller.reconcileWAF(ctx, lbArn, ingressAnnos.LoadBalancer.WebACLId); err != nil {
117+
if err := controller.wafController.Reconcile(ctx, lbArn, ingress); err != nil {
115118
return nil, err
116119
}
117120
}
@@ -252,48 +255,6 @@ func (controller *defaultController) reconcileLBInstance(ctx context.Context, in
252255
return nil
253256
}
254257

255-
func (controller *defaultController) reconcileWAF(ctx context.Context, lbArn string, webACLID *string) error {
256-
webACLSummary, err := controller.cloud.GetWebACLSummary(ctx, aws.String(lbArn))
257-
if err != nil {
258-
return fmt.Errorf("error getting web acl for load balancer %v: %v", lbArn, err)
259-
}
260-
261-
if webACLID != nil {
262-
b, err := controller.cloud.WebACLExists(ctx, webACLID)
263-
if err != nil {
264-
return fmt.Errorf("error fetching web acl %v: %v", aws.StringValue(webACLID), err)
265-
}
266-
if !b {
267-
return fmt.Errorf("web acl %v does not exist", aws.StringValue(webACLID))
268-
}
269-
}
270-
271-
switch {
272-
case webACLSummary != nil && webACLID == nil:
273-
{
274-
albctx.GetLogger(ctx).Infof("disassociate WAF on %v", lbArn)
275-
if _, err := controller.cloud.DisassociateWAF(ctx, aws.String(lbArn)); err != nil {
276-
return fmt.Errorf("failed to disassociate webACL on loadBalancer %v due to %v", lbArn, err)
277-
}
278-
}
279-
case webACLSummary != nil && webACLID != nil && aws.StringValue(webACLSummary.WebACLId) != aws.StringValue(webACLID):
280-
{
281-
albctx.GetLogger(ctx).Infof("associate WAF on %v to %v", lbArn, aws.StringValue(webACLID))
282-
if _, err := controller.cloud.AssociateWAF(ctx, aws.String(lbArn), webACLID); err != nil {
283-
return fmt.Errorf("failed to associate webACL on loadBalancer %v due to %v", lbArn, err)
284-
}
285-
}
286-
case webACLSummary == nil && webACLID != nil:
287-
{
288-
albctx.GetLogger(ctx).Infof("associate WAF on %v to %v", lbArn, aws.StringValue(webACLID))
289-
if _, err := controller.cloud.AssociateWAF(ctx, aws.String(lbArn), webACLID); err != nil {
290-
return fmt.Errorf("failed to associate webACL on loadBalancer %v due to %v", lbArn, err)
291-
}
292-
}
293-
}
294-
return nil
295-
}
296-
297258
func (controller *defaultController) isLBInstanceNeedRecreation(ctx context.Context, instance *elbv2.LoadBalancer, lbConfig *loadBalancerConfig) bool {
298259
if !util.DeepEqual(instance.Scheme, lbConfig.Scheme) {
299260
albctx.GetLogger(ctx).Infof("LoadBalancer %s need recreation due to scheme changed(%s => %s)",

‎internal/alb/lb/waf.go

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
package lb
2+
3+
import (
4+
"context"
5+
"time"
6+
7+
"github.com/kubernetes-sigs/aws-alb-ingress-controller/internal/albctx"
8+
"github.com/kubernetes-sigs/aws-alb-ingress-controller/internal/aws"
9+
"github.com/kubernetes-sigs/aws-alb-ingress-controller/internal/ingress/annotations"
10+
"github.com/pkg/errors"
11+
extensions "k8s.io/api/extensions/v1beta1"
12+
"k8s.io/apimachinery/pkg/util/cache"
13+
)
14+
15+
const (
16+
webACLIdForLBCacheMaxSize = 1024
17+
webACLIdForLBCacheTTL = 10 * time.Minute
18+
)
19+
20+
// WAFController provides functionality to manage ALB's WAF associations.
21+
type WAFController interface {
22+
Reconcile(ctx context.Context, lbArn string, ingress *extensions.Ingress) error
23+
}
24+
25+
func NewWAFController(cloud aws.CloudAPI) WAFController {
26+
return &defaultWAFController{
27+
cloud: cloud,
28+
webACLIdForLBCache: cache.NewLRUExpireCache(webACLIdForLBCacheMaxSize),
29+
}
30+
}
31+
32+
type defaultWAFController struct {
33+
cloud aws.CloudAPI
34+
35+
// cache that stores webACLIdForLBCache for LoadBalancerARN.
36+
// The cache value is string, while "" represents no webACL.
37+
webACLIdForLBCache *cache.LRUExpireCache
38+
}
39+
40+
func (c *defaultWAFController) Reconcile(ctx context.Context, lbArn string, ing *extensions.Ingress) error {
41+
currentWebACLId, err := c.getCurrentWebACLId(ctx, lbArn)
42+
if err != nil {
43+
return err
44+
}
45+
desiredWebACLId := c.getDesiredWebACLId(ctx, ing)
46+
47+
switch {
48+
case desiredWebACLId == "" && currentWebACLId != "":
49+
albctx.GetLogger(ctx).Infof("disassociate WAF on %v", lbArn)
50+
if _, err := c.cloud.DisassociateWAF(ctx, aws.String(lbArn)); err != nil {
51+
return errors.Wrapf(err, "failed to disassociate webACL on LoadBalancer %v", lbArn)
52+
}
53+
c.webACLIdForLBCache.Add(lbArn, desiredWebACLId, webACLIdForLBCacheTTL)
54+
case desiredWebACLId != "" && currentWebACLId != "" && desiredWebACLId != currentWebACLId:
55+
albctx.GetLogger(ctx).Infof("associate WAF on %v from %v to %v", lbArn, currentWebACLId, desiredWebACLId)
56+
if _, err := c.cloud.AssociateWAF(ctx, aws.String(lbArn), aws.String(desiredWebACLId)); err != nil {
57+
return errors.Wrapf(err, "failed to associate webACL on LoadBalancer %v", lbArn)
58+
}
59+
c.webACLIdForLBCache.Add(lbArn, desiredWebACLId, webACLIdForLBCacheTTL)
60+
case desiredWebACLId != "" && currentWebACLId == "":
61+
albctx.GetLogger(ctx).Infof("associate WAF on %v to %v", lbArn, desiredWebACLId)
62+
if _, err := c.cloud.AssociateWAF(ctx, aws.String(lbArn), aws.String(desiredWebACLId)); err != nil {
63+
return errors.Wrapf(err, "failed to associate webACL on LoadBalancer %v", lbArn)
64+
}
65+
c.webACLIdForLBCache.Add(lbArn, desiredWebACLId, webACLIdForLBCacheTTL)
66+
}
67+
return nil
68+
}
69+
70+
func (c *defaultWAFController) getDesiredWebACLId(ctx context.Context, ing *extensions.Ingress) string {
71+
var webACLId string
72+
// support legacy waf-acl-id annotation
73+
_ = annotations.LoadStringAnnotation("waf-acl-id", &webACLId, ing.Annotations)
74+
_ = annotations.LoadStringAnnotation("web-acl-id", &webACLId, ing.Annotations)
75+
return webACLId
76+
}
77+
78+
func (c *defaultWAFController) getCurrentWebACLId(ctx context.Context, lbArn string) (string, error) {
79+
cachedWebACLId, exists := c.webACLIdForLBCache.Get(lbArn)
80+
if exists {
81+
return cachedWebACLId.(string), nil
82+
}
83+
84+
webACLSummary, err := c.cloud.GetWebACLSummary(ctx, aws.String(lbArn))
85+
if err != nil {
86+
return "", errors.Wrapf(err, "failed to get web acl for load balancer %v", lbArn)
87+
}
88+
89+
var webACLId string
90+
if webACLSummary != nil {
91+
webACLId = aws.StringValue(webACLSummary.WebACLId)
92+
}
93+
94+
c.webACLIdForLBCache.Add(lbArn, webACLId, webACLIdForLBCacheTTL)
95+
return webACLId, nil
96+
}

‎internal/alb/lb/waf_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package lb
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/kubernetes-sigs/aws-alb-ingress-controller/internal/ingress/annotations/parser"
8+
"github.com/kubernetes-sigs/aws-alb-ingress-controller/mocks"
9+
extensions "k8s.io/api/extensions/v1beta1"
10+
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
11+
"k8s.io/apimachinery/pkg/util/cache"
12+
)
13+
14+
func Test_defaultWAFController_getDesiredWebACLId(t *testing.T) {
15+
tests := []struct {
16+
name string
17+
ing *extensions.Ingress
18+
want string
19+
}{
20+
{
21+
name: "ingress without waf settings",
22+
ing: &extensions.Ingress{
23+
ObjectMeta: v1.ObjectMeta{
24+
Name: "ingress",
25+
Annotations: map[string]string{},
26+
},
27+
},
28+
want: "",
29+
},
30+
{
31+
name: "ingress with web-acl-id(waf classic)",
32+
ing: &extensions.Ingress{
33+
ObjectMeta: v1.ObjectMeta{
34+
Name: "ingress",
35+
Annotations: map[string]string{
36+
parser.AnnotationsPrefix + "/web-acl-id": "my-web-acl-id",
37+
},
38+
},
39+
},
40+
want: "my-web-acl-id",
41+
},
42+
{
43+
name: "ingress with waf-acl-id(waf classic)",
44+
ing: &extensions.Ingress{
45+
ObjectMeta: v1.ObjectMeta{
46+
Name: "ingress",
47+
Annotations: map[string]string{
48+
parser.AnnotationsPrefix + "/waf-acl-id": "my-web-acl-id",
49+
},
50+
},
51+
},
52+
want: "my-web-acl-id",
53+
},
54+
}
55+
for _, tt := range tests {
56+
t.Run(tt.name, func(t *testing.T) {
57+
c := &defaultWAFController{
58+
cloud: &mocks.CloudAPI{},
59+
webACLIdForLBCache: cache.NewLRUExpireCache(10),
60+
}
61+
if got := c.getDesiredWebACLId(context.Background(), tt.ing); got != tt.want {
62+
t.Errorf("getDesiredWebACLId() = %v, want %v", got, tt.want)
63+
}
64+
})
65+
}
66+
}

‎internal/ingress/annotations/loadbalancer/main.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ type PortData struct {
3939
type Config struct {
4040
Scheme *string
4141
IPAddressType *string
42-
WebACLId *string
4342

4443
InboundCidrs []string
4544
InboundV6CIDRs []string
@@ -65,13 +64,6 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation {
6564

6665
// Parse parses the annotations contained in the resource
6766
func (lb loadBalancer) Parse(ing parser.AnnotationInterface) (interface{}, error) {
68-
// support legacy waf-acl-id annotation
69-
webACLId, _ := parser.GetStringAnnotation("waf-acl-id", ing)
70-
w, err := parser.GetStringAnnotation("web-acl-id", ing)
71-
if err == nil {
72-
webACLId = w
73-
}
74-
7567
ipAddressType, err := parser.GetStringAnnotation("ip-address-type", ing)
7668
if err != nil {
7769
ipAddressType = aws.String(DefaultIPAddressType)
@@ -109,7 +101,6 @@ func (lb loadBalancer) Parse(ing parser.AnnotationInterface) (interface{}, error
109101
}
110102

111103
return &Config{
112-
WebACLId: webACLId,
113104
Scheme: scheme,
114105
IPAddressType: ipAddressType,
115106

0 commit comments

Comments
 (0)