Skip to content

Commit 4a3e136

Browse files
pdabelf5pdabelf5
committed
update hostname/fqdn/ip validation
1 parent 95672a3 commit 4a3e136

File tree

4 files changed

+53
-121
lines changed

4 files changed

+53
-121
lines changed

‎internal/validation/validation.go‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,12 @@ import (
88
)
99

1010
var (
11-
validDNSRegex = regexp.MustCompile(`^(?:[A-Za-z0-9][A-Za-z0-9-]{1,62}\.)([A-Za-z0-9-]{1,63}\.)*[A-Za-z]{2,6}(?::\d{1,5})?$`)
12-
validIPRegex = regexp.MustCompile(`^(\d{1,3}\.){3}\d{1,3}(?::\d{1,5})?$`)
13-
validLocalhostRegex = regexp.MustCompile(`^localhost(?::\d{1,5})?$`)
11+
validDNSRegex = regexp.MustCompile(`^(?:[A-Za-z0-9][A-Za-z0-9-]{1,62}\.)([A-Za-z0-9-]{1,63}\.)*[A-Za-z]{2,6}(?::\d{1,5})?$`)
12+
validIPRegex = regexp.MustCompile(`^(?:(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9][0-9]|[0-9])\.){3}(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9][0-9]|[0-9])(?::\d{1,5})?$`)
13+
validHostnameRegex = regexp.MustCompile(`^[a-z][A-Za-z0-9-]{1,62}(?::\d{1,5})?$`)
1414
)
1515

