Skip to content

Bug 1753930: Adds support for trailing dot in noProxy domain names #2405

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

Merged
openshift-merge-robot merged 1 commit into from
Sep 27, 2019
Merged

Bug 1753930: Adds support for trailing dot in noProxy domain names #2405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion pkg/types/validation/installconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ func validateProxy(p *types.Proxy, fldPath *field.Path) field.ErrorList {
errDomain := validate.NoProxyDomainName(v)
_, _, errCIDR := net.ParseCIDR(v)
if errDomain != nil && errCIDR != nil {
allErrs = append(allErrs, field.Invalid(field.NewPath("NoProxy"), v, "must be a CIDR or domain, without wildcard characters and without trailing dots ('.')"))
allErrs = append(allErrs, field.Invalid(field.NewPath("NoProxy"), v, "must be a CIDR or domain, without wildcard characters"))
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/types/validation/installconfig_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -612,10 +612,10 @@ func TestValidateInstallConfig(t *testing.T) {
name: "invalid NoProxy domain",
installConfig: func() *types.InstallConfig {
c := validInstallConfig()
c.Proxy.NoProxy = "good-no-proxy.com, .bad-proxy."
c.Proxy.NoProxy = "good-no-proxy.com, *.bad-proxy"
return c
}(),
expectedError: `^\QNoProxy: Invalid value: ".bad-proxy.": must be a CIDR or domain, without wildcard characters and without trailing dots ('.')\E$`,
expectedError: `^\QNoProxy: Invalid value: "*.bad-proxy": must be a CIDR or domain, without wildcard characters\E$`,
},
{
name: "invalid NoProxy CIDR",
Expand All @@ -624,16 +624,16 @@ func TestValidateInstallConfig(t *testing.T) {
c.Proxy.NoProxy = "good-no-proxy.com, 172.bad.CIDR.0/16"
return c
}(),
expectedError: `^\QNoProxy: Invalid value: "172.bad.CIDR.0/16": must be a CIDR or domain, without wildcard characters and without trailing dots ('.')\E$`,
expectedError: `^\QNoProxy: Invalid value: "172.bad.CIDR.0/16": must be a CIDR or domain, without wildcard characters\E$`,
},
{
name: "invalid NoProxy domain & CIDR",
installConfig: func() *types.InstallConfig {
c := validInstallConfig()
c.Proxy.NoProxy = "good-no-proxy.com, a-good-one, .bad-proxy., another, 172.bad.CIDR.0/16, good-end"
c.Proxy.NoProxy = "good-no-proxy.com, a-good-one, *.bad-proxy., another, 172.bad.CIDR.0/16, good-end"
return c
}(),
expectedError: `^\Q[NoProxy: Invalid value: ".bad-proxy.": must be a CIDR or domain, without wildcard characters and without trailing dots ('.'), NoProxy: Invalid value: "172.bad.CIDR.0/16": must be a CIDR or domain, without wildcard characters and without trailing dots ('.')]\E$`,
expectedError: `^\Q[NoProxy: Invalid value: "*.bad-proxy.": must be a CIDR or domain, without wildcard characters, NoProxy: Invalid value: "172.bad.CIDR.0/16": must be a CIDR or domain, without wildcard characters]\E$`,
},
{
name: "valid GCP platform",
Expand Down
4 changes: 2 additions & 2 deletions pkg/validate/validate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ func DomainName(v string, acceptTrailingDot bool) error {

// NoProxyDomainName checks if the given string is a valid proxy noProxy domain name
// and returns an error if not. Example valid noProxy domains are ".foo.com", "bar.com",
// but not "*.foo.com", "bar.com."
// "bar.com." but not "*.foo.com".
func NoProxyDomainName(v string) error {
v = strings.TrimPrefix(v, ".")
v = strings.TrimSuffix(strings.TrimPrefix(v, "."), ".")
return validateSubdomain(v)
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/validate/validate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,10 +184,10 @@ func TestNoProxyDomainName(t *testing.T) {
{"1", true},
{"0.0", true},
{"1.2.3.4", true},
{"1.2.3.4.", false},
{"abc.", false},
{"1.2.3.4.", true},
Copy link
Member

@sdodson sdodson Sep 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is everything a string or do IPs have special handling?

Copy link
Contributor Author

@danehans danehans Sep 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.2.3.4. is a valid hostname if we allow a trailing dot. rfc 1123 is being used as the basis for domain name validation:

One aspect of host name syntax is hereby changed: the restriction on the first character is relaxed to allow either a letter or a digit. Host software MUST support this more liberal syntax.

Previously the proceeding dot was stripped prior to rfc 1123 validation, so .1.2.3.4 was and is still considered a valid name. Now the trailing dot will be stripped prior to rfc 1123 validation, so .1.2.3.4, 1.2.3.4. and .1.2.3.4. are also considered valid.

Copy link
Contributor Author

@danehans danehans Sep 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The values are also run against net.ParseCIDR, but since .1.2.3.4, 1.2.3.4. and .1.2.3.4. are considered valid by rfc 1123 after stripping proceeding/trailing dots, they are considered valid.

{"abc.", true},
{"abc.com", true},
{"abc.com.", false},
{"abc.com.", true},
{"a.b.c.d.e.f", true},
{".abc", true},
{".abc.com", true},
Expand Down