From ba5c9b492e2b81c7c2f7bf4ecb31c03508a1a209 Mon Sep 17 00:00:00 2001
From: "Dr. Stefan Schimanski" <stefan.schimanski@gmail.com>
Date: Tue, 28 May 2019 22:52:48 +0200
Subject: [PATCH 1/2] apiextensions: add invalid validation schema regex test

---
 .../pkg/apiserver/validation/validation_test.go    | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go
index 43d62b4591e..c0bf419d8bd 100644
--- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go
+++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go
@@ -139,7 +139,7 @@ func convertNullTypeToNullable(x interface{}) interface{} {
 	}
 }
 
-func TestNullable(t *testing.T) {
+func TestValidateCustomResource(t *testing.T) {
 	type args struct {
 		schema apiextensions.JSONSchemaProps
 		object interface{}
@@ -149,6 +149,7 @@ func TestNullable(t *testing.T) {
 		args    args
 		wantErr bool
 	}{
+		// TODO: make more complete
 		{"!nullable against non-null", args{
 			apiextensions.JSONSchemaProps{
 				Properties: map[string]apiextensions.JSONSchemaProps{
@@ -235,6 +236,17 @@ func TestNullable(t *testing.T) {
 			},
 			map[string]interface{}{"field": nil},
 		}, false},
+		{"invalid regex", args{
+			apiextensions.JSONSchemaProps{
+				Properties: map[string]apiextensions.JSONSchemaProps{
+					"field": {
+						Type:    "string",
+						Pattern: "+",
+					},
+				},
+			},
+			map[string]interface{}{"field": "foo"},
+		}, true},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

From 0ab6db225111678b7046f74374eb4cbce40eb1c0 Mon Sep 17 00:00:00 2001
From: "Dr. Stefan Schimanski" <stefan.schimanski@gmail.com>
Date: Thu, 30 May 2019 16:04:40 +0200
Subject: [PATCH 2/2] apiextensions: verify pattern regex for structural schema

---
 .../apiextensions/validation/validation_test.go     |  1 +
 .../pkg/apiserver/schema/validation.go              |  8 ++++++++
 .../test/integration/validation_test.go             | 13 +++++++++++++
 3 files changed, 22 insertions(+)

diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go
index 1d174e34ead..cd65cdf1050 100644
--- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go
+++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go
@@ -1692,6 +1692,7 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
 				invalid("spec", "validation", "openAPIV3Schema", "properties[a]", "default"),
 				invalid("spec", "validation", "openAPIV3Schema", "properties[c]", "default"),
 				invalid("spec", "validation", "openAPIV3Schema", "properties[d]", "default"),
+				invalid("spec", "validation", "openAPIV3Schema", "properties[d]", "properties[bad]", "pattern"),
 				// we also expected unpruned and valid defaults under x-kubernetes-preserve-unknown-fields. We could be more
 				// strict here, but want to encourage proper specifications by forbidding other defaults.
 				invalid("spec", "validation", "openAPIV3Schema", "properties[e]", "properties[preserveUnknownFields]", "default"),
diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation.go
index e864fe8c249..c9ed78f3679 100644
--- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation.go
+++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation.go
@@ -17,7 +17,9 @@ limitations under the License.
 package schema
 
 import (
+	"fmt"
 	"reflect"
+	"regexp"
 	"sort"
 
 	"k8s.io/apimachinery/pkg/util/validation/field"
@@ -226,6 +228,12 @@ func validateValueValidation(v *ValueValidation, skipAnyOf, skipFirstAllOfAnyOf
 
 	allErrs = append(allErrs, validateNestedValueValidation(v.Not, false, false, lvl, fldPath.Child("not"))...)
 
+	if len(v.Pattern) > 0 {
+		if _, err := regexp.Compile(v.Pattern); err != nil {
+			allErrs = append(allErrs, field.Invalid(fldPath.Child("pattern"), v.Pattern, fmt.Sprintf("must be a valid regular expression, but isn't: %v", err)))
+		}
+	}
+
 	return allErrs
 }
 
diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/validation_test.go
index f4f290e8a51..ec1ac91b05b 100644
--- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/validation_test.go
+++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/validation_test.go
@@ -870,6 +870,19 @@ oneOf:
 				"spec.validation.openAPIV3Schema.not.default",
 			},
 		},
+		{
+			desc: "invalid regex pattern",
+			globalSchema: `
+type: object
+properties:
+  foo:
+    type: string
+    pattern: "+"
+`,
+			expectedViolations: []string{
+				"spec.validation.openAPIV3Schema.properties[foo].pattern: Invalid value: \"+\": must be a valid regular expression, but isn't: error parsing regexp: missing argument to repetition operator: `+`",
+			},
+		},
 		{
 			desc: "forbidden vendor extensions in nested value validation",
 			globalSchema: `