Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

gateway-api: update to v0.5.0 and v1beta1 resources #224

Merged
merged 17 commits into from
Jul 1, 2022

Conversation

mikemorris
Copy link
Contributor

@mikemorris mikemorris commented Jun 13, 2022

Changes proposed in this PR:

  • Update to gateway-api@master
    • This should be updated to gateway-api@v0.5.0-rc1 when available
    • Before merging, this should be updated to gateway-api@v0.5.0
      • This is anticipated to happen within the next two weeks and can happen in a followup PR to avoid this getting too stale
  • Update all v1alpha1 references to v1alpha2 if not graduated to v1beta1 yet
    • N/A, all the v1alpha1 references I had noticed while skimming through were actually for "github.com/hashicorp/consul-api-gateway/pkg/apis/v1alpha1"
  • Update to graduated v1beta1 resources
    • Deferring v1beta1 for HTTPRoute for now due to the complications of shared logic with other route types still on v1alpha2
  • Enable support for deprecated ReferencePolicy

How I've tested this PR:

  • Code compiles, tests are passing

Notes for reviewers:

  • The go.mod updates to k8s.io modules are due to deps: update k8s.io modules to v0.24 kubernetes-sigs/gateway-api#1199 which switches from googleapis/gnostic to google/gnostic - this was handled in Update kubectl kustomize to v4.5.4 kubernetes/kubernetes#108994 to resolve the dependency conflict, but requires updating all dependencies in lockstep
  • The internal/k8s/logger.go changes are to adapt to the breaking change in Rearch for performance: BREAKING CHANGES go-logr/logr#42 hit due to the dependency bump to go-logr/logr from updating the Kubernetes ecosystem libraries
  • Rebased and reordered so this should be relatively reasonable to review by commit now.
  • I may attempt to migrate HTTPRoute to v1beta1 in a future PR, but that was significantly complicating this PR due to mixed versioned types in helper function arguments and returns (e.g. returning either v1beta1.CommonRouteSpec or v1alpha2.CommonRouteSpec depending on the route type) - wanted to keep that out of scope for now to avoid excessive complexity/refactoring.

How I expect reviewers to test this PR:

Verify that the changes seems sensible and nothing was missed.

Checklist:

  • Tests added for ReferencePolicy backwards compatibility support
    • Gateway controller certificateRefs
    • HTTPRoute controller
    • TCPRoute controller
    • e2e
  • CHANGELOG entry added

    Run make changelog-entry for guidance in authoring a changelog entry, and
    commit the resulting file, which should have a name matching your PR number.
    Entries should use imperative present tense (e.g. Add support for...)

@mikemorris
Copy link
Contributor Author

mikemorris commented Jun 16, 2022

Moving internal/k8s/reconciler/route.go and internal/k8s/service/resolver.go is looking like it's gonna be a bit of extra work (possibly using generics, hopefully avoiding too much copy/pasting) due to how we have quite a bit of shared functionality that will need to work with both v1beta1 shared types (like BackendRef and ParentRefernce) for v1beta1.HTTPRoute and v1alpha2 shared types for v1alpha2.TCPRoute 😢

@mikemorris mikemorris force-pushed the gateway-api/v0.5-v1beta1 branch 7 times, most recently from bc5ce1f to 40c4b64 Compare June 20, 2022 16:09
@@ -2,6 +2,7 @@ apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
- github.com/kubernetes-sigs/gateway-api/config/crd?ref=v0.4.1
- github.com/kubernetes-sigs/gateway-api/config/crd/experimental?ref=v0.5.0-rc1
- https://raw.githubusercontent.com/kubernetes-sigs/gateway-api/v0.5.0-rc1/config/crd/experimental/gateway.networking.k8s.io_referencepolicies.yaml
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed to be installed explicitly because kustomize seems to omit the ReferencePolicies CRD by default due to it being marked deprecated: true. Remote resource install refs kubernetes-sigs/kustomize#970 (comment)

@mikemorris mikemorris marked this pull request as ready for review June 20, 2022 21:48
@mikemorris mikemorris requested a review from a team June 20, 2022 21:48
@mikemorris mikemorris added do not merge theme/k8s-gateway-api Related to the Kubernetes Gateway API standard labels Jun 20, 2022
Copy link
Member

@nathancoleman nathancoleman left a comment

Choose a reason for hiding this comment

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

This is looking good! Would like to get merged this week to let it bake for as long as possible while we build up to our v0.4.0 release.

I haven't done so yet but want to make sure we have a ticket for upgrading HTTPRoute to v1beta1. It seems like something we could easily overlook ahead of our release if we don't, and I'd like to be as up-to-date as possible with kubernetes-sigs/gateway-api by our next release.

.changelog/224.txt Show resolved Hide resolved
.changelog/224.txt Outdated Show resolved Hide resolved
.changelog/224.txt Outdated Show resolved Hide resolved
uses: actions/checkout@v2
with:
repository: "hashicorp/consul-k8s"
ref: "charts/capigw-controller-clusterrole-referencegrants"
Copy link
Member

Choose a reason for hiding this comment

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

Note: this branch is from hashicorp/consul-k8s#1299

Copy link
Member

Choose a reason for hiding this comment

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

Since the PR above is so small, it'd be nice if we could merge it first and then have this PR reference main.

Copy link
Member

Choose a reason for hiding this comment

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

Closing the loop: hashicorp/consul-k8s#1299 (comment) resulted in the conclusion that we don't want to merge the above PR just yet.

Are you thinking we move ahead with this PR and just leave the with.ref pointed at the branch above @mikemorris ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, I think that could be reasonable to avoid this getting stale, will open an issue tracking that so we don't forget.

.github/workflows/conformance.yml Show resolved Hide resolved
dev/run Show resolved Hide resolved
internal/k8s/utils/reference.go Show resolved Hide resolved
@mikemorris
Copy link
Contributor Author

mikemorris commented Jun 29, 2022

mikemorris and others added 9 commits June 29, 2022 17:22
k8s/controllers: minor refactor and mocking up future ReferencePolicy support for
backwards compatibility

k8s/gatewayclient: regenerate mocks after ReferenceGrant rename

docs: update supported-features.md for ReferenceGrant

k8s/controller: update http and tcp route controller tests for ReferenceGrant

k8s/reconciler: update listener test for ReferenceGrant

k8s/reconciler: update route test for ReferenceGrant

e2e: update ReferencePolicy -> ReferenceGrant

k8s/reconciler: update utils test for ReferenceGrant

k8s/reconciler: update statuses for ReferenceGrant

rbac: add gateway controller permissions to read ReferenceGrants
…tes and ReferenceGrant

gateway-api: add ReferencePolicy backwards compatibility support

mocks: regenerate

k8s/controller: add gwv1alpha2 to scheme

k8s/gatewayclient: add gwv1alpha2 to scheme for test helpers
Copy link
Member

@nathancoleman nathancoleman left a comment

Choose a reason for hiding this comment

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

Just the one last thing; otherwise LGTM

.github/workflows/conformance.yml Outdated Show resolved Hide resolved
@mikemorris mikemorris merged commit 38dc955 into main Jul 1, 2022
@mikemorris mikemorris deleted the gateway-api/v0.5-v1beta1 branch July 1, 2022 16:39
@mikemorris mikemorris mentioned this pull request Jul 1, 2022
2 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
do not merge theme/k8s-gateway-api Related to the Kubernetes Gateway API standard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants