Merge branch 'master' into add-e2e-test-smoketest-settings

apache · Dec 27, 2020 · 2886009 · 2886009
2 parents f042251 + 678b94d
commit 2886009
Show file tree

Hide file tree

Showing 45 changed files with 376 additions and 62 deletions.
diff --git a/.github/workflows/backend-unit-test.yml b/.github/workflows/backend-unit-test.yml
@@ -51,7 +51,7 @@ jobs:
           export GO111MOUDULE=on
           export APISIX_CONF_PATH=$PWD/conf
           sed -i 's/9000/8088/' conf/conf.yaml
-          go build -o ./manager-api
+          go build -o ./manager-api ./cmd/manager
           ./manager-api > ./api.log 2>&1 &
           sleep 2
           cat ./api.log

diff --git a/.github/workflows/frontend-e2e-test.yml b/.github/workflows/frontend-e2e-test.yml
@@ -41,7 +41,7 @@ jobs:
 
       - name: Start manager-api
         working-directory: ./api
-        run: nohup go run . &
+        run: nohup go run ./cmd/manager &
 
       - name: Install front-end dependencies
         run: yarn install

diff --git a/Dockerfile b/Dockerfile
@@ -45,7 +45,7 @@ RUN wget https://github.com/api7/dag-to-lua/archive/v1.1.tar.gz -O /tmp/v1.1.tar
 RUN if [ "$ENABLE_PROXY" = "true" ] ; then go env -w GOPROXY=https://goproxy.io,direct ; fi
 
 RUN go env -w GO111MODULE=on \
-    && CGO_ENABLED=0 go build -o ../output/manager-api .
+    && CGO_ENABLED=0 go build -o ../output/manager-api ./cmd/manager
 
 FROM node:14-alpine as fe-builder
 

diff --git a/Makefile b/Makefile
@@ -64,7 +64,7 @@ api-test: api-default
 ### api-run:		Run the manager-api
 .PHONY: api-run
 api-run: api-default
-	cd api/ && go run .
+	cd api/ && go run ./cmd/manager
 
 ### api-stop:		Stop the manager-api
 api-stop:

diff --git a/api/README.md b/api/README.md
@@ -24,3 +24,26 @@ This is a backend project which the dashboard depends on, implemented by Golang.
 ## Installation
 
 [Please refer to the doc](../README.md)
+
+## Project structure
+
+```text
+├── README.md
+├── VERSION
+├── build-tools
+├── build.sh
+├── cmd
+├── conf
+├── entry.sh
+├── go.mod
+├── go.sum
+├── internal
+├── run.sh
+└── test
+```
+
+1. The `cmd` directory is the project entrance.
+2. The `internal` directory contains the main logic of manager-api.
+3. The `conf` directory contains the default configuration file.
+4. The `test` directory contains E2E test cases.
+
diff --git a/api/build.sh b/api/build.sh
@@ -31,7 +31,7 @@ if [[ ! -f "dag-to-lua-1.1/lib/dag-to-lua.lua" ]]; then
 fi
 
 # build
-cd ./api && go build -o ../output/manager-api -ldflags "-X github.com/apisix/manager-api/cmd.Version=${VERSION}" . && cd ..
+cd ./api && go build -o ../output/manager-api -ldflags "-X github.com/apisix/manager-api/cmd.Version=${VERSION}" ./cmd/manager && cd ..
 
 cp ./api/conf/schema.json ./output/conf/schema.json
 cp ./api/conf/conf.yaml ./output/conf/conf.yaml

diff --git a/api/main.go → api/cmd/manager/main.go b/api/main.go → api/cmd/manager/main.go
diff --git a/api/cmd/managerapi.go b/api/cmd/managerapi.go
@@ -25,16 +25,15 @@ import (
 	"syscall"
 	"time"
 
-	"github.com/shiningrush/droplet"
-	"github.com/spf13/cobra"
-
-	"github.com/apisix/manager-api/conf"
 	"github.com/apisix/manager-api/internal"
+	"github.com/apisix/manager-api/internal/conf"
 	"github.com/apisix/manager-api/internal/core/storage"
 	"github.com/apisix/manager-api/internal/core/store"
 	"github.com/apisix/manager-api/internal/handler"
+	"github.com/apisix/manager-api/internal/log"
 	"github.com/apisix/manager-api/internal/utils"
-	"github.com/apisix/manager-api/log"
+	"github.com/shiningrush/droplet"
+	"github.com/spf13/cobra"
 )
 
 var Version string
