passbolt / lab-passbolt-ansible-collection

Source repository for https://galaxy.ansible.com/anatomicjc/passbolt
MIT No Attribution
9 stars 10 forks source link

OS environment variables are ignored when (unrelated) environment variables are defined in playbook #13

Closed RobinR1 closed 4 months ago

RobinR1 commented 7 months ago

I've configured the Passbolt ansible lookup plugin using OS environment variables set by AWX. This works without problem unless there is a environment: block defined in the ansible play, even when no passbolt plugin related variables are set in that block.

Example: Environment variables are set on OS level and available to the Ansible process:

PASSBOLT_BASE_URL
PASSBOLT_PRIVATE_KEY
PASSBOLT_PASSPHRASE

And with that, this works correctly:

- name: Test passbolt
  host: localhost
  tasks:
    - name: Lookup predefined resource
      ansible.builtin.debug:
        msg: "Password resource 1 is: {{ lookup('anatomicjc.passbolt.passbolt', 'Ansible predefined test resource').password }}"

However, this fails:

- name: Test passbolt
  host: localhost
  environment:
    PATH: "/usr/local/bin:{{ ansible_env.PATH }}"
  tasks:
    - name: Lookup predefined resource
      ansible.builtin.debug:
        msg: "Password resource 1 is: {{ lookup('anatomicjc.passbolt.passbolt', 'Ansible predefined test resource').password }}"

with:

Error was a <class 'ValueError'>, original message: Expected: ASCII-armored PGP data.

So it seems that the plugin fails to look up the OS environment variables whenever there is a environment: block defined in the play.

Looking at the code in function https://github.com/passbolt/lab-passbolt-ansible-collection/blob/ec035d465d98daf89c5d84bf67057807358d4dd4/plugins/lookup/passbolt.py#L104-L117 I think the OS environment is completely ignored when environment_variables is set even when the required variable is not found inside environment_variables. I'm not familiar enough with Python and Ansible libraries to quickly create a pull request for this, but I think the code should fall back to the OS environment when the required variable is not found in environment_variables .

RobinR1 commented 7 months ago

I managed to find some time to study Python iterators and understand the code. I submitted a pull request that fixes this problem.

AnatomicJC commented 4 months ago

Hi,

I totally missed this message and the associated PR. I think I will make a small refactor but thank you for the description and PR.

I will have a look in the coming days.

Best,

RobinR1 commented 4 months ago

Hi, thanks. Since I read you consider refactoring the lookup, I added a few feature requests you could consider. :-)

AnatomicJC commented 4 months ago

Hi @RobinR1

I finally found time to have a look and merged your PR.

Your contributions and tests with AWX are very appreciated. Thanks again for your patience regarding time to merge your PR.

Best regards,