Conjur Lookup Plugin #34280

jvanderhoof · 2017-12-28T22:04:08Z

SUMMARY

This PR adds an Ansible Lookup Plugin which allows Ansible to retrieve secrets from Conjur using the Ansible controller's Conjur identity.

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

Conjur Variable (ex. lookup('conjur_variable', 'db/password')

ANSIBLE VERSION

ansible 2.3.2.0
  config file =
  configured module search path = Default w/o overrides
  python version = 2.7.13 (default, Dec 17 2016, 23:03:43) [GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.42.1)]

ADDITIONAL INFORMATION

This lookup plugin requires Conjur (https://www.conjur.org/) to be running and accessible from the machine executing the playbook using the Conjur lookup plugin.

I wrote a small project to simplify building and validating this plugin: https://github.com/jvanderhoof/ansible-testing. It uses Docker Compose provide all the parts necessary.

ansibot · 2017-12-28T22:34:06Z

The test ansible-test sanity --test no-underscore-variable [?] failed with the following error:

Command "test/sanity/code-smell/no-underscore-variable.sh" returned exit status 2.
>>> Standard Output
== Underscore used as a variable ==
./lib/ansible/plugins/lookup/conjur_variable.py:                id, _, api_key = identity.authenticators('{}/authn'.format(appliance_url))

The test ansible-test sanity --test pep8 [?] failed with the following errors:

lib/ansible/plugins/lookup/conjur_variable.py:62:161: E501 line too long (161 > 160 characters)
lib/ansible/plugins/lookup/conjur_variable.py:75:9: E303 too many blank lines (2)
lib/ansible/plugins/lookup/conjur_variable.py:80:54: E251 unexpected spaces around keyword / parameter equals
lib/ansible/plugins/lookup/conjur_variable.py:80:56: E251 unexpected spaces around keyword / parameter equals
lib/ansible/plugins/lookup/conjur_variable.py:80:84: E251 unexpected spaces around keyword / parameter equals
lib/ansible/plugins/lookup/conjur_variable.py:80:86: E251 unexpected spaces around keyword / parameter equals
lib/ansible/plugins/lookup/conjur_variable.py:82:54: E251 unexpected spaces around keyword / parameter equals
lib/ansible/plugins/lookup/conjur_variable.py:82:56: E251 unexpected spaces around keyword / parameter equals
lib/ansible/plugins/lookup/conjur_variable.py:97:65: E251 unexpected spaces around keyword / parameter equals
lib/ansible/plugins/lookup/conjur_variable.py:97:67: E251 unexpected spaces around keyword / parameter equals

The test ansible-test sanity --test pylint [?] failed with the following error:

lib/ansible/plugins/lookup/conjur_variable.py:139:54: undefined-variable Undefined variable 'conf_path'

click here for bot help

ansibot · 2017-12-29T13:42:13Z

@jvanderhoof This PR contains @ mentions in at least one commit message. Those mentions can cause cascading notifications through GitHub and need to be removed. Please squash or amend your commits to remove the mentions.

click here for bot help

ansibot · 2017-12-29T13:42:14Z

cc @CiscoUcs @GGabriele @MarkusTeufelberger @Nosmoht @Qalthos @Spredzy @abellotti @adq @Akasurde @akazakov @alcamie101 @atabachnik @brusMX @cben @chouseknecht @cmoberg @cnasten @dagwieers @dav1x @dkorn @dsoper2 @ericsysmin @fabianvf @flaper87 @ganeshrn @garethr @grastogi23 @gtanzillo @gundalow @gurumaia @haroldwongms @jborean93 @jcpowermac @jctanner @jedelman8 @jhawkesworth @jmcgill298 @johnamcdonough @joshludwig @kamsz @kedarX @khaltore @machacekondra @marqelme @matze @maxamillion @mgruener @mikewiebe @mtnbikenc @mwperina @nerzhul @nitzmahone @pekdon @privateip @rahushen @ravibhure @rcarrillocruz @resmo @robinro @ryansb @ryansydnor @s-hertel @samerd @sdoran @sebasdoes @skg-net @smadam813 @sozercan @tchernomax @trishnaguha @tstringer @vallard @vvb @willthames @wimnat @wojtek0806 @xscript @yaacov @yuwzho @zgalor @zikalino
click here for bot help

The machine identity used is that of the Ansible controlling host, not any server being provisioned or instructed. This documentation change aims to make that relationship clear.

These error messages are less likely to confuse a user as to which machine is associated with the files, identities, and configurations being described.

ryanprior · 2018-01-18T21:33:24Z

Thank you @garymoon and @dustinmm80 for helping to validate the plugin. Let me know if you have any questions as to how I addressed your feedback.

@sivel same goes for you, please let me know if this round of patches raises any concerns.

dustinmm80 · 2018-01-18T21:41:38Z

These changes all look good to me, thanks Ryan.

This lookup plugin is helpful when configuring nodes with secrets when the controlling host has been assigned a Conjur identity. It uses the standard paths for Conjur identity and configuration files, that's nice.

My review is 👍 , let's ship it!

garymoon · 2018-01-18T21:43:31Z

LGTM 👍

jvanderhoof · 2018-01-18T22:30:43Z

@sivel - we received and resolved some feedback from two team members not involved in this effort, but with significant Ansible experience. Anything else we can do to get this in?

pilou- · 2018-01-18T23:23:10Z

@jvanderhoof in order to get rid of needs_revision label, you could comment using ready_for_review.

jvanderhoof · 2018-01-19T00:40:13Z

ready_for_review

jvanderhoof · 2018-01-19T00:43:27Z

#ready_for_review

jvanderhoof · 2018-01-19T00:47:24Z

bot_status

ansibot · 2018-01-19T00:48:39Z

Components

.github/BOTMETA.yml
support: core
maintainers:

lib/ansible/plugins/lookup/conjur_variable.py
support: core
maintainers:

test/units/plugins/lookup/test_conjur_variable.py
support: core
maintainers:

Metadata

waiting_on: jvanderhoof
changes_requested_by: null
needs_info: False
needs_revision: True
needs_rebase: False
merge_commits: []
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): False
community_shipits (namespace maintainers): False
ansible_shipits (core team members): False
shipit_actors (maintainer or core team member): None
shipit_actors_other:

click here for bot help

sivel · 2018-01-19T16:30:08Z

We've "identified" what appears to be an issue with GitHub on a few pull requests, where the UI indicates that the PR is "green" (which is correct), but "mergeable_state" is indicated to be "unstable". Although this PR currently states "needs_revision" it should be fine. Removing it will just cause the bot to re-add that label.

jvanderhoof · 2018-01-19T17:35:09Z

@sivel - what does that mean for this PR?

sivel · 2018-01-19T18:14:08Z

@jvanderhoof nothing specifically, other than I am saying if the PR is "green" we can ignore "needs_revision".

It just needs final reviews. I will personally not have time today to get to this.

thaumos · 2018-01-19T19:04:51Z

lib/ansible/plugins/lookup/conjur_variable.py

+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+


@jvanderhoof or @ryanprior,

could you add below here?

ANSIBLE_METADATA = {'metadata_version': '1.1', 'status': ['preview'], 'supported_by': 'community'}

@thaumos - Thanks for the feedback. Mind giving it one more look? I'd love for this to be able to make in today if possible with code freeze on the 22nd.

lgtm. there shouldn't be a concern having this merged before code freeze.

jvanderhoof · 2018-01-22T17:27:58Z

@thaumos, @sivel, @mattclay, @pilou- Thanks for all the feedback. What's the next step with this PR?

sivel · 2018-01-23T16:04:44Z

Next steps? I'm going to click the big green button.

jvanderhoof · 2018-01-24T13:14:19Z

A big thanks to @sivel, @thaumos, @mattclay, @pilou- & the rest of Ansible core team. We really appreciate your help and feedback!

@jvanderhoof

* Imported lookup plugin from Role * Plugin cleanup, including: * Use existing Python YAML parsing * Remove environment variables as connection options * Added initial debugging information * Reworked the lookup plugin using the Python Request library. As it's available through Ansible, it makes communication with Conjur much more straight forward. * Removed un-used libraries * Fixed linting issues * Standardized output on `format` and insure it works for 2.6, 2.7, and 3.x. * Use quote_plus from the six library for improved python 2/3 behavior. * Refactored identity & configuration to prefer user's file. This also includes a refactor to remove an un-needed dictionary merge method. * Removed `requests` in favor of `ansible.module_utils.urls`. * Refactored netrc loading to warn if host is not present. * Tests and a refactor to support easier testing. * Added reference to website * Fixed two linting errors * Fixed an extra line found by linting * Updated file write to use binary to insure config files are written correctly * Resolved linting issues * Refactored config & identity loading to take advantage of plugin options * Cleanup a bunch of small items caught by linting * Removed extra line caught by linting * Swapped in pytest and added some tests with mocked network responses * Pushing to see if this approach works better... * Refactored be open_url mocking based on feedback * Fixed a couple linting issues & refactored mocking into each method to attempt to resolve a failing test * Use a generic MagicMock for python 2.6 * Fixes doc typo require -> required * Use `type: path` in identity_file and config_file Also removes `expanduser` calls below (which will now be called automatically on paths.) * Defines maintainers for conjur_variable plugin * BOTMETA.yml: ** defines $team_cyberark_conjur as maintainers of Conjur Variable plugin ** adds myself and @jvanderhoof to that team * Adds URLs to relevant documentation for Conjur Variable lookup plugin * Clarifies "the server," "the machine" -> "controlling host" The machine identity used is that of the Ansible controlling host, not any server being provisioned or instructed. This documentation change aims to make that relationship clear. * Adds response code to exception message on authentication failure * Enhances exception messages to specify the controlling host These error messages are less likely to confuse a user as to which machine is associated with the files, identities, and configurations being described. * Adds ANSIBLE_METADATA for Conjur variable lookup plugin

