Inventory plugin: restore reading auth token from env variables

[gizero]

SUMMARY

By completely removing any documentation for api_token / oauth_token, commit 609df3e suddenly broke support for reading the auth token from environment variables (which magically happens to work based on plugin options documentation).

Restore the documentation bits by matching docs fragment. While at it, also match the documented list of supported env variables which can be used as auth token sources.

Fixes #315

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

Inventory plugin

[mamercad]

Thanks for this, we just need a changelog fragment please, and, thank you.

[gizero]

@mamercad PR updated to include a changelog fragment.

[mamercad]

❤️

[ad8lmondy]

The release notes for 1.25.0 here: https://github.com/ansible-collections/community.digitalocean/releases/tag/1.25.0 mention:

Inventory plugin: restore reading auth token from env variables by @gizero in #316

but when I try to use it, it still fails. When I use the 1.25.0 tag in GitHub to browse where the fix should be (the env block here: https://github.com/ansible-collections/community.digitalocean/pull/316/files#diff-20875ea48f250483498d60cc2f6cac1a67849d566ece0a4d961038491716bb46R27 ), the env block is not there: https://github.com/ansible-collections/community.digitalocean/blob/1.25.0/plugins/doc_fragments/digital_ocean.py

So I think the #316 PR somehow didn't make it in the release?

[gizero]

@ad8lmondy I can confirm the fix was, likely unintentionally, reverted by 5b66a23 here. Unfortunately the fix as it was implemented in #316 worked but raised a complaint by the ansible-test framework as briefly discussed here. I haven't had the time to get back to it and figure out why the linter complained. My initial thought was that there could be a mismatch between the plugins documentation schema and the implementation... Maybe @mamercad can help and give a clue here: one option could be to revert to the first implementation I proposed for #316, where the environment variable documentation bits where added at the inventory plugin level, and not globally for the entire plugins collection. Still, it should apply to other modules as well...

[grzs]

@ad8lmondy hi, it seems to me that the issue is still with us. At least I can't use the plugin with env variable. Or is it my problem only?

[grzs]

I could make it work with the help of a pipe lookup, following the documentation. By the way, there is a typo in that code snippet there, a closing paren is missing. Should I make a PR?

[ad8lmondy]

@ad8lmondy hi, it seems to me that the issue is still with us. At least I can't use the plugin with env variable. Or is it my problem only?

I just reverted to an older version.

help of a pipe lookup, following the documentation

Could you point me to this? And I'm sure a PR for typos would be welcome :)

[grzs]

#364

[gizero]

Hi @grzs! I confirm the bug is still there. Soon after this PR was merged, the fix was partially reverted because of a failing test on the main branch. Since the fix was automagically generated from documentation, the revert brought the bug back. I haven't had the time to get back to it to fix it again. I'm also using the lookup pipe to work this around at the moment.

[grzs]

Thank for the info! Isn't env lookup more straightforward, or is there any caveat to it?

[gizero]

@grzs Do you mean the implicit env lookup or the explicit pipe lookup? I'd like to see this bug fixed, since it is a breaking change. I don't see any value in deprecating the original behaviour of reading some (well documented) variables from the env without explicit plugin configuration. I'll update this issue if I find the time to get onto it.

[grzs]

@gizero I meant the explicit env lookup like this:

oauth_token: "{{ lookup('env', 'DO_API_TOKEN') }}"

[grzs]

I also look at it as a workaround, no question about it. It would be great to see it fixed somehow.