You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/03/03 05:19:35 UTC
[GitHub] [apisix] soulbird opened a new pull request #6498: docs: fix python script bug
soulbird opened a new pull request #6498:
URL: https://github.com/apache/apisix/pull/6498
When there are only four parameters, the script should also work properly
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] leslie-tsang commented on pull request #6498: docs: fix python script bug
Posted by GitBox <gi...@apache.org>.
leslie-tsang commented on pull request #6498:
URL: https://github.com/apache/apisix/pull/6498#issuecomment-1057796487
Ohh, My mistake, we should add a blank line here like below:
```python
if len(sys.argv) >= 6:
reqParam["client"]["depth"] = int(sys.argv[5])
resp = requests.put("http://127.0.0.1:9080/apisix/admin/ssl/1", json=reqParam, headers={
"X-API-KEY": api_key,
```
Thanks for your patience in replying. :)
LGTM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] leslie-tsang commented on pull request #6498: docs: fix python script bug
Posted by GitBox <gi...@apache.org>.
leslie-tsang commented on pull request #6498:
URL: https://github.com/apache/apisix/pull/6498#issuecomment-1057742537
> >
>
> > Hello there, I'm afraid this change will not have the desired effect.
> > BTW, Could you desc the PR more clearly ? :)
>
> As the example, we can run this python script like:
>
> ```shell
> ./patch_upstream_mtls.py testmtls ./client.pem ./client.key
> ```
>
> it has four parameters, we expect it to work, but we will got 'bad argument'.
```python
if len(sys.argv) >= 5:
```
Can you confirm that this [conditional judgment](https://github.com/apache/apisix/blob/ce8227aaf7e4f0b8aab6274c38deda6e7c243c9d/docs/en/latest/mtls.md?plain=1#L121) continues to obstruct the request's execution?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] bisakhmondal commented on a change in pull request #6498: docs: fix python script bug
Posted by GitBox <gi...@apache.org>.
bisakhmondal commented on a change in pull request #6498:
URL: https://github.com/apache/apisix/pull/6498#discussion_r820236133
##########
File path: docs/en/latest/mtls.md
##########
@@ -103,7 +103,7 @@ import sys
# sudo pip install requests
import requests
-if len(sys.argv) <= 4:
+if len(sys.argv) < 4:
Review comment:
It's okay, I think the script is being adapted to be used in storing server certs and keys in APISIX as well, along with mTLS client ca information. It's a minor change and if it ends up helping people, I think it's okay to be merged : )
##########
File path: docs/en/latest/mtls.md
##########
@@ -166,7 +166,7 @@ import sys
# sudo pip install requests
import requests
-if len(sys.argv) <= 4:
+if len(sys.argv) < 4:
Review comment:
nice catch!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] soulbird removed a comment on pull request #6498: docs: fix python script bug
Posted by GitBox <gi...@apache.org>.
soulbird removed a comment on pull request #6498:
URL: https://github.com/apache/apisix/pull/6498#issuecomment-1057737747
As the example, we can run this python script like:
```shell
./patch_upstream_mtls.py testmtls ./client.pem ./client.key
```
it has four parameters, we expect it to work, but we will got 'bad argument'.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] soulbird commented on pull request #6498: docs: fix python script bug
Posted by GitBox <gi...@apache.org>.
soulbird commented on pull request #6498:
URL: https://github.com/apache/apisix/pull/6498#issuecomment-1057791843
>
> The change from `if len(sys.argv) <= 4` to `if len(sys.argv) < 4` in **ssl.py** don't have the desired effect.
>
> There is another [conditional judgment](https://github.com/apache/apisix/blob/ce8227aaf7e4f0b8aab6274c38deda6e7c243c9d/docs/en/latest/mtls.md?plain=1#L121) `if len(sys.argv) >= 5` will prevent the request execution.
>
> ```diff
> #!/usr/bin/env python
> # coding: utf-8
> # save this file as ssl.py
> import sys
> # sudo pip install requests
> import requests
> - if len(sys.argv) <= 4:
> + if len(sys.argv) < 4:
> print("bad argument")
> sys.exit(1)
> with open(sys.argv[1]) as f:
> cert = f.read()
> with open(sys.argv[2]) as f:
> key = f.read()
> sni = sys.argv[3]
> api_key = "edd1c9f034335f136f87ad84b625c8f1" # Change it
> reqParam = {
> "cert": cert,
> "key": key,
> "snis": [sni],
> }
> if len(sys.argv) >= 5:
> print("Setting mTLS")
> reqParam["client"] = {}
> with open(sys.argv[4]) as f:
> clientCert = f.read()
> reqParam["client"]["ca"] = clientCert
> if len(sys.argv) >= 6:
> reqParam["client"]["depth"] = int(sys.argv[5])
> resp = requests.put("http://127.0.0.1:9080/apisix/admin/ssl/1", json=reqParam, headers={
> "X-API-KEY": api_key,
> })
> print(resp.status_code)
> print(resp.text)
> ```
It's no matter, it will work well with four or five or more parameters
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] lingsamuel commented on a change in pull request #6498: docs: fix python script bug
Posted by GitBox <gi...@apache.org>.
lingsamuel commented on a change in pull request #6498:
URL: https://github.com/apache/apisix/pull/6498#discussion_r820226148
##########
File path: docs/en/latest/mtls.md
##########
@@ -103,7 +103,7 @@ import sys
# sudo pip install requests
import requests
-if len(sys.argv) <= 4:
+if len(sys.argv) < 4:
Review comment:
I am afraid this is necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] soulbird commented on pull request #6498: docs: fix python script bug
Posted by GitBox <gi...@apache.org>.
soulbird commented on pull request #6498:
URL: https://github.com/apache/apisix/pull/6498#issuecomment-1057738137
>
> Hello there, I'm afraid this change will not have the desired effect.
>
> BTW, Could you desc the PR more clearly ? :)
As the example, we can run this python script like:
```shell
./patch_upstream_mtls.py testmtls ./client.pem ./client.key
```
it has four parameters, we expect it to work, but we will got 'bad argument'.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] soulbird commented on pull request #6498: docs: fix python script bug
Posted by GitBox <gi...@apache.org>.
soulbird commented on pull request #6498:
URL: https://github.com/apache/apisix/pull/6498#issuecomment-1057737747
As the example, we can run this python script like:
```shell
./patch_upstream_mtls.py testmtls ./client.pem ./client.key
```
it has four parameters, we expect it to work, but we will got 'bad argument'.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] soulbird commented on pull request #6498: docs: fix python script bug
Posted by GitBox <gi...@apache.org>.
soulbird commented on pull request #6498:
URL: https://github.com/apache/apisix/pull/6498#issuecomment-1057745835
> > >
> >
> >
> > > Hello there, I'm afraid this change will not have the desired effect.
> > > BTW, Could you desc the PR more clearly ? :)
> >
> >
> > As the example, we can run this python script like:
> > ```shell
> > ./patch_upstream_mtls.py testmtls ./client.pem ./client.key
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > it has four parameters, we expect it to work, but we will got 'bad argument'.
>
> ```python
> if len(sys.argv) >= 5:
> ```
>
> Can you confirm that this [conditional judgment](https://github.com/apache/apisix/blob/ce8227aaf7e4f0b8aab6274c38deda6e7c243c9d/docs/en/latest/mtls.md?plain=1#L121) continues to obstruct the request's execution?
Actuall, we expect it to work like:
```shell
./ssl.py ./server.pem ./server.key 'mtls.test.com'
```
or
```shell
./ssl.py ./server.pem ./server.key 'mtls.test.com' ./client_ca.pem 10
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] bzp2010 merged pull request #6498: docs: fix python script bug
Posted by GitBox <gi...@apache.org>.
bzp2010 merged pull request #6498:
URL: https://github.com/apache/apisix/pull/6498
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] leslie-tsang commented on pull request #6498: docs: fix python script bug
Posted by GitBox <gi...@apache.org>.
leslie-tsang commented on pull request #6498:
URL: https://github.com/apache/apisix/pull/6498#issuecomment-1057786886
The change from `if len(sys.argv) <= 4` to `if len(sys.argv) < 4` in **<ins>ssl.py</ins>** don't have the desired effect.
There is another [conditional judgment](https://github.com/apache/apisix/blob/ce8227aaf7e4f0b8aab6274c38deda6e7c243c9d/docs/en/latest/mtls.md?plain=1#L121) `if len(sys.argv) >= 5` will prevent the request execution.
```patch
#!/usr/bin/env python
# coding: utf-8
# save this file as ssl.py
import sys
# sudo pip install requests
import requests
- if len(sys.argv) <= 4:
+ if len(sys.argv) < 4:
print("bad argument")
sys.exit(1)
with open(sys.argv[1]) as f:
cert = f.read()
with open(sys.argv[2]) as f:
key = f.read()
sni = sys.argv[3]
api_key = "edd1c9f034335f136f87ad84b625c8f1" # Change it
reqParam = {
"cert": cert,
"key": key,
"snis": [sni],
}
if len(sys.argv) >= 5:
print("Setting mTLS")
reqParam["client"] = {}
with open(sys.argv[4]) as f:
clientCert = f.read()
reqParam["client"]["ca"] = clientCert
if len(sys.argv) >= 6:
reqParam["client"]["depth"] = int(sys.argv[5])
resp = requests.put("http://127.0.0.1:9080/apisix/admin/ssl/1", json=reqParam, headers={
"X-API-KEY": api_key,
})
print(resp.status_code)
print(resp.text)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] leslie-tsang edited a comment on pull request #6498: docs: fix python script bug
Posted by GitBox <gi...@apache.org>.
leslie-tsang edited a comment on pull request #6498:
URL: https://github.com/apache/apisix/pull/6498#issuecomment-1057733101
Hello there, I'm afraid this change will not have the desired effect.
BTW, Could you desc the PR more clearly ? :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] leslie-tsang commented on pull request #6498: docs: fix python script bug
Posted by GitBox <gi...@apache.org>.
leslie-tsang commented on pull request #6498:
URL: https://github.com/apache/apisix/pull/6498#issuecomment-1057733101
Hello there, I'm afraid this change will not have the desired effect.
BTW, Could desc the PR more clearly ? :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org