You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/10/09 09:13:52 UTC

[GitHub] [apisix-dashboard] monkeyDluffy6017 opened a new pull request, #2634: Next

monkeyDluffy6017 opened a new pull request, #2634:
URL: https://github.com/apache/apisix-dashboard/pull/2634

   Please answer these questions before submitting a pull request, **or your PR will get closed**.
   
   **Why submit this pull request?**
   
   - [ ] Bugfix
   - [x] New feature provided
   - [ ] Improve performance
   - [ ] Backport patches
   
   **What changes will this PR take into?**
   
   The backend of apisix-dashboard currently has many of the same interfaces as apisix, and there are differences in implementation, resulting in inconsistent between apisix and apisix-dashboard, and every change to the apisix interface will result in changes to apisix-dashboard. This is a lot of work.
   The apisix-dashboard backend has now been modified to only proxy related requests to apisix, and is no longer implemented separately.
   
   **Checklist:**
   
   - [x] Did you explain what problem does this PR solve? Or what new features have been added?
   - [x] Have you added corresponding test cases?
   - [x] Have you modified the corresponding document?
   - [ ] Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-dashboard] monkeyDluffy6017 commented on a diff in pull request #2634: feat: Proxy the related requests to apisix

Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #2634:
URL: https://github.com/apache/apisix-dashboard/pull/2634#discussion_r995572373


