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

feat: Listener Generation for Terminating Gateway Destinations #13196

Closed
wants to merge 1 commit into from
Closed

feat: Listener Generation for Terminating Gateway Destinations #13196

wants to merge 1 commit into from

Conversation

DanStough
Copy link
Contributor

@DanStough DanStough commented May 24, 2022 •
edited
Loading

Description

This PR adds xDS listener generation to Destinations (fka "Endpoints") for terminating gateways. To recap, destinations are external services that are not part of the Consul cluster. They are nodeless and also support IP CIDRs. Destinations are referenced the same as regular services in the Terminating Gateway Config Entry but are defined in service-defaults (#13159).

Testing

Tests with Golden files are included for the three types of Destinations.

Links

Related PRs:

  1. update gateway-services table with endpoints #13217 Updates the gateway-service table to qualify different kinds ("default" and "destinations").
  2. Egress gtw/connect destination intentions #13341 to update the intentions to work with destinations.

PR Checklist

  • updated test coverage
  • external facing docs updated
  • not a security concern
  • checklist folder consulted

@DanStough DanStough force-pushed the dstough/test-endpoint-service-query branch 2 times, most recently from 82d3272 to 0d208af Compare May 26, 2022 22:15
DanStough
DanStough commented May 26, 2022
View reviewed changes
@dhiaayachi dhiaayachi added pr/no-changelog PR does not need a corresponding .changelog entry pr/no-metrics-test labels May 30, 2022
@dhiaayachi
Copy link
Collaborator

@DanStough I made few changes in here to fix the order in the sort algorithm and regenerate the files. I also opened #13300 to add the leaf cert generation and some testing around having valid endpoints.

I think we need to do some renaming in this PR before being able to open it for review. So it match #13217 naming convention.

@DanStough DanStough changed the title feat: add endpoint to ServerConfigResponse feat: Listener Generation for Terminating Gateway Destinations May 31, 2022
@DanStough DanStough marked this pull request as ready for review May 31, 2022 16:28
@DanStough
Copy link
Contributor Author

Asking for a preliminary review. I still need Dhia's PR to land and rename all the "endpoint" stuff to "desintations" and also adopt the GatewayServiceKind.

@DanStough DanStough requested review from a team and rboyer and removed request for a team May 31, 2022 16:29
@dhiaayachi dhiaayachi mentioned this pull request May 31, 2022
Merged
2 tasks
@DanStough DanStough removed the request for review from rboyer May 31, 2022 21:44
@DanStough
Copy link
Contributor Author

On second thought, I think this PR needs some more work w.r.t. transparent proxy mode in the tests.

@DanStough DanStough marked this pull request as draft May 31, 2022 21:45
@DanStough DanStough force-pushed the dstough/test-endpoint-service-query branch from 60545a3 to 6806f1f Compare June 2, 2022 16:20
@DanStough DanStough requested a review from dhiaayachi June 2, 2022 19:21
@DanStough DanStough force-pushed the dstough/test-endpoint-service-query branch 2 times, most recently from 0807846 to ad4c262 Compare June 2, 2022 20:01
@DanStough DanStough removed the request for review from dhiaayachi June 2, 2022 20:45
@DanStough
Copy link
Contributor Author

Updates were made based on the new design. Ready to review.

@DanStough DanStough marked this pull request as ready for review June 2, 2022 20:48
@DanStough DanStough requested review from a team and markan and removed request for a team June 2, 2022 20:48
@DanStough DanStough requested a review from dhiaayachi June 2, 2022 20:48
dhiaayachi
dhiaayachi requested changes Jun 3, 2022
View reviewed changes
Copy link
Collaborator

@dhiaayachi dhiaayachi left a comment

Choose a reason for hiding this comment

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

Thank you for working on this @DanStough. I had a few questions/comments.

@DanStough DanStough force-pushed the dstough/test-endpoint-service-query branch from ad4c262 to b312e41 Compare June 3, 2022 16:11
@DanStough DanStough requested a review from dhiaayachi June 3, 2022 16:41
dhiaayachi
dhiaayachi approved these changes Jun 6, 2022
View reviewed changes
Copy link
Collaborator

@dhiaayachi dhiaayachi left a comment

Choose a reason for hiding this comment

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

LGTM!

@DanStough DanStough force-pushed the dstough/test-endpoint-service-query branch from d8e8e0b to b312e41 Compare June 7, 2022 21:59
@DanStough
Copy link
Contributor Author

@markan I'm closing this one in favor of #13391, which has these changes and a few more to cover all of the xDS on the T-gtwy side.

@DanStough DanStough closed this Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhiaayachi dhiaayachi

@markan markan

Assignees
No one assigned
Labels
pr/no-changelog PR does not need a corresponding .changelog entry pr/no-metrics-test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DanStough @dhiaayachi