From 31903fa2b257b74e2801f0be24e77d01061dcdea Mon Sep 17 00:00:00 2001 From: jazzsir Date: Sat, 26 Sep 2020 09:18:48 +0900 Subject: [PATCH] Validation check for InferenceService Name (#1079) * Validation check for InferenceService Name . Check if ISVC name starts with alphabetical character. Fixes #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. --- .../v1alpha2/inferenceservice_validation.go | 20 +++++++++++++++++ .../inferenceservice_validation_test.go | 21 ++++++++++++++++++ pkg/apis/serving/v1beta1/component.go | 1 + .../v1beta1/inference_service_validation.go | 22 +++++++++++++++++++ .../inference_service_validation_test.go | 21 ++++++++++++++++++ 5 files changed, 85 insertions(+) diff --git a/pkg/apis/serving/v1alpha2/inferenceservice_validation.go b/pkg/apis/serving/v1alpha2/inferenceservice_validation.go index b82418e55be..d045d3ee622 100644 --- 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 index d6c46f5da52..8f29e5c3375 100644 --- 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 index eecc9b6bd7e..876736debb7 100644 --- 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 index 381f81df5ee..0c414c3d581 100644 --- 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 index f8da3badc9f..445bbd727bc 100644 --- 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()) +}