Skip to content

WIP ipa-tuura: add demo for ipa-tuura using IPA and Keycloak - #24

Open
f-trivino wants to merge 1 commit into
freeipa:mainfrom
f-trivino:ipa-tuura
Open

WIP ipa-tuura: add demo for ipa-tuura using IPA and Keycloak#24
f-trivino wants to merge 1 commit into
freeipa:mainfrom
f-trivino:ipa-tuura

Conversation

@f-trivino

@f-trivino f-trivino commented May 13, 2025

Copy link
Copy Markdown
Contributor

This demo is based on the ipalab-config configuration of Keycloak, which is based on the official Keycloak container image, using a self-signed certificate for HTTPS connection. Keycloak runs in production mode.

Helper scripts and playbooks for configuring Keycloak, IPA, and ipa-tuura are provided.

Summary by Sourcery

Add a self-contained demo for ipa-tuura integration with FreeIPA and Keycloak, including configuration files, helper scripts, and Ansible playbooks to deploy and test the environment

New Features:

  • Provide a demo environment with lab configuration and container orchestration for FreeIPA–Keycloak integration via ipa-tuura
  • Include helper scripts to manage Firefox profiles, trust certificates, and launch browsers inside rootless Podman networks
  • Add VCR-compatible demo script to automate environment setup, certificate trust, IPA deployment, and user provisioning
  • Supply Ansible playbooks to configure Keycloak as an external IDP in IPA and to provision IPA users using IDP authentication

Documentation:

  • Add a comprehensive README detailing environment preparation, Keycloak federation setup, usage instructions, and troubleshooting

Chores:

  • Introduce requirements.txt to pin ipalab-config and Ansible core dependencies

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey @f-trivino - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 10 issues found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.


usage() {
cat <<EOF
usage: $(basename "$0") [-p PROFILE] [-r] [URL]"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion (typo): Remove stray trailing double quote in usage message

Suggested change
usage: $(basename "$0") [-p PROFILE] [-r] [URL]"
usage: $(basename "$0") [-p PROFILE] [-r] [URL]


WORKSHOP_PROFILE_DIR="${HOME}/.mozilla/firefox/${profile_name}"

if [ "$cmd" == "remove" ]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion (bug_risk): Use single '=' for string comparison in POSIX sh

Suggested change
if [ "$cmd" == "remove" ]
if [ "$cmd" = "remove" ]


if [ "$cmd" == "remove" ]
then
sed -i "/^\# start - Added by ipalab-config: ${profile_name}$/,/^\# end - Added by ipalab-config: ${profile_name}/d" "${MOZILLA_PROFILES}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion (bug_risk): Mismatch between profile markers in sed removal and cat insertion

Replace 'ipalab-config' with 'ipa-demos' in the sed removal command so it matches the insertion marker.

Suggested change
sed -i "/^\# start - Added by ipalab-config: ${profile_name}$/,/^\# end - Added by ipalab-config: ${profile_name}/d" "${MOZILLA_PROFILES}"
sed -i "/^\# start - Added by ipa-demos: ${profile_name}$/,/^\# end - Added by ipa-demos: ${profile_name}/d" "${MOZILLA_PROFILES}"

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.

ipalab-config is the name of the tool, don't replace it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You’re right—ipalab-config is the tool’s name, so the markers should stay as-is. My bad for the confusion; no change needed here.

h) usage && exit 0 ;;
p) profile_name="${OPTARG}" ;;
r) cmd="remove" ;;
*) die -u "Invalid option: ${OPTARG}" ;;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion: Unexpected '-u' flag passed to die()

Remove '-u' (die() doesn’t accept flags), or update die() to handle usage output.

Suggested change
*) die -u "Invalid option: ${OPTARG}" ;;
*) die "Invalid option: ${OPTARG}" ;;


fi

[ -z "$@" ] || detach podman unshare --rootless-netns firefox -P "$profile_name" --new-window "$@"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion (bug_risk): Incorrect empty-arguments test with '-z "$@"'

Use '[ $# -eq 0 ]' to detect no URLs; '-z "$@"' fails when multiple arguments are passed.


Before starting Firefox, add the entries found in the generated `hosts`
file to your machine `/etc/hosts` so the host names can be resolved. The
file `hosts` has all the containers entries needed, add it with:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion (typo): Consider rephrasing 'containers entries'.

Change “containers entries” to “container entries” for clarity.

Suggested change
file `hosts` has all the containers entries needed, add it with:
file `hosts` has all the container entries needed, add it with:

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.

change to ".. has the entries for all containers .."

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sounds good—“hosts” has the entries for all containers needed. I’ll update the README to:

Before starting Firefox, add the entries found in the generated `hosts` file to your machine’s `/etc/hosts` so the host names can be resolved. The `hosts` file has the entries for all containers needed, add it with:

-s 'config.keycloak_hostname=["keycloak.ipa.test"]'
```

CHECK IF WE CAN CREATE A JSON and rely on kcreg:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue: Developer note found in documentation.

Remove or address the 'CHECK IF WE CAN...' developer note and update the documentation accordingly.


## Troubleshooting

ADD ipa-tuura troubleshooting

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue: Placeholder comment in Troubleshooting section.

Please replace the placeholder with the actual ipa-tuura troubleshooting steps or remove it.

This demo is based on the ipalab-config configuration of Keycloak, which
is based on the official Keycloak container image, using a self-signed
certificate for HTTPS connection. Keycloak runs in production mode.

Helper scripts and playbooks for configuring Keycloak, IPA, and ipa-tuura are
provided.
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