bump controller-runtime and k8s deps

[tsaarni]

trafficstars

sigs.k8s.io/controller-runtime v0.19.0 includes a breaking change that necessitates updating it alongside k8s.io/* v1.31.

sigs.k8s.io/controller-tools v0.16.x needs to be updated for k8s.io/* v1.31 compatibility, which also prompted the re-generation of CRDs.

This update consolidates the previously separate PRs #6623 and #6624.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 81.01%. Comparing base (e5a25e2) to head (2c1946c). Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6634      +/-   ##
==========================================
- Coverage   81.76%   81.01%   -0.76%     
==========================================
  Files         133      133              
  Lines       15944    19996    +4052     
==========================================
+ Hits        13037    16200    +3163     
- Misses       2614     3503     +889     
  Partials      293      293              

see 127 files with indirect coverage changes

[tsaarni]

I believe the RBAC rules were changed by make generate because of deduplication done by controller-tools v0.16.0 kubernetes-sigs/controller-tools#937.

[tsaarni]

There is still an additional hurdle with the version bump. Following change in controller-tools v0.16.0

✨ add support for kubernetes +required annotations https://github.com/kubernetes-sigs/controller-tools/pull/944

This change appropriately triggers the generation of new "required" entries in CRDs when +required is specified in the type definition.

In our API, we have a +required field in ExtensionServiceReference https://github.com/projectcontour/contour/blob/20e57df13892996163d185be60929296b0843ffb/apis/projectcontour/v1/httpproxy.go#L238-L240

ExtensionServiceReference is used in AuthorizationServer as an +optional field but it is embedded as a struct rather than a pointer
https://github.com/projectcontour/contour/blob/20e57df13892996163d185be60929296b0843ffb/apis/projectcontour/v1/httpproxy.go#L249-L250

Now consider how AuthorizationServer is used in the Go code https://github.com/projectcontour/contour/blob/20e57df13892996163d185be60929296b0843ffb/test/e2e/httpproxy/global_external_auth_test.go#L234-L238

Even though the optional HTTPProxy.Spec.VirtualHost.Authorization.ExtensionServiceRef is never explicitly set, it gets included in the request since it's embedded in the AuthorizationServer struct. The omitempty tag does not prevent this since the field should have been a pointer for omitempty to work as intended. As a result, ExtensionServiceReference.Name is initialized with a nil value, and the newly introduced +required validation triggers, causing configuration to fail.

https://github.com/projectcontour/contour/actions/runs/10536218743/job/29196551673?pr=6634

  	Error:      	Received unexpected error:
  	            	HTTPProxy.projectcontour.io "external-auth" is invalid: [spec.virtualhost.authorization.extensionRef.name: Required value, <nil>: Invalid value: "null": some validation rules were not checked because the object was invalid; correct the existing errors to complete validation]
  	Test:       	HTTPProxy global external auth with namespace: httpproxy-global-ext-auth-non-tls-disabled with global external auth service global external auth can be disabled on a non TLS HTTPProxy

[tsaarni]

There are several other types that also use +required, but only ExtensionServiceReference.Name is causing the test suite to fail.

I believe backwards compatible fix would be to remove the +required from ExtensionServiceReference.Name. Although it seems illogical to reference to a non-existing resource, this approach shouldn't be any worse than previous state, since +required has never actually been enforced by the CRD.

Another option would be to change HTTPProxy.Spec.VirtualHost.Authorization.ExtensionServiceRef to a pointer, but that could potentially break external Go code relying on the API.

Any thoughts?

[sunjayBhatia]

There are several other types that also use +required, but only ExtensionServiceReference.Name is causing the test suite to fail.

I believe backwards compatible fix would be to remove the +required from ExtensionServiceReference.Name. Although it seems illogical to reference to a non-existing resource, this approach shouldn't be any worse than previous state, since +required has never actually been enforced by the CRD.

Another option would be to change HTTPProxy.Spec.VirtualHost.Authorization.ExtensionServiceRef to a pointer, but that could potentially break external Go code relying on the API.

Any thoughts?

Looks like we missed out making the change to a pointer in https://github.com/projectcontour/contour/pull/4994

Since it is a v1 type seems like we should go the conservative approach here and do the former

[tsaarni]

Since it is a v1 type seems like we should go the conservative approach here and do the former

I agree. I've removed the +required annotation from ExtensionServiceReference.Name so that it will be treated as optional again, like it was before controller-tools v0.16.0.