database secrets engine config #133

drewmullen · 2019-05-31T13:06:21Z

new module hashivault_db_secret_engine_config for enabling a db config. can optionally take a file as input

new module hashivault_db_secret_engine_role for enabling a db role. can optionally take a file as input

new module hashivault_secret_engine for enabling or disabling a secret engine. this module is idempotent on the mount and its tuned values, takes state as a param, is compatible with check mode.

~~deprecate: still usable but should be removed eventually. deprecated by prepending _ to module name~~

~~hashivault_secret_enable~~
~~hashivault_secret_disable~~
~~hashivault_auth_enable~~
see final comment below

tests included but disabled because they require connection to actual db

both require a new hvac module with some fixes pending PR. I was told they'll cut the 9.2 release next week. issues: hvac/hvac#456 , hvac/hvac#454 , hvac/hvac#453, hvac/hvac#459. PR: hvac/hvac#457, https://github.com/hvac/hvac/pull/461/files

fixes for #129, #134

configuration idempotence fixes for hashivault_auth_method, hashivault_azure_secret_engine_config, and hashivault_azure_auth_config

wip

include required_one_of to AnsiMod call

wip

drewmullen · 2019-06-03T17:31:22Z

shoot. thats going to fail until this is merged: hvac/hvac#459

this should be included in the 9.2 release

drewmullen · 2019-06-04T18:47:54Z

looks like the travis job was failing because of 4684e46 where i basically prefixed 3 modules with a _ which makes them still usable to ansible however they throw a deprecation warning. removing but would like to know if theres a way to do this still and pass the travis doc job

nvm. thats not why the doc job is failing...

drewmullen · 2019-06-04T19:52:34Z

@TerryHowe any advice on how to read the build issue travis is catching?

TerryHowe · 2019-06-06T11:58:07Z

We are still waiting on hvac 9.2 here, right? If you have changes not dependent on that, throw them in another PR if you like. Either way, thanks.

drewmullen · 2019-06-06T11:58:51Z

im fine waiting since most of this content will rely on 9.2 release. just trying to figure out why the travis job keeps failing on documentation (not functional testing)

i cant seem to find a true error message in the stdout

drewmullen · 2019-06-10T12:12:53Z

@TerryHowe not sure if im missing something but the output from the doc builder is garbage as far as i can tell. all these modules work so im in favor of disabling the test until we can tune it to get better output

hvac 9.2 was released saturday btw so once we get past this doc hurdle we should be good to release

TerryHowe · 2019-06-10T12:27:27Z

I'll look at the log issue sometime today

TerryHowe · 2019-06-10T18:22:58Z

ansible/modules/hashivault/hashivault_db_secret_engine_config.py

+ default: present
+ config_file:
+ description:
+ - optional: location of file containing relevant db configuration info. use either this or the following ansible params in your play


Can't really hijack username and password, probably should be database_user and database_password or some such

ah. the userpass auth method...

so based on my understanding of the db engine... it may HAVE to hijack it https://www.vaultproject.io/docs/secrets/databases/index.html

username and password appear to be reserved words for the engine to interpolate

so what i will try is set the ansible param to db_username / db_password and after the client is authed, they will overwrite the values in desired state

TerryHowe · 2019-06-10T18:52:19Z

ansible/modules/hashivault/hashivault_secret_engine.py

+ module.exit_json(**result)
+
+
+@hashiwrapper


I'm wondering if the document problem is this should be named hashivault_secret_engine

this has got to be it. now that 9.2 is released i can enable to test too

TerryHowe · 2019-06-10T18:53:06Z

ansible/modules/hashivault/hashivault_secret_engine.py

+ description = params.get('description')
+ config = params.get('config')
+ state = params.get('state')
+ options = params.get('options')


This desired_state isn't used and probably current_state should be set here

drewmullen · 2019-06-10T19:41:18Z

sadly didnt fix the doc issue. im looking into the python issue right now... weird because these work locally for me...

im wondering if its because my venv is 3.7.x

drewmullen · 2019-06-10T19:56:34Z

interesting... so 3.6 is the problem... trying an upgrade to 3.7 and we'll see what breaks...

upgrade python3 revert

drewmullen · 2019-06-10T20:46:25Z

okay so the new tests are enabled and passing.

validation.py", line 250, in check_required_if
    ValueError: too many values to unpack (expected 3)

the ansible engine didnt like how many values were in the required_one_of validator. 3.7 didnt have a problem with it but its not critical so i just removed it

docs are still failing though

revert 37

drewmullen · 2019-06-10T21:13:22Z

@TerryHowe is there any way to get a better output or some kind of error message? ive probably spent more time on this than writing an entire other module

TerryHowe · 2019-06-10T21:38:33Z

I doubt you'd see more if you installed sphinx locally. All I'm seeing is this and it isn't clear if these errors are fatal:

/home/travis/build/TerryHowe/ansible-modules-hashivault/ansible-repo/ansible/docs/docsite/rst/modules/community_maintained.rst:44: WARNING: undefined label: hashivault_policy_set_from_file_module (if the link has no caption the label must precede a section header)
/home/travis/build/TerryHowe/ansible-modules-hashivault/ansible-repo/ansible/docs/docsite/rst/modules/community_maintained.rst:52: WARNING: undefined label: hashivault_secret_engine_module (if the link has no caption the label must precede a section header)
/home/travis/build/TerryHowe/ansible-modules-hashivault/ansible-repo/ansible/docs/docsite/rst/modules/list_of_all_modules.rst:44: WARNING: undefined label: hashivault_policy_set_from_file_module (if the link has no caption the label must precede a section header)
/home/travis/build/TerryHowe/ansible-modules-hashivault/ansible-repo/ansible/docs/docsite/rst/modules/list_of_all_modules.rst:52: WARNING: undefined label: hashivault_secret_engine_module (if the link has no caption the label must precede a section header)
/home/travis/build/TerryHowe/ansible-modules-hashivault/ansible-repo/ansible/docs/docsite/rst/modules/list_of_hashivault_modules.rst:44: WARNING: undefined label: hashivault_policy_set_from_file_module (if the link has no caption the label must precede a section header)
/home/travis/build/TerryHowe/ansible-modules-hashivault/ansible-repo/ansible/docs/docsite/rst/modules/list_of_hashivault_modules.rst:52: WARNING: undefined label: hashivault_secret_engine_module (if the link has no caption the label must precede a section header)

But the real fatal error which may or may not be caused by the above:

RecursionError: maximum recursion depth exceeded while calling a Python object

I'll kick a build with no change and see if it fails in case some other change is triggering the problem

TerryHowe · 2019-06-10T22:28:17Z

Build on master has the same problem, it is something else:

https://travis-ci.org/TerryHowe/ansible-modules-hashivault/jobs/543942010

drewmullen · 2019-06-11T11:42:58Z

that makes sense since my builds started failing 11 days ago and that was when the last PR was merged... hmm

drewmullen · 2019-06-12T14:07:49Z

@TerryHowe can we just disable the doc check? i get what youre going for with the tool but this seems to be more trouble than its worth. id also like to start using this code soon :D

TerryHowe · 2019-06-12T15:27:12Z

The last merge the docs built fine, so it is probably a change in ansible or sphinx that is causing the problem. I'm hoping I can get far enough on the $day_job today to see if I can fix that.

drewmullen · 2019-06-12T15:37:41Z

thank you for helping and taking a look

TerryHowe · 2019-06-13T15:24:28Z

I have been unable to reproduce the problem locally

drewmullen · 2019-06-13T15:26:29Z

I have been unable to reproduce the problem locally

thoughts on next steps?

TerryHowe · 2019-06-13T16:40:13Z

Created a makedoc.sh script to make it easier to test locally.

Created an issue #137

TerryHowe · 2019-06-13T16:41:21Z

It is still broken, but just because I'm ignoring the failure from make. I assume either ansible, sphinxx, or Travis changed and that is what is causing the problem.

drewmullen · 2019-06-17T18:06:01Z

@TerryHowe can you publish some notes on how to run the docs locally?

TerryHowe · 2019-06-17T20:08:40Z

On the master branch is a makedocs.sh script I extracted from the .travis.yml

drewmullen · 2019-06-17T20:22:54Z

thanks for clarifying. i saw your comment about that file but didnt see a PR so i missed the actual file.

yeah, its succeeding for me too... hmm build succeeded, 165 warnings.

TerryHowe · 2019-06-18T16:40:51Z

ansible/modules/hashivault/hashivault_auth_method.py

@@ -71,6 +71,7 @@
 config:
 description:
 - configuration set on auth method. expects a dict


This document seems wrong to me like this should be a yaml dictionary rather than a string.

you talking about the default line? yeah, interestingly it works either way. when i first submitted the PR it was in standard {} format but after i started getting the sphinx issues i was messing with the format because i didnt know what was borking it.

i agree though, the doc example should show without the ", though it does work either way

TerryHowe · 2019-06-18T17:01:36Z

ansible/modules/hashivault/hashivault_azure_auth_role.py

@@ -133,9 +133,9 @@ def main():
 argspec['num_uses'] = dict(required=False, type='int', default=0)

 supports_check_mode=True


Should this really be gone?

so for some reason, python 36 and 27 fail on this line... something about "too many arguments". however, 37 it works... i removed it but left it for future re-integration. i probably should have included a note on that though

TerryHowe · 2019-06-18T17:05:36Z

ansible/modules/hashivault/hashivault_azure_secret_engine_config.py

@@ -105,9 +105,10 @@ def main():
 argspec['client_secret'] = dict(required=False, type='str')
 argspec['environment'] = dict(required=False, type='str', default='AzurePublicCloud')
 argspec['config_file'] = dict(required=False, type='str', default=None)
