multiple fixes to HTTPRequestRedirect filter

[hbagdi]

What type of PR is this?

/kind cleanup
/kind api-change
What this PR does / why we need it:

• Add Filter prefix to the struct to match other filters
• Rename Protocol to Scheme
• Remove support levels on the fields since the filter is marked as Core

Which issue(s) this PR fixes:

None

Does this PR introduce a user-facing change?:

HTTPRequestRedirectFilter's Protocol field has been renamed to Scheme.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hbagdi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [hbagdi]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[robscott]

Thanks for the work on this @hbagdi! This LGTM except for removing the support levels.

[robscott]

I don't think we can remove the support levels here. I can't find/remember where this was documented, but I thought we agreed that support level was essentially a min of field and parent support level. This was particularly important for filters since they could be defined per Route Rule or per Backend. In this case, we'd be removing an "Extended" support annotation and deferring to the "Core" support of redirects in general which does not seem accurate here. This comment applies throughout, but just adding it here.

[robscott]

As a follow up, this PR recovers the docs that got lost around this: #864.

[hbagdi]

I expected some discussion on this front.

I'm a bit confused here so making sure I understand you right.

support level was essentially a min of field and parent support level

I think we agree with this statement. I removed the Extended support level because the filter is marked as "Core". Marking the filter as "Extended" is another option to adhere to this rule.

Or are you saying that we should keep the filter as "Core" and have some fields marked as "Extended" (leave as is)?
The concern I've with this approach is that as a user, I might get surprised when I'm using a Core filter with a conformant implementation but something doesn't work and the reason is a field-level documentation that I didn't peruse.

[robscott]

Although I agree this could get confusing, I think the alternatives are worse. The way I see it we have two options:

1. Features can be marked core but may include some extended capabilities/fields that are not required to use the feature
2. Features must be marked as extended if they include at least one field that is not supported by every implementation.

The current PR actually is more of a third option - it removes the "Extended" annotation on several fields, which may lead users to believe the field had "Core" support. Unfortunately there's at least one implementation that can't support the field, so that seems inaccurate.

Of the options I listed above, I strongly prefer 1 because otherwise I think we'd end up avoiding new fields or with a lot of "Extended" features.

[hbagdi]

I think we'd end up avoiding new fields or with a lot of "Extended" features.

An alternative is to have a filter "RequestRedirectExtended" (with a better name) in addition to the core filter. That lays out the contract much more explicitly and makes the end-user experience free of confusion.
We can't take this approach with more involved features within the guts of the API, but with filters, we can.

Having said that, I've reverted the changes related to conformance levels to minimize the scope of the PR.

[jpeach]

While the protocol was "HTTP", I guess the scheme should be "http"?

[robscott]

Thanks!

/lgtm