From 741d2711d746f577079dbf97022a169a1a157915 Mon Sep 17 00:00:00 2001 From: deads2k Date: Tue, 21 Apr 2015 15:21:51 -0400 Subject: [PATCH 1/2] add imagestream validation --- pkg/api/validation/validation.go | 1 + pkg/image/api/validation/validation.go | 24 ++++++++++++------- pkg/image/api/validation/validation_test.go | 20 +++++++++------- .../imagerepositorymapping/rest_test.go | 6 ++--- 4 files changed, 29 insertions(+), 22 deletions(-) diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index da994a1b3bee..a08750c084f6 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -43,6 +43,7 @@ func ValidateObject(obj runtime.Object) (errors []error) { errors = validation.ValidateMinion(t) case *imageapi.Image: + t.Namespace = "" errors = imagev.ValidateImage(t) case *imageapi.ImageRepository: s := &imageapi.ImageStream{} diff --git a/pkg/image/api/validation/validation.go b/pkg/image/api/validation/validation.go index ef1db20ff8ef..c9149e663a35 100644 --- a/pkg/image/api/validation/validation.go +++ b/pkg/image/api/validation/validation.go @@ -2,6 +2,7 @@ package validation import ( "fmt" + "strings" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" @@ -10,13 +11,21 @@ import ( "github.com/openshift/origin/pkg/image/api" ) +func MinimalNameValidation(name string, prefix bool) (bool, string) { + for _, illegal := range []string{"%", "/"} { + if strings.Contains(name, illegal) { + return false, fmt.Sprintf(`may not contain "%s"`, illegal) + } + } + return true, "" +} + // ValidateImage tests required fields for an Image. func ValidateImage(image *api.Image) fielderrors.ValidationErrorList { result := fielderrors.ValidationErrorList{} - if len(image.Name) == 0 { - result = append(result, fielderrors.NewFieldRequired("name")) - } + result = append(result, validation.ValidateObjectMeta(&image.ObjectMeta, false, MinimalNameValidation).Prefix("metadata")...) + if len(image.DockerImageReference) == 0 { result = append(result, fielderrors.NewFieldRequired("dockerImageReference")) } else { @@ -32,15 +41,12 @@ func ValidateImage(image *api.Image) fielderrors.ValidationErrorList { func ValidateImageStream(stream *api.ImageStream) fielderrors.ValidationErrorList { result := fielderrors.ValidationErrorList{} + result = append(result, validation.ValidateObjectMeta(&stream.ObjectMeta, true, MinimalNameValidation).Prefix("metadata")...) + if stream.Spec.Tags == nil { stream.Spec.Tags = make(map[string]api.TagReference) } - if len(stream.Name) == 0 { - result = append(result, fielderrors.NewFieldRequired("name")) - } - if !util.IsDNS1123Subdomain(stream.Namespace) { - result = append(result, fielderrors.NewFieldInvalid("namespace", stream.Namespace, "")) - } + if len(stream.Spec.DockerImageRepository) != 0 { if _, err := api.ParseDockerImageReference(stream.Spec.DockerImageRepository); err != nil { result = append(result, fielderrors.NewFieldInvalid("spec.dockerImageRepository", stream.Spec.DockerImageRepository, err.Error())) diff --git a/pkg/image/api/validation/validation_test.go b/pkg/image/api/validation/validation_test.go index 7788676d8b9c..6962debbb667 100644 --- a/pkg/image/api/validation/validation_test.go +++ b/pkg/image/api/validation/validation_test.go @@ -12,7 +12,7 @@ import ( func TestValidateImageOK(t *testing.T) { errs := ValidateImage(&api.Image{ - ObjectMeta: kapi.ObjectMeta{Name: "foo", Namespace: "default"}, + ObjectMeta: kapi.ObjectMeta{Name: "foo"}, DockerImageReference: "openshift/ruby-19-centos", }) if len(errs) > 0 { @@ -29,7 +29,12 @@ func TestValidateImageMissingFields(t *testing.T) { "missing Name": { api.Image{DockerImageReference: "ref"}, fielderrors.ValidationErrorTypeRequired, - "name", + "metadata.name", + }, + "no slash in Name": { + api.Image{ObjectMeta: kapi.ObjectMeta{Name: "foo/bar"}}, + fielderrors.ValidationErrorTypeInvalid, + "metadata.name", }, "missing DockerImageReference": { api.Image{ObjectMeta: kapi.ObjectMeta{Name: "foo"}}, @@ -122,14 +127,11 @@ func TestValidateImageStreamMappingNotOK(t *testing.T) { DockerImageRepository: "openshift/ruby-19-centos", Tag: "latest", Image: api.Image{ - ObjectMeta: kapi.ObjectMeta{ - Namespace: "default", - }, DockerImageReference: "openshift/ruby-19-centos", }, }, fielderrors.ValidationErrorTypeRequired, - "image.name", + "image.metadata.name", }, "invalid repository pull spec": { api.ImageStreamMapping{ @@ -183,21 +185,21 @@ func TestValidateImageStream(t *testing.T) { namespace: "foo", name: "", expected: fielderrors.ValidationErrorList{ - fielderrors.NewFieldRequired("name"), + fielderrors.NewFieldRequired("metadata.name"), }, }, "missing namespace": { namespace: "", name: "foo", expected: fielderrors.ValidationErrorList{ - fielderrors.NewFieldInvalid("namespace", "", ""), + fielderrors.NewFieldRequired("metadata.namespace"), }, }, "invalid namespace": { namespace: "!$", name: "foo", expected: fielderrors.ValidationErrorList{ - fielderrors.NewFieldInvalid("namespace", "!$", ""), + fielderrors.NewFieldInvalid("metadata.namespace", "!$", `must have at most 253 characters and match regex [a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*`), }, }, "invalid dockerImageRepository": { diff --git a/pkg/image/registry/imagerepositorymapping/rest_test.go b/pkg/image/registry/imagerepositorymapping/rest_test.go index 3a90e88e386f..00406b1e9df7 100644 --- a/pkg/image/registry/imagerepositorymapping/rest_test.go +++ b/pkg/image/registry/imagerepositorymapping/rest_test.go @@ -356,8 +356,7 @@ func TestAddExistingImageWithNewTag(t *testing.T) { existingImage := &api.Image{ ObjectMeta: kapi.ObjectMeta{ - Name: imageID, - Namespace: "default", + Name: imageID, }, DockerImageReference: "localhost:5000/someproject/somerepo:" + imageID, DockerImageMetadata: api.DockerImage{ @@ -483,8 +482,7 @@ func TestAddExistingImageAndTag(t *testing.T) { existingImage := &api.Image{ ObjectMeta: kapi.ObjectMeta{ - Name: "existingImage", - Namespace: "default", + Name: "existingImage", }, DockerImageReference: "localhost:5000/someproject/somerepo:imageID1", DockerImageMetadata: api.DockerImage{ From 6e359df74e7ead745d4ac0229e98bf1c0b512424 Mon Sep 17 00:00:00 2001 From: deads2k Date: Wed, 22 Apr 2015 14:44:01 -0400 Subject: [PATCH 2/2] more tests --- pkg/image/api/validation/validation_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/pkg/image/api/validation/validation_test.go b/pkg/image/api/validation/validation_test.go index 6962debbb667..93cefc086ce0 100644 --- a/pkg/image/api/validation/validation_test.go +++ b/pkg/image/api/validation/validation_test.go @@ -36,6 +36,11 @@ func TestValidateImageMissingFields(t *testing.T) { fielderrors.ValidationErrorTypeInvalid, "metadata.name", }, + "no percent in Name": { + api.Image{ObjectMeta: kapi.ObjectMeta{Name: "foo%%bar"}}, + fielderrors.ValidationErrorTypeInvalid, + "metadata.name", + }, "missing DockerImageReference": { api.Image{ObjectMeta: kapi.ObjectMeta{Name: "foo"}}, fielderrors.ValidationErrorTypeRequired, @@ -188,6 +193,20 @@ func TestValidateImageStream(t *testing.T) { fielderrors.NewFieldRequired("metadata.name"), }, }, + "no slash in Name": { + namespace: "foo", + name: "foo/bar", + expected: fielderrors.ValidationErrorList{ + fielderrors.NewFieldInvalid("metadata.name", "foo/bar", `may not contain "/"`), + }, + }, + "no percent in Name": { + namespace: "foo", + name: "foo%%bar", + expected: fielderrors.ValidationErrorList{ + fielderrors.NewFieldInvalid("metadata.name", "foo%%bar", `may not contain "%"`), + }, + }, "missing namespace": { namespace: "", name: "foo",