+ supports_check_mode=True
 required_together=[['subscription_id', 'client_id', 'client_secret', 'tenant_id']]



required_together is actually the 4th arg, so I'm going to change this back.

Connect with me on https://www.linkedin.com/in/terrylhowe/ if you like

TerryHowe · 2019-06-18T17:39:59Z

Sorry it took me so long to get this merged, smaller PRs would help me so if I have a concern with X and doesn't stop Y getting in the code. Thanks!

drewmullen · 2019-06-18T17:52:33Z

yeah, no problem man. sorry i pushed a whole bunch at once lol... theres just a lot im trying to do atm and as i was writing some i found some issues and i just decided to resolve them

hopefully that wont happen again in the future

if you're interested, we should consider putting this into its own github org and i can help with maintenance! i was also considering asking you what you thought about making this into a new Ansible Collection. thats a new design pattern they provided but thats how they want users to manage their code going forward

TerryHowe · 2019-06-18T18:16:58Z

Yes, help with maintenance would be good. I'll have to look at the options, thanks!

drewmullen added 4 commits May 31, 2019 08:50

include but disable db tests

3b31b19

wip

include idemp db config module

7261209

include required_one_of to AnsiMod call

init module

e6c3914

include but disable tests

d5b3d4d

This was referenced May 31, 2019

fixes to database secrets engine hvac/hvac#457

Merged

azure and database modules #129

Closed

drewmullen added 3 commits June 3, 2019 13:19

enable tests

851ea04

init module secret eng mgmt

bb3310d

wip

deprecate nonconformative modules

4684e46

drewmullen added 4 commits June 3, 2019 14:35

disable test until hvac 9.2 is released

0d4725a

fix idempotence check for eng config

8e25519

fix idempotent checks az auth cfg

2220906

revert deprecated name change, docs failing

9d17e88

drewmullen added 3 commits June 4, 2019 15:12

fix idempotence check for auth_method

7fdd60e

wip

a283ca0

nocows =1 by default

c892e4f

quoting docstring dictionaries

79a8868

drewmullen added 2 commits June 7, 2019 13:28

default var causing doc job failure?

225acfe

replace non-problems with original changes

0bd59ab

TerryHowe reviewed Jun 10, 2019

View reviewed changes

hvac 9.2 required

d29a420

drewmullen force-pushed the db-engine-config branch from 1eb369b to 2cf0803 Compare June 10, 2019 20:07

upgrade test framework to 3.7. hold breath"

41d0101

upgrade python3 revert

drewmullen force-pushed the db-engine-config branch from 2cf0803 to 41d0101 Compare June 10, 2019 20:20

remove required check

0fc7868

revert 37

drewmullen force-pushed the db-engine-config branch from 2bacb47 to 0fc7868 Compare June 10, 2019 20:51

drewmullen mentioned this pull request Jun 13, 2019

refactor to use ansible modules and align with hashi design standards drewmullen/ansible-vault-namespaces#4

Merged

TerryHowe merged commit 6628cac into TerryHowe:master Jun 18, 2019

TerryHowe reviewed Jun 18, 2019

View reviewed changes

		@@ -133,9 +133,9 @@ def main():
		argspec['num_uses'] = dict(required=False, type='int', default=0)

		supports_check_mode=True

		module.exit_json(**result)


		@hashiwrapper

database secrets engine config #133

database secrets engine config #133

Conversation

drewmullen commented May 31, 2019 • edited Loading

drewmullen commented Jun 3, 2019

drewmullen commented Jun 4, 2019 • edited Loading

drewmullen commented Jun 4, 2019

TerryHowe commented Jun 6, 2019

drewmullen commented Jun 6, 2019 • edited Loading

drewmullen commented Jun 10, 2019

TerryHowe commented Jun 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drewmullen Jun 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drewmullen commented Jun 10, 2019

drewmullen commented Jun 10, 2019

drewmullen commented Jun 10, 2019

drewmullen commented Jun 10, 2019

TerryHowe commented Jun 10, 2019

TerryHowe commented Jun 10, 2019 • edited Loading

drewmullen commented Jun 11, 2019

drewmullen commented Jun 12, 2019

TerryHowe commented Jun 12, 2019

drewmullen commented Jun 12, 2019

TerryHowe commented Jun 13, 2019

drewmullen commented Jun 13, 2019

TerryHowe commented Jun 13, 2019

TerryHowe commented Jun 13, 2019

drewmullen commented Jun 17, 2019

TerryHowe commented Jun 17, 2019

drewmullen commented Jun 17, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TerryHowe commented Jun 18, 2019

drewmullen commented Jun 18, 2019

TerryHowe commented Jun 18, 2019

drewmullen commented May 31, 2019 •

edited

Loading

drewmullen commented Jun 4, 2019 •

edited

Loading

drewmullen commented Jun 6, 2019 •

edited

Loading

drewmullen Jun 10, 2019 •

edited

Loading

TerryHowe commented Jun 10, 2019 •

edited

Loading