Skip to content

Commit 4bb3faf

Browse files
mlavaccamlavacca
committed
feat: reviews comments addressed
Signed-off-by: Mattia Lavacca <[email protected]>
1 parent ed3aee3 commit 4bb3faf

File tree

11 files changed

+119
-31
lines changed

11 files changed

+119
-31
lines changed

‎pkg/i2gw/provider.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ type ProviderConstructor func(conf *ProviderConf) Provider
4242
// ProviderConf contains all the configuration required for every concrete
4343
// Provider implementation.
4444
type ProviderConf struct {
45-
Client client.Client
45+
Client client.Client
46+
Options ProviderImplementationSpecificOptions
4647
}
4748

4849
// The Provider interface specifies the required functionality which needs to be
@@ -73,14 +74,15 @@ type ResourceConverter interface {
7374
ToGatewayAPI(resources InputResources) (GatewayResources, field.ErrorList)
7475
}
7576

76-
// HTTPPathMatchOption is an option to customize the ingress implementationSpecific
77+
// ImplementationSpecificHTTPPathMatchOption is an option to customize the ingress implementationSpecific
7778
// match type conversion.
78-
type HTTPPathMatchOption func(*gatewayv1beta1.HTTPPathMatch)
79+
type ImplementationSpecificHTTPPathMatchOption func(*gatewayv1beta1.HTTPPathMatch)
7980

