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

ipaconfig: Do not require enable_sid for add_sids or netbios_name #961

Merged

Conversation

rjeffman
Copy link
Member

@rjeffman rjeffman commented Oct 13, 2022

Current behavior of ipaconfig mimics FreeIPA CLI and requires that
'enable_sid' is set to True every time add_sids or netbios_name are
used. It is sufficient that SID generation is enabled to use add_sids
and netbios_name, but the IPA API requires 'enable_sid' so that the
operations are executed.

This patch allows ansible-freeipa plugin ipaconfig to run 'add_sids' or
set 'netbios_name without requiring 'enable_sid' to be set on the
playbook.

If SID generation is enabled, 'add_sids' and 'netbios_name' can be used
without 'enable_sid: yes'. If SID generation is not enabled, an error
message will be raised if 'enable_sid: yes' is not used.

Depends on PR #921 being merged (due to commit 3c7bd56).

Fixes RHBZ#2134505, RHBZ#2134530

@rjeffman rjeffman force-pushed the ipaconfig_fix_enable_sid_not_required branch from 942cf03 to 0b40886 Compare October 17, 2022 16:49
Copy link
Collaborator

@varunmylaraiah varunmylaraiah left a comment

Choose a reason for hiding this comment

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

Rafael, the below test, is failing

---
- name: Playbook to ensure not able to update netbios_name with smallcase.
  hosts: ipaserver

  tasks:
  - ipaconfig:
      ipaadmin_password: <xxxxxxxx>
      enable_sid: yes
      netbios_name: newtestnetbios


 [ PLAY [Playbook to ensure not able to update netbios_name with smallcase.] ******
 [
 [ TASK [Gathering Facts] *********************************************************
 [ task path: /root/config_module.yml:2
 ok: [master.ipadomain.test]
 META: ran handlers

 TASK [ipaconfig] ***************************************************************
 task path: /root/config_module.yml:6
ok: [master.ipadomain.test] => {"changed": false, "config": {}}
META: ran handlers
META: ran handlers
2022-10-17T17:28:40+0000 [
PLAY RECAP *********************************************************************
master.ipadomain.test      : ok=2    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

Expected error: "only uppercase ASCII letters, digits and dashes are allowed"

@rjeffman rjeffman force-pushed the ipaconfig_fix_enable_sid_not_required branch from 0b40886 to 7ec8e4c Compare October 17, 2022 18:28
@rjeffman
Copy link
Member Author

This is interesting, this code was available before, did pass all tests.

Nonetheless, this was a bug in the code and is fixed by the latest update.

Copy link
Collaborator

@varunmylaraiah varunmylaraiah left a comment

Choose a reason for hiding this comment

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

Thanks, @rjeffman, the above issue is resolved. The rest of the changes are LGTM.

@t-woerner
Copy link
Member

An old version of #921 is part of this PR.

@rjeffman rjeffman force-pushed the ipaconfig_fix_enable_sid_not_required branch from 7ec8e4c to c7cc01b Compare October 18, 2022 13:26
Current behavior of ipaconfig mimics FreeIPA CLI and requires that
'enable_sid' is set to True every time add_sids or netbios_name are
used. It is sufficient that SID generation is enabled to use add_sids
and netbios_name, but the IPA API requires 'enable_sid' so that the
operations are executed.

This patch allows ansible-freeipa plugin ipaconfig to run 'add_sids' or
set 'netbios_name without requiring 'enable_sid' to be set on the
playbook.

If SID generation is enabled, 'add_sids' and 'netbios_name' can be used
without 'enable_sid: yes'. If SID generation is not enabled, an error
message will be raised if 'enable_sid: yes' is not used.
@rjeffman rjeffman force-pushed the ipaconfig_fix_enable_sid_not_required branch from c7cc01b to c808ad6 Compare October 18, 2022 14:14
Copy link
Member

@t-woerner t-woerner left a comment

Choose a reason for hiding this comment

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

LGTM

@t-woerner t-woerner merged commit f8ca8a7 into freeipa:master Oct 18, 2022
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