Skip to content

code refactor#1

Open
nemerna wants to merge 3 commits into
mainfrom
review
Open

code refactor#1
nemerna wants to merge 3 commits into
mainfrom
review

Conversation

@nemerna

@nemerna nemerna commented Jul 9, 2023

Copy link
Copy Markdown
Owner
  1. replaced the authentication method to the one requested by Babak in the ticket

  2. removed the central control script

  3. containerized the application

  4. added Makefile for make easy run

  5. reduced the number of needed parameters

@nemerna

nemerna commented Jul 9, 2023

Copy link
Copy Markdown
Owner Author

@ilan-pinto

Comment thread Dockerfile
@@ -0,0 +1,31 @@
FROM quay.io/ecosystem-appeng/keycloak-source:latest as keycloak

FROM registry.access.redhat.com/ubi9/openjdk-11-runtime

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you pick this image?
i don't think it's the latest

@nemerna nemerna Jul 9, 2023

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i did not chose it. i Reused from
https://github.com/dmartinol/keycloak-change-admin-kcadm/tree/main/docker

youe mean the openjdk or the Keycloak one?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

latest compatible

Comment thread Dockerfile
RUN chmod +x /opt/keycloak/bin/user-import.sh
RUN chmod +x /opt/keycloak/bin/user-export.sh
RUN chmod +x /opt/keycloak/bin/groups-ids-wrapper.sh

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every line in the docker file adds a layer in the docker image, increasing the size.
https://docs.docker.com/build/guide/layers/
e.g try to cluster the copy commands

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

Comment thread Entrypoint.sh
@@ -0,0 +1,17 @@
#!/bin/bash

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the main file and the only one visible in the root folder - rename it to keyclock-user-cli or similar

Comment thread Entrypoint.sh
elif [ "$1" = "export" ]; then
echo "Performing export..."
export WORK_DIRECTORY=/home/default/EXPORT_DIR
user-export.sh

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move user-export.sh and user-import.sh and accordingly change path

also i would add an "help" option

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i didnt understand
`
Move user-export.sh and user-import.sh and accordingly change path

`

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Goes with the previous comment
user-export. sh and user-import. sh should not be in the root folder as you don't expect users to use them directly

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean in the repo?
because the code you commented about is relevant only inside the container,

Comment thread README.md

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an example of the user.csv file?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, it is a json not a csv

  • below is the username/id for all server users
    [ {
    "id" : "c29a1789-fb7f-4e11-a0a0-61f2d30fe009",
    "username" : "kcm"
    }, {
    "id" : "1a468df5-1ab1-4144-9d8c-736dac74181b",
    "username" : "names"
    }, {
    "id" : "31d4e96a-c982-450b-9dec-d6639c23d65a",
    "username" : "namess"
    } ]

  • this is an example of a single user
    {
    "username" : "kcm",
    "enabled" : true,
    "totp" : false,
    "emailVerified" : true,
    "firstName" : "KCM",
    "lastName" : "Admin",
    "email" : "temp@test.com",
    "attributes" : {
    "test" : [ "test" ]
    },
    "disableableCredentialTypes" : [ ],
    "requiredActions" : [ ],
    "notBefore" : 0,
    "access" : {
    "manageGroupMembership" : true,
    "view" : true,
    "mapRoles" : true,
    "impersonate" : true,
    "manage" : true
    }
    }

Comment thread user-export.sh
Comment thread user-export.sh
kcadm.sh get users/$user_id --fields '*(*(*(*(*(*))))),-id,-createdTimestamp' --realm $EXPORT_REALM --server $EXPORT_KEYCLOAK_SERVER --token $EXPORT_TOKEN --no-config > $WORK_DIRECTORY/$username/USER.json

# If failed to get user, skip the current iteration
if [ $? -ne 0 ]; then

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider writing to a log file

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, will make it through the Makefile
(container output redirect), where ir un the container in interactive mode

Comment thread client/lib/bcprov-jdk15on-1.70.jar Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this file?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its again based on Danielles Dockerfile, and the lib dir is from the official Keycloak release,
but tried to remove the library and nothing got broken ...

nemerna added 2 commits July 9, 2023 14:10
, move scripts to scripts directory, remove all the keycloak libs/modules, as we will be geting them always from the image
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants