You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "hulining (via GitHub)" <gi...@apache.org> on 2023/05/28 03:12:01 UTC

[GitHub] [apisix] hulining opened a new pull request, #9563: feat: add token support for consul_kv discovery(#9532)

hulining opened a new pull request, #9563:
URL: https://github.com/apache/apisix/pull/9563

   ### Description
   This PR supports the consul_kv service discovery support token function described in #9532.
   
   ### Checklist
   
   - [x] I have explained the need for this PR and the problem it solves
   - [x] I have explained the changes or the new features added to this PR
   - [ ] I have added tests corresponding to this change
   For testing, I don't know how to add test cases yet, but I tested locally that this function can be used normally.
   - [x] I have updated the documentation to reflect this change
   - [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
   
   <!--
   
   Note
   
   1. Mark the PR as draft until it's ready to be reviewed.
   2. Always add/update tests for any changes unless you have a good reason.
   3. Always update the documentation to reflect the changes made in the PR.
   4. Make a new commit to resolve conversations instead of `push -f`.
   5. To resolve merge conflicts, merge master instead of rebasing.
   6. Use "request review" to notify the reviewer after making changes.
   7. Only a reviewer can mark a conversation as resolved.
   
   -->
   


-- 
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] monkeyDluffy6017 commented on pull request #9563: feat: add token support for consul_kv discovery(#9532)

Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on PR #9563:
URL: https://github.com/apache/apisix/pull/9563#issuecomment-1732859975

   @leslie-tsang please help to finish this pr


-- 
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] sevensolutions commented on pull request #9563: feat: add token support for consul_kv discovery(#9532)

Posted by "sevensolutions (via GitHub)" <gi...@apache.org>.
sevensolutions commented on PR #9563:
URL: https://github.com/apache/apisix/pull/9563#issuecomment-1734362911

   I think i'am almost there. What i've done:
   1. Added a new consul service to *ci/pod/docker-compose.first.yml* which enables ACL
   ```
   consul_3:
       image: consul:1.15.4
       restart: unless-stopped
       ports:
         - "8502:8500"
       command: [ "consul", "agent", "-server", "-bootstrap-expect=1", "-client", "0.0.0.0", "-log-level", "info", "-data-dir=/consul/data", "-enable-script-checks", "-ui", "-hcl", "acl = {\nenabled = true\ndefault_policy = \"deny\"\nenable_token_persistence = true\n}" ]
       networks:
         consul_net:
   ```
   2. Run `make ci-env-up project_compose_ci=ci/pod/docker-compose.first.yml`
   3. Added a new test in *t/discovery/consul.t* to bootstrap the ACL system
   ```
   === TEST 14: bootstrap acl
   --- config
   location /v1/acl {
       proxy_pass http://127.0.0.1:8502;
   }
   --- request eval
   [
       "PUT /v1/acl/bootstrap\n" . "{\"BootstrapSecret\": \"2b778dd9-f5f1-6f29-b4b4-9a5fa948757a\"}"
   ]
   --- error_code eval
   [200]
   ```
   3. Then i've just copied one of the other tests.
   4. Executed tests with `TEST_NGINX_BINARY=/usr/bin/openresty prove -It/test-nginx/lib -I./ -r t/discovery/consul.t`
   
   Is this the correct way to go?


-- 
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 #9563: feat: add token support for consul_kv discovery(#9532)

Posted by "leslie-tsang (via GitHub)" <gi...@apache.org>.
leslie-tsang commented on PR #9563:
URL: https://github.com/apache/apisix/pull/9563#issuecomment-1565970867

   Hello there, we need a test case for this feature. :)


-- 
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] sevensolutions commented on pull request #9563: feat: add token support for consul_kv discovery(#9532)

Posted by "sevensolutions (via GitHub)" <gi...@apache.org>.
sevensolutions commented on PR #9563:
URL: https://github.com/apache/apisix/pull/9563#issuecomment-1728422786

   I'am really waiting for this feature.
   Where can we find the existing tests for the consul DNS provider, so i check what we need to add?


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


Re: [PR] feat: add token support for consul_kv discovery(#9532) [apisix]

Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on PR #9563:
URL: https://github.com/apache/apisix/pull/9563#issuecomment-1754226076

   Thanks for your contribution! I'will close this pr, it's done by https://github.com/apache/apisix/pull/10278


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


Re: [PR] feat: add token support for consul_kv discovery(#9532) [apisix]

Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 closed pull request #9563: feat: add token support for consul_kv discovery(#9532)
URL: https://github.com/apache/apisix/pull/9563


-- 
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] monkeyDluffy6017 commented on pull request #9563: feat: add token support for consul_kv discovery(#9532)

Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on PR #9563:
URL: https://github.com/apache/apisix/pull/9563#issuecomment-1734689945

   > I think i'am almost there. What i've done:
   > 
   > 1. Added a new consul service to _ci/pod/docker-compose.first.yml_ which enables ACL
   > 
   > ```
   > consul_3:
   >     image: consul:1.15.4
   >     restart: unless-stopped
   >     ports:
   >       - "8502:8500"
   >     command: [ "consul", "agent", "-server", "-bootstrap-expect=1", "-client", "0.0.0.0", "-log-level", "info", "-data-dir=/consul/data", "-enable-script-checks", "-ui", "-hcl", "acl = {\nenabled = true\ndefault_policy = \"deny\"\nenable_token_persistence = true\n}" ]
   >     networks:
   >       consul_net:
   > ```
   > 
   > 2. Run `make ci-env-up project_compose_ci=ci/pod/docker-compose.first.yml`
   > 3. Added a new test in _t/discovery/consul.t_ to bootstrap the ACL system
   > 
   > ```
   > === TEST 14: bootstrap acl
   > --- config
   > location /v1/acl {
   >     proxy_pass http://127.0.0.1:8502;
   > }
   > --- request eval
   > [
   >     "PUT /v1/acl/bootstrap\n" . "{\"BootstrapSecret\": \"2b778dd9-f5f1-6f29-b4b4-9a5fa948757a\"}"
   > ]
   > --- error_code eval
   > [200]
   > ```
   > 
   > 3. Then i've just copied one of the other tests.
   > 4. Executed tests with `TEST_NGINX_BINARY=/usr/bin/openresty prove -It/test-nginx/lib -I./ -r t/discovery/consul.t`
   > 
   > Is this the correct way to go?
   
   Yeah, it's exactly what you said


-- 
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] sevensolutions commented on pull request #9563: feat: add token support for consul_kv discovery(#9532)

Posted by "sevensolutions (via GitHub)" <gi...@apache.org>.
sevensolutions commented on PR #9563:
URL: https://github.com/apache/apisix/pull/9563#issuecomment-1736104565

   @monkeyDluffy6017 thanks.
   
   How can i get some logs from APISIX during development?
   I'am getting a *500 Internal Server Error* but i have no idea why.


-- 
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] sevensolutions commented on pull request #9563: feat: add token support for consul_kv discovery(#9532)

Posted by "sevensolutions (via GitHub)" <gi...@apache.org>.
sevensolutions commented on PR #9563:
URL: https://github.com/apache/apisix/pull/9563#issuecomment-1739904363

   Got it working. I've created #10278 for token support in consul including tests.


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