Skip to content

Commit 43ca155

Browse files
shraddhabangshuqz
shraddhabang
authored and
shuqz
committed
[feat:gw api] Add generic loader for Rules CRD
1 parent 1b09bfa commit 43ca155

19 files changed

+512
-139
lines changed

‎pkg/gateway/model/model_build_listener.go‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -225,19 +225,26 @@ func (l listenerBuilderImpl) buildListenerRules(stack core.Stack, ls *elbv2model
225225
if shouldProvisionActions(targetGroupTuples) {
226226
actions = buildL7ListenerForwardActions(targetGroupTuples, nil)
227227
}
228-
229-
// configure actions based on filters
230-
switch route.GetRouteKind() {
231-
case routeutils.HTTPRouteKind:
232-
httpRule := rule.GetRawRouteRule().(*gwv1.HTTPRouteRule)
233-
if len(httpRule.Filters) > 0 {
234-
finalActions, err := routeutils.BuildHttpRuleActionsBasedOnFilter(httpRule.Filters)
235-
if err != nil {
236-
return err
228+
// Temp log
229+
if rule.GetListenerRuleConfig() != nil {
230+
l.logger.V(1).Info("got ListenerRuleCfg ", "route", route.GetRouteNamespacedName(), "ListenerRuleCfg", k8s.NamespacedName(rule.GetListenerRuleConfig()))
231+
}
232+
// Skipping building actions from filter updates for now since having a ListenerRuleConfig means extesnionRef has been added and for now we dont want to take any actions.
233+
// TODO :Consume ListenerRuleConfig properly to build actions.
234+
if rule.GetListenerRuleConfig() == nil {
235+
// configure actions based on filters
236+
switch route.GetRouteKind() {
237+
case routeutils.HTTPRouteKind:
238+
httpRule := rule.GetRawRouteRule().(*gwv1.HTTPRouteRule)
239+
if len(httpRule.Filters) > 0 {
240+
finalActions, err := routeutils.BuildHttpRuleActionsBasedOnFilter(httpRule.Filters)
241+
if err != nil {
242+
return err
243+
}
244+
actions = finalActions
237245
}
238-
actions = finalActions
246+
// TODO: add case for GRPC
239247
}
240-
// TODO: add case for GRPC
241248
}
242249

243250
if len(actions) == 0 {

‎pkg/gateway/routeutils/backend.go‎

Lines changed: 100 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"k8s.io/apimachinery/pkg/types"
99
elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1"
1010
"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway"
11+
"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/constants"
1112
"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
1213
"sigs.k8s.io/controller-runtime/pkg/client"
1314
gwv1 "sigs.k8s.io/gateway-api/apis/v1"
@@ -35,46 +36,65 @@ type Backend struct {
3536
}
3637

3738
type attachedRuleAccumulator[RuleType any] interface {
38-
accumulateRules(ctx context.Context, k8sClient client.Client, route preLoadRouteDescriptor, rules []RuleType, backendRefIterator func(RuleType) []gwv1.BackendRef, ruleConverter func(*RuleType, []Backend) RouteRule) ([]RouteRule, []routeLoadError)
39+
accumulateRules(ctx context.Context, k8sClient client.Client, route preLoadRouteDescriptor, rules []RuleType, backendRefIterator func(RuleType) []gwv1.BackendRef, listenerRuleConfigRefs func(RuleType) []gwv1.LocalObjectReference, ruleConverter func(*RuleType, []Backend, *elbv2gw.ListenerRuleConfiguration) RouteRule) ([]RouteRule, []routeLoadError)
3940
}
4041

4142
type attachedRuleAccumulatorImpl[RuleType any] struct {
42-
backendLoader func(ctx context.Context, k8sClient client.Client, typeSpecificBackend interface{}, backendRef gwv1.BackendRef, routeIdentifier types.NamespacedName, routeKind RouteKind) (*Backend, error, error)
43+
backendLoader func(ctx context.Context, k8sClient client.Client, typeSpecificBackend interface{}, backendRef gwv1.BackendRef, routeIdentifier types.NamespacedName, routeKind RouteKind) (*Backend, error, error)
44+
listenerRuleConfigLoader func(ctx context.Context, k8sClient client.Client, routeIdentifier types.NamespacedName, routeKind RouteKind, listenerRuleConfigRefs []gwv1.LocalObjectReference) (*elbv2gw.ListenerRuleConfiguration, error, error)
4345
}
4446

45-
func newAttachedRuleAccumulator[RuleType any](backendLoader func(ctx context.Context, k8sClient client.Client, typeSpecificBackend interface{}, backendRef gwv1.BackendRef, routeIdentifier types.NamespacedName, routeKind RouteKind) (*Backend, error, error)) attachedRuleAccumulator[RuleType] {
47+
func newAttachedRuleAccumulator[RuleType any](backendLoader func(ctx context.Context, k8sClient client.Client, typeSpecificBackend interface{}, backendRef gwv1.BackendRef, routeIdentifier types.NamespacedName, routeKind RouteKind) (*Backend, error, error),
48+
listenerRuleConfigLoader func(ctx context.Context, k8sClient client.Client, routeIdentifier types.NamespacedName, routeKind RouteKind, listenerRuleConfigRefs []gwv1.LocalObjectReference) (*elbv2gw.ListenerRuleConfiguration, error, error)) attachedRuleAccumulator[RuleType] {
4649
return &attachedRuleAccumulatorImpl[RuleType]{
47-
backendLoader: backendLoader,
50+
backendLoader: backendLoader,
51+
listenerRuleConfigLoader: listenerRuleConfigLoader,
4852
}
4953
}
5054

51-
func (ara *attachedRuleAccumulatorImpl[RuleType]) accumulateRules(ctx context.Context, k8sClient client.Client, route preLoadRouteDescriptor, rules []RuleType, backendRefIterator func(RuleType) []gwv1.BackendRef, ruleConverter func(*RuleType, []Backend) RouteRule) ([]RouteRule, []routeLoadError) {
55+
func (ara *attachedRuleAccumulatorImpl[RuleType]) accumulateRules(ctx context.Context, k8sClient client.Client, route preLoadRouteDescriptor, rules []RuleType, backendRefIterator func(RuleType) []gwv1.BackendRef, listenerRuleConfigRefs func(RuleType) []gwv1.LocalObjectReference, ruleConverter func(*RuleType, []Backend, *elbv2gw.ListenerRuleConfiguration) RouteRule) ([]RouteRule, []routeLoadError) {
5256
convertedRules := make([]RouteRule, 0)
5357
allErrors := make([]routeLoadError, 0)
5458
for _, rule := range rules {
5559
convertedBackends := make([]Backend, 0)
56-
for _, backend := range backendRefIterator(rule) {
57-
convertedBackend, warningErr, fatalErr := ara.backendLoader(ctx, k8sClient, backend, backend, route.GetRouteNamespacedName(), route.GetRouteKind())
58-
if warningErr != nil {
59-
allErrors = append(allErrors, routeLoadError{
60-
Err: warningErr,
61-
})
62-
}
60+
listenerRuleConfig, lrcWarningErr, lrcfatalErr := ara.listenerRuleConfigLoader(ctx, k8sClient, route.GetRouteNamespacedName(), route.GetRouteKind(), listenerRuleConfigRefs(rule))
61+
if lrcWarningErr != nil {
62+
allErrors = append(allErrors, routeLoadError{
63+
Err: lrcWarningErr,
64+
})
65+
}
66+
// usually happens due to K8s Api outage
67+
if lrcfatalErr != nil {
68+
allErrors = append(allErrors, routeLoadError{
69+
Err: lrcfatalErr,
70+
Fatal: true,
71+
})
72+
return nil, allErrors
73+
}
74+
// If ListenerRuleConfig is loaded properly without any warning errors, then only load backends, else it should be treated as no valid backend to send with fixed 503 response
75+
if lrcWarningErr == nil {
76+
for _, backend := range backendRefIterator(rule) {
77+
convertedBackend, warningErr, fatalErr := ara.backendLoader(ctx, k8sClient, backend, backend, route.GetRouteNamespacedName(), route.GetRouteKind())
78+
if warningErr != nil {
79+
allErrors = append(allErrors, routeLoadError{
80+
Err: warningErr,
81+
})
82+
}
6383

64-
if fatalErr != nil {
65-
allErrors = append(allErrors, routeLoadError{
66-
Err: fatalErr,
67-
Fatal: true,
68-
})
69-
return nil, allErrors
70-
}
84+
if fatalErr != nil {
85+
allErrors = append(allErrors, routeLoadError{
86+
Err: fatalErr,
87+
Fatal: true,
88+
})
89+
return nil, allErrors
90+
}
7191

72-
if convertedBackend != nil {
73-
convertedBackends = append(convertedBackends, *convertedBackend)
92+
if convertedBackend != nil {
93+
convertedBackends = append(convertedBackends, *convertedBackend)
94+
}
7495
}
7596
}
76-
77-
convertedRules = append(convertedRules, ruleConverter(&rule, convertedBackends))
97+
convertedRules = append(convertedRules, ruleConverter(&rule, convertedBackends, listenerRuleConfig))
7898
}
7999
return convertedRules, allErrors
80100
}
@@ -290,3 +310,60 @@ func referenceGrantCheck(ctx context.Context, k8sClient client.Client, svcIdenti
290310

291311
return false, nil
292312
}
313+
314+
func listenerRuleConfigLoader(ctx context.Context, k8sClient client.Client, routeIdentifier types.NamespacedName, routeKind RouteKind, listenerRuleConfigsRefs []gwv1.LocalObjectReference) (*elbv2gw.ListenerRuleConfiguration, error, error) {
315+
if len(listenerRuleConfigsRefs) == 0 {
316+
return nil, nil, nil
317+
}
318+
// This is warning error so that the reconcile cycle does not stop.
319+
if len(listenerRuleConfigsRefs) > 1 {
320+
initialErrorMessage := "Only one listener rule config can be referenced per route rule, found multiple"
321+
wrappedGatewayErrorMessage := generateInvalidMessageWithRouteDetails(initialErrorMessage, routeKind, routeIdentifier)
322+
return nil, wrapError(errors.Errorf("%s", initialErrorMessage), gwv1.GatewayReasonListenersNotValid, gwv1.RouteReasonIncompatibleFilters, &wrappedGatewayErrorMessage, nil), nil
323+
}
324+
listenerRuleCfgId := types.NamespacedName{
325+
Namespace: routeIdentifier.Namespace,
326+
Name: string(listenerRuleConfigsRefs[0].Name),
327+
}
328+
listenerRuleCfg := &elbv2gw.ListenerRuleConfiguration{}
329+
err := k8sClient.Get(ctx, listenerRuleCfgId, listenerRuleCfg)
330+
if err != nil {
331+
convertToNotFoundError := client.IgnoreNotFound(err)
332+
333+
if convertToNotFoundError == nil {
334+
// ListenerRuleConfig not found, post an updated status.
335+
initialErrorMessage := fmt.Sprintf("ListenerRuleConfiguration [%v] not found)", listenerRuleCfgId.String())
336+
wrappedGatewayErrorMessage := generateInvalidMessageWithRouteDetails(initialErrorMessage, routeKind, routeIdentifier)
337+
return nil, wrapError(errors.Errorf("%s", initialErrorMessage), gwv1.GatewayReasonListenersNotValid, gwv1.RouteReasonIncompatibleFilters, &wrappedGatewayErrorMessage, nil), nil
338+
}
339+
340+
return nil, nil, errors.Wrapf(err, "Unable to load listener rule config [%v] for route [%v]", listenerRuleCfgId.String(), routeIdentifier.String())
341+
}
342+
return listenerRuleCfg, nil, nil
343+
}
344+
345+
// getListenerRuleConfigForRuleGeneric is a generic helper that extracts ListenerRuleConfiguration
346+
// references from ExtensionRef filters in route rules
347+
func getListenerRuleConfigForRuleGeneric[FilterType any](
348+
filters []FilterType,
349+
isExtensionRefType func(filter FilterType) bool,
350+
getExtensionRef func(filter FilterType) *gwv1.LocalObjectReference,
351+
) []gwv1.LocalObjectReference {
352+
listenerRuleConfigsRefs := make([]gwv1.LocalObjectReference, 0)
353+
for _, filter := range filters {
354+
if !isExtensionRefType(filter) {
355+
continue
356+
}
357+
extRef := getExtensionRef(filter)
358+
if extRef != nil &&
359+
string(extRef.Group) == constants.ControllerCRDGroupVersion &&
360+
string(extRef.Kind) == constants.ListenerRuleConfiguration {
361+
listenerRuleConfigsRefs = append(listenerRuleConfigsRefs, gwv1.LocalObjectReference{
362+
Group: constants.ControllerCRDGroupVersion,
363+
Kind: constants.ListenerRuleConfiguration,
364+
Name: extRef.Name,
365+
})
366+
}
367+
}
368+
return listenerRuleConfigsRefs
369+
}

‎pkg/gateway/routeutils/backend_test.go‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1010
"k8s.io/apimachinery/pkg/types"
1111
elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1"
12+
"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/constants"
1213
"sigs.k8s.io/aws-load-balancer-controller/pkg/testutils"
1314
gwv1 "sigs.k8s.io/gateway-api/apis/v1"
1415
gwbeta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
@@ -773,3 +774,138 @@ func Test_referenceGrantCheck(t *testing.T) {
773774
})
774775
}
775776
}
777+
778+
func Test_listenerRuleConfigLoader(t *testing.T) {
779+
testCases := []struct {
780+
name string
781+
listenerRuleConfigs []elbv2gw.ListenerRuleConfiguration
782+
listenerRuleConfigsRefs []gwv1.LocalObjectReference
783+
routeIdentifier types.NamespacedName
784+
routeKind RouteKind
785+
expectWarning bool
786+
expectFatal bool
787+
expectedConfig *elbv2gw.ListenerRuleConfiguration
788+
}{
789+
{
790+
name: "no references - should return nil",
791+
listenerRuleConfigsRefs: []gwv1.LocalObjectReference{},
792+
routeIdentifier: types.NamespacedName{
793+
Namespace: "test-ns",
794+
Name: "test-route",
795+
},
796+
routeKind: HTTPRouteKind,
797+
expectedConfig: nil,
798+
},
799+
{
800+
name: "single valid reference - should return config",
801+
listenerRuleConfigs: []elbv2gw.ListenerRuleConfiguration{
802+
{
803+
ObjectMeta: metav1.ObjectMeta{
804+
Name: "test-config",
805+
Namespace: "test-ns",
806+
},
807+
},
808+
},
809+
listenerRuleConfigsRefs: []gwv1.LocalObjectReference{
810+
{
811+
Group: constants.ControllerCRDGroupVersion,
812+
Kind: constants.ListenerRuleConfiguration,
813+
Name: "test-config",
814+
},
815+
},
816+
routeIdentifier: types.NamespacedName{
817+
Namespace: "test-ns",
818+
Name: "test-route",
819+
},
820+
routeKind: HTTPRouteKind,
821+
expectedConfig: &elbv2gw.ListenerRuleConfiguration{
822+
ObjectMeta: metav1.ObjectMeta{
823+
Name: "test-config",
824+
Namespace: "test-ns",
825+
},
826+
},
827+
},
828+
{
829+
name: "multiple references - should return warning error",
830+
listenerRuleConfigsRefs: []gwv1.LocalObjectReference{
831+
{
832+
Group: constants.ControllerCRDGroupVersion,
833+
Kind: constants.ListenerRuleConfiguration,
834+
Name: "config-1",
835+
},
836+
{
837+
Group: constants.ControllerCRDGroupVersion,
838+
Kind: constants.ListenerRuleConfiguration,
839+
Name: "config-2",
840+
},
841+
},
842+
routeIdentifier: types.NamespacedName{
843+
Namespace: "test-ns",
844+
Name: "test-route",
845+
},
846+
routeKind: HTTPRouteKind,
847+
expectWarning: true,
848+
},
849+
{
850+
name: "config not found - should return warning error",
851+
listenerRuleConfigsRefs: []gwv1.LocalObjectReference{
852+
{
853+
Group: constants.ControllerCRDGroupVersion,
854+
Kind: constants.ListenerRuleConfiguration,
855+
Name: "non-existent-config",
856+
},
857+
},
858+
routeIdentifier: types.NamespacedName{
859+
Namespace: "test-ns",
860+
Name: "test-route",
861+
},
862+
routeKind: HTTPRouteKind,
863+
expectWarning: true,
864+
},
865+
}
866+
867+
for _, tc := range testCases {
868+
t.Run(tc.name, func(t *testing.T) {
869+
k8sClient := testutils.GenerateTestClient()
870+
871+
// Create any listener rule configurations needed for the test
872+
for _, config := range tc.listenerRuleConfigs {
873+
err := k8sClient.Create(context.Background(), &config)
874+
assert.NoError(t, err)
875+
}
876+
877+
// Call the function under test
878+
result, warningErr, fatalErr := listenerRuleConfigLoader(
879+
context.Background(),
880+
k8sClient,
881+
tc.routeIdentifier,
882+
tc.routeKind,
883+
tc.listenerRuleConfigsRefs,
884+
)
885+
886+
// Assert error expectations
887+
if tc.expectWarning {
888+
assert.Error(t, warningErr)
889+
assert.NoError(t, fatalErr)
890+
} else if tc.expectFatal {
891+
assert.Error(t, fatalErr)
892+
assert.NoError(t, warningErr)
893+
} else {
894+
assert.NoError(t, warningErr)
895+
assert.NoError(t, fatalErr)
896+
}
897+
898+
// Assert result expectations
899+
if tc.expectedConfig == nil {
900+
assert.Nil(t, result)
901+
} else {
902+
assert.NotNil(t, result)
903+
// Reset resource version from the create call
904+
if result != nil {
905+
result.ResourceVersion = ""
906+
}
907+
assert.Equal(t, tc.expectedConfig, result)
908+
}
909+
})
910+
}
911+
}

‎pkg/gateway/routeutils/descriptor.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ type routeMetadataDescriptor interface {
1818
GetParentRefs() []gwv1.ParentReference
1919
GetRawRoute() interface{}
2020
GetBackendRefs() []gwv1.BackendRef
21-
GetListenerRuleConfigs() []gwv1.LocalObjectReference
21+
GetRouteListenerRuleConfigRefs() []gwv1.LocalObjectReference
2222
GetRouteGeneration() int64
2323
GetRouteCreateTimestamp() time.Time
2424
}

0 commit comments

Comments
 (0)