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