Duplicate endpoint

CHANGELOG.md

@@ -33,6 +33,7 @@ Unreleased changes are available as `avenga/couper:edge` container.
   * Request methods are treated case-insensitively when comparing them to methods in the `allowed_methods` attribute of [`api`](./docs/REFERENCE.md#api-block) or [`endpoint`](./docs/REFERENCE.md#endpoint-block) blocks ([#478](https://github.com/avenga/couper/pull/478))
   * Do not allow multiple `backend` blocks in `proxy` and `request` blocks ([#483](https://github.com/avenga/couper/pull/483))
   * Panic if an [`error_handler` block](./docs/REFERENCE.md#error-handler-block) following another `error_handler` block has no label ([#486](https://github.com/avenga/couper/pull/486))
+  * Spurious `duplicate endpoint /**` error for APIs sharing the same base path ([#507](https://github.com/avenga/couper/pull/507))
   * Invalid (by [OpenAPI validation](./docs/REFERENCE.md#openapi-block)) backend response missing in [`backend_responses`](./docs/REFERENCE.md#backend_responses) ([#501](https://github.com/avenga/couper/pull/501))
   * Ignore the `expected_status` check for a request configured via a [`proxy`](./docs/REFERENCE.md#proxy-block) or [`request`](./docs/REFERENCE.md#request-block) block if a [`backend` error](./docs/ERRORS.md#endpoint-error-types) occured ([#505](https://github.com/avenga/couper/pull/505))
 

config/runtime/access_control_test.go

@@ -1,11 +1,17 @@
 package runtime_test
 
 import (
+	"context"
+	"fmt"
+	"sort"
+	"strings"
 	"testing"
 
+	"github.com/avenga/couper/cache"
 	"github.com/avenga/couper/config/configload"
 	"github.com/avenga/couper/config/runtime"
 	"github.com/avenga/couper/errors"
+	"github.com/avenga/couper/internal/test"
 )
 
 func TestACDefinitions_errors(t *testing.T) {
@@ -118,3 +124,235 @@ func TestACDefinitions_errors(t *testing.T) {
 		})
 	}
 }
+
+func TestDuplicateEndpoint(t *testing.T) {
+	tests := []struct {
+		name      string
+		hcl       string
+		endpoints []string
+	}{
+		{
+			"shared API base path: create catch-all",
+			`api {
+			   access_control = ["a"]
+			 }
+			 api {
+			   access_control = ["a"]
+			 }`,
+			[]string{"/**"},
+		},
+		{
+			"shared API base path: create catch-all",
+			`base_path = "/p"
+			 api {
+			   access_control = ["a"]
+			 }
+			 api {
+			   access_control = ["a"]
+			 }`,
+			[]string{"/p/**"},
+		},
+		{
+			"shared API base path: create catch-all",
+			`api {}
+			 api {
+			   access_control = ["a"]
+			 }`,
+			[]string{"/**"},
+		},
+		{
+			"shared API base path w/o access control: no catch-all",
+			`api {
+			 }
+			 api {
+			 }`,
+			[]string{},
+		},
+		{
+			"shared API base path: create catch-all",
+			`access_control = ["a"]
+			 api {}
+			 api {}
+			 api {
+			   base_path = "/p"
+			   endpoint "/**" {
+			     response {}
+		       }
+			 }`,
+			[]string{"/**", "/p/**"},
+		},
+		{
+			"unique base paths: create catch-all twice",
+			`access_control = ["a"]
+			 api {
+			   base_path = "/"
+			 }
+			 api {
+			   base_path = "/p"
+			 }`,
+			[]string{"/**", "/p/**"},
+		},
+		{
+			"unique base paths: create catch-all twice",
+			`access_control = ["a"]
+			 api {
+			   base_path = "/p"
+			 }
+			 api {
+			   base_path = "/"
+			 }`,
+			[]string{"/**", "/p/**"},
+		},
+		{
+			"user defined /** endpoint in 1st API: no extra catch-all",
+			`api {
+			   endpoint "/**" {
+			     access_control = ["a"]
+			     response {}
+			   }
+			 }
+			 api {
+			   access_control = ["a"]
+			 }
+			`,
+			[]string{"/**"},
+		},
+		{
+			"user defined /** endpoint in 2nd API: no extra catch-all",
+			`access_control = ["a"]
+			 api {}
+			 api {
+			   endpoint "/**" {
+			     response {}
+			   }
+			 }
+			`,
+			[]string{"/**"},
+		},
+		{
+			"files + api: catch-all",
+			`files {
+			   base_path = "/public"
+			   document_root = "."
+			 }
+			 api {
+			   access_control = ["a"]
+			 }
+			`,
+			[]string{"/**"},
+		},
+		{
+			"files + api, same base path: no catch-all",
+			`files {
+			   document_root = "."
+			 }
+			 api {
+			   access_control = ["a"]
+			 }
+			`,
+			[]string{},
+		},
+		{
+			"files + api, same base path: no catch-all",
+			`files {
+			   base_path = "/p"
+			   document_root = "."
+			 }
+			 api {
+			   base_path = "/p"
+			   access_control = ["a"]
+			   endpoint "/**" {
+			     response {}
+			   }
+			 }
+			`,
+			[]string{"/p/**"},
+		},
+		{
+			"spa + api: catch-all",
+			`spa {
+			   base_path = "/public"
+			   bootstrap_file = "foo"
+			   paths = []
+			 }
+			 api {
+			   access_control = ["a"]
+			 }`,
+			[]string{"/**"},
+		},
+		{
+			"spa + api, same base path: no catch-all",
+			`spa {
+			    base_path = "/path"
+				bootstrap_file = "foo"
+				paths = []
+			 }
+			 api {
+			   base_path = "/path"
+			   access_control = ["a"]
+			 }
+			`,
+			[]string{},
+		},
+		{
+			"files + api, same base path: no catch-all",
+			`spa {
+			   base_path = "/p"
+			   bootstrap_file = "foo"
+			   paths = []
+			 }
+			 api {
+			   base_path = "/p"
+			   access_control = ["a"]
+			   endpoint "/**" {
+			     response {}
+			   }
+			 }
+			`,
+			[]string{"/p/**"},
+		},
+	}
+
+	template := `
+		server {
+		  %%
+		}
+		definitions {
+		  jwt "a" {
+		    signature_algorithm = "HS256"
+		    key = "asdf"
+		  }
+		}
+	`
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(subT *testing.T) {
+			conf, err := configload.LoadBytes([]byte(strings.Replace(template, "%%", tt.hcl, -1)), "couper.hcl")
+			if err != nil {
+				subT.Error(err)
+				return
+			}
+			log, _ := test.NewLogger()
+			logger := log.WithContext(context.TODO())
+			tmpStoreCh := make(chan struct{})
+			defer close(tmpStoreCh)
+			server, err := runtime.NewServerConfiguration(conf, logger, cache.New(logger, tmpStoreCh))
+
+			if err != nil {
+				subT.Error("expected no error, got:", err)
+				return
+			}
+
+			endpointMap := server[8080]["*"].EndpointRoutes
+			var endpoints sort.StringSlice
+			for endpoint := range endpointMap {
+				endpoints = append(endpoints, endpoint)
+			}
+			endpoints.Sort()
+			if fmt.Sprint(tt.endpoints) != fmt.Sprint(endpoints) {
+				subT.Errorf("unexpected endpoints, want: %v, got: %v", tt.endpoints, endpoints)
+				return
+			}
+		})
+	}
+}

config/runtime/endpoint.go

@@ -19,10 +19,26 @@ import (
 func newEndpointMap(srvConf *config.Server, serverOptions *server.Options) (endpointMap, error) {
 	endpoints := make(endpointMap)
 
-	apiBasePaths := make(map[string]struct{})
+	catchAllEndpoints := make(map[string]struct{})
+	for _, apiConf := range srvConf.APIs {
+		basePath := serverOptions.APIBasePaths[apiConf]
+		for _, epConf := range apiConf.Endpoints {
+			endpoints[epConf] = apiConf
+			if epConf.Pattern == "/**" {
+				catchAllEndpoints[basePath] = struct{}{}
+			}
+		}
+	}
 
 	for _, apiConf := range srvConf.APIs {
 		basePath := serverOptions.APIBasePaths[apiConf]
+		if _, exists := catchAllEndpoints[basePath]; exists {
+			continue
+		}
+
+		if len(newAC(srvConf, apiConf).List()) == 0 {
+			continue
+		}
 
 		var filesBasePath, spaBasePath string
 		if serverOptions.FilesBasePath != "" {
@@ -34,18 +50,9 @@ func newEndpointMap(srvConf *config.Server, serverOptions *server.Options) (endp
 
 		isAPIBasePathUniqueToFilesAndSPA := basePath != filesBasePath && basePath != spaBasePath
 
-		apiBasePaths[basePath] = struct{}{}
-
-		for _, epConf := range apiConf.Endpoints {
-			endpoints[epConf] = apiConf
-
-			if epConf.Pattern == "/**" {
-				isAPIBasePathUniqueToFilesAndSPA = false
-			}
-		}
-
-		if isAPIBasePathUniqueToFilesAndSPA && len(newAC(srvConf, apiConf).List()) > 0 {
+		if isAPIBasePathUniqueToFilesAndSPA {
 			endpoints[apiConf.CatchAllEndpoint] = apiConf
+			catchAllEndpoints[basePath] = struct{}{}
 		}
 	}