Skip to content

Commit

Permalink
reviews' comments addressed
Browse files Browse the repository at this point in the history
Signed-off-by: Mattia Lavacca <[email protected]>
  • Loading branch information
mlavacca committed Nov 15, 2023
1 parent d586b06 commit 94e1d82
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 19 deletions.
9 changes: 8 additions & 1 deletion PROVIDER.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ func (r *resourceReader) ReadResourcesFromFiles(ctx context.Context, customResou
```
3. Create a struct named `converter` which implements the `ResourceConverter` interface in a file named `converter.go`.
The implemented `ToGatewayAPI` function should simply call every registered `featureParser` function, one by one.
Take a look at `ingressnginx/converter.go` for example.
Take a look at `ingressnginx/converter.go` for an example.
The `ImplementationSpecificOptions` struct contains the handlers to customize native ingress implementation-specific fields.
Take a look at `kong/converter.go` for an example.

```go
package examplegateway

Expand All @@ -74,6 +77,7 @@ type converter struct {
conf *i2gw.ProviderConf

featureParsers []i2gw.FeatureParser
implementationSpecificOptions i2gw.ProviderImplementationSpecificOptions
}

// newConverter returns an ingress-nginx converter instance.
Expand All @@ -83,6 +87,9 @@ func newConverter(conf *i2gw.ProviderConf) *converter {
featureParsers: []i2gw.FeatureParser{
// The list of feature parsers comes here.
},
implementationSpecificOptions: i2gw.ProviderImplementationSpecificOptions{
// The list of the implementationSpecific ingress fields options comes here.
},
}
}
```
Expand Down
12 changes: 6 additions & 6 deletions pkg/i2gw/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,15 @@ type ResourceConverter interface {
ToGatewayAPI(resources InputResources) (GatewayResources, field.ErrorList)
}

// ImplementationSpecificHTTPPathMatchOption is an option to customize the ingress implementationSpecific
// ImplementationSpecificHTTPPathTypeMatchConverter is an option to customize the ingress implementationSpecific
// match type conversion.
type ImplementationSpecificHTTPPathMatchOption func(*gatewayv1beta1.HTTPPathMatch)
type ImplementationSpecificHTTPPathTypeMatchConverter func(*gatewayv1beta1.HTTPPathMatch)

// 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.
// ProviderImplementationSpecificOptions contains customized implementation-specific fields and functions.
// These will be used by the common package to customize the provider-specific behavior for all the
// implementation-specific fields of the ingress API.
type ProviderImplementationSpecificOptions struct {
ToImplementationSpecificHTTPPathMatch ImplementationSpecificHTTPPathMatchOption
ToImplementationSpecificHTTPPathTypeMatch ImplementationSpecificHTTPPathTypeMatchConverter
}

// InputResources contains all Ingress objects, and Provider specific
Expand Down
9 changes: 7 additions & 2 deletions pkg/i2gw/providers/common/converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ func (rg *ingressRuleGroup) toHTTPRoute(options i2gw.ProviderImplementationSpeci
for _, paths := range pathsByMatchGroup {
path := paths[0]
fieldPath := field.NewPath("spec", "rules").Index(path.ruleIdx).Child(path.ruleType).Child("paths").Index(path.pathIdx)
match, err := toHTTPRouteMatch(path.path, fieldPath, options.ToImplementationSpecificHTTPPathMatch)
match, err := toHTTPRouteMatch(path.path, fieldPath, options.ToImplementationSpecificHTTPPathTypeMatch)
if err != nil {
errors = append(errors, err)
continue
Expand Down Expand Up @@ -348,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, toImplementationSpecificPathMatch i2gw.ImplementationSpecificHTTPPathMatchOption) (*gatewayv1beta1.HTTPRouteMatch, *field.Error) {
func toHTTPRouteMatch(routePath networkingv1.HTTPIngressPath, path *field.Path, toImplementationSpecificPathMatch i2gw.ImplementationSpecificHTTPPathTypeMatchConverter) (*gatewayv1beta1.HTTPRouteMatch, *field.Error) {
pmPrefix := gatewayv1beta1.PathMatchPathPrefix
pmExact := gatewayv1beta1.PathMatchExact

Expand All @@ -367,6 +367,11 @@ func toHTTPRouteMatch(routePath networkingv1.HTTPIngressPath, path *field.Path,
} else {
return nil, field.Invalid(path.Child("pathType"), routePath.PathType, "implementationSpecific path type is not supported in generic translation, and your provider does not provide custom support to translate it")
}
default:
// default should never hit, as all the possible cases are already checked
// via proper switch cases.
return nil, field.Invalid(path.Child("pathType"), match.Path.Type, fmt.Sprintf("unsupported path match type: %s", *match.Path.Type))

}

return match, nil
Expand Down
8 changes: 4 additions & 4 deletions pkg/i2gw/providers/kong/converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type converter struct {
conf *i2gw.ProviderConf

featureParsers []i2gw.FeatureParser
ImplementationSpecificOptions i2gw.ProviderImplementationSpecificOptions
implementationSpecificOptions i2gw.ProviderImplementationSpecificOptions
}

// newConverter returns an kong converter instance.
Expand All @@ -39,8 +39,8 @@ func newConverter(conf *i2gw.ProviderConf) *converter {
methodMatchingFeature,
pluginsFeature,
},
ImplementationSpecificOptions: i2gw.ProviderImplementationSpecificOptions{
ToImplementationSpecificHTTPPathMatch: implementationSpecificHTTPPathMatch,
implementationSpecificOptions: i2gw.ProviderImplementationSpecificOptions{
ToImplementationSpecificHTTPPathTypeMatch: implementationSpecificHTTPPathTypeMatch,
},
}
}
Expand All @@ -51,7 +51,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, c.ImplementationSpecificOptions)
gatewayResources, errs := common.ToGateway(resources.Ingresses, c.implementationSpecificOptions)
if len(errs) > 0 {
return i2gw.GatewayResources{}, errs
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/i2gw/providers/kong/converter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import (

func Test_ToGateway(t *testing.T) {
iPrefix := networkingv1.PathTypePrefix
ImplSpecPathType := networkingv1.PathTypeImplementationSpecific
ImplSpecificPathType := networkingv1.PathTypeImplementationSpecific
gPathPrefix := gatewayv1beta1.PathMatchPathPrefix
gPathRegex := gatewayv1beta1.PathMatchRegularExpression

Expand Down Expand Up @@ -340,7 +340,7 @@ func Test_ToGateway(t *testing.T) {
HTTP: &networkingv1.HTTPIngressRuleValue{
Paths: []networkingv1.HTTPIngressPath{{
Path: "/~/echo/**/test",
PathType: &ImplSpecPathType,
PathType: &ImplSpecificPathType,
Backend: networkingv1.IngressBackend{
Service: &networkingv1.IngressServiceBackend{
Name: "test",
Expand Down Expand Up @@ -425,7 +425,7 @@ func Test_ToGateway(t *testing.T) {
HTTP: &networkingv1.HTTPIngressRuleValue{
Paths: []networkingv1.HTTPIngressPath{{
Path: "/echo",
PathType: &ImplSpecPathType,
PathType: &ImplSpecificPathType,
Backend: networkingv1.IngressBackend{
Service: &networkingv1.IngressServiceBackend{
Name: "test",
Expand Down
2 changes: 1 addition & 1 deletion pkg/i2gw/providers/kong/header_matching_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ 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.ProviderImplementationSpecificOptions{
ToImplementationSpecificHTTPPathMatch: implementationSpecificHTTPPathMatch,
ToImplementationSpecificHTTPPathTypeMatch: implementationSpecificHTTPPathTypeMatch,
})
if len(errs) != 0 {
t.Errorf("Expected no errors, got %d: %+v", len(errs), errs)
Expand Down
2 changes: 1 addition & 1 deletion pkg/i2gw/providers/kong/implementation_specific.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
)

func implementationSpecificHTTPPathMatch(path *gatewayv1beta1.HTTPPathMatch) {
func implementationSpecificHTTPPathTypeMatch(path *gatewayv1beta1.HTTPPathMatch) {
pmPrefix := gatewayv1beta1.PathMatchPathPrefix
pmRegex := gatewayv1beta1.PathMatchRegularExpression
if strings.HasPrefix(*path.Value, "/~") {
Expand Down
2 changes: 1 addition & 1 deletion pkg/i2gw/providers/kong/method_matching_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ 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.ProviderImplementationSpecificOptions{
ToImplementationSpecificHTTPPathMatch: implementationSpecificHTTPPathMatch,
ToImplementationSpecificHTTPPathTypeMatch: implementationSpecificHTTPPathTypeMatch,
})
if len(errs) != 0 {
t.Errorf("Expected no errors, got %d: %+v", len(errs), errs)
Expand Down

0 comments on commit 94e1d82

Please sign in to comment.