Introduces a Validation Package #597
Conversation
k8s-ci-robot
added
the
kind/feature
|
@danehans: GitHub didn't allow me to request PR reviews from the following users: stevesloka. Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs. In 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/test-infra repository. |
k8s-ci-robot
added
cncf-cla: yes
|
We should figure out a way how to integrate this with #506. |
|
@bowei thanks for the review. Commit |
| // - A wildcard DNS subdomain as defined by RFC 1034 (section 4.3.3). | ||
| func ValidateListenerHostname(listeners []gatewayv1a1.Listener, path *field.Path) field.ErrorList { | ||
| var errs field.ErrorList | ||
| for i, h := range listeners { |
There was a problem hiding this comment.
suggest early breaks to improve readability:
for i, h := range listeners {
if h.Hostname == nil {
continue
}
switch *h.Hostname {
case "":
// is this really valid?
case "*":
// valid wildcard
default:
// parse and check
}
} There was a problem hiding this comment.
I refactored the conditional logic that I think makes it easier to follow.
| "k8s.io/apimachinery/pkg/util/validation/field" | ||
| ) | ||
|
|
||
| // ValidateGateway validates Gateway objects. |
There was a problem hiding this comment.
Can we elaborate here and give some indication on what sorts of rules this enforces?
There was a problem hiding this comment.
I provided a link to the gateway spec and include details in the functions that are validating specific fields.
| errs = append(errs, field.Invalid(path.Index(i).Child("hostname"), hostname, "must be a DNS hostname, not an IP address")) | ||
| } | ||
| if strings.Contains(hostname, "*") { | ||
| for _, msg := range validation.IsWildcardDNS1123Subdomain(hostname) { |
There was a problem hiding this comment.
Pretty sure we can validate this with a regex on the CRD.
|
@jpeach thanks for the review. Commit |
|
As a follow-on from today's community meeting, I did a quick implementation of the validation pkg as a proof-of-concept for #506, PTAL. |
|
Small changes and then this should be able to go in... |
|
@bowei thanks again for the review. Commit |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bowei, danehans The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR adds a validation package. It's needed so implementors no longer need to create a Gateway API validation package. This approach provides validation consistency across implementations, including the future validating webhook. The package is modeled after the Kubernetes networking APIs validation package.
Which issue(s) this PR fixes:
Partially Fixes # #487
Does this PR introduce a user-facing change?:
/cc @stevesloka @youngnick @jpeach