80-
// ImplementationSpecificOptions contains all the pointers to implementation-specific
81-
// customization functions.
82-
type ImplementationSpecificOptions struct {
83-
HTTPPathmatch HTTPPathMatchOption
81+
// ProviderImplementationSpecificOptions contains all the pointers to implementation-specific
82+
// customization functions. Such functions will be called by the common package to customize
83+
// the provider-specific behavior for all the implementation-specific fields of the API.
84+
type ProviderImplementationSpecificOptions struct {
85+
ToImplementationSpecificHTTPPathMatch ImplementationSpecificHTTPPathMatchOption
8486
}
8587

8688
// InputResources contains all Ingress objects, and Provider specific

‎pkg/i2gw/providers/common/converter.go

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import (
3232

3333
// ToGateway converts the received ingresses to i2gw.GatewayResources,
3434
// without taking into consideration any provider specific logic.
35-
func ToGateway(ingresses []networkingv1.Ingress, options i2gw.ImplementationSpecificOptions) (i2gw.GatewayResources, field.ErrorList) {
35+
func ToGateway(ingresses []networkingv1.Ingress, options i2gw.ProviderImplementationSpecificOptions) (i2gw.GatewayResources, field.ErrorList) {
3636
aggregator := ingressAggregator{ruleGroups: map[ruleGroupKey]*ingressRuleGroup{}}
3737

3838
var errs field.ErrorList
@@ -157,7 +157,7 @@ func (a *ingressAggregator) addIngressRule(namespace, name, ingressClass string,
157157
rg.rules = append(rg.rules, ingressRule{rule: rule})
158158
}
159159

160-
func (a *ingressAggregator) toHTTPRoutesAndGateways(options i2gw.ImplementationSpecificOptions) ([]gatewayv1beta1.HTTPRoute, []gatewayv1beta1.Gateway, field.ErrorList) {
160+
func (a *ingressAggregator) toHTTPRoutesAndGateways(options i2gw.ProviderImplementationSpecificOptions) ([]gatewayv1beta1.HTTPRoute, []gatewayv1beta1.Gateway, field.ErrorList) {
161161
var httpRoutes []gatewayv1beta1.HTTPRoute
162162
var errors field.ErrorList
163163
listenersByNamespacedGateway := map[string][]gatewayv1beta1.Listener{}
@@ -269,7 +269,7 @@ func (a *ingressAggregator) toHTTPRoutesAndGateways(options i2gw.ImplementationS
269269
return httpRoutes, gateways, errors
270270
}
271271

272-
func (rg *ingressRuleGroup) toHTTPRoute(options i2gw.ImplementationSpecificOptions) (gatewayv1beta1.HTTPRoute, field.ErrorList) {
272+
func (rg *ingressRuleGroup) toHTTPRoute(options i2gw.ProviderImplementationSpecificOptions) (gatewayv1beta1.HTTPRoute, field.ErrorList) {
273273
pathsByMatchGroup := map[pathMatchKey][]ingressPath{}
274274
var errors field.ErrorList
275275

@@ -305,9 +305,7 @@ func (rg *ingressRuleGroup) toHTTPRoute(options i2gw.ImplementationSpecificOptio
305305
for _, paths := range pathsByMatchGroup {
306306
path := paths[0]
307307
fieldPath := field.NewPath("spec", "rules").Index(path.ruleIdx).Child(path.ruleType).Child("paths").Index(path.pathIdx)
308-
var match *gatewayv1beta1.HTTPRouteMatch
309-
var err *field.Error
310-
match, err = toHTTPRouteMatch(path.path, fieldPath, options.HTTPPathmatch)
308+
match, err := toHTTPRouteMatch(path.path, fieldPath, options.ToImplementationSpecificHTTPPathMatch)
311309
if err != nil {
312310
errors = append(errors, err)
313311
continue
@@ -350,7 +348,7 @@ func getPathMatchKey(ip ingressPath) pathMatchKey {
350348
return pathMatchKey(fmt.Sprintf("%s/%s", pathType, ip.path.Path))
351349
}
352350

353-
func toHTTPRouteMatch(routePath networkingv1.HTTPIngressPath, path *field.Path, implementationSpecificPath i2gw.HTTPPathMatchOption) (*gatewayv1beta1.HTTPRouteMatch, *field.Error) {
351+
func toHTTPRouteMatch(routePath networkingv1.HTTPIngressPath, path *field.Path, toImplementationSpecificPathMatch i2gw.ImplementationSpecificHTTPPathMatchOption) (*gatewayv1beta1.HTTPRouteMatch, *field.Error) {
354352
pmPrefix := gatewayv1beta1.PathMatchPathPrefix
355353
pmExact := gatewayv1beta1.PathMatchExact
356354

@@ -360,9 +358,12 @@ func toHTTPRouteMatch(routePath networkingv1.HTTPIngressPath, path *field.Path,
360358
match.Path.Type = &pmPrefix
361359
case networkingv1.PathTypeExact:
362360
match.Path.Type = &pmExact
361+
// In case the path type is ImplementationSpecific, the path value and type
362+
// will be set by the provider-specific customization function. If such function
363+
// is not given by the provider, an error is returned.
363364
case networkingv1.PathTypeImplementationSpecific:
364-
if implementationSpecificPath != nil {
365-
implementationSpecificPath(match.Path)
365+
if toImplementationSpecificPathMatch != nil {
366+
toImplementationSpecificPathMatch(match.Path)
366367
} else {
367368
return nil, field.Invalid(path.Child("pathType"), routePath.PathType, fmt.Sprintf("unsupported path match type: %s", *routePath.PathType))
368369
}

‎pkg/i2gw/providers/common/converter_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ func Test_ingresses2GatewaysAndHttpRoutes(t *testing.T) {
316316
for _, tc := range testCases {
317317
t.Run(tc.name, func(t *testing.T) {
318318

319-
gatewayResources, errs := ToGateway(tc.ingresses, i2gw.ImplementationSpecificOptions{})
319+
gatewayResources, errs := ToGateway(tc.ingresses, i2gw.ProviderImplementationSpecificOptions{})
320320

321321
if len(gatewayResources.HTTPRoutes) != len(tc.expectedGatewayResources.HTTPRoutes) {
322322
t.Errorf("Expected %d HTTPRoutes, got %d: %+v",

‎pkg/i2gw/providers/ingressnginx/converter.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ func (c *converter) ToGatewayAPI(resources i2gw.InputResources) (i2gw.GatewayRes
4545

4646
// Convert plain ingress resources to gateway resources, ignoring all
4747
// provider-specific features.
48-
gatewayResources, errs := common.ToGateway(resources.Ingresses, i2gw.ImplementationSpecificOptions{})
48+
gatewayResources, errs := common.ToGateway(resources.Ingresses, i2gw.ProviderImplementationSpecificOptions{})
4949
if len(errs) > 0 {
5050
return i2gw.GatewayResources{}, errs
5151
}

‎pkg/i2gw/providers/ingressnginx/converter_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func Test_ToGateway(t *testing.T) {
175175
Namespace: "default",
176176
},
177177
Spec: networkingv1.IngressSpec{
178-
IngressClassName: ptrTo("ingress-kong"),
178+
IngressClassName: ptrTo("ingress-nginx"),
179179
Rules: []networkingv1.IngressRule{{
180180
Host: "test.mydomain.com",
181181
IngressRuleValue: networkingv1.IngressRuleValue{

‎pkg/i2gw/providers/kong/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Ingress Kong Provider
1+
# Kong Provider
22

33
## Annotations supported
44

@@ -26,4 +26,4 @@ The following implementation-specific features are supported:
2626

2727
- The ingress `ImplementationSpecific` match type is properly converted to
2828
- `RegularExpression` HTTPRoute match type when the path has the prefix `/~`.
29-
- `PathPrefix` HTTPRoute match type when there is no prefix `/~`.
29+
- `PathPrefix` HTTPRoute match type when there is no `/~` prefix.

‎pkg/i2gw/providers/kong/converter.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ func (c *converter) ToGatewayAPI(resources i2gw.InputResources) (i2gw.GatewayRes
4747

4848
// Convert plain ingress resources to gateway resources, ignoring all
4949
// provider-specific features.
50-
gatewayResources, errs := common.ToGateway(resources.Ingresses, i2gw.ImplementationSpecificOptions{
51-
HTTPPathmatch: httpPathMatch,
50+
gatewayResources, errs := common.ToGateway(resources.Ingresses, i2gw.ProviderImplementationSpecificOptions{
51+
ToImplementationSpecificHTTPPathMatch: implementationSpecificHttpPathMatch,
5252
})
5353
if len(errs) > 0 {
5454
return i2gw.GatewayResources{}, errs

‎pkg/i2gw/providers/kong/converter_test.go

Lines changed: 88 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import (
3333

3434
func Test_ToGateway(t *testing.T) {
3535
iPrefix := networkingv1.PathTypePrefix
36-
isPathType := networkingv1.PathTypeImplementationSpecific
36+
ImplSpecPathType := networkingv1.PathTypeImplementationSpecific
3737
gPathPrefix := gatewayv1beta1.PathMatchPathPrefix
3838
gPathRegex := gatewayv1beta1.PathMatchRegularExpression
3939

@@ -325,7 +325,7 @@ func Test_ToGateway(t *testing.T) {
325325
expectedErrors: field.ErrorList{},
326326
},
327327
{
328-
name: "ImplementationSpecific HTTPRouteMatching",
328+
name: "ImplementationSpecific HTTPRouteMatching with regex",
329329
ingresses: []networkingv1.Ingress{
330330
{
331331
ObjectMeta: metav1.ObjectMeta{
@@ -340,7 +340,7 @@ func Test_ToGateway(t *testing.T) {
340340
HTTP: &networkingv1.HTTPIngressRuleValue{
341341
Paths: []networkingv1.HTTPIngressPath{{
342342
Path: "/~/echo/**/test",
343-
PathType: &isPathType,
343+
PathType: &ImplSpecPathType,
344344
Backend: networkingv1.IngressBackend{
345345
Service: &networkingv1.IngressServiceBackend{
346346
Name: "test",
@@ -409,6 +409,91 @@ func Test_ToGateway(t *testing.T) {
409409
},
410410
expectedErrors: field.ErrorList{},
411411
},
412+
{
413+
name: "ImplementationSpecific HTTPRouteMatching without regex",
414+
ingresses: []networkingv1.Ingress{
415+
{
416+
ObjectMeta: metav1.ObjectMeta{
417+
Name: "implementation-specific-no-regex",
418+
Namespace: "default",
419+
},
420+
Spec: networkingv1.IngressSpec{
421+
IngressClassName: ptrTo("ingress-kong"),
422+
Rules: []networkingv1.IngressRule{{
423+
Host: "test.mydomain.com",
424+
IngressRuleValue: networkingv1.IngressRuleValue{
425+
HTTP: &networkingv1.HTTPIngressRuleValue{
426+
Paths: []networkingv1.HTTPIngressPath{{
427+
Path: "/echo",
428+
PathType: &ImplSpecPathType,
429+
Backend: networkingv1.IngressBackend{
430+
Service: &networkingv1.IngressServiceBackend{
431+
Name: "test",
432+
Port: networkingv1.ServiceBackendPort{
433+
Number: 80,
434+
},
435+
},
436+
},
437+
}},
438+
},
439+
},
440+
}},
441+
},
442+
},
443+
},
444+
expectedGatewayResources: i2gw.GatewayResources{
445+
Gateways: map[types.NamespacedName]gatewayv1beta1.Gateway{
446+
{Namespace: "default", Name: "ingress-kong"}: {
447+
ObjectMeta: metav1.ObjectMeta{Name: "ingress-kong", Namespace: "default"},
448+
Spec: gatewayv1beta1.GatewaySpec{
449+
GatewayClassName: "ingress-kong",
450+
Listeners: []gatewayv1beta1.Listener{{
451+
Name: "test-mydomain-com-http",
452+
Port: 80,
453+
Protocol: gatewayv1beta1.HTTPProtocolType,
454+
Hostname: ptrTo(gatewayv1beta1.Hostname("test.mydomain.com")),
455+
}},
456+
},
457+
},
458+
},
459+
HTTPRoutes: map[types.NamespacedName]gatewayv1beta1.HTTPRoute{
460+
{Namespace: "default", Name: "implementation-specific-no-regex-test-mydomain-com"}: {
461+
ObjectMeta: metav1.ObjectMeta{Name: "implementation-specific-no-regex-test-mydomain-com", Namespace: "default"},
462+
Spec: gatewayv1beta1.HTTPRouteSpec{
463+
CommonRouteSpec: gatewayv1beta1.CommonRouteSpec{
464+
ParentRefs: []gatewayv1beta1.ParentReference{{
465+
Name: "ingress-kong",
466+
}},
467+
},
468+
Hostnames: []gatewayv1beta1.Hostname{"test.mydomain.com"},
469+
Rules: []gatewayv1beta1.HTTPRouteRule{
470+
{
471+
Matches: []gatewayv1beta1.HTTPRouteMatch{
472+
{
473+
Path: &gatewayv1beta1.HTTPPathMatch{
474+
Type: &gPathPrefix,
475+
Value: ptrTo("/echo"),
476+
},
477+
},
478+
},
479+
BackendRefs: []gatewayv1beta1.HTTPBackendRef{
480+
{
481+
BackendRef: gatewayv1beta1.BackendRef{
482+
BackendObjectReference: gatewayv1beta1.BackendObjectReference{
483+
Name: "test",
484+
Port: ptrTo(gatewayv1beta1.PortNumber(80)),
485+
},
486+
},
487+
},
488+
},
489+
},
490+
},
491+
},
492+
},
493+
},
494+
},
495+
expectedErrors: field.ErrorList{},
496+
},
412497
}
413498

414499
for _, tc := range testCases {

‎pkg/i2gw/providers/kong/header_matching_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,8 +246,8 @@ func TestHeaderMatchingFeature(t *testing.T) {
246246

247247
for _, tc := range testCases {
248248
t.Run(tc.name, func(t *testing.T) {
249-
gatewayResources, errs := common.ToGateway(tc.inputResources.Ingresses, i2gw.ImplementationSpecificOptions{
250-
HTTPPathmatch: httpPathMatch,
249+
gatewayResources, errs := common.ToGateway(tc.inputResources.Ingresses, i2gw.ProviderImplementationSpecificOptions{
250+
ToImplementationSpecificHTTPPathMatch: implementationSpecificHttpPathMatch,
251251
})
252252
if len(errs) != 0 {
253253
t.Errorf("Expected no errors, got %d: %+v", len(errs), errs)

‎pkg/i2gw/providers/kong/implementation_specific.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
2424
)
2525

26-
func httpPathMatch(path *gatewayv1beta1.HTTPPathMatch) {
26+
func implementationSpecificHTTPPathMatch(path *gatewayv1beta1.HTTPPathMatch) {
2727
pmPrefix := gatewayv1beta1.PathMatchPathPrefix
2828
pmRegex := gatewayv1beta1.PathMatchRegularExpression
2929
if strings.HasPrefix(*path.Value, "/~") {

0 commit comments

Comments
 (0)