jvanderhoof changed the title ~~Conjur lookup~~ Conjur Lookup Plugin Dec 28, 2017

ansibot added affects_2.5 This issue/PR affects Ansible v2.5 feature_pull_request needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Dec 28, 2017

ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Dec 28, 2017

ryanprior added 3 commits January 18, 2018 15:16

Clarifies "the server," "the machine" -> "controlling host"

7b95a9f

The machine identity used is that of the Ansible controlling host, not any server being provisioned or instructed. This documentation change aims to make that relationship clear.

Adds response code to exception message on authentication failure

4fff3a6

Enhances exception messages to specify the controlling host

4ccfea2

These error messages are less likely to confuse a user as to which machine is associated with the files, identities, and configurations being described.

thaumos suggested changes Jan 19, 2018

View reviewed changes

Adds ANSIBLE_METADATA for Conjur variable lookup plugin

a7982fe

thaumos approved these changes Jan 19, 2018

View reviewed changes

ansibot added the new_contributor This PR is the first contribution by a new community member. label Jan 22, 2018

sivel merged commit 7c8e365 into ansible:devel Jan 23, 2018

ansibot added feature This issue/PR relates to a feature request. and removed feature_pull_request labels Mar 5, 2018

dagwieers added the botmeta This PR modifies the BOTMETA.yml and this requires special attention! label Feb 21, 2019

ansible locked and limited conversation to collaborators Apr 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Conjur Lookup Plugin #34280

Conjur Lookup Plugin #34280

jvanderhoof commented Dec 28, 2017

ansibot commented Dec 28, 2017

ansibot commented Dec 29, 2017

ansibot commented Dec 29, 2017 •

edited by caphrim007

ryanprior commented Jan 18, 2018

dustinmm80 commented Jan 18, 2018

garymoon commented Jan 18, 2018

jvanderhoof commented Jan 18, 2018

pilou- commented Jan 18, 2018

jvanderhoof commented Jan 19, 2018

jvanderhoof commented Jan 19, 2018

jvanderhoof commented Jan 19, 2018

ansibot commented Jan 19, 2018

sivel commented Jan 19, 2018

jvanderhoof commented Jan 19, 2018

sivel commented Jan 19, 2018

thaumos Jan 19, 2018

ryanprior Jan 19, 2018

jvanderhoof Jan 19, 2018

thaumos Jan 19, 2018

jvanderhoof commented Jan 22, 2018

sivel commented Jan 23, 2018

jvanderhoof commented Jan 24, 2018

Conjur Lookup Plugin #34280

Conjur Lookup Plugin #34280

Conversation

jvanderhoof commented Dec 28, 2017

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

ADDITIONAL INFORMATION

ansibot commented Dec 28, 2017

ansibot commented Dec 29, 2017

ansibot commented Dec 29, 2017 • edited by caphrim007

ryanprior commented Jan 18, 2018

dustinmm80 commented Jan 18, 2018

garymoon commented Jan 18, 2018

jvanderhoof commented Jan 18, 2018

pilou- commented Jan 18, 2018

jvanderhoof commented Jan 19, 2018

jvanderhoof commented Jan 19, 2018

jvanderhoof commented Jan 19, 2018

ansibot commented Jan 19, 2018

Components

Metadata

sivel commented Jan 19, 2018

jvanderhoof commented Jan 19, 2018

sivel commented Jan 19, 2018

thaumos Jan 19, 2018

Choose a reason for hiding this comment

ryanprior Jan 19, 2018

Choose a reason for hiding this comment

jvanderhoof Jan 19, 2018

Choose a reason for hiding this comment

thaumos Jan 19, 2018

Choose a reason for hiding this comment

jvanderhoof commented Jan 22, 2018

sivel commented Jan 23, 2018

jvanderhoof commented Jan 24, 2018

ansibot commented Dec 29, 2017 •

edited by caphrim007