refactor: dedup the route context code using reflection

[shawnh2]

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #1252

[codecov]

Codecov Report

Merging #1502 (efd5395) into main (ca36e3f) will increase coverage by 0.00%.
The diff coverage is 84.16%.

@@           Coverage Diff            @@
##             main    #1502    +/-   ##
========================================
  Coverage   61.70%   61.71%            
========================================
  Files          81       81            
  Lines       12131    11967   -164     
========================================
- Hits         7485     7385   -100     
+ Misses       4179     4127    -52     
+ Partials      467      455    -12     

Impacted Files	Coverage Δ
internal/gatewayapi/route.go	88.36% <77.27%> (ø)
internal/gatewayapi/contexts.go	84.30% <83.13%> (+8.43%)	⬆️
internal/cmd/egctl/translate.go	82.14% <100.00%> (+0.41%)	⬆️

... and 3 files with indirect coverage changes

[arkodg]

do you think reflect is the right package to use here or should we use https://pkg.go.dev/k8s.io/apimachinery/pkg/util/validation/field which is used heavily in upstream gateway api, here an
example

[shawnh2]

thanks for pointing it out, but i think https://pkg.go.dev/k8s.io/apimachinery/pkg/util/validation/field is not suitable for our case.

despite the fact that this package provides several powerful methods, but its intention is for validations only. in our case, we need more than that, we need to write some values for some fields in one struct as well, but this package does not have that kind of ability.

i thought about whether we could replace current validations with the one provided in that package, and turns out we may not. for example, the strings.HasPrefix(*path.Value, "/") use the variable path directly, which has explicit type *gatewayv1b1.HTTPPathMatch. however, in our case, the only variable we can manipulate with is route, which is a interface RouteContext. so we need the ability provided in package reflect, to play with the real type behinds whatever it represents.

so basically, i think relfect is inevitable.

[arkodg]

thanks for picking this refactor up @shawnh2 !

[arkodg]

curious why this custom HTTPRoute logic is needed

[shawnh2]

becasue there are some differences between the implementation of HTTPRoute and other Routes in GetRouteParentContext method.

for example, HTTPRoute is like:

gateway/internal/gatewayapi/contexts.go

Lines 220 to 226 in 7f8a491

	var parentRef *v1beta1.ParentReference
	for i, p := range h.Spec.ParentRefs {
	if reflect.DeepEqual(p, forParentRef) {
	parentRef = &h.Spec.ParentRefs[i]
	break
	}
	}

as for other Route:

gateway/internal/gatewayapi/contexts.go

Lines 471 to 479 in 7f8a491

	var parentRef *v1beta1.ParentReference
	for i, p := range u.Spec.ParentRefs {
	p := UpgradeParentReference(p)
	if reflect.DeepEqual(p, forParentRef) {
	upgraded := UpgradeParentReference(u.Spec.ParentRefs[i])
	parentRef = &upgraded
	break
	}
	}

so add a if logic to deal with HTTPRoute

[arkodg]

this looks a little intimidating to me, can we use FieldByName here ?

[shawnh2]

sure we can use FieldByName, in that case, we need to add one more field, like type string in struct HTTP/GRPC/.../Context, indicating the type of this struct, and all the value of field type comes from:

gateway/internal/gatewayapi/translator.go

Lines 17 to 22 in 4dd4f87

	KindGRPCRoute = "GRPCRoute"
	KindHTTPRoute = "HTTPRoute"
	KindNamespace = "Namespace"
	KindTLSRoute = "TLSRoute"
	KindTCPRoute = "TCPRoute"
	KindUDPRoute = "UDPRoute"

@arkodg see if this is ok?

[arkodg]

or you could also use GetKind() ?
see it being used in other places in the codebase

gateway/internal/gatewayapi/filters.go

Line 769 in 4dd4f87

if res.GetKind() == string(extFilter.Kind) && res.GetName() == string(extFilter.Name) && res.GetNamespace() == filterNs {

[shawnh2]

thanks for the heads up!

now we can use FieldByName("Kind"), BTW, I add some TypeMeta for xRoute resources in translate func, nor it will keep getting "".

[arkodg]

thanks @shawnh2 for addressing the comments, adding a minor comment, else LGTM !

[arkodg]

LGTM thanks !