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

All gRPC Services have a sanitized authority #4587

Merged
merged 5 commits into from
Jun 24, 2022
Merged

All gRPC Services have a sanitized authority #4587

merged 5 commits into from
Jun 24, 2022

Conversation

sunjayBhatia
Copy link
Member

  • so that cluster name, which may not be a valid host, is not used
  • if SNI is set on ExtensionService, it is passed through as authority
  • otherwise, / is replaced with . in extension cluster name to be used
    as authority

Fixes: #4278

@sunjayBhatia
internal/envoy/v3: use helper for GrpcService
664742d 
Signed-off-by: Sunjay Bhatia <[email protected]>
@sunjayBhatia
gRPC Services get a configured authority
53c396b 
- so that cluster name, which may not be a valid host, is not used
- if SNI is set on ExtensionService, it is passed through as authority
- otherwise, / is replaced with . in extension cluster name to be used
as authority

Fixes: projectcontour#4278

Signed-off-by: Sunjay Bhatia <[email protected]>
@sunjayBhatia sunjayBhatia added the release-note/minor A minor change that needs about a paragraph of explanation in the release notes. label Jun 21, 2022
@sunjayBhatia sunjayBhatia requested a review from a team as a code owner June 21, 2022 22:14
@sunjayBhatia sunjayBhatia requested review from tsaarni and skriss and removed request for a team June 21, 2022 22:14
@sunjayBhatia
changelog
0c74cb1 
Signed-off-by: Sunjay Bhatia <[email protected]>
@sunjayBhatia sunjayBhatia mentioned this pull request Jun 21, 2022
Closed
@codecov
Copy link

codecov bot commented Jun 21, 2022 •
edited
Loading

Codecov Report

Merging #4587 (27539c6) into main (817f857) will decrease coverage by 0.04%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4587      +/-   ##
==========================================
- Coverage   76.85%   76.81%   -0.05%     
==========================================
  Files         140      140              
  Lines       12864    12864              
==========================================
- Hits         9887     9881       -6     
- Misses       2720     2726       +6     
  Partials      257      257
Impacted Files Coverage Δ
cmd/contour/serve.go 12.90% <0.00%> (-0.10%) ⬇️
internal/envoy/v3/cluster.go 95.90% <100.00%> (-0.10%) ⬇️
internal/envoy/v3/listener.go 98.56% <100.00%> (+0.01%) ⬆️
internal/envoy/v3/ratelimit.go 100.00% <100.00%> (ø)
internal/xdscache/v3/listener.go 90.72% <100.00%> (+0.07%) ⬆️
internal/sorter/sorter.go 97.59% <0.00%> (-1.21%) ⬇️

youngnick
youngnick approved these changes Jun 22, 2022
View reviewed changes
Copy link
Member

@youngnick youngnick left a comment

Choose a reason for hiding this comment

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

This LGTM, thanks for sorting this out.

@sunjayBhatia
move changelog to small
2a91c2f 
Signed-off-by: Sunjay Bhatia <[email protected]>
@sunjayBhatia sunjayBhatia added release-note/small A small change that needs one line of explanation in the release notes. and removed release-note/minor A minor change that needs about a paragraph of explanation in the release notes. labels Jun 22, 2022
@sunjayBhatia sunjayBhatia merged commit 3fc5627 into projectcontour:main Jun 24, 2022
@sunjayBhatia sunjayBhatia deleted the extensionservice-authority branch June 24, 2022 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@youngnick youngnick youngnick approved these changes

@tsaarni tsaarni Awaiting requested review from tsaarni tsaarni is a code owner automatically assigned from projectcontour/maintainers

@skriss skriss Awaiting requested review from skriss skriss is a code owner automatically assigned from projectcontour/maintainers

Assignees
No one assigned
Labels
release-note/small A small change that needs one line of explanation in the release notes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

malformed authority header for ExtensionService
2 participants
@sunjayBhatia @youngnick