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.

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

xds: server-side security: export WatchListener() from xdsClient #3817

Merged
merged 2 commits into from
Aug 18, 2020

Conversation

easwars
Copy link
Contributor

@easwars easwars commented Aug 14, 2020

  • This will be used from the xdsServer.
  • Also fix one thing which was missed out in the xDS v3 support PR:
    • Watches registered on the xdsClient should end up sending out
      requests whose resource URL matches the transport version being
      used.
    • Blank import the v3 client package from xds package. Without this,
      attempts to use v3 transport will fail.

@easwars easwars requested a review from menghanl August 14, 2020 15:56
@easwars easwars added no release notes Type: Feature New features or improvements in behavior labels Aug 14, 2020
* This will be used from the xdsServer.
* Also fix one thing which was missed out in the xDS v3 support PR:
  * Watches registered on the xdsClient should end up sending out
    requests whose resource URL matches the transport version being
    used.
  * Blank import the v3 client package from xds package. Without this,
    attempts to use v3 transport will fail.
@@ -213,7 +268,7 @@ func (c *Client) watch(wi *watchInfo) (cancel func()) {
func (c *Client) watchLDS(serviceName string, cb func(ListenerUpdate, error)) (cancel func()) {
wi := &watchInfo{
c: c,
typeURL: version.V2ListenerURL,
typeURL: c.listenerURL(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we make an enum type for the types, and use that here? So that Client wouldn't need to deal with v2 and v3. The underlying versioned client can do the conversion.

We had this enum at the beginning (I think?), but later found it's not necessary, and we can just use the string. Now with two versions, the enum might make more sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think making that change has definitely made the code a little cleaner. But now the change is huge :(

@easwars
Copy link
Contributor Author

easwars commented Aug 15, 2020

PTAL.

@easwars easwars merged commit 0f73133 into grpc:master Aug 18, 2020
@easwars easwars deleted the xds_client_listeners branch August 18, 2020 22:40
@easwars easwars changed the title xds: Export a WatchListener() method on the xdsClient. xds: server-side security: export WatchListener() from xdsClient Mar 4, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Feature New features or improvements in behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants