search

GoogleCloudPlatform / traffic-director-grpc-bootstrap

Apache License 2.0
20 stars 18 forks source link

Fix LDS resource name format for TD authority #39

Closed arvindbr8 closed 1 year ago

arvindbr8 commented 1 year ago

The LDS resource name format was incorrect. The correct format as per the xDS TD documentation is 

xdstp://<authority>/envoy.config.listener.v3.Listener/<project_number>/<(network)|(mesh:mesh_name)>/id

This change fixes the format.

easwars commented 1 year ago

The PR description talks about scope. But there is no mention of it in the code (and in the resource name format mentioned in the comment in the code). If scope is something for which we will add support in the near future, can we keep the PR description and the code consistent (one way or the other)?

arvindbr8 commented 1 year ago

The PR description talks about scope. But there is no mention of it in the code (and in the resource name format mentioned in the comment in the code). If scope is something for which we will add support in the near future, can we keep the PR description and the code consistent (one way or the other)?

IIUC, scope: is for gateway usecases and not applicable for this generator. the reason why i put that in the PR description is to answer if asked the question "the TD documentation has (scope:) but its not part of the LDS resource name format of the generator".

I also see why this is causing confusion. I've updated the PR description to reflect the same format that we are fixing this to.

ejona86 commented 1 year ago

Before merging, make sure to fix the commit description