Skip to content

Conversation

@shridhargadekar
Copy link
Contributor

@shridhargadekar shridhargadekar commented Oct 10, 2025

Adding further methods to adcli class:

  1. Create-user
  2. Delete-user
  3. Create-group, group-members
  4. Delete-group, group-members
  5. Create msa (managed service account)
  6. Password-user

This PR will complete all the methods available with adcli so far.

@gemini-code-assist
Copy link

Summary of Changes

Hello @shridhargadekar, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly expands the capabilities of the adcli utility class within the sssd_test_framework by introducing a comprehensive set of methods for managing Active Directory users and groups. These additions streamline the automation of common Active Directory tasks, enhancing the framework's ability to simulate and test various identity management scenarios.

Highlights

  • User Management: New methods create_user and delete_user have been added to the adcli class, allowing for programmatic creation and removal of users in Active Directory.
  • Group Management: New methods create_group and delete_group have been introduced to facilitate the creation and deletion of groups within Active Directory.
  • Group Membership Management: Functionality to manage group members has been added with add_member and remove_member methods, enabling the addition and removal of users or computers from Active Directory groups.
  • Authentication Options: All new methods support both password-based authentication and Kerberos (krb) authentication for performing Active Directory operations.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds several new methods to the AdcliUtils class for managing users and groups in Active Directory. The changes are functional, but there is a significant amount of code duplication across the new methods. My review focuses on refactoring this duplicated logic into a helper method to improve maintainability and consistency. I've also pointed out some minor inconsistencies in parameter naming and command-line argument order that can be addressed during the refactoring.

@shridhargadekar shridhargadekar force-pushed the adclimethod branch 6 times, most recently from 603e2dd to 5445890 Compare October 14, 2025 10:01
@shridhargadekar shridhargadekar force-pushed the adclimethod branch 2 times, most recently from 883fbb1 to eee1dda Compare October 14, 2025 18:38
Copy link
Member

@pbrezina pbrezina left a comment

Choose a reason for hiding this comment

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

Hi, please, see few comments inside. Otherwise looks good, but I'll let someone else to review functionality.

@sumit-bose
Copy link
Contributor

Hi,

I have not comments about the adcli functionality, but a general comment.

You are using the if krb: ... kinit ... else: --- --stdin-password .. -U login_user ... pattern here and in existing code for nearly every command. I think it would make sense to add a generic helper function to avoid the code duplication. Since this also relates to existing code I'm fine if this will be address in a separate PR. Please let me know if you prefer to start handling this here or if everything should go into a new PR.

Additionally, please check the tox error.

bye,
Sumit

Copy link
Contributor

@jakub-vavra-cz jakub-vavra-cz left a comment

Choose a reason for hiding this comment

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

See my suggestion for create_user.
I think it makes the code more readable and might be applicable to all of the functions.

Alternatively create a generic helper and just add appropriate adcli_cmd :

def _run_adcli(
        self,
        adcli_cmd: list[str],
        domain: str,
        password: str,
        args: list[str] | None = None,
        login_user: str,
        krb: bool = False,
    ) -> ProcessResult:
        if args is not None:
            adcli_cmd.extend(args)
        if krb:
             self.host.conn.exec(["kinit", f"{login_user}@{domain.upper()}"], input=password)
        else:
             adcli_cmd.extend(["--stdin-password", "-U", login_user])
        return self.host.conn.exec(adcli_cmd, input=password, raise_on_error=False)

Comment on lines 344 to 363
if krb:
self.host.conn.exec(["kinit", f"{login_user}@{domain.upper()}"], input=password)
command = self.host.conn.exec(
["adcli", "create-user", user, f"--domain={domain}", "-C", *args], raise_on_error=False
)
else:
command = self.host.conn.exec(
[
"adcli",
"create-user",
user,
"--stdin-password",
f"--domain={domain}",
*args,
"-U",
login_user,
],
input=password,
raise_on_error=False,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if krb:
self.host.conn.exec(["kinit", f"{login_user}@{domain.upper()}"], input=password)
command = self.host.conn.exec(
["adcli", "create-user", user, f"--domain={domain}", "-C", *args], raise_on_error=False
)
else:
command = self.host.conn.exec(
[
"adcli",
"create-user",
user,
"--stdin-password",
f"--domain={domain}",
*args,
"-U",
login_user,
],
input=password,
raise_on_error=False,
)
adcli_cmd = ["adcli", "create-user", user, f"--domain={domain}", "-C", *args]
if krb:
self.host.conn.exec(["kinit", f"{login_user}@{domain.upper()}"], input=password)
else:
adcli_cmd.extend(["--stdin-password", "-U", login_user])
command = self.host.conn.exec(adcli_cmd, input=password, raise_on_error=False)

Adding further methods to adcli class:
 1. Create-user
 2. Delete-user
 3. Create-group, add group-members
 4. Remove-group, remove group-members
 5. Create MSA
 6. (Re)Set password of AD-user
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants