Validation check for InferenceService Name (kubeflow#1079)

* Validation check for InferenceService Name . Check if ISVC name starts with alphabetical character. Fixes kubeflow#1059 * Change the point of ASCII code to check if alphabet or not * Validate ISVC name with Regexp * . Add unit tests . Move const and var section on the top * Apply regular expressions for validation of isvc name to v1beta1 * Remove validation for Max length of isvc name, because default and canary have different length.
magdalenakuhn17 · Sep 26, 2020 · 31903fa · 31903fa
1 parent deba8df
commit 31903fa
Show file tree

Hide file tree

Showing 5 changed files with 85 additions and 0 deletions.
diff --git a/pkg/apis/serving/v1alpha2/inferenceservice_validation.go b/pkg/apis/serving/v1alpha2/inferenceservice_validation.go
@@ -19,6 +19,7 @@ package v1alpha2
 import (
 	"fmt"
 	"k8s.io/apimachinery/pkg/runtime"
+	"regexp"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -33,12 +34,19 @@ const (
 	TrafficBoundsExceededError          = "TrafficPercent must be between [0, 100]."
 	TrafficProvidedWithoutCanaryError   = "Canary must be specified when CanaryTrafficPercent > 0."
 	UnsupportedStorageURIFormatError    = "storageUri, must be one of: [%s] or match https://{}.blob.core.windows.net/{}/{} or be an absolute or relative local path. StorageUri [%s] is not supported."
+	InvalidISVCNameFormatError          = "The InferenceService \"%s\" is invalid: a InferenceService name must consist of lower case alphanumeric characters or '-', and must start with alphabetical character. (e.g. \"my-name\" or \"abc-123\", regex used for validation is '%s')"
+)
+
+// Validation for isvc name
+const (
+	IsvcNameFmt string = "[a-z]([-a-z0-9]*[a-z0-9])?"
 )
 
 var (
 	SupportedStorageURIPrefixList = []string{"gs://", "s3://", "pvc://", "file://", "https://", "http://"}
 	AzureBlobURL                  = "blob.core.windows.net"
 	AzureBlobURIRegEx             = "https://(.+?).blob.core.windows.net/(.+)"
+	IsvcRegexp                    = regexp.MustCompile("^" + IsvcNameFmt + "$")
 )
 
 var _ webhook.Validator = &InferenceService{}
@@ -81,6 +89,11 @@ func validateInferenceService(isvc *InferenceService, client client.Client) erro
 	if isvc == nil {
 		return fmt.Errorf("Unable to validate, InferenceService is nil")
 	}
+
+	if err := validateInferenceServiceName(isvc); err != nil {
+		return err
+	}
+
 	endpoints := []*EndpointSpec{
 		&isvc.Spec.Default,
 		isvc.Spec.Canary,
@@ -140,3 +153,10 @@ func validateCanaryTrafficPercent(spec InferenceServiceSpec) error {
 	}
 	return nil
 }
+
+func validateInferenceServiceName(isvc *InferenceService) error {
+	if !IsvcRegexp.MatchString(isvc.Name) {
+		return fmt.Errorf(InvalidISVCNameFormatError, isvc.Name, IsvcNameFmt)
+	}
+	return nil
+}
diff --git a/pkg/apis/serving/v1alpha2/inferenceservice_validation_test.go b/pkg/apis/serving/v1alpha2/inferenceservice_validation_test.go
@@ -277,3 +277,24 @@ func TestGoodExplainer(t *testing.T) {
 	isvc.applyDefaultsEndpoint(&isvc.Spec.Default, c)
 	g.Expect(isvc.validate(c)).Should(gomega.Succeed())
 }
+
+func TestGoodName(t *testing.T) {
+	g := gomega.NewGomegaWithT(t)
+	isvc := makeTestInferenceService()
+	isvc.Name = "abc-123"
+	g.Expect(isvc.ValidateCreate(c)).Should(gomega.Succeed())
+}
+
+func TestRejectBadNameStartWithNumber(t *testing.T) {
+	g := gomega.NewGomegaWithT(t)
+	isvc := makeTestInferenceService()
+	isvc.Name = "1abcde"
+	g.Expect(isvc.ValidateCreate(c)).ShouldNot(gomega.Succeed())
+}
+
+func TestRejectBadNameIncludeDot(t *testing.T) {
+	g := gomega.NewGomegaWithT(t)
+	isvc := makeTestInferenceService()
+	isvc.Name = "abc.de"
+	g.Expect(isvc.ValidateCreate(c)).ShouldNot(gomega.Succeed())
+}
diff --git a/pkg/apis/serving/v1beta1/component.go b/pkg/apis/serving/v1beta1/component.go
@@ -36,6 +36,7 @@ const (
 	ParallelismLowerBoundExceededError  = "Parallelism cannot be less than 0."
 	UnsupportedStorageURIFormatError    = "storageUri, must be one of: [%s] or match https://{}.blob.core.windows.net/{}/{} or be an absolute or relative local path. StorageUri [%s] is not supported."
 	InvalidLoggerType                   = "Invalid logger type"
+	InvalidISVCNameFormatError          = "The InferenceService \"%s\" is invalid: a InferenceService name must consist of lower case alphanumeric characters or '-', and must start with alphabetical character. (e.g. \"my-name\" or \"abc-123\", regex used for validation is '%s')"
 )
 
 // Constants

diff --git a/pkg/apis/serving/v1beta1/inference_service_validation.go b/pkg/apis/serving/v1beta1/inference_service_validation.go
@@ -17,17 +17,26 @@ limitations under the License.
 package v1beta1
 
 import (
+	"fmt"
 	"reflect"
 
 	"github.com/kubeflow/kfserving/pkg/utils"
 	"k8s.io/apimachinery/pkg/runtime"
+	"regexp"
 	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 )
 
+// regular expressions for validation of isvc name
+const (
+	IsvcNameFmt string = "[a-z]([-a-z0-9]*[a-z0-9])?"
+)
+
 var (
 	// logger for the validation webhook.
 	validatorLogger = logf.Log.WithName("inferenceservice-v1beta1-validation-webhook")
+	// regular expressions for validation of isvc name
+	IsvcRegexp = regexp.MustCompile("^" + IsvcNameFmt + "$")
 )
 
 // +kubebuilder:webhook:verbs=create;update,path=/validate-inferenceservices,mutating=false,failurePolicy=fail,groups=serving.kubeflow.org,resources=inferenceservices,versions=v1beta1,name=inferenceservice.kfserving-webhook-server.validator
@@ -36,6 +45,11 @@ var _ webhook.Validator = &InferenceService{}
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type
 func (isvc *InferenceService) ValidateCreate() error {
 	validatorLogger.Info("validate create", "name", isvc.Name)
+
+	if err := validateInferenceServiceName(isvc); err != nil {
+		return err
+	}
+
 	for _, component := range []Component{
 		&isvc.Spec.Predictor,
 		isvc.Spec.Transformer,
@@ -74,3 +88,11 @@ func GetIntReference(number int) *int {
 	num := number
 	return &num
 }
+
+// Validation of isvc name
+func validateInferenceServiceName(isvc *InferenceService) error {
+	if !IsvcRegexp.MatchString(isvc.Name) {
+		return fmt.Errorf(InvalidISVCNameFormatError, isvc.Name, IsvcNameFmt)
+	}
+	return nil
+}
diff --git a/pkg/apis/serving/v1beta1/inference_service_validation_test.go b/pkg/apis/serving/v1beta1/inference_service_validation_test.go
@@ -236,3 +236,24 @@ func TestGoodExplainer(t *testing.T) {
 	}
 	g.Expect(isvc.ValidateCreate()).Should(gomega.Succeed())
 }
+
+func TestGoodName(t *testing.T) {
+	g := gomega.NewGomegaWithT(t)
+	isvc := makeTestInferenceService()
+	isvc.Name = "abc-123"
+	g.Expect(isvc.ValidateCreate()).Should(gomega.Succeed())
+}
+
+func TestRejectBadNameStartWithNumber(t *testing.T) {
+	g := gomega.NewGomegaWithT(t)
+	isvc := makeTestInferenceService()
+	isvc.Name = "1abcde"
+	g.Expect(isvc.ValidateCreate()).ShouldNot(gomega.Succeed())
+}
+
+func TestRejectBadNameIncludeDot(t *testing.T) {
+	g := gomega.NewGomegaWithT(t)
+	isvc := makeTestInferenceService()
+	isvc.Name = "abc.de"
+	g.Expect(isvc.ValidateCreate()).ShouldNot(gomega.Succeed())
+}