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 2020/11/07 15:46:14 UTC

[GitHub] [apisix-dashboard] nic-chen opened a new pull request #719: fix: dashboard 2.0 failed to fetch ssl certificate not found

nic-chen opened a new pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719


   close #697


----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] moonming commented on a change in pull request #719: fix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519238296



##########
File path: api/test/e2e/ssl_test.go
##########
@@ -0,0 +1,122 @@
+/*
+ * 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 e2e
+
+import (
+	"crypto/tls"
+	"encoding/json"
+	"io/ioutil"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSSL_Basic(t *testing.T) {
+	testCert, err := ioutil.ReadFile("../certs/test2.crt")
+	assert.Nil(t, err)
+	testKey, err := ioutil.ReadFile("../certs/test2.key")
+	assert.Nil(t, err)
+	apisixKey, err := ioutil.ReadFile("../certs/apisix.key")
+	assert.Nil(t, err)
+	body, err := json.Marshal(map[string]string{
+		"id":   "1",
+		"cert": string(testCert),
+		"key":  string(testKey),
+	})
+	assert.Nil(t, err)
+	invalidBody, err := json.Marshal(map[string]string{
+		"id":   "1",
+		"cert": string(testCert),
+		"key":  string(apisixKey),
+	})
+
+	//Before configuring SSL, make a HTTPS request
+	http.DefaultTransport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
+	_, err = http.Get("https://www.test2.com:9443")
+	assert.NotNil(t, err)
+	assert.EqualError(t, err, "Get \"https://www.test2.com:9443\": remote error: tls: internal error")
+
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "create ssl fail - key and cert not match",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodPost,
+			Path:         "/apisix/admin/ssl",
+			Body:         string(invalidBody),
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusBadRequest,
+		},
+		{
+			caseDesc:     "create ssl successfully",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodPost,
+			Path:         "/apisix/admin/ssl",
+			Body:         string(body),
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc: "create route",
+			Object:   MangerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/routes/r1",
+			Body: `{
+				"uri": "/hello_",
+				"hosts": ["test2.com", "*.test2.com"],
+				"upstream": {
+					"nodes": {
+						"172.16.238.20:1980": 1
+					},
+					"type": "roundrobin"
+				}
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc:     "hit the route just created using HTTPS",
+			Object:       APISIXHTTPSExpect(t),
+			Method:       http.MethodGet,
+			Path:         "/hello_",
+			Headers:      map[string]string{"Host": "www.test2.com"},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "hello world\n",
+			Sleep:        sleepTime,
+		},
+		{
+			caseDesc:     "delete route",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodDelete,
+			Path:         "/apisix/admin/routes/r1",
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc:     "delete ssl",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodDelete,
+			Path:         "/apisix/admin/ssl/1",
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,

Review comment:
       We need to check again after delete route and ssl cert




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] membphis commented on a change in pull request #719: bugfix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519276779



##########
File path: api/internal/handler/ssl/ssl.go
##########
@@ -147,6 +148,8 @@ func (h *Handler) Create(c droplet.Context) (interface{}, error) {
 	}
 
 	ssl.ID = input.ID
+	//set default value for SSL status, if not set, it will be 0 which means disable.
+	ssl.Status = conf.SSLDefaultStatus
 	if err := h.sslStore.Create(c.Context(), ssl); err != nil {
 		return handler.SpecCodeResponse(err), err

Review comment:
       Did we catch this error and write it to the error log?  please confirm it. if not, we can create a new issue to fix it

##########
File path: api/internal/handler/ssl/ssl.go
##########
@@ -34,6 +34,7 @@ import (
 	"github.com/shiningrush/droplet/wrapper"
 	wgin "github.com/shiningrush/droplet/wrapper/gin"
 
+	"github.com/apisix/manager-api/conf"

Review comment:
       we'll move this module to another place, is it right?

##########
File path: api/internal/handler/ssl/ssl.go
##########
@@ -169,6 +172,9 @@ func (h *Handler) Update(c droplet.Context) (interface{}, error) {
 	if input.ID != "" {
 		ssl.ID = input.ID
 	}
+
+	//set default value for SSL status, if not set, it will be 0 which means disable.
+	ssl.Status = conf.SSLDefaultStatus
 	if err := h.sslStore.Update(c.Context(), ssl, true); err != nil {
 		return handler.SpecCodeResponse(err), err

Review comment:
       ditto




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] membphis commented on a change in pull request #719: bugfix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519263674



##########
File path: .github/workflows/e2e-test-backend.yml
##########
@@ -28,7 +28,10 @@ jobs:
           sleep 5
           docker logs docker_managerapi_1
 
+      - name: set hosts
+        working-directory: ./api/test/e2e
+        run: echo "127.0.0.1 www.test2.com" | sudo tee -a /etc/hosts

Review comment:
       ok, got it. we can write some comment first, then we can fix it in a better way.




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #719: bugfix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519405057



##########
File path: api/test/e2e/ssl_test.go
##########
@@ -0,0 +1,138 @@
+/*
+ * 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 e2e
+
+import (
+	"context"
+	"crypto/tls"
+	"encoding/json"
+	"io/ioutil"
+	"net"
+	"net/http"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSSL_Basic(t *testing.T) {
+	testCert, err := ioutil.ReadFile("../certs/test2.crt")
+	assert.Nil(t, err)
+	testKey, err := ioutil.ReadFile("../certs/test2.key")
+	assert.Nil(t, err)
+	apisixKey, err := ioutil.ReadFile("../certs/apisix.key")
+	assert.Nil(t, err)
+	body, err := json.Marshal(map[string]string{
+		"id":   "1",
+		"cert": string(testCert),
+		"key":  string(testKey),
+	})
+	assert.Nil(t, err)
+	invalidBody, err := json.Marshal(map[string]string{
+		"id":   "1",
+		"cert": string(testCert),
+		"key":  string(apisixKey),
+	})
+
+	//Before configuring SSL, make a HTTPS request
+	http.DefaultTransport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
+	http.DefaultTransport.(*http.Transport).DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
+		if addr == "www.test2.com:9443" {
+			addr = "127.0.0.1:9443"
+		}
+		dialer := &net.Dialer{}
+		return dialer.DialContext(ctx, network, addr)
+	}
+	_, err = http.Get("https://www.test2.com:9443")
+	assert.NotNil(t, err)
+	assert.EqualError(t, err, "Get https://www.test2.com:9443: remote error: tls: internal error")
+
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "create ssl fail - key and cert not match",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodPost,
+			Path:         "/apisix/admin/ssl",
+			Body:         string(invalidBody),
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusBadRequest,
+		},
+		{
+			caseDesc:     "create ssl successfully",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodPost,
+			Path:         "/apisix/admin/ssl",
+			Body:         string(body),
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc: "create route",
+			Object:   MangerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/routes/r1",
+			Body: `{
+				"uri": "/hello_",
+				"hosts": ["test2.com", "*.test2.com"],
+				"upstream": {
+					"nodes": {
+						"172.16.238.20:1980": 1
+					},
+					"type": "roundrobin"
+				}
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc:     "hit the route just created using HTTPS",
+			Object:       APISIXHTTPSExpect(t),
+			Method:       http.MethodGet,
+			Path:         "/hello_",
+			Headers:      map[string]string{"Host": "www.test2.com"},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "hello world\n",
+			Sleep:        sleepTime,
+		},
+		{
+			caseDesc:     "delete route",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodDelete,
+			Path:         "/apisix/admin/routes/r1",
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc:     "delete ssl",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodDelete,
+			Path:         "/apisix/admin/ssl/1",
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+
+	//try again after deleting SSL, make a HTTPS request
+	time.Sleep(time.Duration(20) * time.Millisecond)
+	_, err = http.Get("https://www.test2.com:9443")
+	assert.NotNil(t, err)
+	assert.EqualError(t, err, "Get https://www.test2.com:9443: remote error: tls: internal error")

Review comment:
       because APISIX exit when no SSL match:
   https://github.com/apache/apisix/blob/master/apisix/init.lua#L174-L180
   




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] membphis commented on a change in pull request #719: bugfix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519751344



##########
File path: api/conf/conf.go
##########
@@ -40,16 +40,17 @@ const (
 )
 
 var (
-	ENV           string
-	Schema        gjson.Result
-	WorkDir       = "."
-	ServerHost    = "127.0.0.1"
-	ServerPort    = 80
-	ETCDEndpoints = []string{"127.0.0.1:2379"}
-	ErrorLogLevel = "warn"
-	ErrorLogPath  = "logs/error.log"
-	UserList      = make(map[string]User, 2)
-	AuthConf      Authentication
+	ENV              string
+	Schema           gjson.Result
+	WorkDir          = "."
+	ServerHost       = "127.0.0.1"
+	ServerPort       = 80
+	ETCDEndpoints    = []string{"127.0.0.1:2379"}
+	ErrorLogLevel    = "warn"
+	ErrorLogPath     = "logs/error.log"
+	UserList         = make(map[string]User, 2)
+	AuthConf         Authentication
+	SSLDefaultStatus = 1 //enable ssl by default

Review comment:
       @nic-chen please create a new issue about this point, we will fix this 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.

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



[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #719: bugfix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#issuecomment-723461416


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=h1) Report
   > Merging [#719](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=desc) (6b864bb) into [v2.0](https://codecov.io/gh/apache/apisix-dashboard/commit/3a405f779c0d65ca68e52a9745662aca4272ecc1?el=desc) (3a405f7) will **increase** coverage by `0.17%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/719/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##             v2.0     #719      +/-   ##
   ==========================================
   + Coverage   43.08%   43.26%   +0.17%     
   ==========================================
     Files          18       18              
     Lines        1237     1239       +2     
   ==========================================
   + Hits          533      536       +3     
   + Misses        615      614       -1     
     Partials       89       89              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/handler/ssl/ssl.go](https://codecov.io/gh/apache/apisix-dashboard/pull/719/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc3NsL3NzbC5nbw==) | `28.17% <100.00%> (+0.80%)` | :arrow_up: |
   | [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/719/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.94% <0.00%> (+0.65%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=footer). Last update [3a405f7...6b864bb](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #719: fix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519246782



##########
File path: api/internal/handler/ssl/ssl.go
##########
@@ -147,6 +147,7 @@ func (h *Handler) Create(c droplet.Context) (interface{}, error) {
 	}
 
 	ssl.ID = input.ID
+	ssl.Status = 1

Review comment:
       yes, it's the cause.
   That is because in Go if does not explicitly assign a value, it will automatically assign a 0 value. So the SSL is disable.
   




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #719: bugfix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519265886



##########
File path: .github/workflows/e2e-test-backend.yml
##########
@@ -28,7 +28,10 @@ jobs:
           sleep 5
           docker logs docker_managerapi_1
 
+      - name: set hosts
+        working-directory: ./api/test/e2e
+        run: echo "127.0.0.1 www.test2.com" | sudo tee -a /etc/hosts

Review comment:
       It works! 




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #719: fix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519247139



##########
File path: api/test/e2e/ssl_test.go
##########
@@ -0,0 +1,122 @@
+/*
+ * 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 e2e
+
+import (
+	"crypto/tls"
+	"encoding/json"
+	"io/ioutil"
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSSL_Basic(t *testing.T) {
+	testCert, err := ioutil.ReadFile("../certs/test2.crt")
+	assert.Nil(t, err)
+	testKey, err := ioutil.ReadFile("../certs/test2.key")
+	assert.Nil(t, err)
+	apisixKey, err := ioutil.ReadFile("../certs/apisix.key")
+	assert.Nil(t, err)
+	body, err := json.Marshal(map[string]string{
+		"id":   "1",
+		"cert": string(testCert),
+		"key":  string(testKey),
+	})
+	assert.Nil(t, err)
+	invalidBody, err := json.Marshal(map[string]string{
+		"id":   "1",
+		"cert": string(testCert),
+		"key":  string(apisixKey),
+	})
+
+	//Before configuring SSL, make a HTTPS request
+	http.DefaultTransport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
+	_, err = http.Get("https://www.test2.com:9443")
+	assert.NotNil(t, err)
+	assert.EqualError(t, err, "Get \"https://www.test2.com:9443\": remote error: tls: internal error")
+
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "create ssl fail - key and cert not match",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodPost,
+			Path:         "/apisix/admin/ssl",
+			Body:         string(invalidBody),
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusBadRequest,
+		},
+		{
+			caseDesc:     "create ssl successfully",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodPost,
+			Path:         "/apisix/admin/ssl",
+			Body:         string(body),
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc: "create route",
+			Object:   MangerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/routes/r1",
+			Body: `{
+				"uri": "/hello_",
+				"hosts": ["test2.com", "*.test2.com"],
+				"upstream": {
+					"nodes": {
+						"172.16.238.20:1980": 1
+					},
+					"type": "roundrobin"
+				}
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc:     "hit the route just created using HTTPS",
+			Object:       APISIXHTTPSExpect(t),
+			Method:       http.MethodGet,
+			Path:         "/hello_",
+			Headers:      map[string]string{"Host": "www.test2.com"},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "hello world\n",
+			Sleep:        sleepTime,
+		},
+		{
+			caseDesc:     "delete route",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodDelete,
+			Path:         "/apisix/admin/routes/r1",
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc:     "delete ssl",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodDelete,
+			Path:         "/apisix/admin/ssl/1",
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,

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.

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



[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #719: fix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519247125



##########
File path: .github/workflows/e2e-test-backend.yml
##########
@@ -28,7 +28,10 @@ jobs:
           sleep 5
           docker logs docker_managerapi_1
 
+      - name: set hosts
+        working-directory: ./api/test/e2e
+        run: echo "127.0.0.1 www.test2.com" | sudo tee -a /etc/hosts

Review comment:
       That's because we can't make https request like this:
   
   ```
   curl https://127.0.0.1:9443  -H 'Host: www.test2.com'
   ```
   
   the SSL of www.test2.com will not match by this way. so need to set host and request www.test2.com directly.
   
   




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] codecov-io commented on pull request #719: fix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#issuecomment-723461416


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=h1) Report
   > Merging [#719](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=desc) (8932536) into [v2.0](https://codecov.io/gh/apache/apisix-dashboard/commit/3a405f779c0d65ca68e52a9745662aca4272ecc1?el=desc) (3a405f7) will **increase** coverage by `0.17%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/719/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##             v2.0     #719      +/-   ##
   ==========================================
   + Coverage   43.08%   43.26%   +0.17%     
   ==========================================
     Files          18       18              
     Lines        1237     1239       +2     
   ==========================================
   + Hits          533      536       +3     
   + Misses        615      614       -1     
     Partials       89       89              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/handler/ssl/ssl.go](https://codecov.io/gh/apache/apisix-dashboard/pull/719/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc3NsL3NzbC5nbw==) | `28.17% <100.00%> (+0.80%)` | :arrow_up: |
   | [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/719/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.94% <0.00%> (+0.65%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=footer). Last update [3a405f7...8932536](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #719: bugfix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519527833



##########
File path: api/test/e2e/ssl_test.go
##########
@@ -0,0 +1,138 @@
+/*
+ * 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 e2e
+
+import (
+	"context"
+	"crypto/tls"
+	"encoding/json"
+	"io/ioutil"
+	"net"
+	"net/http"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSSL_Basic(t *testing.T) {
+	testCert, err := ioutil.ReadFile("../certs/test2.crt")
+	assert.Nil(t, err)
+	testKey, err := ioutil.ReadFile("../certs/test2.key")
+	assert.Nil(t, err)
+	apisixKey, err := ioutil.ReadFile("../certs/apisix.key")
+	assert.Nil(t, err)
+	body, err := json.Marshal(map[string]string{
+		"id":   "1",
+		"cert": string(testCert),
+		"key":  string(testKey),
+	})
+	assert.Nil(t, err)
+	invalidBody, err := json.Marshal(map[string]string{
+		"id":   "1",
+		"cert": string(testCert),
+		"key":  string(apisixKey),
+	})
+
+	//Before configuring SSL, make a HTTPS request
+	http.DefaultTransport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
+	http.DefaultTransport.(*http.Transport).DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
+		if addr == "www.test2.com:9443" {
+			addr = "127.0.0.1:9443"
+		}
+		dialer := &net.Dialer{}
+		return dialer.DialContext(ctx, network, addr)
+	}
+	_, err = http.Get("https://www.test2.com:9443")
+	assert.NotNil(t, err)
+	assert.EqualError(t, err, "Get https://www.test2.com:9443: remote error: tls: internal error")
+
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "create ssl fail - key and cert not match",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodPost,
+			Path:         "/apisix/admin/ssl",
+			Body:         string(invalidBody),
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusBadRequest,
+		},
+		{
+			caseDesc:     "create ssl successfully",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodPost,
+			Path:         "/apisix/admin/ssl",
+			Body:         string(body),
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc: "create route",
+			Object:   MangerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/routes/r1",
+			Body: `{
+				"uri": "/hello_",
+				"hosts": ["test2.com", "*.test2.com"],
+				"upstream": {
+					"nodes": {
+						"172.16.238.20:1980": 1
+					},
+					"type": "roundrobin"
+				}
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc:     "hit the route just created using HTTPS",
+			Object:       APISIXHTTPSExpect(t),
+			Method:       http.MethodGet,
+			Path:         "/hello_",
+			Headers:      map[string]string{"Host": "www.test2.com"},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "hello world\n",
+			Sleep:        sleepTime,
+		},
+		{
+			caseDesc:     "delete route",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodDelete,
+			Path:         "/apisix/admin/routes/r1",
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc:     "delete ssl",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodDelete,
+			Path:         "/apisix/admin/ssl/1",
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+
+	//try again after deleting SSL, make a HTTPS request
+	time.Sleep(time.Duration(20) * time.Millisecond)
+	_, err = http.Get("https://www.test2.com:9443")
+	assert.NotNil(t, err)
+	assert.EqualError(t, err, "Get https://www.test2.com:9443: remote error: tls: internal error")

Review comment:
       updated.




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] moonming commented on a change in pull request #719: fix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519237883



##########
File path: api/internal/handler/ssl/ssl.go
##########
@@ -147,6 +147,7 @@ func (h *Handler) Create(c droplet.Context) (interface{}, error) {
 	}
 
 	ssl.ID = input.ID
+	ssl.Status = 1

Review comment:
       Is this the cause of the bug?  Can you explain the reason?

##########
File path: .github/workflows/e2e-test-backend.yml
##########
@@ -28,7 +28,10 @@ jobs:
           sleep 5
           docker logs docker_managerapi_1
 
+      - name: set hosts
+        working-directory: ./api/test/e2e
+        run: echo "127.0.0.1 www.test2.com" | sudo tee -a /etc/hosts

Review comment:
       why need to change local file?




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] membphis commented on a change in pull request #719: fix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519249743



##########
File path: api/internal/handler/ssl/ssl.go
##########
@@ -147,6 +147,7 @@ func (h *Handler) Create(c droplet.Context) (interface{}, error) {
 	}
 
 	ssl.ID = input.ID
+	ssl.Status = 1

Review comment:
       that is the magic number. we should use a `const` variable for a better name.




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #719: fix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519193131



##########
File path: api/test/e2e/ssl_test.go
##########
@@ -0,0 +1,115 @@
+/*
+ * 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 e2e
+
+import (
+	"encoding/json"
+	"github.com/stretchr/testify/assert"
+	"io/ioutil"
+	"net/http"
+	"testing"
+)
+
+func TestSSL_Basic(t *testing.T) {
+	testCert, err := ioutil.ReadFile("../certs/test2.crt")
+	assert.Nil(t, err)
+	testKey, err := ioutil.ReadFile("../certs/test2.key")
+	assert.Nil(t, err)
+	apisixKey, err := ioutil.ReadFile("../certs/apisix.key")
+	assert.Nil(t, err)
+	body, err := json.Marshal(map[string]string{
+		"id":   "1",
+		"cert": string(testCert),
+		"key":  string(testKey),
+	})
+	assert.Nil(t, err)
+	invalidBody, err := json.Marshal(map[string]string{
+		"id":   "1",
+		"cert": string(testCert),
+		"key":  string(apisixKey),
+	})
+
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "create ssl fail - key and cert not match",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodPost,
+			Path:         "/apisix/admin/ssl",
+			Body:         string(invalidBody),
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusBadRequest,
+		},
+		{
+			caseDesc:     "create ssl successful",

Review comment:
       `successfully`, *adv*




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] membphis commented on a change in pull request #719: fix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519250057



##########
File path: api/internal/handler/ssl/ssl.go
##########
@@ -147,6 +147,7 @@ func (h *Handler) Create(c droplet.Context) (interface{}, error) {
 	}
 
 	ssl.ID = input.ID
+	ssl.Status = 1

Review comment:
       and I think we'd better add some comment here, it should be useful for code reader




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] moonming commented on a change in pull request #719: bugfix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519410091



##########
File path: api/test/e2e/ssl_test.go
##########
@@ -0,0 +1,138 @@
+/*
+ * 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 e2e
+
+import (
+	"context"
+	"crypto/tls"
+	"encoding/json"
+	"io/ioutil"
+	"net"
+	"net/http"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSSL_Basic(t *testing.T) {
+	testCert, err := ioutil.ReadFile("../certs/test2.crt")
+	assert.Nil(t, err)
+	testKey, err := ioutil.ReadFile("../certs/test2.key")
+	assert.Nil(t, err)
+	apisixKey, err := ioutil.ReadFile("../certs/apisix.key")
+	assert.Nil(t, err)
+	body, err := json.Marshal(map[string]string{
+		"id":   "1",
+		"cert": string(testCert),
+		"key":  string(testKey),
+	})
+	assert.Nil(t, err)
+	invalidBody, err := json.Marshal(map[string]string{
+		"id":   "1",
+		"cert": string(testCert),
+		"key":  string(apisixKey),
+	})
+
+	//Before configuring SSL, make a HTTPS request
+	http.DefaultTransport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
+	http.DefaultTransport.(*http.Transport).DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
+		if addr == "www.test2.com:9443" {
+			addr = "127.0.0.1:9443"
+		}
+		dialer := &net.Dialer{}
+		return dialer.DialContext(ctx, network, addr)
+	}
+	_, err = http.Get("https://www.test2.com:9443")
+	assert.NotNil(t, err)
+	assert.EqualError(t, err, "Get https://www.test2.com:9443: remote error: tls: internal error")
+
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "create ssl fail - key and cert not match",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodPost,
+			Path:         "/apisix/admin/ssl",
+			Body:         string(invalidBody),
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusBadRequest,
+		},
+		{
+			caseDesc:     "create ssl successfully",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodPost,
+			Path:         "/apisix/admin/ssl",
+			Body:         string(body),
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc: "create route",
+			Object:   MangerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/routes/r1",
+			Body: `{
+				"uri": "/hello_",
+				"hosts": ["test2.com", "*.test2.com"],
+				"upstream": {
+					"nodes": {
+						"172.16.238.20:1980": 1
+					},
+					"type": "roundrobin"
+				}
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc:     "hit the route just created using HTTPS",
+			Object:       APISIXHTTPSExpect(t),
+			Method:       http.MethodGet,
+			Path:         "/hello_",
+			Headers:      map[string]string{"Host": "www.test2.com"},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "hello world\n",
+			Sleep:        sleepTime,
+		},
+		{
+			caseDesc:     "delete route",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodDelete,
+			Path:         "/apisix/admin/routes/r1",
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc:     "delete ssl",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodDelete,
+			Path:         "/apisix/admin/ssl/1",
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+
+	//try again after deleting SSL, make a HTTPS request
+	time.Sleep(time.Duration(20) * time.Millisecond)
+	_, err = http.Get("https://www.test2.com:9443")
+	assert.NotNil(t, err)
+	assert.EqualError(t, err, "Get https://www.test2.com:9443: remote error: tls: internal error")

Review comment:
       Then why you need to delete router?https://github.com/apache/apisix-dashboard/pull/719/files#diff-c2b7edfd95881ffeb7a02870b56d8c228b7b1f0f83d78be56f4462091bf76dc7R112-R117
    just delete ssl is enough




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] juzhiyuan merged pull request #719: bugfix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
juzhiyuan merged pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719


   


----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #719: fix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#issuecomment-723461416


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=h1) Report
   > Merging [#719](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=desc) (9abb403) into [v2.0](https://codecov.io/gh/apache/apisix-dashboard/commit/3a405f779c0d65ca68e52a9745662aca4272ecc1?el=desc) (3a405f7) will **increase** coverage by `0.09%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/719/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##             v2.0     #719      +/-   ##
   ==========================================
   + Coverage   43.08%   43.17%   +0.09%     
   ==========================================
     Files          18       18              
     Lines        1237     1239       +2     
   ==========================================
   + Hits          533      535       +2     
     Misses        615      615              
     Partials       89       89              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/handler/ssl/ssl.go](https://codecov.io/gh/apache/apisix-dashboard/pull/719/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc3NsL3NzbC5nbw==) | `28.17% <100.00%> (+0.80%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=footer). Last update [3a405f7...9abb403](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] membphis commented on a change in pull request #719: bugfix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519555414



##########
File path: api/conf/conf.go
##########
@@ -40,16 +40,17 @@ const (
 )
 
 var (
-	ENV           string
-	Schema        gjson.Result
-	WorkDir       = "."
-	ServerHost    = "127.0.0.1"
-	ServerPort    = 80
-	ETCDEndpoints = []string{"127.0.0.1:2379"}
-	ErrorLogLevel = "warn"
-	ErrorLogPath  = "logs/error.log"
-	UserList      = make(map[string]User, 2)
-	AuthConf      Authentication
+	ENV              string
+	Schema           gjson.Result
+	WorkDir          = "."
+	ServerHost       = "127.0.0.1"
+	ServerPort       = 80
+	ETCDEndpoints    = []string{"127.0.0.1:2379"}
+	ErrorLogLevel    = "warn"
+	ErrorLogPath     = "logs/error.log"
+	UserList         = make(map[string]User, 2)
+	AuthConf         Authentication
+	SSLDefaultStatus = 1 //enable ssl by default

Review comment:
       `CONST` variable? avoid magic number

##########
File path: api/internal/handler/ssl/ssl.go
##########
@@ -147,6 +148,8 @@ func (h *Handler) Create(c droplet.Context) (interface{}, error) {
 	}
 
 	ssl.ID = input.ID
+	//set default value for SSL status, if not set, it will be 0 which means disable.
+	ssl.Status = conf.SSLDefaultStatus
 	if err := h.sslStore.Create(c.Context(), ssl); err != nil {
 		return handler.SpecCodeResponse(err), err

Review comment:
       if you created, you should better leave an issue id here.




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #719: bugfix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519399075



##########
File path: api/internal/handler/ssl/ssl.go
##########
@@ -147,6 +148,8 @@ func (h *Handler) Create(c droplet.Context) (interface{}, error) {
 	}
 
 	ssl.ID = input.ID
+	//set default value for SSL status, if not set, it will be 0 which means disable.
+	ssl.Status = conf.SSLDefaultStatus
 	if err := h.sslStore.Create(c.Context(), ssl); err != nil {
 		return handler.SpecCodeResponse(err), err

Review comment:
       sure, I will create an issue.




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #719: bugfix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#issuecomment-723461416


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=h1) Report
   > Merging [#719](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=desc) (67dfb1b) into [v2.0](https://codecov.io/gh/apache/apisix-dashboard/commit/3a405f779c0d65ca68e52a9745662aca4272ecc1?el=desc) (3a405f7) will **increase** coverage by `0.17%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/719/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##             v2.0     #719      +/-   ##
   ==========================================
   + Coverage   43.08%   43.26%   +0.17%     
   ==========================================
     Files          18       18              
     Lines        1237     1239       +2     
   ==========================================
   + Hits          533      536       +3     
   + Misses        615      614       -1     
     Partials       89       89              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/handler/ssl/ssl.go](https://codecov.io/gh/apache/apisix-dashboard/pull/719/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc3NsL3NzbC5nbw==) | `28.17% <100.00%> (+0.80%)` | :arrow_up: |
   | [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/719/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.94% <0.00%> (+0.65%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=footer). Last update [3a405f7...67dfb1b](https://codecov.io/gh/apache/apisix-dashboard/pull/719?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] moonming commented on a change in pull request #719: bugfix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519499827



##########
File path: api/test/e2e/ssl_test.go
##########
@@ -0,0 +1,138 @@
+/*
+ * 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 e2e
+
+import (
+	"context"
+	"crypto/tls"
+	"encoding/json"
+	"io/ioutil"
+	"net"
+	"net/http"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSSL_Basic(t *testing.T) {
+	testCert, err := ioutil.ReadFile("../certs/test2.crt")
+	assert.Nil(t, err)
+	testKey, err := ioutil.ReadFile("../certs/test2.key")
+	assert.Nil(t, err)
+	apisixKey, err := ioutil.ReadFile("../certs/apisix.key")
+	assert.Nil(t, err)
+	body, err := json.Marshal(map[string]string{
+		"id":   "1",
+		"cert": string(testCert),
+		"key":  string(testKey),
+	})
+	assert.Nil(t, err)
+	invalidBody, err := json.Marshal(map[string]string{
+		"id":   "1",
+		"cert": string(testCert),
+		"key":  string(apisixKey),
+	})
+
+	//Before configuring SSL, make a HTTPS request
+	http.DefaultTransport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
+	http.DefaultTransport.(*http.Transport).DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
+		if addr == "www.test2.com:9443" {
+			addr = "127.0.0.1:9443"
+		}
+		dialer := &net.Dialer{}
+		return dialer.DialContext(ctx, network, addr)
+	}
+	_, err = http.Get("https://www.test2.com:9443")
+	assert.NotNil(t, err)
+	assert.EqualError(t, err, "Get https://www.test2.com:9443: remote error: tls: internal error")
+
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "create ssl fail - key and cert not match",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodPost,
+			Path:         "/apisix/admin/ssl",
+			Body:         string(invalidBody),
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusBadRequest,
+		},
+		{
+			caseDesc:     "create ssl successfully",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodPost,
+			Path:         "/apisix/admin/ssl",
+			Body:         string(body),
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc: "create route",
+			Object:   MangerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/routes/r1",
+			Body: `{
+				"uri": "/hello_",
+				"hosts": ["test2.com", "*.test2.com"],
+				"upstream": {
+					"nodes": {
+						"172.16.238.20:1980": 1
+					},
+					"type": "roundrobin"
+				}
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc:     "hit the route just created using HTTPS",
+			Object:       APISIXHTTPSExpect(t),
+			Method:       http.MethodGet,
+			Path:         "/hello_",
+			Headers:      map[string]string{"Host": "www.test2.com"},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "hello world\n",
+			Sleep:        sleepTime,
+		},
+		{
+			caseDesc:     "delete route",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodDelete,
+			Path:         "/apisix/admin/routes/r1",
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc:     "delete ssl",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodDelete,
+			Path:         "/apisix/admin/ssl/1",
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+
+	//try again after deleting SSL, make a HTTPS request
+	time.Sleep(time.Duration(20) * time.Millisecond)
+	_, err = http.Get("https://www.test2.com:9443")
+	assert.NotNil(t, err)
+	assert.EqualError(t, err, "Get https://www.test2.com:9443: remote error: tls: internal error")

Review comment:
       Please move delete router to the end, and add comments




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #719: bugfix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519411808



##########
File path: api/test/e2e/ssl_test.go
##########
@@ -0,0 +1,138 @@
+/*
+ * 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 e2e
+
+import (
+	"context"
+	"crypto/tls"
+	"encoding/json"
+	"io/ioutil"
+	"net"
+	"net/http"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSSL_Basic(t *testing.T) {
+	testCert, err := ioutil.ReadFile("../certs/test2.crt")
+	assert.Nil(t, err)
+	testKey, err := ioutil.ReadFile("../certs/test2.key")
+	assert.Nil(t, err)
+	apisixKey, err := ioutil.ReadFile("../certs/apisix.key")
+	assert.Nil(t, err)
+	body, err := json.Marshal(map[string]string{
+		"id":   "1",
+		"cert": string(testCert),
+		"key":  string(testKey),
+	})
+	assert.Nil(t, err)
+	invalidBody, err := json.Marshal(map[string]string{
+		"id":   "1",
+		"cert": string(testCert),
+		"key":  string(apisixKey),
+	})
+
+	//Before configuring SSL, make a HTTPS request
+	http.DefaultTransport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
+	http.DefaultTransport.(*http.Transport).DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
+		if addr == "www.test2.com:9443" {
+			addr = "127.0.0.1:9443"
+		}
+		dialer := &net.Dialer{}
+		return dialer.DialContext(ctx, network, addr)
+	}
+	_, err = http.Get("https://www.test2.com:9443")
+	assert.NotNil(t, err)
+	assert.EqualError(t, err, "Get https://www.test2.com:9443: remote error: tls: internal error")
+
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "create ssl fail - key and cert not match",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodPost,
+			Path:         "/apisix/admin/ssl",
+			Body:         string(invalidBody),
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusBadRequest,
+		},
+		{
+			caseDesc:     "create ssl successfully",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodPost,
+			Path:         "/apisix/admin/ssl",
+			Body:         string(body),
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc: "create route",
+			Object:   MangerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/routes/r1",
+			Body: `{
+				"uri": "/hello_",
+				"hosts": ["test2.com", "*.test2.com"],
+				"upstream": {
+					"nodes": {
+						"172.16.238.20:1980": 1
+					},
+					"type": "roundrobin"
+				}
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc:     "hit the route just created using HTTPS",
+			Object:       APISIXHTTPSExpect(t),
+			Method:       http.MethodGet,
+			Path:         "/hello_",
+			Headers:      map[string]string{"Host": "www.test2.com"},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "hello world\n",
+			Sleep:        sleepTime,
+		},
+		{
+			caseDesc:     "delete route",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodDelete,
+			Path:         "/apisix/admin/routes/r1",
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc:     "delete ssl",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodDelete,
+			Path:         "/apisix/admin/ssl/1",
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+
+	//try again after deleting SSL, make a HTTPS request
+	time.Sleep(time.Duration(20) * time.Millisecond)
+	_, err = http.Get("https://www.test2.com:9443")
+	assert.NotNil(t, err)
+	assert.EqualError(t, err, "Get https://www.test2.com:9443: remote error: tls: internal error")

Review comment:
       just clean test data.




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #719: bugfix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519398163



##########
File path: api/internal/handler/ssl/ssl.go
##########
@@ -34,6 +34,7 @@ import (
 	"github.com/shiningrush/droplet/wrapper"
 	wgin "github.com/shiningrush/droplet/wrapper/gin"
 
+	"github.com/apisix/manager-api/conf"

Review comment:
       yes. when we adjust the code dir.




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] membphis commented on a change in pull request #719: fix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519249979



##########
File path: .github/workflows/e2e-test-backend.yml
##########
@@ -28,7 +28,10 @@ jobs:
           sleep 5
           docker logs docker_managerapi_1
 
+      - name: set hosts
+        working-directory: ./api/test/e2e
+        run: echo "127.0.0.1 www.test2.com" | sudo tee -a /etc/hosts

Review comment:
       `curl --resolve 'www.test2.com:9443:127.0.0.1'  https://www.test2.com:9443`
   
   you can make a try in this way.  ^_^




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #719: fix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519262363



##########
File path: api/internal/handler/ssl/ssl.go
##########
@@ -147,6 +147,7 @@ func (h *Handler) Create(c droplet.Context) (interface{}, error) {
 	}
 
 	ssl.ID = input.ID
+	ssl.Status = 1

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.

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



[GitHub] [apisix-dashboard] moonming commented on a change in pull request #719: bugfix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519309618



##########
File path: api/test/e2e/ssl_test.go
##########
@@ -0,0 +1,138 @@
+/*
+ * 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 e2e
+
+import (
+	"context"
+	"crypto/tls"
+	"encoding/json"
+	"io/ioutil"
+	"net"
+	"net/http"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSSL_Basic(t *testing.T) {
+	testCert, err := ioutil.ReadFile("../certs/test2.crt")
+	assert.Nil(t, err)
+	testKey, err := ioutil.ReadFile("../certs/test2.key")
+	assert.Nil(t, err)
+	apisixKey, err := ioutil.ReadFile("../certs/apisix.key")
+	assert.Nil(t, err)
+	body, err := json.Marshal(map[string]string{
+		"id":   "1",
+		"cert": string(testCert),
+		"key":  string(testKey),
+	})
+	assert.Nil(t, err)
+	invalidBody, err := json.Marshal(map[string]string{
+		"id":   "1",
+		"cert": string(testCert),
+		"key":  string(apisixKey),
+	})
+
+	//Before configuring SSL, make a HTTPS request
+	http.DefaultTransport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
+	http.DefaultTransport.(*http.Transport).DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
+		if addr == "www.test2.com:9443" {
+			addr = "127.0.0.1:9443"
+		}
+		dialer := &net.Dialer{}
+		return dialer.DialContext(ctx, network, addr)
+	}
+	_, err = http.Get("https://www.test2.com:9443")
+	assert.NotNil(t, err)
+	assert.EqualError(t, err, "Get https://www.test2.com:9443: remote error: tls: internal error")
+
+	tests := []HttpTestCase{
+		{
+			caseDesc:     "create ssl fail - key and cert not match",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodPost,
+			Path:         "/apisix/admin/ssl",
+			Body:         string(invalidBody),
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusBadRequest,
+		},
+		{
+			caseDesc:     "create ssl successfully",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodPost,
+			Path:         "/apisix/admin/ssl",
+			Body:         string(body),
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc: "create route",
+			Object:   MangerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/routes/r1",
+			Body: `{
+				"uri": "/hello_",
+				"hosts": ["test2.com", "*.test2.com"],
+				"upstream": {
+					"nodes": {
+						"172.16.238.20:1980": 1
+					},
+					"type": "roundrobin"
+				}
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc:     "hit the route just created using HTTPS",
+			Object:       APISIXHTTPSExpect(t),
+			Method:       http.MethodGet,
+			Path:         "/hello_",
+			Headers:      map[string]string{"Host": "www.test2.com"},
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "hello world\n",
+			Sleep:        sleepTime,
+		},
+		{
+			caseDesc:     "delete route",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodDelete,
+			Path:         "/apisix/admin/routes/r1",
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc:     "delete ssl",
+			Object:       MangerApiExpect(t),
+			Method:       http.MethodDelete,
+			Path:         "/apisix/admin/ssl/1",
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+
+	//try again after deleting SSL, make a HTTPS request
+	time.Sleep(time.Duration(20) * time.Millisecond)
+	_, err = http.Get("https://www.test2.com:9443")
+	assert.NotNil(t, err)
+	assert.EqualError(t, err, "Get https://www.test2.com:9443: remote error: tls: internal error")

Review comment:
       why return ssl error? The router has been deleted




----------------------------------------------------------------
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.

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



[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #719: fix: dashboard 2.0 failed to fetch ssl certificate not found

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #719:
URL: https://github.com/apache/apisix-dashboard/pull/719#discussion_r519262495



##########
File path: .github/workflows/e2e-test-backend.yml
##########
@@ -28,7 +28,10 @@ jobs:
           sleep 5
           docker logs docker_managerapi_1
 
+      - name: set hosts
+        working-directory: ./api/test/e2e
+        run: echo "127.0.0.1 www.test2.com" | sudo tee -a /etc/hosts

Review comment:
       thanks. I will try for it. but it's an example, we are not use curl to test.




----------------------------------------------------------------
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.

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