Implementing GEPs 726 and 922: Rewrites + Experimental CRDs

[robscott]

What type of PR is this?
/kind feature

What this PR does / why we need it:
This is a huge PR that implements GEP #726 and #922. These are bundled because they're difficult to do independently. To add new features to Gateway API, they need to be experimental. To understand how to generate experimental CRDs, we needed to have some kind of experimental feature to test with.

A big thanks to @jpeach for the inspiration here for CRD generation and showing this kind of custom approach was relatively straightforward.

I spent a non-trivial amount of time trying to get these field level annotations to work as "markers" like the other kubebuilder tags. Unfortunately I could not make that work, so chose a different syntax <> so these values would still be parseable after crdgen was complete. If anyone wants to experiment with better approaches here, I'd be very open.

Alternatives I'd considered here:

• Separate types files for experimental CRDs
• A relatively complex set of shell scripts

Although this approach is not perfect, I think it's preferable to either of the above.

Which issue(s) this PR fixes:
This implements both #726 and #922, but both still require documentation updates. I'm leaving that out of this PR because it's already so big.

Does this PR introduce a user-facing change?:

* Gateway API now includes "Experimental" release channel
* Gateway API CRDs now include `gateway.networking.k8s.io/bundle-version` and `gateway.networking.k8s.io/channel` annotations
* URL Rewrites and Path redirects have been added as new "Experimental" features

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: robscott

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 [robscott]

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

[robscott]

This is a big PR, adding a hold until we have consensus on the approach.

/hold

[robscott]

Although I'd like to improve the reference docs, the rendering with this change actually isn't as bad as I'd originally expected. Search for "gateway:experimental" in the deploy preview to take a look.

[youngnick]

This LGTM overall, with just a couple of small queries.

[hbagdi]

I'm a bit hesitant about empty fields, especially string semantics. They are not usually the most intuitive, even more so in kube land.
Have we considered another "Type": "RemovePrefixMatch"?

[brianehlert]

What if explicitly a NULL is the substitution value for the remove case? Something explicit in the value as opposed to an additional type (too many types, while explicit to implement can cause usability issues and API friction)
Also, many JSON parsers and automation tools drop an empty value - thus preventing passing an empty value in a document through a tool chain, and YAML empty value = NULL

[robscott]

@hbagdi We discussed this at the community meeting yesterday. I suggested something similar to what @brianehlert said above, essentially trying to ensure that users had to specify at least something here, even if that was just "". I tested the current CRDs out with the following YAML in a redirect filter:

path:
  type: ReplacePrefixMatch

That resulted in the following error:

error validating data: ValidationError(HTTPRoute.spec.rules[0].filters[0].requestRedirect.path): missing required field "substitution" in io.k8s.networking.gateway.v1alpha2.HTTPRoute.spec.rules.filters.requestRedirect.path

That error could be avoided by specifying an empty string:

path:
  type: ReplacePrefixMatch
  substitution: ""

I think that ends up being pretty clear.

The other key point that came up in the community meeting was that "replace with nothing" is a fairly common way to represent "remove" in a wide variety of tooling. Although I agree that we need to be careful about inferring meaning from unspecified values, I think we're safely avoiding that by requiring user input.

With all that said, my personal preference is to avoid an additional unique filter type here and instead move forward with the ability to replace a prefix match with nothing.

[hbagdi]

The generated YAML has nothing that indicates that those fields are experimental:

gateway-api/config/crd/experimental/gateway.networking.k8s.io_httproutes.yaml

Lines 977 to 1014 in 7821e4e

	urlRewrite:
	description: "URLRewrite defines a schema for a filter
	that responds to the request with an HTTP redirection.
	\n Support: Core \n "
	properties:
	hostname:
	description: "Hostname is the value to be used to
	replace the Host header value during forwarding.
	\n Support: Extended"
	maxLength: 253
	minLength: 1
	pattern: ^(\*\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
	type: string
	path:
	description: "Path defines a path rewrite. \n Support:
	Extended"
	properties:
	substitution:
	description: "Substitution defines the HTTP path
	value to substitute. An empty value (\"\") indicates
	that the portion of the path to be changed should
	be removed from the resulting path. For example,
	a request to \"/foo/bar\" with a prefix match
	of \"/foo\" would be modified to \"/bar\". \n "
	maxLength: 1024
	type: string
	type:
	description: "Type defines the type of path modifier.
	\n "
	enum:
	- Absolute
	- ReplacePrefixMatch
	type: string
	required:
	- substitution
	- type
	type: object
	type: object

Can we not strip out the experimental tags from the YAML?

[robscott]

Can we not strip out the experimental tags from the YAML?

Good call, updated to leave them in place.

[robscott]

Thanks for the great feedback on this @hbagdi and @youngnick! I think I've addressed everything now, PTAL.

[youngnick]

/lgtm

I think the docs followup is a great idea. Thanks @robscott!

[hbagdi]

Do we need to clarify that path doesn't include query string?
I think this may be a potential source of confusion. Not a blocker.

[robscott]

Agree, that's a good point. I'll add that in a follow up.

[hbagdi]

Great work Rob!
/lgtm

[robscott]

Thanks for the reviews!

/hold cancel