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

fix: Switch SSSD config files provider to Proxy Provider #88

Merged

Conversation

justin-stephenson
Copy link
Collaborator

SSSD Files provider is being deprecated and removed in later RHEL/Fedora releases.

@justin-stephenson justin-stephenson force-pushed the proxy_provider_switch branch 2 times, most recently from 099dd62 to 9eaa6eb Compare April 28, 2023 12:54
Copy link
Contributor

@spetrosi spetrosi left a comment

Choose a reason for hiding this comment

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

Will this work on earlier RHEL 8 and 9?
Please update your commit to include a commit type. We now follow conventional commits format to automate changelog generating and updating release version numbers.

templates/sssd.conf Outdated Show resolved Hide resolved
@justin-stephenson justin-stephenson changed the title Switch SSSD config files provider to Proxy Provider fix: Switch SSSD config files provider to Proxy Provider Apr 28, 2023
@justin-stephenson
Copy link
Collaborator Author

Will this work on earlier RHEL 8 and 9?

Yes it will, and all fedora versions.

Please update your commit to include a commit type. We now follow conventional commits format to automate changelog generating and updating release version numbers.

Updated.

@spetrosi
Copy link
Contributor

[citest]

@richm
Copy link
Contributor

richm commented Apr 28, 2023

So it is ok if the role "owns" /etc/sssd/sssd.conf? I guess ini_file was used to preserve changes from users or other apps, and we don't need to preserve these changes anymore?
This is the only place ini_file is used, so we can also get rid of the file meta/collection-requirements.yml (and make a note to change the spec file not to vendor this for the tlog role).
Finally, make the following change to tests_sssd.yml:

    - name: Check for ansible_managed, fingerprint in generated files
      include_tasks: tasks/check_header.yml
      loop:
        - "{{ __tlog_sssd_session_recording_conf }}"
        - "{{ __tlog_sssd_conf }}"
      loop_control:
        loop_var: __file
      vars:
        __fingerprint: "system_role:tlog"

@justin-stephenson
Copy link
Collaborator Author

So it is ok if the role "owns" /etc/sssd/sssd.conf? I guess ini_file was used to preserve changes from users or other apps, and we don't need to preserve these changes anymore? This is the only place ini_file is used, so we can also get rid of the file meta/collection-requirements.yml (and make a note to change the spec file not to vendor this for the tlog role). Finally, make the following change to tests_sssd.yml:

    - name: Check for ansible_managed, fingerprint in generated files
      include_tasks: tasks/check_header.yml
      loop:
        - "{{ __tlog_sssd_session_recording_conf }}"
        - "{{ __tlog_sssd_conf }}"
      loop_control:
        loop_var: __file
      vars:
        __fingerprint: "system_role:tlog"

I would prefer to not have tlog own /etc/sssd/sssd.conf, based on your comments I think it will be better to continue using the ini_file module. I will update the PR once ready.

SSSD Files provider is being deprecated and removed in later RHEL/Fedora
releases.
@richm
Copy link
Contributor

richm commented May 1, 2023

I would prefer to not have tlog own /etc/sssd/sssd.conf

What other app makes changes to sssd.conf that we want to preserve?

@justin-stephenson
Copy link
Collaborator Author

Tlog uses SSSD behind the scenes to configure shell recording configuration of local users, but it is done "behind the scenes" - it is a non-typical use case for majority of SSSD userbase. If someone wants to setup remote authentication as an IDM client (ansible-freeipa), or AD integration role then both those playbooks would generate a new sssd.conf, through realmd or ipa-client-install.

@richm
Copy link
Contributor

richm commented May 1, 2023

Tlog uses SSSD behind the scenes to configure shell recording configuration of local users, but it is done "behind the scenes" - it is a non-typical use case for majority of SSSD userbase. If someone wants to setup remote authentication as an IDM client (ansible-freeipa), or AD integration role then both those playbooks would generate a new sssd.conf, through realmd or ipa-client-install.

ok - then ini_file it is

@justin-stephenson
Copy link
Collaborator Author

@spoore1 @aborah-sudo FYI

@spoore1
Copy link
Contributor

spoore1 commented May 15, 2023

@spoore1 @aborah-sudo FYI

I ran a quick check on RHEL9. LGTM

@richm
Copy link
Contributor

richm commented May 15, 2023

[citest]

Copy link
Contributor

@richm richm left a comment

Choose a reason for hiding this comment

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

lgtm - ready to merge?

@justin-stephenson
Copy link
Collaborator Author

Good with me.

@richm richm merged commit 4f619cd into linux-system-roles:main May 15, 2023
21 checks passed
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

4 participants