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

Bump gateway-api to v1.1.0 #6398

Merged

Conversation

sunjayBhatia
Copy link
Member

Fixes: #6397

@sunjayBhatia sunjayBhatia added area/gateway-api Issues or PRs related to the Gateway (Gateway API working group) API. release-note/minor A minor change that needs about a paragraph of explanation in the release notes. labels May 1, 2024
Copy link

codecov bot commented May 1, 2024

Codecov Report

Attention: Patch coverage is 91.20879% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 81.61%. Comparing base (c07a0ba) to head (e25e4e5).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6398      +/-   ##
==========================================
- Coverage   81.63%   81.61%   -0.03%     
==========================================
  Files         133      133              
  Lines       15862    15858       -4     
==========================================
- Hits        12949    12942       -7     
- Misses       2619     2621       +2     
- Partials      294      295       +1     
Files Coverage Δ
internal/dag/cache.go 97.00% <100.00%> (-0.03%) ⬇️
internal/dag/policy.go 95.52% <100.00%> (ø)
internal/gatewayapi/helpers.go 88.18% <100.00%> (ø)
internal/k8s/helpers.go 57.14% <100.00%> (ø)
internal/k8s/kind.go 65.21% <100.00%> (ø)
internal/provisioner/controller/gatewayclass.go 71.13% <ø> (ø)
internal/provisioner/objects/rbac/util/util.go 100.00% <100.00%> (ø)
internal/provisioner/scheme.go 83.33% <100.00%> (ø)
internal/status/backendtlspolicyconditions.go 91.37% <100.00%> (ø)
internal/status/cache.go 93.22% <100.00%> (ø)
... and 4 more

... and 1 file with indirect coverage changes

@sunjayBhatia
Copy link
Member Author

Looks like HTTPRouteHTTPSListener is failing, it expects a 404 for request to a HTTPS listener with an unknown hostname

Seems like we should either push back on this (expect a TLS error instead) or we'll have to ensure a fallback certificate is configured and enabled so we can terminate TLS and return an HTTP response

@sunjayBhatia
Copy link
Member Author

Looks like HTTPRouteHTTPSListener is failing, it expects a 404 for request to a HTTPS listener with an unknown hostname

Seems like we should either push back on this (expect a TLS error instead) or we'll have to ensure a fallback certificate is configured and enabled so we can terminate TLS and return an HTTP response

hm looks like i approved this test and it passed before, will have to take another look into why its failing in CI

@sunjayBhatia
Copy link
Member Author

sunjayBhatia commented May 1, 2024

Looks like HTTPRouteHTTPSListener is failing, it expects a 404 for request to a HTTPS listener with an unknown hostname
Seems like we should either push back on this (expect a TLS error instead) or we'll have to ensure a fallback certificate is configured and enabled so we can terminate TLS and return an HTTP response

hm looks like i approved this test and it passed before, will have to take another look into why its failing in CI

looks like i approved before the negative hostname test was added and didnt re-check it later

opened kubernetes-sigs/gateway-api#3044

@sunjayBhatia
Copy link
Member Author

Looks like HTTPRouteHTTPSListener is failing, it expects a 404 for request to a HTTPS listener with an unknown hostname
Seems like we should either push back on this (expect a TLS error instead) or we'll have to ensure a fallback certificate is configured and enabled so we can terminate TLS and return an HTTP response

hm looks like i approved this test and it passed before, will have to take another look into why its failing in CI

looks like i approved before the negative hostname test was added and didnt re-check it later

opened kubernetes-sigs/gateway-api#3044

realized this is probably a contour deficiency, I don't think we generate a filter chain if there is no httproute attached to a listener

@sunjayBhatia
Copy link
Member Author

upgrade test probably failing since we started reconciling v1alpha3 backendtlspolicy and that has not been applied to the cluster as part of the upgrade

@sunjayBhatia
Copy link
Member Author

Listener isolation test failing, needs this fix: #6162

@sunjayBhatia
Copy link
Member Author

disabled backendtlspolicy feature in provisioner upgrade test since the upgrade from v1.0.0 to v1.1.0 of gw api removes the old api version and adds v1alpha3 of the resource

i tried instead to install the newer crds as part of the upgrade but the grpcroute crd going to v1 also causes some issues, could be more surgical but this felt easier for now

@sunjayBhatia sunjayBhatia force-pushed the bump-gateway-api-v1.1.0 branch 3 times, most recently from a344c67 to 24a3040 Compare May 6, 2024 18:43
@sunjayBhatia
Copy link
Member Author

disabled backendtlspolicy feature in provisioner upgrade test since the upgrade from v1.0.0 to v1.1.0 of gw api removes the old api version and adds v1alpha3 of the resource

i tried instead to install the newer crds as part of the upgrade but the grpcroute crd going to v1 also causes some issues, could be more surgical but this felt easier for now

nvm, just fixed the crd install as part of the upgrade

found a bug when setting multiple disabled features in the provisioner

@sunjayBhatia sunjayBhatia force-pushed the bump-gateway-api-v1.1.0 branch from 24a3040 to 6b66aa2 Compare May 7, 2024 19:55
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.

Looks pretty good, just a couple nits so far, still need to review the BackendTLSPolicy changes in a little more detail but not too concerned about it.

internal/gatewayapi/helpers.go Outdated Show resolved Hide resolved
internal/gatewayapi/helpers.go Outdated Show resolved Hide resolved
test/conformance/gatewayapi/gateway_conformance_test.go Outdated Show resolved Hide resolved
Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
so we can ignore the gateway crds and other failing files for now

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
@sunjayBhatia sunjayBhatia force-pushed the bump-gateway-api-v1.1.0 branch from 6b66aa2 to 5b88d5a Compare May 8, 2024 21:16
Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
@sunjayBhatia sunjayBhatia marked this pull request as ready for review May 9, 2024 16:26
@sunjayBhatia sunjayBhatia requested a review from a team as a code owner May 9, 2024 16:26
@sunjayBhatia sunjayBhatia requested review from tsaarni, skriss, a team, rajatvig and izturn and removed request for a team May 9, 2024 16:26
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, just a couple typos

changelogs/unreleased/6398-sunjayBhatia-minor.md Outdated Show resolved Hide resolved
internal/dag/cache.go Outdated Show resolved Hide resolved
Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
@sunjayBhatia sunjayBhatia enabled auto-merge (squash) May 9, 2024 17:09
@sunjayBhatia sunjayBhatia merged commit bd33dfc into projectcontour:main May 9, 2024
26 checks passed
@sunjayBhatia sunjayBhatia deleted the bump-gateway-api-v1.1.0 branch May 9, 2024 17:46
SamMHD pushed a commit to SamMHD/contour that referenced this pull request Sep 8, 2024
- update go module dependency and regenerate CRDs
- updates to reconciling GRPCRoute v1
- v1alpha2 of BackendTLSPolicy has been removed, update to reconcile v1alpha3
- disable a couple new conformance tests which will be fixed in subsequent PRs
- provisioner now sets supported CRD version as v1.1.0

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Signed-off-by: Saman Mahdanian <saman@mahdanian.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/gateway-api Issues or PRs related to the Gateway (Gateway API working group) API. release-note/minor A minor change that needs about a paragraph of explanation in the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bump Gateway API support to v1.1.0
2 participants