new-app: appropriately warn/error on circular builds

Bug 1393549
Bugzilla link https://bugzilla.redhat.com/show_bug.cgi?id=1393549
Detailed explanation
Citing of cirular builds were performed for new-build but not new-app;
With new-app, if template, simply warn, if not template fail command

Author: gabemontero

pkg/generate/app/cmd/newapp.go

@@ -684,6 +684,46 @@ func (c *AppConfig) Run() (*AppResult, error) {
 	if err != nil {
 		return nil, err
 	}
+
+	// check for circular reference specifically from the template objects and print warnings if they exist
+	err = c.checkCircularReferences(templateObjects)
+	if err != nil {
+		if err, ok := err.(app.CircularOutputReferenceError); ok {
+			// templates only apply to `oc new-app`
+			addOn := ""
+			if len(c.Name) == 0 {
+				addOn = ", override artifact names with --name"
+			}
+			fmt.Fprintf(c.ErrOut, "--> WARNING: %v\n%s", err, addOn)
+		} else {
+			return nil, err
+		}
+	}
+	// check for circular reference specifically from the newly generated objects, handling new-app vs. new-build nuances as needed
+	err = c.checkCircularReferences(objects)
+	if err != nil {
+		if err, ok := err.(app.CircularOutputReferenceError); ok {
+			if c.ExpectToBuild {
+				// circular reference handling for `oc new-build`.
+				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)
+			} else {
+				// circular reference handling for `oc new-app`
+				if len(c.Name) == 0 {
+					return nil, fmt.Errorf("%v, override artifact names with --name", err)
+				}
+				// Output reference was explicitly provided, print warning.
+				fmt.Fprintf(c.ErrOut, "--> WARNING: %v\n", err)
+			}
+		} else {
+			return nil, err
+		}
+	}
+
 	objects = append(objects, templateObjects...)
 
 	name = c.Name
@@ -707,23 +747,6 @@ func (c *AppConfig) Run() (*AppResult, error) {
 		}
 	}
 
-	// Only check circular references for `oc new-build`.
-	if c.ExpectToBuild {
-		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)
-			} else {
-				return nil, err
-			}
-		}
-	}
-
 	return &AppResult{
 		List:      &kapi.List{Items: objects},
 		Name:      name,

test/cmd/newapp.sh

@@ -264,6 +264,10 @@ os::cmd::expect_success_and_text 'oc new-build --name mybuild --binary --strateg
 os::cmd::expect_failure_and_text 'oc new-build mysql --source-image centos' 'error: --source-image-path must be specified when --source-image is specified.'
 os::cmd::expect_failure_and_text 'oc new-build mysql --source-image-path foo' 'error: --source-image must be specified when --source-image-path is specified.'
 
+# ensure circular ref flagged but allowed for template
+os::cmd::expect_success 'oc create -f test/testdata/circular-is.yaml'
+os::cmd::expect_success_and_text 'oc new-app -f test/testdata/circular.yaml' 'should be different than input'
+
 # do not allow use of non-existent image (should fail)
 os::cmd::expect_failure_and_text 'oc new-app  openshift/bogusimage https://github.com/openshift/ruby-hello-world.git -o yaml' "no match for"
 # allow use of non-existent image (should succeed)

test/testdata/circular-is.yaml

@@ -0,0 +1,16 @@
+apiVersion: v1
+items:
+- apiVersion: v1
+  kind: ImageStream
+  metadata:
+    creationTimestamp: 2016-12-02T18:55:30Z
+    generation: 1
+    name: newapp-circular-template
+    resourceVersion: "764"
+    selfLink: /oapi/v1/namespaces/myproject/imagestreams/newapp-circular-template
+    uid: e5ad1d91-b8c0-11e6-9803-68f7287398a7
+  spec: {}
+  status:
+    dockerImageRepository: 172.30.101.11:5000/myproject/newapp-circular-template
+kind: List
+metadata: {}

test/testdata/circular.yaml

@@ -0,0 +1,41 @@
+apiVersion: v1
+kind: Template
+metadata:
+  creationTimestamp: null
+  name: circular
+objects:
+- apiVersion: v1
+  kind: BuildConfig
+  metadata:
+    creationTimestamp: null
+    name: newapp-circular-template
+  spec:
+    nodeSelector: null
+    output:
+      to:
+        kind: ImageStreamTag
+        name: newapp-circular-template:latest
+    postCommit: {}
+    resources: {}
+    runPolicy: Serial
+    source:
+      git:
+        uri: https://github.com/openshift/ruby-hello-world
+      type: Git
+    strategy:
+      dockerStrategy:
+        from:
+          kind: ImageStreamTag
+          name: newapp-circular-template:latest
+      type: Docker
+    triggers:
+    - github:
+        secret: faSaQS1VY-gdRMwge4eV
+      type: GitHub
+    - generic:
+        secret: u-_J-vtKR5K3GykKwCuK
+      type: Generic
+    - imageChange: {}
+      type: ImageChange
+  status:
+    lastVersion: 0