Disallow @ character in URL patterns, so that people don't mistakenly…

… try

to add URL patterns of the form username@hostname/path.

Extend admission controller to reject invalid URL patterns on secrets to
provide early feedback to end users when their patterns are wrong.

pkg/build/admission/secretinjector/admission.go

@@ -10,9 +10,11 @@ import (
 
 	"k8s.io/kubernetes/pkg/admission"
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/errors"
 	clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	coreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion"
 	"k8s.io/kubernetes/pkg/client/restclient"
+	"k8s.io/kubernetes/pkg/util/validation/field"
 
 	authclient "github.com/openshift/origin/pkg/auth/client"
 	buildapi "github.com/openshift/origin/pkg/build/api"
@@ -23,7 +25,7 @@ import (
 func init() {
 	admission.RegisterPlugin("openshift.io/BuildConfigSecretInjector", func(c clientset.Interface, config io.Reader) (admission.Interface, error) {
 		return &secretInjector{
-			Handler: admission.NewHandler(admission.Create),
+			Handler: admission.NewHandler(admission.Create, admission.Update),
 		}, nil
 	})
 }
@@ -36,11 +38,20 @@ type secretInjector struct {
 var _ = oadmission.WantsRESTClientConfig(&secretInjector{})
 
 func (si *secretInjector) Admit(attr admission.Attributes) (err error) {
-	bc, ok := attr.GetObject().(*buildapi.BuildConfig)
-	if !ok {
-		return nil
+	obj := attr.GetObject()
+
+	if bc, ok := obj.(*buildapi.BuildConfig); ok && attr.GetOperation() == admission.Create {
+		return si.admitNewBuildConfig(attr, bc)
 	}
 
+	if secret, ok := obj.(*api.Secret); ok {
+		return si.admitSecret(attr, secret)
+	}
+
+	return nil
+}
+
+func (si *secretInjector) admitNewBuildConfig(attr admission.Attributes, bc *buildapi.BuildConfig) (err error) {
 	if bc.Spec.Source.SourceSecret != nil || bc.Spec.Source.Git == nil {
 		return nil
 	}
@@ -105,6 +116,30 @@ func (si *secretInjector) Admit(attr admission.Attributes) (err error) {
 	return nil
 }
 
+func (si *secretInjector) admitSecret(attr admission.Attributes, secret *api.Secret) (err error) {
+	errs := field.ErrorList{}
+
+	for k, v := range secret.GetAnnotations() {
+		if strings.HasPrefix(k, buildapi.BuildSourceSecretMatchURIAnnotationPrefix) {
+			v = strings.TrimSpace(v)
+			if v == "" {
+				continue
+			}
+
+			_, err := urlpattern.NewURLPattern(v)
+			if err != nil {
+				errs = append(errs, field.Invalid(field.NewPath("metadata.annotations", k), v, err.Error()))
+			}
+		}
+	}
+
+	if len(errs) > 0 {
+		return errors.NewInvalid(api.Kind("secret"), secret.Name, errs)
+	}
+
+	return nil
+}
+
 func (si *secretInjector) SetRESTClientConfig(restClientConfig restclient.Config) {
 	si.restClientConfig = restClientConfig
 }

pkg/util/urlpattern/urlpattern.go

@@ -12,7 +12,7 @@ var InvalidPatternError = errors.New("invalid pattern")
 
 var urlPatternRegex = regexp.MustCompile(`^` +
 	`(?:(\*|git|http|https|ssh)://)` +
-	`(\*|(?:\*\.)?[^/*]+)` +
+	`(\*|(?:\*\.)?[^@/*]+)` +
 	`(/.*)` +
 	`$`)
 

pkg/util/urlpattern/urlpattern_test.go

@@ -108,6 +108,10 @@ func TestMatchPattern(t *testing.T) {
 			pattern:     `https://git*hub.com/*`,
 			expectedErr: true,
 		},
+		{
+			pattern:     `*://git@github.com/*`,
+			expectedErr: true,
+		},
 		{
 			pattern:        `https://github.com/`,
 			expectedScheme: `^https$`,