fix circular input/output detection

need to actually compare the imagestreams that are being
input/output, not the image represented by the input to
the output.

fixes https://bugzilla.redhat.com/show_bug.cgi?id=1306507

pkg/generate/app/cmd/newapp.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"fmt"
 	"io"
+	"reflect"
 	"strings"
 	"time"
 
@@ -19,6 +20,7 @@ import (
 
 	authapi "github.com/openshift/origin/pkg/authorization/api"
 	buildapi "github.com/openshift/origin/pkg/build/api"
+	buildutil "github.com/openshift/origin/pkg/build/util"
 	"github.com/openshift/origin/pkg/client"
 	cmdutil "github.com/openshift/origin/pkg/cmd/util"
 	"github.com/openshift/origin/pkg/dockerregistry"
@@ -678,19 +680,6 @@ func (c *AppConfig) buildPipelines(components app.ComponentReferences, environme
 			if c.NoOutput {
 				pipeline.Build.Output = nil
 			}
-			if err := pipeline.Validate(); err != nil {
-				switch err.(type) {
-				case app.CircularOutputReferenceError:
-					if len(c.To) == 0 {
-						// Output reference was generated, return error.
-						return nil, err
-					}
-					// Output reference was explicitly provided, print warning.
-					fmt.Fprintf(c.ErrOut, "--> WARNING: %v\n", err)
-				default:
-					return nil, err
-				}
-			}
 			common = append(common, pipeline)
 			if err := common.Reduce(); err != nil {
 				return nil, fmt.Errorf("can't create a pipeline from %s: %v", common, err)
@@ -1063,9 +1052,6 @@ func (c *AppConfig) run(acceptors app.Acceptors) (*AppResult, error) {
 		if err == app.ErrNameRequired {
 			return nil, fmt.Errorf("can't suggest a valid name, please specify a name with --name")
 		}
-		if err, ok := err.(app.CircularOutputReferenceError); ok {
-			return nil, fmt.Errorf("%v, please specify a different output reference with --to", err)
-		}
 		return nil, err
 	}
 
@@ -1105,6 +1091,19 @@ func (c *AppConfig) run(acceptors app.Acceptors) (*AppResult, error) {
 		}
 	}
 
+	err = c.checkCircularReferences(objects)
+	if err != nil {
+		if err, ok := err.(app.CircularOutputReferenceError); ok {
+			if len(c.To) == 0 {
+				// Output reference was generated, return error.
+				return nil, fmt.Errorf("%v, set a different tag with --to", err)
+			}
+			// Output reference was explicitly provided, print warning.
+			fmt.Fprintf(c.ErrOut, "--> WARNING: %v\n", err)
+		}
+		return nil, err
+	}
+
 	return &AppResult{
 		List:      &kapi.List{Items: objects},
 		Name:      name,
@@ -1113,6 +1112,25 @@ func (c *AppConfig) run(acceptors app.Acceptors) (*AppResult, error) {
 	}, nil
 }
 
+// checkCircularReferences ensures there are no builds that can trigger themselves
+// due to an imagechangetrigger that matches the output destination of the image.
+func (c *AppConfig) checkCircularReferences(objects app.Objects) error {
+	for _, obj := range objects {
+		if bc, ok := obj.(*buildapi.BuildConfig); ok {
+			input := buildutil.GetImageStreamForStrategy(bc.Spec.Strategy)
+			if bc.Spec.Output.To != nil && input != nil &&
+				reflect.DeepEqual(input, bc.Spec.Output.To) {
+				ns := input.Namespace
+				if len(ns) == 0 {
+					ns = c.OriginNamespace
+				}
+				return app.CircularOutputReferenceError{Reference: fmt.Sprintf("%s/%s", ns, input.Name)}
+			}
+		}
+	}
+	return nil
+}
+
 func (c *AppConfig) Querying() bool {
 	return c.AsList || c.AsSearch
 }

pkg/generate/app/errors.go

@@ -5,8 +5,6 @@ import (
 	"fmt"
 
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
-
-	imageapi "github.com/openshift/origin/pkg/image/api"
 )
 
 // ErrNoMatch is the error returned by new-app when no match is found for a
@@ -74,9 +72,9 @@ var ErrNameRequired = fmt.Errorf("you must specify a name for your app")
 // CircularOutputReferenceError is the error returned by new-app when the input
 // and output image stream tags are identical.
 type CircularOutputReferenceError struct {
-	Reference imageapi.DockerImageReference
+	Reference string
 }
 
 func (e CircularOutputReferenceError) Error() string {
-	return fmt.Sprintf("the input and output image stream tags are identical (%q)", e.Reference.DockerClientDefaults())
+	return fmt.Sprintf("output image of %q must be different than input", e.Reference)
 }

pkg/generate/app/pipeline.go

@@ -196,24 +196,6 @@ func (p *Pipeline) NeedsDeployment(env Environment, labels map[string]string, as
 	return nil
 }
 
-// Validate checks for logical errors in the pipeline.
-func (p *Pipeline) Validate() error {
-	if p == nil || p.Build == nil {
-		return nil
-	}
-	input, output := p.Build.Input, p.Build.Output
-	if input == nil || output == nil {
-		return nil
-	}
-	if input.AsImageStream && output.AsImageStream && input.Reference.Equal(output.Reference) {
-		// If the build input and output image stream tags are the same, given that
-		// build configs created by new-app/new-build have an image change trigger,
-		// this setup would cause an infinite build loop, most likely unintentionaly.
-		return CircularOutputReferenceError{Reference: output.Reference}
-	}
-	return nil
-}
-
 // Objects converts all the components in the pipeline into runtime objects.
 func (p *Pipeline) Objects(accept, objectAccept Acceptor) (Objects, error) {
 	objects := Objects{}

test/integration/newapp_test.go

@@ -941,6 +941,28 @@ func TestNewAppRunBuilds(t *testing.T) {
 				return nil
 			},
 		},
+		{
+			name: "successful build from dockerfile with identical input and output image references with warning",
+			config: &cmd.AppConfig{
+				Dockerfile: "FROM openshift/ruby-22-centos7\nRUN yum install -y httpd",
+				To:         "ruby-22-centos7",
+			},
+			expected: map[string][]string{
+				"buildConfig": {"ruby-22-centos7"},
+				"imageStream": {"ruby-22-centos7"},
+			},
+			checkOutput: func(stdout, stderr io.Reader) error {
+				got, err := ioutil.ReadAll(stderr)
+				if err != nil {
+					return err
+				}
+				want := "--> WARNING: the input and output image stream tags are identical (\"docker.io/library/centos:latest\")\n"
+				if string(got) != want {
+					return fmt.Errorf("stderr: got %q; want %q", got, want)
+				}
+				return nil
+			},
+		},
 		{
 			name: "successful generation of BC with multiple sources: repo + Dockerfile",
 			config: &cmd.AppConfig{
@@ -1003,11 +1025,21 @@ func TestNewAppRunBuilds(t *testing.T) {
 			},
 			expectedErr: func(err error) bool {
 				e := app.CircularOutputReferenceError{
-					Reference: imageapi.DockerImageReference{
-						Name: "centos",
-					}.DockerClientDefaults(),
+					Reference: "centos",
+				}
+				return err.Error() == fmt.Errorf("%v, set a different tag with --to", e).Error()
+			},
+		},
+		{
+			name: "unsuccessful build from dockerfile due to identical input and output image references",
+			config: &cmd.AppConfig{
+				Dockerfile: "FROM openshift/ruby-22-centos7\nRUN yum install -y httpd",
+			},
+			expectedErr: func(err error) bool {
+				e := app.CircularOutputReferenceError{
+					Reference: "openshift/ruby-22-centos7",
 				}
-				return err.Error() == fmt.Errorf("%v, please specify a different output reference with --to", e).Error()
+				return err.Error() == fmt.Errorf("%v, set a different tag with --to", e).Error()
 			},
 		},
 		{