v1alpha2: Clarify HTTPRouteFilter conformance levels

[banks]

What happened:

Reading the API definition in

gateway-api/apis/v1alpha2/httproute_types.go

Line 443 in 3aa586d

type HTTPRouteFilter struct {

there seems to be some inconsistency in the conformance level annotations for HTTPRouteFilter.

It's possible I'm just reading it wrong, but if so perhaps the presentation could be clarified to ensure the spec is clear to other implementors.

Specifically, in HTTPRouteFilter I see RequestHeaderModifier is Core while RequestMirror is Extended (which seems sensible):

gateway-api/apis/v1alpha2/httproute_types.go

Lines 468 to 481 in 3aa586d

	// RequestHeaderModifier defines a schema for a filter that modifies request
	// headers.
	//
	// Support: Core
	//
	// +optional
	RequestHeaderModifier *HTTPRequestHeaderFilter `json:"requestHeaderModifier,omitempty"`
	// RequestMirror defines a schema for a filter that mirrors requests.
	//
	// Support: Extended
	//
	// +optional
	RequestMirror *HTTPRequestMirrorFilter `json:"requestMirror,omitempty"`

But then in the annotations on those nested struct types:

• all the fields in the Core HTTPRequestHeaderFilter are marked as Extended. If all config fields only require conformance by "extended" implementations, what does it mean that the filter itself is required by Core implementations?
• HTTPRequestMirrorFilter which is marked as Extended above, has a field ServiceName which is marked as Core conformance level. What does it mean for a field to be required by Core conforming implementations if it's part of an Extended struct?

What you expected to happen:

Conformance level annotations for nested configuration to be the same or weaker than the parent configuration level.

Thanks for your great work on this spec, I'm following closely!

[edit] I notice afterwards that v1alpha2 where I saw this is unstable so perhaps this is still a WIP and tracked elsewhere? I didn't see anything obvious though.

[robscott]

Thanks for catching this @banks! You're right that this is unnecessarily confusing. Although each individual support level could make sense in isolation, they don't with the context you've provided here.

[hbagdi]

Doing a bit of git spelunking, we didn't have enough discussion around the conformance level for those fields and they got marked as extended in a cleanup: 034c323#diff-d1c59de21c79ea2bd1a99c2f52d9879981dd785848408064289287cb262fe600R422-R424

I think fields inside RequestHeaderModifier shouldn't even have conformance level as the behavior of the filter is well-defined (except #480).

[hbagdi]

/assign

[hbagdi]

#721 is a partial fix.
#719 should simplify the filter a bit. I'll take care of updating the language to match the discussion here.
Let me know if I'm missing anything else from here.

[robscott]

#723 also attempts to fix this

[robscott]

I think this has been fixed by #721 and #723, feel free to reopen if I missed anything.

/close

[k8s-ci-robot]

@robscott: Closing this issue.

In response to this:

I think this has been fixed by #721 and #723, feel free to reopen if I missed anything.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.