Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

validate imagestreamtag names in build definitions #9945

Merged
merged 1 commit into from
Jul 22, 2016
Merged

validate imagestreamtag names in build definitions #9945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pkg/build/api/validation/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/openshift/origin/pkg/build/api/v1"
buildutil "github.com/openshift/origin/pkg/build/util"
imageapi "github.com/openshift/origin/pkg/image/api"
imageapivalidation "github.com/openshift/origin/pkg/image/api/validation"
)

// ValidateBuild tests required fields for a Build.
Expand Down Expand Up @@ -347,6 +348,8 @@ func validateToImageReference(reference *kapi.ObjectReference, fldPath *field.Pa
allErrs = append(allErrs, field.Required(fldPath.Child("name"), ""))
} else if _, _, ok := imageapi.SplitImageStreamTag(name); !ok {
allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), name, "ImageStreamTag object references must be in the form <name>:<tag>"))
} else if name, _, _ := imageapi.SplitImageStreamTag(name); len(imageapivalidation.ValidateImageStreamName(name, false)) != 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), name, "ImageStreamTag name contains invalid syntax"))
}
if len(namespace) != 0 && len(kvalidation.IsDNS1123Subdomain(namespace)) != 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("namespace"), namespace, "namespace must be a valid subdomain"))
Expand Down Expand Up @@ -377,6 +380,8 @@ func validateFromImageReference(reference *kapi.ObjectReference, fldPath *field.
allErrs = append(allErrs, field.Required(fldPath.Child("name"), ""))
} else if _, _, ok := imageapi.SplitImageStreamTag(name); !ok {
allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), name, "ImageStreamTag object references must be in the form <name>:<tag>"))
} else if name, _, _ := imageapi.SplitImageStreamTag(name); len(imageapivalidation.ValidateImageStreamName(name, false)) != 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), name, "ImageStreamTag name contains invalid syntax"))
}

