Skip to content

Commit b806c53

Browse files
M00nF1shM00nF1sh
authored
refactor ingress model builder (#1422)
* refactor ingress model builder * handle targetGroupNotFound error * output model deploy logs when deleting ingress group * gc securityGroup rules
1 parent 48d7533 commit b806c53

15 files changed

+679
-309
lines changed

‎controllers/ingress/group_controller.go

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,16 @@ package ingress
33
import (
44
"context"
55
"github.com/go-logr/logr"
6+
"github.com/pkg/errors"
7+
corev1 "k8s.io/api/core/v1"
68
networking "k8s.io/api/networking/v1beta1"
79
"k8s.io/client-go/tools/record"
810
"sigs.k8s.io/aws-alb-ingress-controller/controllers/ingress/eventhandlers"
911
"sigs.k8s.io/aws-alb-ingress-controller/pkg/annotations"
1012
"sigs.k8s.io/aws-alb-ingress-controller/pkg/aws/services"
1113
"sigs.k8s.io/aws-alb-ingress-controller/pkg/deploy"
1214
"sigs.k8s.io/aws-alb-ingress-controller/pkg/ingress"
15+
"sigs.k8s.io/aws-alb-ingress-controller/pkg/k8s"
1316
networkingpkg "sigs.k8s.io/aws-alb-ingress-controller/pkg/networking"
1417
ctrl "sigs.k8s.io/controller-runtime"
1518
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -34,6 +37,7 @@ func NewGroupReconciler(k8sClient client.Client, eventRecorder record.EventRecor
3437
finalizerManager := ingress.NewDefaultFinalizerManager(k8sClient)
3538

3639
return &GroupReconciler{
40+
k8sClient: k8sClient,
3741
groupLoader: groupLoader,
3842
finalizerManager: finalizerManager,
3943
modelBuilder: modelBuilder,
@@ -45,6 +49,7 @@ func NewGroupReconciler(k8sClient client.Client, eventRecorder record.EventRecor
4549

4650
// GroupReconciler reconciles a ingress group
4751
type GroupReconciler struct {
52+
k8sClient client.Client
4853
modelBuilder ingress.ModelBuilder
4954
stackMarshaller deploy.StackMarshaller
5055
stackDeployer deploy.StackDeployer
@@ -61,18 +66,17 @@ type GroupReconciler struct {
6166
// Reconcile
6267
func (r *GroupReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
6368
ctx := context.Background()
64-
groupID := ingress.DecodeGroupIDFromReconcileRequest(req)
65-
_ = r.log.WithValues("groupID", groupID)
66-
group, err := r.groupLoader.Load(ctx, groupID)
69+
ingGroupID := ingress.DecodeGroupIDFromReconcileRequest(req)
70+
_ = r.log.WithValues("groupID", ingGroupID)
71+
ingGroup, err := r.groupLoader.Load(ctx, ingGroupID)
6772
if err != nil {
6873
return ctrl.Result{}, err
6974
}
7075

71-
if err := r.finalizerManager.AddGroupFinalizer(ctx, groupID, group.Members...); err != nil {
76+
if err := r.finalizerManager.AddGroupFinalizer(ctx, ingGroupID, ingGroup.Members...); err != nil {
7277
return ctrl.Result{}, err
7378
}
74-
75-
stack, lb, err := r.modelBuilder.Build(ctx, group)
79+
stack, lb, err := r.modelBuilder.Build(ctx, ingGroup)
7680
if err != nil {
7781
return ctrl.Result{}, err
7882
}
@@ -84,18 +88,50 @@ func (r *GroupReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
8488
if err := r.stackDeployer.Deploy(ctx, stack); err != nil {
8589
return ctrl.Result{}, err
8690
}
87-
lbARN, err := lb.LoadBalancerARN().Resolve(ctx)
88-
if err != nil {
89-
return ctrl.Result{}, err
91+
r.log.Info("successfully deployed model")
92+
93+
if len(ingGroup.Members) > 0 {
94+
lbDNS, err := lb.DNSName().Resolve(ctx)
95+
if err != nil {
96+
return ctrl.Result{}, err
97+
}
98+
if err := r.updateIngressGroupStatus(ctx, ingGroup, lbDNS); err != nil {
99+
return ctrl.Result{}, err
100+
}
90101
}
91-
r.log.Info("successfully deployed model", "LB", lbARN)
92102

93-
if err := r.finalizerManager.RemoveGroupFinalizer(ctx, groupID, group.InactiveMembers...); err != nil {
103+
if err := r.finalizerManager.RemoveGroupFinalizer(ctx, ingGroupID, ingGroup.InactiveMembers...); err != nil {
94104
return ctrl.Result{}, err
95105
}
96106
return ctrl.Result{}, nil
97107
}
98108

109+
func (r *GroupReconciler) updateIngressGroupStatus(ctx context.Context, ingGroup ingress.Group, lbDNS string) error {
110+
for _, ing := range ingGroup.Members {
111+
if err := r.updateIngressStatus(ctx, ing, lbDNS); err != nil {
112+
return err
113+
}
114+
}
115+
return nil
116+
}
117+
118+
func (r *GroupReconciler) updateIngressStatus(ctx context.Context, ing *networking.Ingress, lbDNS string) error {
119+
if len(ing.Status.LoadBalancer.Ingress) != 1 ||
120+
ing.Status.LoadBalancer.Ingress[0].IP != "" ||
121+
ing.Status.LoadBalancer.Ingress[0].Hostname != lbDNS {
122+
ingOld := ing.DeepCopy()
123+
ing.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{
124+
{
125+
Hostname: lbDNS,
126+
},
127+
}
128+
if err := r.k8sClient.Status().Patch(ctx, ing, client.MergeFrom(ingOld)); err != nil {
129+
return errors.Wrapf(err, "failed to update ingress status: %v", k8s.NamespacedName(ing))
130+
}
131+
}
132+
return nil
133+
}
134+
99135
func (r *GroupReconciler) SetupWithManager(mgr ctrl.Manager) error {
100136
c, err := controller.New(controllerName, mgr, controller.Options{
101137
MaxConcurrentReconciles: 1,

‎pkg/annotations/constants.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ const (
1616
IngressSuffixLoadBalancerAttributes = "load-balancer-attributes"
1717
IngressSuffixSecurityGroups = "security-groups"
1818
IngressSuffixListenPorts = "listen-ports"
19+
IngressSuffixInboundCIDRs = "inbound-cidrs"
1920
IngressSuffixCertificateARN = "certificate-arn"
2021
IngressSuffixSSLPolicy = "ssl-policy"
2122
IngressSuffixTargetType = "target-type"

‎pkg/aws/errors/utils.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package errors
2+
3+
import (
4+
"github.com/aws/aws-sdk-go/aws/awserr"
5+
"github.com/pkg/errors"
6+
)
7+
8+
func IsELBV2TargetGroupNotFoundError(err error) bool {
9+
var awsErr awserr.Error
10+
if errors.As(err, &awsErr) {
11+
return awsErr.Code() == "TargetGroupNotFound"
12+
}
13+
return false
14+
}
15+
16+
func IsEC2SecurityGroupNotFoundError(err error) bool {
17+
var awsErr awserr.Error
18+
if errors.As(err, &awsErr) {
19+
return awsErr.Code() == "InvalidGroup.NotFound"
20+
}
21+
return false
22+
}

‎pkg/deploy/ec2/security_group_manager.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,9 @@ func buildIPPermissionInfo(permission ec2model.IPPermission) (networking.IPPermi
167167
labels := networking.NewIPPermissionLabelsForRawDescription(permission.IPRanges[0].Description)
168168
return networking.NewCIDRIPPermission(protocol, permission.FromPort, permission.ToPort, permission.IPRanges[0].CIDRIP, labels), nil
169169
}
170-
if len(permission.IPV6Range) == 1 {
171-
labels := networking.NewIPPermissionLabelsForRawDescription(permission.IPV6Range[0].Description)
172-
return networking.NewCIDRIPPermission(protocol, permission.FromPort, permission.ToPort, permission.IPV6Range[0].CIDRIPv6, labels), nil
170+
if len(permission.IPv6Range) == 1 {
171+
labels := networking.NewIPPermissionLabelsForRawDescription(permission.IPv6Range[0].Description)
172+
return networking.NewCIDRIPPermission(protocol, permission.FromPort, permission.ToPort, permission.IPv6Range[0].CIDRIPv6, labels), nil
173173
}
174174
if len(permission.UserIDGroupPairs) == 1 {
175175
labels := networking.NewIPPermissionLabelsForRawDescription(permission.UserIDGroupPairs[0].Description)

‎pkg/ingress/model_build_actions.go

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -14,40 +14,38 @@ import (
1414
"unicode"
1515
)
1616

17-
func (b *defaultModelBuilder) buildActions(ctx context.Context, stack core.Stack, ingGroupID GroupID, tgByID map[string]*elbv2model.TargetGroup,
18-
protocol elbv2model.Protocol, ing *networking.Ingress, backend EnhancedBackend) ([]elbv2model.Action, error) {
17+
func (t *defaultModelBuildTask) buildActions(ctx context.Context, protocol elbv2model.Protocol, ing *networking.Ingress, backend EnhancedBackend) ([]elbv2model.Action, error) {
1918
var actions []elbv2model.Action
2019
if protocol == elbv2model.ProtocolHTTPS {
21-
authAction, err := b.buildAuthAction(ctx, ing, backend)
20+
authAction, err := t.buildAuthAction(ctx, ing, backend)
2221
if err != nil {
2322
return nil, err
2423
}
2524
if authAction != nil {
2625
actions = append(actions, *authAction)
2726
}
2827
}
29-
backendAction, err := b.buildBackendAction(ctx, stack, ingGroupID, tgByID, ing, backend.Action)
28+
backendAction, err := t.buildBackendAction(ctx, ing, backend.Action)
3029
if err != nil {
3130
return nil, err
3231
}
3332
actions = append(actions, backendAction)
3433
return actions, nil
3534
}
3635

37-
func (b *defaultModelBuilder) buildBackendAction(ctx context.Context, stack core.Stack, ingGroupID GroupID,
38-
tgByID map[string]*elbv2model.TargetGroup, ing *networking.Ingress, actionCfg Action) (elbv2model.Action, error) {
36+
func (t *defaultModelBuildTask) buildBackendAction(ctx context.Context, ing *networking.Ingress, actionCfg Action) (elbv2model.Action, error) {
3937
switch actionCfg.Type {
4038
case ActionTypeFixedResponse:
41-
return b.buildFixedResponseAction(ctx, actionCfg)
39+
return t.buildFixedResponseAction(ctx, actionCfg)
4240
case ActionTypeRedirect:
43-
return b.buildRedirectAction(ctx, actionCfg)
41+
return t.buildRedirectAction(ctx, actionCfg)
4442
case ActionTypeForward:
45-
return b.buildForwardAction(ctx, stack, ingGroupID, tgByID, ing, actionCfg)
43+
return t.buildForwardAction(ctx, ing, actionCfg)
4644
}
4745
return elbv2model.Action{}, errors.Errorf("unknown action type: %v", actionCfg.Type)
4846
}
4947

50-
func (b *defaultModelBuilder) buildAuthAction(ctx context.Context, ing *networking.Ingress, backend EnhancedBackend) (*elbv2model.Action, error) {
48+
func (t *defaultModelBuildTask) buildAuthAction(ctx context.Context, ing *networking.Ingress, backend EnhancedBackend) (*elbv2model.Action, error) {
5149
// if a single service is used as backend, then it's auth configuration via annotation will take priority than ingress.
5250
svcAndIngAnnotations := ing.Annotations
5351
if backend.Action.Type == ActionTypeForward && len(backend.Action.ForwardConfig.TargetGroups) == 1 && backend.Action.ForwardConfig.TargetGroups[0].ServiceName != nil {
@@ -57,24 +55,25 @@ func (b *defaultModelBuilder) buildAuthAction(ctx context.Context, ing *networki
5755
Name: svcName,
5856
}
5957
svc := &corev1.Service{}
60-
if err := b.k8sClient.Get(ctx, svcKey, svc); err != nil {
58+
if err := t.k8sClient.Get(ctx, svcKey, svc); err != nil {
6159
return nil, err
6260
}
6361
svcAndIngAnnotations = algorithm.MergeStringMap(svc.Annotations, svcAndIngAnnotations)
6462
}
65-
authCfg, err := b.authConfigBuilder.Build(ctx, svcAndIngAnnotations)
63+
64+
authCfg, err := t.authConfigBuilder.Build(ctx, svcAndIngAnnotations)
6665
if err != nil {
6766
return nil, err
6867
}
6968
switch authCfg.Type {
7069
case AuthTypeCognito:
71-
action, err := b.buildAuthenticateCognitoAction(ctx, authCfg)
70+
action, err := t.buildAuthenticateCognitoAction(ctx, authCfg)
7271
if err != nil {
7372
return nil, err
7473
}
7574
return &action, nil
7675
case AuthTypeOIDC:
77-
action, err := b.buildAuthenticateOIDCAction(ctx, authCfg, ing.Namespace)
76+
action, err := t.buildAuthenticateOIDCAction(ctx, authCfg, ing.Namespace)
7877
if err != nil {
7978
return nil, err
8079
}
@@ -84,7 +83,7 @@ func (b *defaultModelBuilder) buildAuthAction(ctx context.Context, ing *networki
8483
}
8584
}
8685

87-
func (b *defaultModelBuilder) buildFixedResponseAction(ctx context.Context, actionCfg Action) (elbv2model.Action, error) {
86+
func (t *defaultModelBuildTask) buildFixedResponseAction(_ context.Context, actionCfg Action) (elbv2model.Action, error) {
8887
if actionCfg.FixedResponseConfig == nil {
8988
return elbv2model.Action{}, errors.New("missing FixedResponseConfig")
9089
}
@@ -98,7 +97,7 @@ func (b *defaultModelBuilder) buildFixedResponseAction(ctx context.Context, acti
9897
}, nil
9998
}
10099

101-
func (b *defaultModelBuilder) buildRedirectAction(ctx context.Context, actionCfg Action) (elbv2model.Action, error) {
100+
func (t *defaultModelBuildTask) buildRedirectAction(_ context.Context, actionCfg Action) (elbv2model.Action, error) {
102101
if actionCfg.RedirectConfig == nil {
103102
return elbv2model.Action{}, errors.New("missing RedirectConfig")
104103
}
@@ -115,11 +114,11 @@ func (b *defaultModelBuilder) buildRedirectAction(ctx context.Context, actionCfg
115114
}, nil
116115
}
117116

118-
func (b *defaultModelBuilder) buildForwardAction(ctx context.Context, stack core.Stack, ingGroupID GroupID,
119-
tgByID map[string]*elbv2model.TargetGroup, ing *networking.Ingress, actionCfg Action) (elbv2model.Action, error) {
117+
func (t *defaultModelBuildTask) buildForwardAction(ctx context.Context, ing *networking.Ingress, actionCfg Action) (elbv2model.Action, error) {
120118
if actionCfg.ForwardConfig == nil {
121119
return elbv2model.Action{}, errors.New("missing ForwardConfig")
122120
}
121+
123122
var targetGroupTuples []elbv2model.TargetGroupTuple
124123
for _, tgt := range actionCfg.ForwardConfig.TargetGroups {
125124
var tgARN core.StringToken
@@ -131,10 +130,10 @@ func (b *defaultModelBuilder) buildForwardAction(ctx context.Context, stack core
131130
Name: awssdk.StringValue(tgt.ServiceName),
132131
}
133132
svc := &corev1.Service{}
134-
if err := b.k8sClient.Get(ctx, svcKey, svc); err != nil {
133+
if err := t.k8sClient.Get(ctx, svcKey, svc); err != nil {
135134
return elbv2model.Action{}, err
136135
}
137-
tg, err := b.buildTargetGroup(ctx, stack, ingGroupID, tgByID, ing, svc, *tgt.ServicePort)
136+
tg, err := t.buildTargetGroup(ctx, ing, svc, *tgt.ServicePort)
138137
if err != nil {
139138
return elbv2model.Action{}, err
140139
}
@@ -162,7 +161,7 @@ func (b *defaultModelBuilder) buildForwardAction(ctx context.Context, stack core
162161
}, nil
163162
}
164163

165-
func (b *defaultModelBuilder) buildAuthenticateCognitoAction(ctx context.Context, authCfg AuthConfig) (elbv2model.Action, error) {
164+
func (t *defaultModelBuildTask) buildAuthenticateCognitoAction(_ context.Context, authCfg AuthConfig) (elbv2model.Action, error) {
166165
if authCfg.IDPConfigCognito == nil {
167166
return elbv2model.Action{}, errors.New("missing IDPConfigCognito")
168167
}
@@ -182,7 +181,7 @@ func (b *defaultModelBuilder) buildAuthenticateCognitoAction(ctx context.Context
182181
}, nil
183182
}
184183

185-
func (b *defaultModelBuilder) buildAuthenticateOIDCAction(ctx context.Context, authCfg AuthConfig, namespace string) (elbv2model.Action, error) {
184+
func (t *defaultModelBuildTask) buildAuthenticateOIDCAction(ctx context.Context, authCfg AuthConfig, namespace string) (elbv2model.Action, error) {
186185
if authCfg.IDPConfigOIDC == nil {
187186
return elbv2model.Action{}, errors.New("missing IDPConfigOIDC")
188187
}
@@ -192,9 +191,10 @@ func (b *defaultModelBuilder) buildAuthenticateOIDCAction(ctx context.Context, a
192191
Name: authCfg.IDPConfigOIDC.SecretName,
193192
}
194193
secret := &corev1.Secret{}
195-
if err := b.k8sClient.Get(ctx, secretKey, secret); err != nil {
194+
if err := t.k8sClient.Get(ctx, secretKey, secret); err != nil {
196195
return elbv2model.Action{}, err
197196
}
197+
198198
clientID := strings.TrimRightFunc(string(secret.Data["clientId"]), unicode.IsSpace)
199199
clientSecret := string(secret.Data["clientSecret"])
200200
return elbv2model.Action{
@@ -215,7 +215,7 @@ func (b *defaultModelBuilder) buildAuthenticateOIDCAction(ctx context.Context, a
215215
}, nil
216216
}
217217

218-
func (b *defaultModelBuilder) build404Action(ctx context.Context) elbv2model.Action {
218+
func (t *defaultModelBuildTask) build404Action(_ context.Context) elbv2model.Action {
219219
return elbv2model.Action{
220220
Type: elbv2model.ActionTypeFixedResponse,
221221
FixedResponseConfig: &elbv2model.FixedResponseActionConfig{

0 commit comments

Comments
 (0)