Validate project and image stream namespaces and names

[liggitt]

To ensure we end up with valid image repository paths derived from ImageStreams, add the following validation:

• Require Project names to be >= 2 characters on creation
• Require ImageStream namespaces to be >= 2 characters
• Require ImageStream namespace+'/'+name to be <= 255 characters
• Require ImageStream names to be valid docker image repository names

To ensure we don't break updating an existing project/namespace that was created with a single character name:

• Allow updating projects with names < 2 characters

Fixes #1232

[liggitt]

@derekwaynecarr @ncdc PTAL

[ncdc]

<= 255, < 256

[liggitt]

@ncdc imagestream comments addressed, ParseDockerImageReference validation added as well

[ncdc]

Each component must be at least 2 chars - meaning both ns and ref.Name

[ncdc]

You can call this if you want https://github.com/docker/distribution/blob/master/registry/api/v2/names.go#L78

[liggitt]

@ncdc updated to use docker constants/validation

[ncdc]

This is technically only relevant if the stream is using our internal registry. If spec.dockerImageRepository is set, it's ok to have a 1-char ns or name, since they won't be used in the push/pull spec. Not sure if it's worth making that distinction in the code, however.

[liggitt]

Since they can switch between internal/external post-creation, I'd rather keep this "strict" for now

[ncdc]

ImageStream & DockerImageReference changes LGTM

[derekwaynecarr]

I don't think you need to change this signature.

On create we call:
https://github.com/openshift/origin/blob/master/pkg/project/registry/project/rest.go#L57

On update we call:
https://github.com/openshift/origin/blob/master/pkg/project/registry/project/rest.go#L67

[liggitt]

This was calling ValidateProject(), and I couldn't tell how it was used: https://github.com/openshift/origin/pull/2351/files#diff-f19a7777a36882029db668a427f950feL71

[liggitt]

It was only used in an examples test, removed the signature change

[liggitt]

[test]

[openshift-bot]

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/2330/)

[smarterclayton]

Very scary.

[smarterclayton]

I think this code is used by things where this is not a safe assumption.

[ncdc]

We had the defaulting for registry/name but not for name. What code do we have that expects no namespace in a pull spec?

[smarterclayton]

----- Original Message -----

default:
    return ref, fmt.Errorf("the docker pull spec %q must be two or three
    segments separated by slashes", spec)
}

• if len(ref.Namespace) == 0 {

We had the defaulting for registry/name but not for name. What code do we
have that expects no namespace in a pull spec?

Is library even present in v2?

[liggitt]

I'm putting back the "library" defaulting where we had it before, and only validating what they passed.

[ncdc]

Library isn't a strict requirement in v2, but the Docker daemon still prepends library if you only specify foo

[liggitt]

I'm less concerned if they give us an invalid spec and it fails. The crux of this PR was preventing them from giving us namespace/imagestream pieces we would assemble into an invalid image repository

[ncdc]

I'm fine with keeping the library default the way it was originally

[liggitt]

@smarterclayton, you ok with this now?

[liggitt]

@ncdc @smarterclayton backed out the changes in ParseDockerImageReference to limit the scope of this PR. PTAL

[ncdc]

LGTM

[liggitt]

[merge]

[openshift-bot]

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/2052/) (Image: devenv-fedora_1595)

[openshift-bot]

Evaluated for origin up to de47827