Merge pull request #15071 from jim-minter/remove-MungeNoProtocolURL

Merged by openshift-bot

pkg/build/apis/build/validation/validation.go

@@ -2,7 +2,6 @@ package validation
 
 import (
 	"fmt"
-	"net/url"
 	"path"
 	"path/filepath"
 	"strings"
@@ -24,6 +23,7 @@ import (
 	imageapi "github.com/openshift/origin/pkg/image/apis/image"
 	imageapivalidation "github.com/openshift/origin/pkg/image/apis/image/validation"
 	"github.com/openshift/origin/pkg/util/labelselector"
+	s2igit "github.com/openshift/source-to-image/pkg/scm/git"
 )
 
 // ValidateBuild tests required fields for a Build.
@@ -271,14 +271,18 @@ func validateGitSource(git *buildapi.GitBuildSource, fldPath *field.Path) field.
 	allErrs := field.ErrorList{}
 	if len(git.URI) == 0 {
 		allErrs = append(allErrs, field.Required(fldPath.Child("uri"), ""))
-	} else if !IsValidURL(git.URI) {
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("uri"), git.URI, "uri is not a valid url"))
+	} else if _, err := s2igit.Parse(git.URI); err != nil {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("uri"), git.URI, err.Error()))
 	}
-	if git.HTTPProxy != nil && len(*git.HTTPProxy) != 0 && !IsValidURL(*git.HTTPProxy) {
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("httpproxy"), *git.HTTPProxy, "proxy is not a valid url"))
+	if git.HTTPProxy != nil && len(*git.HTTPProxy) != 0 {
+		if _, err := buildutil.ParseProxyURL(*git.HTTPProxy); err != nil {
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("httpproxy"), *git.HTTPProxy, err.Error()))
+		}
 	}
-	if git.HTTPSProxy != nil && len(*git.HTTPSProxy) != 0 && !IsValidURL(*git.HTTPSProxy) {
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("httpsproxy"), *git.HTTPSProxy, "proxy is not a valid url"))
+	if git.HTTPSProxy != nil && len(*git.HTTPSProxy) != 0 {
+		if _, err := buildutil.ParseProxyURL(*git.HTTPSProxy); err != nil {
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("httpsproxy"), *git.HTTPSProxy, err.Error()))
+		}
 	}
 	return allErrs
 }
@@ -637,11 +641,6 @@ func validateWebHook(webHook *buildapi.WebHookTrigger, fldPath *field.Path, isGe
 	return allErrs
 }
 
-func IsValidURL(uri string) bool {
-	_, err := url.Parse(uri)
-	return err == nil
-}
-
 func ValidateBuildLogOptions(opts *buildapi.BuildLogOptions) field.ErrorList {
 	allErrs := field.ErrorList{}
 

pkg/build/apis/build/validation/validation_test.go

@@ -841,7 +841,7 @@ func TestValidateSource(t *testing.T) {
 			path: "git.uri",
 			source: &buildapi.BuildSource{
 				Git: &buildapi.GitBuildSource{
-					URI: "::",
+					URI: "http://%",
 				},
 			},
 		},

pkg/build/builder/sti.go

@@ -440,14 +440,14 @@ func scriptProxyConfig(build *buildapi.Build) (*s2iapi.ProxyConfig, error) {
 	}
 	config := &s2iapi.ProxyConfig{}
 	if len(httpProxy) > 0 {
-		proxyURL, err := url.Parse(httpProxy)
+		proxyURL, err := util.ParseProxyURL(httpProxy)
 		if err != nil {
 			return nil, err
 		}
 		config.HTTPProxy = proxyURL
 	}
 	if len(httpsProxy) > 0 {
-		proxyURL, err := url.Parse(httpsProxy)
+		proxyURL, err := util.ParseProxyURL(httpsProxy)
 		if err != nil {
 			return nil, err
 		}

pkg/build/controller/build/defaults/api/validation/validation.go

@@ -6,13 +6,14 @@ import (
 
 	buildvalidation "github.com/openshift/origin/pkg/build/apis/build/validation"
 	"github.com/openshift/origin/pkg/build/controller/build/defaults/api"
+	"github.com/openshift/origin/pkg/build/util"
 )
 
 // ValidateBuildDefaultsConfig tests required fields for a Build.
 func ValidateBuildDefaultsConfig(config *api.BuildDefaultsConfig) field.ErrorList {
 	allErrs := field.ErrorList{}
-	allErrs = append(allErrs, validateURL(config.GitHTTPProxy, field.NewPath("gitHTTPProxy"))...)
-	allErrs = append(allErrs, validateURL(config.GitHTTPSProxy, field.NewPath("gitHTTPSProxy"))...)
+	allErrs = append(allErrs, validateProxyURL(config.GitHTTPProxy, field.NewPath("gitHTTPProxy"))...)
+	allErrs = append(allErrs, validateProxyURL(config.GitHTTPSProxy, field.NewPath("gitHTTPSProxy"))...)
 	allErrs = append(allErrs, buildvalidation.ValidateStrategyEnv(config.Env, field.NewPath("env"))...)
 	allErrs = append(allErrs, buildvalidation.ValidateImageLabels(config.ImageLabels, field.NewPath("imageLabels"))...)
 	allErrs = append(allErrs, buildvalidation.ValidateNodeSelector(config.NodeSelector, field.NewPath("nodeSelector"))...)
@@ -21,10 +22,10 @@ func ValidateBuildDefaultsConfig(config *api.BuildDefaultsConfig) field.ErrorLis
 }
 
 //
-func validateURL(u string, path *field.Path) field.ErrorList {
+func validateProxyURL(u string, path *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
-	if !buildvalidation.IsValidURL(u) {
-		allErrs = append(allErrs, field.Invalid(path, u, "invalid URL"))
+	if _, err := util.ParseProxyURL(u); err != nil {
+		allErrs = append(allErrs, field.Invalid(path, u, err.Error()))
 	}
 	return allErrs
 }

pkg/build/util/util.go

@@ -461,3 +461,23 @@ func FindDockerSecretAsReference(secrets []kapi.Secret, image string) *kapi.Loca
 	}
 	return nil
 }
+
+// ParseProxyURL parses a proxy URL and allows fallback to non-URLs like
+// myproxy:80 (for example) which url.Parse no longer accepts in Go 1.8.  The
+// logic is copied from net/http.ProxyFromEnvironment to try to maintain
+// backwards compatibility.
+func ParseProxyURL(proxy string) (*url.URL, error) {
+	proxyURL, err := url.Parse(proxy)
+
+	// logic copied from net/http.ProxyFromEnvironment
+	if err != nil || !strings.HasPrefix(proxyURL.Scheme, "http") {
+		// proxy was bogus. Try prepending "http://" to it and see if that
+		// parses correctly. If not, we fall through and complain about the
+		// original one.
+		if proxyURL, err := url.Parse("http://" + proxy); err == nil {
+			return proxyURL, nil
+		}
+	}
+
+	return proxyURL, err
+}

test/extended/testdata/statusfail-genericreason.yaml

@@ -15,4 +15,4 @@ spec:
       scripts: "http://example.org/scripts"
       env:
         - name: http_proxy
-          value: ":http://example.org"
+          value: "http://%"