Skip to content

Conversation

@howardjohn
Copy link
Contributor

What type of PR is this?
/kind feature

What this PR does / why we need it:

Since the removal of alpha from served versions, implementations must start using beta types for HTTPRoute and others. This introduces substantial complexity on implementation side, as the various route types shared a large amount of objects formerly; now they are distinct Go objects. However, they are identical (and must be, or the CRD would be invalid).

We can exploit this by using Go type aliases between the two.

There are a few cases where this doesn't work:

  • The top level (Gateway, GatewayList) cannot be aliases with the generator logic. They can, however, just be newtypes wrapping the old type
  • DeepCopyGen isn't smart enough to handle this automatically, so we explicitly opt-out our aliases so we do not end up with duplicate definitions.

Does this PR introduce a user-facing change?:

Deprecated `v1alpha1` Go types are now aliases to their `v1beta1` versions

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Sep 14, 2022
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 14, 2022
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Sep 14, 2022
@robscott
Copy link
Member

Thanks @howardjohn! This is a great improvement that seems like it will simplify maintaining and implementing the API. It looks like the generated API Spec is almost entirely the same, but there are a few tiny differences like this:

https://deploy-preview-1390--kubernetes-sigs-gateway-api.netlify.app/references/spec/

The Spec and Status for each beta resource has "(Appears on: HTTPRoute, HTTPRoute)" which is both true and also a bit confusing.

I think that's a tiny enough nit that it does not need to block this, but interested in what others think.

@howardjohn
Copy link
Contributor Author

Interesting I hadn't looked at the docs generation. Let me see if there are work around.

The CRD comments shows some comment drift between alpha and beta which I think should be simple to resolve

@howardjohn
Copy link
Contributor Author

Not sure I can fix that one. Who else has opinions on docs I should add as a reviewer?

@robscott
Copy link
Member

I think the reference docs issue is small enough nit that it shouldn't block the PR. I believe this change will remarkably simplify maintaining and implementing the API, so going to go ahead and merge this in.

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 16, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: howardjohn, 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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 16, 2022
@k8s-ci-robot k8s-ci-robot merged commit 124c0c8 into kubernetes-sigs:main Sep 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants