Skip to content

Commit 8148d81

Browse files
authored
Simplify hostname validation for HTTPRoute (#504)
Commit 52fab05 added HTTPRoute validation, which included both data-plane agnostic and data-plane specific (NGINX) rules for hostnames. After recent team discussion, we concluded that the data-plane specific rules are not needed, as the agnostic rules provide sufficient validation and ensure support across data planes.
1 parent c473aba commit 8148d81

File tree

6 files changed

+6
-144
lines changed

6 files changed

+6
-144
lines changed

internal/nginx/config/validation/http_validator.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,3 @@ type HTTPValidator struct {
1313
}
1414

1515
var _ validation.HTTPFieldsValidator = HTTPValidator{}
16-
17-
var hostnameInServerExamples = []string{"host", "example.com"}
18-
19-
// ValidateHostnameInServer validates a hostname to be used in the server_name directive.
20-
func (HTTPValidator) ValidateHostnameInServer(hostname string) error {
21-
return validateEscapedString(hostname, hostnameInServerExamples)
22-
}

internal/nginx/config/validation/http_validator_test.go

Lines changed: 0 additions & 14 deletions
This file was deleted.

internal/state/graph/httproute.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ func buildRoute(
212212
UnattachedSectionNameRefs: map[string]conditions.Condition{},
213213
}
214214

215-
err := validateHostnames(validator, ghr.Spec.Hostnames, field.NewPath("spec").Child("hostnames"))
215+
err := validateHostnames(ghr.Spec.Hostnames, field.NewPath("spec").Child("hostnames"))
216216
if err != nil {
217217
r.Valid = false
218218
r.Conditions = append(r.Conditions, conditions.NewRouteUnsupportedValue(err.Error()))
@@ -395,17 +395,14 @@ func getHostname(h *v1beta1.Hostname) string {
395395
return string(*h)
396396
}
397397

398-
func validateHostnames(validator validation.HTTPFieldsValidator, hostnames []v1beta1.Hostname, path *field.Path) error {
398+
func validateHostnames(hostnames []v1beta1.Hostname, path *field.Path) error {
399399
var allErrs field.ErrorList
400400

401401
for i := range hostnames {
402402
if err := validateHostname(string(hostnames[i])); err != nil {
403403
allErrs = append(allErrs, field.Invalid(path.Index(i), hostnames[i], err.Error()))
404404
continue
405405
}
406-
if err := validator.ValidateHostnameInServer(string(hostnames[i])); err != nil {
407-
allErrs = append(allErrs, field.Invalid(path.Index(i), hostnames[i], err.Error()))
408-
}
409406
}
410407

411408
return allErrs.ToAggregate()

internal/state/graph/httproute_test.go

Lines changed: 1 addition & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -520,29 +520,6 @@ func TestBuildRoute(t *testing.T) {
520520
},
521521
name: "invalid hostname",
522522
},
523-
{
524-
validator: &validationfakes.FakeHTTPFieldsValidator{
525-
ValidateHostnameInServerStub: func(string) error {
526-
return errors.New("invalid hostname")
527-
},
528-
},
529-
hr: hr,
530-
expected: &Route{
531-
Source: hr,
532-
Valid: false,
533-
SectionNameRefs: map[string]ParentRef{
534-
sectionNameOfCreateHTTPRoute: {
535-
Idx: 0,
536-
Gateway: gatewayNsName,
537-
},
538-
},
539-
UnattachedSectionNameRefs: map[string]conditions.Condition{},
540-
Conditions: []conditions.Condition{
541-
conditions.NewRouteUnsupportedValue(`spec.hostnames[0]: Invalid value: "example.com": invalid hostname`),
542-
},
543-
},
544-
name: "invalid hostname by the data-plane",
545-
},
546523
{
547524
validator: validatorInvalidFieldsInRule,
548525
hr: hrInvalidMatches,
@@ -1055,13 +1032,11 @@ func TestValidateHostnames(t *testing.T) {
10551032
const validHostname = "example.com"
10561033

10571034
tests := []struct {
1058-
validator *validationfakes.FakeHTTPFieldsValidator
10591035
name string
10601036
hostnames []v1beta1.Hostname
10611037
expectErr bool
10621038
}{
10631039
{
1064-
validator: &validationfakes.FakeHTTPFieldsValidator{},
10651040
hostnames: []v1beta1.Hostname{
10661041
validHostname,
10671042
"example.org",
@@ -1071,30 +1046,13 @@ func TestValidateHostnames(t *testing.T) {
10711046
name: "multiple valid",
10721047
},
10731048
{
1074-
validator: &validationfakes.FakeHTTPFieldsValidator{},
10751049
hostnames: []v1beta1.Hostname{
10761050
validHostname,
10771051
"",
10781052
},
10791053
expectErr: true,
10801054
name: "valid and invalid",
10811055
},
1082-
{
1083-
validator: &validationfakes.FakeHTTPFieldsValidator{
1084-
ValidateHostnameInServerStub: func(h string) error {
1085-
if h == validHostname {
1086-
return nil
1087-
}
1088-
return errors.New("invalid hostname")
1089-
},
1090-
},
1091-
hostnames: []v1beta1.Hostname{
1092-
validHostname,
1093-
"value", // invalid by the validator
1094-
},
1095-
expectErr: true,
1096-
name: "valid and invalid by the validator",
1097-
},
10981056
}
10991057

11001058
path := field.NewPath("test")
@@ -1103,7 +1061,7 @@ func TestValidateHostnames(t *testing.T) {
11031061
t.Run(test.name, func(t *testing.T) {
11041062
g := NewGomegaWithT(t)
11051063

1106-
err := validateHostnames(test.validator, test.hostnames, path)
1064+
err := validateHostnames(test.hostnames, path)
11071065

11081066
if test.expectErr {
11091067
g.Expect(err).To(HaveOccurred())

internal/state/validation/validationfakes/fake_httpfields_validator.go

Lines changed: 0 additions & 74 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/state/validation/validator.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package validation
22

33
// Validators include validators for Gateway API resources from the perspective of a data-plane.
4+
// It is used for fields that propagate into the data plane configuration. For example, the path in a routing rule.
5+
// However, not all such fields are validated: NKG will not validate a field using Validators if it is confident that
6+
// the field is valid.
47
type Validators struct {
58
HTTPFieldsValidator HTTPFieldsValidator
69
}
@@ -10,7 +13,6 @@ type Validators struct {
1013
//
1114
//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . HTTPFieldsValidator
1215
type HTTPFieldsValidator interface {
13-
ValidateHostnameInServer(hostname string) error
1416
ValidatePathInPrefixMatch(path string) error
1517
ValidateHeaderNameInMatch(name string) error
1618
ValidateHeaderValueInMatch(value string) error

0 commit comments

Comments
 (0)