New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding aws_secret_facts module for AWS Secrets Manager #42159

Open
wants to merge 5 commits into
base: devel
from

Conversation

Projects
None yet
5 participants
@slapula
Contributor

slapula commented Jun 30, 2018

SUMMARY

This PR contains a facts module for secrets managed by AWS Secrets Manager. This work is related to the aws_secret module I submitted in #40093. The integration tests use the aws_secret module from that PR which means they will fail here until that PR is merged into this one.

@wknapik I believe this is relevant to your interests.

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

aws_secret_facts

ANSIBLE VERSION
ansible 2.5.4
  config file = None
  configured module search path = [u'/home/ajsmith/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/ajsmith/.local/lib/python2.7/site-packages/ansible
  executable location = /home/ajsmith/.local/bin/ansible
  python version = 2.7.15rc1 (default, Apr 15 2018, 21:51:34) [GCC 7.3.0]
@ansibot

This comment has been minimized.

Contributor

ansibot commented Jun 30, 2018

@mattclay

This comment has been minimized.

Member

mattclay commented Jul 3, 2018

@mattclay mattclay added the ci_verified label Jul 3, 2018

@mattclay

This comment has been minimized.

Member

mattclay commented Jul 3, 2018

The error from the tests is:

2018-06-30 22:16:22 ERROR! no action detected in task. This often indicates a misspelled module name, or incorrect module path.
2018-06-30 22:16:22 
2018-06-30 22:16:22 The error appears to have been in '/root/ansible/test/integration/targets/aws_secret_facts/tasks/main.yaml': line 83, column 5, but may
2018-06-30 22:16:22 be elsewhere in the file depending on the exact syntax problem.
2018-06-30 22:16:22 
2018-06-30 22:16:22 The offending line appears to be:
2018-06-30 22:16:22 
2018-06-30 22:16:22   always:
2018-06-30 22:16:22   - name: remove secret
2018-06-30 22:16:22     ^ here

@ryansb ryansb requested review from s-hertel and ryansb Jul 3, 2018

@ryansb ryansb removed the needs_triage label Jul 3, 2018

for i in response['SecretList']:
all_secrets.append(i)
try:
kwargs['nextToken'] = response['nextToken']

This comment has been minimized.

@ryansb

ryansb Jul 11, 2018

Contributor

I think this should also break if nextToken is None/False/empty to make sure this loop doesn't run a final query and duplicate secrets.

This comment has been minimized.

@s-hertel

s-hertel Jul 13, 2018

Contributor

I think 'nextToken' is only in the response if there are more results than what was returned by the last call (according to the docs), so breaking on the KeyError should be fine.

Show resolved Hide resolved lib/ansible/modules/cloud/amazon/aws_secret_facts.py

@ansibot ansibot added the stale_ci label Jul 11, 2018

for i in response['SecretList']:
all_secrets.append(i)
try:
kwargs['nextToken'] = response['nextToken']

This comment has been minimized.

@s-hertel

s-hertel Jul 13, 2018

Contributor

I think 'nextToken' is only in the response if there are more results than what was returned by the last call (according to the docs), so breaking on the KeyError should be fine.

Show resolved Hide resolved lib/ansible/modules/cloud/amazon/aws_secret_facts.py
Show resolved Hide resolved lib/ansible/modules/cloud/amazon/aws_secret_facts.py
Show resolved Hide resolved lib/ansible/modules/cloud/amazon/aws_secret_facts.py Outdated
Show resolved Hide resolved test/integration/targets/aws_secret_facts/tasks/main.yaml Outdated

@slapula slapula force-pushed the slapula:aws-secret-facts-module branch from 6ac65b9 to a9c4575 Nov 26, 2018

@ansibot ansibot removed the stale_ci label Nov 26, 2018

slapula added some commits Nov 26, 2018

@slapula

This comment has been minimized.

Contributor

slapula commented Nov 26, 2018

@s-hertel @ryansb @mattclay Sorry for disappearing, I've had a lot of stuff going on since July. I'm going to be reviewing and updating my PRs now that I have available cycles. I'm going to start with this one since secret manager functionality seems to be in high demand at the moment.

@ansibot

This comment has been minimized.

Contributor

ansibot commented Nov 26, 2018

The test ansible-test sanity --test integration-aliases [explain] failed with 1 error:

test/integration/targets/aws_secret_facts/aliases:0:0: missing alias `shippable/aws/group[1-2]` or `unsupported`

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/cloud/amazon/aws_secret_facts.py:0:0: E307 version_added should be 2.8. Currently 2.7

click here for bot help

slapula added some commits Nov 26, 2018

@slapula

This comment has been minimized.

Contributor

slapula commented Nov 26, 2018

Oof. My integration tests are failing because they require my aws_secret module (which has been bumped in priority due to my inactivity).

@mattclay

This comment has been minimized.

Member

mattclay commented Nov 28, 2018

@slapula

This comment has been minimized.

Contributor

slapula commented Nov 28, 2018

Yeah, that's what I was referring to in my previous comment. Not sure of a good way around that just yet (without getting some form of that module in place).

@ansibot ansibot added the stale_ci label Dec 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment