Wrong cycle error (#623)

* modified cycle test to also check no cycles; added test case without cycle

* fix: use hasAncestor() for request sequences, too

* removed not needed `seen` member and `hasSeen()` function

* changelog entry

Co-authored-by: Marcel Ludwig <marcel.ludwig@avenga.com>

CHANGELOG.md

@@ -6,7 +6,8 @@ Unreleased changes are available as `avenga/couper:edge` container.
 
 * **Fixed**
   * CVE-2021-3538 related to our `request_id_format` option if switched to `uuid4`: replaced the underlying package to `github.com/google/uuid` ([#611](https://github.com/avenga/couper/pull/611))
-  * Possible panic for nested endpoint sequences ([#618](https://github.com/avenga/couper/pull/618))
+  * Possible panic for nested [endpoint sequences](https://docs.couper.io/configuration/block/endpoint#endpoint-sequence) ([#618](https://github.com/avenga/couper/pull/618))
+  * Cycle check for [endpoint sequences](https://docs.couper.io/configuration/block/endpoint#endpoint-sequence) ([#623](https://github.com/avenga/couper/pull/623))
 
 ---
 

config/sequence/sequence.go

@@ -22,7 +22,6 @@ type Item struct {
 	backend   bool
 	deps      List
 	parent    *Item
-	seen      map[string]struct{}
 }
 
 func (s *Item) Add(ref *Item) *Item {
@@ -32,7 +31,7 @@ func (s *Item) Add(ref *Item) *Item {
 
 	ref.parent = s
 
-	if s.backend && s.hasAncestor(ref.Name) || !s.backend && s.hasSeen(ref.Name) { // collect names to populate error message
+	if s.hasAncestor(ref.Name) { // collect names to populate error message
 		refs := []string{ref.Name}
 		p := s.parent
 		for p != s {
@@ -96,28 +95,6 @@ func (s *Item) hasAncestor(name string) bool {
 	return s.parent.hasAncestor(name)
 }
 
-func (s *Item) hasSeen(name string) bool {
-	if s == nil {
-		return false
-	}
-
-	if s.seen == nil {
-		s.seen = make(map[string]struct{})
-	}
-
-	if _, exist := s.seen[name]; exist {
-		return true
-	}
-
-	s.seen[name] = struct{}{}
-
-	if s.HasParent() && s.parent.hasSeen(name) {
-		return true
-	}
-
-	return false
-}
-
 func resolveSequence(item *Item, resolved, seen *[]string) {
 	name := item.Name
 	*seen = append(*seen, name)

server/http_endpoints_test.go

@@ -823,6 +823,7 @@ func TestEndpointCyclicSequence(t *testing.T) {
 	for _, testcase := range []struct{ file, exp string }{
 		{file: "15_couper.hcl", exp: "circular sequence reference: a,b,a"},
 		{file: "16_couper.hcl", exp: "circular sequence reference: a,aa,aaa,a"},
+		{file: "20_couper.hcl", exp: ""},
 	} {
 		t.Run(testcase.file, func(st *testing.T) {
 			// since we will switch the working dir, reset afterwards
@@ -832,12 +833,15 @@ func TestEndpointCyclicSequence(t *testing.T) {
 			_, err := configload.LoadFile(path, "")
 
 			diags, ok := err.(*hcl.Diagnostic)
-			if !ok {
-				st.Errorf("Expected an cyclic hcl diagnostics error, got: %v", reflect.TypeOf(err))
+			if !ok && testcase.exp != "" {
+				st.Errorf("Expected a cyclic hcl diagnostics error, got: %v", reflect.TypeOf(err))
+				st.Fatal(err, path)
+			} else if ok && testcase.exp == "" {
+				st.Errorf("Expected no cyclic hcl diagnostics error, got: %v", reflect.TypeOf(err))
 				st.Fatal(err, path)
 			}
 
-			if diags.Detail != testcase.exp {
+			if testcase.exp != "" && diags.Detail != testcase.exp {
 				st.Errorf("\nWant:\t%s\nGot:\t%s", testcase.exp, diags.Detail)
 			}
 		})

server/testdata/endpoints/20_couper.hcl

@@ -0,0 +1,24 @@
+server {
+  api {
+    endpoint "/" {
+      request "r1" {
+        url = "http://localhost/r1"
+      }
+
+      request "r2" {
+        url = "http://localhost/r2"
+        form_body = {
+          r1 = backend_responses.r1.status
+        }
+      }
+
+      request {
+        url = "http://localhost/def"
+        json_body = {
+          r1 = backend_responses.r1.status
+          r2 = backend_responses.r2.status
+        }
+      }
+    }
+  }
+}