##########
api/internal/handler/authentication/authentication.go:
##########
@@ -101,14 +101,14 @@ func (h *Handler) userLogin(_ *gin.Context, input interface{}) handler.Response
 	if user == nil {
 		return handler.Response{
 			StatusCode: http.StatusUnauthorized,
-			Message:    consts.ErrUsernamePassword.Error(),
+			ErrMsg:     consts.ErrUsernamePassword.Error(),

Review Comment:
   ok



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-dashboard] monkeyDluffy6017 commented on a diff in pull request #2634: feat: Proxy the related requests to apisix

Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #2634:
URL: https://github.com/apache/apisix-dashboard/pull/2634#discussion_r995584832


##########
api/internal/handler/authentication/authentication.go:
##########
@@ -51,9 +51,9 @@ type UserSession struct {
 // swagger:model LoginInput
 type LoginInput struct {
 	// username
-	Username string `form:"username" binding:"required"`
+	Username string `json:"username" binding:"required"`

Review Comment:
   The old version can get `form`, but the new one can not, it took me some time to fix this, but it's not working



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-dashboard] bzp2010 commented on a diff in pull request #2634: feat: Proxy the related requests to apisix

Posted by GitBox <gi...@apache.org>.
bzp2010 commented on code in PR #2634:
URL: https://github.com/apache/apisix-dashboard/pull/2634#discussion_r997857648


##########
api/config/config.yaml:
##########
@@ -32,7 +32,8 @@ data_source:                                       # The data source type of API
 
 security:
   allow_list:             # If we don't set any IP list, then localhost IP access is allowed by default.
-    - 127.0.0.1           # The rules are checked in sequence until the first match is found.
+    - 172.16.238.1        # The rules are checked in sequence until the first match is found.

Review Comment:
   Yes, just like I did with `yq` in CI.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-dashboard] bzp2010 commented on a diff in pull request #2634: feat: Proxy the related requests to apisix

Posted by GitBox <gi...@apache.org>.
bzp2010 commented on code in PR #2634:
URL: https://github.com/apache/apisix-dashboard/pull/2634#discussion_r995491943


##########
api/internal/handler/authentication/authentication.go:
##########
@@ -51,9 +51,9 @@ type UserSession struct {
 // swagger:model LoginInput
 type LoginInput struct {
 	// username
-	Username string `form:"username" binding:"required"`
+	Username string `json:"username" binding:"required"`

Review Comment:
   Why did you change the `form` to `json`?



##########
api/config/config.yaml:
##########
@@ -32,7 +32,8 @@ data_source:                                       # The data source type of API
 
 security:
   allow_list:             # If we don't set any IP list, then localhost IP access is allowed by default.
-    - 127.0.0.1           # The rules are checked in sequence until the first match is found.
+    - 172.16.238.1        # The rules are checked in sequence until the first match is found.

Review Comment:
   The default configuration should not be modified.



##########
api/internal/handler/handler.go:
##########
@@ -60,10 +60,8 @@ func Wrap(f Func, inputType reflect.Type) gin.HandlerFunc {
 		}
 
 		resp := f(c, input)
-		if resp.StatusCode == 0 {
-			resp.StatusCode = 200
-		}
-		c.JSON(resp.StatusCode, buildResponse(resp.Success, resp.Message, resp.Data))
+
+		c.JSON(resp.StatusCode, buildResponse(resp.Success, resp.ErrMsg, resp.Data))

Review Comment:
   Default response status codes should be provided.



##########
api/internal/handler/authentication/authentication.go:
##########
@@ -101,14 +101,14 @@ func (h *Handler) userLogin(_ *gin.Context, input interface{}) handler.Response
 	if user == nil {
 		return handler.Response{
 			StatusCode: http.StatusUnauthorized,
-			Message:    consts.ErrUsernamePassword.Error(),
+			ErrMsg:     consts.ErrUsernamePassword.Error(),
 		}
 	}
 
 	if username != user.Username || password != user.Password {
 		return handler.Response{
 			StatusCode: http.StatusUnauthorized,
-			Message:    consts.ErrUsernamePassword.Error(),
+			ErrMsg:     consts.ErrUsernamePassword.Error(),

Review Comment:
   ditto



##########
api/internal/handler/resources/resources.go:
##########
@@ -91,12 +91,19 @@ func (h *Handler) setupClient(cfg config.Config) {
 func (h *Handler) ProxyResource(c *gin.Context, _ interface{}) handler.Response {
 	resp, err := h.proxy(c)
 	if err != nil {
-		return *err
+		return handler.Response{
+			Success: false,
+			ErrMsg:  err.Error(),
+		}
 	}
 
 	data := gjson.ParseBytes(resp.Body())
 	if err := h.extractError(data); err != nil {
-		return *err
+		return handler.Response{
+			StatusCode: resp.StatusCode(),

Review Comment:
   There is no need to pass through the Admin API status code.



##########
api/internal/handler/resources/resources.go:
##########
@@ -106,60 +113,57 @@ func (h *Handler) ProxyResource(c *gin.Context, _ interface{}) handler.Response
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    h.processResponse(version, data),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       h.processResponse(version, data),
 	}
 }
 
 // ProxyMisc sends the Admin API response as-is, adding only the wrapping required by the Dashboard
 func (h *Handler) ProxyMisc(c *gin.Context, _ interface{}) handler.Response {
 	resp, err := h.proxy(c)
 	if err != nil {
-		return *err
+		return handler.Response{
+			Success: false,
+			ErrMsg:  err.Error(),
+		}
 	}
 
 	data := gjson.ParseBytes(resp.Body())
 	if err := h.extractError(data); err != nil {
-		return *err
+		return handler.Response{
+			Success:    false,
+			StatusCode: resp.StatusCode(),

Review Comment:
   ditto



##########
api/internal/handler/resources/resources.go:
##########
@@ -106,60 +113,57 @@ func (h *Handler) ProxyResource(c *gin.Context, _ interface{}) handler.Response
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    h.processResponse(version, data),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       h.processResponse(version, data),
 	}
 }
 
 // ProxyMisc sends the Admin API response as-is, adding only the wrapping required by the Dashboard
 func (h *Handler) ProxyMisc(c *gin.Context, _ interface{}) handler.Response {
 	resp, err := h.proxy(c)
 	if err != nil {
-		return *err
+		return handler.Response{
+			Success: false,
+			ErrMsg:  err.Error(),
+		}
 	}
 
 	data := gjson.ParseBytes(resp.Body())
 	if err := h.extractError(data); err != nil {
-		return *err
+		return handler.Response{
+			Success:    false,
+			StatusCode: resp.StatusCode(),
+			ErrMsg:     err.Error(),
+		}
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    data.Value(),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       data.Value(),
 	}
 }
 
 // proxy forwards user requests for these interfaces to the Admin API as-is.
-func (h *Handler) proxy(c *gin.Context) (*resty.Response, *handler.Response) {
+func (h *Handler) proxy(c *gin.Context) (*resty.Response, error) {

Review Comment:
   No need to modify this for now



##########
api/internal/handler/resources/resources.go:
##########
@@ -106,60 +113,57 @@ func (h *Handler) ProxyResource(c *gin.Context, _ interface{}) handler.Response
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    h.processResponse(version, data),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       h.processResponse(version, data),
 	}
 }
 
 // ProxyMisc sends the Admin API response as-is, adding only the wrapping required by the Dashboard
 func (h *Handler) ProxyMisc(c *gin.Context, _ interface{}) handler.Response {
 	resp, err := h.proxy(c)
 	if err != nil {
-		return *err
+		return handler.Response{
+			Success: false,
+			ErrMsg:  err.Error(),
+		}
 	}
 
 	data := gjson.ParseBytes(resp.Body())
 	if err := h.extractError(data); err != nil {
-		return *err
+		return handler.Response{
+			Success:    false,
+			StatusCode: resp.StatusCode(),
+			ErrMsg:     err.Error(),
+		}
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    data.Value(),
+		Success:    true,
+		StatusCode: resp.StatusCode(),

Review Comment:
   ditto



##########
api/test/docker/setup.sh:
##########
@@ -90,6 +90,7 @@ up() {
 
     #modifying config
     sed -i 's/127.0.0.1:2379/172.16.238.10:2379/' "$YAML_FILE"
+    sed -i 's/127.0.0.1:9180/172.16.238.30:9180/' "$YAML_FILE"

Review Comment:
   As far as I know, we don't use this file anymore.



##########
api/test/e2e/base/base.go:
##########
@@ -304,18 +301,24 @@ func GetResourceList(resource string) string {
 
 func CleanResource(resource string) {
 	resources := GetResourceList(resource)
-	list := gjson.Get(resources, "data.rows").Value().([]interface{})
+	//body, _ := json.Marshal(resources)
+	//res := fmt.Sprintf("%s%s", "resources: ", resources)
+	//panic(res)

Review Comment:
   Are they still useful?



##########
api/internal/handler/authentication/authentication.go:
##########
@@ -101,14 +101,14 @@ func (h *Handler) userLogin(_ *gin.Context, input interface{}) handler.Response
 	if user == nil {
 		return handler.Response{
 			StatusCode: http.StatusUnauthorized,
-			Message:    consts.ErrUsernamePassword.Error(),
+			ErrMsg:     consts.ErrUsernamePassword.Error(),

Review Comment:
   The response body style should not be modified, it is predetermined.



##########
api/internal/handler/handler.go:
##########
@@ -38,8 +38,8 @@ type Func = func(c *gin.Context, input interface{}) Response
 type Response struct {
 	StatusCode int
 	Success    bool
-	Message    string
 	Data       interface{}
+	ErrMsg     string

Review Comment:
   The response body style should not be modified, it is predetermined.



##########
api/internal/handler/resources/resources.go:
##########
@@ -91,12 +91,19 @@ func (h *Handler) setupClient(cfg config.Config) {
 func (h *Handler) ProxyResource(c *gin.Context, _ interface{}) handler.Response {
 	resp, err := h.proxy(c)
 	if err != nil {
-		return *err
+		return handler.Response{
+			Success: false,
+			ErrMsg:  err.Error(),
+		}

Review Comment:
   We'd better not change it for now, maybe it's better to address the code style issue in a later PR.



##########
api/internal/handler/resources/resources.go:
##########
@@ -77,7 +77,7 @@ func (h *Handler) ApplyRoute(r *gin.Engine, cfg config.Config) {
 	}
 
 	// add misc routes
-	r.GET("/apisix/admin/plugins/list", handler.Wrap(h.ProxyMisc, nil))
+	r.GET("/apisix/admin/plugins/*param", handler.Wrap(h.ProxyMisc, nil))

Review Comment:
   `list` is enough. It is used to get the full table of plugins.
   
   We have this usage, and you can verify it this way
   - /apisix/admin/plugins/list
   - /apisix/admin/schema/route
   - /apisix/admin/schema/plugins/real-ip



##########
api/internal/handler/resources/resources.go:
##########
@@ -91,12 +91,19 @@ func (h *Handler) setupClient(cfg config.Config) {
 func (h *Handler) ProxyResource(c *gin.Context, _ interface{}) handler.Response {
 	resp, err := h.proxy(c)
 	if err != nil {
-		return *err
+		return handler.Response{
+			Success: false,
+			ErrMsg:  err.Error(),
+		}
 	}
 
 	data := gjson.ParseBytes(resp.Body())
 	if err := h.extractError(data); err != nil {
-		return *err
+		return handler.Response{
+			StatusCode: resp.StatusCode(),
+			Success:    false,
+			ErrMsg:     err.Error(),
+		}
 	}

Review Comment:
   ditto



##########
api/internal/handler/authentication/authentication.go:
##########
@@ -51,9 +51,9 @@ type UserSession struct {
 // swagger:model LoginInput
 type LoginInput struct {
 	// username
-	Username string `form:"username" binding:"required"`
+	Username string `json:"username" binding:"required"`
 	// password
-	Password string `form:"password" binding:"required"`
+	Password string `json:"password" binding:"required"`

Review Comment:
   ditto



##########
api/internal/handler/resources/resources.go:
##########
@@ -106,60 +113,57 @@ func (h *Handler) ProxyResource(c *gin.Context, _ interface{}) handler.Response
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    h.processResponse(version, data),
+		Success:    true,
+		StatusCode: resp.StatusCode(),

Review Comment:
   There is no need to pass through the Admin API status code.



##########
api/internal/handler/resources/resources.go:
##########
@@ -106,60 +113,57 @@ func (h *Handler) ProxyResource(c *gin.Context, _ interface{}) handler.Response
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    h.processResponse(version, data),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       h.processResponse(version, data),
 	}
 }
 
 // ProxyMisc sends the Admin API response as-is, adding only the wrapping required by the Dashboard
 func (h *Handler) ProxyMisc(c *gin.Context, _ interface{}) handler.Response {
 	resp, err := h.proxy(c)
 	if err != nil {
-		return *err
+		return handler.Response{
+			Success: false,
+			ErrMsg:  err.Error(),
+		}

Review Comment:
   ditto



##########
api/internal/handler/resources/resources.go:
##########
@@ -106,60 +113,57 @@ func (h *Handler) ProxyResource(c *gin.Context, _ interface{}) handler.Response
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    h.processResponse(version, data),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       h.processResponse(version, data),
 	}

Review Comment:
   ditto



##########
api/internal/handler/resources/resources.go:
##########
@@ -106,60 +113,57 @@ func (h *Handler) ProxyResource(c *gin.Context, _ interface{}) handler.Response
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    h.processResponse(version, data),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       h.processResponse(version, data),
 	}
 }
 
 // ProxyMisc sends the Admin API response as-is, adding only the wrapping required by the Dashboard
 func (h *Handler) ProxyMisc(c *gin.Context, _ interface{}) handler.Response {
 	resp, err := h.proxy(c)
 	if err != nil {
-		return *err
+		return handler.Response{
+			Success: false,
+			ErrMsg:  err.Error(),
+		}
 	}
 
 	data := gjson.ParseBytes(resp.Body())
 	if err := h.extractError(data); err != nil {
-		return *err
+		return handler.Response{
+			Success:    false,
+			StatusCode: resp.StatusCode(),
+			ErrMsg:     err.Error(),
+		}

Review Comment:
   ditto



##########
api/internal/handler/resources/resources.go:
##########
@@ -106,60 +113,57 @@ func (h *Handler) ProxyResource(c *gin.Context, _ interface{}) handler.Response
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    h.processResponse(version, data),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       h.processResponse(version, data),
 	}
 }
 
 // ProxyMisc sends the Admin API response as-is, adding only the wrapping required by the Dashboard
 func (h *Handler) ProxyMisc(c *gin.Context, _ interface{}) handler.Response {
 	resp, err := h.proxy(c)
 	if err != nil {
-		return *err
+		return handler.Response{
+			Success: false,
+			ErrMsg:  err.Error(),
+		}
 	}
 
 	data := gjson.ParseBytes(resp.Body())
 	if err := h.extractError(data); err != nil {
-		return *err
+		return handler.Response{
+			Success:    false,
+			StatusCode: resp.StatusCode(),
+			ErrMsg:     err.Error(),
+		}
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    data.Value(),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       data.Value(),
 	}

Review Comment:
   ditto



##########
api/test/docker/apisix_config.yaml:
##########
@@ -57,61 +54,14 @@ apisix:
       - 10095
 
 nginx_config:
-  error_log_level: "debug"
-
-plugins:                          # plugin list (sorted in alphabetical order)
-  - api-breaker
-  - authz-keycloak
-  - basic-auth
-  - batch-requests
-  - consumer-restriction
-  - cors
-  - echo
-  - fault-injection
-  - grpc-transcode
-  - hmac-auth
-  - http-logger
-  - ip-restriction
-  - jwt-auth
-  - kafka-logger
-  - key-auth
-  - limit-conn
-  - limit-count
-  - limit-req
-  - node-status
-  - openid-connect
-  - prometheus
-  - proxy-cache
-  - proxy-mirror
-  - proxy-rewrite
-  - redirect
-  - referer-restriction
-  - request-id
-  - request-validation
-  - response-rewrite
-  - serverless-post-function
-  - serverless-pre-function
-  - skywalking
-  - sls-logger
-  - syslog
-  - tcp-logger
-  - udp-logger
-  - uri-blocker
-  - wolf-rbac
-  - zipkin
-  - server-info
+  error_log_level: debug
 
 plugin_attr:
   skywalking:
-    service_name: APISIX
-    service_instance_name: "APISIX Instance Name"
     endpoint_addr: http://172.16.238.50:12800
   server-info:
-    report_interval: 60
     report_ttl: 3600
   prometheus:
-    export_uri: /apisix/prometheus/metrics
     enable_export_server: true
     export_addr:
-      ip: "0.0.0.0"
-      port: 9091
+      ip: 0.0.0.0

Review Comment:
   Instead of modifying `apisix_config.yaml` for now, we are now using `apisix_config_v3.yaml`.



##########
api/internal/handler/resources/resources.go:
##########
@@ -106,60 +113,57 @@ func (h *Handler) ProxyResource(c *gin.Context, _ interface{}) handler.Response
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    h.processResponse(version, data),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       h.processResponse(version, data),
 	}
 }
 
 // ProxyMisc sends the Admin API response as-is, adding only the wrapping required by the Dashboard
 func (h *Handler) ProxyMisc(c *gin.Context, _ interface{}) handler.Response {
 	resp, err := h.proxy(c)
 	if err != nil {
-		return *err
+		return handler.Response{
+			Success: false,
+			ErrMsg:  err.Error(),
+		}
 	}
 
 	data := gjson.ParseBytes(resp.Body())
 	if err := h.extractError(data); err != nil {
-		return *err
+		return handler.Response{
+			Success:    false,
+			StatusCode: resp.StatusCode(),
+			ErrMsg:     err.Error(),
+		}
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    data.Value(),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       data.Value(),
 	}
 }
 
 // proxy forwards user requests for these interfaces to the Admin API as-is.
-func (h *Handler) proxy(c *gin.Context) (*resty.Response, *handler.Response) {
+func (h *Handler) proxy(c *gin.Context) (*resty.Response, error) {
 	req := h.client.NewRequest()
 	req.SetHeaderMultiValues(c.Request.Header)
 	req.SetQueryParamsFromValues(c.Request.URL.Query())
 	req.SetBody(c.Request.Body)
 
 	resourceURI := strings.Replace(c.Request.URL.Path, "/apisix/admin", "", 1)
 	resp, err := req.Execute(c.Request.Method, resourceURI)
-	if err != nil {
-		return nil, &handler.Response{
-			StatusCode: http.StatusInternalServerError,
-			Message:    "Admin API request failed",
-		}
-	}
-
-	return resp, nil
+	return resp, err
 }
 
-func (h *Handler) extractError(data gjson.Result) *handler.Response {
+func (h *Handler) extractError(data gjson.Result) error {

Review Comment:
   No need to modify this for now



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-dashboard] netlify[bot] commented on pull request #2634: Next

Posted by GitBox <gi...@apache.org>.
netlify[bot] commented on PR #2634:
URL: https://github.com/apache/apisix-dashboard/pull/2634#issuecomment-1272496178

   ### <span aria-hidden="true">👷</span> Deploy Preview for *apisix-dashboard* processing.
   
   
   |  Name | Link |
   |---------------------------------|------------------------|
   |<span aria-hidden="true">🔨</span> Latest commit | 08f25648eb4ad07db252cb378d6e048c470819a3 |
   |<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/sites/apisix-dashboard/deploys/634290d005066200088648ca |


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-dashboard] monkeyDluffy6017 commented on a diff in pull request #2634: feat: Proxy the related requests to apisix

Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #2634:
URL: https://github.com/apache/apisix-dashboard/pull/2634#discussion_r995591277


##########
api/internal/handler/resources/resources.go:
##########
@@ -106,60 +113,57 @@ func (h *Handler) ProxyResource(c *gin.Context, _ interface{}) handler.Response
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    h.processResponse(version, data),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       h.processResponse(version, data),
 	}
 }
 
 // ProxyMisc sends the Admin API response as-is, adding only the wrapping required by the Dashboard
 func (h *Handler) ProxyMisc(c *gin.Context, _ interface{}) handler.Response {
 	resp, err := h.proxy(c)
 	if err != nil {
-		return *err
+		return handler.Response{
+			Success: false,
+			ErrMsg:  err.Error(),
+		}
 	}
 
 	data := gjson.ParseBytes(resp.Body())
 	if err := h.extractError(data); err != nil {
-		return *err
+		return handler.Response{
+			Success:    false,
+			StatusCode: resp.StatusCode(),
+			ErrMsg:     err.Error(),
+		}
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    data.Value(),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       data.Value(),
 	}
 }
 
 // proxy forwards user requests for these interfaces to the Admin API as-is.
-func (h *Handler) proxy(c *gin.Context) (*resty.Response, *handler.Response) {
+func (h *Handler) proxy(c *gin.Context) (*resty.Response, error) {

Review Comment:
   I want to get the type of the error and handle it uniformly in the outer layer



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-dashboard] monkeyDluffy6017 commented on a diff in pull request #2634: feat: Proxy the related requests to apisix

Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #2634:
URL: https://github.com/apache/apisix-dashboard/pull/2634#discussion_r995570681


##########
api/internal/handler/authentication/authentication.go:
##########
@@ -51,9 +51,9 @@ type UserSession struct {
 // swagger:model LoginInput
 type LoginInput struct {
 	// username
-	Username string `form:"username" binding:"required"`
+	Username string `json:"username" binding:"required"`

Review Comment:
   I have tried `form`, but only `json` can be got in userLogin, you could try that



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-dashboard] monkeyDluffy6017 commented on a diff in pull request #2634: feat: Proxy the related requests to apisix

Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #2634:
URL: https://github.com/apache/apisix-dashboard/pull/2634#discussion_r995591277


##########
api/internal/handler/resources/resources.go:
##########
@@ -106,60 +113,57 @@ func (h *Handler) ProxyResource(c *gin.Context, _ interface{}) handler.Response
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    h.processResponse(version, data),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       h.processResponse(version, data),
 	}
 }
 
 // ProxyMisc sends the Admin API response as-is, adding only the wrapping required by the Dashboard
 func (h *Handler) ProxyMisc(c *gin.Context, _ interface{}) handler.Response {
 	resp, err := h.proxy(c)
 	if err != nil {
-		return *err
+		return handler.Response{
+			Success: false,
+			ErrMsg:  err.Error(),
+		}
 	}
 
 	data := gjson.ParseBytes(resp.Body())
 	if err := h.extractError(data); err != nil {
-		return *err
+		return handler.Response{
+			Success:    false,
+			StatusCode: resp.StatusCode(),
+			ErrMsg:     err.Error(),
+		}
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    data.Value(),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       data.Value(),
 	}
 }
 
 // proxy forwards user requests for these interfaces to the Admin API as-is.
-func (h *Handler) proxy(c *gin.Context) (*resty.Response, *handler.Response) {
+func (h *Handler) proxy(c *gin.Context) (*resty.Response, error) {

Review Comment:
   I want to get the type of the error and handle it uniformly in the outer layer



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-dashboard] codecov-commenter commented on pull request #2634: feat: Proxy the related requests to apisix

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #2634:
URL: https://github.com/apache/apisix-dashboard/pull/2634#issuecomment-1272499139

   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/2634?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#2634](https://codecov.io/gh/apache/apisix-dashboard/pull/2634?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (964f396) into [next](https://codecov.io/gh/apache/apisix-dashboard/commit/005ed14c482ecbaef862bb14b839f964ea750f0c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (005ed14) will **not change** coverage.
   > The diff coverage is `n/a`.
   
   ```diff
   @@           Coverage Diff           @@
   ##             next    #2634   +/-   ##
   =======================================
     Coverage   41.72%   41.72%           
   =======================================
     Files          15       15           
     Lines         405      405           
   =======================================
     Hits          169      169           
     Misses        213      213           
     Partials       23       23           
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | backend-unit-test | `41.72% <ø> (ø)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-dashboard] monkeyDluffy6017 commented on a diff in pull request #2634: feat: Proxy the related requests to apisix

Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #2634:
URL: https://github.com/apache/apisix-dashboard/pull/2634#discussion_r995586080


##########
api/internal/handler/resources/resources.go:
##########
@@ -106,60 +113,57 @@ func (h *Handler) ProxyResource(c *gin.Context, _ interface{}) handler.Response
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    h.processResponse(version, data),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       h.processResponse(version, data),
 	}
 }
 
 // ProxyMisc sends the Admin API response as-is, adding only the wrapping required by the Dashboard
 func (h *Handler) ProxyMisc(c *gin.Context, _ interface{}) handler.Response {
 	resp, err := h.proxy(c)
 	if err != nil {
-		return *err
+		return handler.Response{
+			Success: false,
+			ErrMsg:  err.Error(),
+		}
 	}
 
 	data := gjson.ParseBytes(resp.Body())
 	if err := h.extractError(data); err != nil {
-		return *err
+		return handler.Response{
+			Success:    false,
+			StatusCode: resp.StatusCode(),
+			ErrMsg:     err.Error(),
+		}
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    data.Value(),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       data.Value(),
 	}
 }
 
 // proxy forwards user requests for these interfaces to the Admin API as-is.
-func (h *Handler) proxy(c *gin.Context) (*resty.Response, *handler.Response) {
+func (h *Handler) proxy(c *gin.Context) (*resty.Response, error) {

Review Comment:
   Why don't you return the error?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-dashboard] bzp2010 commented on a diff in pull request #2634: feat: Proxy the related requests to apisix

Posted by GitBox <gi...@apache.org>.
bzp2010 commented on code in PR #2634:
URL: https://github.com/apache/apisix-dashboard/pull/2634#discussion_r998885598


##########
api/internal/handler/authentication/authentication.go:
##########
@@ -51,9 +51,9 @@ type UserSession struct {
 // swagger:model LoginInput
 type LoginInput struct {
 	// username
-	Username string `form:"username" binding:"required"`
+	Username string `json:"username" binding:"required"`

Review Comment:
   I'm sure it's available.
   
   ![image](https://user-images.githubusercontent.com/8078418/196583569-24d84da9-94d7-484d-8b31-597d02a6b29c.png)
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-dashboard] monkeyDluffy6017 commented on a diff in pull request #2634: feat: Proxy the related requests to apisix

Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #2634:
URL: https://github.com/apache/apisix-dashboard/pull/2634#discussion_r995571355


##########
api/config/config.yaml:
##########
@@ -32,7 +32,8 @@ data_source:                                       # The data source type of API
 
 security:
   allow_list:             # If we don't set any IP list, then localhost IP access is allowed by default.
-    - 127.0.0.1           # The rules are checked in sequence until the first match is found.
+    - 172.16.238.1        # The rules are checked in sequence until the first match is found.

Review Comment:
   it's test code, modify this in ci ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-dashboard] monkeyDluffy6017 commented on pull request #2634: feat: Proxy the related requests to apisix

Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on PR #2634:
URL: https://github.com/apache/apisix-dashboard/pull/2634#issuecomment-1278762804

   > And, why have many CIs been removed?
   
   @bzp2010 I just want to test backend-e2e-test-v3, i will add the deleted ci files later


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-dashboard] monkeyDluffy6017 commented on a diff in pull request #2634: feat: Proxy the related requests to apisix

Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #2634:
URL: https://github.com/apache/apisix-dashboard/pull/2634#discussion_r995617632


##########
api/internal/handler/resources/resources.go:
##########
@@ -106,60 +113,57 @@ func (h *Handler) ProxyResource(c *gin.Context, _ interface{}) handler.Response
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    h.processResponse(version, data),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       h.processResponse(version, data),
 	}
 }
 
 // ProxyMisc sends the Admin API response as-is, adding only the wrapping required by the Dashboard
 func (h *Handler) ProxyMisc(c *gin.Context, _ interface{}) handler.Response {
 	resp, err := h.proxy(c)
 	if err != nil {
-		return *err
+		return handler.Response{
+			Success: false,
+			ErrMsg:  err.Error(),
+		}
 	}
 
 	data := gjson.ParseBytes(resp.Body())
 	if err := h.extractError(data); err != nil {
-		return *err
+		return handler.Response{
+			Success:    false,
+			StatusCode: resp.StatusCode(),
+			ErrMsg:     err.Error(),
+		}
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    data.Value(),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       data.Value(),
 	}
 }
 
 // proxy forwards user requests for these interfaces to the Admin API as-is.
-func (h *Handler) proxy(c *gin.Context) (*resty.Response, *handler.Response) {
+func (h *Handler) proxy(c *gin.Context) (*resty.Response, error) {

Review Comment:
   It's better for handling kinds of error in outer scope



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-dashboard] monkeyDluffy6017 commented on a diff in pull request #2634: feat: Proxy the related requests to apisix

Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #2634:
URL: https://github.com/apache/apisix-dashboard/pull/2634#discussion_r995617632


##########
api/internal/handler/resources/resources.go:
##########
@@ -106,60 +113,57 @@ func (h *Handler) ProxyResource(c *gin.Context, _ interface{}) handler.Response
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    h.processResponse(version, data),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       h.processResponse(version, data),
 	}
 }
 
 // ProxyMisc sends the Admin API response as-is, adding only the wrapping required by the Dashboard
 func (h *Handler) ProxyMisc(c *gin.Context, _ interface{}) handler.Response {
 	resp, err := h.proxy(c)
 	if err != nil {
-		return *err
+		return handler.Response{
+			Success: false,
+			ErrMsg:  err.Error(),
+		}
 	}
 
 	data := gjson.ParseBytes(resp.Body())
 	if err := h.extractError(data); err != nil {
-		return *err
+		return handler.Response{
+			Success:    false,
+			StatusCode: resp.StatusCode(),
+			ErrMsg:     err.Error(),
+		}
 	}
 
 	return handler.Response{
-		Success: true,
-		Data:    data.Value(),
+		Success:    true,
+		StatusCode: resp.StatusCode(),
+		Data:       data.Value(),
 	}
 }
 
 // proxy forwards user requests for these interfaces to the Admin API as-is.
-func (h *Handler) proxy(c *gin.Context) (*resty.Response, *handler.Response) {
+func (h *Handler) proxy(c *gin.Context) (*resty.Response, error) {

Review Comment:
   It's better for handling kinds of error in outer scope



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org