Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

implement GEP-718: rework RouteForwardTo #750

Merged
merged 1 commit into from Aug 6, 2021
Merged

implement GEP-718: rework RouteForwardTo #750

merged 1 commit into from Aug 6, 2021

Conversation

hbagdi
Copy link
Contributor

@hbagdi hbagdi commented Aug 6, 2021

What type of PR is this?

/kind api-change

What this PR does / why we need it:
This PR implement GEP #718. Please refer to the GEP for more details.

Which issue(s) this PR fixes:

Fixes #718

Does this PR introduce a user-facing change?:

API for forwarding requests from routes to backends has been reworked (breaking change).

@k8s-ci-robot k8s-ci-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 6, 2021
@k8s-ci-robot
Copy link
Contributor

[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:

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 approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 6, 2021
@hbagdi hbagdi force-pushed the feat/consolidate-forwardto branch from 07b2851 to 4acea30 Compare August 6, 2021 20:03
@hbagdi
Copy link
Contributor Author

hbagdi commented Aug 6, 2021

/cc @robscott @bowei @jpeach @youngnick @Miciah

@k8s-ci-robot
Copy link
Contributor

@hbagdi: GitHub didn't allow me to request PR reviews from the following users: Miciah.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @robscott @bowei @jpeach @youngnick @Miciah

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.

@robscott robscott added this to the v1alpha2 milestone Aug 6, 2021
@hbagdi hbagdi force-pushed the feat/consolidate-forwardto branch from 4acea30 to e39460e Compare August 6, 2021 20:20
robscott
robscott reviewed Aug 6, 2021
View reviewed changes
Copy link
Member

@robscott robscott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @hbagdi! This mostly LGTM, just note that this will conflict with #741 from @youngnick.

apis/v1alpha2/udproute_types.go Outdated Show resolved Hide resolved
apis/v1alpha2/udproute_types.go Show resolved Hide resolved
apis/v1alpha2/tcproute_types.go Outdated Show resolved Hide resolved
apis/v1alpha2/tlsroute_types.go Outdated Show resolved Hide resolved
@hbagdi hbagdi force-pushed the feat/consolidate-forwardto branch from e39460e to 2a0d54c Compare August 6, 2021 20:35
@hbagdi
Copy link
Contributor Author

hbagdi commented Aug 6, 2021

that this will conflict with #741 from @youngnick.

And so let's get this in quick. ;)

@hbagdi hbagdi force-pushed the feat/consolidate-forwardto branch from 2a0d54c to 83226ae Compare August 6, 2021 20:37
@hbagdi hbagdi mentioned this pull request Aug 6, 2021
Closed
robscott
robscott reviewed Aug 6, 2021
View reviewed changes
apis/v1alpha2/shared_types.go
Name string `json:"name"`

// Port specifies the destination port number to use for this resource.
// Port is required when the referent is a Kubernetes Service.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #753 to track webhook validation for this.

@robscott
Copy link
Member

robscott commented Aug 6, 2021

Thanks @hbagdi!

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 6, 2021
@k8s-ci-robot k8s-ci-robot merged commit 86faa74 into kubernetes-sigs:master Aug 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robscott robscott robscott left review comments

@thockin thockin Awaiting requested review from thockin

@youngnick youngnick Awaiting requested review from youngnick

@bowei bowei Awaiting requested review from bowei

@jpeach jpeach Awaiting requested review from jpeach

Assignees

@robscott robscott

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/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1alpha2
Development

Successfully merging this pull request may close these issues.

GEP: consolidate serviceName shortcut in RouteForwardTo
3 participants
@hbagdi @k8s-ci-robot @robscott