16+
// ValidatePort ensure port matches rfc6335 https://www.rfc-editor.org/rfc/rfc6335.html
1617
func ValidatePort(value string) error {
1718
port, err := strconv.Atoi(value)
1819
if err != nil {
@@ -24,18 +25,21 @@ func ValidatePort(value string) error {
2425
return nil
2526
}
2627

28+
// ValidateHost ensures the host is a valid hostname/IP address or FQDN with an optional :port appended
2729
func ValidateHost(host string) error {
2830
if host == "" {
2931
return fmt.Errorf("error parsing host: empty host")
3032
}
3133

32-
if validIPRegex.MatchString(host) || validDNSRegex.MatchString(host) || validLocalhostRegex.MatchString(host) {
34+
if validIPRegex.MatchString(host) || validDNSRegex.MatchString(host) || validHostnameRegex.MatchString(host) {
3335
chunks := strings.Split(host, ":")
34-
err := ValidatePort(chunks[1])
35-
if err != nil {
36-
return fmt.Errorf("invalid port: %w", err)
36+
if len(chunks) > 1 {
37+
err := ValidatePort(chunks[1])
38+
if err != nil {
39+
return fmt.Errorf("invalid port: %w", err)
40+
}
3741
}
3842
return nil
3943
}
40-
return fmt.Errorf("invalid host: %s", host)
44+
return fmt.Errorf("error parsing host: %s not a valid host", host)
4145
}

‎internal/validation/validation_test.go‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,20 +39,28 @@ func TestValidateHost(t *testing.T) {
3939
t.Parallel()
4040
// Positive test cases
4141
posHosts := []string{
42-
"10.10.1.1:514",
43-
"localhost:514",
44-
"dns.test.svc.cluster.local:514",
45-
"cluster.local:514",
46-
"dash-test.cluster.local:514",
42+
"10.10.1.1:443",
43+
"10.10.1.1",
44+
"123.112.224.43:443",
45+
"172.120.3.222",
46+
"localhost:80",
47+
"localhost",
48+
"myhost:54321",
49+
"myhost",
50+
"my-host:54321",
51+
"my-host",
52+
"dns.test.svc.cluster.local:8443",
53+
"cluster.local:8443",
4754
"product.example.com",
55+
"product.example.com:443",
4856
}
4957

5058
// Negative test cases item, expected error message
5159
negHosts := [][]string{
52-
{"NotValid", "invalid host: NotValid"},
53-
{"cluster.local", "invalid host: cluster.local"},
54-
{"-cluster.local:514", "invalid host: -cluster.local:514"},
60+
{"NotValid", "not a valid host"},
61+
{"-cluster.local:514", "not a valid host"},
5562
{"10.10.1.1:99999", "not a valid port number"},
63+
{"333.333.333.333", "not a valid host"},
5664
}
5765

5866
for _, tCase := range posHosts {

‎pkg/apis/dos/validation/dos.go‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,17 @@ package validation
22

33
import (
44
"fmt"
5-
k8svalidation "github.com/nginxinc/kubernetes-ingress/internal/validation"
5+
"net"
6+
"net/url"
7+
"regexp"
8+
"strings"
9+
10+
internalValidation "github.com/nginxinc/kubernetes-ingress/internal/validation"
611
validation2 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/validation"
712
"github.com/nginxinc/kubernetes-ingress/pkg/apis/dos/v1beta1"
813
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
914
"k8s.io/apimachinery/pkg/util/validation"
1015
"k8s.io/apimachinery/pkg/util/validation/field"
11-
"net"
12-
"net/url"
1316
)
1417

1518
var appProtectDosPolicyRequiredFields = [][]string{
@@ -113,15 +116,25 @@ func ValidateAppProtectDosLogConf(logConf *unstructured.Unstructured) (string, e
113116
return warning, nil
114117
}
115118

119+
var (
120+
validDNSRegex = regexp.MustCompile(`^([A-Za-z0-9][A-Za-z0-9-]{1,62}\.)([A-Za-z0-9-]{1,63}\.)*[A-Za-z]{2,6}:\d{1,5}$`)
121+
validIPRegex = regexp.MustCompile(`^(\d{1,3}\.){3}\d{1,3}:\d{1,5}$`)
122+
validLocalhostRegex = regexp.MustCompile(`^localhost:\d{1,5}$`)
123+
)
124+
116125
func validateAppProtectDosLogDest(dstAntn string) error {
117126
if dstAntn == "stderr" {
118127
return nil
119128
}
120-
err := k8svalidation.ValidateHost(dstAntn)
121-
if err != nil {
122-
return fmt.Errorf("%w, must follow format: <ip-address | localhost | dns name>:<port> or stderr", err)
129+
if validIPRegex.MatchString(dstAntn) || validDNSRegex.MatchString(dstAntn) || validLocalhostRegex.MatchString(dstAntn) {
130+
chunks := strings.Split(dstAntn, ":")
131+
err := internalValidation.ValidatePort(chunks[1])
132+
if err != nil {
133+
return fmt.Errorf("invalid log destination: %w", err)
134+
}
135+
return nil
123136
}
124-
return nil
137+
return fmt.Errorf("invalid log destination: %s, must follow format: <ip-address | localhost | dns name>:<port> or stderr", dstAntn)
125138
}
126139

127140
func validateAppProtectDosName(name string) error {

‎pkg/apis/dos/validation/dos_test.go‎

Lines changed: 5 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func TestValidateDosProtectedResource(t *testing.T) {
6363
DosAccessLogDest: "bad&$%^logdest",
6464
},
6565
},
66-
expectErr: "error validating DosProtectedResource: invalid field: dosAccessLogDest err: invalid host: bad&$%^logdest, must follow format: <ip-address | localhost | dns name>:<port> or stderr",
66+
expectErr: "error validating DosProtectedResource: invalid field: dosAccessLogDest err: invalid log destination: bad&$%^logdest, must follow format: <ip-address | localhost | dns name>:<port> or stderr",
6767
msg: "invalid DosAccessLogDest specified",
6868
},
6969
{
@@ -105,7 +105,7 @@ func TestValidateDosProtectedResource(t *testing.T) {
105105
DosSecurityLog: &v1beta1.DosSecurityLog{},
106106
},
107107
},
108-
expectErr: "error validating DosProtectedResource: invalid field: dosSecurityLog/dosLogDest err: error parsing host: empty host, must follow format: <ip-address | localhost | dns name>:<port> or stderr",
108+
expectErr: "error validating DosProtectedResource: invalid field: dosSecurityLog/dosLogDest err: invalid log destination: , must follow format: <ip-address | localhost | dns name>:<port> or stderr",
109109
msg: "empty DosSecurityLog specified",
110110
},
111111
{
@@ -159,7 +159,6 @@ func TestValidateDosProtectedResource(t *testing.T) {
159159
msg: "DosSecurityLog with valid apDosLogConf",
160160
},
161161
}
162-
163162
for _, test := range tests {
164163
err := ValidateDosProtectedResource(test.protected)
165164
if err != nil {
@@ -191,9 +190,9 @@ func TestValidateAppProtectDosAccessLogDest(t *testing.T) {
191190

192191
// Negative test cases item, expected error message
193192
negDstAntns := [][]string{
194-
{"NotValid", "invalid host: NotValid, must follow format: <ip-address | localhost | dns name>:<port> or stderr"},
195-
{"cluster.local", "invalid host: cluster.local, must follow format: <ip-address | localhost | dns name>:<port> or stderr"},
196-
{"-cluster.local:514", "invalid host: -cluster.local:514, must follow format: <ip-address | localhost | dns name>:<port> or stderr"},
193+
{"NotValid", "invalid log destination: NotValid, must follow format: <ip-address | localhost | dns name>:<port> or stderr"},
194+
{"cluster.local", "invalid log destination: cluster.local, must follow format: <ip-address | localhost | dns name>:<port> or stderr"},
195+
{"-cluster.local:514", "invalid log destination: -cluster.local:514, must follow format: <ip-address | localhost | dns name>:<port> or stderr"},
197196
{"10.10.1.1:99999", "not a valid port number"},
198197
}
199198

@@ -203,7 +202,6 @@ func TestValidateAppProtectDosAccessLogDest(t *testing.T) {
203202
t.Errorf("expected nil, got %v", err)
204203
}
205204
}
206-
207205
for _, nTCase := range negDstAntns {
208206
err := validateAppProtectDosLogDest(nTCase[0])
209207
if err == nil {
@@ -216,97 +214,6 @@ func TestValidateAppProtectDosAccessLogDest(t *testing.T) {
216214
}
217215
}
218216

219-
func TestValidateAppProtectDosLogConf(t *testing.T) {
220-
t.Parallel()
221-
tests := []struct {
222-
logConf *unstructured.Unstructured
223-
expectErr bool
224-
expectWarn bool
225-
msg string
226-
}{
227-
{
228-
logConf: &unstructured.Unstructured{
229-
Object: map[string]interface{}{
230-
"spec": map[string]interface{}{
231-
"filter": map[string]interface{}{},
232-
},
233-
},
234-
},
235-
expectErr: false,
236-
expectWarn: false,
237-
msg: "valid log conf",
238-
},
239-
{
240-
logConf: &unstructured.Unstructured{
241-
Object: map[string]interface{}{
242-
"spec": map[string]interface{}{},
243-
},
244-
},
245-
expectErr: true,
246-
expectWarn: false,
247-
msg: "invalid log conf with no filter field",
248-
},
249-
{
250-
logConf: &unstructured.Unstructured{
251-
Object: map[string]interface{}{
252-
"something": map[string]interface{}{
253-
"filter": map[string]interface{}{},
254-
},
255-
},
256-
},
257-
expectErr: true,
258-
expectWarn: false,
259-
msg: "invalid log conf with no spec field",
260-
},
261-
{
262-
logConf: &unstructured.Unstructured{
263-
Object: map[string]interface{}{
264-
"spec": map[string]interface{}{
265-
"content": map[string]interface{}{
266-
"format": "user-defined",
267-
},
268-
"filter": map[string]interface{}{},
269-
},
270-
},
271-
},
272-
expectErr: false,
273-
expectWarn: true,
274-
msg: "Support only splunk format",
275-
},
276-
{
277-
logConf: &unstructured.Unstructured{
278-
Object: map[string]interface{}{
279-
"spec": map[string]interface{}{
280-
"filter": map[string]interface{}{},
281-
"content": map[string]interface{}{
282-
"format": "user-defined",
283-
},
284-
},
285-
},
286-
},
287-
expectErr: false,
288-
expectWarn: true,
289-
msg: "valid log conf with warning filter field",
290-
},
291-
}
292-
293-
for _, test := range tests {
294-
warn, err := ValidateAppProtectDosLogConf(test.logConf)
295-
if test.expectErr && err == nil {
296-
t.Errorf("validateAppProtectDosLogConf() returned no error for the case of %s", test.msg)
297-
}
298-
if !test.expectErr && err != nil {
299-
t.Errorf("validateAppProtectDosLogConf() returned unexpected error %v for the case of %s", err, test.msg)
300-
}
301-
if test.expectWarn && warn == "" {
302-
t.Errorf("validateAppProtectDosLogConf() returned no warning for the case of %s", test.msg)
303-
}
304-
if !test.expectWarn && warn != "" {
305-
t.Errorf("validateAppProtectDosLogConf() returned unexpected warning: %s, for the case of %s", warn, test.msg)
306-
}
307-
}
308-
}
309-
310217
func TestValidateAppProtectDosPolicy(t *testing.T) {
311218
t.Parallel()
312219
tests := []struct {

0 commit comments

Comments
 (0)