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/google-c2p: validate url for no authorities #5756

Merged
merged 3 commits into from
Nov 2, 2022

Conversation

arvindbr8
Copy link
Member

@arvindbr8 arvindbr8 commented Nov 2, 2022

As per gRPC DirectPath C2P Resolver doc, The "google-c2p" URI scheme will not support authorities.

This diff will validate that

RELEASE NOTES:

  • xds/google-c2p: validate URI schema for no authorities

@arvindbr8 arvindbr8 self-assigned this Nov 2, 2022
@arvindbr8 arvindbr8 added this to the 1.51 Release milestone Nov 2, 2022
@arvindbr8 arvindbr8 added the Type: Behavior Change Behavior changes not categorized as bugs label Nov 2, 2022
@arvindbr8 arvindbr8 assigned easwars and unassigned arvindbr8 Nov 2, 2022
xds/googledirectpath/googlec2p_test.go Outdated Show resolved Hide resolved
xds/googledirectpath/googlec2p_test.go Outdated Show resolved Hide resolved
@easwars easwars assigned arvindbr8 and unassigned easwars Nov 2, 2022
@arvindbr8 arvindbr8 added Type: Behavior Change Behavior changes not categorized as bugs Type: Bug and removed Type: Behavior Change Behavior changes not categorized as bugs labels Nov 2, 2022
@arvindbr8 arvindbr8 assigned easwars and unassigned arvindbr8 Nov 2, 2022
}()
wantErr := "google-c2p URI scheme does not support authorities"
if err == nil || !strings.Contains(err.Error(), wantErr) {
cc.Close()
Copy link
Contributor

Choose a reason for hiding this comment

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

Just realized that we don't need this cc.Close() here since that will be taken care of by the deferred function.

Copy link
Member Author

Choose a reason for hiding this comment

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

updating that and merging

@easwars easwars removed the Type: Behavior Change Behavior changes not categorized as bugs label Nov 2, 2022
@arvindbr8 arvindbr8 merged commit fcb8bdf into grpc:master Nov 2, 2022
@arvindbr8 arvindbr8 deleted the c2pfederation branch November 2, 2022 20:11
jronak pushed a commit to jronak/grpc-go that referenced this pull request Nov 21, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants