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

internal/dag: Gateway Listeners with invalid references should have reason RefNotPermitted #4664

Conversation

sunjayBhatia
Copy link
Member

See kubernetes-sigs/gateway-api#1305

Fixes conformance test failures on main

…eason RefNotPermitted

See kubernetes-sigs/gateway-api#1305

Fixes conformance test failures on main

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
@sunjayBhatia sunjayBhatia added the release-note/none-required Marks a PR as not requiring a release note. Should only be used for very small changes. label Aug 10, 2022
@sunjayBhatia sunjayBhatia requested a review from a team as a code owner August 10, 2022 16:25
@sunjayBhatia sunjayBhatia requested review from tsaarni and stevesloka and removed request for a team August 10, 2022 16:25
@codecov
Copy link

codecov bot commented Aug 10, 2022

Codecov Report

Merging #4664 (c5dafb3) into main (fc49f3b) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4664      +/-   ##
==========================================
- Coverage   76.13%   76.11%   -0.02%     
==========================================
  Files         140      140              
  Lines       13093    13093              
==========================================
- Hits         9968     9966       -2     
- Misses       2869     2871       +2     
  Partials      256      256              
Impacted Files Coverage Δ
internal/dag/gatewayapi_processor.go 94.16% <100.00%> (ø)
internal/sorter/sorter.go 97.59% <0.00%> (-1.21%) ⬇️

@sunjayBhatia sunjayBhatia added release-note/small A small change that needs one line of explanation in the release notes. and removed release-note/none-required Marks a PR as not requiring a release note. Should only be used for very small changes. labels Aug 10, 2022
@sunjayBhatia
Copy link
Member Author

this is one of those changes where we have a conflict between main of Contour + the latest GW API conformance release

@skriss
Copy link
Member

skriss commented Aug 15, 2022

this is one of those changes where we have a conflict between main of Contour + the latest GW API conformance release

Yeah, I guess this brings up a new issue here - if we merge this, then main Gateway conformance will pass, but PR Gateway conformance will start failing since this is not compatible with the v0.5.0 conformance tests.

Perhaps it makes sense to hold this PR open until the upstream conformance change is included in a released version (hopefully v0.5.1)?

@sunjayBhatia
Copy link
Member Author

this is one of those changes where we have a conflict between main of Contour + the latest GW API conformance release

Yeah, I guess this brings up a new issue here - if we merge this, then main Gateway conformance will pass, but PR Gateway conformance will start failing since this is not compatible with the v0.5.0 conformance tests.

Perhaps it makes sense to hold this PR open until the upstream conformance change is included in a released version (hopefully v0.5.1)?

yeah, was thinking that would probably have to be the case

submitted: kubernetes-sigs/gateway-api#1328 to start the conversation

…efnotpermitted

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
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

…efnotpermitted

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
@sunjayBhatia sunjayBhatia merged commit 6e6099a into projectcontour:main Aug 25, 2022
@sunjayBhatia sunjayBhatia deleted the gateway-conformance-refnotpermitted branch August 25, 2022 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/small A small change that needs one line of explanation in the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants