feat: reviews comments addressed

Signed-off-by: Mattia Lavacca <[email protected]>

pkg/i2gw/provider.go

@@ -42,7 +42,8 @@ type ProviderConstructor func(conf *ProviderConf) Provider
 // ProviderConf contains all the configuration required for every concrete
 // Provider implementation.
 type ProviderConf struct {
-	Client client.Client
+	Client  client.Client
+	Options ProviderImplementationSpecificOptions
 }
 
 // The Provider interface specifies the required functionality which needs to be
@@ -73,14 +74,15 @@ type ResourceConverter interface {
 	ToGatewayAPI(resources InputResources) (GatewayResources, field.ErrorList)
 }
 
-// HTTPPathMatchOption is an option to customize the ingress implementationSpecific
+// ImplementationSpecificHTTPPathMatchOption is an option to customize the ingress implementationSpecific
 // match type conversion.
-type HTTPPathMatchOption func(*gatewayv1beta1.HTTPPathMatch)
+type ImplementationSpecificHTTPPathMatchOption func(*gatewayv1beta1.HTTPPathMatch)
 
-// ImplementationSpecificOptions contains all the pointers to implementation-specific
-// customization functions.
-type ImplementationSpecificOptions struct {
-	HTTPPathmatch HTTPPathMatchOption
+// ProviderImplementationSpecificOptions contains all the pointers to implementation-specific
+// customization functions. Such functions will be called by the common package to customize
+// the provider-specific behavior for all the implementation-specific fields of the API.
+type ProviderImplementationSpecificOptions struct {
+	ToImplementationSpecificHTTPPathMatch ImplementationSpecificHTTPPathMatchOption
 }
 
 // InputResources contains all Ingress objects, and Provider specific

pkg/i2gw/providers/common/converter.go

@@ -32,7 +32,7 @@ import (
 
 // ToGateway converts the received ingresses to i2gw.GatewayResources,
 // without taking into consideration any provider specific logic.
-func ToGateway(ingresses []networkingv1.Ingress, options i2gw.ImplementationSpecificOptions) (i2gw.GatewayResources, field.ErrorList) {
+func ToGateway(ingresses []networkingv1.Ingress, options i2gw.ProviderImplementationSpecificOptions) (i2gw.GatewayResources, field.ErrorList) {
 	aggregator := ingressAggregator{ruleGroups: map[ruleGroupKey]*ingressRuleGroup{}}
 
 	var errs field.ErrorList
@@ -157,7 +157,7 @@ func (a *ingressAggregator) addIngressRule(namespace, name, ingressClass string,
 	rg.rules = append(rg.rules, ingressRule{rule: rule})
 }
 
-func (a *ingressAggregator) toHTTPRoutesAndGateways(options i2gw.ImplementationSpecificOptions) ([]gatewayv1beta1.HTTPRoute, []gatewayv1beta1.Gateway, field.ErrorList) {
+func (a *ingressAggregator) toHTTPRoutesAndGateways(options i2gw.ProviderImplementationSpecificOptions) ([]gatewayv1beta1.HTTPRoute, []gatewayv1beta1.Gateway, field.ErrorList) {
 	var httpRoutes []gatewayv1beta1.HTTPRoute
 	var errors field.ErrorList
 	listenersByNamespacedGateway := map[string][]gatewayv1beta1.Listener{}
@@ -269,7 +269,7 @@ func (a *ingressAggregator) toHTTPRoutesAndGateways(options i2gw.ImplementationS
 	return httpRoutes, gateways, errors
 }
 
-func (rg *ingressRuleGroup) toHTTPRoute(options i2gw.ImplementationSpecificOptions) (gatewayv1beta1.HTTPRoute, field.ErrorList) {
+func (rg *ingressRuleGroup) toHTTPRoute(options i2gw.ProviderImplementationSpecificOptions) (gatewayv1beta1.HTTPRoute, field.ErrorList) {
 	pathsByMatchGroup := map[pathMatchKey][]ingressPath{}
 	var errors field.ErrorList
 
@@ -305,9 +305,7 @@ func (rg *ingressRuleGroup) toHTTPRoute(options i2gw.ImplementationSpecificOptio
 	for _, paths := range pathsByMatchGroup {
 		path := paths[0]
 		fieldPath := field.NewPath("spec", "rules").Index(path.ruleIdx).Child(path.ruleType).Child("paths").Index(path.pathIdx)
-		var match *gatewayv1beta1.HTTPRouteMatch
-		var err *field.Error
-		match, err = toHTTPRouteMatch(path.path, fieldPath, options.HTTPPathmatch)
+		match, err := toHTTPRouteMatch(path.path, fieldPath, options.ToImplementationSpecificHTTPPathMatch)
 		if err != nil {
 			errors = append(errors, err)
 			continue
@@ -350,7 +348,7 @@ func getPathMatchKey(ip ingressPath) pathMatchKey {
 	return pathMatchKey(fmt.Sprintf("%s/%s", pathType, ip.path.Path))
 }
 
-func toHTTPRouteMatch(routePath networkingv1.HTTPIngressPath, path *field.Path, implementationSpecificPath i2gw.HTTPPathMatchOption) (*gatewayv1beta1.HTTPRouteMatch, *field.Error) {
+func toHTTPRouteMatch(routePath networkingv1.HTTPIngressPath, path *field.Path, toImplementationSpecificPathMatch i2gw.ImplementationSpecificHTTPPathMatchOption) (*gatewayv1beta1.HTTPRouteMatch, *field.Error) {
 	pmPrefix := gatewayv1beta1.PathMatchPathPrefix
 	pmExact := gatewayv1beta1.PathMatchExact
 
@@ -360,9 +358,12 @@ func toHTTPRouteMatch(routePath networkingv1.HTTPIngressPath, path *field.Path,
 		match.Path.Type = &pmPrefix
 	case networkingv1.PathTypeExact:
 		match.Path.Type = &pmExact
+	// In case the path type is ImplementationSpecific, the path value and type
+	// will be set by the provider-specific customization function. If such function
+	// is not given by the provider, an error is returned.
 	case networkingv1.PathTypeImplementationSpecific:
-		if implementationSpecificPath != nil {
-			implementationSpecificPath(match.Path)
+		if toImplementationSpecificPathMatch != nil {
+			toImplementationSpecificPathMatch(match.Path)
 		} else {
 			return nil, field.Invalid(path.Child("pathType"), routePath.PathType, fmt.Sprintf("unsupported path match type: %s", *routePath.PathType))
 		}

pkg/i2gw/providers/common/converter_test.go

@@ -316,7 +316,7 @@ func Test_ingresses2GatewaysAndHttpRoutes(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 
-			gatewayResources, errs := ToGateway(tc.ingresses, i2gw.ImplementationSpecificOptions{})
+			gatewayResources, errs := ToGateway(tc.ingresses, i2gw.ProviderImplementationSpecificOptions{})
 
 			if len(gatewayResources.HTTPRoutes) != len(tc.expectedGatewayResources.HTTPRoutes) {
 				t.Errorf("Expected %d HTTPRoutes, got %d: %+v",

pkg/i2gw/providers/ingressnginx/converter.go

@@ -45,7 +45,7 @@ func (c *converter) ToGatewayAPI(resources i2gw.InputResources) (i2gw.GatewayRes
 
 	// Convert plain ingress resources to gateway resources, ignoring all
 	// provider-specific features.
-	gatewayResources, errs := common.ToGateway(resources.Ingresses, i2gw.ImplementationSpecificOptions{})
+	gatewayResources, errs := common.ToGateway(resources.Ingresses, i2gw.ProviderImplementationSpecificOptions{})
 	if len(errs) > 0 {
 		return i2gw.GatewayResources{}, errs
 	}

pkg/i2gw/providers/ingressnginx/converter_test.go

@@ -175,7 +175,7 @@ func Test_ToGateway(t *testing.T) {
 						Namespace: "default",
 					},
 					Spec: networkingv1.IngressSpec{
-						IngressClassName: ptrTo("ingress-kong"),
+						IngressClassName: ptrTo("ingress-nginx"),
 						Rules: []networkingv1.IngressRule{{
 							Host: "test.mydomain.com",
 							IngressRuleValue: networkingv1.IngressRuleValue{

pkg/i2gw/providers/kong/README.md

@@ -1,4 +1,4 @@
-# Ingress Kong Provider
+# Kong Provider
 
 ## Annotations supported
 
@@ -11,7 +11,7 @@ Current supported annotations:
   be specified by separating values with commas. Example: `konghq.com/methods: "POST,GET"`.
 - `konghq.com/headers.*`: If specified, the values of this annotation are used to
   perform header matching on the associated ingress rules. The header name is specified
-  in the annotation key after `.`, and the annotations value can contain multiple
+  in the annotation key after `.`, and the annotation value can contain multiple
   header values separated by commas. All the header values for a specific header
   name are intended to be ORed. Example: `konghq.com/headers.x-routing: "alpha,bravo"`.
 - `konghq.com/plugins`: If specified, the values of this annotation are used to
@@ -26,4 +26,4 @@ The following implementation-specific features are supported:
 
 - The ingress `ImplementationSpecific` match type is properly converted to
   - `RegularExpression` HTTPRoute match type when the path has the prefix `/~`.
-  - `PathPrefix` HTTPRoute match type when there is no prefix `/~`.
+  - `PathPrefix` HTTPRoute match type when there is no `/~` prefix.

pkg/i2gw/providers/kong/converter.go

@@ -47,8 +47,8 @@ func (c *converter) ToGatewayAPI(resources i2gw.InputResources) (i2gw.GatewayRes
 
 	// Convert plain ingress resources to gateway resources, ignoring all
 	// provider-specific features.
-	gatewayResources, errs := common.ToGateway(resources.Ingresses, i2gw.ImplementationSpecificOptions{
-		HTTPPathmatch: httpPathMatch,
+	gatewayResources, errs := common.ToGateway(resources.Ingresses, i2gw.ProviderImplementationSpecificOptions{
+		ToImplementationSpecificHTTPPathMatch: implementationSpecificHTTPPathMatch,
 	})
 	if len(errs) > 0 {
 		return i2gw.GatewayResources{}, errs

pkg/i2gw/providers/kong/converter_test.go

@@ -33,7 +33,7 @@ import (
 
 func Test_ToGateway(t *testing.T) {
 	iPrefix := networkingv1.PathTypePrefix
-	isPathType := networkingv1.PathTypeImplementationSpecific
+	ImplSpecPathType := networkingv1.PathTypeImplementationSpecific
 	gPathPrefix := gatewayv1beta1.PathMatchPathPrefix
 	gPathRegex := gatewayv1beta1.PathMatchRegularExpression
 
@@ -325,7 +325,7 @@ func Test_ToGateway(t *testing.T) {
 			expectedErrors: field.ErrorList{},
 		},
 		{
-			name: "ImplementationSpecific HTTPRouteMatching",
+			name: "ImplementationSpecific HTTPRouteMatching with regex",
 			ingresses: []networkingv1.Ingress{
 				{
 					ObjectMeta: metav1.ObjectMeta{
@@ -340,7 +340,7 @@ func Test_ToGateway(t *testing.T) {
 								HTTP: &networkingv1.HTTPIngressRuleValue{
 									Paths: []networkingv1.HTTPIngressPath{{
 										Path:     "/~/echo/**/test",
-										PathType: &isPathType,
+										PathType: &ImplSpecPathType,
 										Backend: networkingv1.IngressBackend{
 											Service: &networkingv1.IngressServiceBackend{
 												Name: "test",
@@ -409,6 +409,91 @@ func Test_ToGateway(t *testing.T) {
 			},
 			expectedErrors: field.ErrorList{},
 		},
+		{
+			name: "ImplementationSpecific HTTPRouteMatching without regex",
+			ingresses: []networkingv1.Ingress{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "implementation-specific-no-regex",
+						Namespace: "default",
+					},
+					Spec: networkingv1.IngressSpec{
+						IngressClassName: ptrTo("ingress-kong"),
+						Rules: []networkingv1.IngressRule{{
+							Host: "test.mydomain.com",
+							IngressRuleValue: networkingv1.IngressRuleValue{
+								HTTP: &networkingv1.HTTPIngressRuleValue{
+									Paths: []networkingv1.HTTPIngressPath{{
+										Path:     "/echo",
+										PathType: &ImplSpecPathType,
+										Backend: networkingv1.IngressBackend{
+											Service: &networkingv1.IngressServiceBackend{
+												Name: "test",
+												Port: networkingv1.ServiceBackendPort{
+													Number: 80,
+												},
+											},
+										},
+									}},
+								},
+							},
+						}},
+					},
+				},
+			},
+			expectedGatewayResources: i2gw.GatewayResources{
+				Gateways: map[types.NamespacedName]gatewayv1beta1.Gateway{
+					{Namespace: "default", Name: "ingress-kong"}: {
+						ObjectMeta: metav1.ObjectMeta{Name: "ingress-kong", Namespace: "default"},
+						Spec: gatewayv1beta1.GatewaySpec{
+							GatewayClassName: "ingress-kong",
+							Listeners: []gatewayv1beta1.Listener{{
+								Name:     "test-mydomain-com-http",
+								Port:     80,
+								Protocol: gatewayv1beta1.HTTPProtocolType,
+								Hostname: ptrTo(gatewayv1beta1.Hostname("test.mydomain.com")),
+							}},
+						},
+					},
+				},
+				HTTPRoutes: map[types.NamespacedName]gatewayv1beta1.HTTPRoute{
+					{Namespace: "default", Name: "implementation-specific-no-regex-test-mydomain-com"}: {
+						ObjectMeta: metav1.ObjectMeta{Name: "implementation-specific-no-regex-test-mydomain-com", Namespace: "default"},
+						Spec: gatewayv1beta1.HTTPRouteSpec{
+							CommonRouteSpec: gatewayv1beta1.CommonRouteSpec{
+								ParentRefs: []gatewayv1beta1.ParentReference{{
+									Name: "ingress-kong",
+								}},
+							},
+							Hostnames: []gatewayv1beta1.Hostname{"test.mydomain.com"},
+							Rules: []gatewayv1beta1.HTTPRouteRule{
+								{
+									Matches: []gatewayv1beta1.HTTPRouteMatch{
+										{
+											Path: &gatewayv1beta1.HTTPPathMatch{
+												Type:  &gPathPrefix,
+												Value: ptrTo("/echo"),
+											},
+										},
+									},
+									BackendRefs: []gatewayv1beta1.HTTPBackendRef{
+										{
+											BackendRef: gatewayv1beta1.BackendRef{
+												BackendObjectReference: gatewayv1beta1.BackendObjectReference{
+													Name: "test",
+													Port: ptrTo(gatewayv1beta1.PortNumber(80)),
+												},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedErrors: field.ErrorList{},
+		},
 	}
 
 	for _, tc := range testCases {

pkg/i2gw/providers/kong/header_matching_test.go

@@ -246,8 +246,8 @@ func TestHeaderMatchingFeature(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			gatewayResources, errs := common.ToGateway(tc.inputResources.Ingresses, i2gw.ImplementationSpecificOptions{
-				HTTPPathmatch: httpPathMatch,
+			gatewayResources, errs := common.ToGateway(tc.inputResources.Ingresses, i2gw.ProviderImplementationSpecificOptions{
+				ToImplementationSpecificHTTPPathMatch: implementationSpecificHTTPPathMatch,
 			})
 			if len(errs) != 0 {
 				t.Errorf("Expected no errors, got %d: %+v", len(errs), errs)

pkg/i2gw/providers/kong/implementation_specific.go

@@ -23,7 +23,7 @@ import (
 	gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
 )
 
-func httpPathMatch(path *gatewayv1beta1.HTTPPathMatch) {
+func implementationSpecificHTTPPathMatch(path *gatewayv1beta1.HTTPPathMatch) {
 	pmPrefix := gatewayv1beta1.PathMatchPathPrefix
 	pmRegex := gatewayv1beta1.PathMatchRegularExpression
 	if strings.HasPrefix(*path.Value, "/~") {

pkg/i2gw/providers/kong/method_matching_test.go

@@ -189,8 +189,8 @@ func TestMethodMatchingFeature(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			gatewayResources, errs := common.ToGateway(tc.inputResources.Ingresses, i2gw.ImplementationSpecificOptions{
-				HTTPPathmatch: httpPathMatch,
+			gatewayResources, errs := common.ToGateway(tc.inputResources.Ingresses, i2gw.ProviderImplementationSpecificOptions{
+				ToImplementationSpecificHTTPPathMatch: implementationSpecificHTTPPathMatch,
 			})
 			if len(errs) != 0 {
 				t.Errorf("Expected no errors, got %d: %+v", len(errs), errs)