You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "An-DJ (via GitHub)" <gi...@apache.org> on 2023/04/19 16:09:09 UTC

[GitHub] [apisix] An-DJ opened a new pull request, #9341: chore(ci): remove 3rd keycloak docker image

An-DJ opened a new pull request, #9341:
URL: https://github.com/apache/apisix/pull/9341

   ### Description
   
   <!-- Please include a summary of the change and which issue is fixed. -->
   <!-- Please also include relevant motivation and context. -->
   
   ### 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
   - [X] I have added tests corresponding to this change
   - [ ] 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] An-DJ commented on a diff in pull request #9341: chore(ci): remove 3rd keycloak docker image

Posted by "An-DJ (via GitHub)" <gi...@apache.org>.
An-DJ commented on code in PR #9341:
URL: https://github.com/apache/apisix/pull/9341#discussion_r1173254421


##########
ci/pod/keycloak/kcadm_configure_university.sh:
##########
@@ -0,0 +1,90 @@
+#!/usr/bin/env bash

Review Comment:
   There are existed auth resource (or operation) create before in docker image `sshniro/keycloak-apisix:1.0.0`, which affects test cases in t/. 
   
   So we need to record these operations in a script, then repeat them in unified CI Keycloak docker service.



-- 
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] An-DJ commented on a diff in pull request #9341: chore(ci): remove 3rd keycloak docker image

Posted by "An-DJ (via GitHub)" <gi...@apache.org>.
An-DJ commented on code in PR #9341:
URL: https://github.com/apache/apisix/pull/9341#discussion_r1173254421


##########
ci/pod/keycloak/kcadm_configure_university.sh:
##########
@@ -0,0 +1,90 @@
+#!/usr/bin/env bash

Review Comment:
   There are existed auth resources (or operations) created before in docker image `sshniro/keycloak-apisix:1.0.0`, which affects test cases in t/. 
   
   So we need to record these operations in a script, then repeat them in unified CI Keycloak docker service.



-- 
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] An-DJ commented on a diff in pull request #9341: chore(ci): remove 3rd keycloak docker image

Posted by "An-DJ (via GitHub)" <gi...@apache.org>.
An-DJ commented on code in PR #9341:
URL: https://github.com/apache/apisix/pull/9341#discussion_r1173255536


##########
ci/pod/docker-compose.plugin.yml:
##########
@@ -28,42 +28,26 @@ services:
     networks:
       apisix_net:
 
-
   ## keycloak
   apisix_keycloak:
-    image: sshniro/keycloak-apisix:1.0.0
-    environment:
-      KEYCLOAK_USER: admin
-      KEYCLOAK_PASSWORD: 123456
-    restart: unless-stopped
-    ports:
-      - "8090:8080"
-      - "8443:8443"
-    networks:
-      apisix_net:
-
-  ## keycloak
-  # The keycloak official has two types of docker images:
-  # * legacy WildFly distribution
-  # * new Quarkus based distribution
-  # Here we choose new version, because it's mainstream and
-  # supports kcadm.sh to init the container for test.
-  # The original keycloak service `apisix_keycloak` is
-  # third-party personal customized image and for OIDC test only.
-  # We should unify both containers in future.
-  apisix_keycloak_new:
-    container_name: apisix_keycloak_new
+    container_name: apisix_keycloak
     image: quay.io/keycloak/keycloak:18.0.2

Review Comment:
   Email him before, but no response...



-- 
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 a diff in pull request #9341: chore(ci): remove 3rd keycloak docker image

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


##########
ci/init-plugin-test-service.sh:
##########
@@ -42,8 +42,8 @@ after() {
 
     # wait for keycloak ready
     bash -c 'while true; do curl -s localhost:8080 &>/dev/null; ret=$?; [[ $ret -eq 0 ]] && break; sleep 3; done'
-    docker cp ci/kcadm_configure_cas.sh apisix_keycloak_new:/tmp/
-    docker exec apisix_keycloak_new bash /tmp/kcadm_configure_cas.sh
+    docker cp ci/kcadm_configure_cas.sh apisix_keycloak:/tmp/

Review Comment:
   Why don't we use volume feature to mount it ?



-- 
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 a diff in pull request #9341: chore(ci): remove 3rd keycloak docker image

Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #9341:
URL: https://github.com/apache/apisix/pull/9341#discussion_r1174489024


##########
ci/pod/keycloak/kcadm_configure_university.sh:
##########
@@ -0,0 +1,90 @@
+#!/usr/bin/env bash

Review Comment:
   @moonming please help check this



-- 
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] An-DJ commented on a diff in pull request #9341: chore(ci): remove 3rd keycloak docker image

