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

Changes to support xDS Federation #33

Merged
merged 7 commits into from
Nov 2, 2022

Conversation

arvindbr8
Copy link
Member

@arvindbr8 arvindbr8 commented Oct 26, 2022

as per a47 xDS Federation spec we need to add the authorities field.
For the authorities field, we want two entries:

  • Empty string: This will be used for all new style resource names which contain no authority
  • "trafficdirector.googleapis.com:443": This is for new style resource names which explicitly specify TD
    For both these entries, we will leave the value empty or {}. That way, they will still both end up talking to TD with the server config specified by the top-level

Note that in authorities map authority name's value is empty. This is to force default to the xds_server_uri specified in the top level xds_servers field.

Fixes task 1 in #32

@arvindbr8 arvindbr8 changed the title Changes to support xDS Federation (#32) Changes to support xDS Federation Oct 26, 2022
@arvindbr8 arvindbr8 marked this pull request as ready for review October 31, 2022 21:52
@easwars easwars added the enhancement New feature or request label Oct 31, 2022
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main_test.go Outdated Show resolved Hide resolved
@arvindbr8 arvindbr8 removed their assignment Oct 31, 2022
@easwars easwars self-assigned this Nov 1, 2022
main.go Outdated Show resolved Hide resolved
main_test.go Outdated Show resolved Hide resolved
main_test.go Outdated Show resolved Hide resolved
@easwars easwars assigned arvindbr8 and unassigned easwars Nov 1, 2022
@arvindbr8 arvindbr8 removed their assignment Nov 1, 2022
@easwars
Copy link
Collaborator

easwars commented Nov 1, 2022

@ejona86 : I have reviewed this PR and looks fine to me. Do you mind taking a quick look as well? Thanks.

Copy link
Collaborator

@ejona86 ejona86 left a comment

Choose a reason for hiding this comment

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

The changes look fine, but this doesn't actually fix #32 entirely, correct, because we want the c2p stuff to be present?

@easwars
Copy link
Collaborator

easwars commented Nov 2, 2022

The changes look fine, but this doesn't actually fix #32 entirely, correct, because we want the c2p stuff to be present?

Yes, this change only addresses basic federation. The c2p changes would follow after this.

@arvindbr8
Copy link
Member Author

Yes I can add c2p changes as a separate task in #32

@easwars easwars merged commit 8716ced into GoogleCloudPlatform:master Nov 2, 2022
@arvindbr8 arvindbr8 deleted the federation branch November 3, 2023 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants