Skip to content

Gdm c9s updates #250

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
spoore1 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Gdm c9s updates #250

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b1010f9
topologies: hostname and setup/teardown fixes
spoore1 May 27, 2026
581735c
gdm: fixes to better support c9s/rhel9
spoore1 May 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 37 additions & 4 deletions sssd_test_framework/topology_controllers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,21 @@ def join_domain(self, client: ClientHost, provider: IPAHost | ADHost | SambaHost
self.logger.info(f"Running realm join failed with:\n{result.stdout}\n{result.stderr}")
self.logger.info("Trying uninstall and join again.")
if isinstance(provider, (IPAHost)):
client.conn.exec(["ipa-client-install", "--uninstall", "-U"])
result = client.conn.exec(["ipa-client-install", "--uninstall", "-U"], raise_on_error=False)
if result.rc != 0:
self.logger.info(
f"Running ipa-client-install --uninstall failed with:\n{result.stdout}\n{result.stderr}"
)
self.logger.info("Trying to remove sssd.conf now")

@ikerexxe ikerexxe May 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd state Removing sssd.conf now, unless a check for the removal of the file exists somewhere

client.fs.rm("/etc/sssd/sssd.conf")
else:
client.conn.exec(["realm", "leave", "--unattended", provider.domain], input=provider.adminpw)
result = client.conn.exec(
["realm", "leave", "--unattended", provider.domain], input=provider.adminpw, raise_on_error=False
)
if result.rc != 0:
self.logger.info(f"Running realm leave failed with:\n{result.stdout}\n{result.stderr}")
self.logger.info("Trying to remove sssd.conf now")

@ikerexxe ikerexxe May 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here

client.fs.rm("/etc/sssd/sssd.conf")
Comment on lines 97 to +112

@gemini-code-assist gemini-code-assist Bot May 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The error handling logic for ipa-client-install --uninstall and realm leave is duplicated. To improve maintainability and reduce redundancy, this can be refactored into a single block of code.

            if isinstance(provider, (IPAHost)):
                uninstall_cmd = ["ipa-client-install", "--uninstall", "-U"]
                uninstall_input = None
                cmd_desc = "ipa-client-install --uninstall"
            else:
                uninstall_cmd = ["realm", "leave", "--unattended", provider.domain]
                uninstall_input = provider.adminpw
                cmd_desc = "realm leave"

            result = client.conn.exec(uninstall_cmd, input=uninstall_input, raise_on_error=False)
            if result.rc != 0:
                self.logger.info(f"Running {cmd_desc} failed with:\n{result.stdout}\n{result.stderr}")
                self.logger.info("Trying to remove sssd.conf now")
                client.fs.rm("/etc/sssd/sssd.conf")

client.conn.exec(["realm", "join", provider.domain], input=provider.adminpw)


Expand Down Expand Up @@ -361,7 +373,11 @@ class GDMTopologyController(ProvisionedBackupTopologyController):
@BackupTopologyController.restore_vanilla_on_error
def topology_setup(self, client: ClientHost, ipa: IPAHost, keycloak: KeycloakHost) -> None:
short_hostname = client.conn.run("hostname").stdout.split(".")[0].strip()
hostname = f"{short_hostname}.{keycloak.domain}"
hostname = f"{short_hostname}.{ipa.domain}"
client.fs.backup("/etc/hostname")
client.fs.backup("/etc/hosts")
client.conn.run(f"echo {hostname} > /etc/hostname")
client.fs.write("/etc/hosts", client.fs.read("/etc/hosts").replace("client.test", hostname))
Comment on lines 375 to +380

@gemini-code-assist gemini-code-assist Bot May 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

This block of code for setting the client hostname is also present in IPATopologyController. To improve maintainability and avoid code duplication, consider extracting this logic into a private helper method, e.g., _set_client_hostname(self, client: ClientHost, domain: str).


# Change client hostname to match the domain
self.logger.info(f"Changing hostname to {hostname}")
Expand All @@ -382,6 +398,16 @@ def topology_setup(self, client: ClientHost, ipa: IPAHost, keycloak: KeycloakHos

# Create an IdP client
keycloak.kclogin()

# First delete client if it exists
result = keycloak.conn.run(
"/opt/keycloak/bin/kcadm.sh get clients -q clientId=ipa_oidc_client " "--fields=id|jq -r '.[0].id'",
raise_on_error=False,
)
if result.rc == 0 and "null" not in result.stdout:
client_id = result.stdout.strip()
keycloak.conn.run(f"/opt/keycloak/bin/kcadm.sh delete clients/{client_id}")

keycloak.conn.run(
"/opt/keycloak/bin/kcadm.sh create clients -r master "
'-b \'{"clientId": "ipa_oidc_client", "clientAuthenticatorType": "client-secret", '
Expand All @@ -392,7 +418,14 @@ def topology_setup(self, client: ClientHost, ipa: IPAHost, keycloak: KeycloakHos
)

ipa.kinit()
ipa.conn.run(

# Check if IPA entry for Keycloak already exists and delete
result = ipa.conn.run("ipa idp-show keycloak", raise_on_error=False)
if "ipa_oidc_client" in result.stdout:
self.logger.info(f"IPA already enrolled in keycloak. Removing.")

@ikerexxe ikerexxe May 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This doesn't need to be an f-string

@danlavu danlavu Jun 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nitpick, two spaces before 'Removing"

ipa.conn.run("ipa idp-del keycloak")

result = ipa.conn.run(

@ikerexxe ikerexxe May 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Either you check for something on result or you don't assign any value here

f"ipa idp-add keycloak --provider keycloak --base-url {keycloak.hostname}:8443/auth "
"--org master --client-id ipa_oidc_client --secret",
input="IPA_Secret123",
Expand Down