Posted by "An-DJ (via GitHub)" <gi...@apache.org>.
An-DJ commented on code in PR #9341:
URL: https://github.com/apache/apisix/pull/9341#discussion_r1173253193


##########
ci/pod/docker-compose.plugin.yml:
##########
@@ -28,42 +28,26 @@ services:
     networks:
       apisix_net:
 
-
   ## keycloak
   apisix_keycloak:
-    image: sshniro/keycloak-apisix:1.0.0
-    environment:
-      KEYCLOAK_USER: admin
-      KEYCLOAK_PASSWORD: 123456
-    restart: unless-stopped
-    ports:
-      - "8090:8080"
-      - "8443:8443"
-    networks:
-      apisix_net:
-
-  ## keycloak
-  # The keycloak official has two types of docker images:
-  # * legacy WildFly distribution
-  # * new Quarkus based distribution
-  # Here we choose new version, because it's mainstream and
-  # supports kcadm.sh to init the container for test.
-  # The original keycloak service `apisix_keycloak` is
-  # third-party personal customized image and for OIDC test only.
-  # We should unify both containers in future.
-  apisix_keycloak_new:
-    container_name: apisix_keycloak_new
+    container_name: apisix_keycloak
     image: quay.io/keycloak/keycloak:18.0.2

Review Comment:
   `18.0.2` works for now. If needed, we can upgrade the version in another PR. This PR just delete the 3rd old keycloak, not to upgrade.



-- 
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] An-DJ commented on pull request #9341: chore(ci): remove 3rd keycloak docker image

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

   cc @membphis 


-- 
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] An-DJ commented on a diff in pull request #9341: chore(ci): remove 3rd keycloak docker image

Posted by "An-DJ (via GitHub)" <gi...@apache.org>.
An-DJ commented on code in PR #9341:
URL: https://github.com/apache/apisix/pull/9341#discussion_r1173294590


##########
ci/pod/docker-compose.plugin.yml:
##########
@@ -28,42 +28,26 @@ services:
     networks:
       apisix_net:
 
-
   ## keycloak
   apisix_keycloak:
-    image: sshniro/keycloak-apisix:1.0.0
-    environment:
-      KEYCLOAK_USER: admin
-      KEYCLOAK_PASSWORD: 123456
-    restart: unless-stopped
-    ports:
-      - "8090:8080"
-      - "8443:8443"
-    networks:
-      apisix_net:
-
-  ## keycloak
-  # The keycloak official has two types of docker images:
-  # * legacy WildFly distribution
-  # * new Quarkus based distribution
-  # Here we choose new version, because it's mainstream and
-  # supports kcadm.sh to init the container for test.
-  # The original keycloak service `apisix_keycloak` is
-  # third-party personal customized image and for OIDC test only.
-  # We should unify both containers in future.
-  apisix_keycloak_new:
-    container_name: apisix_keycloak_new
+    container_name: apisix_keycloak
     image: quay.io/keycloak/keycloak:18.0.2

Review Comment:
   @sshniro Identifying and exporting these configurations one by one is too tedious... But I'm glad it's done.
   
   By the way, the `kcadm.sh` admin tool provided by Keycloak itself is really hard to be used. 😭 
   



-- 
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] An-DJ commented on a diff in pull request #9341: chore(ci): remove 3rd keycloak docker image

Posted by "An-DJ (via GitHub)" <gi...@apache.org>.
An-DJ commented on code in PR #9341:
URL: https://github.com/apache/apisix/pull/9341#discussion_r1173256141


##########
ci/init-plugin-test-service.sh:
##########
@@ -42,8 +42,8 @@ after() {
 
     # wait for keycloak ready
     bash -c 'while true; do curl -s localhost:8080 &>/dev/null; ret=$?; [[ $ret -eq 0 ]] && break; sleep 3; done'
-    docker cp ci/kcadm_configure_cas.sh apisix_keycloak_new:/tmp/
-    docker exec apisix_keycloak_new bash /tmp/kcadm_configure_cas.sh
+    docker cp ci/kcadm_configure_cas.sh apisix_keycloak:/tmp/

Review Comment:
   Changed.



-- 
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] moonming commented on a diff in pull request #9341: chore(ci): remove 3rd keycloak docker image

