ldap_attrs: "state=exact" doesn't remove attributes not present int "attributes"

[seb4itik]

Summary

The documentation says that "If exact, the set of attribute values will be forced to exactly those provided and no others. If [state=exact] and the attribute value is empty, all values for this attribute will be removed." But this is not the case. Reading the code source of "ldap_attrs.py" shows it has not been made for that.

Issue Type

Bug Report

Component Name

plugins/modules/ldap_attrs.py

Ansible Version

$ ansible --version
ansible [core 2.16.6]
  config file = /Users/seb/dev/iTik/ansible-slapd/.ansible.cfg
  configured module search path = ['/Users/seb/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /opt/homebrew/lib/python3.12/site-packages/ansible
  ansible collection location = /Users/seb/.ansible/collections:/usr/share/ansible/collections
  executable location = /opt/homebrew/bin/ansible
  python version = 3.12.3 (main, Apr  9 2024, 08:09:14) [Clang 15.0.0 (clang-1500.3.9.4)] (/opt/homebrew/opt/python@3.12/bin/python3.12)
  jinja version = 3.1.4
  libyaml = True

Community.general Version

$ ansible-galaxy collection list community.general

# /Users/seb/.ansible/collections/ansible_collections
Collection        Version
----------------- -------
community.general 8.6.0  

Configuration

$ ansible-config dump --only-changed
CONFIG_FILE() = /Users/seb/dev/iTik/ansible-slapd/.ansible.cfg
DEFAULT_HOST_LIST(/Users/seb/dev/iTik/ansible-slapd/.ansible.cfg) = ['/Users/seb/dev/iTik/ansible-slapd/inventory.yml']

OS / Environment

Ansible master: macOS Sonoma 14.4.1 Ansible target: Debian 12

Steps to Reproduce

- name: Test 1
  community.general.ldap_attrs:
    dn: "cn=config"
    attributes:
      olcLogLevel: 64
      olcTLSCertificateFile: "/etc/ssl/certs/ldap1.test.me.crt"
      olcTLSCertificateKeyFile: "/etc/ssl/private/ldap1.test.me.key"
    state: "exact"

- name: Get result test 1
  community.general.ldap_search:
    dn: "cn=config"
    attrs: ["olcLogLevel"]
  register: out

- name: Debug 1
  ansible.builtin.debug:
    msg: "Test 1: {{ out.results }}"

- name: Test 2
  community.general.ldap_attrs:
    dn: "cn=config"
    attributes:
      olcLogLevel: 65
      olcTLSCertificateFile: "/etc/ssl/certs/ldap1.test.me.crt"
      olcTLSCertificateKeyFile: "/etc/ssl/private/ldap1.test.me.key"
    state: "exact"

- name: Get result test 2
  community.general.ldap_search:
    dn: "cn=config"
    attrs: ["olcLogLevel"]
  register: out

- name: Debug 2
  ansible.builtin.debug:
    msg: "Test 2: {{ out.results }}"

- name: Test 3
  community.general.ldap_attrs:
    dn: "cn=config"
    attributes:
      olcTLSCertificateFile: "/etc/ssl/certs/ldap1.test.me.crt"
      olcTLSCertificateKeyFile: "/etc/ssl/private/ldap1.test.me.key"
    state: "exact"

- name: Get result test 3
  community.general.ldap_search:
    dn: "cn=config"
    attrs: ["olcLogLevel"]
  register: out

- name: Debug 3
  ansible.builtin.debug:
    msg: "Test 3: {{ out.results }}"

- name: Test 4
  community.general.ldap_attrs:
    dn: "cn=config"
    attributes:
      olcLogLevel: []
      olcTLSCertificateFile: "/etc/ssl/certs/ldap1.test.me.crt"
      olcTLSCertificateKeyFile: "/etc/ssl/private/ldap1.test.me.key"
    state: "exact"

- name: Get result test 4
  community.general.ldap_search:
    dn: "cn=config"
    attrs: ["olcLogLevel"]
  register: out

- name: Debug 4
  ansible.builtin.debug:
    msg: "Test 4: {{ out.results }}"

Expected Results

Test 1: OK Test 2: OK Test 3: Failed Test 4: OK (but this a workaround)

Test 3 result should be:

TASK [tests : Debug 3] ********************************************************************************************************************************************************************
ok: [ldap1] => {
    "msg": "Test 3: [{'dn': 'cn=config'}]"
}

Actual Results

TASK [tests : Debug 3] ********************************************************************************************************************************************************************
ok: [ldap1] => {
    "msg": "Test 3: [{'dn': 'cn=config', 'olcLogLevel': '65'}]"
}

Code of Conduct

• [X] I agree to follow the Ansible Code of Conduct

[ansibullbot]

Files identified in the description:

• plugins/modules/ldap_attrs.py

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

[ansibullbot]

cc @drybjed @jtyr @noles click here for bot help

[felixfontein]

I think you're misunderstanding the description for state=exact. The description is about the values of a single attribute, not about the set of attributes.

If you pass

    attributes:
      olcTLSCertificateFile: "/etc/ssl/certs/ldap1.test.me.crt"
      olcTLSCertificateKeyFile: "/etc/ssl/private/ldap1.test.me.key"

to the module you do not mention olcLogLevel at all, so you do not tell it to remove the values for it.

If you explicitly pass olcLogLevel: [] (as in your "workaround"), only then do you tell the module to remove the values for that attribute.

So your case 4 is not a workaround, but the only intended working way to achieve what you want.

(I guess that the description of state=exact can be improved. If you have a suggestion to make this better to understand, that would be great :) )

[seb4itik]

Hi @felixfontein, Thank you for your answer. Yes, after reading the description of state=exact again, I think I misunderstood. That said, this doesn't match the idempotency principle that should prevail with Ansible roles. May I propose a patch to change this behavior?

[felixfontein]

It does match the idempotency principle for individual attributes, which is what this module is about. If you are interested in deleting all attributes that haven't been specified yet, that would be a very different behavior (which also makes sense to support).

Changing the current state=exact behavior is not acceptable, as this would be a breaking change and surprise existing users of this feature. But adding a new option which allows to remove attributes that haven't been explicitly specified would be ok. (This could be orthogonal to state=exact and also work with state=present: you could make sure that the specified attributes have these values (but maybe some more), and at the same time get rid of all other attributes.)

[seb4itik]

Ok, I agree. Maybe a new value for state such as record_exact or a new option? Or, because this feature would act at the LDAP record level, it should go into ldap_entry with a new state=exact?

[felixfontein]

I wouldn't add yet another state value.

About ldap_entry: that module so far only uses the provided attributes during creation, but does not update them. Making that module also updating attributes would be quite some more work. (That would definitely be useful.) Having a state: exact state there (which does what you want) would be great - but it's probably worth to move some common code from ldap_entry and ldap_attrs to module_utils then.

[seb4itik]

OK, thanks. I will try to figure out the amount of work required for that...