Skip to content
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.

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

feat: Add SSSD parameters support #76

Merged
merged 8 commits into from
Jan 10, 2024

Conversation

seb2020
Copy link
Contributor

@seb2020 seb2020 commented Jan 3, 2024

Enhancement: Add variable for setting configuration variable inside the [sssd] section of the sssd.conf file

Reason: In my use case, I need to set some additional option like default_domain_suffix to be present inside the [sssd] section. This role doesn't have a variable for doing that. You can only set inside [domain/YOURDOMAIN]

Result: You can use the variable ad_integration_sssd_settings to set some extra paremters like documented in https://linux.die.net/man/5/sssd.conf

Issue Tracker Tickets (Jira or BZ if any): N/A

P.S : It's my first PR. Be kind with me 😉

Correct variables for test
tasks/main.yml Outdated
group: root
mode: u=rw,g=,o=
loop: "{{ ad_integration_sssd_settings }}"
notify: Handler for ad_integration to restart services
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we're going to need two separate handlers - one for realmd and one for sssd - it looks like there are cases where changes are made to config files that need realmd to be restarted but not sssd, and vice versa. Also, are there cases where a change to sssd.conf will require a restart of both realmd and sssd?

Yes, we could just restart both services every time anything changes, but I would strongly prefer not to restart services unnecessarily.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi,
I can be better to do two handlers. I can look if you want. Should I do this in the same PR as this one ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, I can be better to do two handlers. I can look if you want. Should I do this in the same PR as this one ?

Yes, please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi,
I have updated the PR. I have try to squash to have only 1 commit but I didn't make it.

richm and others added 2 commits January 8, 2024 09:30
ci: fix ansible-lint 2.16 and ansible-test 2.16 issues
Signed-off-by: Rich Megginson <rmeggins@redhat.com>
Signed-off-by: Girard Sebastien <sebastien@sgir.ch>
mode: u=rw,g=,o=
loop: "{{ ad_integration_sssd_settings }}"
notify: Handler for ad_integration to restart services - sssd

Copy link
Contributor

Choose a reason for hiding this comment

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

Do any of these sssd settings need to be set, and sssd restarted, before trying to join to the AD domain? The task name: Run realm join command? Do any of these sssd settings also require realmd to be restarted, or otherwise notified (e.g. like a SIGHUP)? @jakub-vavra-cz @justin-stephenson wdyt?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do any of these sssd settings need to be set, and sssd restarted, before trying to join to the AD domain?

No, SSSD only gets involved after the system is joined to the domain. realm join creates a sssd.conf, so SSSD config changes should be made only after the realm join is performed or they will likely be overwritten.

realmd restart is not needed, SSSD restart is enough.

Correct variables for test
Signed-off-by: Girard Sebastien <sebastien@sgir.ch>
@richm
Copy link
Contributor

richm commented Jan 8, 2024

[citest]

Example:

```yaml
ad_integration_sssd_settings:
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be defined in defaults/main.yml like this:

# A list of custom setting to be included into the [sssd] section
# of the sssd.conf file. The list will be composed of two entry:
# - key: "configuration_name"
#   value: "configuration_value"
ad_integration_sssd_settings: []

That should solve the test failures related to "undefined variable"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated the commit

tests/tests_sssd_settings.yml Outdated Show resolved Hide resolved
tests/tests_sssd_settings.yml Outdated Show resolved Hide resolved
Signed-off-by: Sebastien Girard <sebastien@sgir.ch>
@richm
Copy link
Contributor

richm commented Jan 9, 2024

[citest]

@richm richm merged commit c1085b0 into linux-system-roles:main Jan 10, 2024
20 checks passed
@seb2020 seb2020 deleted the add_sssd_standard_options branch January 10, 2024 06:08
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.

None yet

3 participants