Posted by "moonming (via GitHub)" <gi...@apache.org>.
moonming commented on code in PR #9341:
URL: https://github.com/apache/apisix/pull/9341#discussion_r1173254936


##########
ci/pod/docker-compose.plugin.yml:
##########
@@ -28,42 +28,26 @@ services:
     networks:
       apisix_net:
 
-
   ## keycloak
   apisix_keycloak:
-    image: sshniro/keycloak-apisix:1.0.0
-    environment:
-      KEYCLOAK_USER: admin
-      KEYCLOAK_PASSWORD: 123456
-    restart: unless-stopped
-    ports:
-      - "8090:8080"
-      - "8443:8443"
-    networks:
-      apisix_net:
-
-  ## keycloak
-  # The keycloak official has two types of docker images:
-  # * legacy WildFly distribution
-  # * new Quarkus based distribution
-  # Here we choose new version, because it's mainstream and
-  # supports kcadm.sh to init the container for test.
-  # The original keycloak service `apisix_keycloak` is
-  # third-party personal customized image and for OIDC test only.
-  # We should unify both containers in future.
-  apisix_keycloak_new:
-    container_name: apisix_keycloak_new
+    container_name: apisix_keycloak
     image: quay.io/keycloak/keycloak:18.0.2

Review Comment:
   Do you ask suggestions from sshniro about 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] moonming commented on a diff in pull request #9341: chore(ci): remove 3rd keycloak docker image

Posted by "moonming (via GitHub)" <gi...@apache.org>.
moonming commented on code in PR #9341:
URL: https://github.com/apache/apisix/pull/9341#discussion_r1173256163


##########
ci/pod/docker-compose.plugin.yml:
##########
@@ -28,42 +28,26 @@ services:
     networks:
       apisix_net:
 
-
   ## keycloak
   apisix_keycloak:
-    image: sshniro/keycloak-apisix:1.0.0
-    environment:
-      KEYCLOAK_USER: admin
-      KEYCLOAK_PASSWORD: 123456
-    restart: unless-stopped
-    ports:
-      - "8090:8080"
-      - "8443:8443"
-    networks:
-      apisix_net:
-
-  ## keycloak
-  # The keycloak official has two types of docker images:
-  # * legacy WildFly distribution
-  # * new Quarkus based distribution
-  # Here we choose new version, because it's mainstream and
-  # supports kcadm.sh to init the container for test.
-  # The original keycloak service `apisix_keycloak` is
-  # third-party personal customized image and for OIDC test only.
-  # We should unify both containers in future.
-  apisix_keycloak_new:
-    container_name: apisix_keycloak_new
+    container_name: apisix_keycloak
     image: quay.io/keycloak/keycloak:18.0.2

Review Comment:
   @sshniro want to hear from you:)



-- 
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 merged pull request #9341: chore(ci): remove 3rd keycloak docker image

Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 merged PR #9341:
URL: https://github.com/apache/apisix/pull/9341


-- 
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 a diff in pull request #9341: chore(ci): remove 3rd keycloak docker image

Posted by "soulbird (via GitHub)" <gi...@apache.org>.
soulbird commented on code in PR #9341:
URL: https://github.com/apache/apisix/pull/9341#discussion_r1171978407


##########
ci/pod/docker-compose.plugin.yml:
##########
@@ -28,42 +28,25 @@ services:
     networks:
       apisix_net:
 
-
   ## keycloak
   apisix_keycloak:
-    image: sshniro/keycloak-apisix:1.0.0
-    environment:
-      KEYCLOAK_USER: admin
-      KEYCLOAK_PASSWORD: 123456
-    restart: unless-stopped
-    ports:
-      - "8090:8080"
-      - "8443:8443"
-    networks:
-      apisix_net:
-
-  ## keycloak
-  # The keycloak official has two types of docker images:
-  # * legacy WildFly distribution
-  # * new Quarkus based distribution
-  # Here we choose new version, because it's mainstream and
-  # supports kcadm.sh to init the container for test.
-  # The original keycloak service `apisix_keycloak` is
-  # third-party personal customized image and for OIDC test only.
-  # We should unify both containers in future.
-  apisix_keycloak_new:
-    container_name: apisix_keycloak_new
+    container_name: apisix_keycloak
     image: quay.io/keycloak/keycloak:18.0.2
     # use host network because in CAS auth,
     # keycloak needs to send back-channel POST to apisix.
     network_mode: host
     environment:
       KEYCLOAK_ADMIN: admin
       KEYCLOAK_ADMIN_PASSWORD: admin
