diff --git a/pkg/build/api/v1beta1/conversion.go b/pkg/build/api/v1beta1/conversion.go index 01cf185accb4..2536847df981 100644 --- a/pkg/build/api/v1beta1/conversion.go +++ b/pkg/build/api/v1beta1/conversion.go @@ -90,9 +90,13 @@ func init() { out.Tag = in.Tag if len(in.DockerImageReference) > 0 { out.DockerImageReference = in.DockerImageReference - registry, namespace, name, tag, _ := image.SplitDockerPullSpec(in.DockerImageReference) - out.Registry = registry - out.ImageTag = image.JoinDockerPullSpec("", namespace, name, tag) + ref, err := image.ParseDockerImageReference(in.DockerImageReference) + if err != nil { + return err + } + out.Registry = ref.Registry + ref.Registry = "" + out.ImageTag = ref.String() } return nil }, @@ -106,11 +110,12 @@ func init() { return nil } if len(in.ImageTag) != 0 { - _, namespace, name, tag, err := image.SplitDockerPullSpec(in.ImageTag) + ref, err := image.ParseDockerImageReference(in.ImageTag) if err != nil { return err } - out.DockerImageReference = image.JoinDockerPullSpec(in.Registry, namespace, name, tag) + ref.Registry = in.Registry + out.DockerImageReference = ref.String() } return nil }, diff --git a/pkg/build/api/validation/validation.go b/pkg/build/api/validation/validation.go index 3503f7a5c0df..bcff97404ed6 100644 --- a/pkg/build/api/validation/validation.go +++ b/pkg/build/api/validation/validation.go @@ -126,7 +126,7 @@ func validateOutput(output *buildapi.BuildOutput) errs.ValidationErrorList { } if len(output.DockerImageReference) != 0 { - if _, _, _, _, err := imageapi.SplitDockerPullSpec(output.DockerImageReference); err != nil { + if _, err := imageapi.ParseDockerImageReference(output.DockerImageReference); err != nil { allErrs = append(allErrs, errs.NewFieldInvalid("dockerImageReference", output.DockerImageReference, err.Error())) } } diff --git a/pkg/build/builder/cmd/builder.go b/pkg/build/builder/cmd/builder.go index 58d2c2fd9010..63ac6dc3bdb4 100644 --- a/pkg/build/builder/cmd/builder.go +++ b/pkg/build/builder/cmd/builder.go @@ -49,11 +49,11 @@ func run(builderFactory factoryFunc) { output = false } if output { - registry, _, _, _, err := image.SplitDockerPullSpec(build.Parameters.Output.DockerImageReference) + ref, err := image.ParseDockerImageReference(build.Parameters.Output.DockerImageReference) if err != nil { glog.Fatalf("Build output does not have a valid Docker image reference: %v", err) } - authcfg, authPresent = dockercfg.NewHelper().GetDockerAuth(registry) + authcfg, authPresent = dockercfg.NewHelper().GetDockerAuth(ref.Registry) } b := builderFactory(client, endpoint, authcfg, authPresent, &build) if err = b.Build(); err != nil { diff --git a/pkg/build/controller/image_change_controller.go b/pkg/build/controller/image_change_controller.go index ee43d74d8c41..77e920fa7cca 100644 --- a/pkg/build/controller/image_change_controller.go +++ b/pkg/build/controller/image_change_controller.go @@ -62,16 +62,17 @@ func (c *ImageChangeController) HandleImageRepo(imageRepo *imageapi.ImageReposit if len(tag) == 0 { tag = buildapi.DefaultImageTag } - imageID, hasTag := imageRepo.Tags[tag] - if !hasTag { + latest, err := imageapi.LatestTaggedImage(*imageRepo, tag) + if err != nil { + glog.V(2).Info(err) continue } // (must be different) to trigger a build - if icTrigger.LastTriggeredImageID != imageID { - imageSubstitutions[icTrigger.Image] = imageRepo.Status.DockerImageRepository + ":" + imageID + if icTrigger.LastTriggeredImageID != latest.Image { + imageSubstitutions[icTrigger.Image] = latest.DockerImageReference shouldTriggerBuild = true - icTrigger.LastTriggeredImageID = imageID + icTrigger.LastTriggeredImageID = latest.Image } } diff --git a/pkg/build/controller/image_change_controller_test.go b/pkg/build/controller/image_change_controller_test.go index f4eafdef032d..de7d213931bd 100644 --- a/pkg/build/controller/image_change_controller_test.go +++ b/pkg/build/controller/image_change_controller_test.go @@ -73,12 +73,25 @@ func appendTrigger(buildcfg *buildapi.BuildConfig, triggerImage, repoName, repoT } func mockImageRepo(repoName, dockerImageRepo string, tags map[string]string) *imageapi.ImageRepository { + tagHistory := make(map[string]imageapi.TagEventList) + for tag, imageID := range tags { + tagHistory[tag] = imageapi.TagEventList{ + Items: []imageapi.TagEvent{ + { + Image: imageID, + DockerImageReference: fmt.Sprintf("%s:%s", dockerImageRepo, imageID), + }, + }, + } + } + return &imageapi.ImageRepository{ ObjectMeta: kapi.ObjectMeta{ Name: repoName, }, Status: imageapi.ImageRepositoryStatus{ DockerImageRepository: dockerImageRepo, + Tags: tagHistory, }, Tags: tags, } @@ -105,7 +118,7 @@ func TestNewImageID(t *testing.T) { t.Errorf("Unexpected error %v from HandleImageRepo", err) } if buildCreator.build == nil { - t.Error("Expected new build when new image was created!") + t.Fatal("Expected new build when new image was created!") } if buildCreator.build.Parameters.Strategy.DockerStrategy.Image != "registry.com/namespace/imagename:newImageID123" { t.Errorf("Image substitutions not properly setup for new build. Expected %s, got %s |", "registry.com/namespace/imagename:newImageID123", buildCreator.build.Parameters.Strategy.DockerStrategy.Image) @@ -131,7 +144,7 @@ func TestNewImageIDDefaultTag(t *testing.T) { t.Errorf("Unexpected error %v from HandleImageRepo", err) } if buildCreator.build == nil { - t.Error("Expected new build when new image was created!") + t.Fatal("Expected new build when new image was created!") } if buildCreator.build.Parameters.Strategy.DockerStrategy.Image != "registry.com/namespace/imagename:newImageID123" { t.Errorf("Image substitutions not properly setup for new build. Expected %s, got %s |", "registry.com/namespace/imagename:newImageID123", buildCreator.build.Parameters.Strategy.DockerStrategy.Image) @@ -241,7 +254,7 @@ func TestMultipleTriggers(t *testing.T) { t.Errorf("Unexpected error %v from HandleImageRepo", err) } if buildCreator.build == nil { - t.Error("Expected new build when new image was created!") + t.Fatal("Expected new build when new image was created!") } if buildCreator.build.Parameters.Strategy.DockerStrategy.Image != "registry.com/namespace/imagename1" { t.Errorf("Image substitutions not properly setup for new build. Expected %s, got %s |", "registry.com/namespace/imagename1", buildCreator.build.Parameters.Strategy.DockerStrategy.Image) @@ -315,7 +328,7 @@ func TestBuildCreateError(t *testing.T) { t.Error("Expected retryable error from HandleImageRepo") } if buildCreator.build == nil { - t.Error("Expected new build when new image was created!") + t.Fatal("Expected new build when new image was created!") } if buildCreator.build.Parameters.Strategy.DockerStrategy.Image != "registry.com/namespace/imagename:newImageID123" { t.Errorf("Image substitutions not properly setup for new build. Expected %s, got %s |", "registry.com/namespace/imagename:newImageID123", buildCreator.build.Parameters.Strategy.DockerStrategy.Image) @@ -339,7 +352,7 @@ func TestBuildUpdateError(t *testing.T) { t.Error("Expected fatal error from HandleImageRepo") } if buildCreator.build == nil { - t.Error("Expected new build when new image was created!") + t.Fatal("Expected new build when new image was created!") } if buildCreator.build.Parameters.Strategy.DockerStrategy.Image != "registry.com/namespace/imagename:newImageID123" { t.Errorf("Image substitutions not properly setup for new build. Expected %s, got %s |", "registry.com/namespace/imagename:newImageID123", buildCreator.build.Parameters.Strategy.DockerStrategy.Image) diff --git a/pkg/build/controller/strategy/util.go b/pkg/build/controller/strategy/util.go index a09455f38ec6..a0bf8e5a0dcb 100644 --- a/pkg/build/controller/strategy/util.go +++ b/pkg/build/controller/strategy/util.go @@ -77,13 +77,13 @@ func setupBuildEnv(build *buildapi.Build, pod *kapi.Pod) error { // Do nothing for unknown source types } - registry, namespace, name, tag, err := imageapi.SplitDockerPullSpec(build.Parameters.Output.DockerImageReference) + ref, err := imageapi.ParseDockerImageReference(build.Parameters.Output.DockerImageReference) if err != nil { return err } - outputImage := imageapi.JoinDockerPullSpec("", namespace, name, tag) - vars = append(vars, kapi.EnvVar{"OUTPUT_IMAGE", outputImage}) - vars = append(vars, kapi.EnvVar{"OUTPUT_REGISTRY", registry}) + vars = append(vars, kapi.EnvVar{"OUTPUT_REGISTRY", ref.Registry}) + ref.Registry = "" + vars = append(vars, kapi.EnvVar{"OUTPUT_IMAGE", ref.String()}) if len(pod.Spec.Containers) > 0 { pod.Spec.Containers[0].Env = append(pod.Spec.Containers[0].Env, vars...) diff --git a/pkg/build/util/generate.go b/pkg/build/util/generate.go index 2952fea96fb6..5474877f2e69 100644 --- a/pkg/build/util/generate.go +++ b/pkg/build/util/generate.go @@ -102,12 +102,13 @@ func GenerateBuildWithImageTag(config *buildapi.BuildConfig, revision *buildapi. if len(tag) == 0 { tag = buildapi.DefaultImageTag } - imageID, hasTag := imageRepo.Tags[tag] - if !hasTag { + latest, err := imageapi.LatestTaggedImage(*imageRepo, tag) + if err != nil { continue } - glog.V(4).Infof("Adding substitution %s with %s:%s", icTrigger.Image, imageRepo.Status.DockerImageRepository, imageID) - imageSubstitutions[icTrigger.Image] = imageRepo.Status.DockerImageRepository + ":" + imageID + imageRef := latest.DockerImageReference + glog.V(4).Infof("Adding substitution %s with %s", icTrigger.Image, imageRef) + imageSubstitutions[icTrigger.Image] = imageRef } glog.V(4).Infof("Generating build from config for build config %s", config.Name) build := GenerateBuildFromConfig(config, revision, imageSubstitutions) diff --git a/pkg/build/util/generate_test.go b/pkg/build/util/generate_test.go index 28b43131a14d..80c612ea58a3 100644 --- a/pkg/build/util/generate_test.go +++ b/pkg/build/util/generate_test.go @@ -1,6 +1,7 @@ package util import ( + "fmt" "reflect" "testing" @@ -552,6 +553,16 @@ func (m *mockImageRepositoryNamespaceGetter) GetByNamespace(namespace, name stri ObjectMeta: kapi.ObjectMeta{Name: imageRepoName}, Status: imageapi.ImageRepositoryStatus{ DockerImageRepository: originalImage, + Tags: map[string]imageapi.TagEventList{ + tagName: { + Items: []imageapi.TagEvent{ + { + DockerImageReference: fmt.Sprintf("%s:%s", originalImage, newTag), + Image: newTag, + }, + }, + }, + }, }, Tags: map[string]string{tagName: newTag}, }, nil diff --git a/pkg/cmd/experimental/generate/generate.go b/pkg/cmd/experimental/generate/generate.go index 4246fe6b28ef..b1800b4f530f 100644 --- a/pkg/cmd/experimental/generate/generate.go +++ b/pkg/cmd/experimental/generate/generate.go @@ -152,9 +152,9 @@ func newImageResolver(namespace string, osClient osclient.Interface, dockerClien } namespaces = append(namespaces, "default") imageStreamResolver := &genapp.ImageStreamResolver{ - Client: osClient, - Images: osClient, - Namespaces: namespaces, + Client: osClient, + ImageStreamImages: osClient, + Namespaces: namespaces, } resolver = append(resolver, genapp.WeightedResolver{imageStreamResolver, 0.0}) } diff --git a/pkg/deploy/controller/imagechange/controller.go b/pkg/deploy/controller/imagechange/controller.go index 8c8e8f0f3010..f9e7d1450071 100644 --- a/pkg/deploy/controller/imagechange/controller.go +++ b/pkg/deploy/controller/imagechange/controller.go @@ -58,14 +58,25 @@ func (c *ImageChangeController) Handle(imageRepo *imageapi.ImageRepository) erro continue } - // The container image's tag name is by convention the same as the image ID it references - _, _, _, containerImageID, err := imageapi.SplitDockerPullSpec(container.Image) + ref, err := imageapi.ParseDockerImageReference(container.Image) if err != nil { glog.V(4).Infof("Skipping container %s for config %s; container's image is invalid: %v", container.Name, labelFor(config), err) continue } - if repoImageID, repoHasTag := imageRepo.Tags[params.Tag]; repoHasTag && repoImageID != containerImageID { + latest, err := imageapi.LatestTaggedImage(*imageRepo, params.Tag) + if err != nil { + glog.V(4).Infof("Skipping container %s for config %s; %s", container.Name, labelFor(config), err) + continue + } + + containerImageID := ref.ID + if len(containerImageID) == 0 { + // For v1 images, the container image's tag name is by convention the same as the image ID it references + containerImageID = ref.Tag + } + if latest.Image != containerImageID { + glog.V(4).Infof("Container %s for config %s: image id changed from %q to %q; regenerating config", container.Name, labelFor(config), containerImageID, latest.Image) configsToGenerate = append(configsToGenerate, config) firedTriggersForConfig[config.Name] = append(firedTriggersForConfig[config.Name], params) } @@ -135,11 +146,11 @@ func (c *ImageChangeController) regenerate(imageRepo *imageapi.ImageRepository, continue } - id, ok := imageRepo.Tags[trigger.Tag] - if !ok { - // TODO: not really sure what to do here + latest, err := imageapi.LatestTaggedImage(*imageRepo, trigger.Tag) + if err != nil { + return fmt.Errorf("error generating new version of deploymentConfig: %s: %s", labelFor(config), err) } - repoName = fmt.Sprintf("%s:%s", imageRepo.Status.DockerImageRepository, id) + repoName = latest.DockerImageReference } causes = append(causes, diff --git a/pkg/deploy/controller/imagechange/controller_test.go b/pkg/deploy/controller/imagechange/controller_test.go index 2f71012ae684..dbf6e10613fe 100644 --- a/pkg/deploy/controller/imagechange/controller_test.go +++ b/pkg/deploy/controller/imagechange/controller_test.go @@ -1,6 +1,7 @@ package imagechange import ( + "fmt" "testing" kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" @@ -132,25 +133,50 @@ func TestHande_matchScenarios(t *testing.T) { }, } + tagHistoryFor := func(repo, tag, value string) map[string]imageapi.TagEventList { + return map[string]imageapi.TagEventList{ + tag: { + Items: []imageapi.TagEvent{ + { + DockerImageReference: fmt.Sprintf("%s:%s", repo, value), + Image: value, + }, + }, + }, + } + } + updates := map[string]*imageapi.ImageRepository{ "repo.1": { ObjectMeta: kapi.ObjectMeta{Name: "repoA", Namespace: kapi.NamespaceDefault}, - Status: imageapi.ImageRepositoryStatus{DockerImageRepository: "registry:8080/openshift/test-image"}, - Tags: map[string]string{"test-tag": "ref-2"}, + Status: imageapi.ImageRepositoryStatus{ + DockerImageRepository: "registry:8080/openshift/test-image", + Tags: tagHistoryFor("registry:8080/openshift/test-image", "test-tag", "ref-2"), + }, + Tags: map[string]string{"test-tag": "ref-2"}, }, "repo.2": { ObjectMeta: kapi.ObjectMeta{Name: "repoB", Namespace: kapi.NamespaceDefault}, - Status: imageapi.ImageRepositoryStatus{DockerImageRepository: "registry:8080/openshift/test-image"}, - Tags: map[string]string{"test-tag": "ref-3"}, + Status: imageapi.ImageRepositoryStatus{ + DockerImageRepository: "registry:8080/openshift/test-image", + Tags: tagHistoryFor("registry:8080/openshift/test-image", "test-tag", "ref-3"), + }, + Tags: map[string]string{"test-tag": "ref-3"}, }, "repo.3": { ObjectMeta: kapi.ObjectMeta{Name: "repoC", Namespace: kapi.NamespaceDefault}, - Status: imageapi.ImageRepositoryStatus{DockerImageRepository: "registry:8080/openshift/test-image-B"}, - Tags: map[string]string{"test-tag": "ref-2"}, + Status: imageapi.ImageRepositoryStatus{ + DockerImageRepository: "registry:8080/openshift/test-image-B", + Tags: tagHistoryFor("registry:8080/openshift/test-image-B", "test-tag", "ref-2"), + }, + Tags: map[string]string{"test-tag": "ref-2"}, }, "repo.4": { ObjectMeta: kapi.ObjectMeta{Name: "repoA", Namespace: kapi.NamespaceDefault}, - Tags: map[string]string{"test-tag": "ref-2"}, + Status: imageapi.ImageRepositoryStatus{ + Tags: tagHistoryFor("default/repoA", "test-tag", "ref-2"), + }, + Tags: map[string]string{"test-tag": "ref-2"}, }, } diff --git a/pkg/deploy/generator/config_generator.go b/pkg/deploy/generator/config_generator.go index 8c8f4719cf11..30f8a853a046 100644 --- a/pkg/deploy/generator/config_generator.go +++ b/pkg/deploy/generator/config_generator.go @@ -213,6 +213,10 @@ func referencesByIndex(refs triggersByRef, legacy triggersByName) reposByIndex { func replaceReferences(dc *deployapi.DeploymentConfig, repos reposByIndex) (changed bool, errs errors.ValidationErrorList) { template := dc.Template.ControllerTemplate.Template for i, repo := range repos { + if len(repo.Status.DockerImageRepository) == 0 { + errs = append(errs, errors.NewFieldInvalid(fmt.Sprintf("triggers[%d].imageChange.from", i), repo.Name, fmt.Sprintf("image repository %s/%s does not have a Docker image repository reference set and can't be used in a deployment config trigger", repo.Namespace, repo.Name))) + continue + } params := dc.Triggers[i].ImageChangeParams // lookup image id @@ -221,17 +225,14 @@ func replaceReferences(dc *deployapi.DeploymentConfig, repos reposByIndex) (chan // TODO: replace with "preferred tag" from repo tag = "latest" } - id, ok := repo.Tags[tag] - if !ok { - errs = append(errs, errors.NewFieldInvalid(fmt.Sprintf("triggers[%d].imageChange.from", i), repo.Name, fmt.Sprintf("image repository %s/%s does not have tag %q", repo.Namespace, repo.Name, tag))) - continue - } - if len(repo.Status.DockerImageRepository) == 0 { - errs = append(errs, errors.NewFieldInvalid(fmt.Sprintf("triggers[%d].imageChange.from", i), repo.Name, fmt.Sprintf("image repository %s/%s does not have a Docker image repository reference set and can't be used in a deployment config trigger", repo.Namespace, repo.Name))) + + // get the image ref from the repo's tag history + latest, err := imageapi.LatestTaggedImage(*repo, tag) + if err != nil { + errs = append(errs, errors.NewFieldInvalid(fmt.Sprintf("triggers[%d].imageChange.from", i), repo.Name, err.Error())) continue } - // TODO: this assumes that tag value is the image id - image := fmt.Sprintf("%s:%s", repo.Status.DockerImageRepository, id) + image := latest.DockerImageReference // update containers names := util.NewStringSet(params.ContainerNames...) diff --git a/pkg/deploy/generator/config_generator_test.go b/pkg/deploy/generator/config_generator_test.go index bac1148f98fe..df59ea9c7d7f 100644 --- a/pkg/deploy/generator/config_generator_test.go +++ b/pkg/deploy/generator/config_generator_test.go @@ -131,6 +131,8 @@ func TestGenerateFromConfigWithUpdatedImageRef(t *testing.T) { LIRFn: func(ctx kapi.Context) (*imageapi.ImageRepositoryList, error) { list := okImageRepoList() list.Items[0].Tags["tag1"] = "ref2" + list.Items[0].Status.Tags["tag1"].Items[0].Image = "ref2" + list.Items[0].Status.Tags["tag1"].Items[0].DockerImageReference = "registry:8080/repo1@ref2" return list, nil }, }, @@ -150,7 +152,7 @@ func TestGenerateFromConfigWithUpdatedImageRef(t *testing.T) { t.Fatalf("Expected config LatestVersion=2, got %d", config.LatestVersion) } - expected := "registry:8080/repo1:ref2" + expected := "registry:8080/repo1@ref2" actual := config.Template.ControllerTemplate.Template.Spec.Containers[0].Image if expected != actual { t.Fatalf("Expected container image %s, got %s", expected, actual) @@ -246,7 +248,7 @@ func TestGenerateDeploymentConfigWithFrom(t *testing.T) { t.Fatalf("Expected config LatestVersion=2, got %d", config.LatestVersion) } - expected := "internal/namespace/imageRepo1:ref1" + expected := "internal/namespace/imageRepo1@ref1" actual := config.Template.ControllerTemplate.Template.Spec.Containers[0].Image if expected != actual { t.Fatalf("Expected container image %s, got %s", expected, actual) @@ -264,6 +266,16 @@ func okImageRepoList() *imageapi.ImageRepositoryList { }, Status: imageapi.ImageRepositoryStatus{ DockerImageRepository: "registry:8080/repo1", + Tags: map[string]imageapi.TagEventList{ + "tag1": { + Items: []imageapi.TagEvent{ + { + DockerImageReference: "registry:8080/repo1:ref1", + Image: "ref1", + }, + }, + }, + }, }, }, }, @@ -367,6 +379,16 @@ func internalImageRepo() *imageapi.ImageRepositoryList { }, Status: imageapi.ImageRepositoryStatus{ DockerImageRepository: "internal/namespace/imageRepo1", + Tags: map[string]imageapi.TagEventList{ + "tag1": { + Items: []imageapi.TagEvent{ + { + DockerImageReference: "internal/namespace/imageRepo1@ref1", + Image: "ref1", + }, + }, + }, + }, }, }, }, diff --git a/pkg/generate/app/app.go b/pkg/generate/app/app.go index c3e06596b5dd..a6e96ac18b81 100644 --- a/pkg/generate/app/app.go +++ b/pkg/generate/app/app.go @@ -155,30 +155,21 @@ func (s *BuildStrategyRef) BuildStrategy() (*buildapi.BuildStrategy, []buildapi. return &buildapi.BuildStrategy{ Type: buildapi.STIBuildStrategyType, STIStrategy: &buildapi.STIBuildStrategy{ - Image: s.Base.NameReference(), + Image: s.Base.String(), }, }, nil } // ImageRef is a reference to an image type ImageRef struct { - Namespace string - Name string - Tag string - Registry string - + imageapi.DockerImageReference AsImageRepository bool Repository *imageapi.ImageRepository Info *imageapi.DockerImage } -// pullSpec returns the string that can be passed to Docker to fetch this -// image. -func (r *ImageRef) pullSpec() string { - return imageapi.JoinDockerPullSpec(r.Registry, r.Namespace, r.Name, r.Tag) -} - +/* // NameReference returns the name that other OpenShift objects may refer to this // image as. Deployment Configs and Build Configs may look an image up // in an image repository before creating other objects that use the name. @@ -191,6 +182,7 @@ func (r *ImageRef) NameReference() string { } return r.pullSpec() } +*/ func (r *ImageRef) RepoName() string { name := r.Namespace @@ -235,7 +227,7 @@ func (r *ImageRef) ImageRepository() (*imageapi.ImageRepository, error) { name, ok := r.SuggestName() if !ok { - return nil, fmt.Errorf("unable to suggest an image repository name for %q", r.pullSpec()) + return nil, fmt.Errorf("unable to suggest an image repository name for %q", r.String()) } repo := &imageapi.ImageRepository{ @@ -244,7 +236,7 @@ func (r *ImageRef) ImageRepository() (*imageapi.ImageRepository, error) { }, } if !r.AsImageRepository { - repo.DockerImageRepository = r.pullSpec() + repo.DockerImageRepository = r.String() } return repo, nil @@ -253,7 +245,7 @@ func (r *ImageRef) ImageRepository() (*imageapi.ImageRepository, error) { func (r *ImageRef) DeployableContainer() (container *kapi.Container, triggers []deployapi.DeploymentTriggerPolicy, err error) { name, ok := r.SuggestName() if !ok { - return nil, nil, fmt.Errorf("unable to suggest a container name for the image %q", r.pullSpec()) + return nil, nil, fmt.Errorf("unable to suggest a container name for the image %q", r.String()) } if r.AsImageRepository { tag := r.Tag @@ -277,7 +269,7 @@ func (r *ImageRef) DeployableContainer() (container *kapi.Container, triggers [] container = &kapi.Container{ Name: name, - Image: r.NameReference(), + Image: r.String(), } // If imageInfo present, append ports diff --git a/pkg/generate/app/app_test.go b/pkg/generate/app/app_test.go index 72546a227b04..94fc107464d9 100644 --- a/pkg/generate/app/app_test.go +++ b/pkg/generate/app/app_test.go @@ -67,7 +67,13 @@ func TestSimpleBuildConfig(t *testing.T) { t.Errorf("unexpected name: %#v", config) } - output := &ImageRef{Registry: "myregistry", Namespace: "openshift", Name: "origin"} + output := &ImageRef{ + DockerImageReference: imageapi.DockerImageReference{ + Registry: "myregistry", + Namespace: "openshift", + Name: "origin", + }, + } build = &BuildRef{Source: source, Output: output} config, err = build.BuildConfig() if err != nil { @@ -79,13 +85,21 @@ func TestSimpleBuildConfig(t *testing.T) { } func TestSimpleDeploymentConfig(t *testing.T) { - image := &ImageRef{Registry: "myregistry", Namespace: "openshift", Name: "origin", Info: testImageInfo(), AsImageRepository: true} + image := &ImageRef{ + DockerImageReference: imageapi.DockerImageReference{ + Registry: "myregistry", + Namespace: "openshift", + Name: "origin", + }, + Info: testImageInfo(), + AsImageRepository: true, + } deploy := &DeploymentConfigRef{Images: []*ImageRef{image}} config, err := deploy.DeploymentConfig() if err != nil { t.Fatalf("unexpected error: %v", err) } - if config.Name != "origin" || len(config.Triggers) != 2 || config.Template.ControllerTemplate.Template.Spec.Containers[0].Image != image.pullSpec() { + if config.Name != "origin" || len(config.Triggers) != 2 || config.Template.ControllerTemplate.Template.Spec.Containers[0].Image != image.String() { t.Errorf("unexpected value: %#v", config) } } @@ -160,9 +174,11 @@ func TestImageRefDeployableContainerPorts(t *testing.T) { } for _, test := range tests { imageRef := &ImageRef{ - Namespace: "test", - Name: "image", - Tag: "latest", + DockerImageReference: imageapi.DockerImageReference{ + Namespace: "test", + Name: "image", + Tag: "latest", + }, Info: &imageapi.DockerImage{ Config: imageapi.DockerConfig{ ExposedPorts: test.inputPorts, @@ -242,7 +258,12 @@ func ExampleGenerateSimpleDockerApp() { // QUESTION: Is it ok for generation to require a namespace? Do we want to be able to create apps with builds, image repos, and // deployment configs in templates (hint: yes). // SOLUTION? Make deployment config accept unqualified image repo names (foo) and then prior to creating the RC resolve those. - output := &ImageRef{Name: name, AsImageRepository: true} + output := &ImageRef{ + DockerImageReference: imageapi.DockerImageReference{ + Name: name, + }, + AsImageRepository: true, + } // create our build based on source and input // TODO: we might need to pick a base image if this is STI build := &BuildRef{Source: source, Output: output} diff --git a/pkg/generate/app/cmd/newapp.go b/pkg/generate/app/cmd/newapp.go index 592e6a62c6fb..bc45d5271716 100644 --- a/pkg/generate/app/cmd/newapp.go +++ b/pkg/generate/app/cmd/newapp.go @@ -70,9 +70,9 @@ func (c *AppConfig) SetDockerClient(dockerclient *docker.Client) { func (c *AppConfig) SetOpenShiftClient(osclient client.Interface, originNamespace string) { c.imageStreamResolver = app.ImageStreamResolver{ - Client: osclient, - Images: osclient, - Namespaces: []string{originNamespace, "default"}, + Client: osclient, + ImageStreamImages: osclient, + Namespaces: []string{originNamespace, "default"}, } } diff --git a/pkg/generate/app/componentref.go b/pkg/generate/app/componentref.go index 8abfeea610ee..f5b02607ec5c 100644 --- a/pkg/generate/app/componentref.go +++ b/pkg/generate/app/componentref.go @@ -41,8 +41,8 @@ func componentWithSource(s string) (component, repo string, builder bool, err er component = s } // TODO: component must be of the form compatible with a pull spec *or* / - if !imageapi.IsPullSpec(component) { - return "", "", false, fmt.Errorf("%q is not a valid Docker pull specification", component) + if _, err := imageapi.ParseDockerImageReference(component); err != nil { + return "", "", false, fmt.Errorf("%q is not a valid Docker pull specification: %s", component, err) } return } diff --git a/pkg/generate/app/imagelookup.go b/pkg/generate/app/imagelookup.go index 35b84cddb709..f6674a019a99 100644 --- a/pkg/generate/app/imagelookup.go +++ b/pkg/generate/app/imagelookup.go @@ -26,19 +26,19 @@ type DockerClientResolver struct { } func (r DockerClientResolver) Resolve(value string) (*ComponentMatch, error) { - registry, namespace, name, tag, err := imageapi.SplitDockerPullSpec(value) + ref, err := imageapi.ParseDockerImageReference(value) if err != nil { return nil, err } - glog.V(4).Infof("checking local Docker daemon %s/%s/%s with tag %q", registry, namespace, name, tag) + glog.V(4).Infof("checking local Docker daemon for %q", ref.String()) images, err := r.Client.ListImages(docker.ListImagesOptions{}) if err != nil { return nil, err } matches := ScoredComponentMatches{} for _, image := range images { - if tags := matchTag(image, value, registry, namespace, name, tag); len(tags) > 0 { + if tags := matchTag(image, value, ref.Registry, ref.Namespace, ref.Name, ref.Tag); len(tags) > 0 { matches = append(matches, tags...) } } @@ -73,7 +73,7 @@ func (r DockerClientResolver) Resolve(value string) (*ComponentMatch, error) { continue } updated.Score = match.Score - updated.ImageTag = tag + updated.ImageTag = ref.Tag matches[i] = updated } @@ -123,27 +123,27 @@ type DockerRegistryResolver struct { } func (r DockerRegistryResolver) Resolve(value string) (*ComponentMatch, error) { - registry, namespace, name, tag, err := imageapi.SplitDockerPullSpec(value) + ref, err := imageapi.ParseDockerImageReference(value) if err != nil { return nil, err } - glog.V(4).Infof("checking Docker registry %s/%s/%s with tag %q", registry, namespace, name, tag) - connection, err := r.Client.Connect(registry) + glog.V(4).Infof("checking Docker registry for %q", ref.String()) + connection, err := r.Client.Connect(ref.Registry) if err != nil { if dockerregistry.IsRegistryNotFound(err) { return nil, ErrNoMatch{value: value} } - return nil, ErrNoMatch{value: value, qualifier: fmt.Sprintf("can't connect to %q: %v", registry, err)} + return nil, ErrNoMatch{value: value, qualifier: fmt.Sprintf("can't connect to %q: %v", ref.Registry, err)} } - image, err := connection.ImageByTag(namespace, name, tag) + image, err := connection.ImageByTag(ref.Namespace, ref.Name, ref.Tag) if err != nil { if dockerregistry.IsNotFound(err) { return nil, ErrNoMatch{value: value, qualifier: err.Error()} } - return nil, ErrNoMatch{value: value, qualifier: fmt.Sprintf("can't connect to %q: %v", registry, err)} + return nil, ErrNoMatch{value: value, qualifier: fmt.Sprintf("can't connect to %q: %v", ref.Registry, err)} } - if len(tag) == 0 { - tag = "latest" + if len(ref.Tag) == 0 { + ref.Tag = "latest" } glog.V(4).Infof("found image: %#v", image) dockerImage := &imageapi.DockerImage{} @@ -151,9 +151,9 @@ func (r DockerRegistryResolver) Resolve(value string) (*ComponentMatch, error) { return nil, err } - from := registry - if len(registry) == 0 { - registry = "Docker Hub" + from := ref.Registry + if len(ref.Registry) == 0 { + ref.Registry = "Docker Hub" } return &ComponentMatch{ Value: value, @@ -163,7 +163,7 @@ func (r DockerRegistryResolver) Resolve(value string) (*ComponentMatch, error) { Builder: IsBuilderImage(dockerImage), Score: 0, Image: dockerImage, - ImageTag: tag, + ImageTag: ref.Tag, }, nil } @@ -215,24 +215,24 @@ func matchTag(image docker.APIImages, value, registry, namespace, name, tag stri }) continue } - iRegistry, iNamespace, iName, iTag, err := imageapi.SplitDockerPullSpec(s) + iRef, err := imageapi.ParseDockerImageReference(s) if err != nil { continue } - if len(iTag) == 0 { - iTag = "latest" + if len(iRef.Tag) == 0 { + iRef.Tag = "latest" } match := &ComponentMatch{} - ok, score := partialScorer(name, iName, true, 0.5, 1.0) + ok, score := partialScorer(name, iRef.Name, true, 0.5, 1.0) if !ok { continue } match.Score += score - _, score = partialScorer(namespace, iNamespace, false, 0.5, 1.0) + _, score = partialScorer(namespace, iRef.Namespace, false, 0.5, 1.0) match.Score += score - _, score = partialScorer(registry, iRegistry, false, 0.5, 1.0) + _, score = partialScorer(registry, iRef.Registry, false, 0.5, 1.0) match.Score += score - _, score = partialScorer(tag, iTag, false, 0.5, 1.0) + _, score = partialScorer(tag, iRef.Tag, false, 0.5, 1.0) match.Score += score if match.Score >= 4.0 { @@ -247,55 +247,52 @@ func matchTag(image docker.APIImages, value, registry, namespace, name, tag stri } type ImageStreamResolver struct { - Client client.ImageRepositoriesNamespacer - Images client.ImagesInterfacer - Namespaces []string + Client client.ImageRepositoriesNamespacer + ImageStreamImages client.ImageStreamImagesNamespacer + Namespaces []string } func (r ImageStreamResolver) Resolve(value string) (*ComponentMatch, error) { - registry, namespace, name, tag, err := imageapi.SplitOpenShiftPullSpec(value) - if err != nil || len(registry) != 0 { - return nil, fmt.Errorf("image repositories must be of the form [/][:]") + ref, err := imageapi.ParseDockerImageReference(value) + if err != nil || len(ref.Registry) != 0 { + return nil, fmt.Errorf("image repositories must be of the form [/][:|@]") } namespaces := r.Namespaces - if len(namespace) != 0 { - namespaces = []string{namespace} + if len(ref.Namespace) != 0 { + namespaces = []string{ref.Namespace} } for _, namespace := range namespaces { - glog.V(4).Infof("checking image stream %s/%s with tag %q", namespace, name, tag) - repo, err := r.Client.ImageRepositories(namespace).Get(name) + glog.V(4).Infof("checking image stream %s/%s with ref %q", namespace, ref.Name, ref.Tag) + repo, err := r.Client.ImageRepositories(namespace).Get(ref.Name) if err != nil { if errors.IsNotFound(err) { continue } return nil, err } - searchTag := tag + searchTag := ref.Tag // TODO: move to a lookup function on repo, or better yet, have the repo.Status.Tags field automatically infer latest if len(searchTag) == 0 { searchTag = "latest" } - id, ok := repo.Tags[searchTag] - if !ok { - if len(tag) == 0 { - return nil, ErrNoMatch{value: value, qualifier: fmt.Sprintf("the default tag %q has not been set", searchTag)} - } - return nil, ErrNoMatch{value: value, qualifier: fmt.Sprintf("tag %q has not been set", tag)} + latest, err := imageapi.LatestTaggedImage(*repo, searchTag) + if err != nil { + return nil, ErrNoMatch{value: value, qualifier: err.Error()} } - imageData, err := r.Images.Images().Get(id) + imageData, err := r.ImageStreamImages.ImageStreamImages(namespace).Get(ref.Name, latest.Image) if err != nil { if errors.IsNotFound(err) { - return nil, ErrNoMatch{value: value, qualifier: fmt.Sprintf("tag %q is set, but image %q has been removed", tag, id)} + return nil, ErrNoMatch{value: value, qualifier: fmt.Sprintf("tag %q is set, but image %q has been removed", ref.Tag, latest.Image)} } return nil, err } - spec := imageapi.JoinDockerPullSpec("", namespace, name, tag) + ref.Registry = "" return &ComponentMatch{ - Value: spec, - Argument: fmt.Sprintf("--image=%q", spec), - Name: name, - Description: fmt.Sprintf("Image stream %s (tag %q) in namespace %s, tracks %q", name, searchTag, namespace, repo.Status.DockerImageRepository), + Value: ref.String(), + Argument: fmt.Sprintf("--image=%q", ref.String()), + Name: ref.Name, + Description: fmt.Sprintf("Image stream %s (tag %q) in namespace %s, tracks %q", ref.Name, searchTag, ref.Namespace, repo.Status.DockerImageRepository), Builder: IsBuilderImage(&imageData.DockerImageMetadata), Score: 0, diff --git a/pkg/generate/app/initializers.go b/pkg/generate/app/initializers.go index 25b44c9f836d..8cd1b34f347a 100644 --- a/pkg/generate/app/initializers.go +++ b/pkg/generate/app/initializers.go @@ -7,24 +7,21 @@ import ( ) func ImageFromName(name string, tag string) (*ImageRef, error) { - registry, namespace, name, repoTag, err := image.SplitDockerPullSpec(name) + ref, err := image.ParseDockerImageReference(name) if err != nil { return nil, err } if len(tag) == 0 { - if len(repoTag) != 0 { - tag = repoTag + if len(ref.Tag) != 0 { + tag = ref.Tag } else { tag = "latest" } } return &ImageRef{ - Registry: registry, - Namespace: namespace, - Name: name, - Tag: tag, + DockerImageReference: ref, }, nil } @@ -34,25 +31,12 @@ func ImageFromRepository(repo *image.ImageRepository, tag string) (*ImageRef, er // need to know the default OpenShift registry return nil, fmt.Errorf("the repository does not resolve to a pullable Docker repository") } - registry, namespace, name, repoTag, err := image.SplitDockerPullSpec(pullSpec) + + ref, err := ImageFromName(pullSpec, tag) if err != nil { return nil, err } - if len(tag) == 0 { - if len(repoTag) != 0 { - tag = repoTag - } else { - tag = "latest" - } - } - - return &ImageRef{ - Registry: registry, - Namespace: namespace, - Name: name, - Tag: tag, - - Repository: repo, - }, nil + ref.Repository = repo + return ref, nil } diff --git a/pkg/generate/app/pipeline.go b/pkg/generate/app/pipeline.go index dd3eef794531..fc1f623a80fa 100644 --- a/pkg/generate/app/pipeline.go +++ b/pkg/generate/app/pipeline.go @@ -13,6 +13,7 @@ import ( kutil "github.com/GoogleCloudPlatform/kubernetes/pkg/util" deploy "github.com/openshift/origin/pkg/deploy/api" + imageapi "github.com/openshift/origin/pkg/image/api" ) type Pipeline struct { @@ -44,8 +45,10 @@ func NewBuildPipeline(from string, input *ImageRef, strategy *BuildStrategyRef, } output := &ImageRef{ - Name: name, - Tag: "latest", + DockerImageReference: imageapi.DockerImageReference{ + Name: name, + Tag: "latest", + }, AsImageRepository: true, } diff --git a/pkg/generate/generator/imageref.go b/pkg/generate/generator/imageref.go index 2bd326e4a332..337bda56b552 100644 --- a/pkg/generate/generator/imageref.go +++ b/pkg/generate/generator/imageref.go @@ -37,16 +37,13 @@ func NewImageRefGenerator() ImageRefGenerator { // FromName generates an ImageRef from a given name func (g *imageRefGenerator) FromName(name string) (*app.ImageRef, error) { - registry, namespace, name, tag, err := imageapi.SplitDockerPullSpec(name) + ref, err := imageapi.ParseDockerImageReference(name) if err != nil { return nil, err } return &app.ImageRef{ - Registry: registry, - Namespace: namespace, - Name: name, - Tag: tag, - AsImageRepository: true, + DockerImageReference: ref, + AsImageRepository: true, }, nil } @@ -121,25 +118,17 @@ func (g *imageRefGenerator) FromRepository(repo *imageapi.ImageRepository, tag s // need to know the default OpenShift registry return nil, fmt.Errorf("the repository does not resolve to a pullable Docker repository") } - registry, namespace, name, repoTag, err := imageapi.SplitDockerPullSpec(pullSpec) + ref, err := imageapi.ParseDockerImageReference(pullSpec) if err != nil { return nil, err } - if len(tag) == 0 { - if len(repoTag) != 0 { - tag = repoTag - } else { - tag = "latest" - } + if len(tag) == 0 && len(ref.Tag) == 0 { + ref.Tag = "latest" } return &app.ImageRef{ - Registry: registry, - Namespace: namespace, - Name: name, - Tag: tag, - - Repository: repo, + DockerImageReference: ref, + Repository: repo, }, nil } diff --git a/pkg/image/api/helper.go b/pkg/image/api/helper.go index 95f10af73bf2..ec7356a0d8fe 100644 --- a/pkg/image/api/helper.go +++ b/pkg/image/api/helper.go @@ -4,70 +4,104 @@ import ( "encoding/json" "fmt" "strings" - - "github.com/fsouza/go-dockerclient" ) // DockerDefaultNamespace is the value for namespace when a single segment name is provided. const DockerDefaultNamespace = "library" -// SplitDockerPullSpec breaks a Docker pull specification into its components, or returns -// an error if those components are not valid. Attempts to match as closely as possible the -// Docker spec up to 1.3. Future API revisions may change the pull syntax. -func SplitDockerPullSpec(spec string) (registry, namespace, name, tag string, err error) { - registry, namespace, name, tag, err = SplitOpenShiftPullSpec(spec) - if err != nil { - return +// DockerImageReference points to a Docker image. +type DockerImageReference struct { + Registry string + Namespace string + Name string + Tag string + ID string +} + +// COPIED from upstream +// TODO remove +func parseRepositoryTag(repos string) (string, string) { + n := strings.Index(repos, "@") + if n >= 0 { + parts := strings.Split(repos, "@") + return parts[0], parts[1] } - return + n = strings.LastIndex(repos, ":") + if n < 0 { + return repos, "" + } + if tag := repos[n+1:]; !strings.Contains(tag, "/") { + return repos[:n], tag + } + return repos, "" } -// SplitOpenShiftPullSpec breaks an OpenShift pull specification into its components, or returns -// an error if those components are not valid. Attempts to match as closely as possible the -// Docker spec up to 1.3. Future API revisions may change the pull syntax. -func SplitOpenShiftPullSpec(spec string) (registry, namespace, name, tag string, err error) { - spec, tag = docker.ParseRepositoryTag(spec) - arr := strings.Split(spec, "/") - switch len(arr) { +// ParseDockerImageReference parses a Docker pull spec string into a +// DockerImageReference. +func ParseDockerImageReference(spec string) (DockerImageReference, error) { + var ( + ref DockerImageReference + tag, id string + ) + // TODO replace with docker version once docker/docker PR11109 is merged upstream + repo, tagOrID := parseRepositoryTag(spec) + if strings.Contains(tagOrID, ":") { + id = tagOrID + } else { + tag = tagOrID + } + + repoParts := strings.Split(repo, "/") + switch len(repoParts) { case 2: - return "", arr[0], arr[1], tag, nil + // namespace/name + ref.Namespace = repoParts[0] + ref.Name = repoParts[1] + ref.Tag = tag + ref.ID = id + return ref, nil case 3: - return arr[0], arr[1], arr[2], tag, nil + // registry/namespace/name + ref.Registry = repoParts[0] + ref.Namespace = repoParts[1] + ref.Name = repoParts[2] + ref.Tag = tag + ref.ID = id + return ref, nil case 1: - if len(arr[0]) == 0 { - err = fmt.Errorf("the docker pull spec %q must be two or three segments separated by slashes", spec) - return + // name + if len(repoParts[0]) == 0 { + return ref, fmt.Errorf("the docker pull spec %q must be two or three segments separated by slashes", spec) } - return "", "", arr[0], tag, nil + ref.Name = repoParts[0] + ref.Tag = tag + ref.ID = id + return ref, nil default: - err = fmt.Errorf("the docker pull spec %q must be two or three segments separated by slashes", spec) - return + return ref, fmt.Errorf("the docker pull spec %q must be two or three segments separated by slashes", spec) } } -// IsPullSpec returns true if the provided string appears to be a valid Docker pull spec. -func IsPullSpec(spec string) bool { - _, _, _, _, err := SplitDockerPullSpec(spec) - return err == nil -} - -// JoinDockerPullSpec turns a set of components of a Docker pull specification into a single -// string. Attempts to match as closely as possible the Docker spec up to 1.3. Future API -// revisions may change the pull syntax. -func JoinDockerPullSpec(registry, namespace, name, tag string) string { - if len(tag) != 0 { - tag = ":" + tag +// String converts a DockerImageReference to a Docker pull spec. +func (r DockerImageReference) String() string { + registry := r.Registry + if len(registry) > 0 { + registry += "/" } - if len(namespace) == 0 { - if len(registry) == 0 { - return fmt.Sprintf("%s%s", name, tag) - } - namespace = DockerDefaultNamespace + + if len(r.Namespace) == 0 { + r.Namespace = DockerDefaultNamespace } - if len(registry) == 0 { - return fmt.Sprintf("%s/%s%s", namespace, name, tag) + r.Namespace += "/" + + var ref string + if len(r.Tag) > 0 { + ref = ":" + r.Tag + } else if len(r.ID) > 0 { + ref = "@" + r.ID } - return fmt.Sprintf("%s/%s/%s%s", registry, namespace, name, tag) + + return fmt.Sprintf("%s%s%s%s", registry, r.Namespace, r.Name, ref) } // ImageWithMetadata returns a copy of image with the DockerImageMetadata filled in @@ -110,3 +144,22 @@ func ImageWithMetadata(image Image) (*Image, error) { return &image, nil } + +// LatestTaggedImage returns the most recent TagEvent for the specified image +// repository and tag. +func LatestTaggedImage(repo ImageRepository, tag string) (*TagEvent, error) { + if _, ok := repo.Tags[tag]; !ok { + return nil, fmt.Errorf("image repository %s/%s: tag %q not found", repo.Namespace, repo.Name, tag) + } + + tagHistory, ok := repo.Status.Tags[tag] + if !ok { + return nil, fmt.Errorf("image repository %s/%s: tag %q not found in tag history", repo.Namespace, repo.Name, tag) + } + + if len(tagHistory.Items) == 0 { + return nil, fmt.Errorf("image repository %s/%s: tag %q has 0 history items", repo.Namespace, repo.Name, tag) + } + + return &tagHistory.Items[0], nil +} diff --git a/pkg/image/api/helper_test.go b/pkg/image/api/helper_test.go index 4f3b7d7e0fc9..6ccc16321617 100644 --- a/pkg/image/api/helper_test.go +++ b/pkg/image/api/helper_test.go @@ -10,21 +10,43 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) -func TestSplitDockerPullSpec(t *testing.T) { +func TestParseDockerImageReference(t *testing.T) { testCases := []struct { - From string - Registry, Namespace, Name, Tag string - Err bool + From string + Registry, Namespace, Name, Tag, ID string + Err bool }{ { From: "foo", Name: "foo", }, + { + From: "foo:tag", + Name: "foo", + Tag: "tag", + }, + { + From: "foo@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25", + Name: "foo", + ID: "sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25", + }, { From: "bar/foo", Namespace: "bar", Name: "foo", }, + { + From: "bar/foo:tag", + Namespace: "bar", + Name: "foo", + Tag: "tag", + }, + { + From: "bar/foo@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25", + Namespace: "bar", + Name: "foo", + ID: "sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25", + }, { From: "bar/foo/baz", Registry: "bar", @@ -38,6 +60,19 @@ func TestSplitDockerPullSpec(t *testing.T) { Name: "baz", Tag: "tag", }, + { + From: "bar/foo/baz@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25", + Registry: "bar", + Namespace: "foo", + Name: "baz", + ID: "sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25", + }, + { + From: "bar:5000/foo/baz", + Registry: "bar:5000", + Namespace: "foo", + Name: "baz", + }, { From: "bar:5000/foo/baz:tag", Registry: "bar:5000", @@ -45,6 +80,13 @@ func TestSplitDockerPullSpec(t *testing.T) { Name: "baz", Tag: "tag", }, + { + From: "bar:5000/foo/baz@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25", + Registry: "bar:5000", + Namespace: "foo", + Name: "baz", + ID: "sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25", + }, { From: "bar/foo/baz/biz", Err: true, @@ -56,7 +98,7 @@ func TestSplitDockerPullSpec(t *testing.T) { } for _, testCase := range testCases { - r, ns, n, tag, err := SplitDockerPullSpec(testCase.From) + ref, err := ParseDockerImageReference(testCase.From) switch { case err != nil && !testCase.Err: t.Errorf("%s: unexpected error: %v", testCase.From, err) @@ -65,8 +107,113 @@ func TestSplitDockerPullSpec(t *testing.T) { t.Errorf("%s: unexpected non-error", testCase.From) continue } - if r != testCase.Registry || ns != testCase.Namespace || n != testCase.Name || tag != testCase.Tag { - t.Errorf("%s: unexpected result: %q %q %q %q", testCase.From, r, ns, n, tag) + if e, a := testCase.Registry, ref.Registry; e != a { + t.Errorf("%s: registry: expected %q, got %q", testCase.From, e, a) + } + if e, a := testCase.Namespace, ref.Namespace; e != a { + t.Errorf("%s: namespace: expected %q, got %q", testCase.From, e, a) + } + if e, a := testCase.Name, ref.Name; e != a { + t.Errorf("%s: name: expected %q, got %q", testCase.From, e, a) + } + if e, a := testCase.Tag, ref.Tag; e != a { + t.Errorf("%s: tag: expected %q, got %q", testCase.From, e, a) + } + if e, a := testCase.ID, ref.ID; e != a { + t.Errorf("%s: id: expected %q, got %q", testCase.From, e, a) + } + } +} + +func TestDockerImageReferenceString(t *testing.T) { + testCases := []struct { + Registry, Namespace, Name, Tag, ID string + Expected string + }{ + { + Name: "foo", + Expected: "library/foo", + }, + { + Name: "foo", + Tag: "tag", + Expected: "library/foo:tag", + }, + { + Name: "foo", + ID: "sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25", + Expected: "library/foo@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25", + }, + { + Namespace: "bar", + Name: "foo", + Expected: "bar/foo", + }, + { + Namespace: "bar", + Name: "foo", + Tag: "tag", + Expected: "bar/foo:tag", + }, + { + Namespace: "bar", + Name: "foo", + ID: "sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25", + Expected: "bar/foo@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25", + }, + { + Registry: "bar", + Namespace: "foo", + Name: "baz", + Expected: "bar/foo/baz", + }, + { + Registry: "bar", + Namespace: "foo", + Name: "baz", + Tag: "tag", + Expected: "bar/foo/baz:tag", + }, + { + Registry: "bar", + Namespace: "foo", + Name: "baz", + ID: "sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25", + Expected: "bar/foo/baz@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25", + }, + { + Registry: "bar:5000", + Namespace: "foo", + Name: "baz", + Expected: "bar:5000/foo/baz", + }, + { + Registry: "bar:5000", + Namespace: "foo", + Name: "baz", + Tag: "tag", + Expected: "bar:5000/foo/baz:tag", + }, + { + Registry: "bar:5000", + Namespace: "foo", + Name: "baz", + ID: "sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25", + Expected: "bar:5000/foo/baz@sha256:3c87c572822935df60f0f5d3665bd376841a7fcfeb806b5f212de6a00e9a7b25", + }, + } + + for i, testCase := range testCases { + ref := DockerImageReference{ + Registry: testCase.Registry, + Namespace: testCase.Namespace, + Name: testCase.Name, + Tag: testCase.Tag, + ID: testCase.ID, + } + actual := ref.String() + if e, a := testCase.Expected, actual; e != a { + t.Errorf("%d: expected %q, got %q", i, e, a) } } } diff --git a/pkg/image/api/validation/validation.go b/pkg/image/api/validation/validation.go index 3bf4d04e5aa7..8a5102c7e65c 100644 --- a/pkg/image/api/validation/validation.go +++ b/pkg/image/api/validation/validation.go @@ -18,8 +18,7 @@ func ValidateImage(image *api.Image) errors.ValidationErrorList { if len(image.DockerImageReference) == 0 { result = append(result, errors.NewFieldRequired("dockerImageReference", image.DockerImageReference)) } else { - _, _, _, _, err := api.SplitDockerPullSpec(image.DockerImageReference) - if err != nil { + if _, err := api.ParseDockerImageReference(image.DockerImageReference); err != nil { result = append(result, errors.NewFieldInvalid("dockerImageReference", image.DockerImageReference, err.Error())) } } @@ -41,8 +40,7 @@ func ValidateImageRepository(repo *api.ImageRepository) errors.ValidationErrorLi result = append(result, errors.NewFieldInvalid("namespace", repo.Namespace, "")) } if len(repo.DockerImageRepository) != 0 { - _, _, _, _, err := api.SplitDockerPullSpec(repo.DockerImageRepository) - if err != nil { + if _, err := api.ParseDockerImageReference(repo.DockerImageRepository); err != nil { result = append(result, errors.NewFieldInvalid("dockerImageRepository", repo.DockerImageRepository, err.Error())) } } @@ -67,8 +65,7 @@ func ValidateImageRepositoryMapping(mapping *api.ImageRepositoryMapping) errors. hasName := len(mapping.Name) != 0 switch { case hasRepository: - _, _, _, _, err := api.SplitDockerPullSpec(mapping.DockerImageRepository) - if err != nil { + if _, err := api.ParseDockerImageReference(mapping.DockerImageRepository); err != nil { result = append(result, errors.NewFieldInvalid("dockerImageRepository", mapping.DockerImageRepository, err.Error())) } case hasName: diff --git a/pkg/image/registry/imagerepository/strategy.go b/pkg/image/registry/imagerepository/strategy.go index 467889946335..692c6f478a8f 100644 --- a/pkg/image/registry/imagerepository/strategy.go +++ b/pkg/image/registry/imagerepository/strategy.go @@ -72,7 +72,12 @@ func (s imageRepositoryStrategy) dockerImageRepository(repo *api.ImageRepository if len(repo.Namespace) == 0 { repo.Namespace = kapi.NamespaceDefault } - return api.JoinDockerPullSpec(registry, repo.Namespace, repo.Name, "") + ref := api.DockerImageReference{ + Registry: registry, + Namespace: repo.Namespace, + Name: repo.Name, + } + return ref.String() } // updateTagHistory updates repo.Status.Tags to add any new or updated tags