@@ -47,7 +46,6 @@ func printInfo() {
 	fmt.Fprintf(os.Stdout, "%-8s: %s\n\n", "Logfile", conf.ErrorLogPath)
 }
 
-
 // NewManagerAPICommand creates the manager-api command.
 func NewManagerAPICommand() *cobra.Command {
 	cmd := &cobra.Command{

diff --git a/api/conf/conf.go → api/internal/conf/conf.go b/api/conf/conf.go → api/internal/conf/conf.go
diff --git a/api/internal/core/entity/format.go b/api/internal/core/entity/format.go
@@ -20,7 +20,7 @@ import (
 	"net"
 	"strconv"
 
-	"github.com/apisix/manager-api/log"
+	"github.com/apisix/manager-api/internal/log"
 )
 
 func mapKV2Node(key string, val float64) (*Node, error) {

diff --git a/api/internal/core/storage/etcd.go b/api/internal/core/storage/etcd.go
@@ -23,9 +23,9 @@ import (
 
 	"go.etcd.io/etcd/clientv3"
 
-	"github.com/apisix/manager-api/conf"
+	"github.com/apisix/manager-api/internal/conf"
+	"github.com/apisix/manager-api/internal/log"
 	"github.com/apisix/manager-api/internal/utils"
-	"github.com/apisix/manager-api/log"
 )
 
 var (

diff --git a/api/internal/core/store/query.go b/api/internal/core/store/query.go
@@ -16,7 +16,7 @@ package store
 
 import (
 	"github.com/apisix/manager-api/internal/core/entity"
-	"github.com/apisix/manager-api/log"
+	"github.com/apisix/manager-api/internal/log"
 )
 
 type Query struct {

diff --git a/api/internal/core/store/store.go b/api/internal/core/store/store.go
@@ -30,8 +30,8 @@ import (
 
 	"github.com/apisix/manager-api/internal/core/entity"
 	"github.com/apisix/manager-api/internal/core/storage"
+	"github.com/apisix/manager-api/internal/log"
 	"github.com/apisix/manager-api/internal/utils"
-	"github.com/apisix/manager-api/log"
 )
 
 type Interface interface {
@@ -327,7 +327,6 @@ func (s *GenericStore) StringToObjPtr(str, key string) (interface{}, error) {
 	objPtr := reflect.New(s.opt.ObjType)
 	ret := objPtr.Interface()
 	err := json.Unmarshal([]byte(str), ret)
-	fmt.Println("ret:", ret, "s.opt.ObjType", s.opt.ObjType)
 	if err != nil {
 		log.Errorf("json marshal failed: %s", err)
 		return nil, fmt.Errorf("json unmarshal failed: %s", err)

diff --git a/api/internal/core/store/storehub.go b/api/internal/core/store/storehub.go
@@ -21,8 +21,8 @@ import (
 	"reflect"
 
 	"github.com/apisix/manager-api/internal/core/entity"
+	"github.com/apisix/manager-api/internal/log"
 	"github.com/apisix/manager-api/internal/utils"
-	"github.com/apisix/manager-api/log"
 )
 
 type HubKey string

diff --git a/api/internal/core/store/validate.go b/api/internal/core/store/validate.go
@@ -25,9 +25,9 @@ import (
 	"github.com/xeipuuv/gojsonschema"
 	"go.uber.org/zap/buffer"
 
-	"github.com/apisix/manager-api/conf"
+	"github.com/apisix/manager-api/internal/conf"
 	"github.com/apisix/manager-api/internal/core/entity"
-	"github.com/apisix/manager-api/log"
+	"github.com/apisix/manager-api/internal/log"
 )
 
 type Validator interface {

diff --git a/api/filter/authentication.go → api/internal/filter/authentication.go b/api/filter/authentication.go → api/internal/filter/authentication.go
@@ -22,9 +22,8 @@ import (
 
 	"github.com/dgrijalva/jwt-go"
 	"github.com/gin-gonic/gin"
-
-	"github.com/apisix/manager-api/conf"
-	"github.com/apisix/manager-api/log"
+	"github.com/apisix/manager-api/internal/conf"
+	"github.com/apisix/manager-api/internal/log"
 )
 
 func Authentication() gin.HandlerFunc {

diff --git a/api/filter/cors.go → api/internal/filter/cors.go b/api/filter/cors.go → api/internal/filter/cors.go
diff --git a/api/filter/logging.go → api/internal/filter/logging.go b/api/filter/logging.go → api/internal/filter/logging.go
diff --git a/api/filter/logging_test.go → api/internal/filter/logging_test.go b/api/filter/logging_test.go → api/internal/filter/logging_test.go
@@ -24,7 +24,7 @@ import (
 	"github.com/gin-gonic/gin"
 	"github.com/stretchr/testify/assert"
 
-	"github.com/apisix/manager-api/log"
+	"github.com/apisix/manager-api/internal/log"
 )
 
 func performRequest(r http.Handler, method, path string) *httptest.ResponseRecorder {

diff --git a/api/filter/recover.go → api/internal/filter/recover.go b/api/filter/recover.go → api/internal/filter/recover.go
@@ -24,8 +24,8 @@ import (
 	"runtime"
 	"time"
 
-	"github.com/apisix/manager-api/log"
 	"github.com/gin-gonic/gin"
+	"github.com/apisix/manager-api/internal/log"
 )
 
 var (

diff --git a/api/filter/request_id.go → api/internal/filter/request_id.go b/api/filter/request_id.go → api/internal/filter/request_id.go
diff --git a/api/filter/schema.go → api/internal/filter/schema.go b/api/filter/schema.go → api/internal/filter/schema.go
@@ -32,8 +32,8 @@ import (
 
 	"github.com/apisix/manager-api/internal/core/entity"
 	"github.com/apisix/manager-api/internal/core/store"
+	"github.com/apisix/manager-api/internal/log"
 	"github.com/apisix/manager-api/internal/utils/consts"
-	"github.com/apisix/manager-api/log"
 )
 
 var resources = map[string]string{

diff --git a/api/internal/handler/authentication/authentication.go b/api/internal/handler/authentication/authentication.go
@@ -27,7 +27,7 @@ import (
 	"github.com/shiningrush/droplet/wrapper"
 	wgin "github.com/shiningrush/droplet/wrapper/gin"
 
-	"github.com/apisix/manager-api/conf"
+	"github.com/apisix/manager-api/internal/conf"
 	"github.com/apisix/manager-api/internal/handler"
 )
 

diff --git a/api/internal/handler/handler.go b/api/internal/handler/handler.go
@@ -35,13 +35,16 @@
 package handler
 
 import (
-	"github.com/shiningrush/droplet"
-	"github.com/shiningrush/droplet/middleware"
+	"fmt"
 	"net/http"
 	"strings"
 
 	"github.com/gin-gonic/gin"
+	"github.com/shiningrush/droplet"
 	"github.com/shiningrush/droplet/data"
+	"github.com/shiningrush/droplet/middleware"
+
+	"github.com/apisix/manager-api/internal/utils"
 )
 
 type RegisterFactory func() (RouteRegister, error)
@@ -87,3 +90,17 @@ func (mw *ErrorTransformMiddleware) Handle(ctx droplet.Context) error {
 	}
 	return nil
 }
+
+func IDCompare(idOnPath string, idOnBody interface{}) error {
+	idOnBodyStr, ok := idOnBody.(string)
+	if !ok {
+		idOnBodyStr = utils.InterfaceToString(idOnBody)
+	}
+
+	// check if id on path is == to id on body ONLY if both ids are valid
+	if idOnPath != "" && idOnBodyStr != "" && idOnBodyStr != idOnPath {
+		return fmt.Errorf("ID on path (%s) doesn't match ID on body (%s)", idOnPath, idOnBodyStr)
+	}
+
+	return nil
+}
diff --git a/api/internal/handler/handler_test.go b/api/internal/handler/handler_test.go
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package handler
+
+import (
+	"errors"
+	"net/http"
+	"testing"
+
+	"github.com/go-playground/assert/v2"
+	"github.com/shiningrush/droplet/data"
+)
+
+func TestSpecCodeResponse(t *testing.T) {
+	err := errors.New("schema validate failed: remote_addr: Must validate at least one schema (anyOf)")
+	resp := SpecCodeResponse(err)
+	assert.Equal(t, &data.SpecCodeResponse{StatusCode: http.StatusBadRequest}, resp)
+
+	err = errors.New("data not found")
+	resp = SpecCodeResponse(err)
+	assert.Equal(t, &data.SpecCodeResponse{StatusCode: http.StatusNotFound}, resp)
+
+	err = errors.New("system error")
+	resp = SpecCodeResponse(err)
+	assert.Equal(t, &data.SpecCodeResponse{StatusCode: http.StatusInternalServerError}, resp)
+}
+
+func TestIDCompare(t *testing.T) {
+	// init
+	cases := []struct {
+		idOnPath, desc string
+		idOnBody       interface{}
+		wantError      error
+	}{
+		{
+			desc:     "ID on body is int, and it could be considered the same as ID on path",
+			idOnPath: "1",
+			idOnBody: 1,
+		},
+		{
+			desc:      "ID on body is int, and it is different from ID on path",
+			idOnPath:  "1",
+			idOnBody:  2,
+			wantError: errors.New("ID on path (1) doesn't match ID on body (2)"),
+		},
+		{
+			desc:     "ID on body is same as ID on path",
+			idOnPath: "1",
+			idOnBody: "1",
+		},
+		{
+			desc:      "ID on body is different from ID on path",
+			idOnPath:  "a",
+			idOnBody:  "b",
+			wantError: errors.New("ID on path (a) doesn't match ID on body (b)"),
+		},
+		{
+			desc:     "No ID on body",
+			idOnPath: "1",
+		},
+		{
+			desc:     "No ID on path",
+			idOnBody: 1,
+		},
+	}
+	for _, c := range cases {
+		t.Run(c.desc, func(t *testing.T) {
+			err := IDCompare(c.idOnPath, c.idOnBody)
+			assert.Equal(t, c.wantError, err)
+		})
+	}
+}
diff --git a/api/internal/handler/plugin/plugin.go b/api/internal/handler/plugin/plugin.go
@@ -24,7 +24,7 @@ import (
 	"github.com/shiningrush/droplet/wrapper"
 	wgin "github.com/shiningrush/droplet/wrapper/gin"
 
-	"github.com/apisix/manager-api/conf"
+	"github.com/apisix/manager-api/internal/conf"
 	"github.com/apisix/manager-api/internal/handler"
 )
 

diff --git a/api/internal/handler/route/route.go b/api/internal/handler/route/route.go
@@ -29,16 +29,15 @@ import (
 	"github.com/shiningrush/droplet"
 	"github.com/shiningrush/droplet/data"
 	"github.com/shiningrush/droplet/wrapper"
-	wgin "github.com/shiningrush/droplet/wrapper/gin"
 	"github.com/yuin/gopher-lua"
-
-	"github.com/apisix/manager-api/conf"
+	wgin "github.com/shiningrush/droplet/wrapper/gin"
+	"github.com/apisix/manager-api/internal/conf"
 	"github.com/apisix/manager-api/internal/core/entity"
 	"github.com/apisix/manager-api/internal/core/store"
 	"github.com/apisix/manager-api/internal/handler"
+	"github.com/apisix/manager-api/internal/log"
 	"github.com/apisix/manager-api/internal/utils"
 	"github.com/apisix/manager-api/internal/utils/consts"
-	"github.com/apisix/manager-api/log"
 )
 
 type Handler struct {
@@ -348,13 +347,15 @@ type UpdateInput struct {
 
 func (h *Handler) Update(c droplet.Context) (interface{}, error) {
 	input := c.Input().(*UpdateInput)
-	if input.ID != "" {
-		input.Route.ID = input.ID
+
+	// check if ID in body is equal ID in path
+	if err := handler.IDCompare(input.ID, input.Route.ID); err != nil {
+		return &data.SpecCodeResponse{StatusCode: http.StatusBadRequest}, err
 	}
 
-	if input.Route.Host != "" && len(input.Route.Hosts) > 0 {
-		return &data.SpecCodeResponse{StatusCode: http.StatusBadRequest},
-			fmt.Errorf("only one of host or hosts is allowed")
+	// if has id in path, use it
+	if input.ID != "" {
+		input.Route.ID = input.ID
 	}
 
 	//check depend