+      KC_HTTPS_CERTIFICATE_FILE: /opt/keycloak/conf/server.crt.pem
+      KC_HTTPS_CERTIFICATE_KEY_FILE: /opt/keycloak/conf/server.key.pem
     restart: unless-stopped
-    command: ["start-dev", "--http-port 8080"]
+    command: ["start-dev", "--import-realm"]
     volumes:
       - /opt/keycloak-protocol-cas-18.0.2.jar:/opt/keycloak/providers/keycloak-protocol-cas-18.0.2.jar
+      - ./ci/pod/keycloak/server.crt.pem:/opt/keycloak/conf/server.crt.pem
+      - ./ci/pod/keycloak/server.key.pem:/opt/keycloak/conf/server.key.pem
+      - ./ci/pod/keycloak/realm-university.json:/opt/keycloak/data/import/realm-university.json

Review Comment:
   It should be configured using the command line, which is more clear. json configuration files can be difficult to maintain



-- 
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] moonming commented on a diff in pull request #9341: chore(ci): remove 3rd keycloak docker image

Posted by "moonming (via GitHub)" <gi...@apache.org>.
moonming commented on code in PR #9341:
URL: https://github.com/apache/apisix/pull/9341#discussion_r1173250770


##########
ci/pod/keycloak/kcadm_configure_university.sh:
##########
@@ -0,0 +1,90 @@
+#!/usr/bin/env bash

Review Comment:
   why we need this file?



##########
ci/pod/docker-compose.plugin.yml:
##########
@@ -28,42 +28,26 @@ services:
     networks:
       apisix_net:
 
-
   ## keycloak
   apisix_keycloak:
-    image: sshniro/keycloak-apisix:1.0.0
-    environment:
-      KEYCLOAK_USER: admin
-      KEYCLOAK_PASSWORD: 123456
-    restart: unless-stopped
-    ports:
-      - "8090:8080"
-      - "8443:8443"
-    networks:
-      apisix_net:
-
-  ## keycloak
-  # The keycloak official has two types of docker images:
-  # * legacy WildFly distribution
-  # * new Quarkus based distribution
-  # Here we choose new version, because it's mainstream and
-  # supports kcadm.sh to init the container for test.
-  # The original keycloak service `apisix_keycloak` is
-  # third-party personal customized image and for OIDC test only.
-  # We should unify both containers in future.
-  apisix_keycloak_new:
-    container_name: apisix_keycloak_new
+    container_name: apisix_keycloak
     image: quay.io/keycloak/keycloak:18.0.2

Review Comment:
   why not use 21.0 or 21.1?



-- 
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] sshniro commented on a diff in pull request #9341: chore(ci): remove 3rd keycloak docker image

Posted by "sshniro (via GitHub)" <gi...@apache.org>.
sshniro commented on code in PR #9341:
URL: https://github.com/apache/apisix/pull/9341#discussion_r1173277346


##########
ci/pod/docker-compose.plugin.yml:
##########
@@ -28,42 +28,26 @@ services:
     networks:
       apisix_net:
 
-
   ## keycloak
   apisix_keycloak:
-    image: sshniro/keycloak-apisix:1.0.0
-    environment:
-      KEYCLOAK_USER: admin
-      KEYCLOAK_PASSWORD: 123456
-    restart: unless-stopped
-    ports:
-      - "8090:8080"
-      - "8443:8443"
-    networks:
-      apisix_net:
-
-  ## keycloak
-  # The keycloak official has two types of docker images:
-  # * legacy WildFly distribution
-  # * new Quarkus based distribution
-  # Here we choose new version, because it's mainstream and
-  # supports kcadm.sh to init the container for test.
-  # The original keycloak service `apisix_keycloak` is
-  # third-party personal customized image and for OIDC test only.
-  # We should unify both containers in future.
-  apisix_keycloak_new:
-    container_name: apisix_keycloak_new
+    container_name: apisix_keycloak
     image: quay.io/keycloak/keycloak:18.0.2

Review Comment:
   Hi, thanks for this PR, at the time of development I had trouble exporting the configurations/roles/scopes. 
   These configs (roles/scopes) should be present in the Keycloak container to test the Authorization flows with our APISIX Keycloak plugin.
   
   Therefore the configs were created in a Keycloak container, and then I built an image out of it.
   That's what is present in the `sshniro/keycloak-apisix:1.0.0` container.
   
   This PR resolves these issues with roles and scopes created by the script, instead of inbuilt configurations. 
   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