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

feat: Add AllowPrivateNetwork to support Access-Control-Allow-Private-Network #5034

Merged
merged 1 commit into from
Feb 7, 2023
Merged

feat: Add AllowPrivateNetwork to support Access-Control-Allow-Private-Network #5034

merged 1 commit into from
Feb 7, 2023

Conversation

lvyanru8200
Copy link
Contributor

@lvyanru8200 lvyanru8200 commented Jan 31, 2023 •
edited
Loading

Fixes: #4553
Signed-off-by: tigerK yanru.lv@daocloud.io

@lvyanru8200 lvyanru8200 requested a review from a team as a code owner January 31, 2023 14:21
@lvyanru8200 lvyanru8200 requested review from stevesloka and skriss and removed request for a team January 31, 2023 14:21
@github-actions
Copy link

Hi @lvyanru8200! Welcome to our community and thank you for opening your first Pull Request. Someone will review it soon. Thank you for committing to making Contour better. You can also join us on our mailing list and in our channel in the Kubernetes Slack Workspace

@lvyanru8200 lvyanru8200 changed the title feat: Add AllowPrivateNetwork to support Access-Control-Allow-Private… feat: Add AllowPrivateNetwork to support Access-Control-Allow-Privateeee Jan 31, 2023
@lvyanru8200 lvyanru8200 changed the title feat: Add AllowPrivateNetwork to support Access-Control-Allow-Privateeee feat: Add AllowPrivateNetwork to support Access-Control-Allow-Private-Network Jan 31, 2023
@lvyanru8200 lvyanru8200 reopened this Jan 31, 2023
@lvyanru8200
Copy link
Contributor Author

cc @skriss @stevesloka

@skriss skriss added the release-note/small A small change that needs one line of explanation in the release notes. label Feb 1, 2023
@skriss
Copy link
Member

skriss commented Feb 1, 2023

Thanks for the PR @lvyanru8200! Please add a changelog file per the instructions in https://github.com/projectcontour/contour/actions/runs/4068330638/jobs/7006764495. You'll also need to run make generate and commit the changed files.

skriss
skriss requested changes Feb 1, 2023
View reviewed changes
Copy link
Member

@skriss skriss left a comment

Choose a reason for hiding this comment

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

Mostly looks good here, just a couple minor comments.

internal/envoy/v3/route.go Show resolved Hide resolved
internal/featuretests/v3/corspolicy_test.go Outdated Show resolved Hide resolved
@lvyanru8200
Copy link
Contributor Author

cc @skriss

@skriss
Copy link
Member

skriss commented Feb 2, 2023

Looks like a few more failing unit tests to update.

@lvyanru8200
Copy link
Contributor Author

cc @skriss

@codecov
Copy link

codecov bot commented Feb 3, 2023 •
edited
Loading

Codecov Report

Merging #5034 (ab7f2c4) into main (183a76b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head ab7f2c4 differs from pull request most recent head 0168d59. Consider uploading reports for the commit 0168d59 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #5034   +/-   ##
=======================================
  Coverage   77.47%   77.48%           
=======================================
  Files         138      138           
  Lines       16904    16906    +2     
=======================================
+ Hits        13097    13100    +3     
+ Misses       3552     3551    -1     
  Partials      255      255
Impacted Files Coverage Δ
internal/dag/dag.go 96.62% <ø> (ø)
internal/dag/httpproxy_processor.go 92.51% <100.00%> (+<0.01%) ⬆️
internal/envoy/v3/route.go 75.87% <100.00%> (+0.03%) ⬆️
internal/sorter/sorter.go 98.46% <0.00%> (+0.51%) ⬆️

skriss
skriss requested changes Feb 3, 2023
View reviewed changes
Copy link
Member

@skriss skriss left a comment

Choose a reason for hiding this comment

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

Code LGTM. The last thing is to update the docs (https://github.com/projectcontour/contour/blame/main/site/content/docs/main/config/cors.md)

I think mentioning this in https://github.com/projectcontour/contour/blame/main/site/content/docs/main/config/cors.md#L7, plus adding it to one of the YAML examples with a sentence or comment explaining it, should be sufficient.

@lvyanru8200
feat: Add AllowPrivateNetwork to support Access-Control-Allow-Private…
0168d59 
…-Network

Fixes: projectcontour#4553
Signed-off-by: tigerK <yanru.lv@daocloud.io>
@lvyanru8200
Copy link
Contributor Author

cc @skriss

skriss
skriss approved these changes Feb 7, 2023
View reviewed changes
Copy link
Member

@skriss skriss left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @lvyanru8200.

@skriss skriss merged commit b58e470 into projectcontour:main Feb 7, 2023
yangyy93 pushed a commit to projectsesame/contour that referenced this pull request Feb 16, 2023
@lvyanru8200 @yangyy93
HTTPProxy: Add AllowPrivateNetwork to CORSPolicy (projectcontour#5034)
c1b604f 
Adds an AllowPrivateNetwork field to HTTPProxy's CORSPolicy
to allow setting the Access-Control-Allow-Private-Network header.

Closes projectcontour#4553.

Signed-off-by: tigerK <yanru.lv@daocloud.io>
Signed-off-by: yy <yang.yang@daocloud.io>
yangyy93 pushed a commit to projectsesame/contour that referenced this pull request Feb 16, 2023
@lvyanru8200 @yangyy93
HTTPProxy: Add AllowPrivateNetwork to CORSPolicy (projectcontour#5034)
571e2eb 
Adds an AllowPrivateNetwork field to HTTPProxy's CORSPolicy
to allow setting the Access-Control-Allow-Private-Network header.

Closes projectcontour#4553.

Signed-off-by: tigerK <yanru.lv@daocloud.io>
Signed-off-by: yy <yang.yang@daocloud.io>
vmw-yingy pushed a commit to vmw-yingy/contour that referenced this pull request Feb 28, 2023
@lvyanru8200 @vmw-yingy
HTTPProxy: Add AllowPrivateNetwork to CORSPolicy (projectcontour#5034)
648f912 
Adds an AllowPrivateNetwork field to HTTPProxy's CORSPolicy
to allow setting the Access-Control-Allow-Private-Network header.

Closes projectcontour#4553.

Signed-off-by: tigerK <yanru.lv@daocloud.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skriss skriss skriss approved these changes

@stevesloka stevesloka Awaiting requested review from stevesloka stevesloka 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.

CORS Access-Control-Allow-Private-Network
2 participants
@lvyanru8200 @skriss