if len(namespace) != 0 && len(kvalidation.IsDNS1123Subdomain(namespace)) != 0 {
Expand Down
160 changes: 135 additions & 25 deletions pkg/build/api/validation/validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1275,6 +1275,69 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
// 1
{
string(field.ErrorTypeInvalid) + "output.to.kind",
buildapi.CommonSpec{
Source: buildapi.BuildSource{
Git: &buildapi.GitBuildSource{
URI: "http://github.com/my/repository",
},
ContextDir: "context",
},
Strategy: buildapi.BuildStrategy{
DockerStrategy: &buildapi.DockerBuildStrategy{},
},
Output: buildapi.BuildOutput{
To: &kapi.ObjectReference{
Kind: "ImageStream",
Name: "some/long/value/with/no/meaning",
},
},
},
},
// 2
{
string(field.ErrorTypeInvalid) + "output.to.name",
buildapi.CommonSpec{
Source: buildapi.BuildSource{
Git: &buildapi.GitBuildSource{
URI: "http://github.com/my/repository",
},
ContextDir: "context",
},
Strategy: buildapi.BuildStrategy{
DockerStrategy: &buildapi.DockerBuildStrategy{},
},
Output: buildapi.BuildOutput{
To: &kapi.ObjectReference{
Kind: "ImageStreamTag",
Name: "some/long/value/with/no/meaning",
},
},
},
},
// 3
{
string(field.ErrorTypeInvalid) + "output.to.name",
buildapi.CommonSpec{
Source: buildapi.BuildSource{
Git: &buildapi.GitBuildSource{
URI: "http://github.com/my/repository",
},
ContextDir: "context",
},
Strategy: buildapi.BuildStrategy{
DockerStrategy: &buildapi.DockerBuildStrategy{},
},
Output: buildapi.BuildOutput{
To: &kapi.ObjectReference{
Kind: "ImageStreamTag",
Name: "some/long/value/with/no/meaning:latest",
},
},
},
},
// 4
{
string(field.ErrorTypeInvalid) + "output.to.kind",
buildapi.CommonSpec{
Expand All @@ -1295,7 +1358,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 2
// 5
{
string(field.ErrorTypeRequired) + "output.to.kind",
buildapi.CommonSpec{
Expand All @@ -1313,7 +1376,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 3
// 6
{
string(field.ErrorTypeRequired) + "output.to.name",
buildapi.CommonSpec{
Expand All @@ -1333,7 +1396,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 4
// 7
{
string(field.ErrorTypeInvalid) + "output.to.name",
buildapi.CommonSpec{
Expand All @@ -1355,7 +1418,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 5
// 8
{
string(field.ErrorTypeInvalid) + "output.to.namespace",
buildapi.CommonSpec{
Expand All @@ -1377,7 +1440,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 6
// 9
// invalid because from is not specified in the
// sti strategy definition
{
Expand All @@ -1399,7 +1462,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 7
// 10
// Invalid because from.name is not specified
{
string(field.ErrorTypeRequired) + "strategy.sourceStrategy.from.name",
Expand All @@ -1424,7 +1487,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 8
// 11
// invalid because from name is a bad format
{
string(field.ErrorTypeInvalid) + "strategy.sourceStrategy.from.name",
Expand All @@ -1447,7 +1510,54 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 9
// 12
// invalid because from name is a bad format
{
string(field.ErrorTypeInvalid) + "strategy.sourceStrategy.from.name",
buildapi.CommonSpec{
Source: buildapi.BuildSource{
Git: &buildapi.GitBuildSource{
URI: "http://github.com/my/repository",
},
},
Strategy: buildapi.BuildStrategy{
SourceStrategy: &buildapi.SourceBuildStrategy{
From: kapi.ObjectReference{Kind: "ImageStreamTag", Name: "badformat"},
},
},
Output: buildapi.BuildOutput{
To: &kapi.ObjectReference{
Kind: "DockerImage",
Name: "repository/data",
},
},
},
},
// 13
// invalid because from name is a bad format
{
string(field.ErrorTypeInvalid) + "strategy.sourceStrategy.from.name",
buildapi.CommonSpec{
Source: buildapi.BuildSource{
Git: &buildapi.GitBuildSource{
URI: "http://github.com/my/repository",
},
},
Strategy: buildapi.BuildStrategy{
SourceStrategy: &buildapi.SourceBuildStrategy{
From: kapi.ObjectReference{Kind: "ImageStreamTag", Name: "bad/format:latest"},
},
},
Output: buildapi.BuildOutput{
To: &kapi.ObjectReference{
Kind: "DockerImage",
Name: "repository/data",
},
},
},
},

// 14
// invalid because from is not specified in the
// custom strategy definition
{
Expand All @@ -1469,7 +1579,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 10
// 15
// invalid because from.name is not specified in the
// custom strategy definition
{
Expand All @@ -1493,7 +1603,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 11
// 16
{
string(field.ErrorTypeInvalid) + "source.dockerfile",
buildapi.CommonSpec{
Expand All @@ -1505,7 +1615,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 12
// 17
{
string(field.ErrorTypeInvalid) + "source.dockerfile",
buildapi.CommonSpec{
Expand All @@ -1520,7 +1630,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 13
// 18
// invalid because CompletionDeadlineSeconds <= 0
{
string(field.ErrorTypeInvalid) + "completionDeadlineSeconds",
Expand All @@ -1543,7 +1653,7 @@ func TestValidateCommonSpec(t *testing.T) {
CompletionDeadlineSeconds: &zero,
},
},
// 14
// 19
// must provide some source input
{
string(field.ErrorTypeInvalid) + "source",
Expand All @@ -1560,7 +1670,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 15
// 20
// dockerfilePath can't be an absolute path
{
string(field.ErrorTypeInvalid) + "strategy.dockerStrategy.dockerfilePath",
Expand All @@ -1584,7 +1694,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 16
// 21
// dockerfilePath can't start with ../
{
string(field.ErrorTypeInvalid) + "strategy.dockerStrategy.dockerfilePath",
Expand All @@ -1608,7 +1718,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 17
// 22
// dockerfilePath can't reference a path outside of the dir
{
string(field.ErrorTypeInvalid) + "strategy.dockerStrategy.dockerfilePath",
Expand All @@ -1632,7 +1742,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 18
// 23
// dockerfilePath can't equal ..
{
string(field.ErrorTypeInvalid) + "strategy.dockerStrategy.dockerfilePath",
Expand All @@ -1656,7 +1766,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 19
// 24
{
string(field.ErrorTypeInvalid) + "postCommit",
buildapi.CommonSpec{
Expand All @@ -1674,7 +1784,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 20
// 25
{
string(field.ErrorTypeInvalid) + "source.git",
buildapi.CommonSpec{
Expand All @@ -1683,7 +1793,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 21
// 26
{
string(field.ErrorTypeInvalid) + "source.git",
buildapi.CommonSpec{
Expand All @@ -1694,7 +1804,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 22
// 27
// jenkinsfilePath can't be an absolute path
{
string(field.ErrorTypeInvalid) + "strategy.jenkinsPipelineStrategy.jenkinsfilePath",
Expand All @@ -1711,7 +1821,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 23
// 28
// jenkinsfilePath can't start with ../
{
string(field.ErrorTypeInvalid) + "strategy.jenkinsPipelineStrategy.jenkinsfilePath",
Expand All @@ -1728,7 +1838,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 24
// 29
// jenkinsfilePath can't be a reference a path outside of the dir
{
string(field.ErrorTypeInvalid) + "strategy.jenkinsPipelineStrategy.jenkinsfilePath",
Expand All @@ -1745,7 +1855,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 25
// 30
// jenkinsfilePath can't be equal to ..
{
string(field.ErrorTypeInvalid) + "strategy.jenkinsPipelineStrategy.jenkinsfilePath",
Expand All @@ -1762,7 +1872,7 @@ func TestValidateCommonSpec(t *testing.T) {
},
},
},
// 26
// 31
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I've started changing the test to use maps with human readable descriptions instead of indexes 😉

// path must be shorter than 100k
{
string(field.ErrorTypeInvalid) + "strategy.jenkinsPipelineStrategy.jenkinsfile",
Expand Down