-
Notifications
You must be signed in to change notification settings - Fork 634
Fix incorrect Kubebuilder validation markers #4172
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
Fix incorrect Kubebuilder validation markers #4172
Conversation
* Improve some ListenetSet gep descriptions * Add examples of conflicting ListenerSets * Fix some review findings * Fix some comments on GEP * Make the behavior on route attachement explicit * Fix some reviews * Use XListenerSet on example kinds * Final fix on listenerset gep * Fix some more reviews * Add clarification on ListenerSet attachment
* Switch to registry.k8s.io/coredns/coredns, only pull if not present * Properly support multiple embedded manifests
Copy: apis/v1alpha3/backendtlspolicy_types.go -> apis/v1/backendtlspolicy_types.go apis/v1alpha2/policy_types.go -> apis/v1/policy_types.go Update: apis/v1alpha2/policy_types.go apis/v1alpha3/backendtlspolicy_types.go config/crd/kustomization.yaml conformance/tests/backendtlspolicy-conflict-resolution.go conformance/tests/backendtlspolicy-conflict-resolution.yaml conformance/tests/backendtlspolicy-invalid-ca-certificate-ref.go conformance/tests/backendtlspolicy-invalid-ca-certificate-ref.yaml conformance/tests/backendtlspolicy-invalid-kind.go conformance/tests/backendtlspolicy-invalid-kind.yaml conformance/tests/backendtlspolicy-observed-generation-bump.go conformance/tests/backendtlspolicy-observed-generation-bump.yaml conformance/tests/backendtlspolicy-san.go conformance/tests/backendtlspolicy-san.yaml conformance/tests/backendtlspolicy.go conformance/tests/backendtlspolicy.yaml conformance/utils/kubernetes/helpers.go examples/standard/backendtlspolicy/backendtlspolicy-ca-certs.yaml examples/standard/backendtlspolicy/backendtlspolicy-system-certs.yaml pkg/features/backendtlspolicy.go pkg/generator/main.go pkg/test/cel/backendtlspolicy_test.go site-src/guides/tls.md Delete: examples/experimental/v1alpha3/backendtlspolicy-ca-certs.yaml examples/experimental/v1alpha3/backendtlspolicy-system-certs.yaml Regenerate the remaining files: rm -rf pkg/client make generate hack/../hack/verify-golint.sh (if needed)
…rform cleanup (kubernetes-sigs#4094) Signed-off-by: Norwin Schnyder <[email protected]>
Removed mlavacca from gateway-api-maintainers and added to gateway-api-mesh-leads.
* Fix GEP API and geps metadata The GEP API is using a wrong structure/tag name for some fields, so this PR fixes the markers. Additionally the metadata.yaml files from some GEPs are fixed to reflect the right GEP API structure * Implement automatic NAV generation This change makes the navigation section of GW API website to be generated. The modification adds a new Go program that is able to transverse the GEP directory and generate a navigation section correctly from the existing GEPs. Additionally, some scripts and Makefiles are added to verify if the generated nav.yml file reflects the current state of GEPs, a Github Action that fails in case nav.yml is outdated * Add the new generated nav file
Signed-off-by: Shane Utt <[email protected]>
Signed-off-by: Shane Utt <[email protected]>
Signed-off-by: Shane Utt <[email protected]>
Bumps the k8s-io group with 5 updates: | Package | From | To | | --- | --- | --- | | [k8s.io/api](https://github.com/kubernetes/api) | `0.34.0` | `0.34.1` | | [k8s.io/apiextensions-apiserver](https://github.com/kubernetes/apiextensions-apiserver) | `0.34.0` | `0.34.1` | | [k8s.io/apimachinery](https://github.com/kubernetes/apimachinery) | `0.34.0` | `0.34.1` | | [k8s.io/client-go](https://github.com/kubernetes/client-go) | `0.34.0` | `0.34.1` | | [k8s.io/code-generator](https://github.com/kubernetes/code-generator) | `0.34.0` | `0.34.1` | Updates `k8s.io/api` from 0.34.0 to 0.34.1 - [Commits](kubernetes/api@v0.34.0...v0.34.1) Updates `k8s.io/apiextensions-apiserver` from 0.34.0 to 0.34.1 - [Release notes](https://github.com/kubernetes/apiextensions-apiserver/releases) - [Commits](kubernetes/apiextensions-apiserver@v0.34.0...v0.34.1) Updates `k8s.io/apimachinery` from 0.34.0 to 0.34.1 - [Commits](kubernetes/apimachinery@v0.34.0...v0.34.1) Updates `k8s.io/client-go` from 0.34.0 to 0.34.1 - [Changelog](https://github.com/kubernetes/client-go/blob/master/CHANGELOG.md) - [Commits](kubernetes/client-go@v0.34.0...v0.34.1) Updates `k8s.io/code-generator` from 0.34.0 to 0.34.1 - [Commits](kubernetes/code-generator@v0.34.0...v0.34.1) --- updated-dependencies: - dependency-name: k8s.io/api dependency-version: 0.34.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: k8s-io - dependency-name: k8s.io/apiextensions-apiserver dependency-version: 0.34.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: k8s-io - dependency-name: k8s.io/apimachinery dependency-version: 0.34.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: k8s-io - dependency-name: k8s.io/client-go dependency-version: 0.34.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: k8s-io - dependency-name: k8s.io/code-generator dependency-version: 0.34.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: k8s-io ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
kubernetes-sigs#4086) Bumps [sigs.k8s.io/controller-runtime](https://github.com/kubernetes-sigs/controller-runtime) from 0.21.0 to 0.22.1. - [Release notes](https://github.com/kubernetes-sigs/controller-runtime/releases) - [Changelog](https://github.com/kubernetes-sigs/controller-runtime/blob/main/RELEASE.md) - [Commits](kubernetes-sigs/controller-runtime@v0.21.0...v0.22.1) --- updated-dependencies: - dependency-name: sigs.k8s.io/controller-runtime dependency-version: 0.22.1 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Update GEP template with some clarifying sections * Apply suggestions on GEP template update Co-authored-by: Nick Young <[email protected]> --------- Co-authored-by: Nick Young <[email protected]>
… in GWC Status. (kubernetes-sigs#4116) * Removing experimental annotation from supportedFeatures in GWC Status. * Run make generate to update generated files --------- Co-authored-by: Beka Modebadze <[email protected]>
…portedFeatures (kubernetes-sigs#4118) * fix: use inferred supported features to set extendedSupportedFeatures Signed-off-by: Norwin Schnyder <[email protected]> * extend unit tests with assertion for extendedSupportedFeatures Signed-off-by: Norwin Schnyder <[email protected]> * refactor NewConformanceTestSuite Signed-off-by: Norwin Schnyder <[email protected]> --------- Signed-off-by: Norwin Schnyder <[email protected]> Co-authored-by: Norwin Schnyder <[email protected]>
…est (kubernetes-sigs#4117) * conformance: fix invalid BackendTLSPolicy conformance test * Move to a real one --------- Co-authored-by: John Howard <[email protected]>
…atically inferring supported features from Mesh.Status (kubernetes-sigs#4124) * Read SupportedFeatures from XMesh status for conformance test. * Defined Mesh flag for testing. * Renamed mesh flag. * renamed fetched mesh features set. * suite_test formatting. * Set mesh flag empty default value --------- Co-authored-by: Beka Modebadze <[email protected]>
…netes-sigs#4125) Bumps [mkdocs-mermaid2-plugin](https://github.com/fralau/mkdocs-mermaid2-plugin) from 1.2.1 to 1.2.2. - [Release notes](https://github.com/fralau/mkdocs-mermaid2-plugin/releases) - [Changelog](https://github.com/fralau/mkdocs-mermaid2-plugin/blob/master/CHANGELOG.md) - [Commits](fralau/mkdocs-mermaid2-plugin@v1.2.1...v1.2.2) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…netes-sigs#4127) Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.75.0 to 1.75.1. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](grpc/grpc-go@v1.75.0...v1.75.1) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…igs#4128) Bumps [mkdocs-material](https://github.com/squidfunk/mkdocs-material) from 9.6.18 to 9.6.20. - [Release notes](https://github.com/squidfunk/mkdocs-material/releases) - [Changelog](https://github.com/squidfunk/mkdocs-material/blob/master/CHANGELOG) - [Commits](squidfunk/mkdocs-material@9.6.18...9.6.20) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…netes-sigs#4126) Updates the requirements on [markdown](https://github.com/Python-Markdown/markdown) to permit the latest version. - [Release notes](https://github.com/Python-Markdown/markdown/releases) - [Changelog](https://github.com/Python-Markdown/markdown/blob/master/docs/changelog.md) - [Commits](Python-Markdown/markdown@3.8...3.9.0) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…ubernetes-sigs#4129) Bumps [sigs.k8s.io/controller-tools](https://github.com/kubernetes-sigs/controller-tools) from 0.18.0 to 0.19.0. - [Release notes](https://github.com/kubernetes-sigs/controller-tools/releases) - [Changelog](https://github.com/kubernetes-sigs/controller-tools/blob/main/envtest-releases.yaml) - [Commits](kubernetes-sigs/controller-tools@v0.18.0...v0.19.0) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* chore: bump to v1.4.0-rc.2 Signed-off-by: Shane Utt <[email protected]> * docs: add the v1.4.0-rc.2 CHANGELOG entry Signed-off-by: Shane Utt <[email protected]> --------- Signed-off-by: Shane Utt <[email protected]>
otherwise we send requests to `http://[fc00/backendtlspolicy-nonexistent-ca-certificate-ref"` which is very wrong Co-authored-by: John Howard <[email protected]>
Signed-off-by: Shane Utt <[email protected]>
|
Thanks for the feedback @youngnick @robscott! I've addressed the port validation concerns. Problem: So i've:
The Result:
This passes controller-gen validation while preserving the port range semantics. Let me know if you'd like any changes to this approach! |
|
/ok-to-test |
|
@robscott @youngnick @shaneutt worth noticing this is a PR pointing to release-1.4 branch. I think we should:
There may be some breaking changes (maxItems being fixed...) |
|
@KillianGolds: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Keywords which can automatically close issues and at(@) or hashtag(#) mentions are not allowed in commit messages. The list of commits with invalid commit messages:
DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
I am closing this in favor of #4178 and then we cherry-pick it back to release-1.4 Thanks! |
|
@rikatz: Closed this PR. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
What type of PR is this?
/kind bug
/kind cleanup
What this PR does / why we need it:
This PR fixes several incorrect kubebuilder validation markers in the gateway-api repository, similar to the fixes made in kubernetes-sigs/gateway-api-inference-extension#1679.
Background:
These validation errors were discovered while migrating KServe to use gateway-api-inference-extension (GIE) v1. During that migration work, running
controller-gen crdexposed validation marker issues in the GIE repository, which were fixed and merged in kubernetes-sigs/gateway-api-inference-extension#1679.After that fix was merged, continuing the KServe migration work revealed similar validation errors in the gateway-api repository itself. These errors affect downstream consumers like KServe who run
controller-tools genas part of their build process.The reason these issues haven't been caught previously is that gateway-api wasn't running
controller-gen crdvalidation as part of its build process. This PR both fixes the existing validation errors and adds the validation step to prevent future issues.Changes:
Removed
+listType=atomicfrom non-array fields (2 instances)apis/v1/gateway_types.go:806-Namespaces *RouteNamespaces(pointer to struct, not a slice)apis/v1/backendtlspolicy_types.go:192-WellKnownCACertificates *WellKnownCACertificatesType(pointer to single value, not a slice)The
+listTypemarker can only be applied to slice/array types, not pointers to structs.Ref: Kubebuilder CRD Processing
Changed
MaxLengthtoMaxItemsfor array fields (3 instances)apis/v1/httproute_types.go:1674-AllowedRequestHeaders []stringinGRPCAuthConfigapis/v1/httproute_types.go:1722-AllowedRequestHeaders []stringinHTTPAuthConfigapis/v1/httproute_types.go:1733-AllowedResponseHeaders []stringinHTTPAuthConfigMaxLengthcan only be applied to string types, not arrays. For arrays,MaxItemsmust be used instead.Ref: Kubebuilder CRD Validation, controller-tools validation.go
Moved port validation to type level for XListenerSet (2 instances)
apisx/v1alpha1/shared_types.go- DefinedPortNumber(0–65535) andStatusPortNumber(1–65535) locallyapisx/v1alpha1/xlistenerset_types.go:255- Changed status field to useStatusPortNumberThe original Minimum/Maximum markers on
PortNumberfields caused controller-gen errors becausev1alpha1.PortNumberwas type-aliased fromv1.PortNumber, creating a chain that controller-gen couldn't resolve for validation on non-pointer fields. The solution definesPortNumberlocally inv1alpha1with validation markers at the type level, which controller-gen accepts. A separateStatusPortNumbertype is used for status fields to enforce theminimum=1requirement for actual assigned ports, while spec fields allowminimum=0for auto-assignment.Added CRD marker validation to
hack/update-clientset.shcontroller-gen crdvalidation step to themake generatetargetWhich issue(s) this PR fixes:
Fixes validation errors that affect downstream consumers like KServe who run
controller-genduring their build process. These were discovered during KServe migration to GIE v1, following the same issues fixed in kubernetes-sigs/gateway-api-inference-extension#1679.Does this PR introduce a user-facing change?:
Documentation References: