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.

Sign up for GitHub

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

Jump to bottom

Remove duplicate validation from CRD & Webhook #872

Merged
merged 8 commits into from
Sep 24, 2021
Merged

Remove duplicate validation from CRD & Webhook #872

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5278391
Remove Listener.Hostname validation from the Webhook
we4tech Sep 20, 2021
4e37ec7
Add Listener.hostname validation to the CRD
we4tech Sep 20, 2021
3846307
Added dummy validator functions for Gateway
we4tech Sep 22, 2021
907223b
Add comment for the dummy function
we4tech Sep 22, 2021
b5a981b
Revert changes fr v1alpha1
we4tech Sep 22, 2021
1720f39
Fix the linting issue
we4tech Sep 22, 2021
aceb299
Update comments for apis/v1alpha2/validation/gateway.go
we4tech Sep 24, 2021
950144f
Adjust the commit message again
we4tech Sep 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 7 additions & 49 deletions apis/v1alpha1/validation/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,8 @@ limitations under the License.
package validation

import (
"net"
"strings"

gatewayv1a1 "sigs.k8s.io/gateway-api/apis/v1alpha1"

"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
)

Expand All @@ -37,51 +33,13 @@ var (
// ValidateGateway validates gw according to the Gateway API specification.
// For additional details of the Gateway spec, refer to:
// https://gateway-api.sigs.k8s.io/spec/#networking.x-k8s.io/v1alpha1.Gateway
func ValidateGateway(gw *gatewayv1a1.Gateway) field.ErrorList {
return validateGatewaySpec(&gw.Spec, field.NewPath("spec"))
}
we4tech marked this conversation as resolved.
Show resolved Hide resolved

// validateGatewaySpec validates whether required fields of spec are set according to the
// Gateway API specification.
func validateGatewaySpec(spec *gatewayv1a1.GatewaySpec, path *field.Path) field.ErrorList {
// TODO [danehans]: Add additional validation of spec fields.
return validateGatewayListeners(spec.Listeners, path.Child("listeners"))
}

// validateGatewayListeners validates whether required fields of listeners are set according
// to the Gateway API specification.
func validateGatewayListeners(listeners []gatewayv1a1.Listener, path *field.Path) field.ErrorList {
// TODO [danehans]: Add additional validation of listener fields.
return validateListenerHostname(listeners, path)
}

// validateListenerHostname validates each listener hostname is not an IP address and is one
// of the following:
// - A fully qualified domain name of a network host, as defined by RFC 3986.
// - A DNS subdomain as defined by RFC 1123.
// - 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 {
// When unspecified, “”, or *, all hostnames are matched.
if h.Hostname == nil || (*h.Hostname == "" || *h.Hostname == "*") {
continue
}
hostname := string(*h.Hostname)
if ip := net.ParseIP(hostname); ip != nil {
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) {
errs = append(errs, field.Invalid(path.Index(i).Child("hostname"), hostname, msg))
}
} else {
for _, msg := range validation.IsDNS1123Subdomain(hostname) {
errs = append(errs, field.Invalid(path.Index(i).Child("hostname"), hostname, msg))
}
}
}
return errs
//
// This function is currently empty as the validations have been moved to CRD field validations.
// It's expected that further validation will be added back here in the future.
// Here is the related PR for the reference:
// https://github.com/kubernetes-sigs/gateway-api/pull/872
func ValidateGateway(_ *gatewayv1a1.Gateway) field.ErrorList {
return nil
}

// ValidateHTTPRoute validates HTTPRoute according to the Gateway API specification.
Expand Down
138 changes: 0 additions & 138 deletions apis/v1alpha1/validation/validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,148 +22,10 @@ import (

gatewayv1a1 "sigs.k8s.io/gateway-api/apis/v1alpha1"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation/field"
utilpointer "k8s.io/utils/pointer"
)

func TestValidateGateway(t *testing.T) {
listeners := []gatewayv1a1.Listener{
{
Hostname: nil,
},
}
baseGateway := gatewayv1a1.Gateway{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: metav1.NamespaceDefault,
},
Spec: gatewayv1a1.GatewaySpec{
GatewayClassName: "foo",
Listeners: listeners,
},
}

testCases := map[string]struct {
mutate func(gw *gatewayv1a1.Gateway)
expectErrsOnFields []string
}{
"nil hostname": {
mutate: func(gw *gatewayv1a1.Gateway) {},
expectErrsOnFields: []string{},
},
"empty string hostname": {
mutate: func(gw *gatewayv1a1.Gateway) {
hostname := gatewayv1a1.Hostname("")
gw.Spec.Listeners[0].Hostname = &hostname
},
expectErrsOnFields: []string{},
},
"wildcard hostname": {
mutate: func(gw *gatewayv1a1.Gateway) {
hostname := gatewayv1a1.Hostname("*")
gw.Spec.Listeners[0].Hostname = &hostname
},
expectErrsOnFields: []string{},
},
"wildcard-prefixed hostname": {
mutate: func(gw *gatewayv1a1.Gateway) {
hostname := gatewayv1a1.Hostname("*.example.com")
gw.Spec.Listeners[0].Hostname = &hostname
},
expectErrsOnFields: []string{},
},
"valid dns subdomain": {
mutate: func(gw *gatewayv1a1.Gateway) {
hostname := gatewayv1a1.Hostname("foo.example.com")
gw.Spec.Listeners[0].Hostname = &hostname
},
expectErrsOnFields: []string{},
},
// Invalid use cases
"IPv4 address hostname": {
mutate: func(gw *gatewayv1a1.Gateway) {
hostname := gatewayv1a1.Hostname("1.2.3.4")
gw.Spec.Listeners[0].Hostname = &hostname
},
expectErrsOnFields: []string{"spec.listeners[0].hostname"},
},
"Invalid IPv4 address hostname": {
mutate: func(gw *gatewayv1a1.Gateway) {
hostname := gatewayv1a1.Hostname("1.2.3..4")
gw.Spec.Listeners[0].Hostname = &hostname
},
expectErrsOnFields: []string{"spec.listeners[0].hostname"},
},
"IPv4 address with port hostname": {
mutate: func(gw *gatewayv1a1.Gateway) {
hostname := gatewayv1a1.Hostname("1.2.3.4:8080")
gw.Spec.Listeners[0].Hostname = &hostname
},
expectErrsOnFields: []string{"spec.listeners[0].hostname"},
},
"IPv6 address hostname": {
mutate: func(gw *gatewayv1a1.Gateway) {
hostname := gatewayv1a1.Hostname("2001:db8::68")
gw.Spec.Listeners[0].Hostname = &hostname
},
expectErrsOnFields: []string{"spec.listeners[0].hostname", "spec.listeners[0].hostname"},
},
"IPv6 link-local address hostname": {
mutate: func(gw *gatewayv1a1.Gateway) {
hostname := gatewayv1a1.Hostname("fe80::/10")
gw.Spec.Listeners[0].Hostname = &hostname
},
expectErrsOnFields: []string{"spec.listeners[0].hostname"},
},
"dns subdomain with port": {
mutate: func(gw *gatewayv1a1.Gateway) {
hostname := gatewayv1a1.Hostname("foo.example.com:8080")
gw.Spec.Listeners[0].Hostname = &hostname
},
expectErrsOnFields: []string{"spec.listeners[0].hostname"},
},
"dns subdomain with invalid wildcard label": {
mutate: func(gw *gatewayv1a1.Gateway) {
hostname := gatewayv1a1.Hostname("*.*.com")
gw.Spec.Listeners[0].Hostname = &hostname
},
expectErrsOnFields: []string{"spec.listeners[0].hostname"},
},
"dns subdomain with multiple wildcards": {
mutate: func(gw *gatewayv1a1.Gateway) {
hostname := gatewayv1a1.Hostname("*.foo.*.com")
gw.Spec.Listeners[0].Hostname = &hostname
},
expectErrsOnFields: []string{"spec.listeners[0].hostname"},
},
"dns subdomain with wildcard root label": {
mutate: func(gw *gatewayv1a1.Gateway) {
hostname := gatewayv1a1.Hostname("*.foo.*.com")
gw.Spec.Listeners[0].Hostname = &hostname
},
expectErrsOnFields: []string{"spec.listeners[0].hostname"},
},
}

for name, tc := range testCases {
tc := tc
t.Run(name, func(t *testing.T) {
gw := baseGateway.DeepCopy()
tc.mutate(gw)
errs := ValidateGateway(gw)
if len(tc.expectErrsOnFields) != len(errs) {
t.Fatalf("Expected %d errors, got %d errors: %v", len(tc.expectErrsOnFields), len(errs), errs)
}
for i, err := range errs {
if err.Field != tc.expectErrsOnFields[i] {
t.Errorf("Expected error on field: %s, got: %s", tc.expectErrsOnFields[i], err.Error())
}
}
})
}
}

func TestValidateHTTPRoute(t *testing.T) {
testService := "test-service"
specialService := "special-service"
Expand Down
54 changes: 6 additions & 48 deletions apis/v1alpha2/validation/gateway.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@ limitations under the License.
package validation

import (
"net"
"strings"

"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"

gatewayv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
Expand All @@ -29,49 +25,11 @@ import (
// ValidateGateway validates gw according to the Gateway API specification.
// For additional details of the Gateway spec, refer to:
// https://gateway-api.sigs.k8s.io/spec/#gateway.networking.k8s.io/v1alpha2.Gateway
//
// This function is currently empty as the validations have been moved to CRD field validations.
// It's expected that further validation will be added back here in the future.
// Here is the related PR for the reference:
// https://github.com/kubernetes-sigs/gateway-api/pull/872
we4tech marked this conversation as resolved.
Show resolved Hide resolved
func ValidateGateway(gw *gatewayv1a2.Gateway) field.ErrorList {
return validateGatewaySpec(&gw.Spec, field.NewPath("spec"))
}

// validateGatewaySpec validates whether required fields of spec are set according to the
// Gateway API specification.
func validateGatewaySpec(spec *gatewayv1a2.GatewaySpec, path *field.Path) field.ErrorList {
// TODO [danehans]: Add additional validation of spec fields.
return validateGatewayListeners(spec.Listeners, path.Child("listeners"))
}

// validateGatewayListeners validates whether required fields of listeners are set according
// to the Gateway API specification.
func validateGatewayListeners(listeners []gatewayv1a2.Listener, path *field.Path) field.ErrorList {
// TODO [danehans]: Add additional validation of listener fields.
return validateListenerHostname(listeners, path)
}

// validateListenerHostname validates each listener hostname is not an IP address and is one
// of the following:
// - A fully qualified domain name of a network host, as defined by RFC 3986.
// - A DNS subdomain as defined by RFC 1123.
// - A wildcard DNS subdomain as defined by RFC 1034 (section 4.3.3).
func validateListenerHostname(listeners []gatewayv1a2.Listener, path *field.Path) field.ErrorList {
var errs field.ErrorList
for i, h := range listeners {
// When unspecified, “”, or *, all hostnames are matched.
if h.Hostname == nil || (*h.Hostname == "" || *h.Hostname == "*") {
continue
}
hostname := string(*h.Hostname)
if ip := net.ParseIP(hostname); ip != nil {
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) {
errs = append(errs, field.Invalid(path.Index(i).Child("hostname"), hostname, msg))
}
} else {
for _, msg := range validation.IsDNS1123Subdomain(hostname) {
errs = append(errs, field.Invalid(path.Index(i).Child("hostname"), hostname, msg))
}
}
}
return errs
return nil
}
Loading